Add GET /api/video-gen/history/:id so a video id resolves without downloading the whole history - #4268
Merged
Conversation
…nstead of downloading the whole list (#4165) A history entry's id is not its filename stem: videoGen/local.js names a clip <jobId>.mp4, but the timeline renderer mints timeline-<project>-<ts>.mp4 beside an independent randomUUID() id. So a Creative Director finalVideoId can only be resolved through the stored filename — and with no by-id endpoint, useVideoFileSrc pulled the ENTIRE render history to find one row, on four surfaces (CD cards, CD Overview, EpisodeVideoStage, MusicVideo). Adds getHistoryItem() as the single-entry read in videoGen/history.js and the route on top of it, 404ing cleanly for an unknown id. The id is validated as a length-capped string rather than with the UUID historyIdSchema: entries also arrive with a caller-supplied download id and from federated peers, so a .guid() gate would 400 rows that are legitimately in the list. Nothing is interpolated into a path. ScenePreview keeps its <jobId>.mp4 reconstruction on purpose. Its remaining reconstructing caller is SegmentsTab, whose per-scene clip renders ARE named after their job; resolving internally would turn one render into N by-id requests per treatment and buy nothing for those ids.
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
Gives video id → filename resolution a real home:
GET /api/video-gen/history/:id.A history entry's id is not its filename stem.
videoGen/local.jsnames a clip<jobId>.mp4(so path reconstruction happens to work for per-scene renders), but the video-timeline renderer — a Creative Director project's stitched final cut — mintstimeline-<project>-<ts>.mp4beside an independentrandomUUID()id. A CDfinalVideoIdis therefore a history id, and/data/videos/<id>.mp4404s for it. The thumbnail is keyed the other way (generateThumbnailalways writes<jobId>.jpg), which is what made the mismatch easy to miss: the poster renders perfectly while the mp4 behind it does not exist.With no by-id route,
useVideoFileSrcresolved that by downloading the entire render history and scanning it client-side — on four surfaces (CD list cards, CD Overview,EpisodeVideoStage, MusicVideo).getHistoryItem(id)(server/services/videoGen/history.js) — the single-entry read, next to the existingloadHistory/saveHistory/mutateVideoHistoryprimitives. Returnsnull(not a throw) for an absent id so the route owns the 404, while the strictloadHistoryread still throws on an unreadable history file rather than reporting a bogus "no such entry".GET /api/video-gen/history/:id— returns the entry,404 NOT_FOUNDfor an unknown id.getVideoHistoryItem(id, options)(client/src/services/apiImageVideo.js) — the wrapper, anduseVideoFileSrcnow calls it instead oflistVideoHistory. The hook's public shape ({ src, resolving, retry }), its synchronousresolving, itsenabledlaziness, and its silent-failure contract are all unchanged.Two decisions the issue left open, recorded in full at #4165:
/data/video-thumbnails/<jobId>.jpg) wouldn't follow.historyIdSchema. Ids this install mints arerandomUUID(), but history entries also arrive with a caller-supplied download id (videoDownload.js:const jobId = id || randomUUID()) and from federated peers, so a.guid()gate would 400 rows that are legitimately in the list. Nothing is interpolated into a filesystem path — the value is only compared against stored ids.ScenePreviewkeeps its<jobId>.mp4reconstruction — deliberately. The issue notes the endpoint "would also letScenePreviewstop reconstructing paths at all". Its one remaining reconstructing caller isSegmentsTab, which renders aScenePreviewper treatment scene; those are per-scene clip renders where<jobId>.mp4is correct by construction. Resolving internally would turn one render into N by-id requests on mount for a 10-scene treatment, buy nothing for those ids, and change autoplay timing inside a component shared by four surfaces. The reconstruction stays the documented fallback for callers that pass nosrc; every surface where the id genuinely is not the filename stem already passes a resolvedsrc.Test plan
cd server && NODE_ENV=test npx vitest run routes/videoGen services/videoGen— 586 passed. New coverage:server/services/videoGen/history.test.js—getHistoryItemresolves the timeline case (filename unrelated to id), returnsnull(not a throw, notundefined) for an absent id and an empty history, matches ids exactly rather than by filename stem, and survives a malformed row sitting ahead of the target.server/routes/videoGen.test.js— the route returns the one entry and callsgetHistoryItemwith exactly the requested id while never touchingloadHistory; 404s an unknown id; decodes a percent-encoded id before the lookup; 400s an over-length id without reaching the service.cd client && npx vitest run src/pages src/hooks src/components/pipeline src/services src/components/creative-director— 2226 passed.useVideoFileSrc.test.jsnow drives a by-id mock that rejects with a 404-shaped error for an unknown id (matching whatrequest()really does), and adds a guard that the hook asks for exactly one id and never the whole list.cd client && npx biome lint --error-on-warningson the touched files — clean.Closes #4165