Skip to content

refactor(architectures): drive latent-space facts from the registry - #28

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

refactor(architectures): drive latent-space facts from the registry#28
Pfannkuchensack wants to merge 2 commits into
refactor/arch-facet-registryfrom
refactor/arch-latent-space-facet

Conversation

@Pfannkuchensack

Copy link
Copy Markdown
Collaborator

Stacked on #27. Base is refactor/arch-facet-registry; review that first. The diff shown here
is against it, not against main.

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

Why

The preview chain was never really keyed on architecture — the branch comments in the dispatch chain
this removes said so by hand: "Krea-2 decodes with the Qwen-Image VAE", "Z-Image uses
FLUX-compatible VAE"
, "Anima uses Wan 2.1 VAE". Fifteen architectures share eight latent
spaces, and the five most recent introduced no new preview data at all.

That one fact lived in six partial, drifting copies. This replaces four of them.

Reading the diff

Every projection matrix moves byte-identically, comments included — use
git diff --color-moved. step_callback.py goes from 396 lines to 60.

The two Wan dispatches collapse into one

step_callback.py picked the factors by sample.shape[-3] == 48, then forty lines later picked the
spatial scale by the same test. Two halves of one fact, in two places, free to drift. Resolving one
LatentSpace now settles both, because it carries spatial_compression.

LatentSpaceFacet.resolve() matches on channel count — literally what the old comment claimed it was
doing ("the latent channel count uniquely identifies the variant"). An architecture with a single
latent space short-circuits without touching the sample at all, so an unusually shaped tensor
cannot start raising IndexError for the other fourteen.

Deliberately not changed

  • Ideogram 4 keeps its own preview loop. Its callback signature is (step, total, packed_latents)
    and it must unpatchify and denormalize first, so it cannot route through diffusion_step_callback
    without a fabricated PipelineIntermediateState. It now reads the same declaration instead of
    inlining the FLUX.2 factors and a hardcoded × 8. Its try/except fallback stays, and so does its
    missing is_canceled() check — that is a real bug (Ideogram generations do not stop promptly on
    cancel), but fixing it would change behaviour. Follow-up.
  • invocation_context.py is untouched. The spec proposed collapsing flux_step_callback /
    flux2_step_callback into sd_step_callback, but they are already thin wrappers over the same
    function — the change would be pure docs churn in docs/src/generated/invocation-context.json.
  • constants.LATENT_SCALE_FACTOR (blast radius: every latent node) and pid/decode.py (same
    shape, different units — packed latents at 128ch/16×, not VAE latents at 32ch/8×). Both follow-ups
    now that a correct source exists.
  • denoise_latents.py keeps a third, divergent copy of the SDXL BGR rule. It reads the UNet's
    base where the other paths read each adapter's; the two disagree for an SD1 adapter on an SDXL
    UNet. Flagged with a NOTE rather than fixed — the natural home is a bgr_input field on
    UNetDownscaleFacet, together with untangling the two parallel T2I-Adapter paths.

max_unet_downscale

Duplicated verbatim across denoise_latents and T2IAdapterExt, comment and error string included.
It is a UNet property, not VAE geometry, so it gets its own optional facet rather than a field on
LatentSpaceFacet. The accessor — not require() — carries the error, reproducing the old message
exactly, including that BaseModelType is a str, Enum mixin and so renders as BaseModelType.Flux
rather than flux.

The three identical Wan 2.1 matrices

QWEN_IMAGE_, ANIMA_ and WAN_LATENT_RGB_FACTORS were byte-identical (verified programmatically),
as were their three biases. Now one WAN21_16, asserted by object identity so the duplication
cannot creep back.

Two things worth a reviewer's attention

The relocated projection tests now hardcode their expected pixel. The versions they replace
computed the expectation by summing the very matrix under test — a tautology that would have passed
against a corrupted matrix. It had already drifted: that test's docstring claimed column sums of
0.3677 / 0.4577 / 0.9101 where the real ones are 0.3887 / 0.8771 / 1.3152.

The registry fixture now empties Facet.FACET_TYPES too. The first REQUIRED facet exposed that
isolating only one of the two globals let validate() hold the dummy architectures to real facets.

Tests

  • test_latent_space.py — a reference table read off the old elif chain, one row per architecture,
    plus first/last rows and column sums as exact fingerprints (IEEE-754 addition in fixed order, so
    platform-independent; the rows catch a reordering a sum is blind to). Plus the closed-set check and
    the relocated projection tests.
  • test_step_callback.pydiffusion_step_callback end to end for all 15 bases and both Wan cases.
    The first automated coverage the spatial-scale path has ever had.
  • test_unet_downscale.py — both values, and the verbatim error message for the other 13.

Verification

  • pytest tests/backend/architectures tests/app/util tests/test_imports.py — 218 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, so schema.ts and invocation-context.json are unchanged too.

Not done here, and worth doing before merge: a manual generation on SD1, FLUX and Wan TI2V-5B to
see previews render at the right dimensions on hardware. The reference table covers the data; it
cannot cover the screen.

🤖 Generated with Claude Code

The preview chain was never really keyed on architecture. The branch comments in
the dispatch chain this removes said so by hand -- "Krea-2 decodes with the
Qwen-Image VAE", "Z-Image uses FLUX-compatible VAE", "Anima uses Wan 2.1 VAE".
Fifteen architectures share eight latent spaces, and the five most recent
introduced no new preview data at all.

That one fact was written in six partial, drifting copies. This replaces four of
them with LatentSpaceFacet; the remaining two are named below as follow-ups.

No behaviour changes. Every projection matrix moves byte-identically, comments
included -- read the move with `git diff --color-moved`.

Both Wan dispatches collapse into one. step_callback.py used to pick the factors
by `sample.shape[-3] == 48` and then, forty lines later, pick the spatial scale
by the same test. They are two halves of one fact and could drift; now resolving
one LatentSpace settles both. LatentSpaceFacet.resolve matches on channel count,
which is literally what the old comment said it was doing ("the latent channel
count uniquely identifies the variant"). An architecture with a single latent
space short-circuits without touching the sample at all, so an unusually shaped
tensor cannot start raising IndexError for the other fourteen.

Ideogram 4 keeps its own preview loop -- its callback signature is
step/total/packed_latents and it must unpatchify and denormalize first -- but it
now reads the same declaration instead of inlining the FLUX.2 factors and a
hardcoded x8. Its try/except fallback and its missing is_canceled check are left
alone; the latter is a real bug, in that Ideogram generations do not stop
promptly on cancel, but fixing it would change behaviour, which this PR does not.

max_unet_downscale was duplicated verbatim across denoise_latents and
T2IAdapterExt, comment and error string included. It is a UNet property, not VAE
geometry, so it gets its own optional facet; the accessor rather than require
carries the error, reproducing the old message exactly, enum repr included.

The three Wan 2.1 matrices -- QWEN_IMAGE_, ANIMA_ and WAN_LATENT_RGB_FACTORS --
were byte-identical, as were their three biases. They are now one WAN21_16,
asserted by object identity so the duplication cannot creep back.

Two known copies stay: constants.LATENT_SCALE_FACTOR, whose blast radius is
every latent node, and pid/decode.py, which is the same shape in different units
(packed latents at 128ch/16x, not VAE latents at 32ch/8x). Both are follow-ups
now that a correct source exists.

invocation_context.py is untouched. The spec proposed collapsing
flux_step_callback and flux2_step_callback into sd_step_callback, but they are
already thin wrappers over the same function, so the change would be pure docs
churn in docs/src/generated/invocation-context.json.

Tests. test_latent_space.py carries a reference table read off the old chain, one
row per architecture, plus first/last rows and column sums as exact fingerprints
-- IEEE-754 addition in fixed order, so platform-independent, and the rows catch
a reordering a sum is blind to. test_step_callback.py now covers
diffusion_step_callback end to end for all 15 bases and both Wan cases, which is
the first automated coverage the spatial-scale path has ever had.

The relocated projection tests hardcode their expected pixel. The versions they
replace recomputed it by summing the very matrix under test, so they would have
passed against a corrupted one -- and had in fact drifted: that test's docstring
claimed column sums of 0.3677/0.4577/0.9101 where the real ones are
0.3887/0.8771/1.3152.

The registry fixture now empties Facet.FACET_TYPES as well as the registry. The
first REQUIRED facet exposed that isolating only one of the two globals let
validate() hold dummy architectures to real facets.

denoise_latents.py keeps a third, divergent copy of the SDXL BGR rule: it reads
the UNet's base where the other paths read each adapter's. The two disagree for
an SD1 adapter on an SDXL UNet. Left as-is with a NOTE, since fixing it changes
behaviour; the natural home is a bgr_input field on UNetDownscaleFacet.

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