fix(lora): correctly detect flattened layer keys in LayerPatcher - #9295
Conversation
The patcher decided whether a patch's layer keys were flattened (legacy underscore-joined) or real dotted module paths by inspecting only the first key. For FLUX.2 Klein diffusers LoRAs whose first converted layer is a dotless top-level module (e.g. `context_embedder`), the whole patch was misclassified as flattened, causing `assert "." not in layer_key` to fail on subsequent dotted keys and crashing LoRA application. Inspect all keys instead: a flattened key never contains a dot, so the patch is flattened only if no key contains one. Add a regression test covering the mixed dotless/dotted key ordering.
lstein
left a comment
There was a problem hiding this comment.
Approve — correct, minimal, well-tested fix.
What it does
apply_smart_model_patch previously inferred whether a patch's layer keys were flattened (legacy .→_) vs. dotted (real module paths) from only the first key. A FLUX.2 Klein diffusers LoRA whose first converted layer targets a dotless top-level module (context_embedder, x_embedder, proj_out) was misclassified as flattened, so subsequent genuinely-dotted keys tripped assert "." not in layer_key in _get_submodule and crashed LoRA application. The fix inspects all keys — a patch is flattened only if no key contains a dot.
Correctness
- The invariant is sound: a flattened key can never contain a dot, so "flattened ⟺ no key has a dot" is the correct definition. Classification flips exactly on the buggy case (old →
True, new →False). - No regression for all-dotless patches: if a non-flattened patch has only single-token top-level keys, the flattened
_get_submodulepath still resolves them correctly (as it did before). The only behavioral change is for mixed dotted/dotless patches — precisely the bug. - Bonus robustness: the old
next(iter(...))raisedStopIterationon an emptypatch.layers;any()over empty returnsFalsecleanly. - Performance: negligible —
any()short-circuits on the first dotted key (the common modern case).
Test
test_apply_smart_model_patches_mixed_dotted_and_dotless_keys reproduces the exact failure ordering (dotless key first, dotted key second) and verifies patch/unpatch round-trips. It fails on the old single-key heuristic and passes now. Full suite: 11 passed, 9 skipped.
Minor nits (non-blocking)
- The new comment references the converted key as
lora_transformer-context_embedder(hyphen) — reads like a typo if that's not the literal converted format. - Coverage exercises only the direct-patching path, but the sidecar path shares the same classification code, so this is fine.
Summary
The patcher decided whether a patch's layer keys were flattened (legacy underscore-joined) or real dotted module paths by inspecting only the first key. For FLUX.2 Klein diffusers LoRAs whose first converted layer is a dotless top-level module (e.g.
context_embedder), the whole patch was misclassified as flattened, causingassert "." not in layer_keyto fail on subsequent dotted keys and crashing LoRA application.Inspect all keys instead: a flattened key never contains a dot, so the patch is flattened only if no key contains one. Add a regression test covering the mixed dotless/dotted key ordering.
Related Issues / Discussions
https://discord.com/channels/1020123559063990373/1149510134058471514/1517709403858669708
QA Instructions
A FLUX.2 Klein diffusers-format LoRA whose first converted layer targets a dotless top-level module (e.g.
transformer.context_embedder,transformer.x_embedder,transformer.proj_out) previously crashed on load with:File ".../invokeai/backend/patches/layer_patcher.py", line 289, in _get_submodule
assert "." not in layer_key
To verify:
uv run --extra cuda --extra test python -m pytest tests/backend/patches/test_layer_patcher.py -q
All tests pass (including the new
test_apply_smart_model_patches_mixed_dotted_and_dotless_keysregression test, which fails on the old single-first-key heuristic).Merge Plan
Standard merge — isolated, low-risk change to LoRA patching logic. No DB/schema changes.
Checklist
What's Newcopy (if doing a release after this PR)