From eafdbd92ab2bf10e2103bc2f014405222f4d01b1 Mon Sep 17 00:00:00 2001 From: garrytan Date: Tue, 3 Nov 2026 22:55:19 +0000 Subject: [PATCH] fix(review-scope): void only a pass a later pass replaced, never a fix-check ledger (review pass 1) A pN ledger never judged and named by no row is voided only when a later pM pass (M > N) was judged or is named; a fix-check ledger and the last pass keep counting, so a skipped judge can no longer hide a final pass's blockers or drop a rowless fix-check. docs/review-policy.md names the REVIEW_SCOPE_VOIDED line. Review-Finding: p1c/specialist-batched#1 reader=specialist-batched Review-Finding: p1c/red-team#1 reader=red-team Review-Finding: p1c/red-team#2 reader=red-team Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --- docs/review-policy.md | 1 + lib/review-scope.ts | 6 ++++-- test/review-loop-binding.test.ts | 15 +++++++++++++++ 3 files changed, 20 insertions(+), 2 deletions(-) diff --git a/docs/review-policy.md b/docs/review-policy.md index 98ef7c7c5..116ca359f 100644 --- a/docs/review-policy.md +++ b/docs/review-policy.md @@ -506,6 +506,7 @@ puts in the receipt: | `review_loop.unrecorded` | blockers found that no row gives a disposition (fixed, deferred, noted, skipped, a gate answer); absent when 0 | | `review_loop.unbound` | what could not bind (GST-1168), each printed as `REVIEW_SCOPE_UNBOUND: : `: a row with no start token and no readable time, a `pN` ledger with no review row of pass N, a row finding's `ledger_id` no ledger collected, a `Review-Finding` trailer naming nothing; absent when 0 | | `review_loop.ledgers_read` | dispatch ledgers the count read; absent under `--no-ledger` | +| `REVIEW_SCOPE_VOIDED:` (stderr, not a receipt field) | an abandoned pass (GBEV-3): a `pN` ledger never judged that no row names, replaced by a later `pM` ledger (M > N) that was judged or is named. Its findings are left out of every count above instead of counting as unbound and unrecorded. A fix-check ledger and the last pass are never voided | Findings bind by id first (GST-1168). A row finding that came from a dispatched reader carries `ledger_id` (`/`, such as `p2/adversarial-native#3`) and pairs with that ledger entry whatever its path:line says, so a native finding with no path:line, or a row logged after its fix moved the line, still pairs. Each fix commit names what it fixed in a `Review-Finding: reader=` trailer, and a trailer counts the finding as fixed. A late defer re-logs its finding under the same id, so it counts once. Rows that record their head dedupe by kind, pass, head and finding ids, so a logger command run twice counts once. Rows and findings without an id keep the path:line pairing above. The native reader's own verdict line ("all FIXABLE, and none blocks", with no file:line, list mark, lead label or Severity/Blocks field) collects at FIXABLE, not CRITICAL; the findings it summarizes collect on their own lines. | `review_loop.deferred` | blocking findings in unchanged code filed as issues (a Step 11 late defer, logged as a `review-late-defer` row so it never displaces the newest `review` pass, counts here and is not a pass) | diff --git a/lib/review-scope.ts b/lib/review-scope.ts index c3fdc34e4..ec15bce11 100644 --- a/lib/review-scope.ts +++ b/lib/review-scope.ts @@ -383,7 +383,7 @@ function rowLocus(f: Record): string | null { */ export interface LedgerPass { started: number; fixCheck: boolean; ids?: string[]; /** The ledger's pass directory (p1, p2, fc): a pN with no review row of pass N is named unbound. */ name?: string; - /** Whether judge ran on it at all (a rejected judgment counts): a pass never judged that no row names was abandoned (voided) and counts nothing (GBEV-3). */ judged?: boolean; + /** Whether judge ran on it at all (a rejected judgment counts): a pN pass never judged that no row names, replaced by a later judged or named pass, was abandoned (voided) and counts nothing (GBEV-3); a fix-check ledger and the last pass are never voided. */ judged?: boolean; } export function ledgerPass(ledger: string, key = (id: string) => id): LedgerPass { const events = readLedger(ledger); @@ -429,7 +429,9 @@ export function summarizeReviewLoop(rows: readonly Record[], since: const loopRows = [...new Map(rows.filter(r => inRun(r) && loopSkill(r)).map((r, i) => [r.commit_full || r.commit ? rowKey(r) : `#${i}`, r] as const)).values()]; const passes = loopRows.filter(r => r.skill === 'review'); const named = (dir: string) => { const n = /^p(\d+)$/.exec(dir)?.[1]; return loopRows.some(r => (n !== undefined && r.skill === 'review' && String(r.pass) === n) || (Array.isArray(r.findings) && r.findings.some((f: any) => typeof f?.ledger_id === 'string' && f.ledger_id.startsWith(`${dir}/`)))); }; - const voidedAt = new Set(allPasses.flatMap((p, i) => (p.judged === false && p.ids?.length && p.name && !named(p.name) ? [i] : []))); + const passNo = (p: LedgerPass) => (p.fixCheck ? undefined : Number(/^p(\d+)$/.exec(p.name ?? '')?.[1] ?? NaN)); + const replaced = (n: number) => allPasses.some(q => (passNo(q) ?? NaN) > n && (q.judged !== false || named(q.name!))); + const voidedAt = new Set(allPasses.flatMap((p, i) => { const n = passNo(p); return n !== undefined && Number.isFinite(n) && p.judged === false && p.ids?.length && !named(p.name!) && replaced(n) ? [i] : []; })); const voided = [...voidedAt].map(i => `pass ${allPasses[i]!.name}: its ledger was never judged and no review-log row names it (an abandoned pass), so its ${allPasses[i]!.ids!.length} collected findings are left out`); const ledgers = allLedgers.filter((_, i) => !voidedAt.has(i)); const ledgerPasses = allPasses.filter((_, i) => !voidedAt.has(i)); diff --git a/test/review-loop-binding.test.ts b/test/review-loop-binding.test.ts index fa85575b5..03c3871f3 100644 --- a/test/review-loop-binding.test.ts +++ b/test/review-loop-binding.test.ts @@ -68,6 +68,21 @@ describe('review loop counts bind by finding id (GST-1168)', () => { expect(s.voided).toEqual(['pass p1: its ledger was never judged and no review-log row names it (an abandoned pass), so its 2 collected findings are left out']); }); + test('an unjudged fix-check ledger with no row still counts as a fix-check, never voided', () => { + const rows = [row('review', 20, [], { pass: 1 })]; + const passes = [{ ...pass(0, ['p1/red-team#1']), name: 'p1', judged: true }, { ...pass(40, ['fc/fix-check#1'], true), name: 'fc', judged: false }]; + const s = summarizeReviewLoop(rows, SINCE, [[], []], passes); + expect(s.voided).toBeUndefined(); + expect(s.fix_checks).toBe(1); + }); + + test('the last pass is never voided: an unjudged pass no later pass replaced keeps its blockers', () => { + const ledgers = [[blocker('p1', 'red-team#1', 'src/a.ts:10')]]; + const s = summarizeReviewLoop([], SINCE, ledgers, [{ ...pass(0, ['p1/red-team#1']), name: 'p1', judged: false }]); + expect(s.voided).toBeUndefined(); + expect(s.blocking_found).toBe(1); + }); + test('a judged pass with no row is still named unbound, never voided', () => { const rows = [row('review', 30, [], { pass: 2 })]; const s = summarizeReviewLoop(rows, SINCE, [[blocker('p1', 'red-team#1', 'src/a.ts:10')], []], [{ ...pass(0, ['p1/red-team#1']), name: 'p1', judged: true }, { ...pass(20, []), name: 'p2', judged: true }]);