Skip to content

fix(orb): degrade getOrbGlobalStats to zeros on a DB error - #8936

Merged
loopover-orb[bot] merged 1 commit into
JSONbored:mainfrom
jeffrey701:fix-8879-orb-global-stats-failsafe-v2
Jul 26, 2026
Merged

fix(orb): degrade getOrbGlobalStats to zeros on a DB error#8936
loopover-orb[bot] merged 1 commit into
JSONbored:mainfrom
jeffrey701:fix-8879-orb-global-stats-failsafe-v2

Conversation

@jeffrey701

Copy link
Copy Markdown
Contributor

What

getOrbGlobalStats (src/orb/outcomes.ts) runs in
Promise.all([getOrbGlobalStats(env), computeFleetAnalytics(env)]) in
src/review/public-stats.ts. Its sibling computeFleetAnalytics wraps its DB reads in try/catch and
degrades to an all-null/zero report on any D1 error — but getOrbGlobalStats had no such guard, so a
D1 error on its join query (the exact "exceeded its CPU time limit and was reset" failure the function's
own header documents) threw straight out of the Promise.all. The only net was the route-level catch,
which then 503s the entire /v1/public/stats payload (accuracyTrend, reuseRateTrend,
reviewVolumeTrend, rulePrecision all lost) instead of degrading just the orb aggregate.

Change

Wrap the query in try/catch, returning { merged: 0, closed: 0, total: 0 } on failure — matching
computeFleetAnalytics's degrade-gracefully posture exactly, so a D1 error drops only the orb aggregate
rather than the whole stats payload.

Validation

  • New test in test/integration/orb-outcomes.test.ts: an injected DB whose first() rejects makes
    getOrbGlobalStats resolve to { merged: 0, closed: 0, total: 0 } rather than throw.
  • npx vitest run test/integration/orb-outcomes.test.ts → 11/11 pass; the new catch branch is covered
    (lcov-verified), the existing success paths remain covered.

Closes #8879

@jeffrey701
jeffrey701 requested a review from JSONbored as a code owner July 26, 2026 13:48
@superagent-security

Copy link
Copy Markdown
Contributor

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

@codecov

codecov Bot commented Jul 26, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.55%. Comparing base (dd9676a) to head (8725175).
⚠️ Report is 4 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #8936      +/-   ##
==========================================
- Coverage   90.56%   90.55%   -0.01%     
==========================================
  Files          96       97       +1     
  Lines       22490    22508      +18     
  Branches     3884     3893       +9     
==========================================
+ Hits        20367    20383      +16     
  Misses       1945     1945              
- Partials      178      180       +2     
Flag Coverage Δ
backend 88.88% <100.00%> (?)

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

Files with missing lines Coverage Δ
src/orb/outcomes.ts 88.88% <100.00%> (ø)

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

loopover-orb Bot commented Jul 26, 2026

Copy link
Copy Markdown
Contributor

Tip

✅ LoopOver review result - approve/merge recommended

Review updated: 2026-07-26 13:59:18 UTC

2 files · 1 AI reviewer · no blockers · readiness 93/100 · CI green · clean

✅ Suggested Action - Approve/Merge

  • safe to merge

Review summary
This wraps the D1 query in getOrbGlobalStats in try/catch to return zeros on error, mirroring computeFleetAnalytics's degrade-gracefully pattern, so a D1 timeout on the orb join no longer 503s the entire /v1/public/stats payload via the route-level catch in public-stats.ts. The fix is correctly scoped to just the query (not the whole function body), preserves the existing nullish-row guard, and is tied to an open issue (#8879). The new test injecting a rejecting first() correctly exercises the real catch branch by mocking at the DB driver boundary, which is the right layer to fabricate a D1 failure.

Nits — 4 non-blocking
  • The second new test ('returns zeros when the query resolves without a row') exercises the pre-existing `if (!row) return...` guard which is marked `/* v8 ignore next */` for coverage purposes — this test is redundant with that ignore directive and doesn't add meaningful signal beyond documentation.
  • The inline comment above the try block is quite long (3+ lines) for a single guard; consider trimming it since the JSDoc above the function already covers the incident history.
  • Consider whether the `merged`/`closed`/`total` zero-degrade should be distinguishable from a genuine zero-count (e.g. via a log/metric on the catch) so a persistent D1 failure doesn't silently look like 'no PRs reviewed' on the public homepage.
  • Since computeFleetAnalytics's catch is being cited as the mirrored pattern, verify it also logs/reports the error somewhere (e.g. console.error or a metric) and align getOrbGlobalStats's catch block similarly if so.

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 #8879
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: 76 registered-repo PR(s), 35 merged, 18 issue(s).
Contributor context ✅ Confirmed Gittensor contributor jeffrey701; Gittensor profile; 76 PR(s), 18 issue(s).
Improvement ✅ Minor risk: clean · value: minor · LLM: moderate
Linked issue satisfaction

Addressed
The diff wraps getOrbGlobalStats's query in try/catch returning {merged:0, closed:0, total:0} on failure, exactly matching computeFleetAnalytics's degrade posture, and adds a new integration test that injects a rejecting DB call and asserts the zero-value result rather than an unhandled rejection.

Review context
  • Author: jeffrey701
  • 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: 76 PR(s), 18 issue(s).
  • PR-specific overlap: none found.
Contributor next steps
  • Start here: Triage stale or unlinked PRs.
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

@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 ad5f3bf into JSONbored:main Jul 26, 2026
8 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.

fix(orb): getOrbGlobalStats has no fail-safe, unlike the sibling it's always called alongside

1 participant