refactor(architectures): declare architecture facts in one file per architecture - #133
Draft
Pfannkuchensack wants to merge 32 commits into
Draft
refactor(architectures): declare architecture facts in one file per architecture#133Pfannkuchensack wants to merge 32 commits into
Pfannkuchensack wants to merge 32 commits into
Conversation
The node loader globbed `*.py` in `invokeai/app/invocations/` and put the stems in `__all__`, which `services/shared/graph.py` then triggers with `import *`. That only ever sees the top directory, so nodes in a subpackage would not be registered — and the failure would not surface at boot but later, as an "unknown node type" when a user opens a workflow that uses one. Walk the whole package tree instead, and import each module eagerly rather than leaving it to `import *`. `__all__` keeps only the top component of each path, because a dotted name cannot be bound by `import *`; the registration this module exists for has already happened by then. `pkgutil.walk_packages` swallows import errors raised while descending into a subpackage, which would turn "this package's `__init__.py` is broken" into "these nodes quietly do not exist" — the exact failure mode being removed here. Pass an `onerror` that re-raises. The walk is parameterized on root and prefix so it can be exercised against a synthetic tree. A walker with a bug here finds nothing and stays green forever, so its test must not depend on the layout of the package it normally walks. No behaviour change on the current flat layout: the generated `openapi.json` is byte-identical. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`invokeai/app/invocations/` held 164 modules in one flat directory, so the files belonging to one architecture were only findable by their name prefix. Move 106 of them into 16 packages; 58 stay flat. Thirteen packages are per-architecture (`flux/`, `wan/`, `minimax_h3/`, ...). Three group by role instead, because those nodes are shared rather than owned: vae/ a VAE follows the VAE, not the architecture — one latent space serves several text_encoder/ encoders are mixed and shared across architectures pid/ PiD decodes and upscales *on top of* a base architecture Filenames are unchanged, so `flux/flux_denoise.py` keeps its prefix. Inside a per-architecture package that is redundant, but the cross-cutting packages mix architectures by design and the prefix is what keeps a file findable by name. The split was derived, not typed: a file moves only if *every* one of its `@invocation` type strings names the same architecture. "Any hit wins" would have put `image.py` under `flux/` on the strength of one `flux_kontext_image_prep` among 36 nodes. Five files cannot be derived and are listed as named overrides — `ideogram4_caption.py` is a string builder rather than an encoder, `pidi.py` is the PiDiNet edge detector and has nothing to do with PiD decoders, `image_to_latents.py` and `latents_to_image.py` are the unprefixed SD VAE nodes, and `wan_latents_to_video.py` is a VAE node whose type string (`wan_l2v`) says otherwise. No `@invocation` type string changes, so persisted workflows keep resolving and the generated `openapi.json` is byte-identical. `test_pid_memory_optimization_wiring.py` globbed the flat directory and asserted the result was non-empty with "has the invocations directory moved?". It had. Recurse. `test_encoder_offload.py` looped over `invocations.__all__` importing each entry, to be sure every node was registered before enumerating the registry. Importing the package now does that; the loop would only have imported the sixteen packages by name. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The new-model integration guide showed every node file at the top of `invokeai/app/invocations/`. Point the code-block titles and both file-tree summaries at the package each node now belongs to, and state the rule once: one package per architecture, with VAE, text-encoder and PiD nodes grouped by role because they are shared across architectures. Also name the consequence of forgetting `__init__.py` — the package is discovered automatically, so there is no list to edit, but a directory that is not a package contributes no nodes at all. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Two registries are filled by importing modules rather than from a hand-maintained list: node invocations, and — next — model architectures. Both fail identically when discovery is subtly wrong: they find nothing, register nothing, and stay green. Having one walker means the pitfalls (descending into subpackages, not swallowing a broken one, skipping private paths) are handled and tested once instead of drifting apart in two copies. The walk returns names and leaves importing to the caller, which is what lets it be tested against a synthetic tree. That matters more than it looks: a walker with a bug returns an empty list, so a test asserting only "some modules were found" against the real package would pass. Splits the tests accordingly — the walker's own behaviour is pinned in tests/backend/util/, and what remains under tests/app/invocations/ is the check that this package's layout on disk agrees with what was imported. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Adding a model architecture means editing a long tail of core files — a `step_callback` branch, a
`safe_globals` entry, a variant-enum lookup. The count is not the problem; the failure mode is.
Almost all of those edits, when forgotten, fail at *generation* time rather than at boot: a missing
preview branch raises "Unsupported base model" on the first step, an unregistered
`*ConditioningInfo` breaks deserialization halfway through a graph.
This is the container those facts will move into: one module per architecture under `defs/`, and a
`validate()` that turns "you forgot one" into a startup error. Sixteen architectures are registered
here declaring nothing at all, so the structure is sharp and the semantics are still empty. It
becomes load-bearing when the first facet marks itself `REQUIRED`.
The internal direction of dependency is
facet.py <- registry.py <- facets/* <- defs/* <- __init__.py <- the rest of the codebase
which is why `Facet` is its own module rather than part of `registry`: every facet needs both, and
merging them would make each facet import the registry it is registered into.
`defs/` and `facets/` are discovered by walking the package, not from an import list. That removes
the most likely contributor mistake — a new file that nobody imports — and it moves the check to a
better place: a `BaseModelType` member with no module under `defs/` is now caught by `validate()`
directly, rather than by noticing an absent import line. `facets/` is walked for a sharper reason
than symmetry: `validate()` learns which facets are required from `Facet.FACET_TYPES`, filled at
class creation, so a facet module nothing happened to import would have its requirement silently
unchecked — exactly for a facet so new that no architecture declares it yet.
Modules under `defs/` import `architectures.registry` directly and never the `architectures`
package. It is that package's own import that brings them into being, so reaching back for an
attribute would find a half-initialized module. A layering test pins this.
Two details that are easy to get wrong and are commented in place: `Facet.FACET_TYPES` is a dict
rather than a set, because set iteration order is non-deterministic and has already produced a real
bug here; and `ArchitectureError` subclasses `ValueError`, because it replaces
`raise ValueError("Unsupported base model: ...")` at call sites whose handlers must keep working.
Filenames under `defs/` are the base value with `-` replaced by `_`, derived from the one identifier
that cannot change because it is persisted in the model database. That lets an error message compute
the file a contributor has to open instead of consulting a second table that could disagree.
The plan this follows expected `registry.py` to stay torch-free so later lightweight consumers could
import it cheaply. That is not achievable: it needs `BaseModelType`, and `taxonomy` imports torch,
onnxruntime and diffusers at module scope. Keying the registry on strings instead would avoid the
import but break the enum's contract with `openapi.json`, which is worse.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ng policy Three files, guarding three different things. `test_registry.py` exercises the mechanics against throwaway facets rather than the real ones, so it does not restate what production declares — otherwise it would fail on every legitimate change to an architecture, and pass for the wrong reason when a facet is quietly dropped. It includes the negative probe for the boot gate: with a required facet nobody declares, `validate()` must raise and name the file to edit. Without that, `validate()` could be a no-op forever and nothing would notice. The test doubles are removed from `Facet.FACET_TYPES` at import. They land there via `__init_subclass__` the moment the module is read, which is before any fixture runs, and a leftover `REQUIRED` double makes the real registry fail validation for the rest of the session. This was not theoretical — it is how the completeness test caught the leak. `test_registry_completeness.py` runs against the real registry and is what makes the boot check meaningful while no facet is required yet: `generative_bases()` must equal the enum minus the three sentinels. That equation appears in exactly one place, here. Production never computes it — being registered is what makes an architecture generative — so writing it down anywhere else would create a second source of truth. It also checks the directory in both directions, because a stale module for a renamed base is as invisible as a missing one. `test_layering.py` is an AST policy modelled on the frontend's dependencyPolicy.test.ts: named rules, one assertion listing every violation at once, and self-tests proving the checker catches things. The self-tests are the important half. A walker with a bug reports no violations and stays green forever, which is indistinguishable from a codebase that obeys the rules. Two of the seven exist for mistakes already made while writing this: one pins that `from a.b.c import X` is reported as depending on `a.b.c` and not on `a.b`, and one pins the one-character distinction between `facet` (allowed from the registry) and `facets` (forbidden) from both sides. The relative-import ban is scoped to the architecture package rather than the repository. Applied everywhere it flags `app/services/model_records/__init__.py`, which is real but unrelated debt — that file writes `# noqa F401` without the colon, which ruff reads as a blanket suppression, so TID252 has been silenced there by accident. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
An incompletely declared architecture must not be able to start the app. That is the point of the registry, and this is where it becomes true. Two call sites. `run_app` covers the normal path, next to the existing check that invocation outputs are registered — but this one raises where that one warns: the neighbouring check inspects third-party node packs, while architectures are first-party and the set is closed, so an incomplete one is a bug in this repository rather than in someone else's. `dependencies` covers every embedder that never goes through `run_app` — the test suite, and `scripts/generate_openapi_schema.py`. Calling `validate()` twice is harmless; it is idempotent and has no side effects. The import in `dependencies` sits at module scope, not lazily inside `initialize()`, and that is a constraint rather than a style choice: `initialize()` constructs `ObjectSerializerDisk`, which mutates process-global torch state through `add_safe_globals`. Anything the registry is meant to contribute there has to be registered before that point. It buys nothing yet, but establishing it now means the facet that depends on it does not have to also discover it. That constraint cannot be checked in-process — by the time any test runs, half the codebase has been imported and the registry would be full wherever the import sat — so the test runs a fresh interpreter. Its first assertion is on `sys.modules`, before it touches the registry at all, and the ordering is the entire test: importing anything under `invokeai.backend.architectures` fills the registry as a side effect, so reading the registry first would make the assertion true even with no import in `dependencies` whatsoever. The first version of this test did exactly that and passed against a deliberately broken `dependencies`. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ugh them
Which matrix projects latents to RGB, whether a bias or a smoothing kernel applies, and how much the
VAE downscales are all facts about the architecture. They were spread over a fifteen-branch if/elif
in `step_callback.py` ending in `raise ValueError(f"Unsupported base model: {base_model}")`, which
fired on the first preview step — after the model had loaded and generation had begun.
This is the first facet, and it is `REQUIRED`, so the boot check stops being structural and starts
enforcing something: an architecture that declares no latent space cannot start the app, and the
error names the file to add it to.
Nine latent spaces serve sixteen architectures. The sharing was already there and was expressed by
duplicating matrices: `QWEN_IMAGE_LATENT_RGB_FACTORS`, `ANIMA_LATENT_RGB_FACTORS` and
`WAN_LATENT_RGB_FACTORS` were byte-identical, as were their three biases — all of them ComfyUI's
Wan21 latent_format, which is what the merged name now says. A test fails the next time a matrix is
pasted rather than shared.
Two findings that fell out of doing this rather than planning it:
Ideogram 4 was absent from the dispatch entirely. Its node carried a second, divergent copy of the
preview logic — the FLUX.2 constants inlined and the 8x downscale hardcoded — so neither call site
could have revealed the other by being read. Both now resolve through the registry. Its preview is
unchanged; it was already using the right matrix by hand.
MiniMax H3 had grown a second `spatial_scale = 16` special case beside Wan's, in a block whose
comment still described only Wan. `spatial_compression` is a property of the space now, so there is
no place left to put such a case.
Verified by comparing the old chain against the new facet across all sixteen architectures plus
Wan's 48-channel alternate: seventeen for seventeen, byte-identical previews and identical reported
sizes.
`step_callback.py` goes from 435 lines to 64. `sample_to_lowres_estimated_image` moves onto
`LatentSpace.preview`, and the test that covered it moves with it — it asserted a pixel value it
recomputed from the very matrix under test, which made it a tautology, and its docstring's claimed
column sums (0.3677/0.4577/0.9101) had drifted from the truth (0.3887/0.8771/1.3152) without
anything noticing. The replacement writes the pixel down.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ls from them Text encoders write a `ConditioningFieldData` to disk; denoise nodes read it back under `torch.load(weights_only=True)`, so every conditioning class has to be passed to `add_safe_globals` first. That was a hand-maintained list of thirteen imports and thirteen entries in `ApiDependencies.initialize`. Forgetting one produces no error at boot and none at encode. It fails at load, inside the denoise node, after the text encoder has already run and written its output: an `UnpicklingError` naming a class the user has never heard of, halfway through a graph. This is the second failure mode the registry exists for, and the later of the two. It is also what the module-scope import in `dependencies.py` was put there for. `add_safe_globals` mutates process-global torch state at a fixed point during startup, so the registry has to be full before `initialize()` runs — a constraint asserted since the registry landed, and only now actually load-bearing. Which class belongs to which architecture was derived rather than assumed: an AST sweep over the invocation packages for what each one actually *constructs*, not merely imports for a type check. That is how FLUX.2 turns out to encode to `FLUXConditioningInfo` — there is no `Flux2ConditioningInfo` — and it is the only one of the three sharings that was not obvious from the names. Thirteen classes for sixteen architectures. The resulting list is compared against the old one as a set: identical, in both directions. `IPAdapterConditioningInfo` stays out, and now visibly so: it is built in memory and handed to the pipeline, never written through `context.conditioning.save`, so it is never unpickled. `conditioning_infos()` sorts by class name. Registry order is insertion order, which is the order `defs/` happened to be walked in — reproducible in practice but incidental, and this list is worth being able to diff. Widens the facets and defs import allowlists by one module, `conditioning_data`. That is the first widening since the layering policy landed, and it is a line in the change that needs it rather than a blanket permission granted up front. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…e recommends `MainModelDefaultSettings.from_base` was a `match base:` block of twelve cases, four of which sub-dispatched on variant, ending in `case _: return None`. It decides what the generation sliders say when a model is selected — 30 steps for Wan TI2V-5B, CFG off for Krea-2 Turbo — and it is read once, at identification, then stored on the config. These are product decisions rather than derivable facts, so they move to the architecture that owns them and stay data: a mapping from variant to settings, with `None` as the fallback. The facet is deliberately *not* `REQUIRED`. Four architectures (SD 3.5, the SDXL refiner, CogView 4, FLUX.1) reach the old fallback and have no defaults, with a standing `TODO(psyche)` asking whether they should. So a forgotten architecture fails softly here — no crash, just sliders the user sets himself — which is milder than the other facets, and the boot check cannot enforce it. A test pins exactly which four declare nothing, so the set stays a decision rather than an accident. ERNIE-Image's Turbo detection stays name-based, as it must: Turbo and the base model share an architecture and a config, so there is nothing on disk to probe. It becomes a `by_name_hint` mapping rather than a code branch, which keeps ERNIE's two settings objects in `defs/ernie_image.py` with the rest of the product data instead of stranding them in the resolver. The existing behavioural tests — leaf directory counts, ancestor directory must not — now run through it unchanged. `MainModelDefaultSettings` moves to `configs/default_settings.py`, and this is structural rather than tidying. A facet holding instances of that class cannot import `configs/main.py` once `configs/main.py` is the module doing the looking up: main imports the registry, the registry imports the defs, the defs import the facet, and the cycle closes on a half-initialized module. Splitting the data from the behaviour breaks it without a lazy import. `configs/main.py` re-exports the name, so every existing caller is unaffected. Verified by sweeping the old `from_base` against the new resolver over every (base, variant, name, path) combination — 16 bases x 32 variants x 5 names x 4 paths: 10560 for 10560 identical. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…heir model cards recommend
Three architectures reached the old `case _: return None` and had no generation defaults at all, so
a user selecting one got whatever the sliders happened to be showing. Now that the values live next
to the architecture, filling the gaps is a three-line change per architecture rather than three more
cases in a match block.
The numbers are cited, not invented:
cogview4 50 steps, guidance 3.5, 1024x1024 THUDM/CogView4-6B's own example. True CFG — it
takes a negative prompt — so it goes in cfg_scale,
and the denoise node already defaults to 3.5.
Nothing was propagating that to the sliders.
sd-3 40 steps, guidance 4.5 stable-diffusion-3.5-medium. Medium rather than
Large (28/3.5): there is one `sd-3` row and no
variant to tell the two apart, and Medium is the
smaller and more commonly run of them.
flux per variant, see below black-forest-labs' cards for schnell, dev and Fill.
FLUX.1 gets a variant-keyed mapping because its three variants genuinely disagree. `guidance` here
is the distilled guidance embedding, not classifier-free guidance, so cfg_scale stays at its floor
(1.0, meaning off) for all three:
schnell 4 steps, no guidance timestep-distilled; the node already documents that it ignores
guidance entirely
dev 28 steps, guidance 3.5 the card's example says 50; 28 is the de-facto standard and
what FLUX.2 [dev] already declares here, so the two agree
dev_fill 50 steps, guidance 30.0 corroborated in-tree — flux_denoise.py already warns when
guidance drops below 25.0 for a Fill model
Only the SDXL refiner is left declaring nothing, which is right: it is not run on its own, so there
is nothing for it to prefill. That resolves the standing `TODO(psyche)` for the other three.
Not touched, because they need a ruling rather than a citation: SD 1.x/2.x/XL still declare size
only and no steps or CFG, as they always have; and SD 2.x's 768x768 is right for the v-prediction
checkpoints and wrong for the 512 base ones, with nothing distinguishing them here.
Note this changes nothing in webv2 yet. webv2 reads `default_settings` only for LoRA weight and VAE
key — steps, CFG and dimensions come from its own hardcoded BASE_GENERATION table, which disagrees
with these values in four places (ideogram-4 at CFG 1 against the sampler's actual 7.0; z-image at 8
steps against the card's 9; anima at 30/4; sd-2 at 512). Making webv2 read the backend is what the
capabilities endpoint is for.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
SD 1.x, 2.x and XL declared their canvas size and nothing else, so selecting one left the step and CFG sliders at whatever the previous model had set. They get 30 steps at CFG 7 — the classic Stable Diffusion defaults, and what webv2's own table has been using for them all along. Sizes stay native and differ: 512 for 1.x, 1024 for XL, and 768 for 2.x. That last one is a judgment call recorded in place — 768 is right for the v-prediction checkpoints and wrong for the 512 `-base` ones, SD 2.x has no variant modeled to tell them apart, and we ship no starter model for it either way. With this, every architecture but the SDXL refiner declares its defaults, and the refiner is right to declare none: it is not run on its own. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ult settings at boot The refiner declares SDXL's 1024x1024 — it refines an SDXL latent, so it shares its canvas. Steps and CFG stay absent on purpose: it is a second pass driven by the UI's own refiner parameters, so there is nothing here for them to prefill. That was the last architecture without a declaration, which removes the reason this facet was optional. It becomes `REQUIRED`, so a new architecture that forgets its defaults cannot start the app — the same gate that already covers latent spaces and conditioning types. The failure it guards is milder than theirs. A missing latent space raises mid-generation; a missing default just leaves the sliders wherever the previous model put them. But that is an argument for catching it at boot rather than for tolerating it: nothing about an absent prefill is loud enough to be noticed any other way. Verified by removing Anima's declaration — `validate()` refuses to start and names the file. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Two facts that travel together. What an architecture can produce — text-to-image, img2img, inpaint, outpaint, and for Wan and MiniMax H3 video as well — and what its modes are called in image metadata. The second is the load-bearing half. Every generated image records a string like `flux_inpaint`, and those strings sit in user galleries and workflow files. They cannot be changed, only declared: the slug is `z_image` where the enum says `z-image`, `krea2` where it says `krea-2`, `ideogram4`, `sd3`, `ernie_image`, `qwen_image`, `minimax_h3`. Seven of fourteen differ, and SD 1.x and 2.x carry no prefix at all. Deriving them by replacing `-` with `_` would produce `ideogram_4` and orphan every image already tagged `ideogram4`. `GENERATION_MODES` stays a `Literal` — it is a type, and it is what pydantic validates metadata against. What the declarations buy is a test that reconstructs all 50 strings from them and compares: a string missing from the literal is metadata that will not validate, one missing from the declarations is a mode nothing can produce. Both directions are checked. The split was derived from the literal rather than transcribed: fourteen slugs, no ambiguity, and it turned up two things worth stating. The SDXL refiner declares no modes — it refines an SDXL latent and writes no mode string of its own. And MiniMax H3 declares `txt2img`, `t2v` and `i2v` but none of img2img, inpaint or outpaint, which is a real capability boundary the UI can now read instead of guess. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ure supports
Whether to show a negative prompt box, whether a ControlNet layer can attach, how many reference
images to accept, whether clip-skip means anything — none of it derivable from a model file, all of
it living in the frontend.
webv2 holds the working version for thirteen bases in `baseGenerationPolicies.ts`, plus three
predicates elsewhere: `isControlKindSupportedForBase`, `isReferenceImageSupported` and
`isRegionalGuidanceSupportedForBase`. Those values are what is declared here — the point is not to
change them but to put them where a new architecture cannot be added without them.
Which matters, because three bases have no row there at all and got one by derivation from their
own nodes rather than by guesswork:
ernie-image `ernie_image_denoise` takes a negative_conditioning "required when guidance_scale
!= 1.0" — cfg-gated, like the other distilled models. Its scheduler set is
ERNIE_IMAGE_SCHEDULER_MAP, a flow family.
minimax-h3 its module docstring is explicit: "guidance-distilled: no negative prompt, no CFG,
one forward per step". It steps video and audio down two hardcoded flow schedules,
so there is no scheduler to choose — the only base declaring `scheduler_set=None`.
sdxl-refiner declares no modes, so it is not offered as a generation model; it answers as the
SDXL pass it is, so a UI that does surface it needs no special case.
`dimension_grid` was not taken from webv2 but from the node's `multiple_of` on width — the
constraint the graph actually enforces. The two agree for all thirteen, which is the useful part:
two independent sources, no divergence, and a test now pins the declaration to the node so a UI can
never offer dimensions the graph will reject. It also fills in the two missing values, 16 for ERNIE
and 32 for H3.
Three invariants are asserted rather than assumed, and each caught something while being written:
regional negative prompts are a strict subset of regional guidance; a negative prompt box is visible
exactly when its usage is not `never`; and clip-skip belongs to SD 1.x and 2.x alone — legacy's
`CLIP_SKIP_MAP` carries 24 for SDXL, which webv2 never reads and nothing has offered for some time.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`GET /api/v2/models/capabilities` returns what every architecture can generate and which generation features it supports — 24 rows, one per architecture plus one per variant that answers differently. A client fetches it once and joins it against model records locally: look up `(base, variant)`, fall back to `(base, null)`. This is what the facets were for. webv2 currently hardcodes the same table for thirteen bases in `baseGenerationPolicies.ts`, has no row at all for ERNIE-Image, MiniMax H3 or the SDXL refiner, and disagrees with the backend in four places (ideogram-4 at CFG 1 against the sampler's actual 7.0; z-image at 8 steps against the card's 9; anima at 30/4; sd-2 at 512). None of that is fixed by this commit — it is what the commit makes fixable, by giving webv2 something to read instead of a table to maintain. Three shape decisions, each against an alternative the plan proposed: `ArchitectureCapabilities` is not a subclass of `ExternalModelCapabilities` and does not share its name. That model describes one external provider's model — aspect ratios, resolution presets, mask format, per-request limits — and is stored on each such record. This describes an architecture, is identical for every model of it, and is stored nowhere. Subclassing would have put fifteen irrelevant fields on a schema webv2 already consumes, which is the opposite of additive. It is not a computed field on `AnyModelConfig` either: that would add these fields to all 115 config schemas and risk them being persisted into records. And a variant gets its own row only where something differs — the five architectures whose recommended parameters depend on the variant. Qwen-Image's variant-conditional reference-image support stays a field on the base row (`reference_images_require_variant`), because materializing a row for it would mean inventing rules about which fields a variant row may omit. The route takes no auth dependency and touches no service: there is nothing user- or install-specific in the response. A test pins that, so a later version reaching for the invoker fails there rather than in production. Purely additive, as intended and as measured: one new path, four new schemas, zero existing schemas changed, zero removed, `ExternalModelCapabilities` byte-identical. `openapi.json` gains 231 lines and `schema.ts` 178, with no deletions in either. The endpoint is `/api/v2/models/capabilities`, not the `/api/v1/...` the plan named — the model manager router has been on `/v2/models` for some time. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`starter_models.py` had reached 2707 lines and 240 top-level definitions: 201 starter models across seventeen architectures, thirteen bundles, and a block of provider presets for external models. It becomes a package of nineteen modules, none over 500 lines, named the same way as `architectures/defs/` — the base value with `-` replaced by `_`. Every block moved verbatim, by AST line range including the comments above it, so nothing was retyped and nothing could be mistyped. Verified against a snapshot taken beforehand: all 201 models identical field for field *and in the same order*, all thirteen bundles identical. That order is the point. `STARTER_MODELS` is the sequence the install dialog shows, decided by someone, reconstructible from nothing — so it stays written out in `__init__.py` rather than assembled from the per-architecture modules. Assembling it would be tidier and would silently destroy product data. A test asserts the list is *not* sorted, which is the only way to notice a later tidy-up that replaces the curation with something derivable. The dependency graph was derived before the split rather than assumed, and it is a clean DAG: `types` under everything, `common` under most, and exactly three edges between architecture modules — `krea_2 -> qwen_image`, `z_image -> flux`, `sdxl_refiner -> sdxl`. Those are the same three sharings the architecture registry already records: Krea-2 decodes with the Qwen-Image VAE, Z-Image with a FLUX-compatible one, and the refiner is an SDXL pass. Two independent derivations agreeing is worth more than either alone. Every name is re-exported from `__init__.py`, so the nine names other modules import — including `clip_vit_l_image_encoder` and `siglip`, imported by individual nodes — keep resolving unchanged. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`IdealSizeInvocation` dispatched on base over six architectures and ended in
`raise ValueError(f"Unsupported model type: {unet_config.base}")`. Nine of the sixteen fell into
that branch — CogView 4, Z-Image, ERNIE-Image, Ideogram 4, Qwen-Image, Anima, Krea-2, Wan and
MiniMax H3 — and the failure landed at generation time, after the model had loaded, which is
precisely the failure mode this whole series exists to remove.
Both numbers it needed are already declared. The canvas is `DefaultSettingsFacet`'s width, and it
matches the old hardcoded values exactly: 512 for SD 1.x, 768 for SD 2.x, 1024 for the rest. So the
three architectures anyone actually used this node with are unchanged, byte for byte.
The second number was wrong for more than half of them. `trim_to_multiple_of` defaulted to
`LATENT_SCALE_FACTOR`, which is 8 — right for the SD family and wrong for everything with a 16 or
32 grid. A FLUX or CogView 4 ideal size could come back off-grid, and the denoise node would then
reject the width this node had just computed. It now trims to `FeaturesFacet.dimension_grid`, which
a test already pins against the `multiple_of` those nodes enforce.
The node's title still reads "Ideal Size - SD1.5, SDXL". Left alone deliberately: titles are what
users see in saved workflows, and renaming one is a UI decision rather than a consequence of this
fix.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
… scaffolded `scripts/new_architecture.py` writes the three files the registry made mechanical — the declaration under `architectures/defs/`, the invocation package, and the starter-model module — and then prints everything it cannot write. The second half is the more useful one. That list is derived on each run, not stored: any module naming five or more `BaseModelType` members is dispatching on base, so a new base has to be added to it. Twelve modules qualify today. A written-down list would already be wrong — `step_callback.py`, `dependencies.py` and, as of the previous commit, `ideal_size.py` have all dropped off it during this series, and a maintained list would still be naming them. The generated declaration deliberately does not work: every required facet is present with an obviously wrong value — a one-row black projection, a single mode, a placeholder canvas — and five TODOs saying what each one needs. The app refuses to boot until they are filled in. A stub that booted would let a half-integrated architecture reach a user, which is the failure this structure exists to prevent. Verified end to end: written, refused to import, refused to overwrite on a second run, removed cleanly. Tests pin the parts that would rot silently: that each stub parses, that the declaration carries all five required facets, that it still looks unfinished, and that the derived list contains `configs/main.py` while containing none of the three modules the registry has absorbed. That last assertion is what notices a facet being bypassed later. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…expectations Two regressions the full suite caught, both mine. `test_every_route_is_authenticated_or_explicitly_public` failed: the new `GET /api/v2/models/capabilities` had no auth dependency. I had reasoned that the response holds nothing user- or install-specific, which is true and is not the question the test asks. Its message says the allowlist is for routes that *must* be public, and this one does not have to be. It now takes `CurrentUserOrDefault` like every other route in the router. That falsifies a claim the route's own tests made — "it needs no services". `CurrentUserOrDefault` resolves through `ApiDependencies.invoker`, so the route does need one now. The test that asserted otherwise is replaced by one that asserts what actually matters and remains true: the model manager service is never touched, so the response cannot have become per-model. `test_default_settings_main[sdxl-refiner-None]` failed because the refiner now declares SDXL's canvas. The expectation was correct until it did; updated, with a note on why the refiner has dimensions but no steps or CFG. `openapi.json` and `schema.ts` regenerated: the delta is this route's description and its `security` block, nothing else. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`get_max_unet_downscale` replaces two dispatches that were duplicated verbatim -- identical down to the comment above them and the error string below them -- in `denoise_latents.run_t2i_adapters` and `T2IAdapterExt.__init__`. SD1's UNet downscales the latent image 8x internally, SDXL's 4x; every other architecture raised. Kept apart from `LatentSpaceFacet` deliberately. Both are small integers about downscaling, which is exactly why they must not share a field: this one is a property of the UNet, the other of the VAE latent geometry, and an architecture can have one without the other. The facet is not `REQUIRED`. Fourteen of sixteen architectures legitimately have no UNet in the sense T2I-Adapter conditioning means, so the accessor carries the error rather than `require()` -- which also lets the message stay byte-identical to the one the two dispatches raised. It is user-facing, and a test pins it including how the enum renders (`BaseModelType.Flux`, not `flux`, because the enum is a `str, Enum` mixin rather than a `StrEnum`). Also records, in a NOTE and not in code, that a third copy of the SDXL BGR rule sits ~300 lines below the first: it decides the swap from the *UNet's* base while `run_t2i_adapters` decides it from each *adapter's* base, so the two disagree for an SD1 adapter on an SDXL UNet. That is a behaviour bug, fixing it changes output, and this is a refactor. The natural fix is to fold `bgr_input` into this facet so both paths read one declaration. Ported from the abandoned refactor/arch-latent-space-facet branch (#28), the one piece of that PR the rebuilt series had not carried over. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Custom node authors construct `*ConditioningInfo` objects, but only three of the thirteen were reachable through the public `invocation_api` surface -- `BasicConditioningInfo`, `ConditioningFieldData` and `SDXLConditioningInfo`. Anyone writing a node for FLUX, Wan, Qwen-Image or the other nine had to reach into `backend.stable_diffusion.diffusion.conditioning_data` directly, which is not a supported import path. The list is static, because `__all__` is a real re-export rather than a runtime lookup, and both star-imports and editors need the names to exist at module level. What keeps it honest is a test that compares the exported set against `conditioning_infos()` from the registry: declaring a new architecture's conditioning type now also makes it public, or the test says so. That guard is not hypothetical. This change was ported from the abandoned refactor/arch-conditioning-facet branch (#30), where the same list was written out by hand -- and in the weeks since, MiniMax H3 arrived and the hand-written version had silently gone stale. The list is derived-and-asserted for exactly the failure it had already suffered. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
github-actions
Bot
added
root
python
python-tests
backend
api
frontend
invocations
docs
labels
Four resolutions, three of which git could not see: - `starter_models.py` was a modify/delete: this branch split it into the `starter_models/` package, main added `minimax_h3_lightx2v_turbo_lora` and put both turbo LoRAs into the H3 bundle. Ported into `starter_models/minimax_h3.py` and the package's aggregator. - `test_qwen_image_working_memory.py` conflicted on imports: this branch moved the Qwen VAE invocations under `invocations/vae/`, main added the tiling helpers. Kept both, and rewrote the `patch()` target strings main added, which still named the old flat module paths. - `test_minimax_h3_lora_loader.py` is new from main and imported `invocations.minimax_h3_lora_loader`, which this branch moved into the `minimax_h3/` package. Merged clean, would not have imported. - main added three H3 generation modes (`lf2v`, `flf2v`, `extend_video`) to the `GENERATION_MODES` literal. `GenerationModeKind` and the H3 `ModalityFacet` now declare them, so the registry reconstructs the literal again. `test_the_video_architectures` updated to match. - main added a test forbidding `async def` route handlers that await nothing; `list_architecture_capabilities` was one. Now `def`. OpenAPI schema and `schema.ts` regenerated: the new mode kinds widen `ArchitectureModality.modes`.
- Introduced 'ernie-image' model in various components including base generation policies, contracts, and graph builders. - Updated tests to ensure coverage for the new model, including graph structure and node types. - Added a snapshot for node types emitted by the 'ernie-image' model to maintain contract with backend invocation registry. - Enhanced documentation and comments for clarity on model behavior and requirements.
`main` gained `wan_t2v`, `wan_interpolate` and `wan_extend_video` in `GENERATION_MODES`. Both sides merged without a conflict — the literal and the declaration live in different files and neither was touched by the other — and `test_the_declarations_reconstruct_generation_modes` caught the drift. `interpolate` (fill between two given frames) is a new kind, so `GenerationModeKind` grows too; `extend_video` already existed from MiniMax H3. Since that literal is served through the capabilities endpoint, `openapi.json` and `schema.ts` are regenerated — the delta is one added enum value in `ArchitectureModality.modes`, purely additive for any client. Verified the three modes are genuinely new upstream rather than a gap this branch had all along: they are absent from `metadata.py` at the merge base. fix(webv2): exclude the generated graph contract from oxfmt format:check and the test that writes generateGraphNodeTypes.json both formatted the same bytes, so each undid the other's layout and CI failed on whichever ran last. The file is generated, so the test owns it alone.
…ng to prevent import cycles
5 tasks
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
1 participant
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.
Adding a
BaseModelTypecosts ~23 new files and ~16 edits to core modules. The count is not theproblem — the failure mode is. Most of those core edits, when forgotten, fail at generation
time rather than at boot: a missing
step_callbackbranch raisesUnsupported base modelon thefirst preview, a
*ConditioningInfoabsent fromsafe_globalsbreaks deserialization mid-graph.This series makes an architecture declare its own facts once, in one file, and makes an incomplete
declaration fail at boot with a message naming the file to edit.
Replaces #27–#33, which are closed with per-PR notes on what became of each.
What an architecture declares
invokeai/backend/architectures/defs/<base>.py, one file per architecture, sixteen of them:Five facets are
REQUIRED;validate()runs at boot and refuses to start without them, naming thefile.
UNetDownscaleFacetis optional — fourteen of sixteen architectures legitimately have no UNetin the sense T2I-Adapter conditioning means, so its accessor carries the error instead.
Dispatch chains this removes
step_callback.py— 10 latent→RGB branches, three byte-identical Wan matricesLatentSpaceFacet, 9 spacesdependencies.py— hand-listedsafe_globalsConditioningFacetideal_size.py— 6 bases handled, 9 raised at generation timedefault_settings— base-keyed branchesDefaultSettingsFacet, keyed by variantUNetDownscaleFacetBugs found on the way
in its own node.
ideal_sizeraised for 9 of 16 architectures, after the model had loaded, and trimmed everyresult to a hardcoded grid of 8 — so FLUX (grid 16) could be handed a width its denoise node then
rejected.
spatial_scale = 16special case that the declaration makes redundant.Two known bugs are recorded in
NOTEs rather than fixed, because fixing them changes output and thisis a refactor: the third, divergent copy of the SDXL BGR rule in
denoise_latents(it reads theUNet's base where
run_t2i_adaptersreads each adapter's), and the missingis_canceled()checkin
ideogram4_denoise.Also in here
invocations/flux/,invocations/wan/, … plus sharedvae/,text_encoder/,pid/. Discovery is recursive, so a new node needs no registration.starter_models.pysplit into a package — 2707 lines, 201 models, into 19 modules. Verifiedfield-for-field and in the same order against a snapshot;
STARTER_MODELSstays written outbecause that order is curated product data, and a test asserts it is not sorted.
GET /api/v2/models/capabilities— the static architecture table, 24 rows, for webv2 toreplace its hardcoded
BASE_GENERATIONwith. Purely additive to the schema.scripts/new_architecture.py— scaffolds the three mechanical files and derives the residualedits by AST rather than listing them, because that list already shrank twice while this was built.
How it is kept honest
validate()at boot, plus a completeness gate assertinggenerative_bases()equalsBaseModelTypeminus the three sentinels — the one place that subtraction is allowed to exist.buggy walker reports zero violations and stays green forever.
combinations, 201/201 starter models in identical order.
Verification
openapi.jsonwas bit-identical until the capabilities route, where the delta was measured as purelyadditive. Full suite: no regressions against a same-environment baseline, and no tests silently
lost — the branch collects 5703 against 5581 at the branch point, net +122. The two IDs that
disappear are accounted for:
test_step_callback.pywas deleted when its assertions moved into thefacet tests, and one parametrized ID changed name because its expectation changed.
The branch was also test-merged against a
mainthat had moved ~90 commits ahead. Two conflicts,both mechanical; three further breakages merged cleanly and were only found by running the suite.
.ideas/Merge Playbook.mdrecords what to do, in order.🤖 Generated with Claude Code