diff --git a/desktop/src/features/agents/lib/databricksModelNames.test.mjs b/desktop/src/features/agents/lib/databricksModelNames.test.mjs index dface2f37..2f7cec41d 100644 --- a/desktop/src/features/agents/lib/databricksModelNames.test.mjs +++ b/desktop/src/features/agents/lib/databricksModelNames.test.mjs @@ -1,4 +1,5 @@ import assert from "node:assert/strict"; +import { readFileSync } from "node:fs"; import test from "node:test"; import { DATABRICKS_MODEL_NAMES } from "./databricksModelNames.ts"; @@ -137,24 +138,67 @@ test("DATABRICKS_MODEL_NAMES — is a Map (not a plain object — prototype-key }); // --------------------------------------------------------------------------- -// Rust/TS parity: spot-check representative entries from the generated Rust slice -// The generator emits both files from the same source, so any key present in Rust -// must also be present in the TS Map with the same value. +// Rust/TS parity: every entry in the committed Rust slice must appear in the TS +// Map with the same value, and neither side may carry an entry the other lacks. +// The generator emits both files from one models.dev fetch, so any divergence +// means a hand-edit or a half-committed regenerate. // --------------------------------------------------------------------------- -test("DATABRICKS_MODEL_NAMES — parity spot-check: representative entries match Rust slice values", () => { - const expected = [ - ["databricks-gpt-5-5", "GPT-5.5"], - ["databricks-claude-opus-4-7", "Claude Opus 4.7"], - ["databricks-gpt-oss-120b", "GPT OSS 120B"], - ["databricks-claude-sonnet-4-5", "Claude Sonnet 4.5 (latest)"], - ["databricks-gemini-2-5-flash", "Gemini 2.5 Flash"], - ]; - for (const [id, name] of expected) { - assert.equal( - DATABRICKS_MODEL_NAMES.get(id), - name, - `TS registry entry for '${id}' must match Rust registry`, - ); - } +/** + * Parses `(id, name)` pairs out of the generated Rust slice. rustfmt wraps + * long tuples across lines, so the source is matched as one string rather + * than line by line. + */ +function parseRustRegistry(source) { + const body = source.slice( + source.indexOf("&[", source.indexOf("DATABRICKS_MODEL_NAMES")), + ); + const pair = /\(\s*"((?:[^"\\]|\\.)*)"\s*,\s*"((?:[^"\\]|\\.)*)"\s*,?\s*\)/g; + const unescapeRust = (value) => value.replace(/\\(["\\])/g, "$1"); + return new Map( + [...body.matchAll(pair)].map(([, id, name]) => [ + unescapeRust(id), + unescapeRust(name), + ]), + ); +} + +test("DATABRICKS_MODEL_NAMES — full-table parity with the committed Rust registry", () => { + const rustEntries = parseRustRegistry( + readFileSync( + new URL( + "../../../../../crates/buzz-agent/src/databricks_model_names.rs", + import.meta.url, + ), + "utf8", + ), + ); + + assert.ok(rustEntries.size > 0, "Rust registry parsed as empty — bad parse"); + assert.deepEqual( + [...DATABRICKS_MODEL_NAMES.entries()].sort(), + [...rustEntries.entries()].sort(), + "TS and Rust registries must be identical — rerun scripts/generate-databricks-model-names.py and commit both files", + ); +}); + +// --------------------------------------------------------------------------- +// Resolver universality: every surface that renders a model label must go +// through resolveModelLabel. The ModelPicker dropdown rows live inside a Radix +// portal that renders nothing under renderToStaticMarkup (verified: even with +// forceMount the markup is ""), so the callsite is pinned at the source level +// — the same approach motion.test.mjs uses for CSS it cannot execute. +// --------------------------------------------------------------------------- + +test("ModelPicker — discovered rows render through resolveModelLabel", () => { + const source = readFileSync( + new URL("../ui/ModelPicker.tsx", import.meta.url), + "utf8", + ); + + assert.match( + source, + / {modelsData.models.map((model) => ( - {model.name ?? model.id} + {resolveModelLabel(model.id, model.name)} ))} diff --git a/desktop/src/features/agents/ui/usePersonaModelDiscovery.test.mjs b/desktop/src/features/agents/ui/usePersonaModelDiscovery.test.mjs index ecb36a6fc..f209a7c87 100644 --- a/desktop/src/features/agents/ui/usePersonaModelDiscovery.test.mjs +++ b/desktop/src/features/agents/ui/usePersonaModelDiscovery.test.mjs @@ -312,3 +312,66 @@ test("isSuccessfulEmptyDiscovery_stillPending_isFalse", () => { false, ); }); + +// ── Discovered rows resolve through the shared label resolver ──────────────── +// Discovery can return a Databricks endpoint with a null or blank `name` +// (v1 catalogs, and any harness that echoes IDs only). Those rows must still +// show the curated registry name rather than the raw endpoint ID. + +test("discoveredRow_knownDatabricksIdWithNullName_showsCuratedName", () => { + const options = getDiscoveredPersonaModelOptions( + response({ + models: [{ id: "databricks-gpt-5-5", name: null, description: null }], + }), + "", + ); + + assert.deepEqual(options.slice(1), [ + { id: "databricks-gpt-5-5", label: "GPT-5.5" }, + ]); +}); + +test("discoveredRow_knownDatabricksIdWithBlankName_showsCuratedName", () => { + const options = getDiscoveredPersonaModelOptions( + response({ + models: [ + { id: "databricks-claude-opus-4-7", name: " ", description: null }, + ], + }), + "", + ); + + assert.deepEqual(options.slice(1), [ + { id: "databricks-claude-opus-4-7", label: "Claude Opus 4.7" }, + ]); +}); + +test("discoveredRow_unknownCustomEndpointWithNoName_showsRawId", () => { + const options = getDiscoveredPersonaModelOptions( + response({ + models: [ + { id: "databricks-team-2025-01", name: null, description: null }, + ], + }), + "", + ); + + assert.deepEqual(options.slice(1), [ + { id: "databricks-team-2025-01", label: "databricks-team-2025-01" }, + ]); +}); + +test("discoveredRow_nonblankDiscoveredName_winsOverRegistry", () => { + const options = getDiscoveredPersonaModelOptions( + response({ + models: [ + { id: "databricks-gpt-5-5", name: "Workspace GPT", description: null }, + ], + }), + "", + ); + + assert.deepEqual(options.slice(1), [ + { id: "databricks-gpt-5-5", label: "Workspace GPT" }, + ]); +}); diff --git a/desktop/src/features/agents/ui/usePersonaModelDiscovery.ts b/desktop/src/features/agents/ui/usePersonaModelDiscovery.ts index 5450910b1..5966c576a 100644 --- a/desktop/src/features/agents/ui/usePersonaModelDiscovery.ts +++ b/desktop/src/features/agents/ui/usePersonaModelDiscovery.ts @@ -78,7 +78,7 @@ export function getDiscoveredPersonaModelOptions( ...defaultModelOption, ...explicitModels.map((model) => ({ id: model.id, - label: model.name?.trim() || model.id, + label: resolveModelLabel(model.id, model.name), })), ]; } diff --git a/scripts/generate-databricks-model-names.py b/scripts/generate-databricks-model-names.py index cb0437edb..43f0e5c2e 100755 --- a/scripts/generate-databricks-model-names.py +++ b/scripts/generate-databricks-model-names.py @@ -144,6 +144,59 @@ def write_ts(entries: list[tuple[str, str]]) -> None: print(f"Wrote {TS_OUT.relative_to(REPO_ROOT)}") +def extract_entries(data: object) -> list[tuple[str, str]]: + """Pull sorted (id, name) pairs out of the models.dev payload. + + Every container and leaf shape is checked explicitly so an upstream + restructure fails with an actionable message instead of a bare KeyError + or — worse — a silently degraded table where a malformed entry emits + `id -> id` and permanently masks the real curated name. + """ + if not isinstance(data, dict): + raise RuntimeError( + f"Unexpected models.dev shape: root must be an object, got {type(data).__name__}" + ) + provider = data.get("databricks") + if provider is None: + raise RuntimeError("Unexpected models.dev shape: missing data['databricks']") + if not isinstance(provider, dict): + raise RuntimeError( + "Unexpected models.dev shape: data['databricks'] must be an object, " + f"got {type(provider).__name__}" + ) + models = provider.get("models") + if models is None: + raise RuntimeError( + "Unexpected models.dev shape: missing data['databricks']['models']" + ) + if not isinstance(models, dict): + raise RuntimeError( + "Unexpected models.dev shape: data['databricks']['models'] must be an " + f"object, got {type(models).__name__}" + ) + if not models: + raise RuntimeError( + "Unexpected models.dev shape: data['databricks']['models'] is empty" + ) + + entries: list[tuple[str, str]] = [] + for model_id, model in models.items(): + where = f"data['databricks']['models'][{model_id!r}]" + if not isinstance(model, dict): + raise RuntimeError( + f"Unexpected models.dev shape: {where} must be an object, " + f"got {type(model).__name__}" + ) + name = model.get("name") + if not isinstance(name, str): + raise RuntimeError( + f"Unexpected models.dev shape: {where}['name'] must be a string, " + f"got {type(name).__name__}" + ) + entries.append((model_id, name)) + return sorted(entries) + + def main() -> None: raw = fetch(URL) try: @@ -151,16 +204,7 @@ def main() -> None: except json.JSONDecodeError as e: raise RuntimeError(f"models.dev response is not valid JSON: {e}") from e - if "databricks" not in data or "models" not in data["databricks"]: - raise RuntimeError( - "Unexpected models.dev shape: missing data['databricks']['models']" - ) - - models: dict = data["databricks"]["models"] - raw_entries = sorted( - (k, v["name"] if isinstance(v, dict) else k) for k, v in models.items() - ) - entries = validate_entries(raw_entries) + entries = validate_entries(extract_entries(data)) write_rust(entries) write_ts(entries)