fix: reject orphan identity references in update-pose/update-labels#409
Open
keithshep wants to merge 2 commits into
Open
fix: reject orphan identity references in update-pose/update-labels#409keithshep wants to merge 2 commits into
keithshep wants to merge 2 commits into
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR prevents jabs-cli update-pose and jabs-cli update-labels from crashing mid-run when annotation JSONs reference identity indices that no longer exist in the corresponding pose files, by adding a preflight orphan-identity scan plus an optional --tolerate-orphan-identities warning-and-skip mode.
Changes:
- Add orphan-identity detection + consolidated reporting during preflight for both subcommands.
- Add
--tolerate-orphan-identitiesto allow continuing with warnings and skipping orphan(identity, behavior)pairs during remap. - Add/adjust unit tests and update user-guide docs to cover the new behavior and flag.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
src/jabs/scripts/cli/update_pose.py |
Implements orphan detection/reporting and tolerate-mode remap skipping; extends pose validation to return num_identities. |
src/jabs/scripts/cli/update_labels.py |
Wires the same orphan preflight behavior and CLI flag into label import/update. |
tests/project/test_update_pose.py |
Adds unit/regression tests for orphan detection/handling and the inline remap guard; updates stubs for num_identities. |
tests/project/test_update_labels.py |
Adds preflight tests for strict/tolerate orphan handling; updates stubs and CLI forwarding assertions. |
docs/user-guide/cli-tools.md |
Documents strict orphan preflight behavior and the new tolerate flag for both commands. |
src/jabs/resources/docs/user_guide/cli-tools.md |
Same documentation updates for the packaged user guide copy. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+526
to
+533
| label_key = "unfragmented_labels" if "unfragmented_labels" in data else "labels" | ||
| for identity_key in data.get(label_key, {}): | ||
| try: | ||
| identity_index = int(identity_key) | ||
| except (TypeError, ValueError): | ||
| continue | ||
| if identity_index >= num_identities: | ||
| orphans.add(identity_index) |
Comment on lines
+576
to
+582
| for video, num_identities, orphans in issues: | ||
| orphans_str = ", ".join(str(i) for i in orphans) | ||
| lines.append( | ||
| f" {video}: {source_label} pose has {num_identities} identities " | ||
| f"(valid indices 0 to {num_identities - 1}) but annotation references " | ||
| f"identity {orphans_str}" | ||
| ) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
When a JABS project's annotation file references an identity index that the corresponding pose file no longer supplies (typically because the pose was regenerated with fewer identities and stale label entries were never cleaned out),
jabs-cli update-poseandjabs-cli update-labelswould crash mid-batch with an unhandledValueErrorfrompose_est_v8.get_bounding_boxes, after partial work had already been staged.This PR makes both subcommands detect every affected
(video, identity)at preflight and raise a single descriptive error before any backup or mutation. The error message also points users at the new--tolerate-orphan-identitiesescape hatch, which converts the abort into a stderr warning and lets the remap loop skip the affected(identity, behavior)pairs instead of failing.What changed
Strict preflight (default behavior):
_orphan_identities_in_annotationhelper scans a JABS annotation JSON for identity keys (in bothlabels/unfragmented_labelsand identity-scoped timelineannotations) that exceed the corresponding pose'snum_identities._handle_orphan_identitieshelper raises a single compositeValueErrorlisting every offending(video, identity)across the whole run, with a clear remediation message._validate_pose_filenow returns(format_major_version, num_identities)so preflight can do the orphan scan in the same pass over the project._preflight_update_inputs(update-pose) and_preflight_label_update_inputs(update-labels) collect orphan issues across their respective source annotations and call the handler before any backup or staging.--tolerate-orphan-identitiesescape hatch:_handle_orphan_identitiesprints the same comprehensive listing to stderr as aWARNING:and returns without raising.(identity, behavior)guard inside_remap_labels_for_videowarns and skips orphan pairs as the remap loop encounters them, so the algorithm doesn't crash. With the flag off this guard is unreachable (preflight catches first); it's load-bearing only in tolerate mode.skipped_countin the final summary. No failure annotation is written for them, because the root cause is in the source project and target-side annotations give the user no leverage on the fix.User-visible behavior
Default (no flag): preflight aborts on orphan identities with a message like:
With
--tolerate-orphan-identities: preflight emits the same listing as a stderr WARNING and proceeds. Each orphan(identity, behavior)pair gets a one-line warning when the remap loop reaches it; their blocks land in the final skipped count.Tests
_orphan_identities_in_annotationcovering label-track keys, identity-scoped timeline annotations, clean files, missing files, and deduplication._handle_orphan_identitiescovering the strict raise (including that the message mentions the flag), the tolerate warning, and the empty-issues no-op._remap_labels_for_videoregression test for the inline orphan guard.fake_open_pose_file/_stub_pose_readertest helpers updated to exposenum_identitiesfor the new_validate_pose_filereturn type.Docs
Both copies of
cli-tools.md(docs/user-guide/andsrc/jabs/resources/docs/user_guide/) describe the new strict orphan-identity behavior and the--tolerate-orphan-identitiesflag in each subcommand section.