Skip to content

orb(review): the unified comment's blocker list is silently capped at 12 #9670

Description

@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/unified-comment.ts builds the contributor-facing "Why this is blocked" section:

function dedupeLines(items: string[], cap = 12): string[] {          // :494

const blockersAll = dedupeLines(input.blockers ?? []);                // :775
const blockersTrunc = truncateFindingsForDisplay(blockersAll, input.maxFindingsCaps?.blockers);   // :776

// The FULL (pre-display-truncation) blocker set, not blockersTrunc.shown -- an AI agent benefits from
// every blocker, not just the human-scannable capped subset shown above.                        // :783-785
blocks.push(buildAiContextBlock(blockersAll, collapsiblesOpen));      // :786

Three defects follow from blockersAll already being capped at 12:

  1. The comment at :783-785 is false. The "Copy for AI agents" block advertises "every blocker" and
    silently omits blockers 13+.
  2. truncateFindingsForDisplay only ever sees at most 12 items, so its hiddenCount is 0 and
    appendMoreFooter renders no more-items footer for the human list either. The overflow is
    invisible on both surfaces.
  3. The counts disagree with the list. statusChips (src/review/unified-comment.ts:434) and
    codeReviewRow (:641) both count the RAW array:
    const blockerCount = (input.blockers ?? []).length. A PR with 15 blockers renders a
    "15 blockers" chip above a "Why this is blocked" list of 12 items with no indication that three
    are missing.

The Nits path immediately above (src/review/unified-comment.ts:765-772) has the identical
dedupeLines cap, but its heading label prints the same capped number, so it is at least
self-consistent; the blockers path is not.

Requirements

  • dedupeLines must gain an explicit "no cap" call form (e.g. Number.POSITIVE_INFINITY, or an
    optional cap that defaults to unlimited at the blockers call site) so blockersAll genuinely holds
    every deduped blocker line.
  • truncateFindingsForDisplay must therefore receive the full deduped set, so hiddenCount is real and
    appendMoreFooter renders the more-items footer when the display cap truncates.
  • buildAiContextBlock must receive the full deduped set, matching the comment at
    src/review/unified-comment.ts:783-785.
  • statusChips' and codeReviewRow's blockerCount must be derived from the SAME deduped set the
    list renders from, so the chip can never claim a number the section does not account for.
  • The Nits path must be brought to the same consistency: its summary label and the rendered list must
    agree, with a more-items footer when truncated.
  • The 12-item DISPLAY limit must remain the default when input.maxFindingsCaps supplies nothing — this
    issue moves the cap from "silent data loss" to "display truncation with a disclosed remainder", it does
    not remove the cap.

⚠️ Required pattern: truncateFindingsForDisplay + appendMoreFooter in the same file are already
the codebase's disclosed-truncation mechanism, and formatGateCheckOutput
(src/rules/advisory.ts:944-950) applies the same discipline for the check-run ("#8323: disclose
truncation … a contributor with 9+ genuine blockers must know more exist"). What does NOT satisfy
this issue: raising dedupeLines' default cap to some larger number; removing the display cap
entirely; disclosing the remainder only in the AI-context block and not in the human list; or fixing
the chip count by capping it to 12 rather than making the list disclose its own truncation.

Deliverables

  • The unified-comment body builder in src/review/unified-comment.ts passes all 15 deduped
    blockers to buildAiContextBlock given 15 distinct input blockers, asserted by a new named case
    in test/unit/unified-comment.test.ts.
  • The rendered "Why this is blocked" section for those 15 blockers shows the display-capped subset
    followed by a "+3 more" footer, asserted in the same test.
  • The status chip and the Code-review signal row report a count that equals
    shown.length + hiddenCount, asserted for a case where the raw input contains duplicate lines
    (so dedupe actually changes the number) — a named regression test.
  • The Nits collapsible renders a more-items footer and a summary count consistent with its own list
    for a 15-nit input, asserted by a test.

All Deliverables above are required in a single PR. A PR that satisfies only some of them — for
example uncapping blockersAll (Deliverable 1) without making the chip count agree with the rendered
list (Deliverable 3) — does not resolve this issue.

Test Coverage Requirements

This repo enforces 99%+ Codecov patch coverage, branch-counted, and vitest.config.ts's
coverage.include covers src/**/*.ts — this file is measured. Both arms of every changed conditional
need a test: truncation firing and not firing, hiddenCount > 0 and === 0, deduping changing the
count and not changing it, and the maxFindingsCaps present/absent arms.

Expected Outcome

A contributor with more blockers than the display cap sees an explicit remainder count, the chip count
matches what the section accounts for, and the "Copy for AI agents" block actually contains every
blocker its own comment promises.

Links & Resources

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

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions