fix(orb): retire the orphaned review_targets ledger from every live reader (#9136, #9576, #9577)

[JSONbored]

Closes #9136
Closes #9576
Closes #9577

Summary

review_targets has had no writer anywhere since the 2026-06-22 convergence cutover — verified exhaustively: zero INSERT/UPDATE/upsert in src/**, packages/**, scripts/**, or migrations/**. Not dead code, not flag-gated; the write path does not exist. (The only writes are test fixtures and export-grafana-reporting-db.sh, which legitimately mines the historical archive.)

Everything still reading it was therefore inert or expiring, with no alarm on any of it. This removes the last reader — grep "FROM review_targets" src/ is now empty.

Note: I posted a verification pass on #9136 first, because roughly half of what it described had already been fixed and one section was factually stale. The scope below is the corrected remainder, plus two things that issue never named.

What was actually broken

1. computeCalibration read the orphaned table for its confidence curve, close distribution and disputed-close counts.

2. #9577 — a second namespace mismatch, distinct from the SQL join #9136 names, and the one that actually disabled the recommender:

const reverted = new Set(revRows.map((r) => r.target_id)); // review_audit  → "owner/repo#123"
const isKept = !reverted.has(r.id);                        // review_targets → "project:kind:owner/repo#123"

Never intersecting, so !reverted.has(...) was always true: every merge read as kept, revertedMaxConfidence always null, recommendedFloor always null, and the calibration-drift alert structurally unfireable. This would have survived any repopulation of the table.

3. #9576 — GET /v1/internal/decision 404'd for every PR since the cutover, while staying routed, authenticated, and documented as the explain-any-verdict surface. Its audit-trail query carried the same namespace bug independently.

What replaced them

decision_records (#8836) is the live successor: the acted disposition, its reasonCode, and record_json's aiConfidence, keyed repo_full_name || '#' || pull_number — the same namespace as review_audit, so both mismatches retire together rather than being patched.

One subtlety that would have made this a silent no-op: review_targets spelled the field confidence; decision_records spells it aiConfidence. Reading only the new name yields null for every row and leaves the calibration exactly as dead as the orphaned table left it. Both spellings are accepted.

What was deleted rather than repointed — and why

Not everything had a live analogue. Splitting on that, rather than repointing all of it at something approximate:

byStatus + byVerdict → byDecision	From review_audit's gate_decision rows. The disposition half of the old status vocabulary maps exactly onto merge/close/hold, and that is what terminalCount/nonTerminal/manualRate were really measuring. The two fields were also near-duplicates of each other.
manualRate	Gains a correct denominator as a side effect — it divided holds by a terminal count that excluded holds.
stuckRetryable, failed/failedTargets + two Discord alerts	Deleted. These read queued/reviewing/error/error_retryable — processing states the cutover removed as a concept, not just a table. Nothing live records them, so both alerts had been structurally unfireable for two months while reading as coverage. failed was additionally redundant with dlqCount/dlqTargets, which cover "permanently failed", are live, and have their own alert. An alert that cannot fire is worse than no alert.
computeStats / handleStats	Deleted. The last reader, and unrouted — handleStats is exported and referenced only by a comment in routes.ts. The file-level dead-source check missed it because sibling exports from stats.ts are live.
review_targets in checkReviewSourceFreshness	Dropped. A 90-day probe against a permanently frozen table reported stale forever — a gauge no action could ever clear. decision_records takes its slot at review_audit's 7-day window, since it is now the source whose silence would mean these surfaces have gone dark.

Testing

Two fixtures encoded the bugs as expected behaviour and are rewritten rather than adjusted:

• calibrationEnv handed both queries the same bare ids ("a", "b", "c"), making the two namespaces agree in the test and only in the test. The suite passed for exactly as long as production was broken by their disagreement.
• routes-internal-decision-calibration seeded review_audit.target_id with the rowId shape that production never writes.

Both now use real owner/repo#n keys, so a namespace regression fails the test instead of hiding inside it.

Full suite green: 23,891 passed.

CI note

import-specifiers:check reports 5 violations on this branch. They are pre-existing on main and already fixed in #9573 (including one that CI caught me breaking — scripts/actionlint.ts runs under node --experimental-strip-types, where the .ts extension is mandatory). They clear when that merges.

[loopover-orb]

Warning

⏸️ LoopOver review result - manual review recommended

_{Review updated: 2026-07-28 18:28:17 UTC}

7 files · 1 AI reviewer · 1 blocker · CI green · clean

⏸️ Suggested Action - Manual Review

Review summary
This PR retires every remaining live read of the orphaned `review_targets` table, repointing `computeAgentHealth`, `computeCalibration`, and `GET /v1/internal/decision` onto `review_audit`, `decision_records`, and `pull_requests`, and along the way fixes two real namespace-mismatch bugs (`review_audit.target_id` is `owner/repo#n` vs `review_targets.id`'s `project:kind:owner/repo#n`) that had made the calibration reversion check and the decision-endpoint audit trail permanently non-functional since the 2026-06-22 cutover. The tracing is well-evidenced (line-level before/after, tests seeded with the real id shapes instead of matching bare ids), and the byStatus/byVerdict→byDecision and manualRate-denominator changes are all reflected consistently in both `ops.ts` and its duplicated type in `alerts.ts`. However, `src/review/stats.ts` doesn't just drop its `review_targets` read — it deletes the entire `computeStats`/`handleStats` endpoint (bearer auth, CORS, `BUCKET_SQL` whitelist, and the whole `/stats/data` route) wholesale rather than repointing the decision-rows query the way every other reader in this same PR was repointed, which is inconsistent with the PR's own stated pattern and removes a live, authenticated dashboard feed with no visible replacement.

Blockers

• src/review/stats.ts: `computeStats`/`handleStats` (and their auth/CORS/whitelist machinery) are deleted outright instead of repointing the `review_targets`-sourced `decisionRows` query the way every other reader in this PR is repointed — if any router still wires `GET /stats/data` to `handleStats`, this diff breaks that import with no replacement, and even if unwired, it silently retires a bearer-gated dashboard feed rather than fixing it, unlike the precedent set right next to it in ops.ts.

Nits — 6 non-blocking

• src/review/ops.ts: the `disputedRows` query filters `dr` to the latest decision_records row per PR via `LATEST_DECISION_RECORD_FILTER` and then adds a redundant `NOT EXISTS (... later.created_at > a.created_at)` clause that can never exclude anything once `dr` is already constrained to be the latest row overall — dead condition, consider dropping it or adding a comment explaining why it's kept for defense.
• src/review/ops.ts:335 / :557 / :648: several new numeric literals (LIST_CAP=100, audit LIMIT=25, freshness windowDays=7) are unexplained magic numbers reused from the old code without a named constant, per the external review brief.
• src/review/ops.ts is now ~651 lines, comfortably over the repo's ~400-line file-size convention noted in the review brief — consider whether the freshness-check section or the calibration section could be split out.
• The PR description is truncated before it explains what replaces the deleted `handleStats`/`computeStats` dashboard feed — worth confirming in the PR thread that this deletion (rather than a repoint) was an intentional, discussed scope decision.
• If `/stats/data` is still meant to be operator-facing, repoint `computeStats`'s decision-rows query onto `review_audit`'s `gate_decision` rows (the same source `byDecision` now uses in ops.ts) instead of deleting the endpoint.
• PR author also opened the linked issue — Link an issue that was opened by a different contributor, or provide a rationale for why this self-authored issue represents genuine discovery work.

Concerns raised — review before merging

• src/review/stats.ts: `computeStats`/`handleStats` (and their auth/CORS/whitelist machinery) are deleted outright instead of repointing the `review_targets`-sourced `decisionRows` query the way every other reader in this PR is repointed — if any router still wires `GET /stats/data` to `handleStats`, this diff breaks that import with no replacement, and even if unwired, it silently retires a bearer-gated dashboard feed rather than fixing it, unlike the precedent set right next to it in ops.ts.

📋 Copy for AI agents — paste into your coding agent

Fix the following blocker(s) from this PR review:

1. src/review/stats.ts: \`computeStats\`/\`handleStats\` \(and their auth/CORS/whitelist machinery\) are deleted outright instead of repointing the \`review\_targets\`-sourced \`decisionRows\` query the way every other reader in this PR is repointed — if any router still wires \`GET /stats/data\` to \`handleStats\`, this diff breaks that import with no replacement, and even if unwired, it silently retires a bearer-gated dashboard feed rather than fixing it, unlike the precedent set right next to it in ops.ts.

Decision drivers

• ❌ Code review — 1 blocker (1 reviewer)
• ❌ Gate result — Blocking (Repo-configured hard blocker found.)

Context & advisory signals — never blocks the verdict

Signal	Result	Evidence
Linked issue	✅ Linked	#9136, #9576, #9577
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 (3 linked issues).
Validation posture	✅ 25/25	PR body includes validation/test evidence.
Contributor workload	✅ 10/10	Author activity: 14 registered-repo PR(s), 13 merged, 300 issue(s).
Contributor context	✅ Confirmed Gittensor contributor	JSONbored; Gittensor profile; 14 PR(s), 300 issue(s).
Improvement	✅ Minor	risk: clean · value: minor

Linked issue satisfaction

Partially addressed
The visible diff thoroughly repoints ops.ts (anomaly alerter, computeCalibration, /status, /decision endpoint) off review_targets, fixes the target_id namespace mismatch in both the reversal join and the decision endpoint, and updates the freshness-check list — solidly addressing issue point #1 and part of the dedup/namespace requirement. However, none of the shown diff touches submitter-reputatio

Review context

• Author: JSONbored
• Role context: owner (maintainer lane)
• Public audience mode: oss maintainer
• Lane context: Repository is registered but has no active allocation in the current snapshot.
• Public profile languages: not available
• Official Gittensor activity: 14 PR(s), 300 issue(s).
• PR-specific overlap: none found.

Contributor next steps

• Start here: Treat this as maintainer-lane context rather than normal contributor-lane activity.
• Then work through the remaining 1 step 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 <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.

Decision record

• action: hold · clause: ai_consensus_defect
• config: 553f068b9273cb541b51dd192cabaa15accba9861884d52940081f8e5bd0f592 · pack: oss-anti-slop · ci: passed
• model: claude-code · prompt: 7ac46a3f8ed0bb19bf9274163ef5821c41a4b11125e4c2ffc182d26d1c35f412 · confidence: 0.55
• record: d3b7bf0051e8b18684dc90020960def63b2ea71519460bae5511288ea0b4a1be (schema v5, head 56b1fd5)

_{🟩 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

[superagent-security]

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

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 89.03%. Comparing base (0e990a3) to head (b7070de).

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #9578      +/-   ##
==========================================
- Coverage   89.84%   89.03%   -0.81%     
==========================================
  Files         875      875              
  Lines      110980   110977       -3     
  Branches    26402    26403       +1     
==========================================
- Hits        99706    98805     -901     
- Misses       9992    11156    +1164     
+ Partials     1282     1016     -266     

Flag	Coverage Δ
backend	94.16% <100.00%> (-1.47%)	⬇️

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

Files with missing lines	Coverage Δ
src/review/alerts.ts	100.00% <ø> (ø)
src/review/ops.ts	98.67% <100.00%> (-0.66%)	⬇️
src/review/stats.ts	99.07% <100.00%> (+<0.01%)	⬆️

... and 3 files with indirect coverage changes