perf(review): overlap the completeness judge with the fleet, make its PASS intent-sticky, cap 6

[norvalbv]

What

Three latency/cost changes to the ship gate chain, driven by the gate telemetry (~/.devkit/telemetry/gate-events.jsonl): review is ~95% of a ship attempt's wall clock, the correctness lens split dominates the fleet makespan, and completeness (straight opus, mean ~4 min, max 30 min) ran serially at commit-msg on every landed attempt.

1. Review concurrency default 3 → 6 (timing.mts). The lens split schedules ~8 judge tasks on a backend commit; at 6 they run in at most two waves, so the fleet makespan approaches the slowest single lens instead of a 3-slot packing. The bound is subscription slots, not CPU/memory — if this ever needs lowering, watch judge timeout rates, not RSS.

2. Parallel completeness prewarm on the ship path (husky-block.mts review fragment). The sc-1442 composed-message file already reaches pre-commit, so the review fragment now launches guard-review completeness alongside the fleet and applies the commit-msg fragment's exact exit contract, just earlier. A confident PASS lands in the shared verdict store (merge-under-lock, so concurrent writers are safe) and the commit-msg gate re-judges it as a cache hit. If the fleet blocks the commit, the judge is killed rather than awaited. Interactive commits (no message file) are unchanged; devkit review is excluded (it exports the same env var as its reviewer intent file). normalizeCommitMessage() mirrors git's cleanup=whitespace so the ship temp file and git's cleaned COMMIT_EDITMSG compute identical cache keys.

3. Intent-sticky completeness PASS. The gate judges the message's claims against the delivered change, so a retry whose diff was reshaped to satisfy another reviewer — same branch, same message — is not re-judged: a confident PASS is additionally saved under a branch+message+brief key (version-salted). What re-opens the gate: an amended message, a different branch, a changed brief, or a devkit upgrade. FAILs are never sticky. The sticky lookup runs before targets retrieval and diff assembly, so a hit costs ~1s.

Net effect: opus completeness is paid once per branch+message across a retry chain (telemetry: ~4.5 attempts per landed ship), and a passing attempt's judged path drops from fleet + ~4 min serial completeness to max(fleet-at-6, completeness).

Trade-off (deliberate, user-ruled)

The sticky PASS trades a re-judgement for cost. If a retry genuinely guts claimed functionality — e.g. an agent deletes a feature to appease the correctness reviewer — while keeping the same commit message, completeness will not re-catch it on that branch. The commit message is the guard: changing the claim re-judges, and a FAIL always re-judges. We accepted this because the flip data shows completeness verdicts on reshaped-but-same-intent diffs almost never change, while the re-runs were the single largest opus line item (~200 calls/week, ~14 h of opus).

Secondary trade-off: the prewarm starts completeness on every ship attempt rather than only on attempts that survive pre-commit review. The sticky pass neutralises the repeat cost (first attempt pays, later attempts hit the key even when blocked mid-chain), and a fleet block kills the in-flight judge — but a first attempt that is blocked before completeness finishes does pay for a partial judge run that today costs nothing. Worst case is bounded at one completeness run per branch+message, same as the status quo's best case.

Verification

• run-review.test.mts 119/119 (3 new sticky tests, 1 key-normalisation test, concurrency assertions moved to 6)
• husky-block-exec.test.mts 26/26 (7 new prewarm branch tests: armed/unarmed, FAIL/outage/object-fault/fail-open, fleet-block attribution)
• self-host.test.mts parity green — .husky/pre-commit regenerated from the updated fragment
• dist rebuilt from a clean origin/main worktree (4 compiled outputs changed, matching the 4 changed sources)

Committed --no-verify by request; CI runs the full suite. Follow-up at ship time: record the intent-scoped-verdict ruling via guard-decisions.

🤖 Generated with Claude Code

Summary by CodeRabbit

• New Features

  • Completeness checks now run alongside applicable ship-commit reviews.
  • Review results distinguish confirmed gaps, unreadable staged content, and unavailable checks.
  • Confirmed completeness results can be reused for matching branch and commit-message intent.

• Performance

  • Review processing supports higher parallelism for faster results.

• Bug Fixes

  • Commit-message formatting differences are normalized consistently.
  • Failed checks are re-evaluated after relevant fixes, while inconclusive results preserve fail-open behavior.
  • Background checks are stopped cleanly when reviews block a commit.

[coderabbitai]

📝 Walkthrough

Walkthrough

The completeness gate now normalizes commit messages, caches intent-scoped PASS results, and prewarms during ship commits. The Husky hook handles completeness outcomes separately from reviewer verdicts. Review concurrency defaults and related tests now use six concurrent judges.

Changes

Completeness cache and normalization

Layer / File(s)	Summary
Normalize messages and cache PASS results gate-engine/review/completeness.mts, gate-engine/review/__tests__/run-review.test.mts	Commit messages are normalized before processing. Branch-and-message PASS results can skip target and diff retrieval. PASS, FAIL, amended-message, branch, reviewer-brief, and whitespace-normalization behavior is covered by tests.

Ship-commit orchestration

Layer / File(s)	Summary
Prewarm and enforce completeness in ship commits .husky/pre-commit, cli/lib/husky/husky-block.mts, cli/lib/husky/review-fragments.mts, cli/__tests__/husky-block-exec.test.mts	The hook starts completeness checks in parallel when a ship commit has a known message file. It waits after reviewer pass or inconclusive results, terminates and reaps the judge after reviewer block, and applies distinct completeness exit-code handling.

Review concurrency

Layer / File(s)	Summary
Align concurrency defaults and assertions gate-engine/review/run-review.mts, gate-engine/review/telemetry/timing.mts, gate-engine/review/__tests__/run-review.test.mts	The documented and runtime default concurrency changes to six. Telemetry, bounded-concurrency, retry, and fallback expectations are updated.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Sequence Diagram(s)

sequenceDiagram
  participant precommit as .husky/pre-commit
  participant fleet as reviewer fleet
  participant completeness as completeness judge
  participant gate as review gate

  precommit->>fleet: Start reviewer fleet
  precommit->>completeness: Start completeness judge for known ship message
  fleet-->>gate: Return reviewer verdict

  alt Reviewer fleet passes or is inconclusive
    gate->>completeness: Wait for completeness result
    completeness-->>gate: Return completeness exit code
    gate-->>precommit: Allow or block commit
  else Reviewer fleet blocks
    gate->>completeness: Terminate and reap completeness judge
    gate-->>precommit: Block commit
  end

Loading

Possibly related PRs

• norvalbv/devkit#334: Introduced the ship commit flow and DEVKIT_COMMIT_MSG_FILE consumed by this PR.
• norvalbv/devkit#95: Also modifies Husky review-gate orchestration and review-mode execution.
• norvalbv/devkit#348: Adds the unreadable staged-content behavior handled by this PR.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly summarizes the three main changes: parallel completeness review, sticky PASS caching, and a concurrency cap of 6.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches 💡 2

📝 Generate docstrings 💡

• Create stacked PR
• Commit on current branch

🛠️ Fix failing CI checks 💡

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch benjinorval/review-latency-prewarm

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (1)

gate-engine/review/__tests__/run-review.test.mts (1)

1951-1988: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Cover the remaining sticky-cache invalidators.

The sticky completeness-intent key uses DEVKIT_SHIP_BRANCH, the commit message (message), and the reviewer brief (body). The version salt is in the judge cache key, but the intent sticky cache reuses the same cached PASS without re-judging when any sticky-key component changes. Add cases that change the branch and the completeness agent brief, and reassert that exec is invoked again.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@gate-engine/review/__tests__/run-review.test.mts` around lines 1951 - 1988,
Extend the sticky completeness tests around runCompleteness to cover changes to
DEVKIT_SHIP_BRANCH and the reviewer brief body: after an initial PASS, alter
each component while keeping the other sticky-key inputs unchanged, then assert
the result remains PASS and exec is invoked again. Use the existing msg helper
and run-review test setup, and preserve the current message-change and
diff-retry assertions.

Source: Learnings

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@cli/lib/husky/husky-block.mts`:
- Around line 92-99: After terminating the tracked completeness process in
cli/lib/husky/husky-block.mts#L92-L99 and .husky/pre-commit#L163-L170, wait for
"$comp_pid" before returning so the process is reaped. Update
cli/__tests__/husky-block-exec.test.mts#L194-L199 to use a completeness stub
that waits for a signal, then verify the fleet-failure path both terminates and
reaps it.

---

Nitpick comments:
In `@gate-engine/review/__tests__/run-review.test.mts`:
- Around line 1951-1988: Extend the sticky completeness tests around
runCompleteness to cover changes to DEVKIT_SHIP_BRANCH and the reviewer brief
body: after an initial PASS, alter each component while keeping the other
sticky-key inputs unchanged, then assert the result remains PASS and exec is
invoked again. Use the existing msg helper and run-review test setup, and
preserve the current message-change and diff-retry assertions.

🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 36e91b10-3afe-449d-a779-f2c8fb2744cf

📥 Commits

Reviewing files that changed from the base of the PR and between 6250cd9 and 14d8fc1.

⛔ Files ignored due to path filters (4)

• dist/cli/lib/husky/husky-block.mjs is excluded by !**/dist/**
• dist/gate-engine/review/completeness.mjs is excluded by !**/dist/**
• dist/gate-engine/review/run-review.mjs is excluded by !**/dist/**
• dist/gate-engine/review/telemetry/timing.mjs is excluded by !**/dist/**

📒 Files selected for processing (7)

• .husky/pre-commit
• cli/__tests__/husky-block-exec.test.mts
• cli/lib/husky/husky-block.mts
• gate-engine/review/__tests__/run-review.test.mts
• gate-engine/review/completeness.mts
• gate-engine/review/run-review.mts
• gate-engine/review/telemetry/timing.mts

[coderabbitai]

🧹 Nitpick comments (3)

cli/__tests__/husky-block-exec.test.mts (2)

68-74: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

The backgrounded sleep 30 outlives the stub.

The TERM trap exits the stub, but it does not kill the sleep 30 child. That process stays alive for up to 30 seconds after each of the two COMP_SLOW_TERM tests. It cannot hang execFileSync, because exec >/dev/null 2>&1 runs before the fork and stdin is ignore. It only leaves stray processes in the test host.

Kill the child in the trap, or shorten the sleep.

♻️ Proposed cleanup of the background sleep

         if [ -n "\${COMP_SLOW_TERM:-}" ]; then
             exec >/dev/null 2>&1
-            trap 'sleep 1; echo reaped > "$HOME/comp-reaped"; exit 143' TERM
             echo running > "$HOME/comp-running"
             sleep 30 &
+            slp=$!
+            trap 'kill $slp 2>/dev/null; sleep 1; echo reaped > "$HOME/comp-reaped"; exit 143' TERM
             wait $!
         fi

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@cli/__tests__/husky-block-exec.test.mts` around lines 68 - 74, Update the
COMP_SLOW_TERM stub around the background sleep and TERM trap so the trap
terminates the spawned sleep process before exiting. Preserve the existing
reaped marker and exit status, and ensure the child PID is available to the
trap.

---

220-227: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

The test title claims more than the assertions check.

The title states that review mode "exports the same env for its reviewer intent file". The body only asserts that guard-review --gate runs and that guard-review completeness does not run. It never checks the exported message-file path. Either assert the export, or narrow the title to the prewarm exclusion.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@cli/__tests__/husky-block-exec.test.mts` around lines 220 - 227, Update the
test named “review mode does NOT prewarm — it exports the same env for its
reviewer intent file” so its assertions match the title: verify that the
reviewer intent file export uses the expected message-file path in addition to
the existing guard invocation checks. Alternatively, narrow the test title to
describe only the prewarm exclusion if that export is not intended to be tested.

gate-engine/review/__tests__/run-review.test.mts (1)

2019-2030: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Isolate the brief component of the sticky key.

This test keeps the diff unchanged between the two calls. The brief is also part of the exact-bytes key, because wrapCompleteness(body, ...) puts it in the prompt. A second judgement therefore happens even if the sticky key stopped including the brief. The test proves "an edited brief re-judges", but it does not prove the sticky key is brief-scoped.

Apply the same moving-diff technique used in the branch test at Line 1996 to isolate the component.

♻️ Proposed change to isolate the sticky-key brief component

 it('an edited reviewer brief re-judges — a new judge is not covered by the old verdict', async () => {
   const repo = consumerRepo({ backend: true });
   vi.spyOn(console, 'error').mockImplementation(() => {});
   const exec = mkExec(async () => 'VERDICT: PASS');
   expect(await runCompleteness(msg(repo, 'feat: add db layer'), repo, { exec })).toBe(0);
+  // Reshape the diff so the exact-bytes key cannot hit: only the sticky key is under test.
+  writeFileSync(join(repo, 'src', 'main', 'db.ts'), 'export const q = 2;\n');
+  execSync('git add .', { cwd: repo });
   writeFileSync(
     join(repo, '.claude', 'agents', 'feature-completeness-reviewer.md'),
     '---\nname: feature-completeness-reviewer\n---\nBrief for feature-completeness-reviewer. Also check migrations.',
   );
   expect(await runCompleteness(msg(repo, 'feat: add db layer'), repo, { exec })).toBe(0);
   expect(exec).toHaveBeenCalledTimes(2);
 });

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@gate-engine/review/__tests__/run-review.test.mts` around lines 2019 - 2030,
Update the test around the second run of the edited reviewer brief to use a
changed diff, matching the moving-diff technique from the branch test near line
1996. Keep the reviewer brief edit and assertions intact, while ensuring the
changed diff prevents prompt-byte differences from independently triggering
re-judgment and isolates the sticky key’s brief-scoped behavior.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@cli/__tests__/husky-block-exec.test.mts`:
- Around line 68-74: Update the COMP_SLOW_TERM stub around the background sleep
and TERM trap so the trap terminates the spawned sleep process before exiting.
Preserve the existing reaped marker and exit status, and ensure the child PID is
available to the trap.
- Around line 220-227: Update the test named “review mode does NOT prewarm — it
exports the same env for its reviewer intent file” so its assertions match the
title: verify that the reviewer intent file export uses the expected
message-file path in addition to the existing guard invocation checks.
Alternatively, narrow the test title to describe only the prewarm exclusion if
that export is not intended to be tested.

In `@gate-engine/review/__tests__/run-review.test.mts`:
- Around line 2019-2030: Update the test around the second run of the edited
reviewer brief to use a changed diff, matching the moving-diff technique from
the branch test near line 1996. Keep the reviewer brief edit and assertions
intact, while ensuring the changed diff prevents prompt-byte differences from
independently triggering re-judgment and isolates the sticky key’s brief-scoped
behavior.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: f1f6a5c8-55b5-4750-b89f-8da19ce4d29b

📥 Commits

Reviewing files that changed from the base of the PR and between 14d8fc1 and 91b0e7d.

⛔ Files ignored due to path filters (2)

• dist/cli/lib/husky/husky-block.mjs is excluded by !**/dist/**
• dist/cli/lib/husky/review-fragments.mjs is excluded by !**/dist/**

📒 Files selected for processing (5)

• .husky/pre-commit
• cli/__tests__/husky-block-exec.test.mts
• cli/lib/husky/husky-block.mts
• cli/lib/husky/review-fragments.mts
• gate-engine/review/__tests__/run-review.test.mts

🚧 Files skipped from review as they are similar to previous changes (2)

• cli/lib/husky/husky-block.mts
• .husky/pre-commit