test(cli): parse Click JSON output robustly in tests

[mldangelo-oai]

Summary

• add a shared test helper that parses CliRunner output even when Click prepends merged stderr log lines before the JSON body
• use it in brittle JSON assertions for finding-producing scans
• add a subprocess regression proving real CLI execution still keeps JSON on stdout and finding logs on stderr

Validation

• uv run pytest tests/test_cli_output.py tests/test_cli.py tests/test_nested_pickle_integration.py -q
• uv run ruff check tests/cli_output.py tests/test_cli_output.py tests/test_cli.py tests/test_nested_pickle_integration.py tests/conftest.py
• uv run mypy modelaudit/

Summary by CodeRabbit

• Tests
  • Added a CLI JSON-output parsing helper and unit tests to reliably extract JSON payloads from mixed log/output streams.
  • Updated integration and subprocess tests to use the new parser, ensuring JSON results are correctly interpreted even with prefixed log lines.
  • Added checks that issues are reported in JSON and that corresponding human-readable markers appear in console logs.

[coderabbitai]

Warning

Rate limit exceeded

@mldangelo-oai has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 8 minutes and 20 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

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 configuration

Configuration used: Repository UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: ca7adf25-9127-45c8-a95f-a12795b6151a

📥 Commits

Reviewing files that changed from the base of the PR and between 9d88fd3 and 77bc224.

📒 Files selected for processing (1)

• tests/test_cli.py

Walkthrough

Added a resilient JSON parsing helper to extract JSON objects from Click CLI output that may contain interleaved logs; refactored tests to use it and added unit and subprocess tests to validate parsing and log/JSON separation.

Changes

Cohort / File(s)	Summary
JSON Output Parsing Helper tests/cli_output.py, tests/test_cli_output.py	New parse_click_json_output(output: str) -> dict[str, Any] that locates and decodes a JSON object within Click output, tolerates non-JSON prefixes, requires the decoded value be a dict, and includes unit tests for mixed log+JSON input and type validation.
Test Configuration tests/conftest.py	Added tests/test_cli_output.py to the allowed test files whitelist for Python 3.10, 3.12, and 3.13 so the new tests run on those interpreters.
CLI Tests tests/test_cli.py	Replaced direct json.loads(result.output) calls with parse_click_json_output(...); added a subprocess-based test that runs the CLI with --format json, asserts exit code 1, parses JSON from stdout, and checks stderr contains the findings marker. Minor variable rename in tar test setup.
Nested Pickle Integration Test tests/test_nested_pickle_integration.py	Replaced json.loads(...) with parse_click_json_output(...) to obtain output_data["issues"]; existing filtering and assertions unchanged.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~22 minutes

Poem

🐇 I sniff the logs where stray bytes hide,
I find the braces where the JSON resides.
A hop, a parse, the payload is freed—
Clean data for tests, just what I need! 🥕

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately describes the main change: adding robust JSON parsing for Click CLI output in tests. It is specific, concise, and directly relates to the primary objective of the pull request.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch test/robust-cli-json-parsing

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

🤖 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/test_cli.py`:
- Around line 183-204: The test
test_scan_json_subprocess_separates_logs_from_stdout_for_findings reuses the
name payload for both the temporary file (payload variable created for the tar)
and the parsed JSON result (payload = json.loads(...)), which is confusing;
rename one of them (e.g., rename the file variable to payload_file or the parsed
result to result_json or parsed_payload) and update all subsequent references
(the tar.add call and the json.loads assignment plus the issue assertions and
any uses of payload) to use the new name so there is no shadowing or ambiguity.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 819de9ed-5c38-4800-af4e-ce4bbc14d8aa

📥 Commits

Reviewing files that changed from the base of the PR and between 7ee7196 and 9d88fd3.

📒 Files selected for processing (1)

• tests/test_cli.py

[mldangelo-oai]

Addressed the CodeRabbit variable-shadowing feedback in 77bc224 by renaming the tar payload path to payload_file and the parsed JSON object to output_payload in the subprocess test.