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
46 changes: 46 additions & 0 deletions migrations/0126_contributor_gate_history.sql
Original file line number Diff line number Diff line change
@@ -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);
1 change: 1 addition & 0 deletions scripts/check-schema-drift.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
33 changes: 32 additions & 1 deletion src/queue/processors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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");
Expand Down Expand Up @@ -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
Expand Down
68 changes: 68 additions & 0 deletions src/review/contributor-calibration.ts
Original file line number Diff line number Diff line change
@@ -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<Env["SELFHOST_TRANSIENT_CACHE"]>;
};

/**
* 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<void> {
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) }));
}
}
108 changes: 108 additions & 0 deletions test/unit/contributor-calibration.test.ts
Original file line number Diff line number Diff line change
@@ -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<Record<string, unknown>[]> {
const res = await (env.DB as unknown as { prepare: (s: string) => { bind: (...v: unknown[]) => { all: <T>() => Promise<{ results: T[] }> } } })
.prepare(sql)
.bind(...binds)
.all<Record<string, unknown>>();
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();
});
});
Loading