Skip to content

Commit d67d0b5

Browse files
authored
feat(review): add a submission-cadence/inter-arrival-time signal to flag machine-paced submitters (#4549)
* feat(review): add a submission-cadence/inter-arrival-time signal to flag machine-paced submitters Every existing anti-abuse signal (reputation burst-floor, slop score) counts volume or ratios within a time window; none has an inter-arrival-time or rate term. A fast, well-formed, strategically low-value submitter is, by construction, invisible to the one dimension (superhuman pace) that would otherwise be a strong tell -- a submitter can clear every quality bar (good outcomes, real descriptions) while operating at a cadence no human plausibly sustains, and today that cadence carries zero weight anywhere. - computeSubmissionCadence / isMachinePacedCadence (pure): the median gap between a submitter's recent review_targets rows, flagged only once both a real sample size (5+) AND a sub-10-minute median gap are present -- a lone fast submission is not a pattern. - getSubmitterCadence: queries created_at across ALL review_targets rows (not just terminal ones -- a fresh burst of still-open submissions is exactly what this needs to catch, since the AI review already ran on each one by the time it becomes terminal). - Wired into shouldSkipAiForReputation as an independent, additional check alongside the existing quality/burst signal -- a submitter whose individual PRs all look fine can still be caught on cadence alone, and the (extra) cadence read is skipped once the quality/burst check alone already justifies downgrading. Scoped to per-repo cadence for now, matching this PR's own scope; an install-wide variant (mirroring #4513's confirmed-miner-aware cross-repo pattern) is a natural fast-follow once #4513 merges. Fixes #4514 * fix(db): renumber migration 0132 -> 0133 (0132 claimed by merged PR #4554) Rebase onto current main and take the next free number now that migrations/0132_impact_map_query_cache.sql (from #4554, itself a collision fix) occupies the number this branch originally guessed. * fix(db): renumber migration 0133 -> 0134 (0133 claimed by merged PR #4556) Rebase onto current main and take the next free number now that migrations/0133_screenshot_table_gate_skill_link.sql (from #4556) occupies the number this branch previously took.
1 parent 3faad4f commit d67d0b5

5 files changed

Lines changed: 219 additions & 1 deletion

File tree

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
-- Supports the submission-cadence signal (#4514): getSubmitterCadence queries ALL review_targets rows (not
2+
-- just terminal ones -- a fresh burst of still-open submissions is exactly what this needs to catch) filtered
3+
-- by (project, submitter, created_at). This shape isn't covered by the existing review_targets indexes
4+
-- (migrations/0050), which are keyed on status/verdict/terminal_at, not created_at alongside submitter.
5+
CREATE INDEX IF NOT EXISTS idx_review_targets_project_submitter_created ON review_targets (project, submitter, created_at);

src/review/reputation-wire.ts

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,9 @@
1414
// the ported module degrades to "neutral" / no-op on any DB error, so this never throws into the gate.
1515

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

7181
/**

src/review/submitter-reputation.ts

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,58 @@ export const REPUTATION_WINDOW_DAYS = 90;
7070
// Hard ceiling on rows pulled for one submitter's window so a pathological history can't blow the query up.
7171
const REPUTATION_WINDOW_ROW_CAP = 500;
7272

73+
// ── Submission-cadence signal (#4514). Every signal above is QUALITY-based (was the outcome good or bad) --
74+
// none of them have a TIMING dimension, so a fast, well-formed, strategically-low-value submitter is
75+
// invisible to the one dimension (superhuman pace) that would otherwise be a strong tell. This is queried
76+
// from ALL review_targets rows (not just terminal ones, unlike the quality signal above) -- a fresh burst of
77+
// still-open submissions is exactly the case this needs to catch, and by the time they become terminal the
78+
// (paid) AI review has already run on each one. ──
79+
const CADENCE_WINDOW_HOURS = 24;
80+
// Need at least this many recent submissions before judging pace at all -- a lone fast submission (a real
81+
// contributor who happened to open two PRs close together) is not a pattern.
82+
const CADENCE_MIN_SAMPLE = 5;
83+
// A human contributor, even a fast one, does not sustain a sub-10-minute median gap between distinct PR
84+
// submissions across many consecutive attempts -- reading, writing, and testing each change takes real time.
85+
const CADENCE_MAX_MEDIAN_GAP_MS = 10 * 60 * 1000;
86+
87+
export type SubmissionCadence = { count: number; medianGapMs: number | null };
88+
89+
/** Pure: the median gap (ms) between consecutive submissions, given their created_at timestamps in any order.
90+
* `medianGapMs` is `null` when there are fewer than 2 samples (no gap to measure). */
91+
export function computeSubmissionCadence(createdAtIsoTimestamps: readonly string[]): SubmissionCadence {
92+
const sorted = [...createdAtIsoTimestamps].map((t) => new Date(t).getTime()).sort((a, b) => a - b);
93+
if (sorted.length < 2) return { count: sorted.length, medianGapMs: null };
94+
const gaps: number[] = [];
95+
for (let i = 1; i < sorted.length; i++) gaps.push(sorted[i]! - sorted[i - 1]!);
96+
gaps.sort((a, b) => a - b);
97+
const mid = Math.floor(gaps.length / 2);
98+
const medianGapMs = gaps.length % 2 === 0 ? (gaps[mid - 1]! + gaps[mid]!) / 2 : gaps[mid]!;
99+
return { count: sorted.length, medianGapMs };
100+
}
101+
102+
/** Pure: does this cadence read as machine-paced? Needs both a real sample size AND a gap tighter than any
103+
* human contributor plausibly sustains across that many consecutive attempts. */
104+
export function isMachinePacedCadence(cadence: SubmissionCadence): boolean {
105+
return cadence.count >= CADENCE_MIN_SAMPLE && cadence.medianGapMs !== null && cadence.medianGapMs < CADENCE_MAX_MEDIAN_GAP_MS;
106+
}
107+
108+
/** Per-repo submission cadence for one submitter over the last {@link CADENCE_WINDOW_HOURS}. Fail-safe:
109+
* any read error degrades to `{ count: 0, medianGapMs: null }` (never machine-paced), identical in spirit to
110+
* {@link getSubmitterReputation}'s fail-safe-to-neutral. */
111+
export async function getSubmitterCadence(env: Env, project: string, submitter: string | undefined): Promise<SubmissionCadence> {
112+
if (!submitter) return { count: 0, medianGapMs: null };
113+
try {
114+
const result = await storage(env)
115+
.prepare(`SELECT created_at AS createdAt FROM review_targets WHERE project = ? AND submitter = ? AND created_at >= datetime('now', ?) ORDER BY created_at DESC LIMIT ?`)
116+
.bind(project, submitter, `-${CADENCE_WINDOW_HOURS} hours`, REPUTATION_WINDOW_ROW_CAP)
117+
.all<{ createdAt: string }>();
118+
const createdAts = (result?.results ?? []).map((r) => r.createdAt);
119+
return computeSubmissionCadence(createdAts);
120+
} catch {
121+
return { count: 0, medianGapMs: null };
122+
}
123+
}
124+
73125
// ── reasonCode → quality bucket (#reputation-redesign). Buckets reflect the LIVE D1 reasonCode taxonomy. ──
74126
// SUCCESS: a genuine reviewer/merge approval.
75127
// QUALITY_FAIL: a genuine RECENT reviewer reject (real quality signal).

test/unit/reputation-wiring.test.ts

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -149,6 +149,56 @@ describe("shouldSkipAiForReputation (helper)", () => {
149149
expect(await shouldSkipAiForReputation(env, { project: "acme/widgets", submitter: "burster" })).toBe(true);
150150
expect(await shouldSkipAiForReputation(env, { project: "acme/widgets", submitter: "newcomer" })).toBe(false);
151151
});
152+
153+
it("FLAG-ON: false for a null submitter (the ?? undefined coalesce on both the quality and cadence reads)", async () => {
154+
const env = createTestEnv({ GITTENSORY_REVIEW_REPUTATION: "true" });
155+
expect(await shouldSkipAiForReputation(env, { project: "acme/widgets", submitter: null })).toBe(false);
156+
});
157+
158+
describe("submission-cadence signal (#4514)", () => {
159+
async function seedReviewTarget(env: Env, args: { number: number; submitter: string; createdAt: string }) {
160+
await env.DB.prepare(
161+
`INSERT INTO review_targets (id, project, kind, repo, number, submitter, status, decision_json, terminal_at, created_at)
162+
VALUES (?, 'acme/widgets', 'pull_request', 'acme/widgets', ?, ?, 'merged', ?, ?, ?)`,
163+
)
164+
.bind(`acme/widgets:pull_request:acme/widgets#${args.number}`, args.number, args.submitter, JSON.stringify({ reasonCode: "dual_review_approved" }), args.createdAt, args.createdAt)
165+
.run();
166+
}
167+
168+
it("FLAG-ON: true for a machine-paced submitter even though every submission itself looks fine (quality-neutral)", async () => {
169+
const env = createTestEnv({ GITTENSORY_REVIEW_REPUTATION: "true" });
170+
// Anchored to now (minus a couple hours of headroom) -- the cadence query only looks back 24h, so a
171+
// fixed past date would fall outside the window and vacuously read as "0 samples, not machine-paced".
172+
const t0 = Date.now() - 2 * 60 * 60_000;
173+
for (let i = 0; i < 5; i++) {
174+
// All merged/approved -- the QUALITY signal alone stays neutral/trusted; only cadence should trip this.
175+
await seedReviewTarget(env, { number: i, submitter: "speedster", createdAt: new Date(t0 + i * 5 * 60_000).toISOString() });
176+
}
177+
expect(await shouldSkipAiForReputation(env, { project: "acme/widgets", submitter: "speedster" })).toBe(true);
178+
});
179+
180+
it("FLAG-ON: false for the same number of submissions spread naturally over hours (comfortably human pace)", async () => {
181+
const env = createTestEnv({ GITTENSORY_REVIEW_REPUTATION: "true" });
182+
const t0 = Date.now() - 20 * 60 * 60_000;
183+
for (let i = 0; i < 5; i++) {
184+
await seedReviewTarget(env, { number: i + 100, submitter: "steady", createdAt: new Date(t0 + i * 3 * 60 * 60_000).toISOString() });
185+
}
186+
expect(await shouldSkipAiForReputation(env, { project: "acme/widgets", submitter: "steady" })).toBe(false);
187+
});
188+
189+
it("FLAG-ON: skips the (extra) cadence read once the quality/burst signal already justifies downgrading", async () => {
190+
const env = createTestEnv({ GITTENSORY_REVIEW_REPUTATION: "true" });
191+
await seedSubmitter(env, { project: "acme/widgets", submitter: "burster", submissions: 12, merged: 0, closed: 12, manual: 0 });
192+
const spy = vi.spyOn(env.DB, "prepare");
193+
const before = spy.mock.calls.length;
194+
expect(await shouldSkipAiForReputation(env, { project: "acme/widgets", submitter: "burster" })).toBe(true);
195+
// Exactly the calls the quality/burst read itself makes (submitter_stats + review_targets window) --
196+
// no additional prepare() for a cadence query once the burst check alone already returned true.
197+
const afterQualityOnlyCallCount = spy.mock.calls.length - before;
198+
spy.mockRestore();
199+
expect(afterQualityOnlyCallCount).toBe(2);
200+
});
201+
});
152202
});
153203

154204
describe("processGitHubWebhook records the reputation outcome on a terminal PR (flag-ON call site)", () => {

test/unit/submitter-reputation.test.ts

Lines changed: 101 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,12 @@
11
import { describe, expect, it } from "vitest";
22
import {
33
classifyOutcome,
4+
computeSubmissionCadence,
45
countOutcomes,
56
DEFAULT_REPUTATION_CONFIG,
7+
getSubmitterCadence,
68
getSubmitterReputation,
9+
isMachinePacedCadence,
710
recordSubmissionOutcome,
811
REPUTATION_WINDOW_DAYS,
912
type ReputationConfig,
@@ -273,6 +276,104 @@ describe("recordSubmissionOutcome / getSubmitterReputation (D1, fail-safe)", ()
273276
});
274277
});
275278

279+
describe("computeSubmissionCadence (pure) (#4514)", () => {
280+
it("returns count and null medianGapMs for 0 or 1 samples (nothing to measure a gap between)", () => {
281+
expect(computeSubmissionCadence([])).toEqual({ count: 0, medianGapMs: null });
282+
expect(computeSubmissionCadence(["2026-01-01T00:00:00.000Z"])).toEqual({ count: 1, medianGapMs: null });
283+
});
284+
285+
it("computes the median gap between consecutive submissions, order-independent", () => {
286+
// Gaps: 10min, 20min, 30min -> sorted [10,20,30] -> median 20min.
287+
const t0 = new Date("2026-01-01T00:00:00.000Z").getTime();
288+
const timestamps = [t0, t0 + 10 * 60_000, t0 + 30 * 60_000, t0 + 60 * 60_000].map((ms) => new Date(ms).toISOString());
289+
// Shuffle the input order -- the function must sort internally, not assume caller ordering.
290+
const shuffled = [timestamps[2]!, timestamps[0]!, timestamps[3]!, timestamps[1]!];
291+
expect(computeSubmissionCadence(shuffled)).toEqual({ count: 4, medianGapMs: 20 * 60_000 });
292+
});
293+
294+
it("averages the two middle gaps for an even number of gaps", () => {
295+
// 3 timestamps -> 2 gaps: 10min, 30min -> even count -> average = 20min.
296+
const t0 = new Date("2026-01-01T00:00:00.000Z").getTime();
297+
const timestamps = [t0, t0 + 10 * 60_000, t0 + 40 * 60_000].map((ms) => new Date(ms).toISOString());
298+
expect(computeSubmissionCadence(timestamps)).toEqual({ count: 3, medianGapMs: 20 * 60_000 });
299+
});
300+
});
301+
302+
describe("isMachinePacedCadence (pure) (#4514)", () => {
303+
it("requires BOTH the minimum sample size AND a sub-threshold median gap", () => {
304+
// Below minSample (5) -- fast, but not enough samples to call it a pattern.
305+
expect(isMachinePacedCadence({ count: 4, medianGapMs: 60_000 })).toBe(false);
306+
// Enough samples, but the gap is comfortably human (well over 10min).
307+
expect(isMachinePacedCadence({ count: 10, medianGapMs: 60 * 60_000 })).toBe(false);
308+
// No gap at all to measure (count < 2 internally, or explicitly null).
309+
expect(isMachinePacedCadence({ count: 8, medianGapMs: null })).toBe(false);
310+
// Enough samples AND a tight gap -- machine-paced.
311+
expect(isMachinePacedCadence({ count: 5, medianGapMs: 5 * 60_000 })).toBe(true);
312+
expect(isMachinePacedCadence({ count: 20, medianGapMs: 60_000 })).toBe(true);
313+
});
314+
315+
it("is a boundary at exactly the configured thresholds", () => {
316+
// Exactly at minSample, exactly under the max gap -- still counts.
317+
expect(isMachinePacedCadence({ count: 5, medianGapMs: 10 * 60_000 - 1 })).toBe(true);
318+
// Exactly AT the max gap -- not strictly under, so not machine-paced.
319+
expect(isMachinePacedCadence({ count: 5, medianGapMs: 10 * 60_000 })).toBe(false);
320+
});
321+
});
322+
323+
describe("getSubmitterCadence (D1, fail-safe) (#4514)", () => {
324+
function makeCadenceEnv(createdAts: string[]): Env {
325+
return {
326+
DB: {
327+
prepare: () => ({
328+
bind: () => ({
329+
all: async () => ({ results: createdAts.map((createdAt) => ({ createdAt })) }),
330+
}),
331+
}),
332+
},
333+
} as unknown as Env;
334+
}
335+
336+
it("returns count 0 / null with no submitter (early return, no DB touch)", async () => {
337+
expect(await getSubmitterCadence({} as Env, "p", undefined)).toEqual({ count: 0, medianGapMs: null });
338+
});
339+
340+
it("derives cadence from the queried created_at timestamps", async () => {
341+
const t0 = new Date("2026-01-01T00:00:00.000Z").getTime();
342+
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()));
343+
const cadence = await getSubmitterCadence(env, "p", "farmer99");
344+
expect(cadence).toEqual({ count: 5, medianGapMs: 5 * 60_000 });
345+
expect(isMachinePacedCadence(cadence)).toBe(true);
346+
});
347+
348+
it("fail-safe: degrades to count 0 / null when the query throws, never throws into the caller", async () => {
349+
const env = {
350+
DB: {
351+
prepare: () => ({
352+
bind: () => ({
353+
all: async () => {
354+
throw new Error("D1 boom");
355+
},
356+
}),
357+
}),
358+
},
359+
} as unknown as Env;
360+
expect(await getSubmitterCadence(env, "p", "farmer99")).toEqual({ count: 0, medianGapMs: null });
361+
});
362+
363+
it("fail-safe: degrades to count 0 / null when the query returns a malformed result (?? [] fallback)", async () => {
364+
const env = {
365+
DB: {
366+
prepare: () => ({
367+
bind: () => ({
368+
all: async () => undefined,
369+
}),
370+
}),
371+
},
372+
} as unknown as Env;
373+
expect(await getSubmitterCadence(env, "p", "farmer99")).toEqual({ count: 0, medianGapMs: null });
374+
});
375+
});
376+
276377
// A minimal D1 stub: the first query (.first) returns submitter_stats; the window query (.all) returns the
277378
// review_targets rows. Both come off the same prepared-statement stub (the two call sites use .first vs .all).
278379
function makeEnv(opts: { statRow: { submissions: number; merged: number; closed: number; manual: number } | null; windowRows: Row[] }): Env {

0 commit comments

Comments
 (0)