refactor(studio): dedup shadow numeric-equal + GSAP script extraction#1516
Merged
Conversation
This was referenced Jun 16, 2026
Collaborator
Author
james-russo-rames-d-jusso
left a comment
There was a problem hiding this comment.
Cluster context — Reviewing alongside #1507 + #1508 (the two FP-suppression PRs whose helpers this dedups).
Looks good to me at comment quality — no blockers.
Verified — dedup preserves both PRs' semantics
relEqualbody in newsdkShadowNumeric.tsis byte-identical to the inlined version:a === b || Math.abs(a - b) <= 1e-6 * Math.max(1, |a|, |b|). Tolerance formula and short-circuit ordering both preserved. No drift between #1507's GSAP-fidelity path and #1508's timing path — which is the whole point of extracting it, so future-edits to the tolerance can't accidentally desynchronize the two.numericEqualinsdkShadowGsapFidelity.tscorrectly retains its NaN-guard + string-coercion wrapper aroundrelEqual(call sites passunknown, notnumber). Wrapper kept locally, helper extracted — correct factoring.timingFieldEqualswap (timingValuesEqual→relEqual) is a straight rename; thekey !== "trackIndex"gate is preserved, so integer track-slot comparison stays exact. Verified.extractGsapScriptexport fromsdkShadowGsapFidelity.tsand re-import insdkShadowGsapKeyframe.ts— body is byte-identical to the deleted clone (regex pattern, marker setgsap/__timelines/ScrollTrigger). One source of truth, must stay in sync withdocument.ts's extractor (comment correctly calls this out).- New file
sdkShadowNumeric.tsis the right cycle-break:sdkShadow↔sdkShadowGsapFidelitywould have created an import cycle ifrelEquallived in either of them. The standalone helper file avoids it cleanly.
Concerns (non-blocking)
- PR body claims "no behavior change" — verified at the diff level. The risk I'd watch for on this kind of dedup is a subtle short-circuit reordering (e.g.
a === breturning early before NaN check), but the dedup preserves the order. Tests in #1507 / #1508 + the existing shadow suites (timing/fidelity/keyframe all green per PR body) should catch any regression. - No new tests in this PR — appropriate for a pure-refactor PR whose semantics are covered by the inherited test surface from #1507 / #1508. Worth a quick CI cycle to confirm those suites still pass at this tip SHA before merge.
Cross-PR note (stack hoist check)
- Per
feedback_graphite_stack_fix_hoist_to_tip— I scanned this PR's cumulative diff for any behavior change that should have been amended into #1507/#1508 instead. Found none: this PR strictly extracts duplicated code, no new logic, no fixes. Author didn't hoist. Clean refactor. - Base ref is
fix-studio-ease-race— stacked on #1512, not directly on #1507/#1508. PR body confirms "stacked on #1512" — sequencing is consistent.
Stamp recommendation — fine to land after #1507 + #1508 + the rest of the stack (#1509–#1512) land. I'm not stamping (review at --comment quality).
— Rames D Jusso
miga-heygen
previously approved these changes
Jun 16, 2026
miga-heygen
left a comment
There was a problem hiding this comment.
Approve. Clean dedup refactor — net -13 lines, no semantic changes.
Both extractions preserve behavior exactly:
relEqualinsdkShadowNumeric.tsmatches the original tolerance formula (1e-6 * max(1, |a|, |b|)witha === bfast path) in all three original call sites.extractGsapScriptconsolidation eliminates a byte-identical copy from the Keyframe module, keeping the canonical version co-located with the marker set it must stay in sync with.
The import-cycle avoidance rationale (dedicated leaf module for the shared helper) is sound and well-documented.
— Miga
1aada07 to
4d9e9a8
Compare
2edde6e to
4840ae2
Compare
4d9e9a8 to
f33a974
Compare
d3baf12 to
ffcef4f
Compare
f33a974 to
ebca753
Compare
…ace) Rapid GSAP edits (ease/duration/keyframe/property) fired overlapping read-modify-write POSTs to one script file — coalesceKey only dedupes edit history, not requests. The gsap_fidelity shadow then diffed an op against whichever POST's scriptText resolved, which could predate that op → false "expected null, actual power2.out" mismatches. Server persists correctly; a pure client request-pairing race. Adds createKeyedSerializer (per-key promise chain, rejection-safe, self- cleaning). commitMutation now serializes every GSAP-script commit per target file by default (key `gsap-file:<path>`) — covering all op types and all animations, not just one meta family — so same-file POSTs can't interleave. Distinct files run concurrently; an explicit serializeKey still overrides. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Code-review cleanup (no behavior change): - Extract the relative-epsilon float compare into shared sdkShadowNumeric.relEqual, used by both timing parity (sdkShadow) and GSAP value fidelity (numericEqual) — was duplicated verbatim, risking divergent tuning. - Export extractGsapScript from sdkShadowGsapFidelity and import it in the keyframe shadow instead of the byte-identical clone (the regex + marker set must stay in sync with document.ts; one copy is safer). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
ffcef4f to
1b02fcd
Compare
ebca753 to
168bb4f
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

What
Code-review cleanup, no behavior change — collapses duplication the max-effort review flagged on the shadow stack (#1507–#1512).
relEqual(newsdkShadowNumeric.ts) — the relative-epsilon float compare was duplicated verbatim between timing parity (sdkShadow.timingValuesEqual) and GSAP value fidelity (numericEqual). Extracted to one shared helper so the tolerance can't drift between paths. (A separate--numeric helper file avoids thesdkShadow↔sdkShadowGsapFidelityimport cycle.)extractGsapScript— was a byte-identical clone (regex + GSAP marker set) in bothsdkShadowGsapFidelityandsdkShadowGsapKeyframe. Now exported from the former and imported by the latter. The marker set must stay in sync withdocument.ts's extractor; one copy is safer.Verification
build clean · SDK 233 · studio 905 · oxlint/oxfmt clean. Shadow suites (timing/fidelity/keyframe) all green.
Stacked on #1512. The keyframe-reader-on-
ElementSnapshotaltitude item is left as a separate follow-up (net-new SDK type surface).🤖 Generated with Claude Code