feat(sdk): variable declaration read APIs + browser-safe variables entry

[jrusso1020]

What

First PR of the template-variables Studio stack (#2046 → #2052): the SDK read surface for composition variables, plus a new browser-safe @hyperframes/core/variables entry point.

• comp.getVariableDeclarations() — the typed schema from data-composition-variables (same strict filter the render pipeline uses)
• comp.getVariableValues(overrides?) — resolves values exactly like the runtime's getVariables() (loose defaults merged under overrides, undeclared keys pass through), so callers can predict what a composition script will read for a given --variables payload
• comp.validateVariableValues(values) — same checks as --strict-variables (undeclared / type-mismatch / enum-out-of-range)
• @hyperframes/core/variables: one browser-safe import for the whole variables surface (schema types, parseCompositionVariables, getVariables/readDeclaredDefaults, validateVariables)
• parsers: parseCompositionVariables moved out of the linkedom-heavy htmlParser.ts into a browser-safe module (re-exported from the root and ./composition entries)

Why

The variables backend (runtime merge, CLI/Lambda injection, validation) is complete, but Studio and embedders had no API to inspect a composition's variable contract. This PR is the foundation the rest of the stack (edit ops → studio-server plumbing → Variables panel) builds on, SDK-first so agents and embedded hosts get the same contract as Studio.

How

The subpath exists because the core/parsers root barrels pull Node-only modules and break browser bundles — verified by building the studio against a root import (it fails on node:path in assetPaths.ts). getVariableValues deliberately uses the runtime's loose defaults extraction, not the strict declaration parser, so its output matches render behavior byte-for-byte even for malformed entries the strict parser drops (covered by a dedicated test).

Test plan

• Unit tests added: session.variables.test.ts (declarations parsing, runtime-parity merge including the loose-vs-strict divergence case, validation issues)
• Full suites green: sdk 435, parsers 1118, core 1118; studio browser build passes (the regression that motivated the subpath)
• Manual testing performed (see fix(studio): code-review and live-test fixes for the variables stack #2052 — the whole stack was driven end-to-end in a live studio session)

🤖 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
• 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 👈 (View in Graphite)
• main

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

[miga-heygen]

LGTM. Clean extraction of the composition-variables parser into a browser-safe module, and the three read APIs (getVariableDeclarations, getVariableValues, validateVariableValues) form a coherent surface with clearly documented strict-vs-loose semantics. The DEFAULT_TYPEOF table is a net improvement over the original switch statement. Test coverage is thorough, especially the strict/loose divergence case — that test alone justifies the design decision to use readDeclaredDefaults instead of the strict parser for getVariableValues.

One observation (not blocking): the documentElement cast in getVariableValues is a bit defensive — if linkedom ever changes its Document shape, this would silently return {} instead of erroring. Acceptable given it matches the runtime's own document.documentElement access.

— Miga

[james-russo-rames-d-jusso]

Reviewed at 0e3d59ca. Stack context: SDK foundation (#2046-#2048) — bottom of the 12-PR template-variables stack.

Clean read-surface. The three read APIs delegate cleanly, the browser-safe entrypoint pulls only what it advertises, and session.variables.test.ts drives a real openComposition (not a mock fixture), so I trust the coverage. Grepped variables.ts transitively — no node:* imports leak in. The subpath conditions match the existing entries in packages/core/package.json, so bundlers already using this convention pick it up without config.

🟠 Hold

• packages/sdk/src/session.ts L176-185 — getVariableValues() reads defaults only from document.documentElement, but runtime/getVariables() (packages/core/src/runtime/getVariables.ts L32-45) walks documentElement plus every [data-composition-variables] element in the body. The PR body claims getVariableValues mirrors render-time "byte-for-byte" — that's true for the common <html data-composition-variables=…> case, but a composition source carrying sub-comp declarers inside <body> will diverge: the SDK returns a strict subset. Two ways to resolve: (a) match the runtime and walk document.querySelectorAll("[data-composition-variables]") too, or (b) tighten the docstring on the Composition type + PR body to "declarations on the root <html> element only" (which is what Studio inspects anyway). Worth deciding now — if Studio starts calling this to predict render output for a composition-with-nested-declarers, silent divergence is hard to trace.

🟡 Nits / questions

• packages/parsers/src/compositionVariables.ts L26-33 — the doc comment on isScalarVariableValue says "font/image values are object-shaped." That's the object form only; validateVariables (packages/core/src/runtime/validateVariables.ts L88-133) accepts a bare string as a legal font/image value too. If a caller uses isScalarVariableValue to gate CSS writes on an image URL string, they'll write raw URLs to a CSS custom prop (see injectCompositionCssVariables — same behavior at runtime, so consistent, just not what a naïve reader of the comment expects). Consider rephrasing to "…font/image object values are not scalar; string fallbacks are."
• packages/sdk/src/engine/variableModel.ts:readVariableDeclarations — no caching. _gsapLabelCache in session.ts establishes the content-keyed-cache precedent for parse-per-call reads. Not urgent (attribute JSON is tiny), and Studio's expected call frequency is probably fine — but if this ends up in a hot memo dep, the JSON.parse + array filter runs on every call.
• The strict parser drops a declaration whose default typeof mismatches its type, but leaves an enum declaration with a default NOT in its options array intact (validation catches it later at value-check time). Intentional? Studio's Variables panel may want to defense-in-depth here since a "declaration that will never validate" is a weirder UX than one dropped upfront.
• Nice touch: the "loose runtime defaults filter, not strict parser" test at L102-113 pins the exact divergence the PR body flags — future readers see the contract in code, not just prose.

🟢 What's good

• Single canonical predicate (isCompositionVariable) shared between the parser and the writer path prepped for #2047 — the "readers and writers can't disagree" invariant is enforced by construction, not by test discipline.
• The move of parseCompositionVariables out of htmlParser.ts is genuinely tree-shakable — the fn used to sit next to linkedom imports; now it's importable without them. Verified by walking the transitive import graph from @hyperframes/core/variables.
• session.variables.test.ts uses real openComposition + serialize() round-trip semantics via setVariableValue at the end — that's the write→read consistency check I look for on read-API PRs (per feedback_regression_fixture_environment_bypass — schema-object equality alone would prove nothing).
• CI: all required checks green, including the SDK unit + contract + smoke lane.

— Review by Rames D Jusso

[vanceingalls]

R1 — hyperframes #2046 at 0e3d59ca

🟢 LGTM concurring with Miga; holding on Rames-D's SDK-vs-runtime divergence finding.

Verified independently:

• Read APIs delegate cleanly. getVariableDeclarations() returns strict CompositionVariable[], getVariableValues(overrides?) mirrors runtime merge, getVariableUsage() deferred to #2048. Test at session.variables.test.ts:412-422 locks in the intentional loose-vs-strict divergence.
• Browser-safe subpath is real. @hyperframes/core/variables transitively pulls no node:* imports; parseCompositionVariables extracted from htmlParser.ts into a self-contained compositionVariables.ts. Import-graph clean.
• Single canonical predicate. isCompositionVariable shared between parser and the writer path (readied for #2047) — reader-writer invariant enforced by construction, not by test discipline.
• CI 39/39 pass at head.

Concurring with Rames-D on the 🟠 Hold:

• getVariableValues() reads defaults only from document.documentElement; runtime walks documentElement PLUS every [data-composition-variables] element in body. Rames named this cleanly. Cross-referencing forward: #2081's declarationElement() + collectAllDeclaredVariableIds() refactor starts to address the multi-element pattern at write time (SDK now targets root-div for wrapped templates, not <html>), but the SDK read path here in #2046 is where the divergence originates and it isn't resolved by the later PRs' changes. Two paths land Rames flagged (walk-body vs. tighten-docstring) — the walk-body option would keep #2046 as the SSOT for "what render will actually see"; the docstring-tighten option accepts SDK-scoped-to-html, but then downstream #2050 Variables inspector's schema display can silently under-represent a real composition. Worth deciding before Studio starts using this as a prediction API.

Concurring with Miga's LGTM.

Unique observation:

O1 — isScalarVariableValue docstring vs validateVariables grammar. Rames also called this. Reinforcing: validateVariables.ts:88-133 accepts a bare string as a legal font/image value; isScalarVariableValue's docstring "font/image values are object-shaped" reads narrower than the truth. Consistent at runtime (both sides treat the string form the same way) but a naïve reader of the predicate could gate CSS writes on an image-URL string and miss why the raw string gets written to a custom prop. Rephrase to "font/image object-form values are not scalar; string fallbacks are."

R1 by Via

[miguel-heygen]

Stack re-review for the old hold on #2046. I am approving this in stack context: the later Studio integration opens SDK sessions per composition file (including selection.sourceFile for sub-composition selections), so getVariableValues() being scoped to the open composition's declaration element is the intended model, not a bundled-document prediction API. CI is green on this head.

One non-blocking cleanup remains and I called it out on #2101: the public interface comment still says the method mirrors runtime getVariables() exactly. Please tighten that wording in the stack so SDK consumers don't read it as a full bundled-document parity guarantee.

— Magi

Verdict: APPROVE
Reasoning: The previous concern is resolved by the stack's per-file SDK session model; #2046 is safe to approve, with the public doc-comment cleanup tracked in the top review.

[jrusso1020]

Follow-up from the #2101 stack review (Miguel): rewrote the getVariableValues interface JSDoc in packages/sdk/src/types.ts — dropped the "mirrors runtime getVariables() exactly" claim; it now states the per-file/per-declaration-element scope explicitly (the two diverge only when sub-comp declarers are inlined into one bundled document).

[james-russo-rames-d-jusso]

R2 at 8b286e3.

Prior R1 🟠 (SDK getVariableValues() reads documentElement only vs runtime's getVariables() walking every [data-composition-variables]) resolved via docstring narrowing at session.ts:186-193 and public types.ts:461-468 JSDoc — per-file scope now explicitly intentional, divergence with runtime named. This is the "tighten-docstring" resolution (vs "walk-body") and it's a fine call: Studio's --variables payload prediction is a per-file question, so keeping the SDK single-file honors the actual caller shape.

No residuals from my side. CI green. Stack-tip is stackable-onwards from here.

— Review by Rames D Jusso

[vanceingalls]

🟢 LGTM concurring with Rames-D — R2 at 8b286e3.

R1 🟠 hold (SDK getVariableValues() reads only documentElement; runtime walks documentElement + [data-composition-variables] in body — divergence) resolved via the tighten-docstring path rather than the walk-body path. Author picked option (b) of Rames-D's original two-path R1 ask; concurring on that call.

Verified at head:

• packages/sdk/src/session.ts:164-178 — getVariableValues() code unchanged, but the inline comment now explicitly narrows scope: "reads the composition's single declaration element only — NOT a union of every [data-composition-variables] in the document ... the SDK models one composition file, so per-file scope is intended."
• packages/sdk/src/types.ts:420-430 — public Composition.getVariableValues JSDoc gained a "Scope:" paragraph naming the runtime divergence on the API surface.
• packages/core/src/runtime/getVariables.ts:37 — runtime still walks document.querySelectorAll("[data-composition-variables]"). Divergence real, now a documented contract rather than a silent trap.

Rationale for accepting: the SDK models unbundled single-composition files while the runtime handles bundled multi-comp documents. Per-file scope is the intended contract for SDK consumers; Studio's --variables prediction is a per-file question, so narrower SDK scope is correct. Silent-trap → documented-contract is a real quality improvement even without a code change.

No residuals from my side.

R2 by Via

[miga-heygen]

Reconciliation review (post-#2098 rebase): Clean.

The browser-safe @hyperframes/core/variables subpath correctly replaces the root @hyperframes/core imports that #2098 used, and the parser extraction from htmlParser.ts into compositionVariables.ts avoids any duplication with #2098's engine helpers. The #2098 convenience methods (getVariableValue, listVariables) coexist properly as thin wrappers alongside the new canonical read surface. No stale artifacts from #2098 detected.

LGTM.

— Miga

[vanceingalls]

🟢 LGTM at R3 — 24c1982.

R2 verdict stands. Rebase-onto-#2098 preserves the tighten-docstring resolution and lands the canonical API from this stack while demoting #2098's conveniences to aliases. Concurring with the reconciliation shape.

Reconciliation verified at head:

• Canonical write ops: declareVariable({declaration}) at packages/sdk/src/session.ts:193-195; updateVariableDeclaration(id, declaration) at :197-199; removeVariableDeclaration(id) at :201-203. All dispatch via the object-wrapped op payload. Header comment at :191-192 explicitly notes #2098's {decl} form was unified onto {declaration} with identical method signature — #2098 callers unaffected.
• #2098 conveniences as aliases: getVariableValue(id) delegates via this.getVariableValues()[id] at :172-181; listVariables() delegates via this.getVariableDeclarations() at :182-184. Alias-block comment at :169-171 names the intent — "per-file declaration-element scope resolved in exactly one place."
• Patch grammar single-path: grep for /variable-decls/ returns 0 hits across session.ts, engine/mutate.ts, engine/patches.ts, engine/apply-patches.ts — #2098's hyphenated grammar dropped. Canonical /variableDeclarations/ grammar preserved at packages/sdk/src/engine/patches.ts:82, apply-patches.ts:12/:25/:82/:130-131. Undo/replay single-path invariant intact.
• R2 hold — read-scope docstring: getVariableValues() still reads only documentElement; JSDoc "Scope:" paragraph preserved. Documented-contract stands.

Observation (non-blocking, worth James's follow-up eye):

O1 — removeVariable alias delegates one layer deeper than its siblings. getVariableValue and listVariables delegate at the SDK-method-body layer (one-line forwards to the canonical getter). removeVariable(id) at session.ts:186-188 instead dispatches its OWN op (type: "removeVariable"), and the engine handler at packages/sdk/src/engine/mutate.ts:310-313 routes case "removeVariable" into handleRemoveVariableDeclaration. Semantically equivalent — both op types funnel through the same handler — but stylistically inconsistent with the other two aliases. Load-bearing risk: if a future refactor decouples the case "removeVariable" dispatch from handleRemoveVariableDeclaration, the SDK method silently diverges. Consider inlining removeVariable(id) as this.removeVariableDeclaration(id) at the SDK method layer to match its siblings.

Housekeeping: Miguel's approval at pre-rebase SHA 1569719 was dismissed on this PR (unique to #2046 — the other 11 kept APPROVED through the rebase, likely a CODEOWNERS rule on SDK read APIs). @james — you'll need Miguel to re-stamp 24c1982 to unblock mergeStateStatus=BLOCKED.

R3 by Via