From 2c472c6dc9b5f834c323f2397d6071c77afa25f3 Mon Sep 17 00:00:00 2001 From: abrichr Date: Thu, 27 Aug 2026 23:57:22 -0400 Subject: [PATCH] fix(scanner): read recording.db, the format the recorder writes `catalog scan` globbed `capture.db` and queried a `capture` table and an `events` table. openadapt-capture PR #28 replaced that bespoke format on 2026-07-17 with a SQLAlchemy database named `recording.db`. Nothing a user records today matched the glob, so the scanner found zero recordings on a machine full of them, and the catalog could not be populated at all. The rename alone would not have fixed it. The two formats share no table and no column: capture.started_at -> recording.timestamp capture.ended_at -> (gone; derive from the newest event) capture.screen_width -> recording.monitor_width capture.screen_height -> recording.monitor_height events -> action_event screenshots/*.png -> screenshot rows holding png_data blobs Pointed straight at a current recording, the old reader logged "no such table: capture" and registered an entry with the directory mtime as its date and None for duration, frames and events. So this reads the current schema: * `recording` for the start timestamp, task description, platform and display geometry, matching what `CaptureSession.load` selects; * `action_event` rows for the event count, which is what `events` counted; * `screenshot` rows for the frame count, since frames are blobs now, falling back to screenshots/*.png only when the database cannot answer; * the newest timestamp across the event tables for the duration, because there is no ended_at column. Legacy `capture.db` is not read. openadapt-capture's own `scripts/migrate_legacy_capture.py` states that current code cannot load it, and the screenshots workflow already treats it as unusable. A legacy directory now prints the conversion command instead of registering a shell with no date and no counts, and `_extract_recording_info` raises rather than returning one. Also fixes `catalog register`, which raised TypeError for every directory that got past the path check: `Recording` names the field `id` and `register_recording` takes `recording_id`, and TypeError is not in the except clause there. The tests read the two recordings openadapt-capture commits under examples/captures, which CI now checks out. Building a recording.db here to match this repository's idea of the schema would only prove the reader agrees with the test author, which is how the drift went unnoticed for six weeks. Co-Authored-By: Claude Opus 5 --- .github/workflows/test.yml | 34 +++ CATALOG_IMPLEMENTATION.md | 11 +- CATALOG_SYSTEM.md | 7 +- docs/SCREENSHOT_SYSTEM.md | 2 +- docs/SETUP.md | 8 +- src/openadapt_viewer/cli.py | 13 +- src/openadapt_viewer/scanner.py | 190 ++++++++++++---- tests/test_scanner_recording_db.py | 342 ++++++++++++++++++++++++++++ tests/test_screenshot_generation.py | 21 +- 9 files changed, 563 insertions(+), 65 deletions(-) create mode 100644 tests/test_scanner_recording_db.py diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 29c7c89..c5be2fa 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -17,8 +17,33 @@ jobs: python-version: ['3.10', '3.11'] steps: + # Both repositories go in named subdirectories rather than the workspace + # root. `ruff check .` below lints the whole working directory, so a + # second checkout beside this one would put openadapt-capture's source + # through this repository's lint configuration. - name: Checkout code uses: actions/checkout@v4 + with: + path: openadapt-viewer + + # The scanner reads what the recorder writes, so its tests read + # recordings the recorder actually wrote. openadapt-capture commits two + # under examples/captures, regenerated and byte-compared by its own CI. + # Reading those is the point: a recording.db written here to match this + # repository's idea of the schema would prove only that the reader agrees + # with the test author. That is how the format drifted unnoticed for six + # weeks -- the scanner still globbed the pre-2026-07-17 capture.db and + # every test that exercised it built its own capture.db to match. + # + # Only examples/captures is fetched. The rest of that repository is not + # read here and pulling it would slow every matrix leg. + - name: Checkout openadapt-capture example recordings + uses: actions/checkout@v4 + with: + repository: OpenAdaptAI/openadapt-capture + path: openadapt-capture + sparse-checkout: examples/captures + sparse-checkout-cone-mode: false - name: Install uv uses: astral-sh/setup-uv@v4 @@ -29,6 +54,7 @@ jobs: run: uv python install ${{ matrix.python-version }} - name: Install dependencies + working-directory: openadapt-viewer run: uv sync --all-extras # This step used to read: @@ -44,6 +70,7 @@ jobs: # the ruff version is bounded to >=0.16,<0.17 in pyproject.toml so a new # ruff release cannot turn this red without a person raising the ceiling. - name: Run ruff linter (check) + working-directory: openadapt-viewer run: uv run ruff check . # The companion `ruff format --check src/openadapt_viewer/` step was @@ -57,4 +84,11 @@ jobs: # adds the step back. - name: Run pytest + working-directory: openadapt-viewer + env: + # tests/test_scanner_recording_db.py reads the two recordings here and + # fails, rather than skips, if they are absent under GITHUB_ACTIONS. + OPENADAPT_CAPTURE_EXAMPLES: ${{ github.workspace }}/openadapt-capture/examples/captures + # tests/test_screenshot_generation.py takes the checkout root. + OPENADAPT_CAPTURE_DIR: ${{ github.workspace }}/openadapt-capture run: uv run pytest tests/ -v diff --git a/CATALOG_IMPLEMENTATION.md b/CATALOG_IMPLEMENTATION.md index 5ea7846..5275f25 100644 --- a/CATALOG_IMPLEMENTATION.md +++ b/CATALOG_IMPLEMENTATION.md @@ -16,7 +16,7 @@ An automated recording catalog system that makes all captured data automatically - Singleton pattern via `get_catalog()` 2. **Scanner** (`scanner.py`) - - Automatic discovery of recordings (directories with `capture.db`) + - Automatic discovery of recordings (directories with `recording.db`) - Indexing of segmentation results (`*_episodes.json` files) - Metadata extraction (frames, events, duration, timestamps) - Default path detection @@ -72,9 +72,10 @@ openadapt-viewer/ ### Key Algorithms **Recording Discovery**: -1. Glob for `**/capture.db` files -2. Extract metadata from capture.db SQLite tables -3. Count screenshots in `screenshots/` directory +1. Glob for `**/recording.db` files +2. Read the `recording` row for the start timestamp, task description, + platform and display geometry +3. Count `action_event` rows for events and `screenshot` rows for frames 4. Register in catalog with `INSERT OR REPLACE` **Segmentation Indexing**: @@ -136,7 +137,7 @@ Generated: /path/to/viewer.html ### Integration Points Verified -✅ Scanner reads from openadapt-capture `capture.db` files +✅ Scanner reads from openadapt-capture `recording.db` files ✅ Scanner parses openadapt-ml segmentation JSON files ✅ Catalog API exports data as JavaScript ✅ Viewer generator injects dropdown into base HTML diff --git a/CATALOG_SYSTEM.md b/CATALOG_SYSTEM.md index e18966b..8287996 100644 --- a/CATALOG_SYSTEM.md +++ b/CATALOG_SYSTEM.md @@ -22,11 +22,10 @@ The **Recording Catalog System** provides automatic discovery and indexing of al │ │ │ openadapt-capture/ │ │ ├── turn-off-nightshift/ │ -│ │ ├── capture.db ←────┐ │ -│ │ ├── screenshots/ │ │ +│ │ ├── recording.db ←────┐ │ │ │ └── video.mp4 │ │ │ └── demo_new/ │ │ -│ └── capture.db │ Scanner discovers │ +│ └── recording.db │ Scanner discovers │ │ │ recordings │ │ openadapt-ml/ │ │ │ └── segmentation_output/ │ │ @@ -558,7 +557,7 @@ openadapt-viewer catalog stats # Re-scan directories openadapt-viewer catalog scan -# Check recording structure (must have capture.db) +# Check recording structure (must have recording.db) ls -la /path/to/recording/ ``` diff --git a/docs/SCREENSHOT_SYSTEM.md b/docs/SCREENSHOT_SYSTEM.md index 3cb3441..9ab58ea 100644 --- a/docs/SCREENSHOT_SYSTEM.md +++ b/docs/SCREENSHOT_SYSTEM.md @@ -247,7 +247,7 @@ Install with: cd ../openadapt-capture && uv pip install -e . ### Capture Errors ``` Error: Capture not found: /path/to/capture -FileNotFoundError: [Errno 2] No such file or directory: '/path/to/capture/capture.db' +FileNotFoundError: [Errno 2] No such file or directory: '/path/to/capture/recording.db' ``` ### HTML Generation Errors diff --git a/docs/SETUP.md b/docs/SETUP.md index e735702..6c0b093 100644 --- a/docs/SETUP.md +++ b/docs/SETUP.md @@ -161,11 +161,11 @@ uv run playwright install chromium **Solution**: Verify capture paths ```bash # Check captures exist -ls -la /path/to/openadapt-capture/turn-off-nightshift/ -ls -la /path/to/openadapt-capture/demo_new/ +ls -la /path/to/openadapt-capture/examples/captures/turn-off-nightshift/ +ls -la /path/to/openadapt-capture/examples/captures/demo_new/ -# Look for capture.db -ls -la /path/to/openadapt-capture/turn-off-nightshift/capture.db +# Look for recording.db +ls -la /path/to/openadapt-capture/examples/captures/turn-off-nightshift/recording.db ``` **Or use custom path**: diff --git a/src/openadapt_viewer/cli.py b/src/openadapt_viewer/cli.py index 646cf62..cb92212 100644 --- a/src/openadapt_viewer/cli.py +++ b/src/openadapt_viewer/cli.py @@ -538,14 +538,21 @@ def run_catalog_command(args): if args.name: recording.name = args.name - catalog.register_recording(**recording.model_dump()) + # Recording names the field `id`; register_recording takes it as + # `recording_id`. Splatting the model straight in raised TypeError, + # which this except clause does not catch, so `catalog register` + # ended in a traceback for every directory that got this far. + fields = recording.model_dump() + fields["recording_id"] = fields.pop("id") + catalog.register_recording(**fields) print(f"Successfully registered: {recording.name}") print(f" ID: {recording.id}") print(f" Frames: {recording.frame_count}") print(f" Events: {recording.event_count}") except (OSError, sqlite3.Error, ValueError) as e: - # OSError: unreadable recording directory. sqlite3.Error: unreadable - # capture.db or a failed catalog write. ValueError: a row that does + # OSError: an unreadable recording directory, or one holding no + # recording.db. sqlite3.Error: a corrupt recording.db or a failed + # catalog write. ValueError: a row that does # not validate as a Recording. Anything else is a bug in this # package and should surface as a traceback rather than as a # one-line "Error registering recording". diff --git a/src/openadapt_viewer/scanner.py b/src/openadapt_viewer/scanner.py index a333474..3fa2e5f 100644 --- a/src/openadapt_viewer/scanner.py +++ b/src/openadapt_viewer/scanner.py @@ -2,9 +2,30 @@ Automatic discovery and scanning of OpenAdapt recordings and results. This module scans directories to find: -- Recordings from openadapt-capture (directories with capture.db) +- Recordings from openadapt-capture (directories with recording.db) - Segmentation results from openadapt-ml (JSON files with episodes) - Episode data for indexing + +The recorder writes one SQLAlchemy database per capture, at +``/recording.db``. Its tables are ``recording``, ``action_event``, +``screenshot``, ``window_event``, ``browser_event``, ``audio_info``, +``performance_stat`` and ``memory_stat``. The columns this module reads are +listed beside each query below. + +Two notes on what this module deliberately does not do. + +It reads the database with ``sqlite3`` rather than through +``openadapt_capture.CaptureSession.load``. Indexing needs two row counts and a +handful of scalars, and installing ``openadapt-capture`` to get them would +pull mss, sounddevice, soundfile, matplotlib, sqlalchemy, alembic, numpy and +the platform accessibility stack into an HTML generator. It is therefore not +a dependency of this package. Anything that needs decoded frames or replayable +actions should import ``CaptureSession`` rather than extend this module. + +It does not read the pre-2026-07-17 ``capture.db``. That format held a single +``capture`` row and a generic ``events`` table; openadapt-capture PR #28 +replaced it, and current code cannot load it at all. A legacy directory is +reported with the command that converts it rather than skipped in silence. """ import json @@ -14,6 +35,22 @@ from .catalog import Recording, RecordingCatalog, SegmentationResult +#: Filename the current recorder writes, one per capture directory. +RECORDING_DB_NAME = "recording.db" + +#: Filename the pre-2026-07-17 recorder wrote. Detected, never read. +LEGACY_DB_NAME = "capture.db" + +#: Printed when a directory holds only a legacy capture. +LEGACY_HINT = ( + "holds the legacy {legacy} format, which the current viewer cannot read. " + "Convert it with openadapt-capture: " + "python scripts/migrate_legacy_capture.py " +) + +#: Event tables whose timestamps bound the recording's duration. +_TIMESTAMPED_TABLES = ("screenshot", "action_event", "window_event", "browser_event") + class RecordingScanner: """Scanner for discovering and indexing OpenAdapt data.""" @@ -33,7 +70,12 @@ def scan_recording_directory( recursive: bool = False ) -> list[Recording]: """ - Scan a directory for recordings (directories containing capture.db). + Scan a directory for recordings (directories containing recording.db). + + A directory holding only the legacy ``capture.db`` is named in a + warning and skipped. Reporting it is the point: the two formats share + no table, so registering one would produce a catalog entry with no + date, no duration and no counts. Args: base_path: Path to scan for recordings @@ -48,14 +90,16 @@ def scan_recording_directory( if not base_path.exists(): raise FileNotFoundError(f"Directory not found: {base_path}") - # Find all directories with capture.db - if recursive: - pattern = "**/capture.db" - else: - pattern = "*/capture.db" + prefix = "**/" if recursive else "*/" - for capture_db in base_path.glob(pattern): - recording_dir = capture_db.parent + for legacy_db in base_path.glob(f"{prefix}{LEGACY_DB_NAME}"): + if (legacy_db.parent / RECORDING_DB_NAME).exists(): + # Already converted in place; the current database wins. + continue + print(f"Warning: {legacy_db.parent} " + LEGACY_HINT.format(legacy=LEGACY_DB_NAME)) + + for recording_db in base_path.glob(f"{prefix}{RECORDING_DB_NAME}"): + recording_dir = recording_db.parent recording_id = recording_dir.name try: @@ -76,7 +120,7 @@ def scan_recording_directory( recordings.append(registered) except (OSError, sqlite3.Error, ValueError) as e: # Narrow on purpose: an unreadable directory, a corrupt - # capture.db or a row that fails Recording validation should + # recording.db or a row that fails Recording validation should # skip that recording, not the whole scan. Anything else is a # bug in this module and must surface. print(f"Warning: Failed to index {recording_dir}: {e}") @@ -98,53 +142,88 @@ def _extract_recording_info( Returns: Recording object with extracted metadata + + Raises: + FileNotFoundError: If the directory holds no recording.db. A legacy + capture.db is named as such, with the conversion command. """ - capture_db = recording_dir / "capture.db" + recording_dir = Path(recording_dir) + recording_db = recording_dir / RECORDING_DB_NAME screenshots_dir = recording_dir / "screenshots" - # Extract from capture.db + if not recording_db.is_file(): + # Returning a hollow Recording here is what made a legacy directory + # look indexable: no date, no duration, no counts, no complaint. + if (recording_dir / LEGACY_DB_NAME).is_file(): + raise FileNotFoundError( + f"{recording_dir} " + LEGACY_HINT.format(legacy=LEGACY_DB_NAME) + ) + raise FileNotFoundError(f"No {RECORDING_DB_NAME} in {recording_dir}") + metadata = {} created_at = None duration_seconds = None task_description = None + event_count = None + frame_count = None try: - with sqlite3.connect(str(capture_db)) as conn: + with sqlite3.connect(str(recording_db)) as conn: conn.row_factory = sqlite3.Row - cursor = conn.execute("SELECT * FROM capture LIMIT 1") - row = cursor.fetchone() - - if row: - created_at = row["started_at"] - if row["ended_at"]: - duration_seconds = row["ended_at"] - row["started_at"] - task_description = row["task_description"] - - # Store additional metadata - metadata.update({ - "platform": row["platform"], - "screen_width": row["screen_width"], - "screen_height": row["screen_height"], - "pixel_ratio": row["pixel_ratio"], - }) - - # Count events - cursor = conn.execute("SELECT COUNT(*) FROM events") - event_count = cursor.fetchone()[0] + + # One row per capture directory, matching CaptureSession.load's + # `session.query(Recording).first()`. Columns: timestamp (epoch + # seconds at record start), monitor_width, monitor_height, + # pixel_ratio, platform, task_description. + row = conn.execute( + "SELECT * FROM recording ORDER BY id LIMIT 1" + ).fetchone() + + if row is None: + raise sqlite3.DatabaseError("recording table is empty") + + created_at = row["timestamp"] + task_description = row["task_description"] + metadata.update({ + "platform": row["platform"], + # Catalog vocabulary, not the recorder's: these keys are + # what the catalog has always stored. The recorder calls + # them monitor_width and monitor_height. + "screen_width": row["monitor_width"], + "screen_height": row["monitor_height"], + "pixel_ratio": row["pixel_ratio"], + }) + + # An action_event row is one mouse or key event, which is what + # the legacy `events` table counted. + event_count = conn.execute( + "SELECT COUNT(*) FROM action_event" + ).fetchone()[0] + + # Frames are rows now, not files: the recorder stores each PNG + # as a blob in `screenshot` rather than under screenshots/. + frame_count = conn.execute( + "SELECT COUNT(*) FROM screenshot" + ).fetchone()[0] + + # There is no ended_at column. The recording ends at its last + # observation, so take the newest timestamp across the event + # tables that carry one. + if created_at is not None: + duration_seconds = self._duration_from_events(conn, created_at) except (sqlite3.Error, IndexError, TypeError) as e: - # sqlite3.Error covers a missing/corrupt db and a missing table; + # sqlite3.Error covers a corrupt database and a missing table; # IndexError is sqlite3.Row's "no such column"; TypeError is a NULL # in an arithmetic column. Each means "no event metadata", not - # "abort". - print(f"Warning: Could not read capture.db: {e}") - event_count = None + # "abort", so the directory is still indexed by name and mtime. + print(f"Warning: Could not read {recording_db}: {e}") - # Count screenshots - frame_count = None - if screenshots_dir.exists(): + # Some capture directories still carry PNGs under screenshots/. Read + # them only when the database could not answer, so a readable database + # reporting zero retained frames is reported as zero, not overwritten. + if frame_count is None and screenshots_dir.is_dir(): frame_count = len(list(screenshots_dir.glob("*.png"))) - # Use directory modification time if no capture date found if created_at is None: created_at = recording_dir.stat().st_mtime @@ -160,6 +239,35 @@ def _extract_recording_info( metadata=metadata, ) + @staticmethod + def _duration_from_events(conn: sqlite3.Connection, started_at: float) -> float | None: + """ + Return seconds from the recording start to its newest observation. + + Args: + conn: Open connection to a recording.db + started_at: The recording row's `timestamp`, in epoch seconds + + Returns: + Duration in seconds, or None if the database holds no timestamped + event at or after the start. + """ + latest = None + + for table in _TIMESTAMPED_TABLES: + try: + value = conn.execute(f"SELECT MAX(timestamp) FROM {table}").fetchone()[0] + except sqlite3.Error: + # An optional table this build does not have. Other tables can + # still bound the duration. + continue + if value is not None and (latest is None or value > latest): + latest = value + + if latest is None or latest < started_at: + return None + return float(latest) - float(started_at) + def scan_segmentation_results( self, segmentation_dir: str diff --git a/tests/test_scanner_recording_db.py b/tests/test_scanner_recording_db.py new file mode 100644 index 0000000..dded0b2 --- /dev/null +++ b/tests/test_scanner_recording_db.py @@ -0,0 +1,342 @@ +"""The scanner must find and read what the current recorder writes. + +The scanner globbed ``capture.db`` and queried a ``capture`` table and an +``events`` table. openadapt-capture PR #28 replaced that bespoke format on +2026-07-17 with a SQLAlchemy database named ``recording.db`` whose tables are +``recording``, ``action_event`` and ``screenshot``. Nothing a user records +today matched the glob, so ``openadapt-viewer catalog scan`` found zero +recordings on a machine full of them. + +Renaming the glob alone would not have fixed it. The two formats share no +table and no column, so a path-only change registers a directory with no date, +no duration and no counts. These tests therefore assert the values, not the +count of rows returned. + +They run against the recordings openadapt-capture commits under +``examples/captures/``, not against a database written here to match this +module's own assumptions. A test that builds its own input can only prove the +reader agrees with the test author. +""" + +from __future__ import annotations + +import os +import sqlite3 +import sys +from pathlib import Path + +import pytest + +from openadapt_viewer.catalog import RecordingCatalog +from openadapt_viewer.cli import main +from openadapt_viewer.scanner import RecordingScanner + +REPO_ROOT = Path(__file__).parent.parent + +#: Set by CI to the checked-out openadapt-capture's examples/captures. +EXAMPLES_ENV = "OPENADAPT_CAPTURE_EXAMPLES" + +#: The recordings openadapt-capture commits, with what its specs declare. +REAL_RECORDINGS = ("demo_new", "turn-off-nightshift") + + +def _examples_dir() -> Path | None: + """Return the directory holding the committed example recordings, or None. + + Honours ``$OPENADAPT_CAPTURE_EXAMPLES`` first, then looks for an + openadapt-capture checkout beside this one. No absolute path is written + here: this repository is public and one developer's home directory is not + a location any other user has. + """ + named = os.environ.get(EXAMPLES_ENV) + if named: + return Path(named).expanduser() + + sibling = REPO_ROOT.parent / "openadapt-capture" / "examples" / "captures" + return sibling if sibling.is_dir() else None + + +def _require_examples() -> Path: + """Return the examples directory, skipping or failing with a reason.""" + directory = _examples_dir() + if directory is None: + pytest.skip( + "No openadapt-capture checkout found. Clone it beside this " + f"repository, or set ${EXAMPLES_ENV} to its examples/captures." + ) + if not directory.is_dir(): + # The variable was set on purpose, so an absent directory is an error + # in the caller's setup, not a reason to quietly pass. + pytest.fail(f"${EXAMPLES_ENV} names {directory}, which is not a directory") + missing = [ + name for name in REAL_RECORDINGS if not (directory / name / "recording.db").is_file() + ] + if missing: + pytest.fail(f"{directory} is missing recording.db for: {', '.join(missing)}") + return directory + + +@pytest.fixture +def scanner(tmp_path): + return RecordingScanner(RecordingCatalog(db_path=str(tmp_path / "catalog.db"))) + + +def _legacy_capture(directory: Path) -> Path: + """Write a pre-#28 capture directory, in the format this viewer refuses. + + The shape is the one openadapt-capture's ``migrate_legacy_capture.py`` + reads: one ``capture`` row and a generic ``events`` table. + """ + directory.mkdir(parents=True, exist_ok=True) + with sqlite3.connect(directory / "capture.db") as conn: + conn.execute( + "CREATE TABLE capture (id INTEGER PRIMARY KEY, started_at REAL, " + "ended_at REAL, platform TEXT, screen_width INTEGER, " + "screen_height INTEGER, pixel_ratio REAL, task_description TEXT)" + ) + conn.execute( + "INSERT INTO capture VALUES (1, 1000.0, 1012.0, 'darwin', 1920, 1080, 2.0, 'old')" + ) + conn.execute( + "CREATE TABLE events (id INTEGER PRIMARY KEY, timestamp REAL, " + "type TEXT, data TEXT, parent_id INTEGER)" + ) + conn.execute("INSERT INTO events VALUES (1, 1000.5, 'mouse.move', '{}', NULL)") + return directory + + +class TestCommittedRecordingsAreFound: + """The defect, stated as the user sees it: scan finds nothing.""" + + def test_scan_registers_every_committed_recording(self, scanner): + examples = _require_examples() + + found = scanner.scan_recording_directory(str(examples)) + + assert sorted(recording.id for recording in found) == sorted(REAL_RECORDINGS) + + def test_scanned_recordings_reach_the_catalog(self, scanner): + examples = _require_examples() + + scanner.scan_recording_directory(str(examples)) + + listed = scanner.catalog.get_all_recordings() + assert sorted(recording.id for recording in listed) == sorted(REAL_RECORDINGS) + + def test_recursive_scan_finds_a_nested_recording(self, scanner, tmp_path): + examples = _require_examples() + nested = tmp_path / "runs" / "monday" / "demo_new" + nested.mkdir(parents=True) + (nested / "recording.db").write_bytes( + (examples / "demo_new" / "recording.db").read_bytes() + ) + + assert scanner.scan_recording_directory(str(tmp_path)) == [] + + found = scanner.scan_recording_directory(str(tmp_path), recursive=True) + assert [recording.id for recording in found] == ["demo_new"] + + +@pytest.mark.parametrize("name", REAL_RECORDINGS) +class TestCommittedRecordingsAreRead: + """A found recording must carry the file's values, not fallbacks. + + Every expectation below is queried from the same recording.db the scanner + read, with the query written out here rather than shared with the scanner. + The subject is whether the catalog reports what the file says. + """ + + def test_created_at_is_the_recording_timestamp(self, scanner, name): + directory = _require_examples() / name + expected = _scalar(directory, "SELECT timestamp FROM recording ORDER BY id LIMIT 1") + + recording = scanner._extract_recording_info(directory, name) + + assert recording.created_at == pytest.approx(expected) + # The old reader fell back to the directory's mtime whenever the query + # failed, which is a plausible-looking date that means nothing. + assert recording.created_at != directory.stat().st_mtime + + def test_event_count_is_the_action_event_rows(self, scanner, name): + directory = _require_examples() / name + expected = _scalar(directory, "SELECT COUNT(*) FROM action_event") + + recording = scanner._extract_recording_info(directory, name) + + assert expected > 0 + assert recording.event_count == expected + + def test_frame_count_is_the_screenshot_rows(self, scanner, name): + directory = _require_examples() / name + expected = _scalar(directory, "SELECT COUNT(*) FROM screenshot") + + recording = scanner._extract_recording_info(directory, name) + + # Frames are blobs in the database now. The old reader counted PNG + # files under screenshots/, a directory these recordings do not have, + # and so reported None. + assert expected > 0 + assert not (directory / "screenshots").exists() + assert recording.frame_count == expected + + def test_duration_spans_start_to_last_observation(self, scanner, name): + directory = _require_examples() / name + start = _scalar(directory, "SELECT timestamp FROM recording ORDER BY id LIMIT 1") + last = max( + _scalar(directory, "SELECT MAX(timestamp) FROM screenshot"), + _scalar(directory, "SELECT MAX(timestamp) FROM action_event"), + ) + + recording = scanner._extract_recording_info(directory, name) + + # There is no ended_at column, so the duration is derived. + assert recording.duration_seconds == pytest.approx(last - start) + assert recording.duration_seconds > 0 + + def test_task_description_and_display_metadata_are_read(self, scanner, name): + directory = _require_examples() / name + row = _row(directory, "SELECT * FROM recording ORDER BY id LIMIT 1") + + recording = scanner._extract_recording_info(directory, name) + + assert recording.task_description == row["task_description"] + assert recording.task_description + assert recording.metadata == { + "platform": row["platform"], + "screen_width": row["monitor_width"], + "screen_height": row["monitor_height"], + "pixel_ratio": row["pixel_ratio"], + } + assert recording.metadata["screen_width"] > 0 + + +class TestLegacyCapturesAreReportedNotIndexed: + """A capture.db cannot be read, so say so instead of indexing a shell.""" + + def test_scan_skips_a_legacy_directory_and_names_the_migration( + self, scanner, tmp_path, capsys + ): + _legacy_capture(tmp_path / "old-recording") + + found = scanner.scan_recording_directory(str(tmp_path)) + + assert found == [] + out = capsys.readouterr().out + assert "old-recording" in out + assert "capture.db" in out + assert "migrate_legacy_capture.py" in out + + def test_extract_refuses_a_legacy_directory(self, scanner, tmp_path): + directory = _legacy_capture(tmp_path / "old-recording") + + with pytest.raises(FileNotFoundError, match="migrate_legacy_capture.py"): + scanner._extract_recording_info(directory, "old-recording") + + def test_a_converted_directory_is_indexed_without_a_legacy_warning( + self, scanner, tmp_path, capsys + ): + examples = _require_examples() + directory = _legacy_capture(tmp_path / "demo_new") + (directory / "recording.db").write_bytes( + (examples / "demo_new" / "recording.db").read_bytes() + ) + + found = scanner.scan_recording_directory(str(tmp_path)) + + assert [recording.id for recording in found] == ["demo_new"] + assert "migrate_legacy_capture.py" not in capsys.readouterr().out + + def test_extract_refuses_a_directory_holding_no_database(self, scanner, tmp_path): + empty = tmp_path / "not-a-recording" + empty.mkdir() + + with pytest.raises(FileNotFoundError, match="recording.db"): + scanner._extract_recording_info(empty, "not-a-recording") + + +class TestCorruptRecordingDatabase: + """An unreadable database degrades to a named entry, it does not abort.""" + + def test_scan_continues_past_a_corrupt_database(self, scanner, tmp_path, capsys): + examples = _require_examples() + broken = tmp_path / "broken" + broken.mkdir() + (broken / "recording.db").write_text("this is not a database") + good = tmp_path / "demo_new" + good.mkdir() + (good / "recording.db").write_bytes( + (examples / "demo_new" / "recording.db").read_bytes() + ) + + found = scanner.scan_recording_directory(str(tmp_path)) + + assert "demo_new" in [recording.id for recording in found] + assert "Warning" in capsys.readouterr().out + + +class TestCatalogRegisterCommand: + """`openadapt-viewer catalog register` is the manual path around scan.""" + + def test_register_indexes_a_real_recording(self, tmp_path, monkeypatch, capsys): + directory = _require_examples() / "demo_new" + _isolate_catalog_home(monkeypatch, tmp_path / "home") + monkeypatch.setattr( + sys, "argv", ["openadapt-viewer", "catalog", "register", str(directory)] + ) + + main() + + out = capsys.readouterr().out + assert "Successfully registered" in out + # Recording.model_dump() names the field `id` and register_recording + # takes `recording_id`, so splatting it raised an uncaught TypeError. + assert "Frames: 14" in out + assert "Events: 14" in out + + def test_register_rejects_a_legacy_directory(self, tmp_path, monkeypatch, capsys): + directory = _legacy_capture(tmp_path / "captures" / "old-recording") + _isolate_catalog_home(monkeypatch, tmp_path / "home") + monkeypatch.setattr( + sys, "argv", ["openadapt-viewer", "catalog", "register", str(directory)] + ) + + with pytest.raises(SystemExit) as exit_info: + main() + + assert exit_info.value.code == 1 + assert "migrate_legacy_capture.py" in capsys.readouterr().err + + +def test_committed_recordings_must_be_present_in_ci(): + """CI must not go green on a suite that skipped every real recording.""" + if os.environ.get("GITHUB_ACTIONS") != "true": + pytest.skip("Only enforced on CI, which checks openadapt-capture out") + + assert _examples_dir() is not None, ( + f"CI must set ${EXAMPLES_ENV}. Without it every test in this module " + "skips and the scanner is verified against nothing." + ) + + +def _isolate_catalog_home(monkeypatch, home: Path) -> None: + """Point the default catalog (~/.openadapt/catalog.db) at a temp home.""" + home.mkdir(parents=True, exist_ok=True) + monkeypatch.setenv("HOME", str(home)) + monkeypatch.setenv("USERPROFILE", str(home)) + monkeypatch.setattr(Path, "home", classmethod(lambda cls: home)) + + +def _connect(directory: Path) -> sqlite3.Connection: + conn = sqlite3.connect(directory / "recording.db") + conn.row_factory = sqlite3.Row + return conn + + +def _scalar(directory: Path, query: str): + with _connect(directory) as conn: + return conn.execute(query).fetchone()[0] + + +def _row(directory: Path, query: str) -> sqlite3.Row: + with _connect(directory) as conn: + return conn.execute(query).fetchone() diff --git a/tests/test_screenshot_generation.py b/tests/test_screenshot_generation.py index 41506e8..949fe5b 100644 --- a/tests/test_screenshot_generation.py +++ b/tests/test_screenshot_generation.py @@ -94,15 +94,22 @@ def test_dependency_check(): reason="openadapt-capture directory not found", ) def test_captures_exist(): - """Test that required capture directories exist.""" - captures = [ - CAPTURE_DIR / "turn-off-nightshift", - CAPTURE_DIR / "demo_new", - ] + """Test that required capture directories exist. + + Both halves of this used to be stale. The captures live under + ``examples/captures/``, not at the checkout root, where .gitignore excludes + them. And the recorder writes ``recording.db``: the bespoke ``capture.db`` + was replaced by openadapt-capture PR #28 on 2026-07-17, so asserting on it + described a file no current checkout has. + """ + examples = CAPTURE_DIR / "examples" / "captures" - for capture_path in captures: + for name in ("turn-off-nightshift", "demo_new"): + capture_path = examples / name assert capture_path.exists(), f"Capture not found: {capture_path}" - assert (capture_path / "capture.db").exists(), f"No capture.db in {capture_path}" + assert (capture_path / "recording.db").exists(), ( + f"No recording.db in {capture_path}" + ) @pytest.mark.skipif(