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
14 changes: 9 additions & 5 deletions src/queue/processors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7751,14 +7751,18 @@ export async function runLinkedIssueSatisfactionForAdvisory(
model: args.settings.aiReviewModel ?? storedKey.model,
}
: null;
// #linked-issue-satisfaction-cache: the assessment's LLM call is fully deterministic given the same head SHA
// + linked issue number (no RAG/grounding/enrichment feeds into it), so a repeated scheduled sweep pass at
// an unchanged head+issue reuses the stored result instead of re-spending up to 6 free-tier attempts (or a
// BYOK call) on every tick — mirrors ai_slop_cache's confirmed-in-production motivation exactly.
const diff = buildAiReviewDiff(args.files);
// #linked-issue-satisfaction-cache: the assessment's LLM call is fully deterministic for the same
// reviewer configuration and prompt. GitHub issue/PR text can be edited without changing the head SHA, so
// those prompt fields are part of this fingerprint rather than relying only on the row key.
const inputFingerprint = await linkedIssueSatisfactionCacheInputFingerprint({
byok: Boolean(providerKey),
provider: providerKey?.provider,
model: providerKey?.model,
issueText,
prTitle: args.pr.title,
prBody: args.pr.body ?? undefined,
diff,
});
const cached = await getCachedLinkedIssueSatisfaction(
env,
Expand Down Expand Up @@ -7798,7 +7802,7 @@ export async function runLinkedIssueSatisfactionForAdvisory(
issueText,
prTitle: args.pr.title,
prBody: args.pr.body ?? undefined,
diff: buildAiReviewDiff(args.files),
diff,
actor: args.author,
providerKey,
});
Expand Down
23 changes: 14 additions & 9 deletions src/review/linked-issue-satisfaction-cache-input.ts
Original file line number Diff line number Diff line change
@@ -1,19 +1,20 @@
import { sha256Hex } from "../utils/crypto";

// #linked-issue-satisfaction-cache: mirrors ai-slop-cache-input.ts's fingerprint discipline exactly (kept as
// its own small module -- not reused directly -- so the two caches' version strings never collide/alias each
// other in stored rows). The satisfaction assessment's only input that can change independently of the PR's
// head SHA is which provider writes the opinion: the free/default reviewer vs. a maintainer's BYOK key/model
// (see LinkedIssueSatisfactionRunInput in ../services/linked-issue-satisfaction-run). Issue title/body/diff are
// pinned to the head SHA (a fresh commit is what invalidates the cache row itself) and the linked issue number
// is a SEPARATE primary-key column (not folded into this fingerprint) -- see the cache table's migration doc
// for why a changed primary linked issue must miss the cache rather than replay a different issue's verdict.
export const LINKED_ISSUE_SATISFACTION_CACHE_INPUT_VERSION = "linked-issue-satisfaction-input:v1";
// #linked-issue-satisfaction-cache: mirrors ai-slop-cache-input.ts's fingerprint discipline, but includes
// every prompt input that is not already represented by the row key. The row key handles repo/pull/head SHA
// and primary linked issue number; this fingerprint handles reviewer configuration plus mutable GitHub text
// (issue title/body and PR title/body) so edits cannot replay a verdict for an older prompt. Diff text is
// included defensively too, keeping the cache tied to exactly the model prompt that produced the opinion.
export const LINKED_ISSUE_SATISFACTION_CACHE_INPUT_VERSION = "linked-issue-satisfaction-input:v2";

export type LinkedIssueSatisfactionCacheInput = {
byok: boolean;
provider: string | null | undefined;
model: string | null | undefined;
issueText?: string | null | undefined;
prTitle?: string | null | undefined;
prBody?: string | null | undefined;
diff?: string | null | undefined;
};

export async function linkedIssueSatisfactionCacheInputFingerprint(input: LinkedIssueSatisfactionCacheInput): Promise<string> {
Expand All @@ -22,6 +23,10 @@ export async function linkedIssueSatisfactionCacheInputFingerprint(input: Linked
input.byok ? "1" : "0",
input.provider ?? "",
input.model ?? "",
input.issueText ?? "",
input.prTitle ?? "",
input.prBody ?? "",
input.diff ?? "",
].join("|");
return `${LINKED_ISSUE_SATISFACTION_CACHE_INPUT_VERSION}:${await sha256Hex(payload)}`;
}
32 changes: 32 additions & 0 deletions test/unit/linked-issue-satisfaction-cache.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,38 @@ describe("linkedIssueSatisfactionCacheInputFingerprint", () => {
expect(withUndefined).toBe(withNull);
});

it("differs when mutable prompt text changes (regression for stale linked-issue gate cache)", async () => {
const before = await linkedIssueSatisfactionCacheInputFingerprint({
byok: false,
provider: null,
model: null,
issueText: "Need an SSE endpoint",
prTitle: "Add SSE endpoint",
prBody: "Closes the SSE issue",
diff: "+app.get('/stream', sse)",
});
const editedIssue = await linkedIssueSatisfactionCacheInputFingerprint({
byok: false,
provider: null,
model: null,
issueText: "Need a GraphQL subscription",
prTitle: "Add SSE endpoint",
prBody: "Closes the SSE issue",
diff: "+app.get('/stream', sse)",
});
const editedPr = await linkedIssueSatisfactionCacheInputFingerprint({
byok: false,
provider: null,
model: null,
issueText: "Need an SSE endpoint",
prTitle: "Add GraphQL subscription",
prBody: "Closes the GraphQL issue",
diff: "+app.get('/stream', sse)",
});
expect(editedIssue).not.toBe(before);
expect(editedPr).not.toBe(before);
});

it("never collides with the ai_slop_cache fingerprint namespace even for identical inputs", async () => {
const { aiSlopCacheInputFingerprint } = await import("../../src/review/ai-slop-cache-input");
const slop = await aiSlopCacheInputFingerprint({ byok: false, provider: null, model: null });
Expand Down
45 changes: 37 additions & 8 deletions test/unit/linked-issue-satisfaction-run.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { afterEach, describe, expect, it, vi } from "vitest";
import { runGittensoryLinkedIssueSatisfaction, type LinkedIssueSatisfactionRunInput } from "../../src/services/linked-issue-satisfaction-run";
import { processJob, runLinkedIssueSatisfactionForAdvisory } from "../../src/queue/processors";
import { buildAiReviewDiff, processJob, runLinkedIssueSatisfactionForAdvisory } from "../../src/queue/processors";
import { evaluateGateCheck } from "../../src/rules/advisory";
import {
getCachedLinkedIssueSatisfaction,
Expand Down Expand Up @@ -294,6 +294,9 @@ describe("runLinkedIssueSatisfactionForAdvisory (processor wiring, #1961/#3906)"
{ repoFullName: "acme/widgets", pullNumber: 7, path: "src/a.ts", status: "modified", additions: 40, deletions: 2, changes: 42, payload: { patch: "@@\n+app.get('/stream', sse);" } },
];
const pr = { number: 7, title: "Add SSE stream endpoint", body: "Implements the requested SSE stream.", linkedIssues: [1275] };
const issueText = "Enrich SN74 Gittensor — add SSE stream\n\nWe need a live SSE stream surface for SN74 Gittensor.";
const processorFingerprint = () =>
linkedIssueSatisfactionCacheInputFingerprint({ byok: false, provider: null, model: null, issueText, prTitle: pr.title, prBody: pr.body, diff: buildAiReviewDiff(files) });
const advisoryMode = { linkedIssueSatisfactionGateMode: "advisory", aiReviewByok: false } as RepositorySettings;
const blockMode = { linkedIssueSatisfactionGateMode: "block", aiReviewByok: false } as RepositorySettings;

Expand Down Expand Up @@ -532,7 +535,7 @@ describe("runLinkedIssueSatisfactionForAdvisory (processor wiring, #1961/#3906)"
stubIssueFetch();
const run = vi.fn(async () => ({ response: satisfactionJson({ status: "unaddressed", confidence: 0.9 }) }));
const env = enabledEnv(run);
const fingerprint = await linkedIssueSatisfactionCacheInputFingerprint({ byok: false, provider: null, model: null });
const fingerprint = await processorFingerprint();
await putCachedLinkedIssueSatisfaction(env, "acme/widgets", 7, "sha7", 1275, fingerprint, {
status: "ok",
result: { status: "addressed", rationale: "cached: looks done", confidence: 0.8 },
Expand All @@ -548,7 +551,7 @@ describe("runLinkedIssueSatisfactionForAdvisory (processor wiring, #1961/#3906)"
stubIssueFetch();
const run = vi.fn();
const env = enabledEnv(run);
const fingerprint = await linkedIssueSatisfactionCacheInputFingerprint({ byok: false, provider: null, model: null });
const fingerprint = await processorFingerprint();
await putCachedLinkedIssueSatisfaction(env, "acme/widgets", 7, "sha7", 1275, fingerprint, {
status: "ok",
result: { status: "addressed", rationale: "cached: looks done", confidence: 0.8 },
Expand Down Expand Up @@ -589,7 +592,7 @@ describe("runLinkedIssueSatisfactionForAdvisory (processor wiring, #1961/#3906)"
await runLinkedIssueSatisfactionForAdvisory(env, { settings: advisoryMode, advisory: adv, repoFullName: "acme/widgets", pr, author: "alice", files, confirmedContributor: true, installationId: 1 });
expect(run).toHaveBeenCalledTimes(1);

const fingerprint = await linkedIssueSatisfactionCacheInputFingerprint({ byok: false, provider: null, model: null });
const fingerprint = await processorFingerprint();
const cached = await getCachedLinkedIssueSatisfaction(env, "acme/widgets", 7, "sha7", 1275, fingerprint);
expect(cached).toMatchObject({ status: "ok", result: { status: "addressed" } });

Expand All @@ -598,6 +601,27 @@ describe("runLinkedIssueSatisfactionForAdvisory (processor wiring, #1961/#3906)"
expect(run).toHaveBeenCalledTimes(1); // still 1 — second pass was a cache hit
});

it("misses the cache when editable issue or PR text changes at the same head SHA", async () => {
stubIssueFetch({ body: "We need a live SSE stream surface for SN74 Gittensor." });
const run = vi.fn(async () => ({ response: satisfactionJson({ status: "addressed", rationale: "The SSE endpoint satisfies the original ask." }) }));
const env = enabledEnv(run);
const adv = advisory();
await expect(
runLinkedIssueSatisfactionForAdvisory(env, { settings: blockMode, advisory: adv, repoFullName: "acme/widgets", pr, author: "alice", files, confirmedContributor: true, installationId: 1 }),
).resolves.toMatchObject({ status: "addressed" });

stubIssueFetch({ body: "We now need a GraphQL subscription instead of an SSE stream." });
run.mockResolvedValueOnce({ response: satisfactionJson({ status: "unaddressed", confidence: 0.9, rationale: "The changed issue asks for GraphQL, but the diff still adds SSE." }) });
const changedAdvisory = advisory();
const changedPr = { ...pr, title: "Add SSE endpoint for the old issue", body: "Still only implements SSE." };
await expect(
runLinkedIssueSatisfactionForAdvisory(env, { settings: blockMode, advisory: changedAdvisory, repoFullName: "acme/widgets", pr: changedPr, author: "alice", files, confirmedContributor: true, installationId: 1 }),
).resolves.toMatchObject({ status: "unaddressed" });

expect(run).toHaveBeenCalledTimes(2);
expect(changedAdvisory.findings).toContainEqual(expect.objectContaining({ code: "linked_issue_scope_mismatch" }));
});

it("misses the cache when the PR's primary linked issue number changes, even at the same head SHA", async () => {
stubIssueFetch();
const run = vi.fn(async () => ({ response: satisfactionJson({ status: "addressed" }) }));
Expand Down Expand Up @@ -785,10 +809,15 @@ describe("linked-issue satisfaction wired end-to-end through the real webhook pi
expect(gatePatchBody.conclusion).toBe("failure");
expect(gatePatchBody.output?.title).toContain("Linked issue does not appear to be satisfied");

// The assessment was cached under the PR's primary linked issue number.
const fingerprint = await linkedIssueSatisfactionCacheInputFingerprint({ byok: false, provider: null, model: null });
const cached = await getCachedLinkedIssueSatisfaction(env, "JSONbored/metagraphed", 3910, "realvenus3910", 1275, fingerprint);
expect(cached).toMatchObject({ status: "ok", result: { status: "unaddressed" } });
// The assessment was cached under the PR's primary linked issue number. The fingerprint itself includes
// prompt text, which this end-to-end test does not need to reconstruct from the webhook pipeline.
const cached = await env.DB.prepare(
"SELECT status, result_json AS resultJson FROM linked_issue_satisfaction_cache WHERE repo_full_name = ? AND pull_number = ? AND head_sha = ? AND linked_issue_number = ?",
)
.bind("JSONbored/metagraphed", 3910, "realvenus3910", 1275)
.first<{ status: string; resultJson: string }>();
expect(cached?.status).toBe("ok");
expect(JSON.parse(cached?.resultJson ?? "{}")?.status).toBe("unaddressed");
});

it("OFF mode (default): no fetch, no model spend, no cache row, and the comment never mentions linked-issue satisfaction at all — byte-identical to before this feature existed", async () => {
Expand Down