Add feature cache status scanning with train/classify warnings

[gbeane]

Summary

Adds an in-memory picture of a project's on-disk feature cache and uses it in two places: the video Get Info dialog, and a warning before Train / Classify when features are not cached for the selected window size (they would otherwise be computed mid-run, which can take minutes per video with no explanation to the user).

What's new

Cache inspection (jabs-io) — inspect_identity_cache() reports what one per-identity cache directory holds: format, feature version, pose hash, cached window sizes, whether per-frame features are present, size on disk. It reads only metadata (HDF5 attributes or metadata.json), never feature data, and never validates: an unreadable cache is reported the same as an absent one. Parquet window sizes are only reported when the window_{size}.parquet file actually exists, so the result reflects what could really be loaded.

Per-video status (jabs.project.feature_cache_status) — VideoFeatureCacheStatus aggregates the per-identity summaries: window_sizes (cached for every cached identity), partial_window_sizes, cached_identity_count / is_complete, cache_formats, is_stale, cm_units, size_bytes, and has_window_features(window_size, identities=None). Both on-disk layouts are handled: flat features/<video>/<id>/ and the CLI's features/<video>/<hash>/<id>/.

Nothing here validates a cache against its pose file: that would mean hashing every pose file, which is far too slow for a status scan. A cache whose pose file changed still reports as present and is detected when the features are actually loaded.

Background scan at project open — FeatureCacheScanThread scans the project off the main thread when it opens (the scan touches every per-identity cache directory, slow enough on network storage to matter) and the results are stored on the Project. The thread supports cooperative termination and is stopped in MainWindow.closeEvent.

Project API — feature_cache_status, set_feature_cache_status(), refresh_feature_cache_status(video), invalidate_feature_cache_status(), videos_missing_window_features(...), and labeled_identities(behaviors). Videos with no stored status are scanned on demand, so the queries never depend on the background scan having finished. Training and classification invalidate the status when they finish, since a run caches whatever it computed.

Get Info — new Feature Cache section: cache directory, identities cached, window sizes (and partial ones), format, feature version with an out-of-date flag, distance units, size on disk, and a warning row when per-frame features are missing. The dialog re-scans the single video so what it shows is current.

Train / Classify warning — a Yes/No dialog naming the number of videos whose features are missing, with the video list in the details section. The scope differs per action so the warning is accurate rather than noisy: training checks only labeled identities (it never reads features for unlabeled ones), while classification checks every identity of every target video.

Verification

• 960 tests pass (878 before, 82 added), plus 237 in jabs-io. Ruff clean.
• Checked against real IdentityFeatures output for both cache formats: the scanner agrees with what the production write path produces (3 identities, window sizes 5 and 10, correct version, not stale).
• Checked the warning scope end to end on a real project: with nothing cached, Train flags the labeled video and Classify flags both; after a training run caches the labeled identity, Train no longer warns while Classify still does (it needs the other four identities); a different window size warns again.
• Drove a real offscreen MainWindow through the background scan to confirm results cross the thread boundary and land on the project.

Docs

gui.md (both the online copy and the in-app copy) gets a Feature Computation section and an expanded Get Info description.

[gbeane]

Follow-up commit (d77999a): the in-memory status was being invalidated wholesale on run completion and rebuilt lazily, which meant the repopulating scan ran synchronously on the GUI thread inside the next Train/Classify click handler — a visible stall on a large project on network storage.

Two changes:

• Invalidation is now scoped to what the run touched. Training records the labeled videos it will read (_training_cache_targets, set once the warning is accepted) and invalidates only those; classification invalidates _classification_targets (None still means every video). The rest of the map stays warm.
• Repopulation happens off the main thread. CentralWidget.feature_cache_changed is emitted after invalidation and MainWindow connects it to the existing refresh_feature_cache_status(), so the background scan rebuilds the status while the user reads the training report. If they click Train before it finishes, the on-demand rescan in videos_missing_window_features() still covers it.

Because scan results replace the whole map, _feature_cache_scan_complete() now ignores results from a superseded scan (an earlier scan still finishing would otherwise write back what the cache looked like before the run computed features).

Still no eager update at the moment features are written: the workers that write them don't report back what they wrote, and the rescan is metadata-only.

Verified on a real project with real caches on disk, driving a real offscreen MainWindow: after a single-video classification finishes, only that video's status is dropped, the background rescan repopulates both videos, the newly computed features are reflected (videos_missing_window_features(5) == []), and a superseded scan's results are ignored. 969 tests pass (9 added), ruff clean.

[Copilot]

🟡 Changes recommended

There are correctness and lifecycle issues in the new cache-status/threading logic (status completeness across multiple cache dirs and scan-thread management) that should be addressed before approval.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.

Pull request overview

This PR adds a lightweight, read-only scan of a project’s on-disk feature cache, stores the results in memory on the Project, and uses that status to (1) display cache details in Get Info and (2) warn before Train/Classify when required window-size features are missing (and would otherwise be computed mid-run with no user-facing explanation).

Changes:

• Add cache inspection in jabs-io (per-identity metadata-only scanning) and project-level aggregation/scanning utilities.
• Introduce Project APIs to store/refresh/invalidate cache status and query videos missing a required window size (optionally scoped to labeled identities for training).
• Integrate status into the GUI (background scan on project open, Get Info feature-cache section, train/classify preflight warning), plus docs and test coverage.

File summaries

File	Description
tests/ui/test_video_list_widget.py	Adds coverage for “Get Info” rescanning behavior and error fallback.
tests/ui/test_video_info_dialog.py	New tests asserting feature cache section rendering in the dialog.
tests/ui/test_main_window.py	New tests for background scan result handling and thread lifecycle behavior.
tests/ui/test_feature_cache_text.py	New tests for Qt-free formatting helpers used by the dialog.
tests/ui/test_feature_cache_scan_thread.py	New tests for scan thread emission, failure handling, and termination behavior.
tests/ui/test_central_widget.py	Adds tests for window-size selection and train/classify gating confirmation logic.
tests/project/test_project_feature_cache_status.py	New tests for Project’s in-memory status store and query APIs.
tests/project/test_feature_cache_status.py	New tests for per-video aggregation and on-disk scan behavior.
src/jabs/ui/main_window/video_list_widget.py	Passes per-video cache status into the info dialog; adds per-video rescan with fallback.
src/jabs/ui/main_window/menu_handlers.py	After clearing cache, triggers a background rescan to reflect the empty cache.
src/jabs/ui/main_window/main_window.py	Starts/stops background feature-cache scans and stores results on the open project.
src/jabs/ui/main_window/central_widget.py	Adds train/classify warnings when window-size features are missing; invalidates status after runs.
src/jabs/ui/feature_cache_text.py	Adds pure string formatting helpers (size/format/window-size display).
src/jabs/ui/feature_cache_scan_thread.py	Implements background scan thread with cooperative termination.
src/jabs/ui/dialogs/video_info_dialog.py	Adds a Feature Cache section driven by VideoFeatureCacheStatus.
src/jabs/resources/docs/user_guide/gui.md	Documents new Feature Computation warning behavior and expanded Get Info.
src/jabs/project/project.py	Adds cache status storage, refresh/invalidation, and missing-features queries + labeled identity selection.
src/jabs/project/feature_cache_status.py	Implements per-video status aggregation and project/video cache scanners.
src/jabs/project/init.py	Re-exports new cache status types and scan functions.
packages/jabs-io/tests/feature_cache/test_inspection.py	New tests for metadata-only identity cache inspection across formats and failure modes.
packages/jabs-io/src/jabs/io/feature_cache/inspection.py	Adds inspect_identity_cache() + IdentityCacheInfo for read-only cache inspection.
packages/jabs-io/src/jabs/io/feature_cache/init.py	Exposes inspection API from the feature_cache package.
docs/user-guide/gui.md	Mirrors user-guide updates for the online docs copy.

Review details

• Files reviewed: 23/23 changed files
• Comments generated: 3
• Review effort level: Lite

We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.

[gbeane]

all three were real. Addressed in cd42238.

1. Concurrent scan threads (refresh_feature_cache_status). Correct, and the shutdown consequence was the serious part: closeEvent only terminates the tracked thread, so an untracked earlier scan could still be running at teardown — the exact QThread: Destroyed while thread is still running abort that #425 exists to prevent. It was reachable in practice now that every finished training/classification run triggers a refresh.

Fixed by coalescing to a single in-flight scan: when a refresh arrives while one is running, that scan is asked to stop (so its now-stale results are never emitted) and a replacement is queued, started from _feature_cache_scan_finished. closeEvent clears the queue before waiting, so shutdown can't kick off a new scan.

Measured with real threads, five refreshes fired back to back and each video's scan slowed down:

	max concurrent scans	scan passes
before	5	5
after	1	2 (the interrupted one + the coalesced rescan)

2. is_complete and per-frame features across pose-hash directories. Also correct, and it was inconsistent with window_sizes, which already merges per identity. Added identities_missing_per_frame, which folds per-frame presence per identity (an identity is satisfied if any of its cache directories has per-frame features); is_complete now uses it. The Get Info warning row used the same per-directory any(...) test and had the same flaw, so it now uses the property too — and names the affected identities instead of saying "one or more".

3. cache_formats sort key. Right: str(CacheFormat.HDF5) is 'CacheFormat.HDF5', not 'hdf5', so the code didn't match the docstring. Now sorts on .value. Same order today, but it no longer depends on Enum.__str__.

Tests added: per-identity per-frame merging (including the multi-pose-hash case), cache_formats ordering by value, the scan queue/coalescing behavior and its shutdown interaction, _start_feature_cache_scan thread setup, and the dialog's warning row (present and absent). 977 tests pass, ruff clean.

[Copilot]

🟡 Changes recommended

The shutdown path can still allow a running child QThread to outlive the MainWindow (ignored wait() timeout), risking “QThread destroyed while running” instability on slow/unresponsive storage.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.

Review details

Suppressed comments (2)

src/jabs/project/project.py:626

• labeled_identities() calls VideoManager.load_annotations() for every video, and load_annotations() performs a filesystem exists() check and JSON load each time. This means clicking Train in a large project can cause synchronous disk I/O over all annotation files (potentially slow on network storage) just to compute the warning scope.

        labeled: dict[str, set[int]] = {}
        for video in self._video_manager.videos:
            annotations = self._video_manager.load_annotations(video)
            if annotations is None:
                continue

src/jabs/ui/main_window/central_widget.py:1307

• The docstring for _cleanup_classify_thread() says it cleans up the training thread, which is misleading and makes log/traceback reading harder when debugging classification failures.

        """clean up the training thread"""

• Files reviewed: 23/23 changed files
• Comments generated: 1
• Review effort level: Lite

We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.

[gbeane]

Addressed in 43e0c70 (plus the two suppressed comments). Details, including where I landed differently than suggested:

Shutdown wait ignoring wait()'s return value. Real gap — confirmed by simulating storage that doesn't respond (scan thread blocked in an open() syscall): quitting logged QThread: Destroyed while thread is still running and the process exited 134 (SIGABRT).

I did not adopt the "longer/blocking wait" part of the suggestion: an unbounded wait trades a harmless exit-time abort for an unquittable app, on exactly the slow/unresponsive storage this feature targets. The cooperative stop is checked between videos and cannot interrupt a filesystem call that never returns, so waiting longer only postpones the same outcome.

Instead closeEvent now delegates to _stop_feature_cache_scan(), which escalates: ask the scan to stop → wait(2000) → if that returns False, log a warning and terminate() → wait(1000) → if even that fails, log an error. Terminating is safe here in a way it usually isn't: the scan only reads cache metadata, so it cannot corrupt project data, and losing the scan costs nothing but the status it would have produced.

Two alternatives I tested and rejected:

• Detaching the thread from the window (setParent(None) + a module-level reference) so its destructor wouldn't run: still aborts — Python takes ownership and deletes it during finalization.
• An unbounded wait(): hangs the quit indefinitely.

Measured on the wedged-storage simulation, quitting via the window close button:

	close() duration	exit code
before	2.0 s	134 (SIGABRT, QThread: Destroyed while thread is still running)
after	2.0 s	0, with Feature cache scan did not stop within 2000 ms (unresponsive storage?); terminating the thread

A responsive scan is unaffected: it stops cooperatively in ~0.1 s and is never terminated.

Suppressed comment: labeled_identities() reads every annotation file on the Train click. Fair. The check now short-circuits: _confirm_training_features() first asks the cheap whole-video question (in-memory status, no annotation I/O), and only when something is missing does it read annotations to narrow to labeled identities. Since the labeled identities are a subset of all identities, an empty whole-video result means the narrowed result is empty too, so this never changes the answer — it just makes the fully-cached case (a project prepared with jabs-features, the recommended workflow) free. When features genuinely are missing we do read the annotation files, which is the only way to know whether the warning is real; that cost is small next to the run the user is about to start, which re-reads all of them anyway.

Suppressed comment: _cleanup_classify_thread() docstring said "training thread". Pre-existing copy-paste; corrected to "classification thread".

985 tests pass, ruff clean. New tests cover the escalation ladder (responsive stop, forced stop, unstoppable thread logged) and the short-circuit (asserting labeled_identities is not called when everything is cached).

[Copilot]

🟡 Changes recommended

The new jabs-io cache inspection module currently has verified error-handling and optional-dependency issues (notably unconditional h5py import and directory-size scanning raising OSError) that can break cache scans/imports in supported environments.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.

Review details

Suppressed comments (2)

packages/jabs-io/src/jabs/io/feature_cache/inspection.py:110

• _directory_size() can raise OSError if directory.iterdir() fails (e.g., unreadable/vanishing cache dir). That would bubble out of inspect* and contradict inspect_identity_cache()’s contract of returning None (or otherwise not raising) for unreadable caches; it also risks aborting project-wide scans on transient filesystem errors.

def _directory_size(directory: Path) -> int:
    """Return the total size in bytes of the files directly inside a directory."""
    total = 0
    for path in directory.iterdir():
        if path.is_file():
            try:
                total += path.stat().st_size
            except OSError:
                logger.debug("Could not stat cache file %s", path, exc_info=True)
    return total

packages/jabs-io/src/jabs/io/feature_cache/inspection.py:117

• After making h5py optional, _inspect_hdf5_cache() should explicitly handle the h5py is None case; otherwise it will raise AttributeError when attempting h5py.File(...), which again violates inspect_identity_cache()’s “unreadable cache behaves like absent” contract.

def _inspect_hdf5_cache(identity_dir: Path) -> IdentityCacheInfo | None:
    """Inspect an HDF5 (``features.h5``) cache directory."""
    path = identity_dir / _HDF5_FILENAME
    try:
        with h5py.File(path, "r") as f:

• Files reviewed: 23/23 changed files
• Comments generated: 1
• Review effort level: Lite

We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.

[gbeane]

Addressed in 4ae32e2, but by going the other way on the premise: h5py is now a required dependency of jabs-io, not an optional extra.

HDF5 isn't a format-specific backend in JABS — pose files, prediction files and the default feature cache are all HDF5 — and the "optional" extra was already fiction. Five modules in this package import h5py unconditionally at module scope, and have for a long time:

• io/feature_cache/hdf5.py
• io/internal/pose/hdf5.py
• io/internal/prediction/hdf5.py
• io/internal/dataclass/hdf5.py
• io/feature_cache/inspection.py (this PR)

Only io/base.py guarded it, and its guard was unreachable: every HDF5Adapter subclass lives in one of those modules, so you cannot import a subclass without h5py to hit the "install the h5py extra" error. The package README already stated HDF5 needs no extra (h5py arrived transitively via jabs-core, which requires it) — the extra just made the dependency look optional while every HDF5 code path assumed it.

So rather than adding a lazy import and h5py is None handling to a module that would still be dead code:

• h5py>=3.10.0,<4.0.0 moved into jabs-io's dependencies, matching the bound jabs-core uses; the h5py optional-dependency group is removed.
• io/base.py imports h5py unconditionally like its siblings, and the unreachable HDF5Adapter.__init__ guard is gone.
• containers/docker/Dockerfile.pose no longer installs ./packages/jabs-io[h5py] (that extra no longer exists; h5py comes in as a required dependency).
• uv.lock regenerated: jabs-io now lists h5py directly. Verified by resolving jabs-io on its own into a clean venv — h5py==3.16.0 is pulled in.

Suppressed comment: _directory_size() could raise from iterdir(). Valid, and fixed — I had guarded stat() but not the listing, so an unreadable or vanishing cache directory could have escaped the "unreadable cache behaves like absent" contract and aborted a project-wide scan. The listing is now guarded too and contributes a size of 0; a test asserts inspection still returns its other fields when the listing is denied.

Suppressed comment: handle h5py is None in _inspect_hdf5_cache(). Not applicable after the above — there is no None case, since h5py is a hard requirement and imported unconditionally.

983 tests on this branch plus 238 in jabs-io, ruff clean.

[Copilot]

🟡 Changes recommended

The new cache-coverage check used for Train/Classify warnings currently treats stale/incomplete caches as “cached,” and the classification cleanup leaves stale _classification_targets state.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.

Review details

Suppressed comments (2)

src/jabs/project/feature_cache_status.py:144

• has_window_features() currently only checks whether window_size appears in each identity’s window_sizes. This means a video can be treated as “cached” even when the cache is known to be unusable and will be recomputed mid-run (e.g., stale feature_version or missing per-frame features). That undermines the Train/Classify warning gate, since Project.videos_missing_window_features() relies on this method.

IdentityFeatures recomputes on FeatureVersionException/DistanceScaleException/PoseHashException (see src/jabs/feature_extraction/features.py:278-292), so a stale cache should be treated as missing for warning purposes, and identities lacking per-frame features should not be considered covered.

        cached = self._window_sizes_by_identity()
        if identities is None:
            if self.expected_identity_count is None:
                return False
            identities = range(self.expected_identity_count)
        return all(window_size in cached.get(identity, frozenset()) for identity in identities)

src/jabs/ui/main_window/central_widget.py:1334

• _cleanup_classify_thread() invalidates cache status based on _classification_targets but never clears _classification_targets afterward. This leaves stale targets on the widget instance and can cause later invalidations (or any other logic that inspects _classification_targets) to act on the wrong video set.

        # Same as _cleanup_training_thread, for the videos this run classified
        # (``None`` means every video was targeted, so every status is dropped).
        self._project.invalidate_feature_cache_status(self._classification_targets)
        self.feature_cache_changed.emit()

• Files reviewed: 27/28 changed files
• Comments generated: 0 new
• Review effort level: Lite

We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.

[gbeane]

Both fixed in 6ea9a7b.

has_window_features() treating unusable caches as coverage. Correct, and the miss is bigger than it first looks: bumping FEATURE_VERSION is routine in this project (contributors are told to bump it whenever features change), so after any release that does, every cache on disk is stale — and the gate would have stayed silent through the full recompute it exists to announce. Same for an identity whose per-frame features are missing.

The fix separates the two questions the status object answers:

• What is on disk — window_sizes, partial_window_sizes, is_stale, is_complete are unchanged. Get Info should keep showing "window sizes: 5, 10" next to "Feature version: 16 (out of date, current is 17)"; reporting "none" there would hide data that really is present.
• What would load — has_window_features() now goes through _loadable_window_sizes_by_identity(), which only counts a cache written by the current feature version and holding per-frame features. Project.videos_missing_window_features() inherits this, so the warning fires.

One subtlety worth calling out: the loadable sizes are merged across an identity's pose-hash subdirectories per directory, not per field. A window size present only in a stale directory no longer counts because a sibling directory happens to be current — no single cache there could serve the run.

Verified against real caches built by IdentityFeatures (Parquet, the default for new projects): with current caches videos_missing_window_features(5) == []; after aging the recorded version by one, the same status still reports window_sizes == (5,) and is_stale, and the video is reported as needing computation.

Two causes of recomputation still can't be seen from a status scan, and I've documented them on the method rather than leaving them implicit: a pose file that changed since the cache was written (detecting it means hashing every pose file, too slow for a status scan) and a distance-unit setting that no longer matches the cache. The second one is detectable — cm_units is already on the status object, it just needs the run's setting passed in — so if you want that gap closed too, say so and I'll add it.

_cleanup_classify_thread() leaving _classification_targets set. Right, and the asymmetry was mine: _cleanup_training_thread() consumes its target list but this one didn't. The completion path happened to clear it afterwards, but the cancel and failure paths (_classify_thread_error_callback) did not, so a canceled run left the stale list on the widget. The cleanup now owns that lifecycle and clears it, and the redundant assignment in _classify_thread_complete() is gone (it reads the targets into a local first, before the cleanup).

While adding tests I also fixed a latent trap in the ones I wrote earlier: test_project_feature_cache_status.py hard-coded feature version 17 while the code compares against the real FEATURE_VERSION, so the next bump would have broken those tests for no good reason. The helper now defaults to FEATURE_VERSION and takes an explicit older version when a test wants a stale cache.

989 tests on this branch, ruff clean.