Skip to content

Harden recording durability and raise capture quality#13

Merged
tadaspetra merged 1 commit into
mainfrom
recording-durability-quality
Jul 2, 2026
Merged

Harden recording durability and raise capture quality#13
tadaspetra merged 1 commit into
mainfrom
recording-durability-quality

Conversation

@tadaspetra

Copy link
Copy Markdown
Owner

What changed

Implements every finding from a full recording-pipeline audit: no recorded bytes should ever be lost, and capture should run at the highest sane quality.

Durability

  • No rename-clobbering: finalizeRecording and recoverOrphanRecording never overwrite an existing final file — they uniquify to recording-<takeId>-<suffix>-2.webm, -3, … (previously a crash → recover → re-record cycle with the same takeId silently destroyed the earlier file).
  • fsync failures surfaced: finalize still renames (bytes under a final name beat a stuck .part) but now returns a durability warning carried through IPC and logged prominently in the renderer.
  • Periodic fsync (default 5s, injectable) during recording bounds data loss on an OS-level crash; a failed periodic fsync is reported once and never kills the recording.
  • Shutdown backstop: closeAllRecordingHandlesForShutdown() on before-quit fsyncs + closes every open fd, leaving .part files intact for the existing orphan-recovery overlay.
  • Quit guard: closing the window mid-recording now prompts ("Recording in progress — stop and save before quitting?"), finalizes to disk, then closes. Pure close-guard state machine in src/main/app/close-guard.ts, renderer decision helper in close-request.ts; anti-wedge fallbacks so a broken renderer can never block quit.
  • Failures visible immediately: chunk-append IPC failures and recorder.onerror now show a status-line warning the moment they happen. Screen/mic failures auto-stop exactly once (re-entry-guarded) to finalize what's already on disk; camera failures warn and keep recording so partial success still saves the screen file. Silent system-audio fallback now shows a notice.

Quality

  • Codec preference is now VP9 → VP8 → plain webm, with an explicit warning when falling back to the browser default.
  • Camera captures up to 4K, with bitrate scaled to the actual captured resolution via computeCameraVideoBitsPerSecond (≥4K: 30 Mbps, ≥1440p: 20 Mbps, else 12 Mbps — up from a hardcoded 10). Frame rate stays capped at 30 deliberately: camera WebM is VFR and this repo has documented camera/audio sync-drift history.
  • Explicit mic constraints: 48 kHz stereo ideal, echoCancellation/noiseSuppression/autoGainControl off for raw capture, with a one-shot deviceId-only retry so constrained devices still open. Screen constraints and the 30 Mbps screen bitrate are untouched; start-epoch/timeslice logic untouched.

Also in this PR

  • AGENTS.md test-first guidance rewritten to be layer-calibrated (tests-first for pure/main-process logic; honest e2e-or-manual verification for OS/device/MediaRecorder behavior; behavior-level checks for UI glue).
  • Two pre-existing pnpm run check failures fixed (they reproduce on clean main): the waveform-sources.ts:42 typecheck error (widened getTakePlaybackSources param, behavior unchanged) and dead invalidateTakeWaveformCache lint error.
  • docs/production/feature-inventory.md updated for the quit guard.

Testing

Tests were written first per AGENTS.md and confirmed failing before implementation. ~40 new tests across tests/integration/recording-service.test.ts (rename uniquification, fsync warning, periodic fsync with fake clock, shutdown handle closing) and tests/unit/ (close-guard, close-request, create-window close interception, register-handlers, preload bridge, classifyRecorderFailure policy, bitrate tiers, mime ordering, mic constraints). Full pnpm run check passes.

Reviewer notes

  • Per AGENTS.md, helper tests are not sufficient proof for OS/device-level behavior. Manual verification worth doing before relying on this in production: quit mid-recording (confirm prompt → finalize → recoverable take), Cmd+Q force-path (relaunch offers orphan recovery), unplug camera mid-recording (screen still saves), and a screen+camera+mic recording spot-checked for A/V sync on export — VP9 + up-to-4K camera raises encoder load, and sync drift is this repo's most-regressed area.
  • An automated multi-agent adversarial review pass was attempted but its reviewer agents stalled and produced no findings — this diff has not had an independent review beyond its own tests, so human review matters more than usual here.
  • Disabling AEC/NS/AGC changes mic character to raw capture (intended, but audible in echoey rooms).

🤖 Generated with Claude Code

Durability (no recorded bytes lost, ever):
- Never rename over an existing final recording: finalize and orphan
  recovery uniquify to recording-<takeId>-<suffix>-2.webm, -3, ...
- fsync failures at finalize surface a durability warning through IPC
  instead of being swallowed; rename still proceeds
- Periodic fsync (default 5s) during recording bounds data loss on OS
  crash; failures reported once, never kill the recording
- before-quit backstop fsyncs and closes all open handles, leaving
  .part files recoverable
- Quit guard: window close is intercepted mid-recording, user confirms,
  recording finalizes to disk before the window closes
- Capture failures surface immediately in the recording UI; screen/mic
  failures auto-stop once to save what's on disk, camera failures keep
  recording for partial success; system-audio fallback shows a notice

Quality:
- VP9 preferred over VP8, warn when falling back to browser default
- Camera up to 4K with bitrate scaled to actual captured resolution
  (12/20/30 Mbps); frame rate stays capped at 30 to protect sync
- Explicit mic constraints: 48kHz stereo, no AEC/NS/AGC, with a
  deviceId-only retry so constrained mics still open

Also updates AGENTS.md test-first guidance to be layer-calibrated, and
fixes two pre-existing check failures (waveform-sources typecheck,
dead invalidateTakeWaveformCache).

Tests written first per AGENTS.md; full pnpm run check passes.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@tadaspetra tadaspetra merged commit a4f5703 into main Jul 2, 2026
2 checks passed
@tadaspetra tadaspetra deleted the recording-durability-quality branch July 2, 2026 13:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant