fix(services): bound the reviewer_vote track-record read to the newest N votes - #10152
Conversation
…t N votes loadLiveProviderTrackRecords ran an unbounded SELECT over a 90-day slice of audit_events (raw rows, a metadata_json blob each) on every block-mode dual review, and swallowed any failure into []. An oversized result set throws, lands in that catch, and becomes 'no track records' — which computeWouldHaveRouted reads as 'below the decided floor' and indistinguishable from the legitimate no-signal case the module's invariants require. So the stage-1 shadow silently stops recording exactly as the ledger grows large enough for stage 2 to be worth shipping against, with nothing anywhere saying so. Bound the scan to the newest REVIEWER_VOTE_SCAN_LIMIT rows via a newest-first subquery re-ordered ascending in the outer projection (a naive ASC ... LIMIT would keep the OLDEST rows and invert the latest-vote-wins dedup JSONbored#9638 established). Warn reviewer_vote_scan_truncated at a full page and reviewer_vote_scan_failed in the catch before failing safe, so a truncated or failed read is observable rather than silently under-counting. computeWouldHaveRouted, the corpus lookback window, the corrupt-row skip, and the fail-safe [] posture are all unchanged. Closes JSONbored#10023
|
Tip ✅ LoopOver review result - approve/merge recommendedReview updated: 2026-07-31 09:53:06 UTC
Review summary Nits — 5 non-blocking
Decision drivers
Context & advisory signals — never blocks the verdict
Linked issue satisfactionAddressed Review context
Contributor next steps
Signal definitions
🧪 Chat with LoopOverAsk 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.
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.
|
|
Superagent didn't find any vulnerabilities or security issues in this PR. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #10152 +/- ##
===========================================
- Coverage 92.05% 80.14% -11.92%
===========================================
Files 931 283 -648
Lines 114034 58781 -55253
Branches 27537 8688 -18849
===========================================
- Hits 104977 47109 -57868
- Misses 7759 11381 +3622
+ Partials 1298 291 -1007
Flags with carried forward coverage won't be shown. Click here to find out more.
|
What & why
loadLiveProviderTrackRecordsran an unboundedSELECTover a 90-day slice ofaudit_events— raw rows with ametadata_jsonblob each — on every block-mode dual review, and swallowed any failure into[]:An oversized result set is a thrown driver error, so it lands in that
catchand becomes "no track records" — whichcomputeWouldHaveRoutedreads as "below the decided floor" and returnsnullfor, indistinguishable from the legitimate no-signal case the module's own invariant requires ("absence of a record must mean 'no measurable preference'"). So the stage-1 shadow silently stops recording exactly as the ledger grows large enough for stage 2 to be worth shipping against, and nothing says so. This is the same table whose unbounded growth already hit a size cap once (src/db/retention.ts:479-483).The fix
REVIEWER_VOTE_SCAN_LIMIT(2,000) rows. A naiveASC … LIMIT nwould keep the oldest rows and invert the latest-vote-wins dedup (orb(routing): the reviewer-vote read has no ORDER BY #9638), so the read selects newest-first in a subquery (ORDER BY created_at DESC, id DESC LIMIT ?) and re-orders the outer projectionASC, id ASC, socomputeProviderTrackRecordsstill receives ascending order.rows.length === REVIEWER_VOTE_SCAN_LIMIT), emit oneconsole.warnevent: "reviewer_vote_scan_truncated"— a truncated corpus is observable rather than silently under-counted.catch, emit oneconsole.warnevent: "reviewer_vote_scan_failed"(with the error) before returning[], so a read failure is distinguishable from a genuinely empty ledger. The fail-safe[]posture is unchanged.Unchanged:
computeWouldHaveRouted,ROUTING_MIN_DECIDED,recordRoutingShadow,REVIEWER_VOTE_EVENT_TYPE,CORPUS_LOOKBACK_MS, the corrupt-row skip, and the vote-shape filter.Tests (
test/unit/reviewer-routing.test.ts)reviewer_vote_scan_failedand returns[], andrecordRoutingShadowstill resolvesnullwithout touching the review path.REVIEWER_VOTE_SCAN_LIMIT + 5votes: the newest vote's provider survives; a provider whose only vote is in the oldest overflow is absent — proving the newest-first bound.reviewer_vote_scan_truncated; a shorter read does not (both arms of the truncation branch).main(unbounded read, no warns).Validation
src/services/reviewer-routing.tsis 100% line and branch.npm run typecheckclean for these files;npm run engine-parity:drift-checkpasses (host-only file, not a twin);npm run dead-exports:checkclean; the reviewer-routing suite (14 tests) green.git diff --checkclean; no schema/migration/generated-artifact change.Closes #10023