Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
-- Supports the install-wide submitter-reputation read (#4513): a fleet identity spreading thin across many
-- repos in one self-hosted install never accumulates enough same-repo sample density for the per-repo
-- reputation signal (src/review/submitter-reputation.ts) to ever fire, since that signal is scoped to
-- `WHERE project = ? AND submitter = ?`. review_targets already carries installation_id (migrations/0050),
-- so a CONFIRMED official Gittensor miner can additionally be evaluated across every repo in the same
-- install via `WHERE installation_id = ? AND submitter = ?` -- this index makes that query as cheap as the
-- existing per-project one instead of a full-table scan.
CREATE INDEX IF NOT EXISTS idx_review_targets_installation_submitter_terminal ON review_targets (installation_id, submitter, terminal_at);
30 changes: 30 additions & 0 deletions src/gittensor/miner-detection-cache.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
// Minimal, cached "is this login a CONFIRMED official Gittensor miner" check (#4512/#4513), shared by call
// sites that need only a boolean identity check -- not the full audit-logged flow processors.ts's
// getCachedOfficialMinerDetection uses for PR-comment command authorization. Same cache table/TTLs, no
// audit-log side effect. Deliberately its own module (not exported from processors.ts) so review/-layer
// code (unlinked-issue-guardrail.ts, reputation-wire.ts) can import it without a circular dependency —
// processors.ts is the one that imports FROM those modules.

import { getFreshOfficialMinerDetection, upsertOfficialMinerDetection } from "../db/repositories";
import { fetchOfficialGittensorMiner } from "./api";

const OFFICIAL_MINER_DETECTION_TTL_MS = 5 * 60 * 1000;
const OFFICIAL_MINER_DETECTION_UNAVAILABLE_TTL_MS = 60 * 1000;

/** Fail-safe: any lookup failure resolves to "not a confirmed miner," never the reverse. */
export async function isConfirmedOfficialMiner(env: Env, login: string): Promise<boolean> {
const cached = await getFreshOfficialMinerDetection(env, login).catch(() => null);
if (cached) return cached.status === "confirmed";
// fetchOfficialGittensorMiner already converts every failure into a returned {status: "unavailable"}
// value rather than rejecting -- nothing to catch here.
const detection = await fetchOfficialGittensorMiner(login);
// A cache-write failure must never block the caller from using the freshly-fetched (just uncached)
// detection -- worst case, the next call re-fetches instead of hitting the cache.
const cacheable = await upsertOfficialMinerDetection(
env,
login,
detection,
detection.status === "unavailable" ? OFFICIAL_MINER_DETECTION_UNAVAILABLE_TTL_MS : OFFICIAL_MINER_DETECTION_TTL_MS,
).catch(() => detection);
return cacheable.status === "confirmed";
}
5 changes: 3 additions & 2 deletions src/queue/processors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -519,6 +519,7 @@ import { evaluateWithSurfaceLane } from "../review/content-lane-wire";
import { reviewThreadBlockerFinding } from "../review/review-thread-findings";
import { indexRepo, reindexChangedPaths } from "../review/rag-index";
import {
getEffectiveSubmitterReputation,
isReputationEnabled,
recordReputationOutcome,
shouldSkipAiForReputation,
Expand Down Expand Up @@ -569,7 +570,7 @@ import {
import { neutralHoldReasonCode, nativeGateActionFromConclusion, recordNativeGateDecision } from "../review/parity-wire";
import { recordContributorGateDecision } from "../review/contributor-calibration";
import { recordPredictedGateCalibration } from "../review/predicted-gate-calibration-ledger";
import { getSubmitterReputation, type SubmissionOutcome } from "../review/submitter-reputation";
import type { SubmissionOutcome } from "../review/submitter-reputation";
import type {
AdvisoryFinding,
AiContentBlock,
Expand Down Expand Up @@ -8498,7 +8499,7 @@ export async function runVisualVisionForAdvisory(
): Promise<void> {
if (args.mode === "paused" || args.routes.length === 0) return;
try {
const visionReputation = await getSubmitterReputation(env, args.repoFullName, args.author ?? undefined);
const visionReputation = await getEffectiveSubmitterReputation(env, { repoFullName: args.repoFullName, submitter: args.author ?? undefined });
// BYOK resolution mirrors runAiReviewForAdvisory's own (re-resolved per-caller is this codebase's
// established convention for this exact 3-line block, not an anti-pattern — see e.g. runAiSlopForAdvisory).
const storedVisionKey =
Expand Down
46 changes: 42 additions & 4 deletions src/review/reputation-wire.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,15 @@
// the AI-spend decision (private, server-side) and writes the private submitter_stats table. Fully fail-safe:
// the ported module degrades to "neutral" / no-op on any DB error, so this never throws into the gate.

import { getRepository } from "../db/repositories";
import { isConfirmedOfficialMiner } from "../gittensor/miner-detection-cache";
import {
getSubmitterCadence,
getSubmitterReputation,
getSubmitterReputationAcrossInstall,
isMachinePacedCadence,
recordSubmissionOutcome,
type ReputationConfig,
type SubmissionOutcome,
type SubmitterStats,
} from "./submitter-reputation";
Expand Down Expand Up @@ -56,9 +60,40 @@ export function shouldDowngradeToDeterministic(stats: SubmitterStats): boolean {
}

/**
* Flag-gated, fail-safe: read the submitter's INTERNAL reputation and report whether the AI-spend gate should
* downgrade to a deterministic-only review. When the flag is OFF this returns false IMMEDIATELY — no DB read —
* so the AI-spend gate is byte-identical to today. `project` namespaces the per-(project, submitter) rows
* Resolve the EFFECTIVE reputation signal for a submitter (#4513): the per-repo signal from
* {@link getSubmitterReputation}, additionally widened to an 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
* (non-miner) submitter or one already flagged per-repo pays no extra lookup. Closes a real blind spot: a
* fleet identity spreading thin across many repos in one install never accumulates same-repo sample density,
* so the per-repo-only signal stays permanently "neutral" for it even while it burns full AI-review spend on
* every submission. Fail-safe throughout: an identity-check or install-wide-read failure just keeps the
* per-repo result, never throws, never upgrades a signal the per-repo read didn't already produce.
*/
export async function getEffectiveSubmitterReputation(
env: Env,
args: { repoFullName: string; submitter: string | null | undefined },
cfg?: ReputationConfig,
): Promise<SubmitterStats> {
const perRepo = await getSubmitterReputation(env, args.repoFullName, args.submitter ?? undefined, cfg);
if (shouldDowngradeToDeterministic(perRepo)) return perRepo;
const submitter = args.submitter?.trim();
if (!submitter) return perRepo;
/* v8 ignore next -- isConfirmedOfficialMiner already catches every internal failure point itself and never rejects; this guards only a future implementation change. */
const isMiner = await isConfirmedOfficialMiner(env, submitter).catch(() => false);
if (!isMiner) return perRepo;
const repo = await getRepository(env, args.repoFullName).catch(() => null);
if (!repo?.installationId) return perRepo;
// getSubmitterReputationAcrossInstall already degrades to neutral internally on any read failure (mirrors
// getSubmitterReputation) -- nothing to catch here.
const acrossInstall = await getSubmitterReputationAcrossInstall(env, repo.installationId, submitter, cfg);
return shouldDowngradeToDeterministic(acrossInstall) ? acrossInstall : perRepo;
}

/**
* Flag-gated, fail-safe: read the submitter's INTERNAL reputation (install-wide-aware for a confirmed miner,
* see {@link getEffectiveSubmitterReputation}) and report whether the AI-spend gate should downgrade to a
* deterministic-only review. When the flag is OFF this returns false IMMEDIATELY — no DB read — so the
* AI-spend gate is byte-identical to today. `project` namespaces the per-(project, submitter) rows
* (gittensory uses the repo full name). NEVER throws: the ported module already degrades to neutral on error.
*
* Also checks submission CADENCE (#4514): every quality-based signal above only tells you whether a
Expand All @@ -72,7 +107,10 @@ export async function shouldSkipAiForReputation(
args: { project: string; submitter: string | null | undefined },
): Promise<boolean> {
if (!isReputationEnabled(env)) return false;
const stats = await getSubmitterReputation(env, args.project, args.submitter ?? undefined);
// Combines both extensions to the base per-repo signal: install-wide widening for a confirmed miner
// (#4513, getEffectiveSubmitterReputation) first, then the cadence check (#4514) as an independent
// second signal -- neither subsumes the other, so both must run, not just whichever merged more recently.
const stats = await getEffectiveSubmitterReputation(env, { repoFullName: args.project, submitter: args.submitter });
if (shouldDowngradeToDeterministic(stats)) return true;
const cadence = await getSubmitterCadence(env, args.project, args.submitter ?? undefined);
return isMachinePacedCadence(cadence);
Expand Down
39 changes: 39 additions & 0 deletions src/review/submitter-reputation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -308,3 +308,42 @@ export async function getSubmitterReputation(env: Env, project: string, submitte
const decided = agg.merged + agg.closed;
return { ...agg, closeRate: decided > 0 ? agg.closed / decided : 0, signal };
}

/** Install-wide sibling of {@link getSubmitterReputation} (#4513): the SAME quality-weighted, recency-windowed
* signal derivation, but aggregated across EVERY repo `review_targets` has recorded for this installation_id
* (migrations/0050), not just one project. Closes a real blind spot: a fleet identity spreading thin across
* many repos in one self-hosted install never accumulates same-repo sample density for the per-project
* signal to ever leave "neutral," even while it burns full paid AI-review spend on every submission. Callers
* should reserve this for a CONFIRMED official Gittensor miner identity (this function does not itself check
* that) -- an ordinary contributor's reputation stays intentionally scoped per-repo. The all-time
* submitter_stats aggregate (submissions/merged/closed/manual, /stats-view only, not the signal) is NOT
* widened here: that table is keyed (project, submitter) with no installation_id column, and only the
* SIGNAL — not the display counts — gates the AI-spend decision. Fail-safe: any read error degrades to
* "neutral", identical to the per-project function. */
export async function getSubmitterReputationAcrossInstall(
env: Env,
installationId: number,
submitter: string | undefined,
cfg: ReputationConfig = DEFAULT_REPUTATION_CONFIG,
): Promise<SubmitterStats> {
const neutral: SubmitterStats = { submissions: 0, merged: 0, closed: 0, manual: 0, closeRate: 0, signal: "neutral" };
if (!submitter) return neutral;
let signal: ReputationSignal = "neutral";
try {
const result = await storage(env)
.prepare(
`SELECT status, json_extract(decision_json, '$.reasonCode') AS reasonCode
FROM review_targets
WHERE installation_id = ? AND submitter = ? AND terminal_at IS NOT NULL AND terminal_at >= datetime('now', ?)
ORDER BY terminal_at DESC LIMIT ?`,
)
.bind(installationId, submitter, `-${cfg.windowDays} days`, REPUTATION_WINDOW_ROW_CAP)
.all<{ status: string; reasonCode: string | null }>();
/* v8 ignore next -- D1's .all() always populates results; the fallback only protects a driver anomaly. */
const rows = result?.results ?? [];
signal = signalFromCounts(countOutcomes(rows), cfg);
} catch {
signal = "neutral"; // fail-safe — never throw into the gate.
}
return { ...neutral, signal };
}
97 changes: 97 additions & 0 deletions test/unit/miner-detection-cache.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,97 @@
import { describe, expect, it, vi } from "vitest";
import { isConfirmedOfficialMiner } from "../../src/gittensor/miner-detection-cache";
import { createTestEnv } from "../helpers/d1";

function stubMinerFetch(githubUsername: string) {
return vi.fn(async (input: RequestInfo | URL) => {
const url = input.toString();
if (url === "https://api.gittensor.io/miners") return Response.json([{ githubUsername, githubId: "123", totalPrs: 2, totalMergedPrs: 2, isEligible: true, credibility: 1 }]);
if (url === "https://api.gittensor.io/miners/123/prs") return Response.json([]);
if (url === "https://api.gittensor.io/miners/123") return Response.json({});
if (url === "https://mirror.gittensor.io/api/v1/miners/123/issues") return Response.json({ issues: [] });
return Response.json({});
});
}

describe("isConfirmedOfficialMiner (#4513, shared with #4512's unlinked-issue-guardrail)", () => {
it("resolves true for a login present in the /miners roster", async () => {
const env = createTestEnv();
vi.stubGlobal("fetch", stubMinerFetch("farmer99"));
try {
expect(await isConfirmedOfficialMiner(env, "farmer99")).toBe(true);
} finally {
vi.unstubAllGlobals();
}
});

it("resolves false for a login absent from an empty /miners roster (not_found)", async () => {
const env = createTestEnv();
vi.stubGlobal("fetch", async (input: RequestInfo | URL) => {
const url = input.toString();
if (url === "https://api.gittensor.io/miners") return Response.json([]);
return Response.json({});
});
try {
expect(await isConfirmedOfficialMiner(env, "farmer99")).toBe(false);
} finally {
vi.unstubAllGlobals();
}
});

it("resolves false (fail-safe) when the Gittensor API itself is unavailable", async () => {
const env = createTestEnv();
vi.stubGlobal("fetch", async () => {
throw new Error("network down");
});
try {
expect(await isConfirmedOfficialMiner(env, "farmer99")).toBe(false);
} finally {
vi.unstubAllGlobals();
}
});

it("a second call within the TTL hits the cache instead of re-fetching", async () => {
const env = createTestEnv();
const fetchMock = stubMinerFetch("farmer99");
vi.stubGlobal("fetch", fetchMock);
try {
expect(await isConfirmedOfficialMiner(env, "farmer99")).toBe(true);
const callsAfterFirst = fetchMock.mock.calls.length;
expect(callsAfterFirst).toBeGreaterThan(0);
expect(await isConfirmedOfficialMiner(env, "farmer99")).toBe(true);
expect(fetchMock.mock.calls.length).toBe(callsAfterFirst); // cache hit -- no additional fetch
} finally {
vi.unstubAllGlobals();
}
});

it("a cache READ failure falls back to a fresh fetch rather than a false negative", async () => {
const env = createTestEnv();
vi.stubGlobal("fetch", stubMinerFetch("farmer99"));
const realPrepare = env.DB.prepare.bind(env.DB);
env.DB.prepare = ((sql: string) => {
if (/SELECT.*FROM.*official_miner_detections/i.test(sql)) throw new Error("d1 down");
return realPrepare(sql);
}) as typeof env.DB.prepare;
try {
expect(await isConfirmedOfficialMiner(env, "farmer99")).toBe(true);
} finally {
vi.unstubAllGlobals();
}
});

it("a cache WRITE failure still uses the freshly-fetched status for this call", async () => {
const env = createTestEnv();
vi.stubGlobal("fetch", stubMinerFetch("farmer99"));
const realPrepare = env.DB.prepare.bind(env.DB);
env.DB.prepare = ((sql: string) => {
if (/INSERT INTO.*official_miner_detections/i.test(sql)) throw new Error("d1 down");
return realPrepare(sql);
}) as typeof env.DB.prepare;
try {
expect(await isConfirmedOfficialMiner(env, "farmer99")).toBe(true);
} finally {
vi.unstubAllGlobals();
}
});
});
10 changes: 8 additions & 2 deletions test/unit/queue.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3993,19 +3993,25 @@ describe("queue processors", () => {
GITTENSORY_REVIEW_REPUTATION: "true",
});
await seedRegateChurnRepo(env);
await upsertPullRequestFromGitHub(env, "JSONbored/gittensory", { number: 62, title: "Reputation PR", state: "open", user: { login: "contributor" }, head: { sha: "a62" }, labels: [], body: "Closes #1" });
// Deliberately NOT "contributor" -- several other tests earlier in this file (e.g. line ~3732) stub the
// Gittensor miners endpoint to confirm "contributor" as an official miner, and that caches a "confirmed"
// official_miner_detections row (5-min TTL) in this file's shared D1 instance. A submitter this test's own
// fetch stub never confirms must still resolve as NOT a miner, or #4513's install-wide widening adds a 4th
// reputation-scan prepare and this invariant's count goes stale for reasons unrelated to what it's testing.
await upsertPullRequestFromGitHub(env, "JSONbored/gittensory", { number: 62, title: "Reputation PR", state: "open", user: { login: "reputation-single-read-user" }, head: { sha: "a62" }, labels: [], body: "Closes #1" });
await upsertPullRequestDetailSyncState(env, { repoFullName: "JSONbored/gittensory", pullNumber: 62, status: "complete", reviewsSyncedAt: new Date().toISOString() });
vi.stubGlobal("fetch", async (input: RequestInfo | URL, init?: RequestInit) => {
const url = input.toString();
const method = init?.method ?? "GET";
if (url.includes("/access_tokens")) return Response.json({ token: "fake-installation-token" });
if (url.includes("/pulls/62/files")) return Response.json([{ filename: "src/a.ts", status: "modified", additions: 1, deletions: 0, changes: 1, patch: "@@\n+export const ok = true;" }]);
if (url.endsWith("/pulls/62")) return Response.json({ number: 62, title: "Reputation PR", state: "open", user: { login: "contributor" }, head: { sha: "a62" }, labels: [], body: "Closes #1", mergeable_state: "clean" });
if (url.endsWith("/pulls/62")) return Response.json({ number: 62, title: "Reputation PR", state: "open", user: { login: "reputation-single-read-user" }, head: { sha: "a62" }, labels: [], body: "Closes #1", mergeable_state: "clean" });
if (url.includes("/commits/a62/check-runs")) return Response.json({ total_count: 0, check_runs: [] });
if (url.includes("/commits/a62/status")) return Response.json({ state: "success", statuses: [] });
if (url.includes("/issues/62/comments")) return method === "POST" ? Response.json({ id: 62 }, { status: 201 }) : Response.json([]);
if (url.includes("/issues/1")) return Response.json({ number: 1, title: "Issue", state: "open", labels: [], user: { login: "reporter" } });
if (url.includes("/branches/")) return Response.json({ protected: false, protection: { required_status_checks: { contexts: [] } } });
if (url.includes("/miners")) return Response.json([]);
return Response.json({});
});

Expand Down
Loading
Loading