Extract-compute-emit refactor for run() method of parsers#302
Open
dtronmans wants to merge 12 commits into
Open
Extract-compute-emit refactor for run() method of parsers#302dtronmans wants to merge 12 commits into
run() method of parsers#302dtronmans wants to merge 12 commits into
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe PR refactors parser flows into extract, compute, and emit steps, while adding shared utilities for classification, embeddings, regression, image, map, pose, lane, segmentation, detection, and YOLO postprocessing. ChangesParser postprocessing refactor
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 17
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
depthai_nodes/node/parsers/classification_sequence.py (1)
145-175: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winDuplicate layer-validation logic between
run()andextract().
run()(lines 154-161) still resolves/validatesoutput_layer_namefromoutput.getAllLayerNames()before callingextract(output), which repeats the exact same block (lines 168-175). Sincerun()already setsself.output_layer_name, the check insideextract()becomes redundant dead code. Compare with the sibling refactors insuperanimal_landmarker.pyandlane_detection.py, where this logic exists only insideextract().♻️ Proposed fix: remove the duplicated block from `run()`
def run(self): self._logger.debug("ClassificationSequenceParser run started") while self.isRunning(): try: output: dai.NNData = self.input.get() except dai.MessageQueue.QueueException: break - layers = output.getAllLayerNames() - self._logger.debug(f"Processing input with layers: {layers}") - if len(layers) == 1 and self.output_layer_name == "": - self.output_layer_name = layers[0] - elif len(layers) != 1 and self.output_layer_name == "": - raise ValueError( - f"Expected 1 output layer, got {len(layers)} layers. Please provide the output_layer_name." - ) - scores = self.extract(output) scores = self.compute(scores, is_softmax=self.is_softmax) self.emit(output, scores)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@depthai_nodes/node/parsers/classification_sequence.py` around lines 145 - 175, Remove the duplicated output-layer validation from ClassificationSequenceParser.run so it only delegates to extract and compute after reading the NNData; the layer-name resolution/ValueError logic should live in one place only. Update ClassificationSequenceParser.extract to keep the authoritative getAllLayerNames and output_layer_name handling, and ensure run no longer sets self.output_layer_name before calling extract.
🧹 Nitpick comments (3)
depthai_nodes/node/parsers/embeddings.py (1)
89-96: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDead self-assignments in
emit().
output.setSequenceNum(output.getSequenceNum()),setTimestamp,setTimestampDevice, and the transformation re-set all read a value offoutputand immediately write it back onto the same object — a no-op. UnlikeRegressionParser.emit(), which copies metadata fromoutputonto a newly built message, hereoutputis the same object returned unchanged bycompute(), so none of this has any effect and only adds confusing dead code.♻️ Suggested cleanup
def emit(self, output: dai.NNData) -> None: - output.setSequenceNum(output.getSequenceNum()) - output.setTimestamp(output.getTimestamp()) - output.setTimestampDevice(output.getTimestampDevice()) - transformation = output.getTransformation() - if transformation is not None: - output.setTransformation(transformation) - self.out.send(output) self._logger.debug("Message sent successfully")🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@depthai_nodes/node/parsers/embeddings.py` around lines 89 - 96, `EmbeddingsParser.emit()` contains dead self-assignments that read metadata from the same `dai.NNData` object and write it back unchanged. Remove the no-op calls to `setSequenceNum`, `setTimestamp`, `setTimestampDevice`, and the transformation re-set in `emit()`; keep the method focused on the actual emitted data flow, similar to how `RegressionParser.emit()` only copies metadata onto a separate message object when needed.depthai_nodes/node/parsers/map_output.py (1)
119-126: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winKeep value scaling in the compute step.
min_max_scalingchanges the emitted map values, but it remains inemit()viacreate_map_message. Since this refactor exposescompute()for direct tensor post-processing, callers usingMapOutputParser.compute()cannot reproduce parser output when scaling is enabled.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@depthai_nodes/node/parsers/map_output.py` around lines 119 - 126, The scaling logic is still applied in emit() through create_map_message, so MapOutputParser.compute() does not fully reproduce the parser output when min_max_scaling is enabled. Move the value scaling into compute_map_output (or the static compute() path) so direct tensor post-processing yields the same scaled map values as emit(), and keep emit() limited to packaging the already-scaled result via create_map_message.depthai_nodes/node/parsers/classification.py (1)
138-157: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicated output-layer resolution logic across parsers.
This block (auto-detecting
output_layer_namewhen unset, raising when ambiguous) is duplicated verbatim inHRNetParser.extract()andKeypointParser.extract()(and likely other single-output parsers in this refactor). Sinceextract()methods are newly introduced by this PR, this is a good opportunity to consolidate this into a sharedBaseParserhelper (e.g._resolve_output_layer_name(output)).♻️ Suggested consolidation
# in BaseParser def _resolve_single_output_layer(self, output: dai.NNData) -> str: layers = output.getAllLayerNames() self._logger.debug(f"Processing input with layers: {layers}") if len(layers) == 1 and self.output_layer_name == "": self.output_layer_name = layers[0] elif len(layers) != 1 and self.output_layer_name == "": raise ValueError( f"Expected 1 output layer, got {len(layers)} layers. Please provide the output_layer_name." ) return self.output_layer_name🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@depthai_nodes/node/parsers/classification.py` around lines 138 - 157, The output-layer auto-detection and ambiguity check in extract() is duplicated across classification parser logic and other single-output parsers like HRNetParser and KeypointParser. Move that shared resolution into a BaseParser helper such as _resolve_single_output_layer(output) that logs layers, sets output_layer_name when there is exactly one, and raises the same ValueError when it is unset and ambiguous. Update the affected extract() methods to call the shared helper instead of inlining the logic.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@depthai_nodes/node/parsers/embeddings.py`:
- Around line 76-83: The EmbeddingsParser.extract() path is treating
self.output_layer_name as if it were always a list, but setOutputLayerNames()
can store a single string, so the current len() check counts characters instead
of layer names. Update extract() to normalize the value from
self.output_layer_name into a list of layer names before logging and asserting,
and make the logic in EmbeddingsParser consistent with build() and
setOutputLayerNames() so both string and list inputs are handled correctly.
In `@depthai_nodes/node/parsers/fastsam.py`:
- Around line 391-393: The debug message in fastsam.py is reporting the wrong
value because `len(results_masks)` reflects the merged 2D label map height, not
the number of detected masks. Update the logging in `compute_fastsam_mask` to
use the actual mask count from the pre-merge mask collection returned by the
FastSAM pipeline, and keep the message tied to the segmentation creation step so
it matches what `merge_masks` produces.
In `@depthai_nodes/node/parsers/hrnet.py`:
- Around line 119-123: HRNetParser.compute is instance-bound and mutates
self.n_keypoints, which breaks the static extract/compute/emit contract used by
the sibling parsers. Update HRNetParser.compute to be a `@staticmethod` like
ClassificationParser.compute, KeypointParser.compute, and MLSDParser.compute,
and remove the self dependency from the compute path. Move the n_keypoints
update into HRNetParser.emit (or another instance method that already has state)
so external callers can invoke compute directly on heatmaps without constructing
an instance.
In `@depthai_nodes/node/parsers/mediapipe_palm_detection.py`:
- Around line 159-180: The MediaPipe palm parser’s extract method is selecting
tensors by shape from output.getAllLayerNames(), which can mis-pick auxiliary
outputs instead of the validated palm outputs. Update extract in
MediapipePalmDetectionParser to read only self.output_layer_names (set via
build()/setOutputLayerNames()), fetch those tensors explicitly, and then assign
bboxes and scores from the configured names before reshaping and returning them.
In `@depthai_nodes/node/parsers/scrfd.py`:
- Line 196: The SCRFD parser is decoding with stale cached anchors because
setInputSize(), setFeatStrideFPN(), and setNumAnchors() update anchor-defining
fields without refreshing _cached_anchors before the decode path in the parser
that passes self._cached_anchors. Update those public setters (or the shared
anchor-building path used by SCRFD/parser logic) to rebuild the cache whenever
any of those config values change, so subsequent calls use anchors derived from
the latest settings.
In `@depthai_nodes/node/parsers/segmentation.py`:
- Around line 152-154: The debug log in compute_segmentation_class_map is
misleading because class_map.shape[0] refers to the mask height, not the number
of classes. Update the message in the segmentation parser’s logging block to
report the correct metric, using the actual class count source from the
segmentation metadata or class mapping rather than the returned 2D array shape,
and keep the change localized to the segmentation message creation path.
In `@depthai_nodes/node/parsers/utils/detection.py`:
- Around line 16-22: The detection filtering in the NMS path is using the wrong
box format and should be fixed in the utility that calls nms_cv2. Convert the
bboxes to [x, y, width, height] before passing them into nms_cv2, then normalize
the returned indices to a flat 1-D array before using them for indexing. Make
this change in the detection parser flow around xyxy_to_xywh and nms_cv2 so the
downstream filtering works correctly with OpenCV’s return shape.
In `@depthai_nodes/node/parsers/utils/image_output.py`:
- Around line 10-14: The batch-axis handling in image_output parsing is too
broad: the current check in the image tensor utility collapses any input with a
first dimension of 1, which breaks valid 3-D shapes like single-channel or
single-row images. Update the logic in the image output path so only 4-D tensors
have their batch axis removed, and leave 3-D tensors untouched before the ndim
validation in the same helper.
In `@depthai_nodes/node/parsers/utils/map_output.py`:
- Around line 6-9: The map output normalization in map_output currently removes
the first axis whenever shape[0] == 1, which incorrectly collapses valid 2-D
single-row maps into 1-D arrays. Update the logic in map_output to only squeeze
leading singleton axes while the tensor has more than 2 dimensions, then ensure
the result still satisfies the 2-D map contract expected by Map2D.map. Preserve
tensors shaped like (1, W) and only flatten extra batch-like dimensions from
np.asarray(map_tensor) when needed.
In `@depthai_nodes/node/parsers/utils/medipipe.py`:
- Around line 410-443: The NMS input in the hand parsing flow uses center-based
boxes from the decoded hands, but `cv2.dnn.NMSBoxes` expects top-left `[x_min,
y_min, width, height]` boxes. Update the box-building logic in the
`decoded_bboxes` loop to keep a separate NMS box list using top-left coordinates
while preserving the existing center-format values for the returned detections,
and pass that NMS list into `cv2.dnn.NMSBoxes`.
In `@depthai_nodes/node/parsers/utils/ppdet.py`:
- Around line 186-193: The wrapper around parse_paddle_detection_outputs is
assuming NCHW by always unpacking predictions.shape as _, _, height, width,
which breaks NHWC inputs used by compute_pp_text_detections(). Update the ppdet
parsing helper to derive width and height from the tensor’s actual supported
layout before forwarding to parse_paddle_detection_outputs, using the existing
parse_paddle_detection_outputs/compute_pp_text_detections path to detect or
infer whether the input is NCHW or NHWC.
In `@depthai_nodes/node/parsers/utils/segmentation.py`:
- Around line 37-41: The chained expression in the segmentation parser is
failing Ruff formatting because the formatter rewrites the reshape/astype chain;
update the formatting in the code around the class_map assignment in the
segmentation utility so it matches Ruff’s output, and verify the final
shape/astype chain is laid out exactly as produced by ruff format for the
related reduce_fn/mask logic.
In `@depthai_nodes/node/parsers/utils/yolo.py`:
- Around line 642-686: The segmentation path in the YOLO parser builds
final_mask as uint8 with 255 as the background sentinel, which violates
SegmentationMask.mask’s int16 contract and can collide with instance IDs when
max_det exceeds 255. Update the mask construction in the YOLO parsing flow (the
branch that fills final_mask and returns the segmentation result) to use
np.int16-compatible storage and a non-colliding background value, or otherwise
cap/guard instance IDs so they never overlap with the sentinel. Keep the change
localized to the segmentation handling around process_single_mask and the
final_mask accumulation.
- Around line 656-657: The label-name mapping in the YOLO parser is not guarded,
so `mapped_label_names` can raise an IndexError when `label_names` is shorter
than the model’s class ids. Update the label lookup in the YOLO parsing path
around `mapped_label_names` to check whether the requested index exists before
indexing `label_names`, and fall back to a generated `class_<id>` name when it
is missing, matching the RF-DETR fallback behavior.
In `@depthai_nodes/node/parsers/utils/yunet.py`:
- Around line 303-310: The no-detection branch in the Yunet parser returns
rank-1 empty arrays, which breaks consumers expecting the same shapes as the
normal detection path. Update the empty return in the parser function that
handles bboxes so it returns shape-compatible empties for bboxes and keypoints,
matching the non-empty output ranks like (0, 4) and (0, 5, 2), while keeping
labels and scores empty in the same way.
- Around line 290-327: The YuNet decoding helper currently treats nms_fn and
top_left_wh_to_xywh_fn as optional even though decode_yunet_outputs calls both
unconditionally. Update the decode_yunet_outputs signature to either require
these hook arguments or provide real default implementations, and keep the call
sites in that function aligned so direct tensor callers don’t hit None at
runtime.
In `@depthai_nodes/node/parsers/xfeat.py`:
- Around line 387-400: Propagate the device timestamp on every emitted
TrackedFeatures message, since the mono-empty path and the stereo matched/empty
paths currently set only the output timestamp/sequence number and leave
TimestampDevice inconsistent. Update the branches in xfeat.py that build
matched_points, including the result is None case and the
compute_xfeat_matches/create_tracked_features_message flow, so each emitted
message copies output.getTimestampDevice() before self.out.send(). Also apply
the same fix to the other TrackedFeatures emission sites referenced in the
parser so all message paths behave consistently.
---
Outside diff comments:
In `@depthai_nodes/node/parsers/classification_sequence.py`:
- Around line 145-175: Remove the duplicated output-layer validation from
ClassificationSequenceParser.run so it only delegates to extract and compute
after reading the NNData; the layer-name resolution/ValueError logic should live
in one place only. Update ClassificationSequenceParser.extract to keep the
authoritative getAllLayerNames and output_layer_name handling, and ensure run no
longer sets self.output_layer_name before calling extract.
---
Nitpick comments:
In `@depthai_nodes/node/parsers/classification.py`:
- Around line 138-157: The output-layer auto-detection and ambiguity check in
extract() is duplicated across classification parser logic and other
single-output parsers like HRNetParser and KeypointParser. Move that shared
resolution into a BaseParser helper such as _resolve_single_output_layer(output)
that logs layers, sets output_layer_name when there is exactly one, and raises
the same ValueError when it is unset and ambiguous. Update the affected
extract() methods to call the shared helper instead of inlining the logic.
In `@depthai_nodes/node/parsers/embeddings.py`:
- Around line 89-96: `EmbeddingsParser.emit()` contains dead self-assignments
that read metadata from the same `dai.NNData` object and write it back
unchanged. Remove the no-op calls to `setSequenceNum`, `setTimestamp`,
`setTimestampDevice`, and the transformation re-set in `emit()`; keep the method
focused on the actual emitted data flow, similar to how
`RegressionParser.emit()` only copies metadata onto a separate message object
when needed.
In `@depthai_nodes/node/parsers/map_output.py`:
- Around line 119-126: The scaling logic is still applied in emit() through
create_map_message, so MapOutputParser.compute() does not fully reproduce the
parser output when min_max_scaling is enabled. Move the value scaling into
compute_map_output (or the static compute() path) so direct tensor
post-processing yields the same scaled map values as emit(), and keep emit()
limited to packaging the already-scaled result via create_map_message.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 72d5a140-3c20-46f7-a1a4-82f15b43f2c3
📒 Files selected for processing (43)
depthai_nodes/node/parsers/classification.pydepthai_nodes/node/parsers/classification_sequence.pydepthai_nodes/node/parsers/detection.pydepthai_nodes/node/parsers/embeddings.pydepthai_nodes/node/parsers/fastsam.pydepthai_nodes/node/parsers/hrnet.pydepthai_nodes/node/parsers/image_output.pydepthai_nodes/node/parsers/keypoints.pydepthai_nodes/node/parsers/lane_detection.pydepthai_nodes/node/parsers/map_output.pydepthai_nodes/node/parsers/mediapipe_palm_detection.pydepthai_nodes/node/parsers/mlsd.pydepthai_nodes/node/parsers/ppdet.pydepthai_nodes/node/parsers/regression.pydepthai_nodes/node/parsers/rf_detr.pydepthai_nodes/node/parsers/scrfd.pydepthai_nodes/node/parsers/segmentation.pydepthai_nodes/node/parsers/superanimal_landmarker.pydepthai_nodes/node/parsers/utils/__init__.pydepthai_nodes/node/parsers/utils/classification.pydepthai_nodes/node/parsers/utils/classification_sequence.pydepthai_nodes/node/parsers/utils/detection.pydepthai_nodes/node/parsers/utils/embeddings.pydepthai_nodes/node/parsers/utils/fastsam.pydepthai_nodes/node/parsers/utils/hrnet.pydepthai_nodes/node/parsers/utils/image_output.pydepthai_nodes/node/parsers/utils/keypoints.pydepthai_nodes/node/parsers/utils/lane_detection.pydepthai_nodes/node/parsers/utils/map_output.pydepthai_nodes/node/parsers/utils/medipipe.pydepthai_nodes/node/parsers/utils/mlsd.pydepthai_nodes/node/parsers/utils/ppdet.pydepthai_nodes/node/parsers/utils/regression.pydepthai_nodes/node/parsers/utils/rf_detr.pydepthai_nodes/node/parsers/utils/scrfd.pydepthai_nodes/node/parsers/utils/segmentation.pydepthai_nodes/node/parsers/utils/superanimal.pydepthai_nodes/node/parsers/utils/xfeat.pydepthai_nodes/node/parsers/utils/yolo.pydepthai_nodes/node/parsers/utils/yunet.pydepthai_nodes/node/parsers/xfeat.pydepthai_nodes/node/parsers/yolo.pydepthai_nodes/node/parsers/yunet.py
klemen1999
reviewed
Jul 2, 2026
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@depthai_nodes/node/parsers/utils/scrfd.py`:
- Around line 197-198: Clip SCRFD detections before the xywh conversion in
create_detection_message: the current xyxy_to_xywh followed by np.clip can still
leave boxes extending past [0, 1]. Update the SCRFD parsing flow around
xyxy_to_xywh and the bboxes normalization so the decoded xyxy corners are
clamped first, or alternatively clamp the final xywh extents consistently,
ensuring create_detection_message receives fully normalized boxes.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 463ac97f-a39f-48e2-a0e4-47754f094387
📒 Files selected for processing (8)
depthai_nodes/node/parsers/classification.pydepthai_nodes/node/parsers/classification_sequence.pydepthai_nodes/node/parsers/embeddings.pydepthai_nodes/node/parsers/hrnet.pydepthai_nodes/node/parsers/scrfd.pydepthai_nodes/node/parsers/utils/classification.pydepthai_nodes/node/parsers/utils/scrfd.pydepthai_nodes/node/parsers/yunet.py
💤 Files with no reviewable changes (3)
- depthai_nodes/node/parsers/hrnet.py
- depthai_nodes/node/parsers/classification_sequence.py
- depthai_nodes/node/parsers/scrfd.py
🚧 Files skipped from review as they are similar to previous changes (4)
- depthai_nodes/node/parsers/classification.py
- depthai_nodes/node/parsers/embeddings.py
- depthai_nodes/node/parsers/yunet.py
- depthai_nodes/node/parsers/utils/classification.py
Comment on lines
+197
to
+198
| bboxes = xyxy_to_xywh(bboxes) | ||
| bboxes = np.clip(bboxes, 0, 1) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== scrfd.py slice ==\n'
sed -n '150,220p' depthai_nodes/node/parsers/utils/scrfd.py
printf '\n== bbox_format_converters files ==\n'
git ls-files | rg 'bbox_format_converters|scrfd.py$'
printf '\n== locate xyxy_to_xywh ==\n'
rg -n "def xyxy_to_xywh|xyxy_to_xywh\(" -S .Repository: luxonis/depthai-nodes
Length of output: 2717
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Show the converter implementation if found, with surrounding context.
files=$(git ls-files | rg 'bbox_format_converters|bbox.*converters|converters.*bbox')
printf '%s\n' "$files"
for f in $files; do
if rg -n "def xyxy_to_xywh|xyxy_to_xywh\(" "$f" >/dev/null; then
echo
echo "== $f =="
sed -n '1,220p' "$f"
fi
doneRepository: luxonis/depthai-nodes
Length of output: 7740
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== call sites ==\n'
rg -n "compute_scrfd_detections\(|xywh_to_xyxy\(|xyxy_to_xywh\(" depthai_nodes -S
printf '\n== scrfd parser wrapper ==\n'
sed -n '1,220p' depthai_nodes/node/parsers/scrfd.py
printf '\n== nearby docs / comments mentioning bbox format ==\n'
rg -n "xywh|xyxy|center.*width|width.*height|bbox" depthai_nodes/node/parsers -S | sed -n '1,220p'Repository: luxonis/depthai-nodes
Length of output: 32214
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '1,220p' depthai_nodes/node/parsers/scrfd.pyRepository: luxonis/depthai-nodes
Length of output: 8941
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== create_detection_message ==\n'
rg -n "def create_detection_message|create_detection_message\(" depthai_nodes -S
sed -n '1,260p' depthai_nodes/message/creators.py
printf '\n== detection message/box contract ==\n'
rg -n "ImgDetections|DetectionMessage|bboxes are expected|x_center|width|height" depthai_nodes -S | sed -n '1,240p'Repository: luxonis/depthai-nodes
Length of output: 1230
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '1,260p' depthai_nodes/message/creators.pyRepository: luxonis/depthai-nodes
Length of output: 236
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== create_detection_message implementation ==\n'
sed -n '1,240p' depthai_nodes/message/creators/detection.py
printf '\n== SCRFD parser emit path ==\n'
sed -n '240,290p' depthai_nodes/node/parsers/scrfd.py
printf '\n== detection parser contract ==\n'
sed -n '1,240p' depthai_nodes/node/parsers/detection.pyRepository: luxonis/depthai-nodes
Length of output: 19785
Clip SCRFD boxes before converting to xywh
create_detection_message expects normalized xywh, but clipping x_center/y_center/width/height after xyxy_to_xywh doesn’t keep the box inside [0, 1] — edge boxes can still extend past the frame. Clip the decoded xyxy corners first, or clamp the final extents instead.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@depthai_nodes/node/parsers/utils/scrfd.py` around lines 197 - 198, Clip SCRFD
detections before the xywh conversion in create_detection_message: the current
xyxy_to_xywh followed by np.clip can still leave boxes extending past [0, 1].
Update the SCRFD parsing flow around xyxy_to_xywh and the bboxes normalization
so the decoded xyxy corners are clamped first, or alternatively clamp the final
xywh extents consistently, ensuring create_detection_message receives fully
normalized boxes.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
depthai_nodes/node/parsers/yolo.py (1)
463-477: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winValidate the V26 prototype layer before reading it.
Line 474 assumes
_protos_layer_nameexists and that the tensor is present. A V26 segmentation parser configured outsidebuild()can crash here instead of failing with a clear validation error.Proposed guard
- v26_protos = output.getTensor( - self._protos_layer_name, + protos_layer_name = getattr( + self, "_protos_layer_name", "protos_output" + ) + if protos_layer_name not in output.getAllLayerNames(): + raise ValueError( + "YOLO26 segmentation requires a prototype output layer." + ) + v26_protos = output.getTensor( + protos_layer_name, dequantize=True, storageOrder=dai.TensorInfo.StorageOrder.NCHW, ).astype(np.float32)[0]🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@depthai_nodes/node/parsers/yolo.py` around lines 463 - 477, The V26 parsing branch in yolo.py assumes _protos_layer_name is valid and present, which can crash when the parser is used without build(). Add a validation check in the YOLOSubtype.V26 path before reading output.getTensor(self._protos_layer_name, ...) to ensure the prototype layer name is configured and exists in layer_names. If it is missing, raise a clear validation error from the parser instead of letting the tensor read fail later.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@depthai_nodes/node/parsers/yolo.py`:
- Line 408: The YOLO decoding path is still using internally derived default
strides instead of the validated `head_config["strides"]`, which can mismatch
custom-stride models. Update `compute_yolo_detections(...)` to accept a
`strides` parameter and use that resolved value for non-V26 decoding instead of
recomputing defaults; then pass the parsed strides from the YOLO parser flow
into the call sites that use `compute_yolo_detections` so `input_shape` and box
decoding stay consistent.
---
Outside diff comments:
In `@depthai_nodes/node/parsers/yolo.py`:
- Around line 463-477: The V26 parsing branch in yolo.py assumes
_protos_layer_name is valid and present, which can crash when the parser is used
without build(). Add a validation check in the YOLOSubtype.V26 path before
reading output.getTensor(self._protos_layer_name, ...) to ensure the prototype
layer name is configured and exists in layer_names. If it is missing, raise a
clear validation error from the parser instead of letting the tensor read fail
later.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c87505d3-db8b-4b25-9488-bd0950ae6162
📒 Files selected for processing (2)
depthai_nodes/node/parsers/utils/yolo.pydepthai_nodes/node/parsers/yolo.py
🚧 Files skipped from review as they are similar to previous changes (1)
- depthai_nodes/node/parsers/utils/yolo.py
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
depthai_nodes/node/parsers/yolo.py (1)
463-477: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winValidate the V26 prototype layer before reading it.
Line 474 assumes
_protos_layer_nameexists and that the tensor is present. A V26 segmentation parser configured outsidebuild()can crash here instead of failing with a clear validation error.Proposed guard
- v26_protos = output.getTensor( - self._protos_layer_name, + protos_layer_name = getattr( + self, "_protos_layer_name", "protos_output" + ) + if protos_layer_name not in output.getAllLayerNames(): + raise ValueError( + "YOLO26 segmentation requires a prototype output layer." + ) + v26_protos = output.getTensor( + protos_layer_name, dequantize=True, storageOrder=dai.TensorInfo.StorageOrder.NCHW, ).astype(np.float32)[0]🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@depthai_nodes/node/parsers/yolo.py` around lines 463 - 477, The V26 parsing branch in yolo.py assumes _protos_layer_name is valid and present, which can crash when the parser is used without build(). Add a validation check in the YOLOSubtype.V26 path before reading output.getTensor(self._protos_layer_name, ...) to ensure the prototype layer name is configured and exists in layer_names. If it is missing, raise a clear validation error from the parser instead of letting the tensor read fail later.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@depthai_nodes/node/parsers/yolo.py`:
- Line 408: The YOLO decoding path is still using internally derived default
strides instead of the validated `head_config["strides"]`, which can mismatch
custom-stride models. Update `compute_yolo_detections(...)` to accept a
`strides` parameter and use that resolved value for non-V26 decoding instead of
recomputing defaults; then pass the parsed strides from the YOLO parser flow
into the call sites that use `compute_yolo_detections` so `input_shape` and box
decoding stay consistent.
---
Outside diff comments:
In `@depthai_nodes/node/parsers/yolo.py`:
- Around line 463-477: The V26 parsing branch in yolo.py assumes
_protos_layer_name is valid and present, which can crash when the parser is used
without build(). Add a validation check in the YOLOSubtype.V26 path before
reading output.getTensor(self._protos_layer_name, ...) to ensure the prototype
layer name is configured and exists in layer_names. If it is missing, raise a
clear validation error from the parser instead of letting the tensor read fail
later.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c87505d3-db8b-4b25-9488-bd0950ae6162
📒 Files selected for processing (2)
depthai_nodes/node/parsers/utils/yolo.pydepthai_nodes/node/parsers/yolo.py
🚧 Files skipped from review as they are similar to previous changes (1)
- depthai_nodes/node/parsers/utils/yolo.py
🛑 Comments failed to post (1)
depthai_nodes/node/parsers/yolo.py (1)
408-408: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Pass resolved strides into the YOLO decoder.
head_config["strides"]is now read and validated, butcompute_yolo_detections()still decodes non-V26 outputs using its internal defaults. Custom-stride models will deriveinput_shapefrom one stride set and decode boxes with another.Contract-level fix
`@dataclass`(frozen=True) class YOLOComputeInputs: ... input_shape: tuple[int, int] | None + strides: list[int] | None kpts_outputs: list[np.ndarray] | Noneif self.subtype == YOLOSubtype.V26: ... input_shape = self.input_shape + strides = None else: strides = resolve_yolo_strides( self.strides, self.subtype, num_outputs=len(outputs_values), )input_shape=input_shape, + strides=strides,Then add a
stridesparameter tocompute_yolo_detections(...)and use it instead of recomputing defaults internally.Also applies to: 533-590
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@depthai_nodes/node/parsers/yolo.py` at line 408, The YOLO decoding path is still using internally derived default strides instead of the validated `head_config["strides"]`, which can mismatch custom-stride models. Update `compute_yolo_detections(...)` to accept a `strides` parameter and use that resolved value for non-V26 decoding instead of recomputing defaults; then pass the parsed strides from the YOLO parser flow into the call sites that use `compute_yolo_detections` so `input_shape` and box decoding stay consistent.
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
2 participants
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.
Purpose
run()method by calling three methods successively:extract,computeandemitNNData, validate the expected layers/shapes, and return the tensors or intermediate inputs needed for processingcomputeis static so it can be called by external libraries to apply the logic to a tensorcomputelogic is moved to a parallel file inutils/that holds the same name as the parser fileSpecification
None / not applicable
Dependencies & Potential Impact
None / not applicable
Deployment Plan
None / not applicable
Testing & Validation
None / not applicable
AI Usage
Assisted-by: AGENT_NAME:MODEL_VERSION [TOOL1] [TOOL2]
Submitted code was reviewed by a human: YES/NO
The author is taking the responsibility for the contribution: YES/NO
Summary by CodeRabbit
New Features
Refactor