feat: conditional window capture + recording profiling with auto-wormhole

[abrichr]

Summary

• Make window reader/writer threads conditional on RECORD_WINDOW_DATA (defaults to False), eliminating an unnecessary thread + process + expensive platform API calls during recording
• Add sleep throttle to read_window_events (0.1s) and memory_writer (1s) tight loops
• Add profiling summary at end of record() — saves profiling.json to capture dir with duration, event counts/rates, config flags, whether running on main thread, and thread count
• Auto-send profiling.json via Magic Wormhole after recording stops for easy remote debugging

Context

Investigating why record_waa_demos.py is noticeably laggier than python -m openadapt_capture.cli record on Windows. This PR adds instrumentation to compare both paths — after each recording, profiling data is automatically wormhole-sent for analysis.

Test plan

• All 129 tests pass
• Lint clean
• Run CLI recording on Windows, receive profiling via wormhole
• Run record_waa_demos.py on Windows, receive profiling via wormhole
• Compare profiles to identify root cause of lag difference

🤖 Generated with Claude Code

[abrichr]

Code Review: PR #12 — Conditional Window Capture + Recording Profiling

Summary

This PR makes 5 distinct changes:

1. Conditional window capture — wraps window reader/writer threads behind RECORD_WINDOW_DATA (default False)
2. Recording profiling — saves profiling.json with duration, event counts, timing stats, config
3. Auto-wormhole — opt-in --send_profile flag to send profiling data via Magic Wormhole
4. Bug fixes — PyAV pict_type enum, stop sequence IndexError, video finalization crash on early Ctrl+C, matplotlib Agg backend
5. CaptureSession properties — adds pixel_ratio and audio_start_time to fix html.py

CI passes (lint + tests on 3.10/3.11/3.12). Branch is 4 commits behind main (release commits + docs sync) but GitHub reports it as mergeable.

---

Issues Found

1. BREAKING: magic-wormhole moved from optional to required dependency

File: pyproject.toml
Severity: High

The PR removes the [share] optional extra and moves magic-wormhole>=0.17.0 into core dependencies. This is a heavy dependency (pulls in Twisted, autobahn, cryptography, etc.) that will be installed for ALL users, even those who never use sharing or profiling.

The existing share.py has _ensure_wormhole() which auto-installs if missing — and the new _send_profiling_via_wormhole() already handles the case where wormhole is not found gracefully. There's no need to make it a required dependency.

However, I see that on the current main branch, the share extra still exists. The diff removes it. This means existing users doing pip install openadapt-capture[share] will get an error after this merges. The [all] extra also drops share from its list.

Recommendation: Keep magic-wormhole as an optional [share] extra. The _send_profiling_via_wormhole function already handles the missing-wormhole case.

2. Module-level logger.remove() side effect

File: recorder.py, near line 64
Severity: Medium

# Configure loguru to use LOG_LEVEL (default stderr handler is DEBUG)
logger.remove()
logger.add(sys.stderr, level=LOG_LEVEL)

This runs at import time and affects the global loguru logger. Any other module that imports loguru before or after recorder.py is imported will lose its default handler. This is a side effect that affects the entire application — if a library consumer imports openadapt_capture.recorder, their logging config gets silently overwritten.

Recommendation: Move this into record() or into the Recorder.__init__, or at minimum make it conditional (e.g., only configure if the module is run as __main__).

3. process_events: prev_window_event can still be None at line 275

File: recorder.py, process_events() function
Severity: Medium

The diff correctly guards the first prev_window_event is None check (lines 239-243) to skip the requirement when RECORD_WINDOW_DATA=False. But look at what happens next — when RECORD_WINDOW_DATA=False, execution falls through to line 275:

if prev_window_event is not None:
    if prev_saved_window_timestamp < prev_window_event.timestamp:
        ...

The diff wraps this in if prev_window_event is not None: which is correct. This one is fine. But there's a subtler issue: when window capture is disabled and prev_window_event remains None, event.data["window_event_timestamp"] is never set (line 243). If any downstream code reads window_event_timestamp from the event data, it will KeyError. This should be documented or the field should be set to None explicitly.

4. _send_profiling_via_wormhole blocks the recording thread

File: recorder.py
Severity: Low-Medium

_send_profiling_via_wormhole calls subprocess.run([wormhole_bin, "send", ...], check=True) which blocks until the receiver connects and accepts. The wormhole send command prints a code and then waits indefinitely for a receiver. This blocks the record() function's return.

The Recorder._run_record() calls record() in a thread, so this won't block the main thread, but it means recorder.is_recording will remain in an ambiguous state and the Recorder.__exit__ / cleanup may hang.

Recommendation: Either:

• Run the wormhole send in a background thread/daemon
• Add a timeout to the subprocess call
• At minimum, document that send_profile=True will block until a receiver connects

5. capture_dir in profiling JSON is a string, not serializable Path

File: recorder.py, profiling summary section
Severity: Low

"capture_dir": capture_dir,

capture_dir is a str so this is fine for JSON serialization. No issue here — just confirming it's correct.

6. Duplicate wormhole-finding logic

File: recorder.py _send_profiling_via_wormhole vs share.py _find_wormhole
Severity: Low

The _send_profiling_via_wormhole function reimplements wormhole binary discovery (check shutil.which, then Scripts/ directory). This is already implemented in share.py:_find_wormhole(). The PR should reuse _find_wormhole() from share.py instead of duplicating.

7. _screen_timing list grows unbounded in memory

File: recorder.py, read_screen_events()
Severity: Low

The _screen_timing list appends a tuple per screenshot iteration (~10 FPS = ~600 entries/minute). For a long recording (hours), this could accumulate hundreds of thousands of entries in memory. Not a critical issue for typical use, but worth noting. Could use a fixed-size deque or periodic summarization.

8. video.py inconsistency between legacy and class-based API

File: video.py, line 313
Severity: Low (but the PR only partially fixes it)

The PR changes the legacy write_video_frame function to use av.video.frame.PictureType.I instead of "I". However, looking at the current main branch code at line 313, it still says av_frame.pict_type = "I". The diff correctly changes this. The VideoWriter class already uses PictureType.I (lines 132, 167). Good — the PR makes them consistent.

9. video_post_callback guard is good but could be cleaner

File: recorder.py, video_post_callback
Severity: Nit

The added guard handles state is None and "last_frame" not in state. This is the right fix for early Ctrl+C. Clean.

---

Positive Aspects

• The conditional window capture is well-implemented — guards both the thread creation and the event processing
• Stop sequence IndexError fix (>= + reset) is a genuine bug fix
• Matplotlib Agg backend fix is correct for non-main-thread usage
• Test coverage for video pict_type is useful
• pixel_ratio and audio_start_time properties with safe fallbacks is clean
• Sleep throttles (0.1s window, 1s memory) reduce CPU waste without affecting functionality
• Per-screenshot timing stats are useful for the stated debugging goal
• CI is green

Merge Recommendation

Not ready to merge as-is. The PR should address:

1. Must fix: Revert magic-wormhole from required to optional [share] dependency (breaking change for install size, and the code already handles its absence)
2. Must fix: Move logger.remove() out of module-level scope (breaks logging for library consumers)
3. Should fix: Reuse share.py:_find_wormhole() instead of duplicating wormhole discovery logic
4. Should document: send_profile=True blocks until wormhole receiver connects

The branch is 4 commits behind main but the diverged commits are just release/CI changes, so a rebase should be clean.

Overall the PR is 80% good — the core functionality (conditional window capture, profiling, bug fixes) is solid. The issues are mostly about dependency management and side effects.