From eb3d057e2cca361a3ac0a37d9995797d794a72ce Mon Sep 17 00:00:00 2001 From: Keith Sheppard Date: Wed, 20 May 2026 19:31:10 -0400 Subject: [PATCH 1/5] add jabs-cli update-labels subcommand Imports labels from another JABS project, remapping identities via bbox IoU against the target's existing pose. Refactors update_pose helpers to be shared, auto-scaffolds a minimal jabs/ when target lacks one, includes tests. --- src/jabs/scripts/cli/cli.py | 2 + src/jabs/scripts/cli/update_labels.py | 527 +++++++++++++++++++++ src/jabs/scripts/cli/update_pose.py | 66 ++- tests/project/test_update_labels.py | 637 ++++++++++++++++++++++++++ 4 files changed, 1220 insertions(+), 12 deletions(-) create mode 100644 src/jabs/scripts/cli/update_labels.py create mode 100644 tests/project/test_update_labels.py diff --git a/src/jabs/scripts/cli/cli.py b/src/jabs/scripts/cli/cli.py index 29ea31cc..91c21ca0 100644 --- a/src/jabs/scripts/cli/cli.py +++ b/src/jabs/scripts/cli/cli.py @@ -24,6 +24,7 @@ from .postprocessing import apply_postprocessing_command from .sample_frames import sample_frames_command from .sample_pose_intervals import sample_pose_intervals_command +from .update_labels import update_labels_command from .update_pose import update_pose_command # find out which classifiers are supported in this environment @@ -48,6 +49,7 @@ def cli(ctx: click.Context, verbose): cli.add_command(compute_features_command) cli.add_command(convert_parquet_command) cli.add_command(update_pose_command) +cli.add_command(update_labels_command) cli.add_command(sample_pose_intervals_command) cli.add_command(sample_frames_command) diff --git a/src/jabs/scripts/cli/update_labels.py b/src/jabs/scripts/cli/update_labels.py new file mode 100644 index 00000000..cc312ef8 --- /dev/null +++ b/src/jabs/scripts/cli/update_labels.py @@ -0,0 +1,527 @@ +"""Update a JABS project in place to replace labels with labels remapped from another project. + +This is the inverse of ``update-pose``: instead of keeping the target's labels and +replacing its pose, ``update-labels`` keeps the target's pose and replaces its +labels with labels imported from a source JABS project. The source provides both +labels and its own pose; the target's existing pose is the destination for the +identity-aware remap. + +The script validates both projects, then performs the label remap in two +disposable staging projects so the live target stays unchanged while the update +is underway. Labels are processed block by block, matched by median bbox IoU +between the source pose and the target pose, and written directly to the staged +destination label track. + +Timeline annotations are carried forward: video-level annotations are copied +as-is, and identity-scoped annotations are remapped by the same interval-matching +logic. If ``--drop-timeline-annotations`` is provided, source timeline +annotations are discarded instead of being copied or remapped. + +Before the live target is modified, the script creates a timestamped backup zip +under ``/.backup`` containing every live file the update may overwrite +or delete: ``jabs/project.json``, annotations, and predictions (pose files are +left untouched). If a failure occurs after the final live apply begins, the +script prints the backup path plus cleanup and manual restore instructions +instead of restoring automatically. + +Behaviors named in the source project but not present in the target are merged +into the target's ``jabs/project.json`` so the imported labels are usable in the +GUI. Behaviors already configured in the target keep their existing +configuration. Existing target labels for videos that the source does not cover +are left untouched (per-video replace). + +The target's pose is unchanged, so the feature cache stays valid and is not +regenerated. Predictions are cleared because they are stale relative to the new +labels; classifiers and the performance cache are left in place. + +Example: + jabs-cli update-labels /path/to/target_project /path/to/source_project --min-iou-thresh 0.5 +""" + +from __future__ import annotations + +import json +import shutil +import sys +import tempfile +from pathlib import Path + +import click + +from jabs.pose_estimation import get_pose_path +from jabs.project import Project + +from .update_pose import ( + _copy_file_atomic, + _create_backup_archive, + _print_manual_restore, + _project_videos, + _restore_cleanup_paths, + _run_staged_label_remap, + _seed_stage_project, + _validate_live_update_targets, + _validate_pose_file, +) + + +def _source_annotated_videos( + source_project_dir: Path, + source_videos: set[str], +) -> list[str]: + """Return source videos that have an annotation file, sorted by filename. + + Annotation files are matched by stem against the set of video filenames found + in the source project. Source annotation files whose stem does not match a + video in the source project are ignored with a warning. + """ + annotations_dir = source_project_dir / "jabs" / "annotations" + if not annotations_dir.is_dir(): + return [] + + stem_to_video = {Path(v).stem: v for v in source_videos} + annotated: list[str] = [] + for annotation_path in sorted(annotations_dir.glob("*.json")): + video = stem_to_video.get(annotation_path.stem) + if video is None: + print( + f"WARNING: Source annotation {annotation_path.name} has no matching " + f"video file in {source_project_dir}; skipping.", + file=sys.stderr, + ) + continue + annotated.append(video) + return annotated + + +def _scan_target_for_init(target_dir: Path) -> list[str]: + """Verify a non-initialized target directory looks like a JABS project before scaffolding. + + A target directory is acceptable for auto-init when it contains at least one + video file and every video has a paired pose file. This is a stricter check + than the basic "any video + any pose" gate in ``ProjectPaths.create_directories`` + and is meant to prevent silent scaffolding into an unrelated directory if the + user mistyped the path. + + Args: + target_dir: Directory expected to contain video and pose files. + + Returns: + Sorted list of video filenames found in the directory. + + Raises: + ValueError: If the directory has no videos, or any video is missing a paired + pose file. + """ + videos = _project_videos(target_dir) + if not videos: + raise ValueError(f"{target_dir} contains no video files (.avi or .mp4)") + + unpaired: list[str] = [] + for video in videos: + try: + pose_path = get_pose_path(target_dir / video) + except ValueError: + unpaired.append(video) + continue + if not pose_path.exists(): + unpaired.append(video) + + if unpaired: + raise ValueError(f"{target_dir} is missing pose files for: {', '.join(unpaired)}") + + return videos + + +def _scaffold_target_project_if_missing(target_dir: Path) -> bool: + """Scaffold a minimal JABS project in ``target_dir`` if it lacks ``jabs/project.json``. + + The target directory must already contain a self-consistent set of video and + pose files (see ``_scan_target_for_init``) before any directories are created. + Constructing :class:`Project` writes ``jabs/project.json`` with empty defaults + and creates the standard subdirectory layout but does not compute features. + + Returns: + ``True`` if the project was scaffolded, ``False`` if ``target_dir`` was + already a valid JABS project. + """ + if Project.is_valid_project_directory(target_dir): + return False + + _scan_target_for_init(target_dir) + print( + f"INFO: {target_dir} has no jabs/ directory; initializing a minimal JABS project " + "(features are not generated — run jabs-init separately to compute them).", + file=sys.stderr, + ) + Project(target_dir, enable_session_tracker=False) + return True + + +def _preflight_label_update_inputs( + target_dir: Path, + source_project_dir: Path, +) -> tuple[list[str], set[str]]: + """Validate target and source inputs before any live mutation occurs. + + If ``target_dir`` is a directory of videos + pose files without a ``jabs/`` + directory, a minimal JABS project is scaffolded automatically (no features + are computed). The source must already be a valid JABS project with + annotations. + + Returns: + Tuple ``(videos, live_annotation_videos)`` where ``videos`` is the sorted + list of source-labeled videos to operate on (each also exists in the + target project) and ``live_annotation_videos`` is the subset of those + videos that already have an annotation file in the live target. + """ + _scaffold_target_project_if_missing(target_dir) + + if not Project.is_valid_project_directory(target_dir): + raise ValueError(f"{target_dir} is not a valid JABS project directory") + if not Project.is_valid_project_directory(source_project_dir): + raise ValueError( + f"{source_project_dir} is not a valid JABS project directory (source labels)" + ) + + target_videos = set(_project_videos(target_dir)) + if not target_videos: + raise ValueError(f"No video files found in {target_dir}") + + source_videos = set(_project_videos(source_project_dir)) + if not source_videos: + raise ValueError(f"No video files found in {source_project_dir}") + + labeled_videos = _source_annotated_videos(source_project_dir, source_videos) + if not labeled_videos: + raise ValueError(f"No labeled videos found in source project {source_project_dir}") + + missing_in_target = sorted(v for v in labeled_videos if v not in target_videos) + if missing_in_target: + raise ValueError( + f"Source-labeled videos missing from target project: {', '.join(missing_in_target)}" + ) + + for video in labeled_videos: + source_video_path = source_project_dir / video + source_pose_path = get_pose_path(source_video_path) + _validate_pose_file( + video, + source_video_path, + source_pose_path, + "source", + require_bboxes=True, + ) + + target_video_path = target_dir / video + target_pose_path = get_pose_path(target_video_path) + _validate_pose_file( + video, + target_video_path, + target_pose_path, + "target", + require_bboxes=True, + ) + + target_annotations_dir = target_dir / "jabs" / "annotations" + live_annotation_videos: set[str] = set() + if target_annotations_dir.exists(): + for video in labeled_videos: + annotation_path = target_annotations_dir / Path(video).with_suffix(".json") + if annotation_path.exists(): + live_annotation_videos.add(video) + + _validate_live_update_targets( + target_dir, + labeled_videos, + live_annotation_videos, + require_writable_pose_files=False, + derived_dir_names=("predictions",), + ) + + return labeled_videos, live_annotation_videos + + +def _merge_source_behaviors_into_staged_project( + source_project_dir: Path, + dest_stage: Path, +) -> list[str]: + """Merge behaviors from the source project's project.json into the staged target. + + Behaviors already present in the staged target keep their existing configuration; + behaviors that exist only in the source are added with the source's configuration + so the imported labels are usable in the GUI. Returns the list of behavior names + newly added to the staged target. + """ + source_project_file = source_project_dir / "jabs" / "project.json" + staged_project_file = dest_stage / "jabs" / "project.json" + + try: + source_settings = json.loads(source_project_file.read_text()) + except FileNotFoundError: + return [] + except json.JSONDecodeError as exc: + raise ValueError( + f"Source project file {source_project_file} is not valid JSON: {exc}" + ) from exc + + source_behaviors = source_settings.get("behavior") + if not isinstance(source_behaviors, dict) or not source_behaviors: + return [] + + try: + staged_settings = json.loads(staged_project_file.read_text()) + except FileNotFoundError: + staged_settings = {} + except json.JSONDecodeError as exc: + raise ValueError( + f"Staged project file {staged_project_file} is not valid JSON: {exc}" + ) from exc + + staged_behaviors = staged_settings.get("behavior") + if not isinstance(staged_behaviors, dict): + staged_behaviors = {} + + newly_added: list[str] = [] + for name, settings in source_behaviors.items(): + if name in staged_behaviors: + continue + staged_behaviors[name] = settings + newly_added.append(name) + + if not newly_added: + return [] + + staged_settings["behavior"] = staged_behaviors + tmp = staged_project_file.with_suffix(".json.tmp") + tmp.write_text(json.dumps(staged_settings, indent=2, sort_keys=True)) + tmp.replace(staged_project_file) + return sorted(newly_added) + + +def _apply_live_label_update( + project_dir: Path, + label_dest_project: Project, + videos: list[str], + backup_path: Path, +) -> None: + """Apply staged label-update output back to the live target project. + + Copies staged annotations and ``project.json`` into the live target, then + removes the live ``predictions/`` directory. Pose files, the feature/perf + cache, classifiers, and features are left untouched. + """ + live_annotations_dir = project_dir / "jabs" / "annotations" + staged_annotations_dir = label_dest_project.project_paths.annotations_dir + + missing_annotations = sorted( + video + for video in videos + if not (staged_annotations_dir / Path(video).with_suffix(".json")).exists() + ) + if missing_annotations: + raise RuntimeError( + "staged label update did not produce annotations for: " + + ", ".join(missing_annotations) + ) + + cleanup_paths = _restore_cleanup_paths( + project_dir, + videos, + None, + staged_annotations_dir, + ) + + try: + live_annotations_dir.mkdir(parents=True, exist_ok=True) + for video in videos: + staged_annotation = staged_annotations_dir / Path(video).with_suffix(".json") + if staged_annotation.exists(): + _copy_file_atomic( + staged_annotation, + live_annotations_dir / Path(video).with_suffix(".json"), + ) + + _copy_file_atomic( + label_dest_project.project_paths.project_file, + project_dir / "jabs" / "project.json", + ) + + shutil.rmtree(project_dir / "jabs" / "predictions", ignore_errors=True) + except Exception as exc: + print( + f"ERROR: Failed while applying the label update to the live project: {exc}", + file=sys.stderr, + ) + _print_manual_restore(project_dir, backup_path, cleanup_paths) + raise SystemExit(1) from exc + + +def update_project_labels_in_place( + project_dir: Path, + source_project_dir: Path, + min_iou: float, + verbose: bool = False, + annotate_failures: bool = False, + drop_timeline_annotations: bool = False, +) -> tuple[int, int, Path, list[str]]: + """Update a live project in place by replacing its labels from ``source_project_dir``. + + The target's pose is left unchanged; incoming labels are remapped to the + target's existing identity numbering via bbox IoU. + + Args: + project_dir: Path to the live target project directory whose labels will be replaced. + source_project_dir: Path to a JABS project providing the replacement labels and the + pose used for IoU matching. + min_iou: Minimum median IoU required to accept a block match. + verbose: Whether to print successful block matches. + annotate_failures: Whether to write timeline annotations for failed block matches. + drop_timeline_annotations: Whether to discard source timeline annotations + instead of copying or remapping them. + + Returns: + Tuple ``(total_success, total_skipped, backup_path, newly_added_behaviors)`` + describing the completed update. + """ + project_dir = project_dir.resolve() + source_project_dir = source_project_dir.resolve() + + if project_dir == source_project_dir: + raise ValueError("Target and source project directories must be different") + + labeled_videos, _live_annotation_videos = _preflight_label_update_inputs( + project_dir, + source_project_dir, + ) + backup_path = _create_backup_archive( + project_dir, + labeled_videos, + include_pose_files=False, + prefix="update_labels", + ) + + with tempfile.TemporaryDirectory(prefix="jabs-update-labels-") as temp_root: + temp_root_path = Path(temp_root) + source_stage = temp_root_path / "source_stage" + dest_stage = temp_root_path / "dest_stage" + + _seed_stage_project(source_stage, source_project_dir, copy_annotations=True) + _seed_stage_project(dest_stage, project_dir, copy_annotations=False) + + newly_added_behaviors = _merge_source_behaviors_into_staged_project( + source_project_dir, + dest_stage, + ) + + label_source_project = Project( + source_stage, + enable_session_tracker=False, + video_dir=source_project_dir, + pose_dir=source_project_dir, + validate_project_dir=False, + ) + label_dest_project = Project( + dest_stage, + enable_session_tracker=False, + video_dir=project_dir, + pose_dir=project_dir, + validate_project_dir=False, + ) + + total_success, total_skipped = _run_staged_label_remap( + label_source_project, + label_dest_project, + min_iou, + verbose, + annotate_failures, + drop_timeline_annotations, + ) + + _apply_live_label_update( + project_dir, + label_dest_project, + labeled_videos, + backup_path, + ) + + return total_success, total_skipped, backup_path, newly_added_behaviors + + +@click.command( + name="update-labels", + context_settings={"max_content_width": 120}, + help=__doc__, +) +@click.argument( + "project", + type=click.Path( + exists=True, + file_okay=False, + dir_okay=True, + path_type=Path, + ), +) +@click.argument( + "source_project", + type=click.Path( + exists=True, + file_okay=False, + dir_okay=True, + path_type=Path, + ), +) +@click.option( + "--min-iou-thresh", + "min_iou", + type=float, + default=0.5, + show_default=True, + help="Minimum acceptable median IoU for a label remap match.", +) +@click.option( + "--verbose", + is_flag=True, + help="Print successful label remap assignments in addition to warnings.", +) +@click.option( + "--annotate-failures", + is_flag=True, + help="Add timeline annotations to the target for each block whose label remap fails.", +) +@click.option( + "--drop-timeline-annotations", + is_flag=True, + help="Discard source timeline annotations instead of copying or remapping them.", +) +def update_labels_command( + project: Path, + source_project: Path, + min_iou: float, + verbose: bool, + annotate_failures: bool, + drop_timeline_annotations: bool, +) -> None: + """Update a JABS project in place by replacing labels imported from another project.""" + total_success, total_skipped, backup_path, newly_added_behaviors = ( + update_project_labels_in_place( + project, + source_project, + min_iou, + verbose=verbose, + annotate_failures=annotate_failures, + drop_timeline_annotations=drop_timeline_annotations, + ) + ) + + click.echo(f"Backup archive: {backup_path}") + click.echo( + f"Label update summary: {total_success} label blocks assigned, " + f"{total_skipped} label blocks skipped " + f"(IoU threshold={min_iou})" + ) + if newly_added_behaviors: + click.echo( + "Behaviors added to project.json from source: " + ", ".join(newly_added_behaviors) + ) + else: + click.echo("No new behaviors added to project.json.") diff --git a/src/jabs/scripts/cli/update_pose.py b/src/jabs/scripts/cli/update_pose.py index 021117e1..a2108813 100644 --- a/src/jabs/scripts/cli/update_pose.py +++ b/src/jabs/scripts/cli/update_pose.py @@ -470,8 +470,23 @@ def _validate_live_update_targets( project_dir: Path, videos: list[str], live_annotation_videos: set[str], + *, + require_writable_pose_files: bool = True, + derived_dir_names: tuple[str, ...] = ("predictions", "cache"), ) -> None: - """Best-effort preflight check that live update targets can be replaced or removed.""" + """Best-effort preflight check that live update targets can be replaced or removed. + + Args: + project_dir: Live project directory. + videos: Video filenames that may need pose-file writability checks. + live_annotation_videos: Subset of ``videos`` that have existing annotation files + in the live project. These are checked for writability. + require_writable_pose_files: When ``True`` (default), require that live pose files + for every video are writable. Disable when the operation does not replace + pose files (e.g. ``update-labels``). + derived_dir_names: Derived jabs/ subdirectories that the apply step may remove. + Each is checked for writability if it exists. + """ jabs_dir = project_dir / "jabs" annotations_dir = jabs_dir / "annotations" backup_dir = project_dir / ".backup" @@ -486,15 +501,16 @@ def _validate_live_update_targets( _require_writable_existing_path(jabs_dir / "project.json", "live project file") - for video in videos: - for live_pose_path in _pose_files_for_video(video, project_dir): - _require_writable_existing_path(live_pose_path, "live pose file") + if require_writable_pose_files: + for video in videos: + for live_pose_path in _pose_files_for_video(video, project_dir): + _require_writable_existing_path(live_pose_path, "live pose file") for video in live_annotation_videos: annotation_path = annotations_dir / Path(video).with_suffix(".json") _require_writable_existing_path(annotation_path, "live annotation file") - for derived_dir_name in ("predictions", "cache"): + for derived_dir_name in derived_dir_names: derived_dir = jabs_dir / derived_dir_name if not derived_dir.exists(): continue @@ -596,15 +612,30 @@ def _load_preexisting_window_sizes(project_dir: Path) -> tuple[int, ...]: def _create_backup_archive( project_dir: Path, videos: list[str], + *, + include_pose_files: bool = True, + prefix: str = "update_pose", ) -> Path: - """Create a timestamped backup archive of live files that will be replaced or removed.""" + """Create a timestamped backup archive of live files that will be replaced or removed. + + Args: + project_dir: Live project directory whose files should be backed up. + videos: Video filenames whose pose files should be included when + ``include_pose_files`` is true. + include_pose_files: When ``True`` (default), include every per-video pose file + in the archive. Disable when the operation does not replace pose files + (e.g. ``update-labels``). + prefix: Filename prefix for the generated archive. The full name is + ``{prefix}_{YYYYMMDD_HHMMSS}.zip``. + """ backup_dir = project_dir / ".backup" backup_dir.mkdir(exist_ok=True) - backup_path = backup_dir / f"update_pose_{datetime.now().strftime('%Y%m%d_%H%M%S')}.zip" + backup_path = backup_dir / f"{prefix}_{datetime.now().strftime('%Y%m%d_%H%M%S')}.zip" files_to_backup: set[Path] = set() - for video in videos: - files_to_backup.update(_pose_files_for_video(video, project_dir)) + if include_pose_files: + for video in videos: + files_to_backup.update(_pose_files_for_video(video, project_dir)) project_file = project_dir / "jabs" / "project.json" if project_file.exists(): @@ -749,13 +780,24 @@ def _copy_file_atomic(source: Path, destination: Path) -> None: def _restore_cleanup_paths( project_dir: Path, videos: list[str], - replacement_pose_files: dict[str, Path], + replacement_pose_files: dict[str, Path] | None, staged_annotations_dir: Path, ) -> list[Path]: - """Return live-project files that may have been created during the failed apply step.""" + """Return live-project files that may have been created during the failed apply step. + + Args: + project_dir: Live project directory. + videos: Videos that the apply step would touch. + replacement_pose_files: Per-video replacement pose paths whose basenames may have + been freshly written into ``project_dir``. Pass ``None`` (or omit entries) for + operations that do not write new pose files (e.g. ``update-labels``). + staged_annotations_dir: Directory containing staged annotation JSON files that + may have been copied into the live annotations directory. + """ cleanup_paths: set[Path] = set() for video in videos: - cleanup_paths.add(project_dir / replacement_pose_files[video].name) + if replacement_pose_files is not None and video in replacement_pose_files: + cleanup_paths.add(project_dir / replacement_pose_files[video].name) staged_annotation = staged_annotations_dir / Path(video).with_suffix(".json") if staged_annotation.exists(): diff --git a/tests/project/test_update_labels.py b/tests/project/test_update_labels.py new file mode 100644 index 00000000..c2b940a7 --- /dev/null +++ b/tests/project/test_update_labels.py @@ -0,0 +1,637 @@ +import json +import os +from pathlib import Path +from types import SimpleNamespace + +import pytest +from click.testing import CliRunner + +import jabs.scripts.cli.update_labels as update_labels +import jabs.scripts.cli.update_pose as update_pose +from jabs.scripts.cli.cli import cli + + +def _make_valid_project_dir( + project_dir: Path, + video_name: str = "video1.avi", + pose_version: int = 8, + behaviors: dict | None = None, + write_annotation: bool = False, +) -> Path: + """Create a minimal-on-disk JABS project for preflight tests.""" + annotations_dir = project_dir / "jabs" / "annotations" + annotations_dir.mkdir(parents=True) + project_file = project_dir / "jabs" / "project.json" + project_file.write_text(json.dumps({"behavior": behaviors or {}})) + (project_dir / video_name).touch() + (project_dir / f"{Path(video_name).stem}_pose_est_v{pose_version}.h5").touch() + if write_annotation: + (annotations_dir / f"{Path(video_name).stem}.json").write_text("{}") + return project_dir + + +def _stub_pose_reader(monkeypatch, num_frames: int = 10, version: int = 8) -> None: + """Replace pose-file readers used by the preflight with a permissive stub.""" + monkeypatch.setattr( + update_pose, + "open_pose_file", + lambda *_args, **_kwargs: SimpleNamespace( + format_major_version=version, + has_bounding_boxes=True, + num_frames=num_frames, + ), + ) + monkeypatch.setattr( + update_pose.VideoReader, + "get_nframes_from_file", + staticmethod(lambda _path: num_frames), + ) + + +def test_scan_target_for_init_returns_sorted_videos(tmp_path): + """The scan should return the sorted list of videos when every video has a paired pose.""" + target_dir = tmp_path / "target" + target_dir.mkdir() + (target_dir / "b.mp4").touch() + (target_dir / "b_pose_est_v8.h5").touch() + (target_dir / "a.avi").touch() + (target_dir / "a_pose_est_v8.h5").touch() + + assert update_labels._scan_target_for_init(target_dir) == ["a.avi", "b.mp4"] + + +def test_scan_target_for_init_rejects_empty_directory(tmp_path): + """The scan should reject a directory with no video files.""" + target_dir = tmp_path / "target" + target_dir.mkdir() + + with pytest.raises(ValueError, match="no video files"): + update_labels._scan_target_for_init(target_dir) + + +def test_scan_target_for_init_rejects_video_without_pose(tmp_path): + """The scan should reject a directory where a video has no paired pose file.""" + target_dir = tmp_path / "target" + target_dir.mkdir() + (target_dir / "video1.avi").touch() + # no pose file + (target_dir / "video2.avi").touch() + (target_dir / "video2_pose_est_v8.h5").touch() + + with pytest.raises(ValueError, match="missing pose files for: video1.avi"): + update_labels._scan_target_for_init(target_dir) + + +def test_scaffold_target_project_if_missing_creates_jabs_layout(tmp_path, monkeypatch): + """A bare videos+pose directory should be auto-scaffolded into a JABS project.""" + target_dir = tmp_path / "target" + target_dir.mkdir() + (target_dir / "video1.avi").touch() + (target_dir / "video1_pose_est_v8.h5").touch() + + captured: dict[str, object] = {} + + class FakeProject: + def __init__(self, project_path, **kwargs): + captured["project_path"] = project_path + captured["kwargs"] = kwargs + (project_path / "jabs").mkdir(parents=True, exist_ok=True) + (project_path / "jabs" / "project.json").write_text("{}") + + monkeypatch.setattr(update_labels, "Project", FakeProject) + # The real Project.is_valid_project_directory is a staticmethod we still need. + monkeypatch.setattr( + update_labels.Project, + "is_valid_project_directory", + staticmethod(lambda d: (d / "jabs" / "project.json").exists()), + raising=False, + ) + + scaffolded = update_labels._scaffold_target_project_if_missing(target_dir) + + assert scaffolded is True + assert captured["project_path"] == target_dir + assert captured["kwargs"]["enable_session_tracker"] is False + assert (target_dir / "jabs" / "project.json").exists() + + +def test_scaffold_target_project_if_missing_skips_existing_project(tmp_path, monkeypatch): + """An already-initialized target should not be re-scaffolded.""" + target_dir = tmp_path / "target" + (target_dir / "jabs").mkdir(parents=True) + (target_dir / "jabs" / "project.json").write_text("{}") + + def fail_project_init(*_args, **_kwargs): # pragma: no cover - guarded by assertion + raise AssertionError("Project should not be re-constructed when jabs/ exists") + + monkeypatch.setattr(update_labels, "Project", fail_project_init) + monkeypatch.setattr( + update_labels.Project, + "is_valid_project_directory", + staticmethod(lambda _d: True), + raising=False, + ) + + assert update_labels._scaffold_target_project_if_missing(target_dir) is False + + +def test_scaffold_target_project_if_missing_refuses_unpaired_dir(tmp_path): + """Auto-scaffold must refuse a directory with mismatched video and pose files.""" + target_dir = tmp_path / "target" + target_dir.mkdir() + (target_dir / "video1.avi").touch() + # intentional: no pose file for video1 + + with pytest.raises(ValueError, match="missing pose files for: video1.avi"): + update_labels._scaffold_target_project_if_missing(target_dir) + + +def test_preflight_returns_labeled_videos_and_live_annotation_set(tmp_path, monkeypatch): + """The preflight should return source labeled videos and the live-annotated subset.""" + target_dir = tmp_path / "target" + source_dir = tmp_path / "source" + _make_valid_project_dir(target_dir, write_annotation=True) + _make_valid_project_dir(source_dir, behaviors={"Grooming": {}}, write_annotation=True) + _stub_pose_reader(monkeypatch) + + videos, live_annotation_videos = update_labels._preflight_label_update_inputs( + target_dir, source_dir + ) + + assert videos == ["video1.avi"] + assert live_annotation_videos == {"video1.avi"} + + +def test_preflight_rejects_empty_target_dir(tmp_path): + """The preflight should fail when the target dir has no videos and no jabs/ to scaffold from.""" + target_dir = tmp_path / "target" + source_dir = tmp_path / "source" + target_dir.mkdir() + _make_valid_project_dir(source_dir, write_annotation=True) + + with pytest.raises(ValueError, match="no video files"): + update_labels._preflight_label_update_inputs(target_dir, source_dir) + + +def test_preflight_auto_scaffolds_target_when_jabs_missing(tmp_path, monkeypatch): + """A target with videos+pose but no jabs/ should be scaffolded automatically by preflight.""" + target_dir = tmp_path / "target" + source_dir = tmp_path / "source" + target_dir.mkdir() + (target_dir / "video1.avi").touch() + (target_dir / "video1_pose_est_v8.h5").touch() + _make_valid_project_dir(source_dir, write_annotation=True) + _stub_pose_reader(monkeypatch) + + real_project = update_labels.Project + + class FakeProject: + is_valid_project_directory = staticmethod( + lambda d: (Path(d) / "jabs" / "project.json").exists() + ) + + def __init__(self, project_path, **_kwargs): + (Path(project_path) / "jabs" / "annotations").mkdir(parents=True, exist_ok=True) + (Path(project_path) / "jabs" / "project.json").write_text("{}") + + monkeypatch.setattr(update_labels, "Project", FakeProject) + try: + videos, _ = update_labels._preflight_label_update_inputs(target_dir, source_dir) + finally: + monkeypatch.setattr(update_labels, "Project", real_project) + + assert videos == ["video1.avi"] + assert (target_dir / "jabs" / "project.json").exists() + + +def test_preflight_rejects_invalid_source(tmp_path): + """The preflight should fail when the source is not a valid JABS project.""" + target_dir = tmp_path / "target" + source_dir = tmp_path / "source" + _make_valid_project_dir(target_dir, write_annotation=True) + source_dir.mkdir() + + with pytest.raises(ValueError, match="source labels"): + update_labels._preflight_label_update_inputs(target_dir, source_dir) + + +def test_preflight_rejects_source_video_missing_in_target(tmp_path, monkeypatch): + """The preflight should fail when the source has labels for a video not in the target.""" + target_dir = tmp_path / "target" + source_dir = tmp_path / "source" + _make_valid_project_dir(target_dir, video_name="video1.avi", write_annotation=False) + _make_valid_project_dir(source_dir, video_name="other.avi", write_annotation=True) + _stub_pose_reader(monkeypatch) + + with pytest.raises(ValueError, match="missing from target project"): + update_labels._preflight_label_update_inputs(target_dir, source_dir) + + +def test_preflight_requires_source_labels(tmp_path, monkeypatch): + """The preflight should fail when the source project has no annotation files.""" + target_dir = tmp_path / "target" + source_dir = tmp_path / "source" + _make_valid_project_dir(target_dir, write_annotation=True) + _make_valid_project_dir(source_dir, write_annotation=False) + _stub_pose_reader(monkeypatch) + + with pytest.raises(ValueError, match="No labeled videos found"): + update_labels._preflight_label_update_inputs(target_dir, source_dir) + + +def test_preflight_skips_unmatched_annotation_files(tmp_path, monkeypatch, capsys): + """Source annotation files with no matching video file should be skipped with a warning.""" + target_dir = tmp_path / "target" + source_dir = tmp_path / "source" + _make_valid_project_dir(target_dir, write_annotation=False) + _make_valid_project_dir(source_dir, write_annotation=True) + orphan = source_dir / "jabs" / "annotations" / "orphan.json" + orphan.write_text("{}") + _stub_pose_reader(monkeypatch) + + videos, live_annotation_videos = update_labels._preflight_label_update_inputs( + target_dir, source_dir + ) + + assert videos == ["video1.avi"] + assert live_annotation_videos == set() + err = capsys.readouterr().err + assert "orphan.json" in err and "no matching video" in err + + +def test_preflight_does_not_require_pose_writability(tmp_path, monkeypatch): + """Preflight should succeed when live pose files are read-only because pose stays put.""" + target_dir = tmp_path / "target" + source_dir = tmp_path / "source" + _make_valid_project_dir(target_dir, write_annotation=True) + _make_valid_project_dir(source_dir, write_annotation=True) + _stub_pose_reader(monkeypatch) + + pose_path = target_dir / "video1_pose_est_v8.h5" + + def fake_access(path, mode): + return not (Path(path) == pose_path and mode & os.W_OK) + + monkeypatch.setattr(update_pose.os, "access", fake_access) + + videos, _ = update_labels._preflight_label_update_inputs(target_dir, source_dir) + assert videos == ["video1.avi"] + + +def test_merge_source_behaviors_adds_only_missing_entries(tmp_path): + """Source behaviors should only be added if the target does not already have them.""" + source_dir = tmp_path / "source" + dest_stage = tmp_path / "dest_stage" + (source_dir / "jabs").mkdir(parents=True) + (dest_stage / "jabs").mkdir(parents=True) + + (source_dir / "jabs" / "project.json").write_text( + json.dumps( + { + "behavior": { + "Grooming": {"window_size": 5, "source_only": True}, + "Climbing": {"window_size": 7}, + } + } + ) + ) + (dest_stage / "jabs" / "project.json").write_text( + json.dumps( + { + "behavior": { + "Grooming": {"window_size": 99, "target_only": True}, + } + } + ) + ) + + newly_added = update_labels._merge_source_behaviors_into_staged_project(source_dir, dest_stage) + + assert newly_added == ["Climbing"] + updated = json.loads((dest_stage / "jabs" / "project.json").read_text()) + # Grooming kept target's settings; Climbing added from source. + assert updated["behavior"]["Grooming"] == {"window_size": 99, "target_only": True} + assert updated["behavior"]["Climbing"] == {"window_size": 7} + + +def test_merge_source_behaviors_no_op_when_all_present(tmp_path): + """No behaviors should be added when the target already has every source behavior.""" + source_dir = tmp_path / "source" + dest_stage = tmp_path / "dest_stage" + (source_dir / "jabs").mkdir(parents=True) + (dest_stage / "jabs").mkdir(parents=True) + + (source_dir / "jabs" / "project.json").write_text( + json.dumps({"behavior": {"Grooming": {"window_size": 5}}}) + ) + original = json.dumps({"behavior": {"Grooming": {"window_size": 99}}}, indent=2) + (dest_stage / "jabs" / "project.json").write_text(original) + + newly_added = update_labels._merge_source_behaviors_into_staged_project(source_dir, dest_stage) + + assert newly_added == [] + assert (dest_stage / "jabs" / "project.json").read_text() == original + + +def test_merge_source_behaviors_creates_behavior_section_when_missing(tmp_path): + """The merge should add a behavior section to the staged target if missing.""" + source_dir = tmp_path / "source" + dest_stage = tmp_path / "dest_stage" + (source_dir / "jabs").mkdir(parents=True) + (dest_stage / "jabs").mkdir(parents=True) + + (source_dir / "jabs" / "project.json").write_text( + json.dumps({"behavior": {"Grooming": {"window_size": 5}}}) + ) + (dest_stage / "jabs" / "project.json").write_text(json.dumps({})) + + newly_added = update_labels._merge_source_behaviors_into_staged_project(source_dir, dest_stage) + + assert newly_added == ["Grooming"] + updated = json.loads((dest_stage / "jabs" / "project.json").read_text()) + assert updated["behavior"] == {"Grooming": {"window_size": 5}} + + +def test_apply_live_label_update_replaces_annotations_and_clears_predictions(tmp_path): + """Applying a staged label update should rewrite annotations and clear predictions only.""" + project_dir = tmp_path / "project" + live_annotations_dir = project_dir / "jabs" / "annotations" + live_predictions_dir = project_dir / "jabs" / "predictions" + live_cache_dir = project_dir / "jabs" / "cache" + live_annotations_dir.mkdir(parents=True) + live_predictions_dir.mkdir(parents=True) + live_cache_dir.mkdir(parents=True) + + live_annotation = live_annotations_dir / "video1.json" + live_annotation.write_text("live-annotation") + live_project_file = project_dir / "jabs" / "project.json" + live_project_file.write_text("live-project") + pose_file = project_dir / "video1_pose_est_v8.h5" + pose_file.write_text("pose-v8") + (live_predictions_dir / "video1.h5").write_text("prediction") + (live_cache_dir / "cache.bin").write_text("cache") + + stage_root = tmp_path / "stage" + staged_annotations_dir = stage_root / "jabs" / "annotations" + staged_annotations_dir.mkdir(parents=True) + staged_annotation = staged_annotations_dir / "video1.json" + staged_annotation.write_text("staged-annotation") + staged_project_file = stage_root / "jabs" / "project.json" + staged_project_file.write_text("staged-project") + + label_dest_project = SimpleNamespace( + project_paths=SimpleNamespace( + annotations_dir=staged_annotations_dir, + project_file=staged_project_file, + ) + ) + + update_labels._apply_live_label_update( + project_dir, + label_dest_project, + ["video1.avi"], + project_dir / ".backup" / "update_labels_test.zip", + ) + + assert live_annotation.read_text() == "staged-annotation" + assert live_project_file.read_text() == "staged-project" + assert pose_file.read_text() == "pose-v8" + assert not live_predictions_dir.exists() + assert (live_cache_dir / "cache.bin").read_text() == "cache" + + +def test_apply_live_label_update_failure_prints_cleanup_instructions( + tmp_path, monkeypatch, capsys +): + """A failed apply should print restore instructions and exit non-zero.""" + project_dir = tmp_path / "project" + live_annotations_dir = project_dir / "jabs" / "annotations" + live_annotations_dir.mkdir(parents=True) + (project_dir / "jabs" / "project.json").write_text("live-project") + + stage_root = tmp_path / "stage" + staged_annotations_dir = stage_root / "jabs" / "annotations" + staged_annotations_dir.mkdir(parents=True) + (staged_annotations_dir / "video1.json").write_text("staged-annotation") + staged_project_file = stage_root / "jabs" / "project.json" + staged_project_file.write_text("staged-project") + + label_dest_project = SimpleNamespace( + project_paths=SimpleNamespace( + annotations_dir=staged_annotations_dir, + project_file=staged_project_file, + ) + ) + + def fail_rmtree(_path, ignore_errors=False): + raise RuntimeError("boom") + + monkeypatch.setattr(update_labels.shutil, "rmtree", fail_rmtree) + + with pytest.raises(SystemExit): + update_labels._apply_live_label_update( + project_dir, + label_dest_project, + ["video1.avi"], + project_dir / ".backup" / "update_labels_test.zip", + ) + + stderr = capsys.readouterr().err + assert "boom" in stderr + assert "rm -f" in stderr + assert "jabs/annotations/video1.json" in stderr + assert "unzip -o .backup/update_labels_test.zip" in stderr + + +def test_apply_live_label_update_raises_when_staged_annotation_missing(tmp_path): + """Missing staged output for a labeled video should be a hard error before any writes.""" + project_dir = tmp_path / "project" + (project_dir / "jabs" / "annotations").mkdir(parents=True) + stage_root = tmp_path / "stage" + staged_annotations_dir = stage_root / "jabs" / "annotations" + staged_annotations_dir.mkdir(parents=True) + staged_project_file = stage_root / "jabs" / "project.json" + staged_project_file.write_text("staged-project") + + label_dest_project = SimpleNamespace( + project_paths=SimpleNamespace( + annotations_dir=staged_annotations_dir, + project_file=staged_project_file, + ) + ) + + with pytest.raises(RuntimeError, match="did not produce annotations"): + update_labels._apply_live_label_update( + project_dir, + label_dest_project, + ["video1.avi"], + project_dir / ".backup" / "update_labels_test.zip", + ) + + +def test_update_labels_subcommand_forwards_options(tmp_path, monkeypatch): + """The jabs-cli update-labels subcommand should forward parsed options to the helper.""" + project_dir = tmp_path / "project" + source_dir = tmp_path / "source" + project_dir.mkdir() + source_dir.mkdir() + + captured = {} + + def fake_update_project_labels_in_place( + project_dir_arg, + source_project_dir_arg, + min_iou, + *, + verbose, + annotate_failures, + drop_timeline_annotations, + ): + captured.update( + { + "project_dir": project_dir_arg, + "source_project_dir": source_project_dir_arg, + "min_iou": min_iou, + "verbose": verbose, + "annotate_failures": annotate_failures, + "drop_timeline_annotations": drop_timeline_annotations, + } + ) + return 4, 2, project_dir / ".backup" / "update_labels_test.zip", ["Climbing"] + + monkeypatch.setattr( + update_labels, + "update_project_labels_in_place", + fake_update_project_labels_in_place, + ) + + runner = CliRunner() + result = runner.invoke( + cli, + [ + "update-labels", + str(project_dir), + str(source_dir), + "--min-iou-thresh", + "0.75", + "--verbose", + "--annotate-failures", + "--drop-timeline-annotations", + ], + ) + + assert result.exit_code == 0, result.output + assert captured == { + "project_dir": project_dir, + "source_project_dir": source_dir, + "min_iou": 0.75, + "verbose": True, + "annotate_failures": True, + "drop_timeline_annotations": True, + } + assert "Backup archive:" in result.output + assert "Label update summary: 4 label blocks assigned, 2 label blocks skipped" in result.output + assert "Behaviors added to project.json from source: Climbing" in result.output + + +def test_update_labels_subcommand_reports_no_new_behaviors(tmp_path, monkeypatch): + """The CLI summary should mention when no behaviors were newly added.""" + project_dir = tmp_path / "project" + source_dir = tmp_path / "source" + project_dir.mkdir() + source_dir.mkdir() + + monkeypatch.setattr( + update_labels, + "update_project_labels_in_place", + lambda *_args, **_kwargs: ( + 0, + 0, + project_dir / ".backup" / "update_labels_test.zip", + [], + ), + ) + + runner = CliRunner() + result = runner.invoke( + cli, + ["update-labels", str(project_dir), str(source_dir)], + ) + + assert result.exit_code == 0 + assert "No new behaviors added to project.json." in result.output + + +def test_update_project_labels_in_place_invokes_pipeline(tmp_path, monkeypatch): + """The orchestrator should run preflight, backup, staged remap, and apply in order.""" + target_dir = tmp_path / "project" + source_dir = tmp_path / "source" + target_dir.mkdir() + source_dir.mkdir() + + sequence = [] + + monkeypatch.setattr( + update_labels, + "_preflight_label_update_inputs", + lambda *_args: (sequence.append("preflight") or (["video1.avi"], set())), + ) + + def fake_create_backup_archive(project_dir_arg, videos_arg, *, include_pose_files, prefix): + sequence.append("backup") + assert include_pose_files is False + assert prefix == "update_labels" + assert videos_arg == ["video1.avi"] + return project_dir_arg / ".backup" / "update_labels_test.zip" + + monkeypatch.setattr(update_labels, "_create_backup_archive", fake_create_backup_archive) + monkeypatch.setattr( + update_labels, + "_seed_stage_project", + lambda *_args, **_kwargs: sequence.append("seed"), + ) + monkeypatch.setattr( + update_labels, + "_merge_source_behaviors_into_staged_project", + lambda *_args, **_kwargs: (sequence.append("merge_behaviors") or ["Grooming"]), + ) + monkeypatch.setattr( + update_labels, + "Project", + lambda *args, **kwargs: SimpleNamespace(project_paths=SimpleNamespace()), + ) + monkeypatch.setattr( + update_labels, + "_run_staged_label_remap", + lambda *_args, **_kwargs: (sequence.append("remap") or (3, 1)), + ) + monkeypatch.setattr( + update_labels, + "_apply_live_label_update", + lambda *_args, **_kwargs: sequence.append("apply"), + ) + + total_success, total_skipped, backup_path, newly_added = ( + update_labels.update_project_labels_in_place( + target_dir, + source_dir, + min_iou=0.5, + ) + ) + + assert (total_success, total_skipped) == (3, 1) + assert backup_path == target_dir.resolve() / ".backup" / "update_labels_test.zip" + assert newly_added == ["Grooming"] + # Preflight must run before backup, seed before remap, remap before apply. + assert sequence.index("preflight") < sequence.index("backup") + assert sequence.index("seed") < sequence.index("remap") + assert sequence.index("remap") < sequence.index("apply") + + +def test_update_project_labels_in_place_rejects_identical_dirs(tmp_path): + """The orchestrator should refuse to operate when source and target are the same path.""" + project_dir = tmp_path / "project" + project_dir.mkdir() + + with pytest.raises(ValueError, match="must be different"): + update_labels.update_project_labels_in_place(project_dir, project_dir, min_iou=0.5) From 1f21aeb2bf877d510bf42faa0f155d5222c2e0ef Mon Sep 17 00:00:00 2001 From: Keith Sheppard Date: Thu, 21 May 2026 10:57:47 -0400 Subject: [PATCH 2/5] update-labels: address some feature branch issues Thread preflighted videos and failure-tag/description overrides through the shared remap helper so the source's full video list no longer leaks into update-labels iteration. Reorder preflight to validate source before scaffolding target. Document update-labels in cli-tools.md with cross-links to/from update-pose, and surface both subcommands in the in-app user guide tree. --- docs/user-guide/cli-tools.md | 36 +++++++ .../resources/docs/user_guide/cli-tools.md | 36 +++++++ src/jabs/scripts/cli/update_labels.py | 25 +++-- src/jabs/scripts/cli/update_pose.py | 50 ++++++++-- src/jabs/ui/dialogs/user_guide_dialog.py | 6 +- tests/project/test_update_labels.py | 63 +++++++++++++ tests/project/test_update_pose.py | 94 +++++++++++++++++++ 7 files changed, 294 insertions(+), 16 deletions(-) diff --git a/docs/user-guide/cli-tools.md b/docs/user-guide/cli-tools.md index 453ca179..748d269c 100644 --- a/docs/user-guide/cli-tools.md +++ b/docs/user-guide/cli-tools.md @@ -135,6 +135,7 @@ Commands: sample-frames Sample PNG frames from a JABS project filtered by a behavior label. sample-pose-intervals Sample contiguous intervals from a batch of JABS pose and video files. update-pose Update a JABS project to use updated pose files while remapping labels. + update-labels Replace a JABS project's labels with labels imported from another project. ``` For full documentation of the `convert-to-nwb` command, including output modes, subjects @@ -208,6 +209,41 @@ After a successful live pose update, features are regenerated automatically only jabs-cli update-pose /path/to/project /path/to/updated_pose_dir --min-iou-thresh 0.5 ``` +If instead you want to import labels from another JABS project while keeping the target's pose untouched, see [`jabs-cli update-labels`](#jabs-cli-update-labels). + +## jabs-cli update-labels + +The `jabs-cli update-labels` command is the inverse of [`update-pose`](#jabs-cli-update-pose): instead of keeping the target's labels and replacing its pose, it keeps the target's pose and replaces its labels with labels imported from another JABS project. The source project provides both the labels and the pose used for IoU-based identity matching. + +**Usage:** + +```bash +jabs-cli update-labels [--min-iou-thresh ] [--verbose] [--annotate-failures] [--drop-timeline-annotations] +``` + +- ``: Path to the target JABS project whose labels will be replaced in place. The target's pose is unchanged. If `` is a directory of videos + pose files with no `jabs/` subdirectory, a minimal JABS project is scaffolded automatically — features are not generated, so you may want to run `jabs-init` separately afterwards. +- ``: Path to a JABS project providing the replacement labels and the pose used for IoU matching. Must already be a valid JABS project; every source-labeled video must also exist in the target. +- `--min-iou-thresh `: Minimum acceptable median IoU for a label remap match. Blocks below this threshold are skipped. Default: `0.5`. +- `--verbose`: Print successful label remap assignments in addition to warnings. +- `--annotate-failures`: Add timeline annotations to the target for blocks whose label remap fails. Failure tags use the `update-labels-` prefix to distinguish them from `update-pose` failures. +- `--drop-timeline-annotations`: Discard source timeline annotations instead of copying or remapping them. + +Before modifying the project, the command validates both inputs, runs the label remap in disposable staging projects, and creates a timestamped backup zip under `/.backup` covering `jabs/project.json`, annotations, and predictions (pose files are not touched). Labels are processed block by block, matched by median bbox IoU between the source pose and the target's existing pose, and written to the staged destination label track. By default, source timeline annotations are also carried forward and remapped the same way as label blocks; use `--drop-timeline-annotations` to discard them. + +Existing target labels for videos that the source does not cover are left untouched (per-video replace). Behaviors named in the source's `project.json` but not present in the target are merged into the target's `project.json` so the imported labels are usable in the GUI; behaviors already configured in the target keep their existing settings. + +The target's pose is unchanged, so the feature cache stays valid and is **not** regenerated. Predictions are cleared because they are stale relative to the new labels; classifiers, the performance cache, and feature files are all left in place. If you need to retrain after a label import, run training from the GUI or via `jabs-classify`. + +If a failure occurs after the live apply begins, the command prints the backup path plus cleanup and manual restore instructions instead of restoring automatically. + +**Example:** + +```bash +jabs-cli update-labels /path/to/target_project /path/to/source_project --min-iou-thresh 0.5 +``` + +If instead you want to replace the target's pose while keeping its labels, see [`jabs-cli update-pose`](#jabs-cli-update-pose). + ## jabs-cli sample-frames The `jabs-cli sample-frames` command extracts individual video frames as PNG images diff --git a/src/jabs/resources/docs/user_guide/cli-tools.md b/src/jabs/resources/docs/user_guide/cli-tools.md index 453ca179..748d269c 100644 --- a/src/jabs/resources/docs/user_guide/cli-tools.md +++ b/src/jabs/resources/docs/user_guide/cli-tools.md @@ -135,6 +135,7 @@ Commands: sample-frames Sample PNG frames from a JABS project filtered by a behavior label. sample-pose-intervals Sample contiguous intervals from a batch of JABS pose and video files. update-pose Update a JABS project to use updated pose files while remapping labels. + update-labels Replace a JABS project's labels with labels imported from another project. ``` For full documentation of the `convert-to-nwb` command, including output modes, subjects @@ -208,6 +209,41 @@ After a successful live pose update, features are regenerated automatically only jabs-cli update-pose /path/to/project /path/to/updated_pose_dir --min-iou-thresh 0.5 ``` +If instead you want to import labels from another JABS project while keeping the target's pose untouched, see [`jabs-cli update-labels`](#jabs-cli-update-labels). + +## jabs-cli update-labels + +The `jabs-cli update-labels` command is the inverse of [`update-pose`](#jabs-cli-update-pose): instead of keeping the target's labels and replacing its pose, it keeps the target's pose and replaces its labels with labels imported from another JABS project. The source project provides both the labels and the pose used for IoU-based identity matching. + +**Usage:** + +```bash +jabs-cli update-labels [--min-iou-thresh ] [--verbose] [--annotate-failures] [--drop-timeline-annotations] +``` + +- ``: Path to the target JABS project whose labels will be replaced in place. The target's pose is unchanged. If `` is a directory of videos + pose files with no `jabs/` subdirectory, a minimal JABS project is scaffolded automatically — features are not generated, so you may want to run `jabs-init` separately afterwards. +- ``: Path to a JABS project providing the replacement labels and the pose used for IoU matching. Must already be a valid JABS project; every source-labeled video must also exist in the target. +- `--min-iou-thresh `: Minimum acceptable median IoU for a label remap match. Blocks below this threshold are skipped. Default: `0.5`. +- `--verbose`: Print successful label remap assignments in addition to warnings. +- `--annotate-failures`: Add timeline annotations to the target for blocks whose label remap fails. Failure tags use the `update-labels-` prefix to distinguish them from `update-pose` failures. +- `--drop-timeline-annotations`: Discard source timeline annotations instead of copying or remapping them. + +Before modifying the project, the command validates both inputs, runs the label remap in disposable staging projects, and creates a timestamped backup zip under `/.backup` covering `jabs/project.json`, annotations, and predictions (pose files are not touched). Labels are processed block by block, matched by median bbox IoU between the source pose and the target's existing pose, and written to the staged destination label track. By default, source timeline annotations are also carried forward and remapped the same way as label blocks; use `--drop-timeline-annotations` to discard them. + +Existing target labels for videos that the source does not cover are left untouched (per-video replace). Behaviors named in the source's `project.json` but not present in the target are merged into the target's `project.json` so the imported labels are usable in the GUI; behaviors already configured in the target keep their existing settings. + +The target's pose is unchanged, so the feature cache stays valid and is **not** regenerated. Predictions are cleared because they are stale relative to the new labels; classifiers, the performance cache, and feature files are all left in place. If you need to retrain after a label import, run training from the GUI or via `jabs-classify`. + +If a failure occurs after the live apply begins, the command prints the backup path plus cleanup and manual restore instructions instead of restoring automatically. + +**Example:** + +```bash +jabs-cli update-labels /path/to/target_project /path/to/source_project --min-iou-thresh 0.5 +``` + +If instead you want to replace the target's pose while keeping its labels, see [`jabs-cli update-pose`](#jabs-cli-update-pose). + ## jabs-cli sample-frames The `jabs-cli sample-frames` command extracts individual video frames as PNG images diff --git a/src/jabs/scripts/cli/update_labels.py b/src/jabs/scripts/cli/update_labels.py index cc312ef8..5cdc944b 100644 --- a/src/jabs/scripts/cli/update_labels.py +++ b/src/jabs/scripts/cli/update_labels.py @@ -161,12 +161,15 @@ def _preflight_label_update_inputs( target_dir: Path, source_project_dir: Path, ) -> tuple[list[str], set[str]]: - """Validate target and source inputs before any live mutation occurs. + """Validate target and source inputs, scaffolding the target if needed. - If ``target_dir`` is a directory of videos + pose files without a ``jabs/`` - directory, a minimal JABS project is scaffolded automatically (no features - are computed). The source must already be a valid JABS project with - annotations. + The source is validated as a JABS project first, so an invalid source path + cannot leave a partially initialized target on disk. Only after the source + looks valid is ``target_dir`` either accepted (if it is already a JABS + project) or scaffolded (if it is a videos + pose directory with no + ``jabs/``). Scaffolding only creates an empty ``jabs/`` skeleton — no labels, + features, or pose changes — so on any subsequent failure the user is left + with a harmless no-op directory that the next run reuses. Returns: Tuple ``(videos, live_annotation_videos)`` where ``videos`` is the sorted @@ -174,15 +177,16 @@ def _preflight_label_update_inputs( target project) and ``live_annotation_videos`` is the subset of those videos that already have an annotation file in the live target. """ - _scaffold_target_project_if_missing(target_dir) - - if not Project.is_valid_project_directory(target_dir): - raise ValueError(f"{target_dir} is not a valid JABS project directory") if not Project.is_valid_project_directory(source_project_dir): raise ValueError( f"{source_project_dir} is not a valid JABS project directory (source labels)" ) + _scaffold_target_project_if_missing(target_dir) + + if not Project.is_valid_project_directory(target_dir): + raise ValueError(f"{target_dir} is not a valid JABS project directory") + target_videos = set(_project_videos(target_dir)) if not target_videos: raise ValueError(f"No video files found in {target_dir}") @@ -435,6 +439,9 @@ def update_project_labels_in_place( verbose, annotate_failures, drop_timeline_annotations, + videos=labeled_videos, + failure_tag_prefix="update-labels", + failure_description_phrase="label update", ) _apply_live_label_update( diff --git a/src/jabs/scripts/cli/update_pose.py b/src/jabs/scripts/cli/update_pose.py index a2108813..493793ef 100644 --- a/src/jabs/scripts/cli/update_pose.py +++ b/src/jabs/scripts/cli/update_pose.py @@ -250,6 +250,9 @@ def _remap_labels_for_video( verbose: bool = False, annotate_failures: bool = False, drop_timeline_annotations: bool = False, + *, + failure_tag_prefix: str = "update-pose", + failure_description_phrase: str = "pose update", ): """Remap labels for a single video. @@ -269,6 +272,12 @@ def _remap_labels_for_video( annotate_failures: Whether to add timeline annotations for failed block matches. drop_timeline_annotations: Whether to discard existing source timeline annotations instead of copying or remapping them. + failure_tag_prefix: Tag prefix used for failure annotations written when + ``annotate_failures`` is set. Defaults to ``"update-pose"`` for backward + compatibility; ``update-labels`` passes ``"update-labels"``. + failure_description_phrase: Human-readable phrase used in the failure + annotation description (e.g. ``"pose update"`` → "label remap failed + during pose update: ..."). Returns: Tuple of ``(success_count, skipped_count)``. @@ -391,9 +400,9 @@ def _remap_labels_for_video( skipped_count += 1 tag = ( - "update-pose-behavior-remap-failed" + f"{failure_tag_prefix}-behavior-remap-failed" if present - else "update-pose-not-behavior-remap-failed" + else f"{failure_tag_prefix}-not-behavior-remap-failed" ) if annotate_failures and not dest_labels.timeline_annotations.annotation_exists( start=start, end=end, tag=tag, identity_index=None @@ -405,8 +414,8 @@ def _remap_labels_for_video( tag=tag, color="#FF8800" if present else "#8888FF", description=( - f"label remap failed during pose update: behavior={behavior}, " - f"present={present}, " + f"label remap failed during {failure_description_phrase}: " + f"behavior={behavior}, present={present}, " f"src_id={src_identity}, best_iou={iou:.2f}" ), identity_index=None, @@ -940,12 +949,38 @@ def _run_staged_label_remap( verbose: bool, annotate_failures: bool, drop_timeline_annotations: bool, + *, + videos: list[str] | None = None, + failure_tag_prefix: str = "update-pose", + failure_description_phrase: str = "pose update", ) -> tuple[int, int]: - """Run the existing per-video label-remap semantics from source to destination.""" + """Run the existing per-video label-remap semantics from source to destination. + + Args: + label_source_project: Source-side staged project providing labels and source pose. + label_dest_project: Destination-side staged project receiving the remapped labels. + min_iou: Minimum acceptable median IoU for a block match. + verbose: Whether to print successful block matches. + annotate_failures: Whether to write timeline annotations for failed block matches. + drop_timeline_annotations: Whether to discard source timeline annotations + instead of copying or remapping them. + videos: Optional explicit list of video filenames to process. When ``None`` + (the default), iterate ``label_source_project.video_manager.videos``. + Callers should pass the preflighted set of videos to ensure the source + and destination projects can both load pose for every video processed. + failure_tag_prefix: Tag prefix used when ``annotate_failures`` writes a + timeline annotation for a skipped block. The resulting tag is + ``f"{prefix}-behavior-remap-failed"`` (or ``-not-behavior-remap-failed``). + failure_description_phrase: Human-readable phrase used in the failure + annotation description (e.g. ``"pose update"`` → "label remap failed + during pose update: ..."). + """ total_success = 0 total_skipped = 0 - for video in label_source_project.video_manager.videos: + iteration_videos = videos if videos is not None else label_source_project.video_manager.videos + + for video in iteration_videos: success, skipped = _remap_labels_for_video( video, label_source_project, @@ -954,6 +989,8 @@ def _run_staged_label_remap( verbose=verbose, annotate_failures=annotate_failures, drop_timeline_annotations=drop_timeline_annotations, + failure_tag_prefix=failure_tag_prefix, + failure_description_phrase=failure_description_phrase, ) total_success += success total_skipped += skipped @@ -1027,6 +1064,7 @@ def update_project_pose_in_place( verbose, annotate_failures, drop_timeline_annotations, + videos=videos, ) _refresh_project_identity_counts(label_dest_project) _apply_live_update( diff --git a/src/jabs/ui/dialogs/user_guide_dialog.py b/src/jabs/ui/dialogs/user_guide_dialog.py index 7a12a008..00e1d8fb 100644 --- a/src/jabs/ui/dialogs/user_guide_dialog.py +++ b/src/jabs/ui/dialogs/user_guide_dialog.py @@ -196,7 +196,11 @@ def _build_tree(self) -> None: "_file": "cli-tools.md", "jabs-classify": "cli-tools.md#jabs-classify", "jabs-features": "cli-tools.md#jabs-features", - "jabs-cli": "cli-tools.md#jabs-cli", + "jabs-cli": { + "_file": "cli-tools.md#jabs-cli", + "update-pose": "cli-tools.md#jabs-cli-update-pose", + "update-labels": "cli-tools.md#jabs-cli-update-labels", + }, "jabs-init": "cli-tools.md#jabs-init", }, "File Formats": { diff --git a/tests/project/test_update_labels.py b/tests/project/test_update_labels.py index c2b940a7..34e3520a 100644 --- a/tests/project/test_update_labels.py +++ b/tests/project/test_update_labels.py @@ -215,6 +215,22 @@ def test_preflight_rejects_invalid_source(tmp_path): update_labels._preflight_label_update_inputs(target_dir, source_dir) +def test_preflight_invalid_source_does_not_scaffold_target(tmp_path): + """Source validation must happen before target scaffolding to avoid orphan jabs/.""" + target_dir = tmp_path / "target" + source_dir = tmp_path / "source" + target_dir.mkdir() + (target_dir / "video1.avi").touch() + (target_dir / "video1_pose_est_v8.h5").touch() + source_dir.mkdir() # not a valid JABS project — no jabs/project.json + + with pytest.raises(ValueError, match="source labels"): + update_labels._preflight_label_update_inputs(target_dir, source_dir) + + # target should be untouched: no jabs/ scaffolded + assert not (target_dir / "jabs").exists() + + def test_preflight_rejects_source_video_missing_in_target(tmp_path, monkeypatch): """The preflight should fail when the source has labels for a video not in the target.""" target_dir = tmp_path / "target" @@ -628,6 +644,53 @@ def fake_create_backup_archive(project_dir_arg, videos_arg, *, include_pose_file assert sequence.index("remap") < sequence.index("apply") +def test_update_project_labels_in_place_passes_labeled_videos_and_tag_overrides( + tmp_path, monkeypatch +): + """The orchestrator should thread labeled_videos and the update-labels tag overrides.""" + target_dir = tmp_path / "project" + source_dir = tmp_path / "source" + target_dir.mkdir() + source_dir.mkdir() + + captured_remap: dict[str, object] = {} + + monkeypatch.setattr( + update_labels, + "_preflight_label_update_inputs", + lambda *_args: (["video1.avi"], set()), + ) + monkeypatch.setattr( + update_labels, + "_create_backup_archive", + lambda *_args, **_kwargs: target_dir / ".backup" / "update_labels_test.zip", + ) + monkeypatch.setattr(update_labels, "_seed_stage_project", lambda *_args, **_kwargs: None) + monkeypatch.setattr( + update_labels, + "_merge_source_behaviors_into_staged_project", + lambda *_args, **_kwargs: [], + ) + monkeypatch.setattr( + update_labels, + "Project", + lambda *args, **kwargs: SimpleNamespace(project_paths=SimpleNamespace()), + ) + monkeypatch.setattr(update_labels, "_apply_live_label_update", lambda *_args, **_kwargs: None) + + def fake_run_staged_label_remap(*_args, **kwargs): + captured_remap.update(kwargs) + return (1, 0) + + monkeypatch.setattr(update_labels, "_run_staged_label_remap", fake_run_staged_label_remap) + + update_labels.update_project_labels_in_place(target_dir, source_dir, min_iou=0.5) + + assert captured_remap["videos"] == ["video1.avi"] + assert captured_remap["failure_tag_prefix"] == "update-labels" + assert captured_remap["failure_description_phrase"] == "label update" + + def test_update_project_labels_in_place_rejects_identical_dirs(tmp_path): """The orchestrator should refuse to operate when source and target are the same path.""" project_dir = tmp_path / "project" diff --git a/tests/project/test_update_pose.py b/tests/project/test_update_pose.py index 66a9e93a..d906e698 100644 --- a/tests/project/test_update_pose.py +++ b/tests/project/test_update_pose.py @@ -768,6 +768,100 @@ def test_remap_labels_for_video_drops_source_timeline_annotations(): assert saved_labels.timeline_annotations.serialize() == [] +def test_run_staged_label_remap_uses_explicit_videos_list_when_provided(monkeypatch): + """When ``videos`` is passed, ``_run_staged_label_remap`` should only iterate that list.""" + source_videos = ["a.avi", "b.avi"] + captured = [] + + def fake_remap(video, *_args, **_kwargs): + captured.append(video) + return (1, 0) + + monkeypatch.setattr(update_pose, "_remap_labels_for_video", fake_remap) + + label_source_project = MagicMock() + label_source_project.video_manager.videos = source_videos + label_dest_project = MagicMock() + + success, skipped = update_pose._run_staged_label_remap( + label_source_project, + label_dest_project, + min_iou=0.5, + verbose=False, + annotate_failures=False, + drop_timeline_annotations=False, + videos=["a.avi"], + ) + + assert captured == ["a.avi"] + assert (success, skipped) == (1, 0) + + +def test_run_staged_label_remap_falls_back_to_source_videos(monkeypatch): + """When ``videos`` is None, iteration should default to ``label_source_project``'s videos.""" + captured = [] + + def fake_remap(video, *_args, **_kwargs): + captured.append(video) + return (1, 0) + + monkeypatch.setattr(update_pose, "_remap_labels_for_video", fake_remap) + + label_source_project = MagicMock() + label_source_project.video_manager.videos = ["a.avi", "b.avi"] + label_dest_project = MagicMock() + + update_pose._run_staged_label_remap( + label_source_project, + label_dest_project, + min_iou=0.5, + verbose=False, + annotate_failures=False, + drop_timeline_annotations=False, + ) + + assert captured == ["a.avi", "b.avi"] + + +def test_remap_labels_for_video_failure_uses_custom_tag_prefix(): + """Failure annotations should use the configured tag prefix and description phrase.""" + src_boxes = np.array([[[0.0, 0.0], [10.0, 10.0]]] * 10) + dst_boxes = np.array([[[50.0, 50.0], [60.0, 60.0]]] * 10) + + source_pose = _make_pose([0], {0: src_boxes}) + dest_pose = _make_pose([1], {1: dst_boxes}) + + source_labels = VideoLabels("video1.avi", 10) + track_labels = source_labels.get_track_labels("0", "Grooming") + track_labels.label_behavior(3, 4) + + label_source_project = MagicMock() + label_source_project.video_manager.video_path.return_value = Path("video1.avi") + label_source_project.video_manager.load_video_labels.return_value = source_labels + label_source_project.load_pose_est.return_value = source_pose + + label_dest_project = MagicMock() + label_dest_project.video_manager.video_path.return_value = Path("video1.avi") + label_dest_project.load_pose_est.return_value = dest_pose + label_dest_project.project_paths.annotations_dir = Path("/tmp/stage-annotations") + + update_pose._remap_labels_for_video( + "video1.avi", + label_source_project, + label_dest_project, + min_iou=0.5, + annotate_failures=True, + failure_tag_prefix="update-labels", + failure_description_phrase="label update", + ) + + saved_labels = label_dest_project.save_annotations.call_args[0][0] + annotations = saved_labels.timeline_annotations.serialize() + assert len(annotations) == 1 + assert annotations[0]["tag"] == "update-labels-behavior-remap-failed" + assert "label remap failed during label update" in annotations[0]["description"] + + def test_apply_live_update_replaces_pose_set_and_clears_derived_files(tmp_path): """Applying a staged pose update should replace annotations/project metadata and swap the pose set.""" project_dir = tmp_path / "project" From 6e3ddf9b2b15ab497c1e8e167f6eeb1d29744951 Mon Sep 17 00:00:00 2001 From: Keith Sheppard Date: Thu, 21 May 2026 11:05:46 -0400 Subject: [PATCH 3/5] shrink failure-annotation tags below MAX_TAG_LEN; drop tag-prefix param --- docs/user-guide/cli-tools.md | 2 +- .../resources/docs/user_guide/cli-tools.md | 2 +- src/jabs/scripts/cli/update_labels.py | 1 - src/jabs/scripts/cli/update_pose.py | 21 +++++-------------- tests/project/test_update_labels.py | 5 ++--- tests/project/test_update_pose.py | 15 +++++++++---- 6 files changed, 20 insertions(+), 26 deletions(-) diff --git a/docs/user-guide/cli-tools.md b/docs/user-guide/cli-tools.md index 748d269c..6799bb6f 100644 --- a/docs/user-guide/cli-tools.md +++ b/docs/user-guide/cli-tools.md @@ -225,7 +225,7 @@ jabs-cli update-labels [--min-iou-thresh `: Path to a JABS project providing the replacement labels and the pose used for IoU matching. Must already be a valid JABS project; every source-labeled video must also exist in the target. - `--min-iou-thresh `: Minimum acceptable median IoU for a label remap match. Blocks below this threshold are skipped. Default: `0.5`. - `--verbose`: Print successful label remap assignments in addition to warnings. -- `--annotate-failures`: Add timeline annotations to the target for blocks whose label remap fails. Failure tags use the `update-labels-` prefix to distinguish them from `update-pose` failures. +- `--annotate-failures`: Add timeline annotations to the target for blocks whose label remap fails. Annotations use the same `behavior-remap-failed` / `not-behavior-remap-failed` tags as `update-pose`; the description text distinguishes the originating operation. - `--drop-timeline-annotations`: Discard source timeline annotations instead of copying or remapping them. Before modifying the project, the command validates both inputs, runs the label remap in disposable staging projects, and creates a timestamped backup zip under `/.backup` covering `jabs/project.json`, annotations, and predictions (pose files are not touched). Labels are processed block by block, matched by median bbox IoU between the source pose and the target's existing pose, and written to the staged destination label track. By default, source timeline annotations are also carried forward and remapped the same way as label blocks; use `--drop-timeline-annotations` to discard them. diff --git a/src/jabs/resources/docs/user_guide/cli-tools.md b/src/jabs/resources/docs/user_guide/cli-tools.md index 748d269c..6799bb6f 100644 --- a/src/jabs/resources/docs/user_guide/cli-tools.md +++ b/src/jabs/resources/docs/user_guide/cli-tools.md @@ -225,7 +225,7 @@ jabs-cli update-labels [--min-iou-thresh `: Path to a JABS project providing the replacement labels and the pose used for IoU matching. Must already be a valid JABS project; every source-labeled video must also exist in the target. - `--min-iou-thresh `: Minimum acceptable median IoU for a label remap match. Blocks below this threshold are skipped. Default: `0.5`. - `--verbose`: Print successful label remap assignments in addition to warnings. -- `--annotate-failures`: Add timeline annotations to the target for blocks whose label remap fails. Failure tags use the `update-labels-` prefix to distinguish them from `update-pose` failures. +- `--annotate-failures`: Add timeline annotations to the target for blocks whose label remap fails. Annotations use the same `behavior-remap-failed` / `not-behavior-remap-failed` tags as `update-pose`; the description text distinguishes the originating operation. - `--drop-timeline-annotations`: Discard source timeline annotations instead of copying or remapping them. Before modifying the project, the command validates both inputs, runs the label remap in disposable staging projects, and creates a timestamped backup zip under `/.backup` covering `jabs/project.json`, annotations, and predictions (pose files are not touched). Labels are processed block by block, matched by median bbox IoU between the source pose and the target's existing pose, and written to the staged destination label track. By default, source timeline annotations are also carried forward and remapped the same way as label blocks; use `--drop-timeline-annotations` to discard them. diff --git a/src/jabs/scripts/cli/update_labels.py b/src/jabs/scripts/cli/update_labels.py index 5cdc944b..79f30fc8 100644 --- a/src/jabs/scripts/cli/update_labels.py +++ b/src/jabs/scripts/cli/update_labels.py @@ -440,7 +440,6 @@ def update_project_labels_in_place( annotate_failures, drop_timeline_annotations, videos=labeled_videos, - failure_tag_prefix="update-labels", failure_description_phrase="label update", ) diff --git a/src/jabs/scripts/cli/update_pose.py b/src/jabs/scripts/cli/update_pose.py index 493793ef..a348bfa4 100644 --- a/src/jabs/scripts/cli/update_pose.py +++ b/src/jabs/scripts/cli/update_pose.py @@ -251,7 +251,6 @@ def _remap_labels_for_video( annotate_failures: bool = False, drop_timeline_annotations: bool = False, *, - failure_tag_prefix: str = "update-pose", failure_description_phrase: str = "pose update", ): """Remap labels for a single video. @@ -272,12 +271,10 @@ def _remap_labels_for_video( annotate_failures: Whether to add timeline annotations for failed block matches. drop_timeline_annotations: Whether to discard existing source timeline annotations instead of copying or remapping them. - failure_tag_prefix: Tag prefix used for failure annotations written when - ``annotate_failures`` is set. Defaults to ``"update-pose"`` for backward - compatibility; ``update-labels`` passes ``"update-labels"``. failure_description_phrase: Human-readable phrase used in the failure annotation description (e.g. ``"pose update"`` → "label remap failed - during pose update: ..."). + during pose update: ..."). Both ``update-pose`` and ``update-labels`` + share the same tag names; this only affects the description text. Returns: Tuple of ``(success_count, skipped_count)``. @@ -399,11 +396,7 @@ def _remap_labels_for_video( ) skipped_count += 1 - tag = ( - f"{failure_tag_prefix}-behavior-remap-failed" - if present - else f"{failure_tag_prefix}-not-behavior-remap-failed" - ) + tag = "behavior-remap-failed" if present else "not-behavior-remap-failed" if annotate_failures and not dest_labels.timeline_annotations.annotation_exists( start=start, end=end, tag=tag, identity_index=None ): @@ -951,7 +944,6 @@ def _run_staged_label_remap( drop_timeline_annotations: bool, *, videos: list[str] | None = None, - failure_tag_prefix: str = "update-pose", failure_description_phrase: str = "pose update", ) -> tuple[int, int]: """Run the existing per-video label-remap semantics from source to destination. @@ -968,12 +960,10 @@ def _run_staged_label_remap( (the default), iterate ``label_source_project.video_manager.videos``. Callers should pass the preflighted set of videos to ensure the source and destination projects can both load pose for every video processed. - failure_tag_prefix: Tag prefix used when ``annotate_failures`` writes a - timeline annotation for a skipped block. The resulting tag is - ``f"{prefix}-behavior-remap-failed"`` (or ``-not-behavior-remap-failed``). failure_description_phrase: Human-readable phrase used in the failure annotation description (e.g. ``"pose update"`` → "label remap failed - during pose update: ..."). + during pose update: ..."). Both ``update-pose`` and ``update-labels`` + share the same tag names; this only affects the description text. """ total_success = 0 total_skipped = 0 @@ -989,7 +979,6 @@ def _run_staged_label_remap( verbose=verbose, annotate_failures=annotate_failures, drop_timeline_annotations=drop_timeline_annotations, - failure_tag_prefix=failure_tag_prefix, failure_description_phrase=failure_description_phrase, ) total_success += success diff --git a/tests/project/test_update_labels.py b/tests/project/test_update_labels.py index 34e3520a..54712bc5 100644 --- a/tests/project/test_update_labels.py +++ b/tests/project/test_update_labels.py @@ -644,10 +644,10 @@ def fake_create_backup_archive(project_dir_arg, videos_arg, *, include_pose_file assert sequence.index("remap") < sequence.index("apply") -def test_update_project_labels_in_place_passes_labeled_videos_and_tag_overrides( +def test_update_project_labels_in_place_passes_labeled_videos_and_description_phrase( tmp_path, monkeypatch ): - """The orchestrator should thread labeled_videos and the update-labels tag overrides.""" + """The orchestrator should thread labeled_videos and the update-labels description phrase.""" target_dir = tmp_path / "project" source_dir = tmp_path / "source" target_dir.mkdir() @@ -687,7 +687,6 @@ def fake_run_staged_label_remap(*_args, **kwargs): update_labels.update_project_labels_in_place(target_dir, source_dir, min_iou=0.5) assert captured_remap["videos"] == ["video1.avi"] - assert captured_remap["failure_tag_prefix"] == "update-labels" assert captured_remap["failure_description_phrase"] == "label update" diff --git a/tests/project/test_update_pose.py b/tests/project/test_update_pose.py index d906e698..c62c7dbc 100644 --- a/tests/project/test_update_pose.py +++ b/tests/project/test_update_pose.py @@ -11,6 +11,7 @@ import jabs.scripts.cli.update_pose as update_pose from jabs.project import TimelineAnnotations, VideoLabels +from jabs.project.timeline_annotations import MAX_TAG_LEN from jabs.scripts.cli.cli import cli @@ -823,8 +824,14 @@ def fake_remap(video, *_args, **_kwargs): assert captured == ["a.avi", "b.avi"] -def test_remap_labels_for_video_failure_uses_custom_tag_prefix(): - """Failure annotations should use the configured tag prefix and description phrase.""" +def test_remap_labels_for_video_failure_tag_fits_within_max_tag_len(): + """Failure tags must fit within the timeline-annotation MAX_TAG_LEN limit. + + Without this constraint the failure annotation would be written to disk but + silently dropped on the next load (``TimelineAnnotations.load`` rejects tags + longer than ``MAX_TAG_LEN``). The description phrase customization should not + influence the tag itself. + """ src_boxes = np.array([[[0.0, 0.0], [10.0, 10.0]]] * 10) dst_boxes = np.array([[[50.0, 50.0], [60.0, 60.0]]] * 10) @@ -851,14 +858,14 @@ def test_remap_labels_for_video_failure_uses_custom_tag_prefix(): label_dest_project, min_iou=0.5, annotate_failures=True, - failure_tag_prefix="update-labels", failure_description_phrase="label update", ) saved_labels = label_dest_project.save_annotations.call_args[0][0] annotations = saved_labels.timeline_annotations.serialize() assert len(annotations) == 1 - assert annotations[0]["tag"] == "update-labels-behavior-remap-failed" + assert annotations[0]["tag"] == "behavior-remap-failed" + assert len(annotations[0]["tag"]) <= MAX_TAG_LEN assert "label remap failed during label update" in annotations[0]["description"] From 2796e0c24866edfbb88edd9ef462406ed53fc1ba Mon Sep 17 00:00:00 2001 From: Keith Sheppard Date: Thu, 28 May 2026 15:26:13 -0400 Subject: [PATCH 4/5] move copy_file_atomic from update_pose.py to jabs.core.utils The helper is reusable, self-contained, and not specific to either update-pose or update-labels. Promote it to jabs.core.utils with a fuller docstring explaining the same-filesystem rename guarantee and the cross-filesystem-source rationale, and update both subcommands to import it from there. Add unit tests covering replacement, parent-dir creation, no-leftover-tmp on success, mtime preservation, and extensionless destinations. --- .../jabs-core/src/jabs/core/utils/__init__.py | 2 + .../src/jabs/core/utils/utilities.py | 29 +++++++++ packages/jabs-core/tests/test_utilities.py | 63 ++++++++++++++++++- src/jabs/scripts/cli/update_labels.py | 6 +- src/jabs/scripts/cli/update_pose.py | 15 ++--- 5 files changed, 100 insertions(+), 15 deletions(-) diff --git a/packages/jabs-core/src/jabs/core/utils/__init__.py b/packages/jabs-core/src/jabs/core/utils/__init__.py index 297fc254..c1b2fbea 100644 --- a/packages/jabs-core/src/jabs/core/utils/__init__.py +++ b/packages/jabs-core/src/jabs/core/utils/__init__.py @@ -2,6 +2,7 @@ from .update_checker import check_for_update, is_pypi_install from .utilities import ( + copy_file_atomic, get_bool_env_var, hash_file, hide_stderr, @@ -11,6 +12,7 @@ __all__ = [ "check_for_update", + "copy_file_atomic", "get_bool_env_var", "hash_file", "hide_stderr", diff --git a/packages/jabs-core/src/jabs/core/utils/utilities.py b/packages/jabs-core/src/jabs/core/utils/utilities.py index bb64e994..0a9c4f16 100644 --- a/packages/jabs-core/src/jabs/core/utils/utilities.py +++ b/packages/jabs-core/src/jabs/core/utils/utilities.py @@ -1,6 +1,7 @@ import hashlib import os import re +import shutil import sys from collections.abc import Generator from contextlib import contextmanager @@ -86,6 +87,34 @@ def pose_file_stem(path: str | Path) -> str: return _POSE_SUFFIX_RE.sub("", Path(path).stem) +def copy_file_atomic(source: Path, destination: Path) -> None: + """Copy a file to ``destination`` so the replacement is atomic. + + The file is copied (via :func:`shutil.copy2`) into a sibling temporary file + in ``destination``'s parent directory, then renamed into place with + :meth:`pathlib.Path.replace`. ``destination``'s parent directory is created + if it does not exist. + + Because the temporary file is created in the same directory as + ``destination``, the final rename is on the same filesystem and is + therefore atomic on POSIX and Windows. ``source`` may live on a different + filesystem (e.g. a ``tempfile.TemporaryDirectory()`` on ``tmpfs``); the + intermediate copy is what makes cross-filesystem sources safe. + + Readers of ``destination`` never observe a partially written file: they + see either the previous contents or the new contents. + + Args: + source: Path to the file whose contents should be installed at + ``destination``. Metadata is preserved via :func:`shutil.copy2`. + destination: Final path. Any existing file at this path is replaced. + """ + destination.parent.mkdir(parents=True, exist_ok=True) + tmp_path = destination.with_suffix(destination.suffix + ".tmp") + shutil.copy2(source, tmp_path) + tmp_path.replace(destination) + + def to_safe_name(behavior: str) -> str: """Create a version of the given behavior name that is safe to use in filenames. diff --git a/packages/jabs-core/tests/test_utilities.py b/packages/jabs-core/tests/test_utilities.py index 8202493f..d96a6c88 100644 --- a/packages/jabs-core/tests/test_utilities.py +++ b/packages/jabs-core/tests/test_utilities.py @@ -1,10 +1,11 @@ """Unit tests for jabs.core.utils.utilities module.""" +import os from pathlib import Path import pytest -from jabs.core.utils import pose_file_stem +from jabs.core.utils import copy_file_atomic, pose_file_stem @pytest.mark.parametrize( @@ -50,3 +51,63 @@ def test_pose_file_stem_video_and_pose_match() -> None: video file (jabs-init, GUI). """ assert pose_file_stem("video.mp4") == pose_file_stem("video_pose_est_v6.h5") + + +def test_copy_file_atomic_replaces_existing_file(tmp_path: Path) -> None: + """The destination's contents should be overwritten by the source's contents.""" + source = tmp_path / "source.txt" + destination = tmp_path / "destination.txt" + source.write_text("new contents") + destination.write_text("old contents") + + copy_file_atomic(source, destination) + + assert destination.read_text() == "new contents" + + +def test_copy_file_atomic_creates_missing_parent_dirs(tmp_path: Path) -> None: + """Missing parent directories of the destination should be created.""" + source = tmp_path / "source.txt" + destination = tmp_path / "nested" / "dir" / "destination.txt" + source.write_text("payload") + + copy_file_atomic(source, destination) + + assert destination.read_text() == "payload" + + +def test_copy_file_atomic_does_not_leave_temp_file_on_success(tmp_path: Path) -> None: + """A successful copy must leave no ``.tmp`` sibling behind.""" + source = tmp_path / "source.txt" + destination = tmp_path / "destination.json" + source.write_text("payload") + + copy_file_atomic(source, destination) + + assert not (tmp_path / "destination.json.tmp").exists() + assert set(tmp_path.iterdir()) == {source, destination} + + +def test_copy_file_atomic_preserves_mtime(tmp_path: Path) -> None: + """``shutil.copy2`` semantics: file metadata such as mtime should be preserved.""" + source = tmp_path / "source.txt" + destination = tmp_path / "destination.txt" + source.write_text("payload") + original_mtime = 1_700_000_000.0 + os.utime(source, (original_mtime, original_mtime)) + + copy_file_atomic(source, destination) + + assert destination.stat().st_mtime == pytest.approx(original_mtime, abs=1.0) + + +def test_copy_file_atomic_handles_file_without_extension(tmp_path: Path) -> None: + """A destination with no suffix should still receive an atomic replacement.""" + source = tmp_path / "source" + destination = tmp_path / "destination" + source.write_text("payload") + + copy_file_atomic(source, destination) + + assert destination.read_text() == "payload" + assert not (tmp_path / "destination.tmp").exists() diff --git a/src/jabs/scripts/cli/update_labels.py b/src/jabs/scripts/cli/update_labels.py index 79f30fc8..5469efc0 100644 --- a/src/jabs/scripts/cli/update_labels.py +++ b/src/jabs/scripts/cli/update_labels.py @@ -48,11 +48,11 @@ import click +from jabs.core.utils import copy_file_atomic from jabs.pose_estimation import get_pose_path from jabs.project import Project from .update_pose import ( - _copy_file_atomic, _create_backup_archive, _print_manual_restore, _project_videos, @@ -340,12 +340,12 @@ def _apply_live_label_update( for video in videos: staged_annotation = staged_annotations_dir / Path(video).with_suffix(".json") if staged_annotation.exists(): - _copy_file_atomic( + copy_file_atomic( staged_annotation, live_annotations_dir / Path(video).with_suffix(".json"), ) - _copy_file_atomic( + copy_file_atomic( label_dest_project.project_paths.project_file, project_dir / "jabs" / "project.json", ) diff --git a/src/jabs/scripts/cli/update_pose.py b/src/jabs/scripts/cli/update_pose.py index a348bfa4..80d15915 100644 --- a/src/jabs/scripts/cli/update_pose.py +++ b/src/jabs/scripts/cli/update_pose.py @@ -50,6 +50,7 @@ import h5py import numpy as np +from jabs.core.utils import copy_file_atomic from jabs.pose_estimation import PoseEstimation, get_pose_path, open_pose_file from jabs.project import Project from jabs.project.timeline_annotations import TimelineAnnotations @@ -771,14 +772,6 @@ def _inject_consistent_pose_model_metadata(project: Project) -> dict[str, object return first_metadata -def _copy_file_atomic(source: Path, destination: Path) -> None: - """Copy a file into place via a temporary file and atomic replace.""" - destination.parent.mkdir(parents=True, exist_ok=True) - tmp_path = destination.with_suffix(destination.suffix + ".tmp") - shutil.copy2(source, tmp_path) - tmp_path.replace(destination) - - def _restore_cleanup_paths( project_dir: Path, videos: list[str], @@ -867,12 +860,12 @@ def _apply_live_update( for video in videos: staged_annotation = staged_annotations_dir / Path(video).with_suffix(".json") if staged_annotation.exists(): - _copy_file_atomic( + copy_file_atomic( staged_annotation, live_annotations_dir / Path(video).with_suffix(".json"), ) - _copy_file_atomic( + copy_file_atomic( label_dest_project.project_paths.project_file, project_dir / "jabs" / "project.json" ) @@ -882,7 +875,7 @@ def _apply_live_update( for video in videos: replacement_pose_path = replacement_pose_files[video] - _copy_file_atomic(replacement_pose_path, project_dir / replacement_pose_path.name) + copy_file_atomic(replacement_pose_path, project_dir / replacement_pose_path.name) shutil.rmtree(project_dir / "jabs" / "predictions", ignore_errors=True) shutil.rmtree(project_dir / "jabs" / "cache", ignore_errors=True) From 5de477a24535f48ecd3447c15149e21c6ee3add3 Mon Sep 17 00:00:00 2001 From: Keith Sheppard Date: Thu, 28 May 2026 15:33:27 -0400 Subject: [PATCH 5/5] let predictions/cache rmtree failures surface manual-restore path --- src/jabs/scripts/cli/update_labels.py | 4 ++- src/jabs/scripts/cli/update_pose.py | 6 ++-- tests/project/test_update_labels.py | 48 +++++++++++++++++++++++++-- tests/project/test_update_pose.py | 2 +- 4 files changed, 54 insertions(+), 6 deletions(-) diff --git a/src/jabs/scripts/cli/update_labels.py b/src/jabs/scripts/cli/update_labels.py index 5469efc0..6d93f81f 100644 --- a/src/jabs/scripts/cli/update_labels.py +++ b/src/jabs/scripts/cli/update_labels.py @@ -350,7 +350,9 @@ def _apply_live_label_update( project_dir / "jabs" / "project.json", ) - shutil.rmtree(project_dir / "jabs" / "predictions", ignore_errors=True) + predictions_dir = project_dir / "jabs" / "predictions" + if predictions_dir.exists(): + shutil.rmtree(predictions_dir) except Exception as exc: print( f"ERROR: Failed while applying the label update to the live project: {exc}", diff --git a/src/jabs/scripts/cli/update_pose.py b/src/jabs/scripts/cli/update_pose.py index 80d15915..478dfdb3 100644 --- a/src/jabs/scripts/cli/update_pose.py +++ b/src/jabs/scripts/cli/update_pose.py @@ -877,8 +877,10 @@ def _apply_live_update( replacement_pose_path = replacement_pose_files[video] copy_file_atomic(replacement_pose_path, project_dir / replacement_pose_path.name) - shutil.rmtree(project_dir / "jabs" / "predictions", ignore_errors=True) - shutil.rmtree(project_dir / "jabs" / "cache", ignore_errors=True) + for derived_dir_name in ("predictions", "cache"): + derived_dir = project_dir / "jabs" / derived_dir_name + if derived_dir.exists(): + shutil.rmtree(derived_dir) except Exception as exc: print( f"ERROR: Failed while applying the pose update to the live project: {exc}", diff --git a/tests/project/test_update_labels.py b/tests/project/test_update_labels.py index 54712bc5..1c8a2fdd 100644 --- a/tests/project/test_update_labels.py +++ b/tests/project/test_update_labels.py @@ -419,10 +419,19 @@ def test_apply_live_label_update_replaces_annotations_and_clears_predictions(tmp def test_apply_live_label_update_failure_prints_cleanup_instructions( tmp_path, monkeypatch, capsys ): - """A failed apply should print restore instructions and exit non-zero.""" + """A predictions-rmtree failure must propagate so the manual-restore path runs. + + Predictions are deliberately invalidated by ``update-labels`` because they were + trained against the previous labels. If their removal silently fails, the + project ends with new labels alongside stale predictions and the command + misleadingly reports success. + """ project_dir = tmp_path / "project" live_annotations_dir = project_dir / "jabs" / "annotations" + live_predictions_dir = project_dir / "jabs" / "predictions" live_annotations_dir.mkdir(parents=True) + live_predictions_dir.mkdir(parents=True) + (live_predictions_dir / "video1.h5").write_text("prediction") (project_dir / "jabs" / "project.json").write_text("live-project") stage_root = tmp_path / "stage" @@ -439,7 +448,7 @@ def test_apply_live_label_update_failure_prints_cleanup_instructions( ) ) - def fail_rmtree(_path, ignore_errors=False): + def fail_rmtree(_path): raise RuntimeError("boom") monkeypatch.setattr(update_labels.shutil, "rmtree", fail_rmtree) @@ -459,6 +468,41 @@ def fail_rmtree(_path, ignore_errors=False): assert "unzip -o .backup/update_labels_test.zip" in stderr +def test_apply_live_label_update_no_op_when_predictions_dir_absent(tmp_path, monkeypatch): + """A missing predictions directory must not be treated as an error.""" + project_dir = tmp_path / "project" + (project_dir / "jabs" / "annotations").mkdir(parents=True) + # Intentionally do not create jabs/predictions. + (project_dir / "jabs" / "project.json").write_text("live-project") + + stage_root = tmp_path / "stage" + staged_annotations_dir = stage_root / "jabs" / "annotations" + staged_annotations_dir.mkdir(parents=True) + (staged_annotations_dir / "video1.json").write_text("staged-annotation") + staged_project_file = stage_root / "jabs" / "project.json" + staged_project_file.write_text("staged-project") + + label_dest_project = SimpleNamespace( + project_paths=SimpleNamespace( + annotations_dir=staged_annotations_dir, + project_file=staged_project_file, + ) + ) + + def fail_rmtree(_path): # pragma: no cover - asserted not to be called + raise AssertionError("rmtree should not be called when predictions dir is absent") + + monkeypatch.setattr(update_labels.shutil, "rmtree", fail_rmtree) + + # Should complete normally; no rmtree attempted on a non-existent path. + update_labels._apply_live_label_update( + project_dir, + label_dest_project, + ["video1.avi"], + project_dir / ".backup" / "update_labels_test.zip", + ) + + def test_apply_live_label_update_raises_when_staged_annotation_missing(tmp_path): """Missing staged output for a labeled video should be a hard error before any writes.""" project_dir = tmp_path / "project" diff --git a/tests/project/test_update_pose.py b/tests/project/test_update_pose.py index c62c7dbc..19f2f76b 100644 --- a/tests/project/test_update_pose.py +++ b/tests/project/test_update_pose.py @@ -983,7 +983,7 @@ def test_apply_live_update_failure_prints_cleanup_and_restore_instructions( ) ) - def fail_rmtree(_path, ignore_errors=False): + def fail_rmtree(_path): raise RuntimeError("boom") monkeypatch.setattr(update_pose.shutil, "rmtree", fail_rmtree)