books-in-hand/bookmark
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

fix: handle empty-platform captures, no-op fires, and observe races

Browse source 
- IntakeSession.observe_frame now catches ValueError from crop_to_book
  (empty-platform capture), cleans up any orphan wide-image file/dir,
  resets the strategy, and does not advance session state. The error
  is surfaced via a new last_error field.
- /api/observe adds an "error" field to the response when a capture
  fires but fails, instead of an unhandled 500.
- observe_frame only reports fired=True (and only runs capture logic)
  when state is WAITING_FRONT or WAITING_BACK, so a completed book
  left on the platform can no longer produce a no-op "fired" capture
  from an auto-strategy re-triggering.
- observe_frame now holds a per-session threading.Lock for its whole
  body, since the frontend polls /api/observe every ~500ms without
  chaining requests and Catalogue.update_row() does a non-atomic
  read-all-then-rewrite-whole-file rewrite of catalogue.csv.

Adds tests for empty-platform front/back captures and the
BOOK_COMPLETE no-op-fire case, plus an API-level empty-platform test.
This commit is contained in:
pyr0ball 2026-07-13 11:22:08 -07:00
parent 857489a695
commit 2b4875d77c
4 changed files with 144 additions and 13 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

5
bookmark/api/routes.py
View file

@ -21,7 +21,10 @@ def observe(request: Request):
return JSONResponse(status_code=503, content={"error": "Camera unavailable", "detail": str(exc)})
request.app.state.latest_frame = frame
fired = session.observe_frame(frame)
return {"fired": fired, "state": session.state.value, "book_id": session.current_book_id}
response = {"fired": fired, "state": session.state.value, "book_id": session.current_book_id}
if session.last_error:
response["error"] = session.last_error
return response
@router.post("/api/manual-trigger")

80
bookmark/session.py
View file

@ -1,3 +1,4 @@
import threading
from dataclasses import dataclass, field
from datetime import datetime, timezone
from enum import Enum
@ -28,14 +29,38 @@ class IntakeSession:
state: IntakeState = IntakeState.WAITING_FRONT
current_book_id: str | None = None
_size_bucket: str | None = field(default=None, repr=False)
last_error: str | None = field(default=None, repr=False)
# Guards the capture-handling critical section. /api/observe is polled
# every ~500ms from the frontend without waiting for the previous call
# to resolve, and FastAPI's sync `def` routes run concurrently in
# Starlette's threadpool. Catalogue.update_row() does a non-atomic
# read-all-then-rewrite-whole-file, so two overlapping observe_frame()
# calls could interleave and corrupt catalogue.csv (the nonprofit's
# source of truth for tax reporting). Owning the lock on the session
# keeps a single capture in flight at a time regardless of caller.
_lock: threading.Lock = field(default_factory=threading.Lock, repr=False)
def observe_frame(self, frame: np.ndarray) -> bool:
"""Feed a frame to the active capture strategy. Returns True if a capture just fired."""
fired = self.strategy.observe(frame)
if fired:
self._handle_capture(frame)
self.strategy.reset()
return fired
"""Feed a frame to the active capture strategy. Returns True if a capture just fired
and produced a state transition. Sets `last_error` (and returns False) if a capture
fired but failed (e.g. no book detected on the platform)."""
with self._lock:
self.last_error = None
fired = self.strategy.observe(frame)
if not fired:
return False
try:
capturing_state = self.state
if capturing_state not in (IntakeState.WAITING_FRONT, IntakeState.WAITING_BACK):
# Nothing to do (e.g. BOOK_COMPLETE) — not a real capture event.
return False
self._handle_capture(frame)
except ValueError as exc:
self.last_error = str(exc)
return False
finally:
self.strategy.reset()
return True
def _handle_capture(self, frame: np.ndarray) -> None:
if self.state == IntakeState.WAITING_FRONT:
@ -44,11 +69,22 @@ class IntakeSession:
self._capture_back(frame)
def _capture_front(self, frame: np.ndarray) -> None:
self.current_book_id = self.catalogue.new_book_id()
self._size_bucket = determine_size_bucket(frame, self.reference_frame, self.markers)
wide_path = self.captures_dir / self.current_book_id / "front_wide.jpg"
cropped_path = self.captures_dir / self.current_book_id / "front_cropped.jpg"
self._save_pair(frame, wide_path, cropped_path)
book_id = self.catalogue.new_book_id()
size_bucket = determine_size_bucket(frame, self.reference_frame, self.markers)
wide_path = self.captures_dir / book_id / "front_wide.jpg"
cropped_path = self.captures_dir / book_id / "front_cropped.jpg"
try:
self._save_pair(frame, wide_path, cropped_path)
except ValueError:
# crop_to_book found no book (empty platform) — nothing was
# ever catalogued, so don't leave an orphan wide-image file
# (or the book_id directory) behind, and don't consume this
# book_id or advance session state.
self._cleanup_partial_capture(wide_path, cropped_path)
raise
self.current_book_id = book_id
self._size_bucket = size_bucket
self.catalogue.append(
BookRecord(
book_id=self.current_book_id,
@ -63,7 +99,14 @@ class IntakeSession:
def _capture_back(self, frame: np.ndarray) -> None:
wide_path = self.captures_dir / self.current_book_id / "back_wide.jpg"
cropped_path = self.captures_dir / self.current_book_id / "back_cropped.jpg"
self._save_pair(frame, wide_path, cropped_path)
try:
self._save_pair(frame, wide_path, cropped_path)
except ValueError:
# No book detected on the platform for the back capture — the
# front row already exists, so leave it alone, just don't
# leave an orphan back_wide.jpg or advance past WAITING_BACK.
self._cleanup_partial_capture(wide_path, cropped_path)
raise
self.catalogue.update_row(
self.current_book_id,
back_wide_path=str(wide_path),
@ -77,6 +120,19 @@ class IntakeSession:
cropped = crop_to_book(frame, self.reference_frame)
cv2.imwrite(str(cropped_path), cropped)
def _cleanup_partial_capture(self, wide_path: Path, cropped_path: Path) -> None:
"""Remove any files written before a ValueError aborted a capture, and the
book_id directory too if it's now empty (front-capture failure case)."""
for path in (cropped_path, wide_path):
try:
path.unlink()
except FileNotFoundError:
pass
try:
wide_path.parent.rmdir()
except OSError:
pass # not empty (e.g. back-capture failure — front images remain)
def start_next_book(self) -> None:
if self.state != IntakeState.BOOK_COMPLETE:
raise RuntimeError("Cannot start next book before current book is complete")

14
tests/test_api.py
View file

@ -91,6 +91,20 @@ def test_preview_returns_jpeg_after_observe(tmp_path):
assert response.headers["content-type"] == "image/jpeg"
def test_observe_on_empty_platform_does_not_500_and_reports_error(tmp_path):
client, session, camera = make_client(tmp_path)
# camera.frame is already the reference frame (empty platform) by default.
client.post("/api/manual-trigger")
response = client.post("/api/observe")
assert response.status_code == 200
body = response.json()
assert body["fired"] is False
assert body["state"] == "waiting_front"
assert "error" in body
assert session.catalogue.read_all() == []
def test_observe_returns_503_on_camera_failure(tmp_path):
catalogue = Catalogue(tmp_path / "catalogue.csv")
strategy = ManualButtonStrategy()

58
tests/test_session.py
View file

@ -94,3 +94,61 @@ def test_start_next_book_resets_state_and_assigns_new_id(session):
def test_start_next_book_raises_if_book_not_complete(session):
with pytest.raises(RuntimeError):
session.start_next_book()
def test_empty_platform_capture_does_not_corrupt_state_or_files(session):
"""Feeding the unchanged reference frame (nothing on the platform) should not
crash, should not create a catalogue row, should not leave an orphan file on
disk, should leave the strategy un-armed, and should not advance state."""
session.strategy.trigger()
fired = session.observe_frame(session.reference_frame)
assert fired is False
assert session.last_error is not None
assert session.state == IntakeState.WAITING_FRONT
assert session.current_book_id is None
assert session.catalogue.read_all() == []
assert session.strategy.observe(make_book_frame()) is False # not left armed
assert not (session.captures_dir).exists() or not any(session.captures_dir.rglob("*.jpg"))
def test_empty_platform_capture_on_back_step_does_not_advance_or_corrupt(session):
"""Same failure mode, but triggered on the back-capture step: the front row
must survive untouched, and state must stay at WAITING_BACK."""
session.strategy.trigger()
session.observe_frame(make_book_frame())
book_id = session.current_book_id
assert session.state == IntakeState.WAITING_BACK
session.strategy.trigger()
fired = session.observe_frame(session.reference_frame)
assert fired is False
assert session.last_error is not None
assert session.state == IntakeState.WAITING_BACK
assert session.current_book_id == book_id
rows = session.catalogue.read_all()
assert len(rows) == 1
assert rows[0]["back_wide_path"] == ""
assert rows[0]["back_cropped_path"] == ""
back_files = list((session.captures_dir / book_id).glob("back_*.jpg"))
assert back_files == []
def test_book_complete_no_op_fire_does_not_return_true(session):
"""A strategy re-firing while the session is already BOOK_COMPLETE (e.g. an
auto-strategy re-satisfying stability while a finished book sits on the
platform) must not report a capture and must not stay armed."""
session.strategy.trigger()
session.observe_frame(make_book_frame())
session.strategy.trigger()
session.observe_frame(make_book_frame())
assert session.state == IntakeState.BOOK_COMPLETE
session.strategy.trigger()
fired = session.observe_frame(make_book_frame())
assert fired is False
assert session.state == IntakeState.BOOK_COMPLETE
assert session.strategy.observe(make_book_frame()) is False # reset, not left armed