Skip to content

feat(architectures): declare variant enums per architecture and guard the unions - #29

Draft
Pfannkuchensack wants to merge 2 commits into
refactor/arch-latent-space-facetfrom
refactor/arch-variant-facet
Draft

feat(architectures): declare variant enums per architecture and guard the unions#29
Pfannkuchensack wants to merge 2 commits into
refactor/arch-latent-space-facetfrom
refactor/arch-variant-facet

Conversation

@Pfannkuchensack

Copy link
Copy Markdown
Collaborator

Stacked on #28, which is stacked on #27. Base is refactor/arch-latent-space-facet; the diff
shown here is against it. Review #27#28 → this.

Third of the series from .ideas/Backend Modularization Plan.md. No behaviour changes.

Why

One list of eleven variant enums is written by hand four timesAnyVariant, the
variant_type_adapter subscript, that adapter's runtime argument (all three in taxonomy.py), and
ModelRecordChanges.variant. Nothing checked that they agreed.

Nothing checked the invariant they all rest on either: variant values must be globally unique,
because build_common_fields resolves a bare variant string without knowing the base
(configs/factory.py). That invariant has already cost something — it is why
Krea2VariantType.Turbo is krea2_turbo rather than turbo — and until now it lived only in a
docstring.

The plan was wrong about the shape, and the code says so

The spec assumed one variant enum per base. It is not:

enum key base-keyed?
WanVariantType Wan × Main
WanLoRAVariantType Wan × LoRA same base, different enum
PiDDecoderVariantType five bases × PiDDecoder one enum, many bases
ClipVariantType, Qwen3VariantType base=Any impossibleAny is a sentinel the registry refuses

So VariantFacet carries a {ModelType: enum} mapping. Wan's two enums are not interchangeable —
an A14B LoRA (inner_dim=5120) against a TI2V-5B main (3072) crashes in the layer patcher, which is
why the separate enum exists at all.

The mapping was derived, not read off. A script walked Config_Base.CONFIG_CLASSES and extracted
every variant field's annotation. That corrected my hand-reading in one place: sdxl-refiner
mains carry ModelVariantType, which I would have missed.

Keeping the completeness check honest

Two enums cannot be declared by any architecture. Left implicit, the check would silently cover 9 of
11 while looking total. So there is a named BASE_AGNOSTIC_VARIANT_ENUMS allowlist — and a second
test asserting nothing in it is used by a real architecture, so it cannot become a dumping ground
for whatever makes CI pass.

Four architectures — CogView4, ERNIE-Image, Ideogram 4, Anima — model no variants and omit the
facet. An empty facet would be indistinguishable from an absent one.

configs/factory.py is deliberately untouched

Base-aware resolution would still need a fallback for the base=Any models, so it would move the
trap rather than close it — at the cost of touching the identification path, which the spec marks as
the one place not to take risks. The invariant is pinned by a test instead, in two forms:

  1. enum values are globally unique;
  2. variant_type_adapter.validate_strings(value) returns the right enum type for every value —
    the behavioural version of (1), through the exact call factory.py makes.

The facet is checked against reality

test_every_facet_declaration_matches_the_config_classes derives the expected mapping from the
config classes and checks both directions: a (base, type) the configs give a variant must be
declared, and a declaration the configs do not back must not exist.

Verified against both mistakes, then reverted:

VariantFacet declarations disagree with the model config classes:
  wan x lora: registry says WanVariantType, configs say WanLoRAVariantType
  wan x vae: registry declares WanVariantType, but no config class for that combination has a variant field

That message needed a fix: assert problems == [] printed nothing outside pytest's assertion
rewriting, so the assert now carries the list explicitly.

Noted, not fixed

configs/main.py's from_base signature lists only six of the enums and omits
QwenImageVariantType. Left alone — that annotation is on the way out in the capabilities PR, which
derives from_base from the registry.

Verification

  • pytest tests/backend/architectures tests/app/util tests/test_imports.py — 230 passed
  • mypy invokeai/backend/architectures (strict) — clean
  • ruff@0.11.2 check . + format --check — clean
  • openapi.json regenerated and compared normalized — identical. No Pydantic class, invocation
    field or enum is touched.

🤖 Generated with Claude Code

… the unions

One list of eleven variant enums is written by hand four times -- AnyVariant, the
variant_type_adapter subscript, that adapter's runtime argument (all three in
taxonomy.py) and ModelRecordChanges.variant. Nothing checked that they agreed.

Nothing checked the invariant they all rest on either: variant *values* must be
globally unique, because build_common_fields resolves a bare variant string
without knowing the base. That invariant has already cost something -- it is why
Krea2VariantType.Turbo is `krea2_turbo` rather than `turbo` -- and until now it
lived only in a docstring.

VariantFacet is keyed by model type, not by base alone.

The plan assumed one variant enum per base. The code disagrees, and the mapping
here was derived from Config_Base.CONFIG_CLASSES rather than read off by hand:

  - Wan mains carry WanVariantType, Wan LoRAs carry WanLoRAVariantType. Same
    base, different enum, and not interchangeable -- an A14B LoRA against a
    TI2V-5B main crashes in the layer patcher on a tensor shape.
  - PiDDecoderVariantType is one enum shared across five bases.
  - ClipVariantType and Qwen3VariantType sit on base=Any configs. Any is a
    sentinel the registry refuses to register, so they cannot be declared at all.

The last point is why the completeness check carries an explicit
BASE_AGNOSTIC_VARIANT_ENUMS allowlist: without it the check would silently cover
9 of 11 enums while looking total. A second test asserts that nothing in that
allowlist is used by a real architecture, so it cannot become a dumping ground.

Four architectures -- CogView4, ERNIE-Image, Ideogram 4, Anima -- model no
variants and omit the facet. An empty facet would be indistinguishable from an
absent one.

configs/factory.py is deliberately untouched. Base-aware resolution would still
need a fallback for the base=Any models, so it would move the trap rather than
close it, at the cost of touching the identification path. The invariant is
pinned by a test instead, in two forms: enum values are unique, and
variant_type_adapter.validate_strings returns the right enum type for every
value -- the second being the behavioural version of the first, through the
exact call factory.py makes.

test_every_facet_declaration_matches_the_config_classes derives the expected
mapping from the config classes and checks both directions, so a declaration the
configs do not back fails just as loudly as a missing one. Verified against both
mistakes: pointing Wan LoRAs at the main enum, and declaring a model type no
config has a variant field for.

Deriving rather than reading also corrected the mapping: sdxl-refiner mains carry
ModelVariantType, which a hand-written list would have missed.

configs/main.py's from_base signature still lists only six of the enums and omits
QwenImageVariantType. Left alone -- that annotation is on the way out in the
capabilities PR, which derives from_base from the registry.

openapi.json, schema.ts and invocation-context.json are unchanged.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant