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
7 changes: 7 additions & 0 deletions src/queue/processors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3495,6 +3495,10 @@ export async function runAiReviewForAdvisory(
// cached manifest. The CONFIG (not a fetch) is threaded in; the per-PR glob match against `files` happens
// here (pure), so the AI path makes no extra manifest fetch. Absent/empty ⇒ byte-identical reviewer prompt.
reviewPathInstructions?: ReviewPathInstruction[] | undefined;
// `.gittensory.yml` review.instructions (#review-instructions): a repo-level maintainer brief, resolved by the
// caller from the cached manifest, handed to the reviewer on EVERY review (bounded + public-safe at parse time).
// Absent/null ⇒ byte-identical reviewer prompt.
reviewInstructions?: string | null | undefined;
// `.gittensory.yml` review.exclude_paths (#review-exclude-paths), resolved by the caller from the cached
// manifest. Globs whose files are dropped from the AI review (diff + grounding + RAG) — generated/lockfiles
// the maintainer doesn't want reviewed. Empty ⇒ every file is reviewed (byte-identical). The gate is unaffected.
Expand Down Expand Up @@ -3677,6 +3681,7 @@ export async function runAiReviewForAdvisory(
args.reviewPathInstructions ?? [],
files.map((file) => file.path),
),
repoInstructions: args.reviewInstructions ?? null,
});
if (result.status !== "ok") return undefined;
const findings: AdvisoryFinding[] = [];
Expand Down Expand Up @@ -4466,6 +4471,7 @@ async function maybePublishPrPublicSurface(
profile: reviewProfile,
inlineComments: reviewInlineComments,
pathInstructions: reviewPathInstructions,
instructions: reviewInstructions,
excludePaths: reviewExcludePaths,
} = resolveReviewPromptOverrides(
await loadRepoFocusManifest(env, repoFullName).catch(() => null),
Expand All @@ -4485,6 +4491,7 @@ async function maybePublishPrPublicSurface(
files: await getReviewFiles(),
reviewProfile,
reviewPathInstructions,
reviewInstructions,
reviewExcludePaths,
reviewInlineComments,
});
Expand Down
13 changes: 12 additions & 1 deletion src/services/ai-review.ts
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,12 @@ export type GittensoryAiReviewInput = {
* instructions passed the manifest's public-safe filter at parse time).
*/
pathGuidance?: string | null | undefined;
/**
* `.gittensory.yml` `review.instructions` (#review-instructions) — a repo-level maintainer brief appended to EVERY
* review (vs the per-path pathGuidance). Bounded + public-safe at parse time, so it stays cost-cheap. Absent/null ⇒
* the reviewer prompt is byte-identical.
*/
repoInstructions?: string | null | undefined;
/**
* `.gittensory.yml` `review.inline_comments` (#inline-comments) — when true (the caller has already ANDed the
* operator flag + cutover allowlist + the per-repo manifest toggle), the reviewer is asked to ALSO emit an
Expand Down Expand Up @@ -455,8 +461,13 @@ function buildSystemPrompt(input: GittensoryAiReviewInput): string {
// `.gittensory.yml` review.path_instructions (#review-path-instructions): the caller pre-resolved the entries
// matching this PR's files into a prompt section; empty ⇒ nothing appended (byte-identical).
const pathSuffix = input.pathGuidance?.trim() ? input.pathGuidance : "";
// `.gittensory.yml` review.instructions (#review-instructions): a repo-level maintainer brief appended to every
// review; empty ⇒ nothing appended (byte-identical).
const repoInstructionsSuffix = input.repoInstructions?.trim()
? ` REPOSITORY REVIEW INSTRUCTIONS (maintainer conventions for this repo — honor them unless they conflict with a real defect): ${input.repoInstructions.trim()}`
: "";
const inlineSuffix = input.inlineFindings ? INLINE_FINDINGS_SUFFIX : "";
return `${REVIEW_SYSTEM_PROMPT}${groundingSuffix}${enrichmentSuffix}${profileSuffix}${pathSuffix}${inlineSuffix}`;
return `${REVIEW_SYSTEM_PROMPT}${groundingSuffix}${enrichmentSuffix}${profileSuffix}${pathSuffix}${repoInstructionsSuffix}${inlineSuffix}`;
}

/** One Workers-AI opinion with a per-slot reliable fallback and a 3× retry on the primary. */
Expand Down
18 changes: 13 additions & 5 deletions src/signals/focus-manifest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,11 @@ export type FocusManifestReviewConfig = {
/** `review.path_instructions`: per-path natural-language guidance handed to the AI reviewer when the PR's
* changed files match the glob. Empty (default) ⇒ byte-identical reviewer prompt. (#review-path-instructions) */
pathInstructions: ReviewPathInstruction[];
/** `review.instructions`: a repo-level natural-language brief handed to the AI reviewer on EVERY review (vs the
* per-path path_instructions) — the maintainer's conventions/voice for this repo. Bounded + public-safe at parse
* time (so it stays cost-cheap, unlike ingesting a whole CLAUDE.md). null (default, absent) ⇒ byte-identical
* reviewer prompt. (#review-instructions) */
instructions: string | null;
/** `review.exclude_paths`: globs whose matching files are EXCLUDED from the AI review (diff + grounding + RAG)
* — generated/vendored/lockfiles the maintainer doesn't want reviewed. Empty (default) ⇒ every file is
* reviewed (byte-identical). Gate/slop/secret-scan are UNAFFECTED — this only narrows the AI review.
Expand Down Expand Up @@ -266,7 +271,7 @@ const EMPTY_MANIFEST: FocusManifest = {
publicNotes: [],
gate: { ...EMPTY_GATE_CONFIG },
settings: {},
review: { present: false, footerText: null, note: null, fields: {}, profile: null, inlineComments: null, pathInstructions: [], excludePaths: [], preMergeChecks: [] },
review: { present: false, footerText: null, note: null, fields: {}, profile: null, inlineComments: null, pathInstructions: [], instructions: null, excludePaths: [], preMergeChecks: [] },
features: { ...EMPTY_FEATURES_CONFIG },
warnings: [],
};
Expand All @@ -280,7 +285,7 @@ export function isFocusManifestPublicSafe(text: string): boolean {
}

function emptyManifest(source: FocusManifestSource, warnings: string[] = []): FocusManifest {
return { ...EMPTY_MANIFEST, source, warnings, gate: { ...EMPTY_GATE_CONFIG }, settings: {}, review: { present: false, footerText: null, note: null, fields: {}, profile: null, inlineComments: null, pathInstructions: [], excludePaths: [], preMergeChecks: [] }, features: { ...EMPTY_FEATURES_CONFIG } };
return { ...EMPTY_MANIFEST, source, warnings, gate: { ...EMPTY_GATE_CONFIG }, settings: {}, review: { present: false, footerText: null, note: null, fields: {}, profile: null, inlineComments: null, pathInstructions: [], instructions: null, excludePaths: [], preMergeChecks: [] }, features: { ...EMPTY_FEATURES_CONFIG } };
}

function normalizeStringList(value: JsonValue | undefined, field: string, warnings: string[]): string[] {
Expand Down Expand Up @@ -603,7 +608,7 @@ function parsePublicSafeText(value: JsonValue | undefined, field: string, warnin
* throws; invalid/unsafe values are dropped with warnings.
*/
function parseReviewConfig(value: JsonValue | undefined, warnings: string[]): FocusManifestReviewConfig {
const empty: FocusManifestReviewConfig = { present: false, footerText: null, note: null, fields: {}, profile: null, inlineComments: null, pathInstructions: [], excludePaths: [], preMergeChecks: [] };
const empty: FocusManifestReviewConfig = { present: false, footerText: null, note: null, fields: {}, profile: null, inlineComments: null, pathInstructions: [], instructions: null, excludePaths: [], preMergeChecks: [] };
if (value === undefined || value === null) return empty;
if (typeof value !== "object" || Array.isArray(value)) {
warnings.push(`Manifest field "review" must be a mapping; ignoring it.`);
Expand All @@ -626,6 +631,7 @@ function parseReviewConfig(value: JsonValue | undefined, warnings: string[]): Fo
const profile = parseReviewProfile(r.profile, warnings);
const inlineComments = normalizeOptionalBoolean(r.inline_comments, "review.inline_comments", warnings);
const pathInstructions = parseReviewPathInstructions(r.path_instructions, warnings);
const instructions = parsePublicSafeText(r.instructions, "review.instructions", warnings);
const excludePaths = parseReviewExcludePaths(r.exclude_paths, warnings);
const preMergeChecks = parseReviewPreMergeChecks(r.pre_merge_checks, warnings);
return {
Expand All @@ -635,6 +641,7 @@ function parseReviewConfig(value: JsonValue | undefined, warnings: string[]): Fo
profile !== null ||
inlineComments !== null ||
pathInstructions.length > 0 ||
instructions !== null ||
excludePaths.length > 0 ||
preMergeChecks.length > 0 ||
Object.keys(fields).length > 0,
Expand All @@ -644,6 +651,7 @@ function parseReviewConfig(value: JsonValue | undefined, warnings: string[]): Fo
profile,
inlineComments,
pathInstructions,
instructions,
excludePaths,
preMergeChecks,
};
Expand Down Expand Up @@ -823,10 +831,10 @@ export function resolveReviewPathInstructions(pathInstructions: ReviewPathInstru
* a possibly-null manifest (null = load failure). A null manifest yields the byte-identical defaults. Centralized
* so the AI-review caller threads them in one place with the null-manifest branch covered here (unit-tested)
* rather than inline in the processor. (#review-profile / #review-path-instructions / #review-exclude-paths) */
export function resolveReviewPromptOverrides(manifest: FocusManifest | null): { profile: ReviewProfile | null; inlineComments: boolean; pathInstructions: ReviewPathInstruction[]; excludePaths: string[] } {
export function resolveReviewPromptOverrides(manifest: FocusManifest | null): { profile: ReviewProfile | null; inlineComments: boolean; pathInstructions: ReviewPathInstruction[]; instructions: string | null; excludePaths: string[] } {
// inlineComments resolves to a strict boolean — true ONLY when the manifest explicitly set review.inline_comments:
// true; null/false/absent ⇒ false. The caller ANDs this per-repo toggle with the operator flag + cutover allowlist.
return { profile: manifest?.review.profile ?? null, inlineComments: manifest?.review.inlineComments === true, pathInstructions: manifest?.review.pathInstructions ?? [], excludePaths: manifest?.review.excludePaths ?? [] };
return { profile: manifest?.review.profile ?? null, inlineComments: manifest?.review.inlineComments === true, pathInstructions: manifest?.review.pathInstructions ?? [], instructions: manifest?.review.instructions ?? null, excludePaths: manifest?.review.excludePaths ?? [] };
}

/** Resolve `review.pre_merge_checks` from a possibly-null manifest (null = load failure ⇒ no checks). Centralized
Expand Down
27 changes: 27 additions & 0 deletions test/unit/ai-review.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -330,6 +330,33 @@ describe("review.profile shapes the reviewer system prompt (#review-profile)", (
);
});

it("repoInstructions (#review-instructions) is appended to the system prompt; absent leaves it byte-identical", async () => {
const systemPromptOf = (run: ReturnType<typeof vi.fn>): string =>
(run.mock.calls[0]?.[1] as { messages?: Array<{ content?: string }> })
?.messages?.[0]?.content ?? "";
const runInstr = async (repoInstructions: string | undefined) => {
const run = vi.fn(async () => ({ 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",
});
await runGittensoryAiReview(env, { ...baseInput, repoInstructions });
return systemPromptOf(run);
};
const withInstr = await runInstr("Follow our async-error conventions.");
expect(withInstr).toContain("REPOSITORY REVIEW INSTRUCTIONS");
expect(withInstr).toContain("async-error conventions");
// Absent or whitespace-only → no append (byte-identical prompt).
expect(await runInstr(undefined)).not.toContain(
"REPOSITORY REVIEW INSTRUCTIONS",
);
expect(await runInstr(" ")).not.toContain(
"REPOSITORY REVIEW INSTRUCTIONS",
);
});

it("the inline-findings instruction is appended to the system prompt ONLY when requested (#inline-comments)", async () => {
const systemPromptOf = (run: ReturnType<typeof vi.fn>): string =>
(run.mock.calls[0]?.[1] as { messages?: Array<{ content?: string }> })
Expand Down
8 changes: 4 additions & 4 deletions test/unit/focus-manifest.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -478,7 +478,7 @@ describe("compileFocusManifestPolicy", () => {
publicNotes: ["Keep PRs focused.", "Maximize your reward payout"],
gate: { present: false, enabled: null, pack: null, linkedIssue: null, duplicates: null, readinessMode: null, readinessMinScore: null, slopMode: null, slopMinScore: null, slopAiAdvisory: null, aiReviewMode: null, aiReviewByok: null, aiReviewProvider: null, aiReviewModel: null, aiReviewAllAuthors: null, mergeReadiness: null, selfAuthoredLinkedIssue: null, manifestPolicy: null, firstTimeContributorGrace: null },
settings: {},
review: { present: false, footerText: null, note: null, fields: {}, profile: null, inlineComments: null, pathInstructions: [], excludePaths: [], preMergeChecks: [] },
review: { present: false, footerText: null, note: null, fields: {}, profile: null, inlineComments: null, pathInstructions: [], instructions: null, excludePaths: [], preMergeChecks: [] },
features: { present: false, rag: null, reputation: null, unifiedComment: null, safety: null },
warnings: [],
});
Expand Down Expand Up @@ -1242,10 +1242,10 @@ describe("resolveReviewPathInstructions (#review-path-instructions)", () => {
});

it("resolveReviewPromptOverrides: non-null manifest passes the config through; null manifest → defaults", () => {
const manifest = parseFocusManifest({ review: { profile: "chill", inline_comments: true, path_instructions: [{ path: "src/**", instructions: "be strict" }], exclude_paths: ["**/*.lock"] } });
expect(resolveReviewPromptOverrides(manifest)).toEqual({ profile: "chill", inlineComments: true, pathInstructions: [{ path: "src/**", instructions: "be strict" }], excludePaths: ["**/*.lock"] });
const manifest = parseFocusManifest({ review: { profile: "chill", inline_comments: true, path_instructions: [{ path: "src/**", instructions: "be strict" }], instructions: "Follow our async-error conventions.", exclude_paths: ["**/*.lock"] } });
expect(resolveReviewPromptOverrides(manifest)).toEqual({ profile: "chill", inlineComments: true, pathInstructions: [{ path: "src/**", instructions: "be strict" }], instructions: "Follow our async-error conventions.", excludePaths: ["**/*.lock"] });
// A null manifest (load failure) yields the byte-identical defaults; inline comments default OFF.
expect(resolveReviewPromptOverrides(null)).toEqual({ profile: null, inlineComments: false, pathInstructions: [], excludePaths: [] });
expect(resolveReviewPromptOverrides(null)).toEqual({ profile: null, inlineComments: false, pathInstructions: [], instructions: null, excludePaths: [] });
// An explicit false / absent toggle both resolve to the strict-boolean false.
expect(resolveReviewPromptOverrides(parseFocusManifest({ review: { inline_comments: false } })).inlineComments).toBe(false);
expect(resolveReviewPromptOverrides(parseFocusManifest({ review: { profile: "chill" } })).inlineComments).toBe(false);
Expand Down
2 changes: 1 addition & 1 deletion test/unit/signals-coverage.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -904,7 +904,7 @@ describe("signal coverage edge cases", () => {
collisions: buildCollisionReport(directRepo.fullName, [], [currentPr]),
preflight: buildPreflightResult({ repoFullName: directRepo.fullName, title: "Fix isolated issue", body: "Fixes #99", linkedIssues: [99] }, directRepo, [], [currentPr]),
settings: gateSettings,
review: { present: true, footerText: "Reviewed by the Acme maintainer bot.", note: "Run npm test before pushing.", fields: { relatedWork: false }, profile: null, inlineComments: null, pathInstructions: [], excludePaths: [], preMergeChecks: [] },
review: { present: true, footerText: "Reviewed by the Acme maintainer bot.", note: "Run npm test before pushing.", fields: { relatedWork: false }, profile: null, inlineComments: null, pathInstructions: [], instructions: null, excludePaths: [], preMergeChecks: [] },
aiReview: { notes: "The change is focused.\n\n**Suggestions**\n- Add a test for the </details> edge case." },
});
expect(customizedComment).toContain("Reviewed by the Acme maintainer bot."); // custom footer lead
Expand Down
Loading