Repository navigation
Two merge-time invariants have no test: params-slice migration chain integrity, and vendored MiniMax-H3 drift from upstream #9628
Description
Context: landing the 13-PR MiniMax H3 stack (#56-#84). Five of the defects hit during that work were invisible in any individual PR — they only existed once two branches met, and git reported a clean auto-merge for most of them. Two classes have no test today.
Two others already do, and worked: test_bundled_workflow_nodes_match_invocation_registry caught the node-version mismatches in invoke-ai#63 and invoke-ai#64, and openapi-checks / typegen-checks cover generated-schema drift. Nothing needed there.
1. Params-slice migration chain has no structural invariant test
Hit three times: invoke-ai#59, invoke-ai#61, invoke-ai#62.
Each PR added a persisted-state migration step at what was, on its own branch, the next free version. main independently took the same version. On merge, paramsSlice.ts and types.ts auto-merged without a conflict, producing two consecutive steps sharing a from version:
if (state._version === 5) { /* flux2 fields */ state._version = 6; } // wins
if (state._version === 5) { /* H3 fields */ state._version = 6; } // dead codeThe second step never runs. Its fields are required with no default, so zParamsState.parse() throws and redux-persist wipes the entire params slice — prompt, seed, dimensions, every model selection — on upgrade.
The existing tests in paramsSlice.test.ts are all per-migration (backfills X when migrating from vN). They catch this only if a test happens to exercise a blob at exactly the colliding version, and they report it as a confusing downstream parse error rather than naming the cause. Nothing asserts the chain's shape.
Suggested test
Two assertions over the migrate function's source, or over a declarative step table if one is preferred:
- no two steps share a
fromversion - the chain terminates at the
_versionliteral inzParamsState
Ad-hoc version used during the stack:
grep -o "state._version === [0-9]*" paramsSlice.ts | sort | uniq -d # must be empty
grep -n "_version: z.literal" types.ts # must equal the chain terminusA real test should assert the same two properties. It is O(1) to write, cannot be defeated by a merge resolution, and fails with the actual cause.
2. Vendored MiniMax-H3 files can drift from upstream with nothing to catch it
Hit twice: invoke-ai#77 and invoke-ai#82.
invokeai/backend/minimax_h3/ holds five files vendored from huggingface/diffusers@abc5e9bf71 (the unreleased minimax-h3 branch). The contract is stated in invokeai/backend/minimax_h3/__init__.py: keep them byte-identical to upstream apart from the absolute-import rewrite, so that re-syncing when a tagged diffusers ships the H3 classes is a clean copy rather than a manual re-application.
Two ways it drifted, neither detected:
ruff formatreached them. Any branch created before the[tool.ruff.format]exclude landed had its vendored files rewrapped — one line each inpacking.py,transformer_minimax_h3.py,autoencoder_kl_minimax_h3_audio.py.- First-party code was added into a vendored module. feat(minimax-h3): dropdowns for canvas size and frame count InvokeAI-7#82 originally added
MINIMAX_H3_VIDEO_FRAME_CHOICES,_validated_aspect_ratioandresolve_lowres_canvas_sizedirectly topacking.py, and refactored the vendoredresolve_canvas_sizeto share a helper. (Moved topresets.pybefore merge.)
Both are silent. Nothing fails; the cost lands on whoever does the eventual upstream re-sync and finds it is no longer a copy.
Related, and worth guarding at the same time: the exclude was initially written as a directory glob, invokeai/backend/minimax_h3/*.py. That was right when everything in the package was vendored, but the package now also holds first-party modules (presets.py, sampling.py, denoise.py, int8_convrot.py, pruned_adaln_lora.py, taehv_decoder.py) which would silently stop being formatted. It is now a list of five named files — a test should keep it that way.
Suggested test
Vendor the five upstream files as fixtures (or check in their hashes) and assert each differs from its vendored copy only by lines matching ^(from|import) . That catches both formatter drift and added first-party code, and it fails with a diff pointing at the offending lines.
A cheaper variant, if fixtures are unwelcome: assert the [tool.ruff.format] exclude lists exactly the five vendored files by name, and that no other module in the package appears in it. That catches the glob regression but not content drift.
Why file this
Neither check is about MiniMax H3 specifically. The migration-chain one applies to every persisted redux slice with a version chain — uiSlice has its own, and the same collision is available there. The vendoring one applies wherever we vendor upstream code under a byte-identical contract.
Both are cheap. The migration test in particular would have prevented a slice-wiping bug from reaching main three separate times, each of which git presented as a clean merge.
Activity