fix(deep-scan): recover complete coverage without hiding incomplete worker results - #657
fix(deep-scan): recover complete coverage without hiding incomplete worker results#657mldangelo-oai wants to merge 5 commits into
Conversation
|
@codex review |
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. |
|
Codex Review: Didn't find any major issues. 🚀 Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
|
@codex review |
![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: 2db25481cd
ℹ️ 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".
| if (relative != "parent" or not parent_manifest) and not superseded: | ||
| source_coverage = draft["coverage"] | ||
| source_completeness = source_coverage.get("completeness") | ||
| source_complete = draft.get("complete", True) is True |
There was a problem hiding this comment.
When the canonical parent manifest contains scan.complete: null, 0, or "false", complete_scan_locked does not reject it because it checks only is False, and this condition exempts that parent from source_complete validation. A Deep Scan with otherwise valid partial coverage, reviewed surfaces, and no deferred rows is therefore promoted to complete, despite an explicit malformed completion marker indicating that the parent result is unverified. Validate the canonical parent's marker here as well so malformed parent drafts remain partial rather than producing a successful scan conclusion.
AGENTS.md reference: sdk/typescript/AGENTS.md:L22-L24
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cd66656cad
ℹ️ 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".
| and coverage.get("completeness") in {"complete", "unknown"} | ||
| ): | ||
| coverage["completeness"] = "partial" | ||
| if (relative != "parent" or not parent_manifest) and not superseded: |
There was a problem hiding this comment.
For a successful dedup worker whose result exists but has complete: false or partial coverage, worker_id is None; once the canonical parent has complete: true, the preceding superseded predicate is true, so this verification block never records the unverified-coverage warning. If that parent has the motivating partial coverage with reviewed surfaces and no deferred rows, _recover_unsealed_coverage then promotes it to complete and the CLI reports success despite the reducer explicitly remaining incomplete. Inspect reducer completeness before applying parent supersession.
AGENTS.md reference: sdk/typescript/AGENTS.md:L22-L23
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e19ff83712
ℹ️ 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".
| or re.fullmatch( | ||
| r"Recovered finding [0-9]+: " | ||
| r"(?:normalized [a-z, ]+|retained stronger duplicate logical finding)\.", | ||
| warning, |
There was a problem hiding this comment.
When an otherwise complete Deep Scan contains duplicate logical findings with the strongest or equal record first, _recover_unsealed_findings safely retains that record but emits Skipped malformed finding N: duplicate logical finding.; this allowlist accepts only the inverse-order retained stronger duplicate warning. Consequently, equivalent findings seal with partial versus complete coverage solely based on their ordering. Treat the lossless duplicate-discard warning as non-coverage-affecting as well.
AGENTS.md reference: sdk/typescript/AGENTS.md:L23-L23
Useful? React with 👍 / 👎.
| if (relative != "parent" or not parent_manifest) and not superseded: | ||
| source_coverage = draft["coverage"] | ||
| source_completeness = source_coverage.get("completeness") | ||
| source_complete = draft.get("complete", True) is True |
There was a problem hiding this comment.
When a succeeded discovery worker's current result has a malformed completion marker such as null, 0, or "false", this strict check records a warning only after the earlier supersession predicate has accepted that result via current.get("complete") is not False. A non-stopped merge then skips the worker's older checkpoints entirely, so any valid findings present only in those checkpoints disappear from the sealed partial report. Require complete is True before superseding checkpoint history, or continue merging the older findings.
AGENTS.md reference: sdk/typescript/AGENTS.md:L23-L23
Useful? React with 👍 / 👎.
1 participant
Summary
Recover Deep Scan coverage that was marked partial despite having no remaining review work, without promoting results whose worker or parent evidence is incomplete.
Changes
0.1.83, with cache-upgrade checks for prior PR and main versions.Testing
Deep Scan workbench, scan recovery, contract, and finding-detail contract suites at seed 657: 153 passed, zero failures.
The malformed-parent regression failed before the fix and passed afterward, including retry and reducer cases.
Three real bundled Codex cache upgrades (
0.1.37,0.1.59,0.1.60to0.1.83) passed, verifying installed finalizer/saved-result bytes and preserved credentials.Types/generated-model freshness, formatting, build, Ruff, Python 3.10 grammar, and
git diff --check: passed.Static package validation and full installed-package smoke: passed.
Full cross-platform suite, native Windows/Linux, live model behavior, and current-head CI were not run locally. CI is left for the second pass.
Final main refresh (
fd98a90): package 0.1.21 includes the MCP launcher-permission fix; SDK implementation and bundled payload are unchanged. Types/model generation, formatting, build, 28 focused tests, static artifact verification and full installed-package smoke passed, including MCP initialization. CI was not awaited.Risk and rollout
Recovery remains limited to deep repository coverage. Unknown warnings and unverifiable parent or worker evidence keep coverage partial. No public CLI flags, configuration, migrations, or scan modes change. The plugin version refresh ensures existing cached installations receive the corrected finalization behavior. Review threads remain open for reviewer follow-up.
Public disclosure review
Historical commit contact metadata and automated restricted settings links remain in the existing PR history, so the second attestation stays unchecked. New fixtures are synthetic and the new commit uses GitHub noreply metadata.