Skip to content

test(review): cover extractReviewSummary suggestions-merge and sole-reviewer-consensus branches#6673

Merged
JSONbored merged 1 commit into
JSONbored:mainfrom
ultrahighsuper:test/unified-comment-coverage-6634
Jul 16, 2026
Merged

test(review): cover extractReviewSummary suggestions-merge and sole-reviewer-consensus branches#6673
JSONbored merged 1 commit into
JSONbored:mainfrom
ultrahighsuper:test/unified-comment-coverage-6634

Conversation

@ultrahighsuper

Copy link
Copy Markdown
Contributor

What

extractReviewSummary (src/review/unified-comment.ts, lines 130-143) had two untested branches:

  • Line 136 merges each reviewer's suggestions array into the rendered nits list — never exercised, because the shared test helper always defaulted suggestions: [].
  • Line 140's documented "a sole reviewer's blocker still counts as consensus" rule (valid.length === 1 && reviewersWithBlockers === 1) was reachable but never asserted on .consensusBlocker.

Resolves #6634. Test-only — no production code change (extractReviewSummary's logic is not in question, only its coverage).

Tests added (test/unit/unified-comment.test.ts)

  • A reviewer's non-empty suggestions array is merged into the output nits (alongside an explicit nit).
  • A sole valid reviewer reporting a blocker yields consensusBlocker: true (and a blocked status).
  • consensusBlocker stays false when no reviewer reports a blocker.

Locally green: npx vitest run test/unit/unified-comment.test.ts → 101/101. Since there's no src/** change, the codecov patch gate has no diff to score; these tests close the coverage gap on the already-shipped branches.

…-reviewer-consensus branches (JSONbored#6634)

`extractReviewSummary` (src/review/unified-comment.ts) had two untested branches:
its `suggestions`-into-`nits` merge (line 136) was never exercised because the
shared test helper always defaulted `suggestions: []`, and its documented
"a sole reviewer's blocker still counts as consensus" rule
(`valid.length === 1 && reviewersWithBlockers === 1`, line 140) was reachable but
never asserted on `.consensusBlocker`.

Adds three cases to test/unit/unified-comment.test.ts (test-only, no production
change): a reviewer's non-empty `suggestions` array is merged into the output
`nits`; a sole valid reviewer reporting a blocker yields `consensusBlocker: true`
(and a blocked status); and `consensusBlocker` stays `false` when no reviewer
reports a blocker.

Closes JSONbored#6634
@superagent-security

Copy link
Copy Markdown
Contributor

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

@codecov

codecov Bot commented Jul 16, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 89.99%. Comparing base (34583a2) to head (b50e236).
⚠️ Report is 15 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #6673      +/-   ##
==========================================
- Coverage   93.65%   89.99%   -3.66%     
==========================================
  Files         681      681              
  Lines       68099    68108       +9     
  Branches    18699    18701       +2     
==========================================
- Hits        63777    61296    -2481     
- Misses       3347     5288    +1941     
- Partials      975     1524     +549     
Flag Coverage Δ
shard-1 43.92% <ø> (+<0.01%) ⬆️
shard-2 36.95% <ø> (+<0.01%) ⬆️
shard-3 32.40% <ø> (+<0.01%) ⬆️
shard-4 34.14% <ø> (-0.02%) ⬇️
shard-5 ?
shard-6 45.77% <ø> (+<0.01%) ⬆️

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

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

loopover-orb Bot commented Jul 16, 2026

Copy link
Copy Markdown

Tip

✅ LoopOver review result - approve/merge recommended

Review updated: 2026-07-16 18:34:37 UTC

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

✅ Suggested Action - Approve/Merge

  • safe to merge

Review summary
This is a pure test-addition PR that adds three unit tests exercising two previously-untested branches of extractReviewSummary: the suggestions-array-merges-into-nits path and the sole-reviewer-blocker-counts-as-consensus path. The tests use the existing reviewNote/buildUnifiedReviewInput helpers with plausible inputs (a non-empty suggestions array, a single review with a blocker) and assert on the documented behavior, so they appear to exercise real, reachable code paths rather than fabricated states. No production code changes, CI is green across all checks, and the PR explicitly ties itself to open issue #6634.

Nits — 3 non-blocking
  • The third test (no reviewer reports a blocker) isn't explicitly called out in the PR description's bullet list as tied to a specific line number, though it's a reasonable complementary negative-case assertion.
  • Inline comments like `// free-form suggestion merged in` and `// valid.length === 1 && reviewersWithBlockers === 1` restate what the assertion already shows via test naming, but they aid quick scanning of coverage intent so this is minor.
  • Consider a follow-up test asserting nits ordering/dedup behavior when both suggestions and explicit nits overlap in content, if that's a real edge case in extractReviewSummary.

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 #6634
Related work ✅ No active overlap found No same-issue or scoped active PR overlap found.
Change scope ❌ 8/20 High 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: 120 registered-repo PR(s), 72 merged, 21 issue(s).
Contributor context ✅ Confirmed Gittensor contributor ultrahighsuper; Gittensor profile; 120 PR(s), 21 issue(s).
Improvement ℹ️ Insufficient signal risk: clean · value: insufficient-signal
Review context
  • Author: ultrahighsuper
  • 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: 120 PR(s), 21 issue(s).
  • PR-specific overlap: none found.
Contributor next steps
  • Start here: Add a concise scope and risk note.
  • Then work through the remaining 1 step in the Signals table above.
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 &lt;question&gt; answers contribution-quality Q&A with source citations and freshness.
  • @loopover chat &lt;question&gt; 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

@JSONbored
JSONbored merged commit 3502bbb into JSONbored:main Jul 16, 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. manual-review Gittensor contributor context

Projects

None yet

Development

Successfully merging this pull request may close these issues.

extractReviewSummary's suggestions-merge and sole-reviewer-consensus branches are untested

2 participants