Save Ring's detection-time snapshot and log per-capture latency

[fayerman-source]

Motion clips were starting after subjects had already walked past — recordings showed people leaving rather than approaching.

The measurement

Median over 10 real motion events on a battery Spotlight Cam Plus, pairing Ring's own event timestamps against clip timestamps:

Segment	Typical	Whose latency
Ring detects motion → push arrives here	~2.3s	Ring's
Trigger → live stream negotiated	~3.7s	Ring's (WebRTC signaling + camera wake)
Stream open → first bytes on disk	~2.5s	ffmpeg stream analysis
Ring detection → first frame	~8.5s

~12 m of approach at walking pace, never recorded. Almost all of it is Ring-side.

This can't be fixed with pre-roll here. A stream opened after an event cannot contain footage from before it. I also confirmed the cloud route is closed without a subscription: getEvents() reports every motion event as recorded=true, recording_status=ready, but getRecordingUrl() returns "Source video not available" and videoSearch() returns [].

What this adds

The one thing that does predate the first video frame: the still Ring captures at detection time. Its uuid rides along on the motion push (img.snapshot_uuid), so it's obtainable without Ring Protect, costs one REST fetch, and doesn't wake the camera.

• detection.ts — parses the push into a detection context (Ring's detection timestamp, ding id, subtype, snapshot uuid) and computes the latency segments. Pure, so the arithmetic is testable without an account, camera, or clock.
• events.ts — subscribes to onNewNotification alongside the existing boolean motion stream. Fetches the snapshot in parallel with the recording, not after: Ring expires these, so waiting out a 30s clip risks losing the one frame showing the approach.
• recorder.ts — captures triggerAt / streamOpenAt / firstFrameAt. First-frame is detected by polling for non-zero file size, i.e. time-to-first-bytes (the fMP4 header flushes just ahead of the first fragment) — within a frame interval of first-frame, and far below the multi-second latency being measured.
• files.ts — appendJsonLine, which reports failure rather than throwing.

Failure isolation. All of this is diagnostic bookkeeping hung off a recording that already succeeded, so none of it can turn a saved clip into a reported failure. A missing uuid, an expired snapshot, an unwritable log, and a camera exposing no notification stream all degrade to a null field. Each case has a test.

Config

detectionSnapshots and timingLog, both default true. Rows land in <outputDir>/timing.jsonl, one per capture:

{"camera":"Front","clip":"Front_….mp4","detectionSnapshot":"Front_….detection.jpg",
 "subtype":"motion","ringEventAt":"…","firstFrameAt":"…",
 "pushDelaySec":2.27,"streamSetupSec":3.75,"firstFrameSec":2.45,"blindWindowSec":8.5}

blindWindowSec is the headline number, so the effect of a motion-zone or sensitivity change can be measured against the 8.5s baseline rather than eyeballed. Backlog #4's captures.jsonl manifest can supersede this file later.

Also

Corrects the README, which claimed the pre-roll loss was "~1-2s". Measured local latency alone is ~4-6s and the full window ~8.5s — that line is what set the wrong expectation in the first place.

Verification

• 31 tests pass (20 new), no type errors.
• Service starts, subscribes, and shuts down cleanly against the live API.
• A real capture returns populated timings: 4.35s stream setup, 7.01s to first bytes.
• Two bugs were caught by the new tests during development: secs() was returning milliseconds rather than seconds, and a wiring test mixed a real-dated push with fake timings. Both fixed.

Not yet confirmed: whether img.snapshot_uuid is actually populated for this camera. It's optional in Ring's payload and needs a live motion event to verify — which is precisely what this instrumentation will tell us. The no snapshot uuid path is tested and harmless.

🤖 Generated with Claude Code

[codereviewbot-ai]

PR Review Summary

The changes in this PR look well-designed and thoroughly tested. Latency accounting, parallel detection snapshot retrieval, and timing log appending are implemented with careful error handling so diagnostic metadata collection will never interrupt core clip recording.

Key Highlights

• Resilient Snapshot Retrieval: fetchDetectionSnapshot catches errors internally and resolves null, preventing snapshot API failures from blocking or dropping clip recordings.
• Interval Cleanup: watchForFirstBytes guarantees clearing its setInterval via finally in recordClip.
• Test Coverage: Extensive hermetic tests in test/detection.test.mjs cover notification parsing, timing calculations, and RxJS subscription wiring.

Recommendations

• Minor nit in src/events.ts: Use basename from node:path instead of .split('/') to remain cross-platform safe on Windows.

---

🤖 Reviewed by codereviewbot.ai - Catch bugs before your team does.

[chatgpt-codex-connector]

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

[sonarqubecloud]

Quality Gate passed

Issues
1 New issue
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[codereviewbot-ai]

Using .split('/') to extract the file name from a path will fail on Windows systems where path separators are \. Consider using basename from node:path for cross-platform compatibility.

Suggested change

	snapshotFile = path.split('/').pop() ?? null;
	import { basename, join } from 'node:path';
	// ...
	snapshotFile = basename(path);
	// ...
	clipFile: basename(result.path),