Skip to content

test(review): review-diff.ts's non-positive-budget guard and additions/deletions default are untested #5849

Description

@JSONbored

Context

src/review/review-diff.ts builds the bounded, hunk-aware unified diff fed to the AI reviewers
(the #1528/#accuracy-gap-1 fix). A local unsharded npm run test:coverage run shows two of its
exported functions have real, reachable untested branches (file overall: 89.33%, 67/75 branches):

  1. keepHighSignalHunks(patch, budget) (line 84): the very first guard,
    if (budget <= 0) return "… (this file's diff truncated)"; (line 85), is never exercised —
    every existing call in test/unit/review-diff.test.ts passes a positive budget (70, 21, 20, or the
    default DEFAULT_DIFF_BUDGET). buildUnifiedReviewDiff never calls it with a non-positive
    remainder either, since it only calls keepHighSignalHunks after checking
    remaining < 240 → break first — so this guard is reachable only via a direct call to
    keepHighSignalHunks with budget <= 0, which no test does.
  2. buildUnifiedReviewDiff(files, budget)'s per-file header
    (line 133: `### ${file.path} (${status}) +${file.additions ?? 0}/-${file.deletions ?? 0}\n`):
    every existing test passes explicit additions/deletions numbers on every file, so the ?? 0
    fallback for an omitted additions/deletions (both ReviewDiffFile fields are
    | undefined, and buildAiReviewDiff passes through file.additions/file.deletions from
    listPullRequestFiles, which can be null/undefined for some GitHub file-list rows) is never
    taken.

Requirements

Add test cases to test/unit/review-diff.test.ts:

  1. keepHighSignalHunks("some patch text", 0) and a negative budget (e.g. -5) both return the
    literal "… (this file's diff truncated)" string without ever inspecting patch.
  2. buildUnifiedReviewDiff([{ path: "a.ts", patch: "@@\n+x", status: "modified" }]) (no additions/
    deletions provided) produces a header containing +0/-0.

Deliverables

  • New it(...) block(s) covering keepHighSignalHunks's non-positive-budget guard.
  • New it(...) block covering buildUnifiedReviewDiff's additions ?? 0 / deletions ?? 0
    fallback when both fields are omitted from a ReviewDiffFile.

Test Coverage Requirements

src/review/review-diff.ts is scored by the 99%+ codecov/patch gate on any future touching PR.
This closes the two specific ?? 0 / early-return branches identified above (out of the file's 8
currently-uncovered branches; the remainder — splitHunks's empty-preamble edge and
keepHighSignalHunks's keep.size === 0 fallback — may or may not be reachable given the file's own
call graph and are left for a follow-up investigation rather than asserted here).

Expected Outcome

keepHighSignalHunks's non-positive-budget short-circuit and buildUnifiedReviewDiff's
additions/deletions defaulting are both exercised and asserted, closing two real, reachable branch
gaps in the diff-truncation logic the AI reviewers depend on.

Links & Resources

  • src/review/review-diff.ts (lines 84-85, 133)
  • test/unit/review-diff.test.ts (file to extend)

Metadata

Metadata

Assignees

No one assigned

    Labels

    gittensor:bugGittensor-scored bug fix — scores a 0.05x multiplier.help wantedExtra attention is needed

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions