Skip to content

review: aggregate comment-size budget guard - #6095

Merged
JSONbored merged 1 commit into
mainfrom
review/comment-size-budget-guard
Jul 15, 2026
Merged

review: aggregate comment-size budget guard#6095
JSONbored merged 1 commit into
mainfrom
review/comment-size-budget-guard

Conversation

@JSONbored

Copy link
Copy Markdown
Owner

Closes #6069. Last item in #6065.

Stacked on #6086 (the per-blocker fix-context PR) -- same file (unified-comment-bridge.ts), and this guard needs to wrap the render call site that PR touches.

Summary

Up to 18 optional collapsibles can appear on one comment (Nits, Copy-for-AI-agents, Flagged checks, Linked issue satisfaction, Review context, Contributor next steps, Signal definitions, Chat, Manifest validation, Auto-merge readiness, Changed files, Finding categories, Impact map, Fix handoff, Visual findings/preview, Scroll preview), each capping only itself (Changed files alone up to 200 rows / 30KB). Nothing checked the aggregate size before handing the body to GitHub — a maximally-featured PR could silently exceed GitHub's real ~65,536-character comment cap and fail to post, with no proactive truncation anywhere in the path.

  • Added renderWithinBudget: renders the comment; if it exceeds a 60,000-char budget (safe headroom under GitHub's real cap, matching the same limit MAX_STORED_BODY_CHARS in src/db/repositories.ts already codifies for incoming bodies), it drops the lowest-priority optional collapsibles from the end of the chain, one at a time, re-rendering and re-measuring each time, until it fits or none are left to drop.
  • "Lowest-priority" = last in buildUnifiedCommentBody's own construction order, which already puts the heaviest/most decorative sections last (Visual preview / Scroll preview's image tables, Changed files' up-to-200-row table) — no new priority list needed, the existing chain order already sorts roughly biggest/most-optional-last.
  • Disposition-relevant content — headline, chips, verdict, blockers, per-blocker AI fix context, decision drivers — is rendered directly by renderUnifiedReviewComment, never part of extraCollapsibles, so this loop cannot drop it, by construction (not just by convention).
  • When trimming happens, a small "Some detail omitted" note collapsible is appended so the omission is visible in the comment itself to whoever reads it, rather than only in an ops log nobody watching the PR would see.

Test plan

  • npx vitest run test/unit/unified-comment-bridge.test.ts (+ the other comment-rendering suites) — 87/87 (+253 total across the related files), including 3 new tests: a normal-sized comment is completely untouched, a 75KB-of-optional-content comment gets trimmed with the note appended and core content intact, and a single collapsible bigger than the whole budget still returns a valid body
  • npx tsc --noEmit clean
  • npm run docs:drift-check, npm run manifest:drift-check, npm run command-reference:check — pass
  • Coverage-checked the exact diff hunks — no uncovered statements/branches within changed lines

@superagent-security

superagent-security Bot commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

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

@JSONbored
JSONbored force-pushed the review/comment-blocker-fix-context branch from 4de0443 to 6b99f6b Compare July 15, 2026 09:13
@JSONbored
JSONbored force-pushed the review/comment-size-budget-guard branch from ebcb765 to 6f67ba8 Compare July 15, 2026 09:14
@JSONbored JSONbored self-assigned this Jul 15, 2026
Base automatically changed from review/comment-blocker-fix-context to review/comment-decision-driver-chips July 15, 2026 09:23
@JSONbored
JSONbored force-pushed the review/comment-decision-driver-chips branch from dca092b to c1f0695 Compare July 15, 2026 09:26
A maximally-featured PR could carry up to 18 optional collapsibles
(each capping only itself -- e.g. Changed files alone up to 200 rows /
30KB) with no shared budget across them, and no check anywhere before
the body was handed to GitHub -- a comment over GitHub's real
~65536-char cap would just silently fail to post.

Added renderWithinBudget: renders the comment, and if it exceeds a
60,000-char budget, drops the LOWEST-priority optional collapsibles
one at a time from the end of the chain (the construction order
already puts the heaviest/most decorative sections -- Visual preview,
Scroll preview, Changed files -- last) until it fits. Disposition-
relevant content (headline, verdict, blockers, per-blocker AI fix
context, decision drivers) is rendered directly by
renderUnifiedReviewComment, never part of extraCollapsibles, so it can
never be dropped. When trimming happens, a small note is appended to
the comment itself so the omission is visible to whoever reads it.

Closes #6069
@codecov

codecov Bot commented Jul 15, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 95.24%. Comparing base (c1f0695) to head (cd05d7f).
⚠️ Report is 2 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #6095   +/-   ##
=======================================
  Coverage   95.24%   95.24%           
=======================================
  Files         595      595           
  Lines       47062    47073   +11     
  Branches    15021    15025    +4     
=======================================
+ Hits        44824    44835   +11     
  Misses       1493     1493           
  Partials      745      745           
Flag Coverage Δ
shard-1 43.97% <50.00%> (+<0.01%) ⬆️
shard-2 36.61% <45.00%> (-0.01%) ⬇️
shard-3 31.98% <45.00%> (+<0.01%) ⬆️
shard-4 33.84% <100.00%> (+0.01%) ⬆️
shard-5 31.46% <45.00%> (+<0.01%) ⬆️
shard-6 44.84% <35.00%> (-0.01%) ⬇️

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

Files with missing lines Coverage Δ
src/review/unified-comment-bridge.ts 99.60% <100.00%> (+0.01%) ⬆️

@JSONbored
JSONbored force-pushed the review/comment-size-budget-guard branch from 6f67ba8 to cd05d7f Compare July 15, 2026 09:27
Base automatically changed from review/comment-decision-driver-chips to main July 15, 2026 09:36
@JSONbored
JSONbored merged commit eec3721 into main Jul 15, 2026
14 checks passed
@JSONbored
JSONbored deleted the review/comment-size-budget-guard branch July 15, 2026 09:41
@loopover-orb loopover-orb Bot added the gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier. label Jul 15, 2026
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.

review: aggregate size-budget guard across optional comment collapsibles

1 participant