refactor(architectures): declare architecture facts in one file per architecture - #133
Open
Pfannkuchensack wants to merge 49 commits into
Open
refactor(architectures): declare architecture facts in one file per architecture#133Pfannkuchensack wants to merge 49 commits into
Pfannkuchensack wants to merge 49 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
…ng to prevent import cycles
5 tasks
Resolved the starter_models modify/delete by distributing upstream's two new MiniMax H3 models over the split package, keeping STARTER_MODELS' curated order. Repointed three upstream test imports and upstream's new minimax_h3_reference.py at their post-refactor paths, grew GenerationModeKind, MiniMax H3's ModalityFacet and its mode-set guard by ref2v, and regenerated openapi.json, which had auto-merged without any of upstream's new nodes.
Moved upstream's LightX2V Ref2V turbo LoRA, which replaces the 4-step v0.1 repack, into the split starter-model package at its curated position. Repointed three silently-broken references at their post-refactor paths: a new _ResolvedVideoRange import, two mock.patch strings naming minimax_h3_reference, and seven flux2 references in upstream's new test_flux2_working_memory.py. Regenerated openapi.json and schema.ts.
The generator emits raw json.dumps output; both openapi-checks and the frontend prettier check compare against a prettier-formatted file.
Twelve architectures now name the variant enums their models carry, keyed by model type -- Wan is the reason it is a mapping rather than one enum, since its LoRAs use a different enum from its mains and mixing them crashes the patcher. Guards the four hand-maintained copies of that list against drift, and records the FluxVariantType.Dev / Flux2VariantType.Dev value collision as a named exception so a new one still fails.
- webv2 read `cfg_scale ?? guidance`, so the guidance-distilled models — which
declare both, the cfg_scale of 1.0 only meaning "CFG is off" — always showed
1.0. Worst on FLUX Fill, which declares 30.0. Now keyed on `guidanceLabel`,
because `default_settings.guidance` is editable on any main model and must not
displace a true CFG. The slider's number input also could not hold a value
above its 10-wide track and clamped the model's own default away on blur.
- ernie-image omitted `scheduler_applies_to_graph`, so `/models/capabilities`
served a wrong row. A generated cross-stack contract now pins the whole
capability table to webv2's, the way `graphCoverage.test.ts` pins node types.
- `new_architecture.py` listed a shipped SQLite migration as an edit to make;
`calibrate_flux2_working_memory.py` still imported a moved node module; two
contributor docs still described the deleted `from_base()`.
- `DefaultSettingsFacet({})` passed boot validation and resolved to None at
generation time; four bare asserts in an API path vanish under `python -O`.
- Coverage: the layering walk is now checked to reach files at all, ideal_size
pins SD3/FLUX/FLUX.2, and a starter model reachable from nothing now fails.
Pfannkuchensack
added a commit
that referenced
this pull request
Sep 9, 2026
…erver-driven-model-policies Both branches had found and fixed the same two things independently; the merge keeps this branch's versions, which are the better-structured ones: - the guidance/cfg_scale choice, as `getRecordGuidanceValue` keyed on `guidanceLabel` rather than an inline ternary - ernie-image's `scheduler_applies_to_graph` Removed as obsolete here: #133's `capabilityContract.test.ts`, its snapshot and `test_frontend_capability_parity.py`. They pinned webv2's `BASE_GENERATION` to the registry, and this branch deletes that table in favour of reading the capabilities endpoint — `test_capabilities_fixture.py` is the contract now, in the other direction. Their oxfmt exclusion went with them. Kept from #133 because this branch does not have it: the guidance slider could not hold a value above its 10-wide track, so FLUX Fill's 30 clamped to 10 on first blur. Its regression tests were folded into this branch's own guidance describe block, dropping the three cases already covered there and re-pointing the per-base sweep at the policy instead of the deleted table.
- safe_globals is assembled by conditioning_safe_globals() and pinned to its call site by an AST test; the old test never imported dependencies at all. - validate() enforces only first-party facets, and register() accepts an identical re-declaration, so a custom node pack and --dev_reload no longer kill boot blaming defs/. - dimension_grid is variant-keyed: Wan TI2V-5B declares 32, which ideal_size and the served table now resolve from the model's variant. - The cross-stack graph contract records the literal inputs webv2 writes and validates each against the invocation field it lands on. - sd-2, z-image and anima declare the regional guidance they actually support; the default-settings matrix is pinned per (base, variant). - The generated architecture stub now refuses to import, as its docstring already promised, and starter_models exports only its catalogues and types.
Four test modules main added since the last merge import the flat node paths this branch moved into architecture packages. Git takes them without a conflict because they are new on one side only, so they surface as collection errors rather than as something to resolve: test_anima_vae_tiling, test_flux_vae_decode_tiling, test_int8_denoise_working_memory and test_z_image_tiled_decode now import from vae/ and z_image/. The one real conflict was test_krea2_denoise's import, which needed both the package path and requires_sidecar_patching.
…ared width The node squared the architecture's recommended *width*, which is a product decision rather than a training resolution. Fifteen of sixteen architectures declare a square default, so this is identical for them; MiniMax H3 is 1344x768, and squaring 1344 asked for 1.75x the area it was trained on. Reachable in practice: the main-model loaders emit a UNetField only for SD, but MetadataToModelInvocation emits one for any ModelType.Main.
Pfannkuchensack
marked this pull request as ready for review
September 12, 2026 01:09
Pfannkuchensack
requested review from
JPPhoto,
blessedcoolant and
lstein
as code owners
September 12, 2026 01:09
…te the contracts in CI A variant row reused its architecture's primary latent space, so Wan TI2V-5B was served grid 32 next to compression 8 while it denoises in the 48-channel Wan2.2 space at 16x. `LatentSpaceFacet` now names the space a variant uses -- checked to be one the facet already declares, so the two resolvers cannot disagree -- and a variant that differs only there gets a row of its own. The Python change filter skipped the two cross-stack generation contracts: a frontend-only change could regenerate them, pass vitest, and never run the backend half that validates them against the registries.
… from the node The declaration this series exists to protect was itself checked for four of sixteen bases: swapping FLUX_16 for SD3_16 in the FLUX declaration passed every committed test and corrupted every FLUX preview at generation time. A 16-row identity table now pins each base against the pre-refactor dispatch, and each of the nine spaces has a hand-derived preview pixel. The smoothing-kernel test was seeding after it drew its sample, so it had never been deterministic. The guidance control's bounds now come from the architecture too, the way the dimension grid already does. FLUX.2 accepts at most 20 and four architectures reject below 1, while the input allowed 0 to 100 for every model -- so a value the UI offered was rejected at enqueue. Pinned against the ge/le of the field each denoise node actually validates. Also: Ideal Size is no longer titled for SD, since it answers for every architecture; the contributor docs describe the layout this branch created, including the defs module whose absence refuses boot; and the docs-export test no longer depends on pytest's working directory.
- walk_packages imported package initializers before the underscore filter; the walk now reads names only - a broken package fails at the caller's import with its own error
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.
Summary
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:Seven facets, five of them
REQUIRED;validate()runs at boot and refuses to start without them,naming the file. Two are optional because their absence is meaningful rather than incomplete:
UNetDownscaleFacet(fourteen of sixteen architectures have no UNet in the sense T2I-Adapterconditioning means) and
VariantFacet(four architectures model no variants at all, so an emptydeclaration would be indistinguishable from none).
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 variantUNetDownscaleFacetAnyVariant/variant_type_adapter/ModelRecordChanges.variantare not replaced.taxonomy.pystill owns those three hand-written lists;VariantFacetdeclares the same taxonomy afourth time and a test asserts all four agree, so a list that gains a variant and a sibling that does
not now fails CI. That is cross-checking, not removal — see Facets that carry no dispatch below.
Bugs found on the way
in its own node, so the shared matrices and the node's own could drift apart.
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.Found by the reviews of this PR and fixed here:
exposed a bug in webv2:
getGenerationDefaultsreadcfg_scale ?? guidance, and theguidance-distilled models declare both —
cfg_scale=1.0meaning "CFG is off", plus the realguidance. So1.0always won. Worst on FLUX.1 Fill, which declaresguidance=30.0and whosedenoise node warns below 25.0 on every run. FLUX.2 [dev] already declared both before this PR and
was already affected. There is one slider, and
guidanceLabelis what it stands for, so thechoice is keyed on that rather than simply reversed:
default_settings.guidanceis editable onany main model (
guidanceis inMAIN_FIELDSunconditionally), and preferring it on an SDXLmodel would hand a true CFG slider a value meant for a knob that base does not have.
max={10}with nonumberInputMax, soFLUX Fill's 30 rendered as a full track and the number input clamped it to 10 on the first blur —
the same reason the Steps field two blocks up passes a looser input bound.
scheduler_applies_to_graph, which defaults toFalse, while webv2 saystrueand the graphreally does pass the scheduler into
ernie_image_denoise.scripts/new_architecture.pypointed contributors at a shipped SQLite migration. Its derivededit list counted
BaseModelTypereferences without excludingsqlite_migrator/migrations/;editing one changes what already-migrated databases were told they got.
scripts/calibrate_flux2_working_memory.pyno longer imported. The one stale flat node pathleft in executable code after the move.
MainModelDefaultSettings.from_base(), which this PRdeletes.
Two known bugs are not fixed, because fixing them changes output and this is a refactor. Only
the first is recorded in the code, as a
NOTE: the third, divergent copy of the SDXL BGR rule indenoise_latents(it reads the UNet's base whererun_t2i_adaptersreads each adapter's). Thesecond is the missing
is_canceled()check inideogram4_denoise— pre-existing, and named hererather than in a comment.
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 — 19 modules, 210 model definitions, 204 of them inSTARTER_MODELS. The list stays written out because that order is curated product data, and atest asserts it is not sorted. Of the six definitions not in the list, four are dependencies of
listed models and two are reachable from nothing; all six pre-date the split, and a new test pins
that set closed so a new orphan fails.
GET /api/v2/models/capabilities— the static architecture table, 24 rows, for webv2 toreplace its hardcoded
BASE_GENERATIONwith. Purely additive to the schema. Until webv2 consumesit the two tables state the same facts twice, so a generated contract now pins them to each other
(see How the declarations are kept honest).
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.
Not a pure refactor
Three things in here change behaviour and are deliberate:
from_basereturned onlywidth/heightfor those; the declarations add
scheduler="euler_a", steps=30, cfg_scale=7.0. This changes whatis persisted on every newly identified SD model. Existing records are untouched. The SDXL refiner
gains a canvas (1024x1024) and nothing else — it declares no sampler settings, because it refines
an SDXL latent rather than starting one.
cfg_scaleinput, onlyguidance_scale, so the old value was never reaching anything.buildErnieImageGraphand itsBASE_GENERATION/MODEL_BASESentries are new; ERNIE-Image was backend-only before.Deliberately not in here
LoaderFlagsFacet, the loader policy object from.ideas/ArchitectureSpec.md§4. Its only contentwas
supports_fp8_storage=Falsefor Z-Image, and upstream removed that exclusion(invoke-ai#9414, "enable FP8 storage for Z-Image"). After FP8 compute landed (#232) the
generic load path —
load_default.pyand the autocast modules — contains noconfig.basebranch atall, and
should_keep_fp8_weights()decides by device. The per-architecture FP8 work lives in theindividual loaders, which is dispatch and belongs in
ModelLoaderRegistry. The facet would declarenothing; the spec itself says to promote it only when a second real loader rule appears.
Facets that carry no dispatch
Five of the seven have production readers. Two do not, and are worth naming rather than implying:
ModalityFacetisREQUIREDbut only reaches the capabilities endpoint today. Itsgeneration_modes()is checked against theGENERATION_MODESLiteral that pydantic validatesagainst, which is a real gate, but nothing dispatches on it.
VariantFacetis deliberately not wired intoconfigs/factory.py. It exists so the fourhand-written variant lists in
taxonomy.pycan be asserted against each other. That is a CI guardwearing a facet's clothes; it costs 12 declarations to keep in sync. Worth a reviewer's opinion on
whether to wire it in or drop it and keep the taxonomy-derived tests, which stand on their own.
Related Issues / Discussions
GET /api/v2/models/capabilitiesin webv2.main, including feat(fp8): run scaled and raw fp8 checkpoints on the fp8 tensor cores #232 (FP8 compute) and perf(ci): run the python test suite in parallel #233 (parallel python tests).QA Instructions
Boot and generate on SD1 — the module-level import in
dependencies.pyhas to keep the boot order(CUDA allocator before torch), which no test covers. Verified by hand: importing the four modules
run_app.pytouches beforeconfigure_torch_cuda_allocator()leavestorchout ofsys.modules.Generate once on FLUX.1 [dev] and once on FLUX.1 Fill with a freshly installed model, and check the
Guidance slider reads 3.5 and 30 rather than 1.0.
How the declarations are kept honest:
validate()at boot, plus a completeness gate assertinggenerative_bases()equalsBaseModelTypeminus the three sentinels — the one place that subtraction is allowed to exist.
walker reports zero violations and stays green forever. A further test asserts the walk is
actually handed the files the rules are about, so the gate cannot go vacuous from the other end.
graphCoverage.test.tsrecords every node type and edgefield webv2 emits, and a Python test checks them against the real
InvocationRegistry.capabilityContract.test.tsrecords the per-base capability table from the realBASE_GENERATIONobject, and a Python test checks every field against
FeaturesFacet. The second one was addedbecause transcribing the frontend's values into Python assertions is exactly the step that goes
stale — it is what let the ERNIE-Image row ship wrong.
Verified during development but not committed as tests, and stated that way rather than implied:
the 17 preview paths, the default-settings matrix, and the starter catalogue were compared
constant-for-constant against the pre-refactor revision by hand. What is committed is narrower: one
pinned preview pixel, hand-derived default-settings values checked against model cards, and the
structural starter-catalogue invariants.
Checks run locally (Windows, diffusers 0.40.0), on the branch merged with current
main: the fullPython suite is 7769 passed, 9 failed, and the nine are set-identical to a run of
origin/mainon the same machine (a local
HF_ENDPOINTagainst the SSRF guard).mypy invokeai/backend/architecturesreports nothing inside the package.
ruff checkandruff format --checkclean. webv2lint:tsc,lint:oxcandarchitecture:checkclean;vitest run7990 passed, 1 failed —
indexProgress.test.tscallstoLocaleString()without a locale andfails on a German machine only; it is green on CI runners and untouched here. Regenerating
openapi.jsonproduces a byte-identical file.Review
Three independent read-only review subagents were run against
origin/main, with distinct focuses:correctness and spec conformance; architecture, operational safety, performance and unnecessary
complexity; test value, coverage gaps and product quality. A fourth, blocker-only pass then reviewed
the resulting fixes — and found two defects in the FLUX one, both listed above: the first version
reversed the field preference for every base rather than only the guidance-labelled ones, and the
slider's own bound would have discarded the value anyway. Both are covered by tests that reject the
first version.
Every material finding is resolved above, except the two design questions raised under Facets that
carry no dispatch, which are left for a human to decide, and
ideal_size's square-canvasassumption, which is now an accurate
NOTEinstead of a false comment.The parts most worth a human eye are the facet declarations themselves — they are data, and a wrong
value fails at generation time, not at boot — and the starter-model split, where ordering is curated
product data.
Compatibility / Rollout
Additive to the API: one new route and the capabilities schema. No persisted-state migration. Node
modules moved, so anything importing
invokeai.app.invocations.<node>by its old flat path needs thepackage path — the tree is clean, and the merge playbook records the scan that finds stragglers in
both imports and string literals.
What newly identified models now persist, in full, because this is what a release reviewer needs
rather than a pointer into the sections above. Existing records are untouched in every case:
scheduler,steps,cfg_scalecfg_scaleinput)schedulerschedulerFourteen of sixteen architectures now declare a
schedulerwhere the oldfrom_basedeclared none.The runtime risk is low — every declared value equals the fallback webv2 was already applying, so no
prefilled slider moves — but it is a change to what lands in the database, and the earlier revision
of these notes mentioned only SD and Ideogram 4.
Review follow-up
An adversarial review of this branch produced findings that are now fixed here. The ones that
changed behaviour rather than wording:
safe_globalswas never actually asserted.test_it_matches_what_dependencies_installsdidnot import
dependencies; it rebuilt the list and checked its own copy. The list is now assembledonce by
conditioning_safe_globals()and pinned to the call site by an AST test.dimension_gridis variant-keyed. Wan TI2V-5B enforces multiples of 32 in the reference-imageencoder where A14B enforces 16 on the denoise node; a single number could not express that, and
the canvas offered 1280x720 for a model that rejects it.
ideal_sizeand the served table resolveit from the model's variant.
ideal_sizesized from a squared width. The declared default is a product decision, not atraining resolution. Fifteen architectures declare square defaults and are unchanged; MiniMax H3
is 1344x768, and squaring 1344 asked for 1.75x its area — it now returns 1344x736.
scripts/new_architecture.pysaid so twicein print, but
validate()only checks that required facets are present, and the stub declaredall five.
LatentSpacenow rejects an all-zero projection, so the generated module raises atimport.
validate()enforces only first-party facets, so a custom-node pack defining aREQUIREDfacet no longer kills boot blaming
defs/;register()accepts an identical re-declaration, so--dev_reloadsurvives a save underdefs/.discover_modulesfails on a package directory without__init__.pyinstead of silentlyfinding nothing.
discover_modulesimports nothing.pkgutil.walk_packagesimported every package to descendinto it, so a
_disabledpackage's__init__.pyran -- and could register nodes or abort startup-- before the underscore filter saw its name. The walk now reads names only and skips
_entriesbefore descending; a broken package fails at the caller's import with its own error. The module
sets found for invocations,
defsandfacetsare unchanged.paths; the default-settings matrix is now pinned per
(base, variant), including everyscheduler, which was previously unpinned and had already drifted out of the rollout notes below.invocation field it lands on. It found a pre-existing defect on its first run: webv2 writes
color_compensationontol2i, but the field is declared oni2l, so the SDXL colour-compensation toggle does nothing. Left unfixed here — it predates this branch — but recorded in a
ledger the contract checks by equality, so fixing it forces the entry to be removed.
starter_modelsexports the two catalogues and the types; 22 re-exports nothing imported are gone.The response models are no longer
extra="forbid", which stampedadditionalProperties: falseinto a table this PR expects to grow.
Checklist
What's Newcopy (if doing a release after this PR)