diff --git a/migrations/0144_review_audit_miner_authored.sql b/migrations/0144_review_audit_miner_authored.sql new file mode 100644 index 0000000000..4f78819232 --- /dev/null +++ b/migrations/0144_review_audit_miner_authored.sql @@ -0,0 +1,16 @@ +-- #2352: extend the live auto-tune circuit-breaker (src/review/auto-tune.ts) to consider miner-originated PRs +-- as a distinguishable population, so a miner fleet's own self-review accuracy trips the SAME safety breaker +-- independently of the maintainer's overall (mixed) review-stack accuracy. +-- +-- review_audit (migration 0049) deliberately carries NO actor-identifying data (it feeds the anonymized +-- cross-instance export in src/selfhost/orb-collector.ts -- see that migration's own "Privacy: ... No actor +-- logins" comment). This column preserves that: it is a coarse, non-identifying miner/non-miner CATEGORY, not +-- a login -- it reveals no more than "was this PR's author, at decision time, a confirmed official Gittensor +-- miner" (src/queue/processors.ts's `confirmedContributor`, itself an aggregate boolean from the live +-- Gittensor subnet API, not a stored identity). +-- +-- Written by src/review/parity-wire.ts's recordNativeGateDecision alongside the existing `source` column (that +-- write is UNCHANGED -- this is a strictly additive column, so every existing read of review_audit, including +-- computeGateEval's current unscoped/source-scoped passes, is byte-identical unless it opts into the new +-- `minerOnly` filter). Read by src/review/parity.ts's computeGateEval when its new `minerOnly` option is set. +ALTER TABLE review_audit ADD COLUMN miner_authored INTEGER NOT NULL DEFAULT 0; diff --git a/src/queue/processors.ts b/src/queue/processors.ts index 6afb265eb0..3c3aac07fc 100644 --- a/src/queue/processors.ts +++ b/src/queue/processors.ts @@ -9200,6 +9200,9 @@ async function maybePublishPrPublicSurface( headSha: pr.headSha, conclusion: gateEvaluation.conclusion, reasonCode, + // #2352: lets the live auto-tune breaker (src/review/outcomes-wire.ts's runSelfTuneBreaker) scope a + // SEPARATE precision read to miner-originated PRs, independently of the maintainer's overall accuracy. + minerAuthored: confirmedContributor, }); // #2349 (PR 1): additive per-contributor calibration data, mirroring recordNativeGateDecision's own // action derivation above so both writers agree on whether this conclusion is a comparable decision -- diff --git a/src/review/outcomes-wire.ts b/src/review/outcomes-wire.ts index ba22c683b2..fd7c3ea26d 100644 --- a/src/review/outcomes-wire.ts +++ b/src/review/outcomes-wire.ts @@ -513,6 +513,91 @@ const BREAKER_EVAL_WINDOW_DAYS = 90; * break the cron). With no pr_outcome history the eval reads neutral → nothing engages → byte-identical. The * close breaker is INERT until selftune is enabled AND close-outcome data is present, exactly like its merge twin. */ +// #2352: the flag-scope suffix that makes a miner-originated project's breaker flags (holdonly::miner +// / closehold::miner) DISTINCT from the same project's human/mixed-population flags. Every downstream +// primitive that keys on `project` -- applyAutoTune/applyCloseAutoTune/maybeAutoClear* (auto-tune.ts), +// createFlagStore, listEngagedProjectScopes -- is already fully generic over that opaque string, so re-keying +// a report's rows with this suffix is the ENTIRE mechanism; none of those primitives needed to change. +const MINER_BREAKER_SCOPE_SUFFIX = ":miner"; + +function minerBreakerScope(project: string): string { + return `${project}${MINER_BREAKER_SCOPE_SUFFIX}`; +} + +/** Run the full engage + auto-clear sequence for one {@link GateEvalReport} (either the plain project-keyed + * report or a miner-rescoped one). `eventPrefix` namespaces the emitted log events (`""` for the existing + * human/mixed pass, `"miner_"` for the #2352 miner-scoped pass) so an operator can tell which population + * triggered a given line. `engagedHoldonly`/`engagedClosehold` are this SAME scope's already-engaged flags + * (the caller pre-splits {@link listEngagedProjectScopes}'s result by scope) -- passing the WRONG scope's + * engaged list here would auto-clear a flag using the other population's precision, which is exactly the + * cross-scope leak #2352 exists to prevent. */ +async function runBreakerPassForReport( + flags: FlagStore, + report: GateEvalReport, + engagedHoldonly: readonly string[], + engagedClosehold: readonly string[], + nowMs: number, + eventPrefix: string, +): Promise { + const engaged = await applyAutoTune(flags, report); + for (const action of engaged) { + console.error( + JSON.stringify({ + level: "error", + event: `${eventPrefix}breaker_engaged`, + project: action.project, + mergePrecision: action.mergePrecision, + decided: action.decided, + floor: AUTOTUNE_MERGE_PRECISION_FLOOR, + }), + ); + } + // CLOSE-side breaker: engage closehold for any repo whose close precision dropped below the floor. + const closeEngaged = await applyCloseAutoTune(flags, report); + for (const action of closeEngaged) { + console.error( + JSON.stringify({ + level: "error", + event: `${eventPrefix}close_breaker_engaged`, + project: action.project, + closePrecision: action.closePrecision, + decided: action.decided, + floor: AUTOTUNE_CLOSE_PRECISION_FLOOR, + }), + ); + } + // OBSERVABILITY: a single summary line of the engaged close-hold backlog so a human can see, at a glance, + // how many (and which) repos are currently holding would-closes for review. Only emitted when ≥1 engaged. + if (closeEngaged.length > 0) { + console.error( + JSON.stringify({ + level: "error", + event: `${eventPrefix}closehold_backlog`, + count: closeEngaged.length, + projects: closeEngaged.map((a) => a.project), + }), + ); + } + // Auto-clear any auto-engaged breaker (merge AND close) that has cooled down + recovered. Candidates are the + // UNION of report.rows (projects with a fresh decided sample) and every project currently holding a + // per-project flag IN THIS SCOPE (#autoclear-deadlock) — a project whose breaker is suppressing 100% of its + // merges/closes stops producing new decided samples for THAT action class and can drop out of report.rows + // entirely, which would otherwise strand its flag engaged forever regardless of how long the cooldown has + // elapsed. + const mergeClearCandidates = new Set([...report.rows.map((row) => row.project), ...engagedHoldonly]); + const closeClearCandidates = new Set([...report.rows.map((row) => row.project), ...engagedClosehold]); + for (const project of mergeClearCandidates) { + if (await maybeAutoClearHoldOnly(flags, report, project, nowMs)) { + console.log(JSON.stringify({ event: `${eventPrefix}breaker_auto_cleared`, project })); + } + } + for (const project of closeClearCandidates) { + if (await maybeAutoClearCloseHoldOnly(flags, report, project, nowMs)) { + console.log(JSON.stringify({ event: `${eventPrefix}close_breaker_auto_cleared`, project })); + } + } +} + export async function runSelfTuneBreaker(env: Env): Promise { try { const nowMs = Date.now(); @@ -521,64 +606,41 @@ export async function runSelfTuneBreaker(env: Env): Promise { nowMs, source: GITTENSORY_NATIVE_SOURCE, }); + // #2352: a SEPARATE, miner-scoped pass so a miner fleet's own self-review accuracy trips the SAME breaker + // independently of the maintainer's overall (mixed) accuracy. Re-keying every row's `project` with the + // `:miner` suffix (see MINER_BREAKER_SCOPE_SUFFIX's own doc comment) is what makes every downstream + // primitive naturally produce a DISTINCT flag, with zero changes to auto-tune.ts itself. + const minerReportRaw = await computeGateEval(env, { + days: BREAKER_EVAL_WINDOW_DAYS, + nowMs, + source: GITTENSORY_NATIVE_SOURCE, + minerOnly: true, + }); + const minerReport: GateEvalReport = { + hasSignal: minerReportRaw.hasSignal, + rows: minerReportRaw.rows.map((row) => ({ ...row, project: minerBreakerScope(row.project) })), + }; + const flags = createFlagStore(env); - const engaged = await applyAutoTune(flags, report); - for (const action of engaged) { - console.error( - JSON.stringify({ - level: "error", - event: "breaker_engaged", - project: action.project, - mergePrecision: action.mergePrecision, - decided: action.decided, - floor: AUTOTUNE_MERGE_PRECISION_FLOOR, - }), - ); - } - // CLOSE-side breaker: engage closehold for any repo whose close precision dropped below the floor. - const closeEngaged = await applyCloseAutoTune(flags, report); - for (const action of closeEngaged) { - console.error( - JSON.stringify({ - level: "error", - event: "close_breaker_engaged", - project: action.project, - closePrecision: action.closePrecision, - decided: action.decided, - floor: AUTOTUNE_CLOSE_PRECISION_FLOOR, - }), - ); - } - // OBSERVABILITY: a single summary line of the engaged close-hold backlog so a human can see, at a glance, - // how many (and which) repos are currently holding would-closes for review. Only emitted when ≥1 engaged. - if (closeEngaged.length > 0) { - console.error( - JSON.stringify({ - level: "error", - event: "closehold_backlog", - count: closeEngaged.length, - projects: closeEngaged.map((a) => a.project), - }), - ); - } - // Auto-clear any auto-engaged breaker (merge AND close) that has cooled down + recovered. Candidates are the - // UNION of report.rows (projects with a fresh decided sample) and every project currently holding a - // per-project flag (#autoclear-deadlock) — a project whose breaker is suppressing 100% of its merges/closes - // stops producing new decided samples for THAT action class and can drop out of report.rows entirely, which - // would otherwise strand its flag engaged forever regardless of how long the cooldown has elapsed. const engagedScopes = await listEngagedProjectScopes(env); - const mergeClearCandidates = new Set([...report.rows.map((row) => row.project), ...engagedScopes.holdonly]); - const closeClearCandidates = new Set([...report.rows.map((row) => row.project), ...engagedScopes.closehold]); - for (const project of mergeClearCandidates) { - if (await maybeAutoClearHoldOnly(flags, report, project, nowMs)) { - console.log(JSON.stringify({ event: "breaker_auto_cleared", project })); - } - } - for (const project of closeClearCandidates) { - if (await maybeAutoClearCloseHoldOnly(flags, report, project, nowMs)) { - console.log(JSON.stringify({ event: "close_breaker_auto_cleared", project })); - } - } + const isMinerScope = (project: string): boolean => project.endsWith(MINER_BREAKER_SCOPE_SUFFIX); + + await runBreakerPassForReport( + flags, + report, + engagedScopes.holdonly.filter((project) => !isMinerScope(project)), + engagedScopes.closehold.filter((project) => !isMinerScope(project)), + nowMs, + "", + ); + await runBreakerPassForReport( + flags, + minerReport, + engagedScopes.holdonly.filter(isMinerScope), + engagedScopes.closehold.filter(isMinerScope), + nowMs, + "miner_", + ); } catch (error) { console.warn( JSON.stringify({ diff --git a/src/review/parity-wire.ts b/src/review/parity-wire.ts index 918e904006..6adcb09325 100644 --- a/src/review/parity-wire.ts +++ b/src/review/parity-wire.ts @@ -137,7 +137,19 @@ type ParityRecorderEnv = { */ export async function recordNativeGateDecision( env: ParityRecorderEnv, - input: { project: string; pullNumber: number; headSha: string | null | undefined; conclusion: GateCheckConclusion; reasonCode?: string | null | undefined; action?: GateAction | undefined }, + input: { + project: string; + pullNumber: number; + headSha: string | null | undefined; + conclusion: GateCheckConclusion; + reasonCode?: string | null | undefined; + action?: GateAction | undefined; + /** #2352: true when the PR's author is a confirmed official Gittensor miner (processors.ts's + * `confirmedContributor`) at decision time. A coarse, non-identifying category -- NOT a login -- so this + * stays within review_audit's own "no actor-identifying data" design (see migration 0144's own comment). + * Omitted defaults to `false` (not miner-originated), matching every pre-#2352 caller unchanged. */ + minerAuthored?: boolean | undefined; + }, ): Promise { // Self-hosted instances always record (their own local DB; exportOrbBatch needs this data). The cloud // worker keeps the exact flag-gated, byte-identical-when-off contract. @@ -148,16 +160,17 @@ export async function recordNativeGateDecision( const project = input.project.slice(0, 200); const targetId = `${project}#${input.pullNumber}`; const summary = input.reasonCode ? input.reasonCode.slice(0, 200) : null; + const minerAuthored = input.minerAuthored === true ? 1 : 0; try { // Deterministic id per (source, project, pr, sha): a re-run at the SAME commit REPLACES its prior decision // (the latest finalize wins), while a new commit gets its own row. event_type/source default in the schema // but are written explicitly for clarity. await env.DB.prepare( - `INSERT INTO review_audit (id, project, target_id, event_type, decision, source, head_sha, summary, created_at) - VALUES (?, ?, ?, 'gate_decision', ?, ?, ?, ?, ?) - ON CONFLICT(id) DO UPDATE SET decision = excluded.decision, summary = excluded.summary, created_at = excluded.created_at`, + `INSERT INTO review_audit (id, project, target_id, event_type, decision, source, head_sha, summary, miner_authored, created_at) + VALUES (?, ?, ?, 'gate_decision', ?, ?, ?, ?, ?, ?) + ON CONFLICT(id) DO UPDATE SET decision = excluded.decision, summary = excluded.summary, miner_authored = excluded.miner_authored, created_at = excluded.created_at`, ) - .bind(`gate:${GITTENSORY_NATIVE_SOURCE}:${targetId}@${input.headSha}`, project, targetId, action, GITTENSORY_NATIVE_SOURCE, input.headSha, summary, nowIso()) + .bind(`gate:${GITTENSORY_NATIVE_SOURCE}:${targetId}@${input.headSha}`, project, targetId, action, GITTENSORY_NATIVE_SOURCE, input.headSha, summary, minerAuthored, nowIso()) .run(); } catch (error) { // Telemetry must never break finalization. diff --git a/src/review/parity.ts b/src/review/parity.ts index 4b88c2fef7..e240a61d4f 100644 --- a/src/review/parity.ts +++ b/src/review/parity.ts @@ -84,17 +84,22 @@ export const REVERSAL_DISCOUNT_WEIGHT = 0; * pr_outcome (ground truth) is the human's realized merge/close, so it is NOT source-scoped — both * systems are graded against the same answer key. Also LEFT JOINs a reversal existence check (#2348) so the * fold below can additionally compute weightedMergeConfirmed/weightedCloseConfirmed alongside the existing - * raw counts — see REVERSAL_DISCOUNT_WEIGHT's doc comment for the formula. */ -export async function computeGateEval(env: Env, opts: { days: number; nowMs: number; source?: string }): Promise { + * raw counts — see REVERSAL_DISCOUNT_WEIGHT's doc comment for the formula. + * `minerOnly` (#2352) additionally scopes the PREDICTION side to rows recorded with `miner_authored = 1` + * (migration 0144) — orthogonal to `source`: both filters AND together when both are set. Ground truth stays + * unscoped either way (same answer key). Omitted (the default, and every pre-#2352 caller) is byte-identical + * to before this option existed. */ +export async function computeGateEval(env: Env, opts: { days: number; nowMs: number; source?: string; minerOnly?: boolean }): Promise { const days = Number.isFinite(opts.days) && opts.days > 0 ? Math.min(opts.days, 730) : 90; const fromIso = new Date(opts.nowMs - days * 86_400_000).toISOString().slice(0, 10); // SQLite "bare column with MAX()" picks the column from the max-created_at row → the LATEST decision / // outcome per target (a reopened+reclosed PR keeps its final state). const sourceFilter = opts.source ? "AND source = ?" : ""; + const minerFilter = opts.minerOnly ? "AND miner_authored = 1" : ""; const sql = ` WITH gd AS ( SELECT target_id, project, decision AS pred, MAX(created_at) AS t - FROM review_audit WHERE event_type = 'gate_decision' AND decision IS NOT NULL AND created_at >= ? ${sourceFilter} + FROM review_audit WHERE event_type = 'gate_decision' AND decision IS NOT NULL AND created_at >= ? ${sourceFilter} ${minerFilter} GROUP BY target_id ), po AS ( diff --git a/test/unit/outcomes-wire.test.ts b/test/unit/outcomes-wire.test.ts index 66db4c1c52..02bd03215a 100644 --- a/test/unit/outcomes-wire.test.ts +++ b/test/unit/outcomes-wire.test.ts @@ -872,6 +872,148 @@ describe("runSelfTuneBreaker — reads recorded pr_outcome ground truth + engage }); }); +// ── #2352: the miner-scoped breaker pass, independent of the existing human/mixed-population one ────────────── + +describe("runSelfTuneBreaker — miner-scoped breaker (#2352)", () => { + async function seedDecisionAndOutcomeScoped( + env: Env, + project: string, + pr: number, + pred: "merge" | "close", + truth: "merged" | "closed", + minerAuthored: boolean, + ): Promise { + await env.DB.prepare( + "INSERT INTO review_audit (id, project, target_id, event_type, decision, source, head_sha, summary, miner_authored, created_at) VALUES (?, ?, ?, 'gate_decision', ?, 'gittensory-native', ?, NULL, ?, CURRENT_TIMESTAMP)", + ) + .bind(`gd:${minerAuthored ? "m" : "h"}:${project}#${pr}`, project, `${project}#${pr}`, pred, `sha${pr}`, minerAuthored ? 1 : 0) + .run(); + await env.DB.prepare( + "INSERT INTO review_audit (id, project, target_id, event_type, decision, source, head_sha, summary, created_at) VALUES (?, ?, ?, 'pr_outcome', ?, 'gittensory-native', NULL, NULL, CURRENT_TIMESTAMP)", + ) + .bind(`po:${minerAuthored ? "m" : "h"}:${project}#${pr}`, project, `${project}#${pr}`, truth) + .run(); + } + + // IMPORTANT (all scenarios below): the EXISTING/unscoped `report` pass is NOT disjoint from miner-authored + // data — it is `source='gittensory-native'` with NO miner_authored filter, so it counts EVERY prediction for + // a project, miner-authored or not (preserving that pass's existing, unchanged meaning: overall accuracy). + // Only the SEPARATE `minerOnly` pass excludes non-miner rows. So a project's miner-authored rows are counted + // TWICE — once in the mixed/unscoped population, once in the miner-only subset — and demonstrating "engages + // one scope but not the other" requires enough volume on the healthy side to keep the MIXED population's + // precision on the opposite side of the floor from the SUBSET's precision. + + it("ENGAGES the miner-scoped holdonly flag (holdonly::miner) when miner-authored predictions show low merge precision, while the human-scoped flag for the SAME project stays clear", async () => { + const env = createTestEnv(); + // Miner-authored: 12 would-merge, only 4 confirmed → 33% precision, below the floor. + for (let i = 0; i < 4; i += 1) await seedDecisionAndOutcomeScoped(env, "owner/repo", i, "merge", "merged", true); + for (let i = 4; i < 12; i += 1) await seedDecisionAndOutcomeScoped(env, "owner/repo", i, "merge", "closed", true); + // Human-authored, SAME project: 50 would-merge, all confirmed. Diluted into the MIXED population: (4+50) / + // (12+50) = 87.1%, above the floor — the mixed/unscoped pass reads healthy even though the miner SUBSET + // (4/12 = 33%) does not. + for (let i = 100; i < 150; i += 1) await seedDecisionAndOutcomeScoped(env, "owner/repo", i, "merge", "merged", false); + + const err = vi.spyOn(console, "error").mockImplementation(() => {}); + await runSelfTuneBreaker(env); + + expect(await isHoldOnly(env, "owner/repo:miner")).toBe(true); + expect(await isHoldOnly(env, "owner/repo")).toBe(false); + expect(err.mock.calls.some(([l]) => String(l).includes('"event":"miner_breaker_engaged"') && String(l).includes('"project":"owner/repo:miner"'))).toBe(true); + err.mockRestore(); + }); + + it("does NOT engage the miner-scoped flag when the mixed population is only dragged down by NON-miner rows — the leak #2352 exists to prevent, in the other direction", async () => { + const env = createTestEnv(); + // Human-authored: 12 would-merge, only 4 confirmed → 33% precision — drags the MIXED population's precision + // down (there is no dilution on this side: this IS the whole non-miner population). + for (let i = 0; i < 4; i += 1) await seedDecisionAndOutcomeScoped(env, "owner/repo", i, "merge", "merged", false); + for (let i = 4; i < 12; i += 1) await seedDecisionAndOutcomeScoped(env, "owner/repo", i, "merge", "closed", false); + // Miner-authored, SAME project: perfectly healthy on its own (the miner-only SUBSET reads 100%). + for (let i = 100; i < 112; i += 1) await seedDecisionAndOutcomeScoped(env, "owner/repo", i, "merge", "merged", true); + + await runSelfTuneBreaker(env); + + // Mixed: (4+12)/(12+12) = 66.7% < floor → the existing, unscoped breaker still fires (unchanged invariant). + expect(await isHoldOnly(env, "owner/repo")).toBe(true); + // Miner-only subset: 12/12 = 100% >= floor → must NOT engage just because the MIXED population is unhealthy. + expect(await isHoldOnly(env, "owner/repo:miner")).toBe(false); + }); + + it("ENGAGES the miner-scoped CLOSE breaker (closehold::miner) independently of the human-scoped close breaker", async () => { + const env = createTestEnv(); + for (let i = 0; i < 4; i += 1) await seedDecisionAndOutcomeScoped(env, "owner/repo", i, "close", "closed", true); + for (let i = 4; i < 12; i += 1) await seedDecisionAndOutcomeScoped(env, "owner/repo", i, "close", "merged", true); + // Dilute the mixed population with healthy non-miner close predictions, same ratio as the merge scenario. + for (let i = 100; i < 150; i += 1) await seedDecisionAndOutcomeScoped(env, "owner/repo", i, "close", "closed", false); + + await runSelfTuneBreaker(env); + + expect(await isCloseHoldOnly(env, "owner/repo:miner")).toBe(true); + expect(await isCloseHoldOnly(env, "owner/repo")).toBe(false); + }); + + it("clearing the human-scoped holdonly flag does NOT clear the miner-scoped one, and vice versa — they are genuinely distinct flags", async () => { + const env = createTestEnv(); + const flags = createFlagStore(env); + // Engage BOTH scopes directly, then backdate both past the 24h cooldown. + await flags.setFlag("holdonly:owner/repo", true); + await flags.setFlag("holdonly:owner/repo:miner", true); + await env.DB.prepare( + "UPDATE system_flags SET updated_at = datetime('now', '-2 days') WHERE key IN ('holdonly:owner/repo', 'holdonly:owner/repo:miner')", + ).run(); + expect(await isHoldOnly(env, "owner/repo")).toBe(true); + expect(await isHoldOnly(env, "owner/repo:miner")).toBe(true); + + // Miner-authored stays genuinely failing (4/12 = 33%). Enough healthy non-miner volume dilutes the MIXED + // population back above the floor ((4+50)/(12+50) = 87.1%) so the unscoped flag recovers, while the + // miner-only SUBSET (still 33%) does not. + for (let i = 0; i < 4; i += 1) await seedDecisionAndOutcomeScoped(env, "owner/repo", i, "merge", "merged", true); + for (let i = 4; i < 12; i += 1) await seedDecisionAndOutcomeScoped(env, "owner/repo", i, "merge", "closed", true); + for (let i = 100; i < 150; i += 1) await seedDecisionAndOutcomeScoped(env, "owner/repo", i, "merge", "merged", false); + + const log = vi.spyOn(console, "log").mockImplementation(() => {}); + await runSelfTuneBreaker(env); + log.mockRestore(); + + expect(await isHoldOnly(env, "owner/repo")).toBe(false); // human/mixed-scoped: recovered → auto-cleared + expect(await isHoldOnly(env, "owner/repo:miner")).toBe(true); // miner-scoped: still failing → stays engaged + }); + + it("the miner-scoped flag auto-clears independently once ITS cooldown elapses and ITS precision recovers, while a still-engaged (still genuinely failing) mixed-scoped flag is untouched", async () => { + const env = createTestEnv(); + const flags = createFlagStore(env); + await flags.setFlag("holdonly:owner/repo", true); + await flags.setFlag("holdonly:owner/repo:miner", true); + await env.DB.prepare( + "UPDATE system_flags SET updated_at = datetime('now', '-2 days') WHERE key IN ('holdonly:owner/repo', 'holdonly:owner/repo:miner')", + ).run(); + + // Miner-authored fully recovers (12/12 = 100%). Non-miner data for the SAME project stays genuinely bad + // (4/12 = 33%) — with no dilution on that side, the MIXED population is (12+4)/(12+12) = 66.7%, still below + // the floor, so the mixed/unscoped flag correctly stays engaged (this is a REAL still-failing population, + // not merely "no fresh sample" — a stronger claim than the existing #autoclear-deadlock "no signal" case). + for (let i = 0; i < 12; i += 1) await seedDecisionAndOutcomeScoped(env, "owner/repo", i, "merge", "merged", true); + for (let i = 100; i < 104; i += 1) await seedDecisionAndOutcomeScoped(env, "owner/repo", i, "merge", "merged", false); + for (let i = 104; i < 112; i += 1) await seedDecisionAndOutcomeScoped(env, "owner/repo", i, "merge", "closed", false); + + const log = vi.spyOn(console, "log").mockImplementation(() => {}); + await runSelfTuneBreaker(env); + log.mockRestore(); + + expect(await isHoldOnly(env, "owner/repo:miner")).toBe(false); + expect(await isHoldOnly(env, "owner/repo")).toBe(true); + }); + + it("does NOT engage the miner-scoped breaker with no miner-authored history at all (fail-safe / byte-identical)", async () => { + const env = createTestEnv(); + for (let i = 0; i < 12; i += 1) await seedDecisionAndOutcomeScoped(env, "owner/repo", i, "merge", "closed", false); + + await runSelfTuneBreaker(env); + + expect(await isHoldOnly(env, "owner/repo:miner")).toBe(false); + }); +}); + // ── integration: the PR-closed webhook records pr_outcome through processJob ──────────────────────────────────── describe("processJob(github-webhook) wires pr_outcome recording on a PR close", () => { diff --git a/test/unit/parity-wire.test.ts b/test/unit/parity-wire.test.ts index 2750dd44f1..24a890389d 100644 --- a/test/unit/parity-wire.test.ts +++ b/test/unit/parity-wire.test.ts @@ -118,6 +118,34 @@ describe("recordNativeGateDecision — flag-gated SHADOW recording into review_a expect(typeof rows[0]!.created_at).toBe("string"); }); + it("#2352: records miner_authored = 1 when minerAuthored is true", async () => { + const env = createTestEnv({ GITTENSORY_REVIEW_PARITY_AUDIT: "true" }); + await recordNativeGateDecision(env, { project: "owner/repo", pullNumber: 7, headSha: "abc123", conclusion: "success", minerAuthored: true }); + + const rows = await rawAll(env, "SELECT * FROM review_audit"); + expect(rows[0]).toMatchObject({ miner_authored: 1 }); + }); + + it("#2352: records miner_authored = 0 when minerAuthored is false or omitted (default, not a confirmed miner)", async () => { + const env = createTestEnv({ GITTENSORY_REVIEW_PARITY_AUDIT: "true" }); + await recordNativeGateDecision(env, { project: "owner/repo", pullNumber: 7, headSha: "abc123", conclusion: "success", minerAuthored: false }); + await recordNativeGateDecision(env, { project: "owner/repo", pullNumber: 8, headSha: "def456", conclusion: "success" }); + + const rows = await rawAll(env, "SELECT * FROM review_audit ORDER BY target_id"); + expect(rows[0]).toMatchObject({ target_id: "owner/repo#7", miner_authored: 0 }); + expect(rows[1]).toMatchObject({ target_id: "owner/repo#8", miner_authored: 0 }); + }); + + it("#2352: a re-run at the same commit can flip miner_authored (latest finalize wins, mirroring decision/summary)", async () => { + const env = createTestEnv({ GITTENSORY_REVIEW_PARITY_AUDIT: "true" }); + await recordNativeGateDecision(env, { project: "owner/repo", pullNumber: 7, headSha: "abc123", conclusion: "success", minerAuthored: false }); + await recordNativeGateDecision(env, { project: "owner/repo", pullNumber: 7, headSha: "abc123", conclusion: "success", minerAuthored: true }); + + const rows = await rawAll(env, "SELECT * FROM review_audit"); + expect(rows).toHaveLength(1); + expect(rows[0]).toMatchObject({ miner_authored: 1 }); + }); + it("a re-run at the SAME commit REPLACES the prior decision (latest finalize wins, no duplicate)", async () => { const env = createTestEnv({ GITTENSORY_REVIEW_PARITY_AUDIT: "true" }); await recordNativeGateDecision(env, { project: "owner/repo", pullNumber: 7, headSha: "abc123", conclusion: "success", reasonCode: "all_clear" }); @@ -405,8 +433,8 @@ function prWebhook(deliveryId: string, author: string) { }; } -async function nativeRows(env: Env): Promise> { - const res = await env.DB.prepare("SELECT decision, summary, source FROM review_audit WHERE source = ? AND event_type = 'gate_decision'").bind(GITTENSORY_NATIVE_SOURCE).all<{ decision: string; summary: string; source: string }>(); +async function nativeRows(env: Env): Promise> { + const res = await env.DB.prepare("SELECT decision, summary, source, miner_authored FROM review_audit WHERE source = ? AND event_type = 'gate_decision'").bind(GITTENSORY_NATIVE_SOURCE).all<{ decision: string; summary: string; source: string; miner_authored: number }>(); return res.results; } @@ -475,4 +503,33 @@ describe("recordNativeGateDecision wired into the review FINALIZE path (GITTENSO expect(rows.length).toBe(1); expect(rows[0]).toMatchObject({ decision: "hold", source: GITTENSORY_NATIVE_SOURCE, summary: "missing_linked_issue" }); }); + + it("#2352: a confirmed-miner author's gate decision is recorded with miner_authored = 1", async () => { + const env = createTestEnv({ GITTENSORY_REVIEW_PARITY_AUDIT: "true", GITHUB_APP_PRIVATE_KEY: await generatePrivateKeyPem() }); + await seedGateEnabledRepo(env); + await upsertOfficialMinerDetection(env, "contributor", { status: "confirmed", snapshot: parityMinerSnapshot("contributor") }, 60_000); + stubFinalizeFetch("contributor"); + try { + await processJob(env, prWebhook("parity-finalize-miner", "contributor")); + } finally { + vi.unstubAllGlobals(); + } + const rows = await nativeRows(env); + expect(rows.length).toBe(1); + expect(rows[0]).toMatchObject({ miner_authored: 1 }); + }); + + it("#2352: a non-confirmed author's gate decision is recorded with miner_authored = 0", async () => { + const env = createTestEnv({ GITTENSORY_REVIEW_PARITY_AUDIT: "true", GITHUB_APP_PRIVATE_KEY: await generatePrivateKeyPem() }); + await seedGateEnabledRepo(env); + stubFinalizeFetch(null); // miner list empty → author unconfirmed + try { + await processJob(env, prWebhook("parity-finalize-nonminer", "contributor")); + } finally { + vi.unstubAllGlobals(); + } + const rows = await nativeRows(env); + expect(rows.length).toBe(1); + expect(rows[0]).toMatchObject({ miner_authored: 0 }); + }); }); diff --git a/test/unit/parity.test.ts b/test/unit/parity.test.ts index e523265c74..8249dbbaa9 100644 --- a/test/unit/parity.test.ts +++ b/test/unit/parity.test.ts @@ -322,6 +322,54 @@ describe("computeGateEval — source scoping for per-system standalone accuracy expect(bound).toHaveLength(1); // only fromIso }); + it("#2352: adds the miner_authored filter when minerOnly is set, alongside an unrelated source filter", async () => { + let boundSql = ""; + let bound: unknown[] = []; + const env = { + DB: { + prepare: (sql: string) => { + boundSql = sql; + return { bind: (...a: unknown[]) => { bound = a; return { all: async () => ({ results: [] }) }; } }; + }, + }, + } as unknown as Env; + await computeGateEval(env, { days: 90, nowMs: NOW, source: "gittensory-native", minerOnly: true }); + expect(boundSql).toContain("AND miner_authored = 1"); + expect(boundSql).toContain("AND source = ?"); + // miner_authored = 1 is a literal in the SQL, not a bound param — only fromIso + source are bound. + expect(bound).toHaveLength(2); + expect(bound).toContain("gittensory-native"); + }); + + it("#2352: omits the miner_authored filter (scores ALL authorship) when minerOnly is not set — behavior-preserving", async () => { + let boundSql = ""; + const env = { + DB: { + prepare: (sql: string) => { + boundSql = sql; + return { bind: () => ({ all: async () => ({ results: [] }) }) }; + }, + }, + } as unknown as Env; + await computeGateEval(env, { days: 90, nowMs: NOW }); + expect(boundSql).not.toContain("miner_authored"); + }); + + it("#2352: minerOnly works independently of source (no source given, minerOnly set)", async () => { + let boundSql = ""; + const env = { + DB: { + prepare: (sql: string) => { + boundSql = sql; + return { bind: () => ({ all: async () => ({ results: [] }) }) }; + }, + }, + } as unknown as Env; + await computeGateEval(env, { days: 90, nowMs: NOW, minerOnly: true }); + expect(boundSql).toContain("AND miner_authored = 1"); + expect(boundSql).not.toContain("AND source = ?"); + }); + it("folds the prediction-vs-outcome confusion matrix into per-project precisions", async () => { // A stub D1 returning the gd⨝po cells directly (the self-join is exercised against real SQL in prod; // here we drive the FOLD): merge-correct/merge-false/close-correct/close-false/hold buckets.