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
@github-actions github-actions Bot mentioned this pull request Jul 30, 2026
12 tasks
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