Skip to content

services(reviewer-routing): bound the 90-day reviewer_vote read, whose overflow is swallowed into "no evidence" #10023

Description

@JSONbored

⚠️ Definition of Done: this issue must be completed in full, in a single PR. Do not split this
work across multiple PRs, and do not defer any Deliverable below to a follow-up issue. A PR that
satisfies only some of the Deliverables, stubs a required test, or leaves a checkbox
partially-done does NOT resolve this issue and will be closed.

Context

loadLiveProviderTrackRecords runs an unbounded SELECT over a 90-day slice of audit_events and swallows
any failure into an empty result. src/services/reviewer-routing.ts:71-104:

    const votes = await env.DB.prepare(
      "SELECT actor, target_key, metadata_json FROM audit_events WHERE event_type = ? AND created_at >= ? ORDER BY created_at ASC, id ASC",
    )
      .bind(REVIEWER_VOTE_EVENT_TYPE, new Date(nowMs - CORPUS_LOOKBACK_MS).toISOString())
      .all<{ actor: string; target_key: string; metadata_json: string }>();

CORPUS_LOOKBACK_MS is 90 days (src/services/reviewer-routing.ts:27). There is no LIMIT, and the rows are
raw (not aggregated) with a metadata_json blob per row. Every sibling raw-row read in this subsystem is
bounded: src/services/knob-loosening-run.ts:599 (ORDER BY created_at DESC LIMIT ?),
src/services/satisfaction-floor-loosening-run.ts:240-243 (ORDER BY created_at DESC LIMIT ?),
src/services/reviewer-routing.ts's own trend siblings all fold in SQL with GROUP BY.

reviewer_vote rows are written one per reviewer per completed block-mode dual review
(src/queue/ai-review-orchestration.ts:967-975), across every repo, and this read runs on every such
review — recordRoutingShadow is called at src/queue/ai-review-orchestration.ts:981-985 whenever a review
produced ≥2 reviewer votes.

The failure is silent by construction. src/services/reviewer-routing.ts:101-103:

  } catch {
    return []; // fail-safe: no records ⇒ downstream records nothing ⇒ byte-identical behavior
  }

An oversized result set is a thrown driver error, so it lands in that catch and becomes "no track records",
which computeWouldHaveRouted (src/services/reviewer-routing.ts:53) reads as "below the decided floor" and
returns null for — indistinguishable from the legitimate no-signal case the module's own invariant list
requires (src/services/reviewer-routing.ts:13-14: "absence of a record must mean 'no measurable
preference'"). The stage-1 shadow silently stops recording exactly as the ledger grows large enough for stage
2 to be worth shipping against, and nothing anywhere says so.

This is the same table whose unbounded growth is documented as having hit a size cap before
(src/db/retention.ts:479-483).

Requirements

  • The query at src/services/reviewer-routing.ts:73-81 must carry an explicit LIMIT bound to a named
    exported constant REVIEWER_VOTE_SCAN_LIMIT in src/services/reviewer-routing.ts.
  • Because the fold's dedup is latest-vote-wins by taking the LAST array element (the reason the ORDER BY is
    mandatory, src/services/reviewer-routing.ts:74-77), a naive ASC ... LIMIT n would keep the OLDEST rows
    and silently invert that dedup. The bounded read must therefore select the NEWEST REVIEWER_VOTE_SCAN_LIMIT
    rows (ORDER BY created_at DESC, id DESC LIMIT ?) inside a subquery and re-order the outer projection
    ASC, id ASC, so computeProviderTrackRecords still receives ascending order.
  • When the number of rows returned equals REVIEWER_VOTE_SCAN_LIMIT, the function must emit one structured
    console.warn with event: "reviewer_vote_scan_truncated" and the row count, so a truncated corpus is
    observable rather than silently under-counted.
  • The catch at src/services/reviewer-routing.ts:101-103 must emit one structured console.warn with
    event: "reviewer_vote_scan_failed" and the error message before returning [], so a read failure is
    distinguishable from a genuinely empty ledger. It must still return [] — the fail-safe posture is correct
    and must not change.
  • computeWouldHaveRouted, ROUTING_MIN_DECIDED, recordRoutingShadow, and the REVIEWER_VOTE_EVENT_TYPE
    constant must NOT change.
  • The corrupt-row continue at src/services/reviewer-routing.ts:88-89 and the vote-shape filter at
    src/services/reviewer-routing.ts:91 must NOT change.

⚠️ Required pattern: src/services/knob-loosening-run.ts:599-601 — a newest-first ORDER BY ... LIMIT ?
bound to a named constant. What does NOT satisfy this issue: (a) adding LIMIT to the existing
ORDER BY created_at ASC, id ASC clause, which keeps the oldest rows and inverts the latest-vote-wins
dedup #9638 established; (b) shortening CORPUS_LOOKBACK_MS instead of bounding rows, which changes the
evidence window every other consumer of that constant shares; (c) removing the catch so the read throws
into the review path, which the module's zero-behaviour-change invariant
(src/services/reviewer-routing.ts:9-11) forbids; (d) a test-only PR.

Deliverables

  • src/services/reviewer-routing.ts exports REVIEWER_VOTE_SCAN_LIMIT and the query selects the newest
    that many rows in a subquery, re-ordered ascending in the outer projection.
  • A truncated read (rows returned === REVIEWER_VOTE_SCAN_LIMIT) emits
    console.warn with event: "reviewer_vote_scan_truncated".
  • A thrown read emits console.warn with event: "reviewer_vote_scan_failed" and still returns [].
  • A test in test/unit/reviewer-routing.test.ts seeding REVIEWER_VOTE_SCAN_LIMIT + 5 reviewer_vote
    rows and asserting that the NEWEST rows survive: a provider whose only vote is the oldest row must be
    absent from the returned track records, and a provider whose only vote is the newest row must be
    present.
  • A test in test/unit/reviewer-routing.test.ts asserting the returned signals are in ascending
    created_at order after the subquery re-ordering, so the latest-vote-wins dedup (orb(routing): the reviewer-vote read has no ORDER BY #9638) still holds.
  • A regression test at test/unit/reviewer-routing.test.ts named for this bug asserting that a DB.prepare
    that throws produces both the reviewer_vote_scan_failed warn and an empty array, and that
    recordRoutingShadow still returns null without touching the review path.

All Deliverables above are required in a single PR. A PR that satisfies only some of them — for example adding
the LIMIT without the newest-first subquery, so the dedup silently starts resolving to stale votes — does
not resolve this issue.

Test Coverage Requirements

This repo enforces 99%+ Codecov patch coverage, branch-counted. vitest.config.ts's coverage.include
covers src/**/*.ts, so src/services/reviewer-routing.ts is measured and gated. The change introduces two
branches: the truncation warn (rows.length === REVIEWER_VOTE_SCAN_LIMIT) and the failure warn inside the
existing catch. Both arms of the truncation branch need a test — a run at the limit and a run below it — and
the catch arm needs the throwing-driver test above.

Expected Outcome

After this ships, the reviewer-routing shadow's evidence read has a fixed worst-case cost regardless of how
large audit_events grows, keeps the most recent votes rather than the oldest, and says so in the logs when it
truncates or fails — so "the shadow recorded nothing" can be told apart from "the shadow could not read its
evidence", which is the distinction the stage-2 rollout decision depends on.

Links & Resources

  • src/services/reviewer-routing.ts:71-104 — the unbounded read and its silent catch
  • src/services/reviewer-routing.ts:27CORPUS_LOOKBACK_MS (90 days)
  • src/services/reviewer-routing.ts:44-64computeWouldHaveRouted, which reads "" as "no preference"
  • src/queue/ai-review-orchestration.ts:967-985 — where reviewer_vote rows are written and the shadow runs
  • src/services/knob-loosening-run.ts:599-601 — the bounded-read precedent
  • src/db/retention.ts:479-483 — prior unbounded-growth incident on this table
  • orb(routing): the reviewer-vote read has no ORDER BY #9638 — the ORDER BY fix whose dedup semantics this must preserve

Metadata

Metadata

Assignees

No one assigned

    Labels

    gittensor:bugGittensor-scored bug fix — scores a 0.05x multiplier.help wantedExtra attention is needed

    Projects

    No projects

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions