fix(review): make submitter-reputation burst/AI-spend defense install-wide for confirmed miners#4546
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #4546 +/- ##
=======================================
Coverage 94.09% 94.09%
=======================================
Files 430 431 +1
Lines 38215 38242 +27
Branches 13931 13940 +9
=======================================
+ Hits 35958 35985 +27
Misses 1600 1600
Partials 657 657
🚀 New features to boost your workflow:
|
|
Superagent didn't find any vulnerabilities or security issues in this PR. |
|
Warning 🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨 ⏸️ Gittensory review result - manual review recommendedReview updated: 2026-07-10 07:44:45 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.
|
9f9172f to
b014d39
Compare
|
Renumbered the migration 0130 → 0131 and rebased onto main — #4538 merged |
662bf6d to
d597b22
Compare
…-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
…ady merged to main)
…ty 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.
…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.
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.
d597b22 to
c888b2c
Compare
# Conflicts: # src/queue/processors.ts
…oss-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.
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
gittensory-ui | 7a749dd | Commit Preview URL Branch Preview URL |
Jul 10 2026, 07:37 AM |
Summary
getSubmitterReputation's burst/low-sample thresholds are scopedWHERE 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 submission burns full paid AI-review spend indefinitely.getSubmitterReputationAcrossInstall(submitter-reputation.ts): identical quality-weighted signal derivation, scoped byreview_targets.installation_id(already existed,migrations/0050) instead ofproject. New migration adds the supporting index.getEffectiveSubmitterReputation(reputation-wire.ts): the per-repo signal, additionally widened to the install-wide view for a CONFIRMED official Gittensor miner — 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.src/gittensor/miner-detection-cache.tsso both this module andunlinked-issue-guardrail.tscan use it without a circular import throughprocessors.ts.shouldSkipAiForReputation(the main AI-spend gate) and the vision-review reputation check.submitter_statsdisplay aggregate (the/statsview) is deliberately NOT widened — only the SIGNAL (which actually gates AI spend) is; that table has noinstallation_idcolumn and isn't security-relevant.Test plan
test/unit/miner-detection-cache.test.ts(new) — confirmed/not-found/unavailable, cache-hit, cache-read/write-failure pathstest/unit/submitter-reputation.test.ts— 6 new tests forgetSubmitterReputationAcrossInstalltest/unit/reputation-wiring.test.ts— 7 new tests forgetEffectiveSubmitterReputation: widens for a confirmed miner with a real cross-repo pattern, does NOT widen for an unconfirmed submitter with the identical pattern, skips the identity lookup entirely when the per-repo signal already justifies downgrading, and fails safe on no-submitter / no-installationId / repository-lookup-throws / cross-repo-signal-also-neutralnpx tsc --noEmitcleanFixes #4513