diff --git a/packages/gittensory-engine/src/focus-manifest.ts b/packages/gittensory-engine/src/focus-manifest.ts index 915a2ad831..2bd09c0b34 100644 --- a/packages/gittensory-engine/src/focus-manifest.ts +++ b/packages/gittensory-engine/src/focus-manifest.ts @@ -603,6 +603,14 @@ export type SelfHostAiModelConfig = { codexModel: string | null; /** `review.ai_model.codex_effort`: overrides CODEX_AI_EFFORT for this repo's codex reviewer. null (default) ⇒ the operator's global env var, then "medium". */ codexEffort: string | null; + /** `review.ai_model.ollama_model` (#3902): overrides OLLAMA_AI_MODEL for this repo's ollama reviewer. null (default) ⇒ the operator's global env var, then the provider's own default. */ + ollamaModel: string | null; + /** `review.ai_model.openai_model` (#3902): overrides OPENAI_AI_MODEL for this repo's openai reviewer. null (default) ⇒ the operator's global env var, then the provider's own default. */ + openaiModel: string | null; + /** `review.ai_model.openai_compatible_model` (#3902): overrides OPENAI_COMPATIBLE_AI_MODEL for this repo's openai-compatible reviewer. null (default) ⇒ the operator's global env var, then the provider's own default. */ + openaiCompatibleModel: string | null; + /** `review.ai_model.anthropic_model` (#3902): overrides ANTHROPIC_AI_MODEL for this repo's anthropic (BYOK Messages API) reviewer. null (default) ⇒ the operator's global env var, then the provider's own default. */ + anthropicModel: string | null; }; export const EMPTY_SELF_HOST_AI_MODEL_CONFIG: SelfHostAiModelConfig = { @@ -610,6 +618,10 @@ export const EMPTY_SELF_HOST_AI_MODEL_CONFIG: SelfHostAiModelConfig = { claudeEffort: null, codexModel: null, codexEffort: null, + ollamaModel: null, + openaiModel: null, + openaiCompatibleModel: null, + anthropicModel: null, }; /** Per-repo before/after screenshot-capture config under `review.visual` (#3609 / #3610). Generic by design — @@ -2101,7 +2113,11 @@ function selfHostAiModelPresent(config: SelfHostAiModelConfig): boolean { config.claudeModel !== null || config.claudeEffort !== null || config.codexModel !== null || - config.codexEffort !== null + config.codexEffort !== null || + config.ollamaModel !== null || + config.openaiModel !== null || + config.openaiCompatibleModel !== null || + config.anthropicModel !== null ); } @@ -2122,6 +2138,10 @@ function parseSelfHostAiModelConfig(value: JsonValue | undefined, warnings: stri claudeEffort: parsePublicSafeText(record.claude_effort, "review.ai_model.claude_effort", warnings), codexModel: parsePublicSafeText(record.codex_model, "review.ai_model.codex_model", warnings), codexEffort: parsePublicSafeText(record.codex_effort, "review.ai_model.codex_effort", warnings), + ollamaModel: parsePublicSafeText(record.ollama_model, "review.ai_model.ollama_model", warnings), + openaiModel: parsePublicSafeText(record.openai_model, "review.ai_model.openai_model", warnings), + openaiCompatibleModel: parsePublicSafeText(record.openai_compatible_model, "review.ai_model.openai_compatible_model", warnings), + anthropicModel: parsePublicSafeText(record.anthropic_model, "review.ai_model.anthropic_model", warnings), }; } @@ -2506,6 +2526,10 @@ export function reviewConfigToJson(review: FocusManifestReviewConfig): JsonValue if (review.aiModel.claudeEffort !== null) aiModel.claude_effort = review.aiModel.claudeEffort; if (review.aiModel.codexModel !== null) aiModel.codex_model = review.aiModel.codexModel; if (review.aiModel.codexEffort !== null) aiModel.codex_effort = review.aiModel.codexEffort; + if (review.aiModel.ollamaModel !== null) aiModel.ollama_model = review.aiModel.ollamaModel; + if (review.aiModel.openaiModel !== null) aiModel.openai_model = review.aiModel.openaiModel; + if (review.aiModel.openaiCompatibleModel !== null) aiModel.openai_compatible_model = review.aiModel.openaiCompatibleModel; + if (review.aiModel.anthropicModel !== null) aiModel.anthropic_model = review.aiModel.anthropicModel; out.ai_model = aiModel; } if (visualConfigPresent(review.visual)) { diff --git a/src/queue/processors.ts b/src/queue/processors.ts index 312a7888e9..ebe84f0e5e 100644 --- a/src/queue/processors.ts +++ b/src/queue/processors.ts @@ -7089,6 +7089,10 @@ export async function runAiReviewForAdvisory( claudeEffort: args.reviewSelfHostAiModel?.claudeEffort ?? null, codexModel: args.reviewSelfHostAiModel?.codexModel ?? null, codexEffort: args.reviewSelfHostAiModel?.codexEffort ?? null, + ollamaModel: args.reviewSelfHostAiModel?.ollamaModel ?? null, + openaiModel: args.reviewSelfHostAiModel?.openaiModel ?? null, + openaiCompatibleModel: args.reviewSelfHostAiModel?.openaiCompatibleModel ?? null, + anthropicModel: args.reviewSelfHostAiModel?.anthropicModel ?? null, // Inline comments (#inline-comments): ask the model for line-anchored findings only when the operator flag, // the cutover allowlist, AND the per-repo manifest toggle all pass. Otherwise the prompt is byte-identical. inlineFindings: inlineFindingsRequested, diff --git a/src/review/ai-review-cache-input.ts b/src/review/ai-review-cache-input.ts index 026db7f4b2..2cbc66f374 100644 --- a/src/review/ai-review-cache-input.ts +++ b/src/review/ai-review-cache-input.ts @@ -6,10 +6,11 @@ import type { import { sha256Hex } from "../utils/crypto"; // Bumped v1→v2 (#2995): `features` gained a `cultureProfile` member. Bumped v2→v3 (#2182-#2186): `features` -// gained an `impactMap` member. Every prior cached review's fingerprint was computed without that key, so -// bumping the version guarantees a clean cache miss on the first review after upgrade rather than silently -// reusing a hash computed under a different payload shape. -export const AI_REVIEW_CACHE_INPUT_VERSION = "ai-review-input:v3"; +// gained an `impactMap` member. Bumped v3→v4 (#3902): `selfHostAiModelOverride` gained ollamaModel/openaiModel/ +// openaiCompatibleModel/anthropicModel members. Every prior cached review's fingerprint was computed without +// that key, so bumping the version guarantees a clean cache miss on the first review after upgrade rather than +// silently reusing a hash computed under a different payload shape. +export const AI_REVIEW_CACHE_INPUT_VERSION = "ai-review-input:v4"; // #regate-churn (root cause, confirmed in production): this fingerprint USED to also hash the PR's live // `baseSha`, on the theory that a rebase/retarget can change the diff GitHub reports for an otherwise-unchanged @@ -172,6 +173,10 @@ export async function aiReviewCacheInputFingerprint(input: AiReviewCacheInput): claudeEffort: input.selfHostAiModelOverride.claudeEffort ?? null, codexModel: input.selfHostAiModelOverride.codexModel ?? null, codexEffort: input.selfHostAiModelOverride.codexEffort ?? null, + ollamaModel: input.selfHostAiModelOverride.ollamaModel ?? null, + openaiModel: input.selfHostAiModelOverride.openaiModel ?? null, + openaiCompatibleModel: input.selfHostAiModelOverride.openaiCompatibleModel ?? null, + anthropicModel: input.selfHostAiModelOverride.anthropicModel ?? null, } : null, profile: input.profile ?? null, diff --git a/src/selfhost/ai.ts b/src/selfhost/ai.ts index cf5b9f1f92..304c559031 100644 --- a/src/selfhost/ai.ts +++ b/src/selfhost/ai.ts @@ -39,6 +39,14 @@ interface AiRunOptions { claudeEffort?: string; codexModel?: string; codexEffort?: string; + // Same override mechanism, extended to the HTTP-API providers (#3902) -- ollama/openai/openai-compatible/ + // anthropic previously had no way to see a per-repo override at all (their model was resolved ONCE from the + // global env var at buildProvider() construction time, before any repo was known). Read per-call, same + // priority as above: repo override > global env var > this file's own default. + ollamaModel?: string; + openaiModel?: string; + openaiCompatibleModel?: string; + anthropicModel?: string; } /** A chat completion (`response`) or an embedding result (`data`). Both optional: the core reads whichever it * asked for (extractAiText → `response`, embedTexts → `data`), each defensive about the other being absent. @@ -197,6 +205,17 @@ export function resolveCodexFirstOutputTimeoutMs(env: Record undefined), matching the same repo-override + * > global-env-var priority `configuredClaudeModel`/`configuredCodexModel` already enforce for the CLI providers. */ +function resolveOpenAiCompatibleRepoOverride(providerName: string, options: AiRunOptions): string | undefined { + if (providerName === "ollama") return options.ollamaModel; + if (providerName === "openai") return options.openaiModel; + return options.openaiCompatibleModel; +} + /** OpenAI-compatible endpoint (Ollama's /v1, OpenAI, vLLM, LM Studio, …) — chat + embeddings. */ export function createOpenAiCompatibleAi(opts: { baseUrl: string; @@ -204,6 +223,8 @@ export function createOpenAiCompatibleAi(opts: { model?: string | undefined; defaultModel?: string | undefined; embedModel?: string | undefined; + /** Which `.gittensory.yml` `review.ai_model` field this instance's per-call override reads from (#3902). */ + providerName?: "ollama" | "openai" | "openai-compatible" | undefined; }): SelfHostAi { const base = opts.baseUrl.replace(/\/+$/, ""); const headers = (): Record => ({ "content-type": "application/json", ...(opts.apiKey ? { authorization: `Bearer ${opts.apiKey}` } : {}) }); @@ -222,7 +243,8 @@ export function createOpenAiCompatibleAi(opts: { const json = (await res.json()) as { data?: Array<{ embedding: number[] }> }; return { data: (json.data ?? []).map((d) => d.embedding) }; } - const resolvedModel = resolveModel(opts.model, model, opts.defaultModel ?? DEFAULT_OPENAI_COMPATIBLE_CHAT_MODEL); + const repoOverride = opts.providerName ? resolveOpenAiCompatibleRepoOverride(opts.providerName, options) : undefined; + const resolvedModel = resolveModel(firstConfigured(repoOverride, opts.model), model, opts.defaultModel ?? DEFAULT_OPENAI_COMPATIBLE_CHAT_MODEL); const res = await fetch(`${base}/chat/completions`, { method: "POST", headers: headers(), @@ -255,7 +277,9 @@ export function createAnthropicAi(opts: { apiKey: string; model?: string | undef .map((m) => m.content) .join("\n\n") || undefined; const messages = msgs.filter((m) => m.role !== "system").map((m) => ({ role: m.role === "assistant" ? "assistant" : "user", content: m.content })); - const resolvedModel = resolveModel(opts.model, model, "claude-sonnet-5"); + // Repo override > construction-time env-resolved opts.model (#3902), same priority as the OpenAI-compatible + // providers above and the CLI providers' claudeModel/codexModel. + const resolvedModel = resolveModel(firstConfigured(options.anthropicModel, opts.model), model, "claude-sonnet-5"); const res = await fetch(`${base}/v1/messages`, { method: "POST", headers: { "content-type": "application/json", "x-api-key": opts.apiKey, "anthropic-version": "2023-06-01" }, @@ -1070,6 +1094,7 @@ export function buildProvider(name: string, env: Record { expect(effortChanged).not.toBe(original); }); + it("changes when the PER-REPO review.ai_model override gains an ollama/openai/openai-compatible/anthropic model (#3902)", async () => { + const original = await aiReviewCacheInputFingerprint(baseInput()); + for (const key of ["ollamaModel", "openaiModel", "openaiCompatibleModel", "anthropicModel"] as const) { + const changed = await aiReviewCacheInputFingerprint({ + ...baseInput(), + selfHostAiModelOverride: { claudeModel: null, claudeEffort: null, codexModel: null, codexEffort: null, ollamaModel: null, openaiModel: null, openaiCompatibleModel: null, anthropicModel: null, [key]: "repo-override-model" }, + }); + expect(changed, key).not.toBe(original); + } + }); + it("normalizes an absent self-host provider config the same whether omitted or explicitly empty", async () => { const nullConfig = await aiReviewCacheInputFingerprint({ ...baseInput(), selfHostProviderConfig: null }); const emptyConfig = await aiReviewCacheInputFingerprint({ ...baseInput(), selfHostProviderConfig: {} }); diff --git a/test/unit/focus-manifest.test.ts b/test/unit/focus-manifest.test.ts index 7cc3b04f82..da062a2ad9 100644 --- a/test/unit/focus-manifest.test.ts +++ b/test/unit/focus-manifest.test.ts @@ -3575,7 +3575,7 @@ describe("review.auto_review (#1954 / #2038–#2041)", () => { }); describe("review.ai_model (#selfhost-ai-model-override)", () => { - it("parses all four knobs, marks present, and round-trips", () => { + it("parses all eight knobs, marks present, and round-trips", () => { const m = parseFocusManifest({ review: { ai_model: { @@ -3583,6 +3583,10 @@ describe("review.ai_model (#selfhost-ai-model-override)", () => { claude_effort: "high", codex_model: "gpt-5.5-pro", codex_effort: "xhigh", + ollama_model: "llama3.3", + openai_model: "gpt-5.5", + openai_compatible_model: "qwen2.5-coder", + anthropic_model: "claude-opus-4-8", }, }, }); @@ -3591,11 +3595,29 @@ describe("review.ai_model (#selfhost-ai-model-override)", () => { claudeEffort: "high", codexModel: "gpt-5.5-pro", codexEffort: "xhigh", + ollamaModel: "llama3.3", + openaiModel: "gpt-5.5", + openaiCompatibleModel: "qwen2.5-coder", + anthropicModel: "claude-opus-4-8", }); expect(m.review.present).toBe(true); expect(parseFocusManifest({ review: reviewConfigToJson(m.review) }).review.aiModel).toEqual(m.review.aiModel); }); + it("parses each of the four HTTP-API provider knobs independently (#3902)", () => { + for (const [key, camelKey] of [ + ["ollama_model", "ollamaModel"], + ["openai_model", "openaiModel"], + ["openai_compatible_model", "openaiCompatibleModel"], + ["anthropic_model", "anthropicModel"], + ] as const) { + const m = parseFocusManifest({ review: { ai_model: { [key]: "some-model" } } }); + expect(m.review.aiModel).toEqual({ ...EMPTY_SELF_HOST_AI_MODEL_CONFIG, [camelKey]: "some-model" }); + expect(m.review.present).toBe(true); + expect(reviewConfigToJson(m.review)).toEqual({ ai_model: { [key]: "some-model" } }); + } + }); + it("absent/null ai_model yields the empty defaults and does not mark review present on its own", () => { expect(parseFocusManifest({}).review.aiModel).toEqual({ ...EMPTY_SELF_HOST_AI_MODEL_CONFIG }); expect(parseFocusManifest({ review: { ai_model: null } }).review.aiModel).toEqual({ ...EMPTY_SELF_HOST_AI_MODEL_CONFIG }); diff --git a/test/unit/queue.test.ts b/test/unit/queue.test.ts index 153f5aaf0b..a86d27d4f6 100644 --- a/test/unit/queue.test.ts +++ b/test/unit/queue.test.ts @@ -3559,7 +3559,7 @@ describe("queue processors", () => { metadata: { inputFingerprint: await aiReviewCacheInputFingerprint({ title: "Clean PR", mode: "block", byok: false, provider: null, model: null, aiReviewAllAuthors: false, - aiReviewCloseConfidence: undefined, aiReviewCombine: null, aiReviewOnMerge: null, aiReviewReviewers: null, gatePack: "oss-anti-slop", reviewerPlan: env.AI_REVIEW_PLAN, selfHostProviderConfig: null, selfHostAiModelOverride: { claudeModel: null, claudeEffort: null, codexModel: null, codexEffort: null }, + aiReviewCloseConfidence: undefined, aiReviewCombine: null, aiReviewOnMerge: null, aiReviewReviewers: null, gatePack: "oss-anti-slop", reviewerPlan: env.AI_REVIEW_PLAN, selfHostProviderConfig: null, selfHostAiModelOverride: { claudeModel: null, claudeEffort: null, codexModel: null, codexEffort: null, ollamaModel: null, openaiModel: null, openaiCompatibleModel: null, anthropicModel: null }, reviewFiles: [{ path: "src/a.ts", status: "modified", patch: "@@\n+export const ok = true;", additions: 1, deletions: 0 }], profile: null, securityFocus: false, inlineComments: false, pathInstructions: [], pathGuidance: "", repoInstructions: null, excludePaths: [], pathFilters: [], changedPaths: ["src/a.ts"], features: { grounding: false, rag: false, enrichment: false, reputation: false, cultureProfile: false, impactMap: false }, @@ -4138,7 +4138,7 @@ describe("queue processors", () => { aiReviewReviewers: null, gatePack: "oss-anti-slop", reviewerPlan: env.AI_REVIEW_PLAN, - selfHostProviderConfig: null, selfHostAiModelOverride: { claudeModel: null, claudeEffort: null, codexModel: null, codexEffort: null }, + selfHostProviderConfig: null, selfHostAiModelOverride: { claudeModel: null, claudeEffort: null, codexModel: null, codexEffort: null, ollamaModel: null, openaiModel: null, openaiCompatibleModel: null, anthropicModel: null }, reviewFiles: [{ path: "src/a.ts", status: "modified", patch: "@@\n+export const ok = true;", additions: 1, deletions: 0 }], profile: null, securityFocus: false, @@ -4219,7 +4219,7 @@ describe("queue processors", () => { metadata: { inputFingerprint: await aiReviewCacheInputFingerprint({ title: "Current PR", mode: "block", byok: false, provider: null, model: null, aiReviewAllAuthors: false, - aiReviewCloseConfidence: undefined, aiReviewCombine: null, aiReviewOnMerge: null, aiReviewReviewers: null, gatePack: "oss-anti-slop", reviewerPlan: env.AI_REVIEW_PLAN, selfHostProviderConfig: null, selfHostAiModelOverride: { claudeModel: null, claudeEffort: null, codexModel: null, codexEffort: null }, + aiReviewCloseConfidence: undefined, aiReviewCombine: null, aiReviewOnMerge: null, aiReviewReviewers: null, gatePack: "oss-anti-slop", reviewerPlan: env.AI_REVIEW_PLAN, selfHostProviderConfig: null, selfHostAiModelOverride: { claudeModel: null, claudeEffort: null, codexModel: null, codexEffort: null, ollamaModel: null, openaiModel: null, openaiCompatibleModel: null, anthropicModel: null }, reviewFiles: [{ path: "src/a.ts", status: "modified", patch: "@@\n+export const ok = true;", additions: 1, deletions: 0 }], profile: null, securityFocus: false, inlineComments: false, pathInstructions: [], pathGuidance: "", repoInstructions: null, excludePaths: [], pathFilters: [], changedPaths: ["src/a.ts"], features: { grounding: false, rag: false, enrichment: false, reputation: false, cultureProfile: false, impactMap: false }, @@ -4276,7 +4276,7 @@ describe("queue processors", () => { metadata: { inputFingerprint: await aiReviewCacheInputFingerprint({ title: "Current PR", mode: "block", byok: false, provider: null, model: null, aiReviewAllAuthors: false, - aiReviewCloseConfidence: undefined, aiReviewCombine: null, aiReviewOnMerge: null, aiReviewReviewers: null, gatePack: "oss-anti-slop", reviewerPlan: env.AI_REVIEW_PLAN, selfHostProviderConfig: null, selfHostAiModelOverride: { claudeModel: null, claudeEffort: null, codexModel: null, codexEffort: null }, + aiReviewCloseConfidence: undefined, aiReviewCombine: null, aiReviewOnMerge: null, aiReviewReviewers: null, gatePack: "oss-anti-slop", reviewerPlan: env.AI_REVIEW_PLAN, selfHostProviderConfig: null, selfHostAiModelOverride: { claudeModel: null, claudeEffort: null, codexModel: null, codexEffort: null, ollamaModel: null, openaiModel: null, openaiCompatibleModel: null, anthropicModel: null }, reviewFiles: [{ path: "src/a.ts", status: "modified", patch: "@@\n+export const ok = true;", additions: 1, deletions: 0 }], profile: null, securityFocus: false, inlineComments: false, pathInstructions: [], pathGuidance: "", repoInstructions: null, excludePaths: [], pathFilters: [], changedPaths: ["src/a.ts"], features: { grounding: false, rag: false, enrichment: false, reputation: false, cultureProfile: false, impactMap: false }, @@ -4332,7 +4332,7 @@ describe("queue processors", () => { metadata: { inputFingerprint: await aiReviewCacheInputFingerprint({ title: "Current PR", mode: "block", byok: false, provider: null, model: null, aiReviewAllAuthors: false, - aiReviewCloseConfidence: undefined, aiReviewCombine: null, aiReviewOnMerge: null, aiReviewReviewers: null, gatePack: "oss-anti-slop", reviewerPlan: env.AI_REVIEW_PLAN, selfHostProviderConfig: null, selfHostAiModelOverride: { claudeModel: null, claudeEffort: null, codexModel: null, codexEffort: null }, + aiReviewCloseConfidence: undefined, aiReviewCombine: null, aiReviewOnMerge: null, aiReviewReviewers: null, gatePack: "oss-anti-slop", reviewerPlan: env.AI_REVIEW_PLAN, selfHostProviderConfig: null, selfHostAiModelOverride: { claudeModel: null, claudeEffort: null, codexModel: null, codexEffort: null, ollamaModel: null, openaiModel: null, openaiCompatibleModel: null, anthropicModel: null }, reviewFiles: [{ path: "src/a.ts", status: "modified", patch: "@@\n+export const ok = true;", additions: 1, deletions: 0 }], profile: null, securityFocus: false, inlineComments: false, pathInstructions: [], pathGuidance: "", repoInstructions: null, excludePaths: [], pathFilters: [], changedPaths: ["src/a.ts"], features: { grounding: false, rag: false, enrichment: false, reputation: false, cultureProfile: false, impactMap: false }, @@ -4385,7 +4385,7 @@ describe("queue processors", () => { metadata: { inputFingerprint: await aiReviewCacheInputFingerprint({ title: "Current PR", mode: "block", byok: false, provider: null, model: null, aiReviewAllAuthors: false, - aiReviewCloseConfidence: undefined, aiReviewCombine: null, aiReviewOnMerge: null, aiReviewReviewers: null, gatePack: "oss-anti-slop", reviewerPlan: env.AI_REVIEW_PLAN, selfHostProviderConfig: null, selfHostAiModelOverride: { claudeModel: null, claudeEffort: null, codexModel: null, codexEffort: null }, + aiReviewCloseConfidence: undefined, aiReviewCombine: null, aiReviewOnMerge: null, aiReviewReviewers: null, gatePack: "oss-anti-slop", reviewerPlan: env.AI_REVIEW_PLAN, selfHostProviderConfig: null, selfHostAiModelOverride: { claudeModel: null, claudeEffort: null, codexModel: null, codexEffort: null, ollamaModel: null, openaiModel: null, openaiCompatibleModel: null, anthropicModel: null }, reviewFiles: [{ path: "src/a.ts", status: "modified", patch: "@@\n+export const ok = true;", additions: 1, deletions: 0 }], profile: null, securityFocus: false, inlineComments: false, pathInstructions: [], pathGuidance: "", repoInstructions: null, excludePaths: [], pathFilters: [], changedPaths: ["src/a.ts"], features: { grounding: false, rag: false, enrichment: false, reputation: false, cultureProfile: false, impactMap: false }, @@ -4434,7 +4434,7 @@ describe("queue processors", () => { metadata: { inputFingerprint: await aiReviewCacheInputFingerprint({ title: "Partially published PR", mode: "block", byok: false, provider: null, model: null, aiReviewAllAuthors: false, - aiReviewCloseConfidence: undefined, aiReviewCombine: null, aiReviewOnMerge: null, aiReviewReviewers: null, gatePack: "oss-anti-slop", reviewerPlan: env.AI_REVIEW_PLAN, selfHostProviderConfig: null, selfHostAiModelOverride: { claudeModel: null, claudeEffort: null, codexModel: null, codexEffort: null }, + aiReviewCloseConfidence: undefined, aiReviewCombine: null, aiReviewOnMerge: null, aiReviewReviewers: null, gatePack: "oss-anti-slop", reviewerPlan: env.AI_REVIEW_PLAN, selfHostProviderConfig: null, selfHostAiModelOverride: { claudeModel: null, claudeEffort: null, codexModel: null, codexEffort: null, ollamaModel: null, openaiModel: null, openaiCompatibleModel: null, anthropicModel: null }, reviewFiles: [{ path: "src/a.ts", status: "modified", patch: "@@\n+export const ok = true;", additions: 1, deletions: 0 }], profile: null, securityFocus: false, inlineComments: false, pathInstructions: [], pathGuidance: "", repoInstructions: null, excludePaths: [], pathFilters: [], changedPaths: ["src/a.ts"], features: { grounding: false, rag: false, enrichment: false, reputation: false, cultureProfile: false, impactMap: false }, @@ -4486,7 +4486,7 @@ describe("queue processors", () => { metadata: { inputFingerprint: await aiReviewCacheInputFingerprint({ title: "Current PR", mode: "block", byok: false, provider: null, model: null, aiReviewAllAuthors: false, - aiReviewCloseConfidence: undefined, aiReviewCombine: null, aiReviewOnMerge: null, aiReviewReviewers: null, gatePack: "oss-anti-slop", reviewerPlan: env.AI_REVIEW_PLAN, selfHostProviderConfig: null, selfHostAiModelOverride: { claudeModel: null, claudeEffort: null, codexModel: null, codexEffort: null }, + aiReviewCloseConfidence: undefined, aiReviewCombine: null, aiReviewOnMerge: null, aiReviewReviewers: null, gatePack: "oss-anti-slop", reviewerPlan: env.AI_REVIEW_PLAN, selfHostProviderConfig: null, selfHostAiModelOverride: { claudeModel: null, claudeEffort: null, codexModel: null, codexEffort: null, ollamaModel: null, openaiModel: null, openaiCompatibleModel: null, anthropicModel: null }, reviewFiles: [{ path: "src/a.ts", status: "modified", patch: "@@\n+export const ok = true;", additions: 1, deletions: 0 }], profile: null, securityFocus: false, inlineComments: false, pathInstructions: [], pathGuidance: "", repoInstructions: null, excludePaths: [], pathFilters: [], changedPaths: ["src/a.ts"], features: { grounding: false, rag: false, enrichment: false, reputation: false, cultureProfile: false, impactMap: false }, @@ -5402,7 +5402,7 @@ describe("queue processors", () => { aiReviewReviewers: null, gatePack: "oss-anti-slop", reviewerPlan: env.AI_REVIEW_PLAN, - selfHostProviderConfig: null, selfHostAiModelOverride: { claudeModel: null, claudeEffort: null, codexModel: null, codexEffort: null }, + selfHostProviderConfig: null, selfHostAiModelOverride: { claudeModel: null, claudeEffort: null, codexModel: null, codexEffort: null, ollamaModel: null, openaiModel: null, openaiCompatibleModel: null, anthropicModel: null }, reviewFiles: [{ path: "src/a.ts", status: "modified", patch: "@@\n+export const ok = value.length;", additions: 1, deletions: 0 }], profile: null, securityFocus: false, @@ -15023,10 +15023,10 @@ describe("queue processors", () => { const usage = await env.DB.prepare("select feature, status from ai_usage_events where feature = ?").bind("ai_review_pr").first<{ feature: string; status: string }>(); expect(usage).toMatchObject({ feature: "ai_review_pr", status: "ok" }); expect(cacheReadSpy).toHaveBeenCalled(); - expect(cacheReadSpy.mock.calls[0]?.[5]).toMatch(/^ai-review-input:v3:/); + expect(cacheReadSpy.mock.calls[0]?.[5]).toMatch(/^ai-review-input:v4:/); expect(cacheWriteSpy).toHaveBeenCalled(); expect(cacheWriteSpy.mock.calls[0]?.[5]).toMatchObject({ - metadata: { inputFingerprint: expect.stringMatching(/^ai-review-input:v3:/) }, + metadata: { inputFingerprint: expect.stringMatching(/^ai-review-input:v4:/) }, }); cacheReadSpy.mockRestore(); cacheWriteSpy.mockRestore(); diff --git a/test/unit/selfhost-ai.test.ts b/test/unit/selfhost-ai.test.ts index c1486d261e..763f62c055 100644 --- a/test/unit/selfhost-ai.test.ts +++ b/test/unit/selfhost-ai.test.ts @@ -155,6 +155,46 @@ describe("createOpenAiCompatibleAi (#979)", () => { await createOpenAiCompatibleAi({ baseUrl: "http://o/v1" }).run("m", {}); expect(body?.messages).toEqual([{ role: "user", content: "" }]); }); + + it("each providerName's per-repo review.ai_model override (#3902) outranks the construction-time model, which outranks the hardcoded default", async () => { + let sentModel = ""; + vi.stubGlobal("fetch", vi.fn(async (_u: string, init: { body: string }) => { + sentModel = (JSON.parse(init.body) as { model: string }).model; + return new Response(JSON.stringify({ choices: [{ message: { content: "ok" } }] }), { status: 200 }); + })); + const cases: Array<{ providerName: "ollama" | "openai" | "openai-compatible"; overrideKey: "ollamaModel" | "openaiModel" | "openaiCompatibleModel" }> = [ + { providerName: "ollama", overrideKey: "ollamaModel" }, + { providerName: "openai", overrideKey: "openaiModel" }, + { providerName: "openai-compatible", overrideKey: "openaiCompatibleModel" }, + ]; + for (const { providerName, overrideKey } of cases) { + const ai = createOpenAiCompatibleAi({ baseUrl: "http://o/v1", model: "construction-time-model", defaultModel: "hardcoded-default", providerName }); + // Repo override wins over the construction-time-resolved model. + await ai.run("m", { prompt: "x", [overrideKey]: "repo-override-model" }); + expect(sentModel).toBe("repo-override-model"); + // No override on THIS call → falls through to the construction-time model, unaffected by the prior call. + await ai.run("m", { prompt: "x" }); + expect(sentModel).toBe("construction-time-model"); + // A DIFFERENT variant's override field must not leak across providerName -- only its own key applies. + const otherKey = cases.find((c) => c.overrideKey !== overrideKey)!.overrideKey; + await ai.run("m", { prompt: "x", [otherKey]: "wrong-provider-model" }); + expect(sentModel).toBe("construction-time-model"); + } + // No providerName set at all (e.g. an embed-only construction) → override fields are simply never consulted. + const noNameAi = createOpenAiCompatibleAi({ baseUrl: "http://o/v1", model: "construction-time-model" }); + await noNameAi.run("m", { prompt: "x", ollamaModel: "should-be-ignored" }); + expect(sentModel).toBe("construction-time-model"); + }); + + it("falls all the way through to the hardcoded default when no override and no construction-time model are set", async () => { + let sentModel = ""; + vi.stubGlobal("fetch", vi.fn(async (_u: string, init: { body: string }) => { + sentModel = (JSON.parse(init.body) as { model: string }).model; + return new Response(JSON.stringify({ choices: [{ message: { content: "ok" } }] }), { status: 200 }); + })); + await createOpenAiCompatibleAi({ baseUrl: "http://o/v1", defaultModel: "hardcoded-default", providerName: "ollama" }).run("@cf/ignored", { prompt: "x" }); + expect(sentModel).toBe("hardcoded-default"); + }); }); describe("createSelfHostAi — provider selection", () => { @@ -206,6 +246,23 @@ describe("createAnthropicAi (#979 native BYOK)", () => { vi.stubGlobal("fetch", vi.fn(async () => new Response("e", { status: 429 }))); await expect(createAnthropicAi({ apiKey: "k" }).run("m", { prompt: "x" })).rejects.toThrow(/anthropic_http_429/); }); + + it("the per-repo review.ai_model.anthropic_model override (#3902) outranks the construction-time model, which outranks the hardcoded default", async () => { + let sentModel = ""; + vi.stubGlobal("fetch", vi.fn(async (_u: string, init: { body: string }) => { + sentModel = (JSON.parse(init.body) as { model: string }).model; + return new Response(JSON.stringify({ content: [{ type: "text", text: "ok" }] }), { status: 200 }); + })); + const ai = createAnthropicAi({ apiKey: "sk-ant", model: "claude-sonnet-4-6" }); + await ai.run("@cf/ignored", { prompt: "x", anthropicModel: "claude-opus-4-8" }); + expect(sentModel).toBe("claude-opus-4-8"); + // No override on this call → falls through to the construction-time model. + await ai.run("@cf/ignored", { prompt: "x" }); + expect(sentModel).toBe("claude-sonnet-4-6"); + // No override AND no construction-time model → falls all the way through to the hardcoded default. + await createAnthropicAi({ apiKey: "sk-ant" }).run("@cf/ignored", { prompt: "x" }); + expect(sentModel).toBe("claude-sonnet-5"); + }); }); describe("createChainAi (fallback)", () => { @@ -816,6 +873,23 @@ describe("branch coverage — defaults + edge inputs", () => { expect(buildProvider("anthropic", {})).toBeUndefined(); // anthropic is credentialed and requires ANTHROPIC_API_KEY expect(typeof buildProvider("anthropic", { ANTHROPIC_API_KEY: "sk-ant" })?.run).toBe("function"); }); + + it("buildProvider wires each HTTP-API provider's own review.ai_model field end to end (#3902)", async () => { + let sentModel = ""; + vi.stubGlobal("fetch", vi.fn(async (url: string, init: { body: string }) => { + const parsed = JSON.parse(init.body) as { model: string }; + sentModel = parsed.model; + return new Response(url.includes("/v1/messages") ? JSON.stringify({ content: [{ type: "text", text: "ok" }] }) : JSON.stringify({ choices: [{ message: { content: "ok" } }] }), { status: 200 }); + })); + await buildProvider("ollama", { OLLAMA_AI_MODEL: "global-ollama" })!.run("m", { prompt: "x", ollamaModel: "repo-ollama" }); + expect(sentModel).toBe("repo-ollama"); + await buildProvider("openai", { OPENAI_API_KEY: "sk-test", OPENAI_AI_MODEL: "global-openai" })!.run("m", { prompt: "x", openaiModel: "repo-openai" }); + expect(sentModel).toBe("repo-openai"); + await buildProvider("openai-compatible", { OPENAI_COMPATIBLE_AI_MODEL: "global-compat" })!.run("m", { prompt: "x", openaiCompatibleModel: "repo-compat" }); + expect(sentModel).toBe("repo-compat"); + await buildProvider("anthropic", { ANTHROPIC_API_KEY: "sk-ant", ANTHROPIC_AI_MODEL: "global-anthropic" })!.run("m", { prompt: "x", anthropicModel: "repo-anthropic" }); + expect(sentModel).toBe("repo-anthropic"); + }); it("extractCliText reads content + response fields", () => { expect(extractCliText(JSON.stringify({ content: "c" }))).toBe("c"); expect(extractCliText(JSON.stringify({ response: "r" }))).toBe("r"); diff --git a/test/unit/signals-coverage.test.ts b/test/unit/signals-coverage.test.ts index 8419305f26..684351c8b9 100644 --- a/test/unit/signals-coverage.test.ts +++ b/test/unit/signals-coverage.test.ts @@ -1127,7 +1127,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 }, enrichmentAnalyzers: {}, profile: null, tone: null, securityFocus: null, inlineComments: null, fixHandoff: null, autoMergeSummary: null, suggestions: null, changedFilesSummary: null, effortScore: null, testGeneration: null, impactMap: null, cultureProfile: null, reviewMemory: null, findingCategories: null, inlineCommentsPerCategory: null, minFindingSeverity: null, maxFindings: { blockers: null, nits: null }, commentVerbosity: null, pathInstructions: [], instructions: null, excludePaths: [], pathFilters: [], preMergeChecks: [], autoReview: { skipDrafts: null, ignoreAuthors: [], ignoreTitleKeywords: [], skipLabels: [], skipDocsOnly: null, maxAddedLines: 0, maxFiles: 0, baseBranches: [], autoPauseAfterReviewedCommits: null }, labelingRules: [], aiModel: { claudeModel: null, claudeEffort: null, codexModel: null, codexEffort: null }, visual: { preview: { urlTemplate: null }, routes: { paths: [], maxRoutes: null }, themes: [], gif: false }, linkedIssueSatisfaction: null }, + review: { present: true, footerText: "Reviewed by the Acme maintainer bot.", note: "Run npm test before pushing.", fields: { relatedWork: false }, enrichmentAnalyzers: {}, profile: null, tone: null, securityFocus: null, inlineComments: null, fixHandoff: null, autoMergeSummary: null, suggestions: null, changedFilesSummary: null, effortScore: null, testGeneration: null, impactMap: null, cultureProfile: null, reviewMemory: null, findingCategories: null, inlineCommentsPerCategory: null, minFindingSeverity: null, maxFindings: { blockers: null, nits: null }, commentVerbosity: null, pathInstructions: [], instructions: null, excludePaths: [], pathFilters: [], preMergeChecks: [], autoReview: { skipDrafts: null, ignoreAuthors: [], ignoreTitleKeywords: [], skipLabels: [], skipDocsOnly: null, maxAddedLines: 0, maxFiles: 0, baseBranches: [], autoPauseAfterReviewedCommits: null }, labelingRules: [], aiModel: { claudeModel: null, claudeEffort: null, codexModel: null, codexEffort: null, ollamaModel: null, openaiModel: null, openaiCompatibleModel: null, anthropicModel: null }, visual: { preview: { urlTemplate: null }, routes: { paths: [], maxRoutes: null }, themes: [], gif: false }, linkedIssueSatisfaction: null }, aiReview: { notes: "The change is focused.\n\n**Nits (2)**\n- Add a test for the edge case.\n- Keep the validator helper scoped." }, }); expect(customizedComment).toContain("Reviewed by the Acme maintainer bot."); // custom footer lead