Repository navigation
fix(studio): invalidate the preview signature off the watcher that sees project writes - #3364
Conversation
…es project writes The preview ETag is a hash of the project's files, memoised per project directory. That cache was cleared from Vite's own watcher, which `server.watch.ignored` deliberately excludes `data/projects/**` from, so nothing ever cleared it: the ETag stayed frozen for the life of the dev server, the preview answered every revalidation with 304, and the browser went on serving the composition as it was when it first loaded. The visible cost is thumbnails. Their disk cache key already content-hashes the composition, so an edit correctly asks for a fresh capture, but the capture is taken against the stale page, and a clip's filmstrip keeps showing frames of a layout that no longer exists until the dev server is restarted. Studio already runs its own chokidar watcher over exactly these directories, because Vite's would answer a composition edit with a full page reload. That watcher now owns the invalidation, and the cache asks it to follow any project directory it has not seen. All five event types count: an added or deleted asset changes the signature as surely as an edited one. The cache moves behind `createProjectSignatureCache` so the invalidation rule is a unit under test rather than a subscription buried in the adapter.
jrusso1020
left a comment
jrusso1020
left a comment
There was a problem hiding this comment.
Reviewed at b755d97 — all three files end to end, plus the signature / preview / thumbnail path they feed. The diagnosis holds up: ignored: ["**/data/projects/**"] (vite.config.ts:258) really does mean the old server.watcher.on("all") subscription could never fire for a project file, and preview.ts:356 bakes the signature straight into the ETag, so a frozen memo is a permanent 304.
Strengths
- This lands on the pattern the CLI server already proved rather than inventing a third mechanism:
studioServer.ts:356+:362create a project watcher and clearcachedProjectSignaturefrom its listener. Converging the vite adapter on the shipped path's design is the right call. vite.adapter.ts:72replaces the oldif (cached)truthiness test withcached !== undefined. No live difference (a 24-char hex digest is never falsy), but correct by construction instead of by luck.watchedis kept separate fromsignatures(:74), so an invalidation can't trigger a secondadd()for the same dir — andvite.adapter.signature.test.ts:73pins exactly that. Stacking duplicate chokidar registrations is the obvious way to get this wrong.- The move of the watcher block above the middleware registrations keeps
server.httpServer?.on("close", ...)(vite.config.ts:223), so dynamically added dirs don't outlive the server.
important — the invalidation is unfiltered, so the thumbnail cache evicts the memo it depends on
vite.config.ts:101-102 invalidates on all five events for anything under a project dir, with no path filter. But the signature's own walk skips 14 directories (projectSignature.ts:18-32), and .thumbnails is one of them — writes there cannot change the signature.
.thumbnails is exactly where the thumbnail route keeps its disk cache: thumbnail.ts:153 is join(project.dir, ".thumbnails"), each capture is written there, and pruneThumbnailCache deletes from it (so unlink fires too). Each capture also loads /api/projects/:id/preview (thumbnail.ts:147), which is a signature read.
The interleave: capture → preview read populates the memo → capture written into .thumbnails/ → add → memo dropped → next capture's preview read recomputes. Populating a timeline row is dozens of captures, so the memo gets discarded on roughly every request under the one workload this PR is about ("Timeline thumbnails are the visible casualty"). /api/projects/:id/signature, documented as "a cheap poll target" that the storyboard polls (projects.ts:43-46), stops being cheap the same way.
Being fair about the cost: it is bounded. createProjectSignature keeps its own module-level cache keyed on a stat-only fingerprint (projectSignature.ts:52, :175), so a recompute after a .thumbnails write re-walks and re-stats the project but does not re-hash file contents. So this is wasted work on a hot path, not a correctness bug, and nothing regresses against today's behaviour where the memo never invalidates at all. That's why it's important and not a blocker.
The filter belongs where the exclusion set already lives, not in the plugin. One trap if you reach for the existing predicate: shouldWatchProjectFile (fileWatcher.ts:29) uses WATCHER_EXCLUDED_DIRS, which I diffed against SIGNATURE_EXCLUDED_DIRS — the two sets are character-identical, 14 entries each. But it excludes .hyperframes, and the signature deliberately re-adds two files from inside it (projectSignature.ts:35-37, via collectProjectSignatureManifestFiles at :104). Filtering with it verbatim would stop studio-motion.json / studio-manual-edits.json edits from ever invalidating — a fresh instance of this same bug. What you want is that exclusion set minus those two paths, exported from projectSignature.ts so both watchers share one predicate (which also retires the duplication your "Not covered here" note defers).
nit — the third parameter defaults to a cache that can never invalidate
vite.adapter.ts:92 defaults signatureCache to createProjectSignatureCache() — no watch, nothing subscribed to invalidate. vite.config.ts:120 is the only call site today and it passes the wired one, so nothing is broken. But createViteAdapter(dataDir, server) still typechecks, and what it produces is precisely the pre-PR bug: a memo nothing clears. Dropping the default puts that on the compiler instead of on a reviewer.
nit — the wiring is the part that broke, and it's the part with no test
All six new cases inject compute, so they cover the cache object's logic and none of the wiring. Re-pointing vite.config.ts:98-102 back at server.watcher tomorrow leaves all six green.
Related: "recomputes for an asset added or removed" (vite.adapter.signature.test.ts:41) doesn't distinguish add from remove — invalidate takes no event type, so both assertions are the same path-containment check. The add/remove claim actually lives in the five-event list at vite.config.ts:101, which is untested. Nothing imports vite.config in a test today, so I'm not asking for a plugin harness — the predicate from the important above is the natural unit: shouldInvalidateSignature(".thumbnails/x.jpg") === false, shouldInvalidateSignature(".hyperframes/studio-motion.json") === true.
nit — the docstring oversells the cost
vite.adapter.ts:43 says the signature "hashes every file in the project." The content hash is already gated behind the stat fingerprint at projectSignature.ts:175, so what this memo actually saves is the directory walk. Worth being accurate, since it's the sentence a future reader will use to decide how aggressive invalidation is allowed to be.
Checked, not findings
- Symlink spelling.
realProjectPathswatchesrealpathSync(...)while cache keys areresolve(projectDir), which does not resolve symlinks — that looked like a mismatch that would silently skip invalidation for symlinked projects, the case your comment calls out. It isn't one:resolveProjectreturnsdir: realpathSync(projectDir)(vite.adapter.ts:221, also:210), and every signature consumer goes through it —preview.ts:405/:429/:518,projects.ts:49,storyboard.ts:51. Both sides are real paths. (listProjectsat:188does hand back the un-realpath'd spelling, but nothing routes it into the cache.) - PR-body claims.
ignored: ["**/data/projects/**"]present as described;isPathWithinhas no consumer outside the module (only:83,:413, plus studio-server's separate private copy); six test cases as stated. Incidentally the twoisPathWithincopies are not identical — the adapter's resolves both sides,projectSignature.ts:54doesn't — worth knowing whichever one survives the dedup. - Watcher lifetime / block move. Same options, same paths, still closed on server close, nothing ordering-sensitive between it and the middleware registrations.
- I did not rerun the studio suite locally. All 8 required contexts are green at
b755d97,regressionaggregate included, soBLOCKEDhere is the review gate alone.
Adjacent, pre-existing — the same stale ETag is still live in the CLI studio server
Not this PR's scope, but it's the mirror of what you just fixed and you're already in the area. studioServer.ts:362 clears cachedProjectSignature from the CLI watcher, but that watcher filters through shouldWatchProjectFile, which drops .hyperframes/**. Executing the predicate: .hyperframes/studio-motion.json → false, .hyperframes/studio-manual-edits.json → false. Both files are in the signature, and Studio writes the motion one at runtime (usePreviewPersistence.ts:92). So under hyperframes studio, saving motion state leaves the preview ETag stale until restart — same failure mode, shipped path. The shared-predicate fix above closes both at once.
Verdict: COMMENT
Reasoning: The fix is correctly diagnosed and well-targeted, CI is fully green, and I found no blocker — the one important is wasted work inside the new invalidation rather than a regression. Not stamping because the ask was a review and not an approval; happy to convert once the filter question is settled either way.
— Rames Jusso
…issing motion saves Review follow-up on the unfiltered invalidation. The watcher fired on everything under a project dir, but the signature walk skips 14 directories and `.thumbnails` is one of them. That directory is where the thumbnail route keeps its disk cache, and every capture also reads the preview, so populating a timeline row discarded the memo on roughly every request of the one workload it exists for. The filter is a single exported predicate beside the exclusion set it reads, and it is applied inside `invalidate` rather than at the watcher, so no caller can subscribe and forget it. It is deliberately not `WATCHER_EXCLUDED_DIRS`: that set is character-identical but drops all of `.hyperframes/`, and the signature reads two manifest files back out of there. Which is the same bug, still live, in the CLI server: its watcher filters through `shouldWatchProjectFile`, so `.hyperframes/studio-motion.json` never reached the listener that clears the cached signature. Studio writes that file at runtime, so saving motion state left the preview ETag stale until restart. The watcher now admits signature-relevant paths and the reload listener re-applies its own filter, so what triggers a browser reload is unchanged. Also from review: drop the `createViteAdapter` signature-cache default, which produced exactly the memo-nothing-clears bug this PR fixes, and correct the docstring — the content hash is already gated behind a stat fingerprint, so what the memo saves is the walk.
|
Thanks — the important — unfiltered invalidation. Fixed with Took the trap you flagged: it is not Adjacent CLI bug — fixed here too. I executed the predicate and it lands as you described: The fix had a trap of its own. Widening the watcher wholesale would have made live-reload fire on every motion save, since Studio writes that file at runtime — a browser reload mid-edit. So the ingest gate is now the union of the two predicates, which admits exactly today's set plus those two files, and the reload listener re-applies nit — default cache that can never invalidate. Dropped. nit — docstring oversells the cost. Corrected to say it saves the walk, with the stat-fingerprint gate called out for the next reader. nit — the wiring has no test. Took your suggestion rather than a plugin harness: the predicate is the unit, and it's pinned in a new Suites, each with its own package config: studio 4300 passed, cli 2780 passed, studio-server 452 passed. Not doing here: the two |
What
The dev server's preview ETag never changed after the first request, so the browser kept serving the composition as it was when it first loaded. Timeline thumbnails are the visible casualty: after an edit they keep rendering the old layout until the dev server is restarted.
Why
server.watch.ignoreddeliberately excludesdata/projects/**from Vite's watcher, because Vite answers a composition edit with a full page reload and Studio wants to decide that itself. The preview signature cache was subscribed to that same watcher, so nothing ever cleared it.The thumbnail route already content-hashes the composition into its disk cache key, so an edit correctly asks for a fresh capture. The capture is then taken against a stale page, which is why the failure reads as "this clip is showing the wrong frames" rather than "this thumbnail is old".
Reproduction, against a dev server that has served the preview at least once:
after.jpguses atthat was never generated before, so nothing is served from the disk cache, and it still shows the pre-edit composition.How
Studio already runs its own chokidar watcher over exactly these directories. That watcher now owns the invalidation, and the cache asks it to follow any project directory it has not seen yet (project dirs are often symlinks from elsewhere on disk, so they are not covered by the initial scan). All five event types invalidate: an added or deleted asset changes the signature as surely as an edited one.
The cache moves behind
createProjectSignatureCacheso the invalidation rule is a unit under test rather than a subscription buried in the adapter.isPathWithinloses itsexportin the same file, since nothing outside the module consumed it.Not covered here:
packages/studio-server/src/helpers/projectSignature.tscarries its own private copy ofisPathWithin. Deduplicating across the two packages is worth doing but is not this fix.Test plan
vite.adapter.signature.test.ts, 6 cases covering memoisation, invalidation for an edited / added / removed file, isolation between projects, watch registration happening once, and path normalisation.t. Before: blue. After: magenta, and blue again once reverted. The preview ETag now changes on edit and returns to its previous value on revert, which is what a content-addressed signature should do.