Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 12 additions & 4 deletions src/valcore/api/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import importlib
from importlib.resources import files as _package_files
from pathlib import Path
from typing import Any

from fastapi import FastAPI, Request
from fastapi.middleware.cors import CORSMiddleware
Expand All @@ -24,7 +25,7 @@
ValcoreError,
)
from valcore.models import VALID_CAPABILITIES
from valcore.settings import MODEL_CATALOG
from valcore.settings import get_settings, model_catalog
from valcore.tools import tool_names
from valcore.tracing import configure_tracing, instrument_app

Expand Down Expand Up @@ -159,10 +160,17 @@ async def health() -> dict[str, str]:
return {"status": "ok"}

@app.get("/api/config")
async def config() -> dict[str, list[str]]:
"""Return the pickers the SPA needs: models, tools, and capabilities."""
async def config() -> dict[str, Any]:
"""Return the pickers the SPA needs: models, tools, and capabilities.

``models`` is a suggestion list for type-ahead, not a closed set -- the SPA
accepts any well-formed ``gateway/<route>:<name>`` string. ``default_model``
is sent separately because the catalog is sorted alphabetically, so its first
entry is arbitrary and not a sensible default.
"""
return {
"models": list(MODEL_CATALOG),
"models": list(model_catalog()),
"default_model": get_settings().default_model,
"tools": tool_names(),
"capabilities": sorted(VALID_CAPABILITIES),
}
Expand Down
26 changes: 19 additions & 7 deletions src/valcore/settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,13 +29,25 @@
"gateway/groq",
)

MODEL_CATALOG: list[str] = [
"gateway/anthropic:claude-sonnet-5",
"gateway/anthropic:claude-opus-4-5",
"gateway/anthropic:claude-haiku-4-5",
"gateway/openai:gpt-5",
"gateway/google:gemini-2.5-pro",
]

@functools.lru_cache
def model_catalog() -> tuple[str, ...]:
"""Return the gateway-routable model names known to the pinned ``pydantic-ai``.

These are type-ahead suggestions, not a whitelist -- ``validate_model_string``
accepts any well-formed ``gateway/<route>:<name>`` string and leaves it to the
Gateway to reject names it does not serve. Filtering through ``GATEWAY_ROUTES``
(rather than a bare ``gateway/`` prefix) keeps the two in step: every suggestion
is a string ``validate_model_string`` accepts.

``pydantic_ai.models`` is imported here rather than at module scope because it
costs ~235ms to import, and every CLI invocation loads this module while only the
API's config endpoint needs the catalog.
"""
from pydantic_ai.models import known_model_names

prefixes = tuple(f"{route}:" for route in GATEWAY_ROUTES)
return tuple(sorted(n for n in known_model_names() if n.startswith(prefixes)))


class _TomlConfigSource(PydanticBaseSettingsSource):
Expand Down
18 changes: 18 additions & 0 deletions tests/test_api_skeleton.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
ReferencedError,
ValcoreError,
)
from valcore.settings import get_settings


@pytest.fixture
Expand Down Expand Up @@ -58,6 +59,23 @@ async def test_config_populated() -> None:
assert payload["capabilities"]


@pytest.mark.anyio
async def test_config_default_model_is_not_merely_the_head_of_the_catalog() -> None:
"""The SPA seeds new versions from ``default_model``, never ``models[0]``.

The catalog is sorted alphabetically, so its first entry is an arbitrary Bedrock
model. This pins the two apart so a regression to ``models[0]`` is caught here
rather than by a user wondering why their evaluator picked a model they never chose.
"""
app = create_app()
async with _client(app) as client:
payload = (await client.get("/api/config")).json()

assert payload["default_model"] == get_settings().default_model
assert payload["default_model"] in payload["models"]
assert payload["default_model"] != payload["models"][0]


@pytest.mark.anyio
@pytest.mark.parametrize(
("error", "status", "type_name"),
Expand Down
48 changes: 48 additions & 0 deletions tests/test_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -362,3 +362,51 @@ def test_logfire_extra_is_present_in_dev_environment() -> None:
failing loudly here.
"""
assert importlib.util.find_spec("logfire") is not None


# --- model catalog -----------------------------------------------------------------


def test_model_catalog_is_derived_and_covers_every_gateway_route() -> None:
"""The catalog comes from pydantic-ai, not a hand-maintained literal.

The old five-entry list went stale silently. Asserting a floor well above it,
plus coverage of every route, means a future pydantic-ai bump that drops a
provider fails here instead of quietly shrinking the picker.
"""
catalog = settings.model_catalog()

assert len(catalog) > 100
routes_seen = {name.split(":", 1)[0] for name in catalog}
assert routes_seen == set(settings.GATEWAY_ROUTES)


def test_every_catalog_entry_passes_model_validation() -> None:
"""The catalog and the validator cannot drift apart.

`model_catalog` filters through GATEWAY_ROUTES precisely so that suggesting a
model the validator would then reject is impossible.
"""
for name in settings.model_catalog():
settings.validate_model_string(name)


def test_default_model_is_in_the_catalog() -> None:
assert settings.Settings().default_model in settings.model_catalog()


def test_validate_model_string_accepts_names_absent_from_the_catalog() -> None:
"""Shape is the only gate; the Gateway is the authority on what exists.

The Gateway serves more models than any pinned pydantic-ai knows about, so an
unrecognised-but-well-formed name must pass rather than block the user.
"""
unknown = "gateway/anthropic:some-model-released-tomorrow"
assert unknown not in settings.model_catalog()
settings.validate_model_string(unknown) # must not raise


def test_validate_model_string_still_rejects_malformed_names() -> None:
for bad in ("claude-sonnet-5", "gateway/nope:x", "gateway/anthropic:"):
with pytest.raises(ConfigError):
settings.validate_model_string(bad)
88 changes: 86 additions & 2 deletions web/src/components/VersionEditor.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,9 @@ vi.mock("../api/client", () => ({

const config: AppConfig = {
models: ["gateway/anthropic:claude-sonnet-5", "gateway/openai:gpt-5"],
// Deliberately not `models[0]`, so the defaulting assertions below would fail if the
// editor went back to seeding new versions from the head of the catalog.
default_model: "gateway/openai:gpt-5",
tools: ["row_get"],
capabilities: ["FileSystem", "Shell"],
};
Expand Down Expand Up @@ -186,11 +189,89 @@ describe("VersionEditor: draft mode", () => {
expect((screen.getByLabelText("Version name") as HTMLInputElement).value).toBe("");
expect((screen.getByLabelText("Instructions") as HTMLTextAreaElement).value).toBe("");
expect((screen.getByLabelText("Prompt template") as HTMLTextAreaElement).value).toBe("");
expect((screen.getByLabelText("Model") as HTMLSelectElement).value).toBe(config.models[0]);
expect((screen.getByLabelText("Model") as HTMLInputElement).value).toBe(config.default_model);
expect(screen.getByRole("button", { name: "Create version" })).not.toBeNull();
expect(screen.queryByRole("button", { name: "Save changes" })).toBeNull();
});

it("prefills every field from seedFrom, so a new version starts as an edit", async () => {
const source = makeVersion({
id: "v1",
version_name: "tone-check",
notes: "the prior notes",
model: "gateway/openai:gpt-5",
instructions: "Judge the tone.",
prompt_template: "Rate {answer}",
required_columns: ["answer"],
tools: ["row_get"],
});
render(
<VersionEditor version={null} seedFrom={source} evaluatorId="e1" config={config} />,
);

expect((screen.getByLabelText("Version name") as HTMLInputElement).value).toBe("tone-check");
expect((screen.getByLabelText("Instructions") as HTMLTextAreaElement).value).toBe(
"Judge the tone.",
);
expect((screen.getByLabelText("Prompt template") as HTMLTextAreaElement).value).toBe(
"Rate {answer}",
);
expect((screen.getByLabelText("Model") as HTMLInputElement).value).toBe(
"gateway/openai:gpt-5",
);

// Seeded, not adopted: this is still a create, so it saves via createVersion.
expect(screen.getByRole("button", { name: "Create version" })).not.toBeNull();
});

it("seeds a copy, so editing the draft cannot mutate the source version", async () => {
const source = makeVersion({ required_columns: ["answer"], tools: ["row_get"] });
const before = JSON.stringify(source);
const user = userEvent.setup();
render(
<VersionEditor version={null} seedFrom={source} evaluatorId="e1" config={config} />,
);

await user.clear(screen.getByLabelText("Instructions"));
await user.type(screen.getByLabelText("Instructions"), "Different.");

expect(JSON.stringify(source)).toBe(before);
});

it("falls back to a blank form when there is no seed", () => {
render(<VersionEditor version={null} evaluatorId="e1" config={config} />);

expect((screen.getByLabelText("Version name") as HTMLInputElement).value).toBe("");
expect((screen.getByLabelText("Instructions") as HTMLTextAreaElement).value).toBe("");
});

it("accepts a model the catalog does not list", async () => {
// The Gateway serves more models than the pinned pydantic-ai knows about, so the
// field must take a well-formed name that is absent from the suggestions.
const user = userEvent.setup();
render(<VersionEditor version={null} evaluatorId="e1" config={config} />);

const field = screen.getByLabelText("Model") as HTMLInputElement;
const unlisted = "gateway/groq:llama-4-maverick";
expect(config.models).not.toContain(unlisted);

await user.clear(field);
await user.type(field, unlisted);

expect(field.value).toBe(unlisted);
});

it("offers the catalog as suggestions without constraining the field", () => {
render(<VersionEditor version={null} evaluatorId="e1" config={config} />);

const field = screen.getByLabelText("Model") as HTMLInputElement;
expect(field.tagName).toBe("INPUT");
expect(field.getAttribute("list")).toBeTruthy();

const options = document.querySelectorAll(`#${field.getAttribute("list")} option`);
expect([...options].map((o) => o.getAttribute("value"))).toEqual(config.models);
});

it("disables Save and shows an inline error for an incomplete draft", () => {
render(<VersionEditor version={null} evaluatorId="e1" config={config} />);

Expand Down Expand Up @@ -307,7 +388,10 @@ describe("VersionEditor: sectioned layout", () => {
expect(screen.getByText(/read-only/i)).not.toBeNull();
expect((screen.getByLabelText("Version name") as HTMLInputElement).readOnly).toBe(true);
expect((screen.getByLabelText("Instructions") as HTMLTextAreaElement).readOnly).toBe(true);
expect((screen.getByLabelText("Model") as HTMLSelectElement).disabled).toBe(true);
// `readOnly`, not `disabled`: the old control was a <select>, which has no readOnly
// attribute. As an input it now matches its siblings and stays selectable, so a frozen
// version's model string can still be copied.
expect((screen.getByLabelText("Model") as HTMLInputElement).readOnly).toBe(true);
});
});

Expand Down
35 changes: 29 additions & 6 deletions web/src/components/VersionEditor.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,10 @@ import type { VersionErrors } from "./versionValidation";
import { Badge, Button, ErrorBanner, Spinner } from "./ui";

export type AppConfig = {
/** Type-ahead suggestions, not a closed set -- any `gateway/<route>:<name>` is accepted. */
models: string[];
/** Seed for new versions. The catalog is sorted, so `models[0]` is not a sensible default. */
default_model: string;
tools: string[];
capabilities: string[];
};
Expand Down Expand Up @@ -55,6 +58,8 @@ type VersionEditorProps = {
config: AppConfig;
evaluatorName?: string;
initialDraft?: GeneratedConfig;
/** Prefill a new draft from this version, so "New version" starts as an edit, not a blank slate. */
seedFrom?: EvaluatorVersion | null;
onCreateDraft?: (version: Partial<EvaluatorVersion>) => Promise<EvaluatorVersion>;
onSaved?: (version: EvaluatorVersion) => void;
};
Expand Down Expand Up @@ -82,7 +87,7 @@ function blankForm(config: AppConfig): FormState {
return {
version_name: "",
notes: "",
model: config.models[0] ?? "",
model: config.default_model,
instructions: "",
prompt_template: "",
required_columns: [],
Expand All @@ -97,11 +102,28 @@ function blankForm(config: AppConfig): FormState {
};
}

// Which form a mount starts from, in precedence order: an existing version being edited, a
// generated draft, the version the user was looking at when they hit "New version", and only
// then an empty form. Seeding from `seedFrom` mirrors the server's `copy_version` (the frozen
// "save as new version" path), so both routes to a new version behave the same -- including
// carrying `version_name` over verbatim, which the server does not constrain to be unique.
function initialForm(
version: EvaluatorVersion | null,
initialDraft: GeneratedConfig | undefined,
seedFrom: EvaluatorVersion | null | undefined,
config: AppConfig,
): FormState {
if (version) return toForm(version);
if (initialDraft) return generatedForm(initialDraft, config);
if (seedFrom) return toForm(seedFrom);
return blankForm(config);
}

function generatedForm(draft: GeneratedConfig, config: AppConfig): FormState {
return {
version_name: draft.version_name,
notes: "",
model: config.models[0] ?? "",
model: config.default_model,
instructions: draft.instructions,
prompt_template: draft.prompt_template,
required_columns: [...draft.required_columns],
Expand Down Expand Up @@ -171,22 +193,23 @@ export function VersionEditor({
config,
evaluatorName,
initialDraft,
seedFrom,
onCreateDraft,
onSaved,
}: VersionEditorProps) {
const [form, setForm] = useState<FormState>(() =>
version ? toForm(version) : initialDraft ? generatedForm(initialDraft, config) : blankForm(config),
initialForm(version, initialDraft, seedFrom, config),
);
const [error, setError] = useState<unknown>(null);
const [saving, setSaving] = useState(false);
const [columnDraft, setColumnDraft] = useState("");

useEffect(() => {
setForm(
version ? toForm(version) : initialDraft ? generatedForm(initialDraft, config) : blankForm(config),
);
setForm(initialForm(version, initialDraft, seedFrom, config));
setError(null);
// config is stable for the lifetime of an editor; only a version swap resets the form.
// `seedFrom` is deliberately excluded: it seeds the initial draft, and re-running on a
// parent re-render would discard whatever the user has typed since.
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [version, initialDraft]);

Expand Down
Loading
Loading