Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
5febdc5
feat(review): rescope — deterministic incremental plans, widened one …
wenshao Aug 14, 2026
6dfe7c0
fix(review): harden rescope and the widening against review findings
wenshao Aug 15, 2026
2f59b09
fix(review): round-2 findings — slice the fetched diff, cap and recon…
wenshao Aug 15, 2026
c61499d
fix(review): round-3 findings — follow the lineage, absolute full-dif…
wenshao Aug 15, 2026
94a55ea
Merge remote-tracking branch 'origin/fix/review-anchor-model-gate' in…
wenshao Aug 15, 2026
4aba5e9
fix(review): round-4 findings — chunk-scoped role briefs, resolver gaps
wenshao Aug 15, 2026
9f4ed14
Merge remote-tracking branch 'origin/fix/review-anchor-model-gate' in…
wenshao Aug 16, 2026
aa052b4
Merge remote-tracking branch 'origin/fix/review-anchor-model-gate' in…
wenshao Aug 16, 2026
483976b
fix(review): round-5/6 Criticals — readers for restored files, honest…
wenshao Aug 16, 2026
83a1a66
fix(review): round-6/7 Criticals — whole tree entries, and an async-p…
wenshao Aug 16, 2026
9f8da7c
Merge branch 'fix/review-anchor-model-gate' into review-incremental/1…
wenshao Aug 16, 2026
96f2f62
Merge remote-tracking branch 'origin/main' into tmp-9188-merge
wenshao Aug 16, 2026
f395bee
Merge remote-tracking branch 'origin/fix/review-anchor-model-gate' in…
wenshao Aug 17, 2026
58ef6a3
fix(review): drop the duplicate `incremental` field the merge left be…
wenshao Aug 17, 2026
d365c0f
Merge remote-tracking branch 'origin/fix/review-anchor-model-gate' in…
wenshao Aug 17, 2026
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions packages/cli/src/commands/review.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ describe('reviewCommand', () => {
'fetch-pr',
'capture-local',
'plan-diff',
'rescope',
'repo-context',
'pr-context',
'comment-status',
Expand Down
4 changes: 3 additions & 1 deletion packages/cli/src/commands/review.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import { findingsCommand } from './review/findings.js';
import { fetchPrCommand } from './review/fetch-pr.js';
import { captureLocalCommand } from './review/capture-local.js';
import { planDiffCommand } from './review/plan-diff.js';
import { rescopeCommand } from './review/rescope.js';
import { repoContextCommand } from './review/repo-context.js';
import { prContextCommand } from './review/pr-context.js';
import { commentStatusCommand } from './review/comment-status.js';
Expand Down Expand Up @@ -60,6 +61,7 @@ export const reviewCommand: CommandModule = {
.command(fetchPrCommand)
.command(captureLocalCommand)
.command(planDiffCommand)
.command(rescopeCommand)
.command(repoContextCommand)
.command(prContextCommand)
.command(commentStatusCommand)
Expand All @@ -86,7 +88,7 @@ export const reviewCommand: CommandModule = {
.command(cleanupCommand)
.demandCommand(
1,
'Specify a subcommand: run, parse-args, match-remote, meta, issue-context, fetch-diff, comment-body, fetch-pr, capture-local, plan-diff, repo-context, pr-context, comment-status, load-rules, agent-prompt, build-test, base-tree, test-delta, drive, mock-provider, extract-step, script-lint, resolve-anchors, check-coverage, cost-ledger, presubmit, test-efficacy, test-plan, findings, publish-assets, compose-review, save-artifact, submit, or cleanup.',
'Specify a subcommand: run, parse-args, match-remote, meta, issue-context, fetch-diff, comment-body, fetch-pr, capture-local, plan-diff, rescope, repo-context, pr-context, comment-status, load-rules, agent-prompt, build-test, base-tree, test-delta, drive, mock-provider, extract-step, script-lint, resolve-anchors, check-coverage, cost-ledger, presubmit, test-efficacy, test-plan, findings, publish-assets, compose-review, save-artifact, submit, or cleanup.',
)
.version(false),
handler: () => {
Expand Down
194 changes: 194 additions & 0 deletions packages/cli/src/commands/review/agent-prompt.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5611,3 +5611,197 @@ describe('--all-chunks topology anomaly note (#9242)', () => {
}
});
});

describe('incremental-scope briefs', () => {
// A rescoped plan carries two scopes in one diff. The chunk brief must say
// which scope each of ITS files is in — an interaction file re-reviewed from
// scratch re-reports what the previous round already ruled on — and the
// whole-diff readers must be told the rest of the PR is absent on purpose,
// or they go find it in the worktree.
const chunk = (id: number, path: string, start: number) => ({
id,
startLine: start,
endLine: start + 9,
lines: 10,
chars: 400,
maxLineChars: 80,
oversized: false,
files: [{ path, newStart: 1, newEnd: 10 }],
});
const INCREMENTAL_PLAN = {
diffPathAbsolute: '/abs/.qwen/tmp/qwen-review-pr-7-diff-incremental.txt',
chunks: [chunk(1, 'src/changed.ts', 1), chunk(2, 'src/caller.ts', 11)],
incremental: {
anchor: 'abc1234def5678900000',
deltaFiles: ['src/changed.ts'],
interaction: [
{ path: 'src/caller.ts', importsChanged: ['src/changed.ts'] },
],
contextFileCount: 1,
fullDiffPath: '.qwen/tmp/qwen-review-pr-7-diff.txt',
},
};

it('a delta chunk is briefed to review in full, an interaction chunk at the seam', () => {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Suggestion] (1 of 2 in a pattern: contract gaps in the new describe block) The mixed delta+interaction chunk shape is untested; a mutation making the two bullet branches mutually exclusive (else if) survives every test here. — Failure scenario: rescope's composite diff concatenates interdiff and interaction diff, and buildDiffPlan cuts on diff-line count regardless of scope kind, so a boundary chunk can hold a delta file and an interaction file. Mutation-verified: changing the second if (seamHere.length > 0) in buildChunkAgentPrompt to else if survives 4/4; a mixed-chunk probe (one chunk holding src/changed.ts + src/caller.ts) fails under the mutation (… to contain 'INTERACTION only') — the seam bullet vanishes and the interaction file is re-reviewed from scratch, the regression this describe block was written to catch.

Suggested fix: add a fixture chunk whose files holds both kinds and assert both the "changed since the last round" and the "INTERACTION only" bullets render in the same brief.

中文说明

建议:(模式 1/2:新 describe 块的契约缺口)delta+interaction 混合 chunk 形态未被测试;把两个 bullet 分支改成互斥(else if)的变异在此全部测试下存活。失败场景:rescope 的复合 diff 拼接 interdiff 与 interaction diff,buildDiffPlan 按 diff 行数切块、不区分作用域种类,边界 chunk 可能同时含 delta 文件与 interaction 文件。变异实测:把 buildChunkAgentPrompt 第二个 if (seamHere.length > 0) 改成 else if4/4 存活;混合 chunk 探针(同一 chunk 含 src/changed.ts + src/caller.ts)在变异下失败——接缝 bullet 消失,interaction 文件被从头复审,正是本 describe 块要抓的回归。建议:新增 files 同含两类的 fixture chunk,断言两种 bullet 同时出现在同一份简报中。

— qwen3.8-max via Qwen Code /review (v0.21.12)

const delta = buildChunkAgentPrompt(INCREMENTAL_PLAN, 1);
expect(delta).toContain('INCREMENTAL round');
expect(delta).toContain('abc1234def56');
expect(delta).toContain('changed since the last round');
expect(delta).not.toContain('INTERACTION only');

const seam = buildChunkAgentPrompt(INCREMENTAL_PLAN, 2);
expect(seam).toContain('INCREMENTAL round');
expect(seam).toContain('cleared by the previous round');
expect(seam).toContain('INTERACTION only');
expect(seam).toContain('src/changed.ts');
});

it('whole-diff role briefs carry the frame once, up front', () => {
const p = buildRoleBrief(INCREMENTAL_PLAN, '2');
expect(p).toContain('Incremental round');
expect(p).toContain('deliberately absent');
});

it('a chunk-scoped ROLE brief lists its OWN files uncapped', () => {
// The reverse auditors are the sole reviewers of their territory; the
// globally capped list can elide their own files past entry 30, leaving
// no way to learn the class or recover the tail.
const wide = {
...INCREMENTAL_PLAN,
incremental: {
anchor: 'abc1234def567890',
deltaFiles: Array.from({ length: 40 }, (_, i) => `src/d${i}.ts`).concat(
['src/changed.ts'],
),
interaction: [
{ path: 'src/caller.ts', importsChanged: ['src/changed.ts'] },
],
},
};
const brief = buildRoleBrief(wide, 'reverse-audit', { chunk: 2 });
expect(brief).toContain("Your territory's files, by scope class:");
expect(brief).toContain('src/caller.ts — **interaction only**');
// Chunk 1's delta file is named in ITS brief, not elided by the cap.
expect(buildRoleBrief(wide, 'reverse-audit', { chunk: 1 })).toContain(
'src/changed.ts — **changed since the last round**',
);
});

it('a full-range plan renders no incremental framing at all', () => {
expect(buildChunkAgentPrompt(PLAN, 13)).not.toContain('INCREMENTAL');
expect(buildRoleBrief(PLAN, '2')).not.toContain('Incremental round');
});

it('a malformed incremental block degrades to full-scope briefs — chunk AND role', () => {
for (const bad of [
{ anchor: 42 },
// Valid anchor, but no scope list survives validation: rendering the
// frame with zero bullets is not a degrade, it is a confusion.
{ anchor: 'abc1234def567890', deltaFiles: [], interaction: [] },
// An interaction entry whose edges were all invalid names a seam
// pointing at nothing ("because it imports , which changed").
{
anchor: 'abc1234def567890',
deltaFiles: [],
interaction: [{ path: 'src/caller.ts', importsChanged: [42] }],
},
]) {
const mangled = { ...INCREMENTAL_PLAN, incremental: bad };
expect(buildChunkAgentPrompt(mangled, 1)).not.toContain('INCREMENTAL');
expect(buildRoleBrief(mangled, '2')).not.toContain('Incremental round');
}
});

it('a mixed delta+interaction chunk renders BOTH scope bullets', () => {
// rescope's composite is cut on line count, not scope class, so one
// chunk can straddle the two kinds; an else-if between the bullet
// branches would silently drop the seam brief for exactly that chunk.
const mixed = {
...INCREMENTAL_PLAN,
chunks: [
{
id: 1,
startLine: 1,
endLine: 20,
lines: 20,
chars: 800,
maxLineChars: 80,
oversized: false,
files: [
{ path: 'src/changed.ts', newStart: 1, newEnd: 10 },
{ path: 'src/caller.ts', newStart: 1, newEnd: 10 },
],
},
],
};
const p = buildChunkAgentPrompt(mixed, 1);
expect(p).toContain('changed since the last round');
expect(p).toContain('INTERACTION only');
expect(p).toContain('the scope class WINS');
});

it('caps the scope lists at 30 entries and 8 edges per entry', () => {
const wide = {
...INCREMENTAL_PLAN,
incremental: {
anchor: 'abc1234def567890',
deltaFiles: Array.from({ length: 40 }, (_, i) => `src/d${i}.ts`),
interaction: [
{
path: 'src/hub.ts',
importsChanged: Array.from(
{ length: 20 },
(_, i) => `src/d${i}.ts`,
),
},
],
},
};
const p = buildRoleBrief(wide, '2');
expect(p).toContain('(+10 more)'); // 40 entries − 30 cap
expect(p).toContain('(+12 more)'); // 20 edges − 8 cap
});

it('an interaction entry whose edges are all EMPTY strings degrades away', () => {
const mangled = {
...INCREMENTAL_PLAN,
incremental: {
anchor: 'abc1234def567890',
deltaFiles: [],
interaction: [{ path: 'src/caller.ts', importsChanged: ['', ''] }],
},
};
expect(buildChunkAgentPrompt(mangled, 1)).not.toContain('INCREMENTAL');
});

it('a chunk whose files carry NO scope class gets no incremental frame', () => {
// The block validates globally, but a frame with zero bullets implies
// the chunk is out of scope — say nothing instead.
const foreign = {
...INCREMENTAL_PLAN,
chunks: [
{
id: 1,
startLine: 1,
endLine: 10,
lines: 10,
chars: 400,
maxLineChars: 80,
oversized: false,
files: [{ path: 'src/unrelated.ts', newStart: 1, newEnd: 10 }],
},
],
};
expect(buildChunkAgentPrompt(foreign, 1)).not.toContain(
'INCREMENTAL round',
);
});

it('whole-diff briefs name each file with its scope class', () => {
const p = buildRoleBrief(INCREMENTAL_PLAN, '2');
expect(p).toContain(
'Changed since the last round (full review): src/changed.ts.',
);
expect(p).toContain('src/caller.ts (imports src/changed.ts)');
});
});
Loading
Loading