Skip to content

qodo-code-review fixes - #13

Merged
chcavignx merged 3 commits into
mainfrom
test
Aug 7, 2026
Merged

qodo-code-review fixes#13
chcavignx merged 3 commits into
mainfrom
test

Conversation

@chcavignx

Copy link
Copy Markdown
Owner

In src/vision/yolo_cpu.py:

import cv2
+import numpy as np
+import onnxruntime as ort

  1. Onnxruntime import-time crash 🐞 Bug ☼ Reliability

src/vision/yolo_cpu.py now imports onnxruntime at module import time, so importing vision
modules can crash with ModuleNotFoundError in environments where onnxruntime is intentionally
optional. This can also indirectly affect IMX500 code paths because yolo_imx500.py imports
Detection from yolo_cpu.py.

In src/vision/yolo_imx500.py:

     if not metadata or not self.imx500:
  •        return None
    
  •        return DummyResults([], (frame_h, frame_w))
    

Action required

  1. Dummyboxes len() typeerror 🐞 Bug ≡ Correctness

Imx500Detector.infer() now returns DummyResults even when metadata/IMX500 is unavailable, but
DummyResults.boxes is a DummyBoxes without len. VideoCapture calls
len(self.latest_results.boxes) and len(results.boxes) unconditionally in overlay/photo-capture
paths, which will raise TypeError at runtime.

LiveReview Pre-Commit Check: ran (iter:3, coverage:82%)
LiveReview Pre-Commit Check: ran (iter:2, coverage:100%)
LiveReview Pre-Commit Check: ran (iter:1, coverage:0%)
@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

Fix optional onnxruntime import, IMX500 dummy results, and model resolution

🐞 Bug fix 🧪 Tests ⚙️ Configuration changes 🕐 20-40 Minutes

Grey Divider

AI Description

• Lazily import onnxruntime for LibreYOLO ONNX fallback with actionable ImportError.
• Make IMX500 dummy results compatible with len()/iteration and harden object counting.
• Correct IMX500 model resolution (640x480) and add regression tests.
Diagram

graph TD
  VC["VideoCapture"] --> OIF["ObjectInsightFrame"] --> SEL{"Model type"}
  SEL --> CPU["YoloCpuDetector"] --> ONNX["LibreYoloOnnxPredictor"] --> ORT{{"onnxruntime"}}
  SEL --> IMX["Imx500Detector"]
  CFG[("Config model_resolutions")] --> CPU --> ONNX
  CFG --> IMX

  subgraph Legend
    direction LR
    _svc["Runtime component"] ~~~ _cfg[("Config")] ~~~ _dec{"Decision"} ~~~ _ext{{"Optional dependency"}}
  end
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Package extras for optional runtimes (e.g., vision[onnx])
  • ➕ Clarifies optional dependency contract for users
  • ➕ Avoids runtime surprises by documenting/installing the right extras
  • ➖ Still requires runtime guardrails for environments that intentionally omit extras
  • ➖ Packaging-only change doesn’t prevent accidental import-time crashes without code changes
2. Unify result shape via a small Results protocol/adapter
  • ➕ Eliminates scattered hasattr/len checks across UI/overlay paths
  • ➕ Makes future detector integrations safer
  • ➖ More refactor scope than this targeted fix
  • ➖ Requires careful alignment with Ultralytics Results expectations

Recommendation: Keep the PR’s approach: lazy-import onnxruntime at instantiation time with a clear error message, and ensure IMX500 fallback results behave like Ultralytics results for len()/iteration. If this project expects multiple optional backends, consider adding documented pip extras and a thin Results adapter later to reduce defensive checks in VideoCapture.

Files changed (7) +80 / -14

Bug fix (3) +48 / -12
video_capture.pyHarden frame validation and object-count overlay/message generation +34/-9

Harden frame validation and object-count overlay/message generation

• Adds a reusable _is_valid_frame() guard to avoid processing empty/None frames (including test doubles). Updates overlay and capture-photo messaging to compute object counts safely across different Results shapes (detections list vs boxes with __len__).

src/vision/video_capture.py

yolo_cpu.pyMove onnxruntime import into LibreYoloOnnxPredictor with better error +8/-1

Move onnxruntime import into LibreYoloOnnxPredictor with better error

• Removes module import-time dependency on onnxruntime. LibreYoloOnnxPredictor now imports onnxruntime lazily and raises an actionable ImportError if unavailable.

src/vision/yolo_cpu.py

yolo_imx500.pyMake DummyBoxes support len() and iteration +6/-2

Make DummyBoxes support len() and iteration

• Implements __len__ and updates __iter__ to iterate over stored xyxy boxes. This makes DummyResults.boxes compatible with code paths that call len(boxes) or iterate over boxes.

src/vision/yolo_imx500.py

Tests (3) +31 / -1
test_config.pyAdd resolution lookup coverage for IMX500 +2/-1

Add resolution lookup coverage for IMX500

• Extends get_model_resolution() test to assert the IMX500 entry resolves to (640, 480).

tests/utils/test_config.py

test_libreyolo.pyTest ONNX fallback raises ImportError when onnxruntime is missing +11/-0

Test ONNX fallback raises ImportError when onnxruntime is missing

• Adds a regression test that simulates missing onnxruntime and asserts LibreYoloOnnxPredictor fails with a clear ImportError message.

tests/vision/test_libreyolo.py

test_vision_imx500.pyTest DummyBoxes len()/iter behavior via DummyResults +18/-0

Test DummyBoxes len()/iter behavior via DummyResults

• Adds a unit test verifying DummyBoxes returns correct length and iterates expected xyxy coordinates when populated through DummyResults.

tests/vision/test_vision_imx500.py

Other (1) +1 / -1
config.yamlFix IMX500 model resolution to 640x480 +1/-1

Fix IMX500 model resolution to 640x480

• Updates the model_resolutions entry for "imx500" from 640x640 to 640x480 to match the expected sensor/network input shape.

config.yaml

@qodo-code-review

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0)

Grey Divider

Great, no issues found!

Qodo reviewed your code and found no material issues that require review

Grey Divider

To customize comments, go to the Qodo configuration screen, or learn more in the docs.

Qodo Logo

@chcavignx
chcavignx merged commit c2dd394 into main Aug 7, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant