fix(detection): drop unmergeable per-slice metadata before InferenceSlicer merge - #2599
Pushpak731 wants to merge 2 commits into
Conversation
…licer merge InferenceSlicer crashed with 'Conflicting metadata for key: source_image' (or 'All metadata dictionaries must have the same keys to merge.') whenever the callback returned Detections whose metadata differed between slices. Detections.metadata is collection-level state and merge_metadata correctly requires identical values, but callbacks routinely attach per-call metadata - RF-DETR's predict() stores each slice's source image - so the tutorial code for tiled inference was guaranteed to crash. Before merging slice results, InferenceSlicer now keeps only metadata keys present in every non-empty slice result with equal values and drops the rest, emitting a SupervisionWarnings warning once per slicer instance that points per-slice information at Detections.data. The merge_metadata contract itself is unchanged; its per-key value rule was extracted into metadata_values_equal and reused so the two cannot drift. Fixes roboflow#2594
There was a problem hiding this comment.
🟡 Changes recommended
Fix the dropped-key union bug and update the changelog PR link and modification date.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Fixes InferenceSlicer crashes caused by inconsistent metadata across slice results.
Changes:
- Filters unmergeable metadata before merging.
- Shares metadata equality logic with merge operations.
- Adds regression tests and changelog documentation.
File summaries
| File | Summary |
|---|---|
tests/detection/tools/test_inference_slicer.py |
Adds metadata merge regression tests. |
src/supervision/detection/utils/internal.py |
Adds shared metadata comparison logic. |
src/supervision/detection/tools/inference_slicer.py |
Filters incompatible slice metadata. |
docs/changelog.md |
Documents the behavior change. |
Review details
Suppressed comments (1)
docs/changelog.md:10
- This changelog change is dated 2026-09-18, but the file's front matter still has
date_modified: 2026-09-17. Since the docs theme emits that field as JSON-LDdateModified(docs/theme/main.html:382-383), update the front matter so the published metadata reflects this change.
- `sv.InferenceSlicer` no longer raises `ValueError: Conflicting metadata for key: 'source_image'` (or `All metadata dictionaries must have the same keys to merge.`) when the callback returns `sv.Detections` whose `metadata` differs between slices — e.g. RF-DETR's `predict()`, which stores each call's source image as collection-level metadata, so the code from the Detect Small Objects tutorial crashed at the merge. Metadata keys equal across all non-empty slice results still merge as before; keys that differ between slices, or are missing from some slices, are now dropped from the tiled result with a `SupervisionWarnings` warning (emitted once per slicer instance) advising that per-slice information belongs in `Detections.data`. [#2594](https://github.com/roboflow/supervision/issues/2594)
- Files reviewed: 4/4 changed files
- Comments generated: 2
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Review follow-up on roboflow#2599. - The dropped-key set was seeded from the first non-empty slice result only, so a key carried by later slices alone stayed in their dictionaries and Detections.merge still raised 'All metadata dictionaries must have the same keys to merge.' Seed from the union of all non-empty metadata keys instead, with a regression test for both directions of the mismatch. - Adopt the source_image restoration pattern proposed by @vardhans07 in roboflow#2596: after the merge, a dropped 'source_image' key is restored to the full input image (when the slicer input is an ndarray), so detectors and annotators that follow the source-image metadata convention (e.g. RF-DETR) keep a parent image reference on the tiled result. The drop warning now mentions the restoration when it applies. - Changelog: link this PR per AGENTS.md and refresh date_modified.
@Pushpak731 ^^ 🙏 |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #2599 +/- ##
=======================================
Coverage 91% 91%
=======================================
Files 78 78
Lines 11382 11432 +50
=======================================
+ Hits 10381 10431 +50
Misses 1001 1001 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
🟡 Changes recommended
Sparse or identical slice results can retain a tile-sized source_image instead of the full input image.
Get a fresh assessment by requesting another Copilot review.
Review details
- Files reviewed: 4/4 changed files
- Comments generated: 2
- Review effort level: Balanced
| dropped_keys = self._drop_unmergeable_metadata(detections_list) | ||
| merged = Detections.merge(detections_list=detections_list) | ||
| restored_source_image = "source_image" in dropped_keys and isinstance( | ||
| image, np.ndarray | ||
| ) |
| def test_batch_path_drops_conflicting_metadata(self) -> None: | ||
| """The batch_size > 1 code path drops conflicting metadata as well.""" | ||
|
|
||
| def callback(tiles: list) -> list: |
Description
InferenceSlicercrashed withValueError: Conflicting metadata for key: 'source_image'(orAll metadata dictionaries must have the same keys to merge.) whenever the callback returnedsv.Detectionswhosemetadatadiffered between slices — which is what RF-DETR'spredict()does (it stores each call's source image as collection-level metadata), so the code from the Detect Small Objects tutorial crashed at the merge.Minimal repro (no model needed, fails on
develop):Note an all-zeros image does not trigger the crash —
np.array_equalthen sees identical slices — which is why plain synthetic tests never caught it; any real photo differs per slice.Motivation and Context
Closes #2594.
Detections.metadatais documented as collection-level state ("video name, camera parameters, timestamp"), andmerge_metadatacorrectly requires identical values across inputs — a contract locked in bytests/detection/utils/test_internal.py, left untouched here. The defect was inInferenceSlicer: it merges N independent callback invocations whose real-world metadata is call-local, and one dict cannot represent N different source images, so the crash was guaranteed by design.Changes Made
InferenceSlicernow funnels both merge call sites (single-slice andbatch_size > 1paths) through_merge_detections, which keeps only metadata keys present in every non-empty slice result with equal values and drops the rest (computed from the union of all slice keys — seeding from the first slice alone missed later-only keys and still crashed; fixed per review with regression tests for both mismatch directions).source_imagekey is restored to the full input image (when the slicer input is annp.ndarray), so detectors and annotators that follow the source-image metadata convention — e.g. RF-DETR — keep a parent image reference on the tiled result. Pattern adopted from @vardhans07's earlier PR fix: handle conflictingsource_imagemetadata inInferenceSlicer#2596; the drop warning mentions the restoration when it applies.merge_metadata's per-key value rule intometadata_values_equalindetection/utils/internal.pyand reused it in both places, so the merge contract and the slicer's compatibility check cannot drift.merge_metadata's behavior and error messages are byte-for-byte unchanged.merge_metadataitself, and any change to howDetections.mergetreats metadata — only the slicer reconciles, per its role of turning per-slice results into one tiled result.Testing
TestInferenceSlicerMetadata(8 tests) intests/detection/tools/test_inference_slicer.py: conflicting values dropped +source_imagerestored to the full image (regression for [Bug]: InferenceSlicer ValueError: Conflicting metadata for key: 'source_image': <class 'numpy.ndarray'>, <class 'numpy.ndarray'> #2594), identical metadata preserved without warning, mixed agreeing/conflicting keys, keys missing from some slices in both directions (first-slice-has / first-slice-lacks — the latter is the union-bug regression),source_imagerestored even when the first slice lacked it, warning emitted only once, and the batch path. The crash-regression tests fail ondevelopand pass with this change.pytest tests/detection/tools/test_inference_slicer.py tests/detection/utils/test_internal.py+ doctests of both changed modules → 143 passed before the review follow-up; 142 (slicer file grew to 8 metadata tests, suite totals re-verified) passing after.pytest tests/detection/→ 1781 passed, 1 skipped (GeoTIFF/rasterio not installed locally) before the follow-up; re-ran the slicer + internal files after it.Full suite
pytest→ 3902 passed, 20 skipped, 8 failed — all 8 areImportError: metrics extra is requiredfrom the optional pandas extra missing in my env; afterpip install pandas,pytest tests/metrics/→ 395 passed.ruff checkandruff formatclean on all touched files (pre-commit.ci also passed).mypy: same 10 pre-existing errors ondevelopand on this branch (verified by stashing the change) — all incrop_image/get_image_resolution_whTypeVar handling, none in the new code. Thenumpystubs in my local env also require running with--python-version 3.13vs the repo's pinned 3.10 config.Added/updated tests, and the full suite passes locally
Updated docs (docstrings / mkdocs entry) for new or changed public API
Added a changelog entry in
docs/changelog.mdunderUnreleased(skip for lint/type/format-only or pure doc changes)Additional Notes
video_name). Intersection-with-equal-values keeps global state intact and only removes what has no single merged representation, then restores the one key with an established ecosystem meaning.Detectionsobjects are never mutated:_run_callback/_run_callback_batchreturnmove_detections(...)copies whosemetadatadicts are fresh (selectshallow-copies the dict), so only those copies are modified.source_imagerestoration semantics come from @vardhans07's analysis in fix: handle conflictingsource_imagemetadata inInferenceSlicer#2596 / the issue thread. Happy to consolidate on whichever PR the maintainers prefer — this one or fix: handle conflictingsource_imagemetadata inInferenceSlicer#2596.