Skip to content

fix(stats): drop counterfactual-replay rows from the public per-rule precision - #9720

Merged
JSONbored merged 1 commit into
mainfrom
fix/public-precision-counterfactual-rows
Jul 29, 2026
Merged

fix(stats): drop counterfactual-replay rows from the public per-rule precision#9720
JSONbored merged 1 commit into
mainfrom
fix/public-precision-counterfactual-rows

Conversation

@JSONbored

Copy link
Copy Markdown
Owner

Summary

The public "Measured accuracy per rule" table published ai_consensus_defect and slop_gate_score with byte-identical decided: 460, confirmed: 287, precision: 62.4%. Not a coincidence — slop_gate_score's override rows are the same label set, copied.

#8277's slop backfill re-scores the deterministic slop signals over archived diffs, but takes each label verbatim from the ai_consensus_defect corpus's human verdict on the same target: manifestToSourceCases (scripts/backfill-slop-corpus.ts:51-58) passes backtestCase.label straight through, and replaySlopCorpus writes it unchanged into the new rule's verdict (scripts/backfill-slop-corpus-core.ts:133-134,165). Only the score side is recomputed.

That is exactly what it was built to be — internal evidence for a flip-to-live decision, and by its own module header "a LOWER BOUND on what live scoring would have produced". But the public table describes itself as "Precision of each automated rule over its human-decided cases in the last 90 days", and those were another rule's human-decided cases. It also made the table print one number twice, since both rules then shared a label set and a target set.

Corroborating that the published rows cannot be live measurements: slop_gate_score is in GATE_SCORE_SIGNAL_CODES (src/rules/advisory.ts:207), which recordImplicitTerminalConfirmations explicitly excludes (src/review/outcomes-wire.ts:654), and recordConfiguredGateBlockerOverrides only ever writes reversed. On the live path that rule has no route to a confirmed verdict at all, so a 62.4% could only ever have come from replayed rows.

Excluded by provenance, not by rule id, so the next cross-rule replay inherits the same protection. review_targets_decision_level (#8083's own backfill) is deliberately not excluded — its labels come from what actually happened to the PRs that rule fired on, so they do support a precision claim for it, and there is a test pinning that distinction.

Refs #9676

Scope

  • The PR title follows type(scope): short summary Conventional Commit format, for example fix(api): restore profile access checks.
  • This PR is focused and does not mix unrelated backend, UI, MCP, docs, dependency, and deploy changes.
  • This follows CONTRIBUTING.md and does not reintroduce GitHub Pages, VitePress, site/, or CNAME.
  • I linked a currently open issue this PR resolves (e.g. Closes #123) — a linked open issue is required for every contributor PR.

Validation

  • git diff --check
  • npm run actionlint
  • npm run typecheck
  • npm run test:coverage locally; codecov/patch requires ≥99% coverage of the lines AND branches you changed (aim for 100% on your diff so CI variance does not fail near the threshold). Global coverage is a non-blocking trend with a loose 90% backstop, not the gate.
  • npm run test:workers
  • npm run build:mcp
  • npm run test:mcp-pack
  • npm run ui:openapi:check
  • npm run ui:lint
  • npm run ui:typecheck
  • npm run ui:build
  • npm audit --audit-level=moderate
  • New or changed behavior has unit/integration tests for new branches, fallback paths, and sanitizer boundaries

If any required check was skipped, explain why:

  • Coverage measured scoped: vitest run --coverage over public-rule-precision.test.ts restricted to the changed module reports 100% statements / branches / functions / lines (19/19, 10/10, 6/6, 17/17). Both sides of the new exclusion are pinned — a cross-rule replay row is dropped, a same-rule synthesized row is kept.
  • Ran the neighbouring suites that consume this block (public-stats, public-stats-route, public-eval-scores-route): 59 tests green.
  • No UI files touched, so ui:lint/ui:typecheck/ui:build were not run. ui:openapi:check passes — response shape is unchanged, only which rows qualify.
  • test:workers, build:mcp, test:mcp-pack untouched by this diff; left to CI.

Safety

  • No secrets, wallet details, hotkeys, coldkeys, user PATs, private keys, raw trust scores, private rankings, or private maintainer evidence are exposed.
  • Public GitHub text stays sanitized, low-noise, and does not imply compensation guarantees or optimization tactics.
  • Auth, cookie, CORS, GitHub App, Cloudflare, or session changes include negative-path tests.
  • API/OpenAPI/MCP behavior is updated and tested where needed.
  • UI changes use live API data or real empty/error/loading states, not production mock/demo fallbacks.
  • Visible UI changes include a UI Evidence section below with JPG/JPEG or PNG screenshots arranged as organized, captioned, clickable thumbnails. SVG screenshots are not used as review evidence. Review-only screenshots or recordings are not committed to the repository.
  • Public docs/changelogs are updated where needed; changelogs are only edited for release-prep PRs.

No auth/CORS/session change. No UI file is touched — the fairness page already renders whatever rows the block returns, so this removes a row rather than changing a component; that is why there is no UI Evidence table here. The existing INVARIANT: the public payload never carries target keys, repos, confidences, or private terms test still passes.

Notes

On the current production data every slop_gate_score row carries this provenance, so that rule drops out of the public table entirely and ai_consensus_defect remains at 460 / 287 / 62.4%. That is the honest outcome: one rule with a real (if synthesized-from-its-own-history) measurement, instead of two rules appearing to independently corroborate each other while sharing one label set.

@loopover-orb

loopover-orb Bot commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

Warning

⏸️ LoopOver review result - manual review recommended

Review updated: 2026-07-29 06:39:50 UTC

2 files · 1 AI reviewer · 2 blockers · CI green · clean

⏸️ Suggested Action - Manual Review

Review summary
The routeTree.gen.ts diff is just a reordering of an auto-generated file (alphabetized imports/routes), effectively a no-op regeneration with no behavioral change. The real fix is in public-rule-precision.ts: it adds a provenance-based exclusion (`slop_replay_backfill_v1`) to the audit_events query so cross-rule-copied replay labels no longer inflate the public per-rule precision table, and it's well-targeted — excluding by provenance rather than rule id so future replays inherit the same protection. Both new test cases exercise the real SQL path (via `recordHumanOverride` writing `metadata.provenance` into `metadata_json`) and assert the exact expected filtering, which is genuine coverage of the new branch, not a fabricated scenario.

Nits — 5 non-blocking
  • src/review/public-rule-precision.ts:73 builds the SQL IN-list via string interpolation of `NON_ATTRIBUTABLE_OVERRIDE_PROVENANCES`; since it's a fixed internal constant array today it isn't attacker-controlled, but consider a parameterized `IN (?)` or documenting that this array must never be extended with dynamic/user input.
  • routeTree.gen.ts is a generated file with a pure reordering diff (+281/-281) — worth confirming this is just a `tanstack-router` regen artifact and not hand-edited, since the PR title/description say nothing about routing.
  • The `NON_ATTRIBUTABLE_OVERRIDE_PROVENANCES` array is currently a single-entry allowlist-by-exclusion; consider a short comment/test asserting the SQL fragment escapes/quotes correctly if a provenance tag ever contains a quote character, though today's single hardcoded literal makes this low risk.
  • Consider adding a test where `provenance` is absent (undefined) to explicitly document that `COALESCE(...,'')` treats missing provenance as attributable (included) — the existing seedVerdicts helper implicitly covers this but an explicit assertion would make the default behavior self-documenting.
  • If more replay/backfill scripts are added later, consider deriving `NON_ATTRIBUTABLE_OVERRIDE_PROVENANCES` from a shared enum/const also referenced by `scripts/backfill-slop-corpus-core.ts` so the exclusion list and the writer can't drift apart silently.

Concerns raised — review before merging

  • No linked issue detected: No closing reference or linked issue number was found in the PR metadata/body. — If this PR is intended to solve an issue, link it explicitly in the PR body.
  • Maintainer requires a linked issue: This repo's maintainer focus manifest requires every PR to reference a tracked issue. — Link the relevant issue (for example Closes #123) before opening the PR.
📋 Copy for AI agents — paste into your coding agent
Fix the following blocker(s) from this PR review:

1. No linked issue detected: No closing reference or linked issue number was found in the PR metadata/body. — If this PR is intended to solve an issue, link it explicitly in the PR body.

2. Maintainer requires a linked issue: This repo's maintainer focus manifest requires every PR to reference a tracked issue. — Link the relevant issue (for example `Closes #123`) before opening the PR.

Decision drivers

  • ❌ Code review — 2 blockers (1 reviewer)
  • ❌ Gate result — Blocking (Repo-configured hard blocker found.)
Context & advisory signals — never blocks the verdict
Signal Result Evidence
Linked issue ⚠️ Missing No linked issue or no-issue rationale found.
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 (no linked issue context).
Validation posture ✅ 25/25 PR body includes validation/test evidence.
Contributor workload ✅ 10/10 Author activity: 14 registered-repo PR(s), 13 merged, 359 issue(s).
Contributor context ✅ Confirmed Gittensor contributor JSONbored; Gittensor profile; 14 PR(s), 359 issue(s).
Improvement ✅ Minor risk: clean · value: minor
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), 359 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 4 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 <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: missing_linked_issue
  • config: b0e925cba548a81dcf8e404515d90eee4eedb6f09a0a611df707c5e8f84a1664 · pack: oss-anti-slop · ci: passed
  • record: 2af8bc9941c60c3bf8c6879cea29159be69ec340f8e5372d249a7dc406875f5a (schema v5, head bb5835b)

🟩 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

Copy link
Copy Markdown
Contributor

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

@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented Jul 29, 2026

Copy link
Copy Markdown

Deploying with  Cloudflare Workers  Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status Name Latest Commit Updated (UTC)
❌ Deployment failed
View logs
loopover-ui 1a79901 Jul 29 2026, 04:55 AM

@codecov

codecov Bot commented Jul 29, 2026

Copy link
Copy Markdown

Bundle Report

Bundle size has no change ✅

@JSONbored

Copy link
Copy Markdown
Owner Author

Merge-order note: this PR and #9675 both edit src/review/public-rule-precision.ts. They originally conflicted — both inserted a constant directly after HUMAN_OVERRIDE_EVENT_TYPE_PREFIX — which would have given whichever merged second a base conflict. This branch now declares NON_ATTRIBUTABLE_OVERRIDE_PROVENANCES immediately above loadPublicRulePrecision instead, leaving the top-of-file constants block byte-identical to main.

Verified: all four open branches (#9675, #9718, #9720, #9721) merge cleanly into main in both forward and reverse order, and with all four merged together typecheck is clean and the eight affected suites pass (122 tests). No merge sequence is required.

@codecov

codecov Bot commented Jul 29, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 89.45%. Comparing base (5ecb771) to head (bb5835b).
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #9720      +/-   ##
==========================================
- Coverage   90.27%   89.45%   -0.82%     
==========================================
  Files         904      904              
  Lines      113177   113179       +2     
  Branches    26840    26840              
==========================================
- Hits       102171   101249     -922     
- Misses       9676    10842    +1166     
+ Partials     1330     1088     -242     
Flag Coverage Δ
backend 94.06% <100.00%> (-1.48%) ⬇️

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

Files with missing lines Coverage Δ
src/review/public-rule-precision.ts 100.00% <100.00%> (ø)

... and 3 files with indirect coverage changes

…precision

The public "Measured accuracy per rule" table published
`ai_consensus_defect` and `slop_gate_score` with byte-identical
decided=460 / confirmed=287 / precision=62.4%. That is not a coincidence:
`slop_gate_score`'s override rows are the same label set, copied.

#8277's slop backfill re-scores the deterministic slop signals over archived
diffs but takes each label verbatim from the `ai_consensus_defect` corpus's
human verdict on the same target (`manifestToSourceCases` in
backfill-slop-corpus.ts passes `backtestCase.label` straight through). That is
exactly what it was built to be — internal evidence for a flip-to-live
decision, and by its own module header a LOWER BOUND on live scoring — but the
public table describes itself as "precision of each automated rule over its
human-decided cases", and those were another rule's human-decided cases.

Corroborating that the rows cannot be live: `slop_gate_score` is in
GATE_SCORE_SIGNAL_CODES, which `recordImplicitTerminalConfirmations` excludes,
and `recordConfiguredGateBlockerOverrides` only ever writes `reversed` — so on
the live path the rule has no route to a `confirmed` verdict at all. A 62.4%
could only ever have come from replayed rows.

Excluded by provenance rather than by rule id, so the next cross-rule replay
inherits the same protection. `review_targets_decision_level` (#8083's own
backfill) is deliberately NOT excluded: its labels come from what happened to
the PRs that rule actually fired on, so they do support a precision claim for
it.

Refs #9676
@JSONbored
JSONbored force-pushed the fix/public-precision-counterfactual-rows branch from 1a79901 to bb5835b Compare July 29, 2026 06:21
@JSONbored
JSONbored merged commit d79470f into main Jul 29, 2026
7 checks passed
@JSONbored
JSONbored deleted the fix/public-precision-counterfactual-rows branch July 29, 2026 06:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

manual-review Gittensor contributor context

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant