The codex image provider's fail-fast gate required an `imagegenext` row in
`codex features list`. Codex CLI 0.145 renamed that flag to `image_generation`
and dropped the old row, so the gate rejected every up-to-date CLI:
media-use: codex image upsell unavailable: codex imagegenext unavailable
(upgrade Codex CLI)
`resolve --type image --provider codex` returned that without ever attempting
a render, on hosts where image generation works fine. Codex itself now warns
`[features].imagegenext is deprecated. Use [features].image_generation`.
Resolve the flag name from the feature list instead of hardcoding it, and pass
whichever name the installed CLI exposes to `--enable`. The old name is
preferred when present, since CLIs that still list it reject the new one.
`codexUnavailableReason()` now returns `{ flag }` / `{ reason }` so the
resolved name reaches the exec.
Verified against codex-cli 0.145.0: `resolve --type image --provider codex`
generates and freezes an image. New unit tests pin both flag layouts so a
future rename fails loudly instead of silently disabling the provider.
The narrowing this branch adds missed the motion-path overlay, and every
caller that could not narrow fell back to the exact bare class the narrowing
exists to replace.
- motionPathSelection.selectorFor now goes through writeTargetSelector. It
feeds both the geometry read and the "set destination" write, so a class
sibling measured its home off the FIRST sibling and then authored
add-motion-path onto all of them. The toolbar toggle hides when no
one-element form exists rather than arming a press that is dropped.
- The five new-tween writers that fell back to the selection's own selector
now drop the commit instead. A gesture that does not persist reverts on the
next reload; a tween silently aimed at five elements does not.
- tweenTargetsElement only follows the DOM to a target that matches exactly
one element. A target the element merely shares with its siblings is a group
tween, and these callers mutate what they find, so an individual nudge was
rewriting the group's own tween and moving all five.
U3 fixed "add keyframe at playhead" widening a write to every sibling
sharing a class, but wired writeTargetSelector into only two paths. The
same bug was still reachable from the add-animation button, drag, resize,
rotate, gesture recording, and the property panel: each derived its target
from selectorFromSelection, which hands back a bare class for an id-less
element, so one edit authored a tween over all five siblings and the
timeline collapsed their rows into one.
Route every path that authors a NEW tween through the existing ladder:
- ensureElementAddressable now accepts selection.selector only when it
addresses exactly one element, so the id-minting fallback right below it
(previously unreachable whenever any selector was present) does the work.
- gsapDragCommit's five new-tween branches go through one newTweenTarget
helper; instant patches reuse the written target so the runtime moves the
element the source write names.
- useGestureCommit and useAnimatedPropertyCommit keep the existing selector
for matching/retargeting and author new tweens with a separate write
selector.
Retargets of an EXISTING tween are deliberately untouched: they keep
anim.targetSelector, so a tween aimed at a whole group stays aimed at it.
Narrowing the write alone regressed idempotency, verified by test: the
"is there already a write for this element" lookups matched targetSelector
by string, so the next nudge missed the write it had just made and appended
a second, conflicting one. The read half now falls back to the live DOM
(tweenTargetsElement, same contract as getAnimationsForElement), which also
still matches a deliberate group tween.
Tests reproduce each site through a real writer, re-parse with the real
parser, and resolve through resolveSelectorElementIds (what feeds the
keyframe cache and the lanes), plus pins for the new-tween vs
retarget-existing distinction so a future change cannot collapse the two.
Two latent defects in the announcement path this branch adds.
- lanesId was keyed by render row alone, so a second TimelineLanes on the
page (a mini-timeline beside the main one) would mint the same
timeline-lanes-track-0 and every caret's aria-controls would resolve to
whichever instance mounted first. The prefix now comes from useId, with the
colons stripped so the id stays a legal CSS selector.
- trackDisplayNumber returned trackOrder.length + 1 for a key it could not
find, which is indistinguishable from a real row: the label announced a row
the user can see is wrong and nothing upstream could tell it had guessed. It
returns null now, and trackDisplaySuffix drops the number from the label
rather than inventing one.
writeTargetSelector returned the selection's bare selector whenever the
structural walk failed, including when a live DOM was there to check
against. An element detached between selecting and committing takes that
path, so the add re-authored the exact `.group` string the function exists
to replace. Return null instead: a failed walk against a live DOM is
evidence, not absence of it. Callers that cannot drop a user edit opt back
in with `?? selectorFromSelection` where the trade is visible.
The replace-with-keyframes paths had the mirror defect. The server deletes
and re-adds the tween, so their target string is a full rewrite, and they
derived it from the selection: promoting a set on a tween already narrowed
to `#scene > div:nth-child(3)` widened it back onto every class sibling,
undoing the narrowing an earlier add had made. They now keep the tween's
own authored target, matching what eight sibling commit modules already do.
The timeline track key is a fractional z-order sort key: an expanded
sub-composition child gets `host.track + n / (siblings + 2)`. The undo
history entry for the eye toggle interpolated that key directly, so
hiding an expanded child recorded "Hide track 0.16666666666666666".
Give the key-to-display-row conversion a single owner
(timelineTrackDisplay.ts) and route both the track header labels and the
history label through it, so the two cannot drift apart again. The raw
key still routes the callbacks and lookups that need it.
Adds a regression test that toggles a track keyed 1 / 6; a test on track
0 formats cleanly and proves nothing.
"Add keyframe at playhead" on an element with no id authored the bare class
buildStableSelector hands back, so one add on a `.group` wrote
`tl.to(".group", ...)`: a tween that animates all five siblings and that
resolveSelectorElementIds reads back as all five, collapsing their timeline
rows into one. It survived a reload, so the written file stayed un-editable.
writeTargetSelector is the write-side counterpart to selectorFromSelection
(which must keep returning the exact string findTweenAtTime compares against).
It resolves the element's own identity to a selector that addresses exactly
one element: `#id`, else `[data-hf-id="..."]`, else the selection's selector
when it is already unique, else a `:nth-child` path anchored on the nearest
identifiable ancestor (the selector + selectorIndex pair, resolved through the
DOM the index was counted in).
Applied to the two paths that author a NEW tween: the no-animation branch of
useEnableKeyframes and commitKeyframeAtTimeImpl. replace-with-keyframes still
writes the selection's own selector, since retargeting a tween the author
aimed at a whole group is a different decision from adding a keyframe.
The timeline's track key is a fractional z-order sort value, and the header
built its visibility label straight from it, so screen readers announced
"Hide track 0.16666666666666666". A track's 1-based display row is now passed
alongside the key: the row number goes in every label, the key keeps routing
every callback (visibility toggle, lane context menu). The same fix covers the
`Track N` fallback used when a track holds no labelled element.
The layer disclosure caret's aria-controls named a div in the sticky label
column. That subtree is not empty, it holds the per-lane keyframe controls, but
its children are all absolutely positioned so the div computes to 0x0, and the
diamonds the caret visibly reveals live on the canvas instead. The caret expands
two disjoint subtrees and was naming the less useful one. TimelinePropertyLanes
now renders one static wrapper (static, not relative, so it establishes no
containing block and the absolutely-positioned lanes keep resolving against the
track-content div with identical geometry) and takes the id. TimelineLanes mints
that id, since it is the only place that sees both ends of the disclosure, and
mounts the wrapper for the track's keyframe clip in both disclosure states so
the reference still resolves while collapsed.
TimelineLaneBaseProps moves to its own module: it is the contract shared by
TimelineCanvas and TimelineLanes, and lifting it out keeps TimelineLanes.tsx
well under the 600-line cap instead of pushing past it.
animIdForProp matched on the parser's whole-tween propertyGroup, which is
undefined for a legacy mixed tween such as {x, opacity}. Such a tween never
matched, so an edit to either property fell through to the selection's
default animation, a different tween than the lane the user is editing.
Resolve through animationLaneGroups, the same per-keyframe helper the
rendered lanes and the reserved row heights already count groups with.
Two halves of one inversion in the expanded timeline lanes: the tweens
that should show were filtered out, and a tween that should not be there
was the only survivor.
Lane classification read the parser's whole-tween verdict, which is
undefined for anything spanning more than one property group. `{x,
opacity}` is the canonical HyperFrames entrance tween, so five of the
seven tweens in the swiss-grid graphics example had no caret, no
reserved row and no diamonds. Classify per property instead, through one
helper both the rendered lanes and the reserved row heights count
through so they cannot drift again.
Attribution matched an unanchored leading id, so `#stat3 .block` was
filed under `#stat3`. The child's diamonds landed on its ancestor and
collided with the ancestor's own tween at the shared percentage, which
the same-percentage merge then resolved by dropping the ease. Route
attribution through resolveSelectorElementIds, which anchors a
whole-selector id and otherwise resolves through the live preview DOM,
and anchor its no-DOM fallback so a descendant selector resolves to
nothing rather than to its ancestor. The merge rule is unchanged.
Also brings the last property-lane call site onto the shared clip timing
basis: an expanded sub-composition child's start is host-absolute while
its tweens are local to its own file.
Drilling two levels deep spared only the top-level row, so the middle host
lost its row and its keyframe lane with it. Spare every host between the
drilled one and the top, and anchor the children under the deepest host that
actually has a row.
Also stops resolveClipTimingBasis handing back a main-timeline start when a
clip names a parent composition that is absent from the element list. The
mount is unknowable there, so the child's own window is the only safe frame.
The sub-composition drill-in replaced the host row with its children. Since
expansion is also driven by the playhead alone (paused auto-expand), an
ordinary seek into a sub-composition made the host row disappear, taking its
keyframe lane with it: diamonds render per row from keyframeCache.get(
elementKey), so no row means no diamonds. Reproduced live with no drag at
all, seek 0 gave 3 diamonds, seek 7.68 gave 0, seek 0.2 gave 3.
Make the expansion additive instead. The host row stays and its children are
appended directly below it. The synthetic fractional lanes already used for
children sit strictly between the host's lane and the next integer, so the
host keeps its own row without colliding with anything.
The time-keyed auto-expand itself is unchanged.
buildExpandedElements synthesized DOM-only sub-composition children against
the top-level element rather than the parentHost it resolves immediately
after. Under two-level nesting every child row therefore inherited the
top-level window instead of its own host's, so the rows drew at the wrong
offset and duration.
A sub-composition tween's resolvedStart is composition-local, while the timeline
element resolved for it is the sub-comp HOST, whose start is main-timeline
absolute. toClipPercentage subtracted the two frames from each other, so a host
mounted at 1.5s cached its 0s tween at -12% and its last tween's end keyframe at
88% instead of 100%. A clip-relative percentage can never be negative.
resolveClipTimingBasis now returns the clip start in the frame the tween's own
times are measured in: the composition mount (expandedParentStart for an
expanded child, the parent composition clip's start otherwise, 0 for a
root-composition element) is subtracted, and a sub-comp inner element that falls
back to its host's window starts at 0 in that window. It moves to gsapShared so
the post-commit cache writer can share it instead of resolving its own basis,
which also gives that writer the sub-comp host fallback it was missing.
The gesture resolver gated on six digits while parseCssColor accepts both
lengths, so #F00 previewed as nothing and committed nothing. The old onBlur
path parsed it, making this a behavioural loss rather than a pre-existing gap.
Also asserts that an incomplete hex is restored on outside-click, not merely
left uncommitted.
The panel does not trap focus and leaves the rest of the editor operable,
so aria-modal would tell assistive tech the whole app is inert while it is
open. role=dialog plus aria-controls and Escape is the correct non-modal
disclosure shape.
Swap the panel's hand-rolled bubble-phase mousedown listener for the
shared useContextMenuDismiss hook, which adds Escape support and fixes
outside-click dismissal when a canvas gesture (e.g. marquee start)
calls preventDefault on pointerdown, which otherwise suppresses the
mousedown compat event entirely. Also wires up dialog ARIA (role,
aria-modal, id/aria-controls) between the trigger and panel.
Split hex-draft ownership so the hex input is the sole author of its
own text while editing (updateColorDraft no longer stamps a canonical
hex back over every keystroke), fixing snap-back on backspace and the
silent wrong-colour clobber on non-repeating hex values. Route hex
typing through the shared gesture transaction so outside-click and
Escape settle/cancel it like the other inspector fields, instead of
relying on a private onBlur commit that never fires once the panel
unmounts on outside-click.
The 24x24 WCAG 2.5.8 overlay sits on a wrapper that outranks the diamonds,
so on a segment narrower than 24px it overhung them and won their hit test at
fit zoom. Gate the overlay on the clear span between the two diamonds and let
the button keep its 16x16 box below that.
Also renames the pointer target suite to say it asserts the classes that
produce the size, not the measured geometry, which happy-dom cannot see.
A ruler press with no pointer movement settled the playhead at t=0 instead
of the clicked time. handlePointerUp replays pendingClientXRef, which only
the pointermove path wrote, so a plain click fell back to the ref's initial
0 and overwrote the correct pointerdown seek. Seed the ref on pointerdown.
The keyframe retime move branch also returned the raw quotient while the
resize branch rounded to 3dp, so values like 74.81203007518799% landed in
the user's source and churned the diff on every drag. Round at the point of
computation so the no-op test and the written value agree.
Review follow-ups on the expanded keyframe lanes.
Writer:
- `onMoveKeyframe`'s flat-tween boundary branch answered `true` the moment it
dispatched update-meta, so a rejected write left the diamond parked at its drop
position. `observeGsapMutation` now resolves to whether the mutation landed and
the boundary branch returns it, matching the other branches.
- "Delete All Keyframes" cleared only the first keyframed tween on the layer, so
a layer with position AND opacity keyframes kept half of them. It now walks
every keyframed tween, serially, through the clicked element's selection.
- The post-convert lookup in `commitFlatViaKeyframes` matched by target selector,
which picks an arbitrary tween when a target carries several. Match by id first.
Interaction and a11y:
- A rejected retime whose commit settled after a newer drag reverted the
selection to its own source keyframe, undoing a retime the user could see. The
revert now only runs while it is still the lane's latest gesture.
- Diamonds key on the authored identity instead of index plus rendered clip-%, so
a neighbour's retime no longer remounts the button mid-drag.
- The disclosure caret gets `aria-controls` on an always-mounted lanes container,
and both it and the property-group toggle grow to the 24x24 WCAG 2.2 minimum.
- `LayerDisclosureRow` takes the same adaptive `columnWidth` as its sibling lane
rows instead of hardcoding LABEL_COL_W over the canvas.
Test locks:
- The timeline callbacks harness resolves a DISTINCT selection per element, so
the clicked-element writes are actually pinned; three assertions that passed
either way now name the clicked element's selection.
- New: null-selection aborts every mutation, delete-all covers both tweens, a
rejected boundary retime reports `false`, and a stale revert leaves selection.
- The playhead-percentage assertion checks 25, not `expect.any(Number)` (which
also accepts NaN); ease segments assert their label ORDER, not just that the
three curves differ; the collapsed-diamond callback asserts the whole target.
- Dropped a duplicate `selection override` describe left by a rebase.
## What
Fix Windows cleanup ordering for timed-out CLI downloads so `downloadFile()` rejects only after the active response pipeline has closed and the `.tmp` file has been removed.
## Why
Main CI run 30299854376 failed in `packages/cli/src/utils/download.test.ts` because the timeout path rejected from the request `error` handler while the response pipeline still held the temporary file open. Unix permits unlinking an open file, but Windows reports the file as locked; that cleanup error was swallowed and the promise settled while the partial file still existed.
The race was introduced by #2415 / `75eedf5cc1`, which added the inactivity timeout and separate request-error cleanup path.
## How
- Track whether a response pipeline owns the temporary file.
- When the request errors after streaming has begun, destroy the active response and let `pipeline()` finish tearing down the writer.
- Preserve the original request/timeout error, then remove the partial file before rejecting.
- Make the regression deterministic on every OS by simulating Windows `EBUSY` until the response closes.
## Test plan
- [x] Unit test updated to reproduce the Windows locked-file lifecycle on Linux
- [x] Focused regression test (20 consecutive runs)
- [x] Full CLI suite: 164 files passed, 1 skipped; 2,162 tests passed, 2 skipped
- [x] CLI typecheck
- [x] Full workspace build
- [x] Changed-file oxlint, oxfmt, Fallow, tracked-artifact check, and pre-commit hooks
- [ ] CI green on Linux, macOS, and Windows
An explicit null selection override now aborts the write instead of falling
back to domEditSelection: a caller that resolved a selection for its own
element and found none was committing onto whichever element happened to be
selected. Ease changes and the playhead keyframe toggle resolve the edited
element's animations and selection instead of the current selection's, and
lane header rows follow the real label-column width so a narrowed column no
longer hangs its value readout over the canvas.
Review follow-ups on the expanded-lanes tip:
- TimelinePropertyLanes.groupKeyframes re-derived the clip-relative
percentage inline and skipped toClipKeyframes' rounding, the one
precision every keyframe-cache writer has to agree on (selection keys
embed the number). It now loops the shared helper per animation with
the group filter and only overrides the lane's own group and ease.
- TimelineClipDiamonds rebuilt and re-sorted each tween's sibling row
inside the marker loop, so a row of N diamonds allocated N sorted
copies of itself on every playhead tick. Built once per render instead,
keyed by animation id.
- Add the missing symmetry test at the real callback boundary: the
diamond test mocks onMoveKeyframe, so nothing proved the rapid-second
retime resolves a pending clip-% the keyframe cache has not caught up
to. The new test asserts the identity-carrying target retimes and that
the same drag without identity fields cannot.
CONTRIBUTING.md allows `as unknown as T` with a justification, not a bare
`as T`; the gsapShared fixtures only carry the fields under test.
The fallow complexity gate flagged three functions on this branch. Each is
split at its natural seam rather than suppressed: the auto-expand scan moves
out of the effect, the four repeated attribute guards in
nodeMatchesManifestClip collapse into one table-driven check, and the
segment-% interpolation moves out of onPathDown.
Selector reads now go through one inverse of `idSelector`. Every writer emits
`[id="01-hook-hero"]` for an id a `#id` selector can't address, but the readers
still matched `#id` only, so the post-commit keyframe-cache refresh, the AST
load and the remove-all-keyframes clear all silently skipped exactly the ids
`idSelector` was added to support.
A keyframe merged from two tweens with different eases kept whichever ease
iterated last. Readers that don't check `easeAmbiguous` showed a curve from a
different animation than an edit would target, so the ambiguous flag now clears
`ease` instead of leaving an arbitrary one behind.
One tolerance for "the playhead is on this keyframe". The motion-path drag used
0.05% while the toolbar and the playhead apply used 1%, so a drag that landed a
fraction of a percent off an authored waypoint skipped the update-point branch
and appended a near-duplicate. `buildTemporalArcKeyframes` now owns the
invariant and replaces any keyframe inside the tolerance, rather than trusting
each caller's own pre-check.
The pending-retime bookkeeping matches on keyframe identity, not just on
"something is near that percentage" — an evenly spaced row cleared the entry off
an unrelated sibling. The neighbour clamp composes pending destinations in
before sorting, so a second drag can't cross a neighbour that already moved.
Also: `keyframeCache`/`gsapAnimations` setters return the same state for a write
that changes nothing (every no-op re-rendered every subscriber), the auto-expand
set drops clips that left the source so an undo/paste under the same id expands
again, `invalidateGsapCache` has a stable identity instead of re-creating the
whole timeline edit context each render, the studio test hook deletes its window
key rather than leaving it enumerable as undefined, and the past-last-row
extrapolation documents why it uses TRACK_H where the pre-first-row branch uses
row 0's own height.
Covers `idFromSelector` round-trips, the insert boundary band across plain,
expanded and unusable row heights, and the collapsed selection key for a
colon-bearing element id.
Two more regressions the QA triage attributed to this stack.
Dragging a flat tween's boundary diamond did nothing: the handler bailed on
`!anim.keyframes` even though resolveKeyframeRetime already resolves that case
to a position/duration resize. The empty remap now dispatches update-meta, which
moves the window without rewriting the authored flat tween into keyframes form
(what the keyframed-resize writer would do as a side effect).
The property panel's keyframe gutter guessed one animation per property group,
so clicking a keyframe authored by a sibling tween on a merged row named the
wrong tween and the writer silently removed nothing. The diamond now reports the
clicked keyframe's own animationId, with the group guess kept as the fallback for
cache rows that carry no identity.
The QA fleet's 295 findings were replayed against the merge-base. Most were
pre-existing, but these were caused by this stack:
Deleting one keyframe destroyed the whole tween. The lane-header remove toggle
escalated a flat tween to a whole-animation delete, which took the authored
`tl.to(...)` and its source comment with it on a single click. The base build
posts remove-keyframe and lets the writer refuse it; restore that.
A keyframed layer could not be hidden at all. The visibility eye had moved off
the always-mounted layer row onto a hover-gated property-group row, so it only
existed while the lanes were expanded AND the pointer was over that lane. A
keyboard-only user could reach no eye at all, and its label named a track its
row did not act on. It goes back on the layer row.
A drag from the centre of a clip bar did nothing, because the 16px inline ease
button sits exactly there and swallowed the press. It now lets the press through
to the clip and keeps only the click, dropped if the pointer travelled.
Dragging a diamond onto a neighbour silently discarded the retime. The clamp
bounded the dragged keyframe by the whole merged row, so two animations
colliding at one percentage pinned each other in place and the drag resolved
back to a click. Clamp against the dragged keyframe's own tween instead.
Also: floor the diamond hit box at 12px (the gap-derived size fell to ~7px at
the zoom floor), round the diamond tooltip percentage, and prune the keyframe
caches when a composition switch drops a file from the scan set — each file only
ever cleared its own entries, so the previous composition leaked every element
into both keyframeCache and gsapAnimations, with nothing to evict it.
Validate the parse response before reading `.animations` instead of casting
the JSON blind, and narrow the fetch's return type to the slice callers read.
Route the AST cache load through the shared clip-keyframe and cache-key
helpers so it can't drift from the other writer. Drop the unused numeric
track-count branches from `trackHeights`/`getTimelineCanvasHeight`, and pick
the widest keyframed clip with a reduce so there's no index assertion.
Carries the gsapAnimations/keyframeCache symmetry fix into the extracted
AST loader: an ungrouped tween now lands in both stores, so the collapsed
row and the expanded lanes agree on what a clip animates.
Auto-expansion picks the composition whose clip window contains the
playhead, and those windows are half-open. With the playhead parked on
the last clip's end - where playback stops - it sat inside nothing, so
every expanded sub-composition row and its keyframe lanes collapsed to a
single host row.
Accept the closing boundary, but only when the strict pass matched
nothing, so a playhead on the seam between two adjacent clips still
expands the one that is starting.
The keyframe cache was fetched for a single source file: the selected
element's, else the active composition. On open nothing is selected, so
a project whose clips live in sub-compositions loaded only index.html
and every property lane rendered empty until a clip was clicked.
Load the cache for each composition file the timeline has rows for, so
keyframed clips are expanded on open as intended. The AST load path
moves to keyframeCacheAstLoad.ts to keep useGsapTweenCache.ts under the
600-line cap.
- revert diamond selection when a rejected retime leaves the source in place
- clear project-local ease focus and expansion on player store reset
- share one static-position-hold predicate across the tween cache
- invalidate the GSAP cache even when a group timing rewrite throws
- stamp each lane keyframe's ease from its own source tween
- memoize property lanes and row offsets so memo'd diamond lanes hold
- use the editable tween duration for drag position commits
- restore the pre-t=0 pad in the all-collapsed content origin
- clamp the drag ghost and drop placeholder to the collapsed clip height
- aria-expanded on the layer disclosure, aria-pressed plus state-specific
labels on the keyframe toggle, 24px chevron targets, focus-visible parity
The diamond context menu still declared `onChangeEase` and `onCopyProperties`
props, and `TimelineOverlays` still threaded `onChangeKeyframeEase` plus a
`keyframeCache` it never read. Nothing on any timeline branch calls them, so
they are removed along with the `onChangeKeyframeEase` callback implementation.
Also from review:
- `deleteSelectedKeyframes` only falls back to the sole keyframed animation when
there is exactly one. A collapsed selection key carries no animation id, so
taking the first of several deleted an arbitrary tween's keyframe.
- The duration-less retime test asserts the real 87.601% instead of
`expect.any(Number)`, so a wrong timing basis fails it.
- `Timeline` wires the keyframe handlers' `onSelectSegment` through to the
diamonds; it was built and then dropped, so segment ease selection never fired.
- The flat text section arms auto-focus in state rather than reading and
clearing a ref during render, which Strict Mode's double render swallowed.
The context menu handed its actions loose positional arguments, so the
overlay adapters forwarded the percentage alone and dropped the property
group, tween percentage, and animation id the menu had resolved. The target
then fell back to first-match-by-percentage and deleted or retimed the wrong
animation whenever two collide at one percentage. Every action now carries a
TimelineKeyframeTarget, which leaves no adapter shape that can drop it.
TimelineClipDiamonds.tsx had grown past the 600-line studio file gate, and
its keyframe/props types were declared a second time next to the ones the
lanes already share. The types, the render constants, and the keyframe
identity helper move to timelineDiamondTypes.ts; the rendering stays put.
The collapsed clip row dropped a keyframe's property group and animation id
before handing it to a callback, so the same keyframe hashed to a different
selection key than the expanded property lane did. Selecting a diamond in one
view left it unselected in the other, and retime/delete on the collapsed row
lost the animation id they use to pick between two animations that collide at
one percentage.
Diamonds now always carry their full identity, the collapsed shim just curries
the element id, and Timeline reuses useTimelineKeyframeHandlers instead of its
own inline copy of the same three handlers.
Neighbour geometry moves into one marker record per diamond, which drops the
index-lookup non-null assertions the connector pass needed.
Two sibling call paths still answered from GSAP's 0.5s default while the
toggle path had moved to the clip-wide fallback.
isPlayheadWithinTween now takes the selection, so the toolbar stops
promising "extends animation" on a duration-less tween whose click actually
toggles an interior keyframe. The arc drag commit resolves its replacement
duration the same way its click-path sibling does, instead of authoring
duration 0.5 and collapsing the arc window.
The flat text section drops its two marker-clearing effects: the autofocus
marker is a ref now, read and cleared by the render that consumes it.
Each timeline keyframe callback now reads and writes through the SAME
element: the clicked element's animations resolve the target, its selection
commits the mutation, and its animation computes the playhead percentage.
onMoveKeyframeToPlayhead previously took the percentage from the clicked
element and the animation plus selection from the current one, so a context
menu on a non-selected diamond retimed against one tween and wrote into
another file. onChangeKeyframeEase and onToggleKeyframeAtPlayhead had the
same split. An explicit null selection override now aborts the write instead
of falling back to the current selection.
The timeline-element animation lookup rebuilt the three cache-key variants
by hand. elementCacheKeys already owns that list for the writers, so this
reader takes it from there instead of drifting from it.
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.
R3 review follow-ups on the keyframe cache:
- clearKeyframeCacheForFile collected ids from the index.html alias prefix
too, so a re-scan of one composition file wiped rows a sibling file had
just written (several files re-scan concurrently). Only the file's own
prefixed keys name the ids now; clearKeyframeCacheForElement still takes
the alias and bare key with them.
- toClipKeyframes fell back to a fixed 1s tween duration, which put a
duration-less tween's keyframes at a percentage no edit path agreed with.
It now spans the clip, matching resolveEditableTweenDuration.
- collectAnimatableKeyframeProperties takes `object` so call sites drop
their `as Record<string, unknown>` casts.
Regression tests cover both fixes.
Each keyframe-cache writer re-derived a clip-relative percentage inline, and the
post-commit writer rounded to 0.1% while the others used 0.001%. Selection keys
embed that number, so a commit-time rewrite could orphan a live key.
toClipPercentage owns the rounding, toClipKeyframes owns the whole row (percentage
plus the tween percentage and animation identity the lanes read), and the parsed
write reuses elementCacheKeys instead of open-coding the three key variants.
## What
Normalize both padded and trimmed render audio on the decoded sample timeline, encode the result as AAC in M4A, and keep the no-op path on stream copy. Final video muxing remains stream-copy only.
## Why
AAC packet-copy operations cannot guarantee the requested presentation duration. Trimming could retain a packet-boundary tail, while padding by concatenating a separately encoded raw-ADTS silence segment could create a timestamp/bitrate discontinuity that surfaced as roughly 606 ms of apparent audio drift in the production-style regression.
## How
- Trim with `atrim` plus `asetpts`, then AAC-encode into M4A.
- Pad with `apad=whole_dur`, cap the output at the target duration, and AAC-encode the continuous sample timeline into M4A.
- Preserve the M4A encoder-delay edit list when the normalized audio is copied into the final MP4.
- Use the normalized M4A path in both local and distributed assembly; leave already-correct audio unchanged.
- Remove the obsolete raw-ADTS concat helpers and the disproven final-mux duration workaround.
## Test plan
- [x] 18 focused pad/trim unit tests
- [x] Real-media integration regression covering the packet-timeline boundary
- [x] Producer typecheck and repository commit hooks
- [x] Faithful Docker `style-3-prod` regression: video/audio approximately 16.07 s, 1 ms drift, zero visual failures
- [x] Exact-head required CI was green before the review-cleanup commit; fresh exact-head CI is running
- [ ] Documentation updated (not applicable)
The regression was reproduced before the fix with 0.563416 s of reported audio/video drift. The sample-timeline normalization removes the malformed tail while preserving stream-copy final muxing and preview behavior.
An ungrouped tween (mixed property groups classify to propertyGroup
undefined) fed keyframeCache but was skipped by every gsapAnimations
writer, so the collapsed row drew diamonds the expanded lanes had no
source animation to render. Drop the property-group gate at all three
writers; lane consumers already filter by group.
Also route the same-percentage merge in updateKeyframeCacheFromParsed
through deduplicateKeyframes so the easeAmbiguous rule has one owner.
