diff --git a/docs/swarm/ui.md b/docs/swarm/ui.md index f20a9e2b..4bcddfc7 100644 --- a/docs/swarm/ui.md +++ b/docs/swarm/ui.md @@ -118,17 +118,24 @@ filters in the browser. It shows no token counts, cache counts or cost. ### Classifier settings -Under the try-out form, **classifier settings** shows the model and effort the -[review classifier](review-classifier.md) runs with and how many requested -reviews run at once, and lets you change them. The model is free text, one word -as claude's `--model` takes it, such as `haiku` or `sonnet`. The effort is one -of low, medium, high, xhigh and max, or claude's own default. Concurrent -reviews is a whole number from 1 to 8; try-outs don't count toward it. +Under the try-out form, **classifier settings** shows the runtime, model and +effort the [review classifier](review-classifier.md) runs with and how many +requested reviews run at once, and lets you change them. The runtime is **API +key** (claude on the Anthropic key) or **ACP** (opencode on a provider). On the +API key runtime the model is free text, one word as claude's `--model` takes +it, such as `haiku` or `sonnet`. Choosing ACP shows the provider's fields in +place of the model: its `https://` URL, its model name, and what it charges in +USD per million tokens for input and output, with optional cache read and +cache write prices. The effort is one of low, medium, high, xhigh and max, or +the runtime's own default. Concurrent reviews is a whole number from 1 to 8; +try-outs don't count toward it. -**save** is on once something changed and the fields are valid. A save replaces -all three settings. If the controller refuses it, the page shows the reason and -leaves the form as you typed it. A change takes effect from the next run, with -no restart. The page has no API key field: admins set the key in the bao UI. +**save** is on once something changed and the fields are valid. A save changes +the runtime, model, effort and limit, and the provider too while the runtime is +ACP, so switching back to API key keeps the stored provider for next time. If +the controller refuses it, the page shows the reason and leaves the form as you +typed it. A change takes effect from the next run, with no restart. The page +has no API key field: admins set each runtime's key in the bao UI. ### Scheduled prompts diff --git a/frontend/packages/swarm-ui/package.json b/frontend/packages/swarm-ui/package.json index dae581ff..d83a859b 100644 --- a/frontend/packages/swarm-ui/package.json +++ b/frontend/packages/swarm-ui/package.json @@ -6,7 +6,8 @@ "type": "module", "scripts": { "build": "node ./build.mjs", - "typecheck": "tsc --noEmit" + "typecheck": "tsc --noEmit", + "test": "node --test test/*.test.mjs" }, "dependencies": { "@hive/shared": "*", diff --git a/frontend/packages/swarm-ui/src/pages/review-settings/ClassifierSettings.css b/frontend/packages/swarm-ui/src/pages/review-settings/ClassifierSettings.css index 01757fca..570ad988 100644 --- a/frontend/packages/swarm-ui/src/pages/review-settings/ClassifierSettings.css +++ b/frontend/packages/swarm-ui/src/pages/review-settings/ClassifierSettings.css @@ -1,4 +1,5 @@ -/* The settings form: the three fields on one line, then the save action. */ +/* The settings form: the fields wrap in a row, the provider fields on a row of + * their own under the ACP runtime, then the save action. */ .ui-classifier-settings { display: flex; flex-direction: column; diff --git a/frontend/packages/swarm-ui/src/pages/review-settings/ClassifierSettings.tsx b/frontend/packages/swarm-ui/src/pages/review-settings/ClassifierSettings.tsx index 154eab73..0d6cfaba 100644 --- a/frontend/packages/swarm-ui/src/pages/review-settings/ClassifierSettings.tsx +++ b/frontend/packages/swarm-ui/src/pages/review-settings/ClassifierSettings.tsx @@ -1,9 +1,11 @@ -// — the review classifier's model, effort and how many -// requested reviews run at once, shown and saved here -// (`GET`/`PUT /api/review-classifier/settings`). A PUT replaces the stored -// settings whole, so the form sends all three fields. A refused save shows the -// route's own reason and leaves the form as typed. There is no API key field: -// admins set the key in the bao UI. +// — the review classifier's runtime, model, effort and +// how many requested reviews run at once, shown and saved here +// (`GET`/`PUT /api/review-classifier/settings`). A PUT changes only the +// fields it carries: the form sends the runtime, model, effort and limit, and +// the ACP provider only while the runtime is ACP, so the stored provider +// survives a switch back. A refused save shows the route's own reason and +// leaves the form as typed. There is no API key field: admins set each +// runtime's key in the bao UI. import { useEffect, useState } from "preact/hooks"; import { ApiErrorPanel } from "@hive/shared/api-error-panel.js"; import { readApiError, type ProblemDetails } from "@hive/shared/api-error.js"; @@ -20,17 +22,30 @@ import { settingsProblem, toSettings, type ClassifierSettingsBody, + type Runtime, type SettingsDraft, } from "./SettingsTypes.js"; import "./ClassifierSettings.css"; const SETTINGS = "/api/review-classifier/settings"; -const EFFORT_OPTIONS = [ - { value: "", label: "claude's default" }, - ...EFFORTS.map((e) => ({ value: e, label: e })), +const RUNTIME_OPTIONS = [ + { value: "api-key", label: "API key (claude)" }, + { value: "acp", label: "ACP (opencode)" }, ]; +// The empty effort option is the runtime's own default: claude's on the API +// key runtime, opencode's on ACP. +function effortOptions(runtime: Runtime) { + return [ + { + value: "", + label: runtime === "acp" ? "opencode's default" : "claude's default", + }, + ...EFFORTS.map((e) => ({ value: e, label: e })), + ]; +} + export function ClassifierSettings() { // The settings as the controller holds them; null until the first read. const [saved, setSaved] = useState(null); @@ -101,10 +116,12 @@ export function ClassifierSettings() { return (

- The model and effort the review classifier runs with, and how many - requested reviews run at once. A change takes effect from the next run, - with no restart. Try-outs don't count toward the limit. The API key is - set in the bao UI, not here. + The runtime, model and effort the review classifier runs with, and how + many requested reviews run at once. The API key runtime runs claude on + the Anthropic key; the ACP runtime runs opencode on the provider given + here. A change takes effect from the next run, with no restart. Try-outs + don't count toward the limit. Each runtime's key is set in the bao UI, + not here.

{loadError ? ( void save(e)}>
- edit({ ...draft, model })} + + edit({ ...draft, runtime: runtime as Runtime }) + } /> + {draft.runtime === "api-key" ? ( + edit({ ...draft, model })} + /> + ) : null} edit({ ...draft, effort })} /> edit({ ...draft, max })} />
+ {draft.runtime === "acp" ? ( +
+ edit({ ...draft, acpUrl })} + /> + edit({ ...draft, acpModel })} + /> + edit({ ...draft, priceInput })} + /> + edit({ ...draft, priceOutput })} + /> + edit({ ...draft, priceCacheRead })} + /> + + edit({ ...draft, priceCacheWrite }) + } + /> +
+ ) : null} {saveError ? ( = MIN_REVIEWS && n <= MAX_REVIEWS ? n : null; } +/** A price as a number, or null when it is not a number of 0 or more. */ +export function priceValue(text: string): number | null { + const t = text.trim(); + if (!/^\d+(\.\d+)?$/.test(t)) return null; + return Number(t); +} + +/** Why the provider fields are not ready to save, or null when they are. */ +function acpProblem(d: SettingsDraft): string | null { + const url = d.acpUrl.trim(); + if (!url.startsWith("https://") || /\s/.test(url)) { + return "the provider URL starts with https://"; + } + const model = d.acpModel.trim(); + if (model === "" || /\s/.test(model)) { + return "give the provider's model, one word as the provider names it"; + } + if (priceValue(d.priceInput) === null || priceValue(d.priceOutput) === null) { + return "give the input and output price, a number of 0 or more"; + } + for (const cache of [d.priceCacheRead, d.priceCacheWrite]) { + if (cache.trim() !== "" && priceValue(cache) === null) { + return "a cache price is a number of 0 or more, or empty"; + } + } + return null; +} + /** Why the draft is not ready to save, or null when it is. */ export function settingsProblem(d: SettingsDraft): string | null { const model = d.model.trim(); @@ -52,16 +124,38 @@ export function settingsProblem(d: SettingsDraft): string | null { if (maxValue(d.max) === null) { return `concurrent reviews is a whole number from ${MIN_REVIEWS} to ${MAX_REVIEWS}`; } - return null; + return d.runtime === "acp" ? acpProblem(d) : null; } -/** The draft as the request body. Call only when `settingsProblem` is null. */ -export function toSettings(d: SettingsDraft): ClassifierSettingsBody { - return { +/** The provider fields as the request carries them. Call only when + * `acpProblem` is null. */ +function toAcp(d: SettingsDraft): AcpSettings { + const prices: AcpPrices = { + input: priceValue(d.priceInput) ?? 0, + output: priceValue(d.priceOutput) ?? 0, + }; + if (d.priceCacheRead.trim() !== "") { + prices.cache_read = priceValue(d.priceCacheRead) ?? 0; + } + if (d.priceCacheWrite.trim() !== "") { + prices.cache_write = priceValue(d.priceCacheWrite) ?? 0; + } + return { base_url: d.acpUrl.trim(), model: d.acpModel.trim(), prices }; +} + +/** The draft as the request body. Call only when `settingsProblem` is null. + * On the API key runtime the body leaves `acp` out, so the stored provider + * stays for the next switch back. */ +export function toSettings( + d: SettingsDraft, +): Omit & { acp?: AcpSettings } { + const body = { model: d.model.trim(), effort: d.effort === "" ? null : (d.effort as Effort), max_concurrent_reviews: maxValue(d.max) ?? MIN_REVIEWS, + runtime: d.runtime, }; + return d.runtime === "acp" ? { ...body, acp: toAcp(d) } : body; } /** Whether `d` would save something other than `saved`. */ @@ -71,9 +165,20 @@ export function changed( ): boolean { if (saved === null) return true; const now = draftOf(saved); - return ( + const base = d.model.trim() !== now.model || d.effort !== now.effort || - d.max.trim() !== now.max + d.max.trim() !== now.max || + d.runtime !== now.runtime; + if (base) return true; + // The provider fields only save on the ACP runtime. + return ( + d.runtime === "acp" && + (d.acpUrl.trim() !== now.acpUrl || + d.acpModel.trim() !== now.acpModel || + d.priceInput.trim() !== now.priceInput || + d.priceOutput.trim() !== now.priceOutput || + d.priceCacheRead.trim() !== now.priceCacheRead || + d.priceCacheWrite.trim() !== now.priceCacheWrite) ); } diff --git a/frontend/packages/swarm-ui/test/settings-types.test.mjs b/frontend/packages/swarm-ui/test/settings-types.test.mjs new file mode 100644 index 00000000..e76cddfa --- /dev/null +++ b/frontend/packages/swarm-ui/test/settings-types.test.mjs @@ -0,0 +1,119 @@ +// The classifier settings form's pure logic: the draft a stored document +// becomes, the checks before a save, the request body, and what counts as a +// change. Run by `npm test`, which Node runs with its built-in test runner and +// type stripping, so the `.ts` module is imported as it is. +import assert from "node:assert/strict"; +import { test } from "node:test"; +import { + changed, + draftOf, + maxValue, + priceValue, + settingsProblem, + toSettings, +} from "../src/pages/review-settings/SettingsTypes.ts"; + +const apiKey = { + model: "haiku", + effort: null, + max_concurrent_reviews: 2, + runtime: "api-key", + acp: null, +}; + +const acpStored = { + ...apiKey, + runtime: "acp", + acp: { + base_url: "https://llm.example/v1", + model: "Qwen/Qwen3-35B", + prices: { input: 0.5, output: 2, cache_read: 0.1 }, + }, +}; + +// An ACP draft over the API key settings, with every provider field filled. +function acpDraft(over = {}) { + return { + ...draftOf(apiKey), + runtime: "acp", + acpUrl: "https://llm.example/v1", + acpModel: "Qwen/Qwen3-35B", + priceInput: "0.5", + priceOutput: "2", + ...over, + }; +} + +test("an unedited draft is valid and changes nothing", () => { + const d = draftOf(apiKey); + assert.equal(settingsProblem(d), null); + assert.equal(changed(d, apiKey), false); +}); + +test("the API key request leaves the provider out, so the stored one stays", () => { + const body = toSettings({ ...draftOf(acpStored), runtime: "api-key" }); + assert.deepEqual(body, { + model: "haiku", + effort: null, + max_concurrent_reviews: 2, + runtime: "api-key", + }); + assert.equal("acp" in body, false); +}); + +test("the ACP request carries the provider and its prices", () => { + const body = toSettings(acpDraft({ priceCacheRead: "0.1" })); + assert.equal(body.runtime, "acp"); + assert.deepEqual(body.acp, { + base_url: "https://llm.example/v1", + model: "Qwen/Qwen3-35B", + prices: { input: 0.5, output: 2, cache_read: 0.1 }, + }); +}); + +test("a cache price left empty is left out of the request", () => { + const { prices } = toSettings(acpDraft()).acp; + assert.equal("cache_read" in prices, false); + assert.equal("cache_write" in prices, false); +}); + +test("the draft of a stored provider round-trips to the same request", () => { + const body = toSettings(draftOf(acpStored)); + assert.deepEqual(body.acp, acpStored.acp); + assert.equal(changed(draftOf(acpStored), acpStored), false); +}); + +test("the provider fields are checked only on the ACP runtime", () => { + const empty = { ...draftOf(apiKey), acpUrl: "http://nope" }; + assert.equal(settingsProblem(empty), null); + assert.match(settingsProblem({ ...empty, runtime: "acp" }), /https:\/\//); +}); + +test("the ACP checks name the field at fault", () => { + assert.match(settingsProblem(acpDraft({ acpUrl: "http://x" })), /https:\/\//); + assert.match(settingsProblem(acpDraft({ acpModel: "a b" })), /model/); + assert.match(settingsProblem(acpDraft({ priceOutput: "-1" })), /price/); + assert.match(settingsProblem(acpDraft({ priceInput: "" })), /price/); + assert.match(settingsProblem(acpDraft({ priceCacheWrite: "x" })), /cache/); + assert.equal(settingsProblem(acpDraft()), null); +}); + +test("switching the runtime is a change; edits to the hidden provider are not", () => { + const saved = apiKey; + assert.equal(changed({ ...draftOf(saved), runtime: "acp" }, saved), true); + assert.equal( + changed({ ...draftOf(saved), acpUrl: "https://x" }, saved), + false, + ); + assert.equal(changed(acpDraft({ priceOutput: "3" }), acpStored), true); +}); + +test("the reviews limit and a price parse as the form needs", () => { + assert.equal(maxValue("8"), 8); + assert.equal(maxValue("9"), null); + assert.equal(maxValue("1.5"), null); + assert.equal(priceValue("0"), 0); + assert.equal(priceValue("0.25"), 0.25); + assert.equal(priceValue("-1"), null); + assert.equal(priceValue("1e3"), null); +}); diff --git a/nix/checks.nix b/nix/checks.nix index 6314207e..1bcf7fc3 100644 --- a/nix/checks.nix +++ b/nix/checks.nix @@ -369,6 +369,10 @@ in buildPhase = '' runHook preBuild npm run typecheck --workspace=packages/swarm-ui + # The package's own tests (`test/`, Node's built-in runner) ride this + # derivation: it already has the workspace installed, and a second + # `npm ci` for the same tree would only repeat it. + npm run test --workspace=packages/swarm-ui runHook postBuild '';