Repository navigation
fix(review): serve cached frame-set images over the review server instead of file:// URLs - #257
Merged
Merged
Conversation
…tead of file:// URLs A PixelLab frame set reviewed in `pick` (or the gallery's Review sheet) showed nothing but broken images. The console logged one error per frame: Not allowed to load local resource: file:///var/folders/.../T/pixelkiln-pixflux/<job>-<n>.png pollAnimateRevision decodes inline frames into the provider's temp cache and reports them as file:// URLs, which is right for fetch: download reads them straight off disk. prepareReview copied those URLs into each group's frameUrls unchanged, though, and the sheet is served from http://127.0.0.1, which a browser will not let load a file:// image. The same was true of imagePro and uiElement candidates. prepareReview now gives every file:// frame or candidate its own exact route, `${routePrefix}/review-frame/<group>/<index>`, in the session's assets with a content type from its extension (or its bytes), the same way revision sources and current art are served. runPicker's review server and the gallery's /review/<job>/ handler both serve session.assets, so both sheets are covered. Hosted http(s) URLs pass through unchanged. apply keeps the provider's own URLs, so a review route never reaches the lockfile.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
🎉 This PR is included in version 0.86.3 🎉 The release is available on: Your semantic-release bot 📦🚀 |
This branch was successfully deployed
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.
Symptom
Reviewing a PixelLab frame set in
pixelkiln pickshowed a broken image for every frame. The browser console logged one error per frame:We hit this on an
animate-skeletonrevision (animate-with-skeleton-v3) in Super Disc Golf's throw pilot. Generation was fine:gendownloaded every frame and marked the entriesdownloaded. Only the review page was broken. The gallery's Review sheet goes through the same code, so it was broken the same way.Cause
PixelLabProvider.pollAnimateRevision(src/providers/pixellab.ts) decodes inline base64 frames intocacheDir()($TMPDIR/pixelkiln-pixflux). It reports them asfile://<cacheDir>/<jobId>-<index>.png. That is right forfetch, becausedownloadreadsfile://straight off disk.pollImageProandpollUiElementdo the same for their candidates.prepareReview(src/pick/server.ts) copiedstate.frameUrls/state.candidateUrlsinto each group'sframeUrlswithout changing them, and the sheet uses those as<img src>.http://127.0.0.1:<port>, and a browser won't let a page served over HTTP load afile://image. Revision sources and current art were already served through the session'sassets. Frames were not.Fix
prepareReviewnow gives eachfile://frame or candidate its own exact route,${routePrefix}/review-frame/<group>/<index>, and adds it tosession.assets. This is howrevision-sourceandcurrent-artalready work. The content type comes from the file extension, or from the PNG/GIF signature if the extension is missing.runPicker's review server and the gallery's/review/<job>/…handler both servesession.assets, so the CLI sheet and the gallery sheet (withroutePrefix) are both fixed.http(s)URLs, plus ComfyUI view URLs and Retro Diffusiondata:URLs, still reach the sheet unchanged.applykeeps the provider's own URLs for itssourceUrlfallback, so a review route never ends up in the lockfile.docs/CLI.md(pick) now says PixelLab animations and frame-set revisions appear as an animated strip, and that locally cached frames are served by the review server.docs/LIBRARY.mdsays a host embedding the sheet must serve every route insession.assets.No lockfile, manifest or provider-call changes, and no cost impact.
Tests
Both new tests fail on
mainand pass with the fix:test/pixellab-revision.test.ts, "serves the decoded frames to the pick sheet over HTTP, never as file:// URLs": the realPixelLabProviderwith a mockedanimate-with-skeleton-v3job runs submit, poll andrunPicker. The page contains nofile://, each/review-frame/0/<i>returns that frame's PNG bytes asimage/png, an out-of-range index is a 404, and the accepted lock entry still records the provider'sfile://sources.test/pick.test.ts, "serves a candidate the provider cached on disk on its own route, never as a file:// URL": a candidates review with onefile://and onehttps://candidate. WithroutePrefix: "/review/abc"the routes are prefixed. The hosted URL is unchanged and gets no route. The standalone server serves the cached bytes.applyrecords thefile://URL rather than the route.I also checked it in a real browser by opening a
file://frame-set sheet in headless Chrome. Onmainthere is oneNot allowed to load local resourceerror per frame. With the fix, every<img>loads (naturalWidth > 0) and that error is gone. The only console error left is a 404 for/favicon.ico, which the review server has always returned.Checks run
Local, Node v24.20.0, npm 11.19.0:
npm run test:releasenpm run typechecknpm run test:docsnpm run test:securitynpm testnpm run test:qualitynpm run buildnpm run test:packagenpm run test:gallery(Chrome)npm run test:godot(Godot 4.7.2)npm run website:check/website:lint/website:buildThe repo has no separate lint step beyond
typecheckfor the package.Three
npm testfailures, none in files this PR touches:artifacts"writes a verifiable companion through the no-manifest CLI",hand-edit"is reachable from the CLI…", andworkspace"list --jsonandstatus --jsonagree…". Each is a 5000 ms timeout on a test that spawns the CLI. The machine was at a load average of about 300 on 10 cores while they ran. An unmodifiedmainrun under the same load failed 5 tests the same way: these three plusquality-profileandrevision. Re-running all five files on this branch with--testTimeout=60000passes 69/69. CI on this PR is the real signal for them.