Skip to content

feat(review): persist a login-keyed predict-gate-vs-live-gate calibration ledger - #4577

Merged
JSONbored merged 3 commits into
mainfrom
feat/predicted-gate-calibration-ledger
Jul 10, 2026
Merged

feat(review): persist a login-keyed predict-gate-vs-live-gate calibration ledger#4577
JSONbored merged 3 commits into
mainfrom
feat/predicted-gate-calibration-ledger

Conversation

@JSONbored

Copy link
Copy Markdown
Owner

Summary

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.

Fixes #4517

Test plan

  • npx tsc --noEmit clean
  • npm run db:migrations:check / db:schema-drift:check clean (contiguous 0001-0138)
  • New tests (predicted-gate-calibration-ledger.test.ts, 100% coverage): agreement/disagreement recorded correctly, cold-start (no prior prediction) records nothing, correlation-window boundary, cross-repo isolation for the same login, non-binary decision/predicted-action skip, missing-login skip, immutability (a replay at the same commit — even with a different decision — never changes the original row; a new commit gets its own row), flag gating, fail-safe on read/write errors
  • Full related suite green: 127 tests across all 7 touched/adjacent test files
  • Confirmed via git diff origin/main --stat that this branch's diff contains only its own intended files (caught and fixed a stale-merge-base issue from an unrelated PR along the way)

JSONbored added 3 commits July 9, 2026 21:53
…tion 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
… 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.
…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.
@superagent-security

Copy link
Copy Markdown
Contributor

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

JSONbored added a commit that referenced this pull request Jul 10, 2026
…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.
@loopover-orb loopover-orb Bot added the gittensor:feature Gittensor-scored feature linked to a feature issue — scores a 0.25x multiplier. label Jul 10, 2026
@codecov

codecov Bot commented Jul 10, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
⚠️ Please upload report for BASE (main@f366b1d). Learn more about missing BASE report.
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #4577   +/-   ##
=======================================
  Coverage        ?   94.12%           
=======================================
  Files           ?      430           
  Lines           ?    38155           
  Branches        ?    13912           
=======================================
  Hits            ?    35913           
  Misses          ?     1585           
  Partials        ?      657           
Files with missing lines Coverage Δ
src/queue/processors.ts 95.30% <100.00%> (ø)
src/review/predicted-gate-calibration-ledger.ts 100.00% <100.00%> (ø)
🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@loopover-orb

loopover-orb Bot commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

Warning

🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨

⏸️ Gittensory review result - manual review recommended

Review updated: 2026-07-10 05:12:43 UTC

7 files · 1 AI reviewer · no blockers · readiness 100/100 · CI green · clean

⏸️ Suggested Action - Manual Review

  • Touches a guarded path — held for manual review: This PR changes guardrail-protected path(s): scripts/check-schema-drift.mjs (matched scripts/**), src/queue/processors.ts (matched src/queue/**).

Review summary
This PR adds a new immutable, write-only ledger (predicted_gate_calibration_ledger, migration 0138) pairing MCP predict_gate self-reports against real gate decisions, wired at the same two call sites as the existing recordContributorGateDecision in src/queue/processors.ts. The migration/schema-drift wiring is correct (RAW_SQL_ONLY_TABLES updated, contiguous migration numbering after the described 0134/0135 collision fix), and the ON CONFLICT DO NOTHING + deterministic id gives genuine replay-safety. The one real design question is whether the chosen immutability (first-write-wins per (login,project,pr,headSha)) can diverge from contributor_gate_history's own REPLACE-on-revision semantics for the same commit, silently freezing a stale decision as 'ground truth'.

Nits — 7 non-blocking
  • src/review/predicted-gate-calibration-ledger.ts: the two recordPredictedGateCalibration call sites in src/queue/processors.ts fire for the same (login, project, pr, headSha) key that recordContributorGateDecision already treats as REPLACE-able (per-commit revision-on-retry) — since this ledger uses ON CONFLICT DO NOTHING (first write wins forever), confirm the two call sites can never legitimately disagree on `decision` for the same head_sha, or the ledger will permanently record a stale/incorrect real_decision that diverges from contributor_gate_history for the same key.
  • src/review/predicted-gate-calibration-ledger.ts:52-59: the correlation query matches the most recent predicted_gate_calls row for (project, login) within a 7-day window without any PR-specific key (inherited from feat(review): record MCP predicted-gate verdicts to review_audit and measure predicted-vs-live gate agreement #4516's predicted_gate_calls design, which has no PR number at predict-time) — for a contributor with multiple concurrent PRs on the same repo, this can mis-pair a prediction meant for PR A to the real decision of PR B; worth flagging since this PR upgrades that noise-tolerant aggregate signal into a persisted, immutable per-PR 'ground truth' row.
  • The CORRELATION_WINDOW_MS constant is intentionally duplicated rather than imported from predicted-gate-agreement.ts (per the module comment) — reasonable, but consider a shared test asserting the two constants stay equal so a future edit to one doesn't silently desync them.
  • No test exercises the actual wiring at the two processors.ts call sites (only the isolated ledger function is unit-tested) — given the ~97% patch-coverage bar applies to src/**, confirm the new call-site lines/branches in processors.ts are covered by existing processor-level tests.
  • Add a code comment or test asserting the invariant that the two recordPredictedGateCalibration call sites never fire with differing `decision` values for the same headSha, to make the first-write-wins design's safety explicit rather than implicit.
  • 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.
  • Touches a guarded path — held for manual review — A maintainer must review and merge this change.
Signal Result Evidence
Code review ✅ No blockers 1 reviewer
Linked issue ✅ Linked #4517
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 (1 linked issue).
Validation posture ✅ 25/25 PR body includes validation/test evidence.
Contributor workload ✅ 10/10 Author activity: 48 registered-repo PR(s), 40 merged, 353 issue(s).
Contributor context ✅ Confirmed Gittensor contributor JSONbored; Gittensor profile; 48 PR(s), 353 issue(s).
Gate result ⚠️ Not blocking Advisory; not blocking this PR.
Linked issue satisfaction

Addressed
The PR adds a new server-side-only table (predicted_gate_calibration_ledger, migration 0138) and writer (recordPredictedGateCalibration) that pairs a login's recent predict_gate verdict against the real gate decision at the same call sites as recordContributorGateDecision, using immutable ON CONFLICT DO NOTHING inserts and no miner-reachable read/write surface, directly matching the issue's core d

Review context
  • Author: JSONbored
  • Role context: owner (maintainer lane)
  • Public audience mode: oss maintainer
  • Lane context: Repository is configured for direct PR review.
  • Public profile languages: not available
  • Official Gittensor activity: 48 PR(s), 353 issue(s).
  • PR-specific overlap: none found.
Contributor next steps
  • Treat this as maintainer-lane context rather than normal contributor-lane activity.
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.

🟩 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.

  • Re-run Gittensory review

@loopover-orb loopover-orb Bot added the manual-review Gittensor contributor context label Jul 10, 2026
JSONbored added a commit that referenced this pull request Jul 10, 2026
…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.
@JSONbored
JSONbored merged commit d1cb719 into main Jul 10, 2026
12 checks passed
@JSONbored
JSONbored deleted the feat/predicted-gate-calibration-ledger branch July 10, 2026 05:43
JSONbored added a commit that referenced this pull request Jul 10, 2026
…-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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

gittensor:feature Gittensor-scored feature linked to a feature issue — scores a 0.25x multiplier. manual-review Gittensor contributor context

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat(review): persist a login-keyed predict-gate-vs-live-gate disagreement ledger as the review stack's own calibration ground truth

1 participant