fix: preserve accepted coverage through publication and recovery - #921
mldangelo-oai wants to merge 16 commits into
Conversation
Update the package version to 0.1.28 and refresh release notes.
* fix(logs): stream large saved scan JSON output * test(logs): isolate installed skill state * test(logs): simplify saved log fixtures
* fix(logs): retain complete saved rollout copies * fix(logs): compare copies only for selected sessions * fix(logs): pass the iterator cleanup return value * test: give copied-log boolean cases unique names
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5f36ad8e5c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| for (const key of ["workerId", "attempt", "sourceId", "candidateId"]) delete projected[key]; | ||
| result.provenance = { | ||
| ...projected, | ||
| ...provenance, |
There was a problem hiding this comment.
When a discovery's first attempt saves coverage and is archived before a later attempt succeeds, readArchivedWorkerCheckpoints merges those records into the successful result, but this projection deletes any existing attempt and stamps every record with the successful worker's latest attempt. Consequently, evidence whose receipt points into attempt-01 is reported as originating from attempt 2, and the generated reviews list omits attempt 1 entirely. Preserve or attach attempt provenance while loading each archived checkpoint instead of assigning one attempt to the combined result.
Useful? React with 👍 / 👎.
| for field in ("surfaces", "explicitExclusions", "deferred", "openQuestions", "reviews"): | ||
| items = draft["coverage"].get(field, []) | ||
| if not isinstance(items, list): | ||
| if not isinstance(items, list) or (field == "reviews" and not items): |
There was a problem hiding this comment.
When a Deep discovery result contains a nonempty coverage.reviews extension, stopped-scan recovery now copies those rows directly into canonical coverage. Unlike the other coverage collections, reviews is explicitly excluded from project_missing_record, so model-controlled workerId, attempt, and completeness values are published beside the host-generated review records and can falsely attribute a review to another worker or attempt. Recovery should derive this collection only from accepted worker state or a validated host projection, not merge it from the discovery payload.
AGENTS.md reference: AGENTS.md:L42-L43
Useful? React with 👍 / 👎.
1 participant
Summary
Preserve accepted audit coverage through Deep scan reduction, publication and recovery. Deferred work and unknown coverage must survive a completed reducer. This change depends on #905.
Changes
Testing
At
5f36ad8e5ce630463a6769fd76f1237bd0af67b3:Supporting results and limits
Independent verification of this change and its matching reader/writer ports passed 12 publication/recovery cases, 197 Python checks and 36 projection comparisons. These are separate source-bound results.
Two earlier full SDK orders each passed 3,008 tests with 50 skips at
f3a30dc8; they do not qualify the final head. Original fixture and setup failures remain recorded. Deterministic checks do not establish scan quality, cost or latency equivalence.Risk and rollout
Review host projection and stopped recovery together. #905 must land first. Accepted legacy files retain their bytes and shape; this change adds no migration or writer activation. A later writer integration requires separate reader-before-writer compatibility checks.
Public disclosure review