fix: report completion within the requested scope - #586
Conversation
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: f7ca16c408
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d384888a9a
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 855e95acd6
ℹ️ 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".
mldangelo-oai
changed the title
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4a377d56f9
ℹ️ 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".
|
@codex review Please review exact head |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 696d996b8d
ℹ️ 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".
|
@codex review Please review exact head |
|
Codex Review: Didn't find any major issues. Another round soon, please! 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 security review Please review exact head |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5e7a0de67a
ℹ️ 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".
|
@codex review Please review exact head |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: db6ccfb7dd
ℹ️ 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".
kmbroai
left a comment
kmbroai
left a comment
There was a problem hiding this comment.
Reviewed head db6ccfb7dd52dd5353241d790d4aca289865ce72 for correctness, necessity, and simplification.
Necessity and contract
Clarifying complete for the requested scope is worthwhile: a scoped scan must not imply whole-repository coverage. Separating optional open questions from unfinished requested work also makes the report more accurate. I checked the bundled coverage validator: deferred and needs_follow_up work still prevent a complete result. This is not simply a prompt change that removes the underlying completion gate. SARIF execution status and history presentation remain tied to completeness, and unavailable legacy coverage is not silently presented as complete.
Remaining correctness issue
[P2] Escape non-ASCII spacing characters in the human-readable scope representation. src/coverage-presentation.ts:formatScopePath detects whitespace and adds JSON quotes, but the replacement expression does not escape U+00A0, U+2007, or U+202F. I directly called this head's formatter with each character: all three remained literal inside the quoted result. Consequently src/a b and visually similar but different filenames still have ambiguous rendered scope labels. The Python report formatter uses the same distinction between quoting and escaping, so keep the two implementations aligned when fixing this.
This confirms the remaining display-ambiguity concern, not a claim that the scanner selected the wrong file. Escape these characters explicitly and add paired formatter cases that assert the visible escape, including ordinary ASCII space as a control. The earlier normalization/default-ignorable fixes should be retained.
Simplification and integration
Keep this as a semantic clarification plus a shared presentation contract. Do not introduce a second completion state machine in the CLI or renderer. The small scope formatter is useful; a broad generic Unicode-display framework is not needed. Prefer behavioral report assertions over pinning entire prose documents unless byte-for-byte projection compatibility is itself the contract under test.
The bundle is bumped to 0.1.23, but other open plugin-changing PRs also choose that version. Coordinate the final integrated version so two different helper payloads are not delivered under a cache-identical version.
Verification
Ran coverage presentation, history renderer, contract, CLI export, and recovery suites: 128 passed, 0 failed, with cached dependencies. Also inspected the decompressed runtime validator and ran the formatter probe. This did not evaluate model-generated coverage decisions or exercise a live scan. Apart from the scoped-display issue above, I did not reproduce a new functional blocker.
|
Codex Review: Didn't find any major issues. Can't wait for the next one! 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". |
2 participants
Summary
Scoped scans now judge completion against what the user asked to scan. A scan can finish successfully when every requested path was reviewed, even if the request excluded the rest of the repository. Unfinished requested work and essential in-scope proof gaps remain partial and keep exit code 2.
Changes
Testing
Changed and integration-adjacent suites under Python 3.10.20: 493 passed, 11 Windows-only tests skipped, 0 failed.
Git 2.43 and
jqintegration suites: 75 passed, 1 Windows-only test skipped, 0 failed.Broad seeded SDK run: 1,602 passed and 28 platform tests skipped before environment and integration failures were isolated. The integration failures were fixed and rerun above; one unrelated Linux sandbox probe remains unavailable because the test host disables nested user namespaces.
pnpm run types: passed.pnpm run format: passed.python3 -m py_compile _bundled_plugin/scripts/report_projection.py: passed.pnpm run build: passed.pnpm packandnode scripts/check-package.mjs <tarball>: passed for 259 package entries, including the public import, NodeNext types, CLI, 111 bundled plugin files, bundled Codex version, and a nested worker without a global Codex executable.Bundled MCP inspection: preserved stdin transport, unbounded response handling, and requested-scope schema parity.
Current reviewed head
623a3411cdd000b639657de2d8648e7a846ba562: coverage presentation, scan history, canonical contract, CLI export, scan recovery, plugin finding-detail, and runtime/cache suites: 291 passed, 11 platform-specific skips, 0 failed (2,640 assertions).Current reviewed head: TypeScript checking, generated-model verification, touched-file formatting, Python syntax, and a 0.1.23-to-0.1.42 cached-plugin upgrade regression passed.
Risk and rollout
This adds no command, flag, environment variable, default, or exit code. JSON retains exact path strings; only human presentation changes for ambiguous paths. A scan still cannot claim complete coverage when requested work or an essential in-scope proof gap remains. There is no database migration, schema-version change, or rewrite of existing reports. The bundled plugin version change follows the normal release flow.
Public disclosure review