Improve CyclomaticComplexityAssessor with streaming subprocess and lizard scoring

[mikebonnet]

Description

Summary

• Add StreamingSubprocess / safe_subprocess_run_stream() to stream subprocess stdout line-by-line instead of buffering, with background stderr accumulation, watchdog timeout, and the same security guardrails as safe_subprocess_run()
• Migrate radon, lizard, and gocyclo execution in CyclomaticComplexityAssessor to use streaming, reducing peak memory on large repos
• Implement actual lizard output parsing and scoring (was previously a not_applicable stub), using -i -1 (all languages) and -t N (parallel threads matching CPU count)
• Reorder Go complexity assessment to check golangci-lint config before falling back to gocyclo (avoids running a binary that may not be installed)
• Allow sanitize_subprocess_error() to accept raw strings in addition to exceptions
• Remove redundant except MissingToolError: raise blocks
• Raise CalledProcessError (with sanitized stdout/stderr) instead of MissingToolError on non-zero return codes, so tool-missing vs tool-failed errors are distinguished
• Parse radon average from the last output line only instead of scanning the full buffer

Type of Change

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)
• Documentation update
• Refactoring (no functional changes)
• Performance improvement
• Test coverage improvement

Changes Made

• chore: remove redundant except MissingToolError re-raises
• feat: include sanitized stdout in CyclomaticComplexityAssessor errors
• refactor: allow sanitize_subprocess_error to accept strings directly
• fix: add -i -1 flag and returncode check to lizard assessment
• refactor: parse radon average from last line only
• refactor: improve _assess_go_complexity
• fix: replace MissingToolError with CalledProcessError for radon failures
• feat: implement lizard output parsing and remove returncode check
• refactor: use safe_subprocess_run_stream in CyclomaticComplexityAssessor
• feat: add safe_subprocess_run_stream for streaming subprocess output

Testing

Test plan

• pytest tests/unit/utils/test_subprocess_utils.py — 18 new tests covering streaming basics, timeout, stderr accumulation, size limits, cleanup, early close, idempotent close, and kwarg stripping

• pytest tests/unit/test_assessors_code_quality.py — existing cyclomatic complexity tests still pass

• pytest --cov=src/agentready — verify coverage for new code

• Run agentready assess against a real repo with Python, Go, and multi-language files to validate radon/lizard/gocyclo scoring end-to-end

• Unit tests pass (pytest)

• Integration tests pass

• Manual testing performed

• No new warnings or errors

Checklist

• My code follows the project's code style
• I have performed a self-review of my own code
• I have commented my code, particularly in hard-to-understand areas
• I have made corresponding changes to the documentation
• My changes generate no new warnings
• I have added tests that prove my fix is effective or that my feature works
• New and existing unit tests pass locally with my changes
• Any dependent changes have been merged and published

Summary by CodeRabbit

• Refactor

  • Improved error reporting and robustness in code quality assessments, yielding clearer diagnostics and more reliable complexity scores.
  • Subprocess execution now streams output to reduce resource use and improve responsiveness during analysis.
  • Enhanced complexity analysis across Python, Go, and other codebases for more accurate results.

• Tests

  • Added comprehensive tests covering streaming subprocess behavior, timeouts, error handling, and cleanup.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Enterprise

Run ID: 1aa0986f-d3cc-4b04-83c1-241b5567c03b

📥 Commits

Reviewing files that changed from the base of the PR and between 68035dc and fa12171.

📒 Files selected for processing (2)

• src/agentready/assessors/code_quality.py
• src/agentready/utils/subprocess_utils.py

---

📝 Walkthrough

Walkthrough

Adds a StreamingSubprocess API and safe_subprocess_run_stream, re-exports streaming helpers, tests streaming semantics, and refactors Python/Lizard/Go complexity assessors to stream tool output and parse average complexity from final lines with improved sanitized error reporting.

Changes

Streaming subprocess infrastructure and integration

Layer / File(s)	Summary
Core streaming subprocess API and error handling src/agentready/utils/subprocess_utils.py	StreamingSubprocess wraps Popen to iterate stdout lines, collects stderr asynchronously with a byte limit, enforces timeouts, and exposes cleanup. safe_subprocess_run_stream provides guarded startup and defaults. sanitize_subprocess_error accepts `Exception
Public exports for streaming helpers src/agentready/utils/__init__.py	Re-exports safe_subprocess_run_stream, StreamingSubprocess, sanitize_subprocess_error, and validate_repository_path from subprocess_utils.
Complexity assessor imports wiring src/agentready/assessors/code_quality.py	Assessor module imports updated to use streaming helpers and sanitized error handling.
Python radon assessor (streamed) src/agentready/assessors/code_quality.py	Runs radon via streaming, captures the last emitted line to parse Average complexity:, and raises sanitized CalledProcessError on non-zero exit; missing-tool yields MissingToolError("radon", install_command="pip install radon").
Lizard assessor (streamed) src/agentready/assessors/code_quality.py	Runs lizard with -t <cpu_count> via streaming, extracts average complexity from the final token (or returns not-applicable), computes score/status/remediation, and raises sanitized CalledProcessError on non-zero exit; missing-tool mapped to MissingToolError("lizard", ...).
Go gocyclo fallback (streamed) src/agentready/assessors/code_quality.py	Runs gocyclo via streaming (-top 1), parses Average: from the final line to compute score/status, maps FileNotFoundError to MissingToolError("gocyclo", install_command=...), and returns Finding.error for other exceptions.
Test coverage for streaming subprocess API tests/unit/utils/test_subprocess_utils.py	TestSafeSubprocessRunStream exercises iterator/context-manager semantics, line streaming, return codes, stderr accumulation, shell-security blocking, timeout and stderr-size enforcement, cleanup, cwd handling, Popen kwarg passthrough, missing-command handling, default timeout, empty stdout, and idempotent close behavior.

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name	Status	Explanation	Resolution
Title check	⚠️ Warning	Title uses natural language without Conventional Commits format (type(scope): description); should follow: feat/fix/refactor(scope): description.	Reformat title as: refactor(assessor): stream subprocess for CyclomaticComplexityAssessor with lizard scoring
Docstring Coverage	⚠️ Warning	Docstring coverage is 68.29% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

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

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

✨ Simplify code

• Create PR with simplified code

---

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: 3

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

src/agentready/assessors/code_quality.py (1)

658-659: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Missing tools should degrade this assessor to skipped, not escape as exceptions.

These branches re-raise MissingToolError for radon, lizard, and gocyclo. That pushes tool availability out of the assessor boundary instead of returning a skipped finding for this attribute.

As per coding guidelines, src/agentready/assessors/*.py: "Return 'skipped' status for graceful degradation when required tools are missing, never crash."

Also applies to: 725-726, 818-822

🤖 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 `@src/agentready/assessors/code_quality.py` around lines 658 - 659, Replace the
except FileNotFoundError handlers that currently raise MissingToolError for
radon, lizard, and gocyclo with logic that returns a "skipped" assessment result
so the assessor gracefully degrades instead of propagating an exception;
specifically, in the except blocks for the radon/lizard/gocyclo tool invocations
inside src/agentready/assessors/code_quality.py (the FileNotFoundError handlers
shown and the similar handlers around the other occurrences) return a
finding/assessment object or status marked skipped (consistent with other
assessors' skipped pattern) and include a short message that the external tool
is unavailable rather than raising MissingToolError.

Source: Coding guidelines

🤖 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 `@src/agentready/utils/subprocess_utils.py`:
- Around line 193-200: The normal-success path currently skips stream cleanup
because _finalize() sets self._closed before __exit__()/close() can run
_cleanup(), leaking descriptors; change the lifecycle so cleanup always runs:
either have _finalize() call self.close() (so close() performs _cleanup and
joins the watchdog) instead of setting self._closed directly, or modify close()
to run _cleanup() even if _closed is True (and ensure _closed is set only after
_cleanup()), and confirm the watchdog join happens inside _cleanup()/close();
apply the same change for the other identical block handling lines 259-262.
- Around line 123-188: The streamed stdout path in __next__ currently returns
full lines without enforcing _max_output_size, so add a byte counter for stdout
(e.g., self._stdout_bytes_read) and check length of each read line (preferably
bytes length via encoding or reading as bytes) against _max_output_size before
returning; if returning the line would exceed the cap, set self._stderr_error
(or raise/SubprocessSecurityError) and perform the same cleanup/timed-out
behavior used for stderr overflows, ensuring the check is also applied in the
other streaming loop referenced (the similar read loop around lines 202-213) so
large newline-free output cannot exceed the advertised cap.

---

Outside diff comments:
In `@src/agentready/assessors/code_quality.py`:
- Around line 658-659: Replace the except FileNotFoundError handlers that
currently raise MissingToolError for radon, lizard, and gocyclo with logic that
returns a "skipped" assessment result so the assessor gracefully degrades
instead of propagating an exception; specifically, in the except blocks for the
radon/lizard/gocyclo tool invocations inside
src/agentready/assessors/code_quality.py (the FileNotFoundError handlers shown
and the similar handlers around the other occurrences) return a
finding/assessment object or status marked skipped (consistent with other
assessors' skipped pattern) and include a short message that the external tool
is unavailable rather than raising MissingToolError.

🪄 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: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Enterprise

Run ID: de8d9a47-e394-4673-a210-69fae340d482

📥 Commits

Reviewing files that changed from the base of the PR and between 710169f and 68035dc.

📒 Files selected for processing (4)

• src/agentready/assessors/code_quality.py
• src/agentready/utils/__init__.py
• src/agentready/utils/subprocess_utils.py
• tests/unit/utils/test_subprocess_utils.py