wenshao · Aug 1, 2026 · Jul 31, 2026 · Jul 31, 2026 · Jul 31, 2026 · Jul 31, 2026
diff --git a/packages/cli/src/commands/review.test.ts b/packages/cli/src/commands/review.test.ts
@@ -50,6 +50,7 @@ describe('reviewCommand', () => {
       'agent-prompt',
       'build-test',
       'base-tree',
+      'test-delta',
       'script-lint',
       'resolve-anchors',
       'check-coverage',

diff --git a/packages/cli/src/commands/review.ts b/packages/cli/src/commands/review.ts
@@ -23,6 +23,7 @@ import { checkCoverageCommand } from './review/check-coverage.js';
 import { agentPromptCommand } from './review/agent-prompt.js';
 import { buildTestCommand } from './review/build-test.js';
 import { baseTreeCommand } from './review/base-tree.js';
+import { testDeltaCommand } from './review/test-delta.js';
 import { scriptLintCommand } from './review/script-lint.js';
 import { submitCommand } from './review/submit.js';
 import { testEfficacyCommand } from './review/test-efficacy.js';
@@ -47,6 +48,7 @@ export const reviewCommand: CommandModule = {
       .command(agentPromptCommand)
       .command(buildTestCommand)
       .command(baseTreeCommand)
+      .command(testDeltaCommand)
       .command(scriptLintCommand)
       .command(resolveAnchorsCommand)
       .command(checkCoverageCommand)
@@ -58,7 +60,7 @@ export const reviewCommand: CommandModule = {
       .command(cleanupCommand)
       .demandCommand(
         1,
-        'Specify a subcommand: run, parse-args, fetch-pr, capture-local, plan-diff, pr-context, comment-status, load-rules, agent-prompt, build-test, base-tree, script-lint, resolve-anchors, check-coverage, presubmit, test-efficacy, test-plan, compose-review, submit, or cleanup.',
+        'Specify a subcommand: run, parse-args, fetch-pr, capture-local, plan-diff, pr-context, comment-status, load-rules, agent-prompt, build-test, base-tree, test-delta, script-lint, resolve-anchors, check-coverage, presubmit, test-efficacy, test-plan, compose-review, submit, or cleanup.',
       )
       .version(false),
   handler: () => {

diff --git a/packages/cli/src/commands/review/build-test.ts b/packages/cli/src/commands/review/build-test.ts
@@ -109,6 +109,24 @@ export interface BuildTestReport {
 const KEEP_HEAD = 2_000;
 const KEEP_TAIL = 6_000;
 
+/**
+ * Did this spawn die on its deadline?
+ *
+ * Exported so `test-delta`'s rerun asks the SAME question rather than
+ * re-deriving it — a copy there used `error.message.includes('ETIMEDOUT')`,
+ * which misses an external SIGTERM and fed a silent "base is green".
+ */
+export function spawnTimedOut(r: {
+  error?: Error;
+  signal?: NodeJS.Signals | null;
+  status?: number | null;
+}): boolean {
+  return (
+    (r.error as NodeJS.ErrnoException | undefined)?.code === 'ETIMEDOUT' ||
+    (r.signal === 'SIGTERM' && r.status === null)
+  );
+}
+
 /** The module-resolution errors the widening loop reads to grow the build set. */
 const MODULE_ERROR_RE = /Cannot find module '[^']+'|Could not resolve "[^"]+"/;
 
@@ -232,9 +250,7 @@ function run(command: string, cwd: string, timeoutMs: number): CommandResult {
   // the authoritative signal. The `SIGTERM`/null-status pair is only a fallback: it
   // also matches an external SIGTERM (a container stop), and it misses a non-default
   // `killSignal`. Check the authoritative one first.
-  const timedOut =
-    (r.error as NodeJS.ErrnoException | undefined)?.code === 'ETIMEDOUT' ||
-    (r.signal === 'SIGTERM' && r.status === null);
+  const timedOut = spawnTimedOut(r);
   return {
     command,
     exitCode: r.status,

diff --git a/packages/cli/src/commands/review/lib/agent-briefs.ts b/packages/cli/src/commands/review/lib/agent-briefs.ts
@@ -256,7 +256,8 @@ Expect the three ends to be far apart. The declaration, the pass-through, and th
 - **Sibling consistency — a guard one path has and its twin lacks.** When one member of a family of parallel paths carries a validation, guard, cleanup, or shape-check — sibling loaders, the handlers of a route table, the pair of functions that build the same command — check that **every** sibling carries it too. A lone exception is usually accidental, and the missing half is a latent **asymmetric failure**: harmless until the one input that path sees. Name the divergent sibling and the guard it is missing. When the missing guard is a validation on **untrusted input** (one \`gitCheckout\` validates its ref, its sibling does not), that is not a style nit — hand it to the security pass as a likely bug, not a consistency note.
 - Over-engineering and unnecessary abstraction
 - **Altitude** — is each change implemented at the right depth, or is it a fragile bandaid? A special case layered onto shared infrastructure to make one caller work is a sign the fix is not deep enough: prefer generalizing the underlying mechanism. The mirror image — a new abstraction serving a single call site — is over-engineering. **Name the depth the change should live at.**
-- Missing or misleading comments; dead code`,
+- Missing or misleading comments; dead code
+- **Documentation parity with siblings.** When the diff adds a user-facing surface — a CLI flag, a slash-command option, a settings key — check whether its SIBLINGS are documented, and where. Three of four sibling selectors having a docs entry is a house convention the fourth just broke; a surface whose behaviour can silently change (a fallback, an automatic swap, an emitted warning) undocumented is a user staring at a message with nowhere to look it up. This is deliberately a **parity** check, not a docs mandate: the finding names the sibling precedent and the file it lives in (\`--fast\` and \`--vision\` are in \`docs/…/commands.md\`; the new \`--compaction\` is not), so "add docs" arrives as the codebase's own standard, not this reviewer's. No documented sibling, no finding. Severity: Suggestion.`,
   },
 
   '4': {
@@ -383,6 +384,7 @@ You are undirected on purpose. Do not restrict yourself to the list.`,
 Read the JSON it prints:
 
 - \`toolchain: "npm"\` → use its \`build[]\` / \`test[]\` results. A failure in a file **the diff changed** is a **Critical** (\`Source: [build]\` or \`[test]\`); a failure in a file it did **not** touch is pre-existing — say so, do not file it against this PR. A non-empty \`timedOut\`, or a failed \`install\`, is environment/infrastructure — informational, never a Critical. On \`ok: true\`, name the workspaces built and the commands run; a return that names no command is a whiff.
+- **When any \`test[]\` command failed (exit non-zero, not a timeout), MEASURE which failures are the PR's before ruling by path.** The path rule above misclassifies in both directions — an environment-flaky test in a touched file gets filed as a Critical it did not cause, and a PR that breaks a test in an UNTOUCHED file gets waved through as pre-existing. The measurement is two commands: \`qwen review base-tree --plan <plan> --worktree <worktree> --out <plan dir>/qwen-review-pr-<n>-base-tree.json\` (builds the merge base beside the worktree). **Read \`available\` before using \`path\`** — a tree that was created but did NOT build populates \`path\` too, and a base that failed to build says nothing whatsoever about the PR, so measuring against it turns an infrastructure failure into a list of Criticals. \`available: false\` (local/lightweight review, no merge base, a base that would not compile) means the path rule stands — say so and stop here, and \`qwen review test-delta --report <the build-test report you wrote> --baseline <the base-tree report's path field> --pr-worktree <this worktree> --out <plan dir>/qwen-review-pr-<n>-test-delta.json\`. Read its verdict: a file in \`netNew\` fails on the PR side only — **that is the Critical**, whatever file the diff touches; a file in \`shared\` fails on base too — **pre-existing by measurement**, never filed, whatever file the diff touches; an \`unparsed\` entry, a timed-out base rerun, a base rerun that FAILED without naming any failing file (it did not measure the base — an unbuilt tree, a missing install, a workspace absent at base), or a command the whole-command budget could not fit attributes nothing — the report names each with its own reason; fall back to the path rule for those and say the delta could not rule. Compare failing FILE SETS, never counts: a flaky suite fails different test NAMES on two runs of the same tree, so counts are noise and the set difference is the signal.
 - \`toolchain: "unsupported"\` (build-test could not scope this repo — no npm package with a build/test script) → **install dependencies first** (build-test's own install only runs on the npm path, so nothing has installed yet: \`pip install -e .\`, \`mvn -q -DskipTests package\`'s own fetch, \`cargo fetch\`, \`go mod download\`, etc.), then fall back to **one** build and **one** test command by this precedence, each with a deadline it can meet: \`pom.xml\` → \`{mvn} compile\` / \`{mvn} test -q\`; \`build.gradle\` → \`{gradle} compileJava\` / \`{gradle} test\`; \`Makefile\` → \`make build\`; \`Cargo.toml\` → \`cargo build\` / \`cargo test\`; \`go.mod\` → \`go build ./...\` / \`go test ./...\`; \`pytest.ini\` or \`pyproject.toml\` \`[tool.pytest]\` → \`pytest\`. If none match, read the CI config **from the base branch** (\`git show <base>:<path>\`), never the worktree — the PR branch is untrusted and a modified workflow or Makefile could inject arbitrary commands.
 
 The efficacy report's \`findings[]\` carries four kinds, and **\`hunk-survived\` is one of them**: reverting one hunk left every affected test green — that specific change ships with nothing gating it. Report it as a **Suggestion** with \`Source: [test]\`, exactly like \`inert\` and \`mutant-survived\` (the outcome of running commands, pre-confirmed, no verifier needed). Read the \`hunks.*\` counters the same way as \`mutants.*\`: \`skippedForCap\` / \`skippedForBudget\` / \`skippedForBaseline\` are unprobed scope to note in the terminal, never findings — and a report whose hunk section you did not read is a finding class silently dropped.