refactor(starter-models): split the 2428-line catalog into one module per architecture - #31
Draft
Pfannkuchensack wants to merge 2 commits into
Draft
Conversation
… per architecture 185 definitions, the curated display order and twelve bundles lived in one file, so every new architecture appended entries to several regions of it. The definitions now live one module per architecture; only what cannot be derived from them stayed behind. Every definition moves verbatim, comments included -- read it with `git diff --color-moved`. The only text dropped is the `# region` markers, which grouped by model type inside the single file and have no meaning once the files are grouped by architecture. Two of the plan's premises did not survive contact with the data, and the design here follows the measurement instead. There are no import cycles to break. Of 97 dependency edges exactly five cross architectures -- krea-2 and z-image and sdxl-refiner each reaching for a VAE -- and they form a DAG, not a cycle. Shared components go in `common`, which imports nothing architecture-specific, and that is the whole of it. The plan proposed stable string ids specifically to break cycles that do not exist. Those ids would also have been an API change. `StarterModel`, `StarterModelWithoutDependencies` and `StarterModelBundle` are served by GET /starter_models and appear in openapi.json, so turning `dependencies` into a list of ids would have changed the schema and broken both frontends. It stays a list of model references. That also keeps dangling dependencies structurally impossible -- an unknown name is an ImportError -- which is the property the ids were meant to reconstruct. STARTER_MODELS keeps its curated order, explicitly. It follows neither base nor model type nor definition order: 179 entries in 47 contiguous runs by base, with sdxl and flux each appearing in eight separate runs. It is product data, and the file it lives in says the frontend does not re-sort it. An aggregator that concatenated the per-architecture modules would have silently reordered the model manager. So `__init__` still holds that list -- 179 names rather than 2428 lines of definitions -- and the risk this introduces, an entry defined but never listed, is what test_every_entry_is_reachable covers. `__init__` re-exports every entry, so no consumer changed. Individual models are imported by name elsewhere: `siglip` in flux_redux, three image encoders in ip_adapter, one external model in a test. Bundle keys are untouched, including that Wan has no base-keyed bundle at all -- it ships `wan_t2v` and `wan_i2v` as plain strings, next to ten keyed by BaseModelType. Those keys are serialised into the API response. Proof of equality: the catalog was fingerprinted before the split -- all 179 entries with every field, the resolved dependency graph by source, the bundles and their key types -- and compared after. Identical. Two findings the split surfaced, neither caused by it and neither fixed here. `esrgan_srx4` and `flux_kontext` are defined but listed nowhere, in no bundle, depended on by nothing and imported by nobody -- dead, and `flux_kontext` looks like something someone meant to offer. And four entries are referenced only as dependencies: nobody picks a CLIP image encoder from the model manager, it arrives with the IP-Adapter that needs it. Both groups are named in the tests so they stay decisions rather than oversights. openapi.json is unchanged. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This was referenced Aug 2, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fifth of the series from
.ideas/Backend Modularization Plan.md. No behaviour changes.185 definitions, the curated display order and twelve bundles lived in one 2428-line file, so every
new architecture appended entries to several regions of it. The definitions now live one module per
architecture.
Every definition moves verbatim, comments included — read it with
git diff --color-moved. Theonly text dropped is the
# regionmarkers, which grouped by model type inside the single file andhave no meaning once the files are grouped by architecture.
Two of the plan's premises did not survive contact with the data
There are no import cycles to break. Of 97 dependency edges, exactly five cross
architectures —
krea-2 → qwen_image_vae,z-image → flux_vae(×2),sdxl-refiner → sdxl_fp16_vae_fix— and they form a DAG. Shared components go incommon, which imports nothingarchitecture-specific, and that is the whole of it. §3.7 proposed stable string ids specifically to
break cycles that do not exist.
Those ids would also have been an API change.
StarterModel,StarterModelWithoutDependenciesand
StarterModelBundleare served byGET /starter_modelsand appear inopenapi.json, so turningdependenciesinto a list of ids would have changed the schema and broken both frontends.It stays a list of model references — which also keeps dangling dependencies structurally
impossible: an unknown name is an
ImportError, not a runtime surprise. That is exactly the propertythe ids were meant to reconstruct with "fail on unresolved id".
STARTER_MODELSkeeps its curated order, explicitlyIt follows neither base, nor model type, nor definition order: 179 entries in 47 contiguous runs by
base, with
sdxlandfluxeach appearing in eight separate runs. The file it lives in says thefrontend does not re-sort it. It is product data.
An aggregator that concatenated the per-architecture modules would therefore have silently
reordered the model manager. So
__init__still holds that list — 179 names rather than 2428 linesof definitions — and the risk this introduces, an entry defined but never listed, is what
test_every_entry_is_reachablecovers.This is the honest limitation of this PR: a new architecture still adds its entries to the curated
order. What it no longer does is add 200 lines of definitions to a shared file.
No consumer changed
__init__re-exports every entry, so every existing import keeps working — including the ones thatreach for individual models:
siglipinflux_redux, three image encoders inip_adapter, oneexternal model in a test. The pre-existing
test_starter_models.pypasses untouched.Bundle keys are preserved exactly, including that Wan has no base-keyed bundle at all — it ships
wan_t2vandwan_i2vas plain strings next to ten keyed byBaseModelType. Those keys areserialised into the API response.
Proof of equality
The catalog was fingerprinted before the split — all 179 entries with every field, the resolved
dependency graph by source, the bundles and their key types — and compared after:
A set comparison on
sourcealone would have passed while every dependency list silently emptied,which is why the graph is part of the fingerprint.
Two findings the split surfaced
Neither caused by it, neither fixed here:
esrgan_srx4andflux_kontextare dead — defined, but listed nowhere, in no bundle, dependedon by nothing, imported by nobody.
flux_kontextin particular looks like a model someone meant tooffer. Removing them is a content change, not a mechanical move; flagged for a follow-up decision.
manager, it arrives with the IP-Adapter that needs it.
Both groups are named in the tests, so they stay decisions rather than oversights.
Verification
pytest tests/backend/model_manager tests/backend/architectures tests/test_imports.py— 836 passedmypy --follow-imports=silent .../starter_models— clean across all 18 modulesruff@0.11.2 check .+format --check— cleanopenapi.jsonregenerated and compared normalized — identical.🤖 Generated with Claude Code