fix(review): fix-range narrows only DIFF_BASE outside Step 4.8; Step 5.0 ignores this invocation's records (#3339)

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

CHANGELOG.md

@@ -2,19 +2,29 @@
 
 ## [Unreleased]
 
-**/review can re-review just its own fixes after a fixing pass, under the same rules as /ship (#3339).** Before, every /review fix cycle reran the whole review over the whole branch, so a large branch could hit the three-cycle stop on findings in code the earlier passes had already read. Now the pass after an edited pass starts with `gstack-review-log --start review --fix-range` and reviews only what changed since that pass. A narrowed pass is never the final review: when it makes no edits, /review runs one full pass before writing the final record, and a fix-range record is never CURRENT. Garry's "do it" on 2026-10-24 for a fix-range re-review with a final full pass covers it.
+**/review can re-review just its own fixes after a fixing pass, under the same rules as /ship (#3339).** Before, every /review fix cycle reran the whole review over the whole branch, so a large branch could hit the three-cycle stop on findings in code the earlier passes had already read. Now the pass after an edited pass starts with `gstack-review-log --start review --fix-range` and its checklist, specialists and QA read only what changed since that pass. The Step 4.8 adversarial review still reads the whole branch on every pass, because its rows carry their own freshness. A narrowed pass is never the final review: when it makes no edits, /review runs one full pass before writing the final record, and a fix-range record is never CURRENT. Garry's "do it" on 2026-10-24 for a fix-range re-review with a final full pass covers it.
 
 ### Changed
-- **`review/SKILL.md.tmpl`** Step 3: the first pass always starts full. The pass right after an edited pass starts with `--start review --fix-range`, and the printed `FIX_BASE=<sha>` replaces DIFF_BASE, `origin/<base>` and `<base>` for that pass. If the logger refuses (for example, the first pass read uncommitted work, so no commit marks what it read), the pass starts full.
-- **Step 5.8**: an edited pass now saves its own record with `converged:false` before the repeat, because the fix-range start takes its base from that record. Before, only the final pass was logged. "No edits" fills the final record only after a full pass; after a fix-range pass it repeats Steps 3–5 with a full start. Each logged pass finishes its own REVIEW_START.
-- Nearby prose was tightened to pay for the growth (the slop-scan note, the Step 4.7 probe-reuse rule, the final-report and Greptile headers). Every host's review SKILL.md shrank by 28 characters and its size pin was lowered to match; the Claude skeleton is 80,589 bytes against its 80,900 cap.
+- **`review/SKILL.md.tmpl`** Step 3: the first pass always starts full. The pass right after an edited pass starts with `--start review --fix-range`, and wherever that pass sets DIFF_BASE outside Step 4.8 it uses the printed `FIX_BASE=<sha>` instead. Step 4.8 and every other `<base>` use (state-matrix detection, the gate, banned-terms and slop scans, the queue check) keep the base branch. State-matrix detection builds `origin/<base>` from its argument, and the slop scan diffs `<base>...HEAD`, which is empty while /review's fixes are uncommitted. If the logger refuses (for example, the first pass read uncommitted work, so no commit marks what it read), the pass starts full.
+- **Step 5.8**: an edited pass now saves its own record with `converged:false` before the repeat, because the fix-range start takes its base from that record. Before, only the final pass was logged. Step 5.0 never treats this invocation's own records as prior reviews, so a defect the user skipped this invocation is not suppressed on the next pass and still counts as unresolved. "No edits" fills the final record only after a full pass; after a fix-range pass it repeats Steps 3–5 with a full start. Each logged pass finishes its own REVIEW_START.
+- 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 every base, a refusal falls back to a full start, and the edited pass is logged before the repeat. Three run /review's real order (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 pass is UNVERIFIED; and 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.
+- `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-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
-- One gate change to review: /review may narrow a repeat pass. Verdict: not a weakening of what the final review covers, because a narrowed pass can't be the final record. The logger never marks a fix-range row CURRENT, and Step 5.8 requires a full pass after a zero-edit fix-range pass. Mutation checks on the rendered skill, each caught by the suite: letting a fix-range pass be final, dropping the full-pass condition on "No edits", narrowing every repeat, dropping the refusal fallback, narrowing the first pass, not logging the edited pass, not replacing `origin/<base>`/`<base>`, and logging after the repeat. No threshold, budget, timeout, trial count or case kind changed; size pins only went down.
+- One gate change to review: /review may narrow a repeat pass. Verdict: this does not weaken what the final review covers, because a narrowed pass can't be the final record. The logger never marks a fix-range row CURRENT, Step 5.8 requires a full pass after a zero-edit fix-range pass, Step 4.8 stays full, and Step 5.0 can't suppress this invocation's skipped defects through its own intermediate records. Mutation checks on the rendered skill, each caught by the suite:
+  - letting a fix-range pass be final;
+  - dropping the full-pass condition on "No edits";
+  - narrowing every repeat;
+  - dropping the refusal fallback;
+  - narrowing the first pass;
+  - not logging the edited pass;
+  - logging after the repeat;
+  - narrowing Step 4.8 and other base uses;
+  - letting Step 5.0 read this invocation's records.
+- No threshold, budget, timeout, trial count or case kind changed. Size pins only went down.
 
 ## [1.91.205.0] - 2026-10-25
 

review/SKILL.md

@@ -507,8 +507,9 @@ git diff "$DIFF_BASE"
 1. Save the printed REVIEW_START for this core candidate before reading its diff.
 2. Each re-review captures a new token before reading, never at log time. After an
    edited pass, start the next with `--start review --fix-range`: it prints
-   REVIEW_START, then `FIX_BASE=<sha>`, which replaces DIFF_BASE, `origin/<base>` and
-   `<base>` in this pass. On refusal, start a full pass.
+   REVIEW_START, then `FIX_BASE=<sha>`. Wherever this pass sets DIFF_BASE outside
+   Step 4.8, use FIX_BASE instead; Step 4.8 and every other `<base>` use stay full.
+   On refusal, start a full pass.
 3. Native/outside reviewer attempts own separate PASS_START tokens, not REVIEW_START.
 4. Read non-ignored untracked source too (`git ls-files --others --exclude-standard`);
    the captured candidate includes it.
@@ -542,13 +543,13 @@ OFFLINE=$(echo "$QUEUE_JSON" | jq -r '.offline // false')
 - `OFFLINE=true`: skip this section.
 - Otherwise, include ONE line in the review output: `Version claimed: v<BRANCH_VERSION>. Queue: <CLAIMED_COUNT> PR(s) ahead. <VERDICT>` where VERDICT is either `Slot free` (if `BRANCH_VERSION >= NEXT_SLOT`) or `⚠ queue moved — rerun /ship to reconcile v<BRANCH_VERSION> → v<NEXT_SLOT>`.
 
-Compare dotted version components as integers from left to right; missing trailing components count as zero.
+Compare versions numerically per dotted component; missing ones count as zero.
 
 ---
 
 ## Step 3.5: Diff scans
 
-Run the gate scan first (local, seconds; every diff size), then the slop scan.
+Run the gate scan first (local; every diff size), then the slop scan.
 
 ### Gate scan
 
@@ -718,10 +719,8 @@ a clean result. Gate findings are ASK in Step 5 regardless of how mechanical the
 revert looks. A test-only flake fix (timeout, retry, looser assertion, skip) states `P(fail | regression)` before and after, from a forced probe; without both numbers it is RH-15.
 
 **Search-before-recommending:** Research proposed fixes through Aside, especially
-concurrency, caching, auth and framework behavior:
-- Check current best practice for the installed framework version.
-- Look for a newer built-in before proposing a workaround.
-- Verify API signatures against current docs.
+concurrency, caching, auth and framework behavior: current best practice for the
+installed version, newer built-ins that replace a workaround, and current API signatures.
 
 Prompt file text (create, write and send it with the Web research runs in Aside blocks above): `{framework} {version} {pattern} current best practice and whether a built-in replaces it. Reply with up to 5 bullets, each with its source URL.`
 
@@ -1066,7 +1065,7 @@ Before replying to any comment, run greptile-triage.md's **Escalation Detection*
 
 3. **VALID BUT ALREADY FIXED comments:** Reply with greptile-triage.md's **Already Fixed reply template**, no AskUserQuestion: what was done and the fixing commit SHA; save to both per-project and global greptile-history.
 
-4. **SUPPRESSED comments:** Skip silently (known false positives from previous triage).
+4. **SUPPRESSED comments:** Skip silently.
 
 ---
 
@@ -1076,7 +1075,8 @@ Before replying to any comment, run greptile-triage.md's **Escalation Detection*
 
 1. A pass covers Steps 3–5, including all reviewers before fixes. Allow at most 3 fix cycles:
    - Edited: increment CYCLES once, then fill and save the record below with
-     `converged:false`. Below 3, repeat Steps 3–5 with a new REVIEW_START. At 3, report
+     `converged:false`. Step 5.0 never treats this invocation's records as prior
+     reviews. Below 3, repeat Steps 3–5 with a new REVIEW_START. At 3, report
      nonconvergence and coverage gaps, then STOP this invocation, without a clean
      summary or a fourth pass.
    - No edits: after a full pass, fill the record below. A fix-range pass is never

review/SKILL.md.tmpl

@@ -89,8 +89,9 @@ git diff "$DIFF_BASE"
 1. Save the printed REVIEW_START for this core candidate before reading its diff.
 2. Each re-review captures a new token before reading, never at log time. After an
    edited pass, start the next with `--start review --fix-range`: it prints
-   REVIEW_START, then `FIX_BASE=<sha>`, which replaces DIFF_BASE, `origin/<base>` and
-   `<base>` in this pass. On refusal, start a full pass.
+   REVIEW_START, then `FIX_BASE=<sha>`. Wherever this pass sets DIFF_BASE outside
+   Step 4.8, use FIX_BASE instead; Step 4.8 and every other `<base>` use stay full.
+   On refusal, start a full pass.
 3. Native/outside reviewer attempts own separate PASS_START tokens, not REVIEW_START.
 4. Read non-ignored untracked source too (`git ls-files --others --exclude-standard`);
    the captured candidate includes it.
@@ -124,13 +125,13 @@ OFFLINE=$(echo "$QUEUE_JSON" | jq -r '.offline // false')
 - `OFFLINE=true`: skip this section.
 - Otherwise, include ONE line in the review output: `Version claimed: v<BRANCH_VERSION>. Queue: <CLAIMED_COUNT> PR(s) ahead. <VERDICT>` where VERDICT is either `Slot free` (if `BRANCH_VERSION >= NEXT_SLOT`) or `⚠ queue moved — rerun /ship to reconcile v<BRANCH_VERSION> → v<NEXT_SLOT>`.
 
-Compare dotted version components as integers from left to right; missing trailing components count as zero.
+Compare versions numerically per dotted component; missing ones count as zero.
 
 ---
 
 ## Step 3.5: Diff scans
 
-Run the gate scan first (local, seconds; every diff size), then the slop scan.
+Run the gate scan first (local; every diff size), then the slop scan.
 
 ### Gate scan
 
@@ -179,10 +180,8 @@ a clean result. Gate findings are ASK in Step 5 regardless of how mechanical the
 revert looks. {{FLAKE_FIX_NUMBER:review}}
 
 **Search-before-recommending:** Research proposed fixes through Aside, especially
-concurrency, caching, auth and framework behavior:
-- Check current best practice for the installed framework version.
-- Look for a newer built-in before proposing a workaround.
-- Verify API signatures against current docs.
+concurrency, caching, auth and framework behavior: current best practice for the
+installed version, newer built-ins that replace a workaround, and current API signatures.
 
 Prompt file text (create, write and send it with the Web research runs in Aside blocks above): `{framework} {version} {pattern} current best practice and whether a built-in replaces it. Reply with up to 5 bullets, each with its source URL.`
 
@@ -366,7 +365,7 @@ Before replying to any comment, run greptile-triage.md's **Escalation Detection*
 
 3. **VALID BUT ALREADY FIXED comments:** Reply with greptile-triage.md's **Already Fixed reply template**, no AskUserQuestion: what was done and the fixing commit SHA; save to both per-project and global greptile-history.
 
-4. **SUPPRESSED comments:** Skip silently (known false positives from previous triage).
+4. **SUPPRESSED comments:** Skip silently.
 
 ---
 
@@ -376,7 +375,8 @@ Before replying to any comment, run greptile-triage.md's **Escalation Detection*
 
 1. A pass covers Steps 3–5, including all reviewers before fixes. Allow at most 3 fix cycles:
    - Edited: increment CYCLES once, then fill and save the record below with
-     `converged:false`. Below 3, repeat Steps 3–5 with a new REVIEW_START. At 3, report
+     `converged:false`. Step 5.0 never treats this invocation's records as prior
+     reviews. Below 3, repeat Steps 3–5 with a new REVIEW_START. At 3, report
      nonconvergence and coverage gaps, then STOP this invocation, without a clean
      summary or a fourth pass.
    - No edits: after a full pass, fill the record below. A fix-range pass is never

test/fixtures/skill-md-size-allowlist.json

@@ -9,7 +9,7 @@
     "agy/gstack-plan-design-review": 140013,
     "agy/gstack-plan-devex-review": 135760,
     "agy/gstack-plan-eng-review": 152626,
-    "agy/gstack-review": 124190,
+    "agy/gstack-review": 124182,
     "claude/design-review": 136531,
     "codex/gstack-autoplan": 130628,
     "codex/gstack-design-consultation": 109452,
@@ -19,7 +19,7 @@
     "codex/gstack-plan-design-review": 142466,
     "codex/gstack-plan-devex-review": 138649,
     "codex/gstack-plan-eng-review": 155172,
-    "codex/gstack-review": 125094,
+    "codex/gstack-review": 125086,
     "copilot/gstack-autoplan": 131637,
     "copilot/gstack-codex": 100235,
     "copilot/gstack-design-consultation": 109536,
@@ -29,7 +29,7 @@
     "copilot/gstack-plan-design-review": 140884,
     "copilot/gstack-plan-devex-review": 136912,
     "copilot/gstack-plan-eng-review": 153351,
-    "copilot/gstack-review": 125461,
+    "copilot/gstack-review": 125453,
     "cursor/gstack-autoplan": 129278,
     "cursor/gstack-design-consultation": 107222,
     "cursor/gstack-design-review": 142507,
@@ -37,7 +37,7 @@
     "cursor/gstack-plan-design-review": 138968,
     "cursor/gstack-plan-devex-review": 134591,
     "cursor/gstack-plan-eng-review": 151480,
-    "cursor/gstack-review": 140722,
+    "cursor/gstack-review": 140714,
     "factory/gstack-autoplan": 131562,
     "factory/gstack-codex": 100655,
     "factory/gstack-design-consultation": 108670,
@@ -47,7 +47,7 @@
     "factory/gstack-plan-design-review": 141749,
     "factory/gstack-plan-devex-review": 137960,
     "factory/gstack-plan-eng-review": 154594,
-    "factory/gstack-review": 144501,
+    "factory/gstack-review": 144493,
     "gbrain/gstack-autoplan": 129815,
     "gbrain/gstack-design-review": 132842,
     "gbrain/gstack-land-and-deploy": 117167,
@@ -67,7 +67,7 @@
     "kiro/gstack-plan-design-review": 138808,
     "kiro/gstack-plan-devex-review": 134431,
     "kiro/gstack-plan-eng-review": 151330,
-    "kiro/gstack-review": 140534,
+    "kiro/gstack-review": 140526,
     "openclaw/gstack-autoplan": 129520,
     "openclaw/gstack-design-review": 131647,
     "openclaw/gstack-land-and-deploy": 116757,
@@ -82,7 +82,7 @@
     "opencode/gstack-plan-design-review": 140738,
     "opencode/gstack-plan-devex-review": 136812,
     "opencode/gstack-plan-eng-review": 153232,
-    "opencode/gstack-review": 142957,
+    "opencode/gstack-review": 142949,
     "slate/gstack-autoplan": 129159,
     "slate/gstack-design-consultation": 107143,
     "slate/gstack-design-review": 142414,
@@ -90,6 +90,6 @@
     "slate/gstack-plan-design-review": 138888,
     "slate/gstack-plan-devex-review": 134511,
     "slate/gstack-plan-eng-review": 151405,
-    "slate/gstack-review": 140628
+    "slate/gstack-review": 140620
   }
 }

test/review-fix-range.test.ts

@@ -221,16 +221,16 @@ describe('review Step 3 and Step 5.8 instructions', () => {
   const step58 = skill.slice(skill.indexOf('## Step 5.8: Persist Eng Review result'), skill.indexOf('### Report the final review'));
   const flat3 = step3.replace(/\s+/g, ' '), flat58 = step58.replace(/\s+/g, ' ');
 
-  test('the first pass starts full and only the pass after an edited pass narrows, with FIX_BASE replacing every base', () => {
+  test('the first pass starts full and only the pass after an edited pass narrows its DIFF_BASE, never Step 4.8 or other base uses', () => {
     const block = step3.slice(step3.indexOf('```bash'), step3.indexOf('```', step3.indexOf('```bash') + 7));
     expect(block).toContain('gstack-review-log --start review\ngit diff "$DIFF_BASE"');
     expect(block).not.toContain('--fix-range');
-    expect(flat3).toContain('After an edited pass, start the next with `--start review --fix-range`: it prints REVIEW_START, then `FIX_BASE=<sha>`, which replaces DIFF_BASE, `origin/<base>` and `<base>` in this pass. On refusal, start a full pass.');
+    expect(flat3).toContain('After an edited pass, start the next with `--start review --fix-range`: it prints REVIEW_START, then `FIX_BASE=<sha>`. Wherever this pass sets DIFF_BASE outside Step 4.8, use FIX_BASE instead; Step 4.8 and every other `<base>` use stay full. On refusal, start a full pass.');
     expect(skill.match(/--fix-range/g)).toHaveLength(1);
   });
 
-  test('an edited pass is logged before the repeat, and a fix-range pass is never the final review', () => {
-    const edited = flat58.indexOf('Edited: increment CYCLES once, then fill and save the record below with `converged:false`.');
+  test('an edited pass is logged before the repeat, kept out of Step 5.0, and a fix-range pass is never the final review', () => {
+    const edited = flat58.indexOf("Edited: increment CYCLES once, then fill and save the record below with `converged:false`. Step 5.0 never treats this invocation's records as prior reviews.");
     const repeat = flat58.indexOf('Below 3, repeat Steps 3–5 with a new REVIEW_START.');
     const none = flat58.indexOf('No edits: after a full pass, fill the record below. A fix-range pass is never final: repeat Steps 3–5 with a full start.');
     expect(edited).toBeGreaterThan(-1);
@@ -254,6 +254,9 @@ 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');
@@ -266,7 +269,7 @@ describe('review Step 3/5.8 order against the real logger', () => {
     expect(reviewFreshness(last, last.wtree)?.status).toBe('CURRENT');
   });
 
-  test('a zero-edit fix-range pass is never CURRENT, so the final record needs a full pass', () => {
+  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');
     reviewPass(clean, ['--start', 'review', '--fix-range']);
     const narrowed = rows().at(-1)!;

test/review-workflow-clarity.test.ts

@@ -249,7 +249,7 @@ test('review finalization ownership: late findings use Fix-First before the boun
   expect(positions.every(position => position >= 0)).toBe(true);
   expect(positions).toEqual([...positions].sort((a, b) => a - b));
   expect(flat).toContain('at most 3 fix cycles');
-  expect(flat).toContain('Edited: increment CYCLES once, then fill and save the record below with `converged:false`. Below 3,');
+  expect(flat).toContain('Edited: increment CYCLES once, then fill and save the record below with `converged:false`.');
   const limit = flat.slice(flat.indexOf('At 3,'), flat.indexOf('No edits:'));
   expect(limit).toMatch(/then stop this invocation/i);
   expectMentions(limit, [['without', 'summary', 'fourth']], 'limit');