fix(lint): stop overlapping_gsap_tweens flagging distinct unresolved targets

[miguel-heygen]

Problem

The GSAP parser assigns the sentinel __unresolved__ to any tween whose target it cannot statically resolve to a concrete element (a computed variable, a helper call, an expression, etc.). The overlapping_gsap_tweens rule then compared tweens by that target string.

The result: two tweens aimed at completely different elements through unresolvable selectors (for example #s0 .hl .w and #s1 .hl .w, or two pickWord(i) helper calls) both collapse to the single __unresolved__ string and get reported as overlapping the same element. That is a false positive, and these noisy reports were appearing on perfectly valid timelines.

Fix

An unresolved target is an unknown element. Two unresolved targets are not provably the same element, so an overlap between them cannot be asserted. Skip overlap analysis when the tween target is the __unresolved__ sentinel. Genuine overlaps on a resolved element are still flagged exactly as before.

Tests

Added a case in gsap.test.ts: two tweens at the same time targeting distinct elements via an unresolvable helper call must not produce an overlapping_gsap_tweens finding. Full lint package suite passes (263 tests).

[james-russo-rames-d-jusso]

Verified at HEAD 50aec8b38a. Clean correctness fix. One sibling-asymmetry concern worth a look.

✅ What works

• The fix at packages/lint/src/rules/gsap.ts:557 is exactly right: __unresolved__ is a sentinel for "I couldn't statically resolve this," and two unresolved selectors are not provably the same element. Asserting overlap between them is unsound. Short-circuiting via the outer-loop left guard cleanly handles both the (unresolved, unresolved) and (unresolved, resolved) cases — the (resolved, unresolved) case was already handled by the existing equality check at :562.
• The UNRESOLVED_TARGET = "__unresolved__" constant with its rationale comment at :53-57 is the right shape. Centralizes the sentinel so future readers don't grep for the string.
• Test at gsap.test.ts:955-979 is the canonical shape: two distinct unresolved targets via pickWord(0) / pickWord(1), same time, must NOT report. Bounds the FP.

🟡 Sibling-rule asymmetry — worth a one-pass check

Other rules in the same gsap.ts file read win.targetSelector and may also produce nonsense output / silent misbehavior when the selector is __unresolved__:

• gsap_exit_missing_hard_kill (gsap.ts:587-602) — emits GSAP exit on "__unresolved__" ends at the ...s clip start boundary ... and a fix-hint of tl.set("__unresolved__", {...}, X). The message is technically suppressible by isHardKillSet returning true for the unresolved sentinel, but a user-facing message containing the literal sentinel is confusing. Probably want to skip this rule too when win.targetSelector === UNRESOLVED_TARGET.
• unscoped_gsap_selector (gsap.ts:686-694) — isSuspiciousGlobalSelector("__unresolved__") likely returns false (the sentinel isn't a CSS selector shape), so this is probably safe-by-accident. Worth confirming with a one-line test.
• gsap_animates_clip_element (gsap.ts:663-682) — clipIds.get("__unresolved__") returns falsy, so continue. Safe.

Not a blocker for this PR — your fix correctness is bounded to overlapping_gsap_tweens per the title — but if you're touching the unresolved-target handling here, a follow-up PR (or extending scope here) to gate the other rules on the same sentinel would close the cluster cleanly. The user-facing message in gsap_exit_missing_hard_kill is the highest-leverage one.

✅ Test coverage shape

Negative regression assertion is implicit in "Genuine overlaps on a resolved element are still flagged exactly as before" (PR body). Existing tests in gsap.test.ts covering resolved-target overlap should hold — the guard is keyed only on the sentinel string. No belt-and-suspenders test needed; this is a "we already test resolved, now we don't FP on unresolved" shape.

LGTM from my side — leaving as a comment.

— Review by Rames D Jusso

[vanceingalls]

Review — fix(lint): stop overlapping_gsap_tweens flagging distinct unresolved targets

Verdict: LGTM with nits
Repo: heygen-com/hyperframes Head: 50aec8b
Scope: packages/lint/src/rules/gsap.{ts,test.ts} — sentinel-collapse fix: skip the outer-loop tween when its targetSelector is the __unresolved__ sentinel, so two distinct unresolved targets no longer falsely collide on the equality check at :562.

Summary

__unresolved__ is the parser's "I can't statically resolve this target" sentinel. Treating it as an identity in the overlap predicate was unsound — two unresolved selectors are not provably the same element. The fix is the correct one-sided guard: skipping left removes the (unresolved, unresolved) and (unresolved, *) cases; the existing equality check at :562 already handles (resolved, unresolved) (different strings ⇒ no match). The named constant is the right factoring. Convergent with Rames on the sibling-rule asymmetry note; one additional under-reporting trade-off worth a sentence.

Findings

💭 — legitimate unresolved-self overlap is now silently allowed

File: packages/lint/src/rules/gsap.ts:550-580

A timeline that tweens the same element twice via the same unresolvable helper call (e.g. pickWord(0) invoked twice at overlapping times, where both calls return the same element) used to be reported (sentinel-equality firing the FP, but matching reality in this niche case) and is now silently allowed. The new test at gsap.test.ts:955-979 deliberately exercises pickWord(0) vs pickWord(1) — distinct results.

This is the right call: the linter cannot prove sameness without static resolution, so it cannot assert overlap. But it does mean that authors who deliberately use a single helper twice on the same element will lose this category of warning. Worth a line in the rule docstring (or RuleId summary) that overlap is checked only against statically-resolved selectors — sets expectations.

💭 — sibling-rule asymmetry — convergent with Rames

File: packages/lint/src/rules/gsap.ts:582-606 (gsap_exit_missing_hard_kill)

I independently audited the other rules in gsap.ts that read win.targetSelector:

• gsap_exit_missing_hard_kill (:582-606) — formats the literal "__unresolved__" into both the user-facing message and the tl.set("__unresolved__", ...) fix-hint. Confusing. The hard-kill discovery via isHardKillSet(candidate, win.targetSelector, boundary) likely never matches the sentinel either, so the rule will tend to fire spuriously on unresolved exits AND print a useless fix-hint. Highest-leverage follow-up.
• unscoped_gsap_selector (:686-694) — isSuspiciousGlobalSelector("__unresolved__") almost certainly returns false (it isn't a CSS selector shape), so safe-by-accident.
• gsap_animates_clip_element (:663-682) — clipIds.get("__unresolved__") returns falsy → continue. Safe.
• gsap_fullscreen_overlay_starts_visible (:608+) — uses visibilityWindows filtered against tag selectors; sentinel won't match a real selector. Safe-by-accident.

Not blocking this PR — its title is bounded to overlapping_gsap_tweens — but a stacked follow-up gating these on the sentinel would close the cluster cleanly. The gsap_exit_missing_hard_kill message contamination is the user-visible item; the others are robust by coincidence. Convergent with Rames.

🟢 — constant + comment hygiene is exactly right

File: packages/lint/src/rules/gsap.ts:46-50

Hoisting UNRESOLVED_TARGET = "__unresolved__" with the "this is a sentinel, not an identity" rationale is the load-bearing piece for future readers — the band-aid trap on this PR was an author somewhere writing if (sel === "__unresolved__") deep in a sibling rule. Centralizing the magic string lets a grep find every consumer. Good.

🟢 — test bounds the FP precisely

File: packages/lint/src/rules/gsap.test.ts:955-979

Two tweens, same time, distinct unresolved targets via pickWord(0) / pickWord(1), asserting overlapping_gsap_tweens is not reported. This is the rubric question-2 (inverse-direction risk) check from the catch side: the test asserts the FP is gone, and the body comment names the structural reason. Existing resolved-target overlap tests in the file hold by inspection — the guard is keyed strictly on the sentinel string.

Verification

• Read packages/lint/src/rules/gsap.ts@50aec8b directly via gh api .../contents.
• Symmetry check: confirmed the inner-loop side does NOT need the same skip — the equality check at :562 short-circuits (resolved, unresolved) pairs because the strings differ. The (unresolved, resolved) pair is killed by the outer-loop guard, and (unresolved, unresolved) is the case the outer-loop guard is meant to kill. All four quadrants handled.
• Sibling-rule audit (full pass, summarized in the asymmetry finding above). gsap_exit_missing_hard_kill is the only user-visible item.
• Inverse-input check: same-element-via-same-helper-call overlap is now silently allowed (documented above as a niche trade-off, not a regression-class bug).
• Convergent with Rames D Jusso (james-russo-rames-d-jusso, comment posted 17:11 UTC) on the sibling-asymmetry concern; my audit independently reached the same gsap_exit_missing_hard_kill finding.

---

Review by Via

[jrusso1020]

Approved — independently confirmed CI green at HEAD and the change matches its described scope; converges with RDJ + Via's LGTM (their detailed reviews stand). — Rames Jusso

[james-russo-rames-d-jusso]

R2 verification

HEAD 50aec8b38a → 28a4a1c6fe. Tight delta: +34 / -0 across gsap.ts (+3) and gsap.test.ts (+31).

My R1 🟡 — sibling rule gsap_exit_missing_hard_kill should skip UNRESOLVED_TARGET

Status: ✅ resolved (canonical fix shape)

packages/lint/src/rules/gsap.ts:584-586 now has:

// Unresolved targets are unknown elements: you cannot assert a missing
// hard kill on one, and a `tl.set("__unresolved__", ...)` hint is meaningless.
if (win.targetSelector === UNRESOLVED_TARGET) continue;

Inside the gsapWindows loop in gsap_exit_missing_hard_kill, gating on UNRESOLVED_TARGET before the boundary / hard-kill checks. Exactly the shape I suggested — short-circuits both the confusing GSAP exit on "__unresolved__" user-facing message and the meaningless tl.set("__unresolved__", ...) fix-hint.

Regression test at gsap.test.ts:1008-1037 is the canonical co-occurrence shape: one unresolved exit + one resolved exit in the same timeline, asserts the unresolved one does NOT fire and the resolved #headline one still does (expect(exitFindings).toHaveLength(1); expect(exitFindings[0]?.selector).toBe("#headline")). Bounds both sides of the fix correctly.

Other 🟡 from R1 (unscoped_gsap_selector, gsap_animates_clip_element)

I'd noted these were safe-by-accident / already-safe and not blocking. Confirmed unchanged — fix correctly scoped to the one rule that needed the explicit guard.

Adjacent issue introduced by the fix

None. Pure-additive guard + test. No other rule in gsap.ts references UNRESOLVED_TARGET newly. Clean.

Net verdict

LGTM — my R1 sibling-asymmetry concern is fully resolved with the canonical guard shape. Approving on my side; converges with Rames Jusso's APPROVE at 50aec8b.

— Review by Rames D Jusso

[vanceingalls]

R2 — fix(lint): gsap overlap — sibling-symmetry nit landed

Verdict: 🟢 LGTM — R1 nit cleared
Repo: heygen-com/hyperframes R1 head: 50aec8b R2 head: 28a4a1c
Status: R1 sibling-symmetry nit addressed in new commit 28a4a1c fix(lint): guard gsap_exit_missing_hard_kill against the unresolved-target sentinel.

R1 → R2 delta

R1 🟡 (Rames-convergent) — gsap_exit_missing_hard_kill sibling-asymmetry

Status: ✅ resolved

packages/lint/src/rules/gsap.ts:584-587 at 28a4a1c:

for (const win of gsapWindows) {
  // Unresolved targets are unknown elements: you cannot assert a missing
  // hard kill on one, and a `tl.set("__unresolved__", ...)` hint is meaningless.
  if (win.targetSelector === UNRESOLVED_TARGET) continue;
  if (!isSceneBoundaryExit(win)) continue;
  ...
}

Canonical guard shape, matches the original overlapping_gsap_tweens skip at :557. Regression test at gsap.test.ts:1008-1037 (co-occurrence of one unresolved exit + one resolved exit, asserts only #headline fires) bounds both sides.

Converges with Rames Jusso's R2 (17:33:59 UTC) — independently verified the same guard shape and regression test.

Verification

• Direct fetch of gsap.ts@28a4a1c and gsap.test.ts@28a4a1c via gh api .../contents.
• Pure-additive guard + test — no other rule in gsap.ts was modified.
• Diff is +34 / -0 across gsap.ts (+3) and gsap.test.ts (+31), per gh api /pulls/1798/files.

---

R2 by Via

[jrusso1020]

Re-approving at the new head — the gsap_exit_missing_hard_kill UNRESOLVED guard + regression test landed (RDJ R2 confirmed). — Rames Jusso