From 257e28a0d95a15111e5bf4f03c0cbc0e0cf57def Mon Sep 17 00:00:00 2001 From: Lincoln Stein Date: Thu, 30 Jul 2026 20:21:42 -0400 Subject: [PATCH 1/6] fix(ui): pin the conditional half of the Wan v3->v4 seeds MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Wan video PR (#9163) added five keys to zParamsState (wanTransformerLowNoise, wanComponentSource, wanVaeModel, wanT5EncoderModel, wanGuidanceScaleLowNoise) while the persisted params schema was still at _version 3, without a version bump or migration seed. The keys are .nullable() with no .default(), which zod treats as required, and migrate() ends with zParamsState.parse() whose failure makes the store silently replace the slice with its initial state. Released v6.13.x builds write v3 blobs without these keys, so any user upgrading from a release to a build containing Wan loses their entire params slice (prompts, prompt history, model selection, dimensions, generation settings) on first launch. Dev machines don't reproduce it because v3 blobs written after the Wan merge already carry the keys. The seeds themselves are no longer this commit's job: they reached main with the FLUX.2 [dev] merge (f10d2a4f5a), together with a field-accurate released-build v3 fixture that fails without them. What main does not cover is the other half of the contract — that the seeds are written with `??` rather than assigned, so a v3 blob from a dev build after the Wan merge keeps the values it already holds instead of having them reset to null. Add that test. Co-Authored-By: Claude Fable 5 --- .../controlLayers/store/paramsSlice.test.ts | 21 +++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/invokeai/frontend/web/src/features/controlLayers/store/paramsSlice.test.ts b/invokeai/frontend/web/src/features/controlLayers/store/paramsSlice.test.ts index 9e95dde86b8..10d4004a4a8 100644 --- a/invokeai/frontend/web/src/features/controlLayers/store/paramsSlice.test.ts +++ b/invokeai/frontend/web/src/features/controlLayers/store/paramsSlice.test.ts @@ -367,6 +367,27 @@ describe('paramsSliceConfig persisted state migration', () => { expect(result.seed).toBe(99); }); + it('preserves Wan values already present in a dev-build v3 blob', () => { + expect(migrate).toBeDefined(); + + const initial = getInitialParamsState(); + const wanVae = { key: 'wan-vae', hash: 'h', name: 'Wan VAE', base: 'wan', type: 'vae' }; + // v3 blobs written by dev builds after the Wan merge already carry the keys, possibly with + // real values — the conditional seeds must not clobber them. + const v3State: Record = { + ...initial, + _version: 3, + wanVaeModel: wanVae, + wanGuidanceScaleLowNoise: 3.5, + }; + + const result = migrate?.(v3State) as ReturnType; + + expect(result._version).toBe(5); + expect((result.wanVaeModel as { key: string } | null)?.key).toBe('wan-vae'); + expect(result.wanGuidanceScaleLowNoise).toBe(3.5); + }); + it('migrates old positive prompt history entries to prompt pairs', () => { expect(migrate).toBeDefined(); From 827717dfd2996a97e58d117ec96a1ec6c822bcf4 Mon Sep 17 00:00:00 2001 From: Lincoln Stein Date: Sun, 2 Aug 2026 14:01:58 -0400 Subject: [PATCH 2/6] fix(ui): seed post-v3 params fields and add a migration safety net Addresses review feedback on #9408. Finding 1 (incomplete fix): the v3->v4 Wan seeds fixed upgrades from v6.13.x, but releases v6.10.0 - v6.12.0 persist _version 2 blobs, and 15 keys added to zParamsState after v3 was cut are required (no .default(), .optional() or .catch()) and seeded nowhere: fluxDype{Preset,Scale, Exponent}, zImageShift, zImageSeedVariance{Enabled,Strength, RandomizePercent}, anima{VaeModel,Qwen3EncoderModel,Scheduler}, klein{VaeModel,Qwen3EncoderModel} and qwenImage{ComponentSource, Quantization,Shift}. Seed them conditionally in the v2->v3 step, so released v2 blobs migrate cleanly and dev-build v2 blobs keep the values they already hold. Verified by running the real migrate() over a blob built from the v6.10.0 release key set: it threw on exactly those 15 paths before this change. Finding 2 (tests can't catch the next occurrence): the fixtures spread getInitialParamsState(), so they carry every current key and are inert against the general defect. Replace them with the top-level zParamsState key sets as actually shipped, read out of the release tags and checked in, one per persisted version still in the wild (v6.10.0 for v2 and v6.13.7 for v3 - each the narrowest key set among the releases writing that version, so a subset of every real blob). Add a schema-completeness test that runs the version steps over each release blob and asserts no key of the current schema is left unhandled, naming the offending keys and the step to fix. It fails on any future required-no-default key added without a seed. Finding 3 (fail-open-and-destroy): a single missing key made zParamsState.parse() throw, and the caller in store.ts falls back to the initial state, wiping prompts, model selection and dimensions with only a log.warn. Add backfillMissingParamsKeys(): after the version steps, fill any key that is absent and that the schema cannot fill itself, and warn with the key names. Narrow by design - a key that is present but invalid still throws, and anything with a .default()/.catch()/.optional() is left to zod. So a forgotten seed now costs one field at its default instead of the user's whole params slice. The completeness test above deliberately bypasses the net so it still fails CI. Co-Authored-By: Claude Opus 5 (1M context) --- .../controlLayers/store/paramsSlice.test.ts | 358 ++++++++++++++++-- .../controlLayers/store/paramsSlice.ts | 251 ++++++++---- 2 files changed, 497 insertions(+), 112 deletions(-) diff --git a/invokeai/frontend/web/src/features/controlLayers/store/paramsSlice.test.ts b/invokeai/frontend/web/src/features/controlLayers/store/paramsSlice.test.ts index 10d4004a4a8..bf4da97aab6 100644 --- a/invokeai/frontend/web/src/features/controlLayers/store/paramsSlice.test.ts +++ b/invokeai/frontend/web/src/features/controlLayers/store/paramsSlice.test.ts @@ -8,6 +8,8 @@ import type { import { describe, expect, it } from 'vitest'; import { + applyParamsVersionMigrations, + backfillMissingParamsKeys, isValidKrea2RebalanceWeights, KREA2_REBALANCE_WEIGHT_COUNT, modelChanged, @@ -23,7 +25,7 @@ import { selectModelSupportsSteps, setIdeogram4Steps, } from './paramsSlice'; -import { getInitialParamsState } from './types'; +import { getInitialParamsState, zParamsState } from './types'; const buildExternalModelIdentifier = (config: ExternalApiModelConfig) => ({ @@ -141,6 +143,181 @@ describe('paramsSlice selectors for external models', () => { }); }); +/** + * Top-level `zParamsState` key sets as actually shipped, taken from + * `git show :...features/controlLayers/store/types.ts`. + * + * These are historical facts and must not be regenerated from `getInitialParamsState()` — a fixture + * spread from the current initial state carries every current key and therefore cannot detect a + * newly added key that the migration chain forgets to seed. That is precisely what masked the Wan + * regression these tests exist to catch. + * + * One entry per persisted `_version` still in the wild, each picked as the *narrowest* key set among + * the releases writing that version, so it is a subset of every real blob of that version: + * - v2: v6.10.0 - v6.12.0. v6.10.0 is the narrowest (later v2 releases only added keys). + * - v3: v6.13.0 - v6.13.7. v6.13.7 is the narrowest (v6.13.0 additionally had `animaT5EncoderModel`, + * since removed from the schema; unknown keys are stripped by the non-strict object parse). + * v4 blobs are written by v6.14.0-rc1 onward; the v4 -> v5 step is covered by its own tests below. + */ +const RELEASE_PARAMS_KEYS = { + 'v6.10.0': { + version: 2, + keys: [ + '_version', + 'canvasCoherenceEdgeSize', + 'canvasCoherenceMinDenoise', + 'canvasCoherenceMode', + 'cfgRescaleMultiplier', + 'cfgScale', + 'clipEmbedModel', + 'clipGEmbedModel', + 'clipLEmbedModel', + 'clipSkip', + 'colorCompensation', + 'controlLora', + 'dimensions', + 'fluxScheduler', + 'fluxVAE', + 'guidance', + 'img2imgStrength', + 'infillColorValue', + 'infillMethod', + 'infillPatchmatchDownscaleSize', + 'infillTileSize', + 'iterations', + 'maskBlur', + 'maskBlurMethod', + 'model', + 'negativePrompt', + 'optimizedDenoisingEnabled', + 'positivePrompt', + 'positivePromptHistory', + 'refinerCFGScale', + 'refinerModel', + 'refinerNegativeAestheticScore', + 'refinerPositiveAestheticScore', + 'refinerScheduler', + 'refinerStart', + 'refinerSteps', + 'scheduler', + 'seamlessXAxis', + 'seamlessYAxis', + 'seed', + 'shouldRandomizeSeed', + 'shouldUseCpuNoise', + 'steps', + 't5EncoderModel', + 'upscaleCfgScale', + 'upscaleScheduler', + 'vae', + 'vaePrecision', + 'zImageQwen3EncoderModel', + 'zImageQwen3SourceModel', + 'zImageScheduler', + 'zImageVaeModel', + ], + }, + 'v6.13.7': { + version: 3, + keys: [ + '_version', + 'animaQwen3EncoderModel', + 'animaScheduler', + 'animaVaeModel', + 'canvasCoherenceEdgeSize', + 'canvasCoherenceMinDenoise', + 'canvasCoherenceMode', + 'cfgRescaleMultiplier', + 'cfgScale', + 'clipEmbedModel', + 'clipGEmbedModel', + 'clipLEmbedModel', + 'clipSkip', + 'colorCompensation', + 'controlLora', + 'dimensions', + 'fluxDypeExponent', + 'fluxDypePreset', + 'fluxDypeScale', + 'fluxScheduler', + 'fluxVAE', + 'geminiTemperature', + 'geminiThinkingLevel', + 'guidance', + 'imageSize', + 'img2imgStrength', + 'infillColorValue', + 'infillMethod', + 'infillPatchmatchDownscaleSize', + 'infillTileSize', + 'iterations', + 'kleinQwen3EncoderModel', + 'kleinVaeModel', + 'maskBlur', + 'maskBlurMethod', + 'model', + 'negativePrompt', + 'openaiBackground', + 'openaiInputFidelity', + 'openaiQuality', + 'optimizedDenoisingEnabled', + 'positivePrompt', + 'positivePromptHistory', + 'qwenImageComponentSource', + 'qwenImageQuantization', + 'qwenImageQwenVLEncoderModel', + 'qwenImageShift', + 'qwenImageVaeModel', + 'refinerCFGScale', + 'refinerModel', + 'refinerNegativeAestheticScore', + 'refinerPositiveAestheticScore', + 'refinerScheduler', + 'refinerStart', + 'refinerSteps', + 'scheduler', + 'seamlessXAxis', + 'seamlessYAxis', + 'seed', + 'seedreamOptimizePrompt', + 'seedreamWatermark', + 'shouldRandomizeSeed', + 'shouldUseCpuNoise', + 'steps', + 't5EncoderModel', + 'upscaleCfgScale', + 'upscaleScheduler', + 'vae', + 'vaePrecision', + 'zImageQwen3EncoderModel', + 'zImageQwen3SourceModel', + 'zImageScheduler', + 'zImageSeedVarianceEnabled', + 'zImageSeedVarianceRandomizePercent', + 'zImageSeedVarianceStrength', + 'zImageShift', + 'zImageVaeModel', + ], + }, +} as const satisfies Record; + +/** + * Build a blob shaped exactly like the one the given release persisted: current initial values, but + * restricted to the keys that release's schema actually had. + */ +const buildReleaseBlob = (release: keyof typeof RELEASE_PARAMS_KEYS, overrides: Record = {}) => { + const { version, keys } = RELEASE_PARAMS_KEYS[release]; + const initial = getInitialParamsState() as unknown as Record; + const blob: Record = {}; + for (const key of keys) { + if (key in initial) { + blob[key] = initial[key]; + } + } + blob._version = version; + return { ...blob, ...overrides }; +}; + describe('paramsSliceConfig persisted state migration', () => { const migrate = paramsSliceConfig.persistConfig?.migrate; @@ -255,37 +432,6 @@ describe('paramsSliceConfig persisted state migration', () => { expect(result.dimensions).toMatchObject({ width: 640, height: 896 }); }); - it('seeds the Wan fields for a released-build v3 blob that predates the Wan merge', () => { - expect(migrate).toBeDefined(); - - const initial = getInitialParamsState(); - // Released v6.13.x builds wrote v3 blobs before the Wan fields existed. They're nullable - // with no default, so if the v3 -> v4 step didn't seed them, parse() would throw and the - // whole slice would be wiped on upgrade. - const v3State: Record = { - ...initial, - _version: 3, - positivePrompt: 'a fluffy cat', - }; - delete v3State.wanTransformerLowNoise; - delete v3State.wanComponentSource; - delete v3State.wanVaeModel; - delete v3State.wanT5EncoderModel; - delete v3State.wanGuidanceScaleLowNoise; - delete v3State.flux2VaeModel; - delete v3State.flux2DevMistralEncoderModel; - - const result = migrate?.(v3State) as ReturnType; - - expect(result._version).toBe(5); - expect(result.wanTransformerLowNoise).toBeNull(); - expect(result.wanComponentSource).toBeNull(); - expect(result.wanVaeModel).toBeNull(); - expect(result.wanT5EncoderModel).toBeNull(); - expect(result.wanGuidanceScaleLowNoise).toBeNull(); - expect(result.positivePrompt).toBe('a fluffy cat'); - }); - it('migrates a v4 blob written by main (PiD fields, no flux2 fields) without wiping it', () => { expect(migrate).toBeDefined(); @@ -367,6 +513,154 @@ describe('paramsSliceConfig persisted state migration', () => { expect(result.seed).toBe(99); }); + it.each(['v6.10.0', 'v6.13.7'] as const)('migrates a genuine %s blob without losing the user params', (release) => { + expect(migrate).toBeDefined(); + + // A real released-build blob only has the keys that release's schema declared. Any key added + // since that has neither a zod default nor a migration seed fails the parse() at the end of + // migrate(), and the caller in store.ts swallows the throw and falls back to the initial + // state — silently wiping the user's prompts, model selection and dimensions on upgrade. + const blob = buildReleaseBlob(release, { + positivePrompt: 'a fluffy cat', + seed: 42, + shouldRandomizeSeed: false, + }); + + const result = migrate?.(blob) as ReturnType; + + expect(result._version).toBe(5); + expect(result.positivePrompt).toBe('a fluffy cat'); + expect(result.seed).toBe(42); + expect(result.shouldRandomizeSeed).toBe(false); + }); + + it('seeds every key of the current schema in the version steps themselves, for each released blob version', () => { + // The general form of the defect this suite guards against: a key is added to zParamsState with + // no `.default()`/`.optional()`/`.catch()` (so zod treats it as required) and no seed in the + // migration chain. Every such key is a whole-slice wipe for anyone upgrading from a release + // that predates it. Rather than enumerate keys by hand, assert the invariant over the whole + // schema, so the next occurrence fails here instead of shipping. + // + // This deliberately runs the version steps *without* going through migrate(), because + // backfillMissingParamsKeys() would otherwise repair the omission and hide it. The safety net + // is there to protect users from a forgotten seed; this test is what stops one being merged. + for (const release of Object.keys(RELEASE_PARAMS_KEYS) as (keyof typeof RELEASE_PARAMS_KEYS)[]) { + const blob = buildReleaseBlob(release); + + applyParamsVersionMigrations(blob); + const unseeded = backfillMissingParamsKeys(blob); + + expect( + unseeded, + `Keys missing from a genuine ${release} blob that neither carry a zod default nor get seeded by ` + + `the migration chain. Upgrading from ${release} would throw in zParamsState.parse() and wipe the ` + + `user's whole params slice. Give each key a zod default, or seed it in the _version ` + + `${RELEASE_PARAMS_KEYS[release].version} migration step.` + ).toEqual([]); + } + }); + + it('backfills a key the version steps forget, instead of wiping the slice', () => { + expect(migrate).toBeDefined(); + + // Simulate the next occurrence of the defect: a required key that no migration step seeds. The + // safety net must fill it and let everything else through, rather than throwing and handing the + // caller in store.ts an excuse to reset the slice. + const blob = buildReleaseBlob('v6.13.7', { positivePrompt: 'a fluffy cat', seed: 42 }); + applyParamsVersionMigrations(blob); + delete blob.pidSteps; + + const backfilled = backfillMissingParamsKeys(blob); + + expect(backfilled).toEqual(['pidSteps']); + expect(blob.pidSteps).toBe(4); + expect(() => zParamsState.parse(blob)).not.toThrow(); + expect(blob.positivePrompt).toBe('a fluffy cat'); + }); + + it('does not backfill over a key the persisted state already holds', () => { + // The net must only fill omissions — never overwrite a real persisted value, and never mask a + // present-but-invalid one (that still throws, same as before). + const blob = buildReleaseBlob('v6.13.7'); + applyParamsVersionMigrations(blob); + blob.pidSteps = 2; + blob.positivePrompt = 'a fluffy cat'; + + expect(backfillMissingParamsKeys(blob)).toEqual([]); + expect(blob.pidSteps).toBe(2); + expect(blob.positivePrompt).toBe('a fluffy cat'); + }); + + it('seeds the Wan fields for a released-build v3 blob that predates the Wan merge', () => { + expect(migrate).toBeDefined(); + + // Released v6.13.x builds wrote v3 blobs before the Wan fields existed. They're nullable with + // no default, so if the v3 -> v4 step didn't seed them, parse() would throw and the whole + // slice would be wiped on upgrade. + const v3State = buildReleaseBlob('v6.13.7', { positivePrompt: 'a fluffy cat', seed: 42 }); + expect('wanVaeModel' in v3State).toBe(false); + + const result = migrate?.(v3State) as ReturnType; + + expect(result._version).toBe(5); + expect(result.wanTransformerLowNoise).toBeNull(); + expect(result.wanComponentSource).toBeNull(); + expect(result.wanVaeModel).toBeNull(); + expect(result.wanT5EncoderModel).toBeNull(); + expect(result.wanGuidanceScaleLowNoise).toBeNull(); + // Unrelated params must survive the migration (they'd be wiped if parse() threw). + expect(result.positivePrompt).toBe('a fluffy cat'); + expect(result.seed).toBe(42); + }); + + it('seeds the post-v3 fields for a released-build v2 blob (v6.10.0 - v6.12.0)', () => { + expect(migrate).toBeDefined(); + + // Same class of defect one version earlier: these keys were added to the schema after v3 was + // cut, but releases were still persisting v2 blobs, and the v2 -> v3 step seeded only the two + // Qwen Image fields. + const v2State = buildReleaseBlob('v6.10.0', { positivePrompt: 'a fluffy cat' }); + + const result = migrate?.(v2State) as ReturnType; + + expect(result._version).toBe(5); + expect(result.fluxDypePreset).toBe('off'); + expect(result.fluxDypeScale).toBe(2.0); + expect(result.fluxDypeExponent).toBe(2.0); + expect(result.zImageShift).toBeNull(); + expect(result.zImageSeedVarianceEnabled).toBe(false); + expect(result.zImageSeedVarianceStrength).toBe(0.1); + expect(result.zImageSeedVarianceRandomizePercent).toBe(50); + expect(result.animaVaeModel).toBeNull(); + expect(result.animaQwen3EncoderModel).toBeNull(); + expect(result.animaScheduler).toBe('euler'); + expect(result.kleinQwen3EncoderModel).toBeNull(); + expect(result.qwenImageComponentSource).toBeNull(); + expect(result.qwenImageQuantization).toBe('none'); + expect(result.qwenImageShift).toBeNull(); + expect(result.positivePrompt).toBe('a fluffy cat'); + }); + + it('preserves post-v3 values already present in a dev-build v2 blob', () => { + expect(migrate).toBeDefined(); + + // v2 blobs written by dev builds after each field landed already carry the keys, possibly with + // real values — the conditional seeds must not clobber them. + const v2State = buildReleaseBlob('v6.10.0', { + fluxDypePreset: 'auto', + qwenImageQuantization: 'int8', + qwenImageShift: 3.0, + zImageSeedVarianceEnabled: true, + }); + + const result = migrate?.(v2State) as ReturnType; + + expect(result.fluxDypePreset).toBe('auto'); + expect(result.qwenImageQuantization).toBe('int8'); + expect(result.qwenImageShift).toBe(3.0); + expect(result.zImageSeedVarianceEnabled).toBe(true); + }); + it('preserves Wan values already present in a dev-build v3 blob', () => { expect(migrate).toBeDefined(); diff --git a/invokeai/frontend/web/src/features/controlLayers/store/paramsSlice.ts b/invokeai/frontend/web/src/features/controlLayers/store/paramsSlice.ts index 7bb2e9781ca..04187cd886f 100644 --- a/invokeai/frontend/web/src/features/controlLayers/store/paramsSlice.ts +++ b/invokeai/frontend/web/src/features/controlLayers/store/paramsSlice.ts @@ -1,5 +1,6 @@ import type { PayloadAction, Selector } from '@reduxjs/toolkit'; import { createSelector, createSlice } from '@reduxjs/toolkit'; +import { logger } from 'app/logging/logger'; import type { RootState } from 'app/store/store'; import type { SliceConfig } from 'app/store/types'; import { deepClone } from 'common/util/deepClone'; @@ -63,6 +64,8 @@ import type { AnyModelConfigWithExternal } from 'services/api/types'; import { isExternalApiModelConfig, isNonRefinerMainModelConfig } from 'services/api/types'; import { assert } from 'tsafe'; +const log = logger('system'); + const slice = createSlice({ name: 'params', initialState: getInitialParamsState(), @@ -971,6 +974,166 @@ export const { setAnimaScheduler, } = slice.actions; +/** + * Last-resort repair for the persisted params slice, applied after the version steps have run. + * + * The `zParamsState.parse()` at the end of `migrate()` is all-or-nothing: a single missing required + * key throws, and the caller in `store.ts` catches it and falls back to the initial state — silently + * wiping every generation param the user had (prompts, prompt history, model selection, dimensions). + * That has happened whenever a key was added to the schema with neither a `.default()` nor a seed in + * the migration chain, which is what the Wan and post-v3 seeds above exist to undo. + * + * Backfilling those keys turns a forgotten seed into "one new field sits at its default" instead of + * "the user lost everything". Deliberately narrow: + * - Only keys that are *absent*. A key that is present but holds an invalid value still throws; + * this repairs omissions, not corruption. + * - Only keys the schema cannot fill itself. Anything with `.default()` / `.catch()` / `.optional()` + * is left to zod, so the schema's own default stays authoritative. + * + * This is a safety net, not a substitute for a migration step — it fills fields with *today's* + * initial value, which is only the right answer for genuinely new fields. `paramsSlice.test.ts` + * asserts the returned list is empty for real released blobs, so a forgotten seed still fails CI. + * + * Exported for that test. + */ +export const backfillMissingParamsKeys = (state: Record): string[] => { + const initial = getInitialParamsState() as unknown as Record; + const backfilled: string[] = []; + + for (const [key, fieldSchema] of Object.entries(zParamsState.shape)) { + // `undefined` is the only value that counts as missing: persisted JSON can't hold it, and every + // nullable field in the schema uses `null` for "unset", so this never overwrites a real value. + if (state[key] !== undefined || fieldSchema.safeParse(undefined).success) { + continue; + } + state[key] = initial[key]; + backfilled.push(key); + } + + return backfilled; +}; + +/** + * Bring a persisted params blob up to the current `_version` in place. + * + * Every key added to `zParamsState` without a zod default must be seeded by the step for the version + * that predates it, or upgrading users lose the whole slice — see `backfillMissingParamsKeys`. + * + * Exported so the tests can assert the version steps alone are complete, without the safety net + * hiding a missing seed. + */ +// eslint-disable-next-line @typescript-eslint/no-explicit-any +export const applyParamsVersionMigrations = (state: any): void => { + if (!('_version' in state)) { + // v0 -> v1, add _version and remove x/y from dimensions, lifting width/height to top level + state._version = 1; + state.dimensions.width = state.dimensions.rect.width; + state.dimensions.height = state.dimensions.rect.height; + } + + if (state._version === 1) { + // v1 -> v2, add positive prompt history + state._version = 2; + state.positivePromptHistory = []; + } + + if (state._version === 2) { + // v2 -> v3, add standalone Qwen Image VAE and Qwen VL encoder fields + state._version = 3; + state.qwenImageVaeModel = null; + state.qwenImageQwenVLEncoderModel = null; + + // Everything below was added to the schema after v3 was cut but without a version bump, so + // released builds that persist v2 blobs (v6.10.0 - v6.12.0) never wrote these keys. They + // have no zod default, which makes them required, so their absence fails the parse() below + // and silently wipes the whole slice on upgrade. Seed only when missing so that v2 blobs + // written by dev builds after each field landed keep the values they already hold. + state.fluxDypePreset = state.fluxDypePreset ?? 'off'; + state.fluxDypeScale = state.fluxDypeScale ?? 2.0; + state.fluxDypeExponent = state.fluxDypeExponent ?? 2.0; + state.zImageShift = state.zImageShift ?? null; + state.zImageSeedVarianceEnabled = state.zImageSeedVarianceEnabled ?? false; + state.zImageSeedVarianceStrength = state.zImageSeedVarianceStrength ?? 0.1; + state.zImageSeedVarianceRandomizePercent = state.zImageSeedVarianceRandomizePercent ?? 50; + state.animaVaeModel = state.animaVaeModel ?? null; + state.animaQwen3EncoderModel = state.animaQwen3EncoderModel ?? null; + state.animaScheduler = state.animaScheduler ?? 'euler'; + // No `kleinVaeModel` seed: the v4 -> v5 step below folds that slot into `flux2VaeModel` and + // deletes it, so it is no longer part of the schema and needs nothing here. + state.kleinQwen3EncoderModel = state.kleinQwen3EncoderModel ?? null; + state.qwenImageComponentSource = state.qwenImageComponentSource ?? null; + state.qwenImageQuantization = state.qwenImageQuantization ?? 'none'; + state.qwenImageShift = state.qwenImageShift ?? null; + } + + if (state._version === 3) { + // v3 -> v4, add Krea-2 standalone component and conditioning enhancer fields, and the + // PiD (Pixel Diffusion Decoder) fields. Also seed the Wan component fields — they were + // added to the schema without a version bump while releases were still writing v3 blobs, + // and they're nullable with no default, so a genuine released-build (v6.13.x) v3 blob + // without them fails zParamsState.parse() below, which wipes the whole slice on upgrade. + // Seed only when missing: dev-build v3 blobs written after the Wan merge already carry + // (possibly non-null) values. + state._version = 4; + state.krea2VaeModel = null; + state.krea2Qwen3VlEncoderModel = null; + state.krea2SeedVarianceEnabled = false; + state.krea2SeedVarianceStrength = 0.1; + state.krea2SeedVarianceRandomizePercent = 50; + state.krea2RebalanceEnabled = false; + state.krea2RebalanceMultiplier = 4; + state.krea2RebalanceWeights = '1.0,1.0,1.0,1.0,1.0,1.0,1.0,2.5,5.0,1.1,4.0,1.0'; + state.pidMode = 'off'; + state.pidDecoderModel = null; + state.gemma2EncoderModel = null; + state.pidSteps = 4; + state.wanTransformerLowNoise = state.wanTransformerLowNoise ?? null; + state.wanComponentSource = state.wanComponentSource ?? null; + state.wanVaeModel = state.wanVaeModel ?? null; + state.wanT5EncoderModel = state.wanT5EncoderModel ?? null; + state.wanGuidanceScaleLowNoise = state.wanGuidanceScaleLowNoise ?? null; + } + + if (state._version === 4) { + // v4 -> v5, merge the separate Klein / [dev] FLUX.2 VAE slots into one shared + // flux2VaeModel (both drew from the same FLUX.2 VAE pool — keep whichever was set) and + // seed the new standalone [dev] Mistral encoder slot. Both parents of the FLUX.2 [dev] + // merge shipped incompatible schemas under _version 4 (main added the PiD fields; the + // [dev] branch added the flux2 fields), so a v4 blob may be missing either side's keys — + // every seed here is conditional, and the PiD keys are re-seeded for blobs written by + // pre-merge [dev] builds. All are nullable-with-no-default, so any missing key would + // fail zParamsState.parse() and wipe the whole slice. + state._version = 5; + state.flux2VaeModel = state.flux2VaeModel ?? state.kleinVaeModel ?? state.flux2DevVaeModel ?? null; + state.flux2DevMistralEncoderModel = state.flux2DevMistralEncoderModel ?? null; + delete state.kleinVaeModel; + delete state.flux2DevVaeModel; + state.pidMode = state.pidMode ?? 'off'; + state.pidDecoderModel = state.pidDecoderModel ?? null; + state.gemma2EncoderModel = state.gemma2EncoderModel ?? null; + state.pidSteps = state.pidSteps ?? 4; + } + + // The HiDiffusion fields were added to the schema without a version bump, so they can be missing + // from a blob of any version — seeded outside the version steps for that reason. They have no zod + // default, so an absent key would fail the parse() at the end of migrate() and wipe the slice. + if (!('hiDiffusionEnabled' in state)) { + state.hiDiffusionEnabled = false; + } + if (!('hiDiffusionRauNetEnabled' in state)) { + state.hiDiffusionRauNetEnabled = true; + } + if (!('hiDiffusionWindowAttnEnabled' in state)) { + state.hiDiffusionWindowAttnEnabled = true; + } + if (!('hiDiffusionT1Ratio' in state)) { + state.hiDiffusionT1Ratio = 0.4; + } + if (!('hiDiffusionT2Ratio' in state)) { + state.hiDiffusionT2Ratio = 0.0; + } +}; + export const paramsSliceConfig: SliceConfig = { slice, schema: zParamsState, @@ -979,87 +1142,15 @@ export const paramsSliceConfig: SliceConfig = { migrate: (state) => { assert(isPlainObject(state)); - if (!('_version' in state)) { - // v0 -> v1, add _version and remove x/y from dimensions, lifting width/height to top level - state._version = 1; - state.dimensions.width = state.dimensions.rect.width; - state.dimensions.height = state.dimensions.rect.height; - } - - if (state._version === 1) { - // v1 -> v2, add positive prompt history - state._version = 2; - state.positivePromptHistory = []; - } - - if (state._version === 2) { - // v2 -> v3, add standalone Qwen Image VAE and Qwen VL encoder fields - state._version = 3; - state.qwenImageVaeModel = null; - state.qwenImageQwenVLEncoderModel = null; - } - - if (state._version === 3) { - // v3 -> v4, add Krea-2 standalone component and conditioning enhancer fields and the - // PiD (Pixel Diffusion Decoder) fields. Also seed the Wan component fields — they were - // added to the schema without a version bump while releases were still writing v3 blobs, - // and they're nullable with no default, so a genuine released-build v3 blob without them - // fails zParamsState.parse() and wipes the whole slice. Seed only when missing: dev-build - // v3 blobs written after the Wan merge already carry (possibly non-null) values. - state._version = 4; - state.krea2VaeModel = null; - state.krea2Qwen3VlEncoderModel = null; - state.krea2SeedVarianceEnabled = false; - state.krea2SeedVarianceStrength = 0.1; - state.krea2SeedVarianceRandomizePercent = 50; - state.krea2RebalanceEnabled = false; - state.krea2RebalanceMultiplier = 4; - state.krea2RebalanceWeights = '1.0,1.0,1.0,1.0,1.0,1.0,1.0,2.5,5.0,1.1,4.0,1.0'; - state.pidMode = 'off'; - state.pidDecoderModel = null; - state.gemma2EncoderModel = null; - state.pidSteps = 4; - state.wanTransformerLowNoise = state.wanTransformerLowNoise ?? null; - state.wanComponentSource = state.wanComponentSource ?? null; - state.wanVaeModel = state.wanVaeModel ?? null; - state.wanT5EncoderModel = state.wanT5EncoderModel ?? null; - state.wanGuidanceScaleLowNoise = state.wanGuidanceScaleLowNoise ?? null; - } - - if (state._version === 4) { - // v4 -> v5, merge the separate Klein / [dev] FLUX.2 VAE slots into one shared - // flux2VaeModel (both drew from the same FLUX.2 VAE pool — keep whichever was set) and - // seed the new standalone [dev] Mistral encoder slot. Both parents of the FLUX.2 [dev] - // merge shipped incompatible schemas under _version 4 (main added the PiD fields; the - // [dev] branch added the flux2 fields), so a v4 blob may be missing either side's keys — - // every seed here is conditional, and the PiD keys are re-seeded for blobs written by - // pre-merge [dev] builds. All are nullable-with-no-default, so any missing key would - // fail zParamsState.parse() and wipe the whole slice. - state._version = 5; - state.flux2VaeModel = state.flux2VaeModel ?? state.kleinVaeModel ?? state.flux2DevVaeModel ?? null; - state.flux2DevMistralEncoderModel = state.flux2DevMistralEncoderModel ?? null; - delete state.kleinVaeModel; - delete state.flux2DevVaeModel; - state.pidMode = state.pidMode ?? 'off'; - state.pidDecoderModel = state.pidDecoderModel ?? null; - state.gemma2EncoderModel = state.gemma2EncoderModel ?? null; - state.pidSteps = state.pidSteps ?? 4; - } + applyParamsVersionMigrations(state); - if (!('hiDiffusionEnabled' in state)) { - state.hiDiffusionEnabled = false; - } - if (!('hiDiffusionRauNetEnabled' in state)) { - state.hiDiffusionRauNetEnabled = true; - } - if (!('hiDiffusionWindowAttnEnabled' in state)) { - state.hiDiffusionWindowAttnEnabled = true; - } - if (!('hiDiffusionT1Ratio' in state)) { - state.hiDiffusionT1Ratio = 0.4; - } - if (!('hiDiffusionT2Ratio' in state)) { - state.hiDiffusionT2Ratio = 0.0; + const backfilled = backfillMissingParamsKeys(state); + if (backfilled.length > 0) { + log.warn( + { backfilled }, + `Backfilled ${backfilled.length} params key(s) missing from the persisted state: ${backfilled.join(', ')}. ` + + `These need a zod default or a seed in the migration chain.` + ); } return zParamsState.parse(state); From 4c9f7a923ccfc650fdb70233102492c786a6cd52 Mon Sep 17 00:00:00 2001 From: Lincoln Stein Date: Sun, 2 Aug 2026 14:18:46 -0400 Subject: [PATCH 3/6] fix(ui): cover the oldest v2 releases and harden the migration edges MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Follow-up from an adversarial review of the previous commit. The v2 fixture was not the narrowest v2 release. The earlier survey globbed tags as v6.1*, which silently excluded v6.7.0 - v6.9.0 — four stable releases that also persist _version 2, with only 46 keys against v6.10.0's 52. Six further keys are required-with-no-default and seeded nowhere: fluxScheduler, zImageScheduler, colorCompensation, zImageVaeModel, zImageQwen3EncoderModel and zImageQwen3SourceModel. Verified by running the version steps over a v6.7.0-shaped blob: parse() throws on exactly those six. Seed them in the v2 -> v3 step and replace the fixture with the true narrowest set (v6.7.0, confirmed a strict subset of v6.10.0/v6.11.x/v6.12.0). Add a v6.6.0 (_version 1) fixture too; it is the v6.7.0 set minus positivePromptHistory, which the v1 step already seeds. Also close three edges the safety net did not cover: - The v0 step dereferenced state.dimensions.rect unguarded, so a blob lacking dimensions threw a TypeError straight out of migrate() — the one remaining path that could still wipe the slice. Guard it and let the backfill repair dimensions instead. - The v0 branch tested key presence (!('_version' in state)) while the backfill tests value (!== undefined). A blob with an explicit undefined _version matched no branch, reached the parse and took the slice down. Detect v0 by value so the two agree. - Exclude _version from the backfill loop, so a future change cannot turn it into a version-detection bypass that stamps a blob current without running a single step. Each fix is mutation-checked: reverting any one of them fails at least one test, and the six seeds fail the schema-completeness test. Not covered: v6.2.0a1 - v6.5.1 persist a blob with no _version at all and predate the current dimensions shape, so a faithful fixture can't be built by filtering getInitialParamsState(). Noted in the test file. Co-Authored-By: Claude Opus 5 (1M context) --- .../controlLayers/store/paramsSlice.test.ts | 219 ++++++++++++++++-- .../controlLayers/store/paramsSlice.ts | 41 +++- 2 files changed, 227 insertions(+), 33 deletions(-) diff --git a/invokeai/frontend/web/src/features/controlLayers/store/paramsSlice.test.ts b/invokeai/frontend/web/src/features/controlLayers/store/paramsSlice.test.ts index bf4da97aab6..adf8b3bfec8 100644 --- a/invokeai/frontend/web/src/features/controlLayers/store/paramsSlice.test.ts +++ b/invokeai/frontend/web/src/features/controlLayers/store/paramsSlice.test.ts @@ -154,12 +154,121 @@ describe('paramsSlice selectors for external models', () => { * * One entry per persisted `_version` still in the wild, each picked as the *narrowest* key set among * the releases writing that version, so it is a subset of every real blob of that version: - * - v2: v6.10.0 - v6.12.0. v6.10.0 is the narrowest (later v2 releases only added keys). + * - v1: v6.6.0 only. Identical to the v6.7.0 set below minus `positivePromptHistory`, which the + * v1 -> v2 step seeds. + * - v2: v6.7.0 - v6.12.0. v6.7.0 is the narrowest and a strict subset of the rest; note v6.7.0 - + * v6.9.0 (46 keys) are considerably narrower than v6.10.0 (52) and v6.11.0 - v6.12.0 (60), so + * testing only the newest v2 release would miss six keys. * - v3: v6.13.0 - v6.13.7. v6.13.7 is the narrowest (v6.13.0 additionally had `animaT5EncoderModel`, * since removed from the schema; unknown keys are stripped by the non-strict object parse). * v4 blobs are written by v6.14.0-rc1 onward; the v4 -> v5 step is covered by its own tests below. + * + * Not covered here: v6.2.0a1 - v6.5.1 persist a blob with no `_version` at all (the v0 path). Those + * also predate the current `dimensions` shape, so a faithful fixture cannot be built by filtering + * `getInitialParamsState()` the way `buildReleaseBlob` does. */ const RELEASE_PARAMS_KEYS = { + 'v6.6.0': { + version: 1, + keys: [ + '_version', + 'canvasCoherenceEdgeSize', + 'canvasCoherenceMinDenoise', + 'canvasCoherenceMode', + 'cfgRescaleMultiplier', + 'cfgScale', + 'clipEmbedModel', + 'clipGEmbedModel', + 'clipLEmbedModel', + 'clipSkip', + 'controlLora', + 'dimensions', + 'fluxVAE', + 'guidance', + 'img2imgStrength', + 'infillColorValue', + 'infillMethod', + 'infillPatchmatchDownscaleSize', + 'infillTileSize', + 'iterations', + 'maskBlur', + 'maskBlurMethod', + 'model', + 'negativePrompt', + 'optimizedDenoisingEnabled', + 'positivePrompt', + 'refinerCFGScale', + 'refinerModel', + 'refinerNegativeAestheticScore', + 'refinerPositiveAestheticScore', + 'refinerScheduler', + 'refinerStart', + 'refinerSteps', + 'scheduler', + 'seamlessXAxis', + 'seamlessYAxis', + 'seed', + 'shouldRandomizeSeed', + 'shouldUseCpuNoise', + 'steps', + 't5EncoderModel', + 'upscaleCfgScale', + 'upscaleScheduler', + 'vae', + 'vaePrecision', + ], + }, + 'v6.7.0': { + version: 2, + keys: [ + '_version', + 'canvasCoherenceEdgeSize', + 'canvasCoherenceMinDenoise', + 'canvasCoherenceMode', + 'cfgRescaleMultiplier', + 'cfgScale', + 'clipEmbedModel', + 'clipGEmbedModel', + 'clipLEmbedModel', + 'clipSkip', + 'controlLora', + 'dimensions', + 'fluxVAE', + 'guidance', + 'img2imgStrength', + 'infillColorValue', + 'infillMethod', + 'infillPatchmatchDownscaleSize', + 'infillTileSize', + 'iterations', + 'maskBlur', + 'maskBlurMethod', + 'model', + 'negativePrompt', + 'optimizedDenoisingEnabled', + 'positivePrompt', + 'positivePromptHistory', + 'refinerCFGScale', + 'refinerModel', + 'refinerNegativeAestheticScore', + 'refinerPositiveAestheticScore', + 'refinerScheduler', + 'refinerStart', + 'refinerSteps', + 'scheduler', + 'seamlessXAxis', + 'seamlessYAxis', + 'seed', + 'shouldRandomizeSeed', + 'shouldUseCpuNoise', + 'steps', + 't5EncoderModel', + 'upscaleCfgScale', + 'upscaleScheduler', + 'vae', + 'vaePrecision', + ], + }, 'v6.10.0': { version: 2, keys: [ @@ -513,26 +622,29 @@ describe('paramsSliceConfig persisted state migration', () => { expect(result.seed).toBe(99); }); - it.each(['v6.10.0', 'v6.13.7'] as const)('migrates a genuine %s blob without losing the user params', (release) => { - expect(migrate).toBeDefined(); + it.each(['v6.6.0', 'v6.7.0', 'v6.10.0', 'v6.13.7'] as const)( + 'migrates a genuine %s blob without losing the user params', + (release) => { + expect(migrate).toBeDefined(); - // A real released-build blob only has the keys that release's schema declared. Any key added - // since that has neither a zod default nor a migration seed fails the parse() at the end of - // migrate(), and the caller in store.ts swallows the throw and falls back to the initial - // state — silently wiping the user's prompts, model selection and dimensions on upgrade. - const blob = buildReleaseBlob(release, { - positivePrompt: 'a fluffy cat', - seed: 42, - shouldRandomizeSeed: false, - }); + // A real released-build blob only has the keys that release's schema declared. Any key added + // since that has neither a zod default nor a migration seed fails the parse() at the end of + // migrate(), and the caller in store.ts swallows the throw and falls back to the initial + // state — silently wiping the user's prompts, model selection and dimensions on upgrade. + const blob = buildReleaseBlob(release, { + positivePrompt: 'a fluffy cat', + seed: 42, + shouldRandomizeSeed: false, + }); - const result = migrate?.(blob) as ReturnType; + const result = migrate?.(blob) as ReturnType; - expect(result._version).toBe(5); - expect(result.positivePrompt).toBe('a fluffy cat'); - expect(result.seed).toBe(42); - expect(result.shouldRandomizeSeed).toBe(false); - }); + expect(result._version).toBe(5); + expect(result.positivePrompt).toBe('a fluffy cat'); + expect(result.seed).toBe(42); + expect(result.shouldRandomizeSeed).toBe(false); + } + ); it('seeds every key of the current schema in the version steps themselves, for each released blob version', () => { // The general form of the defect this suite guards against: a key is added to zParamsState with @@ -591,6 +703,54 @@ describe('paramsSliceConfig persisted state migration', () => { expect(blob.positivePrompt).toBe('a fluffy cat'); }); + it('leaves a defaulted key to zod rather than backfilling it from the initial state', () => { + // The net must not pre-empt a field the schema can fill itself, or the schema's `.default()` + // stops being authoritative the moment it diverges from getInitialParamsState(). `pidSteps` + // (required) must be filled; `ernieImageScheduler` (`.default('euler')`) must not be. + const blob = buildReleaseBlob('v6.13.7'); + applyParamsVersionMigrations(blob); + delete blob.ernieImageScheduler; + delete blob.pidSteps; + + expect(backfillMissingParamsKeys(blob)).toEqual(['pidSteps']); + expect(blob.ernieImageScheduler).toBeUndefined(); + expect(zParamsState.parse(blob).ernieImageScheduler).toBe('euler'); + }); + + it('never backfills _version, so version detection cannot be bypassed', () => { + expect(migrate).toBeDefined(); + + // The v0 branch keys off `!('_version' in state)` (presence) while the net keys off `undefined` + // (value). If the net filled `_version`, a blob carrying an explicit undefined would be stamped + // as current having run no migration step at all. + const blob = buildReleaseBlob('v6.7.0', { _version: undefined, positivePrompt: 'a fluffy cat' }); + + const result = migrate?.(blob) as ReturnType; + + // It is treated as a v0 blob and walked through the whole chain, not stamped v5 in place. + expect(result._version).toBe(5); + expect(result.positivePromptHistory).toEqual([]); + expect(result.qwenImageVaeModel).toBeNull(); + expect(result.wanVaeModel).toBeNull(); + expect(result.positivePrompt).toBe('a fluffy cat'); + }); + + it('does not throw on a v0 blob whose dimensions are missing', () => { + expect(migrate).toBeDefined(); + + // A truncated or hand-edited pre-_version blob. The v0 step used to dereference + // state.dimensions.rect unguarded, and the TypeError escaped migrate() — the one path that + // could still cost the user the whole slice despite the safety net. + const blob: Record = { positivePrompt: 'a fluffy cat', seed: 7 }; + + const result = migrate?.(blob) as ReturnType; + + expect(result._version).toBe(5); + expect(result.positivePrompt).toBe('a fluffy cat'); + expect(result.seed).toBe(7); + expect(result.dimensions).toBeDefined(); + }); + it('seeds the Wan fields for a released-build v3 blob that predates the Wan merge', () => { expect(migrate).toBeDefined(); @@ -613,17 +773,24 @@ describe('paramsSliceConfig persisted state migration', () => { expect(result.seed).toBe(42); }); - it('seeds the post-v3 fields for a released-build v2 blob (v6.10.0 - v6.12.0)', () => { + it('seeds the post-v3 fields for the oldest released v2 blob (v6.7.0 - v6.9.0)', () => { expect(migrate).toBeDefined(); - // Same class of defect one version earlier: these keys were added to the schema after v3 was - // cut, but releases were still persisting v2 blobs, and the v2 -> v3 step seeded only the two - // Qwen Image fields. - const v2State = buildReleaseBlob('v6.10.0', { positivePrompt: 'a fluffy cat' }); + // Same class of defect one version earlier: these keys were added to the schema while releases + // were still persisting v2 blobs, and the v2 -> v3 step seeded only the two Qwen Image fields. + // v6.7.0 - v6.9.0 are the narrowest v2 blobs, missing the first six below in addition to + // everything v6.10.0 is missing. + const v2State = buildReleaseBlob('v6.7.0', { positivePrompt: 'a fluffy cat' }); const result = migrate?.(v2State) as ReturnType; expect(result._version).toBe(5); + expect(result.fluxScheduler).toBe('euler'); + expect(result.zImageScheduler).toBe('euler'); + expect(result.colorCompensation).toBe(false); + expect(result.zImageVaeModel).toBeNull(); + expect(result.zImageQwen3EncoderModel).toBeNull(); + expect(result.zImageQwen3SourceModel).toBeNull(); expect(result.fluxDypePreset).toBe('off'); expect(result.fluxDypeScale).toBe(2.0); expect(result.fluxDypeExponent).toBe(2.0); @@ -646,7 +813,9 @@ describe('paramsSliceConfig persisted state migration', () => { // v2 blobs written by dev builds after each field landed already carry the keys, possibly with // real values — the conditional seeds must not clobber them. - const v2State = buildReleaseBlob('v6.10.0', { + const v2State = buildReleaseBlob('v6.7.0', { + fluxScheduler: 'heun', + colorCompensation: true, fluxDypePreset: 'auto', qwenImageQuantization: 'int8', qwenImageShift: 3.0, @@ -655,6 +824,8 @@ describe('paramsSliceConfig persisted state migration', () => { const result = migrate?.(v2State) as ReturnType; + expect(result.fluxScheduler).toBe('heun'); + expect(result.colorCompensation).toBe(true); expect(result.fluxDypePreset).toBe('auto'); expect(result.qwenImageQuantization).toBe('int8'); expect(result.qwenImageShift).toBe(3.0); diff --git a/invokeai/frontend/web/src/features/controlLayers/store/paramsSlice.ts b/invokeai/frontend/web/src/features/controlLayers/store/paramsSlice.ts index 04187cd886f..9e39c8cd8dd 100644 --- a/invokeai/frontend/web/src/features/controlLayers/store/paramsSlice.ts +++ b/invokeai/frontend/web/src/features/controlLayers/store/paramsSlice.ts @@ -1001,6 +1001,12 @@ export const backfillMissingParamsKeys = (state: Record): strin const backfilled: string[] = []; for (const [key, fieldSchema] of Object.entries(zParamsState.shape)) { + // Never touch `_version`. The version steps detect a v0 blob with `!('_version' in state)`, i.e. + // key presence, so filling it here on a value check would stamp a blob as current having run no + // migration step at all. + if (key === '_version') { + continue; + } // `undefined` is the only value that counts as missing: persisted JSON can't hold it, and every // nullable field in the schema uses `null` for "unset", so this never overwrites a real value. if (state[key] !== undefined || fieldSchema.safeParse(undefined).success) { @@ -1024,11 +1030,19 @@ export const backfillMissingParamsKeys = (state: Record): strin */ // eslint-disable-next-line @typescript-eslint/no-explicit-any export const applyParamsVersionMigrations = (state: any): void => { - if (!('_version' in state)) { - // v0 -> v1, add _version and remove x/y from dimensions, lifting width/height to top level + // Value check rather than `!('_version' in state)`, so that a blob carrying an explicit undefined + // is treated as v0 and walked through the chain. With a presence check it matches no branch at + // all, reaches the parse with `_version: undefined` and takes the whole slice down with it. + if (state._version === undefined) { + // v0 -> v1, add _version and remove x/y from dimensions, lifting width/height to top level. + // `dimensions.rect` is optional-chained: a truncated or hand-edited blob that lacks it would + // otherwise throw a TypeError out of migrate() and cost the user the whole slice. Leaving + // width/height undefined instead lets backfillMissingParamsKeys() repair `dimensions`. state._version = 1; - state.dimensions.width = state.dimensions.rect.width; - state.dimensions.height = state.dimensions.rect.height; + if (state.dimensions && state.dimensions.rect) { + state.dimensions.width = state.dimensions.rect.width; + state.dimensions.height = state.dimensions.rect.height; + } } if (state._version === 1) { @@ -1043,11 +1057,20 @@ export const applyParamsVersionMigrations = (state: any): void => { state.qwenImageVaeModel = null; state.qwenImageQwenVLEncoderModel = null; - // Everything below was added to the schema after v3 was cut but without a version bump, so - // released builds that persist v2 blobs (v6.10.0 - v6.12.0) never wrote these keys. They - // have no zod default, which makes them required, so their absence fails the parse() below - // and silently wipes the whole slice on upgrade. Seed only when missing so that v2 blobs - // written by dev builds after each field landed keep the values they already hold. + // Everything below was added to the schema while releases were still persisting v2 blobs + // (v6.7.0 - v6.12.0), but without a version bump. None has a zod default, which makes them + // required, so their absence fails the parse() at the end of migrate() and silently wipes the + // whole slice on upgrade. Seed only when missing so that v2 blobs written by dev builds after + // each field landed keep the values they already hold. + // + // The oldest v2 releases (v6.7.0 - v6.9.0) are missing these six as well as everything below. + state.fluxScheduler = state.fluxScheduler ?? 'euler'; + state.zImageScheduler = state.zImageScheduler ?? 'euler'; + state.colorCompensation = state.colorCompensation ?? false; + state.zImageVaeModel = state.zImageVaeModel ?? null; + state.zImageQwen3EncoderModel = state.zImageQwen3EncoderModel ?? null; + state.zImageQwen3SourceModel = state.zImageQwen3SourceModel ?? null; + // Added by v6.10.0 - v6.12.0 and later. state.fluxDypePreset = state.fluxDypePreset ?? 'off'; state.fluxDypeScale = state.fluxDypeScale ?? 2.0; state.fluxDypeExponent = state.fluxDypeExponent ?? 2.0; From ece340988c2331004409654a71600780909adeba Mon Sep 17 00:00:00 2001 From: Lincoln Stein Date: Sun, 2 Aug 2026 19:01:42 -0400 Subject: [PATCH 4/6] fix(ui): close the v4 and v0 gaps in the params migration invariant MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses the four follow-ups from Pfannkuchensack's second review. The v4 tier was unguarded: the PiD fields landed a day after the _version 3 -> 4 bump, so dev builds from that window persist v4 blobs without them, and a v4 blob matches no branch in the migration chain. Give the four fields zod defaults, matching the ernieImage* precedent set by the other two post-bump additions, and pin a RELEASE_PARAMS_KEYS entry to the bump commit's key set so the invariant covers the tier no version step can reach. Add v0 fixtures. The claimed v0 range was wrong: it spans v6.0.0a1 - v6.6.0rc2, and the oldest builds have no `dimensions` key at all, which no step seeded — the invariant only held there because the safety net caught it. Seed `dimensions` in the v0 step and cover both v0 shapes. Widen the safety net from omissions to any key whose persisted value fails its own field schema, so a `dimensions` the v0 guard left incomplete, or a `model` whose base has since left zBaseModelType, costs that one field instead of the whole slice. This makes true what the guard's comment already claimed. Fix the comments that still described the presence check this branch replaced with a value check. Also close three holes in the tests themselves, all found by mutating the production code and watching nothing fail: the _version guard test never reached the guard (the version steps normalise _version first), the PiD test could not distinguish the new defaults from the safety net backfilling the same values, and the fixtures carried initial values throughout — including `model: null`, so nothing noticed a model being silently cleared. Co-Authored-By: Claude Opus 5 --- .../controlLayers/store/paramsSlice.test.ts | 569 ++++++++++++++++-- .../controlLayers/store/paramsSlice.ts | 84 ++- .../src/features/controlLayers/store/types.ts | 14 +- 3 files changed, 592 insertions(+), 75 deletions(-) diff --git a/invokeai/frontend/web/src/features/controlLayers/store/paramsSlice.test.ts b/invokeai/frontend/web/src/features/controlLayers/store/paramsSlice.test.ts index adf8b3bfec8..389e58d42b0 100644 --- a/invokeai/frontend/web/src/features/controlLayers/store/paramsSlice.test.ts +++ b/invokeai/frontend/web/src/features/controlLayers/store/paramsSlice.test.ts @@ -9,7 +9,6 @@ import { describe, expect, it } from 'vitest'; import { applyParamsVersionMigrations, - backfillMissingParamsKeys, isValidKrea2RebalanceWeights, KREA2_REBALANCE_WEIGHT_COUNT, modelChanged, @@ -17,6 +16,7 @@ import { parseKrea2RebalanceWeights, positivePromptAddedToHistory, promptRemovedFromHistory, + repairParamsState, selectModelSupportsDimensions, selectModelSupportsGuidance, selectModelSupportsNegativePrompt, @@ -152,22 +152,133 @@ describe('paramsSlice selectors for external models', () => { * newly added key that the migration chain forgets to seed. That is precisely what masked the Wan * regression these tests exist to catch. * - * One entry per persisted `_version` still in the wild, each picked as the *narrowest* key set among - * the releases writing that version, so it is a subset of every real blob of that version: - * - v1: v6.6.0 only. Identical to the v6.7.0 set below minus `positivePromptHistory`, which the - * v1 -> v2 step seeds. - * - v2: v6.7.0 - v6.12.0. v6.7.0 is the narrowest and a strict subset of the rest; note v6.7.0 - - * v6.9.0 (46 keys) are considerably narrower than v6.10.0 (52) and v6.11.0 - v6.12.0 (60), so - * testing only the newest v2 release would miss six keys. - * - v3: v6.13.0 - v6.13.7. v6.13.7 is the narrowest (v6.13.0 additionally had `animaT5EncoderModel`, - * since removed from the schema; unknown keys are stripped by the non-strict object parse). - * v4 blobs are written by v6.14.0-rc1 onward; the v4 -> v5 step is covered by its own tests below. - * - * Not covered here: v6.2.0a1 - v6.5.1 persist a blob with no `_version` at all (the v0 path). Those - * also predate the current `dimensions` shape, so a faithful fixture cannot be built by filtering - * `getInitialParamsState()` the way `buildReleaseBlob` does. + * What matters per version is the *intersection* of every build that wrote it, not the newest or even + * the smallest single one: if any build omitted a key, the chain has to cope with that key's absence. + * The ranges below come from reading `_version` and the key set out of all 66 `v6.*` tags. Do not + * survey them with a glob like `v6.1*` — that matches v6.10 - v6.14 and silently drops v6.7 - v6.9. + * - v0: v6.0.0a1 - v6.6.0rc2, which persist a blob with no `_version` at all. Two entries, because + * no single v0 build has the intersecting key set (43 keys): + * - `v6.0.0a1` (46 keys) is the intersection once the three keys since removed from the schema + * are dropped — `positivePrompt2` / `negativePrompt2` / `shouldConcatPrompts`, which + * `buildReleaseBlob` filters out and the non-strict parse would strip anyway. Critically it + * has no `dimensions` key at all; that arrives in v6.0.0rc4. + * - `v6.5.1` (44 keys) is the widest-coverage v0 shape that *does* carry `dimensions`, in the + * pre-flattening form, which is the only way to exercise the v0 -> v1 lift. + * - v1: v6.6.0 and v6.7.0rc1, whose key sets are identical (45). + * - v2: v6.7.0 - v6.13.0.rc1. v6.7.0 is the intersection (46) and a strict subset of the rest; note + * v6.7.0 - v6.9.0 (46 keys) are considerably narrower than v6.10.0 (52), v6.11.0 - v6.12.0 (60) + * and v6.13.0.rc1 (73), so testing only the newest v2 build would miss six keys. + * - v3: v6.13.0 - v6.13.7. v6.13.7 is the intersection (77); v6.13.0 additionally had + * `animaT5EncoderModel`, since removed from the schema. + * - v4: the narrowest v4 blob is not a release at all — it is the one written by the build that did + * the bump, `1aeb05bbf0` (97 keys). Releases writing v4 start at v6.14.0-rc1. + * - v5: the current version, reached by the FLUX.2 [dev] merge `f10d2a4f5a`, which is also the + * build that wrote the narrowest v5 blob. Pinning the fixture at the bump commit is what keeps + * the invariant below meaningful for the current tier: the version steps can never cover it (a + * v5 blob matches no branch), so every key added since the bump has to carry a zod default, and + * this entry is what proves it does. */ const RELEASE_PARAMS_KEYS = { + 'v6.0.0a1': { + version: 0, + keys: [ + 'canvasCoherenceEdgeSize', + 'canvasCoherenceMinDenoise', + 'canvasCoherenceMode', + 'cfgRescaleMultiplier', + 'cfgScale', + 'clipEmbedModel', + 'clipGEmbedModel', + 'clipLEmbedModel', + 'clipSkip', + 'controlLora', + 'fluxVAE', + 'guidance', + 'img2imgStrength', + 'infillColorValue', + 'infillMethod', + 'infillPatchmatchDownscaleSize', + 'infillTileSize', + 'iterations', + 'maskBlur', + 'maskBlurMethod', + 'model', + 'negativePrompt', + 'negativePrompt2', + 'optimizedDenoisingEnabled', + 'positivePrompt', + 'positivePrompt2', + 'refinerCFGScale', + 'refinerModel', + 'refinerNegativeAestheticScore', + 'refinerPositiveAestheticScore', + 'refinerScheduler', + 'refinerStart', + 'refinerSteps', + 'scheduler', + 'seamlessXAxis', + 'seamlessYAxis', + 'seed', + 'shouldConcatPrompts', + 'shouldRandomizeSeed', + 'shouldUseCpuNoise', + 'steps', + 't5EncoderModel', + 'upscaleCfgScale', + 'upscaleScheduler', + 'vae', + 'vaePrecision', + ], + }, + 'v6.5.1': { + version: 0, + keys: [ + 'canvasCoherenceEdgeSize', + 'canvasCoherenceMinDenoise', + 'canvasCoherenceMode', + 'cfgRescaleMultiplier', + 'cfgScale', + 'clipEmbedModel', + 'clipGEmbedModel', + 'clipLEmbedModel', + 'clipSkip', + 'controlLora', + 'dimensions', + 'fluxVAE', + 'guidance', + 'img2imgStrength', + 'infillColorValue', + 'infillMethod', + 'infillPatchmatchDownscaleSize', + 'infillTileSize', + 'iterations', + 'maskBlur', + 'maskBlurMethod', + 'model', + 'negativePrompt', + 'optimizedDenoisingEnabled', + 'positivePrompt', + 'refinerCFGScale', + 'refinerModel', + 'refinerNegativeAestheticScore', + 'refinerPositiveAestheticScore', + 'refinerScheduler', + 'refinerStart', + 'refinerSteps', + 'scheduler', + 'seamlessXAxis', + 'seamlessYAxis', + 'seed', + 'shouldRandomizeSeed', + 'shouldUseCpuNoise', + 'steps', + 't5EncoderModel', + 'upscaleCfgScale', + 'upscaleScheduler', + 'vae', + 'vaePrecision', + ], + }, 'v6.6.0': { version: 1, keys: [ @@ -408,11 +519,227 @@ const RELEASE_PARAMS_KEYS = { 'zImageVaeModel', ], }, + '1aeb05bbf0': { + version: 4, + keys: [ + '_version', + 'animaLLLiteModel', + 'animaLLLiteWeight', + 'animaQwen3EncoderModel', + 'animaScheduler', + 'animaVaeModel', + 'canvasCoherenceEdgeSize', + 'canvasCoherenceMinDenoise', + 'canvasCoherenceMode', + 'cfgRescaleMultiplier', + 'cfgScale', + 'clipEmbedModel', + 'clipGEmbedModel', + 'clipLEmbedModel', + 'clipSkip', + 'colorCompensation', + 'controlLora', + 'dimensions', + 'fluxDypeExponent', + 'fluxDypePreset', + 'fluxDypeScale', + 'fluxScheduler', + 'fluxVAE', + 'geminiTemperature', + 'geminiThinkingLevel', + 'guidance', + 'ideogram4ColorPalette', + 'ideogram4GuidanceScale', + 'ideogram4Mu', + 'ideogram4SamplerPreset', + 'ideogram4Steps', + 'imageSize', + 'img2imgStrength', + 'infillColorValue', + 'infillMethod', + 'infillPatchmatchDownscaleSize', + 'infillTileSize', + 'iterations', + 'kleinQwen3EncoderModel', + 'kleinVaeModel', + 'krea2Qwen3VlEncoderModel', + 'krea2RebalanceEnabled', + 'krea2RebalanceMultiplier', + 'krea2RebalanceWeights', + 'krea2SeedVarianceEnabled', + 'krea2SeedVarianceRandomizePercent', + 'krea2SeedVarianceStrength', + 'krea2VaeModel', + 'maskBlur', + 'maskBlurMethod', + 'model', + 'negativePrompt', + 'openaiBackground', + 'openaiInputFidelity', + 'openaiQuality', + 'optimizedDenoisingEnabled', + 'positivePrompt', + 'positivePromptHistory', + 'qwenImageComponentSource', + 'qwenImageQuantization', + 'qwenImageQwenVLEncoderModel', + 'qwenImageShift', + 'qwenImageVaeModel', + 'refinerCFGScale', + 'refinerModel', + 'refinerNegativeAestheticScore', + 'refinerPositiveAestheticScore', + 'refinerScheduler', + 'refinerStart', + 'refinerSteps', + 'scheduler', + 'seamlessXAxis', + 'seamlessYAxis', + 'seed', + 'seedreamOptimizePrompt', + 'seedreamWatermark', + 'shouldRandomizeSeed', + 'shouldUseCpuNoise', + 'steps', + 't5EncoderModel', + 'upscaleCfgScale', + 'upscaleScheduler', + 'vae', + 'vaePrecision', + 'wanComponentSource', + 'wanGuidanceScaleLowNoise', + 'wanT5EncoderModel', + 'wanTransformerLowNoise', + 'wanVaeModel', + 'zImageQwen3EncoderModel', + 'zImageQwen3SourceModel', + 'zImageScheduler', + 'zImageSeedVarianceEnabled', + 'zImageSeedVarianceRandomizePercent', + 'zImageSeedVarianceStrength', + 'zImageShift', + 'zImageVaeModel', + ], + }, + 'f10d2a4f5a': { + version: 5, + keys: [ + 'animaLLLiteModel', + 'animaLLLiteWeight', + 'animaQwen3EncoderModel', + 'animaScheduler', + 'animaVaeModel', + 'canvasCoherenceEdgeSize', + 'canvasCoherenceMinDenoise', + 'canvasCoherenceMode', + 'cfgRescaleMultiplier', + 'cfgScale', + 'clipEmbedModel', + 'clipGEmbedModel', + 'clipLEmbedModel', + 'clipSkip', + 'colorCompensation', + 'controlLora', + 'dimensions', + 'ernieImageScheduler', + 'ernieImageUsePromptEnhancer', + 'flux2DevMistralEncoderModel', + 'flux2VaeModel', + 'fluxDypeExponent', + 'fluxDypePreset', + 'fluxDypeScale', + 'fluxScheduler', + 'fluxVAE', + 'geminiTemperature', + 'geminiThinkingLevel', + 'gemma2EncoderModel', + 'guidance', + 'hiDiffusionEnabled', + 'hiDiffusionRauNetEnabled', + 'hiDiffusionT1Ratio', + 'hiDiffusionT2Ratio', + 'hiDiffusionWindowAttnEnabled', + 'ideogram4ColorPalette', + 'ideogram4GuidanceScale', + 'ideogram4Mu', + 'ideogram4SamplerPreset', + 'ideogram4Steps', + 'imageSize', + 'img2imgStrength', + 'infillColorValue', + 'infillMethod', + 'infillPatchmatchDownscaleSize', + 'infillTileSize', + 'iterations', + 'kleinQwen3EncoderModel', + 'krea2Qwen3VlEncoderModel', + 'krea2RebalanceEnabled', + 'krea2RebalanceMultiplier', + 'krea2RebalanceWeights', + 'krea2SeedVarianceEnabled', + 'krea2SeedVarianceRandomizePercent', + 'krea2SeedVarianceStrength', + 'krea2VaeModel', + 'maskBlur', + 'maskBlurMethod', + 'model', + 'negativePrompt', + 'openaiBackground', + 'openaiInputFidelity', + 'openaiQuality', + 'optimizedDenoisingEnabled', + 'pidDecoderModel', + 'pidMode', + 'pidSteps', + 'positivePrompt', + 'positivePromptHistory', + 'qwenImageComponentSource', + 'qwenImageQuantization', + 'qwenImageQwenVLEncoderModel', + 'qwenImageShift', + 'qwenImageVaeModel', + 'refinerCFGScale', + 'refinerModel', + 'refinerNegativeAestheticScore', + 'refinerPositiveAestheticScore', + 'refinerScheduler', + 'refinerStart', + 'refinerSteps', + 'scheduler', + 'seamlessXAxis', + 'seamlessYAxis', + 'seed', + 'seedreamOptimizePrompt', + 'seedreamWatermark', + 'shouldRandomizeSeed', + 'shouldUseCpuNoise', + 'steps', + 't5EncoderModel', + 'upscaleCfgScale', + 'upscaleScheduler', + 'vae', + 'vaePrecision', + '_version', + 'wanComponentSource', + 'wanGuidanceScaleLowNoise', + 'wanT5EncoderModel', + 'wanTransformerLowNoise', + 'wanVaeModel', + 'zImageQwen3EncoderModel', + 'zImageQwen3SourceModel', + 'zImageScheduler', + 'zImageSeedVarianceEnabled', + 'zImageSeedVarianceRandomizePercent', + 'zImageSeedVarianceStrength', + 'zImageShift', + 'zImageVaeModel', + ], + }, } as const satisfies Record; /** - * Build a blob shaped exactly like the one the given release persisted: current initial values, but - * restricted to the keys that release's schema actually had. + * Build a blob shaped exactly like the one the given build persisted: current initial values, but + * restricted to the keys that build's schema actually had. */ const buildReleaseBlob = (release: keyof typeof RELEASE_PARAMS_KEYS, overrides: Record = {}) => { const { version, keys } = RELEASE_PARAMS_KEYS[release]; @@ -423,10 +750,31 @@ const buildReleaseBlob = (release: keyof typeof RELEASE_PARAMS_KEYS, overrides: blob[key] = initial[key]; } } - blob._version = version; + if (version === 0) { + // v0 blobs carry no `_version` at all, and where they have `dimensions` it is the pre-flattening + // shape — the one field `keys` alone cannot reproduce, since filtering the current initial state + // would hand back the flat shape. `v6.0.0a1` has no `dimensions` key at all and must not gain + // one here, or it stops testing the v0 seed. + if ((keys as readonly string[]).includes('dimensions')) { + blob.dimensions = buildReleaseDimensions(release, 512, 512); + } + } else { + blob._version = version; + } return { ...blob, ...overrides }; }; +/** + * `dimensions` in the shape the given build persisted it: `{ rect: { x, y, width, height }, + * aspectRatio }` before the v0 -> v1 flattening, `{ width, height, aspectRatio }` after. + */ +const buildReleaseDimensions = (release: keyof typeof RELEASE_PARAMS_KEYS, width: number, height: number) => { + const { aspectRatio } = getInitialParamsState().dimensions; + return RELEASE_PARAMS_KEYS[release].version === 0 + ? { rect: { x: 0, y: 0, width, height }, aspectRatio } + : { width, height, aspectRatio }; +}; + describe('paramsSliceConfig persisted state migration', () => { const migrate = paramsSliceConfig.persistConfig?.migrate; @@ -622,7 +970,7 @@ describe('paramsSliceConfig persisted state migration', () => { expect(result.seed).toBe(99); }); - it.each(['v6.6.0', 'v6.7.0', 'v6.10.0', 'v6.13.7'] as const)( + it.each(['v6.0.0a1', 'v6.5.1', 'v6.6.0', 'v6.7.0', 'v6.10.0', 'v6.13.7', '1aeb05bbf0', 'f10d2a4f5a'] as const)( 'migrates a genuine %s blob without losing the user params', (release) => { expect(migrate).toBeDefined(); @@ -631,10 +979,17 @@ describe('paramsSliceConfig persisted state migration', () => { // since that has neither a zod default nor a migration seed fails the parse() at the end of // migrate(), and the caller in store.ts swallows the throw and falls back to the initial // state — silently wiping the user's prompts, model selection and dimensions on upgrade. + const hasDimensions = (RELEASE_PARAMS_KEYS[release].keys as readonly string[]).includes('dimensions'); const blob = buildReleaseBlob(release, { positivePrompt: 'a fluffy cat', seed: 42, shouldRandomizeSeed: false, + // Every value here is non-default on purpose. The initial state is what a wiped slice looks + // like, so a fixture carrying initial values cannot tell "migrated correctly" apart from + // "silently reset" — `getInitialParamsState().model` is null, which is exactly why nothing + // used to notice the repair pass clearing a model whose base has left the schema. + model: { key: 'm1', hash: 'h1', name: 'Some SDXL Model', base: 'sdxl', type: 'main' }, + ...(hasDimensions ? { dimensions: buildReleaseDimensions(release, 768, 1024) } : {}), }); const result = migrate?.(blob) as ReturnType; @@ -643,10 +998,18 @@ describe('paramsSliceConfig persisted state migration', () => { expect(result.positivePrompt).toBe('a fluffy cat'); expect(result.seed).toBe(42); expect(result.shouldRandomizeSeed).toBe(false); + expect(result.model).toMatchObject({ key: 'm1', base: 'sdxl' }); + if (hasDimensions) { + expect(result.dimensions.width).toBe(768); + expect(result.dimensions.height).toBe(1024); + } else { + // v6.0.0a1 - v6.0.0rc3 had no `dimensions` key; the v0 step seeds it. + expect(result.dimensions).toEqual(getInitialParamsState().dimensions); + } } ); - it('seeds every key of the current schema in the version steps themselves, for each released blob version', () => { + it('seeds every key of the current schema in the version steps themselves, for each persisted blob version', () => { // The general form of the defect this suite guards against: a key is added to zParamsState with // no `.default()`/`.optional()`/`.catch()` (so zod treats it as required) and no seed in the // migration chain. Every such key is a whole-slice wipe for anyone upgrading from a release @@ -654,20 +1017,26 @@ describe('paramsSliceConfig persisted state migration', () => { // schema, so the next occurrence fails here instead of shipping. // // This deliberately runs the version steps *without* going through migrate(), because - // backfillMissingParamsKeys() would otherwise repair the omission and hide it. The safety net - // is there to protect users from a forgotten seed; this test is what stops one being merged. + // repairParamsState() would otherwise repair the omission and hide it. The safety net is there + // to protect users from a forgotten seed; this test is what stops one being merged. for (const release of Object.keys(RELEASE_PARAMS_KEYS) as (keyof typeof RELEASE_PARAMS_KEYS)[]) { + const { version } = RELEASE_PARAMS_KEYS[release]; const blob = buildReleaseBlob(release); applyParamsVersionMigrations(blob); - const unseeded = backfillMissingParamsKeys(blob); + const { backfilled } = repairParamsState(blob); expect( - unseeded, - `Keys missing from a genuine ${release} blob that neither carry a zod default nor get seeded by ` + - `the migration chain. Upgrading from ${release} would throw in zParamsState.parse() and wipe the ` + - `user's whole params slice. Give each key a zod default, or seed it in the _version ` + - `${RELEASE_PARAMS_KEYS[release].version} migration step.` + backfilled, + version === getInitialParamsState()._version + ? `Keys missing from a blob written at ${release}, the commit that bumped _version to ${version}. ` + + `A blob already at the current version matches no branch in the migration chain, so no step can ` + + `seed these — each needs a zod default, or upgrading throws in zParamsState.parse() and wipes ` + + `the user's whole params slice.` + : `Keys missing from a genuine ${release} blob that neither carry a zod default nor get seeded by ` + + `the migration chain. Upgrading from ${release} would throw in zParamsState.parse() and wipe the ` + + `user's whole params slice. Give each key a zod default, or seed it in the _version ` + + `${version} migration step.` ).toEqual([]); } }); @@ -680,54 +1049,95 @@ describe('paramsSliceConfig persisted state migration', () => { // caller in store.ts an excuse to reset the slice. const blob = buildReleaseBlob('v6.13.7', { positivePrompt: 'a fluffy cat', seed: 42 }); applyParamsVersionMigrations(blob); - delete blob.pidSteps; + delete blob.krea2VaeModel; - const backfilled = backfillMissingParamsKeys(blob); + const { backfilled } = repairParamsState(blob); - expect(backfilled).toEqual(['pidSteps']); - expect(blob.pidSteps).toBe(4); + expect(backfilled).toEqual(['krea2VaeModel']); + expect(blob.krea2VaeModel).toBeNull(); expect(() => zParamsState.parse(blob)).not.toThrow(); expect(blob.positivePrompt).toBe('a fluffy cat'); }); - it('does not backfill over a key the persisted state already holds', () => { - // The net must only fill omissions — never overwrite a real persisted value, and never mask a - // present-but-invalid one (that still throws, same as before). + it('does not repair over a key the persisted state already holds', () => { + // The net must never overwrite a real persisted value — only fill omissions and replace values + // the field's own schema rejects. const blob = buildReleaseBlob('v6.13.7'); applyParamsVersionMigrations(blob); blob.pidSteps = 2; blob.positivePrompt = 'a fluffy cat'; - expect(backfillMissingParamsKeys(blob)).toEqual([]); + expect(repairParamsState(blob)).toEqual({ backfilled: [], reset: [] }); expect(blob.pidSteps).toBe(2); expect(blob.positivePrompt).toBe('a fluffy cat'); }); it('leaves a defaulted key to zod rather than backfilling it from the initial state', () => { // The net must not pre-empt a field the schema can fill itself, or the schema's `.default()` - // stops being authoritative the moment it diverges from getInitialParamsState(). `pidSteps` - // (required) must be filled; `ernieImageScheduler` (`.default('euler')`) must not be. + // stops being authoritative the moment it diverges from getInitialParamsState(). + // `krea2VaeModel` (required) must be filled; `ernieImageScheduler` (`.default('euler')`) must not. const blob = buildReleaseBlob('v6.13.7'); applyParamsVersionMigrations(blob); delete blob.ernieImageScheduler; - delete blob.pidSteps; + delete blob.krea2VaeModel; - expect(backfillMissingParamsKeys(blob)).toEqual(['pidSteps']); + expect(repairParamsState(blob).backfilled).toEqual(['krea2VaeModel']); expect(blob.ernieImageScheduler).toBeUndefined(); expect(zParamsState.parse(blob).ernieImageScheduler).toBe('euler'); }); - it('never backfills _version, so version detection cannot be bypassed', () => { + it('resets a present-but-invalid key instead of wiping the slice', () => { + // The other half of the net: a value that no longer satisfies its field's schema costs the user + // that one field, not every generation param they have. Without this, the parse at the end of + // migrate() throws and store.ts falls back to the initial state wholesale. + expect(migrate).toBeDefined(); + + const blob = buildReleaseBlob('v6.13.7', { positivePrompt: 'a fluffy cat', seed: 42 }); + applyParamsVersionMigrations(blob); + blob.pidSteps = 99; // out of the schema's 1-4 range + blob.scheduler = 'not_a_scheduler'; + + const { backfilled, reset } = repairParamsState(blob); + + expect(backfilled).toEqual([]); + // Sorted: the net walks `zParamsState.shape`, so the raw order tracks field declaration order. + expect([...reset].sort()).toEqual(['pidSteps', 'scheduler']); + expect(blob.pidSteps).toBe(4); + expect(blob.scheduler).toBe(getInitialParamsState().scheduler); + expect(blob.positivePrompt).toBe('a fluffy cat'); + expect(blob.seed).toBe(42); + }); + + it.each([ + ['a dimensions object left without width/height', { rect: { x: 0, y: 0 }, aspectRatio: undefined }], + ['a null dimensions', null], + ])('repairs %s on a v0 blob rather than wiping the slice', (_label, dimensions) => { + // The v0 step's `state.dimensions && state.dimensions.rect` guard only stops a TypeError; what + // it leaves behind is a `dimensions` that the schema rejects. The net has to replace it, or the + // guard buys nothing and the user still loses the slice — just via ZodError instead. expect(migrate).toBeDefined(); - // The v0 branch keys off `!('_version' in state)` (presence) while the net keys off `undefined` - // (value). If the net filled `_version`, a blob carrying an explicit undefined would be stamped - // as current having run no migration step at all. + const blob = buildReleaseBlob('v6.5.1', { dimensions, positivePrompt: 'a fluffy cat', seed: 7 }); + + const result = migrate?.(blob) as ReturnType; + + expect(result._version).toBe(5); + expect(result.dimensions).toEqual(getInitialParamsState().dimensions); + expect(result.positivePrompt).toBe('a fluffy cat'); + expect(result.seed).toBe(7); + }); + + it('treats a blob whose _version has no value as v0 and walks the whole chain', () => { + expect(migrate).toBeDefined(); + + // The v0 branch keys off the *value*, not `'_version' in state`. With a presence check this blob + // matches no branch at all, reaches the parse still at `_version: undefined` and takes the whole + // slice down. (A real persisted blob comes from JSON.parse and so can never hold an explicit + // `undefined` — this pins the invariant, not a reachable input.) const blob = buildReleaseBlob('v6.7.0', { _version: undefined, positivePrompt: 'a fluffy cat' }); const result = migrate?.(blob) as ReturnType; - // It is treated as a v0 blob and walked through the whole chain, not stamped v5 in place. expect(result._version).toBe(5); expect(result.positivePromptHistory).toEqual([]); expect(result.qwenImageVaeModel).toBeNull(); @@ -735,6 +1145,26 @@ describe('paramsSliceConfig persisted state migration', () => { expect(result.positivePrompt).toBe('a fluffy cat'); }); + it('never repairs _version, so version detection cannot be bypassed', () => { + // `_version` is the input to the version steps, so the net must leave it alone. If it repaired + // it, any blob whose version is not the current literal — including one written by a *newer* + // build — would be silently stamped v5 having run no step, and its stale field values would be + // accepted as current. Deliberately not routed through migrate(): the version steps normalise + // `_version` before the net ever sees it, so only calling the net directly tests the guard. + // The blob is otherwise complete (the current tier's key set), so `_version` is the only thing + // the parse below can object to. + const blob = buildReleaseBlob('f10d2a4f5a', { _version: 6, positivePrompt: 'a fluffy cat' }); + + const { backfilled, reset } = repairParamsState(blob); + + expect(backfilled).toEqual([]); + expect(reset).toEqual([]); + expect(blob._version).toBe(6); + // Still fatal, which is the correct outcome for a downgrade: that slice really was written by a + // schema this build does not know. + expect(() => zParamsState.parse(blob)).toThrow(); + }); + it('does not throw on a v0 blob whose dimensions are missing', () => { expect(migrate).toBeDefined(); @@ -853,6 +1283,55 @@ describe('paramsSliceConfig persisted state migration', () => { expect(result.wanGuidanceScaleLowNoise).toBe(3.5); }); + it('fills the PiD fields on a v4 blob written before they existed, without the repair pass', () => { + expect(migrate).toBeDefined(); + + // The PiD fields landed a day *after* the _version 3 -> 4 bump (1aeb05bbf0 on 2026-07-29, + // 3f5588f21f on 2026-07-30), so dev builds from that window persist v4 blobs without them. For + // as long as 4 was the current version no branch in the chain could reach those blobs, and the + // zod defaults were the only thing standing between those users and a wiped slice. main's + // v4 -> v5 step now seeds them as well; the defaults are what covers the same gap on the + // current tier, which by definition still has no step. + const blob = buildReleaseBlob('1aeb05bbf0', { positivePrompt: 'a fluffy cat', seed: 42 }); + expect('pidMode' in blob).toBe(false); + + applyParamsVersionMigrations(blob); + expect(blob._version).toBe(5); + + // Deliberately parsed directly rather than through migrate(). The repair pass would backfill + // these four from getInitialParamsState() to the very same values, so going through migrate() + // cannot tell the version steps and the zod defaults apart from the safety net catching their + // absence — the assertions would hold with both reverted. + const result = zParamsState.parse(blob); + + expect(result.pidMode).toBe('off'); + expect(result.pidDecoderModel).toBeNull(); + expect(result.gemma2EncoderModel).toBeNull(); + expect(result.pidSteps).toBe(4); + expect(result.positivePrompt).toBe('a fluffy cat'); + expect(result.seed).toBe(42); + }); + + it('clears only the model when its base has since been removed from the schema', () => { + expect(migrate).toBeDefined(); + + // `zBaseModelType` dropped the external-API bases ('chatgpt-4o', 'imagen3', 'veo3', ...) in + // v6.9.0rc1, so a genuine v6.5.1 - v6.8.1 blob can hold a `model` the current schema rejects. + // Before the repair pass that failed zParamsState.parse() and cost the user every param; now it + // costs them the model selection alone. + const blob = buildReleaseBlob('v6.7.0', { + model: { key: 'm1', hash: 'h1', name: 'GPT Image', base: 'chatgpt-4o', type: 'main' }, + positivePrompt: 'a fluffy cat', + seed: 42, + }); + + const result = migrate?.(blob) as ReturnType; + + expect(result.model).toBeNull(); + expect(result.positivePrompt).toBe('a fluffy cat'); + expect(result.seed).toBe(42); + }); + it('migrates old positive prompt history entries to prompt pairs', () => { expect(migrate).toBeDefined(); diff --git a/invokeai/frontend/web/src/features/controlLayers/store/paramsSlice.ts b/invokeai/frontend/web/src/features/controlLayers/store/paramsSlice.ts index 9e39c8cd8dd..cbc3368c905 100644 --- a/invokeai/frontend/web/src/features/controlLayers/store/paramsSlice.ts +++ b/invokeai/frontend/web/src/features/controlLayers/store/paramsSlice.ts @@ -983,63 +983,89 @@ export const { * That has happened whenever a key was added to the schema with neither a `.default()` nor a seed in * the migration chain, which is what the Wan and post-v3 seeds above exist to undo. * - * Backfilling those keys turns a forgotten seed into "one new field sits at its default" instead of - * "the user lost everything". Deliberately narrow: - * - Only keys that are *absent*. A key that is present but holds an invalid value still throws; - * this repairs omissions, not corruption. - * - Only keys the schema cannot fill itself. Anything with `.default()` / `.catch()` / `.optional()` - * is left to zod, so the schema's own default stays authoritative. + * Repairing the offending key turns that into "one field sits at its default" instead of "the user + * lost everything". Two kinds of damage are repaired, both per key: + * - `backfilled`: the key is *absent* and the schema cannot fill it itself. Anything with + * `.default()` / `.catch()` / `.optional()` is left to zod, so the schema's own default stays + * authoritative. + * - `reset`: the key is present but its value does not satisfy that field's schema. Whatever the + * cause — a tightened field schema, a hand-edited blob, a half-applied migration — resetting the + * one field is strictly better than the alternative, which is `store.ts` discarding all of them. + * Note the granularity is one top-level key, so this is not always cheap: one malformed entry in + * `positivePromptHistory` costs the whole history, and a `model` whose `base` has since been + * removed from `zBaseModelType` (the external-API bases dropped in v6.9.0rc1, say) clears the + * user's model selection. Both are still a single field rather than every field. * * This is a safety net, not a substitute for a migration step — it fills fields with *today's* * initial value, which is only the right answer for genuinely new fields. `paramsSlice.test.ts` - * asserts the returned list is empty for real released blobs, so a forgotten seed still fails CI. + * asserts nothing needs repairing for real persisted blobs, so a forgotten seed still fails CI. * * Exported for that test. */ -export const backfillMissingParamsKeys = (state: Record): string[] => { +export const repairParamsState = (state: Record): { backfilled: string[]; reset: string[] } => { const initial = getInitialParamsState() as unknown as Record; const backfilled: string[] = []; + const reset: string[] = []; for (const [key, fieldSchema] of Object.entries(zParamsState.shape)) { - // Never touch `_version`. The version steps detect a v0 blob with `!('_version' in state)`, i.e. - // key presence, so filling it here on a value check would stamp a blob as current having run no - // migration step at all. + // Never touch `_version`: it is the input to the version steps, so repairing it would stamp a + // blob as current having run no step at all. A `_version` from the future (a downgrade) is + // deliberately still fatal — that slice really was written by a newer schema. if (key === '_version') { continue; } - // `undefined` is the only value that counts as missing: persisted JSON can't hold it, and every - // nullable field in the schema uses `null` for "unset", so this never overwrites a real value. - if (state[key] !== undefined || fieldSchema.safeParse(undefined).success) { + + if (state[key] === undefined) { + // `undefined` is the only value that counts as missing: persisted JSON can't hold it, and + // every nullable field in the schema uses `null` for "unset". + if (!fieldSchema.safeParse(undefined).success) { + state[key] = initial[key]; + backfilled.push(key); + } continue; } - state[key] = initial[key]; - backfilled.push(key); + + if (!fieldSchema.safeParse(state[key]).success) { + state[key] = initial[key]; + reset.push(key); + } } - return backfilled; + return { backfilled, reset }; }; /** * Bring a persisted params blob up to the current `_version` in place. * * Every key added to `zParamsState` without a zod default must be seeded by the step for the version - * that predates it, or upgrading users lose the whole slice — see `backfillMissingParamsKeys`. + * that predates it, or upgrading users lose the whole slice — see `repairParamsState`. Note that + * the *current* version has no step by definition, so a key added after the last bump needs a zod + * default (as the ERNIE-Image and PiD fields have) rather than a seed. * * Exported so the tests can assert the version steps alone are complete, without the safety net * hiding a missing seed. */ // eslint-disable-next-line @typescript-eslint/no-explicit-any export const applyParamsVersionMigrations = (state: any): void => { - // Value check rather than `!('_version' in state)`, so that a blob carrying an explicit undefined - // is treated as v0 and walked through the chain. With a presence check it matches no branch at - // all, reaches the parse with `_version: undefined` and takes the whole slice down with it. + // Value check rather than `!('_version' in state)`. The two are equivalent on real input, since + // the only production caller feeds this `JSON.parse` output, which cannot produce an explicit + // `undefined` — but a value check is what the rest of this file uses, and with a presence check a + // blob carrying `_version: undefined` would match no branch at all, reach the parse and take the + // whole slice down. if (state._version === undefined) { // v0 -> v1, add _version and remove x/y from dimensions, lifting width/height to top level. - // `dimensions.rect` is optional-chained: a truncated or hand-edited blob that lacks it would - // otherwise throw a TypeError out of migrate() and cost the user the whole slice. Leaving - // width/height undefined instead lets backfillMissingParamsKeys() repair `dimensions`. + // `dimensions.rect` is guarded: a truncated or hand-edited blob that lacks it would otherwise + // throw a TypeError out of migrate() and cost the user the whole slice. What that leaves behind + // — a `dimensions` object with no width/height — fails its own field schema, so + // repairParamsState() swaps in the initial dimensions instead of letting the parse wipe + // everything else along with it. state._version = 1; - if (state.dimensions && state.dimensions.rect) { + if (state.dimensions === undefined) { + // The oldest v0 builds (v6.0.0a1 - v6.0.0rc3) had no `dimensions` key at all; it arrives in + // v6.0.0rc4. Seeding it here rather than leaving it to the repair pass is what keeps the + // version steps self-sufficient for the v0 tier. + state.dimensions = getInitialParamsState().dimensions; + } else if (state.dimensions && state.dimensions.rect) { state.dimensions.width = state.dimensions.rect.width; state.dimensions.height = state.dimensions.rect.height; } @@ -1167,7 +1193,7 @@ export const paramsSliceConfig: SliceConfig = { applyParamsVersionMigrations(state); - const backfilled = backfillMissingParamsKeys(state); + const { backfilled, reset } = repairParamsState(state); if (backfilled.length > 0) { log.warn( { backfilled }, @@ -1175,6 +1201,12 @@ export const paramsSliceConfig: SliceConfig = { `These need a zod default or a seed in the migration chain.` ); } + if (reset.length > 0) { + log.warn( + { reset }, + `Reset ${reset.length} params key(s) whose persisted value no longer satisfies the schema: ${reset.join(', ')}.` + ); + } return zParamsState.parse(state); }, diff --git a/invokeai/frontend/web/src/features/controlLayers/store/types.ts b/invokeai/frontend/web/src/features/controlLayers/store/types.ts index 898587a3f11..e446d1b82db 100644 --- a/invokeai/frontend/web/src/features/controlLayers/store/types.ts +++ b/invokeai/frontend/web/src/features/controlLayers/store/types.ts @@ -908,10 +908,16 @@ export const zParamsState = z.object({ // - 'off': regular VAE decode // - 'fit': PiD decodes 4x internally, then downscales back to the bbox (compositing-safe; works in canvas/inpaint) // - 'native': PiD's full 4x output IS the result; the user-facing dimensions are the target, generation runs at target / 4 - pidMode: zPidMode, - pidDecoderModel: zModelIdentifierField.nullable(), // PiD decoder checkpoint (matched to the main model's base) - gemma2EncoderModel: zModelIdentifierField.nullable(), // Gemma-2 caption encoder required by PiD - pidSteps: z.number().int().min(1).max(4), // PiD distill steps: student schedule has only 4 transitions, so 1-4 + // These four landed *after* the `_version` 3 -> 4 bump, so for as long as 4 was the current + // version no migration step could reach them: a blob already at v4 (dev builds from that window) + // matched no branch in the chain. They carry zod defaults for the same reason the ERNIE-Image + // fields above do — without one they would be required, and their absence would fail the parse in + // `migrate()` and wipe the whole slice. That is the rule for any field added after the last bump, + // which is why it still applies now that the v4 -> v5 step also seeds these. + pidMode: zPidMode.default('off'), + pidDecoderModel: zModelIdentifierField.nullable().default(null), // PiD decoder checkpoint (matched to the main model's base) + gemma2EncoderModel: zModelIdentifierField.nullable().default(null), // Gemma-2 caption encoder required by PiD + pidSteps: z.number().int().min(1).max(4).default(4), // PiD distill steps: student schedule has only 4 transitions, so 1-4 // Qwen Image Edit model components - GGUF transformer needs a Diffusers source for VAE/encoder qwenImageComponentSource: zParameterModel.nullable(), // Diffusers model providing VAE + text encoder qwenImageVaeModel: zParameterVAEModel.nullable(), // Optional: Standalone Qwen Image VAE checkpoint From 46b1c52646d9b6d5c4d3ebe92c824c3d7f7ea5bd Mon Sep 17 00:00:00 2001 From: Lincoln Stein Date: Thu, 6 Aug 2026 20:46:10 -0400 Subject: [PATCH 5/6] fix(ui): give the HiDiffusion params fields zod defaults MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit HiDiffusion (#8787) landed in main while this PR was in review, adding five keys to zParamsState after the _version 3 -> 4 bump. Keys added after a bump land in a tier the migration chain cannot reach — a v4 blob matches no branch — so they were seeded by an ad-hoc block inside migrate() instead. That works, but it sits outside applyParamsVersionMigrations(), so the completeness invariant added by this PR cannot see it and reports the five keys as unseeded. Give them zod defaults, the same route the ERNIE-Image and PiD fields take, and drop the now-redundant block: the defaults carry the identical values, and the repair pass covers the blob before the parse either way. Co-Authored-By: Claude Opus 5 --- .../controlLayers/store/paramsSlice.test.ts | 28 +++++++++++-------- .../controlLayers/store/paramsSlice.ts | 19 ------------- .../src/features/controlLayers/store/types.ts | 14 ++++++---- 3 files changed, 26 insertions(+), 35 deletions(-) diff --git a/invokeai/frontend/web/src/features/controlLayers/store/paramsSlice.test.ts b/invokeai/frontend/web/src/features/controlLayers/store/paramsSlice.test.ts index 389e58d42b0..1fabb779a1c 100644 --- a/invokeai/frontend/web/src/features/controlLayers/store/paramsSlice.test.ts +++ b/invokeai/frontend/web/src/features/controlLayers/store/paramsSlice.test.ts @@ -621,7 +621,7 @@ const RELEASE_PARAMS_KEYS = { 'zImageVaeModel', ], }, - 'f10d2a4f5a': { + f10d2a4f5a: { version: 5, keys: [ 'animaLLLiteModel', @@ -1283,31 +1283,37 @@ describe('paramsSliceConfig persisted state migration', () => { expect(result.wanGuidanceScaleLowNoise).toBe(3.5); }); - it('fills the PiD fields on a v4 blob written before they existed, without the repair pass', () => { + it('fills the fields added after the v4 bump from their zod defaults', () => { expect(migrate).toBeDefined(); - // The PiD fields landed a day *after* the _version 3 -> 4 bump (1aeb05bbf0 on 2026-07-29, - // 3f5588f21f on 2026-07-30), so dev builds from that window persist v4 blobs without them. For - // as long as 4 was the current version no branch in the chain could reach those blobs, and the - // zod defaults were the only thing standing between those users and a wiped slice. main's - // v4 -> v5 step now seeds them as well; the defaults are what covers the same gap on the - // current tier, which by definition still has no step. + // Everything added since 1aeb05bbf0 bumped _version to 4 landed in a tier the chain could not + // reach for as long as 4 was current: a v4 blob matched no branch, so no step could seed it and + // a zod default was the only option. The PiD fields (3f5588f21f, one day after the bump) are the + // case that dev builds actually hit; the ERNIE-Image and HiDiffusion fields followed the same + // route. main's v4 -> v5 step now also seeds the PiD fields, but the defaults are what covers + // the same gap on the current tier, which by definition still has no step. const blob = buildReleaseBlob('1aeb05bbf0', { positivePrompt: 'a fluffy cat', seed: 42 }); expect('pidMode' in blob).toBe(false); + expect('hiDiffusionEnabled' in blob).toBe(false); applyParamsVersionMigrations(blob); expect(blob._version).toBe(5); // Deliberately parsed directly rather than through migrate(). The repair pass would backfill - // these four from getInitialParamsState() to the very same values, so going through migrate() - // cannot tell the version steps and the zod defaults apart from the safety net catching their - // absence — the assertions would hold with both reverted. + // these from getInitialParamsState() to the very same values, so going through migrate() cannot + // tell the version steps and the zod defaults apart from the safety net catching their absence — + // the assertions would hold with both reverted. const result = zParamsState.parse(blob); expect(result.pidMode).toBe('off'); expect(result.pidDecoderModel).toBeNull(); expect(result.gemma2EncoderModel).toBeNull(); expect(result.pidSteps).toBe(4); + expect(result.hiDiffusionEnabled).toBe(false); + expect(result.hiDiffusionRauNetEnabled).toBe(true); + expect(result.hiDiffusionWindowAttnEnabled).toBe(true); + expect(result.hiDiffusionT1Ratio).toBe(0.4); + expect(result.hiDiffusionT2Ratio).toBe(0.0); expect(result.positivePrompt).toBe('a fluffy cat'); expect(result.seed).toBe(42); }); diff --git a/invokeai/frontend/web/src/features/controlLayers/store/paramsSlice.ts b/invokeai/frontend/web/src/features/controlLayers/store/paramsSlice.ts index cbc3368c905..f7c8420dade 100644 --- a/invokeai/frontend/web/src/features/controlLayers/store/paramsSlice.ts +++ b/invokeai/frontend/web/src/features/controlLayers/store/paramsSlice.ts @@ -1162,25 +1162,6 @@ export const applyParamsVersionMigrations = (state: any): void => { state.gemma2EncoderModel = state.gemma2EncoderModel ?? null; state.pidSteps = state.pidSteps ?? 4; } - - // The HiDiffusion fields were added to the schema without a version bump, so they can be missing - // from a blob of any version — seeded outside the version steps for that reason. They have no zod - // default, so an absent key would fail the parse() at the end of migrate() and wipe the slice. - if (!('hiDiffusionEnabled' in state)) { - state.hiDiffusionEnabled = false; - } - if (!('hiDiffusionRauNetEnabled' in state)) { - state.hiDiffusionRauNetEnabled = true; - } - if (!('hiDiffusionWindowAttnEnabled' in state)) { - state.hiDiffusionWindowAttnEnabled = true; - } - if (!('hiDiffusionT1Ratio' in state)) { - state.hiDiffusionT1Ratio = 0.4; - } - if (!('hiDiffusionT2Ratio' in state)) { - state.hiDiffusionT2Ratio = 0.0; - } }; export const paramsSliceConfig: SliceConfig = { diff --git a/invokeai/frontend/web/src/features/controlLayers/store/types.ts b/invokeai/frontend/web/src/features/controlLayers/store/types.ts index e446d1b82db..2143e58f997 100644 --- a/invokeai/frontend/web/src/features/controlLayers/store/types.ts +++ b/invokeai/frontend/web/src/features/controlLayers/store/types.ts @@ -832,11 +832,15 @@ export const zParamsState = z.object({ guidance: zParameterGuidance, img2imgStrength: zParameterStrength, optimizedDenoisingEnabled: z.boolean(), - hiDiffusionEnabled: z.boolean(), - hiDiffusionRauNetEnabled: z.boolean(), - hiDiffusionWindowAttnEnabled: z.boolean(), - hiDiffusionT1Ratio: z.number(), - hiDiffusionT2Ratio: z.number(), + // Added after the `_version` 3 -> 4 bump, so while 4 was current no migration step could seed them + // — a blob already at v4 matched no branch in the chain. Without defaults they are required, and + // every persisted blob in existence fails the parse in `migrate()`, wiping the user's whole params + // slice on upgrade. The defaults are still what covers the current tier, which has no step either. + hiDiffusionEnabled: z.boolean().default(false), + hiDiffusionRauNetEnabled: z.boolean().default(true), + hiDiffusionWindowAttnEnabled: z.boolean().default(true), + hiDiffusionT1Ratio: z.number().default(0.4), + hiDiffusionT2Ratio: z.number().default(0.0), iterations: z.number(), scheduler: zParameterScheduler, fluxScheduler: zParameterFluxScheduler, From f740de71df5c3ce4867ece43d07f296b039d6b2d Mon Sep 17 00:00:00 2001 From: Lincoln Stein Date: Fri, 7 Aug 2026 21:34:27 -0400 Subject: [PATCH 6/6] test(ui): close three inert spots in the params migration suite MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Found by mutation-testing this branch's own suite: 116 single-line mutations of the production code, each run against the tests. Three classes of mutation left the suite green. Conditional seeds. Turning `state.X = state.X ?? V` into `state.X = V` went undetected for 21 of the 31 conditional seeds, because only 10 were probed by the two hand-written "preserves ... dev-build" tests. The `??` is the entire point of those lines: the field landed in the schema before the bump that seeds it, so a blob from that window already holds a real user value, and an unconditional assignment resets it. Replace the two tests with a table covering every conditional seed, one probe per key. The table self-validates — each probe must satisfy the field's own schema and must differ from what the step would seed — so a probe that stops discriminating fails rather than going quiet. Verified: seven representative mutations now fail, each naming its own key. Fixture erosion. `buildReleaseBlob` sources values from getInitialParamsState(), so a fixture key that leaves zParamsState is dropped from the blob silently while still sitting in the table, and the fixture stops reproducing the shape it names. That already happened: kleinVaeModel left the schema when the v4 -> v5 step folded it into flux2VaeModel, and it is the input to that fold. Add REMOVED_SCHEMA_KEY_VALUES so removed keys are still reproduced, plus a guard test that fails on any fixture key in neither the schema nor that table. Post-bump defaults. Three mechanisms now write the PiD fields with identical values — the v3 -> v4 seed, the v4 -> v5 seed and the zod default — so any one could be reverted with the other two covering for it, and the test that claims to prove the defaults could not see it. Pin the property directly: every key added after the last bump must satisfy `shape[key].safeParse(undefined)`, which is what lets it survive on a tier that has no migration step. Also assert the version steps leave no key holding a schema-rejecting value, and note in the completeness test what it does not cover: fixture values are initial values, so a tightened *nested* schema still costs a top-level key via `reset` without any fixture noticing. Co-Authored-By: Claude Opus 5 (1M context) --- .../controlLayers/store/paramsSlice.test.ts | 215 ++++++++++++++---- 1 file changed, 169 insertions(+), 46 deletions(-) diff --git a/invokeai/frontend/web/src/features/controlLayers/store/paramsSlice.test.ts b/invokeai/frontend/web/src/features/controlLayers/store/paramsSlice.test.ts index 1fabb779a1c..e2a2713aa41 100644 --- a/invokeai/frontend/web/src/features/controlLayers/store/paramsSlice.test.ts +++ b/invokeai/frontend/web/src/features/controlLayers/store/paramsSlice.test.ts @@ -624,6 +624,7 @@ const RELEASE_PARAMS_KEYS = { f10d2a4f5a: { version: 5, keys: [ + '_version', 'animaLLLiteModel', 'animaLLLiteWeight', 'animaQwen3EncoderModel', @@ -719,7 +720,6 @@ const RELEASE_PARAMS_KEYS = { 'upscaleScheduler', 'vae', 'vaePrecision', - '_version', 'wanComponentSource', 'wanGuidanceScaleLowNoise', 'wanT5EncoderModel', @@ -737,6 +737,31 @@ const RELEASE_PARAMS_KEYS = { }, } as const satisfies Record; +/** + * Values for fixture keys that have since been removed from `zParamsState`, so a blob can still + * reproduce them. + * + * `buildReleaseBlob` sources its values from `getInitialParamsState()`, which by definition only + * knows today's keys. Without this table a fixture key that leaves the schema is dropped from the + * blob *silently*, and the fixture quietly stops reproducing the shape the build actually persisted + * — which in turn stops exercising whatever migration step reads that key. That is not theoretical: + * `kleinVaeModel` left the schema when the v4 -> v5 step folded it into `flux2VaeModel`, and it is + * the input to that fold. The guard test below fails on any fixture key that is in neither the + * current schema nor this table, so the next removal has to be a decision rather than an accident. + */ +const REMOVED_SCHEMA_KEY_VALUES: Record = { + // Folded into `flux2VaeModel` and deleted by the v4 -> v5 step. + kleinVaeModel: null, + // The pre-merge FLUX.2 [dev] branch's VAE slot, folded into `flux2VaeModel` by the same step. + flux2DevVaeModel: null, + // The v0-era second prompt box, dropped when prompt concatenation was removed. + positivePrompt2: '', + negativePrompt2: '', + shouldConcatPrompts: true, + // Present in v6.13.0 only, removed before v6.13.7. + animaT5EncoderModel: null, +}; + /** * Build a blob shaped exactly like the one the given build persisted: current initial values, but * restricted to the keys that build's schema actually had. @@ -748,6 +773,8 @@ const buildReleaseBlob = (release: keyof typeof RELEASE_PARAMS_KEYS, overrides: for (const key of keys) { if (key in initial) { blob[key] = initial[key]; + } else if (key in REMOVED_SCHEMA_KEY_VALUES) { + blob[key] = REMOVED_SCHEMA_KEY_VALUES[key]; } } if (version === 0) { @@ -764,6 +791,76 @@ const buildReleaseBlob = (release: keyof typeof RELEASE_PARAMS_KEYS, overrides: return { ...blob, ...overrides }; }; +const probeModel = (key: string, base: string, type: string) => ({ key, hash: `${key}-hash`, name: key, base, type }); + +/** + * Keys added to `zParamsState` after `1aeb05bbf0` bumped `_version` to 4, which reach users only via + * a zod default. A tier that is current has no migration step by definition, so this is the only + * route available to anything added after the most recent bump. + */ +const POST_V4_BUMP_DEFAULTED_KEYS = [ + 'pidMode', + 'pidDecoderModel', + 'gemma2EncoderModel', + 'pidSteps', + 'hiDiffusionEnabled', + 'hiDiffusionRauNetEnabled', + 'hiDiffusionWindowAttnEnabled', + 'hiDiffusionT1Ratio', + 'hiDiffusionT2Ratio', +] as const satisfies readonly (keyof typeof zParamsState.shape)[]; + +/** + * Every seed in the version steps written conditionally (`state.X = state.X ?? V`), paired with a + * valid value that differs from what the seed would write. + * + * The `??` is the whole point of those lines: the field landed in the schema *before* the version + * bump that seeds it, so a blob written by a dev build in that window already carries a real user + * value, and an unconditional assignment would silently reset it. That is a one-field data loss per + * key, invisible unless something probes the key specifically — so every conditional seed gets a + * row here. `tier` is the fixture whose `_version` the seeding step consumes. + */ +const CONDITIONAL_SEED_PROBES: { + tier: keyof typeof RELEASE_PARAMS_KEYS; + key: keyof typeof zParamsState.shape; + value: unknown; +}[] = [ + // v2 -> v3 + { tier: 'v6.7.0', key: 'fluxScheduler', value: 'lcm' }, + { tier: 'v6.7.0', key: 'zImageScheduler', value: 'lcm' }, + { tier: 'v6.7.0', key: 'colorCompensation', value: true }, + { tier: 'v6.7.0', key: 'zImageVaeModel', value: probeModel('z-vae', 'z-image', 'vae') }, + { tier: 'v6.7.0', key: 'zImageQwen3EncoderModel', value: probeModel('z-enc', 'z-image', 'main') }, + { tier: 'v6.7.0', key: 'zImageQwen3SourceModel', value: probeModel('z-src', 'z-image', 'main') }, + { tier: 'v6.7.0', key: 'fluxDypePreset', value: 'auto' }, + { tier: 'v6.7.0', key: 'fluxDypeScale', value: 3.5 }, + { tier: 'v6.7.0', key: 'fluxDypeExponent', value: 3.5 }, + { tier: 'v6.7.0', key: 'zImageShift', value: 3.0 }, + { tier: 'v6.7.0', key: 'zImageSeedVarianceEnabled', value: true }, + { tier: 'v6.7.0', key: 'zImageSeedVarianceStrength', value: 0.3 }, + { tier: 'v6.7.0', key: 'zImageSeedVarianceRandomizePercent', value: 75 }, + { tier: 'v6.7.0', key: 'animaVaeModel', value: probeModel('anima-vae', 'anima', 'vae') }, + { tier: 'v6.7.0', key: 'animaQwen3EncoderModel', value: probeModel('anima-enc', 'anima', 'main') }, + { tier: 'v6.7.0', key: 'animaScheduler', value: 'dpmpp_2m' }, + { tier: 'v6.7.0', key: 'kleinQwen3EncoderModel', value: probeModel('klein-enc', 'flux2', 'main') }, + { tier: 'v6.7.0', key: 'qwenImageComponentSource', value: probeModel('qwen-src', 'qwen-image', 'main') }, + { tier: 'v6.7.0', key: 'qwenImageQuantization', value: 'int8' }, + { tier: 'v6.7.0', key: 'qwenImageShift', value: 3.0 }, + // v3 -> v4 + { tier: 'v6.13.7', key: 'wanTransformerLowNoise', value: probeModel('wan-low', 'wan', 'main') }, + { tier: 'v6.13.7', key: 'wanComponentSource', value: probeModel('wan-src', 'wan', 'main') }, + { tier: 'v6.13.7', key: 'wanVaeModel', value: probeModel('wan-vae', 'wan', 'vae') }, + { tier: 'v6.13.7', key: 'wanT5EncoderModel', value: probeModel('wan-t5', 'wan', 'main') }, + { tier: 'v6.13.7', key: 'wanGuidanceScaleLowNoise', value: 3.5 }, + // v4 -> v5 + { tier: '1aeb05bbf0', key: 'flux2VaeModel', value: probeModel('flux2-vae', 'flux2', 'vae') }, + { tier: '1aeb05bbf0', key: 'flux2DevMistralEncoderModel', value: probeModel('mistral', 'flux2', 'main') }, + { tier: '1aeb05bbf0', key: 'pidMode', value: 'native' }, + { tier: '1aeb05bbf0', key: 'pidDecoderModel', value: probeModel('pid-dec', 'flux', 'main') }, + { tier: '1aeb05bbf0', key: 'gemma2EncoderModel', value: probeModel('gemma2', 'flux', 'main') }, + { tier: '1aeb05bbf0', key: 'pidSteps', value: 2 }, +]; + /** * `dimensions` in the shape the given build persisted it: `{ rect: { x, y, width, height }, * aspectRatio }` before the v0 -> v1 flattening, `{ width, height, aspectRatio }` after. @@ -1019,12 +1116,26 @@ describe('paramsSliceConfig persisted state migration', () => { // This deliberately runs the version steps *without* going through migrate(), because // repairParamsState() would otherwise repair the omission and hide it. The safety net is there // to protect users from a forgotten seed; this test is what stops one being merged. + // + // Scope limit worth knowing: fixture values all come from getInitialParamsState(), so this + // covers *missing* keys, not keys whose persisted value a tightened schema would now reject. + // Tightening a nested schema (an item in `positivePromptHistory`, say) still costs that whole + // top-level key via `reset`, and no fixture here would notice. Realistic per-key persisted + // values would be needed to close that, which is a bigger change than this suite. for (const release of Object.keys(RELEASE_PARAMS_KEYS) as (keyof typeof RELEASE_PARAMS_KEYS)[]) { const { version } = RELEASE_PARAMS_KEYS[release]; const blob = buildReleaseBlob(release); applyParamsVersionMigrations(blob); - const { backfilled } = repairParamsState(blob); + const { backfilled, reset } = repairParamsState(blob); + + // The steps must not leave a value the schema rejects either — a half-applied migration that + // writes a malformed value costs the user that field, silently. + expect( + reset, + `The version steps left these keys holding a value that fails its own field schema, on a ` + + `blob written at ${release}. Each costs the user that field on upgrade.` + ).toEqual([]); expect( backfilled, @@ -1041,6 +1152,31 @@ describe('paramsSliceConfig persisted state migration', () => { } }); + it('reproduces every fixture key, including those since removed from the schema', () => { + // `buildReleaseBlob` can only source values it knows about, so a fixture key that is neither in + // the current schema nor in REMOVED_SCHEMA_KEY_VALUES is dropped from the blob without a word. + // The fixture then still *looks* faithful — the key is right there in the table — while having + // quietly stopped exercising whatever step consumes it. Fail instead, so removing a key from + // zParamsState forces a decision about the fixtures that name it. + const initial = getInitialParamsState() as unknown as Record; + + for (const release of Object.keys(RELEASE_PARAMS_KEYS) as (keyof typeof RELEASE_PARAMS_KEYS)[]) { + const { keys } = RELEASE_PARAMS_KEYS[release]; + const blob = buildReleaseBlob(release); + const dropped = (keys as readonly string[]).filter( + (key) => !(key in initial) && !(key in REMOVED_SCHEMA_KEY_VALUES) && !(key in blob) + ); + + expect( + dropped, + `The ${release} fixture lists these keys, but they are neither in the current zParamsState nor ` + + `in REMOVED_SCHEMA_KEY_VALUES, so buildReleaseBlob silently omits them and the fixture no ` + + `longer reproduces the blob that build persisted. Add each to REMOVED_SCHEMA_KEY_VALUES with ` + + `the value that build wrote, or drop it from the fixture if it never existed.` + ).toEqual([]); + } + }); + it('backfills a key the version steps forget, instead of wiping the slice', () => { expect(migrate).toBeDefined(); @@ -1238,50 +1374,27 @@ describe('paramsSliceConfig persisted state migration', () => { expect(result.positivePrompt).toBe('a fluffy cat'); }); - it('preserves post-v3 values already present in a dev-build v2 blob', () => { - expect(migrate).toBeDefined(); + it.each(CONDITIONAL_SEED_PROBES)( + 'preserves a persisted $key rather than reseeding it (dev-build $tier blob)', + ({ tier, key, value }) => { + expect(migrate).toBeDefined(); - // v2 blobs written by dev builds after each field landed already carry the keys, possibly with - // real values — the conditional seeds must not clobber them. - const v2State = buildReleaseBlob('v6.7.0', { - fluxScheduler: 'heun', - colorCompensation: true, - fluxDypePreset: 'auto', - qwenImageQuantization: 'int8', - qwenImageShift: 3.0, - zImageSeedVarianceEnabled: true, - }); + // Pin the probe itself, or this test goes quietly inert: the value has to satisfy the field's + // own schema, and it has to differ from what the step would seed — otherwise "preserved" and + // "clobbered" look identical and an unconditional assignment slips through. + expect(zParamsState.shape[key].safeParse(value).success).toBe(true); + const reseeded = migrate?.(buildReleaseBlob(tier)) as Record; + expect(reseeded[key]).not.toEqual(value); - const result = migrate?.(v2State) as ReturnType; + const blob = buildReleaseBlob(tier, { [key]: value, positivePrompt: 'a fluffy cat' }); - expect(result.fluxScheduler).toBe('heun'); - expect(result.colorCompensation).toBe(true); - expect(result.fluxDypePreset).toBe('auto'); - expect(result.qwenImageQuantization).toBe('int8'); - expect(result.qwenImageShift).toBe(3.0); - expect(result.zImageSeedVarianceEnabled).toBe(true); - }); + const result = migrate?.(blob) as Record; - it('preserves Wan values already present in a dev-build v3 blob', () => { - expect(migrate).toBeDefined(); - - const initial = getInitialParamsState(); - const wanVae = { key: 'wan-vae', hash: 'h', name: 'Wan VAE', base: 'wan', type: 'vae' }; - // v3 blobs written by dev builds after the Wan merge already carry the keys, possibly with - // real values — the conditional seeds must not clobber them. - const v3State: Record = { - ...initial, - _version: 3, - wanVaeModel: wanVae, - wanGuidanceScaleLowNoise: 3.5, - }; - - const result = migrate?.(v3State) as ReturnType; - - expect(result._version).toBe(5); - expect((result.wanVaeModel as { key: string } | null)?.key).toBe('wan-vae'); - expect(result.wanGuidanceScaleLowNoise).toBe(3.5); - }); + expect(result[key]).toEqual(value); + // The rest of the slice has to come through too — a reset here would be the whole-slice wipe. + expect(result.positivePrompt).toBe('a fluffy cat'); + } + ); it('fills the fields added after the v4 bump from their zod defaults', () => { expect(migrate).toBeDefined(); @@ -1299,10 +1412,20 @@ describe('paramsSliceConfig persisted state migration', () => { applyParamsVersionMigrations(blob); expect(blob._version).toBe(5); - // Deliberately parsed directly rather than through migrate(). The repair pass would backfill - // these from getInitialParamsState() to the very same values, so going through migrate() cannot - // tell the version steps and the zod defaults apart from the safety net catching their absence — - // the assertions would hold with both reverted. + // The value assertions below cannot, on their own, prove the defaults exist: three mechanisms + // produce the identical values, so any two can hide the third being reverted. Parsing directly + // rather than through migrate() rules out the repair pass, but not the v4 -> v5 step, which + // seeds all four PiD fields. Pin the property itself — carrying a default is exactly what lets a + // field survive on a tier with no step, and it is the thing the current tier depends on. + for (const key of POST_V4_BUMP_DEFAULTED_KEYS) { + expect( + zParamsState.shape[key].safeParse(undefined).success, + `${key} was added after the _version 4 bump and must carry a zod default: the current tier ` + + `has no migration step by definition, so a required key there fails the parse in migrate() ` + + `and wipes the user's whole params slice.` + ).toBe(true); + } + const result = zParamsState.parse(blob); expect(result.pidMode).toBe('off');