diff --git a/docs/swarm/ui.md b/docs/swarm/ui.md index 4bcddfc7..f20a9e2b 100644 --- a/docs/swarm/ui.md +++ b/docs/swarm/ui.md @@ -118,24 +118,17 @@ filters in the browser. It shows no token counts, cache counts or cost. ### Classifier settings -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. +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. -**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. +**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. ### Scheduled prompts diff --git a/frontend/packages/swarm-ui/package.json b/frontend/packages/swarm-ui/package.json index d83a859b..dae581ff 100644 --- a/frontend/packages/swarm-ui/package.json +++ b/frontend/packages/swarm-ui/package.json @@ -6,8 +6,7 @@ "type": "module", "scripts": { "build": "node ./build.mjs", - "typecheck": "tsc --noEmit", - "test": "node --test test/*.test.mjs" + "typecheck": "tsc --noEmit" }, "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 570ad988..01757fca 100644 --- a/frontend/packages/swarm-ui/src/pages/review-settings/ClassifierSettings.css +++ b/frontend/packages/swarm-ui/src/pages/review-settings/ClassifierSettings.css @@ -1,5 +1,4 @@ -/* 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. */ +/* The settings form: the three fields on one line, 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 0d6cfaba..154eab73 100644 --- a/frontend/packages/swarm-ui/src/pages/review-settings/ClassifierSettings.tsx +++ b/frontend/packages/swarm-ui/src/pages/review-settings/ClassifierSettings.tsx @@ -1,11 +1,9 @@ -// — 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. +// — 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. 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"; @@ -22,30 +20,17 @@ import { settingsProblem, toSettings, type ClassifierSettingsBody, - type Runtime, type SettingsDraft, } from "./SettingsTypes.js"; import "./ClassifierSettings.css"; const SETTINGS = "/api/review-classifier/settings"; -const RUNTIME_OPTIONS = [ - { value: "api-key", label: "API key (claude)" }, - { value: "acp", label: "ACP (opencode)" }, +const EFFORT_OPTIONS = [ + { value: "", label: "claude's default" }, + ...EFFORTS.map((e) => ({ value: e, label: e })), ]; -// 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); @@ -116,12 +101,10 @@ export function ClassifierSettings() { return (

- 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. + 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.

{loadError ? ( void save(e)}>
- - edit({ ...draft, runtime: runtime as Runtime }) - } + edit({ ...draft, model })} /> - {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(); @@ -124,38 +52,16 @@ 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 d.runtime === "acp" ? acpProblem(d) : null; + return null; } -/** 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 = { +/** The draft as the request body. Call only when `settingsProblem` is null. */ +export function toSettings(d: SettingsDraft): ClassifierSettingsBody { + return { 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`. */ @@ -165,20 +71,9 @@ export function changed( ): boolean { if (saved === null) return true; const now = draftOf(saved); - const base = + return ( d.model.trim() !== now.model || d.effort !== now.effort || - 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) + d.max.trim() !== now.max ); } diff --git a/frontend/packages/swarm-ui/test/settings-types.test.mjs b/frontend/packages/swarm-ui/test/settings-types.test.mjs deleted file mode 100644 index e76cddfa..00000000 --- a/frontend/packages/swarm-ui/test/settings-types.test.mjs +++ /dev/null @@ -1,119 +0,0 @@ -// 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 1bcf7fc3..6314207e 100644 --- a/nix/checks.nix +++ b/nix/checks.nix @@ -369,10 +369,6 @@ 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 '';