Pass conf= directly into YOLO inference calls instead of post-filtering - #203
Pass conf= directly into YOLO inference calls instead of post-filtering#203Cassiebastress wants to merge 1 commit into
Conversation
infer()/infer_text_music()/infer_staves() called Ultralytics bare, so its internal default (0.25) governed candidate generation regardless of the configured threshold; _append_boxes() only discarded boxes below that threshold afterward, in Python. The effective confidence floor could therefore never go below 0.25, even though the UI sliders already allow 0.00-1.00. Passes conf= straight into each call, matching infer_staves_raw_boxes()'s existing pattern. No default threshold values changed. Relates to #200 Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
📝 WalkthroughWalkthroughYOLO confidence filtering now occurs during model invocation through the ChangesYOLO confidence handling
Estimated code review effort: 3 (Moderate) | ~20 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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: 2
🤖 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 `@landing-page/scripts/tests/test_yolo_inference.py`:
- Around line 29-37: Update test module setup around the yolo_inference import
to snapshot the existing sys.modules entries and sys.path, then restore them
after importing YoloModelSet and _append_boxes. Ensure both the yolo_inference
entry and any temporary models_api stub are reverted to their prior state so
later tests can install their own stubs.
In `@landing-page/scripts/yolo_inference.py`:
- Line 35: Update the _append_boxes function signature to include the missing ->
None return annotation, preserving its existing parameters and implementation.
🪄 Autofix
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: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: e1954da9-2ff2-46c8-baf0-809410859f3d
📒 Files selected for processing (2)
landing-page/scripts/tests/test_yolo_inference.pylanding-page/scripts/yolo_inference.py
| sys.path.insert(0, str(Path(__file__).resolve().parent.parent)) | ||
|
|
||
| if "models_api" not in sys.modules: | ||
| models_api_stub = types.ModuleType("models_api") | ||
| models_api_stub.get_model_file_path = lambda *a, **k: None | ||
| sys.modules["models_api"] = models_api_stub | ||
|
|
||
| sys.modules.pop("yolo_inference", None) | ||
| from yolo_inference import YoloModelSet, _append_boxes # noqa: E402 |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find test and application imports that can observe the module-cache changes.
rg -n -C 3 \
'(^|\s)(import|from)\s+(models_api|yolo_inference)\b|sys\.modules|sys\.path\.insert' \
landing-page/scriptsRepository: DDMAL/mothra
Length of output: 22030
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- test_yolo_inference.py ---'
cat -n landing-page/scripts/tests/test_yolo_inference.py | sed -n '1,55p'
printf '%s\n' '--- test_tasks_text_batch_logs.py import setup ---'
cat -n landing-page/scripts/tests/test_tasks_text_batch_logs.py | sed -n '1,125p'
printf '%s\n' '--- pytest configuration and test ordering hints ---'
find . -maxdepth 3 -type f \( -name 'pytest.ini' -o -name 'pyproject.toml' -o -name 'tox.ini' -o -name 'conftest.py' \) -print
rg -n -C 3 'testpaths|python_files|pytest_collection|import_mode|order|test_yolo_inference|test_tasks_text_batch_logs' \
pytest.ini pyproject.toml tox.ini landing-page 2>/dev/null || true
printf '%s\n' '--- direct import-cache mutations in tests ---'
rg -n -C 2 'sys\.modules\.(pop|clear)|sys\.modules\[[^]]+\]\s*=|sys\.path\.(insert|remove)|sys\.path\s*=' \
landing-page/scripts/testsRepository: DDMAL/mothra
Length of output: 50369
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
import ast
from pathlib import Path
root = Path("landing-page/scripts/tests")
for path in sorted(root.glob("test_*.py")):
tree = ast.parse(path.read_text(), filename=str(path))
imports = []
mutations = []
for node in ast.walk(tree):
if isinstance(node, ast.Import):
imports.extend(a.name for a in node.names if a.name in {"models_api", "yolo_inference"})
elif isinstance(node, ast.ImportFrom):
if node.module in {"models_api", "yolo_inference"}:
imports.append(node.module)
elif isinstance(node, ast.Call):
text = ast.unparse(node)
if "sys.modules" in text or "sys.path.insert" in text:
mutations.append(text)
if imports or mutations:
print(path.name)
for item in imports:
print(f" import: {item}")
for item in mutations:
print(f" mutation: {item}")
PY
printf '%s\n' '--- focused module-cache setup ---'
sed -n '1,115p' landing-page/scripts/tests/test_tasks_text_batch_logs.py | \
rg -n 'sys\.modules|sys\.path|tasks_text_batch|yolo_inference|models_api'Repository: DDMAL/mothra
Length of output: 2785
Restore the import state after module setup.
test_yolo_inference.py leaves the real yolo_inference module in sys.modules. If this module loads before test_tasks_text_batch_logs.py, the batch test skips its stub and imports the real dependencies. Restore the previous sys.modules entries and sys.path after importing YoloModelSet and _append_boxes.
🧰 Tools
🪛 Ruff (0.16.1)
[warning] 33-33: Unused lambda argument: a
(ARG005)
[warning] 33-33: Unused lambda argument: k
(ARG005)
🤖 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 `@landing-page/scripts/tests/test_yolo_inference.py` around lines 29 - 37,
Update test module setup around the yolo_inference import to snapshot the
existing sys.modules entries and sys.path, then restore them after importing
YoloModelSet and _append_boxes. Ensure both the yolo_inference entry and any
temporary models_api stub are reverted to their prior state so later tests can
install their own stubs.
| return "cpu" | ||
|
|
||
| def _append_boxes(lines, inference, cls_map, threshold): | ||
| def _append_boxes(lines, inference, cls_map): |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Add the missing return annotation.
Ruff reports ANN202 for _append_boxes. Add -> None to keep this changed function compliant with the active lint rules.
Proposed fix
-def _append_boxes(lines, inference, cls_map):
+def _append_boxes(lines, inference, cls_map) -> None:📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| def _append_boxes(lines, inference, cls_map): | |
| def _append_boxes(lines, inference, cls_map) -> None: |
🧰 Tools
🪛 Ruff (0.16.1)
[warning] 35-35: Missing return type annotation for private function _append_boxes
Add return type annotation: None
(ANN202)
🤖 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 `@landing-page/scripts/yolo_inference.py` at line 35, Update the _append_boxes
function signature to include the missing -> None return annotation, preserving
its existing parameters and implementation.
Source: Linters/SAST tools
1 participant
Status: draft, do not merge yet. This fixes only the mechanical bug below; no default confidence threshold values are changed. Holding for Gen's testing to pick the right default before this merges.
infer(),infer_text_music(), andinfer_staves()inyolo_inference.pycalled the Ultralytics model bare (noconf=), so Ultralytics' own internal default (0.25) governed candidate generation/NMS regardless of what was configured._append_boxes()only discarded boxes below the caller's threshold afterward, in Python, so the effective confidence floor could never go below0.25no matter what a caller set. The frontend sliders already allow0.00-1.00in steps of0.05, so this was a live, reachable bug, not theoretical.This passes
conf=straight into each model call, matchinginfer_staves_raw_boxes()'s existing pattern, so the threshold governs candidate generation for real. Addstest_yolo_inference.pyto lock in the fix (no prior test coverage existed for this module).Refs #200
Summary by CodeRabbit
Bug Fixes
Tests