From d0ae52360c86cbff920033922466cfcc88945110 Mon Sep 17 00:00:00 2001 From: garrytan Date: Fri, 30 Oct 2026 19:33:43 +0000 Subject: [PATCH] fix(review-dispatch): only the newest render of a path checks its file; absolute ledger paths - a later render into the same briefs directory overwrites the earlier brief by design, so verify checks a brief file only for the newest render of that path - collect and judge record absolute output, merged and judged paths, so verify reads them from any directory - docs: a failed reader's failure text is parsed like any output Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --- docs/review-policy.md | 5 +++-- lib/review-ledger.ts | 9 ++++++--- test/review-dispatch.test.ts | 7 ++++--- 3 files changed, 13 insertions(+), 8 deletions(-) diff --git a/docs/review-policy.md b/docs/review-policy.md index ca0eee31a..032d87fea 100644 --- a/docs/review-policy.md +++ b/docs/review-policy.md @@ -387,8 +387,9 @@ ones, and the arm lost blockers. `bin/gstack-review-dispatch` closes both ends: dispatched hashes per render (each render is one dispatch of its reader, so a two-pass ledger checks pass-1 sends against pass-1 briefs; a later render prints as `@2`), a collected output for every sent reader (`NOT_COLLECTED` - otherwise; a reader that failed is collected from its failure text, with zero - findings, and its coverage is logged `unavailable`). A mismatched send re-sent + otherwise; a reader that failed is collected from its failure text, where any + label word becomes a finding the judge folds, and its coverage is logged + `unavailable` in the review-log row). A mismatched send re-sent byte-exact before anything was collected is void and counted (`voided=N` on the verdict line); one that produced an output stays `MISMATCH`. Then output hashes against the merged lines they produced, and the judgment rerun against the diff --git a/lib/review-ledger.ts b/lib/review-ledger.ts index 050522ef5..defc5208a 100644 --- a/lib/review-ledger.ts +++ b/lib/review-ledger.ts @@ -13,6 +13,7 @@ * as severe; it never drops or downgrades one. verify replays every check from the ledger alone. */ import * as fs from 'node:fs'; +import * as path from 'node:path'; import { promptHash } from './review-brief'; export interface LedgerEvent { event: 'rendered' | 'sent' | 'output' | 'judgment' | 'judge'; [k: string]: unknown } @@ -85,7 +86,7 @@ export function collect(ledger: string, reader: string, output: string, mergedFi const prior = readLedger(ledger).filter(e => e.event === 'output' && e.reader === reader).length; const lines = parseFindings(reader, output).map((f, i) => ({ id: `${reader}#${prior ? `${prior + 1}.` : ''}${i + 1}`, reader, label: f.label, raw: f.raw, output_sha256: sha })); fs.appendFileSync(mergedFile, lines.map(l => JSON.stringify(l) + '\n').join(''), { mode: 0o600 }); - appendEvent(ledger, { event: 'output', reader, sha256: sha, file: outputFile ?? null, merged: mergedFile, ids: lines.map(l => l.id) }); + appendEvent(ledger, { event: 'output', reader, sha256: sha, file: outputFile ? path.resolve(outputFile) : null, merged: path.resolve(mergedFile), ids: lines.map(l => l.id) }); return { sha, lines }; } @@ -132,7 +133,7 @@ export function recordJudgment(ledger: string, merged: MergedLine[], judged: Jud for (const id of res.added) appendEvent(ledger, { event: 'judgment', kind: 'added', id, to: byId.get(id)!.severity, reason: byId.get(id)!.reason }); for (const id of res.promoted) appendEvent(ledger, { event: 'judgment', kind: 'promoted', id, from: labels.get(id), to: byId.get(id)!.severity, reason: byId.get(id)!.reason }); } - appendEvent(ledger, { event: 'judge', ok: res.ok, judged: judgedFile, judged_sha256: promptHash(fs.readFileSync(judgedFile, 'utf8')), merged: mergedFile, merged_ids: merged.map(m => m.id), dropped: res.dropped, downgraded: res.downgraded, unexplained: res.unexplained, invalid: res.invalid, added: res.added.length, promoted: res.promoted.length, folded: res.folded.length }); + appendEvent(ledger, { event: 'judge', ok: res.ok, judged: path.resolve(judgedFile), judged_sha256: promptHash(fs.readFileSync(judgedFile, 'utf8')), merged: path.resolve(mergedFile), merged_ids: merged.map(m => m.id), dropped: res.dropped, downgraded: res.downgraded, unexplained: res.unexplained, invalid: res.invalid, added: res.added.length, promoted: res.promoted.length, folded: res.folded.length }); return res; } @@ -159,8 +160,10 @@ export function verify(ledger: string): { rows: VerifyRow[]; clean: boolean; bri const r = events[p.at]!; const inside = events.map((e, i) => ({ e, i })).filter(({ e, i }) => i > p.at && i < p.next && e.reader === p.reader); const sent = inside.filter(({ e }) => e.event === 'sent'); + // A later render into the same path overwrote this one's file by design; only the newest render of a path can check it. + const overwritten = renders.some(j => j > p.at && events[j]!.file === r.file); let fileNow = ''; - try { fileNow = typeof r.file === 'string' && fs.existsSync(r.file) ? promptHash(fs.readFileSync(r.file, 'utf8')) : ''; } catch { /* unreadable */ } + try { fileNow = !overwritten && typeof r.file === 'string' && fs.existsSync(r.file) ? promptHash(fs.readFileSync(r.file, 'utf8')) : ''; } catch { /* unreadable */ } // A mismatched send is void when the reader was re-sent byte-exact before anything was collected from it. let mismatched = 0, epVoided = 0; for (const s of sent.filter(({ e }) => e.sha256 !== r.sha256)) { diff --git a/test/review-dispatch.test.ts b/test/review-dispatch.test.ts index 42719905e..44c7ef8c6 100644 --- a/test/review-dispatch.test.ts +++ b/test/review-dispatch.test.ts @@ -100,18 +100,19 @@ describe('gstack-review-dispatch sent, collect, judge and verify: nothing change }); // Value: protects=verify checks each render as its own dispatch, so a two-pass ledger compares pass-1 sends with pass-1 briefs, and a mismatched send re-sent byte-exact before any collect is reported as voided, never hidden; a mismatched send that produced an output stays dirty; fails_when=a run-wide verify prints dirty for a clean two-pass run, a superseded mismatch blocks, voided is not printed, or an output collected under a mismatched send reads clean (GSTA-189 #36 replay: VERIFY dirty briefs=9 outputs=16, every output MATCH); - // why_new=verify compared every send with the reader's last render; seam=sent, collect and verify; no_claim=which pass a render belongs to beyond ledger order + // why_new=verify compared every send with the reader's last render; both passes render into one briefs directory and judge runs with relative paths, as a dispatch program does (pass-3 fix-check); seam=sent, collect and verify; no_claim=which pass a render belongs to beyond ledger order test('two passes verify per render; a superseded mismatch is voided=1; a mismatched send with an output stays dirty', () => { const p = pass(); const out = p.w('spec.out', 'NO FINDINGS\n'); const send = (file: string) => run(['sent', '--ledger', p.ledger, '--role', 'specialist', '--name', 'testing', '--sent', file]); const take = () => run(['collect', '--ledger', p.ledger, '--role', 'specialist', '--name', 'testing', '--output', out, '--merged', p.merged]); send(p.file); take(); - const second = field(run(['render', '--role', 'specialist', '--name', 'testing', '--out', tmp(), '--ledger', p.ledger, '--diff-base', 'b'.repeat(40)]).stdout, 'file'); + const second = field(run(['render', '--role', 'specialist', '--name', 'testing', '--out', p.out, '--ledger', p.ledger, '--diff-base', 'b'.repeat(40)]).stdout, 'file'); const truncated = p.w('truncated.txt', fs.readFileSync(second, 'utf8').slice(0, 200)); expect(send(truncated).status).toBe(1); send(second); take(); - expect(run(['judge', '--ledger', p.ledger, '--merged', p.merged, '--judged', p.w('judged.jsonl', '')]).status).toBe(0); + p.w('judged.jsonl', ''); + expect(run(['judge', '--ledger', 'review-ledger.jsonl', '--merged', 'merged.jsonl', '--judged', 'judged.jsonl'], p.out).status).toBe(0); const v = run(['verify', '--ledger', p.ledger]); expect([v.status, v.stdout.includes('BRIEF specialist/testing MATCH'), v.stdout.includes('BRIEF specialist/testing@2 MATCH'), v.stdout.includes(' voided=1\n'), v.stdout.includes('VERIFY: clean briefs=2 outputs=2 voided=1')]).toEqual([0, true, true, true, true]); const q = pass();