Validate parent_id alignment between images and predictions in Dynamic Crop - #2746
Open
rs-03 wants to merge 1 commit into
Open
Validate parent_id alignment between images and predictions in Dynamic Crop#2746rs-03 wants to merge 1 commit into
rs-03 wants to merge 1 commit into
Conversation
rs-03
requested review from
PawelPeczek-Roboflow,
dkosowski87,
grzegorz-roboflow,
hansent,
probicheaux,
rafel-roboflow and
yeldarby
as code owners
August 2, 2026 21:35
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.
What changed
DynamicCropBlockV1.runpairsimageswithpredictionspositionally viazip(images, predictions). When those two inputs are produced by independent workflow branches they can be misaligned, so a crop gets taken from one image using another image's detections, producing silently incorrect results (the "nonsense results" described in the issue).This adds a contract check in
crop_image: when the predictions carry aparent_id(as model-producedsv.Detectionsdo), it must match the image'sparent_metadata.parent_id. On a mismatch the block now raises a clearValueErrorexplaining the misalignment instead of returning wrong crops. The check mirrors the block's existingdetection_idcontract validation and is skipped when predictions carry noparent_id, so behavior is unchanged for correctly aligned inputs.Closes #390
Design note
I implemented this as validation with an explicit error, consistent with the block's existing
detection_idcontract check, because a silent realignment would hide a genuinely misconfigured workflow rather than surface it. If you would instead prefer the block to match predictions to images byparent_id, I am happy to switch to that approach.Tests
Added two unit tests in
tests/workflows/unit_tests/core_steps/transformations/test_crop.py:test_crop_image_raises_when_predictions_parent_id_does_not_match_image(fails on the pre-fix code, confirming the guard is effective)test_crop_image_accepts_predictions_whose_parent_id_matches_imageCommand run:
Result: 47 passed. Files were formatted with
black(26.3.1) andisort(5.13.2); both leave the changes unchanged.