Skip to content

Deep research: resumed runs silently delete previously-valid material findings #2219

Description

@JSv4

Found during review of #2215.

Summary

ResearchReportService.finalize (opencontractserver/research/services/research_reports.py:780-806) withholds — and deletes from report.findings — any material obligation card whose citations do not intersect retrieved_annotation_ids.

Both callers build that list from PydanticAIDependencies.retrieved_annotation_ids, an in-memory accumulator created empty on every invocation of _run_deep_research_async (opencontractserver/tasks/research_tasks.py:871+).

Why this fires in practice

reap_stalled_research (research_tasks.py:833-863) is a real periodic path: it resumes RUNNING reports whose progress clock went cold by re-enqueuing run_deep_research as a fresh task.

  1. Run 1 records a material finding citing annotation Bump actions/upload-artifact from 3 to 4 #123.
  2. The worker dies before finalize_report.
  3. The resumed run does not happen to re-retrieve Bump actions/upload-artifact from 3 to 4 #123 before finalizing.
  4. The evidence gate at 788-806 matches (material, no citation in the near-empty cited_ids) and strips the finding.

The emitted warning says the citation "was not surfaced by retrieval in this run" — misleading, since it was surfaced, just in a prior run segment. Because this mutates persisted report.findings, the loss compounds on every further resume.

Why it was not fixed in #2215

The gate is cited_ids &= set(retrieved_annotation_ids). That line carries a comment identifying it as the enforcement point for two things at once:

  • the closed citation graph, and
  • the permission boundary — "Every retrieval tool is permission-filtered, so this is also what keeps a citation inside what the run's creator may read."

Widening the set across resume segments therefore loosens a deliberate security invariant, not just a correctness check. That needs an explicit design decision.

Possible directions

  • Persist retrieved_annotation_ids on the report so a resume inherits the prior segment's retrieval set — but the persisted set must still be re-filtered against the creator's current permissions at finalize time, or it becomes a stale-permission bypass.
  • Or: on resume, re-validate pre-existing findings' citations through a permission-filtered lookup rather than requiring re-retrieval.
  • Or: make the gate non-destructive (withhold from render without deleting from report.findings), so a later resume can still surface them.

Test gap

No test covers resume-then-finalize interaction with pre-existing findings.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions