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
74 changes: 71 additions & 3 deletions src/services/ai-review.ts
Original file line number Diff line number Diff line change
Expand Up @@ -789,6 +789,43 @@ export function demoteCiClaimBlockers(review: ModelReview): { review: ModelRevie
};
}

/** The prompt's PR-description window (buildUserPrompt slices the body here). Exported so the truncation
* FACT and the evidence-absence demotion below stay pinned to the same number the prompt actually uses. */
export const PR_BODY_PROMPT_LIMIT = 2000;

/** #8961: whether a PR body carries image/video attachments is a deterministic FACT — count it from the
* FULL body (markdown images, GitHub user-attachments links, <img> tags, bare media URLs), never let a
* model infer it from a truncated window. PURE. */
export function countBodyAttachments(body: string): number {
const matches = body.match(/!\[[^\]]*\]\([^)]+\)|github\.com\/user-attachments\/|<img\s|https?:\/\/\S+\.(?:png|jpe?g|gif|webp|mp4|mov)\b/gi);
return matches ? matches.length : 0;
}

/** An evidence-ABSENCE claim about visual proof (screenshots / recordings / before-after), in either
* direction ("no screenshots provided" / "screenshots are missing"). Deliberately scoped to visual
* evidence: scope or issue-text absence claims are judgments the model may still make. */
export const EVIDENCE_ABSENCE_PATTERN =
/\b(?:no|missing|absent|lacks?|lacking|without|not\s+(?:provided|included|attached|supplied|confirmed)|none\s+(?:provided|included|attached|supplied)|cannot\s+confirm|fails?\s+to\s+(?:provide|include|attach))\b[^.]{0,80}\b(?:screenshots?|screen\s+recordings?|before\/after|image\s+evidence|visual\s+evidence)\b|\b(?:screenshots?|screen\s+recordings?|visual\s+evidence)\b[^.]{0,80}\b(?:missing|absent|not\s+provided|none|lacking|omitted)\b/i;

/** #8961: when the description was TRUNCATED for the prompt, absence of visual evidence inside the window
* is not evidence of absence — the 2026-07-26 decision audit confirmed a production close on a PR whose 6
* screenshots sat beyond the cut. Deterministically demote such blockers to nits, parse-time, mirroring
* demoteCiClaimBlockers (the prompt carries the attachment fact; this guarantees it). Untruncated bodies
* are untouched — there the model saw everything and the claim is a legitimate judgment. PURE. */
export function demoteEvidenceAbsenceBlockers(review: ModelReview, bodyTruncated: boolean): { review: ModelReview; demoted: string[] } {
if (!bodyTruncated) return { review, demoted: [] };
const demoted = review.blockers.filter((blocker) => EVIDENCE_ABSENCE_PATTERN.test(blocker));
if (demoted.length === 0) return { review, demoted };
return {
review: {
...review,
blockers: review.blockers.filter((blocker) => !EVIDENCE_ABSENCE_PATTERN.test(blocker)),
nits: [...review.nits, ...demoted.map((claim) => `${claim} (demoted: the PR description was truncated for review — absence of evidence inside the truncated window is not evidence of absence)`)],
},
demoted,
};
}

/** Parse a model's JSON review into a normalized {@link ModelReview}, or null when unparseable. */
export function parseModelReview(text: string): ModelReview | null {
const jsonText = extractLastJsonObject(text);
Expand Down Expand Up @@ -983,8 +1020,13 @@ function buildUserPrompt(input: LoopOverAiReviewInput): string {
const lines = [
`Repository: ${input.repoFullName}`,
`Pull request #${input.prNumber}: ${input.title}`,
// #8961: when the body exceeds the window, say so and carry the attachment COUNT as a structured fact
// computed from the FULL body — a reviewer must never conclude required visual evidence is absent just
// because the truncation point fell before a Screenshots section (confirmed production failure class).
input.body
? `Description:\n${input.body.slice(0, 2000)}`
? input.body.length > PR_BODY_PROMPT_LIMIT
? `Description (TRUNCATED at ${PR_BODY_PROMPT_LIMIT} chars — the FULL body contains ${countBodyAttachments(input.body)} image/video attachment(s) beyond what you can see; NEVER claim screenshots or visual evidence are missing):\n${input.body.slice(0, PR_BODY_PROMPT_LIMIT)}`
: `Description:\n${input.body}`
: "Description: (none)",
"",
"Unified diff (truncated if large):",
Expand Down Expand Up @@ -1250,6 +1292,8 @@ async function runWorkersOpinion(
// wiring a real caller (source images, invoke with them) is a deliberately deferred follow-up; see
// review/visual/visual-findings.ts.
images?: readonly AiContentBlock[] | undefined,
// #8961: true when the PR description exceeded the prompt window — arms the evidence-absence demotion.
bodyTruncated = false,
): Promise<ReviewerOpinionOutcome> {
const ai = env.AI as unknown as AiRunner | undefined;
if (!ai || typeof ai.run !== "function") return { review: null };
Expand Down Expand Up @@ -1355,10 +1399,15 @@ async function runWorkersOpinion(
const parsedRaw = parseModelReview(text);
// #8833: enforce the CI-adjudication ban at parse time — the prompt REQUESTS it, this guarantees it.
const demotion = parsedRaw ? demoteCiClaimBlockers(parsedRaw) : null;
const parsed = demotion?.review ?? null;
// #8961: same guarantee for evidence-absence claims against a truncated description.
const evidenceDemotion = demotion ? demoteEvidenceAbsenceBlockers(demotion.review, bodyTruncated) : null;
const parsed = evidenceDemotion?.review ?? null;
if (demotion && demotion.demoted.length > 0) {
console.warn(JSON.stringify({ level: "warn", event: "ai_review_ci_claim_demoted", model, count: demotion.demoted.length }));
}
if (evidenceDemotion && evidenceDemotion.demoted.length > 0) {
console.warn(JSON.stringify({ level: "warn", event: "ai_review_evidence_absence_demoted", model, count: evidenceDemotion.demoted.length }));
}
if (parsed && parsed.assessment.trim() !== "") {
diagnostics.push({ model, attempt, status: "parsed", responseChars: text.length, hasJsonObject: Boolean(extractLastJsonObject(text)), ...usageFields });
return { review: parsed };
Expand Down Expand Up @@ -1675,6 +1724,7 @@ async function runProviderReview(
user: string,
maxTokens: number,
images?: readonly AiContentBlock[] | undefined,
bodyTruncated = false, // #8961: arms the evidence-absence demotion, same contract as runWorkersOpinion
): Promise<ProviderReviewOutcome> {
const { text, usage, failure } = await callAiProvider(
providerKey,
Expand All @@ -1693,7 +1743,12 @@ async function runProviderReview(
if (providerDemotion && providerDemotion.demoted.length > 0) {
console.warn(JSON.stringify({ level: "warn", event: "ai_review_ci_claim_demoted", provider: providerKey.provider, count: providerDemotion.demoted.length }));
}
const review = providerDemotion?.review ?? null;
// #8961: same evidence-absence enforcement as the Workers path.
const providerEvidenceDemotion = providerDemotion ? demoteEvidenceAbsenceBlockers(providerDemotion.review, bodyTruncated) : null;
if (providerEvidenceDemotion && providerEvidenceDemotion.demoted.length > 0) {
console.warn(JSON.stringify({ level: "warn", event: "ai_review_evidence_absence_demoted", provider: providerKey.provider, count: providerEvidenceDemotion.demoted.length }));
}
const review = providerEvidenceDemotion?.review ?? null;
return {
review,
diagnostic: {
Expand Down Expand Up @@ -2374,6 +2429,9 @@ export async function runLoopOverAiReview(
? { ...input, ...defangReviewInput(input) }
: input;
const user = buildUserPrompt(promptInput);
// #8961: pinned to the SAME body buildUserPrompt just sliced, so the demotion can never disagree with
// the prompt about whether the description was cut.
const bodyTruncated = (promptInput.body?.length ?? 0) > PR_BODY_PROMPT_LIMIT;
// Grounding-discipline SYSTEM suffix (convergence, flag-gated). When the caller supplied grounding, the
// reviewers are told to verify claims against the attached CI/files; otherwise this is REVIEW_SYSTEM_PROMPT
// unchanged (byte-identical). Computed from `promptInput` so it travels with the (possibly defanged) input.
Expand Down Expand Up @@ -2519,6 +2577,8 @@ export async function runLoopOverAiReview(
system,
user,
maxTokens,
undefined,
bodyTruncated,
);
advisoryReview = outcome.review;
byokFailure = outcome.failure;
Expand All @@ -2535,6 +2595,8 @@ export async function runLoopOverAiReview(
reviewDiagnostics,
repoInstructionsSystemAppend,
aiRunCorrelation,
undefined,
bodyTruncated,
);
advisoryReview = outcome.review;
if (outcome.fallbackNote) fallbackNotes.push(outcome.fallbackNote);
Expand Down Expand Up @@ -2563,6 +2625,8 @@ export async function runLoopOverAiReview(
reviewDiagnostics,
repoInstructionsSystemAppend,
aiRunCorrelation,
undefined,
bodyTruncated,
)
: Promise.resolve<ReviewerOpinionOutcome>({ review: advisoryReview }),
runWorkersOpinion(
Expand All @@ -2575,6 +2639,8 @@ export async function runLoopOverAiReview(
reviewDiagnostics,
repoInstructionsSystemAppend,
aiRunCorrelation,
undefined,
bodyTruncated,
),
]);
if (a.fallbackNote) fallbackNotes.push(a.fallbackNote);
Expand Down Expand Up @@ -2639,6 +2705,8 @@ export async function runLoopOverAiReview(
reviewDiagnostics,
repoInstructionsSystemAppend,
aiRunCorrelation,
undefined,
bodyTruncated,
)
: ({ review: advisoryReview } as ReviewerOpinionOutcome);
if (a.fallbackNote) fallbackNotes.push(a.fallbackNote);
Expand Down
72 changes: 68 additions & 4 deletions test/unit/ai-review.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -262,7 +262,7 @@ describe("runLoopOverAiReview gating", () => {
content: [
{
type: "text",
text: '{"assessment":"provider view","blockers":["The tests are failing on CI","Race in src/lock.ts"],"nits":[],"suggestions":[]}',
text: '{"assessment":"provider view","blockers":["The tests are failing on CI","No before/after screenshots provided for this visual change","Race in src/lock.ts"],"nits":[],"suggestions":[]}',
},
],
}),
Expand All @@ -277,7 +277,8 @@ describe("runLoopOverAiReview gating", () => {
AI_DAILY_NEURON_BUDGET: "1",
AI_BYOK_DAILY_REPO_LIMIT: "5",
});
const result = await runLoopOverAiReview(env, { ...baseInput, providerKey: { provider: "anthropic", key: "sk-ant-secret" } });
// #8961: a body past the prompt window arms the evidence-absence demotion on the provider path too.
const result = await runLoopOverAiReview(env, { ...baseInput, body: `${"y".repeat(2100)}\n![after](https://x.io/1.png)`, providerKey: { provider: "anthropic", key: "sk-ant-secret" } });
expect(result.status).toBe("ok");
if (result.status === "ok") {
// Single-provider BYOK is advisory-only (no consensus defect) — the observable contract is the
Expand All @@ -286,11 +287,14 @@ describe("runLoopOverAiReview gating", () => {
const blockersSection = notes.split("**Nits")[0] ?? notes;
expect(blockersSection).toContain("Race in src/lock.ts");
expect(blockersSection).not.toContain("tests are failing");
expect(blockersSection).not.toContain("screenshots"); // #8961: demoted, never a blocker on a truncated body
expect(notes).toContain("decided deterministically");
expect(notes).toContain("absence of evidence inside the truncated window");
}
// The demotion arm executed on the provider path (its log fired) — the strong behavioral assertions live
// in the pure demoteCiClaimBlockers tests and the workers-path test above.
// The demotion arms executed on the provider path (their logs fired) — the strong behavioral assertions
// live in the pure demotion tests and the workers-path tests above.
expect(warn.mock.calls.some(([line]) => String(line).includes("ai_review_ci_claim_demoted"))).toBe(true);
expect(warn.mock.calls.some(([line]) => String(line).includes("ai_review_evidence_absence_demoted"))).toBe(true);
warn.mockRestore();
});

Expand Down Expand Up @@ -3249,6 +3253,22 @@ describe("pure helpers", () => {
warn.mockRestore();
});

it("#8961: runWorkersOpinion demotes an evidence-absence blocker ONLY when the body was truncated", async () => {
const warn = vi.spyOn(console, "warn").mockImplementation(() => undefined);
const run = vi.fn(async () => ({
response: '{"assessment":"looks off","blockers":["No before/after screenshots provided for this visual change","Null deref in src/a.ts"],"nits":[],"suggestions":[]}',
}));
const env = createTestEnv({ AI: { run } as unknown as Ai });
const truncated = await runWorkersOpinion(env, "@cf/x/model", "@cf/x/model", "sys", "user", 256, [], "", undefined, undefined, true);
expect(truncated.review?.blockers).toEqual(["Null deref in src/a.ts"]);
expect(truncated.review?.nits.some((nit) => nit.includes("absence of evidence inside the truncated window"))).toBe(true);
expect(warn.mock.calls.some(([line]) => String(line).includes("ai_review_evidence_absence_demoted"))).toBe(true);
// Untruncated body: the model saw everything — the same claim is a legitimate judgment and stays a blocker.
const full = await runWorkersOpinion(env, "@cf/x/model", "@cf/x/model", "sys", "user", 256);
expect(full.review?.blockers).toContain("No before/after screenshots provided for this visual change");
warn.mockRestore();
});

it("REGRESSION (#4111): runWorkersOpinion attaches supplied images to the user message; omits them (plain string) when absent", async () => {
const seenContents: unknown[] = [];
const run = vi.fn(async (_model: string, options: Record<string, unknown>) => {
Expand Down Expand Up @@ -4844,6 +4864,50 @@ describe("#8833: enforced boundaries between model judgment and deterministic fa
expect(demoteCiClaimBlockers(untouched).review).toBe(untouched);
});

it("#8961: attachment counting is a deterministic fact — markdown images, user-attachments links, img tags, bare media URLs", async () => {
const { countBodyAttachments } = await import("../../src/services/ai-review");
expect(countBodyAttachments("plain prose, no media")).toBe(0);
expect(countBodyAttachments("![before](https://example.com/a.png)")).toBe(1);
expect(countBodyAttachments("see https://github.com/user-attachments/assets/abc-123")).toBe(1);
expect(countBodyAttachments('<img src="x" width="400"> and https://cdn.example.com/demo.mp4')).toBe(2);
expect(countBodyAttachments("![a](u1) ![b](u2)\nhttps://x.io/shot.jpeg")).toBe(3);
});

it("#8961: evidence-absence blockers demote ONLY under a truncated body; other blockers and directions both match", async () => {
const { demoteEvidenceAbsenceBlockers, EVIDENCE_ABSENCE_PATTERN } = await import("../../src/services/ai-review");
const claims = ["No before/after screenshots are provided for this visual change", "Screenshots are missing for the rendered change", "The added check silently swallows the failure"];
// Untruncated: untouched, same object (the model saw the whole body — its claim is a judgment).
const untouched = review(claims);
expect(demoteEvidenceAbsenceBlockers(untouched, false).review).toBe(untouched);
// Truncated: both phrasing directions demote; the code-content blocker survives.
const { review: out, demoted } = demoteEvidenceAbsenceBlockers(review(claims), true);
expect(demoted).toHaveLength(2);
expect(out.blockers).toEqual(["The added check silently swallows the failure"]);
expect(out.nits.filter((nit: string) => nit.includes("absence of evidence inside the truncated window"))).toHaveLength(2);
// Truncated but no evidence claims: zero-demotion returns the same object.
const clean = review(["Null deref in src/a.ts"]);
expect(demoteEvidenceAbsenceBlockers(clean, true).review).toBe(clean);
for (const positive of ["cannot confirm before/after screenshot evidence", "fails to provide screen recordings", "visual evidence is omitted"]) {
expect(EVIDENCE_ABSENCE_PATTERN.test(positive)).toBe(true);
}
expect(EVIDENCE_ABSENCE_PATTERN.test("Missing test coverage for the error branch")).toBe(false);
});

it("#8961: the prompt carries the truncation + attachment FACT for a long body, and stays plain otherwise", async () => {
const { PR_BODY_PROMPT_LIMIT } = await import("../../src/services/ai-review");
const images = "![a](https://x.io/1.png) ![b](https://x.io/2.png)";
const longBody = "y".repeat(PR_BODY_PROMPT_LIMIT + 10) + "\n## Screenshots\n" + images;
const long = buildUserPrompt({ repoFullName: "o/r", prNumber: 1, title: "t", body: longBody, diff: "d", actor: "a", mode: "advisory" } as never);
expect(long).toContain(`TRUNCATED at ${PR_BODY_PROMPT_LIMIT}`);
expect(long).toContain("2 image/video attachment(s)");
expect(long).toContain("NEVER claim screenshots or visual evidence are missing");
const short = buildUserPrompt({ repoFullName: "o/r", prNumber: 1, title: "t", body: `hello ${images}`, diff: "d", actor: "a", mode: "advisory" } as never);
expect(short).toContain("Description:\nhello");
expect(short).not.toContain("TRUNCATED");
const bodiless = buildUserPrompt({ repoFullName: "o/r", prNumber: 1, title: "t", body: "", diff: "d", actor: "a", mode: "advisory" } as never);
expect(bodiless).toContain("Description: (none)");
});

it("silence is not certainty: an unstated/garbage confidence parses to CONFIDENCE_WHEN_UNSTATED, a stated one is honored", () => {
expect(parseReviewConfidence(undefined)).toBe(CONFIDENCE_WHEN_UNSTATED);
expect(parseReviewConfidence("very sure")).toBe(CONFIDENCE_WHEN_UNSTATED);
Expand Down
Loading