feat(backend): add the canonical LLM catalog file and read cache - #13622
feat(backend): add the canonical LLM catalog file and read cache#13622ntindle wants to merge 4 commits into
Conversation
Part 2 of 5 of the catalog-as-code stack. backend/data/llm_registry/ catalog.py IS the model database: 85 models / 8 providers / 17 creators / 4 copilot routing cells, generated from the current MODEL_METADATA, MODEL_COST, TOKEN_COST, and ChatConfig defaults so file == reality on day one. Updates happen by PR (catalog-only diffs may ride hotfix-> master for CD-speed changes); git history is the audit log. - catalog_model.py: validated schema — per-model costs (flat credits + per-1M token rates), visibility (GA/EMPLOYEES/ADMINS/HIDDEN), min_subscription_tier, fallback slugs, routing (surface->mode->tier) - registry.py: load_catalog() builds the in-process read cache at startup; same read interface the copilot resolver (part 3), public endpoint (part 4), and Phase B consumers use. No Redis, no pub/sub, no DB — the file only changes at deploy, so coherence is free - forever-guard tests: parse, unique slugs, referential integrity (providers/creators/fallbacks/routing cells), cost bounds - cost-drift tripwires: catalog costs must equal MODEL_COST/TOKEN_COST in both directions until Phase B3 flips the reader and deletes the dicts — centralizing costs now cannot silently diverge - lifespan: fail-soft load; empty catalog degrades to pre-catalog behavior Co-authored-by: Bentlybro <Github@bentlybro.com> Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
github-project-automation
Bot
moved this to 🆕 Needs initial review
in AutoGPT development kanban
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. 🗂️ Base branches to auto review (1)
Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## feat/llm-catalog-schema #13622 +/- ##
========================================================
Coverage 76.23% 76.23%
========================================================
Files 2698 2698
Lines 205494 205494
Branches 19703 19704 +1
========================================================
+ Hits 156656 156661 +5
+ Misses 44494 44490 -4
+ Partials 4344 4343 -1
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
…ruction Pre-review-gate findings B2(part)/S1: - routing cells now carry the vendor-prefixed DOT-form slugs (anthropic/claude-sonnet-4.6) — the spelling OpenRouter actually serves; bare dashed canonical slugs 404 there. New spelling-convention guard test; the reference guard is now slug-tolerant like the router - catalog construction moved out of import time (get_catalog build-once accessor): a bad literal now degrades fail-soft in load_catalog callers instead of ImportError-crashing every process — the bad- catalog vector IS the fast-edit lane, so this matters Co-authored-by: Bentlybro <Github@bentlybro.com> Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
/review |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 61b10ec. Configure here.
Populated default cells (seeded from ChatConfig CODE defaults) would have silently shadowed the CHAT_*_MODEL env config of any deployment whose env differs from code defaults — including prod — the moment this deployed, and would flip models during LD outages (fall-through lands on the cell, not the env value). Cells now start empty: env stays authoritative for a (mode, tier) until an operator claims that cell in a catalog PR. Reference/spelling guards still govern any cells added. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
/review |
|
…with context_window cursor-bot medium on #13622: None means unknown/no published cap; substituting context_window published wrong output limits through the catalog endpoint (a 1M-context model does not emit 1M output tokens). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
/review |
![autogpt-pr-reviewer[bot]](https://avatars.githubusercontent.com/u/130738209?s=60&v=4){kind=small}
There was a problem hiding this comment.
📋 Automated Review — PR #13622
PR #13622 — feat(backend): add the canonical LLM catalog file and read cache
Author: ntindle | Files: 8
🎯 Verdict: REQUEST_CHANGES
PR Description Quality
routing={} (zero cells). Update the description to match the intentionally-empty routing matrix before merge (product).
What This PR Does
Introduces a canonical, schema-validated "catalog-as-code" file (catalog.py) describing ~85 LLM models across 8 providers/17 creators, a Pydantic schema (catalog_model.py), and an in-process build-once/read-many cache (registry.py). The cache is wired fail-soft into the FastAPI lifespan so a broken catalog degrades to pre-catalog behavior instead of crashing startup. Nothing consumes the cache yet — it is dormant by design, with consumers deferred to Phase B.
Specialist Findings
🛡️ Security _build_schema_options) filters only on is_enabled and ignores visibility/min_subscription_tier, so a HIDDEN/EMPLOYEES/ADMINS enabled model would be surfaced as GA to any future consumer (registry.py:166).
🟠 Latent authorization/data-exposure gap — must be closed when the first user-facing consumer lands.
🏗️ Architecture registry.py:210 vs _build_schema_options), a reverse-integrity gap in the cost tripwire (catalog_test.py:118), triple-duplicated slug-resolver logic, and mutable singleton globals.
🟠 Sort key mismatch and the enabled-slug↔transport integrity gap should be tightened before consumers depend on this surface.
⚡ Performance ✅ — Sound build-once/read-many design; O(1) get_model/get_route lookups at trivial scale (n≈85). Several accessors (get_default_model_slug, get_enabled_models, get_all_model_slugs_for_validation) recompute derived views O(n)/O(n log n) per call instead of materializing once at load like _schema_options — microseconds today, cheap to fix before consumers land.
🧪 Testing rest_api.py:165), half of RegistryModel's field mapping is unverified (registry_test.py:78), and three routing/fallback "guard" tests are vacuous — they loop over the empty routing={} and cannot fail today.
🟠 Missing negative/degradation coverage on a billing-adjacent path.
📖 Quality ✅ — Grade A-. Excellent docstrings that explain why, well-typed Pydantic models. Minor DRY (_slug_candidates duplicated in catalog_test.py:54) and the same sort-key inconsistency.
📦 Product supports_* capabilities default all-False (catalog_model.py:88) — confirm these were genuinely absent upstream so a future picker doesn't misreport model capabilities.
📬 Discussion ✅ — 27/27 checks green, no merge conflicts, sole Cursor Bugbot thread resolved + outdated (the "wrong max output fallback" claim is a false positive refuted by code + test_null_max_output_tokens_is_preserved). Note: this is a stacked PR on feat/llm-catalog-schema (#13621 must merge first); CodeRabbit auto-review was skipped due to the non-default base, and there is no human sign-off yet.
🔎 QA ✅ — Ran the code live in the rest_server container: startup log confirms LLM catalog loaded: 85 models, 85 schema options, 0 routing cells; all 39 tests pass on independent re-run; cost-drift tripwires match live MODEL_COST/TOKEN_COST dicts in both directions and trip on injected drift; fail-soft degradation returns None/[] without crashing.
🟠 Should Fix
- Fail-soft catalog load is untested (
rest_api.py:165) — the headline "degrades on broken catalog" behavior has no test. Monkeypatchload_catalogto raise and assert the exception is swallowed, a warning logs, andget_all_models()returns[]. (Flagged by: testing — 1) - Default-model sort disagrees with dropdown sort (
registry.py:213) —get_default_model_slugsorts bydisplay_name(case-sensitive) while_build_schema_optionssorts bydisplay_name.lower(); the computed default can differ from the visually-first picker entry once >1 model is enabled/recommended. Unify on.lower(). (Flagged by: architect, performance, quality, product — 4 specialists) - Read interface ignores
visibility/min_subscription_tier(registry.py:166) — restricted (HIDDEN/EMPLOYEES/ADMINS) enabled models surface as GA, andget_all_model_slugs_for_validationwould accept them as user-supplied ids. Filter on visibility (or take a caller-supplied floor) and add a test asserting a restricted model is excluded. (Flagged by: security — 1) - RegistryModel field propagation half-verified (
registry_test.py:78) —visibility,min_subscription_tier,fallback_model_slug,kind, and thesupports_*/capabilities/extra_metadatafields copied by_build_modelsare never asserted; a mapping regression would pass silently. Load a model with all non-default fields set and assert each survives the join. (Flagged by: testing — 1) - Vacuous routing/fallback guard tests (
catalog_test.py:47,:66,:130) — three guards loop over the emptyrouting={}/unsetfallback_model_slug, so_resolve_cell/tolerant_matchare never exercised. Add seeded-payload tests so the resolver logic is proven now, not only when someone edits the catalog. (Flagged by: testing, architect, quality — 3 specialists) - Reverse cost-drift tripwire only checks costed models (
catalog_test.py:118) — an enabled slug with a typo and no cost is never cross-checked against a real transport identifier, yet slugs are sent to providers nearly verbatim. Assert every enabled catalog slug resolves to a knownLlmModel(or an explicit allow-list). (Flagged by: architect — 1)
🟡 Nice to Have
- Precompute read views at load time (
registry.py:197,:211,:228) — materialize enabled-models / default-slug / validation-frozenset once inload_catalog()before Phase-B consumers put these on request paths. (performance) - Encapsulate singleton globals in a swappable registry object (
registry.py:110) — removes the autouse test-restore fixture and enables clean reload/injection; also makes the multi-global swap atomic. (architect, performance) - Confirm
supports_*capability defaults (catalog_model.py:88) — all-Falsemay misreport capable models when a future picker surfaces them. (product)
🔵 Nits
- Extract
_slug_candidates(value)helper (catalog_test.py:54) — candidate-expansion logic is duplicated verbatim in_resolve_cellandtolerant_match. (architect, quality) - Constrain
kindto aLiteral(catalog_model.py:76) — free-formstrwhile sibling enumerated dimensions areLiteral-gated. (architect) - Fix "4 routing cells" in PR description — code ships
routing={}. (product) - Test the 2000-model boundary (
catalog_model_test.py:88) — only 2001 (reject) is checked, not exactlyMAX_CATALOG_MODELS(accept). (testing)
QA Screenshots
| Screenshot | Description |
|---|---|
 |
Frontend loads healthy after the new fail-soft startup wiring ✅ |
Human Review Needed
YES — This centralizes billing-relevant pricing data and adds startup lifespan wiring; CodeRabbit was skipped (non-default base) and there is no human sign-off yet, so a human should eyeball the pricing centralization before merge.
Risk Assessment
Merge risk: LOW | Rollback: EASY — the code is dormant (nothing consumes it), fail-soft, and lives in a self-contained new package; reverting is a clean file removal.
CI Status
❌ 1 check failed — frontend pnpm test:unit failed, but this is a backend-only PR with no frontend source changes, so the failure is unrelated (likely flaky/pre-existing) and should be confirmed against dev. All other checks pass: frontend lint/types/build ✅, backend lint ✅, and per the discussion specialist 27/27 GitHub checks are green. The 39 new backend tests pass on independent re-run.
UI Testing — Variant Results
✅ local: Backend-only dormant catalog package: startup load fires fail-soft, read interface returns 85 models, all 39 tests pass on independent re-run, and cost-drift tripwires match live billing dicts and trip on injected drift.
✅ hosted: Dormant backend catalog PR verified live: catalog loads 85 models at startup, 39/39 tests pass, cost-drift is zero across 137 priced entries, negative/schema validation and fail-soft all behave correctly.
|
|
||
| def load_catalog(payload: CatalogPayload | None = None) -> None: | ||
| """Build the L1 structures from the catalog file. Called at startup. | ||
|
|
There was a problem hiding this comment.
🤖 🟡 medium (security/authorization / data exposure)
_build_schema_options() (backing get_schema_options) filters only on is_enabled, not on visibility. A model with is_enabled=True and visibility=HIDDEN/EMPLOYEES/ADMINS is returned in the user-facing dropdown options, contradicting the schema's documented contract that HIDDEN serves-but-is-never-shown. Any Phase-B consumer trusting this as user-facing would leak internal/pre-launch models.
Suggestion: Filter on visibility == 'GA' (or accept a caller-supplied visibility floor) in _build_schema_options; add a test asserting a HIDDEN enabled model is excluded from schema options.
|
|
||
|
|
||
| def get_route(surface: str, mode: str, tier: str) -> str | None: | ||
| """Return the catalog's routing-cell slug, if the cell is set. |
There was a problem hiding this comment.
🤖 🟢 low (security/authorization / input validation)
get_all_model_slugs_for_validation() returns every enabled slug regardless of visibility or min_subscription_tier. Its stated purpose is validating user-supplied model ids, so a caller using it as an allow-list would accept EMPLOYEES/ADMINS-only or higher-tier models from a regular user once Phase B wires it up.
Suggestion: When enforcement lands, have this function apply the caller's allowed visibility and subscription tier; add tests covering restricted models being rejected.
|
|
||
| class CatalogCreator(BaseModel): | ||
| model_config = ConfigDict(frozen=True) | ||
|
|
There was a problem hiding this comment.
🤖 🟢 low (security/input validation)
_SLUG_PATTERN permits '/', ':', '.', '-' and slugs/routing cells are forwarded to provider transports nearly verbatim. There is no runtime allow-list on the outbound model string; safety rests entirely on PR review of catalog.py, which now includes a hotfix->master fast lane for catalog-only diffs.
Suggestion: Keep the forever-guard and cost-drift tripwire tests as required status checks on the catalog fast-lane so a single reviewed file cannot silently change what strings are sent to external LLM APIs or what users are billed.
| """Model-selection dropdown options (enabled models only).""" | ||
| return list(_schema_options) | ||
|
|
||
|
|
There was a problem hiding this comment.
🤖 🟡 medium (architect/determinism/consistency)
get_default_model_slug() sorts models by display_name (case-sensitive), while _build_schema_options() sorts by display_name.lower(). With more than one recommended/enabled model these two orderings can disagree, so the computed default may differ from the picker's first entry.
Suggestion: Use a single shared sort key (e.g. display_name.lower()) in both get_default_model_slug and _build_schema_options.
|
|
||
| def test_cost_drift_tripwire_reverse_direction(): | ||
| """Every catalog cost corresponds to a real enum model still priced in | ||
| the dicts — a model removed from code must not keep a ghost price here.""" |
There was a problem hiding this comment.
🤖 🟡 medium (architect/data-integrity)
The reverse cost-drift/enum tripwire only validates models where cost is not None. An enabled catalog entry with a bogus or typo'd slug and no cost is never cross-checked against a real LlmModel/transport identifier, yet these slugs are sent to providers nearly verbatim.
Suggestion: Add a guard asserting every enabled catalog slug resolves to a known LlmModel enum member (or an explicit catalog-only allow-list), independent of whether the model carries a cost.
| provider: str | ||
| context_window: int | ||
| max_output_tokens: int | None | ||
| display_name: str |
There was a problem hiding this comment.
🤖 🟢 low (quality/data model redundancy)
RegistryModelMetadata.display_name (L41) and provider_name (L42) duplicate RegistryModel.display_name (L63) and provider_display_name (L68); both are populated from the same values, giving two ways to read one fact.
Suggestion: Either drop the nested duplicates, or add a one-line comment noting the mirror is required for block-schema compatibility to prevent a future 'fix'.
|
|
||
| def get_default_model_slug() -> str | None: | ||
| """First recommended enabled model, else first enabled model.""" | ||
| models = sorted(_dynamic_models.values(), key=lambda m: m.display_name) |
There was a problem hiding this comment.
🤖 🟢 low (quality/consistency)
get_default_model_slug sorts by m.display_name (case-sensitive) while _build_schema_options sorts by m.display_name.lower(); ordering of the same collection can diverge by capitalization.
Suggestion: Use a single consistent sort key (lowercased) in both functions.
| # shadow the CHAT_*_MODEL env config of any deployment whose env | ||
| # differs from code defaults, and would flip models during LD | ||
| # outages. Cells apply on cloud deployments (behave_as=CLOUD) only. | ||
| routing={}, |
There was a problem hiding this comment.
🤖 🟢 low (product/requirements-mismatch)
PR description claims '4 copilot routing cells' but the catalog ships routing={} (zero cells). The behavior is intentional and well-justified in the inline comment, but the description misrepresents what shipped.
Suggestion: Update the PR description to say routing cells ship empty (env stays authoritative), matching the code and the Cursor summary.
|
|
||
| def get_default_model_slug() -> str | None: | ||
| """First recommended enabled model, else first enabled model.""" | ||
| models = sorted(_dynamic_models.values(), key=lambda m: m.display_name) |
There was a problem hiding this comment.
🤖 🟢 low (product/ux-consistency)
get_default_model_slug() sorts by display_name (case-sensitive) while get_schema_options() sorts by display_name.lower(). The default-selected model can differ from the visually first dropdown option once fallback-to-first-enabled kicks in.
Suggestion: Sort both by display_name.lower() so the default model matches the top of the picker list.
| # the pre-launch testing state. | ||
| visibility: Literal["GA", "EMPLOYEES", "ADMINS", "HIDDEN"] = "GA" | ||
| # Null = available on every subscription tier. Enforcement lands with | ||
| # the registry-driven picker (Phase B). |
There was a problem hiding this comment.
🤖 🟢 low (product/data-accuracy)
supports_tools/json_output/reasoning/parallel_tool_calls default to False and no catalog model overrides them, so capable models (e.g. gpt-4o, claude-*) will publish as non-capable when a future picker/catalog endpoint surfaces these flags.
Suggestion: Confirm these capabilities were genuinely absent in the live source data; if not, populate them so the eventual user-facing catalog does not misreport model capabilities.
github-project-automation
Bot
moved this from 🆕 Needs initial review
to 🚧 Needs work
in AutoGPT development kanban
|
Consolidated into #13627 — the stack now lands as a single PR that also performs the cutover (the catalog replaces the hand-maintained dicts in the same diff, per review feedback that dormant-then-flip phasing added process without adding safety). All commits from this branch are contained in the consolidated branch. |
github-project-automation
Bot
moved this from 🚧 Needs work
to ✅ Done
in AutoGPT development kanban
1 participant
Why
Part 2 of 5 of the catalog-as-code stack (#13621 → this). Model definitions today are spread across the
LlmModelenum,MODEL_METADATA,MODEL_COST/TOKEN_COST, an LD flag, and env vars — five places, no shared schema, nothing validating cross-references. This PR creates the one canonical, schema-validated, PR-reviewed file that centralizes them, updated through git and propagated by CD (catalog-only diffs may ride hotfix→master; the/reviewbot covers the fast lane).What
catalog.py— the database: 85 models / 8 providers / 17 creators / 4 copilot routing cells, generated from current dev code so file == reality on day one (costs mirrorMODEL_COST/TOKEN_COST; routing cells mirror theChatConfigdefaults in canonical slug form;gpt-5.2recommended perDEFAULT_LLM_MODEL)catalog_model.py— the validated schema: per-model costs, capabilities, visibility (GA/EMPLOYEES/ADMINS/HIDDEN), tier gating, fallback slugs, routing cellsregistry.py—load_catalog()builds an in-process read cache at startup behind the stable read interface (get_model,get_enabled_models,get_route,get_schema_options, …) that parts 3–4 and Phase B consume. No Redis, no pub/sub, no DB: the file only changes at deploy, so cache coherence is structuralNothing consumes the cache yet — dormant by design, same as the schema PR.
Verification
poetry run format+poetry run lintcleanChecklist
Note
Medium Risk
Touches centralized model metadata and credit pricing for all future LLM routing/billing consumers, but startup load is fail-soft and no code paths consume the cache yet; cost tripwires limit silent billing drift.
Overview
Introduces catalog-as-code for LLMs: a new
backend.data.llm_registrypackage with a validated canonicalcatalog.py(~85 models, providers/creators, per-model credit costs, empty routing matrix for rollout-safe env defaults),catalog_model.pyschema, andregistry.pythat builds an in-process L1 cache (get_model,get_enabled_models,get_route,get_schema_options, etc.) at startup.rest_apilifespan now callsload_catalog()inside a fail-soft try/except so a bad catalog logs a warning and leaves the registry empty instead of crashing the API.Adds forever-guard tests (unique slugs, FK refs, routing cell rules) and bidirectional cost-drift tripwires against
MODEL_COST/TOKEN_COSTso catalog pricing cannot diverge from billing until Phase B3. Nothing reads the cache in production paths yet—dormant until follow-up PRs wire copilot, catalog API, and blocks.Reviewed by Cursor Bugbot for commit 995cae5. Bugbot is set up for automated code reviews on this repo. Configure here.