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>
This commit is contained in:
garrytanandcapy-ai[bot] committed 2026-11-03 22:55:19 +00:00
1 parent 91d2131c4c
commit eafdbd92ab
3 files changed
+20 -2

No files matched your search

+1
View File
@@ -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: <what>: <why>`: 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` (`<pass dir>/<merged.jsonl 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: <ledger_id or fingerprint> reader=<raised_by>` 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) |
+4 -2
View File
@@ -383,7 +383,7 @@ function rowLocus(f: Record<string, any>): 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<string, any>[], 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));
+15
View File
@@ -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 }]);