feat(cli): add --at-transitions to inspect for sampling at tween boundaries#1386
feat(cli): add --at-transitions to inspect for sampling at tween boundaries#1386leorivastech wants to merge 2 commits into
Conversation
miguel-heygen
left a comment
There was a problem hiding this comment.
Thanks for picking this up. I think this needs one correctness fix before merge: --at-transitions is documented and titled as sampling every tween start/end boundary, but buildTransitionSampleTimes silently caps the transition-derived samples at 40 using an evenly-strided subset. On a dense timeline, that means the new mode can still skip the specific short boundary window it was added to catch, with no warning or dropped-count metadata in the CLI output. Please either sample all collected transition times, or expose an explicit cap/dropped count and make the default high enough for the issue shape so users are not told they sampled transitions when some were omitted.
|
Agreed on all points — the silent cap contradicts the flag's promise. I have time right now, so on it: default will sample every collected boundary (no cap), with an optional |
…plicit request Review follow-up on heygen-com#1386: the silent cap of 40 contradicted the flag's promise - on a dense timeline the strided subset could skip the exact short boundary window the mode exists to catch, with no indication that samples were omitted. --at-transitions now samples every collected boundary by default. The cap only applies when the new --max-transition-samples flag is passed, and when it truncates, the omitted count is reported both as a console warning and as transitionSamplesDropped in the JSON output.
|
Done in e41b795 — went with both halves of your either/or:
Verified E2E on the gsap-letters fixture: default returns all 7 samples with |
|
Heads up: commits need signatures to be mergeable (flagged on #1385) — setting up signing now and will re-push this branch signed as well. |
…daries Even spacing samples are structurally blind to sub-second overlap windows at transition seams - a 0.2s caption collision slips between samples by construction (heygen-com#1380). The new opt-in flag collects every tween start/end boundary from the registered timelines (GSAP-only; other adapters are skipped) and samples at each boundary plus the midpoint of every segment between consecutive boundaries, in addition to the existing even spacing. Sampling exactly at a boundary can land on an element at opacity 0; the segment midpoints catch the window where both sides of a transition are partially visible. Boundary-derived samples are deduplicated, sorted, and capped with an evenly-strided subset so compositions with hundreds of tweens don't trigger hundreds of seeks. Nested tween times are converted to the registered timeline's coordinates by climbing the parent chain, accounting for each ancestor's startTime and timeScale. The JSON output gains a transitionSamples field when the flag is on. Fixes heygen-com#1380
…plicit request Review follow-up on heygen-com#1386: the silent cap of 40 contradicted the flag's promise - on a dense timeline the strided subset could skip the exact short boundary window the mode exists to catch, with no indication that samples were omitted. --at-transitions now samples every collected boundary by default. The cap only applies when the new --max-transition-samples flag is passed, and when it truncates, the omitted count is reported both as a console warning and as transitionSamplesDropped in the JSON output.
e41b795 to
a497a5c
Compare
Summary
Fixes #1380.
inspectsamples evenly spaced timestamps, so a sub-second overlap window at a transition boundary slips between samples by construction — no sample count fixes that. This adds the proposed--at-transitionsmode.What it does
With
--at-transitions, inspect collects every tween start/end boundary from the registered timelines (window.__timelines, viagetChildren()— the same mechanicruntime/timeline.tsuses for scene ranges) and samples at each boundary plus the midpoint of every segment between consecutive boundaries, in addition to the existing even spacing.The midpoints matter: sampling exactly at a boundary can land on an element at opacity 0. In the issue's repro, t=11.35 shows capB still invisible — the midpoint t≈11.45 has both captions half-visible at the same position, which is where the overlap is undeniable. The unit tests use that exact repro shape.
Implementation notes: nested tween times are converted to the registered timeline's coordinates by climbing the parent chain (startTime + timeScale per ancestor, invoked bound — GSAP getters read internal state through
this); GSAP-only — adapters withoutgetChildrenare skipped gracefully. Every collected boundary is sampled by default. An optional--max-transition-samplescaps the count; when it truncates, the omitted count is reported as a console warning and astransitionSamplesDroppedin the JSON output (always present with the flag,0when coverage was complete).Design choices left open (happy to flip any of these)
--no-transitionsopt-out — trivial flip on the same machinery.document.getAnimations()) would be a natural follow-up for non-GSAP compositions.Verification
layoutAudit.test.ts— 16/16 pass (new cases include the issue's repro shape, a 200-tween dense shape sampled in full with no cap, and the explicit cap reporting the exact omitted count)packages/producer/tests/gsap-letters-render-compat: default returnstransitionSamples: [0, 0.65, 1.3, 1.9, 2.5, 3.1, 3.7]withtransitionSamplesDropped: 0;--max-transition-samples 3returns 3 samples (extremes kept) withdropped: 4plus the console warning; without the flag, output is byte-identical to current behavior.typecheckon @hyperframes/cli passes;oxlint/oxfmt --checkclean; fallow audit gate clean.