docs(ce-code-review): synthetic returns carry independence_verified; interpreter-aware dependency evidence - #1633
Conversation
…interpreter-aware dependency evidence Follow-up to EveryInc#1614 for the remaining halves of EveryInc#1612: - finish-review.md: when the orchestrator serializes a synthetic reviewer return, it must copy independence_verified: true from the peer's on-disk artifact into the return entry - the findings-mechanics helper reads the field only from the return entry, so artifact-only carriage silently drops the peer from independent_reviewers. - subagent-template.md: new false-positive suppression class - dependency/ import findings must be verified under the project's own interpreter and the declared package's metadata (Requires-Dist), not a bare host interpreter import.
![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: b4830cb453
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| Before the first helper run, load every available per-reviewer artifact and build a source-detail map keyed by reviewer plus the helper fingerprint: normalized `file`, string `line`, and whitespace-normalized lowercase `title`. The map owns each source finding's `why_it_matters` and `evidence`; compact returns are merge inputs, not final report objects. | ||
|
|
||
| Inspect the helper's `findings`, `pre_existing_findings`, and `suppressed_findings` for semantic duplicates that use different wording or nearby anchors, for the direct-dependency exception, and for settlement conflicts below. Merge only when candidates describe the same defect and fix path. If semantic reconciliation, direct-dependency reclassification, or settlement stamping changed the set, serialize every reconciled candidate from all three partitions as one valid synthetic reviewer return and run the helper again to restore deterministic gates, partitions, sort order, and numbering. Never ask the helper to decide semantic equivalence or settlement conflicts. When reconciling a semantic duplicate, carry its original source-map keys alongside the candidate in working memory so detail hydration does not depend on the rewritten title. The helper's deterministic `suppressed_findings` partition is not primary review output; after settlement reconciliation, inspect it for the soft-bucket route below, then discard the remainder while preserving `suppressed_by_confidence` counts. | ||
| Inspect the helper's `findings`, `pre_existing_findings`, and `suppressed_findings` for semantic duplicates that use different wording or nearby anchors, for the direct-dependency exception, and for settlement conflicts below. Merge only when candidates describe the same defect and fix path. If semantic reconciliation, direct-dependency reclassification, or settlement stamping changed the set, serialize every reconciled candidate from all three partitions as one valid synthetic reviewer return and run the helper again to restore deterministic gates, partitions, sort order, and numbering. When the synthetic return carries an `adversarial-<provider>` entry, copy `independence_verified: true` from that peer's on-disk artifact into the return entry itself: the helper reads the field only from the return entry, so leaving it in the artifact alone silently drops the peer from `independent_reviewers`. Never ask the helper to decide semantic equivalence or settlement conflicts. When reconciling a semantic duplicate, carry its original source-map keys alongside the candidate in working memory so detail hydration does not depend on the rewritten title. The helper's deterministic `suppressed_findings` partition is not primary review output; after settlement reconciliation, inspect it for the soft-bucket route below, then discard the remainder while preserving `suppressed_by_confidence` counts. |
There was a problem hiding this comment.
When semantic reconciliation triggers the single-return rerun described here, findings-mechanics.py handles it as reviewer: "synthesis" and reads independence only from each finding's independent_reviewers; the top-level independence_verified check runs only when the return's reviewer itself starts with adversarial-. Consequently, copying this flag onto the synthetic return still leaves the peer non-independent, so a corroborated confidence-50 finding can remain suppressed. Preserve the peer in the finding-level independent list only when its artifact verifies independence, or reconstruct separate source returns.
AGENTS.md reference: AGENTS.md:L136-L138
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Correct, and fixed in b7220df. Verified in findings-mechanics.py: a synthesis return derives independence only from each finding's independent_reviewers intersected with reviewers; the return-level flag is consulted only for adversarial-* returns. The instruction now says: keep the verified peer in the finding's reviewers and add it to the finding's independent_reviewers only when that peer's on-disk artifact carries independence_verified: true. Contract test pins the new wording.
| test("finish-review requires copying independence_verified into the synthetic return entry", () => { | ||
| expect(FINISH_BODY).toMatch(/copy `independence_verified: true`.*on-disk artifact.*return entry/s) | ||
| expect(FINISH_BODY).toMatch(/helper reads the field only from the return entry/) | ||
| expect(FINISH_BODY).toMatch(/artifact alone/) |
There was a problem hiding this comment.
These tests only assert that specified wording exists, while both edited references change runtime reviewer and orchestrator decisions; the commit's validation records neither a fresh-agent evaluation nor an explicit skip reason. AGENTS.md requires behavioral skill prose to be tested by injecting the current on-disk skill into a fresh agent, so add a targeted evaluation demonstrating that the new conditions produce the intended downstream behavior rather than relying solely on regex checks.
AGENTS.md reference: AGENTS.md:L163-L167
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Explicit skip reason: this environment has no claude/codex/grok host CLIs (the eval cell bills those products on PATH), so I cannot run bun run test:skill-eval-cell here. The change pins deterministic helper mechanics (the synthesis branch of findings-mechanics.py), and the contract test fails if the instruction drifts from the stated rule. If a maintainer can run the cell, happy to incorporate what it shows.
… level Codex review on EveryInc#1633 was right: findings-mechanics.py handles a synthesis return by reading independence only from each finding's independent_reviewers (intersected with reviewers); the return-level independence_verified flag is consulted only for returns whose reviewer starts with adversarial-. The prior wording told the reconciler to copy the flag onto the synthetic return entry, which the helper ignores. Restate the rule at the layer the helper actually reads: keep the verified peer in the finding's reviewers and add it to independent_reviewers only when the peer's on-disk artifact verifies independence.
2 participants
Summary
Follow-up to #1614 covering the two halves of #1612 it named as remaining work.
independence_verifiedpropagation (#1612 part 2). The findings-mechanics helper readsindependence_verifiedonly from the reviewer return entry (independent_reviewer()infindings-mechanics.py), so when the orchestrator synthesizes or reconciles a return from on-disk artifacts, carrying the field in the artifact alone silently drops anadversarial-<provider>peer fromindependent_reviewers.finish-review.mdnow instructs copyingindependence_verified: truefrom the peer artifact into the synthetic return entry. Docs-side of the issue's "either, not both" - the helper-only-reads-stdin interface makes the alternative (accept from either location) a payload-format change; happy to do that instead if you prefer enforcement over contract.Interpreter-aware dependency evidence (#1612 part 3).
subagent-template.mdgains a false-positive suppression class: before filing a dependency/import mismatch, verify under the project's own interpreter (venv/toolchain/container) and check the declared package's metadata (Requires-Dist) for transitive provision. A bare host-interpreter import failure proves nothing about the project's environment - the real defect in the issue's example was a routine invokingpythonthat was not on PATH.Validation
tests/skills/ce-code-review-synthetic-return.test.ts- 2 contract tests pinning both instructions.bun test tests/skills/ce-code-review-synthetic-return.test.ts tests/review-skill-contract.test.ts tests/ce-code-review-mechanics.test.ts- 111 pass, 0 fail.Related: #1612
Security Disclosure
No security-relevant changes.
Agent Disclosure