fix(studio): code-review and live-test fixes for the variables stack

[jrusso1020]

What

Top of the template-variables stack: fixes from an 8-angle code review + a full in-browser test pass over #2046–#2051. Hunks touching stack-authored lines were absorbed into their PRs (gt absorb); this PR carries the fixes that touch pre-stack code.

• studio: the SDK session never opened on the master view (activeCompPath is null there), so the Variables/Slideshow panels were dead on single-file projects — the primary use case. useStudioSdkSessions opens the session with an index.html fallback while edit-flow consumers keep the legacy null gating (cutover behavior unchanged). Found by driving the panel in a real browser
• studio: "variables" registered in the URL-state tab allowlist so ?tab=variables deep links survive normalization; preview variable overrides are cleared when the composition or project changes so values can't leak into another composition's preview/render
• studio: usePreviewDocumentVersion extracted from App.tsx (file-size gate)
• sdk: handleSetVariableValue's CSS compat sync routed through cssCompatChange — one implementation of the --{id} channel; object values now clear a stale scalar prop instead of leaving it behind
• sdk: content-keyed cache for getVariableUsage script scans
• studio-server: shared variables-payload validation helper for the preview + render routes

How it was tested

The whole stack was exercised end-to-end against a live studio session (local agent-browser driving a real project): every control type committing to the preview, override pill/reset semantics, Set-default → disk → undo round-trip (verbatim file restore incl. CSS compat prop), add/remove declarations → disk, ?tab=variables deep links, HTTP injection/400s/ETag variance, and wire-level render forwarding. Review verdicts: 13 confirmed findings (9 fixed across the stack, others tracked), 4 candidates refuted by verification.

Test plan

• Full suites green after fixes: sdk 435, studio 1392, studio-server 259
• Manual browser test matrix (above) re-run against the fixed build
• All pre-commit gates pass on every branch

🤖 Generated with Claude Code

[jrusso1020]

• feat(studio): fold variables into the design panel + summonable manager #2101
• feat(core): aggregate template sub-comp variable defaults from the root div on the render path #2084
• feat(sdk,studio): editable template sub-compositions + promote sub-comp element properties #2081
• feat(studio): promote to variable from the design panel #2071
• feat(studio): bind selected element properties to variables #2055
• feat(core): declarative variable bindings — data-var-src, data-var-text, css custom props #2054
• fix(studio): code-review and live-test fixes for the variables stack #2052 👈 (View in Graphite)
• feat(studio): render with preview variables + template handoff + docs #2051
• feat(studio): variables inspector panel with live preview values #2050
• feat(studio-server): preview variable injection + render variables forwarding #2049
• feat(sdk): variable usage scan + preview-values adapter seam #2048
• feat(sdk): variable declaration edit ops (declare/update/remove) #2047
• feat(sdk): variable declaration read APIs + browser-safe variables entry #2046
• main

This stack of pull requests is managed by Graphite. Learn more about stacking.

[miga-heygen]

LGTM — all fixes address real bugs with correct root causes.

The varDecl before var ordering fix in applyOverrideSet is the most important: without it, snapshot-vs-value ordering was non-deterministic. The cssCompatChange routing in handleSetVariableValue is a clean SSOT consolidation (two paths that each had to know about CSS compat props become one call, and the stale-scalar-on-object-write bug is fixed as a consequence). The useStudioSdkSessions extraction correctly gives the master view a live SDK session while preserving edit-flow gating, and the preview-variable clearing on composition/project change prevents cross-composition value leaks.

Clean work.

— Miga

[miguel-heygen]

Stamped after Miga's stack review. GitHub state checked: mergeable, no red checks visible; #2046 base conflict still needs resolution separately.

[james-russo-rames-d-jusso]

Reviewed at 3976c7e. Stack context: studio-UI lane (#2050/#2051/#2052/#2055/#2071) — 5-of-12 in template-variables stack.

Layering on Miga's review — Miga covered the applyOverrideSet sort, cssCompatChange consolidation, and useStudioSdkSessions master-view fix. Two follow-ups on the fallback.

🟠 Hold

• useStudioSdkSessions(projectId, activeCompPath ?? "index.html", ...) — behavior when the project has no index.html? The body's justification is "single-file projects — the primary use case," which implies index.html is a naming convention. But packages/parsers/src/composition.ts and the compositions test fixtures across the repo don't hard-require the filename — templates can use card.html, main.html, hero.html, etc. Two questions this raises:

  • What does useSdkSession do when passed a path that doesn't exist? Silent-return-null (Variables panel just shows "Open a composition"), throw an error (studio crash), or open on an empty document (worse — user edits, save writes to a phantom index.html)?
  • The editFlowSdkSession = activeCompPath ? sdkHandle.session : null gating keeps the pre-#2052 behavior for edit consumers — good — but the Variables panel and the "Handoff footer" in #2051 both bypass that gate. On a card.html-only project, the panel is now writing to index.html, and #2051's copy-command uses activeCompPath ?? "index.html" — the user copies a command referencing a file that doesn't exist.

  Suggested resolution: fall back to "first .html in the project root at the sort-order the composition browser uses" rather than the hardcoded "index.html", OR keep the panel's session-null branch ("Open a composition to manage its variables.") active whenever activeCompPath is null AND index.html isn't confirmed on disk. The current code makes the master view work for the 80% case but silently mis-targets the 20%.

🟡 Nits

• studioUrlState.ts's VALID_TABS — this PR adds "variables" to the allowlist, but "slideshow" remains missing from the list even though it's a valid RightPanelTab. Pre-existing gap, not a regression of this PR, but since the file is being touched anyway it'd be a two-character win to add it. (If deliberately excluded because slideshow deep links break something else, worth a code comment.)

• applyOverrideSet's new sort — the comparator only orders varDecl.* before var.*. Other keys (style.*, attr.*) go through with return-0, so their relative order is stable-insertion. The sort is stable in V8/JSC (ECMA-262 spec compliance since 2018), so this is safe today; if you ever run this in an older JS engine (jsdom pre-2018?), an unstable sort could permute non-var keys. Not blocking — noting for future-proofing.

• cssCompatChange route now handles the object-value-clears-stale-scalar case implicitly. Nice consolidation. The one edge case to double-check: if a variable's default was previously scalar-set (writes --{id} to the root style), then a subsequent setVariableValue(id, {name: ...}) (object) is issued — the compat channel clears via String(null)... actually looking again, the cssCompatChange is passed isObjectVariableValue(value) ? null : String(value), so null triggers the clear path. Good. Worth a one-line test that exercises the scalar-to-object transition and asserts the CSS prop is gone on the root after.

🟢 Good

• useStudioSdkSessions cleanly splits "session everyone can read" from "session only edit consumers write via" — the cutover is a small, well-labeled change to the App.tsx call sites (sdkHandle.session → editFlowSdkSession at 3 spots). Grep-friendly rename.
• The preview-variables clear on [projectId, activeCompPath] change is exactly the right leak-prevention primitive. Values are per-composition; clearing on either identity change kills the two failure modes (project switch, composition switch within a project) in one hook.
• _collectCssText extraction is a nice cache-friendly refactor — the content-keyed cache in #2050's diff mentioned by the body would benefit from the extraction.
• The varDecl before var ordering fix is the highest-value change here — I'd expect Vance to write a comment on the order guarantee at the call site of applyOverrideSet if there isn't one already, so future patch authors don't accidentally revert this via a "clean up the sort" refactor.

— Review by Rames D Jusso

[vanceingalls]

R1 — hyperframes #2052 at 3976c7ea

🟢 LGTM on the fix-set correctness, concurring with Miga; concur with Rames-D's 🟠 Hold on the hardcoded "index.html" fallback — real gap for non-conventional filenames.

Verified independently, seven discrete fixes:

1. SDK applyOverrideSet patch ordering (apply-patches.ts) — sort varDecl.{id} before var.{id}. Real bug (snapshot could clobber fresher value), correct fix.
2. SDK cssCompatChange unification (mutate.ts) — shared helper wired through declare/update/remove. CI validates the definition exists.
3. SDK _collectCssText extraction — CSS-var usage scan dedup. Refactor.
4. Studio useStudioSdkSessions fallback — opens session with activeCompPath ?? "index.html" for schema panels. editFlowSdkSession = activeCompPath ? sdkHandle.session : null preserves legacy null gating on edit-flow. (Concurring hold below.)
5. Studio usePreviewDocumentVersion extraction — refresh + pending-timer collapse pulled from App.tsx. Refactor.
6. Studio preview cleanup on project/comp change — setValues(null) on projectId/activeCompPath change. Real bug (override leak between comps), closes the invariant I flagged pending on #2050.
7. Studio ?tab=variables deep-link allowlist — was being stripped by normalization. Real bug, one-liner.

CI green across SDK 435 / Studio 1392 / Studio-Server 259.

Concurring with Miga's LGTM on the fix-set correctness.

Concurring with Rames-D's 🟠 Hold:

• Hardcoded "index.html" fallback breaks on projects with a different entry filename. Rames-D's read is right: packages/parsers/src/composition.ts and the compositions test fixtures don't require the index.html filename — templates can use card.html, main.html, hero.html. Two failure modes:
  1. If useSdkSession silent-return-nulls on a nonexistent path: Variables panel shows "Open a composition" perpetually on a valid card.html-only project.
  2. If it opens on an empty document: the panel writes declarations to a phantom index.html, silently corrupting the project on save.
• The editFlowSdkSession = activeCompPath ? sdkHandle.session : null gating keeps the pre-#2052 behavior for edit consumers, but the Variables panel + #2051's Handoff footer both bypass that gate. On a card.html-only project, #2051's copy-command references a file that doesn't exist.

Concur on Rames-D's suggested resolution: fall back to "first .html in the project root at the sort-order the composition browser uses" rather than a hardcoded literal, OR keep the panel's session-null branch active whenever activeCompPath is null AND index.html isn't confirmed on disk. Current code works for the conventional 80%; the 20% silently mis-targets.

No decorative-gate patterns in the remaining six fixes — read + populate paths trace cleanly.

R1 by Via

[jrusso1020]

Addressed R1 🟠 Hold (hardcoded index.html fallback). New resolveMasterCompositionPath(fileTree) — prefers index.html, else the first .html in composition-browser order, else null. It now drives the master-view SDK session, the persist write target (so an edit can never land in a phantom index.html), and #2051's handoff command. useSdkSession returns null for a genuinely comp-less project, leaving the panel inert rather than corrupting. Also added "slideshow" to VALID_TABS (nit). Tests cover all three resolver branches + the tab allowlist.

[james-russo-rames-d-jusso]

R2 at 9d2b297.

Prior R1 🟠 (useStudioSdkSessions(projectId, activeCompPath ?? "index.html", …) silently mistargets template projects that use card.html / main.html / etc.) resolved at useStudioSdkSessions.ts:17-31: hook now takes a masterCompPath parameter resolved via resolveMasterCompositionPath(fileTree) (first .html in the tree), and coalesces activeCompPath ?? masterCompPath — no more hardcoded "index.html". Docstring at line 23-26 explicitly calls out "the first .html in the tree, not a hardcoded 'index.html' that may not exist." Projects with no composition file yet correctly leave the session null (session and panels empty).

No residuals from my side. CI green.

— Review by Rames D Jusso

[vanceingalls]

🟢 LGTM concurring with Rames-D — R2 at 9d2b297.

R1 🟠 hold (hardcoded "index.html" fallback in useStudioSdkSessions — card.html / main.html-only projects show empty Variables panel or write declarations to a phantom file) resolved via Rames-D's proposed option (a): first .html in project root at composition-browser sort order, with belt-and-braces session-null gating when the project has zero .html files.

Verified at head:

• packages/studio/src/hooks/useStudioSdkSessions.ts:17-31 — hook now accepts masterCompPath: string | null and uses useSdkSession(projectId, activeCompPath ?? masterCompPath, ...). Hardcoded literal is gone.
• Docstring at lines 23-26: "the first .html in the tree, not a hardcoded 'index.html' that may not exist. null when the project has no composition yet, which correctly leaves the session (and the panels) empty."
• packages/studio/src/App.tsx:139-148 — supplies masterCompPath via resolveMasterCompositionPath(fileManager.fileTree), memoized on file-tree changes.
• packages/studio/src/utils/studioUrlState.ts — new resolveMasterCompositionPath helper: prefers index.html when present, falls back to first .html at composition-browser sort order.
• studioUrlState.test.ts new describe("resolveMasterCompositionPath") block covers the sort-order preference.

No residuals from my side.

R2 by Via

[vanceingalls]

🟢 LGTM at R3 — 3878270.

R2 hold preserved through the rebase-onto-#2098. R2 verdict stands.

Verified at head:

• packages/studio/src/hooks/useStudioSdkSessions.ts:20 — masterCompPath: string | null, 4th prop preserved.
• :30 — useSdkSession(projectId, activeCompPath ?? masterCompPath, ...) fallback wire preserved.
• JSDoc at :4-14 documenting master-view + preview-clearing semantics intact.

resolveMasterCompositionPath(fileTree) caller-side signature also intact.

R3 by Via

The base branch was changed.