Skip to content

Extract shared hook timing utilities - #73

Merged
natashaannn merged 4 commits into
mainfrom
refactor/s2-hook-timing
May 15, 2026
Merged

Extract shared hook timing utilities#73
natashaannn merged 4 commits into
mainfrom
refactor/s2-hook-timing

Conversation

@natashaannn

Copy link
Copy Markdown
Member

Closes #18

Summary

  • Consolidated hookClipEnd() into a single remotion/lib/hookTiming.ts — previously four separate implementations in SegmentPlayer, CameraPlayer, HookOverlay, and Composition could disagree by 1–3 frames per hook, causing hooks to cut early or extend too far on some renders.
  • Extracted getHookSubClips() and buildHookSections() into the shared lib so all hook-section builders use identical logic; de-overlap pass moved here too.
  • SegmentPlayer re-exports SubClip and Section for backward compatibility — no changes required in any downstream consumer.
  • Added 21 unit tests covering bounded/unbounded hooks, token-tail extension, next-hook bridging, de-overlap, and Whisper marker filtering.

How to review

  • remotion/lib/hookTiming.ts — new canonical implementation; verify the bounded vs unbounded logic, tail pad constants, and bridge condition match the intent in CLAUDE.md (constants: HOOK_TAIL_PAD_UNBOUNDED_SECONDS=0.16, HOOK_TAIL_PAD_BOUNDED_SECONDS=0.02, HOOK_BRIDGE_MAX_GAP_SECONDS=1.0).
  • remotion/lib/hookTiming.test.ts — 21 tests; confirm edge cases (Whisper markers, bridging, overlap) are covered and assertions are not trivially true.
  • remotion/components/SegmentPlayer.tsx — diff should show only: deletion of local getHookSubClips + buildSections hook logic, addition of buildHookSections import, re-export of SubClip/Section types.
  • remotion/components/CameraPlayer.tsx — diff should show only: deletion of local hook branch in getOutputDuration, addition of hookClipEnd import.
  • remotion/components/HookOverlay.tsx — diff should show only: deletion of local buildHookTimings implementation, addition of hookClipEnd import.
  • remotion/Composition.tsx / remotion/ShortFormClip.tsx — each now imports from hookTiming.ts rather than duplicating the calculation inline.

Test plan

  • npm test passes — 14 suites, 207 tests, 0 failures
  • React and node Jest projects both green
  • 3 Playwright E2E smoke tests pass
  • Frame parity confirmed — hookClipEnd produces identical results to pre-refactor SegmentPlayer implementation for all hook segment shapes (verified against live transcript data)
  • Remotion Studio visual check — hook clip boundaries correct, camera switches at hook boundaries work

🤖 Generated with Claude Code

natashaannn and others added 4 commits May 14, 2026 16:41
- Mark hookClipEnd() duplication bug as fixed in Known correctness bugs list
- Update HOOK_TAIL_PAD_* and HOOK_BRIDGE_MAX_GAP_SECONDS constants table
  to point to remotion/lib/hookTiming.ts (their new canonical home)
- Add remotion/lib/hookTiming.ts row to Key Source Files table
- Update SegmentPlayer refactor note (hookTiming extraction is done)
…silon constant

- W1: replace conditional if(sections.length>=2) with unconditional
  expect(sections).toHaveLength(2) so a de-overlap regression fails loudly
- W2: extract 0.02 literal as SEGMENT_TAIL_EPSILON_SECONDS; import and use
  it in the new bridging edge-case test

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@natashaannn
natashaannn merged commit 9371dfd into main May 15, 2026
1 check passed
@natashaannn
natashaannn deleted the refactor/s2-hook-timing branch May 15, 2026 09:04
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.

Extract shared hook timing utilities

1 participant