From 28f7eec38274042efd819e6098efca7eb756d821 Mon Sep 17 00:00:00 2001 From: JSONbored <49853598+JSONbored@users.noreply.github.com> Date: Wed, 8 Jul 2026 01:02:27 -0700 Subject: [PATCH] feat(review): add per-contributor gate-decision history table Adds contributor_gate_history (migration 0126), populated from the same call sites as recordNativeGateDecision, as the data substrate a future personalized gate-prediction confidence adjustment would read. Keyed by login rather than an HMAC hash (unlike review_audit's cross-instance export path, this table never leaves the instance and is never rendered publicly) and never wired into the fleet telemetry export. Write-only in this PR -- no confidence-adjustment logic reads it yet. That consumption, and the safety-critical invariant that a personalization adjustment must never bypass a hard blocker, are deliberate follow-up work. --- migrations/0126_contributor_gate_history.sql | 46 ++++++++ scripts/check-schema-drift.mjs | 1 + src/queue/processors.ts | 33 +++++- src/review/contributor-calibration.ts | 68 ++++++++++++ test/unit/contributor-calibration.test.ts | 108 +++++++++++++++++++ 5 files changed, 255 insertions(+), 1 deletion(-) create mode 100644 migrations/0126_contributor_gate_history.sql create mode 100644 src/review/contributor-calibration.ts create mode 100644 test/unit/contributor-calibration.test.ts diff --git a/migrations/0126_contributor_gate_history.sql b/migrations/0126_contributor_gate_history.sql new file mode 100644 index 0000000000..b2ff9b8577 --- /dev/null +++ b/migrations/0126_contributor_gate_history.sql @@ -0,0 +1,46 @@ +-- #personalized-calibration-ledger (PR 1 of #2349): the per-contributor gate-decision data substrate. +-- +-- #2349 wants a personalized gate-prediction confidence adjustment per contributor/miner history. That needs +-- a per-actor accuracy query, but `review_audit` (migrations/0049) is DELIBERATELY actor-login-free for +-- privacy — its own migration comment states "No actor logins, no PR content, no trust/reward internals." That +-- constraint exists because `review_audit` feeds `exportOrbBatch` (src/selfhost/orb-collector.ts), an +-- anonymized cross-instance export pipeline; adding a login column there would leak actor identity into that +-- export path. This table is a SEPARATE, LOCAL-ONLY substrate — structurally a sibling of `review_audit` (one +-- row per finalized gate decision, written from the exact same call sites as `recordNativeGateDecision` in +-- src/review/parity-wire.ts), but keyed by login, and it must NEVER be wired into `exportOrbBatch` or any +-- other cross-instance/public export path. +-- +-- Login (not an HMAC hash) is used deliberately: unlike `review_audit`'s cross-instance export concern, this +-- table never leaves the instance and is never rendered on any public surface (see the design-note comment at +-- the top of src/review/contributor-calibration.ts) — the same precedent `contributor_evidence` and +-- `contributor_scoring_profiles` (migrations/0004) already establish for per-login local-only data. Hashing +-- would only add a lookup-key translation step with no privacy benefit for this specific access pattern. +-- +-- THIS PR ONLY POPULATES THE TABLE. Nothing reads it yet — the confidence-adjustment function that would +-- consume it (in packages/gittensory-engine/src/predicted-gate.ts) is explicit follow-up work, deliberately +-- deferred so the safety-critical "a personalization adjustment must never bypass a hard blocker" invariant +-- gets its own focused review. +CREATE TABLE IF NOT EXISTS contributor_gate_history ( + id TEXT PRIMARY KEY NOT NULL, + -- The GitHub login this decision's PR was authored by. Public information (tied to a public PR) — not a + -- secret the way a trust score or reward value is; the privacy concern this table's design addresses is + -- never rendering an AGGREGATE per-login accuracy figure publicly, not the raw login itself. + login TEXT NOT NULL, + -- Mirrors review_audit.source: which writer made this decision. Always 'gittensory-native' today (the only + -- writer of this table), carried as its own column for the same self-join-ready shape as review_audit + -- rather than hardcoding an assumption that never changes into every reader. + source TEXT NOT NULL DEFAULT 'gittensory-native', + -- Which repo the decision is for. + project TEXT NOT NULL, + -- The reviewed target, `repo#pr`. + target_id TEXT NOT NULL, + -- The gate action: 'merge' | 'close' | 'hold' — mirrors review_audit.decision. + decision TEXT NOT NULL, + -- The commit the decision was made on. + head_sha TEXT, + created_at TEXT NOT NULL DEFAULT CURRENT_TIMESTAMP +); + +-- The future per-actor calibration read is "this login's decisions in a recent window" — index the hot path. +CREATE INDEX IF NOT EXISTS contributor_gate_history_login_idx + ON contributor_gate_history(login, source, created_at); diff --git a/scripts/check-schema-drift.mjs b/scripts/check-schema-drift.mjs index e686f6d596..7798a139a2 100755 --- a/scripts/check-schema-drift.mjs +++ b/scripts/check-schema-drift.mjs @@ -37,6 +37,7 @@ const MIGRATIONS_DIR = process.env.CHECK_SCHEMA_DRIFT_DIR || "migrations"; // table here without also confirming it is genuinely raw-SQL-only is a reviewer-visible diff, not a silent // gap this check would otherwise catch. export const RAW_SQL_ONLY_TABLES = new Set([ + "contributor_gate_history", "global_agent_controls", "global_contributor_blacklist", "global_moderation_config", diff --git a/src/queue/processors.ts b/src/queue/processors.ts index b25375a842..f6df1fe34e 100644 --- a/src/queue/processors.ts +++ b/src/queue/processors.ts @@ -542,7 +542,8 @@ import { recordReversalSignals, runSelfTuneBreaker, } from "../review/outcomes-wire"; -import { neutralHoldReasonCode, recordNativeGateDecision } from "../review/parity-wire"; +import { neutralHoldReasonCode, nativeGateActionFromConclusion, recordNativeGateDecision } from "../review/parity-wire"; +import { recordContributorGateDecision } from "../review/contributor-calibration"; import { getSubmitterReputation, type SubmissionOutcome } from "../review/submitter-reputation"; import type { AdvisoryFinding, @@ -3079,6 +3080,16 @@ async function runAgentMaintenancePlanAndExecute( action: disposition.actionClass, reasonCode: disposition.blockerClass === "none" ? gate.conclusion : disposition.blockerClass, }); + // #2349 (PR 1): additive per-contributor calibration data, gated identically to recordNativeGateDecision + // above -- see src/review/contributor-calibration.ts's doc comment. Currently write-only; nothing reads + // contributor_gate_history yet. + await recordContributorGateDecision(env, { + login: pr.authorLogin, + project: repoFullName, + pullNumber: pr.number, + headSha: pr.headSha, + decision: disposition.actionClass, + }); if (disposition.actionClass === "hold") { const gateBlockerCodes = gate.blockers.map((blocker) => blocker.code); const mergeAutonomy = resolveAutonomy(settings.autonomy, "merge"); @@ -9923,6 +9934,26 @@ async function maybePublishPrPublicSurface( conclusion: gateEvaluation.conclusion, reasonCode, }); + // #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 -- + // see src/review/contributor-calibration.ts's doc comment. Currently write-only. + const contributorDecision = nativeGateActionFromConclusion(gateEvaluation.conclusion); + // unreachable implicit-else at THIS call site: gateEvaluation is only "skipped" (the one conclusion + // nativeGateActionFromConclusion maps to null) when shouldEvaluateGate is false, which leaves + // gateEvaluation itself undefined and never reaches this branch (see the outer `if (gateEvaluation)` + // above) -- neither evaluateGateCheck/evaluateGateCheckCore nor evaluateWithSurfaceLane ever construct + // a "skipped" conclusion object. Kept as a real (not asserted-away) null check for robustness against a + // future caller that does produce one, mirroring recordNativeGateDecision's own defensive null-check. + /* v8 ignore else */ + if (contributorDecision !== null) { + await recordContributorGateDecision(env, { + login: pr.authorLogin, + project: repoFullName, + pullNumber: pr.number, + headSha: pr.headSha, + decision: contributorDecision, + }); + } } // Review-evasion protection (#review-evasion-protection): the cost-bearing review pass for this head has // now concluded (the gate decision is made) -- terminalize the active-review row so a close/draft-convert diff --git a/src/review/contributor-calibration.ts b/src/review/contributor-calibration.ts new file mode 100644 index 0000000000..8d80912d48 --- /dev/null +++ b/src/review/contributor-calibration.ts @@ -0,0 +1,68 @@ +// Per-contributor gate-decision history (#2349, PR 1 of a multi-PR epic) -- the data substrate a future +// personalized gate-prediction confidence adjustment would read. `review_audit` (migrations/0049) is +// DELIBERATELY actor-login-free for privacy (it feeds the anonymized cross-instance orb-collector export +// path); this is a SEPARATE, LOCAL-ONLY table populated from the exact same call sites as +// recordNativeGateDecision (src/review/parity-wire.ts), structurally a sibling of review_audit but keyed by +// login. See migrations/0126_contributor_gate_history.sql for the full design rationale. +// +// DESIGN NOTE -- READ BEFORE ADDING A CONSUMER: this table (and anything derived from it) must NEVER be +// rendered on any public surface -- a PR comment, a check-run body, an MCP tool response, a public dashboard, +// or any other contributor-facing output. src/signals/redaction.ts exists specifically to prevent aggregate +// per-actor accuracy/trust signals from leaking publicly; an eventual confidence-adjustment reader of this +// table must only ever feed the INTERNAL predicted-gate verdict computation, and only strictly downstream of +// that verdict's blocker determination (a personalization adjustment must never be able to flip a hard +// blocker off -- it may only narrow/widen an advisory confidence band). That consumer does not exist yet; +// this PR only writes the data. +// +// THIS TABLE IS NEVER EXPORTED. It must not be wired into exportOrbBatch (src/selfhost/orb-collector.ts) or +// any other cross-instance/fleet telemetry path -- that is the exact leak review_audit's own "no actor +// logins" design deliberately avoids, and this table exists precisely so review_audit doesn't have to. + +import type { GateAction } from "./parity"; +import { isParityAuditEnabled } from "./parity-wire"; +import { isSelfHostedReviewRuntime } from "../selfhost/review-runtime"; +import { errorMessage, nowIso } from "../utils/json"; + +/** The minimal env shape the recorder needs -- mirrors parity-wire.ts's own ParityRecorderEnv exactly, since + * this records under the identical self-hosted/parity-flag gate (see recordContributorGateDecision's doc + * comment for why: this is additive telemetry alongside recordNativeGateDecision, not a separate feature + * with its own on/off knob). */ +type ContributorCalibrationEnv = { + DB: D1Database; + GITTENSORY_REVIEW_PARITY_AUDIT?: string | undefined; + SELFHOST_TRANSIENT_CACHE?: NonNullable; +}; + +/** + * Record one gittensory-native gate decision into `contributor_gate_history`, keyed by the PR author's login. + * + * Gated identically to {@link recordNativeGateDecision} in parity-wire.ts (same self-hosted-always-records / + * cloud-flag-gated contract) so this is always safe to call alongside it without a separate on/off knob to + * keep in sync. Best-effort: a write failure is swallowed (telemetry must never break gate finalization). A + * missing/empty login records nothing -- there is no meaningful per-actor row to write without one (a deleted + * account, a bot author with no resolvable login, etc.). + */ +export async function recordContributorGateDecision( + env: ContributorCalibrationEnv, + input: { login: string | null | undefined; project: string; pullNumber: number; headSha: string | null | undefined; decision: GateAction }, +): Promise { + if (!isSelfHostedReviewRuntime(env) && !isParityAuditEnabled(env)) return; + const login = input.login?.trim(); + if (!login) return; + const project = input.project.slice(0, 200); + const targetId = `${project}#${input.pullNumber}`; + const source = "gittensory-native"; + try { + // Deterministic id per (login, source, project, pr, sha) -- mirrors recordNativeGateDecision's own + // per-commit dedup: a re-run at the SAME commit replaces its prior row, a new commit gets its own. + await env.DB.prepare( + `INSERT INTO contributor_gate_history (id, login, source, project, target_id, decision, head_sha, created_at) + VALUES (?, ?, ?, ?, ?, ?, ?, ?) + ON CONFLICT(id) DO UPDATE SET decision = excluded.decision, created_at = excluded.created_at`, + ) + .bind(`contrib:${login}:${source}:${targetId}@${input.headSha ?? "none"}`, login, source, project, targetId, input.decision, input.headSha ?? null, nowIso()) + .run(); + } catch (error) { + console.warn(JSON.stringify({ event: "contributor_gate_history_record_error", project, pr: input.pullNumber, message: errorMessage(error).slice(0, 200) })); + } +} diff --git a/test/unit/contributor-calibration.test.ts b/test/unit/contributor-calibration.test.ts new file mode 100644 index 0000000000..dc5af2bfd6 --- /dev/null +++ b/test/unit/contributor-calibration.test.ts @@ -0,0 +1,108 @@ +import { describe, expect, it, vi } from "vitest"; +import { recordContributorGateDecision } from "../../src/review/contributor-calibration"; +import { createTestEnv } from "../helpers/d1"; + +// ── Direct D1 helpers over the real migrated schema (0126 contributor_gate_history) ───────────────────────── + +async function rawAll(env: Env, sql: string, ...binds: unknown[]): Promise[]> { + const res = await (env.DB as unknown as { prepare: (s: string) => { bind: (...v: unknown[]) => { all: () => Promise<{ results: T[] }> } } }) + .prepare(sql) + .bind(...binds) + .all>(); + return res.results; +} + +describe("recordContributorGateDecision — write-only per-contributor gate history (0126 round-trip, #2349 PR 1)", () => { + it("SELF-HOSTED instances record ONE row keyed by login (createTestEnv's default self-host signal)", async () => { + const env = createTestEnv(); // flag unset → OFF, but SELFHOST_TRANSIENT_CACHE present → self-hosted + await recordContributorGateDecision(env, { login: "octocat", project: "owner/repo", pullNumber: 7, headSha: "abc123", decision: "merge" }); + + const rows = await rawAll(env, "SELECT * FROM contributor_gate_history"); + expect(rows.length).toBe(1); + expect(rows[0]).toMatchObject({ + login: "octocat", + source: "gittensory-native", + project: "owner/repo", + target_id: "owner/repo#7", + decision: "merge", + head_sha: "abc123", + }); + expect(typeof rows[0]!.created_at).toBe("string"); + }); + + it("a re-run at the SAME (login, source, project, pr, sha) REPLACES the prior row (no duplicate)", async () => { + const env = createTestEnv(); + await recordContributorGateDecision(env, { login: "octocat", project: "owner/repo", pullNumber: 7, headSha: "abc123", decision: "merge" }); + await recordContributorGateDecision(env, { login: "octocat", project: "owner/repo", pullNumber: 7, headSha: "abc123", decision: "hold" }); + + const rows = await rawAll(env, "SELECT * FROM contributor_gate_history"); + expect(rows.length).toBe(1); + expect(rows[0]).toMatchObject({ decision: "hold" }); + }); + + it("a new commit gets its OWN row", async () => { + const env = createTestEnv(); + await recordContributorGateDecision(env, { login: "octocat", project: "owner/repo", pullNumber: 7, headSha: "sha1", decision: "merge" }); + await recordContributorGateDecision(env, { login: "octocat", project: "owner/repo", pullNumber: 7, headSha: "sha2", decision: "close" }); + expect((await rawAll(env, "SELECT * FROM contributor_gate_history")).length).toBe(2); + }); + + it("a different login on the SAME PR/commit gets its own row (keyed by login too)", async () => { + const env = createTestEnv(); + await recordContributorGateDecision(env, { login: "octocat", project: "owner/repo", pullNumber: 7, headSha: "abc123", decision: "merge" }); + await recordContributorGateDecision(env, { login: "hubot", project: "owner/repo", pullNumber: 7, headSha: "abc123", decision: "merge" }); + expect((await rawAll(env, "SELECT * FROM contributor_gate_history")).length).toBe(2); + }); + + it("does NOT record when the login is missing, null, or blank (no meaningful per-actor row to write)", async () => { + const env = createTestEnv(); + await recordContributorGateDecision(env, { login: undefined, project: "owner/repo", pullNumber: 1, headSha: "sha", decision: "merge" }); + await recordContributorGateDecision(env, { login: null, project: "owner/repo", pullNumber: 2, headSha: "sha", decision: "merge" }); + await recordContributorGateDecision(env, { login: " ", project: "owner/repo", pullNumber: 3, headSha: "sha", decision: "merge" }); + expect((await rawAll(env, "SELECT * FROM contributor_gate_history")).length).toBe(0); + }); + + it("records even with a null head_sha (unlike recordNativeGateDecision, this has no parity self-join to protect)", async () => { + const env = createTestEnv(); + await recordContributorGateDecision(env, { login: "octocat", project: "owner/repo", pullNumber: 4, headSha: null, decision: "close" }); + const rows = await rawAll(env, "SELECT * FROM contributor_gate_history"); + expect(rows).toHaveLength(1); + expect(rows[0]).toMatchObject({ head_sha: null, decision: "close" }); + }); + + it("flag-OFF records NOTHING on the CLOUD WORKER — no D1 write (byte-identical, same gate as recordNativeGateDecision)", async () => { + const env = createTestEnv(); + delete env.SELFHOST_TRANSIENT_CACHE; // simulate the cloud worker (no self-host binding) + await recordContributorGateDecision(env, { login: "octocat", project: "owner/repo", pullNumber: 7, headSha: "abc123", decision: "merge" }); + expect((await rawAll(env, "SELECT * FROM contributor_gate_history")).length).toBe(0); + + const envFalse = createTestEnv({ GITTENSORY_REVIEW_PARITY_AUDIT: "false" }); + delete envFalse.SELFHOST_TRANSIENT_CACHE; + await recordContributorGateDecision(envFalse, { login: "octocat", project: "owner/repo", pullNumber: 7, headSha: "abc123", decision: "close" }); + expect((await rawAll(envFalse, "SELECT * FROM contributor_gate_history")).length).toBe(0); + }); + + it("the cloud worker records when GITTENSORY_REVIEW_PARITY_AUDIT is explicitly ON", async () => { + const env = createTestEnv({ GITTENSORY_REVIEW_PARITY_AUDIT: "true" }); + delete env.SELFHOST_TRANSIENT_CACHE; + await recordContributorGateDecision(env, { login: "octocat", project: "owner/repo", pullNumber: 7, headSha: "abc123", decision: "merge" }); + expect((await rawAll(env, "SELECT * FROM contributor_gate_history")).length).toBe(1); + }); + + it("fails safe: a D1 write error is swallowed + logged (telemetry never breaks finalization)", async () => { + const env = createTestEnv(); + const realPrepare = env.DB.prepare.bind(env.DB); + env.DB.prepare = ((sql: string) => { + if (/contributor_gate_history/i.test(sql)) throw new Error("poisoned write"); + return realPrepare(sql); + }) as typeof env.DB.prepare; + const warn = vi.spyOn(console, "warn").mockImplementation(() => {}); + + await expect( + recordContributorGateDecision(env, { login: "octocat", project: "owner/repo", pullNumber: 7, headSha: "abc123", decision: "merge" }), + ).resolves.toBeUndefined(); + + expect(warn.mock.calls.map((c) => String(c[0])).some((line) => line.includes("contributor_gate_history_record_error"))).toBe(true); + warn.mockRestore(); + }); +});