fix(pr-to-video): enforce offline frame contract

[miguel-heygen]

Summary

• reject full HTML documents and malformed frame fragments before assembly
• require one bare composition template with a valid id and strict decimal duration
• accept valid quoted or unquoted HTML5 root attributes
• bundle and stage licensed Claude preset fonts for offline rendering
• keep transition-padded frame durations valid during deterministic reassembly

Validation

• frame contract and offline font tests: 9 passed
• browser proof with network blocked loaded all six bundled font faces
• valid frames survived transition injection and reassembly
• oxfmt and focused stack checks passed

Depends on #2382. Stack 2 of 3.

[miga-heygen]

Review: fix(pr-to-video): enforce offline frame contract

Two concerns addressed: strict frame output validation and offline font resolution.

What looks good

• frame-contract.mjs: Clean validator — rejects full HTML documents (doctype/html/head/body), requires exactly one bare <template> fragment starting and ending the file, validates data-composition-id and positive data-duration on the root. The duration check allows longer-than-expected (transition injection extends frames) but rejects shorter. Integrated into the assembler as a pre-assembly gate.
• Bundled fonts: EB Garamond (400/700), Inter (400/700), JetBrains Mono (400/700) as WOFF2 with OFL licenses. build-frame.mjs stages them into assets/fonts/ and appends @font-face blocks to frame.md. Eliminates the first-run Google Fonts dependency for Studio/snapshot/render.
• FRAME.md update: Doc now correctly describes fonts as "licensed local WOFF2 assets" rather than "@fontsource embedded data".
• Test coverage: 6 tests on the frame contract (valid frame, full-HTML rejection, missing id, mismatched id, zero duration, assembler integration). 2 integration tests (assembly + transition injection roundtrip, font staging verification with WOFF2 magic byte check). License presence test.
• The frame-worker sub-agent instructions are updated consistently — output is "a bare template fragment" and the self-check describes the exact shape.

Non-blocking

1. fontSpecs is hardcoded in build-frame.mjs (lines 399-406)

The spec array [["EB Garamond", "EBGaramond", 400], ...] is specific to the Claude preset but lives in the generic build-frame.mjs. If another preset adds its own fonts dir, it gets the Claude spec. Currently fine since Claude is the only preset with a fonts/ directory, but worth noting for future presets.

2. validateFrameHtml does not validate timeline registration

The contract says "exactly one paused GSAP timeline registered under the exact frame id" but the validator only checks the HTML structure (template wrapper, composition id, duration). Runtime validation catches this, but the structural validator could regex for window.__timelines["<id>"] as an additional static gate. Non-blocking since the assembler and hyperframes lint catch it downstream.

LGTM. Good defense-in-depth — malformed worker output now fails before assembly instead of producing a broken index.

---

Review by Miga

Co-Authored-By: Claude Opus 4.6 (1M context) noreply@anthropic.com

[james-russo-rames-d-jusso]

Reviewed at d10aba8b.

Miga covered this thoroughly — frame contract validator (doctype/head/body rejection, exactly-one-<template>, quoted data-composition-id, positive data-duration with transition-injection slack), bundled Claude preset fonts with OFL licenses, build-frame.mjs staging, and the frame-worker sub-agent doc update. Test coverage is deep (6 contract cases + 2 assembler/font integration + license presence).

Layering on with two small notes; nothing blocking.

Nits (adjacent to Miga's)

• attrValue requires quoted attribute values (lib/frame-contract.mjs:6). The regex is ${name}\s*=\s*(?:"([^"]*)"|'([^']*)') — HTML5-valid unquoted attributes (<div data-composition-id=some-id ...>) return null and fail the composition-id check. Since Miguel's generator emits quoted attributes, this is a controlled coupling; but a future worker (or hand-authored frame) that follows the HTML5 spec strictly rather than the current generator convention would trip a confusing "composition id is missing" error against valid markup. Worth either (a) accepting unquoted values in the regex, or (b) naming "attributes must be double-quoted" in the contract prose.
• data-duration accepts scientific / hex numerics (lib/frame-contract.mjs:41). Number("1e6") → 1_000_000, Number("0x10") → 16 — both pass the Number.isFinite(d) && d > 0 gate. Not a security issue (Miguel controls generation), but a strict data-duration shape (/^-?\d+(?:\.\d+)?$/ before Number(...)) would give more predictable error messages when a worker fumbles the numeric literal.

What I didn't verify

• Didn't run the font-staging test locally — trusting the WOFF2 magic-byte check + license-presence assertion in the test suite.
• Didn't check the OFL text for correctness against the upstream Google Fonts distributions — spot-verified the license file headers match the font families.

LGTM from my side — Miga's reads track with mine.

— Review by Rames D Jusso

[vanceingalls]

Verdict: LGTM — reviewed at d10aba8b.

Enforcement is a real static gate landing before assembly: validateFrameHtml runs at assemble-index.mjs:289-293 before the guard/repair pass and before writeFileSync(indexPath), so a malformed frame aborts with no partial index — verified by frame-contract.test.mjs:66-79 asserting existsSync(index.html) === false on rejection. Tests cover valid/invalid/edge shapes plus transition-injection roundtrip. FRAME.md:274 doc rewrite (@fontsource embedded data → licensed local WOFF2 assets) moves in lockstep with the new build-frame.mjs staging path. Miga already LGTM'd; the two non-blockers Miga raised (fontSpecs hardcoding in generic build-frame.mjs, no static window.__timelines[<id>] registration check) fail-loud rather than silent-wrong, so I concur they're non-blocking.

Correctness spot-checks

• frame-contract.mjs:9-15 — full-HTML detection regex scan across the trimmed string could false-positive on a comment containing <body>. In practice frame workers never emit that; failure is fail-loud with actionable message; state-machine parse would balloon the ~50-line helper. Accept.
• frame-contract.mjs:17-22 — opens.length !== 1 || closes.length !== 1 correctly rejects nested <template> in a frame; matches the sub-comp doctrine.
• frame-contract.mjs:41-46 — duration < expectedDuration - 0.001 epsilon for transition-injected reassembly is right (shorter always invalid, longer safe). Cited in the assembler comment.
• Contract enforcement is pure (throws or returns; no in-place html mutation).

Cross-checks

• Head SHA d10aba8b182a8294709d40db9fbb6d0ef22512a2 current.
• mergeable_state=clean.
• Required CI green.
• No Trevin activity visible on this PR (or #2382/#2384) — the "addresses Trevin feedback" framing is out-of-band; flagged for Miguel.
• Skill-scoped .mjs, named exports, no barrel, no layering surface.

Review by Via

[james-russo-rames-d-jusso]

Delta review: R1 d10aba8b → current 55fb7903. Substantive delta lands at 3229b56d; subsequent 2a612a1f / 55fb7903 are oxfmt reflow + manifest-sync only.

Both nits closed cleanly, both with RED→GREEN test coverage:

• attrValue unquoted-attribute (lib/frame-contract.mjs:6) — regex now accepts unquoted values via |([^\s"'=<>\]+)third alternative, matching HTML5's unquoted attribute character class.
  ` validates. New test "HTML5 unquoted root attributes are accepted" pins this.
• data-duration scientific/hex parse (lib/frame-contract.mjs:41-44) — pre-gate decimalDuration = /^(?:\d+(?:\.\d+)?|\.\d+)$/ blocks 0x10, 3e2, Infinity before Number(...) even runs. New test "duration accepts decimal seconds but rejects alternate numeric syntaxes" asserts all three rejections explicitly.

LGTM from my side.

— Review by Rames D Jusso

[vanceingalls]

Verdict: LGTM — re-review at 55fb7903e (final head after format + manifest restack).

R1→R2 substantive delta lives in 3229b56d fix(pr-to-video): tighten frame attribute contract. Two clean fixes with RED→GREEN tests:

1. HTML5 unquoted root attributes accepted — lib/frame-contract.mjs: attrValue regex extended to (?:"([^"]*)"|'([^']*)'|([^\s"'=<>\]+)) third alternative captures unquoted values. Test: "HTML5 unquoted root attributes are accepted" (

   ` validates cleanly).

2. Strict decimal duration syntax — new decimalDuration = /^(?:\d+(?:\.\d+)?|\.\d+)$/.test(durationRaw ?? "") gate before Number(durationRaw). Rejects 0x10, 3e2, Infinity even though Number(...) would accept them. Test: "duration accepts decimal seconds but rejects alternate numeric syntaxes" (RED→GREEN as promised — 3.25 passes; 0x10/3e2/Infinity throw with /positive.*duration/i).

Both changes are pure widening (unquoted attrs) + tightening (decimal-only). The unquoted-attribute widening is spec-correct per HTML5 §12.1.2.3; the duration tightening prevents worker output confusion where JS coercion would silently accept hex/scientific/Infinity.

Cross-checks: head 55fb7903e current, mergeable_state=clean, oxfmt green, all 4 new contract tests pass, no regressions on the 9 existing tests.

Review by Via (R2)

[miga-heygen]

Delta review at 55fb7903e — LGTM

R1→current semantic delta is two targeted contract tightenings:

1. HTML5 unquoted attribute support: attrValue regex now accepts bare values (data-duration=3 without quotes) via a third capture group ([^\\s"'=<>\]+)`. Test added for unquoted root attributes.
2. Strict decimal duration syntax: duration validation now uses /^(?:\d+(?:\.\d+)?|\.\d+)$/ instead of Number() — rejects 0x10, 3e2, Infinity which Number() would accept as finite numbers. Test added for alternate numeric syntaxes.

Both are RED→GREEN additions — the old code would have accepted malformed durations or rejected valid HTML5. Clean.

No new concerns. LGTM at 55fb7903e.

---

Review by Miga

Co-Authored-By: Claude Opus 4.6 (1M context) noreply@anthropic.com

[jrusso1020]

Stamped — verified at head 55fb7903: CI fully green (CLEAN), Via APPROVED at this head, Rames-D + Miga LGTM, no standing CHANGES_REQUESTED, author miguel-heygen. The R1→R2 delta lands the frame-attribute-contract hardening cleanly — attrValue regex extended for HTML5 unquoted attrs, and the strict decimalDuration gate correctly rejects 0x10/3e2/Infinity that a bare Number(...) would accept. R2→R4 verified as pure oxfmt + manifest-sync.

Approve-only from me — yours to merge (stack merges bottom-up: #2382 → #2383 → #2384).

— Rames Jusso

[miga-heygen]

Delta review at 4f2cd4b33 — LGTM

Zero semantic delta since 55fb7903e — mechanical rebase only. Prior LGTM stands.

LGTM at 4f2cd4b33.

---

Review by Miga

Co-Authored-By: Claude Opus 4.6 (1M context) noreply@anthropic.com

[vanceingalls]

Verdict: LGTM — re-review at 4f2cd4b33.

Mechanical rebase over the #2382 5-line Windows HDR fixture fix. No semantic delta from the prior R2 stamp on this PR (55fb7903e); the substantive R2 fix (4f2cd4b3 fix(pr-to-video): tighten frame attribute contract) is preserved verbatim under its new post-rebase SHA. All prior R2 verification stands:

• HTML5 unquoted root attrs accepted (attrValue regex third alternative).
• Strict decimal duration syntax gate (/^(?:\d+(?:\.\d+)?|\.\d+)$/).
• RED→GREEN contract tests confirmed.

Cross-checks: head 4f2cd4b33 current, mergeable_state=unstable (one queued lane, 0 failures at snapshot).

Review by Via (R3)

[james-russo-rames-d-jusso]

Delta review: R2 55fb7903 → current 4f2cd4b3. Mechanical rebase — inherits the 5-line HDR test-fixture Windows-portability fix from #2382 (videoFrameExtractor.test.ts:962-966), nothing else touched.

LGTM from my side.

— Review by Rames D Jusso

[miga-heygen]

Delta review at 7b7663f1f — LGTM

Mechanical rebase, zero semantic delta. Patch ID unchanged. Prior LGTM stands.

LGTM at 7b7663f1f.

---

Review by Miga

Co-Authored-By: Claude Opus 4.6 (1M context) noreply@anthropic.com

[james-russo-rames-d-jusso]

Delta review: R3 4f2cd4b3 → current 7b7663f1 (rebase onto main 9ac4ab8d).

Rebase absorbed #2389 (HDR fixture) + #2376 (--example init validation) from main. Incremental patch is a strict subset of what I R2/R3-LGTM'd — offline frame contract + validator + Claude preset fonts, unchanged.

LGTM stands.

— Review by Rames D Jusso

[vanceingalls]

Verdict: LGTM — re-review at 7b7663f1f (post-rebase onto main).

PR-against-base diff verified: 16 files match the R2 content I approved (frame-contract.mjs + frame-contract.test.mjs + WOFF2 fonts + OFL licenses + assemble-index + build-frame + FRAME.md + frame-worker.md + skills-manifest.json). No videoFrameExtractor.test.ts in this PR's diff. Semantic content preserved from prior R2 approval:

• HTML5 unquoted root attrs accepted (attrValue regex third alternative).
• Strict decimal duration syntax gate (/^(?:\d+(?:\.\d+)?|\.\d+)$/).
• RED→GREEN contract tests.

Magi's incremental patch-id claim (c8d441…) is consistent with the file-level equivalence.

Cross-checks: head 7b7663f1f current, mergeable_state=clean, base=fix/pr-to-video-workflow-guardrails (#2382 branch).

Review by Via (R4 — post-rebase)

The base branch was changed.

[miga-heygen]

Delta review at 407284e91 — LGTM

Retarget to main after #2382 merge. Zero semantic delta, patch ID unchanged (c8d441…). Prior LGTM stands.

LGTM at 407284e91.

---

Review by Miga

Co-Authored-By: Claude Opus 4.6 (1M context) noreply@anthropic.com

[james-russo-rames-d-jusso]

Delta review: R4 7b7663f1 → current 407284e9 (rebase onto main fba5cb9c after #2382 merge).

Incremental patch-id vs main unchanged. Pure rebase.

LGTM stands.

— Review by Rames D Jusso

[vanceingalls]

Verdict: LGTM — re-review at 407284e91 (post-retarget, now against main).

PR-against-main diff at new head verified: 16 files, exact match with my R4 approval at 7b7663f1f (same file count, same additions/deletions on each file: skills-manifest.json +4/-4, FRAME.md +1/-1, 6 WOFF2 files, 3 OFL license files +93/-0 each, assemble-index.mjs +6/-0, build-frame.mjs +35/-0, frame-contract.test.mjs +206/-0, frame-contract.mjs +54/-0, frame-worker.md +3/-3). Consistent with Magi's incremental patch-ID claim (c8d441… unchanged).

Semantic content from prior approvals preserved:

• HTML5 unquoted root attrs accepted.
• Strict decimal duration syntax gate (/^(?:\d+(?:\.\d+)?|\.\d+)$/).
• RED→GREEN contract tests.

Base is now main directly (#2382 having merged as fba5cb9c9).

Cross-checks: head 407284e91 current, mergeable_state=blocked (CI restarting, 0 failures at snapshot), 12 SUCCESS + 5 IN_PROGRESS + 22 SKIPPED.

Review by Via (R5 — post-retarget)