diff --git a/src/queue/processors.ts b/src/queue/processors.ts index ffbb345d69..98dc62d369 100644 --- a/src/queue/processors.ts +++ b/src/queue/processors.ts @@ -5271,6 +5271,7 @@ export async function runAiReviewForAdvisory( files.map((file) => file.path), ), repoInstructions: args.reviewInstructions ?? null, + changedFiles: files, }); if (result.status !== "ok") return undefined; const findings: AdvisoryFinding[] = []; diff --git a/src/services/ai-review.ts b/src/services/ai-review.ts index 09b5dd274b..bca2cca2ae 100644 --- a/src/services/ai-review.ts +++ b/src/services/ai-review.ts @@ -28,6 +28,8 @@ import { labelSelfHostReviewerModels, labelSelfHostReviewerNames, resolveConfigu import { incr } from "../selfhost/metrics"; import { errorMessage } from "../utils/json"; import type { ReviewProfile } from "../signals/focus-manifest"; +import { isCodeFile } from "../signals/local-branch"; +import { isTestPath } from "../signals/test-evidence"; /** * The best free Workers-AI model pair for review accuracy — two different families for independence, @@ -178,6 +180,14 @@ export type GittensoryAiReviewInput = { * (the default) ⇒ no instruction is appended, so the prompt is byte-identical and the model emits none. */ inlineFindings?: boolean | undefined; + /** + * This PR's changed file paths (#2558) — reused to splice a concise "changed code files with zero + * test-path evidence" section into the user prompt via the engine's own deterministic classifier + * (src/signals/test-evidence.ts), so the reviewer can name specific untested files instead of guessing + * from the raw diff. Additional CONTEXT only, never a new blocker/nit rule. Absent/empty, or when the PR + * has ANY test-path changes ⇒ no section is appended (byte-identical to today). + */ + changedFiles?: ReadonlyArray<{ path: string }> | null | undefined; }; /** A consensus critical defect, already public-safe, ready to become a gate blocker finding. */ @@ -491,9 +501,28 @@ function buildUserPrompt(input: GittensoryAiReviewInput): string { // GITTENSORY_REVIEW_ENRICHMENT on AND REES_URL set). Absent/empty (the default) → the prompt is byte-identical. const enrichmentSection = input.enrichment?.promptSection; if (enrichmentSection) lines.push("", enrichmentSection); + // Test-evidence classifier (#2558): ground the reviewer's test-adequacy judgment in the engine's own + // deterministic classification instead of eyeballing the diff. Absent/no changed code files without test + // evidence ⇒ the prompt is byte-identical. + const testEvidenceSection = buildTestEvidencePromptSection(input.changedFiles ?? []); + if (testEvidenceSection) lines.push("", testEvidenceSection); return lines.join("\n"); } +/** + * A concise "changed code files with zero test-path evidence" section for the user prompt (#2558). Reuses the + * existing deterministic classifiers (isCodeFile, isTestPath) — no new signal, this is a wiring gap only. + * Mirrors slop.ts's buildMissingTestEvidenceFinding's whole-PR semantics: ANY changed path that already looks + * like a test file means there IS test evidence for this PR, so nothing is called out (a partial-but-real test + * change is not "zero evidence") — only a fully test-free PR touching real code files gets a section. + */ +export function buildTestEvidencePromptSection(files: ReadonlyArray<{ path: string }>): string | undefined { + const codePaths = [...new Set(files.map((file) => file.path).filter(Boolean).filter(isCodeFile))]; + if (codePaths.length === 0) return undefined; + if (files.some((file) => isTestPath(file.path))) return undefined; + return `Test evidence (engine classifier): this PR has NO test-path changes. The following changed code file(s) have zero test-path evidence: ${codePaths.join(", ")}.`; +} + // `.gittensory.yml` review.profile → an appended tone instruction (#review-profile). `balanced`/absent appends // nothing (byte-identical). PRESENTATION ONLY: it shapes how many nits the write-up surfaces, never the verdict. const REVIEW_PROFILE_SUFFIX: Record<"chill" | "assertive", string> = { diff --git a/test/unit/ai-review.test.ts b/test/unit/ai-review.test.ts index 1d5b083117..5273177154 100644 --- a/test/unit/ai-review.test.ts +++ b/test/unit/ai-review.test.ts @@ -2,6 +2,7 @@ import { afterEach, describe, expect, it, vi } from "vitest"; import { __aiReviewInternals, BEST_REVIEW_MODELS, + buildTestEvidencePromptSection, runGittensoryAiReview, type GittensoryAiReviewInput, } from "../../src/services/ai-review"; @@ -1784,4 +1785,119 @@ describe("pure helpers", () => { expect(user).toContain("## EXTERNAL REVIEW BRIEF"); expect(system).toContain("untrusted advisory context"); }); + + it("splices the test-evidence classifier section into the user prompt when changed code files have zero test-path evidence (#2558)", async () => { + const run = vi.fn( + async ( + _model: string, + _options: { messages: Array<{ content: string }> }, + ) => ({ response: reviewJson() }), + ); + const env = createTestEnv({ + AI: { run } as unknown as Ai, + AI_SUMMARIES_ENABLED: "true", + AI_PUBLIC_COMMENTS_ENABLED: "true", + AI_DAILY_NEURON_BUDGET: "100000", + }); + const result = await runGittensoryAiReview(env, { + ...baseInput, + changedFiles: [{ path: "src/a.ts" }, { path: "src/b.ts" }], + }); + expect(result.status).toBe("ok"); + const opts = run.mock.calls[0]?.[1] as { + messages: Array<{ role?: string; content: string }>; + }; + const user = + opts.messages.find((m) => m.role === "user")?.content ?? + String(opts.messages[1]?.content); + expect(user).toContain("Test evidence (engine classifier)"); + expect(user).toContain("src/a.ts"); + expect(user).toContain("src/b.ts"); + }); + + it("does NOT splice a test-evidence section when the PR includes a test-path change (#2558)", async () => { + const run = vi.fn( + async ( + _model: string, + _options: { messages: Array<{ content: string }> }, + ) => ({ response: reviewJson() }), + ); + const env = createTestEnv({ + AI: { run } as unknown as Ai, + AI_SUMMARIES_ENABLED: "true", + AI_PUBLIC_COMMENTS_ENABLED: "true", + AI_DAILY_NEURON_BUDGET: "100000", + }); + const result = await runGittensoryAiReview(env, { + ...baseInput, + changedFiles: [{ path: "src/a.ts" }, { path: "test/unit/a.test.ts" }], + }); + expect(result.status).toBe("ok"); + const opts = run.mock.calls[0]?.[1] as { + messages: Array<{ role?: string; content: string }>; + }; + const user = + opts.messages.find((m) => m.role === "user")?.content ?? + String(opts.messages[1]?.content); + expect(user).not.toContain("Test evidence (engine classifier)"); + }); + + it("does NOT splice a test-evidence section when changedFiles is absent (byte-identical to today)", async () => { + const run = vi.fn( + async ( + _model: string, + _options: { messages: Array<{ content: string }> }, + ) => ({ response: reviewJson() }), + ); + const env = createTestEnv({ + AI: { run } as unknown as Ai, + AI_SUMMARIES_ENABLED: "true", + AI_PUBLIC_COMMENTS_ENABLED: "true", + AI_DAILY_NEURON_BUDGET: "100000", + }); + const result = await runGittensoryAiReview(env, baseInput); + expect(result.status).toBe("ok"); + const opts = run.mock.calls[0]?.[1] as { + messages: Array<{ role?: string; content: string }>; + }; + const user = + opts.messages.find((m) => m.role === "user")?.content ?? + String(opts.messages[1]?.content); + expect(user).not.toContain("Test evidence (engine classifier)"); + }); +}); + +describe("buildTestEvidencePromptSection (#2558)", () => { + it("returns undefined when there are no changed code files", () => { + expect(buildTestEvidencePromptSection([])).toBeUndefined(); + expect(buildTestEvidencePromptSection([{ path: "README.md" }])).toBeUndefined(); + }); + + it("lists changed code files with zero test-path evidence", () => { + const section = buildTestEvidencePromptSection([ + { path: "src/a.ts" }, + { path: "src/b.ts" }, + { path: "README.md" }, + ]); + expect(section).toContain("src/a.ts"); + expect(section).toContain("src/b.ts"); + expect(section).not.toContain("README.md"); + }); + + it("returns undefined when ANY changed path already looks like a test file", () => { + expect( + buildTestEvidencePromptSection([ + { path: "src/a.ts" }, + { path: "test/unit/a.test.ts" }, + ]), + ).toBeUndefined(); + }); + + it("de-duplicates a repeated file path so the section doesn't get noisier than the actual changed-file set", () => { + const section = buildTestEvidencePromptSection([ + { path: "src/a.ts" }, + { path: "src/a.ts" }, + ]); + expect(section?.match(/src\/a\.ts/g)).toHaveLength(1); + }); });