Conversation
LiveReview Pre-Commit Check: ran (iter:1, coverage:0%)
…g Tools LiveReview Pre-Commit Check: ran (iter:2, coverage:0%)
…anced Audio Testing LiveReview Pre-Commit Check: ran (iter:2, coverage:90%)
LiveReview Pre-Commit Check: ran (iter:2, coverage:88%)
…ts and samples. scripts to preload models to work offline LiveReview Pre-Commit Check: ran (iter:11, coverage:100%)
LiveReview Pre-Commit Check: vouched (iter:1, coverage:0%)
LiveReview Pre-Commit Check: ran (iter:7, coverage:94%)
LiveReview Pre-Commit Check: ran (iter:3, coverage:100%)
LiveReview Pre-Commit Check: ran (iter:1, coverage:0%)
LiveReview Pre-Commit Check: ran (iter:3, coverage:46%)
LiveReview Pre-Commit Check: ran (iter:1, coverage:0%)
PR Summary by QodoAdd modular vision pipeline + multilingual voice-agent responses with LLM fallback
AI Description
Diagram
High-Level Assessment
Files changed (88)
|
Code Review by Qodo
1. Logger name split crash
|
| @@ -525,7 +525,7 @@ def _store_audio(self, audio_bytes: bytes) -> None: | |||
| with self._wav_writer_lock: | |||
There was a problem hiding this comment.
1. Misleading asr storage log 🐞 Bug ◔ Observability
ASR._store_audio() logs "Audio chunk stored" even when _wav_writer is None, because the log statement was moved outside the guarded write block. This produces incorrect debug telemetry for audio storage.
Agent Prompt
### Issue description
`src/audio/asr.py` logs a successful audio-store message even when no writer exists because `logger.debug(...)` is outside the `if self._wav_writer is not None` block.
### Issue Context
This is an observability defect: it can mislead debugging/monitoring by indicating chunks were stored when they were not.
### Fix
Move the `logger.debug("Audio chunk stored...")` call under the `if self._wav_writer is not None:` block (optionally log a different message when storage is disabled).
### Fix Focus Areas
- src/audio/asr.py[523-528]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| face_model_name: "buffalo_l" # "imx500_yolo11n.rpk" # "buffalo_l" #"imx500_network_mobilenet_v2.rpk" #"buffalo_l" #"haarcascade_frontalface_default.xml" , "buffalo_l" for insightface | ||
| face_model_path: null # "/data/imx500_yolo11n.rpk" # for "/data/imx500_yolo11n.rpk" imx500 # null for insightface | ||
| face_recognition_threshold: 0.4 | ||
| post_processing_enabled: false |
There was a problem hiding this comment.
2. Vision postprocess key mismatch 🐞 Bug ≡ Correctness
config.yaml sets vision.post_processing_model, but VisionConfig defines post_processing_model_name, so the configured value won’t populate that field (it will be dropped or cause validation failure depending on Pydantic extra-field settings). This makes the configured post-processing model selection ineffective.
Agent Prompt
### Issue description
`config.yaml` uses `vision.post_processing_model`, but the code reads `VisionConfig.post_processing_model_name`. As a result, the intended model filename from YAML does not actually control the runtime model choice.
### Issue Context
The mismatch is between the YAML key and the Pydantic model field name. There is no alias defined for `post_processing_model_name`.
### Fix
Choose one:
1) Rename the YAML key to `post_processing_model_name`, or
2) Add a Pydantic alias (e.g., `Field(validation_alias="post_processing_model")`) so both keys work.
### Fix Focus Areas
- config.yaml[88-96]
- src/utils/config.py[249-265]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
|
|
||
| import requests | ||
|
|
||
| module_name = __name__ |
There was a problem hiding this comment.
3. Logger name split crash 🐞 Bug ☼ Reliability
Multiple new modules compute lib_name via __name__.split('.')[1], which raises IndexError when
the module is executed with python -m (where __name__ == '__main__'). This prevents running the
new vision/LLM entrypoints in module mode.
Agent Prompt
### Issue description
Several modules assume `__name__` contains a dot and do `__name__.split('.')[1]`. When run via `python -m ...`, `__name__` becomes `__main__` and this crashes at import time.
### Issue Context
These files include shebangs / are used as runnable components, so module-mode execution is a realistic invocation.
### Fix
Prefer `logger = logging.getLogger(__name__)` (simplest), or use a safe fallback:
```py
module_name = __name__
lib_name = module_name.split(".")[1] if "." in module_name else module_name
logger = logging.getLogger(lib_name)
```
Apply consistently across all new modules using this pattern.
### Fix Focus Areas
- src/llm/llm_client.py[9-11]
- src/vision/video_capture.py[34-36]
- src/vision/yolo_cpu.py[26-28]
- src/utils/camera.py[21-23]
- src/vision/face_detector.py[16-18]
- src/vision/face_detector_cascade.py[17-19]
- src/vision/face_in_frame.py[17-19]
- src/vision/face_insight_pipeline.py[22-25]
- src/vision/face_recognizer.py[14-16]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| self.thread.start() | ||
| return self | ||
|
|
||
| def update(self) -> None: |
There was a problem hiding this comment.
4. Unreleased picamera2 request 🐞 Bug ☼ Reliability
ThreadedCamera.update() only calls req.release() on the success path; if make_array()/get_metadata() raises, the request is never released. This can leak camera buffers/requests and degrade or stall capture over time.
Agent Prompt
### Issue description
In the Picamera2 path, `capture_request()` is followed by work that can raise before `req.release()` is reached; the `except` block doesn’t release the request.
### Issue Context
This code runs in a tight background loop, so even occasional exceptions can accumulate unreleased requests/buffers.
### Fix
Wrap request usage in `try/finally`:
- Initialize `req = None`
- After successful `capture_request()`, ensure `req.release()` is called in `finally` if `req` is not `None`.
### Fix Focus Areas
- src/utils/camera.py[106-120]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| def test_model_loaded(self, vc): | ||
| assert vc.model is not None, "YOLO model should be loaded" | ||
|
|
||
| def test_model_path_exists(self, cfg): |
There was a problem hiding this comment.
5. Vision test needs model 🐞 Bug ☼ Reliability
tests/vision/test_video_capture_init.py unconditionally asserts that cfg.vision.object_model_full_path exists, but the default path resolves under .cache/vision/models/... and the test does not provision it. This makes the integration test non-reproducible on clean environments unless an explicit download/setup step is added.
Agent Prompt
### Issue description
The integration test asserts the YOLO model file exists at `cfg.vision.object_model_full_path` without ensuring it has been downloaded/created.
### Issue Context
`VisionConfig.object_model_full_path` defaults to `.cache/vision/models/<type>/<name>` when `object_model_path` is unset. The repo provides a separate script to download/convert YOLO models into that cache directory, but the test doesn’t call it.
### Fix
Pick one (prefer fast/reliable CI behavior):
1) Change the test to `pytest.skip(...)` when the model file is missing, with a message pointing to the download script.
2) In a fixture, download/provision the model (or a small test model) before asserting existence.
3) Add a CI step that runs the download/conversion script before executing `-m integration`.
### Fix Focus Areas
- tests/vision/test_video_capture_init.py[129-132]
- src/utils/config.py[282-303]
- scripts/models/vision/yolo_onnx.py[19-34]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
No description provided.