chore(release): release v1.8.0 into main - #245
Merged
Merged
Conversation
The macOS x64 leg of v1.8.0-rc.6 died on the ffmpeg tarball fetch, twice,
twenty minutes apart:
curl: (35) Recv failure: Connection reset by peer
This is not congestion and not a bad pin. In both runs the arm64 leg on
`macos-latest` fetched the same tarball from the same host with the same cold
cache and succeeded, while the x64 leg on `macos-15-intel` was reset about a
second after starting. An immediate reset that reproduces on one runner pool
and never on the other is an egress-level block, so no amount of retrying
clears it — a first attempt at this shipped only `--retry-all-errors` and would
not have fixed the build.
So the source list grows a second entry. Debian's `.orig.tar.xz` is the upstream
tarball unmodified — verified byte-identical to the pinned sha256 — and
deb.debian.org is CDN-backed. Each source is tried in turn and must both
download and match the checksum; the checksum is what makes a second origin safe
to trust, and it gates every source equally. Nothing is downgraded: ffmpeg.org
stays first, and a mirror that lacks a future version is skipped rather than
fatal.
Three flags carry their own reasons:
- `--retry-all-errors`, because curl only auto-retries what it classes as
transient (timeouts, 429, 5xx) — not a reset, not a handshake failure, which
are exactly the errors seen here. Verified against a refused connection:
`--retry 2 --retry-delay 1` gives up after 0s, adding `--retry-all-errors`
spends 2s retrying.
- `--connect-timeout 20`, because a throttled origin hangs rather than refuses.
Measured: after a few rapid fetches ffmpeg.org left a connect sitting for 75s
before failing. Times four attempts, that is five minutes burned before the
second source is even tried.
- `-f`, so an HTTP error page is not written to the tarball and resurfaced as a
checksum mismatch, which reads like a moved pin rather than a bad response.
Exercised by extracting the function and running it against real endpoints:
canonical alone succeeds; a dead first source falls through to Debian; a source
serving the wrong bytes (ffmpeg 8.1.1) is rejected on checksum and the next one
is used; and with every source broken it throws listing each attempt with its
reason — curl status or the actual hash.
The sibling `scripts/fetch-ffmpeg.mjs` has the same single-source shape at line
359 but is left alone: it is the Windows/Linux path, pulls from GitHub releases
rather than ffmpeg.org, and uses node's fetch rather than curl.
A 1920x1032 window capture in a 16:9 project exports into a 1920x1080 frame: the 48 extra rows are wallpaper, and the clip itself still renders at 1:1. The export dialog decided whether a tier upscaled by comparing short sides, so it read those rows as stretched pixels and flagged "1080p - Upscale" on the exact frame the "Source" tier produced unflagged, since both tiers resolve to the same 1920x1080 there. The two tiles sat side by side with identical dimensions and different warnings. wouldUpscale() now asks the contain-fit question the compositor actually answers: min(out.w/src.w, out.h/src.h) > 1. Nothing changes for a source whose shape matches the project ratio, which is why a full-screen capture never showed this and a window capture did. That also makes the "Source" exemption provable instead of asserted - its frame is the source's long side at the project ratio, so its contain scale is never above 1 - and the general test now covers it. The exemption goes, and with it the MEDIUM_SHORT_SIDE/HIGH_SHORT_SIDE constants duplicated from mp4ExportSettings and the write-only targetShortSide field they fed.
…ers get promote.yml pushes the stable tag on this branch's tip, and nix/package.nix here still carries what v1.6.0 left on 2026-07-05. `src` is the repo tree rather than a fetched tarball, so `nix run github:…/v1.8.0` builds this file and would refuse the mismatch on sight. bump-nix-package.yml cannot cover it: it fires after the release is published, reads main, and opens its PR against main — the tag is never in its path. So the hash has to be right here, before the tag is cut. package-lock.json is byte-identical between this branch and main, so the value is the one CI computed on main in run 30789963366, not a fresh guess: sha256-SggSPoDnKzmvgXpIGP11y6h390SkoZszeMjFTaokRjQ= Version goes straight to 1.8.0 — promote rewrites package.json at tag time but never touches this file. With both lines correct here, the bump workflow finds nothing to change after the release and no-ops, which is the intended resting state rather than a repair.
…uming one per tick Free-running preview playback (and the poc-d3d harness) decoded exactly one real frame per 1/60s tick, assuming a constant ~60fps source. ScreenCaptureKit (and equivalent screen captures) only emits a frame when the screen changes, so a recording with long static stretches could contain only a few hundred real frames over its whole duration. Consuming one frame per tick regardless exhausted the stream long before elapsed wall time reached the recording's duration, so the decoder hit EOF, looped back to the start, and the preview appeared to accelerate then jump back to the beginning. Adds a peek/commit lookahead (peek_next_time_sec / commit_peek) to each platform decoder (linux, macos, windows) so a frame is only adopted once its pts is actually due; otherwise the current frame is held. live::Player::step and timeline_walk::advance_decoder_to (already correct on the export path) now share this hold semantics, and render_thread's accumulator tracks source time actually consumed instead of a fixed 1/60s step per tick. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…test it Review follow-ups on the frame-hold fix — three from CodeRabbit, three from a second read. An unusable pts no longer reads as "due" (CodeRabbit). `peek_next_time_sec` returned `Some(0.0)` when `best_effort_timestamp` is `i64::MIN` or the time_base is zero, and `0.0` satisfies the commit condition against every target — so a stream with no usable pts was drained frame after frame to EOF, which is the exact failure the hold semantics exist to prevent, only worse. The three decoders now return a `NextFrameTime` that says `At`, `Unknown` or `Eof`, and `Unknown` advances exactly ONE frame before yielding — the pre-hold behaviour, restricted to the broken stream that warrants it instead of being the general rule. `commit_peek` enforces its precondition with `bail!` instead of `debug_assert!` (CodeRabbit). Compiled out in release, the assertion let a caller promote an `AVFrame` that was never filled, with an undefined `best_effort_timestamp`, all the way into the presentation path. `rewind()` clears the pending peek (CodeRabbit). `seek_to` already did; `rewind` ran the same `av_seek_frame` + `avcodec_flush_buffers` without it, so the next `next()` promoted a frame decoded at the pre-rewind position, carrying its old `cur_pts`. Windows and macOS both. The export's EOF behaviour is now stated rather than implied. `advance_decoder_to` returning `true` at EOF (it returned `false`) stops the clip loop from breaking early, so a clip whose declared window outruns its last real pts fills that window by holding its last frame. That is what the audio already assumes — `on_clip_end` reports the clip's frame count and the audio is stretched over the DECLARED duration — so a video that stopped short used to shift the next clip's junction. The trade is real and now documented: a genuinely truncated source freezes to the end of its window instead of stopping. `advance_decoder_to` also re-checks the frame it just adopted, restoring the per-iteration invariant the entry guard used to provide. And it has tests now, which the original change did not: the decision is extracted into a pure `frame_step`, and the accumulator arithmetic into `consume_acc`, so both are exercisable without ffmpeg or a file. They cover what the bug was actually about — a 24fps source adopting 24 frames per real second, not 60 — plus the sparse-source gap, the exact-pts boundary, the webcam offset, EOF hold, and the unusable-pts case. 122/122 pass.
…every tick Review follow-up: a webcam track shorter than the screen — the camera stopping before the capture does, which the doc already called a normal case — put the player into permanent full-speed decoding. `target_webcam_t` is derived from the screen clock and keeps growing. At the webcam's EOF the catch-up loop seeked back to 0 and left the target untouched, so the NEXT tick restarted the catch-up from t=0 against a target still tens of minutes away and committed frames until the 1000-frame guard cut it off. Every tick. The webcam file was re-decoded end to end, forever, to display a track that had nothing left to show. Holding its last frame is both the fix and the semantics this PR is built on. Once EOF is a hold, the webcam decision is exactly the screen's at export time, so the loop now calls `frame_step` directly instead of carrying its own copy of the four cases — and `frame_step`'s tests cover this path too. 122/122 pass.
A 30-minute recording was one `/inference` call: ~10 minutes with no progress, no recovery from a transient failure, and — the reported bug — killed outright before it ever finished. Node's global fetch (undici) applies a 300s `headersTimeout`, and whisper sends no response header until the whole upload is transcribed. Measured on the reporter's file: whisper needed 574s, undici cut the connection at 300s, and the renderer surfaced an unactionable "Transcription failed / fetch failed". The helper kept burning CPU on a transcription nobody was listening for. Split the audio into ~90s chunks and run them one at a time: - `chunking.ts` nudges each boundary to the quietest 20ms frame within ±3s of the ideal cut, so a chunk break lands in a pause instead of mid-word. Energy minimum, not a VAD: whisper.cpp's Silero VAD runs per REQUEST, so it cannot tell us where to cut before we upload. - `SttManager.transcribe` shifts each chunk's timestamps to absolute time, emits `completedSec`/`totalSec` per chunk, retries a chunk up to 3 times, and re-runs `server.start()` between attempts (idempotent when the helper is alive, a respawn when it isn't) — the usual cause of a mid-run failure is a dead helper, not a bad chunk. - The language detected on the first chunk is pinned for the rest. Left to auto-detect, whisper can flip mid-recording on a chunk that opens with a proper noun and "transcribe" the remainder as another language. - `whisperServer` bounds each request at 280s and names the failure with the helper's stderr, instead of letting undici's 300s ceiling surface as a bare "fetch failed". Chunks run SEQUENTIALLY, measured rather than assumed: whisper-stt-server holds a single model context, and two 120s chunks took 76.9s one after the other vs 144.1s fired together (0.53x — concurrency is ~1.9x SLOWER). A client-side worker pool would be a pessimisation. The progress reaches the UI: the renderer's status callback forwarded only a phase string and never subscribed to the main process's events, so the toast showed one static "transcribing" for the whole run. It now carries the chunk progress and renders a real bar. Verified end to end on a 32-minute source: 22 chunks, timestamps monotonic across every seam, last segment at 1950.7s of 1951.0s, slowest chunk 76.8s against the 280s ceiling.
…nner The chunked pipeline now reports how much audio it has transcribed, so surface it. A 30-minute recording spends minutes in "Transcribing…", and a spinner that never changes is indistinguishable from a hang. Progress travels the same road the phase already did — `status.ts` owns the vocabulary, the store owns the queue, `TranscriptionStatus.tsx` owns how a job reads on screen: - `TranscriptionProgress` + `progressFraction` join `TranscriptionPhase` in status.ts, and `deriveAssetStatus` carries them onto the view. - The store's `onStatus` no longer casts its argument to `TranscriptionPhase`: the renderer's `TranscribeStatus` and `TranscriptionPhase` are now genuinely the same vocabulary, including the `"loading-model"` phase the cast used to paper over. Failure paths clear `progress` with `phase`, so a failed job cannot leave a stale bar. - `TranscriptionStatusDot`'s sibling `TranscriptionProgressBar` renders a determinate bar, and the label gains a percentage. Both render nothing until the run reports measurable progress. Queued, extracting audio and downloading the model have no fraction to report, and a bar pinned at 0% reads as "stuck" where the spinner reads as "working".
The mock listener in transcribe.test.ts declared its own event shape and still had the pre-progress one, so `tsc -p tsconfig.test.json` rejected the `completedSec`/`totalSec` the test itself asserts on. Runtime was fine, which is why vitest passed and only the CI typecheck caught it.
Five defects from the review of this PR, all in the new chunk loop. `"auto"` defeated the cross-chunk language pin. The request contract spells it as the explicit way to ask for detection, and it is truthy, so `if (!language)` never fired and every chunk detected independently — the exact flip the pin exists to prevent. Normalized to `undefined`. Verified end-to-end on 352s of real speech: the chunks now go `[undefined, "en", "en", "en"]` where they were all-detect before. Nothing could stop a run. The chunk boundary is the natural interruption point and went unused: cancelling in the renderer left main transcribing every remaining chunk while the replacement request queued behind it, which is what made "regenerate in another language" look dead. `cancel()` bumps an epoch the loop checks between chunks, reachable over a new `stt:cancel` channel, and surfaces as an `AbortError` so the store drops the job quietly. Measured on a 375s clip: cancelling after chunk 1 returns in 19s, not 143s. The IPC status sink was a single slot each request saved and restored, so two overlapping transcriptions ended with the first one to finish silencing the other for the rest of its run — no progress, which reads as a hang. It is a Set now, and each request detaches only its own. Every fetch rejection claimed "after 280s". A helper that died a moment ago rejects in a millisecond, and telling the reader it spent 280s on an over-long chunk sends them somewhere else entirely. The timeout wording is reserved for an actual timeout, the cause is attached, and the body read is named too. A chunk that exhausts its retries now says where it died rather than failing a 30-minute recording with no position at all. Energy ties broke toward the earliest frame, so digital silence — a muted track, a gap between takes — pulled every cut back by the whole search window: 90s chunks became 87s, and the silent test case 10s became 5s. Ties now break toward the target. The pin's comment also records what a stale staged helper does to it, since that cost a full debugging detour: electron/native/bin is gitignored, a binary from before cc78180 echoes the request instead of resolving it, and the failure is completely silent.
…nload The bar was wrapped in a div carrying `margin: -8px 0 16px`, rendered unconditionally. `.mediaDetail` is a flex column, so those margins do not collapse: every media card that was not transcribing paid 8px of dead gap, and the failure hint below it sat 8px lower than intended. A wrapper cannot render itself away with its child — the margin belongs on the bar. `"loading-model"` was added to `TranscriptionPhase` in this PR, plumbed through `TranscribeStatus` and the store, and then read by nothing: every label switches on `status`, so the 253MB first-run download was labelled "Transcribing" like everything else. That download is the wait most often mistaken for a hang, which is what this PR is about, so it gets its own words rather than the plumbing getting deleted.
`--connect-timeout` only covers the handshake. An origin that accepts the
connection and then trickles never errors, so nothing retried and nothing
fell through to the Debian mirror — the job would sit until the runner's
six-hour limit, which is the failure the mirror was added to survive.
`--speed-limit 1024 --speed-time 30` bounds the part `--connect-timeout`
cannot reach, and cannot fire on a merely slow link for a ~10MB tarball.
`spawnSync` reports a missing curl as `{ status: null, error: ENOENT }`, so
"curl exited with null" twice over sent the reader hunting a network problem
instead of a missing binary. The error is surfaced when present.
The comment claimed a 404 mirror is "simply skipped"; with
`--retry-all-errors` it is retried three times first. The flags stay — that
retry is the whole point on a connection reset — and the comment now says
what actually happens.
nix/package.nix restated the version by hand and had already drifted two
minors behind the app it names (1.6.0, then 1.8.0 against a 1.8.0-rc.6
tree). It reads package.json now. `npmDepsHash` still needs updating by
hand, but that one fails loudly.
The language whisper resolved on chunk 1 — the one every later chunk is pinned to — reached the document and was rendered nowhere. It had a pill, in `SourceTranscriptModal`. That modal is mounted by `LeftPanel`'s `MediaPane`, and `LeftPanel` is `active === "chat" ? <ChatStripPanel /> : <MediaPane />` with exactly one mount site passing the literal `active="chat"`. So `MediaPane`, `MediaList` and the pill are unreachable, and "did the language pin work?" had no answer in the UI at all — only in the saved .openscreen. Put it on the live surface, next to the status badge in the v4 MediaStage detail panel. It also belongs beside "Regenerate as": that selector is the control you set BECAUSE of what was detected, and it currently reads "Auto" next to a transcript that resolved "en".
Scrubbing a 30-minute recording (4501 words) stutters. The transcript pane
subscribes to the playhead and re-renders per frame, which is by design —
`TranscriptClipBlock` is memoised on `cueWordId` precisely so that the frames
in between cost nothing.
That memo's premise is playback: "cueWordId changes at word boundaries, a few
times per second, not sixty". A scrub breaks it. Dragging the playhead crosses
many words per frame, so `cueWordId` changes on EVERY frame, the block
re-renders every frame, and it renders one component per transcript word.
`TranscriptWord` had no memo, so all 4501 re-rendered to move one underline.
Measured over a 40-frame scrub in jsdom, median per frame:
words before after
100 19.6 ms 11.5 ms
500 24.0 ms 13.9 ms
1500 83.9 ms 24.7 ms
4501 132.6 ms 57.4 ms
The same benchmark with the cue word held inside one word — so the block's
memo bails out — costs 0.1 ms/frame at 4501 words. That isolates it: the
linear `findCueWordId` scan and the playhead subscription are both free, and
100% of what remains is the block re-render itself.
So this halves it but does not fix the shape: the cost is still proportional
to transcript length, because the block still builds 4501 React elements per
frame for the memo to then discard. The O(1) fix is to stop routing the cue
through render at all — subscribe outside React and toggle a class on the two
nodes that changed. That is a real change to a contentEditable with caret and
selection handling, so it is not smuggled in here.
The Media tab's timeline exists to add, remove and reorder clips. It carried the Edit tab's furniture anyway: a transport (play, prev/next, timecode, scrub bar), the Shift/Ctrl+Scroll hints, the zoom/pan window, and a playhead — none of which has anything to act on when there is no playback and no per-clip editing on that surface. All four are now gated on the `variant === "edit"` flag the component already had, and the "Arrange clips" caption centres, being alone in the header. Two behaviours go with them rather than being left inert, which is the part that would have bitten: - `startScrub` returns early. Seeking a playhead that is not rendered still wrote `currentTimeSec`, so a click on the Media timeline silently moved the Edit tab's preview from a screen displaying no time at all. - the Ctrl/Shift+wheel listener is not attached. Zooming with the zoom window gone leaves no control to undo it and no ruler reading to explain it.
Measured head-to-head in Chromium on a 32-minute recording (68 MB on disk),
which corrects what an earlier version of this message claimed:
STREAMING total 12296 ms (peaks 12259 ms, ~192 kB peak memory)
IN-MEMORY total 12198 ms (decodeAudioData 12003 ms → 714 MB,
channel slice copy 160 ms)
So the two pipelines take the SAME time, and the allocation is not the cost:
the slice copy I suspected is 160 ms, 1.3% of the total. Browser audio
decoding is the cost, in both paths, at roughly 160x realtime.
What this commit therefore does and does not do:
- Routing now estimates DECODED bytes from duration instead of comparing the
file's size, which says nothing about decoded size on compressed video (68 MB
→ 714 MB here). That is a MEMORY fix — 714 MB down to ~192 kB — and buys no
speed. The existing 256 MB threshold was already meant to protect memory; it
just measured the wrong quantity, so the streaming path never ran for
recordings that clearly needed it.
- The cache moves from a `useRef` (one per mounted component) to module scope,
with an in-flight map so N clips of one asset share a single decode. Every
clip used to decode independently, and every unmount threw the result away,
so a Media↔Edit switch re-decoded the whole recording. That is a real fix,
but only for REPEAT mounts — the first waveform of a session still waits the
full 12 s.
Making the first one fast needs a different pipeline, not a different route:
the bundled ffmpeg produces waveform-grade PCM for this same file in 2038 ms
(`-vn -ac 1 -ar 1000 -f s16le`, 3.9 MB out), 6x faster than the browser and in
the main process. With peaks cached on disk it would be paid once per
recording, ever. Not attempted here.
Both behaviours are covered, and both tests fail against the previous code.
The waveform took ~12s to appear on a 32-minute recording because both renderer pipelines decode the whole audio track in Chromium. Measured on the same 68 MB file: decodeAudioData (whole track) 12003 ms 714 MB resident WebCodecs chunk-by-chunk 12259 ms ~192 kB resident ffmpeg in the main process 3382 ms nothing resident 3.6x, off the UI process, and the result is cached on disk keyed by path+size+mtime — so it is paid once per recording rather than once per session. The renderer keeps both old pipelines and falls back to them when no native binary resolves, so nothing loses its waveform. WHICH ffmpeg, because this is where it would have broken silently: electron-builder deliberately excludes the static `ffmpeg.exe` (109 MB, "nothing in the app spawns" it). Spawning that one would have worked in dev and failed in every installed build. The SHARED build is 1 MB and links the same av*.dll set the compositor already ships, so fetch-ffmpeg.mjs now stages it as `ffmpeg-shared.exe` — named apart so the packager's `!win32-*/ffmpeg.exe` rule keeps dropping the static build while this one ships under the existing `win32-*/*` include. No packaging rule changes. Peaks are folded incrementally out of ffmpeg's stdout (mono s16 at 16 kHz), so the 62 MB of PCM never exists at once, and the block maths matches audioPeaksWorker.ts and streamingAudioPeaks.ts exactly — a clip must not change shape depending on which pipeline drew it. macOS and Linux have no binary staged in electron/native/bin, so `resolveFfmpeg` returns null there and they keep today's behaviour unchanged. ponytail: the CLI, not libav bindings in the compositor addon. Skipping the spawn saves ~20 ms against a ~2000 ms decode, and would cost a new Rust surface, an N-API entry point and a build story on three platforms. Parallelism was measured and rejected too: 4 processes over segments ran 1735 ms against 1893 ms for one, and 8 ran 2100 ms — it is demux- and spawn-bound, not CPU-bound. The disk cache is the real win.
… refs
Two findings from the automated review on the main-line version of this change:
- the mocked `tl` was cast to `any` behind a biome-ignore, which AGENTS.md rules
out ("don't add new `any`"). Cast through `unknown` to the real
`ReturnType<typeof useTimeline>` instead: the prop keeps its type and the
suppression goes away.
- the "add a region kind" checklist pointed at pre-change line numbers in
V4Timeline.tsx, and this work moved them by ~130 lines. Recomputed against
this branch: :463-511 for the pill call site, :1504-1512 for the lane render
block, :334 for the `kind` union. (They were already drifting beforehand —
check-docs does not verify line numbers.)
The third finding, `currentColor` → `currentcolor` for stylelint's
value-keyword-case, does not apply here: the repo has no stylelint at all (CI's
Lint job is `biome check`, green on both spellings), and the convention in
website/src/css/custom.css is `currentColor`. Changing it would leave the only
lowercase spelling in the codebase.
… win Two store-level changes the editor shell needs to fix what the user sees. **Creation duration is now a parameter.** All five `add*` hardcoded 2 s. On a 30-minute recording zoomed out that is one pixel of pill — it only ever looked right because the removed 1.5%-of-the-timeline minimum width inflated it in the rendering. The timeline's toolbar now passes a duration worth a fixed number of pixels at the current zoom; every other entry point (keyboard shortcuts, the agent, auto-zooms) keeps `DEFAULT_NEW_REGION_SEC`, the same 2 s as before. **A clip and a pill can no longer both be selected.** `selectRegion` left `clipSelection` standing and `selectClip` left `selection` standing, so the app could hold one element while the user was looking at another highlighted. Every consumer that asks "is a clip selected?" — copy, paste, delete — then acted on the invisible one, which is why Ctrl+C/Ctrl+V always operated on the clip. They now cancel each other, and `clearSelection` (background click) drops both. `clearRegionClipboard` comes along for the shell: copying a clip has to retire a copied region, or the two clipboards stay loaded at once and paste has to guess. Both are covered in useTimeline.test.ts, ablation-checked — dropping either `setClipSelection(null)` or the duration parameter turns the matching test red.
…pied **Creating from the toolbar.** The five buttons now ask for a duration worth PILL_CREATE_PX (40px) at the current zoom, so a new pill is the same size on screen whatever the zoom: zoomed out you get a long region, zoomed in a short one. Floored at 0.25 s, which only bites past ~30x where 40px is worth hundredths of a second and the region would be born unusable. Only this path scales — the keyboard shortcuts live in NewEditorShell and have no access to the zoom (`nav` is local state in V4Timeline), so they keep the flat default. **Copy/paste.** Ctrl+V always duplicated a clip. Three reasons, all fixed here: - paste fell back to `tl.clipSelection`, so a clip merely being SELECTED hijacked it — you never got the region you had copied; - `copiedClipId` was never cleared, so one Ctrl+C on a clip turned every later Ctrl+V into a clip duplication for the rest of the session; - copy preferred the clip over the pill, and since both could be selected at once (fixed in the commit before), it usually found one. Copy now reads the same arrays the lanes render, so what lands in the clipboard is what the user is looking at. That also repairs two kinds that could never be copied: `cameraFullscreen` went down the speed branch and `trim` was rewritten to "zoom" — neither was ever found, so Ctrl+C silently did nothing. Trims stay out on purpose: they are stored in source time against a clip anchor, so pasting one is a ventilation problem, not a copy (cut already excludes them). Pasted regions are now anchored with `anchorRegionsWithDerivedMs` like every add* does. Paste used to store a bare startMs/endMs, so the region survived until the first clip reorder or trim and then drifted off its content.
Reported on rc.8: adding a pill on a 30-minute recording still produced
something invisible, hidden behind the playhead it was created at.
The previous commit scaled only the toolbar buttons, because the zoom lives in
V4Timeline's local `nav` state and the shortcuts are handled in NewEditorShell.
That was the wrong line to draw: the empty lanes advertise the shortcut ("Press
Z to add zoom") and the buttons are labelled "Add Zoom (Z)", so the keyboard is
how most regions actually get created — and it kept the flat 2 s, which is 0.8px
at 0.42 px/s. Measured in a browser on the reported case (30 min across a 761px
panel): the button asked for 94.6 s, the shortcut for 2 s.
The rule now lives in timeline/newRegionDuration, outside the component: the
timeline publishes its scale (px per second) and both creation paths read it at
the moment the user acts. Module state read imperatively, never subscribed —
the value changes on every zoom notch and nothing renders it, so a subscription
would re-render the whole editor shell for a number only a keypress reads. Same
reasoning as playheadSec() in useTimeline.
Reading it at CLICK time rather than at render time also drops a one-notch
staleness: the old handler captured the duration computed by the render that
preceded the effect publishing the new scale.
Verified in a real browser, both paths: 761px / 1800 s → 94.61 s → a pill 40.0px
wide, identical for the button and for what the shortcut passes. Unit-tested in
newRegionDuration.test.ts, including the invariant that duration × scale is
PILL_CREATE_PX at any zoom.
…tation" The string is the pill's own label (and its tooltip) — the only place the key is used. A pill is not new for long, so "New annotation" read as a stale button caption sitting on the timeline. Each locale keeps its existing noun with the "new" qualifier dropped, so nothing needed translating from scratch. The key stays `toolbar.newAnnotation`: renaming it would touch 13 locale files plus the component and its test for no user-visible gain.
40px was picked to clear the icon-and-label threshold; it lands right on it, so a fresh pill showed its icon and an ellipsis. Measured in a browser, the width each label needs before the ellipsis bites — icon + gap + text + padding — is "Full Camera" 93px, "Annotation" 90px, "1.80×" 61px, "1.5×" 55px. 96 clears all four, verified: at that width none of the four is clipped. The cost is a longer default region: at full zoom-out on a 30-minute recording (~0.42 px/s) one create is now about 3 min 45 s of timeline instead of 1 min 35. That is what a constant width means — the duration is the variable — and the pill lands with both handles in reach to trim it down.
Trims were excluded because they are stored in source time against a clip
anchor, so there is no row to clone onto another position. That framing was
wrong: a paste never reuses the copied position anyway — a pasted zoom keeps its
properties and takes its start from the playhead, so its start and end change by
definition. A trim simply has no properties left once position is removed, which
makes the copy exactly one number: how long the cut was.
So copying a trim stores `{ durationSec }` (read off the coalesced pill, so a
trim ventilated across a clip boundary copies as the single length the user
sees), and pasting calls the same `addTrim(duration)` the toolbar's cut button
uses — which resolves the span down to the carrying clip's source time.
Cut (Ctrl+X) drops its trim exclusion for the same reason; removeRegion already
handled the kind.
Also fixes a stale-closure bug this exposed: pasteRegion was memoized on
[saveDocument] while now calling tl.addTrim, and useTimeline returns a fresh
object every render — the callback would have pasted through an old document.
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (71)
📝 WalkthroughWalkthroughThis PR adds timestamp-based compositor playback, chunked STT with progress and cancellation, native FFmpeg audio peaks, timeline region sizing, media-mode controls, export validation, localization updates, and FFmpeg packaging changes. ChangesTimestamp-driven compositor playback
Electron media and transcription
Editor timeline and transcription UI
Export and release support
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
EtienneLescot
force-pushed
the
release/v1.8.0-sync
branch
from
August 4, 2026 03:03
e99dbe6 to
59f3f53
Compare
EtienneLescot
pushed a commit
that referenced
this pull request
Aug 4, 2026
The 1.8.0 sync turned `onStatus` from a bare phase into an `SttRendererStatus` object carrying chunk progress, but the CLI captions runner still declared the old `(phase) => void` callback. Neither branch was broken alone; the rebase in #245 put them together and main has not typechecked since. `onStatus` now also fires once per transcribed chunk rather than once per phase, so log only on a phase change -- otherwise a long transcription emits one identical line per chunk.
EtienneLescot
pushed a commit
that referenced
this pull request
Aug 4, 2026
The 1.8.0 sync turned `onStatus` from a bare phase into an `SttRendererStatus` object carrying chunk progress, but the CLI captions runner still declared the old `(phase) => void` callback. Neither branch was broken alone; the rebase in #245 put them together and main has not typechecked since. `onStatus` now also fires once per transcribed chunk rather than once per phase, so log only on a phase change -- otherwise a long transcription emits one identical line per chunk.
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
2 participants
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.
Sync main with the released snapshot (RC + cherry-picked bugfixes + version bump). Rebase-merged via PAT; bypass applies because EtienneLescot is a ruleset bypass actor.
Summary by CodeRabbit