Skip to content

fix(orb): report null mergeRate for a zero-sample slop band - #9921

Merged
loopover-orb[bot] merged 1 commit into
JSONbored:mainfrom
bitfathers94:fix/issue-9641
Jul 29, 2026
Merged

fix(orb): report null mergeRate for a zero-sample slop band#9921
loopover-orb[bot] merged 1 commit into
JSONbored:mainfrom
bitfathers94:fix/issue-9641

Conversation

@bitfathers94

Copy link
Copy Markdown
Contributor

Summary

buildSlopOutcomeCalibration fabricated a mergeRate: 0 for a slop band with zero
resolved PRs, which for a per-band discrimination table reads as "every PR in this
band was closed" — the strongest possible claim the table can make. overallMergeRate
five lines later already treats an empty pool as null; this makes the per-band figure
consistent with it.

  • SlopBandCalibration.mergeRate is now number | null; buildSlopOutcomeCalibration
    returns null for any band with sampleSize === 0 (src/services/outcome-calibration.ts).
  • computeDiscriminates narrows the sampled-bands array with an isSampledBand type
    guard instead of comparing a nullable field directly — behaviour is unchanged (a
    sampled band's sampleSize >= MIN_BAND_SAMPLE already guarantees a non-null rate).
  • The MCP CLI's maintain outcome-calibration renderer (packages/loopover-mcp/bin/loopover-mcp.ts)
    already had a shared n/a (below sample) guard for null/undefined rates — it just
    never had a null per-band mergeRate to actually exercise. Added a regression test
    that drives it and confirms the plain-text output reads n/a, not 0%.
  • src/openapi/schemas.ts's OutcomeCalibrationResponse.slop was z.unknown() (no
    per-band shape modeled at all); gave it the real structured shape with a nullable
    mergeRate, and regenerated apps/loopover-ui/public/openapi.json via npm run ui:openapi.
  • Widened the UI card's local SlopBandCalibration type to match (apps/loopover-ui/src/components/site/app-panels/slop-band-calibration-card.tsx)
    — its render logic already gated on sampleSize > 0, not on mergeRate itself, so no
    behaviour change.

Not a UI-visible change: the card already showed "— no samples" for a zero-sample band
via the sampleSize gate, so there's no rendered difference to screenshot.

Test plan

  • npx vitest run test/unit/outcome-calibration.test.ts — new regression test asserts a
    corpus with zero high-band resolved PRs produces mergeRate: null for that band, and
    that computeDiscriminates's verdict over the three sampled bands is unaffected.
  • npx vitest run test/unit/mcp-cli-maintain.test.ts — new regression test asserts the
    CLI's plain-text output renders a null-mergeRate band as n/a, not 0%.
  • npx vitest run test/unit/openapi.test.ts — route/schema parity checks still pass.
  • npm run ui:openapi:check — regenerated artifact is stable.

Closes #9641

UI Evidence

Viewport · Theme Before After
Desktop · Dark Desktop · Dark before
before
Desktop · Dark after
after
Tablet · Dark Tablet · Dark before
before
Tablet · Dark after
after
Mobile · Dark Mobile · Dark before
before
Mobile · Dark after
after

buildSlopOutcomeCalibration fabricated a 0% merge rate for a slop band with no
resolved PRs, which for a discrimination table reads as "every PR in this band
was closed" -- the strongest possible claim the table can make. Mirror
overallMergeRate's existing null-for-empty treatment: SlopBandCalibration.mergeRate
is now number | null, computeDiscriminates narrows sampled bands with a type
guard instead of comparing against a nullable field, and the MCP CLI's shared
n/a-below-sample renderer now actually exercises its null arm for a per-band
rate. Widened the OpenAPI response shape and the UI card's type to match.
@bitfathers94
bitfathers94 requested a review from JSONbored as a code owner July 29, 2026 23:09
@superagent-security

Copy link
Copy Markdown
Contributor

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

@loopover-orb

loopover-orb Bot commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

Tip

✅ LoopOver review result - approve/merge recommended

Review updated: 2026-07-29 23:24:12 UTC

8 files · 1 AI reviewer · no blockers · readiness 98/100 · CI green · clean

✅ Suggested Action - Approve/Merge

  • safe to merge

Review summary
This PR fixes a real bug: buildSlopOutcomeCalibration previously reported mergeRate: 0 for a zero-sample slop band, which reads as "every PR in this band was closed" rather than "no data." The fix threads number | null through the type, the OpenAPI schema, the UI card, and the CLI renderer consistently, and the regression tests actually exercise the new null path (the CLI test drives a fixture through the real renderer, and the calculation test asserts mergeRate is null with discriminates unaffected). The isSampledBand type guard and the UI's `row.mergeRate ?? 0` fallback are both correctly reasoned as unreachable-but-required-by-the-wider-type, which the comments explain accurately. It closes issue #9641 as described.

Nits — 3 non-blocking
  • apps/loopover-ui/src/components/site/app-panels/slop-band-calibration-card.tsx:41 — the `row.mergeRate ?? 0` fallback is dead code since hasSamples already guarantees non-null; consider a non-null assertion (`row.mergeRate!`) instead of a silent fallback that could mask a future contract violation.
  • src/openapi/schemas.ts diff comment references 'orb(calibration): an empty slop band reports `mergeRate: 0 #9641' inline — verify that issue number matches the actual linked issue for this PR.
  • Since `hasSamples` and `mergeRate != null` are meant to always agree, a runtime assertion or dev-only invariant check in BandRow could catch a future server-side regression faster than a silent 0% fallback would.

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 #9641
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: 111 registered-repo PR(s), 77 merged, 1 issue(s).
Contributor context ✅ Confirmed Gittensor contributor bitfathers94; Gittensor profile; 111 PR(s), 1 issue(s).
Improvement ✅ Minor risk: clean · value: minor · LLM: minor
Linked issue satisfaction

Addressed
The PR widens SlopBandCalibration.mergeRate to number|null, returns null for zero-sample bands mirroring overallMergeRate's existing shape, makes computeDiscriminates null-safe via a type guard without changing its filtering behavior, updates the MCP renderer's existing n/a guard with a regression test, and updates src/openapi/schemas.ts plus the regenerated openapi.json artifact and UI card type.

Review context
  • Author: bitfathers94
  • Role context: outside_contributor
  • Public audience mode: oss maintainer
  • Lane context: Repository is configured for direct PR review.
  • Public profile languages: TypeScript
  • Official Gittensor activity: 111 PR(s), 1 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.
🧪 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 <question> answers contribution-quality Q&A with source citations and freshness.
  • @loopover chat <question> 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.

Visual preview
Route Viewport Before (production) After (this PR's preview) Diff
/ desktop before /
before /
after /
after /
/ mobile before / (mobile)
before / (mobile)
after / (mobile)
after / (mobile)

Click any thumbnail to open the full-size screenshot. Before = production · After = this PR's preview deploy.

Scroll preview
Route Before (production) After (this PR's preview)
/ before / (scroll)
before / (scroll)
after / (scroll)
after / (scroll)

A short scroll-through clip (desktop) — click either thumbnail to open the full animation. Evidence for scroll-linked behavior a single screenshot can't show.

🟩 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 added the gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier. label Jul 29, 2026
@codecov

codecov Bot commented Jul 29, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 79.25%. Comparing base (4b57927) to head (c3ff97d).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #9921      +/-   ##
==========================================
- Coverage   79.52%   79.25%   -0.27%     
==========================================
  Files         281      284       +3     
  Lines       58577    61033    +2456     
  Branches     6833     7683     +850     
==========================================
+ Hits        46581    48374    +1793     
- Misses      11706    12028     +322     
- Partials      290      631     +341     
Flag Coverage Δ
backend 73.00% <100.00%> (?)

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

Files with missing lines Coverage Δ
packages/loopover-mcp/bin/loopover-mcp.ts 70.00% <ø> (ø)
src/openapi/schemas.ts 100.00% <ø> (ø)
src/services/outcome-calibration.ts 98.55% <100.00%> (ø)

@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.

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

@loopover-orb
loopover-orb Bot merged commit c5a6cb4 into JSONbored:main Jul 29, 2026
10 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.

orb(calibration): an empty slop band reports `mergeRate: 0

1 participant