Skip to content

Commit b1540df

Browse files
committed
fix(review): record unlinked-issue verifier's own AI spend into the shared budget counter
Review feedback on #4551: the shared-budget check added in this PR read sumAiEstimatedNeuronsSince before running the verifier, but nothing wrote this feature's own actual calls back into that counter -- a free rider that respected every other feature's spend while never counting its own, so the true aggregate could silently exceed AI_DAILY_NEURON_BUDGET by however much this feature used. Record the same worst-case per-candidate estimate the budget check itself uses, mirroring ai-slop.ts's pre-computed-estimate recording convention.
1 parent 9a6ba94 commit b1540df

2 files changed

Lines changed: 69 additions & 2 deletions

File tree

src/review/unlinked-issue-guardrail.ts

Lines changed: 30 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,12 +18,13 @@ import {
1818
getFreshOfficialMinerDetection,
1919
mostRecentAuditEventForOtherTarget,
2020
listOpenIssues,
21+
recordAiUsageEvent,
2122
recordAuditEvent,
2223
sumAiEstimatedNeuronsSince,
2324
upsertOfficialMinerDetection,
2425
} from "../db/repositories";
2526
import { fetchOfficialGittensorMiner } from "../gittensor/api";
26-
import { clampNumber, estimateNeurons, utcDayStartIso } from "../services/ai-review";
27+
import { BEST_REVIEW_MODELS, RELIABLE_FALLBACK_MODELS, clampNumber, estimateNeurons, utcDayStartIso } from "../services/ai-review";
2728
import { findUnlinkedIssueCandidates, MAX_CANDIDATES, type CandidateOpenIssue } from "../signals/unlinked-issue-candidates";
2829
import type { UnlinkedIssueGuardrailConfig } from "../types";
2930
import { DIFF_CHAR_BUDGET, MAX_TOKENS, verifyUnlinkedIssueMatch } from "./unlinked-issue-match";
@@ -62,6 +63,12 @@ const VERIFY_PROMPT_OVERHEAD_CHAR_ESTIMATE = 2_000;
6263
// verifyUnlinkedIssueMatch tries a primary model and, ONLY on a thrown error, a fallback -- two calls is the
6364
// real worst case per candidate, not the common case, but this budget check must size for the worst case.
6465
const VERIFY_MAX_MODEL_ATTEMPTS_PER_CANDIDATE = 2;
66+
// Review feedback on #4551: the budget check above reads sumAiEstimatedNeuronsSince, but without a writer
67+
// this feature's own real AI spend never contributed to that counter -- a free rider that respects every
68+
// OTHER feature's usage but never counts its own, so the true aggregate spend could silently exceed
69+
// AI_DAILY_NEURON_BUDGET by however much this feature actually used. recordUnlinkedIssueVerifyUsage (below)
70+
// closes that gap by recording into the SAME shared ai_usage_events table this check reads from.
71+
const UNLINKED_ISSUE_VERIFY_USAGE_FEATURE = "unlinked_issue_verify";
6572

6673
/** Has this actor already run the AI verifier at or beyond the rate ceiling in the last window, across every
6774
* repo/PR? Fail-safe: a read error resolves to "not rate-limited," so the pre-#4515 unconditional-
@@ -94,6 +101,25 @@ async function isUnlinkedIssueVerifyBudgetExceeded(env: Env, candidateCount: num
94101
return estimatedNeurons > remainingBudget;
95102
}
96103

104+
/** Record ONE candidate's actual AI spend into the shared `ai_usage_events` ledger -- the SAME table
105+
* {@link isUnlinkedIssueVerifyBudgetExceeded} sums from, so this feature's own usage counts against the
106+
* budget it itself enforces on others (see the module-level comment on {@link UNLINKED_ISSUE_VERIFY_USAGE_FEATURE}).
107+
* Records the same worst-case per-candidate estimate the budget check itself uses -- a deliberate over-count
108+
* (verifyUnlinkedIssueMatch's common case is ONE model call, not the two this sizes for), not a precise
109+
* post-hoc token read, mirroring ai-slop.ts's own pre-computed-estimate recording convention. Best-effort: a
110+
* write failure is swallowed (telemetry must never block the gate). */
111+
async function recordUnlinkedIssueVerifyUsage(env: Env, repoFullName: string, pullNumber: number): Promise<void> {
112+
const estimatedNeurons = estimateNeurons(DIFF_CHAR_BUDGET + VERIFY_PROMPT_OVERHEAD_CHAR_ESTIMATE, MAX_TOKENS, VERIFY_MAX_MODEL_ATTEMPTS_PER_CANDIDATE);
113+
await recordAiUsageEvent(env, {
114+
feature: UNLINKED_ISSUE_VERIFY_USAGE_FEATURE,
115+
route: "github_app.unlinked_issue_verify",
116+
model: [BEST_REVIEW_MODELS[0], RELIABLE_FALLBACK_MODELS[0]].join("+"),
117+
status: "ok",
118+
estimatedNeurons,
119+
detail: `unlinked-issue-match verification for ${repoFullName}#${pullNumber}`,
120+
}).catch(() => undefined);
121+
}
122+
97123
/** Minimal cached miner-identity check, deliberately independent of processors.ts's getCachedOfficialMinerDetection
98124
* (same cache table and TTLs, no audit-log side effect -- this call site doesn't need one). Fail-safe: any
99125
* lookup failure resolves to "not a confirmed miner," never the reverse. */
@@ -202,6 +228,9 @@ export async function resolveUnlinkedIssueMatchDisposition(env: Env, input: Reso
202228
if (await isUnlinkedIssueVerifyBudgetExceeded(env, candidates.length)) return undefined;
203229
for (const candidate of candidates) {
204230
if (authorLogin) await recordUnlinkedIssueVerifyAttempt(env, input.repoFullName, input.pullNumber, authorLogin);
231+
// Record spend regardless of authorLogin -- an AI call happens either way; only the PER-ACTOR rate
232+
// ceiling above needs a known actor, this shared-budget accounting does not.
233+
await recordUnlinkedIssueVerifyUsage(env, input.repoFullName, input.pullNumber);
205234
const verdict = await verifyUnlinkedIssueMatch(env, {
206235
prTitle: input.prTitle,
207236
prBody: input.prBody,

test/unit/unlinked-issue-guardrail.test.ts

Lines changed: 39 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import { describe, expect, it, vi } from "vitest";
22
import { createTestEnv } from "../helpers/d1";
3-
import { countRecentAuditEventsForActor, recordAuditEvent, upsertIssueFromGitHub, hasRecentAuditEvent } from "../../src/db/repositories";
3+
import { countRecentAuditEventsForActor, recordAuditEvent, sumAiEstimatedNeuronsSince, upsertIssueFromGitHub, hasRecentAuditEvent } from "../../src/db/repositories";
44
import {
55
resolveUnlinkedIssueMatchDisposition,
66
UNLINKED_ISSUE_MATCH_AUDIT_EVENT_TYPE,
@@ -425,6 +425,44 @@ describe("resolveUnlinkedIssueMatchDisposition", () => {
425425
expect(await countRecentAuditEventsForActor(env, "contributor-a", UNLINKED_ISSUE_VERIFY_ATTEMPT_AUDIT_EVENT_TYPE, "2000-01-01T00:00:00.000Z")).toBe(1);
426426
});
427427

428+
it("records this candidate's actual spend into the SAME shared ai_usage_events counter the budget check reads (review feedback on #4551)", async () => {
429+
const run = vi.fn(async () => ({ response: JSON.stringify(aiVerdict()) }));
430+
const env = createTestEnv({ AI: { run } as unknown as Ai });
431+
await seedIssue(env, 7, "webhook retry duplicate bug", "retries duplicate events under load, needs a dedup key");
432+
433+
const usedBefore = await sumAiEstimatedNeuronsSince(env, "2000-01-01T00:00:00.000Z");
434+
expect(usedBefore).toBe(0);
435+
436+
await resolveUnlinkedIssueMatchDisposition(env, { ...BASE_INPUT, config: config() });
437+
438+
const usedAfter = await sumAiEstimatedNeuronsSince(env, "2000-01-01T00:00:00.000Z");
439+
expect(usedAfter).toBeGreaterThan(0);
440+
});
441+
442+
it("records spend even when the author login is unknown (unlike the rate ceiling, this does not need a known actor)", async () => {
443+
const run = vi.fn(async () => ({ response: JSON.stringify(aiVerdict()) }));
444+
const env = createTestEnv({ AI: { run } as unknown as Ai });
445+
await seedIssue(env, 7, "webhook retry duplicate bug", "retries duplicate events under load, needs a dedup key");
446+
447+
await resolveUnlinkedIssueMatchDisposition(env, { ...BASE_INPUT, prAuthorLogin: null, config: config() });
448+
449+
expect(await sumAiEstimatedNeuronsSince(env, "2000-01-01T00:00:00.000Z")).toBeGreaterThan(0);
450+
});
451+
452+
it("swallows a usage-recording write failure without affecting the verification result", async () => {
453+
const run = vi.fn(async () => ({ response: JSON.stringify(aiVerdict()) }));
454+
const env = createTestEnv({ AI: { run } as unknown as Ai });
455+
await seedIssue(env, 7, "webhook retry duplicate bug", "retries duplicate events under load, needs a dedup key");
456+
const realPrepare = env.DB.prepare.bind(env.DB);
457+
env.DB.prepare = ((sql: string) => {
458+
if (/INSERT INTO.*ai_usage_events/i.test(sql)) throw new Error("d1 down");
459+
return realPrepare(sql);
460+
}) as typeof env.DB.prepare;
461+
462+
const result = await resolveUnlinkedIssueMatchDisposition(env, { ...BASE_INPUT, config: config() });
463+
expect(result?.kind).toBe("hold");
464+
});
465+
428466
it("fails open (verification still runs) when the rate-ceiling read itself errors", async () => {
429467
const run = vi.fn(async () => ({ response: JSON.stringify(aiVerdict()) }));
430468
const env = createTestEnv({ AI: { run } as unknown as Ai });

0 commit comments

Comments
 (0)