Repository navigation
feat: verify a claim, and ask what covers a part - #93
Merged
Merged
Conversation
Two more asks join the part's context menu, each defined in the ASKS registry, which now also says whether an ask checks a claim the reviewer picks or selects. Verify this claim runs the judging pass on one claim: the text the reviewer selected on the head side of the part's diff, or else one of the part's claims they pick. A selection must sit on head-side lines the part shows and be on those lines of the head copy as selected; it becomes a claim of a new source, the reviewer. The verdicts prompt judges it alone, its quote fenced as untrusted, every citation is re-read, and a claim that needs a pinned or named library offers its library fetch, which downloads nothing until pressed. The judged claim joins the engine's latest review and the review shown, so its finding and fetch are there to press. The verdicts prompt is now version 5: it names a reviewer's selection as where its claim is made. What covers this? is a new prompt, cover: the agent lists the tests, in the change or the head copy, that exercise the part, and the manual checks the description reports for it, or says none were found, which is a valid answer. Every test line is re-read in the head copy and every manual check found in the description before the answer is shown. Both land with cases and scores: four labelled selections on canary-python, canary-csharp and sindresorhus-ky-880 scored by verify-accuracy, verify-false-verified and verify-fetch-offered, and seven labelled parts on five cases, three of them covered by nothing, scored by cover-cites-checked, cover-tests-recall, cover-tests-precision, cover-manual-recall and cover-none-found. The baseline records Pi 0.86.1 with zai-coding-cn/glm-5.3: verify-accuracy 0.75, verify-false-verified 0, verify-fetch-offered 1; cover-none-found 1, cover-tests-recall 0.8, every other cover score 1. The review result is now version 17.
…ndefined-judging note
…ims when unjudged
…/extension/test/integration/extension.test.ts ('verifies a claim the reviewer picks...' and 'verifies the text selected...') that asserted the verify-ask request params with toEqual but omitted the agent field. The extension always sends reviewAgentChoice(readAgentSettings()) with every engine request (extension.ts:761; the engine's ask handler consumes it at server.ts:519-523), so with the default stub settings the params include agent: {agent: 'pi', model: '', account: ''} — the extra field shown in the CI diff. Fixed the two test expectations to include the default agent choice; no production code changed. Siblings swept: the explain-ask test uses toMatchObject (unaffected) and the engine-client unit test passes agent undefined so omitting the key remains correct. Verified locally: npm run test:integration 54/54 (both previously failing tests pass), npm run build, tsc -p tsconfig.test.json, npm run lint, npm test 1325/1325 all green
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Intent
Closes #48.
This change adds two asks to a part's context menu, following the explain ask from #47 and the judging pass from #34.
Verify this claim runs the judging pass on one claim: text the reviewer selects on the head side of the part's diff, or one of the part's claims the reviewer picks. When the claim needs a library's source, the verdict offers the library fetch, which the reviewer still starts.
What covers this? lists the automated tests, in the change or in the head copy, that exercise the part, and the manual checks the pull request reports for it. When nothing covers the part, it says that none were found.
Both prompts land with their evaluation cases and scores, and "none found" is a valid, scored answer. Pull request text reaches the agent only as fenced, untrusted data.
Validation never launches, downloads or installs VS Code or any other tool. Extension tests that need the editor run only in CI; local validation goes through the engine protocol, the unit tests and the rendered output.
What Changed
ASKSregistry (verify.ts): it runs the verdicts judging pass on the text the reviewer selected on the head side of a part's diff or on a claim they pick, re-reads every citation against the head copy and CI logs, offers the library fetch when needed, and keeps the judged claim — marked as judged by the ask, so its verdict stands even when the claims pass fell back — in the review's claims (review result protocol v17).cover.ts): it lists the automated tests in the change or head copy that exercise the part and the manual checks the pull request's description reports, every cited test line re-read in the head copy and every manual quote checked against the description, with "none found" a valid answer shown as such.Risk Assessment
✅ Low: The change is well-bounded and follows the repo's established ask/prompt/evaluation patterns; every prior fix-round decision (asked mark, fell-back/unjudged protocol acceptance, verdictsNote and tree accounting, fetchLibrary preservation of the asked mark) is correctly implemented with behavioral tests, and I found no reachable wrong-result, disclosure, or regression path in the intended usage.
Testing
Drove the real engine process over its JSON-RPC protocol against a disposable GitHub/PyPI fixture and a stand-in agent: the verify ask judged a picked claim and a diff selection (marked asked, cited head lines re-read), its library fetch offer was pressed and the fetched verdict landed while the claims pass stayed fell back, a fell-back empty listing and a judged listing both accepted asked claims with every updated review passing the extension protocol guard, five malformed or misplaced asks were refused with plain messages, and the cover ask returned a covering test line plus the description's manual check and a valid scored 'None found'; the reviewer-facing overview and tree rendered the asked-claim accounting from those live results, and the evaluation's own loaders confirmed both prompts registered at the engine's versions with verify/cover cases including none-found. Everything passed; transient harness files were removed and the worktree is clean.
Evidence: Full JSON-RPC transcript of the live engine drive (3 reviews, 7 asks, 2 library fetches, 17 stage notifications)
Evidence: The 34 live engine-protocol checks, all passing
~/.no-mistakes/evidence/01M4B2A2NKK4QDQ8Y05HBFTRKJ/rendered-overview-after-first-verify.html)~/.no-mistakes/evidence/01M4B2A2NKK4QDQ8Y05HBFTRKJ/rendered-overview-listing-fell-back.html)~/.no-mistakes/evidence/01M4B2A2NKK4QDQ8Y05HBFTRKJ/rendered-overview-fell-back-empty.html)~/.no-mistakes/evidence/01M4B2A2NKK4QDQ8Y05HBFTRKJ/rendered-overview-judged-with-asked.html)~/.no-mistakes/evidence/01M4B2A2NKK4QDQ8Y05HBFTRKJ/rendered-overview-asks.html)~/.no-mistakes/evidence/01M4B2A2NKK4QDQ8Y05HBFTRKJ/live-review-after-fetch.json)~/.no-mistakes/evidence/01M4B2A2NKK4QDQ8Y05HBFTRKJ/live-review-2-after-fetch.json)~/.no-mistakes/evidence/01M4B2A2NKK4QDQ8Y05HBFTRKJ/live-review-3-judged.json)Evidence: The ask answers the live engine produced (verify picked/selection, cover found/none found)
Evidence: Evaluation registry and case-loader checks: both prompts at the engine's versions, verify selections and cover expectations including none found
PASS the verdicts prompt (the verify ask's prompt) is registered at the engine's version — registry=5 engine=5 PASS the cover prompt is registered at the engine version, pointing at cover.ts — registry=1 engine=1 PASS cases carry verify selections — 4 selections in 3 cases PASS cases carry cover expectations — 7 cover expectations in 5 cases PASS a verify selection expects the library fetch a claim needs PASS cover expectations include none found (the scored 'None found' answer) PASS cover expectations include parts covered by tests and by manual checksPipeline
Updates from git push no-mistakes
✅ **intent** - passed
✅ No issues found.
✅ **Rebase** - passed
✅ No issues found.
🔧 **Review** - 2 issues found → auto-fixed (4) ✅
packages/engine/src/verify.ts:159- withVerifiedClaim joins a claim with a checked verdict into a review whose claims pass fell back, producing a review result the extension's protocol rejects — so a pressed library fetch is wasted. Concrete sequence: (1) a review completes with the claims listed but the judging pass fallen back (packages/engine/src/review.ts:455 spreads judged.judging; every claim stays 'not checked', judging.outcome 'fell back' — a state the README documents as real); (2) the reviewer runs Verify this claim on a picked claim or a selection: the ask succeeds, and withVerifiedClaim (verify.ts:159-166) keeps the claims pass's judging untouched while inserting a checked verdict — the same merge is applied engine-side at server.ts:550 and extension-side at extension.ts:770-775; (3) the reviewer presses the offered library fetch on the claim's finding: the engine downloads and judges (server.ts:422), then answers fetchLibrary with the full updated review, which the extension refuses to parse because isClaims (packages/extension/src/protocol.ts:507) requires every claim's verdict to be 'not checked' unless judging.outcome === 'judged'. The reviewer sees a ProtocolError after the fetch already happened, and the fetched verdict is stranded in the engine (a second press hits a claim whose verdict.library is already set, and claimToVerify at verify.ts:71 refuses to re-judge it). The invariant 'a claim carries a checked verdict only once the claims were judged' (protocol.ts comment on Claims) is violated by construction whenever the pass itself fell back; the same violation affects every consumer of that invariant, namely the extension validator at protocol.ts:507. The smallest honest remedy is a protocol-semantics decision — accept claims judged singly by the verify ask in isClaims, or record the verify-ask judgement in the claims pass — so the remedy, not the defect, needs the author's call.packages/engine/src/protocol.ts:13- REVIEW_RESULT_VERSION is bumped to 17 (the reviewer claim source, ReviewerSelection, and the ask answer's judged claim), but the version-history comment at protocol.ts:15-45 stops at version 16; every earlier bump added its sentence. Add the version 17 sentence to keep the documented contract complete.🔧 Fix applied.
2 warnings still open:
packages/extension/src/protocol.ts:507- Sibling of round 1's verify-claim finding that the fix round left behind: the fix taught the verdict disjunct (protocol.ts:509-511) to accept anaskedclaim's checked verdict, but the other rule in isClaims — a claims listing that fell back may hold only pipeline-sourced claims — still rejects the review the verify ask produces. Concrete sequence: (1) a review's claims LISTING falls back (claimsStage, review.ts:410-425, still prepends pipeline findings, so claims.claims is an array, possibly empty, and claims.outcome stays 'fell back'); (2) the reviewer selects text on the head side and runs Verify this claim — engine-side claimToVerify (verify.ts:60-83) only refuses when result.claims?.claims is undefined, so the selection is judged and withVerifiedClaim (verify.ts:159-166) appends a reviewer-sourced claim marked asked:true to a listing whose outcome stays 'fell back'; (3) the ask answer itself parses (isVerifiedClaim applies no source rule), but when the verdict's offered library fetch is pressed from the finding, the engine answers fetchLibrary with the whole updated review, and isClaims at protocol.ts:507 rejects it because not every claim's source is 'pipeline' — the reviewer sees a ProtocolError after the download and judging already ran, and the fetched verdict is stranded (every later press answers the same shape and fails the same check). Display sibling of the same rule: overview.ts:768 prints 'Only the pipeline's claims are listed' over a list that then holds the reviewer's asked claim. The remedy is again a protocol-semantics decision the recorded round-1 instruction did not cover — accept an asked reviewer claim in a fell-back listing (loosen the documented 'fallen back with only the pipeline's' contract, its comment, and the overview note), or have the engine refuse a selection verify while the listing fell back — so the remedy, not the defect, needs the author's call.packages/extension/src/overview.ts:741- Sibling the round-1 fix round left behind: verdictsNote's judging-undefined branch still returns 'None is checked yet.' while an asked claim in the same list shows a checked verdict. The fix extended only the outcome==='fell back' branch (overview.ts:742-747) to count asked claims. Concrete sequence: (1) a review lists zero claims and the pipeline report contributes none, so claimsStage leaves claims.claims empty and verdictsStage (review.ts:443) skips, leaving judging undefined — an ordinary state for a small pull request; (2) the reviewer selects a head-side line and runs Verify this claim — the ticket's primary by-hand path, and exactly the case the extension's claimToVerify handles with no claims to pick (extension.ts:791-794); (3) the judged claim (asked:true, checked verdict) joins the review, claimsSection now renders the list (claims.claims.length is 1), and the note above the claim's own checked verdict reads 'None is checked yet.' — a wrong label that contradicts the verdict shown directly beneath it, where the fell-back branch now says the checked verdicts came from the ask. Fix by giving the undefined branch the same asked-claim accounting the fell-back branch got (and note the verdicts pass never ran, rather than implying it is pending).🔧 Fix applied.
1 warning still open:
packages/extension/src/tree.ts:263- Sibling the round-2 fix round (98d0ab4) left behind, despite its recorded instruction to sweep every consumer that reasons about the listing or judging outcome: withClaims's !judged state still writes 'not checked yet; the overview lists them' while an asked claim in the same part carries a checked verdict. Concrete sequence: (1) a review's claims listing falls back or holds no claim, so the verdicts pass never runs (judging undefined) or its judging fell back — exactly the state the engine's new server test exercises (listing fell back empty, judging undefined); (2) the reviewer selects a head-side line and runs Verify this claim: the judged claim joins the review marked asked:true with a refuted or unverifiable verdict, and extension.ts's this.show(updated, true) rebuilds the tree; (3) findingCounts (consumed at tree.ts:124) counts the asked claim, so the part's row reads '⚠ 1 findings · 1 claim' while its tooltip says '1 claim, not checked yet; the overview lists them' — a wrong label contradicting both the badge beside it and the overview's own note ('The verdicts pass did not run; the one checked verdict came from the Verify this claim ask.'). Before this change the combination findings>0 with judged false was unreachable, which the doc comment at tree.ts:110-111 ('a badge counting its findings once the claims are judged') still asserts. Remedy: give the tree the same asked-claim accounting verdictsNote got (count claims with asked===true; when the pass did not judge and some are asked, say the verdicts pass did not run or fell back and the checked verdicts came from the ask), and update the comment at tree.ts:110.🔧 Fix applied.
1 warning still open:
packages/extension/src/overview.ts:745- Sibling the round-2 fix round (98d0ab4) left behind, despite its recorded instruction to sweep every note that reasons about the judging outcome: verdictsNote's judged branch still attributes every claim's verdict to the verdicts pass's stamp, while an asked claim in the same list was judged singly by the Verify this claim ask — valid in a judged listing too, per the recorded principle. Concrete sequence: (1) a review completes with its verdicts pass judged — the ordinary outcome for any review with claims; (2) the reviewer selects a head-side line and runs Verify this claim, the ticket's primary by-hand path; withVerifiedClaim (verify.ts:153-166) appends the reviewer-sourced claim marked asked:true and judging stays 'judged'; (3) claimsSection renders the note 'Each is judged against the change, its read-only copy and any failed check's CI log by <pass stamp>…' over a list whose last item's own detail (overview.ts:695) reads 'judged singly by the Verify this claim ask', stamped with the ask's own stamp (possibly a different agent/model chosen at ask time) — a wrong attribution that contradicts the detail shown directly beneath it, in the state the primary hand path most often produces. The undefined-judging (overview.ts:743) and fell-back (overview.ts:744) branches got the asked-claim accounting; the judged branch is the only remaining site (the tree's judged branch in tree.ts:264-267 reports only counts and stays accurate, and the 'How these results were made' Verdicts row at overview.ts:896-905 describes the pass itself). Fix by appending the same fromAsk accounting to the judged branch, e.g. '…by <stamp>, save the N checked verdict(s) that came from the Verify this claim ask; the refuted and unverifiable ones are findings…'.🔧 Fix applied.
✅ Re-checked - no issues remain.
✅ **Test** - passed
✅ No issues found.
npx vitest run packages/engine/test/verify.test.ts packages/engine/test/cover.test.ts packages/engine/test/review.test.ts packages/engine/test/explain.test.ts packages/engine/test/cli.test.ts (69 tests)npx vitest run packages/engine/test/server.test.ts (35 tests)npx vitest run packages/extension/test/overview.test.ts packages/extension/test/asked-claim.test.ts packages/extension/test/tree.test.ts packages/extension/test/review-result.test.ts packages/extension/test/engine-client.test.ts packages/extension/test/findings.test.ts (185 tests)npx vitest run packages/evaluation/test/run.test.ts packages/evaluation/test/score.test.ts (70 tests)node scratch-live/drive.mjs — spawned packages/engine/dist/main.js serve and drove initialize, three review requests, five verify asks, two cover asks, two fetchLibrary presses and five refusal asks over the real protocol (34 checks, evidence live-checks.txt + live-transcript.jsonrpc.txt)npx vitest run packages/extension/test/live-render.test.ts (scratch harness, deleted after) — rendered the extension's overviewHtml and buildTree from the live-captured review results and ask answers (4 checks)node scratch-live/eval-cases.mjs — drove the evaluation's loadRegistry and loadCase over prompts.json and all case folders (7 checks, evidence eval-cases-checks.txt)✅ **Document** - passed
✅ No issues found.
✅ **Lint** - passed
✅ No issues found.
✅ **Push** - passed
✅ No issues found.