fix: mark incomplete MXNet scans inconclusive#923
fix: mark incomplete MXNet scans inconclusive#923mldangelo-oai wants to merge 2 commits intomainfrom
Conversation
|
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 14 minutes and 8 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: Repository UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
WalkthroughThe changes implement explicit handling of inconclusive MXNet artifact scans. When artifact reads or parsing fail (truncation, corruption, JSON parse errors, etc.), scans are marked as inconclusive with tracked reasons. Results fail only if analysis is inconclusive and no security findings were recovered. Changes
Sequence Diagram(s)sequenceDiagram
participant Scanner as MXNetScanner
participant Result as ScanResult
participant Metadata as metadata dict
Scanner->>Scanner: scan artifact
alt Artifact Read/Parse Succeeds
Scanner->>Result: process findings
Scanner->>Result: finish with success status
else Artifact Read Fails (truncated/empty/invalid)
Scanner->>Scanner: _mark_inconclusive_scan_result()
Scanner->>Metadata: set analysis_incomplete = True
Scanner->>Metadata: append reason to scan_outcome_reasons
Scanner->>Scanner: _scan_result_has_security_findings()
alt Found Security Findings
Scanner->>Result: finish as success
else No Security Findings
Scanner->>Result: finish as failure
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
Performance BenchmarksCompared Top improvements:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tests/scanners/test_mxnet_scanner.py (1)
1-228: 🧹 Nitpick | 🔵 TrivialConsider adding coverage for remaining inconclusive reasons.
The new tests cover
mxnet_params_empty,mxnet_params_truncated, andmxnet_symbol_parse_failed. For comprehensive regression coverage, consider adding tests for the other inconclusive paths in a follow-up:
mxnet_unsupported_extensionmxnet_symbol_read_failed(OSError on read)mxnet_symbol_truncated(bounded-read truncation)mxnet_symbol_emptymxnet_symbol_invalid_structuremxnet_params_read_failed(OSError on read)Would you like me to open an issue to track adding these additional test cases?
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/scanners/test_mxnet_scanner.py` around lines 1 - 228, Add unit tests exercising the remaining inconclusive scan reasons by creating small test functions that call MXNetScanner.scan and scan_model_directory_or_file similar to existing tests: (1) mxnet_unsupported_extension — create a file with a non-matching extension and assert metadata["scan_outcome_reasons"] contains "mxnet_unsupported_extension"; (2) mxnet_symbol_read_failed and mxnet_params_read_failed — simulate OSError on read by monkeypatching Path.read_text/Path.read_bytes (or open) to raise OSError and assert the appropriate reason is set; (3) mxnet_symbol_truncated — write a partially truncated JSON (bounded-read style) and assert "mxnet_symbol_truncated"; (4) mxnet_symbol_empty — write an empty symbol file and assert "mxnet_symbol_empty"; (5) mxnet_symbol_invalid_structure — write valid JSON that lacks required keys (e.g., missing "nodes"/"heads") and assert "mxnet_symbol_invalid_structure". Use the same helper functions (_write_symbol_file/_write_params_file), MXNetScanner.scan, scan_model_directory_or_file, and determine_exit_code to validate outcomes and exit codes where applicable.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@tests/scanners/test_mxnet_scanner.py`:
- Around line 1-228: Add unit tests exercising the remaining inconclusive scan
reasons by creating small test functions that call MXNetScanner.scan and
scan_model_directory_or_file similar to existing tests: (1)
mxnet_unsupported_extension — create a file with a non-matching extension and
assert metadata["scan_outcome_reasons"] contains "mxnet_unsupported_extension";
(2) mxnet_symbol_read_failed and mxnet_params_read_failed — simulate OSError on
read by monkeypatching Path.read_text/Path.read_bytes (or open) to raise OSError
and assert the appropriate reason is set; (3) mxnet_symbol_truncated — write a
partially truncated JSON (bounded-read style) and assert
"mxnet_symbol_truncated"; (4) mxnet_symbol_empty — write an empty symbol file
and assert "mxnet_symbol_empty"; (5) mxnet_symbol_invalid_structure — write
valid JSON that lacks required keys (e.g., missing "nodes"/"heads") and assert
"mxnet_symbol_invalid_structure". Use the same helper functions
(_write_symbol_file/_write_params_file), MXNetScanner.scan,
scan_model_directory_or_file, and determine_exit_code to validate outcomes and
exit codes where applicable.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 035369bb-6f8a-477d-8078-5160ca90e370
📒 Files selected for processing (3)
CHANGELOG.mdmodelaudit/scanners/mxnet_scanner.pytests/scanners/test_mxnet_scanner.py
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
|
|
||
| import pytest | ||
|
|
||
| import modelaudit.scanners.mxnet_scanner as mxnet_scanner |
1 participant
Summary
This PR fixes an aggregate false-clean result in the MXNet scanner. Corrupt or truncated MXNet artifacts could produce direct scanner success=false while the aggregate scan still reported success and exit code 0 because the failure was INFO-level and lacked explicit scan_outcome=inconclusive metadata.
Root Cause
The MXNet scanner returned analysis_complete=false for unreadable, malformed, empty, invalid, or truncated artifacts, but it did not attach scan outcome metadata. ModelAudit exit-code aggregation intentionally treats INFO findings as non-security findings, so incomplete MXNet scans without WARNING/CRITICAL findings could be collapsed into a clean aggregate result.
Changes
Validation
Summary by CodeRabbit
Release Notes
Bug Fixes
Tests