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
15 changes: 15 additions & 0 deletions src/db/repositories.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2636,6 +2636,21 @@ export async function mostRecentAuditEventForOtherTarget(env: Env, actor: string
return rows[0]?.createdAt ?? null;
}

/** Count-returning, cross-repo variant (#4515): how many recent events of this type has this actor generated,
* across EVERY target (repo/PR), within the recency window? Unlike {@link countRecentAuditEventsForActorAndTarget}
* (scoped to one target thread), this counts an actor's ACTIVITY VOLUME irrespective of which PR/repo each
* event landed on -- backs a per-actor rate ceiling on an expensive operation (e.g. a paid AI call) that a
* single-target-scoped counter would never catch for an actor spreading attempts across many repos. */
export async function countRecentAuditEventsForActor(env: Env, actor: string, eventType: string, sinceIso: string): Promise<number> {
const db = getDb(env.DB);
const [row] = await db
.select({ count: sql<number>`count(*)` })
.from(auditEvents)
.where(and(eq(auditEvents.actor, actor), eq(auditEvents.eventType, eventType), gte(auditEvents.createdAt, sinceIso)));
/* v8 ignore next -- count(*) always returns exactly one row; the empty-array guard only satisfies the destructure type. */
return row?.count ?? 0;
}

/** Count-returning variant of {@link hasRecentAuditEvent}, additionally scoped to one `targetKey` (e.g. a single
* `owner/repo#123` PR/issue) rather than the actor's activity across the whole repo. Backs the review-request
* nagging cooldown (#2463): counting how many `@gittensory` pings a contributor has sent on ONE thread within
Expand Down
112 changes: 109 additions & 3 deletions src/review/unlinked-issue-guardrail.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,21 @@
// repo that hasn't opted in (the default) or a PR that already links an issue (the common case) pays
// nothing beyond two boolean checks.

import { getFreshOfficialMinerDetection, mostRecentAuditEventForOtherTarget, listOpenIssues, recordAuditEvent, upsertOfficialMinerDetection } from "../db/repositories";
import {
countRecentAuditEventsForActor,
getFreshOfficialMinerDetection,
mostRecentAuditEventForOtherTarget,
listOpenIssues,
recordAiUsageEvent,
recordAuditEvent,
sumAiEstimatedNeuronsSince,
upsertOfficialMinerDetection,
} from "../db/repositories";
import { fetchOfficialGittensorMiner } from "../gittensor/api";
import { findUnlinkedIssueCandidates, type CandidateOpenIssue } from "../signals/unlinked-issue-candidates";
import { BEST_REVIEW_MODELS, RELIABLE_FALLBACK_MODELS, clampNumber, estimateNeurons, utcDayStartIso } from "../services/ai-review";
import { findUnlinkedIssueCandidates, MAX_CANDIDATES, type CandidateOpenIssue } from "../signals/unlinked-issue-candidates";
import type { UnlinkedIssueGuardrailConfig } from "../types";
import { verifyUnlinkedIssueMatch } from "./unlinked-issue-match";
import { DIFF_CHAR_BUDGET, MAX_TOKENS, verifyUnlinkedIssueMatch } from "./unlinked-issue-match";

/** Shared with any future reader that wants to correlate these holds/closes across repos for one contributor. */
export const UNLINKED_ISSUE_MATCH_AUDIT_EVENT_TYPE = "github_app.unlinked_issue_match_hold";
Expand All @@ -35,6 +45,81 @@ const VELOCITY_EXCEPTION_MAX_GAP_MS = 60 * 60 * 1000;
const OFFICIAL_MINER_DETECTION_TTL_MS = 5 * 60 * 1000;
const OFFICIAL_MINER_DETECTION_UNAVAILABLE_TTL_MS = 60 * 1000;

// #4515: every candidate below costs one real (if small) AI call, so two cost-control gates run ahead of the
// loop -- a per-actor RATE ceiling, and a check against the shared daily neuron budget every other free-tier
// AI feature draws from (sumAiEstimatedNeuronsSince/AI_DAILY_NEURON_BUDGET, mirroring ai-slop.ts's own
// pre-call budget check). Both are cost controls, not correctness gates: on any read failure they fail
// toward "proceed as if this layer didn't exist" (full verification runs), never toward silently disabling
// the guardrail they sit in front of by skipping straight to `undefined`.
export const UNLINKED_ISSUE_VERIFY_ATTEMPT_AUDIT_EVENT_TYPE = "github_app.unlinked_issue_verify_attempt";
// Generous by design: a legitimate contributor never approaches this in an hour even opening several PRs
// back-to-back. Sized to catch a scripted/abusive burst hammering the AI verifier, not ordinary human cadence.
const VERIFY_RATE_CEILING_MAX_ATTEMPTS = 15;
const VERIFY_RATE_CEILING_WINDOW_MS = 60 * 60 * 1000;
// Flat overhead for the parts of the verifier's prompt that aren't the (already-bounded) diff -- the system
// prompt, PR title/body, and candidate issue title/body. None of these are cheaply boundable per candidate
// ahead of time, so this deliberately over-, never under-, estimates: a budget check must never undercount.
const VERIFY_PROMPT_OVERHEAD_CHAR_ESTIMATE = 2_000;
// verifyUnlinkedIssueMatch tries a primary model and, ONLY on a thrown error, a fallback -- two calls is the
// real worst case per candidate, not the common case, but this budget check must size for the worst case.
const VERIFY_MAX_MODEL_ATTEMPTS_PER_CANDIDATE = 2;
// Review feedback on #4551: the budget check above reads sumAiEstimatedNeuronsSince, but without a writer
// this feature's own real AI spend never contributed to that counter -- a free rider that respects every
// OTHER feature's usage but never counts its own, so the true aggregate spend could silently exceed
// AI_DAILY_NEURON_BUDGET by however much this feature actually used. recordUnlinkedIssueVerifyUsage (below)
// closes that gap by recording into the SAME shared ai_usage_events table this check reads from.
const UNLINKED_ISSUE_VERIFY_USAGE_FEATURE = "unlinked_issue_verify";

/** Has this actor already run the AI verifier at or beyond the rate ceiling in the last window, across every
* repo/PR? Fail-safe: a read error resolves to "not rate-limited," so the pre-#4515 unconditional-
* verification behavior takes over rather than a DB hiccup silently disabling this guardrail. */
async function isOverUnlinkedIssueVerifyRateCeiling(env: Env, authorLogin: string): Promise<boolean> {
const sinceIso = new Date(Date.now() - VERIFY_RATE_CEILING_WINDOW_MS).toISOString();
const count = await countRecentAuditEventsForActor(env, authorLogin, UNLINKED_ISSUE_VERIFY_ATTEMPT_AUDIT_EVENT_TYPE, sinceIso).catch(() => 0);
return count >= VERIFY_RATE_CEILING_MAX_ATTEMPTS;
}

/** Would verifying `candidateCount` candidates (each up to {@link VERIFY_MAX_MODEL_ATTEMPTS_PER_CANDIDATE}
* model calls) risk exceeding the shared daily AI neuron budget -- the same counter/env var every other
* free-tier AI feature draws from? `candidateCount` is defensively re-clamped to {@link MAX_CANDIDATES}: the
* caller already bounds it there, but this estimate must never balloon even if that invariant ever slips.
* Fail-safe for the same reason as the rate ceiling above: a read error resolves to "budget available." */
async function isUnlinkedIssueVerifyBudgetExceeded(env: Env, candidateCount: number): Promise<boolean> {
const worstCaseCandidateCount = Math.min(candidateCount, MAX_CANDIDATES);
const estimatedNeurons = estimateNeurons(
DIFF_CHAR_BUDGET + VERIFY_PROMPT_OVERHEAD_CHAR_ESTIMATE,
MAX_TOKENS,
worstCaseCandidateCount * VERIFY_MAX_MODEL_ATTEMPTS_PER_CANDIDATE,
);
// Resolved IDENTICALLY to ai-slop.ts's own pre-call check -- both features sum into the same
// sumAiEstimatedNeuronsSince counter, so a divergent default/ceiling here would under- or over-count
// against the one real shared budget.
const rawNeuronBudget = Number(env.AI_DAILY_NEURON_BUDGET);
const budget = clampNumber(env.AI_DAILY_NEURON_BUDGET && Number.isFinite(rawNeuronBudget) ? rawNeuronBudget : 10_000_000, 0, 10_000_000);
const used = await sumAiEstimatedNeuronsSince(env, utcDayStartIso()).catch(() => 0);
const remainingBudget = Math.max(0, budget - used);
return estimatedNeurons > remainingBudget;
}

/** Record ONE candidate's actual AI spend into the shared `ai_usage_events` ledger -- the SAME table
* {@link isUnlinkedIssueVerifyBudgetExceeded} sums from, so this feature's own usage counts against the
* budget it itself enforces on others (see the module-level comment on {@link UNLINKED_ISSUE_VERIFY_USAGE_FEATURE}).
* Records the same worst-case per-candidate estimate the budget check itself uses -- a deliberate over-count
* (verifyUnlinkedIssueMatch's common case is ONE model call, not the two this sizes for), not a precise
* post-hoc token read, mirroring ai-slop.ts's own pre-computed-estimate recording convention. Best-effort: a
* write failure is swallowed (telemetry must never block the gate). */
async function recordUnlinkedIssueVerifyUsage(env: Env, repoFullName: string, pullNumber: number): Promise<void> {
const estimatedNeurons = estimateNeurons(DIFF_CHAR_BUDGET + VERIFY_PROMPT_OVERHEAD_CHAR_ESTIMATE, MAX_TOKENS, VERIFY_MAX_MODEL_ATTEMPTS_PER_CANDIDATE);
await recordAiUsageEvent(env, {
feature: UNLINKED_ISSUE_VERIFY_USAGE_FEATURE,
route: "github_app.unlinked_issue_verify",
model: [BEST_REVIEW_MODELS[0], RELIABLE_FALLBACK_MODELS[0]].join("+"),
status: "ok",
estimatedNeurons,
detail: `unlinked-issue-match verification for ${repoFullName}#${pullNumber}`,
}).catch(() => undefined);
}

/** Minimal cached miner-identity check, deliberately independent of processors.ts's getCachedOfficialMinerDetection
* (same cache table and TTLs, no audit-log side effect -- this call site doesn't need one). Fail-safe: any
* lookup failure resolves to "not a confirmed miner," never the reverse. */
Expand Down Expand Up @@ -101,6 +186,19 @@ async function recordUnlinkedIssueMatchOccurrence(env: Env, repoFullName: string
}).catch(() => undefined);
}

/** Record that the AI verifier actually ran against one candidate, so {@link isOverUnlinkedIssueVerifyRateCeiling}
* accumulates this actor's volume correctly across every repo/PR they touch, not just this thread. Fire-and-
* forget, same rationale as {@link recordUnlinkedIssueMatchOccurrence}: a write failure must never block the gate. */
async function recordUnlinkedIssueVerifyAttempt(env: Env, repoFullName: string, pullNumber: number, authorLogin: string): Promise<void> {
await recordAuditEvent(env, {
eventType: UNLINKED_ISSUE_VERIFY_ATTEMPT_AUDIT_EVENT_TYPE,
actor: authorLogin,
targetKey: unlinkedIssueMatchTargetKey(repoFullName, pullNumber),
outcome: "completed",
detail: "unlinked-issue-match AI verifier invoked",
}).catch(() => undefined);
}

/**
* Resolve the unlinked-issue-match disposition for one PR, or `undefined` when nothing should hold or close
* it. Checks candidates in the pre-filter's ranked order and acts on the FIRST one that clears
Expand All @@ -124,7 +222,15 @@ export async function resolveUnlinkedIssueMatchDisposition(env: Env, input: Reso
});
if (candidates.length === 0) return undefined;
const authorLogin = input.prAuthorLogin?.trim() || null;
// #4515: cost-control gates ahead of the AI loop below. An unidentifiable author can't be rate-limited
// individually (nothing to key the ceiling on), so only the shared budget check applies to them.
if (authorLogin && (await isOverUnlinkedIssueVerifyRateCeiling(env, authorLogin))) return undefined;
if (await isUnlinkedIssueVerifyBudgetExceeded(env, candidates.length)) return undefined;
for (const candidate of candidates) {
if (authorLogin) await recordUnlinkedIssueVerifyAttempt(env, input.repoFullName, input.pullNumber, authorLogin);
// Record spend regardless of authorLogin -- an AI call happens either way; only the PER-ACTOR rate
// ceiling above needs a known actor, this shared-budget accounting does not.
await recordUnlinkedIssueVerifyUsage(env, input.repoFullName, input.pullNumber);
const verdict = await verifyUnlinkedIssueMatch(env, {
prTitle: input.prTitle,
prBody: input.prBody,
Expand Down
6 changes: 4 additions & 2 deletions src/review/unlinked-issue-match.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,11 @@ export type UnlinkedIssueMatchVerdict = {

const NO_MATCH: UnlinkedIssueMatchVerdict = { matched: false, confidence: 0, evidence: "" };

const MAX_TOKENS = 400;
// Exported so the guardrail orchestrator (unlinked-issue-guardrail.ts, #4515) can size its own worst-case
// per-PR AI-spend estimate off the same numbers, rather than a second, driftable copy of them.
export const MAX_TOKENS = 400;
// This check only needs enough diff to judge scope overlap, not the full multi-file review budget.
const DIFF_CHAR_BUDGET = 6_000;
export const DIFF_CHAR_BUDGET = 6_000;

type AiRunner = { run: (model: string, options: unknown, extra?: unknown) => Promise<unknown> };

Expand Down
6 changes: 4 additions & 2 deletions src/signals/unlinked-issue-candidates.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,10 @@ export type FindUnlinkedIssueCandidatesInput = {
};

// Bound the AI-verifier fan-out per PR: even a repo with hundreds of open issues only ever sends its
// top-scoring handful for a real (paid/self-host-compute) AI call.
const MAX_CANDIDATES = 3;
// top-scoring handful for a real (paid/self-host-compute) AI call. Exported so the guardrail orchestrator
// (unlinked-issue-guardrail.ts, #4515) can size its own worst-case per-PR AI-spend estimate off the same
// number, rather than a second, driftable copy of this constant.
export const MAX_CANDIDATES = 3;
// A path/basename mention is a much stronger signal than shared vocabulary — worth several tokens' score,
// and (deliberately) enough on its own to qualify a candidate even with zero token overlap (an issue that
// names the exact file this PR touches is worth checking regardless of shared wording).
Expand Down
Loading