feat(lint): flag relative-value second writers and tl.set initial hides

[xuanruli]

What

Part 2 of the GSAP seek-safety rules (stacks on #2611): the two rules that touch existing catalog content and required reconciliation with an existing rule.

• gsap_relative_value_second_writer (error) — a relative var value (y: "-=15") on a property whose target has another writer active at the relative tween's start. The relative base is captured at tween init, which reads a different partial state per seek path: sequential seek inits it mid-entrance, a cold render worker inits it at the entrance's end state, and the element teleports at chunk boundaries (production case: all scene nodes jumping ~20px mid-scene). Writers that complete strictly before the start are safe (children render in start-time order within a seek pass — verified against gsap 3.15.0) and are not flagged; neither are single-writer relatives, from()/fromTo(), build-time gsap.set, or relative position parameters ("+=0.5"). Selector resolution bails on combinators and cross-composition scoping rather than guessing. Findings aggregate per tween pair and report the overlap window.
• gsap_timeline_set_initial_hide (warning) — initial-state hiding via tl.set(target, vars, 0) on a paused timeline is not rendered while the playhead sits at exactly 0, so frame 0 shows the unhidden state (verified against gsap 3.15.0: opacity stays 1 after tl.time(0), applies only past 0). Exempt when the target is already hidden by authored CSS/inline styles or a standalone gsap.set(), and only sets preceding every tween in source order qualify (mutated position variables resolve to their initial binding in the parser — outro hard-kills don't masquerade as position-0 sets).
• Reconciliation: gsap_fullscreen_overlay_starts_visible's fixHint previously recommended exactly the flagged tl.set(sel, {opacity:0}, 0) pattern; it now recommends authored CSS hiding or immediate gsap.set().
• Docs for the full rule family in docs/packages/lint.mdx.

Corpus impact (the reason this is its own PR)

These two rules are the ones that fire on repo-shipped content:

• gsap_relative_value_second_writer: 4 errors in gooey-metaball, all genuine overlaps. Measured with gsap 3.15.0: ballD diverges 3.31 xPercent / 1.99 yPercent (~8px/5px at 240px ball size) between sequential and cold seek — a permanent base offset that appears as a teleport at a chunk boundary. Real but modest; happy to fix the block in a follow-up (start the drift at the entrance's end, or use absolute fromTo).
• gsap_timeline_set_initial_hide: 10 warnings across the catalog after the CSS-hidden exemption (down from 54 pre-narrowing); spot-checked as genuine frame-0 pops with no authored hide (e.g. vfx-text-cursor #phrase-b).

Adversarially reviewed the same way as #2611 (393-composition corpus + gsap semantics experiments); FP classes fixed and locked as negative tests: precede-only second writers, descendant/cross-composition selector mis-joins, CSS-hidden re-assertions, mutated position variables.

Tests

Full packages/lint suite green at 440 tests including multi-composition roots; tsc, oxlint, fallow audit clean.

[xuanruli]

• feat(lint): flag relative-value second writers and tl.set initial hides #2612 👈 (View in Graphite)
• feat(lint): seek-order safety and SVG draw-on rules for GSAP timelines #2611
• main

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

[miga-heygen]

Review — feat(lint): flag relative-value second writers and tl.set initial hides

Scope: 3 files, +479/−2 — Part 2 of the GSAP seek-safety rules (stacks on #2611). Two new rules + fixHint reconciliation + docs.

SSOT inventory

Artifact	Role	Verdict
isRelativeTweenValue / RELATIVE_TWEEN_VALUE	Canonical predicate for +=/-= tween values	Clean — single owner
targetsShareElement	Canonical element-overlap check (identity + token-based intersection)	Clean — single consumer
selectorResolvesFaithfully	Predicate: no combinators, no attribute selectors (safe for token matching)	Clean — single consumer
resolveSelectorTagIndexes	Token → tag index resolution	Clean — single consumer
indexTagsByToken / collectCssOpacityZeroSelectors	Shared helpers (extracted in #2611)	Clean — reused here correctly
gsap_fullscreen_overlay_starts_visible fixHint	Reconciled to recommend gsap.set(...) instead of tl.set(..., 0)	Clean — cross-rule comment documents the invariant

What I checked

1. Temporal overlap logic. other.position > win.position || other.end <= win.position correctly excludes writers that complete strictly before the relative tween starts. The <= (not <) means a writer ending exactly at the start is safe — correct per the GSAP seek semantics described (children render in start-time order within a seek pass).

2. Exemptions. from()/fromTo() (resolve at build time via immediateRender), standalone gsap.set (end = 0, so other.end <= win.position catches it for any non-zero position; excluded from windows for position 0 by win.global), single-writer (no other with shared props), position parameters ("+=0.5" stays in position, not properties). All correct.

3. Selector resolution bail. selectorResolvesFaithfully rejects combinators, attribute selectors, and whitespace. The descendant-selector test case (.card-a .icon vs .card-b .icon) and cross-composition test case confirm the bail works — no false joins.

4. Initial-hide exemptions. win.global correctly exempts standalone gsap.set() (the fix). cssHiddenSelectors + alreadyHidden check covers both CSS-authored hides and standalone gsap.set hides. The hiddenByElement path handles the cross-selector case (id-targeted set over a class-hidden element). All 4 exemption test cases pass.

5. Source-order gate. firstTweenIndex ensures only sets before the first tween are considered — late hard-kills with mutated position variables can't masquerade as initial hides. Correct.

6. fixHint reconciliation. The old gsap_fullscreen_overlay_starts_visible hint recommended tl.set(selector, { opacity: 0 }, 0) — exactly what gsap_timeline_set_initial_hide now warns about. Updated to recommend gsap.set(...) (outside timeline) or authored CSS. Comment documents the cross-rule dependency.

7. Finding aggregation. Multiple relative props on the same tween pair produce one finding (the sharedProps join). Reversed pairs (both tweens have relative values on the same property) would produce two findings — correct, both are independently hazardous.

Verdict

Clean PR. No SSOT violations. Both rules are well-exempted (9 negative test cases for the second-writer rule, 3 for the initial-hide rule), the fixHint reconciliation is explicitly documented, and the shared helpers from #2611 are correctly reused without duplication.

— Miga

[james-russo-rames-d-jusso]

First-look adversarial review at HEAD 056171c1 (base lint/gsap-seek-safety-and-drawon — top of the two-PR stack, reviewed independently of #2611). Delta vs #2611's tip is +203 lines in gsap.ts (the two new rules + fixHint reconciliation) + 263 lines of new tests + 13 lines of docs.

Findings

Blockers: none.

Concerns: 1 medium-high (standalone gsap.set exemption ignores callsite scope) + 6 lows/mediums across the two rules. All inline.

Nits: 0.

Green surface (verified)

• fixHint reconciliation with gsap_fullscreen_overlay_starts_visible — done correctly. gsap.ts:1143-1145 new fixHint recommends authored CSS hiding or immediate gsap.set() (outside the timeline) — the old fixHint on main recommended exactly the tl.set(sel, {opacity:0}, 0) pattern this PR now flags. Coordination is clean; the two rules no longer contradict.
• Rule 7 immediate-set exemption reach — extractStandaloneHiddenSelectors (gsap.ts:205) picks up top-level gsap.set(sel, { opacity: 0 }) calls and aliased-selector forms. Basic case works.
• Rule 7 CSS-opacity-zero exemption (collectCssOpacityZeroSelectors, pre-existing) — reused correctly; the CSS-hidden test at gsap.test.ts:2455 proves the exemption fires.
• Rule 1 selector-safety bails — descendant / cross-composition selector mis-joins have a dedicated test (gsap.test.ts:2010) and are correctly not matched.
• Rule 1 aggregation — xPercent + yPercent collision on the same overlap window aggregates to one finding with both prop names in the message (gsap.test.ts:1936).
• Corpus true-positive claim verified: skills/music-to-video/references/motion-primitives/gooey-metaball/index.html:206,211,216,221,226 — #ballD (and siblings) has back-to-back relative-value .to() calls on xPercent/yPercent within overlapping windows. The 3.31 xPercent / 1.99 yPercent divergence claim is structurally plausible; can't verify the exact numeric without rendering a cold worker, but the pattern is real and matches the rule's target.

What I didn't verify

• The exact 3.31 xPercent / 1.99 yPercent divergence measurement — no test in the diff reproduces those numbers. Believing the pattern is real (verified above); the specific numeric is untested.
• Full bun test packages/lint run — trusting the CI green signal on Test job.
• GSAP version bind: the rule's core precede-only assumption ("children render in start-time order within a seek pass") is stated as "verified against gsap 3.15.0", but only sdk-playground pins ^3.15.0; corpus HTMLs load gsap@3.14.2 from jsdelivr and studio/player pin ^3.13.0/^3.12.5. No version guard in the rule. If a future gsap upgrade breaks the ordering invariant, this rule silently mis-classifies.
• The "mutated position variable resolves to initial binding" test coverage — the PR body claims this FP class is locked as a negative test, but the FP-guard tests at gsap.test.ts:2436 use only literal position values. Worth adding one test case that reassigns a position variable.

Merge gate

Both rules target real classes of production defects (the 8px/5px cold-seek teleport on gooey-metaball is a good example — worth fixing in a follow-up), the FP-guard design is careful, and the fixHint reconciliation is well-done. The medium-high concern (F1 standalone-set scope) is the one I'd want addressed before ship — click-handler / setTimeout gsap.sets silently exempting real bugs is easy to overlook and hard to reproduce later. The other concerns (implicit-position window arithmetic, string-position exclusion, immediateRender: true opt-in, zero-duration .to() at position 0) are worth author judgment before ship, all ≤10-line fixes.

LGTM on the mechanism + reconciliation; holding F1 (scope of standalone-set exemption) as the substantive concern worth Xuanru's decision. Leaving as COMMENTED.

— Review by Rames D Jusso

[james-russo-rames-d-jusso]

R2 delta review from 056171c1 → 726935b3 (+204 lines gsap.ts, +353 lines gsap.test.ts on top of the new #2611 base at c2d9ac0b). All R1 findings addressed with code + test; one low-severity trade-off note on the new function-range guard.

R1 concerns → R2 disposition

#	R1 concern	R2 disposition
F1	extractStandaloneHiddenSelectors walked the whole script — gsap.set(...) in a click handler / setTimeout counted as an initial hide and silently exempted a real tl.set(..., 0) bug	🟢 Fixed: new collectFunctionBodyRanges + indexInsideAnyRange(match.index, functionRanges) guard skips any gsap.set inside a function ... { … } or => { … } body. Test gsap_timeline_set_initial_hide: does NOT exempt a gsap.set nested inside a callback covers. See C1 below for a note on the other end of the trade-off.
R1-a	Rule 1 arithmetic read .position (number-only) — dropped chained-implicit tweens (tl.to(a,…) after tl.to(b,…, "+=0")) whose parser-side position is a string but whose resolvedStart is numeric	🟢 Fixed: extractGsapWindows now uses animation.resolvedStart ?? (typeof animation.position === "number" ? animation.position : null), so string-position tweens participate. Applies uniformly to rule 1 (gsap_relative_value_second_writer) and rule 2 (gsap_timeline_set_initial_hide) via the shared helper.
R1-b	repeat: -1 produced a finite end = position + duration * 1 → an infinite writer's overlap window was under-reported	🟢 Fixed: infiniteRepeat = repeat < 0 branch sets end = Number.POSITIVE_INFINITY (for non-set), and rule 1's formatTime prints ∞. Any writer that starts after an infinite one now correctly reports the open-ended overlap.
R1-c	No opt-out for the sanctioned second-writer fix — overwrite: "auto" on the tween itself kills concurrent writers at init, so it's a false alarm	🟢 Fixed: rule 1 now short-circuits with if (win.overwriteAuto) continue;. Test gsap_relative_value_second_writer: does NOT flag when the relative writer has overwrite auto.
F2	Zero-duration .to() / .fromTo() at position 0 escaped rule 2 (only method === "set" was treated as instant-hold)	🟢 Fixed: new isInstantHold predicate covers set and any to/fromTo where win.end === win.position. Test warns on zero-duration tl.to at position 0.
F3	immediateRender: true was not honored — the per-tween opt-in for GSAP to render at frame 0 was ignored	🟢 Fixed: immediateRender field is now on GsapWindow, and rule 2 has if (win.global || win.immediateRender) continue;. Test does NOT warn when immediateRender is true.
nit-2436	Test file nit (backtick-in-tokenizing snippet)	🟢 Fixed (rolled into the svg_drawon_css_dasharray_conflict fixHint cleanup in #2611).

Additional net-new checks I ran on R2 code

• targetsShareElement: new helper collapses #foo + .bar writer-vs-writer detection when both selectors resolve to the same element. Reads correctly:
  • identity/selector equality path handles the "both stable" case,
  • selectorResolvesFaithfully bails on combinators (.card .icon) and attribute selectors ([data-composition-id="c1"] .dot) — the right conservative shape,
  • resolveSelectorTagIndexes intersection catches the cross-token overlap.
• Rule 2's initialHolds prefix (windows before the first non-instant-hold in source order) correctly excludes late-position hard-kills whose parser-resolved position happens to be 0 — matches the comment's stated invariant. Test does NOT warn on mutated position variables resolved as 0.
• Rule 2's fix-hint on gsap_fullscreen_overlay_starts_visible was updated from tl.set("…", { opacity: 0 }, 0) to gsap.set("…", { opacity: 0 }) outside the timeline — necessary to keep the two rules' advice consistent (the old hint would trigger the new rule). Nice catch during the restack.

Concerns

• C1 [low, design trade-off — flagging for your judgement] collectFunctionBodyRanges is now under-inclusive for IIFE- and DOMContentLoaded-wrapped scripts. The pendulum swung the other way from F1: any script wrapped in (function() { … })() or document.addEventListener("DOMContentLoaded", () => { … }) — a common studio-composition idiom — has all its gsap.set(...) calls inside a matched function range, so extractStandaloneHiddenSelectors returns empty. If the author has both a load-time gsap.set('#foo', {opacity: 0}) (which DOES hide at load, for the IIFE case) and a defensive tl.set('#foo', ..., 0), rule 2 will now warn on the defensive tl.set. Two considerations that make this defensible:
  1. Severity is warning, and the fix-hint tells the author how to satisfy the rule (CSS or lift the gsap.set out of the wrapper).
  2. For DOMContentLoaded specifically, the warning is actually correct — cold render workers may render frame 0 before DOMContentLoaded fires, so the gsap.set won't have run yet and #foo will show through.
     IIFE is the case that stings — synchronous execution during script parsing DOES hide at load. Worth an in-comment acknowledgement of the trade-off, or an explicit IIFE-detection pass if you want to be surgical about it. Not a blocker — the tests demonstrate the shape you intended.
     Also note the range-collection regex openerPatterns = [/\bfunction\b…\{/g, /=>\s*\{/g] misses method-shorthand ({ handler() { … } }) — even rarer authoring shape, mentioning only for completeness.

Nits

• N1 collectFunctionBodyRanges uses raw regex over the (comment-stripped) source without a string-literal guard — a string containing => { or a function-looking substring can produce a spurious range. The matchBalanced walk then finds a random } and defines a garbage range. Likelihood is very low (needs a very specific literal shape), but it's the same class of "regex-over-JS" scan as pattern itself. If you're doing another pass on the regex layer, consider dropping strings the same way comments are dropped. Otherwise skip.

What I didn't verify

• Did not run the test suite locally at 726935b3; trusting CI at head.
• Did not audit gsap_timeline_set_initial_hide for every combination of hiddenByToken × hiddenByElement × hides branches — the shape reads clean and the test file adds targeted coverage per new predicate.
• Skipped a deep look at gsap.test.ts:2436 because R1's nit-2436 disposition rolled into #2611's fixHint fix.

LGTM from my side. All 7 prior findings resolved with matching tests; one low-severity design-trade-off worth acknowledging (C1). Leaving as COMMENTED — approval is James's call.

— Review by Rames D Jusso

[xuanruli]

@james-russo-rames-d-jusso re R2 C1: fixed — IIFE-wrapped gsap.set again counts as a load-time hide exemption; callback/DOMContentLoaded bodies still do not. Test locked in on #2612 tip.

[miguel-heygen]

Reviewed exact head 61cb7d2a.

I read the complete three-file stacked diff and the full prior review history. The current head resolves the callback-scope and IIFE exemption trade-off with focused regression coverage, preserves the conservative selector and timing boundaries, and has no unresolved review threads. The PR is mergeable and all applicable required checks, including Graphite mergeability, are green.

Verdict: APPROVE
Reasoning: The prior findings are resolved on the current head, no blocking correctness issue remains, and required CI is green.

— Magi

The base branch was changed.

[xuanruli]

@miguel-heygen — your APPROVE on 61cb7d2a got dismissed by GitHub after we retargeted this PR onto main (post–#2611 squash merge).

No code change in the retarget: gsap.ts / gsap.test.ts / core.ts / core.test.ts are byte-identical to the stamped tip; the PR patch is still the same 3 files / +608. New head is 3e3d865b — could you re-stamp when you have a sec?

Still waiting on Tests on windows-latest for mergeability.

[xuanruli]

Merge activity

• Jul 17, 6:49 AM UTC: @xuanruli merged this pull request with Graphite.