fix(compiler): skip CSS var() in font resolver

[miga-heygen]

Summary

• Skip var() expressions in extractRequestedFontFamilies() — they are CSS custom property references, not font family names
• Add fail-closed test verifying var() font-family values don't trigger FONT_FETCH_FAILED

Closes #1654

Problem

The regex-based font scanner in deterministicFonts.ts treated var(--ui-font) as a literal font family name. When a composition uses CSS variables for font-family values, the distributed renderer's fail-closed mode threw FONT_FETCH_FAILED because var(--ui-font) cannot be resolved as a Google Font, bundled alias, or local font.

Fix

One-line guard: skip any family that starts with var(. These expressions resolve at browser paint time via CSS custom property substitution. The distributed renderer uses headless Chrome, which handles variable resolution normally at paint time — the font just won't be pre-embedded as a @font-face rule, which is correct behavior.

Test plan

• New test: does NOT throw when font-family uses CSS var() references — verifies compositions with var(--ui-font) and var(--vowel-font) pass through fail-closed mode without error
• Existing tests still pass (no regressions in font resolution pipeline)
• CI green

— Miga

[james-russo-rames-d-jusso]

Reviewed at HEAD 9c3cf77e. Small, well-scoped fix — one-line skip in extractRequestedFontFamilies for tokens starting with var(, plus a fail-closed regression test and a distributed-render fixture. Trace through the code is clean.

Verdict matrix

	item
✅	Skip logic at deterministicFonts.ts:211 lands at the right layer — after lowercasing, after generic-family pruning, before the canonical/Google-Fonts dispatch. Each parsed family is checked independently, so font-family: var(--main), Inter, sans-serif still resolves Inter correctly.
✅	Both surfaces covered: font-family CSS prop and data-font-family attribute flow through the same iterateFontFamilyDeclarations iterator, so the skip applies uniformly.
✅	Fail-closed test pins the load-bearing behavior (the FONT_FETCH_FAILED was the actual prod failure mode). failClosedFontFetch: true + makeFailingFetch() is the right harness.
✅	Distributed fixture at tests/distributed/css-var-fonts/ exercises the end-to-end render path with both mixed (var() + sans-serif) and pure-var declarations — good cross-stack coverage.
🟡	validateNoSystemFonts at planValidation.ts:108-123 shares the same iterator but has no var() awareness. With var() as the primary family, the lowercased token (e.g. "var(--ui-font)") is not in GENERIC_FAMILIES, so the system-font check silently passes through — the validator can no longer enforce its SYSTEM_FONT_USED invariant when the primary family is var()-driven. That may be intentional (runtime CSS engine resolves to whatever's declared) but worth being deliberate about: the var's value could be system-ui or undefined, and you wouldn't catch it at plan time. Either accept and document, or have the validator emit a softer warning when primary is var(.
🟡	Test assertion expect(result).toBe(html) is correct-by-construction here (all families are var() or generic, so no @font-face should be injected) but doesn't cover the mixed case — font-family: var(--ui-font), "Inter", sans-serif should still inject @font-face for Inter. The distributed fixture covers end-to-end render, but a unit-test assertion that explicitly verifies a non-var sibling still gets resolved would pin the skip's non-aggression. Per the parity-test rubric: identity assertions on no-op inputs are weak; one positive-resolution test on a mixed input would harden coverage.
🟡	parseFontFamilyValue splits on , blindly — var(--font, "Helvetica") (var with quoted fallback containing a comma is unusual, but legal-ish) would split into ["var(--font", " \"Helvetica\")"]. The first token now skips cleanly (improvement over status quo). The second becomes a corrupted family name Helvetica") and would still flow to the resolver. Pre-existing parser limitation; this PR doesn't make it worse, just worth flagging if someone hits it later. Out of scope for this fix.
🟢	One question: any reason to skip inherit / initial / unset / revert keywords from font-family resolution? Same class of "CSS-wide keyword the runtime handles, not a literal font name." Probably rare enough in compiled HTML that it's never hit, but worth a memo. Not blocking.

What I didn't verify

• The distributed fixture's output.mp4 PSNR floor (binary, can't read; trusting CI's regression shard).
• Whether data-font-family attribute is ever observed with var(...) values in compiled HTML — the skip would still work, but it's an unusual surface for a CSS function.

Net: clean, minimal, lands at the right layer. The 🟡s are coverage suggestions for a follow-up, not landing blockers.

Review by Rames D Jusso

[vanceingalls]

Reviewed at HEAD 9c3cf77e (base: main, standalone — not part of a stack). Co-reviewing with Rames; I've read his review and converge on the same shape — small, well-scoped, lands at the right layer. Same 🟡s, plus one angle on the deterministic-render contract worth raising explicitly.

Trace through the fix

The one-line guard at packages/producer/src/services/deterministicFonts.ts:211:

for (const originalCase of families) {
  const normalized = originalCase.toLowerCase();
  if (!normalized || GENERIC_FAMILIES.has(normalized)) continue;
  if (normalized.startsWith("var(")) continue;   // ← new
  if (!requested.has(normalized)) requested.set(normalized, originalCase);
}

Verified the parse chain:

1. iterateFontFamilyDeclarations (deterministicFonts.ts:73-86) yields every font-family: and data-font-family= value.
2. parseFontFamilyValue (:57-62) splits on ,, trims, strips leading/trailing quotes — for var(--ui-font), sans-serif, this produces ["var(--ui-font)", "sans-serif"].
3. .toLowerCase() at :209 makes the guard case-insensitive — VAR(--X) is valid CSS and handled.
4. The skip lands AFTER generic-family pruning and BEFORE the resolver dispatch, so a mixed font-family: var(--main), Inter, sans-serif still resolves Inter correctly. ✅

Verdict matrix

	item
✅	Skip lands at the right layer — per-family, after lowercasing, before dispatch. Cross-family resolution still works for mixed declarations.
✅	Both surfaces (font-family CSS prop and data-font-family attr) flow through the same iterator, so the skip applies uniformly without a second guard site. No duplicate-guard risk.
✅	Test uses the real injectDeterministicFontFaces primitive (deterministicFonts-failClosed.test.ts:181-192) with failClosedFontFetch: true + makeFailingFetch(). The injection point under test is the production code path, not a mocked stand-in. Pre-fix the failing fetch would propagate as FONT_FETCH_FAILED; post-fix the var() entry is skipped before fetch is even attempted. The expect(result).toBe(html) assertion is correct-by-construction (only var() + generic in the input → zero requested families → no @font-face block injected → raw HTML returned).
✅	Distributed regression fixture at tests/distributed/css-var-fonts/ covers the end-to-end render with both mixed and pure-var declarations. The fixture's <head> carries a Google Fonts @import for Inter + Montserrat, so headless Chrome resolves the var() values to real font data at paint time.
✅	No upstream var( filter found in htmlCompiler.ts or elsewhere — the new guard isn't a duplicate. (Searched the producer package for var(-handling and for callers of parseFontFamilyValue / iterateFontFamilyDeclarations; only consumers are deterministicFonts.ts itself and planValidation.ts.)
🟡	validateNoSystemFonts (planValidation.ts:108-123) has no var() awareness. It inspects families[0] (primary family) and rejects if it's in GENERIC_FAMILIES. For font-family: var(--ui-font), sans-serif, the primary is var(--ui-font) — not generic, so the validator passes. The PR's stated guarantee is that headless Chrome resolves var() at paint time, which is fine IF the resolved value is either (a) pre-embedded via the bundled-font path, or (b) loaded via @import (as the fixture does). In the narrower case where a composition declares --ui-font: "Inter" (or anything else NOT pre-embedded AND without an @import), the post-fix behavior is: validateNoSystemFonts passes, the font scanner skips, Chrome can't find Inter, falls back to sans-serif on the Linux container — which validateNoSystemFonts was designed to prevent. The fix moves a loud FONT_FETCH_FAILED to a silent wrong-font render in that edge case. Rames noted the same gap. Not a landing blocker (the dominant case — @import + var() — is strictly improved), but worth a TODO in planValidation.ts or a follow-up issue to either reject var( as a primary family OR substitute :root definitions before scan. Issue #1654's bottom paragraph already acknowledges this as future work.
🟡	Missing unit-test coverage on the non-aggression of the skip. The fail-closed test asserts result === html for a pure var()+generic input (so no @font-face would have been injected anyway, with or without the fix). A second unit test on a mixed declaration — e.g. font-family: var(--x), "Inter", sans-serif with failClosedFontFetch: false — that asserts the resulting HTML still contains an Inter @font-face rule would pin the per-family iteration's correctness. The distributed fixture covers it end-to-end, but a unit-level pin is cheaper to debug if the skip ever regresses. (Rames raised the same point.)
🟡	Pre-existing parser limitation in parseFontFamilyValue: it splits on , blindly, so var(--display-font, "Montserrat") (var with quoted fallback) splits into ["var(--display-font", ' "Montserrat")']. After trim/strip-quote, the first token still skips cleanly via the new guard (good), but the second becomes Montserrat") and flows to the resolver. Not introduced by this PR; just a known sharp edge. Issue #1654 explicitly flags this as out-of-scope.
🟢	Minor — inherit / initial / unset / revert font-family values would also flow through as literal "font name" candidates. Almost certainly never seen in compiled HTML, but the same class of bug. No action needed; flagging for the next time someone touches extractRequestedFontFamilies.

What I did NOT verify

• The distributed fixture's output.mp4 PSNR baseline — binary blob, trusting the regression shard's verdict.
• Whether data-font-family attribute ever carries var(...) in real-world compiled HTML — the skip works regardless, just an unusual surface for a CSS function.

CI

At review time: 0 failing, 10 in progress (Typecheck, Test, CLI smoke, Smoke global, regression shards 1-8, Windows render, CodeQL JS). All other completed checks green.

Verdict

NIT — small, focused, lands cleanly at the right layer. The validateNoSystemFonts gap is real but matches the issue's own scoping (acknowledged as follow-up work) and is a strict improvement on the dominant @import + var() shape. Recommend tracking the validator-side TODO as a follow-up issue, and adding the mixed-declaration unit test before next touch of this file.

Review by Via