Repository navigation
Conversation
…and should-fix After synthesis, CODE_REVIEW_VERIFICATION=on sends each merged blocker and should-fix, a file's findings per call, to a Sonnet call with the checkout behind the reviewers' path guard. Confirmed findings get the code path in their evidence, refuted ones are dropped and listed with the reason in the check run and the trace, and uncertain ones become questions one severity lower. Nits aren't verified. Off by default until the eval shows a gain. Refs #41
![ncode-review[bot]](https://avatars.githubusercontent.com/u/12114478?s=60&v=4){kind=small}
There was a problem hiding this comment.
🤖 NCode Review at 001fb69: 9 new findings on the changed lines.
🤖 Review completeVerdict: 💬 Comment — This PR adds an opt-in verification stage that runs after synthesis. Sonnet calls with the checkout confirm, refute or question each blocker and should-fix. Refuted findings are dropped and recorded, uncertain ones become questions one severity lower, and the verdict eases from request-changes to comment but never to approve. The code holds together and has good tests for failures, thrown calls, verdicts outside a batch and renumbering. The main risk is a design one: a refutation is accepted on the model's word alone, while the model reads code the PR author controls. The concurrency ordering also has no test. 7 reviewers run — 2 findings (run details)
Skipped: api-compat, data-model, infrastructure, intent, performance, tests Findings
Trial: findings are posted as review threads and in the check run. $0.60 on Amazon Bedrock, 64s, at dbf6003. (how to interact) |
…l, note what it changed Verification no longer turns a review into an approval: a request for changes with nothing serious left becomes a comment, and so does a review without synthesis's verdict whose findings were all refuted. The summary, written before verification, now names the refuted findings and counts the ones made questions, and the trace maps each verified finding to the id it was posted with. A verification agent that throws is recorded as a failed call instead of aborting the review. verifyReview carries the stage's wiring out of review() so it is tested. mapWithConcurrency moves to src/concurrency.ts to break the verify/review import cycle, and the verification concurrency is a setting in config.ts.
|
/review |
There was a problem hiding this comment.
🤖 NCode Review at dbf6003: 2 new findings on the changed lines.
|
Autopilot: Round 2 answered. No blocker or should-fix is left open. The one should-fix, F1, repeats round 1's F1 and is declined as a design point of #41. It could be a follow-up once the eval runs. The remaining thread is a nit, answered above. Recommend merging once you've resolved them. |
# Conflicts: # README.md # src/results.ts
Adds an opt-in verification stage after synthesis: with
CODE_REVIEW_VERIFICATION=on, each merged blocker and should-fix goes to a Sonnet call with Read/Grep/Glob in the checkout (same path guard asagent.ts), given the finding, its diff hunk and its reviewers' charters, batched by file (up to 8 per call, $0.50 budget each, shared prefix cached).Confirmed → kept, code path appended to evidence; refuted → dropped and listed under "Refuted by verification" in the check run and in the trace; uncertain → rephrased as a question and lowered one severity step. Nits aren't verified. Calls are costed as role
verification; the eval report now counts posted blockers/should-fixes and refuted findings.The verification agent is injectable, and
test/verify.test.tscovers each verdict path, nits skipped, batching, the off switch and cost recording hermetically.npm run typecheckandnpm test(88 tests) pass; the paid eval was not run.Decisions
CODE_REVIEW_VERIFICATIONmust beon), per the issue's eval-gated rule; README and CHANGELOG say so.questionfield next to{ verdict, path, reason }so the model writes the "Is X intended? If not, …" rephrasing; without one, the finding's problem is prefixed with "Is this intended?".refutedverdict without a reason drops nothing and is treated asuncertain; a failed call leaves its findings unchanged (recorded in the trace).request-changesverdict with no blocker or should-fix left becomescomment; verification never turns a review intoapprove. The summary names each refuted finding, and the trace maps each finding's old id to its posted id (postedAs).Refs #41 — remaining: the acceptance eval (
node evals/run.ts, held-out guidance, 2 runs each with verification on and off), recording the result in the issue, and turning it on by default only if it shows a gain.