video-mode: a selector trim start never lands after a highlight (fixes main CI) - #40
Merged
Merged
Conversation
Since #38 the render window is calibrated from the calibration cover. On a slow runner the screencast trails the paints it maps by a few frames, and the cover-based offset inherits that lag, so a selector- driven trim start could land past the first fill - the render then opened on footage from AFTER the action (the filled textarea before its reveal). CI has been red on main since #38 for exactly this ('reveals an expanding textarea one line at a time at its final geometry', frames 0-1 showing dark text); it never reproduced locally. The existing race guard clamped a trim start back to a highlight only when it landed within one frame after it. Nothing acts on the app before it is ready, so ANY trim start after a recorded highlight is measurement error, and starting at the highlight is always harmless. Clamp unconditionally. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
commit: |
mmkal
marked this pull request as ready for review
August 26, 2026 21:39
mmkal
added a commit
that referenced
this pull request
Aug 28, 2026
…ghlight (main CI flake) (#41) Follow-up to #40, which fixed the *after* case. Main still flakes on `reveals an expanding textarea one line at a time at its final geometry` (1/2 on main's latest run, 3/3 on #39) with the mirror case. Diagnosed from the CI run's own artifacts (`gh run download`): the selector trim start lands a sliver **before** the first highlight (36ms / 92ms / 54ms across the three attempts), so the render opens with ~1 frame of raw footage before the waitFor hold. On a slow runner the screencast starts so late that its very first frames already show the fill's result — the CI raw video is 2.0s long with text present at t=0.04s — so that opening frame is the *filled, expanded* textarea, one frame before the reveal's empty still: First six rendered frames from the failing CI run — frame 0 is the filled, expanded textarea; frames 1-5 are the hold with the empty textarea:  A lead-in shorter than the existing fill stabilization window (`max(0, timelineOffset) + 3 frames`) isn't worth keeping and can't be trusted on a lagging recorder, so the start now snaps forward to the first highlight. Longer lead-ins are untouched; #40's after-case clamp stays. Runner-timing-specific like #40 — this PR's CI run is the verification. Blocks #39. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Session: `b7f6f792-6606-44be-9ec3-207eb762c4b6` <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Medium Risk** > Changes rendered video trim boundaries for selector-based starts on slow runners; scoped to short lead-ins but affects pixel-accurate video assertions if they depend on exact first frames. > > **Overview** > Extends **selector trim start** correction in `video-mode` render finalization: after the existing clamp when trim start lands *after* a highlight (#40), it now handles the mirror case when trim start sits a **short sliver before** the first highlight. > > If `sourceRange.start` is within the fill stabilization window (`max(0, timelineOffset) + 3 frame durations`), it **snaps forward** to `firstHighlightStart` instead of keeping ~1 frame of raw screencast. That avoids opening on lagging-recorder frames that already show post-fill UI (e.g. expanded textarea) before a fill-reveal hold. > > Longer lead-ins are unchanged; only sub-tolerance gaps are adjusted. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit 52054c5. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY --> Co-authored-by: Claude Fable 5 <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
1 participant
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.
Main CI has been red since #38 (
a65cda0):reveals an expanding textarea one line at a time at its final geometryfails 2/2 there with frames 0-1 showing the filled text before the reveal — the render window starts after the fill. It never reproduces locally (3/3 green), which fits the cause:#38 calibrates the render window from the calibration cover. On a slow runner the screencast trails the paints it maps by a few frames, and the cover-based offset inherits that lag — so a selector-driven trim start (
trimStart: ["selector", ...]) can land past the first fill. The existing race guard only clamped a trim start back to a highlight when it landed within one frame after it; CI's lag is bigger.Nothing acts on the app before it's ready, so any trim start after a recorded highlight is measurement error, and starting at the highlight is always harmless (the video opens on the action instead of a beat before it). The guard now clamps unconditionally:
No new spec: the failure is runner-timing-specific and this PR's CI run is the verification. Unrelated:
uses a normal pointer tail after text cursor holdsis a pre-existing local flake on main (1/3 fails with--repeat-each=3), untouched here.Blocks #39's CI (inherited the same red).
🤖 Generated with Claude Code
Session:
b7f6f792-6606-44be-9ec3-207eb762c4b6Note
Medium Risk
Changes render trim boundaries for all selector-driven starts when highlights precede the computed start; behavior is intentional but affects every video-mode render path that sets
sourceRange.start.Overview
Fixes rendered videos that could open after the first recorded action when
trimStartuses a selector or cover-based calibration lags on slow CI runners.During finalization, if
sourceRange.startis set but sits past one or more highlight timestamps, the render window start is pulled back to the earliest such highlight. The previous guard only did this when the gap was within one frame; multi-frame calibration/screencast lag is now covered too.Comments in
video-mode.tsspell out the rationale: a trim start after a highlight is treated as timing error, and clamping to the highlight is safe because it keeps the opening frame on the action instead of post-fill footage.Reviewed by Cursor Bugbot for commit c737189. Bugbot is set up for automated code reviews on this repo. Configure here.