From f4a1cf093b806bcf98afbea2bc93ae0c85cf7b1d Mon Sep 17 00:00:00 2001 From: garrytan Date: Mon, 2 Nov 2026 07:18:21 +0000 Subject: [PATCH 1/5] fix(ship): the receipt counts every blocker the readers found (GST-951) Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --- bin/gstack-doctor-components.sh | 1 + bin/gstack-review-scope | 34 +++- .../gst-951-receipt-counts-blockers.md | 2 + docs/review-policy.md | 9 +- docs/ship-policy.md | 8 +- docs/troubleshooting.md | 18 +++ docs/unattended.md | 3 +- lib/headless-artifacts.ts | 2 +- lib/result-codes-release.ts | 5 + lib/review-ledger.ts | 34 ++++ lib/review-scope.ts | 43 ++++- lib/ship-receipt.ts | 2 + ship/DRIVER.md | 6 +- ship/DRIVER.md.tmpl | 6 +- ship/sections/pr-body.md | 6 +- ship/sections/pr-body.md.tmpl | 6 +- test/helpers/ship-frozen-scope-actor.ts | 2 +- test/review-scope-ledger-count.test.ts | 153 ++++++++++++++++++ test/review-scope-windows-launch.test.ts | 2 +- test/review-scope.test.ts | 10 +- test/review-targeted.test.ts | 2 +- .../ship/gst-951-receipt-counts-blockers.json | 4 + 22 files changed, 322 insertions(+), 36 deletions(-) create mode 100644 changelog.d/gst-951-receipt-counts-blockers.md create mode 100644 test/review-scope-ledger-count.test.ts create mode 100644 test/size-ledger/ship/gst-951-receipt-counts-blockers.json diff --git a/bin/gstack-doctor-components.sh b/bin/gstack-doctor-components.sh index 960129724..6dba8bf82 100644 --- a/bin/gstack-doctor-components.sh +++ b/bin/gstack-doctor-components.sh @@ -326,6 +326,7 @@ gstack_result_anchor() { REGEN_DEPS_MISSING) printf '%s' 'regen-deps-missing' ;; REVIEW_SCOPE_INPUT) printf '%s' 'review-scope-input' ;; REVIEW_SCOPE_RANGE) printf '%s' 'review-scope-range' ;; + REVIEW_SCOPE_LEDGER) printf '%s' 'review-scope-ledger' ;; RECEIPT_CARRY_REFUSED) printf '%s' 'receipt-carry-refused' ;; PROSE_LINT_FOUND) printf '%s' 'prose-lint-found' ;; REVIEW_LOOP_EXCEPTION_INVALID) printf '%s' 'review-loop-exception-invalid' ;; diff --git a/bin/gstack-review-scope b/bin/gstack-review-scope index fb2f2388e..8f071c17c 100755 --- a/bin/gstack-review-scope +++ b/bin/gstack-review-scope @@ -5,7 +5,7 @@ * gstack-review-scope classify --base --pass [--range-base ] [--policy frozen|diff-plus-blocking|all] [--findings |-] [--json] * gstack-review-scope defer --findings [--pr ] [--repo ] [--branch ] [--out ] [--dry-run] [--json] * gstack-review-scope render --deferred [--findings ] - * gstack-review-scope summary --since [--out ] [--json] + * gstack-review-scope summary --since (--run | --ledger ... | --no-ledger) [--out ] [--json] * * `classify` reads a JSON array of findings (the Step 9.3 result) and stamps * each with `locus` (diff | fix-range | outside), `kind` (defect | test-gap | @@ -30,6 +30,7 @@ import { parseHunks, renderDeferredIssue, renderDeferredSection, reviewLoopLine, scopeFindings, scopeLine, summarizeReviewLoop, ALREADY_FILED_REASON, DEFERRED_MARKER, findingIdentity, findingWhere, REVIEW_SCOPE_POLICIES, type DeferredRecord, type ReviewScopePolicy, type ScopeFinding, type ScopedFinding, } from '../lib/review-scope'; +import { ledgerBlockers } from '../lib/review-ledger'; const HELP = `gstack-review-scope — finding scope for /ship's review loop (docs/ship-policy.md#review-scope) @@ -42,8 +43,12 @@ const HELP = `gstack-review-scope — finding scope for /ship's review loop (doc render --deferred [--findings ] the PR-body sections: ## Deferred findings (references, never closing keywords) and ## Review notes (scope=note findings) - summary --since [--out ] the invocation's passes from this branch's review log: REVIEW_LOOP: passes=… - blocking_found=… deferred=… noted=… (the ship receipt's --review-loop input) + summary --since (--run | --ledger ... | --no-ledger) [--out ] + the invocation's passes from this branch's review log: REVIEW_LOOP: passes=… + blocking_found=… deferred=… noted=… (the ship receipt's --review-loop input); + blocking_found also counts every CRITICAL the dispatch ledgers collected or judged + (--run finds each review-ledger.jsonl under the run directory); unrecorded=N names + blockers no row gives a disposition. An unreadable ledger: REVIEW_SCOPE_LEDGER Defaults: --policy from the base branch's .gstack/ship-policy.json review_scope (frozen); an unresolvable base refuses (POLICY_SOURCE_UNAVAILABLE) unless --policy is passed; --findings - reads stdin; --branch the current branch. @@ -117,7 +122,7 @@ function readFindings(spec: string | undefined, what: string): ScopeFinding[] { if (!list || !list.every((f: unknown) => f && typeof f === 'object' && !Array.isArray(f))) throw new Error(`${what}: expected a JSON array of finding objects`); return list as ScopeFinding[]; } -function refuse(cmd: string, code: 'REVIEW_SCOPE_INPUT' | 'REVIEW_SCOPE_RANGE', detail: string): number { +function refuse(cmd: string, code: 'REVIEW_SCOPE_INPUT' | 'REVIEW_SCOPE_RANGE' | 'REVIEW_SCOPE_LEDGER', detail: string): number { process.stderr.write(resultLineWithDocs(`gstack-review-scope ${cmd}`, code, detail) + '\n'); return EXIT.fail; } @@ -245,7 +250,26 @@ function cmdSummary(rest: string[], json: boolean): number { if (!line.trim() || line === 'NO_REVIEWS') continue; try { rows.push(JSON.parse(line)); } catch { /* malformed row: gstack-review-read already warned */ } } - const summary = summarizeReviewLoop(rows, since); + const files: string[] = []; + for (let i = 0; i < rest.length; i++) { + if (rest[i] !== '--ledger') continue; + if (!rest[i + 1] || rest[i + 1]!.startsWith('--')) return refuse('summary', 'REVIEW_SCOPE_INPUT', '--ledger needs a file'); + files.push(rest[++i]!); + } + const run = flag(rest, '--run'); + if (run) { + let found: string[]; + try { found = (fs.readdirSync(run, { recursive: true }) as string[]).filter(p => path.basename(p) === 'review-ledger.jsonl').sort().map(p => path.join(run, p)); } catch (e: any) { return refuse('summary', 'REVIEW_SCOPE_LEDGER', `--run ${run}: ${e.code ?? e.message}`); } + if (!found.length) return refuse('summary', 'REVIEW_SCOPE_LEDGER', `--run ${run} holds no review-ledger.jsonl: name the run directory the passes wrote, or each --ledger`); + files.push(...found); + } + const noLedger = rest.includes('--no-ledger'); + if (noLedger && files.length) return refuse('summary', 'REVIEW_SCOPE_INPUT', '--no-ledger contradicts --run and --ledger'); + // The ledgers hold every reader's findings, the voices' too, before any row classifies them; a summary without them can undercount. + if (!noLedger && !files.length) return refuse('summary', 'REVIEW_SCOPE_LEDGER', 'name the review\'s dispatch ledgers (--run or --ledger ), or --no-ledger when no reader ran through gstack-review-dispatch'); + let ledgers: string[][]; + try { ledgers = files.map(ledgerBlockers); } catch (e: any) { return refuse('summary', 'REVIEW_SCOPE_LEDGER', e.message); } + const summary = summarizeReviewLoop(rows, since, ledgers); 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})`); diff --git a/changelog.d/gst-951-receipt-counts-blockers.md b/changelog.d/gst-951-receipt-counts-blockers.md new file mode 100644 index 000000000..4a1fbc0c8 --- /dev/null +++ b/changelog.d/gst-951-receipt-counts-blockers.md @@ -0,0 +1,2 @@ +- **The /ship receipt counts every blocker its readers found** (GST-951). `gstack-review-scope summary` read only the review-log rows, but /ship's readers report through the dispatch ledgers (`gstack-review-dispatch collect` and `judge`). So a blocker that only the native adversarial pass raised, a CRITICAL logged fixed without classify's blocking stamp, and findings logged before they were classified all left `blocking_found` at 0. That happened on #3841, #3838 and #3837. The summary now also reads each ledger's collected and judged CRITICAL findings, takes the larger count per reviewer so a blocker held in both places counts once, and counts an unclassified CRITICAL row as blocking. `unrecorded=N` names blockers no row gives a disposition, and `ledgers=N` says how many ledgers it read. +- **The summary refuses rather than undercount.** It now needs `--run ` (it finds every `review-ledger.jsonl` under it), `--ledger `, or `--no-ledger` when no reader ran through `gstack-review-dispatch`. A missing choice, a run directory with no ledger, or an unreadable ledger, merged file or judged set, or a judged set changed since `judge`, is `REVIEW_SCOPE_LEDGER` with nothing written. /ship on Capy (`ship/DRIVER.md`) passes `--run`; the Claude Code flow passes `--no-ledger`. diff --git a/docs/review-policy.md b/docs/review-policy.md index 51182ed28..cde94b9a8 100644 --- a/docs/review-policy.md +++ b/docs/review-policy.md @@ -387,14 +387,17 @@ narrowed to the gate scan's tagged hunks. ## What the receipt records -`gstack-review-scope summary --since ` reads this invocation's -review-log rows and writes the counters `gstack-ship-receipt write --review-loop` +`gstack-review-scope summary --since --run ` reads this invocation's +review-log rows and the dispatch ledgers under the run directory (`--ledger ` names one; +`--no-ledger` when no reader ran through `gstack-review-dispatch`) and writes the counters `gstack-ship-receipt write --review-loop` puts in the receipt: | Field | Meaning | |---|---| | `review_loop.passes` | full review passes this ship ran (1 or 2; up to 3 under `review_scope: all`) | -| `review_loop.blocking_found` | distinct blocking findings across the passes | +| `review_loop.blocking_found` | every blocker found (GST-951): per reviewer, the larger of the rows' blocking findings (classified, or an unclassified CRITICAL) and the ledgers' CRITICAL findings as collected or judged, so an adversarial, outside-voice or not-yet-classified blocker counts. An unreadable ledger, merged file or judged set, or a judged set changed since `judge`, is `REVIEW_SCOPE_LEDGER`, never a count | +| `review_loop.unrecorded` | blockers found that no row gives a disposition (fixed, deferred, noted, skipped, a gate answer); absent when 0 | +| `review_loop.ledgers` | dispatch ledgers the count read; absent under `--no-ledger` | | `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) | | `review_loop.noted` | below-floor findings listed in the PR body | | `review_loop.fixed` | findings fixed in this PR | diff --git a/docs/ship-policy.md b/docs/ship-policy.md index 26f3ef71e..016af873d 100644 --- a/docs/ship-policy.md +++ b/docs/ship-policy.md @@ -1268,11 +1268,11 @@ construction: the run and names the finding (path, severity, problem) for the owner instead of a third round; the owner may grant one scoped fix-check. -`gstack-review-scope summary --since ` reads the branch's review-log -rows for this invocation and writes the counters the ship receipt carries +`gstack-review-scope summary --since --run ` reads the branch's review-log +rows for this invocation and its dispatch ledgers, and writes the counters the ship receipt carries (`gstack-ship-receipt write … --review-loop @`): `review_loop: { passes, -blocking_found, deferred, noted, fixed, scope }`, plus `fix_checks`, `dispatches` -and `est_input_tokens` when the run recorded them, printed on the receipt summary +blocking_found, deferred, noted, fixed, scope }`, plus `fix_checks`, `dispatches`, +`est_input_tokens`, `unrecorded` and `ledgers` when the run recorded them, printed on the receipt summary line as `passes= blocking_found= deferred=`, so a scorecard can track passes per PR. diff --git a/docs/troubleshooting.md b/docs/troubleshooting.md index d73967bb1..ecb64d3f1 100644 --- a/docs/troubleshooting.md +++ b/docs/troubleshooting.md @@ -5646,6 +5646,24 @@ pass). the invocation record, and start a full pass (`--pass 1`) when the branch was rebased; /ship's own rule is to merge the base in, never rebase a reviewed head. + +### `REVIEW_SCOPE_LEDGER` + +**Meaning.** `gstack-review-scope summary` could not count the review's +dispatch ledgers, so it wrote no `REVIEW_LOOP:` line rather than a count that +might be low (GST-951). Either no ledger was named (`--run`, `--ledger` or +`--no-ledger`), `--run` holds no `review-ledger.jsonl`, or a ledger, the +`merged.jsonl` it names or its judged set is unreadable, or the judged set +changed after `judge` checked it. The ledgers hold every reader's findings +(specialists, Red Team, both adversarial voices, targeted, fix-check) before any +review-log row classifies them, which is why the count reads them. + +**Fix.** Pass `--run `, the directory the review passes wrote (on +Capy, `ship/DRIVER.md`'s run directory), or each `--ledger `. Pass +`--no-ledger` only when no reader ran through `gstack-review-dispatch`. When a +file moved, restore it; when a judged set changed, rerun `gstack-review-dispatch +judge` on it. + ### `RECEIPT_CARRY_REFUSED` diff --git a/docs/unattended.md b/docs/unattended.md index 822f38e73..5946ce12f 100644 --- a/docs/unattended.md +++ b/docs/unattended.md @@ -172,7 +172,8 @@ unpushed head is `QUEUE_RECEIPT_STALE` to the queue. The receipt's `review_loop` object, written from `gstack-review-scope summary --since `, is the loop's machine-readable outcome: `passes` (Step 9 full passes this ship ran, 1 or 2; up to 3 under `review_scope: all`), -`blocking_found` (distinct blocking findings across them), `deferred` +`blocking_found` (every blocker the readers found, rows and dispatch ledgers +both, [review-policy.md](review-policy.md#what-the-receipt-records)), `deferred` (blocking findings in unchanged code filed as issues, Step 11's late defers included), `noted`, `fixed` and `scope`. The summary line a parent reads prints it as `passes= blocking_found= diff --git a/lib/headless-artifacts.ts b/lib/headless-artifacts.ts index 433bec17b..355a51cf5 100644 --- a/lib/headless-artifacts.ts +++ b/lib/headless-artifacts.ts @@ -106,7 +106,7 @@ export interface ShipReceipt { // Related open PRs by other authors (Lane D): `supersedes` / `credit` only on the owner's confirmation. issues?: { fixes: number[]; refs: number[]; flakes: Array<{ issue: number; run: string }>; related?: Array<{ pr: number; repo?: string; author: string; closes: number[]; supersedes?: boolean; credit?: boolean }> }; /** Harness lane E: the review loop's counters (gstack-review-scope summary) and the carry record when the receipt followed a head whose product diff was unchanged (gstack-ship-receipt carry). */ - review_loop?: { passes: number; blocking_found: number; deferred: number; noted?: number; fixed?: number; scope?: string; fix_checks?: number; rounds?: number; dispatches?: Record; est_input_tokens?: number }; + review_loop?: { passes: number; blocking_found: number; deferred: number; noted?: number; fixed?: number; scope?: string; fix_checks?: number; rounds?: number; dispatches?: Record; est_input_tokens?: number; unrecorded?: number; ledgers?: number }; carry?: { from: string; reason: string; product_patch_id: string; changed_paths: string[]; merge_commits: number; range_diff: string; covers: string[]; reruns: string[]; trees?: string[]; ancestry?: 'linear' | 'rewritten' }; /** The reviewed head's tree and its product patch-id against the base (gstack-ship-receipt write; carry rebinds both): what a carry across rewritten history compares. */ head_tree?: string; product_patch_id?: string; diff --git a/lib/result-codes-release.ts b/lib/result-codes-release.ts index 3797aeae2..f85c80bc0 100644 --- a/lib/result-codes-release.ts +++ b/lib/result-codes-release.ts @@ -360,6 +360,11 @@ export const RELEASE_RESULT_CODES = { summary: 'gstack-review-scope could not read the diff it scopes findings against: the base or the range base does not resolve, or the range base is not an ancestor of HEAD', fix: 'fetch origin/, check --range-base against the saved pass heads, and start a full pass (--pass 1) when history was rewritten', }, + REVIEW_SCOPE_LEDGER: { + anchor: 'review-scope-ledger', + summary: 'gstack-review-scope summary could not count the dispatch ledgers: none was named (no --run, --ledger or --no-ledger), --run holds no review-ledger.jsonl, or a ledger, its merged file or its judged set is unreadable or changed since judge', + fix: 'pass --run (the directory the review passes wrote) or each --ledger ; --no-ledger only when no reader ran through gstack-review-dispatch; restore or re-collect a ledger file that moved', + }, RECEIPT_CARRY_REFUSED: { anchor: 'receipt-carry-refused', summary: 'gstack-ship-receipt carry could not carry the /ship receipt to HEAD: the PR body has no receipt, the working tree has uncommitted changes, the receipt head is not an ancestor of HEAD and the receipt records no tree to compare, or the product diff could not be compared', diff --git a/lib/review-ledger.ts b/lib/review-ledger.ts index cfbc91656..7e8aa54b1 100644 --- a/lib/review-ledger.ts +++ b/lib/review-ledger.ts @@ -236,6 +236,40 @@ export function recordJudgment(ledger: string, merged: MergedLine[], judged: Jud return res; } +/** + * The blocking findings a run's ledger holds (GST-951): every collected line whose label ranks CRITICAL, raised to its judged + * severity when the newest judge accepted a judged set that is unchanged since; lead judgment's additions count and a folded + * duplicate does not. Every reader counts the same way, whatever it is and whether or not Step 9.4 classified its findings. + * A ledger, merged file or judged set it cannot read, or a judged set changed since judge, throws: the count is never a guess. + */ +export function ledgerBlockers(ledger: string): string[] { + const fail = (why: string): never => { throw new Error(`${ledger}: ${why}`); }; + const read = (file: unknown, what: string): string => { + try { return fs.readFileSync(String(file), 'utf8'); } catch { return fail(`${what} ${String(file)} is unreadable`); } + }; + const jsonLines = (text: string, what: string): any[] => + text.split('\n').filter(l => l.trim()).map(l => { try { return JSON.parse(l); } catch { return fail(`${what} has a line that is not JSON`); } }); + const events = jsonLines(read(ledger, 'ledger'), 'the ledger') as LedgerEvent[]; + const ranks = new Map(); + for (const o of events.filter(e => e.event === 'output' && Array.isArray(e.ids) && e.ids.length)) { + const lines = jsonLines(read(o.merged, `merged file of ${String(o.reader)}`), String(o.merged)) as MergedLine[]; + for (const id of o.ids as string[]) { + const line = lines.find(l => l.id === id) ?? fail(`collected id ${id} is missing from ${String(o.merged)}`); + ranks.set(id, rank(line.label) ?? 0); + } + } + const verdict = [...events].reverse().find(e => e.event === 'judge'); + if (verdict?.ok === true) { + const text = read(verdict.judged, 'judged set'); + if (promptHash(text) !== verdict.judged_sha256) fail(`judged set ${String(verdict.judged)} changed since judge`); + for (const j of jsonLines(text, String(verdict.judged)) as JudgedLine[]) { + if (j.duplicate_of) ranks.delete(j.id); + else ranks.set(j.id, Math.max(ranks.get(j.id) ?? 0, rank(j.severity) ?? 0)); + } + } + return [...ranks].filter(([, r]) => r >= 4).map(([id]) => id); +} + /** * Whether a recorded output was a review: it has findings, or its file (unchanged since collect) carries the reader's clean marker, so * a ledger written before collect recorded the field is judged by the same rule; with no readable file, collect's own review field. diff --git a/lib/review-scope.ts b/lib/review-scope.ts index 833562591..43ead5e09 100644 --- a/lib/review-scope.ts +++ b/lib/review-scope.ts @@ -14,6 +14,7 @@ * Pure functions here; bin/gstack-review-scope runs git and gh. */ import { SPECIALISTS } from './review-depth'; +import { rank } from './review-ledger'; import { parseSpecialistOutput } from './state-matrix'; export type Locus = 'diff' | 'fix-range' | 'outside'; @@ -306,6 +307,10 @@ export interface ReviewLoopSummary { dispatches?: Record; /** Prompt plus diff bytes the dispatches read, / 4: a relative estimate, never a bill. */ est_input_tokens?: number; + /** Blocking findings no row gives a disposition (fixed, deferred, noted, skipped or a gate answer): found, outcome unrecorded. */ + unrecorded?: number; + /** Dispatch ledgers whose collected and judged findings were counted (gstack-review-dispatch). */ + ledgers?: number; } /** Review-log rows of one /ship invocation (skill review, via ship, at or after `since`) → the loop summary. */ @@ -348,12 +353,29 @@ function rowDispatches(row: Record): Array<{ role: string; bytes: n .map(([name]) => ({ role: name === 'red-team' ? 'red-team' : 'specialist', bytes: null })); } -export function summarizeReviewLoop(rows: readonly Record[], since: string): ReviewLoopSummary { +/** Row actions that record what happened to a finding. */ +const DISPOSITIONS = ['fixed', 'auto-fixed', 'deferred', 'noted', 'skipped', 'kept', 'restored', 'open']; +/** A row finding that blocks: classified blocking, or an unclassified CRITICAL defect (logged before Step 9.4's classify). */ +const rowBlocks = (f: Record) => f.blocking === true || (f.blocking === undefined && f.advisory !== true && rank(f.severity) === 4); +/** The reviewer a finding counts under: a row's raised_by or a ledger id's reader; every specialist is one group, as in the ledger. */ +function raiserGroup(raiser: unknown): string { + const role = typeof raiser === 'string' ? raiser.split(/[/#]/)[0]!.trim() : ''; + if (!role || role === 'unknown') return 'unattributed'; + return ['core', 'qa', 'red-team', 'adversarial-native', 'outside-voice', 'fix-check', 'targeted'].includes(role) ? role : 'specialist'; +} + +/** + * The loop summary. blocking_found counts every blocker found (GST-951): per reviewer group, the larger of the rows' + * blockers and the dispatch ledgers' CRITICAL findings (`ledgerBlockers`, one id list per ledger), so a blocker that only + * a ledger holds (an adversarial or outside-voice finding, a finding logged before classification) still counts. Two + * sources can each hold one the other lacks only when the rows name no reviewer; then the count may run high, never low. + */ +export function summarizeReviewLoop(rows: readonly Record[], since: string, ledgers: readonly (readonly string[])[] = []): ReviewLoopSummary { const sinceMs = Date.parse(since); const inRun = (r: Record) => r && r.via === 'ship' && Number.isFinite(rowTime(r)) && (!Number.isFinite(sinceMs) || rowTime(r) >= sinceMs); const loopRows = rows.filter(r => inRun(r) && (r.skill === 'review' || r.skill === LATE_DEFER_SKILL || r.skill === FIX_CHECK_SKILL)); const passes = loopRows.filter(r => r.skill === 'review'); - const blocking = new Set(); + const blocking = new Map(); const deferred = new Set(); const noted = new Set(); const fixed = new Set(); @@ -363,13 +385,25 @@ export function summarizeReviewLoop(rows: readonly Record[], since: for (const f of Array.isArray(row.findings) ? row.findings : []) { if (!f || typeof f !== 'object') continue; const id = findingIdentity(f); - if (f.blocking === true) blocking.add(id); + if (rowBlocks(f)) { + const b = blocking.get(id) ?? { group: raiserGroup(f.raised_by), disposed: false }; + if (DISPOSITIONS.includes(f.action)) b.disposed = true; + blocking.set(id, b); + } if (f.action === 'deferred') deferred.add(id); if (f.action === 'noted') noted.add(id); if (f.action === 'fixed' || f.action === 'auto-fixed') fixed.add(id); } } - const out: ReviewLoopSummary = { passes: passes.length, blocking_found: blocking.size, deferred: deferred.size, noted: noted.size, fixed: fixed.size, scope }; + const groups = new Map(); + const tally = (g: string) => groups.get(g) ?? groups.set(g, { rows: 0, disposed: 0, ledger: 0 }).get(g)!; + for (const b of blocking.values()) { const t = tally(b.group); t.rows++; if (b.disposed) t.disposed++; } + for (const id of ledgers.flat()) tally(raiserGroup(id)).ledger++; + let found = 0; let unrecorded = 0; + for (const t of groups.values()) { const n = Math.max(t.rows, t.ledger); found += n; unrecorded += n - t.disposed; } + const out: ReviewLoopSummary = { passes: passes.length, blocking_found: found, deferred: deferred.size, noted: noted.size, fixed: fixed.size, scope }; + if (unrecorded) out.unrecorded = unrecorded; + if (ledgers.length) out.ledgers = ledgers.length; const fixChecks = loopRows.filter(r => r.skill === FIX_CHECK_SKILL).length; if (fixChecks) out.fix_checks = fixChecks; out.rounds = passes.length + fixChecks; @@ -390,5 +424,6 @@ export function dispatchTotal(d: Record | undefined): number | u export function reviewLoopLine(s: ReviewLoopSummary): string { const total = dispatchTotal(s.dispatches); return `REVIEW_LOOP: passes=${s.passes} blocking_found=${s.blocking_found} deferred=${s.deferred} noted=${s.noted} fixed=${s.fixed} scope=${s.scope}` + + (s.unrecorded ? ` unrecorded=${s.unrecorded}` : '') + (s.ledgers ? ` ledgers=${s.ledgers}` : '') + (s.fix_checks ? ` fix_checks=${s.fix_checks}` : '') + (total !== undefined ? ` dispatches=${total}` : '') + (s.est_input_tokens !== undefined ? ` est_input_tokens=${s.est_input_tokens}` : ''); } diff --git a/lib/ship-receipt.ts b/lib/ship-receipt.ts index aba864ed5..4c58e0404 100644 --- a/lib/ship-receipt.ts +++ b/lib/ship-receipt.ts @@ -328,6 +328,8 @@ export function parseReviewLoop(value: unknown): ReviewLoopReceipt | undefined { if (n('fix_checks') !== undefined) out.fix_checks = n('fix_checks'); if (n('rounds') !== undefined) out.rounds = n('rounds'); if (n('est_input_tokens') !== undefined) out.est_input_tokens = n('est_input_tokens'); + if (n('unrecorded')) out.unrecorded = n('unrecorded'); + if (n('ledgers')) out.ledgers = n('ledgers'); if (v.dispatches && typeof v.dispatches === 'object' && !Array.isArray(v.dispatches)) { const d = Object.fromEntries(Object.entries(v.dispatches as Record) .filter(([k, c]) => /^[a-z][a-z0-9-]{0,39}$/.test(k) && typeof c === 'number' && Number.isInteger(c) && c >= 0).slice(0, 12)) as Record; diff --git a/ship/DRIVER.md b/ship/DRIVER.md index ac19dd0bd..549a005fd 100644 --- a/ship/DRIVER.md +++ b/ship/DRIVER.md @@ -14,7 +14,7 @@ gates and the report. `STOP:` (exit 3) means nothing after it ran; `NEXT:` names the next act. Read a section only when a phase or the list at the end sends you, never up front. A STOP or decision this file does not cover: read `ship/SKILL.md` at that step, finish it there, come back. Unattended, take a question's recommended option (log it); gate -dispositions and approvals stay pending and end the run. Keep a private invocation record (`--record`): write its six headings at Phase 1, then fill each as its phase ends: **Checks** +dispositions and approvals stay pending and end the run. Keep a private invocation record (`--record`): write its six headings at Phase 1 and fill each as its phase ends: **Checks** (Phases 2-4: lanes, pregate, evals, `Coverage:`, the plan checks, `Exploratory QA:`, `Scope Check:`), **Decisions** (any phase: each answered or auto-taken question, verbatim), **Reviews** (Phases 3-5: the plan line, start tokens, `Pre-Landing Review:`, `Gate edits:`, the reviewed tree), **Release** (Phases 5-7: @@ -28,7 +28,7 @@ file, unchanged; its findings reach `merged.jsonl` only via `collect`; `judge` r small model at low effort; their findings go to the PR body. **Calls.** One pass with no fixes is 9 calls: Phases 1-8 and the report; at most 12 with a STOP's -section read or a fix-check, plus a keep-alive per 200 s waited. A fix repeats Phases 3-4. +section read or a fix-check, plus a keep-alive per 200 s waited. Fixes redo 3-4. **Budget: observe.** `gstack-ship-budget start --mode observe` records, never admits: each program ends with `gstack-ship-budget note --step --reading R --credits C --record `, R and C read in that program @@ -119,7 +119,7 @@ STALE or MISSING reruns once), `PREGATE:`; print them, the last `Gate edits:` an ## 8. Publish: Steps 18, 19, 19-C, 20, 21, 22 - Step 18: the open-PR lookup; a title file starting `v `. `UNATTENDED_SHIP:` with no open PR at HEAD takes Step 18-H. -- Step 19 body: the receipt block, then the sections `sections/pr-body.md` lists (Pre-Landing Review: `REVIEW_LOOP:`, `Gate edits:`, deferred, notes), led under `CAPY_PR: on` (Step 19-C) by What changed, Why, Blast radius and Approvals (`gstack-approvals render`). `gstack-prose-lint --sections 'Why,Summary,Blast radius' --gate`; `gstack-issue-links`; `gstack-review-scope summary --since "$REVIEW_SINCE" && gstack-ship-receipt write --pr --base --gate @ --pregate --issues @ --review-loop @ --docs @ --body `; `gstack-post pr-body` and `pr-title` (a new PR via the host's PR tool; a draft while a gate finding is `open`). `SHIP_RECEIPT:` opens your report. +- Step 19 body: the receipt block, then the sections `sections/pr-body.md` lists (Pre-Landing Review: `REVIEW_LOOP:`, `Gate edits:`, deferred, notes), led under `CAPY_PR: on` (Step 19-C) by What changed, Why, Blast radius and Approvals (`gstack-approvals render`). `gstack-prose-lint --sections 'Why,Summary,Blast radius' --gate`; `gstack-issue-links`; `gstack-review-scope summary --since "$REVIEW_SINCE" --run && gstack-ship-receipt write --pr --base --gate @ --pregate --issues @ --review-loop @ --docs @ --body `; `gstack-post pr-body` and `pr-title` (a new PR via the host's PR tool; a draft while a gate finding is `open`). `SHIP_RECEIPT:` opens your report. - Step 20: the metrics row via `gstack-review-log`. - Step 22: `gstack-ship-budget close --reading R --credits C` (report `BUDGET_FINAL:`, `BUDGET_TIMELINE:` (tokens, credits), any `METER:` line), then `gstack-ship-receipt result --pr `: `GSTACK_RESULT: skill=ship status=complete run=`. Under a merge queue, `gstack-ship-queue ready --pr [--screen|--no-screen]` at the pushed head; paste its output. diff --git a/ship/DRIVER.md.tmpl b/ship/DRIVER.md.tmpl index 0298fb93e..7d4abf27d 100644 --- a/ship/DRIVER.md.tmpl +++ b/ship/DRIVER.md.tmpl @@ -12,7 +12,7 @@ gates and the report. `STOP:` (exit 3) means nothing after it ran; `NEXT:` names the next act. Read a section only when a phase or the list at the end sends you, never up front. A STOP or decision this file does not cover: read `ship/SKILL.md` at that step, finish it there, come back. Unattended, take a question's recommended option (log it); gate -dispositions and approvals stay pending and end the run. Keep a private invocation record (`--record`): write its six headings at Phase 1, then fill each as its phase ends: **Checks** +dispositions and approvals stay pending and end the run. Keep a private invocation record (`--record`): write its six headings at Phase 1 and fill each as its phase ends: **Checks** (Phases 2-4: lanes, pregate, evals, `Coverage:`, the plan checks, `Exploratory QA:`, `Scope Check:`), **Decisions** (any phase: each answered or auto-taken question, verbatim), **Reviews** (Phases 3-5: the plan line, start tokens, `Pre-Landing Review:`, `Gate edits:`, the reviewed tree), **Release** (Phases 5-7: @@ -26,7 +26,7 @@ file, unchanged; its findings reach `merged.jsonl` only via `collect`; `judge` r small model at low effort; their findings go to the PR body. **Calls.** One pass with no fixes is 9 calls: Phases 1-8 and the report; at most 12 with a STOP's -section read or a fix-check, plus a keep-alive per 200 s waited. A fix repeats Phases 3-4. +section read or a fix-check, plus a keep-alive per 200 s waited. Fixes redo 3-4. **Budget: observe.** `gstack-ship-budget start --mode observe` records, never admits: each program ends with `gstack-ship-budget note --step --reading R --credits C --record `, R and C read in that program @@ -117,7 +117,7 @@ STALE or MISSING reruns once), `PREGATE:`; print them, the last `Gate edits:` an ## 8. Publish: Steps 18, 19, 19-C, 20, 21, 22 - Step 18: the open-PR lookup; a title file starting `v `. `UNATTENDED_SHIP:` with no open PR at HEAD takes Step 18-H. -- Step 19 body: the receipt block, then the sections `sections/pr-body.md` lists (Pre-Landing Review: `REVIEW_LOOP:`, `Gate edits:`, deferred, notes), led under `CAPY_PR: on` (Step 19-C) by What changed, Why, Blast radius and Approvals (`gstack-approvals render`). `gstack-prose-lint --sections 'Why,Summary,Blast radius' --gate`; `gstack-issue-links`; `gstack-review-scope summary --since "$REVIEW_SINCE" && gstack-ship-receipt write --pr --base --gate @ --pregate --issues @ --review-loop @ --docs @ --body `; `gstack-post pr-body` and `pr-title` (a new PR via the host's PR tool; a draft while a gate finding is `open`). `SHIP_RECEIPT:` opens your report. +- Step 19 body: the receipt block, then the sections `sections/pr-body.md` lists (Pre-Landing Review: `REVIEW_LOOP:`, `Gate edits:`, deferred, notes), led under `CAPY_PR: on` (Step 19-C) by What changed, Why, Blast radius and Approvals (`gstack-approvals render`). `gstack-prose-lint --sections 'Why,Summary,Blast radius' --gate`; `gstack-issue-links`; `gstack-review-scope summary --since "$REVIEW_SINCE" --run && gstack-ship-receipt write --pr --base --gate @ --pregate --issues @ --review-loop @ --docs @ --body `; `gstack-post pr-body` and `pr-title` (a new PR via the host's PR tool; a draft while a gate finding is `open`). `SHIP_RECEIPT:` opens your report. - Step 20: the metrics row via `gstack-review-log`. - Step 22: `gstack-ship-budget close --reading R --credits C` (report `BUDGET_FINAL:`, `BUDGET_TIMELINE:` (tokens, credits), any `METER:` line), then `gstack-ship-receipt result --pr `: `GSTACK_RESULT: skill=ship status=complete run=`. Under a merge queue, `gstack-ship-queue ready --pr [--screen|--no-screen]` at the pushed head; paste its output. diff --git a/ship/sections/pr-body.md b/ship/sections/pr-body.md index 64e2f50cc..85178819c 100644 --- a/ship/sections/pr-body.md +++ b/ship/sections/pr-body.md @@ -220,7 +220,7 @@ per-lane counts (`{"":{"pass":N,"fail":N,"skip":N}}`) to ```bash rm -f .gstack/tmp/review-loop.json -~/.claude/skills/gstack/bin/gstack-review-scope summary --since "$REVIEW_SINCE" --out .gstack/tmp/review-loop.json && +~/.claude/skills/gstack/bin/gstack-review-scope summary --since "$REVIEW_SINCE" --no-ledger --out .gstack/tmp/review-loop.json && ~/.claude/skills/gstack/bin/gstack-ship-receipt write --pr --base --gate @.gstack/tmp/ship-gate.json --pregate .gstack/tmp/pregate.json --issues @.gstack/tmp/issues.json --review-loop @.gstack/tmp/review-loop.json --docs @.gstack/tmp/docs-audit.json --body "$PR_BODY_FILE" ``` @@ -233,7 +233,9 @@ When the owner accepted a stop after a fix-check edit, add and put its printed `Review-loop exception:` line in the body; never a prose line under `REVIEW_LOOP:`. The summary reads this branch's review log for the invocation's passes (its -`REVIEW_LOOP:` line is the one the Pre-Landing Review section carries). The `&&` +`REVIEW_LOOP:` line is the one the Pre-Landing Review section carries). A review whose +readers ran through `gstack-review-dispatch` passes `--run ` instead of +`--no-ledger`, so their CRITICAL findings count. The `&&` matters: a summary that refuses (no pass matched `REVIEW_SINCE`) must not let `write` record a stale or empty loop file. diff --git a/ship/sections/pr-body.md.tmpl b/ship/sections/pr-body.md.tmpl index 8786e9daa..ca81552d4 100644 --- a/ship/sections/pr-body.md.tmpl +++ b/ship/sections/pr-body.md.tmpl @@ -203,7 +203,7 @@ per-lane counts (`{"":{"pass":N,"fail":N,"skip":N}}`) to ```bash rm -f .gstack/tmp/review-loop.json -~/.claude/skills/gstack/bin/gstack-review-scope summary --since "$REVIEW_SINCE" --out .gstack/tmp/review-loop.json && +~/.claude/skills/gstack/bin/gstack-review-scope summary --since "$REVIEW_SINCE" --no-ledger --out .gstack/tmp/review-loop.json && ~/.claude/skills/gstack/bin/gstack-ship-receipt write --pr --base --gate @.gstack/tmp/ship-gate.json --pregate .gstack/tmp/pregate.json --issues @.gstack/tmp/issues.json --review-loop @.gstack/tmp/review-loop.json --docs @.gstack/tmp/docs-audit.json --body "$PR_BODY_FILE" ``` @@ -216,7 +216,9 @@ When the owner accepted a stop after a fix-check edit, add and put its printed `Review-loop exception:` line in the body; never a prose line under `REVIEW_LOOP:`. The summary reads this branch's review log for the invocation's passes (its -`REVIEW_LOOP:` line is the one the Pre-Landing Review section carries). The `&&` +`REVIEW_LOOP:` line is the one the Pre-Landing Review section carries). A review whose +readers ran through `gstack-review-dispatch` passes `--run ` instead of +`--no-ledger`, so their CRITICAL findings count. The `&&` matters: a summary that refuses (no pass matched `REVIEW_SINCE`) must not let `write` record a stale or empty loop file. diff --git a/test/helpers/ship-frozen-scope-actor.ts b/test/helpers/ship-frozen-scope-actor.ts index 85c6dca67..19a2a8d28 100644 --- a/test/helpers/ship-frozen-scope-actor.ts +++ b/test/helpers/ship-frozen-scope-actor.ts @@ -190,7 +190,7 @@ function reviewRows(repo: string, env: NodeJS.ProcessEnv) { } function loopSummary(repo: string, env: NodeJS.ProcessEnv, since: string) { - return JSON.parse(command(repo, env, path.join(ROOT, 'bin/gstack-review-scope'), ['summary', '--since', since, '--json']).stdout.trim()); + return JSON.parse(command(repo, env, path.join(ROOT, 'bin/gstack-review-scope'), ['summary', '--no-ledger', '--since', since, '--json']).stdout.trim()); } export async function shipFrozenScopeFailures(fixture: ReturnType, result: Pick) { diff --git a/test/review-scope-ledger-count.test.ts b/test/review-scope-ledger-count.test.ts new file mode 100644 index 000000000..38802cf8b --- /dev/null +++ b/test/review-scope-ledger-count.test.ts @@ -0,0 +1,153 @@ +/** + * GST-951: the /ship receipt's review_loop counters (gstack-review-scope summary) missed real blockers three times on + * 2026-11-02, because summary read only the review-log rows while /ship's readers report through the dispatch ledgers + * (gstack-review-dispatch collect and judge). Each case below is a minimal reconstruction of that PR's rows and ledgers, + * built from its public PR body and receipt (the rows and ledgers themselves stayed on the lanes' machines): the rows + * reproduce the receipt's published counters on the old summary, and the ledgers hold what the PR body says the readers found. + * Ledgers are written with the real collect and judge, so the parser and judge rules apply as in a run. + */ +import { afterEach, describe, expect, test } from 'bun:test'; +import * as fs from 'node:fs'; +import * as path from 'node:path'; +import { EXIT } from '../lib/headless-artifacts'; +import { collect, readJsonl, recordJudgment, type JudgedLine, type MergedLine } from '../lib/review-ledger'; +import { parseReviewLoop } from '../lib/ship-receipt'; +import { cleanup, makeFixtureRepo, runBin, type FixtureRepo } from './helpers/restamp-fixture'; + +const repos: FixtureRepo[] = []; +afterEach(() => { while (repos.length) cleanup(repos.pop()!); }); + +const SINCE = '2026-11-02T01:00:00Z'; +const at = (min: number) => new Date(Date.parse(SINCE) + min * 60_000).toISOString(); +const review = (min: number, pass: number, findings: object[]) => ({ skill: 'review', via: 'ship', timestamp: at(min), status: findings.length ? 'issues_found' : 'clean', review_scope: 'frozen', pass, findings }); +const fixCheck = (min: number, findings: object[]) => ({ skill: 'review-fix-check', via: 'ship', timestamp: at(min), review_scope: 'frozen', findings }); +const note = (fp: string, raised_by: string) => ({ fingerprint: fp, severity: 'INFORMATIONAL', action: 'noted', blocking: false, raised_by }); + +/** One pass directory: each reader's output collected into its ledger, then lead judgment keeping every label. */ +function pass(dir: string, readers: Array<[string, string]>): string { + fs.mkdirSync(dir, { recursive: true }); + const ledger = path.join(dir, 'review-ledger.jsonl'); + const mergedFile = path.join(dir, 'merged.jsonl'); + fs.writeFileSync(mergedFile, ''); + for (const [reader, output] of readers) expect(collect(ledger, reader, output, mergedFile).review).toBe(true); + const merged = readJsonl(mergedFile); + const judged: JudgedLine[] = merged.map(m => ({ id: m.id, severity: m.label })); + const judgedFile = path.join(dir, 'judged.jsonl'); + fs.writeFileSync(judgedFile, judged.map(j => JSON.stringify(j) + '\n').join('')); + expect(recordJudgment(ledger, merged, judged, judgedFile, mergedFile).ok).toBe(true); + return ledger; +} + +function branchWith(rows: object[]): FixtureRepo { + const repo = makeFixtureRepo({ prefix: 'gst951-' }); + repos.push(repo); + repo.git('checkout', '-qb', 'feat'); + const slug = runBin(repo, 'gstack-slug', ['--get', 'SLUG']).out.trim(); + const logDir = path.join(repo.home, 'projects', slug); + fs.mkdirSync(logDir, { recursive: true }); + fs.writeFileSync(path.join(logDir, 'feat-reviews.jsonl'), rows.map(r => JSON.stringify(r) + '\n').join('')); + return repo; +} +const summary = (repo: FixtureRepo, ...args: string[]) => runBin(repo, 'gstack-review-scope', ['summary', '--since', SINCE, '--json', ...args]); +const loop = (r: { status: number | null; out: string; err: string }) => { expect(r.err).not.toContain('(REVIEW_SCOPE_'); expect(r.status).toBe(EXIT.ok); return JSON.parse(r.out); }; + +const CLEAN = 'NO FINDINGS'; +const OUTSIDE_CLEAN = 'No issues found in the diff.\nRecommendation: ship because no finding reaches the floor'; + +describe('GST-951: the receipt counts every blocker the readers found', () => { + // #3841 (reconstruction): the native adversarial pass's FIXABLE (CRITICAL) finding was fixed, but no row recorded it; the + // receipt's blocking_found=1 fixed=1 was entered by hand. Its two other findings were noted in pass 2. + test('#3841: an adversarial-only blocker counts', () => { + const repo = branchWith([ + review(0, 1, []), + review(40, 2, [note('lib/pregate.ts:88:docs', 'adversarial-native'), note('docs/pregate.md:12:docs', 'adversarial-native')]), + ]); + const run = path.join(repo.root, 'run'); + pass(path.join(run, 'p1'), [['specialist/testing', CLEAN], ['red-team', CLEAN]]); + pass(path.join(run, 'p1-voices'), [['outside-voice', OUTSIDE_CLEAN], ['adversarial-native', [ + '1. lib/pregate-clear.ts:140 The receipt stores the raw run URL, not the attempt it judged. FIXABLE', + '2. lib/pregate.ts:88 The refusal text does not name the attempt. INVESTIGATE', + '3. docs/pregate.md:12 The docs do not say a rerun replaces the attempt. INVESTIGATE', + 'Recommendation: fix 1 because a later attempt would be judged by the first', + ].join('\n')]]); + expect(loop(summary(repo, '--no-ledger'))).toMatchObject({ passes: 2, blocking_found: 0, noted: 2, fixed: 0 }); + const s = loop(summary(repo, '--run', run)); + expect(s).toMatchObject({ passes: 2, blocking_found: 1, noted: 2, fixed: 0, unrecorded: 1, ledgers: 2, rounds: 2 }); + }); + + // #3838 (reconstruction): the rows reproduce its receipt (passes=2 blocking_found=0 noted=8 fixed=4 fix_checks=3). Pass 2's + // red-team CRITICAL was logged fixed without classify's blocking stamp; Step 11's native adversarial CRITICAL (CSS + // image-set loading an outside image) was fixed in 0f61362c1 and no row holds it. + test('#3838: CRITICALs fixed without a classified row count', () => { + const repo = branchWith([ + review(0, 1, ['a', 'b', 'c'].map(x => ({ fingerprint: `lib/review-depth.ts:1${x}:style`, severity: 'INFORMATIONAL', action: 'fixed', blocking: false, raised_by: 'security' }))), + review(50, 2, [{ fingerprint: 'lib/review-depth.ts:212:security', severity: 'CRITICAL', action: 'fixed', raised_by: 'red-team' }, + note('lib/review-depth.ts:220:style', 'red-team'), note('lib/review-depth.ts:230:style', 'red-team')]), + fixCheck(60, [note('lib/review-depth.ts:240:style', 'fix-check'), note('lib/review-depth.ts:241:style', 'fix-check')]), + fixCheck(90, [note('lib/review-depth.ts:250:style', 'fix-check')]), + fixCheck(120, [note('lib/review-depth.ts:260:perf', 'fix-check'), note('lib/review-depth.ts:261:perf', 'fix-check'), note('lib/review-depth.ts:262:perf', 'fix-check')]), + ]); + const run = path.join(repo.root, 'run'); + pass(path.join(run, 'p1'), [['specialist/security', '- [INFORMATIONAL] lib/review-depth.ts:1a a CSS escape\n- [INFORMATIONAL] lib/review-depth.ts:1b a zero-padded reference\n- [INFORMATIONAL] lib/review-depth.ts:1c a split handler line'], ['red-team', CLEAN]]); + pass(path.join(run, 'p2'), [['red-team', '[CRITICAL] lib/review-depth.ts:212 a stray quote hides an external href from the drawing check']]); + pass(path.join(run, 'p2-voices'), [['outside-voice', OUTSIDE_CLEAN], ['adversarial-native', 'lib/review-depth.ts:300 CSS image-set() loads an outside image. FIXABLE\nRecommendation: fix because the drawing loads a remote URL']]); + expect(loop(summary(repo, '--no-ledger'))).toMatchObject({ passes: 2, blocking_found: 1, noted: 8, fixed: 4, fix_checks: 3, rounds: 5 }); + const s = loop(summary(repo, '--run', run)); + expect(s).toMatchObject({ passes: 2, blocking_found: 2, deferred: 0, noted: 8, fixed: 4, fix_checks: 3, rounds: 5, unrecorded: 1, ledgers: 3 }); + }); + + // #3837 (reconstruction): pass 1's row was logged before the outside voice's seven findings were classified, so they sit in the + // row with no severity, blocking stamp or action; the fix batch under the Captain's exception fixed them, and the receipt said + // blocking_found=0 fixed=0 noted=11 fix_checks=3. + test('#3837: blockers logged before classification count', () => { + const seven = Array.from({ length: 7 }, (_, i) => ({ fingerprint: `lib/inventory.ts:${10 + i}:logic`, raised_by: 'outside-voice' })); + const repo = branchWith([ + review(0, 1, [...seven, ...Array.from({ length: 9 }, (_, i) => note(`docs/inventory.md:${i + 1}:docs`, 'specialist'))]), + review(60, 2, [note('lib/inventory.ts:90:docs', 'red-team'), note('lib/inventory.ts:91:docs', 'red-team')]), + fixCheck(80, []), fixCheck(100, []), fixCheck(120, []), + ]); + const run = path.join(repo.root, 'run'); + pass(path.join(run, 'p1'), [['specialist-batched/1', CLEAN], ['red-team', CLEAN]]); + pass(path.join(run, 'p1-voices'), [['adversarial-native', CLEAN], ['outside-voice', [ + ...seven.map((f, i) => `${i + 1}. ${f.fingerprint.split(':').slice(0, 2).join(':')} finding ${i + 1} Severity: High Blocks: yes`), + 'Recommendation: fix 1-6 because a self-grandfathered row skips every plan rule', + ].join('\n')]]); + expect(loop(summary(repo, '--no-ledger'))).toMatchObject({ passes: 2, blocking_found: 0, noted: 11, fixed: 0, fix_checks: 3, rounds: 5 }); + const s = loop(summary(repo, '--run', run)); + expect(s).toMatchObject({ passes: 2, blocking_found: 7, noted: 11, fixed: 0, unrecorded: 7, ledgers: 2, rounds: 5 }); + const line = runBin(repo, 'gstack-review-scope', ['summary', '--since', SINCE, '--run', run]); + expect(line.out.trim()).toBe('REVIEW_LOOP: passes=2 blocking_found=7 deferred=0 noted=11 fixed=0 scope=frozen unrecorded=7 ledgers=2 fix_checks=3'); + expect(parseReviewLoop(s)).toMatchObject({ blocking_found: 7, unrecorded: 7, ledgers: 2, rounds: 5 }); + }); + + test('a blocker both sources hold counts once, and a disposed one is not unrecorded', () => { + const repo = branchWith([review(0, 1, [{ fingerprint: 'src/a.ts:3:logic', severity: 'CRITICAL', blocking: true, action: 'fixed', raised_by: 'red-team' }])]); + const run = path.join(repo.root, 'run'); + pass(path.join(run, 'p1'), [['specialist/testing', CLEAN], ['red-team', '[CRITICAL] src/a.ts:3 the guard is inverted']]); + const s = loop(summary(repo, '--run', run)); + expect(s).toMatchObject({ blocking_found: 1, fixed: 1, ledgers: 1 }); + expect(s.unrecorded).toBeUndefined(); + }); + + test('summary refuses REVIEW_SCOPE_LEDGER instead of counting without a ledger it needs, and writes nothing', () => { + const repo = branchWith([review(0, 1, [])]); + const run = path.join(repo.root, 'run'); + const ledger = pass(path.join(run, 'p1'), [['red-team', '[CRITICAL] src/a.ts:3 the guard is inverted']]); + const out = path.join(repo.root, 'loop.json'); + const refused = (r: { status: number | null; err: string }, why: string) => { expect(r.status).toBe(EXIT.fail); expect(r.err).toContain('(REVIEW_SCOPE_LEDGER;'); expect(r.err).toContain(why); expect(fs.existsSync(out)).toBe(false); }; + refused(summary(repo, '--out', out), '--no-ledger when no reader ran through gstack-review-dispatch'); + fs.mkdirSync(path.join(repo.root, 'empty')); + refused(summary(repo, '--run', path.join(repo.root, 'empty'), '--out', out), 'holds no review-ledger.jsonl'); + refused(summary(repo, '--ledger', path.join(run, 'nope.jsonl'), '--out', out), 'is unreadable'); + const judged = path.join(run, 'p1', 'judged.jsonl'); + const kept = fs.readFileSync(judged, 'utf8'); + fs.writeFileSync(judged, kept.replace('CRITICAL', 'INFORMATIONAL')); + refused(summary(repo, '--ledger', ledger, '--out', out), 'changed since judge'); + fs.writeFileSync(judged, kept); + fs.rmSync(path.join(run, 'p1', 'merged.jsonl')); + refused(summary(repo, '--run', run, '--out', out), 'is unreadable'); + const both = summary(repo, '--run', run, '--no-ledger'); + expect(both.status).toBe(EXIT.fail); + expect(both.err).toContain('(REVIEW_SCOPE_INPUT;'); + }); +}); diff --git a/test/review-scope-windows-launch.test.ts b/test/review-scope-windows-launch.test.ts index 68c077bd5..5a7405455 100644 --- a/test/review-scope-windows-launch.test.ts +++ b/test/review-scope-windows-launch.test.ts @@ -50,7 +50,7 @@ test('summary reads the review log through gstack-review-read (#3541)', () => { ]; fs.writeFileSync(path.join(state, 'projects', SLUG, 'feat-reviews.jsonl'), rows.map(r => JSON.stringify(r)).join('\n') + '\n'); const out = path.join(tmp, 'review-loop.json'); - const r = runScriptSync(path.join(ROOT, 'bin', 'gstack-review-scope'), ['summary', '--since', '2026-10-26T09:00:00Z', '--out', out, '--json'], { + const r = runScriptSync(path.join(ROOT, 'bin', 'gstack-review-scope'), ['summary', '--no-ledger', '--since', '2026-10-26T09:00:00Z', '--out', out, '--json'], { cwd: repo, env, encoding: 'utf8', timeout: 120_000, }); expect(r.error).toBeUndefined(); diff --git a/test/review-scope.test.ts b/test/review-scope.test.ts index e6bdc2ace..8b40ad3af 100644 --- a/test/review-scope.test.ts +++ b/test/review-scope.test.ts @@ -423,30 +423,30 @@ describe('gstack-review-scope (bin)', () => { fs.writeFileSync(path.join(logDir, `${branch}-reviews.jsonl`), rows.map(r => JSON.stringify(r)).join('\n') + '\n{not json\n'); const expected = { passes: 2, blocking_found: 2, deferred: 1, noted: 1, fixed: 1, scope: 'frozen', rounds: 2 }; const out = path.join(repo.root, 'review-loop.json'); - const r = runBin(repo, 'gstack-review-scope', ['summary', '--since', '2026-10-26T09:30:00Z', '--out', out, '--json']); + const r = runBin(repo, 'gstack-review-scope', ['summary', '--no-ledger', '--since', '2026-10-26T09:30:00Z', '--out', out, '--json']); expect(r.status).toBe(EXIT.ok); expect(r.err).toContain('REVIEW_LOOP: passes=2 blocking_found=2 deferred=1 noted=1 fixed=1 scope=frozen'); expect(JSON.parse(r.out)).toEqual(expected); expect(JSON.parse(fs.readFileSync(out, 'utf8'))).toEqual(expected); - const plain = runBin(repo, 'gstack-review-scope', ['summary', '--since', '2026-10-26T08:00:00Z']); + const plain = runBin(repo, 'gstack-review-scope', ['summary', '--no-ledger', '--since', '2026-10-26T08:00:00Z']); expect(plain.status).toBe(EXIT.ok); expect(plain.out.trim()).toBe('REVIEW_LOOP: passes=3 blocking_found=3 deferred=1 noted=1 fixed=2 scope=frozen'); const noSince = runBin(repo, 'gstack-review-scope', ['summary', '--json']); expect(noSince.status).toBe(EXIT.fail); expect(noSince.err).toContain('(REVIEW_SCOPE_INPUT;'); expect(noSince.out).toBe(''); - const badSince = runBin(repo, 'gstack-review-scope', ['summary', '--since', 'yesterday']); + const badSince = runBin(repo, 'gstack-review-scope', ['summary', '--no-ledger', '--since', 'yesterday']); expect(badSince.status).toBe(EXIT.fail); expect(badSince.err).toContain('(REVIEW_SCOPE_INPUT;'); // zero matching passes is a wrong --since or a log written elsewhere, never a loop to record: refused, nothing written const bare = fixture(); const noneOut = path.join(bare.root, 'review-loop.json'); - const none = runBin(bare, 'gstack-review-scope', ['summary', '--since', '2026-10-26T09:30:00Z', '--out', noneOut, '--json']); + const none = runBin(bare, 'gstack-review-scope', ['summary', '--no-ledger', '--since', '2026-10-26T09:30:00Z', '--out', noneOut, '--json']); expect(none.status).toBe(EXIT.fail); expect(none.err).toContain('(REVIEW_SCOPE_INPUT;'); expect(none.err).toContain('REVIEW_LOOP: passes=0'); expect(fs.existsSync(noneOut)).toBe(false); - const late = runBin(repo, 'gstack-review-scope', ['summary', '--since', '2026-10-27T00:00:00Z']); + const late = runBin(repo, 'gstack-review-scope', ['summary', '--no-ledger', '--since', '2026-10-27T00:00:00Z']); expect(late.status).toBe(EXIT.fail); }); }); diff --git a/test/review-targeted.test.ts b/test/review-targeted.test.ts index 375cf1d01..9ef2217d3 100644 --- a/test/review-targeted.test.ts +++ b/test/review-targeted.test.ts @@ -324,7 +324,7 @@ describe('the targeted role end to end on a fixture repository', () => { fs.mkdirSync(logDir, { recursive: true }); const row = { skill: 'review', via: 'ship', timestamp: '2026-10-30T10:00:00Z', status: 'clean', review_scope: 'frozen', findings: [], dispatches: [{ role: 'specialist' }, { role: 'adversarial-native' }, { role: 'targeted' }] }; fs.writeFileSync(path.join(logDir, 'feat-reviews.jsonl'), JSON.stringify(row) + '\n'); - const summary = runBin(repo, 'gstack-review-scope', ['summary', '--since', '2026-10-30T09:00:00Z', '--json']); + const summary = runBin(repo, 'gstack-review-scope', ['summary', '--no-ledger', '--since', '2026-10-30T09:00:00Z', '--json']); expect(summary.status, summary.err).toBe(0); expect(JSON.parse(summary.out).dispatches).toEqual({ specialist: 1, 'adversarial-native': 1, targeted: 1 }); diff --git a/test/size-ledger/ship/gst-951-receipt-counts-blockers.json b/test/size-ledger/ship/gst-951-receipt-counts-blockers.json new file mode 100644 index 000000000..be4f7a928 --- /dev/null +++ b/test/size-ledger/ship/gst-951-receipt-counts-blockers.json @@ -0,0 +1,4 @@ +{ + "bytes": 157, + "why": "GST-951: summary names its ledgers (--run in DRIVER, --no-ledger in pr-body)" +} From 11a9a24906f87b54680c37efb3a77b381330316e Mon Sep 17 00:00:00 2001 From: garrytan Date: Mon, 2 Nov 2026 07:30:49 +0000 Subject: [PATCH 2/5] fix(ship): match ledger blockers to rows by path:line; schema, prose and tests from review pass 1 (GST-951) Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --- bin/gstack-review-scope | 4 +- .../gst-951-receipt-counts-blockers.md | 2 +- docs/review-policy.md | 2 +- lib/headless-artifacts.ts | 2 +- lib/review-ledger.ts | 15 ++++++- lib/review-scope.ts | 44 +++++++++---------- ship/sections/pr-body.md | 7 +-- ship/sections/pr-body.md.tmpl | 7 +-- test/review-scope-ledger-count.test.ts | 44 +++++++++++++++++-- .../ship/gst-951-receipt-counts-blockers.json | 4 +- 10 files changed, 88 insertions(+), 43 deletions(-) diff --git a/bin/gstack-review-scope b/bin/gstack-review-scope index 8f071c17c..402090911 100755 --- a/bin/gstack-review-scope +++ b/bin/gstack-review-scope @@ -30,7 +30,7 @@ import { parseHunks, renderDeferredIssue, renderDeferredSection, reviewLoopLine, scopeFindings, scopeLine, summarizeReviewLoop, ALREADY_FILED_REASON, DEFERRED_MARKER, findingIdentity, findingWhere, REVIEW_SCOPE_POLICIES, type DeferredRecord, type ReviewScopePolicy, type ScopeFinding, type ScopedFinding, } from '../lib/review-scope'; -import { ledgerBlockers } from '../lib/review-ledger'; +import { ledgerBlockers, type LedgerBlocker } from '../lib/review-ledger'; const HELP = `gstack-review-scope — finding scope for /ship's review loop (docs/ship-policy.md#review-scope) @@ -267,7 +267,7 @@ function cmdSummary(rest: string[], json: boolean): number { if (noLedger && files.length) return refuse('summary', 'REVIEW_SCOPE_INPUT', '--no-ledger contradicts --run and --ledger'); // The ledgers hold every reader's findings, the voices' too, before any row classifies them; a summary without them can undercount. if (!noLedger && !files.length) return refuse('summary', 'REVIEW_SCOPE_LEDGER', 'name the review\'s dispatch ledgers (--run or --ledger ), or --no-ledger when no reader ran through gstack-review-dispatch'); - let ledgers: string[][]; + let ledgers: LedgerBlocker[][]; try { ledgers = files.map(ledgerBlockers); } catch (e: any) { return refuse('summary', 'REVIEW_SCOPE_LEDGER', e.message); } const summary = summarizeReviewLoop(rows, since, ledgers); const line = reviewLoopLine(summary); diff --git a/changelog.d/gst-951-receipt-counts-blockers.md b/changelog.d/gst-951-receipt-counts-blockers.md index 4a1fbc0c8..6cc425d33 100644 --- a/changelog.d/gst-951-receipt-counts-blockers.md +++ b/changelog.d/gst-951-receipt-counts-blockers.md @@ -1,2 +1,2 @@ -- **The /ship receipt counts every blocker its readers found** (GST-951). `gstack-review-scope summary` read only the review-log rows, but /ship's readers report through the dispatch ledgers (`gstack-review-dispatch collect` and `judge`). So a blocker that only the native adversarial pass raised, a CRITICAL logged fixed without classify's blocking stamp, and findings logged before they were classified all left `blocking_found` at 0. That happened on #3841, #3838 and #3837. The summary now also reads each ledger's collected and judged CRITICAL findings, takes the larger count per reviewer so a blocker held in both places counts once, and counts an unclassified CRITICAL row as blocking. `unrecorded=N` names blockers no row gives a disposition, and `ledgers=N` says how many ledgers it read. +- **The /ship receipt counts every blocker its readers found** (GST-951). `gstack-review-scope summary` read only the review-log rows, but /ship's readers report through the dispatch ledgers (`gstack-review-dispatch collect` and `judge`). So a blocker that only the native adversarial pass raised, a CRITICAL logged fixed without classify's blocking stamp, and findings logged before they were classified all left `blocking_found` at 0. That happened on #3841, #3838 and #3837. The summary now also reads each ledger's collected and judged CRITICAL findings, matches each one to its row by path:line so a blocker held in both places counts once and one classified below the floor does not, and counts an unclassified CRITICAL row as blocking. `unrecorded=N` names blockers no row gives a disposition, and `ledgers=N` says how many ledgers it read. - **The summary refuses rather than undercount.** It now needs `--run ` (it finds every `review-ledger.jsonl` under it), `--ledger `, or `--no-ledger` when no reader ran through `gstack-review-dispatch`. A missing choice, a run directory with no ledger, or an unreadable ledger, merged file or judged set, or a judged set changed since `judge`, is `REVIEW_SCOPE_LEDGER` with nothing written. /ship on Capy (`ship/DRIVER.md`) passes `--run`; the Claude Code flow passes `--no-ledger`. diff --git a/docs/review-policy.md b/docs/review-policy.md index cde94b9a8..7bc1067fe 100644 --- a/docs/review-policy.md +++ b/docs/review-policy.md @@ -395,7 +395,7 @@ puts in the receipt: | Field | Meaning | |---|---| | `review_loop.passes` | full review passes this ship ran (1 or 2; up to 3 under `review_scope: all`) | -| `review_loop.blocking_found` | every blocker found (GST-951): per reviewer, the larger of the rows' blocking findings (classified, or an unclassified CRITICAL) and the ledgers' CRITICAL findings as collected or judged, so an adversarial, outside-voice or not-yet-classified blocker counts. An unreadable ledger, merged file or judged set, or a judged set changed since `judge`, is `REVIEW_SCOPE_LEDGER`, never a count | +| `review_loop.blocking_found` | every blocker found (GST-951): the rows' blocking findings (classified, or an unclassified CRITICAL), plus each ledger CRITICAL (as collected or judged) that no row logged as a CRITICAL at the same path:line, so an adversarial, outside-voice or not-yet-classified blocker counts and one classify put below the floor does not. An unreadable ledger, merged file or judged set, or a judged set changed since `judge`, is `REVIEW_SCOPE_LEDGER`, never a count | | `review_loop.unrecorded` | blockers found that no row gives a disposition (fixed, deferred, noted, skipped, a gate answer); absent when 0 | | `review_loop.ledgers` | dispatch ledgers the count read; absent under `--no-ledger` | | `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/headless-artifacts.ts b/lib/headless-artifacts.ts index 355a51cf5..be03c0010 100644 --- a/lib/headless-artifacts.ts +++ b/lib/headless-artifacts.ts @@ -238,7 +238,7 @@ export const ARTIFACT_SCHEMAS = { thread: str, tree: { enum: ['same-modulo-stamps', 'changed', 'not-compared'] }, gate_reuse: str, policy: str, queue_mode: str, preregistration: str, history: str, pregate: { ...str, description: 'pass|warn|requires-remote cleared … @ from pregate.json, current for this head' }, requires_remote: strList, approval: { type: 'object', description: 'approval classes: { source, status: checked|unavailable, classes: [{ class, scope, effects, citation: verified|unverified, approved_by, date, via, binding, evidence }], report_only }' }, - review_loop: { type: 'object', required: ['passes', 'blocking_found', 'deferred'], properties: { passes: { type: 'integer', minimum: 0 }, blocking_found: { type: 'integer', minimum: 0 }, deferred: { type: 'integer', minimum: 0 }, noted: { type: 'integer', minimum: 0 }, fixed: { type: 'integer', minimum: 0 }, scope: str, fix_checks: { type: 'integer', minimum: 0 }, rounds: { type: 'integer', minimum: 0 }, dispatches: { type: 'object', description: 'reviewer dispatches by role' }, est_input_tokens: { type: 'integer', minimum: 0 } } }, + review_loop: { type: 'object', required: ['passes', 'blocking_found', 'deferred'], properties: { passes: { type: 'integer', minimum: 0 }, blocking_found: { type: 'integer', minimum: 0 }, deferred: { type: 'integer', minimum: 0 }, noted: { type: 'integer', minimum: 0 }, fixed: { type: 'integer', minimum: 0 }, scope: str, fix_checks: { type: 'integer', minimum: 0 }, rounds: { type: 'integer', minimum: 0 }, dispatches: { type: 'object', description: 'reviewer dispatches by role' }, est_input_tokens: { type: 'integer', minimum: 0 }, unrecorded: { type: 'integer', minimum: 0 }, ledgers: { type: 'integer', minimum: 0 } } }, carry: { type: 'object', required: ['from', 'reason', 'product_patch_id'], properties: { from: str, reason: str, product_patch_id: str, changed_paths: strList, merge_commits: { type: 'integer', minimum: 0 }, range_diff: str, covers: strList, reruns: strList, trees: strList, ancestry: { enum: ['linear', 'rewritten'] } } }, head_tree: str, product_patch_id: str, gstack_version: str, base_red: { type: 'object', required: ['cases', 'files', 'base', 'lane', 'expires_at'], properties: { cases: strList, files: strList, base: str, lane: str, expires_at: str } }, diff --git a/lib/review-ledger.ts b/lib/review-ledger.ts index 7e8aa54b1..84e5dc3ab 100644 --- a/lib/review-ledger.ts +++ b/lib/review-ledger.ts @@ -240,9 +240,18 @@ export function recordJudgment(ledger: string, merged: MergedLine[], judged: Jud * The blocking findings a run's ledger holds (GST-951): every collected line whose label ranks CRITICAL, raised to its judged * severity when the newest judge accepted a judged set that is unchanged since; lead judgment's additions count and a folded * duplicate does not. Every reader counts the same way, whatever it is and whether or not Step 9.4 classified its findings. + * Each carries its locus (a JSON finding's path:line, else the first file:line its text names) so the summary can find its row. * A ledger, merged file or judged set it cannot read, or a judged set changed since judge, throws: the count is never a guess. */ -export function ledgerBlockers(ledger: string): string[] { +export interface LedgerBlocker { id: string; locus: string | null } +function lineLocus(raw: string): string | null { + try { + const o = JSON.parse(raw); + if (o && typeof o.path === 'string' && Number.isInteger(o.line)) return `${o.path.replace(/^\.\//, '')}:${o.line}`; + } catch { /* a prose finding */ } + return LOCUS.exec(raw)?.[0]?.replace(/^\.\//, '') ?? null; +} +export function ledgerBlockers(ledger: string): LedgerBlocker[] { const fail = (why: string): never => { throw new Error(`${ledger}: ${why}`); }; const read = (file: unknown, what: string): string => { try { return fs.readFileSync(String(file), 'utf8'); } catch { return fail(`${what} ${String(file)} is unreadable`); } @@ -251,11 +260,13 @@ export function ledgerBlockers(ledger: string): string[] { text.split('\n').filter(l => l.trim()).map(l => { try { return JSON.parse(l); } catch { return fail(`${what} has a line that is not JSON`); } }); const events = jsonLines(read(ledger, 'ledger'), 'the ledger') as LedgerEvent[]; const ranks = new Map(); + const loci = new Map(); for (const o of events.filter(e => e.event === 'output' && Array.isArray(e.ids) && e.ids.length)) { const lines = jsonLines(read(o.merged, `merged file of ${String(o.reader)}`), String(o.merged)) as MergedLine[]; for (const id of o.ids as string[]) { const line = lines.find(l => l.id === id) ?? fail(`collected id ${id} is missing from ${String(o.merged)}`); ranks.set(id, rank(line.label) ?? 0); + loci.set(id, lineLocus(line.raw)); } } const verdict = [...events].reverse().find(e => e.event === 'judge'); @@ -267,7 +278,7 @@ export function ledgerBlockers(ledger: string): string[] { else ranks.set(j.id, Math.max(ranks.get(j.id) ?? 0, rank(j.severity) ?? 0)); } } - return [...ranks].filter(([, r]) => r >= 4).map(([id]) => id); + return [...ranks].filter(([, r]) => r >= 4).map(([id]) => ({ id, locus: loci.get(id) ?? null })); } /** diff --git a/lib/review-scope.ts b/lib/review-scope.ts index 43ead5e09..8ecfe3f54 100644 --- a/lib/review-scope.ts +++ b/lib/review-scope.ts @@ -14,7 +14,7 @@ * Pure functions here; bin/gstack-review-scope runs git and gh. */ import { SPECIALISTS } from './review-depth'; -import { rank } from './review-ledger'; +import { rank, type LedgerBlocker } from './review-ledger'; import { parseSpecialistOutput } from './state-matrix'; export type Locus = 'diff' | 'fix-range' | 'outside'; @@ -357,25 +357,27 @@ function rowDispatches(row: Record): Array<{ role: string; bytes: n const DISPOSITIONS = ['fixed', 'auto-fixed', 'deferred', 'noted', 'skipped', 'kept', 'restored', 'open']; /** A row finding that blocks: classified blocking, or an unclassified CRITICAL defect (logged before Step 9.4's classify). */ const rowBlocks = (f: Record) => f.blocking === true || (f.blocking === undefined && f.advisory !== true && rank(f.severity) === 4); -/** The reviewer a finding counts under: a row's raised_by or a ledger id's reader; every specialist is one group, as in the ledger. */ -function raiserGroup(raiser: unknown): string { - const role = typeof raiser === 'string' ? raiser.split(/[/#]/)[0]!.trim() : ''; - if (!role || role === 'unknown') return 'unattributed'; - return ['core', 'qa', 'red-team', 'adversarial-native', 'outside-voice', 'fix-check', 'targeted'].includes(role) ? role : 'specialist'; +/** A row finding's path:line, from its path and line or its fingerprint (`path:line:category`). */ +function rowLocus(f: Record): string | null { + if (typeof f.path === 'string' && Number.isInteger(f.line)) return `${f.path.replace(/^\.\//, '')}:${f.line}`; + const m = /^(.+?):(\d+):/.exec(typeof f.fingerprint === 'string' ? f.fingerprint : ''); + return m ? `${m[1]!.replace(/^\.\//, '')}:${m[2]}` : null; } /** - * The loop summary. blocking_found counts every blocker found (GST-951): per reviewer group, the larger of the rows' - * blockers and the dispatch ledgers' CRITICAL findings (`ledgerBlockers`, one id list per ledger), so a blocker that only - * a ledger holds (an adversarial or outside-voice finding, a finding logged before classification) still counts. Two - * sources can each hold one the other lacks only when the rows name no reviewer; then the count may run high, never low. + * The loop summary. blocking_found counts every blocker found (GST-951): the rows' blockers, plus each CRITICAL the dispatch + * ledgers hold (`ledgerBlockers`, one list per ledger) that no row logged as a CRITICAL at the same path:line. So a blocker + * only a ledger holds (an adversarial or outside-voice finding, one logged before classification) counts, and unrecorded, + * while one a row classified below the floor (blocking: false) does not. A ledger blocker with no locus, or one raised again + * in another pass's ledger with no row, counts each time: the count may run high, never low. */ -export function summarizeReviewLoop(rows: readonly Record[], since: string, ledgers: readonly (readonly string[])[] = []): ReviewLoopSummary { +export function summarizeReviewLoop(rows: readonly Record[], since: string, ledgers: readonly (readonly LedgerBlocker[])[] = []): ReviewLoopSummary { const sinceMs = Date.parse(since); const inRun = (r: Record) => r && r.via === 'ship' && Number.isFinite(rowTime(r)) && (!Number.isFinite(sinceMs) || rowTime(r) >= sinceMs); const loopRows = rows.filter(r => inRun(r) && (r.skill === 'review' || r.skill === LATE_DEFER_SKILL || r.skill === FIX_CHECK_SKILL)); const passes = loopRows.filter(r => r.skill === 'review'); - const blocking = new Map(); + const blocking = new Map(); + const rowCriticals = new Set(); const deferred = new Set(); const noted = new Set(); const fixed = new Set(); @@ -385,23 +387,17 @@ export function summarizeReviewLoop(rows: readonly Record[], since: for (const f of Array.isArray(row.findings) ? row.findings : []) { if (!f || typeof f !== 'object') continue; const id = findingIdentity(f); - if (rowBlocks(f)) { - const b = blocking.get(id) ?? { group: raiserGroup(f.raised_by), disposed: false }; - if (DISPOSITIONS.includes(f.action)) b.disposed = true; - blocking.set(id, b); - } + if (rowBlocks(f)) blocking.set(id, blocking.get(id) === true || DISPOSITIONS.includes(f.action)); + const locus = rowLocus(f); + if (locus && (rowBlocks(f) || rank(f.severity) === 4)) rowCriticals.add(locus); if (f.action === 'deferred') deferred.add(id); if (f.action === 'noted') noted.add(id); if (f.action === 'fixed' || f.action === 'auto-fixed') fixed.add(id); } } - const groups = new Map(); - const tally = (g: string) => groups.get(g) ?? groups.set(g, { rows: 0, disposed: 0, ledger: 0 }).get(g)!; - for (const b of blocking.values()) { const t = tally(b.group); t.rows++; if (b.disposed) t.disposed++; } - for (const id of ledgers.flat()) tally(raiserGroup(id)).ledger++; - let found = 0; let unrecorded = 0; - for (const t of groups.values()) { const n = Math.max(t.rows, t.ledger); found += n; unrecorded += n - t.disposed; } - const out: ReviewLoopSummary = { passes: passes.length, blocking_found: found, deferred: deferred.size, noted: noted.size, fixed: fixed.size, scope }; + const ledgerOnly = ledgers.flat().filter(b => !b.locus || !rowCriticals.has(b.locus)).length; + const unrecorded = ledgerOnly + [...blocking.values()].filter(disposed => !disposed).length; + const out: ReviewLoopSummary = { passes: passes.length, blocking_found: blocking.size + ledgerOnly, deferred: deferred.size, noted: noted.size, fixed: fixed.size, scope }; if (unrecorded) out.unrecorded = unrecorded; if (ledgers.length) out.ledgers = ledgers.length; const fixChecks = loopRows.filter(r => r.skill === FIX_CHECK_SKILL).length; diff --git a/ship/sections/pr-body.md b/ship/sections/pr-body.md index 85178819c..4d02514cb 100644 --- a/ship/sections/pr-body.md +++ b/ship/sections/pr-body.md @@ -233,9 +233,10 @@ When the owner accepted a stop after a fix-check edit, add and put its printed `Review-loop exception:` line in the body; never a prose line under `REVIEW_LOOP:`. The summary reads this branch's review log for the invocation's passes (its -`REVIEW_LOOP:` line is the one the Pre-Landing Review section carries). A review whose -readers ran through `gstack-review-dispatch` passes `--run ` instead of -`--no-ledger`, so their CRITICAL findings count. The `&&` +`REVIEW_LOOP:` line is the one the Pre-Landing Review section carries). When Step 11 +rendered the targeted reader, replace `--no-ledger` with `--ledger /review-ledger.jsonl` +(a run whose passes all ran through `gstack-review-dispatch` passes `--run `), so +every reader's CRITICAL findings count. The `&&` matters: a summary that refuses (no pass matched `REVIEW_SINCE`) must not let `write` record a stale or empty loop file. diff --git a/ship/sections/pr-body.md.tmpl b/ship/sections/pr-body.md.tmpl index ca81552d4..68b4c4ffc 100644 --- a/ship/sections/pr-body.md.tmpl +++ b/ship/sections/pr-body.md.tmpl @@ -216,9 +216,10 @@ When the owner accepted a stop after a fix-check edit, add and put its printed `Review-loop exception:` line in the body; never a prose line under `REVIEW_LOOP:`. The summary reads this branch's review log for the invocation's passes (its -`REVIEW_LOOP:` line is the one the Pre-Landing Review section carries). A review whose -readers ran through `gstack-review-dispatch` passes `--run ` instead of -`--no-ledger`, so their CRITICAL findings count. The `&&` +`REVIEW_LOOP:` line is the one the Pre-Landing Review section carries). When Step 11 +rendered the targeted reader, replace `--no-ledger` with `--ledger /review-ledger.jsonl` +(a run whose passes all ran through `gstack-review-dispatch` passes `--run `), so +every reader's CRITICAL findings count. The `&&` matters: a summary that refuses (no pass matched `REVIEW_SINCE`) must not let `write` record a stale or empty loop file. diff --git a/test/review-scope-ledger-count.test.ts b/test/review-scope-ledger-count.test.ts index 38802cf8b..048a64823 100644 --- a/test/review-scope-ledger-count.test.ts +++ b/test/review-scope-ledger-count.test.ts @@ -6,7 +6,7 @@ * reproduce the receipt's published counters on the old summary, and the ledgers hold what the PR body says the readers found. * Ledgers are written with the real collect and judge, so the parser and judge rules apply as in a run. */ -import { afterEach, describe, expect, test } from 'bun:test'; +import { afterEach, describe, expect, setDefaultTimeout, test } from 'bun:test'; import * as fs from 'node:fs'; import * as path from 'node:path'; import { EXIT } from '../lib/headless-artifacts'; @@ -14,6 +14,7 @@ import { collect, readJsonl, recordJudgment, type JudgedLine, type MergedLine } import { parseReviewLoop } from '../lib/ship-receipt'; import { cleanup, makeFixtureRepo, runBin, type FixtureRepo } from './helpers/restamp-fixture'; +setDefaultTimeout(60_000); const repos: FixtureRepo[] = []; afterEach(() => { while (repos.length) cleanup(repos.pop()!); }); @@ -23,15 +24,15 @@ const review = (min: number, pass: number, findings: object[]) => ({ skill: 'rev const fixCheck = (min: number, findings: object[]) => ({ skill: 'review-fix-check', via: 'ship', timestamp: at(min), review_scope: 'frozen', findings }); const note = (fp: string, raised_by: string) => ({ fingerprint: fp, severity: 'INFORMATIONAL', action: 'noted', blocking: false, raised_by }); -/** One pass directory: each reader's output collected into its ledger, then lead judgment keeping every label. */ -function pass(dir: string, readers: Array<[string, string]>): string { +/** One pass directory: each reader's output collected into its ledger, then lead judgment (default: every label kept). */ +function pass(dir: string, readers: Array<[string, string]>, judgeWith = (merged: MergedLine[]): JudgedLine[] => merged.map(m => ({ id: m.id, severity: m.label }))): string { fs.mkdirSync(dir, { recursive: true }); const ledger = path.join(dir, 'review-ledger.jsonl'); const mergedFile = path.join(dir, 'merged.jsonl'); fs.writeFileSync(mergedFile, ''); for (const [reader, output] of readers) expect(collect(ledger, reader, output, mergedFile).review).toBe(true); const merged = readJsonl(mergedFile); - const judged: JudgedLine[] = merged.map(m => ({ id: m.id, severity: m.label })); + const judged = judgeWith(merged); const judgedFile = path.join(dir, 'judged.jsonl'); fs.writeFileSync(judgedFile, judged.map(j => JSON.stringify(j) + '\n').join('')); expect(recordJudgment(ledger, merged, judged, judgedFile, mergedFile).ok).toBe(true); @@ -129,6 +130,37 @@ describe('GST-951: the receipt counts every blocker the readers found', () => { expect(s.unrecorded).toBeUndefined(); }); + test('a CRITICAL a row classified below the floor is not a blocker; distinct blockers from one reviewer each count', () => { + const repo = branchWith([review(0, 1, [ + { fingerprint: 'src/a.ts:3:testing', path: 'src/a.ts', line: 3, severity: 'CRITICAL', kind: 'test-gap', blocking: false, action: 'noted', raised_by: 'testing' }, + { fingerprint: 'src/b.ts:9:security', path: 'src/b.ts', line: 9, severity: 'CRITICAL', blocking: true, action: 'fixed', raised_by: 'security' }, + { fingerprint: 'src/c.ts:1:logic', severity: 'CRITICAL', advisory: true, action: 'noted', raised_by: 'security' }, + ])]); + const run = path.join(repo.root, 'run'); + pass(path.join(run, 'p1'), [['specialist/testing', '[CRITICAL] src/a.ts:3 the denied path has no test\n[CRITICAL] src/d.ts:7 the retry loop never ends']]); + const s = loop(summary(repo, '--run', run)); + expect(s).toMatchObject({ blocking_found: 2, fixed: 1, noted: 2, unrecorded: 1, ledgers: 1 }); + }); + + test('judgment decides: a folded duplicate counts once, a lead addition counts, a rejected judgment keeps the collected labels', () => { + const repo = branchWith([review(0, 1, [])]); + const run = path.join(repo.root, 'run'); + pass(path.join(run, 'fold'), [['red-team', '[CRITICAL] src/a.ts:3 the guard is inverted\n[CRITICAL] src/a.ts:4 the guard is inverted again']], + m => [{ id: m[0]!.id, severity: 'CRITICAL' }, { id: m[1]!.id, severity: 'CRITICAL', duplicate_of: m[0]!.id }, { id: 'lead#1', severity: 'CRITICAL', reason: 'src/e.ts:2 the lead found an unchecked write' }]); + expect(loop(summary(repo, '--run', run))).toMatchObject({ blocking_found: 2, unrecorded: 2, ledgers: 1 }); + const rejected = path.join(repo.root, 'rejected'); + fs.mkdirSync(rejected); + const ledger = path.join(rejected, 'review-ledger.jsonl'); + const mergedFile = path.join(rejected, 'merged.jsonl'); + fs.writeFileSync(mergedFile, ''); + collect(ledger, 'red-team', '[CRITICAL] src/a.ts:3 the guard is inverted', mergedFile); + const merged = readJsonl(mergedFile); + const judgedFile = path.join(rejected, 'judged.jsonl'); + fs.writeFileSync(judgedFile, JSON.stringify({ id: merged[0]!.id, severity: 'INFORMATIONAL' }) + '\n'); + expect(recordJudgment(ledger, merged, [{ id: merged[0]!.id, severity: 'INFORMATIONAL' }], judgedFile, mergedFile).ok).toBe(false); + expect(loop(summary(repo, '--ledger', ledger))).toMatchObject({ blocking_found: 1, unrecorded: 1, ledgers: 1 }); + }); + test('summary refuses REVIEW_SCOPE_LEDGER instead of counting without a ledger it needs, and writes nothing', () => { const repo = branchWith([review(0, 1, [])]); const run = path.join(repo.root, 'run'); @@ -146,6 +178,10 @@ describe('GST-951: the receipt counts every blocker the readers found', () => { fs.writeFileSync(judged, kept); fs.rmSync(path.join(run, 'p1', 'merged.jsonl')); refused(summary(repo, '--run', run, '--out', out), 'is unreadable'); + refused(summary(repo, '--run', path.join(repo.root, 'nonesuch'), '--out', out), 'ENOENT'); + const bare = summary(repo, '--out', out, '--ledger'); + expect(bare.err).toContain('(REVIEW_SCOPE_INPUT;'); + expect(bare.err).toContain('--ledger needs a file'); const both = summary(repo, '--run', run, '--no-ledger'); expect(both.status).toBe(EXIT.fail); expect(both.err).toContain('(REVIEW_SCOPE_INPUT;'); diff --git a/test/size-ledger/ship/gst-951-receipt-counts-blockers.json b/test/size-ledger/ship/gst-951-receipt-counts-blockers.json index be4f7a928..5adfe051c 100644 --- a/test/size-ledger/ship/gst-951-receipt-counts-blockers.json +++ b/test/size-ledger/ship/gst-951-receipt-counts-blockers.json @@ -1,4 +1,4 @@ { - "bytes": 157, - "why": "GST-951: summary names its ledgers (--run in DRIVER, --no-ledger in pr-body)" + "bytes": 263, + "why": "GST-951: summary names its ledgers (--run in DRIVER, --ledger or --no-ledger in pr-body)" } From 49421ee874e84e362c30665f8f51f99ace343820 Mon Sep 17 00:00:00 2001 From: garrytan Date: Mon, 2 Nov 2026 07:39:26 +0000 Subject: [PATCH 3/5] fix(ship): pair each ledger blocker with one row finding; newest classification wins; ledgers_read (GST-951 Step 11 findings) Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --- .../gst-951-receipt-counts-blockers.md | 2 +- docs/review-policy.md | 4 +- docs/ship-policy.md | 2 +- lib/headless-artifacts.ts | 4 +- lib/review-ledger.ts | 5 ++- lib/review-scope.ts | 45 ++++++++++++------- lib/ship-receipt.ts | 2 +- test/review-scope-ledger-count.test.ts | 40 +++++++++++++---- 8 files changed, 70 insertions(+), 34 deletions(-) diff --git a/changelog.d/gst-951-receipt-counts-blockers.md b/changelog.d/gst-951-receipt-counts-blockers.md index 6cc425d33..a7175c3f7 100644 --- a/changelog.d/gst-951-receipt-counts-blockers.md +++ b/changelog.d/gst-951-receipt-counts-blockers.md @@ -1,2 +1,2 @@ -- **The /ship receipt counts every blocker its readers found** (GST-951). `gstack-review-scope summary` read only the review-log rows, but /ship's readers report through the dispatch ledgers (`gstack-review-dispatch collect` and `judge`). So a blocker that only the native adversarial pass raised, a CRITICAL logged fixed without classify's blocking stamp, and findings logged before they were classified all left `blocking_found` at 0. That happened on #3841, #3838 and #3837. The summary now also reads each ledger's collected and judged CRITICAL findings, matches each one to its row by path:line so a blocker held in both places counts once and one classified below the floor does not, and counts an unclassified CRITICAL row as blocking. `unrecorded=N` names blockers no row gives a disposition, and `ledgers=N` says how many ledgers it read. +- **The /ship receipt counts every blocker its readers found** (GST-951). `gstack-review-scope summary` read only the review-log rows, but /ship's readers report through the dispatch ledgers (`gstack-review-dispatch collect` and `judge`). So a blocker that only the native adversarial pass raised, a CRITICAL logged fixed without classify's blocking stamp, and findings logged before they were classified all left `blocking_found` at 0. That happened on #3841, #3838 and #3837. The summary now also reads each ledger's collected and judged CRITICAL findings, pairs each one with at most one row finding at the same path:line so a blocker held in both places counts once and one classified below the floor does not, and counts an unclassified CRITICAL row as blocking. `unrecorded=N` names blockers no row gives a disposition, and `ledgers_read=N` says how many it read. - **The summary refuses rather than undercount.** It now needs `--run ` (it finds every `review-ledger.jsonl` under it), `--ledger `, or `--no-ledger` when no reader ran through `gstack-review-dispatch`. A missing choice, a run directory with no ledger, or an unreadable ledger, merged file or judged set, or a judged set changed since `judge`, is `REVIEW_SCOPE_LEDGER` with nothing written. /ship on Capy (`ship/DRIVER.md`) passes `--run`; the Claude Code flow passes `--no-ledger`. diff --git a/docs/review-policy.md b/docs/review-policy.md index 7bc1067fe..8b75ecb3d 100644 --- a/docs/review-policy.md +++ b/docs/review-policy.md @@ -395,9 +395,9 @@ puts in the receipt: | Field | Meaning | |---|---| | `review_loop.passes` | full review passes this ship ran (1 or 2; up to 3 under `review_scope: all`) | -| `review_loop.blocking_found` | every blocker found (GST-951): the rows' blocking findings (classified, or an unclassified CRITICAL), plus each ledger CRITICAL (as collected or judged) that no row logged as a CRITICAL at the same path:line, so an adversarial, outside-voice or not-yet-classified blocker counts and one classify put below the floor does not. An unreadable ledger, merged file or judged set, or a judged set changed since `judge`, is `REVIEW_SCOPE_LEDGER`, never a count | +| `review_loop.blocking_found` | every blocker found (GST-951): the rows' blocking findings (classified, or an unclassified CRITICAL), plus each ledger CRITICAL (as collected or judged) that no row finding at the same path:line accounts for (each row finding pairs with one), so an adversarial, outside-voice or not-yet-classified blocker counts and one whose newest classification is below the floor does not. An unreadable ledger, merged file or judged set, or a judged set changed since `judge`, is `REVIEW_SCOPE_LEDGER`, never a count | | `review_loop.unrecorded` | blockers found that no row gives a disposition (fixed, deferred, noted, skipped, a gate answer); absent when 0 | -| `review_loop.ledgers` | dispatch ledgers the count read; absent under `--no-ledger` | +| `review_loop.ledgers_read` | dispatch ledgers the count read; absent under `--no-ledger` | | `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) | | `review_loop.noted` | below-floor findings listed in the PR body | | `review_loop.fixed` | findings fixed in this PR | diff --git a/docs/ship-policy.md b/docs/ship-policy.md index 016af873d..b0c58c716 100644 --- a/docs/ship-policy.md +++ b/docs/ship-policy.md @@ -1272,7 +1272,7 @@ construction: rows for this invocation and its dispatch ledgers, and writes the counters the ship receipt carries (`gstack-ship-receipt write … --review-loop @`): `review_loop: { passes, blocking_found, deferred, noted, fixed, scope }`, plus `fix_checks`, `dispatches`, -`est_input_tokens`, `unrecorded` and `ledgers` when the run recorded them, printed on the receipt summary +`est_input_tokens`, `unrecorded` and `ledgers_read` when the run recorded them, printed on the receipt summary line as `passes= blocking_found= deferred=`, so a scorecard can track passes per PR. diff --git a/lib/headless-artifacts.ts b/lib/headless-artifacts.ts index be03c0010..06d9f5d93 100644 --- a/lib/headless-artifacts.ts +++ b/lib/headless-artifacts.ts @@ -106,7 +106,7 @@ export interface ShipReceipt { // Related open PRs by other authors (Lane D): `supersedes` / `credit` only on the owner's confirmation. issues?: { fixes: number[]; refs: number[]; flakes: Array<{ issue: number; run: string }>; related?: Array<{ pr: number; repo?: string; author: string; closes: number[]; supersedes?: boolean; credit?: boolean }> }; /** Harness lane E: the review loop's counters (gstack-review-scope summary) and the carry record when the receipt followed a head whose product diff was unchanged (gstack-ship-receipt carry). */ - review_loop?: { passes: number; blocking_found: number; deferred: number; noted?: number; fixed?: number; scope?: string; fix_checks?: number; rounds?: number; dispatches?: Record; est_input_tokens?: number; unrecorded?: number; ledgers?: number }; + review_loop?: { passes: number; blocking_found: number; deferred: number; noted?: number; fixed?: number; scope?: string; fix_checks?: number; rounds?: number; dispatches?: Record; est_input_tokens?: number; unrecorded?: number; ledgers_read?: number }; carry?: { from: string; reason: string; product_patch_id: string; changed_paths: string[]; merge_commits: number; range_diff: string; covers: string[]; reruns: string[]; trees?: string[]; ancestry?: 'linear' | 'rewritten' }; /** The reviewed head's tree and its product patch-id against the base (gstack-ship-receipt write; carry rebinds both): what a carry across rewritten history compares. */ head_tree?: string; product_patch_id?: string; @@ -238,7 +238,7 @@ export const ARTIFACT_SCHEMAS = { thread: str, tree: { enum: ['same-modulo-stamps', 'changed', 'not-compared'] }, gate_reuse: str, policy: str, queue_mode: str, preregistration: str, history: str, pregate: { ...str, description: 'pass|warn|requires-remote cleared … @ from pregate.json, current for this head' }, requires_remote: strList, approval: { type: 'object', description: 'approval classes: { source, status: checked|unavailable, classes: [{ class, scope, effects, citation: verified|unverified, approved_by, date, via, binding, evidence }], report_only }' }, - review_loop: { type: 'object', required: ['passes', 'blocking_found', 'deferred'], properties: { passes: { type: 'integer', minimum: 0 }, blocking_found: { type: 'integer', minimum: 0 }, deferred: { type: 'integer', minimum: 0 }, noted: { type: 'integer', minimum: 0 }, fixed: { type: 'integer', minimum: 0 }, scope: str, fix_checks: { type: 'integer', minimum: 0 }, rounds: { type: 'integer', minimum: 0 }, dispatches: { type: 'object', description: 'reviewer dispatches by role' }, est_input_tokens: { type: 'integer', minimum: 0 }, unrecorded: { type: 'integer', minimum: 0 }, ledgers: { type: 'integer', minimum: 0 } } }, + review_loop: { type: 'object', required: ['passes', 'blocking_found', 'deferred'], properties: { passes: { type: 'integer', minimum: 0 }, blocking_found: { type: 'integer', minimum: 0 }, deferred: { type: 'integer', minimum: 0 }, noted: { type: 'integer', minimum: 0 }, fixed: { type: 'integer', minimum: 0 }, scope: str, fix_checks: { type: 'integer', minimum: 0 }, rounds: { type: 'integer', minimum: 0 }, dispatches: { type: 'object', description: 'reviewer dispatches by role' }, est_input_tokens: { type: 'integer', minimum: 0 }, unrecorded: { type: 'integer', minimum: 0 }, ledgers_read: { type: 'integer', minimum: 0 } } }, carry: { type: 'object', required: ['from', 'reason', 'product_patch_id'], properties: { from: str, reason: str, product_patch_id: str, changed_paths: strList, merge_commits: { type: 'integer', minimum: 0 }, range_diff: str, covers: strList, reruns: strList, trees: strList, ancestry: { enum: ['linear', 'rewritten'] } } }, head_tree: str, product_patch_id: str, gstack_version: str, base_red: { type: 'object', required: ['cases', 'files', 'base', 'lane', 'expires_at'], properties: { cases: strList, files: strList, base: str, lane: str, expires_at: str } }, diff --git a/lib/review-ledger.ts b/lib/review-ledger.ts index 84e5dc3ab..7c3a1ff3e 100644 --- a/lib/review-ledger.ts +++ b/lib/review-ledger.ts @@ -274,8 +274,9 @@ export function ledgerBlockers(ledger: string): LedgerBlocker[] { const text = read(verdict.judged, 'judged set'); if (promptHash(text) !== verdict.judged_sha256) fail(`judged set ${String(verdict.judged)} changed since judge`); for (const j of jsonLines(text, String(verdict.judged)) as JudgedLine[]) { - if (j.duplicate_of) ranks.delete(j.id); - else ranks.set(j.id, Math.max(ranks.get(j.id) ?? 0, rank(j.severity) ?? 0)); + if (j.duplicate_of) { ranks.delete(j.id); continue; } + ranks.set(j.id, Math.max(ranks.get(j.id) ?? 0, rank(j.severity) ?? 0)); + if (!loci.has(j.id)) loci.set(j.id, lineLocus(j.reason ?? '')); } } return [...ranks].filter(([, r]) => r >= 4).map(([id]) => ({ id, locus: loci.get(id) ?? null })); diff --git a/lib/review-scope.ts b/lib/review-scope.ts index 8ecfe3f54..e5e86c453 100644 --- a/lib/review-scope.ts +++ b/lib/review-scope.ts @@ -310,7 +310,7 @@ export interface ReviewLoopSummary { /** Blocking findings no row gives a disposition (fixed, deferred, noted, skipped or a gate answer): found, outcome unrecorded. */ unrecorded?: number; /** Dispatch ledgers whose collected and judged findings were counted (gstack-review-dispatch). */ - ledgers?: number; + ledgers_read?: number; } /** Review-log rows of one /ship invocation (skill review, via ship, at or after `since`) → the loop summary. */ @@ -365,19 +365,20 @@ function rowLocus(f: Record): string | null { } /** - * The loop summary. blocking_found counts every blocker found (GST-951): the rows' blockers, plus each CRITICAL the dispatch - * ledgers hold (`ledgerBlockers`, one list per ledger) that no row logged as a CRITICAL at the same path:line. So a blocker - * only a ledger holds (an adversarial or outside-voice finding, one logged before classification) counts, and unrecorded, - * while one a row classified below the floor (blocking: false) does not. A ledger blocker with no locus, or one raised again - * in another pass's ledger with no row, counts each time: the count may run high, never low. + * The loop summary. blocking_found counts every blocker found (GST-951). Each CRITICAL the dispatch ledgers hold + * (`ledgerBlockers`, one list per ledger) is paired with at most one row finding at the same path:line that may be it (no + * severity yet, CRITICAL as logged or before a mapper lowered it, or blocking). A paired finding blocks unless its newest + * classification put it below the floor (blocking: false), and its row's action is its disposition. An unpaired ledger + * CRITICAL blocks and is unrecorded. A row finding blocks by its newest classification, else as an unclassified CRITICAL. + * A ledger CRITICAL with no locus, or a second one at a paired line, counts on its own: the count may run high, never low. */ export function summarizeReviewLoop(rows: readonly Record[], since: string, ledgers: readonly (readonly LedgerBlocker[])[] = []): ReviewLoopSummary { const sinceMs = Date.parse(since); const inRun = (r: Record) => r && r.via === 'ship' && Number.isFinite(rowTime(r)) && (!Number.isFinite(sinceMs) || rowTime(r) >= sinceMs); const loopRows = rows.filter(r => inRun(r) && (r.skill === 'review' || r.skill === LATE_DEFER_SKILL || r.skill === FIX_CHECK_SKILL)); const passes = loopRows.filter(r => r.skill === 'review'); - const blocking = new Map(); - const rowCriticals = new Set(); + type Seen = { locus: string | null; classified: boolean; blocks: boolean; candidate: boolean; disposed: boolean }; + const seen = new Map(); const deferred = new Set(); const noted = new Set(); const fixed = new Set(); @@ -387,19 +388,31 @@ export function summarizeReviewLoop(rows: readonly Record[], since: for (const f of Array.isArray(row.findings) ? row.findings : []) { if (!f || typeof f !== 'object') continue; const id = findingIdentity(f); - if (rowBlocks(f)) blocking.set(id, blocking.get(id) === true || DISPOSITIONS.includes(f.action)); - const locus = rowLocus(f); - if (locus && (rowBlocks(f) || rank(f.severity) === 4)) rowCriticals.add(locus); + const prior = seen.get(id); + const classified = typeof f.blocking === 'boolean'; + seen.set(id, { + locus: rowLocus(f) ?? prior?.locus ?? null, + classified: classified || !!prior?.classified, + blocks: classified ? f.blocking : prior?.classified ? prior.blocks : !!prior?.blocks || rowBlocks(f), + candidate: !!prior?.candidate || f.severity === undefined || rank(f.severity) === 4 || rank(f.raw_severity) === 4 || f.blocking === true, + disposed: !!prior?.disposed || DISPOSITIONS.includes(f.action), + }); if (f.action === 'deferred') deferred.add(id); if (f.action === 'noted') noted.add(id); if (f.action === 'fixed' || f.action === 'auto-fixed') fixed.add(id); } } - const ledgerOnly = ledgers.flat().filter(b => !b.locus || !rowCriticals.has(b.locus)).length; - const unrecorded = ledgerOnly + [...blocking.values()].filter(disposed => !disposed).length; - const out: ReviewLoopSummary = { passes: passes.length, blocking_found: blocking.size + ledgerOnly, deferred: deferred.size, noted: noted.size, fixed: fixed.size, scope }; + const paired = new Set(); + let ledgerOnly = 0; + for (const b of ledgers.flat()) { + const id = b.locus ? [...seen].find(([k, s]) => s.candidate && s.locus === b.locus && !paired.has(k))?.[0] : undefined; + if (id) paired.add(id); else ledgerOnly++; + } + const blockers = [...seen].filter(([k, s]) => s.blocks || (paired.has(k) && !s.classified)); + const unrecorded = ledgerOnly + blockers.filter(([, s]) => !s.disposed).length; + const out: ReviewLoopSummary = { passes: passes.length, blocking_found: blockers.length + ledgerOnly, deferred: deferred.size, noted: noted.size, fixed: fixed.size, scope }; if (unrecorded) out.unrecorded = unrecorded; - if (ledgers.length) out.ledgers = ledgers.length; + if (ledgers.length) out.ledgers_read = ledgers.length; const fixChecks = loopRows.filter(r => r.skill === FIX_CHECK_SKILL).length; if (fixChecks) out.fix_checks = fixChecks; out.rounds = passes.length + fixChecks; @@ -420,6 +433,6 @@ export function dispatchTotal(d: Record | undefined): number | u export function reviewLoopLine(s: ReviewLoopSummary): string { const total = dispatchTotal(s.dispatches); return `REVIEW_LOOP: passes=${s.passes} blocking_found=${s.blocking_found} deferred=${s.deferred} noted=${s.noted} fixed=${s.fixed} scope=${s.scope}` - + (s.unrecorded ? ` unrecorded=${s.unrecorded}` : '') + (s.ledgers ? ` ledgers=${s.ledgers}` : '') + + (s.unrecorded ? ` unrecorded=${s.unrecorded}` : '') + (s.ledgers_read ? ` ledgers_read=${s.ledgers_read}` : '') + (s.fix_checks ? ` fix_checks=${s.fix_checks}` : '') + (total !== undefined ? ` dispatches=${total}` : '') + (s.est_input_tokens !== undefined ? ` est_input_tokens=${s.est_input_tokens}` : ''); } diff --git a/lib/ship-receipt.ts b/lib/ship-receipt.ts index 4c58e0404..b628a0aba 100644 --- a/lib/ship-receipt.ts +++ b/lib/ship-receipt.ts @@ -329,7 +329,7 @@ export function parseReviewLoop(value: unknown): ReviewLoopReceipt | undefined { if (n('rounds') !== undefined) out.rounds = n('rounds'); if (n('est_input_tokens') !== undefined) out.est_input_tokens = n('est_input_tokens'); if (n('unrecorded')) out.unrecorded = n('unrecorded'); - if (n('ledgers')) out.ledgers = n('ledgers'); + if (n('ledgers_read')) out.ledgers_read = n('ledgers_read'); if (v.dispatches && typeof v.dispatches === 'object' && !Array.isArray(v.dispatches)) { const d = Object.fromEntries(Object.entries(v.dispatches as Record) .filter(([k, c]) => /^[a-z][a-z0-9-]{0,39}$/.test(k) && typeof c === 'number' && Number.isInteger(c) && c >= 0).slice(0, 12)) as Record; diff --git a/test/review-scope-ledger-count.test.ts b/test/review-scope-ledger-count.test.ts index 048a64823..08b835bbd 100644 --- a/test/review-scope-ledger-count.test.ts +++ b/test/review-scope-ledger-count.test.ts @@ -73,7 +73,7 @@ describe('GST-951: the receipt counts every blocker the readers found', () => { ].join('\n')]]); expect(loop(summary(repo, '--no-ledger'))).toMatchObject({ passes: 2, blocking_found: 0, noted: 2, fixed: 0 }); const s = loop(summary(repo, '--run', run)); - expect(s).toMatchObject({ passes: 2, blocking_found: 1, noted: 2, fixed: 0, unrecorded: 1, ledgers: 2, rounds: 2 }); + expect(s).toMatchObject({ passes: 2, blocking_found: 1, noted: 2, fixed: 0, unrecorded: 1, ledgers_read: 2, rounds: 2 }); }); // #3838 (reconstruction): the rows reproduce its receipt (passes=2 blocking_found=0 noted=8 fixed=4 fix_checks=3). Pass 2's @@ -94,7 +94,7 @@ describe('GST-951: the receipt counts every blocker the readers found', () => { pass(path.join(run, 'p2-voices'), [['outside-voice', OUTSIDE_CLEAN], ['adversarial-native', 'lib/review-depth.ts:300 CSS image-set() loads an outside image. FIXABLE\nRecommendation: fix because the drawing loads a remote URL']]); expect(loop(summary(repo, '--no-ledger'))).toMatchObject({ passes: 2, blocking_found: 1, noted: 8, fixed: 4, fix_checks: 3, rounds: 5 }); const s = loop(summary(repo, '--run', run)); - expect(s).toMatchObject({ passes: 2, blocking_found: 2, deferred: 0, noted: 8, fixed: 4, fix_checks: 3, rounds: 5, unrecorded: 1, ledgers: 3 }); + expect(s).toMatchObject({ passes: 2, blocking_found: 2, deferred: 0, noted: 8, fixed: 4, fix_checks: 3, rounds: 5, unrecorded: 1, ledgers_read: 3 }); }); // #3837 (reconstruction): pass 1's row was logged before the outside voice's seven findings were classified, so they sit in the @@ -115,10 +115,10 @@ describe('GST-951: the receipt counts every blocker the readers found', () => { ].join('\n')]]); expect(loop(summary(repo, '--no-ledger'))).toMatchObject({ passes: 2, blocking_found: 0, noted: 11, fixed: 0, fix_checks: 3, rounds: 5 }); const s = loop(summary(repo, '--run', run)); - expect(s).toMatchObject({ passes: 2, blocking_found: 7, noted: 11, fixed: 0, unrecorded: 7, ledgers: 2, rounds: 5 }); + expect(s).toMatchObject({ passes: 2, blocking_found: 7, noted: 11, fixed: 0, unrecorded: 7, ledgers_read: 2, rounds: 5 }); const line = runBin(repo, 'gstack-review-scope', ['summary', '--since', SINCE, '--run', run]); - expect(line.out.trim()).toBe('REVIEW_LOOP: passes=2 blocking_found=7 deferred=0 noted=11 fixed=0 scope=frozen unrecorded=7 ledgers=2 fix_checks=3'); - expect(parseReviewLoop(s)).toMatchObject({ blocking_found: 7, unrecorded: 7, ledgers: 2, rounds: 5 }); + expect(line.out.trim()).toBe('REVIEW_LOOP: passes=2 blocking_found=7 deferred=0 noted=11 fixed=0 scope=frozen unrecorded=7 ledgers_read=2 fix_checks=3'); + expect(parseReviewLoop(s)).toMatchObject({ blocking_found: 7, unrecorded: 7, ledgers_read: 2, rounds: 5 }); }); test('a blocker both sources hold counts once, and a disposed one is not unrecorded', () => { @@ -126,7 +126,7 @@ describe('GST-951: the receipt counts every blocker the readers found', () => { const run = path.join(repo.root, 'run'); pass(path.join(run, 'p1'), [['specialist/testing', CLEAN], ['red-team', '[CRITICAL] src/a.ts:3 the guard is inverted']]); const s = loop(summary(repo, '--run', run)); - expect(s).toMatchObject({ blocking_found: 1, fixed: 1, ledgers: 1 }); + expect(s).toMatchObject({ blocking_found: 1, fixed: 1, ledgers_read: 1 }); expect(s.unrecorded).toBeUndefined(); }); @@ -139,7 +139,29 @@ describe('GST-951: the receipt counts every blocker the readers found', () => { const run = path.join(repo.root, 'run'); pass(path.join(run, 'p1'), [['specialist/testing', '[CRITICAL] src/a.ts:3 the denied path has no test\n[CRITICAL] src/d.ts:7 the retry loop never ends']]); const s = loop(summary(repo, '--run', run)); - expect(s).toMatchObject({ blocking_found: 2, fixed: 1, noted: 2, unrecorded: 1, ledgers: 1 }); + expect(s).toMatchObject({ blocking_found: 2, fixed: 1, noted: 2, unrecorded: 1, ledgers_read: 1 }); + }); + + test('Step 11 review cases: a row finding pairs with one ledger CRITICAL; the newest classification and a mapped severity decide; a bare fixed row is a disposition', () => { + const row = (fp: string, f: object) => ({ fingerprint: fp, ...f }); + const repo = branchWith([ + review(0, 1, [ + row('src/auth.ts:42:testing', { path: 'src/auth.ts', line: 42, severity: 'CRITICAL', kind: 'test-gap', blocking: false, action: 'noted', raised_by: 'testing' }), + row('src/m.ts:5:state-matrix', { path: 'src/m.ts', line: 5, severity: 'INFORMATIONAL', raw_severity: 'CRITICAL', kind: 'test-gap', blocking: false, action: 'noted', raised_by: 'state-machines' }), + row('src/f.ts:3:logic', { action: 'fixed', raised_by: 'red-team' }), + row('src/n.ts:8:logic', { severity: 'CRITICAL', raised_by: 'red-team' }), + row('src/g.ts:7:logic', { action: 'fixed', raised_by: 'core' }), + ]), + review(30, 2, [row('src/n.ts:8:logic', { severity: 'CRITICAL', blocking: false, action: 'noted', raised_by: 'red-team' })]), + ]); + const run = path.join(repo.root, 'run'); + pass(path.join(run, 'p1'), [ + ['specialist/testing', '[CRITICAL] src/auth.ts:42 the denied path has no test\n[CRITICAL] src/m.ts:5 untested cell'], + ['outside-voice', '1. src/auth.ts:42 an unauthenticated caller reaches the handler. Severity: High Blocks: yes\n2. src/f.ts:3 the guard is inverted. Blocks: yes\nRecommendation: fix 1 because it is an open handler'], + ['red-team', '[CRITICAL] src/n.ts:8 looked wrong, classified below the floor later'], + ], m => [...m.map(l => ({ id: l.id, severity: l.label })), { id: 'lead#1', severity: 'CRITICAL', reason: 'src/g.ts:7 the lead found an unchecked write' }]); + const s = loop(summary(repo, '--run', run)); + expect(s).toMatchObject({ passes: 2, blocking_found: 3, fixed: 2, unrecorded: 1, ledgers_read: 1 }); }); test('judgment decides: a folded duplicate counts once, a lead addition counts, a rejected judgment keeps the collected labels', () => { @@ -147,7 +169,7 @@ describe('GST-951: the receipt counts every blocker the readers found', () => { const run = path.join(repo.root, 'run'); pass(path.join(run, 'fold'), [['red-team', '[CRITICAL] src/a.ts:3 the guard is inverted\n[CRITICAL] src/a.ts:4 the guard is inverted again']], m => [{ id: m[0]!.id, severity: 'CRITICAL' }, { id: m[1]!.id, severity: 'CRITICAL', duplicate_of: m[0]!.id }, { id: 'lead#1', severity: 'CRITICAL', reason: 'src/e.ts:2 the lead found an unchecked write' }]); - expect(loop(summary(repo, '--run', run))).toMatchObject({ blocking_found: 2, unrecorded: 2, ledgers: 1 }); + expect(loop(summary(repo, '--run', run))).toMatchObject({ blocking_found: 2, unrecorded: 2, ledgers_read: 1 }); const rejected = path.join(repo.root, 'rejected'); fs.mkdirSync(rejected); const ledger = path.join(rejected, 'review-ledger.jsonl'); @@ -158,7 +180,7 @@ describe('GST-951: the receipt counts every blocker the readers found', () => { const judgedFile = path.join(rejected, 'judged.jsonl'); fs.writeFileSync(judgedFile, JSON.stringify({ id: merged[0]!.id, severity: 'INFORMATIONAL' }) + '\n'); expect(recordJudgment(ledger, merged, [{ id: merged[0]!.id, severity: 'INFORMATIONAL' }], judgedFile, mergedFile).ok).toBe(false); - expect(loop(summary(repo, '--ledger', ledger))).toMatchObject({ blocking_found: 1, unrecorded: 1, ledgers: 1 }); + expect(loop(summary(repo, '--ledger', ledger))).toMatchObject({ blocking_found: 1, unrecorded: 1, ledgers_read: 1 }); }); test('summary refuses REVIEW_SCOPE_LEDGER instead of counting without a ledger it needs, and writes nothing', () => { From 15823cc46656deb6c2e1b17c355321feffdb801b Mon Sep 17 00:00:00 2001 From: garrytan Date: Mon, 2 Nov 2026 07:50:44 +0000 Subject: [PATCH 4/5] fix(ship): pair ledger blockers by reviewer and per ledger; extensionless loci; pr-body names no targeted ledger (GST-951 round-2 findings, pending grant) Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --- .../gst-951-receipt-counts-blockers.md | 2 +- docs/review-policy.md | 2 +- lib/review-ledger.ts | 5 +- lib/review-scope.ts | 30 +++++++--- ship/DRIVER.md | 6 +- ship/DRIVER.md.tmpl | 6 +- ship/sections/pr-body.md | 7 +-- ship/sections/pr-body.md.tmpl | 7 +-- test/review-scope-ledger-count.test.ts | 58 +++++++++++++++---- .../ship/gst-951-receipt-counts-blockers.json | 4 +- 10 files changed, 89 insertions(+), 38 deletions(-) diff --git a/changelog.d/gst-951-receipt-counts-blockers.md b/changelog.d/gst-951-receipt-counts-blockers.md index a7175c3f7..ed0cce59a 100644 --- a/changelog.d/gst-951-receipt-counts-blockers.md +++ b/changelog.d/gst-951-receipt-counts-blockers.md @@ -1,2 +1,2 @@ -- **The /ship receipt counts every blocker its readers found** (GST-951). `gstack-review-scope summary` read only the review-log rows, but /ship's readers report through the dispatch ledgers (`gstack-review-dispatch collect` and `judge`). So a blocker that only the native adversarial pass raised, a CRITICAL logged fixed without classify's blocking stamp, and findings logged before they were classified all left `blocking_found` at 0. That happened on #3841, #3838 and #3837. The summary now also reads each ledger's collected and judged CRITICAL findings, pairs each one with at most one row finding at the same path:line so a blocker held in both places counts once and one classified below the floor does not, and counts an unclassified CRITICAL row as blocking. `unrecorded=N` names blockers no row gives a disposition, and `ledgers_read=N` says how many it read. +- **The /ship receipt counts every blocker its readers found** (GST-951). `gstack-review-scope summary` read only the review-log rows, but /ship's readers report through the dispatch ledgers (`gstack-review-dispatch collect` and `judge`). So a blocker that only the native adversarial pass raised, a CRITICAL logged fixed without classify's blocking stamp, and findings logged before they were classified all left `blocking_found` at 0. That happened on #3841, #3838 and #3837. The summary now also reads each ledger's collected and judged CRITICAL findings, pairs each one with at most one row finding from the same reviewer at the same path:line so a blocker held in both places counts once and one classified below the floor does not, and counts an unclassified CRITICAL row as blocking. `unrecorded=N` names blockers no row gives a disposition, and `ledgers_read=N` says how many it read. - **The summary refuses rather than undercount.** It now needs `--run ` (it finds every `review-ledger.jsonl` under it), `--ledger `, or `--no-ledger` when no reader ran through `gstack-review-dispatch`. A missing choice, a run directory with no ledger, or an unreadable ledger, merged file or judged set, or a judged set changed since `judge`, is `REVIEW_SCOPE_LEDGER` with nothing written. /ship on Capy (`ship/DRIVER.md`) passes `--run`; the Claude Code flow passes `--no-ledger`. diff --git a/docs/review-policy.md b/docs/review-policy.md index 8b75ecb3d..b3660c34c 100644 --- a/docs/review-policy.md +++ b/docs/review-policy.md @@ -395,7 +395,7 @@ puts in the receipt: | Field | Meaning | |---|---| | `review_loop.passes` | full review passes this ship ran (1 or 2; up to 3 under `review_scope: all`) | -| `review_loop.blocking_found` | every blocker found (GST-951): the rows' blocking findings (classified, or an unclassified CRITICAL), plus each ledger CRITICAL (as collected or judged) that no row finding at the same path:line accounts for (each row finding pairs with one), so an adversarial, outside-voice or not-yet-classified blocker counts and one whose newest classification is below the floor does not. An unreadable ledger, merged file or judged set, or a judged set changed since `judge`, is `REVIEW_SCOPE_LEDGER`, never a count | +| `review_loop.blocking_found` | every blocker found (GST-951): the rows' blocking findings (classified, or an unclassified CRITICAL), plus each ledger CRITICAL (as collected or judged) that no row finding from the same reviewer at the same path:line accounts for (each row finding pairs with one per ledger), so an adversarial, outside-voice or not-yet-classified blocker counts and one whose newest classification is below the floor does not. An unreadable ledger, merged file or judged set, or a judged set changed since `judge`, is `REVIEW_SCOPE_LEDGER`, never a count | | `review_loop.unrecorded` | blockers found that no row gives a disposition (fixed, deferred, noted, skipped, a gate answer); absent when 0 | | `review_loop.ledgers_read` | dispatch ledgers the count read; absent under `--no-ledger` | | `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-ledger.ts b/lib/review-ledger.ts index 7c3a1ff3e..5f2973f66 100644 --- a/lib/review-ledger.ts +++ b/lib/review-ledger.ts @@ -240,7 +240,8 @@ export function recordJudgment(ledger: string, merged: MergedLine[], judged: Jud * The blocking findings a run's ledger holds (GST-951): every collected line whose label ranks CRITICAL, raised to its judged * severity when the newest judge accepted a judged set that is unchanged since; lead judgment's additions count and a folded * duplicate does not. Every reader counts the same way, whatever it is and whether or not Step 9.4 classified its findings. - * Each carries its locus (a JSON finding's path:line, else the first file:line its text names) so the summary can find its row. + * Each carries its locus (a JSON finding's path:line, else the first path:line its text names, an extensionless bin/ path + * included) so the summary can find its row. * A ledger, merged file or judged set it cannot read, or a judged set changed since judge, throws: the count is never a guess. */ export interface LedgerBlocker { id: string; locus: string | null } @@ -249,7 +250,7 @@ function lineLocus(raw: string): string | null { const o = JSON.parse(raw); if (o && typeof o.path === 'string' && Number.isInteger(o.line)) return `${o.path.replace(/^\.\//, '')}:${o.line}`; } catch { /* a prose finding */ } - return LOCUS.exec(raw)?.[0]?.replace(/^\.\//, '') ?? null; + return /(?:[\w.-]+\/)+[\w.-]+:\d+|[\w.-]+\.\w+:\d+/.exec(raw)?.[0]?.replace(/^\.\//, '') ?? null; } export function ledgerBlockers(ledger: string): LedgerBlocker[] { const fail = (why: string): never => { throw new Error(`${ledger}: ${why}`); }; diff --git a/lib/review-scope.ts b/lib/review-scope.ts index e5e86c453..6ab96923f 100644 --- a/lib/review-scope.ts +++ b/lib/review-scope.ts @@ -357,6 +357,12 @@ function rowDispatches(row: Record): Array<{ role: string; bytes: n const DISPOSITIONS = ['fixed', 'auto-fixed', 'deferred', 'noted', 'skipped', 'kept', 'restored', 'open']; /** A row finding that blocks: classified blocking, or an unclassified CRITICAL defect (logged before Step 9.4's classify). */ const rowBlocks = (f: Record) => f.blocking === true || (f.blocking === undefined && f.advisory !== true && rank(f.severity) === 4); +/** The reviewer behind a row's raised_by or a ledger id: every specialist is one; the lead's additions and unknowns name none. */ +function raiserOf(raiser: unknown): string | null { + const role = typeof raiser === 'string' ? raiser.split(/[/#]/)[0]!.trim() : ''; + if (!role || role === 'unknown' || role === 'lead' || role === 'core') return null; + return ['qa', 'red-team', 'adversarial-native', 'outside-voice', 'fix-check', 'targeted'].includes(role) ? role : 'specialist'; +} /** A row finding's path:line, from its path and line or its fingerprint (`path:line:category`). */ function rowLocus(f: Record): string | null { if (typeof f.path === 'string' && Number.isInteger(f.line)) return `${f.path.replace(/^\.\//, '')}:${f.line}`; @@ -366,18 +372,21 @@ function rowLocus(f: Record): string | null { /** * The loop summary. blocking_found counts every blocker found (GST-951). Each CRITICAL the dispatch ledgers hold - * (`ledgerBlockers`, one list per ledger) is paired with at most one row finding at the same path:line that may be it (no - * severity yet, CRITICAL as logged or before a mapper lowered it, or blocking). A paired finding blocks unless its newest + * (`ledgerBlockers`, one list per ledger) is paired with at most one row finding per ledger at the same path:line, raised + * by the same reviewer (or naming none; a fix-check re-reads every reviewer's fixes), that may be it (no severity yet, CRITICAL as logged or before a mapper lowered + * it, or blocking), so one finding re-raised by a later pass's ledger pairs again. A paired finding blocks unless its newest * classification put it below the floor (blocking: false), and its row's action is its disposition. An unpaired ledger * CRITICAL blocks and is unrecorded. A row finding blocks by its newest classification, else as an unclassified CRITICAL. - * A ledger CRITICAL with no locus, or a second one at a paired line, counts on its own: the count may run high, never low. + * A ledger CRITICAL with no locus, or a second one at a paired line, counts on its own. A different defect the same reviewer + * raises at the exact line of a finding a row classified below the floor pairs with that finding: path:line is the only + * identity rows and ledgers share until rows carry the ledger id. */ export function summarizeReviewLoop(rows: readonly Record[], since: string, ledgers: readonly (readonly LedgerBlocker[])[] = []): ReviewLoopSummary { const sinceMs = Date.parse(since); const inRun = (r: Record) => r && r.via === 'ship' && Number.isFinite(rowTime(r)) && (!Number.isFinite(sinceMs) || rowTime(r) >= sinceMs); const loopRows = rows.filter(r => inRun(r) && (r.skill === 'review' || r.skill === LATE_DEFER_SKILL || r.skill === FIX_CHECK_SKILL)); const passes = loopRows.filter(r => r.skill === 'review'); - type Seen = { locus: string | null; classified: boolean; blocks: boolean; candidate: boolean; disposed: boolean }; + type Seen = { locus: string | null; raiser: string | null; classified: boolean; blocks: boolean; candidate: boolean; disposed: boolean }; const seen = new Map(); const deferred = new Set(); const noted = new Set(); @@ -392,6 +401,7 @@ export function summarizeReviewLoop(rows: readonly Record[], since: const classified = typeof f.blocking === 'boolean'; seen.set(id, { locus: rowLocus(f) ?? prior?.locus ?? null, + raiser: prior?.raiser ?? raiserOf(f.raised_by), classified: classified || !!prior?.classified, blocks: classified ? f.blocking : prior?.classified ? prior.blocks : !!prior?.blocks || rowBlocks(f), candidate: !!prior?.candidate || f.severity === undefined || rank(f.severity) === 4 || rank(f.raw_severity) === 4 || f.blocking === true, @@ -404,11 +414,15 @@ export function summarizeReviewLoop(rows: readonly Record[], since: } const paired = new Set(); let ledgerOnly = 0; - for (const b of ledgers.flat()) { - const id = b.locus ? [...seen].find(([k, s]) => s.candidate && s.locus === b.locus && !paired.has(k))?.[0] : undefined; - if (id) paired.add(id); else ledgerOnly++; + const pairedIds = new Set(); + for (const [n, list] of ledgers.entries()) { + for (const b of list) { + const by = b.id.startsWith('fix-check') ? null : raiserOf(b.id); + const id = b.locus ? [...seen].find(([k, s]) => s.candidate && s.locus === b.locus && (!s.raiser || !by || s.raiser === by) && !paired.has(`${n}\0${k}`))?.[0] : undefined; + if (id) { paired.add(`${n}\0${id}`); pairedIds.add(id); } else ledgerOnly++; + } } - const blockers = [...seen].filter(([k, s]) => s.blocks || (paired.has(k) && !s.classified)); + const blockers = [...seen].filter(([k, s]) => s.blocks || (pairedIds.has(k) && !s.classified)); const unrecorded = ledgerOnly + blockers.filter(([, s]) => !s.disposed).length; const out: ReviewLoopSummary = { passes: passes.length, blocking_found: blockers.length + ledgerOnly, deferred: deferred.size, noted: noted.size, fixed: fixed.size, scope }; if (unrecorded) out.unrecorded = unrecorded; diff --git a/ship/DRIVER.md b/ship/DRIVER.md index 549a005fd..d799205cf 100644 --- a/ship/DRIVER.md +++ b/ship/DRIVER.md @@ -14,7 +14,7 @@ gates and the report. `STOP:` (exit 3) means nothing after it ran; `NEXT:` names the next act. Read a section only when a phase or the list at the end sends you, never up front. A STOP or decision this file does not cover: read `ship/SKILL.md` at that step, finish it there, come back. Unattended, take a question's recommended option (log it); gate -dispositions and approvals stay pending and end the run. Keep a private invocation record (`--record`): write its six headings at Phase 1 and fill each as its phase ends: **Checks** +dispositions and approvals stay pending and end the run. Keep a private invocation record (`--record`): write its six headings at Phase 1, fill each as its phase ends: **Checks** (Phases 2-4: lanes, pregate, evals, `Coverage:`, the plan checks, `Exploratory QA:`, `Scope Check:`), **Decisions** (any phase: each answered or auto-taken question, verbatim), **Reviews** (Phases 3-5: the plan line, start tokens, `Pre-Landing Review:`, `Gate edits:`, the reviewed tree), **Release** (Phases 5-7: @@ -28,7 +28,7 @@ file, unchanged; its findings reach `merged.jsonl` only via `collect`; `judge` r small model at low effort; their findings go to the PR body. **Calls.** One pass with no fixes is 9 calls: Phases 1-8 and the report; at most 12 with a STOP's -section read or a fix-check, plus a keep-alive per 200 s waited. Fixes redo 3-4. +section read or a fix-check, plus a keep-alive per 200 s waited. **Budget: observe.** `gstack-ship-budget start --mode observe` records, never admits: each program ends with `gstack-ship-budget note --step --reading R --credits C --record `, R and C read in that program @@ -119,7 +119,7 @@ STALE or MISSING reruns once), `PREGATE:`; print them, the last `Gate edits:` an ## 8. Publish: Steps 18, 19, 19-C, 20, 21, 22 - Step 18: the open-PR lookup; a title file starting `v `. `UNATTENDED_SHIP:` with no open PR at HEAD takes Step 18-H. -- Step 19 body: the receipt block, then the sections `sections/pr-body.md` lists (Pre-Landing Review: `REVIEW_LOOP:`, `Gate edits:`, deferred, notes), led under `CAPY_PR: on` (Step 19-C) by What changed, Why, Blast radius and Approvals (`gstack-approvals render`). `gstack-prose-lint --sections 'Why,Summary,Blast radius' --gate`; `gstack-issue-links`; `gstack-review-scope summary --since "$REVIEW_SINCE" --run && gstack-ship-receipt write --pr --base --gate @ --pregate --issues @ --review-loop @ --docs @ --body `; `gstack-post pr-body` and `pr-title` (a new PR via the host's PR tool; a draft while a gate finding is `open`). `SHIP_RECEIPT:` opens your report. +- Step 19 body: the receipt block, then the sections `sections/pr-body.md` lists (Pre-Landing Review: `REVIEW_LOOP:`, `Gate edits:`, deferred, notes), led under `CAPY_PR: on` (Step 19-C) by What changed, Why, Blast radius and Approvals (`gstack-approvals render`). `gstack-prose-lint --sections 'Why,Summary,Blast radius' --gate`; `gstack-issue-links`; `gstack-review-scope summary --since "$REVIEW_SINCE" --run && gstack-ship-receipt write --pr --base --gate @ --pregate --issues @ --review-loop @ --docs @ --body `; `gstack-post pr-body` and `pr-title` (a new PR via the host's PR tool; draft while a gate finding is `open`). `SHIP_RECEIPT:` opens your report. - Step 20: the metrics row via `gstack-review-log`. - Step 22: `gstack-ship-budget close --reading R --credits C` (report `BUDGET_FINAL:`, `BUDGET_TIMELINE:` (tokens, credits), any `METER:` line), then `gstack-ship-receipt result --pr `: `GSTACK_RESULT: skill=ship status=complete run=`. Under a merge queue, `gstack-ship-queue ready --pr [--screen|--no-screen]` at the pushed head; paste its output. diff --git a/ship/DRIVER.md.tmpl b/ship/DRIVER.md.tmpl index 7d4abf27d..9670902dd 100644 --- a/ship/DRIVER.md.tmpl +++ b/ship/DRIVER.md.tmpl @@ -12,7 +12,7 @@ gates and the report. `STOP:` (exit 3) means nothing after it ran; `NEXT:` names the next act. Read a section only when a phase or the list at the end sends you, never up front. A STOP or decision this file does not cover: read `ship/SKILL.md` at that step, finish it there, come back. Unattended, take a question's recommended option (log it); gate -dispositions and approvals stay pending and end the run. Keep a private invocation record (`--record`): write its six headings at Phase 1 and fill each as its phase ends: **Checks** +dispositions and approvals stay pending and end the run. Keep a private invocation record (`--record`): write its six headings at Phase 1, fill each as its phase ends: **Checks** (Phases 2-4: lanes, pregate, evals, `Coverage:`, the plan checks, `Exploratory QA:`, `Scope Check:`), **Decisions** (any phase: each answered or auto-taken question, verbatim), **Reviews** (Phases 3-5: the plan line, start tokens, `Pre-Landing Review:`, `Gate edits:`, the reviewed tree), **Release** (Phases 5-7: @@ -26,7 +26,7 @@ file, unchanged; its findings reach `merged.jsonl` only via `collect`; `judge` r small model at low effort; their findings go to the PR body. **Calls.** One pass with no fixes is 9 calls: Phases 1-8 and the report; at most 12 with a STOP's -section read or a fix-check, plus a keep-alive per 200 s waited. Fixes redo 3-4. +section read or a fix-check, plus a keep-alive per 200 s waited. **Budget: observe.** `gstack-ship-budget start --mode observe` records, never admits: each program ends with `gstack-ship-budget note --step --reading R --credits C --record `, R and C read in that program @@ -117,7 +117,7 @@ STALE or MISSING reruns once), `PREGATE:`; print them, the last `Gate edits:` an ## 8. Publish: Steps 18, 19, 19-C, 20, 21, 22 - Step 18: the open-PR lookup; a title file starting `v `. `UNATTENDED_SHIP:` with no open PR at HEAD takes Step 18-H. -- Step 19 body: the receipt block, then the sections `sections/pr-body.md` lists (Pre-Landing Review: `REVIEW_LOOP:`, `Gate edits:`, deferred, notes), led under `CAPY_PR: on` (Step 19-C) by What changed, Why, Blast radius and Approvals (`gstack-approvals render`). `gstack-prose-lint --sections 'Why,Summary,Blast radius' --gate`; `gstack-issue-links`; `gstack-review-scope summary --since "$REVIEW_SINCE" --run && gstack-ship-receipt write --pr --base --gate @ --pregate --issues @ --review-loop @ --docs @ --body `; `gstack-post pr-body` and `pr-title` (a new PR via the host's PR tool; a draft while a gate finding is `open`). `SHIP_RECEIPT:` opens your report. +- Step 19 body: the receipt block, then the sections `sections/pr-body.md` lists (Pre-Landing Review: `REVIEW_LOOP:`, `Gate edits:`, deferred, notes), led under `CAPY_PR: on` (Step 19-C) by What changed, Why, Blast radius and Approvals (`gstack-approvals render`). `gstack-prose-lint --sections 'Why,Summary,Blast radius' --gate`; `gstack-issue-links`; `gstack-review-scope summary --since "$REVIEW_SINCE" --run && gstack-ship-receipt write --pr --base --gate @ --pregate --issues @ --review-loop @ --docs @ --body `; `gstack-post pr-body` and `pr-title` (a new PR via the host's PR tool; draft while a gate finding is `open`). `SHIP_RECEIPT:` opens your report. - Step 20: the metrics row via `gstack-review-log`. - Step 22: `gstack-ship-budget close --reading R --credits C` (report `BUDGET_FINAL:`, `BUDGET_TIMELINE:` (tokens, credits), any `METER:` line), then `gstack-ship-receipt result --pr `: `GSTACK_RESULT: skill=ship status=complete run=`. Under a merge queue, `gstack-ship-queue ready --pr [--screen|--no-screen]` at the pushed head; paste its output. diff --git a/ship/sections/pr-body.md b/ship/sections/pr-body.md index 4d02514cb..79af0c5d9 100644 --- a/ship/sections/pr-body.md +++ b/ship/sections/pr-body.md @@ -233,10 +233,9 @@ When the owner accepted a stop after a fix-check edit, add and put its printed `Review-loop exception:` line in the body; never a prose line under `REVIEW_LOOP:`. The summary reads this branch's review log for the invocation's passes (its -`REVIEW_LOOP:` line is the one the Pre-Landing Review section carries). When Step 11 -rendered the targeted reader, replace `--no-ledger` with `--ledger /review-ledger.jsonl` -(a run whose passes all ran through `gstack-review-dispatch` passes `--run `), so -every reader's CRITICAL findings count. The `&&` +`REVIEW_LOOP:` line is the one the Pre-Landing Review section carries). Here every +finding reaches a row through Step 9.4; a run whose readers were collected by +`gstack-review-dispatch` (ship/DRIVER.md) passes `--run ` instead. The `&&` matters: a summary that refuses (no pass matched `REVIEW_SINCE`) must not let `write` record a stale or empty loop file. diff --git a/ship/sections/pr-body.md.tmpl b/ship/sections/pr-body.md.tmpl index 68b4c4ffc..611c1e553 100644 --- a/ship/sections/pr-body.md.tmpl +++ b/ship/sections/pr-body.md.tmpl @@ -216,10 +216,9 @@ When the owner accepted a stop after a fix-check edit, add and put its printed `Review-loop exception:` line in the body; never a prose line under `REVIEW_LOOP:`. The summary reads this branch's review log for the invocation's passes (its -`REVIEW_LOOP:` line is the one the Pre-Landing Review section carries). When Step 11 -rendered the targeted reader, replace `--no-ledger` with `--ledger /review-ledger.jsonl` -(a run whose passes all ran through `gstack-review-dispatch` passes `--run `), so -every reader's CRITICAL findings count. The `&&` +`REVIEW_LOOP:` line is the one the Pre-Landing Review section carries). Here every +finding reaches a row through Step 9.4; a run whose readers were collected by +`gstack-review-dispatch` (ship/DRIVER.md) passes `--run ` instead. The `&&` matters: a summary that refuses (no pass matched `REVIEW_SINCE`) must not let `write` record a stale or empty loop file. diff --git a/test/review-scope-ledger-count.test.ts b/test/review-scope-ledger-count.test.ts index 08b835bbd..955d57a61 100644 --- a/test/review-scope-ledger-count.test.ts +++ b/test/review-scope-ledger-count.test.ts @@ -56,21 +56,39 @@ const CLEAN = 'NO FINDINGS'; const OUTSIDE_CLEAN = 'No issues found in the diff.\nRecommendation: ship because no finding reaches the floor'; describe('GST-951: the receipt counts every blocker the readers found', () => { - // #3841 (reconstruction): the native adversarial pass's FIXABLE (CRITICAL) finding was fixed, but no row recorded it; the - // receipt's blocking_found=1 fixed=1 was entered by hand. Its two other findings were noted in pass 2. - test('#3841: an adversarial-only blocker counts', () => { + // #3841 (reconstruction; GSTA-203 PR 8 trail rows 9 and 14): Step 11's native adversarial FIXABLE (CRITICAL) finding was + // fixed in round 2, but no row recorded it (pass 2's row held no findings), so summary printed blocking_found=0 fixed=0 and + // the lane hand-set blocking_found=1 fixed=1. Its two other findings are the noted pair. + const ADVERSARIAL_3841 = [ + '1. lib/pregate-clear.ts:140 The receipt stores the raw run URL, not the attempt it judged. FIXABLE', + '2. lib/pregate.ts:88 The refusal text does not name the attempt. INVESTIGATE', + '3. docs/pregate.md:12 The docs do not say a rerun replaces the attempt. INVESTIGATE', + 'Recommendation: fix 1 because a later attempt would be judged by the first', + ].join('\n'); + test('#3841 as /ship logs it: the queued adversarial blocker in pass 2\'s row counts as 1 found and 1 fixed', () => { + const repo = branchWith([ + review(0, 1, []), + review(40, 2, [{ fingerprint: 'lib/pregate-clear.ts:140:security', action: 'fixed', raised_by: 'adversarial-native' }, + note('lib/pregate.ts:88:docs', 'adversarial-native'), note('docs/pregate.md:12:docs', 'adversarial-native')]), + ]); + const run = path.join(repo.root, 'run'); + pass(path.join(run, 'p1'), [['specialist/testing', CLEAN], ['red-team', CLEAN]]); + pass(path.join(run, 'p1-voices'), [['outside-voice', OUTSIDE_CLEAN], ['adversarial-native', ADVERSARIAL_3841]]); + pass(path.join(run, 'p2'), [['specialist/testing', CLEAN], ['red-team', CLEAN]]); + expect(loop(summary(repo, '--no-ledger'))).toMatchObject({ passes: 2, blocking_found: 0, noted: 2, fixed: 1 }); + const s = loop(summary(repo, '--run', run)); + expect(s).toMatchObject({ passes: 2, blocking_found: 1, noted: 2, fixed: 1, ledgers_read: 3, rounds: 2 }); + expect(s.unrecorded).toBeUndefined(); + }); + + test('#3841 as the lane logged it: an adversarial-only blocker counts, and its missing fix record shows as unrecorded', () => { const repo = branchWith([ review(0, 1, []), review(40, 2, [note('lib/pregate.ts:88:docs', 'adversarial-native'), note('docs/pregate.md:12:docs', 'adversarial-native')]), ]); const run = path.join(repo.root, 'run'); pass(path.join(run, 'p1'), [['specialist/testing', CLEAN], ['red-team', CLEAN]]); - pass(path.join(run, 'p1-voices'), [['outside-voice', OUTSIDE_CLEAN], ['adversarial-native', [ - '1. lib/pregate-clear.ts:140 The receipt stores the raw run URL, not the attempt it judged. FIXABLE', - '2. lib/pregate.ts:88 The refusal text does not name the attempt. INVESTIGATE', - '3. docs/pregate.md:12 The docs do not say a rerun replaces the attempt. INVESTIGATE', - 'Recommendation: fix 1 because a later attempt would be judged by the first', - ].join('\n')]]); + pass(path.join(run, 'p1-voices'), [['outside-voice', OUTSIDE_CLEAN], ['adversarial-native', ADVERSARIAL_3841]]); expect(loop(summary(repo, '--no-ledger'))).toMatchObject({ passes: 2, blocking_found: 0, noted: 2, fixed: 0 }); const s = loop(summary(repo, '--run', run)); expect(s).toMatchObject({ passes: 2, blocking_found: 1, noted: 2, fixed: 0, unrecorded: 1, ledgers_read: 2, rounds: 2 }); @@ -148,7 +166,7 @@ describe('GST-951: the receipt counts every blocker the readers found', () => { review(0, 1, [ row('src/auth.ts:42:testing', { path: 'src/auth.ts', line: 42, severity: 'CRITICAL', kind: 'test-gap', blocking: false, action: 'noted', raised_by: 'testing' }), row('src/m.ts:5:state-matrix', { path: 'src/m.ts', line: 5, severity: 'INFORMATIONAL', raw_severity: 'CRITICAL', kind: 'test-gap', blocking: false, action: 'noted', raised_by: 'state-machines' }), - row('src/f.ts:3:logic', { action: 'fixed', raised_by: 'red-team' }), + row('src/f.ts:3:logic', { action: 'fixed', raised_by: 'outside-voice' }), row('src/n.ts:8:logic', { severity: 'CRITICAL', raised_by: 'red-team' }), row('src/g.ts:7:logic', { action: 'fixed', raised_by: 'core' }), ]), @@ -164,6 +182,26 @@ describe('GST-951: the receipt counts every blocker the readers found', () => { expect(s).toMatchObject({ passes: 2, blocking_found: 3, fixed: 2, unrecorded: 1, ledgers_read: 1 }); }); + test('round 2 cases: another reviewer at a below-floor line, an extensionless path, one finding in two ledgers, no locus', () => { + const repo = branchWith([ + review(0, 1, [ + { fingerprint: 'src/auth.ts:42:testing', path: 'src/auth.ts', line: 42, severity: 'CRITICAL', blocking: false, action: 'noted', raised_by: 'testing' }, + { fingerprint: 'bin/gstack-review-scope:262:logic', path: 'bin/gstack-review-scope', line: 262, severity: 'CRITICAL', blocking: true, action: 'fixed', raised_by: 'red-team' }, + { fingerprint: 'x.ts:5:logic', path: 'x.ts', line: 5, severity: 'CRITICAL', blocking: true, action: 'fixed', raised_by: 'red-team' }, + ]), + fixCheck(30, []), + ]); + const run = path.join(repo.root, 'run'); + pass(path.join(run, 'p1'), [ + ['specialist/testing', '[CRITICAL] src/auth.ts:42 the denied path has no test'], + ['outside-voice', '1. src/auth.ts:42 an unauthenticated caller reaches the handler. Blocks: yes\nRecommendation: fix 1 because it is open'], + ['red-team', '[CRITICAL] bin/gstack-review-scope:262 the refusal is skipped\n[CRITICAL] x.ts:5 the guard is inverted'], + ]); + pass(path.join(run, 'fix-check'), [['fix-check', '[CRITICAL] x.ts:5 the guard is still inverted\n[CRITICAL] the retry loop never ends']]); + const s = loop(summary(repo, '--run', run)); + expect(s).toMatchObject({ blocking_found: 4, fixed: 2, noted: 1, unrecorded: 2, ledgers_read: 2 }); + }); + test('judgment decides: a folded duplicate counts once, a lead addition counts, a rejected judgment keeps the collected labels', () => { const repo = branchWith([review(0, 1, [])]); const run = path.join(repo.root, 'run'); diff --git a/test/size-ledger/ship/gst-951-receipt-counts-blockers.json b/test/size-ledger/ship/gst-951-receipt-counts-blockers.json index 5adfe051c..7509a6666 100644 --- a/test/size-ledger/ship/gst-951-receipt-counts-blockers.json +++ b/test/size-ledger/ship/gst-951-receipt-counts-blockers.json @@ -1,4 +1,4 @@ { - "bytes": 263, - "why": "GST-951: summary names its ledgers (--run in DRIVER, --ledger or --no-ledger in pr-body)" + "bytes": 177, + "why": "GST-951: summary names its ledgers (--run in DRIVER, --no-ledger in pr-body)" } From b593334e44099ed5a8e49f746e4472914465616e Mon Sep 17 00:00:00 2001 From: garrytan Date: Mon, 2 Nov 2026 08:10:35 +0000 Subject: [PATCH 5/5] docs: Step 14.5 audit (GST-951) Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --- docs/review-policy.md | 2 +- docs/ship-policy.md | 5 +++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/docs/review-policy.md b/docs/review-policy.md index b3660c34c..3e149828d 100644 --- a/docs/review-policy.md +++ b/docs/review-policy.md @@ -604,7 +604,7 @@ ones, and the arm lost blockers. `bin/gstack-review-dispatch` closes both ends: - `ship/sections/review-army.md` Step 9: the plan's `REVIEW_DEPTH`, `GATE_SCAN` and `--depth-floor` on every run, the depth fields of the review-log row, passes, `RANGE_BASE`, classify, defer, render, the cap check before any edit, the Step 11 late-defer row, scratch files under `.gstack/tmp/`, the cap decisions; `ship/sections/adversarial.md` Step 11: the outside voice reads the same range and scopes queued fixes the same way. - `lib/review-depth.ts`, `bin/gstack-review-plan`: the diff class, the depth (`depthFor`, `DEPTH_RULES`, `depthOfCommitInput`), each dispatch's `exclude` pathspecs and `DIFF_CMD`, the transition table, the numeric-claim scan and each role's effort tier (`test/review-depth.test.ts`, `test/review-plan-cli.test.ts`). - `lib/review-collect.ts`: `collectCommit` (git objects only, from any checkout) and `collectLive` (the working tree), per-path lines and the base commit's registry (`test/review-collect.test.ts`). -- `lib/review-scope.ts`, `bin/gstack-review-scope`: `classify`, `defer`, `render`, `summary` (with `fix_checks`, `dispatches`, `est_input_tokens`); refusals are `REVIEW_SCOPE_INPUT` and `REVIEW_SCOPE_RANGE` ([troubleshooting.md](troubleshooting.md#review-scope-input)). +- `lib/review-scope.ts`, `bin/gstack-review-scope`: `classify`, `defer`, `render`, `summary` (with `fix_checks`, `dispatches`, `est_input_tokens`, `unrecorded`, `ledgers_read`); refusals are `REVIEW_SCOPE_INPUT`, `REVIEW_SCOPE_RANGE` and `REVIEW_SCOPE_LEDGER` ([troubleshooting.md](troubleshooting.md#review-scope-input)). - `lib/ship-policy.ts`: the `review_scope` key and its `ship: review_scope …` consequence line. - `review/checklist.md`: `prereg-with-results` under Gate Integrity and `test-keyboard-count-loop` under Pass 2; `bin/gstack-coauthor-trailers` for the contributor-credit trailers of a port. - `lib/ship-receipt-prose.ts`, `gstack-ship-receipt read --check-prose`: the body's quoted counters against the receipt (`RECEIPT_PROSE_MISMATCH`, [ship-policy.md](ship-policy.md#ship-receipt)). diff --git a/docs/ship-policy.md b/docs/ship-policy.md index b0c58c716..1e08417be 100644 --- a/docs/ship-policy.md +++ b/docs/ship-policy.md @@ -1286,8 +1286,9 @@ instead of `defer`. `review_scope: all` is the opt-back to the earlier loop: every finding is `scope: fix` on every pass, reviewers read the whole diff each time, and only the three-cycle fix cap stops the loop. `show-effective` prints the choice as a `ship: review_scope …` line, and the codes a refusal ends with are -[`REVIEW_SCOPE_INPUT`](troubleshooting.md#review-scope-input) and -[`REVIEW_SCOPE_RANGE`](troubleshooting.md#review-scope-range). +[`REVIEW_SCOPE_INPUT`](troubleshooting.md#review-scope-input), +[`REVIEW_SCOPE_RANGE`](troubleshooting.md#review-scope-range) and +[`REVIEW_SCOPE_LEDGER`](troubleshooting.md#review-scope-ledger). ## `base_red`: pausing /ship on a red base