fix(review): keep the bounded review diff actually within its char budget

[shin-core]

What & why

src/review/review-diff.ts is the bounded diff builder for the reviewer prompt, and its contract — stated in three doc comments — is that the diff fits a char budget. keepHighSignalHunks didn't hold it:

• ≥2-hunk path: when no hunk fit whole it force-kept the highest-signal hunk whole (over budget), and it appended the … (N dropped) notice after selecting hunks, so kept + notice could exceed budget.
• single-hunk path: sliced to exactly budget, then appended the truncation notice on top.

So the "bounded" diff fed to the 120B review models was not actually bounded.

The fix

• Reserve the dropped-notice length before selecting hunks; only charge it when a drop actually happens (all-fit ⇒ no notice ⇒ exact patch survives).
• When no whole hunk fits, return the highest-signal hunk's content, truncated to fit (with the notice) — the same shape the single-hunk path uses — never nothing, never over budget.
• The single-hunk path reserves the truncation notice's length too.
• buildUnifiedReviewDiff reserves the …diff truncated (N files total) line's length in each file's body budget, so appending it on the remaining < 240 break can never push the total over — upholding its own boundary invariant.

Unchanged: the signal ranking, kept-hunks emitted in file order, DEFAULT_DIFF_BUDGET (80,000), diffFilePriority's ordering and its @loopover/engine/review-grounding.ts parity (untouched — engine-parity drift-check passes), addedLineCount/splitHunks/patch-less-file listing.

Tests (test/unit/review-diff.test.ts)

• keepHighSignalHunks(patch, budget).length <= budget for the ≥2-hunk no-hunk-fits case, the single-hunk case, and a spread of budgets/hunk-counts (fail on main).
• Two-hunk patch, smallest hunk 500 chars, budget: 100 → <= 100 and contains a char of the higher-signal hunk's added lines.
• Single-hunk path, budget: 50 over a 5,000-char patch → <= 50.
• buildUnifiedReviewDiff never exceeds budget across budgets 50–2000, even with the truncation notice appended.
• Updated two existing tests that encoded the old over-budget behaviour; added the fits-exactly / notice-smaller-than-budget coverage cases.

Validation

• npm run typecheck clean for this file; npm run engine-parity:drift-check passes; the review-diff suite (31 tests) green.
• Diff coverage on review-diff.ts is 100% line and branch.
• git diff --check <base> HEAD clean; single-file source change + its test.

Closes #10017

[loopover-orb]

Tip

✅ LoopOver review result - approve/merge recommended

_{Review updated: 2026-07-31 06:30:25 UTC}

2 files · 1 AI reviewer · no blockers · readiness 98/100 · CI green · clean

✅ Suggested Action - Approve/Merge

• safe to merge

Review summary
This PR fixes a real invariant violation in the bounded diff builder: previously `keepHighSignalHunks` could return content that, combined with the dropped-hunk or truncation notice, exceeded `budget`, and `buildUnifiedReviewDiff` didn't reserve room for its own truncation line. The rewrite reserves notice lengths before selecting/trimming hunks and adds a solid battery of tests asserting `output.length <= budget` across many budget/hunk-count combinations, which is exactly the right verification strategy for this kind of bug. I traced the selection/trim/emit logic by hand and the core invariant holds for the paths exercised by the tests; the change is tied to and closes issue #10017.

Nits — 5 non-blocking

• src/review/review-diff.ts:132-137: the trimming `while` loop checks `fits(chosen.length, usedChars, droppedNotice(droppedCount + 1).length)` instead of `droppedNotice(droppedCount).length` — it's testing against a notice one drop ahead of the real state, so it can trim one extra hunk it didn't need to; safe (stays in budget) but loses more signal than necessary.
• src/review/review-diff.ts:196 (`keepHighSignalHunks(file.patch, remaining - header.length - 4 - truncationNotice.length)`): when a file's `header.length` alone approaches or exceeds `remaining`, this call passes a negative budget into `keepHighSignalHunks`, which then returns the fixed-length `TRUNCATED_NOTICE` (~33 chars) regardless of how negative the budget is — for a sufficiently long file path this can still push the running `diff` past the outer `budget` by the time the trailing truncation notice is appended; this shortcut predates the PR (same `budget <= 0` fallback existed before) so it isn't a new regression, but the PR's stated goal is exactly to close this class of overrun.
• The new `fits`/`droppedNotice` helper duo adds non-trivial branching without unit tests targeting them directly (only end-to-end `keepHighSignalHunks` behavior is tested) — consider a couple of narrow tests exercising the >1-hunk trim loop in isolation to pin its exact trimming order.
• Fix the off-by-one in the trim-loop condition at src/review/review-diff.ts:134 to check `droppedNotice(droppedCount)` (the notice for the current, not projected, drop count) so it stops trimming as soon as the real output fits.
• Guard the case where `header.length` alone can exceed the per-file remaining budget in `buildUnifiedReviewDiff` (src/review/review-diff.ts:196) — e.g. skip straight to the outer truncation notice when `header.length + 4 + truncationNotice.length >= remaining` instead of calling `keepHighSignalHunks` with a negative budget.

Decision drivers

• ✅ Code review — No blockers (1 reviewer)
• ✅ Gate result — Passing (No configured blocker found.)

Context & advisory signals — never blocks the verdict

Signal	Result	Evidence
Linked issue	✅ Linked	#10017
Related work	✅ No active overlap found	No same-issue or scoped active PR overlap found.
Change scope	✅ 20/20	Low review scope from cached public metadata (1 linked issue).
Validation posture	✅ 25/25	PR body includes validation/test evidence.
Contributor workload	✅ 10/10	Author activity: 64 registered-repo PR(s), 50 merged, 0 issue(s).
Contributor context	✅ Confirmed Gittensor contributor	shin-core; Gittensor profile; 64 PR(s), 0 issue(s).
Improvement	✅ Minor	risk: clean · value: minor · LLM: significant

Linked issue satisfaction

Addressed
The diff rewrites keepHighSignalHunks so the always-keep-top-hunk fallback truncates the hunk's content (reserving the dropped-notice length) instead of returning it whole, the single-hunk path reserves the notice length before slicing, and buildUnifiedReviewDiff now reserves the truncation-notice length in each file's body budget so its own boundary invariant holds; new tests directly assert `.le

Review context

• Author: shin-core
• Role context: outside_contributor
• Public audience mode: oss maintainer
• Lane context: Repository is configured for direct PR review.
• Public profile languages: not available
• Official Gittensor activity: 64 PR(s), 0 issue(s).
• PR-specific overlap: none found.

Contributor next steps

• Keep the PR focused and include validation evidence before maintainer review.

Signal definitions

• Related work = same linked issue, overlapping active PRs, or title/path similarity.
• Change scope = cached public metadata such as size labels, draft state, and review-burden hints.
• Validation posture = whether the PR provides enough public validation/test evidence for maintainer review.
• Contributor workload = public contributor activity and cleanup pressure, not a repo-wide quality failure.
• Contributor context = public GitHub/Gittensor identity context; non-Gittensor status is not a blocker.

🧪 Chat with LoopOver

Ask LoopOver a question about this PR directly in a comment — grounded only in the same cached, public-safe facts shown above, never a new claim.

• @loopover ask <question> answers contribution-quality Q&A with source citations and freshness.
• @loopover chat <question> answers in natural prose from cached decision-pack facts via local inference (maintainer/collaborator; read-only).
• A plain-language @loopover mention with a real question is routed to the closest matching read-only command automatically — no exact syntax required.

Full command reference: https://loopover.ai/docs/loopover-commands

🧪 Experimental — new and may change.

_{🟩 Safe / merged · 🟦 Advisory · 🟨 Held for review · 🟥 Blocked / closed}

---

💰 Earn for open-source contributions like this. Gittensor lets GitHub contributors earn for the work they already do — register to start earning →.

Checked by LoopOver, a quiet PR intelligence layer for OSS maintainers.

• Re-run LoopOver review

[superagent-security]

Superagent didn't find any vulnerabilities or security issues in this PR.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 79.60%. Comparing base (32e3886) to head (65d132a).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #10071      +/-   ##
==========================================
+ Coverage   79.57%   79.60%   +0.02%     
==========================================
  Files         282      283       +1     
  Lines       58664    58752      +88     
  Branches     6842     6877      +35     
==========================================
+ Hits        46682    46769      +87     
  Misses      11694    11694              
- Partials      288      289       +1     

Flag	Coverage Δ
backend	98.86% <100.00%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines	Coverage Δ
src/review/review-diff.ts	98.86% <100.00%> (ø)

[loopover-orb]

LoopOver approves — the gate is satisfied and CI is green.