feat(review): record MCP predicted-gate verdicts and measure predicted-vs-live gate agreement - #4558
Conversation
|
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 #4558 +/- ##
=======================================
Coverage 94.08% 94.09%
=======================================
Files 427 429 +2
Lines 37961 38023 +62
Branches 13864 13882 +18
=======================================
+ Hits 35715 35777 +62
Misses 1586 1586
Partials 660 660
🚀 New features to boost your workflow:
|
|
Warning 🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨 ⏸️ Gittensory review result - manual review recommendedReview updated: 2026-07-10 02:47:52 UTC
⏸️ Suggested Action - Manual Review
Review summary Nits — 7 non-blocking
Linked issue satisfactionAddressed Review context
Contributor next steps
Signal definitions
🟩 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 Gittensory, a quiet PR intelligence layer for OSS maintainers.
|
…d-vs-live gate agreement Records every gittensory_predict_gate/explain_gate_disposition call into a new local-only, login-keyed predicted_gate_calls table (mirroring contributor_gate_history's 0126 precedent -- review_audit stays deliberately actor-login-free, per its own migration's privacy design). A new sibling to computeGateEval/computeGateParity, computePredictedGateAgreement, joins predicted_gate_calls against the already-existing contributor_gate_history by (project, login, timestamp) -- the correlation key predict-time calls can actually provide, since predictGateShape has no PR-number field pre-submission -- and folds the result into a per-project agreement/disagreement confusion matrix, surfaced via a new GET /v1/internal/predicted-agreement endpoint alongside the existing /v1/internal/parity one. Fixes #4516
8f38ac8 to
8f053bd
Compare
…ay collision on main) main currently has three DIFFERENT already-merged files at 0134 (#4549/#4558/#4563) -- tracked separately as its own fix (PR #4577, not yet merged). Since that fix already claims through 0138, take 0139 here to avoid colliding with it once it lands; this branch's own db:migrations:check will stay red until #4577 merges (unrelated to this branch's own changes), and will need one more rebase afterward.
…ay collision on main) main currently has three DIFFERENT already-merged files at 0134 (#4549/#4558/#4563) -- tracked separately as its own fix (PR #4577, not yet merged). Since that fix already claims through 0138, take 0139 here to avoid colliding with it once it lands; this branch's own db:migrations:check will stay red until #4577 merges (unrelated to this branch's own changes), and will need one more rebase afterward.
…tion ledger (#4577) * feat(review): persist a login-keyed predict-gate-vs-live-gate calibration ledger Neither the MCP predict_gate tool nor contributor_gate_history persists whether a contributor's self-reported predict-gate verdict matched the REAL gate decision their PR eventually received -- the one calibration signal the review stack itself uniquely owns, since it terminates both the self-review call and the live gate for the same login/repo. predicted_gate_calibration_ledger pairs the most recent predict_gate_calls row (#4516) for a (login, project) against the real decision at the same call sites as recordContributorGateDecision, writing one immutable row per (login, project, pr, commit) -- ON CONFLICT DO NOTHING, never DO UPDATE, so a webhook replay can never overwrite an already-recorded pairing. Write-only and server-side only: nothing reads it yet (mirrors contributor_gate_history's own precedent), and no MCP tool or other contributor-reachable surface can write to or read from it, preserving its value as anti-farming-resistant ground truth for a future #2349 consumer. Depends on #4516 (predicted_gate_calls) -- built stacked on that branch per the issue's own explicit note that the two may be built together; will be rebased onto main once that PR merges. Fixes #4517 * fix(db): renumber ledger migration 0135 -> 0138 (0134/0135 collide on main) origin/main independently landed three files at 0134 and two at 0135 (from already-merged PRs #4549, #4558, #4563) since this branch's last rebase. Rather than touch already-merged migration files here, take the next genuinely free number for this branch's own new migration; the 0134/0135 collision among already-merged files is a separate main-red issue handled in its own PR. * fix(db): resolve migration collision at 0134 on main (already-merged PRs) Three already-merged PRs independently claimed 0134 (#4549's review_targets_cadence_idx, #4558's predicted_gate_calls, and #4563's pr_last_backlog_convergence_regated_at). CI on this branch inherits main's full migrations/ directory regardless of which PR fixes it, so this can't be deferred to a separate PR without also blocking this one. Renumber the two newer files (keeping #4563's oldest 0134 file in place); this branch's own new migration was already moved to 0138 to stay clear of both this collision and #4563's separate 0135 collision.
…PRs) (#4581) Three already-merged PRs independently claimed 0134 (#4549's review_targets_cadence_idx, #4558's predicted_gate_calls, and #4563's pr_last_backlog_convergence_regated_at). CI on this branch inherits main's full migrations/ directory regardless of which PR fixes it, so this can't be deferred to a separate PR without also blocking this one. Renumber the two newer files (keeping #4563's oldest 0134 file in place); this branch's own new migration was already moved to 0138 to stay clear of both this collision and #4563's separate 0135 collision.
…-wide for confirmed miners (#4546) * fix(review): make submitter-reputation burst/AI-spend defense install-wide for confirmed miners getSubmitterReputation's burst/low-sample thresholds are scoped WHERE project = ? AND submitter = ? -- a single repo. A fleet identity spreading a handful of gate-passing-but-low-value PRs across dozens of repos in one self-hosted install never accumulates enough same-repo sample density to read as "burst" or "low" anywhere, so the reputation defense never fires for it and every one of its submissions burns full paid AI-review spend indefinitely. - New getSubmitterReputationAcrossInstall in submitter-reputation.ts: the identical quality-weighted signal derivation, scoped by review_targets.installation_id instead of project (that column already existed, migrations/0050; this adds the supporting index). - New getEffectiveSubmitterReputation in reputation-wire.ts: the per-repo signal, additionally widened to the install-wide view for a CONFIRMED official Gittensor miner -- but only when the per-repo signal alone doesn't already justify caution, so an ordinary contributor or an already-flagged submitter pays no extra lookup. - Extracted the miner-identity check (shared with #4512) into src/gittensor/miner-detection-cache.ts so both this module and unlinked-issue-guardrail.ts can use it without a circular import through processors.ts. - Wired into both call sites: shouldSkipAiForReputation (the main AI-spend gate) and the vision-review reputation check. Fixes #4513 * fix: renumber migration 0130 -> 0131 (0130 was claimed by #4538, already merged to main) * fix(test): account for getEffectiveSubmitterReputation's miner-identity check in visual-vision tests runVisualVisionForAdvisory now resolves reputation via getEffectiveSubmitterReputation (#4513), which checks confirmed-official-miner identity (a fetch to api.gittensor.io/miners) whenever the submitter's per-repo signal is neutral -- a real, intentional behavior change this test file's existing "no network calls at all" assertions didn't account for. Stub that one identity check to resolve cleanly and assert precisely that no OTHER (BYOK/vision-spend) network call happens, rather than asserting zero fetch calls outright. * fix(db): renumber migration 0131 -> 0133 (0131/0132 claimed by merged PRs) origin/main has since merged #4545 (0131_screenshot_table_gate_matrix.sql) and #4554 (0132_impact_map_query_cache.sql, itself a collision fix); rebase onto current main and take the next free number. * fix(db): renumber migration 0133 -> 0134 (0133 claimed by merged PR #4556) Rebase onto current main and take the next free number now that migrations/0133_screenshot_table_gate_skill_link.sql (from #4556) occupies the number this branch previously took. * fix(db): renumber migration 0134 -> 0139 (0134 has a pre-existing 3-way collision on main) main currently has three DIFFERENT already-merged files at 0134 (#4549/#4558/#4563) -- tracked separately as its own fix (PR #4577, not yet merged). Since that fix already claims through 0138, take 0139 here to avoid colliding with it once it lands; this branch's own db:migrations:check will stay red until #4577 merges (unrelated to this branch's own changes), and will need one more rebase afterward. * test(review): close 2 coverage gaps surfaced by the #4514 rebase-merge getEffectiveSubmitterReputation's getRepository().catch() needed a real read-failure test (added); its isConfirmedOfficialMiner().catch() is unreachable (that function already catches every internal failure point itself) and getSubmitterReputationAcrossInstall's results ?? [] fallback is the same "D1 always populates results" case already v8-ignored elsewhere in this codebase -- both marked accordingly. * fix(review): isolate #4507's reputation-single-read invariant from cross-test miner-cache pollution Several earlier tests in this file cache "contributor" as a confirmed official Gittensor miner (5-min TTL) in the shared per-file D1 instance. That collided with #4513's new install-wide reputation widening, which correctly detects the stale cache and adds a 4th reputation-scan D1 read for a submitter this test never intended to be a miner. Use a submitter login unique to this test instead.
Summary
gittensory_predict_gate/gittensory_explain_gate_dispositioncompute a verdict but never record it — there was no way to measure how often the MCP tool's pre-submission prediction actually agrees with the REAL gate decision a contributor's PR later receives.src/review/predicted-gate-calls.ts(new, migration0134): records every predict-gate call into a local-only, login-keyedpredicted_gate_callstable, structurally a sibling ofcontributor_gate_history(migration 0126) —review_audit(migration 0049) is deliberately actor-login-free (feeds the anonymized orb-collector export), so this follows that exact precedent rather than adding a login column there. Wired intocomputePredictedGateVerdict(src/mcp/server.ts), shared by both tools.src/review/predicted-gate-agreement.ts(new): a sibling tocomputeGateEval/computeGateParity(src/review/parity.ts), answering a different question than either — joinspredicted_gate_callsagainst the already-existingcontributor_gate_history(the login-keyed real-decision data recorded at the same call sites asrecordNativeGateDecision) by(project, login, timestamp), since a predicted call has no PR number to key on the way a real gate decision does. Folds into a 2-way (merge/hold — the gate never predicts "close") agreement matrix with an explicit unsafe-direction count (predicted merge, real hold).GET /v1/internal/predicted-agreement(new endpoint,src/api/routes.ts): same bearer/flag gate as the existing/v1/internal/parity, surfaces the new metric.scripts/check-schema-drift.mjs: allowlistspredicted_gate_callsas raw-SQL-only (matching the existing convention forcontributor_gate_historyetc.)Deliberately scoped out (documented in code): a per-login breakdown of this report is explicitly issue #4517's separate, deliberately-scoped concern (a login-keyed disagreement ledger with its own data-ownership boundary) — this PR's output stays project-level aggregate only.
Fixes #4516
Test plan
npx tsc --noEmitcleannpm run db:migrations:check/db:schema-drift:checkcleanpredicted-gate-calls.test.ts, 100% coverage), reader/join logic (predicted-gate-agreement.test.ts— pairing, multi-candidate iteration, cross-login/cross-project isolation, non-binary real-decision skip-through, correlation-window boundary,hasSignalthreshold, fail-safe on read errors, the new endpoint), MCP wiring (mcp-predict-gate.test.ts— both tools record a row, cloud-worker default is a no-op)