refactor: compose the video registry upgraders from one ordered list (#6056) - #6134
Merged
Conversation
…6056) `normalizeRegistry`'s `videoEntries` hand-nested six load-time registry upgraders four parens deep, plus a conditional pair for the CUDA bucket. Every addition since has cost a nesting level, and the ordering — which is load-bearing, since `upgradeFastMetalDownloadSizes` has to run ahead of `applyVideoDisclosures` (that decorator only fills an ABSENT disclosure) and `dropRetiredEntries` has to run first so a withdrawn model isn't decorated on its way out — read as arbitrary nesting. Replace the nest with an ordered `VIDEO_REGISTRY_UPGRADES` list reduced in one place. Each row names its upgrader and states its ordering constraint beside it; a `buckets` field replaces the `upgradeLegacyCudaLtx` flag for the two CUDA-only rows. Adding the seventh upgrader is now appending a row. Behavior-preserving: every upgrader keeps its own shipped-value guard, the chain applies in exactly the prior order, and the exported upgraders the registry migrations call are unchanged. Adds two guards: the chain order is pinned by name, so a reorder becomes a deliberate edit rather than an invisible re-nesting, and the CUDA-only rows are asserted not to reach the MLX bucket. Claude-Session: https://claude.ai/code/session_01RA3pD5YM2dukQwbZ3pC6WA
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
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
normalizeRegistry'svideoEntriescomposed six load-time registry upgraders by hand — four parens deep, plus a conditionally-applied pair for the CUDA bucket driven by anupgradeLegacyCudaLtxflag. Every upgrader added since has cost a nesting level, and the ordering (which is load-bearing) read as arbitrary nesting.This replaces the nest with an ordered
VIDEO_REGISTRY_UPGRADESlist reduced in one place:upgradeFastMetalDownloadSizesmust precedeapplyVideoDisclosures(that decorator only fills an absent disclosure, so a stale persistedestimatedDownloadGbhas to be corrected before it), anddropRetiredEntriesmust run first so a withdrawn model is not handed a disclosure / Finish edge / speed profile / draft decoder on its way out.bucketsfield on a row replaces theupgradeLegacyCudaLtxboolean for the two CUDA-only upgraders.Behavior preservation
This is a pure composition change — no upgrader's rules were touched:
dropRetiredEntriesRETIRED_VIDEO_MODELSandrepo === shippedRepoupgradeMiniMaxH3OutputControls(+ nested…DenoisingCount)upgradeLtx25AudioControlsupgradeFastMetalDownloadSizesoldName/oldEstimatedDownloadGbexact matchbackfillRuntimeLEGACY_MLX_VIDEO_IDS, only whenruntimeunsetupgradeLegacyCudaLtxRuntimeltx_video, norepo, runtime unset/mlx_videobucket === 'cuda'buckets: ['cuda']upgradeLtx25CudaMemoryFloorminMemoryGb === 32bucket === 'cuda'buckets: ['cuda']The upgraders the registry migrations import (
upgradeMiniMaxH3OutputControls,upgradeMiniMaxH3DenoisingCount,upgradeFastMetalDownloadSizes,upgradeLtx25CudaMemoryFloor) keep their exports and signatures, so migrations 267 / 271 / 336 are unaffected — no compatibility or upgrade path is removed.Scope note
The issue also floated folding the pure
(id, shippedRepo, oldValue) -> newValueupgraders into "data plus one shared applier". Not done, deliberately: FastMetal is already data + applier (FASTMETAL_DOWNLOAD_SIZE_PROFILES+upgradeFastMetalEntry), while the LTX-2.5 audio backfill guards onrevisionas well and does absent-key filling rather than shipped-value replacement. One applier covering both would need an optional revision matcher, a nested-object rewrite op, and an absent-key-fill op for two callers — a speculative abstraction over the exact guard logic the issue says to leave alone. Left as-is.Test plan
server/lib/mediaModels.test.js:VIDEO_REGISTRY_UPGRADE_NAMESexport, so a reorder is a deliberate two-file edit rather than a silently-shipped behavior change;expected 'cuda_video' to be undefined— with thebucketsgate removed).server/lib/mediaModels.test.js+server/lib/index.test.js: 87 passed.routes/settings.secretsStrip.test.js,services/voice/facetimeBridge.test.js— the latter compiles a Swift helper); both pass in isolation and neither touches this code.Closes #6056
https://claude.ai/code/session_01RA3pD5YM2dukQwbZ3pC6WA