fix(tracking): address independent review of the pulse experiment

- Watch for a second capture directory on every poll; stop and remove every
  directory that appeared, and fail loudly if an eye image can't be deleted.
- Start the worker pool inside the cleanup block.
- Flat patches no longer rank first (zero-power SNR is 0, not infinity).
- Align eyes to the truly nearest frame.
- Keep patch grids as float arrays (a 300 s run no longer needs ~0.4 GB).
- heart-check: tolerate unusual flags, close the Health archive, give a clear
  error when export.xml is missing, and build the lookup index once.

Found by a SWE-2 Max read-only review. Part of #27

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

docs/tracking.md

@@ -213,15 +213,17 @@ How it works:
   42 and 180 BPM are combined in the frequency domain. Output is an overall
   estimate plus one estimate per second over 15-second windows. A result
   counts as **clear** only when the top patches agree and the combined signal
-  stands out from the noise. Otherwise the command exits 3 and sends nothing.
+  stands out from the noise. Otherwise the command exits 3 and sends no OSC.
+  `--log` still records the per-second estimates, so a comparison shows how
+  far off an unclear result was.
   The thresholds are provisional until checked on real wearers.
 
 **Verified on the Frame, unworn, 2026-09-29:** a 30-second run captured
 5,362 eye frames, never had more than 7 images on disk, finished 3 s after
 the capture ended and left no capture directory. It reported no clear pulse
 (exit 3), as it should with nobody wearing it. Worth knowing: the unworn
-patches agreed on a steady rhythm near 129 BPM (2.15 Hz) with low
-signal/noise (0.19). That is a camera or illumination artifact, not a pulse,
+patches agreed on a steady rhythm near 127-129 BPM (about 2.1 Hz) with low
+signal/noise (0.15-0.19), in two separate runs. That is a camera or illumination artifact, not a pulse,
 and the signal/noise gate kept it from being reported. A worn test should
 also record an unworn baseline, to rule out the same artifact.
 

frame/tracking/pulse.py

@@ -11,6 +11,7 @@ complete, then deleted; the capture directory is removed on exit. No image
 leaves the Frame or outlives the run. This is an experiment, not a medical
 measurement.
 """
+from array import array
 import bisect
 import cmath
 import json
@@ -81,6 +82,7 @@ class Capture:
         self.pool = None
         self.submitted = set()
         self.futures = {}
+        self.new = set()  # every capture directory that appeared during our run
 
     def candidates(self):
         parent, stem = os.path.split(self.PREFIX)
@@ -93,23 +95,26 @@ class Capture:
         # found by watching for a new one rather than waiting for its name.
         import tempfile
         before = self.candidates()
+        process = None
+        with tempfile.TemporaryFile("w+") as log:
+            try:
                 if self.workers:
                     # SteamVR pins its eye tracker to cores 0-1; decoding runs beside it.
                     import concurrent.futures
                     import multiprocessing
                     self.pool = concurrent.futures.ProcessPoolExecutor(
                         self.workers, mp_context=multiprocessing.get_context("fork"))
-        with tempfile.TemporaryFile("w+") as log:
                 process = self.runner([str(ET_BIN), "-b", "CDSP", "-w", str(ET_WEIGHTS), "--calib", str(self.seconds)],
                                       cwd=str(ET_BIN.parent), stdout=log, stderr=subprocess.STDOUT)
-            try:
                 deadline = time.monotonic() + self.seconds + 30
                 while True:
-                    if not self.directory:
-                        new = self.candidates() - before
-                        if len(new) > 1:
+                    # Checked on every poll: a second capture directory means
+                    # we can't tell which images are ours, so stop.
+                    self.new |= self.candidates() - before
+                    if len(self.new) > 1:
                         raise RuntimeError("another eye-camera capture is running")
-                        self.directory = new.pop() if new else None
+                    if not self.directory and self.new:
+                        self.directory = next(iter(self.new))
                     finished = process.poll() is not None
                     self.reduce(final=finished)
                     if finished:
@@ -124,7 +129,7 @@ class Capture:
                     raise RuntimeError(f"eye-camera capture failed ({reason})")
                 return self.frames()
             finally:
-                if process.poll() is None:
+                if process and process.poll() is None:
                     process.terminate()
                     try:
                         process.wait(timeout=5)
@@ -157,10 +162,10 @@ class Capture:
                 if self.pool:
                     self.futures[(eye, index)] = self.pool.submit(reduce_file, self.loader, path)
                 else:
-                    self.grids[eye][index] = reduce_file(self.loader, path)
+                    self.grids[eye][index] = array("f", reduce_file(self.loader, path))
         for key, future in list(self.futures.items()):
             if final or future.done():
-                self.grids[key[0]][key[1]] = future.result()
+                self.grids[key[0]][key[1]] = array("f", future.result())
                 del self.futures[key]
 
     def frames(self):
@@ -177,8 +182,11 @@ class Capture:
         return frames
 
     def remove(self):
-        if self.directory and str(self.directory).startswith(self.PREFIX):
-            shutil.rmtree(self.directory, ignore_errors=True)
+        """Remove every capture directory that appeared during the run. Eye
+        images must not outlive it, so a failure to delete is an error."""
+        for directory in self.new | ({self.directory} if self.directory else set()):
+            if str(directory).startswith(self.PREFIX) and directory.is_dir() and not directory.is_symlink():
+                shutil.rmtree(directory)
 
 
 # --------------------------------------------------------------- analysis ---
@@ -283,7 +291,7 @@ def snr(spec, bpm, width=4.0):
     """Power near the pulse and its first harmonic against the rest of the band."""
     near = sum(p for f, p in spec if abs(f - bpm) <= width or abs(f - 2 * bpm) <= width)
     rest = sum(p for f, p in spec) - near
-    return near / rest if rest > 0 else float("inf")
+    return near / rest if near > 0 and rest > 0 else 0.0
 
 
 def normalised(spec):
@@ -308,7 +316,7 @@ def estimate(times, patches, rate=RATE, share=0.2, window=WINDOW):
     for name, values in patches.items():
         if usable(values):
             signal = clean(resample(times, values, rate), rate)
-            if signal is not None:
+            if signal is not None and any(signal):  # flat patches carry no rhythm
                 cleaned[name] = signal
     if not cleaned or len(next(iter(cleaned.values()))) < rate * 8:
         raise ValueError("need at least 8 seconds of usable eye-camera frames")
@@ -352,7 +360,12 @@ def analyse(frames):
         if not eye_frames:
             continue
         eye_times = [t for t, _ in eye_frames]
-        nearest = [min(bisect.bisect_left(eye_times, t), len(eye_times) - 1) for t in times]
+        nearest = []
+        for t in times:
+            i = bisect.bisect_left(eye_times, t)
+            if i == len(eye_times) or (i > 0 and t - eye_times[i - 1] <= eye_times[i] - t):
+                i -= 1
+            nearest.append(i)
         for k in range(len(eye_frames[0][1])):
             patches[f"{eye}{k}"] = [eye_frames[i][1][k] for i in nearest]
     return estimate(times, patches)

scripts/heart-check.py

@@ -118,11 +118,10 @@ def read_csv(path):
                 continue
             try:
                 when, bpm = parse_time(row[0]), int(float(row[1]))
+                flags = int(float(row[2])) if len(row) > 2 and row[2].strip() else None
             except ValueError:
                 continue  # header or unparseable line
-            if len(row) > 2 and row[2].strip():
-                flags = int(row[2])
-                if flags & 4 and not flags & 2:
+            if flags is not None and flags & 4 and not flags & 2:
                 bpm = None  # contact supported and not detected: no reading
             samples.append((when, bpm))
     return sorted(samples, key=lambda s: s[0])
@@ -130,20 +129,28 @@ def read_csv(path):
 
 def read_health(path, start, end):
     """Heart-rate records from an Apple Health export between start and end."""
-    if zipfile.is_zipfile(path):
-        archive = zipfile.ZipFile(path)
-        name = next(n for n in archive.namelist() if n.endswith("/export.xml") or n == "export.xml")
-        source = archive.open(name)
-    else:
-        source = open(path, "rb")
     samples = []
-    with source:
+    def scan(source):
         for _, element in ElementTree.iterparse(source):
             if element.tag == "Record" and element.get("type") == "HKQuantityTypeIdentifierHeartRate":
-                when = parse_time(element.get("startDate"))
+                try:
+                    when = parse_time(element.get("startDate") or "")
+                    value = round(float(element.get("value") or ""))
+                except ValueError:
+                    continue
                 if start <= when <= end:
-                    samples.append((when, round(float(element.get("value")))))
+                    samples.append((when, value))
             element.clear()
+    if zipfile.is_zipfile(path):
+        with zipfile.ZipFile(path) as archive:
+            names = [n for n in archive.namelist() if n.endswith("/export.xml") or n == "export.xml"]
+            if not names:
+                raise ValueError("no export.xml in that archive; use Health's Export All Health Data")
+            with archive.open(names[0]) as source:
+                scan(source)
+    else:
+        with open(path, "rb") as source:
+            scan(source)
     return sorted(samples)
 
 
@@ -154,9 +161,10 @@ def read_any(path, start=None, end=None):
     return read_csv(path)
 
 
-def value_at(samples, when, hold):
-    """Our reading at a moment: the latest sample no older than `hold` seconds."""
-    times = [s[0] for s in samples]
+def value_at(samples, when, hold, times=None):
+    """Our reading at a moment: the latest sample no older than `hold` seconds.
+    `times` is the samples' time column, if the caller has already built it."""
+    times = times if times is not None else [s[0] for s in samples]
     i = bisect.bisect_right(times, when) - 1
     if i < 0 or when - times[i] > hold:
         return None
@@ -173,10 +181,11 @@ def compare(ours, reference, max_lag=10.0, hold=5.0):
     if not real or not any(b is not None for _, b in ours):
         raise ValueError("both recordings need at least one reading")
     best = None
+    ours_times = [t for t, _ in ours]
     steps = int(max_lag * 4)
     for step in range(-steps, steps + 1):
         lag = step / 4
-        pairs = [(value_at(ours, t + lag, hold), b) for t, b in real]
+        pairs = [(value_at(ours, t + lag, hold, ours_times), b) for t, b in real]
         pairs = [(o, r) for o, r in pairs if o is not None]
         if not pairs:
             continue
@@ -249,8 +258,8 @@ def main(argv=None):
     ours = read_csv(args.ours)
     if not ours:
         parser.error("our recording has no readings")
-    reference = read_any(args.reference, ours[0][0] - 60, ours[-1][0] + 60)
     try:
+        reference = read_any(args.reference, ours[0][0] - 60, ours[-1][0] + 60)
         result = compare(ours, reference, args.max_lag)
     except ValueError as error:
         print(str(error))

tests/test_pulse.py

@@ -79,6 +79,24 @@ class Estimate(unittest.TestCase):
         self.assertAlmostEqual(series[0][0], pulse.WINDOW, delta=0.1)
         self.assertEqual(len(series), 20 - int(pulse.WINDOW) + 1)
 
+    def test_flat_patches_do_not_rank_first(self):
+        times, values = synthetic(72, 16)
+        values["flat"] = [100.0] * len(times)
+        result = pulse.estimate(times, values)
+        self.assertAlmostEqual(result["bpm"], 72, delta=1.5)
+        self.assertEqual(pulse.snr([(60.0, 0.0), (61.0, 0.0)], 60), 0.0)
+
+    def test_analyse_uses_nearest_frame(self):
+        times, values = synthetic(72, 4, patches=4, seconds=10)
+        # Right-eye frames 1 ms before each left frame: nearest is that frame.
+        frames = [(t, "left", [values[k][i] for k in range(4)]) for i, t in enumerate(times)]
+        frames += [(t - 0.001, "right", [float(i)] * 4) for i, t in enumerate(times)]
+        seen = {}
+        original = pulse.estimate
+        with patch.object(pulse, "estimate", lambda t, p: seen.update(p) or original(t, p)):
+            pulse.analyse(frames)
+        self.assertEqual(seen["right0"][:5], [0.0, 0.0, 1.0, 1.0, 2.0])
+
     def test_fft_matches_dft(self):
         signal = [math.sin(i) + (i % 3) for i in range(16)]
         fast = pulse.fft(signal)
@@ -213,6 +231,22 @@ class CaptureLifecycle(unittest.TestCase):
         self.assertEqual(tool.returncode, -15)       # capture tool stopped
         self.assertFalse(self.directory.exists())    # no image left behind
 
+    def test_second_capture_directory_stops_and_removes_both(self):
+        foreign = self.root / "etcalib_foreign"
+        class Racing(FakeCaptureTool):
+            def poll(self):
+                if self.polls == 2:
+                    foreign.mkdir()
+                    (foreign / "left_0.png").write_bytes(b"png")
+                return super().poll()
+        tool = Racing(self.directory)
+        with self.assertRaises(RuntimeError) as raised:
+            self.capture(tool)
+        self.assertIn("another eye-camera capture", str(raised.exception))
+        self.assertFalse(self.directory.exists())
+        self.assertFalse(foreign.exists())  # no eye image outlives the run
+        self.assertTrue((self.root / "etcalib_older").exists())
+
     def test_only_removes_capture_directories(self):
         capture = pulse.Capture(20)
         capture.directory = self.root
@@ -292,6 +326,16 @@ class HeartCheck(unittest.TestCase):
             result = check.compare(ours, samples)
             self.assertLessEqual(result["mean_abs_error"], 1)
 
+    def test_unusual_flags_and_missing_export(self):
+        path = self.write("ref.csv", "time,bpm,flags\n1000,70,6.0\n1001,71,yes\n1002,72,4\n")
+        self.assertEqual(check.read_csv(path), [(1000.0, 70), (1002.0, None)])
+        with tempfile.TemporaryDirectory() as directory:
+            archive = Path(directory) / "other.zip"
+            with zipfile.ZipFile(archive, "w") as z:
+                z.writestr("notes.txt", "x")
+            with self.assertRaises(ValueError):
+                check.read_any(archive, 0, 1)
+
     def test_time_formats(self):
         self.assertEqual(check.parse_time("1700000000.5"), 1700000000.5)
         self.assertEqual(check.parse_time("2023-11-14T22:13:20Z"), 1700000000)