Extract shared hook timing utilities - #73
Merged
Merged
Conversation
- 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>
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.
Closes #18
Summary
hookClipEnd()into a singleremotion/lib/hookTiming.ts— previously four separate implementations inSegmentPlayer,CameraPlayer,HookOverlay, andCompositioncould disagree by 1–3 frames per hook, causing hooks to cut early or extend too far on some renders.getHookSubClips()andbuildHookSections()into the shared lib so all hook-section builders use identical logic; de-overlap pass moved here too.SegmentPlayerre-exportsSubClipandSectionfor backward compatibility — no changes required in any downstream consumer.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 localgetHookSubClips+buildSectionshook logic, addition ofbuildHookSectionsimport, re-export ofSubClip/Sectiontypes.remotion/components/CameraPlayer.tsx— diff should show only: deletion of local hook branch ingetOutputDuration, addition ofhookClipEndimport.remotion/components/HookOverlay.tsx— diff should show only: deletion of localbuildHookTimingsimplementation, addition ofhookClipEndimport.remotion/Composition.tsx/remotion/ShortFormClip.tsx— each now imports fromhookTiming.tsrather than duplicating the calculation inline.Test plan
npm testpasses — 14 suites, 207 tests, 0 failureshookClipEndproduces identical results to pre-refactor SegmentPlayer implementation for all hook segment shapes (verified against live transcript data)🤖 Generated with Claude Code