orb(review-diff): `keepHighSignalHunks` returns more than its budget, so the "bounded" review diff is not bounded

[JSONbored]

⚠️ Definition of Done: this issue must be completed in full, in a single PR. Do not split this
work across multiple PRs, and do not defer any Deliverable below to a follow-up issue. A PR that
satisfies only some of the Deliverables, stubs a required test, or leaves a checkbox
partially-done does NOT resolve this issue and will be closed.

Context

src/review/review-diff.ts is the bounded diff builder for the reviewer prompt. Its contract is stated three
times: DEFAULT_DIFF_BUDGET's doc ("Char budget of the diff fed to the review models… Only a genuinely huge
PR truncates", :12-16), keepHighSignalHunks' doc ("Fit a file's patch into budget chars…", :98-103),
and buildAiReviewDiff's doc ("Caps total size so a huge PR cannot blow the model context or the neuron
budget", :170-172).

keepHighSignalHunks does not enforce it (src/review/review-diff.ts:104-121):

export function keepHighSignalHunks(patch: string, budget: number): string {
  if (budget <= 0) return "… (this file's diff truncated)";
  const hunks = splitHunks(patch);
  if (hunks.length <= 1) {
    return patch.length > budget ? `${patch.slice(0, budget)}\n… (this file's diff truncated)` : patch;
  }
  const ranked = hunks.map((h, i) => ({ i, len: h.length, sig: addedLineCount(h) })).sort((a, b) => b.sig - a.sig);
  const keep = new Set<number>();
  let used = 0;
  for (const r of ranked) {
    const sep = keep.size > 0 ? 1 : 0;
    if (used + r.len + sep > budget) continue;
    keep.add(r.i);
    used += r.len + sep;
  }
  const top = ranked[0];
  if (keep.size === 0 && top) keep.add(top.i); // always keep the single highest-signal hunk
  const dropped = hunks.length - keep.size;
  const kept = hunks.filter((_, i) => keep.has(i)).join("\n");
  return dropped > 0 ? `${kept}\n… (${dropped} lower-signal hunk(s) dropped)` : kept;
}

Two paths return more than budget:

1. The always-keep-top fallback. When a patch has ≥ 2 hunks and none of them fits, keep.size === 0
   and the highest-signal hunk is added at FULL length. `keepHighSignalHunks("@@ -1 +1 @@\n" + "+a\n".repeat(500)
   • "@@ -9 +9 @@\n+b", 100)` returns a string well over 1,000 characters for a budget of 100. A single hunk
     in a real PR is routinely tens of thousands of characters.
2. The single-hunk path. patch.slice(0, budget) is then concatenated with a 31-character suffix (a
   newline plus … (this file's diff truncated)), so the return is budget + 31.

buildUnifiedReviewDiff calls it with whatever budget is left (src/review/review-diff.ts:166,
keepHighSignalHunks(file.patch, remaining - header.length - 4)), and its own per-file guard only breaks out
once remaining < 240 (:155-158) — a check made BEFORE the oversized body is appended, never after. So the
returned string can exceed DEFAULT_DIFF_BUDGET (80,000) by roughly one hunk's full length, and the loop
then terminates on the next iteration having already blown the budget.

The existing test suite already demonstrates the overshoot without asserting on it
(test/unit/review-diff.test.ts:149-157): keepHighSignalHunks("@@ a\n+x\n+y\n@@ b\n+p\n+q", 20) keeps the
first 10-char hunk, drops the second, and appends "\n… (1 lower-signal hunk(s) dropped)" — a 45-character
return for a 20-character budget. The test only asserts .toContain("dropped").

The consequence is real spend, not just a doc mismatch: runAiReview slices the prompt's diff at 120,000
characters (src/services/ai-review.ts:1344) and the per-call neuron estimate is computed from the ACTUAL
prompt length (src/services/ai-review.ts:3133, estimateNeurons(system.length + user.length, …)). A
budget the builder does not honour therefore raises the real per-review cost by up to the difference between
80,000 and that downstream 120,000 cap, on every PR that trips the fallback.

Requirements

• keepHighSignalHunks must never return a string longer than budget for any budget > 0.
• The always-keep-top-hunk behaviour must be preserved in spirit: when no hunk fits whole, the function must
  still return the highest-signal hunk's CONTENT, truncated to fit, rather than returning nothing. Use the
  same slice + … (this file's diff truncated) shape the single-hunk path already uses.
• The dropped-hunk notice must still be emitted when hunks were dropped, and the combined
  kept + "\n" + notice string must itself fit inside budget — reserve the notice's length before selecting
  hunks rather than appending it afterwards.
• The single-hunk path must reserve the truncation notice's length too, so patch.slice(...) + notice fits
  budget.
• buildUnifiedReviewDiff must assert the invariant at its own boundary: the string it returns must never
  exceed budget.
• Behaviour that must NOT change: DEFAULT_DIFF_BUDGET (80,000); diffFilePriority's ordering and its
  documented parity with packages/loopover-engine/src/review/diff-file-priority.ts and
  review-grounding.ts; addedLineCount, extractAddedLines, totalAddedLineCount, splitHunks; the
  source-first sort in buildUnifiedReviewDiff; the remaining < 240 early break and its
  ### …diff truncated (N files total) line; the patch-less-file listing; the ranking by addedLineCount
  descending; and buildSecretScanDiff, which is deliberately UNBUDGETED and must stay so.

⚠️ Required pattern: reuse the truncation shape already in this function — ${text.slice(0, n)}\n… (this file's diff truncated) — and reserve the notice length up front, the same way buildUnifiedReviewDiff
already reserves 4 characters at :166. What does NOT satisfy this issue: raising DEFAULT_DIFF_BUDGET;
dropping the always-keep-top-hunk behaviour so an over-budget file contributes nothing; adding a post-hoc
.slice(0, budget) at the buildUnifiedReviewDiff boundary only (it would cut mid-hunk and leave
keepHighSignalHunks still lying about its contract); touching buildSecretScanDiff; a test-only PR.

Deliverables

• src/review/review-diff.ts: keepHighSignalHunks(patch, budget).length <= budget holds for every
  budget > 0, including the ≥2-hunk no-hunk-fits case and the single-hunk case.
• src/review/review-diff.ts: when no hunk fits, the returned string still contains content from the
  highest-signal hunk (not just the notice).
• A test in test/unit/review-diff.test.ts:
  keepHighSignalHunks over a two-hunk patch whose smallest hunk is 500 chars, with budget: 100,
  returns a string of length <= 100 that contains at least one character of the higher-signal hunk's
  added lines.
• A test in test/unit/review-diff.test.ts: the single-hunk path with budget: 50 over a 5,000-char
  patch returns a string of length <= 50.
• A test in test/unit/review-diff.test.ts: buildUnifiedReviewDiff over a file list whose last
  in-budget file carries a multi-hunk patch far larger than the remaining budget returns a string of
  length <= budget.
• A test in test/unit/review-diff.test.ts pinning the unchanged behaviours: budget <= 0 still returns
  exactly "… (this file's diff truncated)"; a patch that fits whole is returned verbatim with no notice;
  and the dropped-hunk notice still names the correct count.
• The existing "keeps every hunk when they fit exactly" case
  (test/unit/review-diff.test.ts:149-157) is updated so its budget: 20 assertion also checks the
  return length is <= 20, and the "head-slices a single oversized hunk" case (:168-174) is updated
  to assert the same for its budget: 30.
• A regression test at test/unit/review-diff.test.ts named for this bug (e.g.
  "REGRESSION: keepHighSignalHunks never exceeds its budget, even when no hunk fits").

All Deliverables above are required in a single PR. A PR that satisfies only some of them — for example
fixing the always-keep-top fallback while leaving the single-hunk path's notice unaccounted for, or adding
the length assertions without the "still contains content" assertion — does not resolve this issue.

Test Coverage Requirements

This repo enforces 99%+ Codecov patch coverage, branch-counted. vitest.config.ts's coverage.include
covers src/**/*.ts and packages/loopover-engine/src/**/*.ts; src/review/review-diff.ts is measured and
gated. Every branch the change touches needs both arms: budget <= 0; hunks.length <= 1 and its
patch.length > budget sub-branch; the used + r.len + sep > budget continue vs. keep arms; keep.size > 0
for the separator charge (first hunk vs. subsequent); the keep.size === 0 && top fallback (taken and not
taken); and dropped > 0 vs. dropped === 0. In buildUnifiedReviewDiff, the remaining < 240 break arm,
the !file.patch arm, and the header.length + body.length + 2 > remaining arm all need both sides.

Expected Outcome

buildAiReviewDiff returns a string that actually respects DEFAULT_DIFF_BUDGET, so a PR containing one
oversized hunk no longer silently pushes the reviewer prompt — and the per-review spend estimated from it —
above the budget the module documents.

Links & Resources

• src/review/review-diff.ts:12-16 — DEFAULT_DIFF_BUDGET and its stated contract
• src/review/review-diff.ts:98-121 — keepHighSignalHunks, both over-budget paths
• src/review/review-diff.ts:138-173 — buildUnifiedReviewDiff and the remaining < 240 guard
• src/review/review-diff.ts:170-198 — buildAiReviewDiff's "caps total size" claim
• src/services/ai-review.ts:1344, :3133 — the 120,000-char prompt slice and the length-derived spend estimate
• src/review/review-diff.ts:200-218 — buildSecretScanDiff, deliberately unbudgeted and out of scope

[linear-code]

JSO-6992