diff --git a/src/api/routes.ts b/src/api/routes.ts index ab9d16b2b1..75d25881fb 100644 --- a/src/api/routes.ts +++ b/src/api/routes.ts @@ -2497,14 +2497,21 @@ export function createApp() { // #554 gate false-positive telemetry: is the gate PRECISE? Read-only measurement of blocked-then-merged // (and overridden) per gate type — the evidence a maintainer needs before promoting a gate to block. NEVER // adjusts a gate. Maintainer-authenticated, repo-scoped; no public route. Optional ?windowDays bounds the - // block ledger window. + // block ledger window. Optional ?includeCohorts=true (#4520) adds an additive miner-vs-human split — an + // extra Gittensor API call, so it's opt-in rather than always computed. app.get("/v1/repos/:owner/:repo/gate-precision", async (c) => { const fullName = `${c.req.param("owner")}/${c.req.param("repo")}`; const gate = await requireRepoMaintainer(c, fullName); if (gate instanceof Response) return gate; const windowDaysRaw = Number(c.req.query("windowDays")); const windowDays = windowDaysRaw > 0 ? windowDaysRaw : undefined; - return c.json(await loadGatePrecisionReport(c.env, fullName, windowDays !== undefined ? { windowDays } : {})); + const includeCohorts = c.req.query("includeCohorts") === "true"; + return c.json( + await loadGatePrecisionReport(c.env, fullName, { + ...(windowDays !== undefined ? { windowDays } : {}), + ...(includeCohorts ? { includeCohorts } : {}), + }), + ); }); // #2228 maintainer queue-noise triage: read-only report for MCP stdio proxy + maintainer tooling. diff --git a/src/gittensor/api.ts b/src/gittensor/api.ts index 355564c5fa..83c666211f 100644 --- a/src/gittensor/api.ts +++ b/src/gittensor/api.ts @@ -186,6 +186,28 @@ export async function fetchOfficialGittensorMiner(login: string): Promise> { + try { + const miners = await fetchJson(`${GITTENSOR_API_BASE}/miners`); + const logins = new Set(); + for (const miner of miners) { + const login = miner.githubUsername?.toLowerCase(); + if (login) logins.add(login); + } + return logins; + } catch { + return new Set(); + } +} + export function contributorRepoStatsFromGittensor(snapshot: GittensorContributorSnapshot | null): ContributorRepoStatRecord[] { if (!snapshot) return []; return snapshot.repositories.map((repo) => ({ diff --git a/src/services/gate-precision.ts b/src/services/gate-precision.ts index ee96f47c4e..8492f81511 100644 --- a/src/services/gate-precision.ts +++ b/src/services/gate-precision.ts @@ -15,6 +15,7 @@ // Privacy: the report carries repo full name + PR-derived counts + gate-type codes ONLY — no actor logins, no // trust/reward/credibility numbers. Internal/maintainer-authenticated; never publicly exposed. import { listGateOutcomes, listPullRequests } from "../db/repositories"; +import { fetchOfficialGittensorMinerLogins } from "../gittensor/api"; import type { GateOutcomeRecord, PullRequestRecord } from "../types"; import { nowIso } from "../utils/json"; @@ -29,6 +30,13 @@ export type GatePrecisionPerType = { falsePositiveRate: number | null; }; +/** #4520: one cohort's fold result -- the SAME shape the blended report already carries, so a dashboard can + * render miner/human side by side with the identical component it already uses for the blended totals. */ +export type GatePrecisionCohortReport = { + perGateType: GatePrecisionPerType[]; + overall: { blocked: number; blockedThenMerged: number; falsePositiveRate: number | null }; +}; + export type GatePrecisionReport = { repoFullName: string; generatedAt: string; @@ -36,6 +44,11 @@ export type GatePrecisionReport = { perGateType: GatePrecisionPerType[]; overall: { blocked: number; blockedThenMerged: number; falsePositiveRate: number | null }; signals: string[]; + /** #4520: miner-vs-human split, present only when the caller supplied minerLogins (loadGatePrecisionReport's + * includeCohorts option). Purely additive -- never replaces the blended perGateType/overall above, and + * every existing caller that doesn't ask for it sees byte-identical output. An outcome whose PR author is + * unresolvable or not a confirmed miner falls into `human` (fail-safe: never over-classify as miner). */ + cohorts?: { miner: GatePrecisionCohortReport; human: GatePrecisionCohortReport } | undefined; }; function round(value: number): number { @@ -54,31 +67,15 @@ function sameRepo(a: string | null | undefined, b: string): boolean { return (a ?? "").toLowerCase() === b.toLowerCase(); } -/** - * Per-gate-type false-positive measurement over recorded gate blocks. Pure. For each block row we look up the - * PR's terminal outcome; a blocked PR that later MERGED is a false positive. Each blocker `code` on the row - * contributes to that code's bucket (a block citing two codes counts toward both). Overridden-then-merged is - * the strongest signal — `overridden` is counted separately per type. When `options.repoFullName` is given, - * only blocks for that repo are counted. The rate is null below MIN_SAMPLE. - */ -export function buildGatePrecisionReport( - outcomes: GateOutcomeRecord[], - pullRequests: PullRequestRecord[], - options: { repoFullName?: string } = {}, -): Omit { - const repoFullName = options.repoFullName; - // Index PRs by number for an O(1) terminal-outcome lookup, scoped to the repo when one is given. - const prByNumber = new Map(); - for (const pr of pullRequests) { - if (repoFullName && !sameRepo(pr.repoFullName, repoFullName)) continue; - prByNumber.set(pr.number, pr); - } - const scoped = repoFullName ? outcomes.filter((o) => sameRepo(o.repoFullName, repoFullName)) : outcomes; - +/** #4520: the fold core, extracted so buildGatePrecisionReport can run it up to three times (blended, miner, + * human) over disjoint outcome subsets without duplicating the accumulation logic. Pure -- the same + * MIN_SAMPLE floor is applied independently per call, so a small cohort correctly reads null rather than a + * noisy rate. */ +function foldGateOutcomes(outcomes: GateOutcomeRecord[], prByNumber: Map): GatePrecisionCohortReport { const perType = new Map(); let overallBlocked = 0; let overallMerged = 0; - for (const outcome of scoped) { + for (const outcome of outcomes) { const pr = prByNumber.get(outcome.pullNumber); // A blocked PR that later MERGED is a false positive; closed/open are not (the block held or is unresolved). const merged = pr ? terminalOutcome(pr) === "merged" : false; @@ -111,7 +108,58 @@ export function buildGatePrecisionReport( blockedThenMerged: overallMerged, falsePositiveRate: overallBlocked >= MIN_SAMPLE ? round(overallMerged / overallBlocked) : null, }, - signals: buildGatePrecisionSignals(perGateType, overallBlocked, overallMerged), + }; +} + +/** #4520: true when the outcome's PR author (looked up via prByNumber) is a confirmed miner login. Fail-safe + * on every unresolvable path (no PR record, no author) -- defaults to NOT a miner, never the reverse, + * matching this codebase's "unconfirmed defaults to human/non-miner" convention throughout. */ +function isMinerAuthoredOutcome(outcome: GateOutcomeRecord, prByNumber: Map, minerLogins: ReadonlySet): boolean { + const authorLogin = prByNumber.get(outcome.pullNumber)?.authorLogin; + return authorLogin ? minerLogins.has(authorLogin.toLowerCase()) : false; +} + +/** + * Per-gate-type false-positive measurement over recorded gate blocks. Pure. For each block row we look up the + * PR's terminal outcome; a blocked PR that later MERGED is a false positive. Each blocker `code` on the row + * contributes to that code's bucket (a block citing two codes counts toward both). Overridden-then-merged is + * the strongest signal — `overridden` is counted separately per type. When `options.repoFullName` is given, + * only blocks for that repo are counted. The rate is null below MIN_SAMPLE. When `options.minerLogins` is + * given (#4520), an additive miner-vs-human `cohorts` split is computed on top of the SAME blended fold; + * omitting it keeps every existing caller byte-identical. + */ +export function buildGatePrecisionReport( + outcomes: GateOutcomeRecord[], + pullRequests: PullRequestRecord[], + options: { repoFullName?: string; minerLogins?: ReadonlySet } = {}, +): Omit { + const repoFullName = options.repoFullName; + // Index PRs by number for an O(1) terminal-outcome lookup, scoped to the repo when one is given. + const prByNumber = new Map(); + for (const pr of pullRequests) { + if (repoFullName && !sameRepo(pr.repoFullName, repoFullName)) continue; + prByNumber.set(pr.number, pr); + } + const scoped = repoFullName ? outcomes.filter((o) => sameRepo(o.repoFullName, repoFullName)) : outcomes; + + const { perGateType, overall } = foldGateOutcomes(scoped, prByNumber); + + let cohorts: GatePrecisionReport["cohorts"]; + if (options.minerLogins) { + const minerLogins = options.minerLogins; + const minerOutcomes: GateOutcomeRecord[] = []; + const humanOutcomes: GateOutcomeRecord[] = []; + for (const outcome of scoped) { + (isMinerAuthoredOutcome(outcome, prByNumber, minerLogins) ? minerOutcomes : humanOutcomes).push(outcome); + } + cohorts = { miner: foldGateOutcomes(minerOutcomes, prByNumber), human: foldGateOutcomes(humanOutcomes, prByNumber) }; + } + + return { + perGateType, + overall, + signals: buildGatePrecisionSignals(perGateType, overall.blocked, overall.blockedThenMerged), + ...(cohorts ? { cohorts } : {}), }; } @@ -133,12 +181,19 @@ export function buildGatePrecisionSignals(perGateType: GatePrecisionPerType[], o return signals; } -/** Load a repo's gate-block ledger + PRs and assemble the precision report. */ -export async function loadGatePrecisionReport(env: Env, repoFullName: string, options: { windowDays?: number } = {}): Promise { - const [pullRequests, outcomes] = await Promise.all([ +/** Load a repo's gate-block ledger + PRs and assemble the precision report. `includeCohorts` (#4520) fetches + * the full confirmed-miner login set ONCE and threads it into buildGatePrecisionReport for the additive + * miner-vs-human split; omitted (default) keeps this byte-identical to before the split existed. */ +export async function loadGatePrecisionReport( + env: Env, + repoFullName: string, + options: { windowDays?: number; includeCohorts?: boolean } = {}, +): Promise { + const [pullRequests, outcomes, minerLogins] = await Promise.all([ listPullRequests(env, repoFullName), listGateOutcomes(env, { repoFullName, ...(options.windowDays !== undefined ? { windowDays: options.windowDays } : {}) }), + options.includeCohorts ? fetchOfficialGittensorMinerLogins() : Promise.resolve(undefined), ]); - const report = buildGatePrecisionReport(outcomes, pullRequests, { repoFullName }); + const report = buildGatePrecisionReport(outcomes, pullRequests, { repoFullName, ...(minerLogins ? { minerLogins } : {}) }); return { repoFullName, generatedAt: nowIso(), windowDays: options.windowDays ?? null, ...report }; } diff --git a/test/integration/api.test.ts b/test/integration/api.test.ts index 2bfd7bc5d1..4582c65a05 100644 --- a/test/integration/api.test.ts +++ b/test/integration/api.test.ts @@ -826,6 +826,13 @@ describe("api routes", () => { // No windowDays → full window (covers the param-absent path). const gatePrecisionNoWindow = await app.request("/v1/repos/entrius/allways-ui/gate-precision", { headers: apiHeaders(env) }, env); await expect(gatePrecisionNoWindow.json()).resolves.toMatchObject({ windowDays: null }); + // #4520: ?includeCohorts=true reuses this test's own already-stubbed /miners endpoint (above) -- an + // opt-in extra Gittensor API call, so the default requests above must never trigger it. + const gatePrecisionCohorts = await app.request("/v1/repos/entrius/allways-ui/gate-precision?includeCohorts=true", { headers: apiHeaders(env) }, env); + expect(gatePrecisionCohorts.status).toBe(200); + await expect(gatePrecisionCohorts.json()).resolves.toMatchObject({ + cohorts: { miner: { overall: expect.any(Object) }, human: { overall: expect.any(Object) } }, + }); const maintainerNoiseUnauthenticated = await app.request("/v1/repos/entrius/allways-ui/maintainer-noise", {}, env); expect(maintainerNoiseUnauthenticated.status).toBe(401); diff --git a/test/unit/gate-precision.test.ts b/test/unit/gate-precision.test.ts index 07e548e507..7668bd03b8 100644 --- a/test/unit/gate-precision.test.ts +++ b/test/unit/gate-precision.test.ts @@ -1,4 +1,4 @@ -import { describe, expect, it } from "vitest"; +import { describe, expect, it, vi } from "vitest"; import { buildGatePrecisionReport, buildGatePrecisionSignals, loadGatePrecisionReport } from "../../src/services/gate-precision"; import type { GatePrecisionPerType } from "../../src/services/gate-precision"; import { @@ -16,8 +16,9 @@ function block(pullNumber: number, blockerCodes: string[], overridden = false): } // A resolved PR: `merged` → has a merge timestamp (a false positive when it was also blocked); otherwise -// closed-unmerged (the block held). `open` PRs have no terminal outcome yet. -function pr(number: number, outcome: "merged" | "closed" | "open"): PullRequestRecord { +// closed-unmerged (the block held). `open` PRs have no terminal outcome yet. `authorLogin` is optional +// (#4520 cohort-split tests only) — omitted matches every pre-existing call site exactly. +function pr(number: number, outcome: "merged" | "closed" | "open", authorLogin?: string): PullRequestRecord { return { repoFullName: "owner/repo", number, @@ -26,6 +27,7 @@ function pr(number: number, outcome: "merged" | "closed" | "open"): PullRequestR mergedAt: outcome === "merged" ? "2026-06-01T00:00:00.000Z" : null, labels: [], linkedIssues: [], + ...(authorLogin !== undefined ? { authorLogin } : {}), }; } @@ -98,6 +100,58 @@ describe("buildGatePrecisionReport", () => { }); }); +// #4520: additive miner-vs-human split, only computed when options.minerLogins is supplied. Reuses the SAME +// blocked/blockedThenMerged/overridden fold every existing test above already exercises for the blended +// report — these tests only pin the NEW split behavior, not the fold itself. +describe("buildGatePrecisionReport cohort split (#4520)", () => { + it("is absent from the report when minerLogins is not supplied (byte-identical to before the split existed)", () => { + const report = buildGatePrecisionReport([block(1, ["x"])], [pr(1, "merged", "some-miner")]); + expect(report.cohorts).toBeUndefined(); + }); + + it("splits blocked/blockedThenMerged/overridden between miner and human authors, each independently below/above MIN_SAMPLE", () => { + const minerLogins = new Set(["miner-alice"]); + // Miner: 2 blocks, 1 merged (below MIN_SAMPLE=5 -> null rate). Human: 6 blocks, 3 merged (>= 5 -> real rate). + const blocks = [ + block(1, ["x"]), block(2, ["x"]), + block(10, ["x"]), block(11, ["x"]), block(12, ["x"], true), block(13, ["x"]), block(14, ["x"]), block(15, ["x"]), + ]; + const prs = [ + pr(1, "merged", "miner-alice"), pr(2, "closed", "miner-alice"), + pr(10, "merged", "human-bob"), pr(11, "merged", "human-bob"), pr(12, "merged", "human-bob"), + pr(13, "closed", "human-bob"), pr(14, "closed", "human-bob"), pr(15, "closed", "human-bob"), + ]; + const report = buildGatePrecisionReport(blocks, prs, { minerLogins }); + expect(report.cohorts?.miner.overall).toMatchObject({ blocked: 2, blockedThenMerged: 1, falsePositiveRate: null }); + expect(report.cohorts?.human.overall).toMatchObject({ blocked: 6, blockedThenMerged: 3, falsePositiveRate: 0.5 }); + expect(report.cohorts?.human.perGateType[0]).toMatchObject({ overridden: 1 }); + // The blended totals are UNCHANGED by the split -- miner + human always reconciles to overall. + expect(report.overall).toMatchObject({ blocked: 8, blockedThenMerged: 4 }); + }); + + it("classifies an unresolvable author (no PR record, or no authorLogin) as human -- never over-classifies as miner", () => { + const minerLogins = new Set(["miner-alice"]); + // pr 1: authorLogin present but blank; pr 2: no matching PR record at all (block cites pullNumber 3). + const blocks = [block(1, ["x"]), block(3, ["x"])]; + const prs = [pr(1, "merged", "")]; + const report = buildGatePrecisionReport(blocks, prs, { minerLogins }); + expect(report.cohorts?.miner.overall.blocked).toBe(0); + expect(report.cohorts?.human.overall.blocked).toBe(2); + }); + + it("is case-insensitive when matching a PR author against minerLogins", () => { + const minerLogins = new Set(["miner-alice"]); + const report = buildGatePrecisionReport([block(1, ["x"])], [pr(1, "merged", "Miner-Alice")], { minerLogins }); + expect(report.cohorts?.miner.overall.blocked).toBe(1); + expect(report.cohorts?.human.overall.blocked).toBe(0); + }); + + it("carries no actor login in the cohort split either (privacy — only aggregate counts)", () => { + const report = buildGatePrecisionReport([block(1, ["x"])], [pr(1, "merged", "miner-alice")], { minerLogins: new Set(["miner-alice"]) }); + expect(JSON.stringify(report.cohorts)).not.toMatch(/miner-alice|login|actor/i); + }); +}); + describe("buildGatePrecisionSignals", () => { const type = (gateType: string, blocked: number, blockedThenMerged: number, falsePositiveRate: number | null, overridden = 0): GatePrecisionPerType => ({ gateType, @@ -199,4 +253,44 @@ describe("loadGatePrecisionReport (env loader)", () => { // No repoFullName → unscoped listing (exercises the absent-repo branch + the empty-conditions path). expect(await listGateOutcomes(env, {})).toHaveLength(1); }); + + // #4520: includeCohorts fetches the full miner login set ONCE (not omitted) and threads it through. + it("includeCohorts fetches the miner login set once and produces a miner-vs-human split", async () => { + const env = createTestEnv(); + await recordGateBlockOutcome(env, { repoFullName: "owner/repo", pullNumber: 1, blockerCodes: ["slop_risk"] }); + await upsertPullRequestFromGitHub(env, "owner/repo", { number: 1, title: "merged", state: "closed", user: { login: "miner-alice" }, merged_at: "2026-06-01T00:00:00.000Z" }); + await recordGateBlockOutcome(env, { repoFullName: "owner/repo", pullNumber: 2, blockerCodes: ["slop_risk"] }); + await upsertPullRequestFromGitHub(env, "owner/repo", { number: 2, title: "closed", state: "closed", user: { login: "human-bob" } }); + let minersCalls = 0; + vi.stubGlobal("fetch", async (input: RequestInfo | URL) => { + const url = input.toString(); + if (url === "https://api.gittensor.io/miners") { + minersCalls += 1; + return Response.json([{ uid: 1, githubUsername: "miner-alice", githubId: "1" }]); + } + return new Response("not found", { status: 404 }); + }); + + const withoutCohorts = await loadGatePrecisionReport(env, "owner/repo"); + expect(withoutCohorts.cohorts).toBeUndefined(); + expect(minersCalls).toBe(0); // opt-in only -- the default path never spends the extra Gittensor API call + + const withCohorts = await loadGatePrecisionReport(env, "owner/repo", { includeCohorts: true }); + expect(minersCalls).toBe(1); + expect(withCohorts.cohorts?.miner.overall).toMatchObject({ blocked: 1, blockedThenMerged: 1 }); + expect(withCohorts.cohorts?.human.overall).toMatchObject({ blocked: 1, blockedThenMerged: 0 }); + vi.unstubAllGlobals(); + }); + + it("includeCohorts degrades to an EMPTY miner set (every author reads as human) when the Gittensor API call fails", async () => { + const env = createTestEnv(); + await recordGateBlockOutcome(env, { repoFullName: "owner/repo", pullNumber: 1, blockerCodes: ["x"] }); + await upsertPullRequestFromGitHub(env, "owner/repo", { number: 1, title: "merged", state: "closed", user: { login: "miner-alice" }, merged_at: "2026-06-01T00:00:00.000Z" }); + vi.stubGlobal("fetch", async () => new Response("service unavailable", { status: 503 })); + + const report = await loadGatePrecisionReport(env, "owner/repo", { includeCohorts: true }); + expect(report.cohorts?.miner.overall.blocked).toBe(0); + expect(report.cohorts?.human.overall.blocked).toBe(1); + vi.unstubAllGlobals(); + }); }); diff --git a/test/unit/gittensor-api.test.ts b/test/unit/gittensor-api.test.ts index 9c739ff72a..ce279a9af5 100644 --- a/test/unit/gittensor-api.test.ts +++ b/test/unit/gittensor-api.test.ts @@ -1,5 +1,5 @@ import { afterEach, describe, expect, it, vi } from "vitest"; -import { contributorRepoStatsFromGittensor, fetchGittensorContributorSnapshot, fetchOfficialGittensorMiner } from "../../src/gittensor/api"; +import { contributorRepoStatsFromGittensor, fetchGittensorContributorSnapshot, fetchOfficialGittensorMiner, fetchOfficialGittensorMinerLogins } from "../../src/gittensor/api"; describe("Gittensor API contributor snapshots", () => { afterEach(() => { @@ -276,3 +276,43 @@ describe("Gittensor API contributor snapshots", () => { ); }); }); + +// #4520: the batch (fetch-once, classify-many) counterpart to fetchOfficialGittensorMiner above -- for a +// caller (a maintainer-dashboard miner-vs-human cohort split) that needs to classify many distinct +// submitters at once without one network call per login. +describe("fetchOfficialGittensorMinerLogins (#4520)", () => { + afterEach(() => { + vi.unstubAllGlobals(); + }); + + it("returns the lowercased login set from the full miner list in one call", async () => { + let calls = 0; + vi.stubGlobal("fetch", async () => { + calls += 1; + return Response.json([ + { githubUsername: "Miner-Alice", githubId: "1" }, + { githubUsername: "miner-bob", githubId: "2" }, + ]); + }); + const logins = await fetchOfficialGittensorMinerLogins(); + expect(logins).toEqual(new Set(["miner-alice", "miner-bob"])); + expect(calls).toBe(1); + }); + + it("skips a miner entry with no githubUsername rather than adding an undefined/empty login", async () => { + vi.stubGlobal("fetch", async () => Response.json([{ githubId: "1" }, { githubUsername: "miner-carol", githubId: "2" }])); + expect(await fetchOfficialGittensorMinerLogins()).toEqual(new Set(["miner-carol"])); + }); + + it("returns an empty set (never throws) when the Gittensor API call fails", async () => { + vi.stubGlobal("fetch", async () => new Response("service unavailable", { status: 503 })); + expect(await fetchOfficialGittensorMinerLogins()).toEqual(new Set()); + }); + + it("returns an empty set (never throws) on a network-level rejection", async () => { + vi.stubGlobal("fetch", async () => { + throw new Error("network down"); + }); + expect(await fetchOfficialGittensorMinerLogins()).toEqual(new Set()); + }); +});