v1.94.41.0 fix(review-scope): leave an abandoned review pass out of the receipt counts (GST-1168, GBEV-3) (#3931)

* fix(review-scope): an abandoned review pass is left out of the receipt's counts, by name (GST-1168, GBEV-3)

A pass whose dispatch ledger was never judged and that no review-log row names (the pass a lane abandoned and restarted) no longer adds its collected findings to blocking_found and unrecorded; the summary prints REVIEW_SCOPE_VOIDED for it. A judged pass with no row stays unbound, as before.

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>

* 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>

* changelog: name when a pass is voided (GBEV-3)

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>

---------

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>

bin/gstack-review-scope

@@ -278,6 +278,7 @@ function cmdSummary(rest: string[], json: boolean): number {
   const trailers = log.stdout.split('\x1e').flatMap(rec => { const [sha, values] = rec.trim().split('\0'); return sha && values ? parseFixTrailers(sha, values.split('\x1f').map(v => v.trim()).filter(Boolean)) : []; });
   const summary = summarizeReviewLoop(rows, since, ledgers, files.map(f => ledgerPass(f, keyOf(f))), trailers);
   for (const u of summary.unbound ?? []) process.stderr.write(`REVIEW_SCOPE_UNBOUND: ${u}\n`);
+  for (const v of summary.voided ?? []) process.stderr.write(`REVIEW_SCOPE_VOIDED: ${v}\n`);
   const line = reviewLoopLine(summary);
   // A /ship invocation always runs Step 9 once; zero matching rows means the wrong --since or a review log written elsewhere, never a loop to record.
   if (summary.passes === 0) return refuse('summary', 'REVIEW_SCOPE_INPUT', `no review-log row of this branch is a via:ship review at or after ${since}; check REVIEW_SINCE (${line})`);

changelog.d/gsta258-l4-voided-pass.md

@@ -0,0 +1 @@
+- **The review summary leaves an abandoned review pass out of the receipt's counts** (`gstack-review-scope summary`, GST-1168). A pass whose dispatch ledger was never judged, that no review-log row names, and that a later judged or logged pass replaced is left out, and the summary names it on a `REVIEW_SCOPE_VOIDED:` line instead of counting its findings as found and unrecorded. A fix-check ledger and the last pass always count.

docs/review-policy.md

@@ -510,6 +510,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) |

lib/review-scope.ts

@@ -316,6 +316,8 @@ export interface ReviewLoopSummary {
   ledgers_read?: number;
   /** What could not bind (GST-1168), each named with its reason: a row with no start token or readable time, a row finding's ledger_id no ledger holds, a Review-Finding trailer naming no finding. Never counted as zero silently. */
   unbound?: string[];
+  /** Abandoned passes left out of every count, each named (GBEV-3). */
+  voided?: string[];
 }
 
 /** Review-log rows of one /ship invocation (skill review, via ship, at or after `since`) → the loop summary. */
@@ -379,13 +381,16 @@ function rowLocus(f: Record<string, any>): string | null {
  * When a dispatch ledger started (its first event) and whether it holds a fix-check reader's review: a fix-check run through
  * gstack-review-dispatch (the pass program's `fix-check` step) leaves this ledger even when no review-fix-check row was logged.
  */
-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 }
+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 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);
   const started = Date.parse(String(events[0]?.at ?? ''));
   const fixCheck = events.some(e => e.event === 'output' && e.reader === 'fix-check' && e.tree_ok !== false && ((Array.isArray(e.ids) && e.ids.length > 0) || e.review === true));
   const ids = events.flatMap(e => (e.event === 'output' && Array.isArray(e.ids) ? (e.ids as string[]) : e.event === 'judgment' && e.kind === 'added' && typeof e.id === 'string' ? [e.id] : [])).map(key);
-  return { started: Number.isFinite(started) ? started : NaN, fixCheck, ids, name: path.basename(path.dirname(path.resolve(ledger))) };
+  return { started: Number.isFinite(started) ? started : NaN, fixCheck, ids, name: path.basename(path.dirname(path.resolve(ledger))), judged: events.some(e => e.event === 'judge') };
 }
 
 /** A fix commit's `Review-Finding: <ledger_id or fingerprint> reader=<role>` trailer (receipt attribution): the finding that commit fixed. */
@@ -413,7 +418,7 @@ export function parseFixTrailers(commit: string, values: readonly string[]): Fix
  * decision 3.5 runs a fix-check only after the loop applied that pass's fixes, and a blocker the loop does not fix is deferred
  * or noted by a row. A blocker the last pass found stays unrecorded, and with every fix-check logged the rows decide.
  */
-export function summarizeReviewLoop(rows: readonly Record<string, any>[], since: string, ledgers: readonly (readonly LedgerBlocker[])[] = [], ledgerPasses: readonly LedgerPass[] = [], trailers: readonly FixTrailer[] = []): ReviewLoopSummary {
+export function summarizeReviewLoop(rows: readonly Record<string, any>[], since: string, allLedgers: readonly (readonly LedgerBlocker[])[] = [], allPasses: readonly LedgerPass[] = [], trailers: readonly FixTrailer[] = []): ReviewLoopSummary {
   const sinceMs = Date.parse(since);
   const loopSkill = (r: Record<string, any>) => r.skill === 'review' || r.skill === LATE_DEFER_SKILL || r.skill === FIX_CHECK_SKILL;
   const inRun = (r: Record<string, any>) => r && r.via === 'ship' && Number.isFinite(rowTime(r)) && (!Number.isFinite(sinceMs) || rowTime(r) >= sinceMs);
@@ -423,6 +428,13 @@ export function summarizeReviewLoop(rows: readonly Record<string, any>[], since:
   const rowKey = (r: Record<string, any>) => JSON.stringify([r.skill, r.pass ?? null, r.commit_full ?? r.commit ?? '', (Array.isArray(r.findings) ? r.findings : []).map((f: any) => (f && typeof f === 'object' ? (typeof f.ledger_id === 'string' ? f.ledger_id : findingIdentity(f)) : '')).sort()]);
   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 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));
   type Seen = { locus: string | null; raiser: string | null; classified: boolean; blocks: boolean; candidate: boolean; disposed: boolean; ledgerId?: string; fingerprint?: string };
   const seen = new Map<string, Seen>();
   const deferred = new Set<string>();
@@ -494,6 +506,7 @@ export function summarizeReviewLoop(rows: readonly Record<string, any>[], since:
   if (unrecorded) out.unrecorded = unrecorded;
   if (ledgers.length) out.ledgers_read = ledgers.length;
   if (unbound.length) out.unbound = unbound;
+  if (voided.length) out.voided = voided;
   const fixChecks = Math.max(rowFixChecks, fixCheckStarts.length);
   if (fixChecks) out.fix_checks = fixChecks;
   out.rounds = passes.length + fixChecks;

test/review-loop-binding.test.ts

@@ -59,6 +59,37 @@ describe('review loop counts bind by finding id (GST-1168)', () => {
     ]);
   });
 
+  test('GBEV-3: a voided pass (its ledger never judged, no row names it) is left out by name, and the restarted pass counts alone', () => {
+    const rows = [row('review', 30, [{ fingerprint: 'src/a.ts:10:correctness', severity: 'CRITICAL', action: 'fixed', blocking: true, raised_by: 'red-team', ledger_id: 'p2/red-team#1' }], { pass: 2 })];
+    const ledgers = [[blocker('p1', 'red-team#1', 'src/a.ts:10'), blocker('p1', 'red-team#2', 'src/c.ts:5')], [blocker('p2', 'red-team#1', 'src/a.ts:10')]];
+    const passes = [{ ...pass(0, ['p1/red-team#1', 'p1/red-team#2']), name: 'p1', judged: false }, { ...pass(20, ['p2/red-team#1']), name: 'p2', judged: true }];
+    const s = summarizeReviewLoop(rows, SINCE, ledgers, passes);
+    expect({ found: s.blocking_found, fixed: s.fixed, unrecorded: s.unrecorded, unbound: s.unbound }).toEqual({ found: 1, fixed: 1, unrecorded: undefined, unbound: undefined });
+    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 }]);
+    expect(s.voided).toBeUndefined();
+    expect(s.unbound).toEqual(['pass 1 (ledger p1): no review-log row, so its fixes and dispositions cannot be counted']);
+  });
+
   test('#3911 at 4340ed4c: a fix-check row written twice counts once, and pass 1 with no row is named', () => {
     const fc = { skill: 'review-fix-check', via: 'ship', pass: 2, commit: '4340ed4c', review_binding: { started_at: at(40) }, findings: [{ fingerprint: 'lib/p.ts:9:correctness', severity: 'CRITICAL', action: 'fixed', blocking: true, raised_by: 'fix-check', ledger_id: 'fc/fix-check#1' }] };
     const rows = [row('review', 20, [], { pass: 2, commit: '1111111' }), fc, { ...fc }];