diff --git a/apps/review-desktop/package.json b/apps/review-desktop/package.json index c2f87c98c..4532936e0 100644 --- a/apps/review-desktop/package.json +++ b/apps/review-desktop/package.json @@ -29,7 +29,7 @@ "@dev.fast/review-protocol": "workspace:*" }, "devDependencies": { - "@dev.fast/diffr": "0.1.8", + "@dev.fast/diffr": "0.1.9", "@dev.fast/review-canvas": "workspace:*", "@types/node": "24.12.2", "jsdom": "29.1.1", diff --git a/apps/review-desktop/scripts/e2e/journeys/diffr-settings.mjs b/apps/review-desktop/scripts/e2e/journeys/diffr-settings.mjs index e86b61dc9..7e5f60d92 100644 --- a/apps/review-desktop/scripts/e2e/journeys/diffr-settings.mjs +++ b/apps/review-desktop/scripts/e2e/journeys/diffr-settings.mjs @@ -10,7 +10,13 @@ export const phase = 1; export const options = { settings: { "review.experimental.structuralDiff.enabled": true }, - env: { REVIEW_DIFFR_BINARY: "", GEMINI_API_KEY: "", GOOGLE_API_KEY: "" }, + env: { + REVIEW_DIFFR_BINARY: "", + GEMINI_API_KEY: "", + GOOGLE_API_KEY: "", + OPENAI_API_KEY: "", + ANTHROPIC_API_KEY: "", + }, async beforeLaunch(ctx) { ctx.env.XDG_CONFIG_HOME = path.join(ctx.root, "config"); await mkdir(path.join(ctx.repo, "tests"), { recursive: true }); diff --git a/packages/review-protocol/package.json b/packages/review-protocol/package.json index 394a39573..3c876806b 100644 --- a/packages/review-protocol/package.json +++ b/packages/review-protocol/package.json @@ -20,7 +20,7 @@ "typecheck": "pnpm -w exec tsc -p packages/review-protocol/tsconfig.json --noEmit" }, "dependencies": { - "@dev.fast/diffr": "0.1.8", + "@dev.fast/diffr": "0.1.9", "@dev.fast/json": "workspace:*", "@dev.fast/trace-protocol": "workspace:*", "zod": "4.4.3" diff --git a/packages/review-protocol/src/contracts.ts b/packages/review-protocol/src/contracts.ts index a6c88e9b3..b1a842543 100644 --- a/packages/review-protocol/src/contracts.ts +++ b/packages/review-protocol/src/contracts.ts @@ -458,16 +458,32 @@ export interface ReviewCanvasTutorialBridge { * setter resolves with the value that actually landed, so a row re-renders from * the authoritative result instead of an optimistic one. */ +/** What diffr's schema says about one summary provider. */ +export interface ReviewDiffrProvider { + id: string; + title: string; + model: string; + endpoint: string; + keyVariables: string[]; + keylessCustomEndpoint: boolean; +} + export interface ReviewDiffrConfig { values: JsonObject; credentialSource: "config" | "environment" | "missing"; + providers?: ReviewDiffrProvider[]; + defaultPrompt?: string; changed?: boolean; error?: string; } export const reviewDiffrSummarizerInputSchema = z.object({ enabled: z.boolean(), + provider: z.string().min(1), model: z.string().trim().min(1), + endpoint: z.string().trim(), + // Blank means diffr's own default; kept as written, not trimmed. + systemPrompt: z.string(), tests: z.boolean(), apiKey: z.string().optional(), }); @@ -479,6 +495,19 @@ export type ReviewDiffrSummarizerInput = z.infer< const reviewDiffrConfigSchema = z.object({ values: z.custom(isJsonObject), credentialSource: z.enum(["config", "environment", "missing"]), + providers: z + .array( + z.object({ + id: z.string(), + title: z.string(), + model: z.string(), + endpoint: z.string(), + keyVariables: z.array(z.string()), + keylessCustomEndpoint: z.boolean(), + }), + ) + .optional(), + defaultPrompt: z.string().optional(), changed: z.boolean().optional(), error: z.string().optional(), }); diff --git a/packages/review/app/src/diffr-config-section.browser.test.tsx b/packages/review/app/src/diffr-config-section.browser.test.tsx index ef4a8b527..ed7820a79 100644 --- a/packages/review/app/src/diffr-config-section.browser.test.tsx +++ b/packages/review/app/src/diffr-config-section.browser.test.tsx @@ -1,4 +1,5 @@ import { + type JsonObject, type ReviewDiffrConfig, type ReviewDiffrConfigActions, } from "@dev.fast/review-protocol"; @@ -19,6 +20,30 @@ afterEach(async () => { function config(): ReviewDiffrConfig { return { credentialSource: "config", + providers: [ + [ + "gemini", + "Gemini", + "gemini-3.8-flash", + "https://generativelanguage.googleapis.com", + ], + ["openai", "OpenAI", "gpt-6-luna", "https://api.openai.com/v1"], + [ + "anthropic", + "Anthropic", + "claude-haiku-4-5", + "https://api.anthropic.com", + ], + ["mistral", "Mistral", "mistral-small", "https://api.mistral.ai/v1"], + ].map(([id, title, model, endpoint]) => ({ + id, + title, + model, + endpoint, + keyVariables: [], + keylessCustomEndpoint: id === "openai", + })), + defaultPrompt: "Default prompt.", values: { plugins: { bundled: { @@ -28,15 +53,21 @@ function config(): ReviewDiffrConfig { "removed-runs": { enabled: true }, group: { enabled: true }, "hide-files": { enabled: true, deleted: true, tags: ["test"] }, - summarize: { enabled: false, model: "test-model", tests: true }, + summarize: { + enabled: false, + provider: "gemini", + model: "test-model", + tests: true, + system_prompt: "Default prompt.", + }, }, }, }, }; } -async function mount() { - const current = config(); +async function mount(overrides: Partial = {}) { + const current = { ...config(), ...overrides }; const actions: ReviewDiffrConfigActions = { read: vi.fn(async () => current), @@ -53,7 +84,10 @@ async function mount() { bundled: { summarize: { enabled: input.enabled, + provider: input.provider, model: input.model, + endpoint: input.endpoint, + system_prompt: input.systemPrompt, tests: input.tests, }, }, @@ -166,7 +200,10 @@ test("tests draft settings without saving, then saves and clears the key", async }); expect(actions.saveSummarizer).toHaveBeenCalledWith({ enabled: false, + provider: "gemini", model: "test-model", + endpoint: "", + systemPrompt: "Default prompt.", tests: true, apiKey: "test-secret", }); @@ -257,3 +294,134 @@ test("shows partial save errors with authoritative values and a reload prompt", .element(page.getByRole("button", { name: "Reload window", exact: true })) .toBeVisible(); }); + +test("defaults to Gemini and switching provider resets the model and key", async () => { + const { actions } = await mount(); + await open(); + await expect + .element(page.getByRole("radio", { name: "Gemini" })) + .toHaveAttribute("aria-checked", "true"); + await act(async () => { + await page.getByLabelText("API key", { exact: true }).fill("typed-secret"); + }); + await act(async () => { + await page.getByRole("radio", { name: "Anthropic" }).click(); + }); + await expect + .element(page.getByLabelText("Model", { exact: true })) + .toHaveValue("claude-haiku-4-5"); + await expect + .element(page.getByLabelText("API key", { exact: true })) + .toHaveValue(""); + await expect + .element(page.getByText("Saving clears the key saved for Gemini.")) + .toBeVisible(); + await act(async () => { + await page + .getByLabelText("Endpoint URL", { exact: true }) + .fill(" https://proxy.example/anthropic "); + }); + await act(async () => { + await page.getByRole("button", { name: "Save summaries" }).click(); + }); + expect(actions.saveSummarizer).toHaveBeenCalledWith({ + enabled: false, + provider: "anthropic", + model: "claude-haiku-4-5", + endpoint: " https://proxy.example/anthropic ", + systemPrompt: "Default prompt.", + tests: true, + apiKey: "", + }); +}); + +test("edits the prompt, links its default, and resets it", async () => { + const { actions } = await mount(); + await open(); + const prompt = page.getByLabelText("Prompt", { exact: true }); + await expect.element(prompt).toHaveValue("Default prompt."); + await expect + .element(page.getByText("Default", { exact: true })) + .toBeVisible(); + await act(async () => { + await prompt.fill("Be terse."); + }); + await expect + .element(page.getByText("Customized", { exact: true })) + .toBeVisible(); + await act(async () => { + await page.getByRole("button", { name: "Test setup" }).click(); + }); + expect(actions.testSummarizer).toHaveBeenCalledWith( + expect.objectContaining({ systemPrompt: "Be terse." }), + ); + await act(async () => { + await page.getByRole("button", { name: "Reset to default" }).click(); + }); + await expect.element(prompt).toHaveValue("Default prompt."); + await expect + .element(page.getByRole("button", { name: "Save summaries" })) + .toBeDisabled(); +}); + +test("switching provider without a saved key does not promise to clear one", async () => { + await mount({ credentialSource: "missing" }); + await open(); + await act(async () => { + await page.getByRole("radio", { name: "OpenAI" }).click(); + }); + await expect + .element( + page.getByText( + "Enter a key for OpenAI, or leave blank to use its environment variable.", + ), + ) + .toBeVisible(); + expect(document.body.textContent).not.toContain("Saving clears"); +}); + +test("switching provider clears a custom endpoint, and a new endpoint warns about the saved key", async () => { + const { values } = config(); + + const summarize: JsonObject = { + enabled: false, + provider: "openai", + model: "test-model", + endpoint: "https://openrouter.ai/api/v1", + tests: true, + system_prompt: "Default prompt.", + }; + + await mount({ values: { ...values, plugins: { bundled: { summarize } } } }); + await open(); + const endpoint = page.getByLabelText("Endpoint URL", { exact: true }); + await expect.element(endpoint).toHaveValue("https://openrouter.ai/api/v1"); + await act(async () => { + await endpoint.fill("https://other.example/v1"); + }); + await expect + .element( + page.getByText( + "Saving clears the saved key, since the endpoint changed.", + ), + ) + .toBeVisible(); + await act(async () => { + await page.getByRole("radio", { name: "Anthropic" }).click(); + }); + await expect.element(endpoint).toHaveValue(""); +}); + +test("offers whatever providers diffr describes, with their defaults", async () => { + await mount(); + await open(); + await act(async () => { + await page.getByRole("radio", { name: "Mistral" }).click(); + }); + await expect + .element(page.getByLabelText("Model", { exact: true })) + .toHaveValue("mistral-small"); + await expect + .element(page.getByLabelText("Endpoint URL", { exact: true })) + .toHaveAttribute("placeholder", "https://api.mistral.ai/v1"); +}); diff --git a/packages/review/app/src/diffr-config-section.tsx b/packages/review/app/src/diffr-config-section.tsx index c88d536ca..0d2931bdc 100644 --- a/packages/review/app/src/diffr-config-section.tsx +++ b/packages/review/app/src/diffr-config-section.tsx @@ -1,5 +1,5 @@ import { Button } from "@canvas/ui/button"; -import { TextField } from "@canvas/ui/text-field"; +import { TextField, fieldStyles } from "@canvas/ui/text-field"; import { type JsonValue, type ReviewDiffrConfig, @@ -10,6 +10,7 @@ import { import * as stylex from "@stylexjs/stylex"; import { type ReactNode, useEffect, useRef, useState } from "react"; +import { Choice } from "./settings-choice"; import { settingsStyles as styles } from "./settings-styles"; const displaySettings = [ @@ -34,10 +35,28 @@ function setting( return value; } +function provider(config: ReviewDiffrConfig): string { + return String(setting(config, "summarize.provider") ?? ""); +} + +/** The provider diffr describes as `id`, if any. */ +function described(config: ReviewDiffrConfig, id: string) { + return config.providers?.find((known) => known.id === id); +} + +function title(config: ReviewDiffrConfig, id: string): string { + return described(config, id)?.title ?? id; +} + function summaryDraft(config: ReviewDiffrConfig): ReviewDiffrSummarizerInput { return { enabled: setting(config, "summarize.enabled") === true, + provider: provider(config), model: String(setting(config, "summarize.model") ?? ""), + endpoint: String(setting(config, "summarize.endpoint") ?? ""), + systemPrompt: String( + setting(config, "summarize.system_prompt") ?? config.defaultPrompt ?? "", + ), tests: setting(config, "summarize.tests") === true, apiKey: "", }; @@ -118,6 +137,7 @@ export function DiffrConfigSection({ !config || setting(config, "summarize.enabled") === undefined; const summaryValid = !!draft?.model.trim(); + const savedDraft = config && summaryDraft(config); const hiddenTags = config ? setting(config, "hide-files.tags") : undefined; return ( @@ -192,8 +212,8 @@ export function DiffrConfigSection({ ))}

AI summaries

- Sends source-file contents to Gemini to summarize large new - functions and tests. + Sends source-file contents to the selected provider to + summarize large new functions and tests.

{unavailable && (

@@ -215,6 +235,30 @@ export function DiffrConfigSection({ } /> + {!!config.providers?.length && ( + + [ + known.id, + known.title, + ]), + )} + disabled={busy || unavailable} + onChange={(choice) => + setDraft({ + ...draft, + provider: choice, + model: described(config, choice)?.model ?? "", + endpoint: "", + apiKey: "", + }) + } + /> + + )}

- {config.credentialSource === "config" - ? "Saved key" - : config.credentialSource === "environment" - ? "Environment key available" - : "Not configured"} - . Replacement keys are stored in diffr’s config. Leave - blank to keep the current key. + {draft.provider !== provider(config) || + draft.endpoint !== savedDraft?.endpoint + ? config.credentialSource !== "config" + ? `Enter a key for ${title(config, draft.provider)}, or leave blank to use its environment variable.` + : draft.provider !== provider(config) + ? `Saving clears the key saved for ${title(config, provider(config))}.` + : "Saving clears the saved key, since the endpoint changed." + : `${ + config.credentialSource === "config" + ? "Saved key" + : config.credentialSource === "environment" + ? "Environment key available" + : "Not configured" + }. Replacement keys are stored in diffr’s config. Leave blank to keep the current key.`}

+ + + setDraft({ ...draft, endpoint: event.target.value }) + } + /> + +

+ Optional. With OpenAI, point this at any compatible + server, such as OpenRouter or Ollama. +

+