Skip to content

test(producer): regenerate sub-comp-t0 baseline + add gsap_from_opacity_noop lint rule#1001

Merged
miguel-heygen merged 4 commits into
mainfrom
fix/sub-comp-t0-baseline-regen
May 21, 2026
Merged

test(producer): regenerate sub-comp-t0 baseline + add gsap_from_opacity_noop lint rule#1001
miguel-heygen merged 4 commits into
mainfrom
fix/sub-comp-t0-baseline-regen

Conversation

@miguel-heygen
Copy link
Copy Markdown
Collaborator

@miguel-heygen miguel-heygen commented May 21, 2026

Summary

Two changes:

  1. Regenerate sub-comp-t0 baseline — missed when text-rendering: geometricPrecision was introduced (b7bd956). Same root cause as the 3 baselines already regenerated. Also improves fixture visual contrast (backgrounds from near-black to visible slate tones).

  2. New lint rule: gsap_from_opacity_noop — catches when an element has CSS opacity: 0 AND is targeted by gsap.from({opacity: 0}). Since from() animates FROM the specified value TO the CSS value, this produces a 0→0 animation where the element never becomes visible. This was the root cause of all-black renders from the product-launch-video skill. Fires as error (not warning) to block the render pipeline.

    ✗ gsap_from_opacity_noop: "#title" has CSS `opacity: 0` and a gsap.from()
      that also sets opacity to 0. gsap.from() animates FROM the specified value
      TO the current CSS value — since CSS is already 0, the element animates
      from 0→0 and never becomes visible.
    

Test plan

  • sub-comp-t0 baseline regenerated inside Dockerfile.test
  • docker:test sub-comp-t0 passes: 0 failed frames, 100/100 checkpoints
  • gsap_from_opacity_noop — 4 test cases: inline style, style block, clean code (no FP), gsap.to exit (no FP)
  • All 40 gsap lint tests pass
  • CI regression suite passes

🤖 Generated with Claude Code

miguel-heygen and others added 2 commits May 21, 2026 17:19
…rast

The text-rendering:geometricPrecision rule (b7bd956) shifted glyph advances
~1% in headless Chrome. Baselines for png-sequence, heygen-promo-preview-assets,
and sub-composition-video were already regenerated — sub-comp-t0 was missed.

Also improves the fixture's visual contrast:
- Background: #07110d (near-black) → #0f172a (slate-900)
- Hook scene: background #1e293b, text #ef4444/#ffffff
- Later scene: background #334155, text #ffffff
- Absolute positioning for text elements (compiler inlines sub-comp
  content at intrinsic width, collapsing the flex container)

Baseline regenerated inside Dockerfile.test per CLAUDE.md. Test passes:
0 failed frames, 100/100 checkpoints green.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Detects when an element has CSS `opacity: 0` (inline or style block) AND
is targeted by gsap.from({opacity: 0}). Since from() animates FROM the
specified value TO the CSS value, this produces a 0→0 animation where
the element never becomes visible.

Root cause of all-black renders from the product-launch-video skill:
every text element had opacity:0 in CSS + gsap.from({opacity:0}),
making all text permanently invisible despite the timeline "working."

Fires as error (not warning) to block the render pipeline. Includes
actionable fix hint. 4 test cases: inline style, style block, clean
code (no false positive), and gsap.to() exit (no false positive).

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@miguel-heygen miguel-heygen changed the title test(producer): regenerate sub-comp-t0 baseline for geometricPrecision test(producer): regenerate sub-comp-t0 baseline + add gsap_from_opacity_noop lint rule May 21, 2026
Copy link
Copy Markdown
Collaborator

@jrusso1020 jrusso1020 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verdict: COMMENT (would-be APPROVE pending one item)

Two-part PR — separating the review by piece.


Part 1: gsap_from_opacity_noop lint rule

Solid catch — the "all-black render" failure mode is exactly the kind of agent-mistake-class that deserves an error-severity lint. Good fixture coverage on the four canonical cases (inline + style-block positive, clean negative, gsap.to negative).

One substantive concern: fromTo will produce false positives

The rule triggers on both from AND fromTo (gsap.ts:868), but those methods have different destination semantics:

  • gsap.from(target, vars) — animates FROM vars TO the current CSS value. If CSS opacity is 0 AND vars.opacity is 0, animation is 0→0. ✓ This is the agent-bug class the rule targets.
  • gsap.fromTo(target, fromVars, toVars) — animates FROM fromVars TO toVars. The destination is toVars, not current CSS. So gsap.fromTo("#hero", { opacity: 0 }, { opacity: 1 }) with CSS opacity:0 on #hero produces a perfectly valid 0→1 animation — the inline opacity:1 overrides CSS at animation end. This is the defensive fade-in pattern (explicit start AND end). It's actually the exact pattern used in this PR's own regenerated fixture (packages/producer/tests/sub-comp-t0/src/compositions/hook.html:54-58 after the change).

Running the rule on fromTo("#hero", { opacity: 0 }, { opacity: 1 }) with CSS opacity:0:

  1. cssOpacityZeroSelectors set includes #hero
  2. extractGsapWindows → method fromTo, meta.properties is the union of fromVars + toVars keys (from parseGsapWindowMeta:200-206), so ["opacity"] regardless of values
  3. Rule check if (win.method !== "from" && win.method !== "fromTo") continue; — passes
  4. if (!win.properties.includes("opacity")) continue; — passes
  5. cssKey "#hero" in set — passes
  6. Fires → render pipeline blocks on a legitimate composition

Note that the rule's message already only mentions from():

gsap.from() animates FROM the specified value TO the current CSS value...

And the fixHint only mentions gsap.from({opacity:0}). So the implementation includes fromTo but the messaging assumes from-only — suggesting fromTo was added without intent.

The regenerated sub-comp-t0 fixture doesn't trip this only because it uses scope + " .hook-title" (concatenated selector), which parseGsapCall:157 rejects (requires a quoted literal as first arg). So the PR doesn't visibly trigger its own rule against itself — but the moment an author/agent writes a literal-selector fromTo({opacity:0}, {opacity:1}), the rule fires as error and blocks the render.

Suggested fix (one line):

-      if (win.method !== "from" && win.method !== "fromTo") continue;
+      if (win.method !== "from") continue;

Alternative: if you want to keep fromTo coverage for the truly-broken fromTo({opacity:0}, {opacity:0}) case, check meta.propertyValues.opacity === 0 (currently propertyValues is only the toVars per parseGsapWindowMeta:211). But that case is so rare I'd just drop fromTo entirely.

A fromTo({opacity:0}, {opacity:1}) test case would catch this — and would fail in the current implementation, confirming the bug.

Non-blocking observations on the CSS-selector matcher

The regex /([#.][a-zA-Z0-9_-]+)\s*\{([^}]+)\}/g (gsap.ts:849) has several silent limitations that are fine for the current scope but worth a comment in the rule:

  1. Compound selectors: #hook .hook-title { opacity: 0 } matches only .hook-title (the rightmost [#.]name before {). If a gsap call uses the literal "#hook .hook-title", the rule's selector normalization (if (!startsWith("#") || ".") is already true → uses sel as-is) doesn't match .hook-title in the set → false negative. Combined with the scope + concatenation issue, the rule won't catch nested-scoped patterns that hyperframes' own conventions encourage.

  2. Pseudo-classes: #hero:hover { opacity: 0 } — the regex stops at : so #hero gets captured + body has opacity: 0 → adds #hero to set. But the CSS only applies on hover; the element's normal opacity is whatever it is. False-positive risk on hover/focus rules.

  3. Comma-separated selectors: #hero, #title { opacity: 0 } — regex matches only #title (rightmost). #hero missed. False negative.

  4. opacity: 0 followed by opacity: 1 in a later rule for the same selector — regex doesn't track override; first match wins for the set. False positive in theory; rare in practice.

None of these are blockers — the rule is intentionally narrow and targets the agent-generated pattern. But a one-line comment like // Note: matches only single ID/class selectors at the leaf; doesn't handle compound, pseudo-class, or comma-separated forms would save the next reader a re-derivation.


Part 2: sub-comp-t0 baseline regen

The PR description frames this as "regenerate baseline (missed when text-rendering: geometricPrecision was introduced)" with a parenthetical "also improves fixture visual contrast." That undersells the change: the source compositions (src/compositions/hook.html, src/compositions/later.html, src/index.html) are substantially rewritten — backgrounds changed from near-black to slate/red/blue, layout changed from flex-centered to absolute-positioned, new .hook-bg / .later-bg elements added, font sizes increased, colors changed to white. This is a fixture rewrite, not a re-render.

Sanity-checked the rewrite doesn't lose the regression's value: the fixture's purpose is testing sub-composition at data-start=0.001 (the t=0 trigger bug). Both rewritten scenes still use data-start="0.001" and data-start="2.5", so the timing assertion still exercises the original bug class. ✓

Scope-creep note: bundling a fixture rewrite with a new lint rule is borderline. Both are small and reviewable in one pass, but the lint rule deserves its own focused PR for git blame clarity. Not a blocker — just worth splitting in the future. If the lint rule needs a follow-up (e.g., the fromTo fix), the fixture-rewrite half being already-merged makes that follow-up smaller.


CI

All required green so far. Windows checks + regression-shards still in_progress (matches recent PR cadence). The regression-shards validating the regenerated baseline is the load-bearing one.

— Rames Jusso

gsap.fromTo(target, fromVars, toVars) animates to toVars, not to
the current CSS value — so fromTo({opacity:0}, {opacity:1}) with
CSS opacity:0 is a legitimate 0→1 fade-in, not a noop. The rule
was false-positiving on these calls with error severity, which
would block the render pipeline.

Drop the fromTo branch from the trigger guard and add a test case
that proves fromTo does not fire.
Previous baseline was generated on arm64 host, causing 87 failed
frames in CI (amd64). Regenerated with --platform linux/amd64 to
match CI's Chrome/FFmpeg pixel output.
@miguel-heygen miguel-heygen merged commit 4c358e6 into main May 21, 2026
46 checks passed
@miguel-heygen miguel-heygen deleted the fix/sub-comp-t0-baseline-regen branch May 21, 2026 19:42
WaterrrForever added a commit that referenced this pull request May 22, 2026
transitions/overview.md rule #2 and gsap-easing-and-stagger.md stagger
examples used gsap.from({opacity: 0}), which contradicts the
"prefer fromTo() for entrances" guidance in gsap-timeline-and-labels.md,
motion-principles.md, and sub-compositions.md. The inconsistency teaches
agents the from() pattern, which becomes a 0→0 noop (all-black render)
when paired with CSS opacity: 0 — exactly the case PR #1001
adds an error-level lint rule for. Aligning all 5 skill files on fromTo().
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants