Skip to content

Commit c040211

Browse files
abrichrclaude
andcommitted
fix: address PR #12 review feedback (5 issues)
- Move magic-wormhole back to optional [share] extra (was accidentally made a required dependency; recorder.py already handles ImportError) - Remove module-level logger.remove() that destroyed global loguru config for all library consumers; configure inside record() instead - Replace duplicate wormhole-finding logic with _find_wormhole() from share.py to eliminate code duplication - Add 60s timeout to _send_profiling_via_wormhole to prevent blocking indefinitely waiting for a receiver - Replace unbounded _screen_timing list with _ScreenTimingStats class that computes running stats (count/sum/min/max) in constant memory Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 15deac5 commit c040211

File tree

2 files changed

+64
-33
lines changed

2 files changed

+64
-33
lines changed

openadapt_capture/recorder.py

Lines changed: 58 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -57,31 +57,30 @@ def set_browser_mode(
5757
websocket.send(message)
5858

5959

60-
def _send_profiling_via_wormhole(profile_path: str) -> None:
61-
"""Auto-send profiling JSON via Magic Wormhole after recording."""
62-
import shutil
60+
def _send_profiling_via_wormhole(profile_path: str, timeout: int = 60) -> None:
61+
"""Auto-send profiling JSON via Magic Wormhole after recording.
62+
63+
Args:
64+
profile_path: Path to the profiling JSON file.
65+
timeout: Maximum seconds to wait for a receiver (default: 60).
66+
"""
6367
import subprocess as _sp
6468

65-
wormhole_bin = shutil.which("wormhole")
66-
if not wormhole_bin:
67-
# Check Python Scripts dir (Windows)
68-
from pathlib import Path
69+
from openadapt_capture.share import _find_wormhole
6970

70-
scripts_dir = Path(sys.executable).parent / "Scripts"
71-
for candidate in [scripts_dir / "wormhole.exe", scripts_dir / "wormhole"]:
72-
if candidate.exists():
73-
wormhole_bin = str(candidate)
74-
break
71+
wormhole_bin = _find_wormhole()
7572
if not wormhole_bin:
7673
print("wormhole not found. To enable auto-send:")
77-
print(" pip install magic-wormhole")
74+
print(" pip install 'openadapt-capture[share]'")
7875
print(f"Profiling saved to: {profile_path}")
7976
return
8077

81-
print("Sending profiling via wormhole (waiting for receiver)...")
78+
print(f"Sending profiling via wormhole (waiting up to {timeout}s for receiver)...")
8279
print("Give the wormhole code below to the receiver.\n")
8380
try:
84-
_sp.run([wormhole_bin, "send", profile_path], check=True)
81+
_sp.run([wormhole_bin, "send", profile_path], check=True, timeout=timeout)
82+
except _sp.TimeoutExpired:
83+
logger.warning(f"Wormhole send timed out after {timeout}s. File at: {profile_path}")
8584
except _sp.CalledProcessError:
8685
print(f"Wormhole send failed. File at: {profile_path}")
8786
except KeyboardInterrupt:
@@ -93,9 +92,43 @@ def _send_profiling_via_wormhole(profile_path: str) -> None:
9392
EVENT_TYPES = ("screen", "action", "window", "browser")
9493
LOG_LEVEL = "INFO"
9594

96-
# Configure loguru to use LOG_LEVEL (default stderr handler is DEBUG)
97-
logger.remove()
98-
logger.add(sys.stderr, level=LOG_LEVEL)
95+
96+
class _ScreenTimingStats:
97+
"""Accumulate screen timing stats without storing every data point."""
98+
99+
def __init__(self):
100+
self.count = 0
101+
self.ss_sum = 0.0
102+
self.ss_max = 0.0
103+
self.ss_min = float("inf")
104+
self.total_sum = 0.0
105+
self.total_max = 0.0
106+
107+
def append(self, pair):
108+
ss_dur, total_dur = pair
109+
self.count += 1
110+
self.ss_sum += ss_dur
111+
self.ss_max = max(self.ss_max, ss_dur)
112+
self.ss_min = min(self.ss_min, ss_dur)
113+
self.total_sum += total_dur
114+
self.total_max = max(self.total_max, total_dur)
115+
116+
def to_dict(self):
117+
if self.count == 0:
118+
return {}
119+
return {
120+
"iterations": self.count,
121+
"screenshot_avg_ms": round(self.ss_sum / self.count * 1000, 1),
122+
"screenshot_max_ms": round(self.ss_max * 1000, 1),
123+
"screenshot_min_ms": round(self.ss_min * 1000, 1),
124+
"total_avg_ms": round(self.total_sum / self.count * 1000, 1),
125+
"total_max_ms": round(self.total_max * 1000, 1),
126+
}
127+
128+
def __bool__(self):
129+
return self.count > 0
130+
131+
99132
# whether to write events of each type in a separate process
100133
PROC_WRITE_BY_EVENT_TYPE = {
101134
"screen": True,
@@ -762,7 +795,7 @@ def read_screen_events(
762795
terminate_processing: multiprocessing.Event,
763796
recording: Recording,
764797
started_event: threading.Event,
765-
_screen_timing: list | None = None,
798+
_screen_timing: _ScreenTimingStats | None = None,
766799
) -> None:
767800
"""Read screen events and add them to the event queue.
768801
@@ -774,7 +807,7 @@ def read_screen_events(
774807
terminate_processing: An event to signal the termination of the process.
775808
recording: The recording object.
776809
started_event: Event to set once started.
777-
_screen_timing: If provided, append (screenshot_dur, total_dur) per iteration.
810+
_screen_timing: If provided, record (screenshot_dur, total_dur) per iteration.
778811
"""
779812
utils.set_start_time(recording.timestamp)
780813

@@ -1389,6 +1422,9 @@ def record(
13891422
config.RECORD_IMAGES,
13901423
)
13911424

1425+
# Configure loguru level for recording (without destroying global config)
1426+
logger.configure(handlers=[{"sink": sys.stderr, "level": LOG_LEVEL}])
1427+
13921428
# logically it makes sense to communicate from here, but when running
13931429
# from the tray it takes too long
13941430
# TODO: fix this
@@ -1417,7 +1453,7 @@ def record(
14171453
terminate_processing = multiprocessing.Event()
14181454
task_by_name = {}
14191455
task_started_events = {}
1420-
_screen_timing = [] # per-iteration (screenshot_dur, total_dur) for profiling
1456+
_screen_timing = _ScreenTimingStats() # running stats, no unbounded list
14211457

14221458
if config.RECORD_WINDOW_DATA:
14231459
window_event_reader = threading.Thread(
@@ -1783,16 +1819,7 @@ def join_tasks(task_names: list[str]) -> None:
17831819
}
17841820
# Compute screen timing stats
17851821
if _screen_timing:
1786-
ss_durs = [t[0] for t in _screen_timing]
1787-
total_durs = [t[1] for t in _screen_timing]
1788-
_profile_data["screen_timing"] = {
1789-
"iterations": len(_screen_timing),
1790-
"screenshot_avg_ms": round(sum(ss_durs) / len(ss_durs) * 1000, 1),
1791-
"screenshot_max_ms": round(max(ss_durs) * 1000, 1),
1792-
"screenshot_min_ms": round(min(ss_durs) * 1000, 1),
1793-
"total_avg_ms": round(sum(total_durs) / len(total_durs) * 1000, 1),
1794-
"total_max_ms": round(max(total_durs) * 1000, 1),
1795-
}
1822+
_profile_data["screen_timing"] = _screen_timing.to_dict()
17961823

17971824
_profile_path = os.path.join(capture_dir, "profiling.json")
17981825
try:

pyproject.toml

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,6 @@ dependencies = [
4141
"pympler>=1.0.0",
4242
"tqdm>=4.0.0",
4343
"numpy>=1.20.0",
44-
"magic-wormhole>=0.17.0",
4544
]
4645

4746
[project.optional-dependencies]
@@ -60,9 +59,14 @@ privacy = [
6059
"openadapt-privacy>=0.1.0",
6160
]
6261

62+
# Sharing via Magic Wormhole
63+
share = [
64+
"magic-wormhole>=0.17.0",
65+
]
66+
6367
# Everything
6468
all = [
65-
"openadapt-capture[transcribe-fast,transcribe,privacy]",
69+
"openadapt-capture[transcribe-fast,transcribe,privacy,share]",
6670
]
6771

6872
dev = [

0 commit comments

Comments
 (0)