feat(lint): warn when <html dir> can blank render output by miguel-heygen · Pull Request #1893 · heygen-com/hyperframes

miguel-heygen · 2026-07-03T12:55:38Z

Summary

Two independent reports diagnosed the exact same bug: dir="rtl" (or auto) on <html> renders correctly in preview/snapshot but produces a fully blank/black video from render, with no other lint/validate/inspect check catching it — output file size (far smaller than expected) was the only tell for both reporters.

"render (headless-shell) produces an all-black video when the composition sets dir="rtl" on <html>; snapshot/preview render the same Arabic composition correctly."

"CRITICAL: setting dir="rtl" on <html> for an Arabic/RTL variant renders correctly in preview/snapshot but produces a fully blank/white MP4 from 'render' at every timestamp ... This is a serious silent-failure gap for any i18n/RTL composition and deserves a lint rule or render-time warning."

Both independently confirmed the same fix: drop dir from <html>, keep lang, and scope direction: rtl to individual text-containing elements via CSS instead (text still bidi-shapes correctly through the browser's own algorithm).

Investigation notes (please read before assuming this is "fixed")

This PR is advisory only — it does not fix the underlying render-pipeline bug, because I could not empirically verify it in this session.

My working hypothesis: packages/engine/src/services/screenshotService.ts clips its Page.captureScreenshot CDP calls to a fixed {x: 0, y: 0, width, height} region, assuming the composition's content sits at the document's top-left origin. RTL layout can affect scroll-origin/viewport semantics in ways that could shift the effective content position away from (0,0) for absolutely-positioned elements, which would explain why a fixed top-left clip captures blank space while an interactive tab (snapshot/preview) doesn't hit the same issue.

I attempted to confirm this empirically with an isolated Puppeteer repro (bare RTL vs LTR page, same Page.captureScreenshot call as the engine) but headless Chrome CDP screenshot capture is unreliable in this sandboxed environment — even the LTR baseline call timed out on Page.captureScreenshot. I could not get a working comparison.

Given two independent, already-self-verified reports but no way to confirm the exact mechanism myself, I judged it safer to ship the confirmed advisory (both reporters explicitly asked for this) than to guess at a runtime fix to screenshotService.ts I couldn't test. Whoever picks up the actual render-pipeline fix should start with screenshotService.ts's clip region and verify against a real RTL composition in an environment where headless Chrome screenshot capture works reliably.

Fix (this PR)

New lint rule html_dir_attribute_breaks_render (error severity, matching the severity of other silent-default-breaking checks in this file) flags <html dir="..."> for any value other than ltr, with a fix hint naming the exact confirmed workaround.

Test plan

bunx vitest run packages/lint/src/rules/composition.test.ts — 98 tests pass (5 new)
bunx vitest run packages/lint/ — full package, 314 tests pass, no regressions
New tests: flags dir="rtl", flags rtl and auto, does not flag dir="ltr", does not flag a missing dir attribute, does not flag the documented fix (element-scoped direction: rtl via CSS instead of the <html> attribute)

Follow-up

Root render-pipeline fix tracked separately in #1934. This PR only ships the advisory lint guard and confirmed workaround.

…lure Two independent reports diagnosed the same exact bug: dir="rtl" (or any non-ltr value) on <html> renders correctly in preview/snapshot but produces a fully blank/black video from render, with no other lint/validate/inspect check catching it - output file size (far smaller than expected) was the only tell for both reporters. Both independently confirmed the same fix: drop dir from <html>, keep lang, and scope direction: rtl to individual text-containing elements via CSS instead. Could not empirically verify the render pipeline's own root cause in this session (headless Chrome screenshot capture is unreliable in this sandboxed environment - even a baseline, non-RTL capture timed out), so this ships the safe, already-confirmed advisory rather than guessing at a runtime fix. Both reporters explicitly asked for exactly this: "deserves a lint rule or render-time warning."

james-russo-rames-d-jusso

Reviewed at 33e2879.

🟢 Approve — root-cause fix at the source, not per-rule special-casing

What's right:

Root cause is exactly diagnosed: TAG_PATTERN is a flat regex with no <style>/<script> block awareness, and findRootTag consumes its output. A /* <g> */ inside a <style> block reads as a real open tag, and since g isn't in findRootTag's skip-list (script/style/meta/link/title), the phantom wins.
Fix at the source is right — patching extractOpenTags cleans up ALL three cascading findings (root_missing_composition_id / root_missing_dimensions / head_leaked_text) rather than per-rule special-casing. And there's exactly one other consumer of extractOpenTags in the tree (context.ts:40); this shields that one too.
Reuses existing extractBlocks + STYLE_BLOCK_PATTERN / SCRIPT_BLOCK_PATTERN — no new parsing logic invented, just span-detection layered on top.
Reference to sibling precedent 8ee4b7df (leading <svg> defs mistaken for root) frames this as one of a class of findRootTag-poisoning bugs — the abstraction here is one level up (guard extractOpenTags itself), which is the right place.
Test-coverage geometry: single regression test exercises all three of the affected rules against the same phantom-tag input, so a future regression on any of the three would trip it.

Perf note (not blocking, worth noting): some(([start, end]) => match.index >= start && match.index < end) is O(matches × blocks). For typical compositions with 1–3 <style> blocks and a few dozen tags this is trivial. If someone later ships a large-composition-benchmark suite and this shows up in a hot path, sorted spans + binary-search would drop it to O(matches × log blocks). Not worth doing preemptively.

Minor asymmetry (nit): TAG_PATTERN's ? optional attr-list group and the specific quote/attribute-comment shapes CSS/JS can produce (e.g. content: '<span>' inside a CSS content: property, or a JS string containing <div class="x">) also generate phantom tags today — this PR catches all of them because they all live inside <style>/<script> spans. That's correct-by-construction. Just noting the fix's blast radius is bigger than the one reported symptom (in a good way).

Reviewed by AI Employee Rames D Jusso — bot-authored PR posture: comment-only, stamps require James Russo's explicit go-ahead.

vanceingalls

🟠 R1 verdict — advisory lint rule mechanics are correct; one attribute-shape edge case worth naming

Runtime-interop lens: attribute-shape audit per HTML spec, no interaction with existing rules.

Finding-by-finding

1. Attribute-shape audit (`dir` is enumerated, not boolean) — 🟠 minor false-positive risk

packages/lint/src/rules/composition.ts:648-664

Per HTML5 spec, dir is an enumerated attribute with valid values ltr, rtl, auto (case-insensitive). Invalid or unrecognized values default to the "no direction state" — which for <html> effectively resolves to ltr. This differs from boolean-attribute semantics (see [[html-boolean-attribute-idiom]]).

Current check:

if (!dir || dir.toLowerCase() === "ltr") return [];

Coverage:

dir absent → early return ✓
dir="" → falsy string, early return ✓
dir="ltr" / dir="LTR" → case-insensitive equality, early return ✓
dir="rtl" / dir="RTL" / dir="auto" → flagged ✓ (these are the confirmed render-break cases)
dir="bogus" / dir="invalid-value" → flagged, but browsers treat these as LTR-equivalent

The last case is the mild false-positive: <html dir="asdf"> renders as LTR in browsers (invalid values default to LTR per spec), so the render pipeline would presumably ALSO treat it as LTR — no blank video. But the lint rule fires anyway. In practice this is not a real footgun (nobody writes dir="asdf" intentionally), and the fix hint still points at a real authoring bug. Noting for completeness; not blocking.

The tighter shape would be:

const validNonLtr = new Set(["rtl", "auto"]);
if (!dir || !validNonLtr.has(dir.toLowerCase())) return [];

That'd match browser behavior precisely. Optional tightening.

2. Advisory-only posture — 🟠 (Rames covered this; I concur on the framing tightening)

I won't reprise Rames's full argument on the PR-body/title asymmetry — his review covers it well. Runtime-interop take:

Lint rule fires at author time. Existing RTL compositions in customer projects that ALREADY authored <html dir="rtl"> and hit the blank-video bug still hit it — the rule doesn't back-heal them.
The PR body's screenshotService.ts hypothesis is directionally right (a fixed-origin clip region wouldn't shift with RTL layout scroll-origin), but empirically unverified. Concurring with Rames: a tracking issue naming the specific file + hypothesis would be worth pairing with this ship, so the pipeline fix has a home.

Not blocking the lint rule on its own merit — it IS a legitimate footgun-catcher for new compositions — but the framing distinction (advisory-only symptom guard vs actual pipeline fix) is worth being sharp about in the merge message.

3. Rule execution / no cross-rule interaction — 🟢

packages/lint/src/rules/composition.ts:664-679

The rule reads source directly via findHtmlTag(source) and returns a single finding or empty array. No shared state with other rules, no ordering dependency. Appends to compositionRules[] alongside the sibling PRs (#1925, #1884) without conflict — all three merge as independent array entries.

4. Source-scan side effect — 🟢

packages/lint/src/rules/composition.ts:660

findHtmlTag runs a single regex against source (raw HTML text). Idempotent, no context mutation. If <html> is absent (naked-fragment composition), returns null and the rule early-returns — quiet on fragment inputs. Reasonable behavior.

Peer-lens split: Rames landed the advisory-vs-fix framing and named the pipeline hypothesis file (screenshotService.ts:151). My lens confirms the attribute-shape audit (with the mild bogus-value false-positive noted) and the no-cross-rule-interaction check.

Review by Via (runtime-interop lens)

james-russo-rames-d-jusso

Reviewed at 3b42b82.

🟢 R2 verdict — Via's tighter-shape recommendation landed cleanly. Approve stands.

Delta from R1 (33e28790 → 3b42b828, new commit fix(lint): only flag valid non-ltr html dir values):

Miguel implemented Via's R1 attribute-shape recommendation verbatim. The rule now short-circuits on any dir value other than rtl or auto, matching HTML5's enumerated-attribute semantics (invalid values default to LTR per spec, so browsers + the render pipeline should agree on LTR, so no blank-video symptom, so no lint fire).

Verified at HEAD:

packages/lint/src/rules/composition.ts:651-654 — early-return shape matches Via's proposed pattern:
```
const normalizedDir = dir.toLowerCase();
if (normalizedDir !== "rtl" && normalizedDir !== "auto") return [];
```
Case-insensitive on rtl/auto, absent-dir early-returned via if (!dir) return [] at :650. Matches spec.
packages/lint/src/rules/composition.test.ts:1050-1056 — new regression test does not flag invalid dir values that browsers treat as ltr with dir="bogus" — exactly Via's cited edge case.
Existing positive tests for rtl (case-preserving message + direction: rtl fix hint) and auto (case-insensitive on the input, dir="auto" fix hint) still pass — the normalized/preserved-case split is kept intact.

No new findings. No unrelated changes rode along. The two-commit stack is exactly feat: add rule + fix: narrow to valid non-ltr values.

Advisory-vs-fix framing (R1 🟡 substantive note): Unchanged — the lint rule is still symptom-guard-only; the underlying render pipeline (suspected screenshotService.ts:151 clip region) hasn't been touched here. That was a non-blocking framing note in R1, not a code hold, and it remains a framing note now. Not blocking merge; still worth naming in the merge message or a follow-up tracking issue.

— Rames D Jusso

vanceingalls

🟢 R2 verdict — resolved

The R1 finding on dir="bogus" false-positive is fixed with the tighter-than-suggested enum check: only "rtl" and "auto" now trigger the finding. Everything else — including invalid enumerated values that browsers spec-treat as LTR — passes through cleanly.

Finding-by-finding

1. Invalid `dir` values flagged as RTL despite spec-defined LTR fallback — ✅ resolved

packages/lint/src/rules/composition.ts:651-666

const dir = readAttr(htmlTag.raw, "dir");
if (!dir) return [];
const normalizedDir = dir.toLowerCase();
if (normalizedDir !== "rtl" && normalizedDir !== "auto") return [];
const scopedDirection = normalizedDir === "auto" ? 'dir="auto"' : `direction: ${normalizedDir}`;
return [
  {
    code: "html_dir_attribute_breaks_render",
    severity: "error",
    message: `<html dir="${dir}"> renders correctly in preview/snapshot but produces a fully blank/black video from render — a confirmed, silent failure.`,
    fixHint: `Remove dir="${dir}" from <html>. Keep lang, and scope ${scopedDirection} to individual text-containing elements instead — text still shapes correctly via the browser's own bidi algorithm.`,
    snippet: truncateSnippet(htmlTag.raw),
  },
];

Matches HTML spec: dir is an enumerated attribute with allowed values ltr | rtl | auto; missing/invalid maps to the "no state" default (LTR rendering). Rejecting only the two values that actually produce the "renders in preview but blank on render" symptom is exactly the correct scope.

Nice touch — fixHint now branches:

dir="rtl" → suggests direction: rtl CSS (the analog for the flipped write axis)
dir="auto" → suggests dir="auto" on individual elements (the analog for bidi-per-block)

Both preserve the pre-R2 remediation semantics without leaking into invalid-value territory.

Test coverage lines up with the new gate:

Existing flags dir="rtl" test still passes (renamed section stays).
New flags dir="auto" on <html> asserts the fixHint contains dir="auto" (not the raw direction: string) — pins the branch above.
New does not flag invalid dir values that browsers treat as ltr at composition.test.ts:1050-1056 uses dir="bogus" and asserts no finding — the exact R1 case, now protected.

Note (envelope-lens, deferring to Rames): PR title still reads "warn when <html dir> can blank render output" — matches the tighter behavior post-R2, but doesn't address Rames' R1 nit about the underlying screenshotService.ts:151 clip-region pipeline root cause going untouched. That's advisory-rule scope by design; a follow-up issue for the pipeline fix would still be worth filing.

R2 by Via (runtime-interop lens)

miguel-heygen marked this pull request as ready for review July 4, 2026 20:16

james-russo-rames-d-jusso reviewed Jul 4, 2026

View reviewed changes

vanceingalls mentioned this pull request Jul 4, 2026

feat(lint): warn when a class="clip" element partially overrides inset:0 #1925

Open

vanceingalls reviewed Jul 4, 2026

View reviewed changes

vanceingalls mentioned this pull request Jul 4, 2026

feat(lint): warn on same-track clips that touch at an exact boundary #1884

Open

4 tasks

miguel-heygen mentioned this pull request Jul 4, 2026

Fix render blank output when <html dir="rtl"> changes capture behavior #1934

Open

miguel-heygen changed the title ~~feat(lint): flag dir="rtl" on <html> as a confirmed silent render failure~~ feat(lint): warn when <html dir> can blank render output Jul 4, 2026

fix(lint): only flag valid non-ltr html dir values

3b42b82

james-russo-rames-d-jusso reviewed Jul 4, 2026

View reviewed changes

vanceingalls reviewed Jul 4, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

feat(lint): warn when <html dir> can blank render output#1893

feat(lint): warn when <html dir> can blank render output#1893
miguel-heygen wants to merge 2 commits into
mainfrom
fix/lint-html-dir-breaks-render

miguel-heygen commented Jul 3, 2026 •

edited

Loading

Uh oh!

james-russo-rames-d-jusso left a comment

Uh oh!

vanceingalls left a comment

Uh oh!

james-russo-rames-d-jusso left a comment •

edited

Loading

Uh oh!

vanceingalls left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

miguel-heygen commented Jul 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Investigation notes (please read before assuming this is "fixed")

Fix (this PR)

Test plan

Follow-up

Uh oh!

james-russo-rames-d-jusso left a comment

Choose a reason for hiding this comment

🟢 Approve — root-cause fix at the source, not per-rule special-casing

Uh oh!

vanceingalls left a comment

Choose a reason for hiding this comment

🟠 R1 verdict — advisory lint rule mechanics are correct; one attribute-shape edge case worth naming

Finding-by-finding

1. Attribute-shape audit (dir is enumerated, not boolean) — 🟠 minor false-positive risk

2. Advisory-only posture — 🟠 (Rames covered this; I concur on the framing tightening)

3. Rule execution / no cross-rule interaction — 🟢

4. Source-scan side effect — 🟢

Uh oh!

james-russo-rames-d-jusso left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

🟢 R2 verdict — Via's tighter-shape recommendation landed cleanly. Approve stands.

Uh oh!

vanceingalls left a comment

Choose a reason for hiding this comment

🟢 R2 verdict — resolved

Finding-by-finding

1. Invalid dir values flagged as RTL despite spec-defined LTR fallback — ✅ resolved

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

miguel-heygen commented Jul 3, 2026 •

edited

Loading

1. Attribute-shape audit (`dir` is enumerated, not boolean) — 🟠 minor false-positive risk

james-russo-rames-d-jusso left a comment •

edited

Loading

1. Invalid `dir` values flagged as RTL despite spec-defined LTR fallback — ✅ resolved