Drag-to-retime resolved the dragged diamond against the selected element's
animations and committed through the selected element's DOM selection, so
dragging a diamond on a non-selected clip retimed the wrong tween. It now
resolves against the clicked element's animations and commits through that
element's selection, matching the delete path.
The three diamond callbacks also take the TimelineKeyframeTarget they already
had instead of five positional fields, and the two copies of the
sourceFile#domId split share splitTimelineElementKey.
The lane-header keyframe toggle fires on whichever element owns the lane,
which need not be the selected one. The remove path looked the animation up
in the selected element's animations, so a non-selected element's flat tween
missed and silently took the remove-one-keyframe branch, stranding the tween
instead of deleting it.
Three review follow-ups on the editor-callback consolidation.
The keyframe-target resolve now takes the clicked element's key and reads
that element's keyframe cache. The diamond context menu and move-to-playhead
pass no explicit target, so they fell through to the cache of whatever
element happened to be selected: opening the menu on a non-selected
element's diamond resolved against the wrong keyframes.
PropertyPanelFlat opens the Motion group by adjusting state during render
instead of in an effect, so the AnimationCard mounts on the same commit the
focus request arrives on rather than a frame later.
Both animation sections pass a module-level focus consumer instead of a
fresh inline arrow, so AnimationCard's focus effect stops re-running on
every parent render.
LegacyTrackHeader reads as deprecated code. It is the live path for every
track that has no keyframe clip to disclose, so call it PlainTrackHeader and
say so in a comment.
The header file owned value sampling, readout formatting, lane-state
resolution and the JSX at once, so a formatting change and a layout change
edited the same file. Sampling and formatting now live in
trackHeaderLaneValues, lane-state resolution in trackHeaderLaneState, and
resolveLaneHeaderState returns only the four fields its caller reads.
Also shows the track's clip count next to the track identity, which the
header promised but never rendered.
The prev/next keyframe chevrons and the group toggle diamond let their
click bubble to the ancestor track row, so seeking to a keyframe also
reselected the track. The disclosure caret and the eye already stop it;
these now match.
Truncated labels (layer name, track label, group label, value readout)
also carry a title so the full text is reachable on hover.
CONTRIBUTING.md asks for a guard clause rather than a non-null assertion outside
an already-checked path. The index check and the lookup are now the same guard.
Escape now ends an in-flight diamond drag the way it already ends clip
and element drags: the armed gesture is marked cancelled, the preview is
dropped, and the pointerup that follows is swallowed instead of falling
through to the click branch.
The preview also flushes once per animation frame instead of once per
pointermove, so a high-rate trackpad no longer re-renders every diamond
in the row several times a frame. Single-diamond retime stays the
documented scope; multi-select drag needs a batched mutation the script
ops do not express yet.
Rows stopped sharing one pixel height when lanes gained expansion, so the only
production caller was passing cumulative row coordinates with trackHeight 1 and
both scrollTops zeroed. The parameter names described units the values no longer
carried. The vertical axis is now a row index and the caller keeps ownership of
folding scroll and per-row heights into it.
The local extractIdFromSelector duplicated the `#id`-only regex that
idFromSelector replaced, so both DOM-less paths in
resolveSelectorElementIds (no-iframe fallback and querySelectorAll-throw
recovery) read no id at all for the bracketed `[id="..."]` form writers
emit for CSS-unsafe ids. Deleted the duplicate and imported the shared
reader; both forms now resolve.
Dragging the playhead to the start of the composition needed a very slow
drag. The scrub surface begins GUTTER + TRACKS_LEFT_PAD px right of the
viewport edge, and both scrub paths bailed out when the pointer sat left of
that origin rather than clamping. So the last 80px of the drag toward zero
silently did nothing: the playhead stuck at whatever the last in-range sample
reported, and only a drag slow enough to sample inside the thin sliver before
the origin ever reached 0.
Both paths now share getTimelineScrubTime, which clamps to [0, duration]. One
owner, so the live-feedback path and the committed-seek path cannot disagree
about the edge again.
The R1/R3 residuals on this PR were fixed at the top of the stack, so they
only cleared once every branch above landed. They belong here, next to the
code they correct:
- `idFromSelector` inverts `idSelector` for both regex readers, so the
post-commit cache refresh stops skipping the CSS-unsafe ids `idSelector`
exists to support.
- `deduplicateKeyframes` drops `ease` when it is ambiguous; the flag was the
only honest answer and the last-writer-wins curve belonged to an arbitrary
colliding tween.
- `isStaticPositionHold` is now the single owner of the hold skip. The
`sourceAnimations` filter and the `allKeyframes` filter had diverged on
whether `immediateRender` counts as a property.
- The keyframe-cache setters no-op when the write changes nothing, instead of
handing every subscriber a fresh Map.
- `reset()` clears `focusedEaseSegment`.
- The test hook `delete`s its window key rather than setting it to undefined,
so feature detection still works.
- The `toClipKeyframes` fixture uses `as unknown as T` with the justification
CONTRIBUTING.md asks for.
CLI and Studio feedback were emitted as `survey sent` with `$survey_*`
properties, so every rating was ingested as a PostHog survey response even
though no survey definition, targeting, or popover backs them.
Emit `cli_render_feedback` and `studio_feedback` with plain `rating` /
`comment` properties instead. Same fields, same call sites, same opt-out.
* ci(regression): compute the shard matrix from recorded fixture timings
* ci(regression): refresh shard timings from a green post-PSNR run
* fix(ci): close two silent-skip holes in the shard schedule contract
* ci(regression): schedule the new static-volume-future-set fixture
* test(producer): regenerate static-volume-future-set golden in the pinned container
* fix(producer): accept partial color metadata in plan v2
* test(engine): make partial color probe hermetic
* fix(producer): validate plan v2 sentinels in fallback mode
## What
- model expected video-frame counts using the same rounding contract as the extraction branch:
- CFR `-vf fps=<fps>`: nearest output-frame boundary
- VFR `-fps_mode cfr -r <fps>`: ceil
- retain fail-closed ceil behavior when extraction metadata is missing
- keep positive sub-frame clips at a minimum of one expected frame
- preserve the existing 95% truncation gate and source-duration credit
## Why
The coverage gate universally used `ceil(duration * fps)`, but FFmpeg's CFR fps filter rounds to the nearest boundary. This made successfully extracted short CFR clips such as 0.616666s at 30 fps look truncated (18 captured vs 19 expected).
In the dashboard window, 124 of 335 video-coverage failures were exactly one frame short. Historical logs do not include `isVFR`, so that is the maximum addressable cohort rather than a guaranteed reduction. Zero-frame and materially truncated extraction failures remain fail-closed.
## Safety
- VFR still uses ceil and the existing strict coverage threshold.
- Missing extraction metadata still uses ceil.
- No retry, fallback, Temporal workflow, or render-plan behavior changes.
- Intended rollout is through the producer sidecar canary with explicit internal in-process jobs before dev and production promotion.
## Test
- `bunx vitest run packages/producer/src/services/render/videoFrameCoverage.test.ts` (28 passed)
- `bun run --filter @hyperframes/producer typecheck`
- `bunx oxlint ...videoFrameCoverage.ts ...videoFrameCoverage.test.ts`
- `bunx oxfmt --check ...videoFrameCoverage.ts ...videoFrameCoverage.test.ts`
- `bunx fallow audit --base origin/main --fail-on-issues`
- repository pre-commit gates
## Summary
- classify per-source video download/probe/decode/extraction failures with a bounded taxonomy and safe producer-facing summaries
- add candidate-only, at-most-one transient retry with cleanup and retry telemetry
- preserve default engine/producer behavior when the policy is off
- carry allowlisted extraction error codes through blocking JSON and SSE responses
## Stack
Depends on #2774 for atomic remote downloads and its single owned download retry. This PR is intentionally based on `fix/atomic-video-download-retry`; rebase/change the base to `main` after #2774 merges.
## Default compatibility
`HF_VIDEO_EXTRACTION_FAILURE_MODE` defaults to `off` and forces `maxTransientRetries=0`.
With the feature off:
- metadata probe failures keep the legacy Promise rejection
- grouped extraction keeps the existing grouped-to-direct fallback
- no new producer failure gate is enforced
- render-plan schema, Plan v1 artifacts, chunk routing, and distributed execution are unchanged
Typed metadata aggregation is explicit and enabled only by the candidate enforce lane.
## Retry ownership
- remote downloads: exactly one retry owned by #2774
- metadata/FFmpeg extraction: at most one retry only when `HF_VIDEO_EXTRACTION_MAX_RETRIES=1`
- invalid, missing, rejected, out-of-range, zero-output, cancellation, and unknown/internal failures do not retry
- non-finite or invalid runtime retry budgets fail closed to zero
- the superset optimization is never retried; on failure it preserves direct-member fallback, and only the individual ranges can use the bounded retry
- retry counters increment when a retry is scheduled, including exhausted retries
The internal sidecar and Experiment Framework must treat both exhausted stage codes as workflow-terminal after the producer-local budget. Candidate enforcement must not be enabled until those companion mappings are deployed, or Temporal can multiply producer attempts.
## Failure contract
- `VIDEO_SOURCE_UNRENDERABLE`: at least one deterministic/unknown source failure
- `VIDEO_EXTRACTION_FAILED`: all source failures are transient but the producer-local budget is exhausted
Only the allowlisted code and kind/count summaries cross JSON/SSE. Raw diagnostics remain engine-local because they may contain signed URLs or local paths.
## Rollout
1. merge and deploy with stable/candidate both `off`
2. candidate `observe`, retries 0
3. candidate `observe`, retries 1
4. deploy internal + EF terminal transport mappings
5. candidate `enforce`, retries 1
6. keep stable off until success delta, retry counts, extraction latency, CPU/disk, and queue backlog are acceptable
## Validation
- engine focused suites: 105 passed
- producer focused suites: 15 passed
- full engine suite: 1,176 passed, 3 skipped
- full producer unit lane: 32 Vitest files / 393 tests plus all classified Bun unit tests
- engine and producer typechecks passed
- oxlint, oxfmt, Fallow, tracked-artifact, and commit hooks passed
- independent review: approved for merge default-off; candidate enforcement held on companion transport rollout
## Summary
- stage remote video downloads in private per-attempt directories and atomically publish only complete non-empty files
- keep the deadline active through response-body streaming and retry one bounded transient failure
- preserve render cancellation without sharing abort ownership across independent callers
- manually follow at most five redirects, validating HTTPS/public-host policy before every hop
- remove stale zero-byte finals and avoid a permanent render-scoped cache map
## Root cause
The previous downloader wrote directly to the final cache path and cleared its timeout as soon as response headers arrived. A body timeout or mid-stream socket reset could therefore leave a truncated file at a path that later extraction treated as complete. The downstream symptom was zero extracted frames followed by a generic video coverage failure.
## Retry policy
Exactly one retry is allowed for 408, 429, 5xx, timeouts, empty successful bodies, and network/socket failures including nested Undici errors. Cancellation, 404/410, other 4xx, URL/redirect validation errors, and filesystem errors are not retried.
## Security and portability
- `mkdtempSync` creates an unguessable same-filesystem staging directory; exclusive writes plus atomic rename prevent symlink planting and partial publication
- redirects use `redirect: "manual"` and every resolved `Location` is revalidated before the next request, blocking redirect-to-private/IMDS bypasses
- the partial file is opened read/write for `fsync`, which preserves flush semantics and avoids Windows `EPERM`
## Rollout safety
This does not change render-plan schema, Plan v1 artifacts, chunk routing, or distributed rendering semantics. It is suitable for the candidate sidecar lane first; stable can remain pinned while we compare video extraction and coverage failures.
## Validation
- focused urlDownloader suite: 28 passed
- full engine suite: 1,169 passed, 3 skipped
- engine typecheck passed
- oxlint and oxfmt passed
- fallow audit passed
- independent code review: approved, no blockers
Miguel Angel Simon Sierra