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
5 changes: 5 additions & 0 deletions migrations/0134_review_targets_cadence_idx.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
-- Supports the submission-cadence signal (#4514): getSubmitterCadence queries ALL review_targets rows (not
-- just terminal ones -- a fresh burst of still-open submissions is exactly what this needs to catch) filtered
-- by (project, submitter, created_at). This shape isn't covered by the existing review_targets indexes
-- (migrations/0050), which are keyed on status/verdict/terminal_at, not created_at alongside submitter.
CREATE INDEX IF NOT EXISTS idx_review_targets_project_submitter_created ON review_targets (project, submitter, created_at);
12 changes: 11 additions & 1 deletion src/review/reputation-wire.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,9 @@
// the ported module degrades to "neutral" / no-op on any DB error, so this never throws into the gate.

import {
getSubmitterCadence,
getSubmitterReputation,
isMachinePacedCadence,
recordSubmissionOutcome,
type SubmissionOutcome,
type SubmitterStats,
Expand Down Expand Up @@ -58,14 +60,22 @@ export function shouldDowngradeToDeterministic(stats: SubmitterStats): boolean {
* downgrade to a deterministic-only review. When the flag is OFF this returns false IMMEDIATELY — no DB read —
* so the AI-spend gate is byte-identical to today. `project` namespaces the per-(project, submitter) rows
* (gittensory uses the repo full name). NEVER throws: the ported module already degrades to neutral on error.
*
* Also checks submission CADENCE (#4514): every quality-based signal above only tells you whether a
* submitter's outcomes were good or bad, never how FAST they arrived -- a fast, well-formed, strategically
* low-value submitter clears every quality bar while still being invisible to those signals. A cadence read
* this tight, sustained across this many consecutive submissions, is not a pattern any human contributor
* plausibly sustains, independent of whether the submissions themselves look fine.
*/
export async function shouldSkipAiForReputation(
env: Env,
args: { project: string; submitter: string | null | undefined },
): Promise<boolean> {
if (!isReputationEnabled(env)) return false;
const stats = await getSubmitterReputation(env, args.project, args.submitter ?? undefined);
return shouldDowngradeToDeterministic(stats);
if (shouldDowngradeToDeterministic(stats)) return true;
const cadence = await getSubmitterCadence(env, args.project, args.submitter ?? undefined);
return isMachinePacedCadence(cadence);
}

/**
Expand Down
52 changes: 52 additions & 0 deletions src/review/submitter-reputation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,58 @@ export const REPUTATION_WINDOW_DAYS = 90;
// Hard ceiling on rows pulled for one submitter's window so a pathological history can't blow the query up.
const REPUTATION_WINDOW_ROW_CAP = 500;

// ── Submission-cadence signal (#4514). Every signal above is QUALITY-based (was the outcome good or bad) --
// none of them have a TIMING dimension, so a fast, well-formed, strategically-low-value submitter is
// invisible to the one dimension (superhuman pace) that would otherwise be a strong tell. This is queried
// from ALL review_targets rows (not just terminal ones, unlike the quality signal above) -- a fresh burst of
// still-open submissions is exactly the case this needs to catch, and by the time they become terminal the
// (paid) AI review has already run on each one. ──
const CADENCE_WINDOW_HOURS = 24;
// Need at least this many recent submissions before judging pace at all -- a lone fast submission (a real
// contributor who happened to open two PRs close together) is not a pattern.
const CADENCE_MIN_SAMPLE = 5;
// A human contributor, even a fast one, does not sustain a sub-10-minute median gap between distinct PR
// submissions across many consecutive attempts -- reading, writing, and testing each change takes real time.
const CADENCE_MAX_MEDIAN_GAP_MS = 10 * 60 * 1000;

export type SubmissionCadence = { count: number; medianGapMs: number | null };

/** Pure: the median gap (ms) between consecutive submissions, given their created_at timestamps in any order.
* `medianGapMs` is `null` when there are fewer than 2 samples (no gap to measure). */
export function computeSubmissionCadence(createdAtIsoTimestamps: readonly string[]): SubmissionCadence {
const sorted = [...createdAtIsoTimestamps].map((t) => new Date(t).getTime()).sort((a, b) => a - b);
if (sorted.length < 2) return { count: sorted.length, medianGapMs: null };
const gaps: number[] = [];
for (let i = 1; i < sorted.length; i++) gaps.push(sorted[i]! - sorted[i - 1]!);
gaps.sort((a, b) => a - b);
const mid = Math.floor(gaps.length / 2);
const medianGapMs = gaps.length % 2 === 0 ? (gaps[mid - 1]! + gaps[mid]!) / 2 : gaps[mid]!;
return { count: sorted.length, medianGapMs };
}

/** Pure: does this cadence read as machine-paced? Needs both a real sample size AND a gap tighter than any
* human contributor plausibly sustains across that many consecutive attempts. */
export function isMachinePacedCadence(cadence: SubmissionCadence): boolean {
return cadence.count >= CADENCE_MIN_SAMPLE && cadence.medianGapMs !== null && cadence.medianGapMs < CADENCE_MAX_MEDIAN_GAP_MS;
}

/** Per-repo submission cadence for one submitter over the last {@link CADENCE_WINDOW_HOURS}. Fail-safe:
* any read error degrades to `{ count: 0, medianGapMs: null }` (never machine-paced), identical in spirit to
* {@link getSubmitterReputation}'s fail-safe-to-neutral. */
export async function getSubmitterCadence(env: Env, project: string, submitter: string | undefined): Promise<SubmissionCadence> {
if (!submitter) return { count: 0, medianGapMs: null };
try {
const result = await storage(env)
.prepare(`SELECT created_at AS createdAt FROM review_targets WHERE project = ? AND submitter = ? AND created_at >= datetime('now', ?) ORDER BY created_at DESC LIMIT ?`)
.bind(project, submitter, `-${CADENCE_WINDOW_HOURS} hours`, REPUTATION_WINDOW_ROW_CAP)
.all<{ createdAt: string }>();
const createdAts = (result?.results ?? []).map((r) => r.createdAt);
return computeSubmissionCadence(createdAts);
} catch {
return { count: 0, medianGapMs: null };
}
}

// ── reasonCode → quality bucket (#reputation-redesign). Buckets reflect the LIVE D1 reasonCode taxonomy. ──
// SUCCESS: a genuine reviewer/merge approval.
// QUALITY_FAIL: a genuine RECENT reviewer reject (real quality signal).
Expand Down
50 changes: 50 additions & 0 deletions test/unit/reputation-wiring.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,56 @@ describe("shouldSkipAiForReputation (helper)", () => {
expect(await shouldSkipAiForReputation(env, { project: "acme/widgets", submitter: "burster" })).toBe(true);
expect(await shouldSkipAiForReputation(env, { project: "acme/widgets", submitter: "newcomer" })).toBe(false);
});

it("FLAG-ON: false for a null submitter (the ?? undefined coalesce on both the quality and cadence reads)", async () => {
const env = createTestEnv({ GITTENSORY_REVIEW_REPUTATION: "true" });
expect(await shouldSkipAiForReputation(env, { project: "acme/widgets", submitter: null })).toBe(false);
});

describe("submission-cadence signal (#4514)", () => {
async function seedReviewTarget(env: Env, args: { number: number; submitter: string; createdAt: string }) {
await env.DB.prepare(
`INSERT INTO review_targets (id, project, kind, repo, number, submitter, status, decision_json, terminal_at, created_at)
VALUES (?, 'acme/widgets', 'pull_request', 'acme/widgets', ?, ?, 'merged', ?, ?, ?)`,
)
.bind(`acme/widgets:pull_request:acme/widgets#${args.number}`, args.number, args.submitter, JSON.stringify({ reasonCode: "dual_review_approved" }), args.createdAt, args.createdAt)
.run();
}

it("FLAG-ON: true for a machine-paced submitter even though every submission itself looks fine (quality-neutral)", async () => {
const env = createTestEnv({ GITTENSORY_REVIEW_REPUTATION: "true" });
// Anchored to now (minus a couple hours of headroom) -- the cadence query only looks back 24h, so a
// fixed past date would fall outside the window and vacuously read as "0 samples, not machine-paced".
const t0 = Date.now() - 2 * 60 * 60_000;
for (let i = 0; i < 5; i++) {
// All merged/approved -- the QUALITY signal alone stays neutral/trusted; only cadence should trip this.
await seedReviewTarget(env, { number: i, submitter: "speedster", createdAt: new Date(t0 + i * 5 * 60_000).toISOString() });
}
expect(await shouldSkipAiForReputation(env, { project: "acme/widgets", submitter: "speedster" })).toBe(true);
});

it("FLAG-ON: false for the same number of submissions spread naturally over hours (comfortably human pace)", async () => {
const env = createTestEnv({ GITTENSORY_REVIEW_REPUTATION: "true" });
const t0 = Date.now() - 20 * 60 * 60_000;
for (let i = 0; i < 5; i++) {
await seedReviewTarget(env, { number: i + 100, submitter: "steady", createdAt: new Date(t0 + i * 3 * 60 * 60_000).toISOString() });
}
expect(await shouldSkipAiForReputation(env, { project: "acme/widgets", submitter: "steady" })).toBe(false);
});

it("FLAG-ON: skips the (extra) cadence read once the quality/burst signal already justifies downgrading", async () => {
const env = createTestEnv({ GITTENSORY_REVIEW_REPUTATION: "true" });
await seedSubmitter(env, { project: "acme/widgets", submitter: "burster", submissions: 12, merged: 0, closed: 12, manual: 0 });
const spy = vi.spyOn(env.DB, "prepare");
const before = spy.mock.calls.length;
expect(await shouldSkipAiForReputation(env, { project: "acme/widgets", submitter: "burster" })).toBe(true);
// Exactly the calls the quality/burst read itself makes (submitter_stats + review_targets window) --
// no additional prepare() for a cadence query once the burst check alone already returned true.
const afterQualityOnlyCallCount = spy.mock.calls.length - before;
spy.mockRestore();
expect(afterQualityOnlyCallCount).toBe(2);
});
});
});

describe("processGitHubWebhook records the reputation outcome on a terminal PR (flag-ON call site)", () => {
Expand Down
101 changes: 101 additions & 0 deletions test/unit/submitter-reputation.test.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,12 @@
import { describe, expect, it } from "vitest";
import {
classifyOutcome,
computeSubmissionCadence,
countOutcomes,
DEFAULT_REPUTATION_CONFIG,
getSubmitterCadence,
getSubmitterReputation,
isMachinePacedCadence,
recordSubmissionOutcome,
REPUTATION_WINDOW_DAYS,
type ReputationConfig,
Expand Down Expand Up @@ -273,6 +276,104 @@ describe("recordSubmissionOutcome / getSubmitterReputation (D1, fail-safe)", ()
});
});

describe("computeSubmissionCadence (pure) (#4514)", () => {
it("returns count and null medianGapMs for 0 or 1 samples (nothing to measure a gap between)", () => {
expect(computeSubmissionCadence([])).toEqual({ count: 0, medianGapMs: null });
expect(computeSubmissionCadence(["2026-01-01T00:00:00.000Z"])).toEqual({ count: 1, medianGapMs: null });
});

it("computes the median gap between consecutive submissions, order-independent", () => {
// Gaps: 10min, 20min, 30min -> sorted [10,20,30] -> median 20min.
const t0 = new Date("2026-01-01T00:00:00.000Z").getTime();
const timestamps = [t0, t0 + 10 * 60_000, t0 + 30 * 60_000, t0 + 60 * 60_000].map((ms) => new Date(ms).toISOString());
// Shuffle the input order -- the function must sort internally, not assume caller ordering.
const shuffled = [timestamps[2]!, timestamps[0]!, timestamps[3]!, timestamps[1]!];
expect(computeSubmissionCadence(shuffled)).toEqual({ count: 4, medianGapMs: 20 * 60_000 });
});

it("averages the two middle gaps for an even number of gaps", () => {
// 3 timestamps -> 2 gaps: 10min, 30min -> even count -> average = 20min.
const t0 = new Date("2026-01-01T00:00:00.000Z").getTime();
const timestamps = [t0, t0 + 10 * 60_000, t0 + 40 * 60_000].map((ms) => new Date(ms).toISOString());
expect(computeSubmissionCadence(timestamps)).toEqual({ count: 3, medianGapMs: 20 * 60_000 });
});
});

describe("isMachinePacedCadence (pure) (#4514)", () => {
it("requires BOTH the minimum sample size AND a sub-threshold median gap", () => {
// Below minSample (5) -- fast, but not enough samples to call it a pattern.
expect(isMachinePacedCadence({ count: 4, medianGapMs: 60_000 })).toBe(false);
// Enough samples, but the gap is comfortably human (well over 10min).
expect(isMachinePacedCadence({ count: 10, medianGapMs: 60 * 60_000 })).toBe(false);
// No gap at all to measure (count < 2 internally, or explicitly null).
expect(isMachinePacedCadence({ count: 8, medianGapMs: null })).toBe(false);
// Enough samples AND a tight gap -- machine-paced.
expect(isMachinePacedCadence({ count: 5, medianGapMs: 5 * 60_000 })).toBe(true);
expect(isMachinePacedCadence({ count: 20, medianGapMs: 60_000 })).toBe(true);
});

it("is a boundary at exactly the configured thresholds", () => {
// Exactly at minSample, exactly under the max gap -- still counts.
expect(isMachinePacedCadence({ count: 5, medianGapMs: 10 * 60_000 - 1 })).toBe(true);
// Exactly AT the max gap -- not strictly under, so not machine-paced.
expect(isMachinePacedCadence({ count: 5, medianGapMs: 10 * 60_000 })).toBe(false);
});
});

describe("getSubmitterCadence (D1, fail-safe) (#4514)", () => {
function makeCadenceEnv(createdAts: string[]): Env {
return {
DB: {
prepare: () => ({
bind: () => ({
all: async () => ({ results: createdAts.map((createdAt) => ({ createdAt })) }),
}),
}),
},
} as unknown as Env;
}

it("returns count 0 / null with no submitter (early return, no DB touch)", async () => {
expect(await getSubmitterCadence({} as Env, "p", undefined)).toEqual({ count: 0, medianGapMs: null });
});

it("derives cadence from the queried created_at timestamps", async () => {
const t0 = new Date("2026-01-01T00:00:00.000Z").getTime();
const env = makeCadenceEnv([t0, t0 + 5 * 60_000, t0 + 10 * 60_000, t0 + 15 * 60_000, t0 + 20 * 60_000].map((ms) => new Date(ms).toISOString()));
const cadence = await getSubmitterCadence(env, "p", "farmer99");
expect(cadence).toEqual({ count: 5, medianGapMs: 5 * 60_000 });
expect(isMachinePacedCadence(cadence)).toBe(true);
});

it("fail-safe: degrades to count 0 / null when the query throws, never throws into the caller", async () => {
const env = {
DB: {
prepare: () => ({
bind: () => ({
all: async () => {
throw new Error("D1 boom");
},
}),
}),
},
} as unknown as Env;
expect(await getSubmitterCadence(env, "p", "farmer99")).toEqual({ count: 0, medianGapMs: null });
});

it("fail-safe: degrades to count 0 / null when the query returns a malformed result (?? [] fallback)", async () => {
const env = {
DB: {
prepare: () => ({
bind: () => ({
all: async () => undefined,
}),
}),
},
} as unknown as Env;
expect(await getSubmitterCadence(env, "p", "farmer99")).toEqual({ count: 0, medianGapMs: null });
});
});

// A minimal D1 stub: the first query (.first) returns submitter_stats; the window query (.all) returns the
// review_targets rows. Both come off the same prepared-statement stub (the two call sites use .first vs .all).
function makeEnv(opts: { statRow: { submissions: number; merged: number; closed: number; manual: number } | null; windowRows: Row[] }): Env {
Expand Down
Loading