Skip to content

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

Closed
jeffrey701 wants to merge 1 commit into
JSONbored:mainfrom
jeffrey701:fix-8879-orb-global-stats-failsafe
Closed

fix(orb): degrade getOrbGlobalStats to zeros on a DB error#8912
jeffrey701 wants to merge 1 commit into
JSONbored:mainfrom
jeffrey701:fix-8879-orb-global-stats-failsafe

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:22
@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

❌ Patch coverage is 75.00000% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 90.55%. Comparing base (8a3d2c9) to head (f9ec1f1).
⚠️ Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
src/orb/outcomes.ts 75.00% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #8912      +/-   ##
==========================================
- Coverage   93.84%   90.55%   -3.29%     
==========================================
  Files         803       97     -706     
  Lines       80084    22508   -57576     
  Branches    24277     3893   -20384     
==========================================
- Hits        75157    20383   -54774     
+ Misses       3562     1945    -1617     
+ Partials     1365      180    -1185     
Flag Coverage Δ
backend 88.88% <75.00%> (-6.25%) ⬇️

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% <75.00%> (+1.38%) ⬆️

... and 706 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 26, 2026
@loopover-orb

loopover-orb Bot commented Jul 26, 2026

Copy link
Copy Markdown
Contributor

Caution

🛑 LoopOver review result - fixes required

Review updated: 2026-07-26 13:35:05 UTC

2 files · 1 AI reviewer · no blockers · CI failing · unstable

🛑 Suggested Action - Fix Blockers

Review summary
This PR wraps the getOrbGlobalStats D1 query in try/catch, returning zeroed stats on failure instead of throwing out of Promise.all in public-stats.ts, exactly mirroring computeFleetAnalytics's existing degrade-gracefully pattern. The fix targets the real source of the bug (the missing guard at the call site that produces the error) rather than patching a symptom, and the new test correctly exercises the failure path via a rejecting first() rather than fabricating an unreachable state. This closes #8879 and is a narrow, well-targeted operator-facing fix.

Nits — 4 non-blocking
  • codecov/patch failed at 75% vs 99% target — likely the catch block's three lines aren't fully branch-covered even though the test exercises the reject path; worth checking the lcov report to see which line is uncovered.
  • The duplicated three-line rationale comment inside the catch block (src/orb/outcomes.ts) restates the PR description almost verbatim — could be trimmed to a one-liner referencing the JSDoc above.
  • Consider extracting the shared degrade-to-zeros literal `{ merged: 0, closed: 0, total: 0 }` into a small constant since it now appears twice in the same function (src/orb/outcomes.ts), matching the pattern in computeFleetAnalytics if it does the same.
  • Double check the codecov/patch failure isn't from an uncovered branch in the new try/catch before merging, since the target is 99% and this diff landed at 75%.

CI checks failing

  • codecov/patch — 75.00% of diff hit (target 99.00%)

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 ⚠️ 1 scoped overlap Top overlaps are listed below; lower-confidence bulk is hidden.
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: 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 the DB query in getOrbGlobalStats in a try/catch that returns {merged:0, closed:0, total:0} on failure, matching computeFleetAnalytics's degrade-gracefully behavior, and adds a test that injects a rejecting DB call and asserts the zero-value degraded result.

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: Rust, TypeScript, Clojure, JavaScript
  • Official Gittensor activity: 76 PR(s), 18 issue(s).
  • Related work: Titles/paths share 3 meaningful terms. (PR #8919)
Contributor next steps
  • Start here: Review top overlaps.
  • Then work through the remaining 3 steps 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

@loopover-orb

loopover-orb Bot commented Jul 26, 2026

Copy link
Copy Markdown
Contributor

LoopOver is closing this pull request on the maintainer's behalf (CI is failing (codecov/patch)). This is an automated maintenance action — to pursue this change, please open a new pull request with the issues resolved. Closed PRs may be analyzed later to improve review accuracy, but they are not automatically reopened or re-reviewed.

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