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
1 change: 1 addition & 0 deletions src/queue/processors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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[] = [];
Expand Down
29 changes: 29 additions & 0 deletions src/services/ai-review.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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. */
Expand Down Expand Up @@ -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> = {
Expand Down
116 changes: 116 additions & 0 deletions test/unit/ai-review.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -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);
});
});
Loading