mirror of
https://github.com/block/buzz.git
synced 2026-08-18 06:50:31 +02:00
fix(catalog): route all model rows through the shared label resolver
Two discovered-row callsites still formatted labels inline, so a known Databricks endpoint returned by discovery without a name rendered as its raw ID instead of the curated registry name — the exact inconsistency the shared resolver exists to prevent. The generator also trusted the models.dev payload shape: a non-object model value silently emitted `id -> id`, baking a wrong label into both committed artifacts where no test could distinguish it from a genuine pass-through. Validation now rejects that instead. Registry parity is pinned across the whole table rather than five sampled rows, so a half-committed regenerate cannot pass. Co-authored-by: Will Pfleger <pfleger.will@gmail.com> Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
This commit is contained in:
co-authored by
Will Pfleger
parent
38558504ca
commit
be7d66ff3b
@@ -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,
|
||||
/<DropdownMenuRadioItem[\s\S]*?\{resolveModelLabel\(model\.id, model\.name\)\}/,
|
||||
"dropdown rows must resolve labels through resolveModelLabel, not raw model.name/model.id",
|
||||
);
|
||||
});
|
||||
|
||||
@@ -240,7 +240,7 @@ export function ModelPicker({
|
||||
>
|
||||
{modelsData.models.map((model) => (
|
||||
<DropdownMenuRadioItem key={model.id} value={model.id}>
|
||||
{model.name ?? model.id}
|
||||
{resolveModelLabel(model.id, model.name)}
|
||||
</DropdownMenuRadioItem>
|
||||
))}
|
||||
</DropdownMenuRadioGroup>
|
||||
|
||||
@@ -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" },
|
||||
]);
|
||||
});
|
||||
|
||||
@@ -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),
|
||||
})),
|
||||
];
|
||||
}
|
||||
|
||||
@@ -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)
|
||||
|
||||
Reference in New Issue
Block a user