From ca2bc785e028b6193ac75ce2f685b6fb494f1e88 Mon Sep 17 00:00:00 2001 From: garrytan Date: Sun, 25 Oct 2026 11:33:20 +0000 Subject: [PATCH] test(review): fix-range walk logs each pass's Step 4.8 attempt in /review's real order (#3339) Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --- CHANGELOG.md | 2 +- test/review-fix-range.test.ts | 15 ++++++++------- 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b9e8dc95d..aa72eec8c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,7 +10,7 @@ - Nearby prose was tightened to pay for the growth (the slop-scan note, the Step 4.7 probe-reuse rule, the search-before-recommending list, the version-compare note, the final-report and Greptile lines). Every host's review SKILL.md shrank by 36 characters, and its size pin was lowered to match. The Claude skeleton is 80,581 bytes against its 80,900 cap. ### Tests -- `test/review-fix-range.test.ts` adds five cases. Two check the rendered rules: the first pass's start block has no `--fix-range`, only the pass after an edited pass narrows, FIX_BASE replaces only DIFF_BASE outside Step 4.8, a refusal falls back to a full start, the edited pass is logged before the repeat and kept out of Step 5.0, and a fix-range pass is never final. Three run /review's real order (Step 4.8's attempt row, then start, fix in the working tree, log with the pass's own token, never commit) against the real logger. A committed first pass narrows the next repeat to the uncommitted fixes, and the following repeat is refused back to a full pass that is CURRENT. A zero-edit fix-range row is UNVERIFIED even if it were logged. A first pass over uncommitted work can't be narrowed. +- `test/review-fix-range.test.ts` adds five cases. Two check the rendered rules: the first pass's start block has no `--fix-range`, only the pass after an edited pass narrows, FIX_BASE replaces only DIFF_BASE outside Step 4.8, a refusal falls back to a full start, the edited pass is logged before the repeat and kept out of Step 5.0, and a fix-range pass is never final. Three run /review's real order on every pass against the real logger: core start, then the Step 4.8 attempt's start and finish, then fixes in the working tree, then the core finish with the pass's own token, and never a commit. A committed first pass narrows the next repeat to the uncommitted fixes, and the following repeat is refused back to a full pass that is CURRENT. A zero-edit fix-range row is UNVERIFIED even if it were logged. A first pass over uncommitted work can't be narrowed. - `test/review-workflow-clarity.test.ts`: three expectations follow the new contract: the "No edits" marker, the edited pass saving its record with `converged:false`, and each pass finishing its own token instead of only the final one. The test formerly named "finish only the final core token without log-time capture" is now "each logged pass finishes its own core token without log-time capture". It still requires one `--finish REVIEW_START`, no `--start review` in Step 5.8 and no log-time token capture. ### Gate integrity diff --git a/test/review-fix-range.test.ts b/test/review-fix-range.test.ts index 6f6d2a05e..757b6be1c 100644 --- a/test/review-fix-range.test.ts +++ b/test/review-fix-range.test.ts @@ -241,12 +241,16 @@ describe('review Step 3 and Step 5.8 instructions', () => { }); describe('review Step 3/5.8 order against the real logger', () => { - // /review never commits: each pass starts, applies fixes in the working tree and logs with its own token. + // /review's order on every pass: core start, Step 4.8 attempt start and finish, fixes in the working tree, core finish; never a commit. const reviewRow = (fields: Record) => JSON.stringify({ skill: 'review', status: 'clean', issues_found: 0, critical: 0, informational: 0, ...fields }); function reviewPass(fields: Record, start: string[], fixes?: string) { const started = bin('gstack-review-log', start); expect(started.code).toBe(0); const [token, base] = started.out.split('\n'); + // Step 4.8: each pass's adversarial attempt saves its row before any fix and before the core row. + const attempt = bin('gstack-review-log', ['--start', 'adversarial-review']); + expect(attempt.code).toBe(0); + expect(bin('gstack-review-log', [JSON.stringify({ skill: 'adversarial-review', status: 'clean', source: 'claude', ...clean }), '--finish', attempt.out]).code).toBe(0); if (fixes) edit(fixes); expect(bin('gstack-review-log', [reviewRow(fields), '--finish', token!]).code).toBe(0); return base; @@ -254,9 +258,6 @@ describe('review Step 3/5.8 order against the real logger', () => { test('a committed first pass narrows the next repeat to the uncommitted fixes, then the chain returns to a full pass', () => { const reviewed = head(); - // Step 4.8 saves its attempt row before fixes, so it precedes the core row and never blocks the narrowed start. - const attempt = bin('gstack-review-log', ['--start', 'adversarial-review']).out; - expect(bin('gstack-review-log', [JSON.stringify({ skill: 'adversarial-review', status: 'clean', source: 'claude', ...clean }), '--finish', attempt]).code).toBe(0); reviewPass(fixing, ['--start', 'review'], 'export const a = 2;\n'); expect(reviewPass(fixing, ['--start', 'review', '--fix-range'], 'export const a = 3;\n')).toBe(`FIX_BASE=${reviewed}`); expect(gitIn(repo, `diff --name-only ${reviewed}`).trim()).toBe('a.ts'); @@ -267,7 +268,7 @@ describe('review Step 3/5.8 order against the real logger', () => { const last = rows().at(-1)!; expect(last.scope).toBeUndefined(); expect(reviewFreshness(last, last.wtree)?.status).toBe('CURRENT'); - }); + }, 30_000); test('a zero-edit fix-range pass is never CURRENT even if it were logged, so the final record needs a full pass', () => { reviewPass(fixing, ['--start', 'review'], 'export const a = 2;\n'); @@ -275,7 +276,7 @@ describe('review Step 3/5.8 order against the real logger', () => { const narrowed = rows().at(-1)!; expect(narrowed.scope).toBe('fix-range'); expect(reviewFreshness(narrowed, narrowed.review_binding.range_end_wtree)?.status).toBe('UNVERIFIED'); - }); + }, 30_000); test('a first pass over uncommitted work cannot be narrowed', () => { edit('export const a = 1; // uncommitted\n'); @@ -283,5 +284,5 @@ describe('review Step 3/5.8 order against the real logger', () => { const r = narrowStart(); expect(r.code).toBe(1); expect(r.err).toContain('started from uncommitted content'); - }); + }, 30_000); });