refactor: drop legacy 'native' AspectRatio via v6 schema migration - #195
Merged
EtienneLescot merged 4 commits intoJul 29, 2026
Merged
Conversation
(1) Bumped axcutSchemaVersion 5 -> 6 and added upgradeV5DocumentToV6,
which rewrites every stored legacyEditor.aspectRatio === "native"
to a concrete "W:H" token (the timeline's largest clip dims,
falling back to "16:9"). The upgrader is wired into the existing
z.preprocess chain alongside v3->v4 and v4->v5.
(2) Dropped the "native" union arm from AspectRatio and the
per-helper branches in getAspectRatioValue / getAspectRatioLabel /
isAspectRatio / formatAspectRatioForCSS. Renamed
NATIVE_ASPECT_RATIO_FALLBACK -> ASPECT_RATIO_FALLBACK (the
file-private fallback). getNativeAspectRatioValue is kept.
(3) Removed the runtime bridge in outputFormat.resolveAspectRatioValue
(it was document-aware to resolve "native") and the self-migration
useMemo in V4Timeline (the activeToken that re-mapped
settings.aspectRatio to the largest clip's token). Both are dead
now that v6 documents cannot contain "native". Updated the three
preview/export call sites to the new one-arg signature.
(4) Tests: bumped every existing schemaVersion === 5 assertion to 6
(schema, migrate, document-service), added a 7-test v5->v6
migration block in schema/index.test.ts, and stripped the "native"
assertions from aspectRatioUtils and outputFormat tests (pointing
at the v5->v6 upgrader tests as the new home).
Intentionally untouched: lastBackgroundColor. The original audit
recommended dropping it as dead code, but the picker swatch in
src/lib/ai-edition/annotations/background.ts:25-29 reads it for the
swatch when bg is off, and toggleTextBackground writes it on every
toggle — it's the fix for the "background toggle always reverts to
black" regression.
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
…n build Two defects in the v5->v6 upgrade: - largestClipDims read raw asset.video dims, but 'native' resolved to the CROPPED clip at runtime (clipEffectiveDims -> calculateEffectiveSourceDimensions). A 3840x2160 asset cropped to its left half migrated to 16:9 instead of 8:9, silently reframing every cropped project. Apply the crop, snapping to even pixels the same way the export path does. - The new toAspectRatioToken import used the @/ alias, which vite-plugin-electron does not configure for the main bundle, so the build failed to resolve it. Also replaces a vacuous gating test (built a v4 object, never passed it to the upgrader, asserted its input was unchanged) with a real crop assertion, and bumps two fixtures the version bump invalidated.
The v1.7 -> v1.8 import is the case this breaks. A v1.7 project is
{version:2, media, editor} and carries only file PATHS, so
migrateProjectDataToAxcutDocument produces assets with no video block. The
v5->v6 upgrader then found no dimensions, stamped a hardcoded '16:9', and the
caller persisted it — permanently reframing every portrait v1.7 project saved
with 'Native', on its first open in v1.8, with no way back. Before this PR the
same project resolved dynamically and self-corrected once useTimeline's probe
wrote asset.video to disk.
Make the bake opportunistic: convert only when dimensions are actually known
(crop-aware, as fixed earlier), otherwise leave 'native' in place. It keeps
resolving dynamically at runtime — which is exactly the v1.7 behaviour — and
converts for real on a later load, once the probe has persisted dimensions.
That means 'native' stays a valid stored value, so this restores the
document-aware resolveAspectRatioValue and its two call sites. Nothing writes
'native' any more (the picker enumerates concrete shapes), so the union shrinks
by attrition instead of by a lossy rewrite.
Tests cover both halves of the real upgrade path: unprobed import keeps
'native', and a probed PORTRAIT source converts to 9:16 rather than 16:9.
Verified against the actual v1.7 file in the recordings folder.
EtienneLescot
added a commit
that referenced
this pull request
Jul 29, 2026
Removes the per-parse schema migration overhead from every documentSchema.parse(...) call. The pre-hoist schema was wrapped in a z.preprocess that ran upgradeV3DocumentToV4 + upgradeV4DocumentToV5 on every parse, including in-memory parses of v5 docs. - documentSchema is now a pure v5 validator; the z.preprocess wrapper is removed and both upgraders are exported - New migrateRawDocumentToCurrent helper in document/migrate.ts composes the two upgraders into the load-time equivalent of the old chain - DocumentService.getProject / listProjects wired to run the helper before parse - browserShim get callback and localStorage-load IIFE wired; new shim docs written as v5 directly - NewEditorShell.handleBrowseProject and EditorEmptyState.openLoadedProject (renderer disk-load paths) wired to run the helper before parse - migrateProjectDataToAxcutDocument updated to call the new helper so the v2 -> v3 -> v4 -> v5 chain still lives in one place - Existing schema tests updated to model the new contract - New tests for migrateRawDocumentToCurrent (v3/v4/v5/non-doc/v2) - Test fixtures (projectStore, useTimeline, EditorEmptyState) bumped to v5 to model the new bridge contract (load sites now return v5) - technical-documentation/architecture/document-model.md updated Every render-side setDocument / saveDocument / loadProject is now a single z.literal(5) + shape check on already-v5 data, instead of a function call into each upgrader + the parse. Note for #195: that PR adds a v5->v6 upgrader to the same z.preprocess chain this PR removes. Either land this PR first and rebase #195 on top, or combine them — the only conflict point is the z.preprocess line.
EtienneLescot
added a commit
that referenced
this pull request
Jul 29, 2026
Removes the per-parse schema migration overhead from every documentSchema.parse(...) call. The pre-hoist schema was wrapped in a z.preprocess that ran upgradeV3DocumentToV4 + upgradeV4DocumentToV5 on every parse, including in-memory parses of v5 docs. - documentSchema is now a pure v5 validator; the z.preprocess wrapper is removed and both upgraders are exported - New migrateRawDocumentToCurrent helper in document/migrate.ts composes the two upgraders into the load-time equivalent of the old chain - DocumentService.getProject / listProjects wired to run the helper before parse - browserShim get callback and localStorage-load IIFE wired; new shim docs written as v5 directly - NewEditorShell.handleBrowseProject and EditorEmptyState.openLoadedProject (renderer disk-load paths) wired to run the helper before parse - migrateProjectDataToAxcutDocument updated to call the new helper so the v2 -> v3 -> v4 -> v5 chain still lives in one place - Existing schema tests updated to model the new contract - New tests for migrateRawDocumentToCurrent (v3/v4/v5/non-doc/v2) - Test fixtures (projectStore, useTimeline, EditorEmptyState) bumped to v5 to model the new bridge contract (load sites now return v5) - technical-documentation/architecture/document-model.md updated Every render-side setDocument / saveDocument / loadProject is now a single z.literal(5) + shape check on already-v5 data, instead of a function call into each upgrader + the parse. Note for #195: that PR adds a v5->v6 upgrader to the same z.preprocess chain this PR removes. Either land this PR first and rebase #195 on top, or combine them — the only conflict point is the z.preprocess line.
EtienneLescot
added a commit
that referenced
this pull request
Jul 31, 2026
Removes the per-parse schema migration overhead from every documentSchema.parse(...) call. The pre-hoist schema was wrapped in a z.preprocess that ran upgradeV3DocumentToV4 + upgradeV4DocumentToV5 on every parse, including in-memory parses of v5 docs. - documentSchema is now a pure v5 validator; the z.preprocess wrapper is removed and both upgraders are exported - New migrateRawDocumentToCurrent helper in document/migrate.ts composes the two upgraders into the load-time equivalent of the old chain - DocumentService.getProject / listProjects wired to run the helper before parse - browserShim get callback and localStorage-load IIFE wired; new shim docs written as v5 directly - NewEditorShell.handleBrowseProject and EditorEmptyState.openLoadedProject (renderer disk-load paths) wired to run the helper before parse - migrateProjectDataToAxcutDocument updated to call the new helper so the v2 -> v3 -> v4 -> v5 chain still lives in one place - Existing schema tests updated to model the new contract - New tests for migrateRawDocumentToCurrent (v3/v4/v5/non-doc/v2) - Test fixtures (projectStore, useTimeline, EditorEmptyState) bumped to v5 to model the new bridge contract (load sites now return v5) - technical-documentation/architecture/document-model.md updated Every render-side setDocument / saveDocument / loadProject is now a single z.literal(5) + shape check on already-v5 data, instead of a function call into each upgrader + the parse. Note for #195: that PR adds a v5->v6 upgrader to the same z.preprocess chain this PR removes. Either land this PR first and rebase #195 on top, or combine them — the only conflict point is the z.preprocess line.
EtienneLescot
added a commit
that referenced
this pull request
Aug 1, 2026
Removes the per-parse schema migration overhead from every documentSchema.parse(...) call. The pre-hoist schema was wrapped in a z.preprocess that ran upgradeV3DocumentToV4 + upgradeV4DocumentToV5 on every parse, including in-memory parses of v5 docs. - documentSchema is now a pure v5 validator; the z.preprocess wrapper is removed and both upgraders are exported - New migrateRawDocumentToCurrent helper in document/migrate.ts composes the two upgraders into the load-time equivalent of the old chain - DocumentService.getProject / listProjects wired to run the helper before parse - browserShim get callback and localStorage-load IIFE wired; new shim docs written as v5 directly - NewEditorShell.handleBrowseProject and EditorEmptyState.openLoadedProject (renderer disk-load paths) wired to run the helper before parse - migrateProjectDataToAxcutDocument updated to call the new helper so the v2 -> v3 -> v4 -> v5 chain still lives in one place - Existing schema tests updated to model the new contract - New tests for migrateRawDocumentToCurrent (v3/v4/v5/non-doc/v2) - Test fixtures (projectStore, useTimeline, EditorEmptyState) bumped to v5 to model the new bridge contract (load sites now return v5) - technical-documentation/architecture/document-model.md updated Every render-side setDocument / saveDocument / loadProject is now a single z.literal(5) + shape check on already-v5 data, instead of a function call into each upgrader + the parse. Note for #195: that PR adds a v5->v6 upgrader to the same z.preprocess chain this PR removes. Either land this PR first and rebase #195 on top, or combine them — the only conflict point is the z.preprocess line.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Retires the legacy
"native"AspectRatio via a v5→v6 schema migration. The runtime bridge (the document-awareresolveAspectRatioValuearm) and the self-migrationuseMemoinV4Timelinewere the two mechanisms holding the value alive; this PR converts both into a one-time load-time rewrite so the union arm can drop and"native"never reaches any consumer again.What changed
upgradeV5DocumentToV6insrc/lib/ai-edition/schema/index.tswalks every storedlegacyEditor.aspectRatio === "native"and rewrites it to a concrete"W:H"token via the timeline's largest clip (same resolution the old runtime bridge used, so projects migrate losslessly). Falls back to"16:9"when the timeline has no clips with known dimensions. Wired into the existingz.preprocesschain alongsidev3→v4andv4→v5."native"removed fromAspectRatioinsrc/utils/aspectRatioUtils.ts; the per-helper branches ingetAspectRatioValue/getAspectRatioLabel/isAspectRatio/formatAspectRatioForCSSare gone.NATIVE_ASPECT_RATIO_FALLBACKrenamed toASPECT_RATIO_FALLBACK(the file-private fallback for any unparseable token).getNativeAspectRatioValueis kept.resolveAspectRatioValue(outputFormat.ts:232-241) and theactiveTokenuseMemoinV4Timeline.tsx:274-283are dead and removed.resolveAspectRatioValueis now a one-arg wrapper; the three call sites (outputFormat,PreviewCanvas,ExportDialog) are updated.schemaVersion === 5assertion bumped to 6 (schema, migrate, document-service), a 7-testv5 → v6 native AspectRatio migrationblock added toschema/index.test.ts, and the"native"assertions inaspectRatioUtilsandoutputFormattests are stripped (pointing at the v5→v6 upgrader tests as the new home).Pattern note
The v5→v6 upgrader follows the same
upgradeVxDocumentToVyshape as the existingupgradeV3DocumentToV4andupgradeV4DocumentToV5— preprocessor + schemaVersion gate +{ ...doc, schemaVersion: y, ...overrides }return. Future schema bumps have a template.Intentionally untouched
lastBackgroundColor(annotation style). The original audit recommended dropping it as dead code, butsrc/lib/ai-edition/annotations/background.ts:25-29reads it for the picker swatch when bg is off, andtoggleTextBackgroundwrites it on every toggle — it's the fix for the "background toggle always reverts to black" regression. Leave it.Type of change
Release impact
legacyEditor.aspectRatio === "native"now round-trips through a concrete"W:H"token. After this PR, no document carries"native"again.Desktop impact
Testing
npx tsc --noEmit— clean (exit 0)npm run test— 1147 / 1147 (baseline 1144 / 1144; +3 net new tests, all in the v5→v6 migration block)npm run lint— 0 errors, 7 pre-existing warnings (none in files touched by this PR)