Skip to content

test(review): cover review-diff.ts non-positive-budget guard and header count defaults - #5857

Merged
loopover-orb[bot] merged 1 commit into
JSONbored:mainfrom
jaytbarimbao-collab:test-review-diff-budget-defaults
Jul 14, 2026
Merged

test(review): cover review-diff.ts non-positive-budget guard and header count defaults#5857
loopover-orb[bot] merged 1 commit into
JSONbored:mainfrom
jaytbarimbao-collab:test-review-diff-budget-defaults

Conversation

@jaytbarimbao-collab

Copy link
Copy Markdown
Contributor

Fixes #5849.

src/review/review-diff.ts had untested branches around its budget/size handling. This adds direct unit tests (no production change) for:

  • keepHighSignalHunks non-positive budget guardbudget <= 0 (zero and negative) returns the truncation marker.
  • single-hunk head-slice — a patch with no second @@ header (hunks.length <= 1) that exceeds the budget is head-sliced with the truncation marker, not dropped whole.
  • buildUnifiedReviewDiff header defaults — a file with missing additions/deletions renders +0/-0, and a missing status defaults to modified; provided counts are used when present.
  • below-floor truncation — once the remaining budget falls under the per-file floor, the diff stops with the …diff truncated (N files total) notice.
  • patch-less file listing — a binary/too-large file is still listed with its counts.

Branch coverage of the file rises from 66.7% to 77.3%. Test-only; all existing tests still pass (19 total).

…er count defaults

Fixes JSONbored#5849.

keepHighSignalHunks's budget<=0 guard, its single-hunk head-slice branch,
and buildUnifiedReviewDiff's +0/-0 additions/deletions defaults, missing-status
default, and below-floor truncation notice had no direct coverage. Adds unit
tests for each (branch coverage 66.7%->77.3%).
@superagent-security

Copy link
Copy Markdown
Contributor

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

@codecov

codecov Bot commented Jul 14, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 74.75%. Comparing base (db74aa3) to head (dde8fc4).
⚠️ Report is 10 commits behind head on main.

❗ There is a different number of reports uploaded between BASE (db74aa3) and HEAD (dde8fc4). Click for more details.

HEAD has 3 uploads less than BASE
Flag BASE (db74aa3) HEAD (dde8fc4)
shard-5 1 0
shard-3 1 0
shard-4 1 0
Additional details and impacted files
@@             Coverage Diff             @@
##             main    #5857       +/-   ##
===========================================
- Coverage   95.11%   74.75%   -20.37%     
===========================================
  Files         586      586               
  Lines       46480    46480               
  Branches    14860    14860               
===========================================
- Hits        44209    34744     -9465     
- Misses       1515     8682     +7167     
- Partials      756     3054     +2298     
Flag Coverage Δ
shard-1 43.31% <ø> (-0.42%) ⬇️
shard-2 36.19% <ø> (-0.05%) ⬇️
shard-3 ?
shard-4 ?
shard-5 ?
shard-6 44.89% <ø> (+0.35%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.
see 282 files with indirect coverage changes

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@loopover-orb loopover-orb Bot added the gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier. label Jul 14, 2026
@loopover-orb

loopover-orb Bot commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

Tip

🟩🟩🟩🟩🟩🟩🟩🟩🟩🟩🟩🟩

✅ LoopOver review result - approve/merge recommended

Review updated: 2026-07-14 20:19:33 UTC

1 file · 1 AI reviewer · no blockers · readiness 100/100 · CI green · clean

✅ Suggested Action - Approve/Merge

  • safe to merge

Review summary
This is a test-only PR that adds direct unit tests for four previously-uncovered branches in review-diff.ts: the keepHighSignalHunks non-positive-budget guard, the single-hunk head-slice path, buildUnifiedReviewDiff's header defaults for missing additions/deletions/status, and the below-floor truncation notice. I traced each new test against the current source in review-diff.ts and all assertions match actual behavior (budget<=0 returns the marker, hunks.length<=1 head-slices with slice(0,budget), header uses `??0`/`??"modified"` defaults, and the loop breaks with the truncation message when remaining<240). No production code is touched.

Nits — 4 non-blocking
  • The below-floor truncation test (test/unit/review-diff.test.ts new describe block) doesn't assert which file triggered the truncation cutoff, just that the message appears — a slightly tighter assertion (checking diff.length or that only one file's header appears) would pin down the exact boundary being exercised.
  • The head-slice test only checks `out.startsWith(single.slice(0, 30))` and that it contains the truncation marker, but doesn't assert the exact expected output string, leaving a small gap versus the more precise equality checks used elsewhere in the file (e.g. the exact-budget-fit test).
  • Consider adding one boundary test at exactly `budget === 0` vs `budget === -1` collapsing into the same code path is already implied by the two guard tests, so no change needed there — this is just an observation, not an ask.
  • The PR could mention the resulting branch-coverage percentage per describe block for clarity in the description, though the aggregate 66.7%→77.3% figure is already given.
Signal Result Evidence
Code review ✅ No blockers 1 reviewer
Linked issue ✅ Linked #5849
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: 75 registered-repo PR(s), 36 merged, 4 issue(s).
Contributor context ✅ Confirmed Gittensor contributor jaytbarimbao-collab; Gittensor profile; 75 PR(s), 4 issue(s).
Gate result ✅ Passing No configured blocker found.
Improvement ⚠️ ℹ️ Insufficient signal risk: clean · value: insufficient-signal — Nothing measurable for the structural-improvement analyzers on this PR (e.g. no code files changed). LLM value judgment: moderate — The PR adds real, source-traced unit tests for genuinely untested branches (non-positive budget guard, single-hunk head-slice, header defaults, floor truncation) in a hot-path diff-building module, raising branch coverage by ~10.6 points with no risk since it's test-only.
Linked issue satisfaction

Addressed
The PR adds direct test cases for both requested gaps — keepHighSignalHunks with budget 0 and negative returning the truncation marker, and buildUnifiedReviewDiff producing +0/-0 for a file missing additions/deletions — matching the issue's exact requirements, plus additional related coverage.

Review context
  • Author: jaytbarimbao-collab
  • 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: 75 PR(s), 4 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.
[BETA] Chat with Gittensory

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

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

Full command reference: https://gittensory.aethereal.dev/docs/gittensory-commands

🟩 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

@loopover-orb loopover-orb Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@loopover-orb
loopover-orb Bot merged commit cf63f1c into JSONbored:main Jul 14, 2026
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

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

1 participant