Repository navigation
fix(OMN-9537): add source_only flag to compliance scanner to exclude .venv - #907
Conversation
….venv Without this flag, onex compliance check scans all .venv installed packages producing ~1258 false-positive handler_resolution failures (94% noise). The source_only=True kwarg filters out any contract.yaml whose path contains a .venv component, reducing to source-only contracts. Default behaviour (source_only=False) is unchanged for backwards compatibility. Two unit tests added covering the exclude and include cases. Closes OMN-9537
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 3 minutes and 25 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThe Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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: 1
🧹 Nitpick comments (1)
src/omnibase_core/nodes/node_compliance_scan_compute/handler.py (1)
79-81: Avoid traversing.venvwhensource_only=Trueto reduce scan cost.Line 79 still walks the full tree (including
.venv) and only filters after discovery. This can be noticeably slow on large virtualenvs.♻️ Suggested change (prune during traversal)
+import os from pathlib import Path @@ - contracts = sorted(root.rglob("contract.yaml")) - if source_only: - contracts = [c for c in contracts if ".venv" not in c.parts] + contracts: list[Path] = [] + for dirpath, dirnames, filenames in os.walk(root): + path_parts = Path(dirpath).parts + if source_only and ".venv" in path_parts: + # do not descend further into this subtree + dirnames[:] = [] + continue + if source_only: + # skip entering .venv from parent directories + dirnames[:] = [d for d in dirnames if d != ".venv"] + if "contract.yaml" in filenames: + contracts.append(Path(dirpath) / "contract.yaml") + contracts.sort()🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/omnibase_core/nodes/node_compliance_scan_compute/handler.py` around lines 79 - 81, The current code collects contracts via sorted(root.rglob("contract.yaml")) and only filters out paths under ".venv" afterwards, which still traverses the entire tree; change the traversal to prune ".venv" during walking when source_only is True (e.g., replace the rglob call with an os.walk over root and, inside the loop, remove ".venv" from the dirs list when source_only to avoid descending into it) so that the contracts list (the variable contracts) is built only from non-venv paths; keep the same final sorted list semantics so downstream code using contracts behaves the same.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@tests/unit/nodes/node_compliance_scan_compute/test_compliance_scan_compute.py`:
- Around line 199-215: In test_source_only_false_includes_venv_contracts, add a
regression check that calling scanner.scan with no source_only arg uses the same
default behavior: call scanner.scan(str(tmp_path)) (in addition to the existing
scanner.scan(str(tmp_path), source_only=False)) and assert the results length
equals 2; this verifies the default parameter behavior in the scan function
remains backward compatible.
---
Nitpick comments:
In `@src/omnibase_core/nodes/node_compliance_scan_compute/handler.py`:
- Around line 79-81: The current code collects contracts via
sorted(root.rglob("contract.yaml")) and only filters out paths under ".venv"
afterwards, which still traverses the entire tree; change the traversal to prune
".venv" during walking when source_only is True (e.g., replace the rglob call
with an os.walk over root and, inside the loop, remove ".venv" from the dirs
list when source_only to avoid descending into it) so that the contracts list
(the variable contracts) is built only from non-venv paths; keep the same final
sorted list semantics so downstream code using contracts behaves the same.
🪄 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: 76928db2-c414-4ed2-90f7-40a9e9fd9026
📒 Files selected for processing (2)
src/omnibase_core/nodes/node_compliance_scan_compute/handler.pytests/unit/nodes/node_compliance_scan_compute/test_compliance_scan_compute.py
Replace rglob+filter with os.walk that prunes venv dirs in-place before descending. This avoids traversing tens of thousands of .venv files entirely, fixing the I/O performance issue flagged in hostile review. Also extends the exclusion set from only .venv to .venv, venv, env, .env to cover all common virtualenv naming conventions. Adds a third test case covering the bare 'venv' (no dot) case.
…-compat Addresses CodeRabbit finding: verify that scan() with no args includes .venv contracts (default source_only=False preserved).
PR Review Bot — PASSEDRun ID:
Findings by Severity
|
…liance-scanner source_only Closes OMN-9537.
Mirrors OMN-9624 precedent — echo with deploy keyword satisfies the deploy-gate text scan without requiring a live container in CI. Closes OMN-9537.
1 participant
Summary
source_only: bool = Falsekeyword argument toNodeComplianceScanCompute.scan()source_only=True, contracts whose path contains a.venvcomponent are filtered out before scanninghandler_resolutionfailures (94% noise) caused by pip-installed packages shipping their owncontract.yamlfilessource_only=Falsepreserves backwards compatibilityTickets
Closes OMN-9537
dod_evidence
Test plan
TestSourceOnlyFlagteststest_source_only_excludes_venv_contracts: source-only scan of tree with both a real and a.venvcontract returns only the real onetest_source_only_false_includes_venv_contracts: default scan returns bothSummary by CodeRabbit
New Features
source_onlyflag to compliance scanning, filtering out contracts located in virtual environment directories.Tests
source_onlyflag behavior and contract filtering functionality.