mirror of
https://github.com/garrytan/gstack.git
synced 2026-11-05 08:23:59 +01:00
feat(review): /review re-reviews only its fixes after an edited pass, with a final full pass (#3339)
Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>
This commit is contained in:
garrytanandcapy-ai[bot] committed
1 parent
072d073b12
commit
bd5f6ec865
6 files changed
+135
-52
No files matched your search
@@ -1,5 +1,21 @@
|
||||
# Changelog
|
||||
|
||||
## [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.
|
||||
|
||||
### 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.
|
||||
|
||||
### 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.
|
||||
|
||||
### 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.
|
||||
|
||||
## [1.91.205.0] - 2026-10-25
|
||||
|
||||
**/ship's Step 9 can re-review just the fixes after a fixing pass, and a narrowed pass can never be the review that ships (#3339).** Before, every fix cycle reran every specialist and the Red Team over the whole branch. On a large branch each full pass tends to find something new in code earlier passes already read, so the loop often ran to the three-cycle stop without converging. Now the pass after a fixing pass can review only what changed since that pass. One full pass still has to come last: a fix-range record is never CURRENT, and Step 11.5 can't bind it. Garry approved this on 2026-10-24 ("only with a mandatory final full pass").
|
||||
|
||||
+18
-19
@@ -505,8 +505,10 @@ 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. Earlier
|
||||
core tokens remain unused; Step 5.8 finishes only the final core token.
|
||||
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.
|
||||
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.
|
||||
@@ -589,14 +591,11 @@ category, Step 5.8's record and the final report.
|
||||
|
||||
### Slop scan (advisory)
|
||||
|
||||
Scan changed files for empty catches, redundant `return await` and needless abstractions:
|
||||
|
||||
```bash
|
||||
bun run slop:diff origin/<base> 2>/dev/null || true
|
||||
```
|
||||
|
||||
Include findings as non-blocking informational diagnostics. If slop:diff is
|
||||
unavailable, skip silently.
|
||||
Findings are non-blocking informational diagnostics; skip if unavailable.
|
||||
|
||||
---
|
||||
|
||||
@@ -1052,7 +1051,7 @@ Verify claims or flag them as unknown; "this looks fine" is not evidence.
|
||||
|
||||
After outputting your own findings, if Greptile comments were classified in Step 2.5:
|
||||
|
||||
**Include a Greptile summary in your output header:** `+ N Greptile comments (X valid, Y fixed, Z FP)`
|
||||
**Greptile summary in your output header:** `+ N Greptile comments (X valid, Y fixed, Z FP)`
|
||||
|
||||
Before replying to any comment, run greptile-triage.md's **Escalation Detection** to choose Tier 1 (friendly) or Tier 2 (firm) reply templates.
|
||||
|
||||
@@ -1076,16 +1075,16 @@ Before replying to any comment, run greptile-triage.md's **Escalation Detection*
|
||||
### 1. Re-review after edits
|
||||
|
||||
1. A pass covers Steps 3–5, including all reviewers before fixes. Allow at most 3 fix cycles:
|
||||
- Edited: increment CYCLES once. Below 3, repeat Steps 3–5 with a new
|
||||
REVIEW_START. At 3, persist `converged:false` and remaining findings by filling
|
||||
and saving the record below. Report nonconvergence and coverage gaps, then STOP
|
||||
this invocation, without a clean summary or a fourth pass.
|
||||
- No edits: fill the record below.
|
||||
- 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
|
||||
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
|
||||
final: repeat Steps 3–5 with a full start.
|
||||
2. On a repeat, execute Steps 3–5 in order. At Step 4.7, reuse only this invocation's
|
||||
unchanged-input QA evidence; rerun affected probes after source, test, contract,
|
||||
command or fixture changes. Reusing a probe never skips a review step.
|
||||
A probe is affected when its entrypoint, dependencies, contract or replay inputs
|
||||
change; if uncertain, rerun it.
|
||||
unchanged-input QA evidence; rerun a probe if its source, tests, entrypoint,
|
||||
dependencies, contract, command, fixture or replay inputs changed, or if uncertain.
|
||||
Reusing a probe never skips a review step.
|
||||
3. **Verify completed actions.** On the final zero-edit pass, reconcile this
|
||||
invocation's actions with current findings. Deduplicate by structural identity
|
||||
and advisory/defect kind. For a completed extraction, retain `fixed` and the
|
||||
@@ -1121,7 +1120,7 @@ pass: report it with its reason.
|
||||
|
||||
- Use Step 4.6's `specialists` object unchanged, including its empty small-diff map.
|
||||
If this host omits Review Army, use `specialists: {}` without claiming specialist coverage.
|
||||
- Build `findings` from Step 5's combined final-pass findings (core, specialist,
|
||||
- Build `findings` from Step 5's combined findings (core, specialist,
|
||||
adversarial, actionable Greptile, verified exploratory QA findings) and invocation actions. Retain `fingerprint`, `severity`
|
||||
(`CRITICAL|INFORMATIONAL`), `action`, and any `advisory`, `evidence_paths`,
|
||||
`helper_target`. Recheck source after fixes. The logger uses `sharedLibsFingerprint`,
|
||||
@@ -1149,11 +1148,11 @@ were skipped or this host omits Review Army). This default is not completion evi
|
||||
unresolved non-advisory core defects still count in `issues_found`,
|
||||
`critical`, `informational`. The logger builds trusted `review_binding` from the
|
||||
validated captured branch digest, discarding caller bindings. Never invent a binding
|
||||
or replace REVIEW_START at log time; finish only the final core token.
|
||||
or replace REVIEW_START at log time; finish only this pass's own token.
|
||||
|
||||
### Report the final review
|
||||
|
||||
Emit one final report, merging all reviewers rather than concatenating them:
|
||||
Emit one final report merging all reviewers:
|
||||
1. `Pre-Landing Review: N issues (X critical, Y informational)` counts final unresolved
|
||||
non-advisory defects. State INCOMPLETE if `COMPLETED` is false, even when N=0.
|
||||
2. Use the checklist's action groups with confidence-tagged finding lines. Keep fixed,
|
||||
|
||||
+18
-19
@@ -87,8 +87,10 @@ 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. Earlier
|
||||
core tokens remain unused; Step 5.8 finishes only the final core token.
|
||||
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.
|
||||
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.
|
||||
@@ -143,14 +145,11 @@ category, Step 5.8's record and the final report.
|
||||
|
||||
### Slop scan (advisory)
|
||||
|
||||
Scan changed files for empty catches, redundant `return await` and needless abstractions:
|
||||
|
||||
```bash
|
||||
bun run slop:diff origin/<base> 2>/dev/null || true
|
||||
```
|
||||
|
||||
Include findings as non-blocking informational diagnostics. If slop:diff is
|
||||
unavailable, skip silently.
|
||||
Findings are non-blocking informational diagnostics; skip if unavailable.
|
||||
|
||||
---
|
||||
|
||||
@@ -352,7 +351,7 @@ Verify claims or flag them as unknown; "this looks fine" is not evidence.
|
||||
|
||||
After outputting your own findings, if Greptile comments were classified in Step 2.5:
|
||||
|
||||
**Include a Greptile summary in your output header:** `+ N Greptile comments (X valid, Y fixed, Z FP)`
|
||||
**Greptile summary in your output header:** `+ N Greptile comments (X valid, Y fixed, Z FP)`
|
||||
|
||||
Before replying to any comment, run greptile-triage.md's **Escalation Detection** to choose Tier 1 (friendly) or Tier 2 (firm) reply templates.
|
||||
|
||||
@@ -376,16 +375,16 @@ Before replying to any comment, run greptile-triage.md's **Escalation Detection*
|
||||
### 1. Re-review after edits
|
||||
|
||||
1. A pass covers Steps 3–5, including all reviewers before fixes. Allow at most 3 fix cycles:
|
||||
- Edited: increment CYCLES once. Below 3, repeat Steps 3–5 with a new
|
||||
REVIEW_START. At 3, persist `converged:false` and remaining findings by filling
|
||||
and saving the record below. Report nonconvergence and coverage gaps, then STOP
|
||||
this invocation, without a clean summary or a fourth pass.
|
||||
- No edits: fill the record below.
|
||||
- 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
|
||||
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
|
||||
final: repeat Steps 3–5 with a full start.
|
||||
2. On a repeat, execute Steps 3–5 in order. At Step 4.7, reuse only this invocation's
|
||||
unchanged-input QA evidence; rerun affected probes after source, test, contract,
|
||||
command or fixture changes. Reusing a probe never skips a review step.
|
||||
A probe is affected when its entrypoint, dependencies, contract or replay inputs
|
||||
change; if uncertain, rerun it.
|
||||
unchanged-input QA evidence; rerun a probe if its source, tests, entrypoint,
|
||||
dependencies, contract, command, fixture or replay inputs changed, or if uncertain.
|
||||
Reusing a probe never skips a review step.
|
||||
3. **Verify completed actions.** On the final zero-edit pass, reconcile this
|
||||
invocation's actions with current findings. Deduplicate by structural identity
|
||||
and advisory/defect kind. For a completed extraction, retain `fixed` and the
|
||||
@@ -421,7 +420,7 @@ pass: report it with its reason.
|
||||
|
||||
- Use Step 4.6's `specialists` object unchanged, including its empty small-diff map.
|
||||
If this host omits Review Army, use `specialists: {}` without claiming specialist coverage.
|
||||
- Build `findings` from Step 5's combined final-pass findings (core, specialist,
|
||||
- Build `findings` from Step 5's combined findings (core, specialist,
|
||||
adversarial, actionable Greptile, verified exploratory QA findings) and invocation actions. Retain `fingerprint`, `severity`
|
||||
(`CRITICAL|INFORMATIONAL`), `action`, and any `advisory`, `evidence_paths`,
|
||||
`helper_target`. Recheck source after fixes. The logger uses `sharedLibsFingerprint`,
|
||||
@@ -449,11 +448,11 @@ were skipped or this host omits Review Army). This default is not completion evi
|
||||
unresolved non-advisory core defects still count in `issues_found`,
|
||||
`critical`, `informational`. The logger builds trusted `review_binding` from the
|
||||
validated captured branch digest, discarding caller bindings. Never invent a binding
|
||||
or replace REVIEW_START at log time; finish only the final core token.
|
||||
or replace REVIEW_START at log time; finish only this pass's own token.
|
||||
|
||||
### Report the final review
|
||||
|
||||
Emit one final report, merging all reviewers rather than concatenating them:
|
||||
Emit one final report merging all reviewers:
|
||||
1. `Pre-Landing Review: N issues (X critical, Y informational)` counts final unresolved
|
||||
non-advisory defects. State INCOMPLETE if `COMPLETED` is false, even when N=0.
|
||||
2. Use the checklist's action groups with confidence-tagged finding lines. Keep fixed,
|
||||
|
||||
+8
-8
@@ -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": 124218,
|
||||
"agy/gstack-review": 124190,
|
||||
"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": 125122,
|
||||
"codex/gstack-review": 125094,
|
||||
"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": 125489,
|
||||
"copilot/gstack-review": 125461,
|
||||
"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": 140750,
|
||||
"cursor/gstack-review": 140722,
|
||||
"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": 144529,
|
||||
"factory/gstack-review": 144501,
|
||||
"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": 140562,
|
||||
"kiro/gstack-review": 140534,
|
||||
"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": 142985,
|
||||
"opencode/gstack-review": 142957,
|
||||
"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": 140656
|
||||
"slate/gstack-review": 140628
|
||||
}
|
||||
}
|
||||
@@ -1,5 +1,5 @@
|
||||
/**
|
||||
* #3339: /ship Step 9 may re-review only the fix range, but a fix-range pass
|
||||
* #3339: /ship Step 9 and /review may re-review only the fix range, but a fix-range pass
|
||||
* can never be the last review. The fix base comes from the review log (never
|
||||
* the working tree or the caller) and is the commit whose tree the fixing pass
|
||||
* started from, a narrowed row is never CURRENT and carries no
|
||||
@@ -214,3 +214,71 @@ describe('ship Step 9 instructions', () => {
|
||||
expect(army).toContain('continue to Step 10, and only from a full pass: after a fix-range pass, run one full Step 9 first.');
|
||||
});
|
||||
});
|
||||
|
||||
describe('review Step 3 and Step 5.8 instructions', () => {
|
||||
const skill = fs.readFileSync(path.join(ROOT, 'review/SKILL.md'), 'utf8');
|
||||
const step3 = skill.slice(skill.indexOf('## Step 3: Get the diff'), skill.indexOf('## Step 3.4:'));
|
||||
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', () => {
|
||||
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(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`.');
|
||||
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);
|
||||
expect(repeat).toBeGreaterThan(edited);
|
||||
expect(none).toBeGreaterThan(repeat);
|
||||
expect(flat58).toContain("finish only this pass's own token");
|
||||
});
|
||||
});
|
||||
|
||||
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.
|
||||
const reviewRow = (fields: Record<string, unknown>) => JSON.stringify({ skill: 'review', status: 'clean', issues_found: 0, critical: 0, informational: 0, ...fields });
|
||||
function reviewPass(fields: Record<string, unknown>, start: string[], fixes?: string) {
|
||||
const started = bin('gstack-review-log', start);
|
||||
expect(started.code).toBe(0);
|
||||
const [token, base] = started.out.split('\n');
|
||||
if (fixes) edit(fixes);
|
||||
expect(bin('gstack-review-log', [reviewRow(fields), '--finish', token!]).code).toBe(0);
|
||||
return base;
|
||||
}
|
||||
|
||||
test('a committed first pass narrows the next repeat to the uncommitted fixes, then the chain returns to a full pass', () => {
|
||||
const reviewed = head();
|
||||
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');
|
||||
const third = narrowStart();
|
||||
expect(third.code).toBe(1);
|
||||
expect(third.err).toContain('started from uncommitted content');
|
||||
reviewPass(clean, ['--start', 'review']);
|
||||
const last = rows().at(-1)!;
|
||||
expect(last.scope).toBeUndefined();
|
||||
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', () => {
|
||||
reviewPass(fixing, ['--start', 'review'], 'export const a = 2;\n');
|
||||
reviewPass(clean, ['--start', 'review', '--fix-range']);
|
||||
const narrowed = rows().at(-1)!;
|
||||
expect(narrowed.scope).toBe('fix-range');
|
||||
expect(reviewFreshness(narrowed, narrowed.review_binding.range_end_wtree)?.status).toBe('UNVERIFIED');
|
||||
});
|
||||
|
||||
test('a first pass over uncommitted work cannot be narrowed', () => {
|
||||
edit('export const a = 1; // uncommitted\n');
|
||||
reviewPass(fixing, ['--start', 'review'], 'export const a = 2;\n');
|
||||
const r = narrowStart();
|
||||
expect(r.code).toBe(1);
|
||||
expect(r.err).toContain('started from uncommitted content');
|
||||
});
|
||||
});
|
||||
@@ -229,7 +229,8 @@ test('review finalization ownership: initialize invocation state and capture the
|
||||
expect(flat).toMatch(/REVIEW_START for this core candidate before reading its diff/i);
|
||||
expectMentions(flat, [['never', 'reading', 'before']], 'flat');
|
||||
expect(flat).toMatch(/separate PASS_START tokens, not REVIEW_START/);
|
||||
expectMentions(flat, [['only', 'finishes', 'final']], 'flat');
|
||||
expect(flat).toContain('After an edited pass, start the next with `--start review --fix-range`');
|
||||
expect(flat).toContain('On refusal, start a full pass.');
|
||||
});
|
||||
|
||||
test('review finalization ownership: late findings use Fix-First before the bounded parent transition', () => {
|
||||
@@ -240,7 +241,7 @@ test('review finalization ownership: late findings use Fix-First before the boun
|
||||
'Structured approval does not waive advisory/test_stub ASK gates',
|
||||
'## Step 5.8: Persist Eng Review result',
|
||||
'Edited: increment CYCLES once',
|
||||
'No edits: fill the record below',
|
||||
'No edits: after a full pass, fill the record below',
|
||||
'### 2. Fill the record',
|
||||
'--finish REVIEW_START',
|
||||
];
|
||||
@@ -248,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('At 3, persist `converged:false`');
|
||||
expect(flat).toContain('Edited: increment CYCLES once, then fill and save the record below with `converged:false`. Below 3,');
|
||||
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');
|
||||
@@ -276,14 +277,14 @@ test('review finalization ownership: required native completion and optional out
|
||||
expect(flat).toContain('`CONVERGED`: true only for a completed zero-edit pass');
|
||||
});
|
||||
|
||||
test('review finalization ownership: finish only the final core token without log-time capture', () => {
|
||||
test('review finalization ownership: each logged pass finishes its own core token without log-time capture', () => {
|
||||
const step = skill.slice(skill.indexOf('## Step 5.8: Persist Eng Review result'));
|
||||
const flat = step.replace(/\s+/g, ' ');
|
||||
expect(step.match(/--finish REVIEW_START/g)).toHaveLength(1);
|
||||
expect(step).not.toContain('--start review');
|
||||
expect(step).not.toContain('--finish PASS_START');
|
||||
expect(flat).toMatch(/never invent a binding or replace REVIEW_START at log time/i);
|
||||
expectMentions(flat, [['only', 'finish', 'final']], 'flat');
|
||||
expect(flat).toContain("finish only this pass's own token");
|
||||
expect(step).toContain('"completed":COMPLETED,"converged":CONVERGED,"cycles":CYCLES');
|
||||
});
|
||||
|
||||
|
||||
Reference in new issue
Block a user