Skip to content

Fix inspect defect check level at L3 and drop --check-level flag - #26

Merged
zipsonken merged 1 commit into
mainfrom
feat/fix-inspect-level-l3
Nov 5, 2026
Merged

zipsonken merged 1 commit into
mainfrom
feat/fix-inspect-level-l3

Conversation

@zipsonken

Copy link
Copy Markdown
Collaborator

Summary

The defect check depth is no longer user-configurable: every inspect run now executes at the fixed deep L3 level.

  • Remove the --check-level option and its L1/L2/L3 validation from the inspect command
  • Add a DEFAULT_CHECK_LEVEL constant (L3) used by both the defect check and the saved markdown report
  • Change DefectChecker.check and save_markdown_report defaults from L2 to L3
  • Update README: remove --check-level examples and the Check Levels table
  • Add regression tests covering the removed flag and the fixed L3 level
  • Record the breaking change in CHANGELOG

Test plan

  • Full test suite passes (358 tests)
  • ruff check clean on all changed files
  • inspect --help no longer lists --check-level
  • New regression tests: unknown --check-level flag is rejected (exit code 2); defect check always receives L3

The defect check depth is no longer user-configurable. Every inspect
run now executes at the deep L3 level:

- Remove the --check-level option and its L1/L2/L3 validation from the
  inspect command
- Add a DEFAULT_CHECK_LEVEL constant (L3) used by both the defect check
  and the saved markdown report
- Change DefectChecker.check and save_markdown_report defaults from L2
  to L3
- Update README: remove check-level examples and the Check Levels table
- Add regression tests covering the removed flag and the fixed L3 level
- Record the breaking change in CHANGELOG

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 Claude Code Review

Code Review: PR #26 — Remove --check-level option

Summary

This PR removes the --check-level CLI option and fixes defect checks at L3 depth. The change is properly documented as breaking in the CHANGELOG.


Findings

Important

  1. Inconsistent default in test mock — tests/unit/commands/test_inspect.py:386
    async def fake_check(self, findings, check_level="L2"):
    The mock function's default is "L2" while the actual implementation now defaults to "L3". While the test passes (L3 is explicitly passed), this is misleading for future maintainers. Update to check_level="L3" for consistency.

Minor

  1. Scattered default value — DEFAULT_CHECK_LEVEL is defined in inspect.py but checker.py and markdown_reporter.py hardcode "L3" in their function signatures. Consider centralizing the constant (e.g., in a shared constants module) to avoid drift if the level ever changes.

  2. No config file migration/deprecation — If users had check_level in their inspect_config.yaml, this change doesn't address it. Consider whether config file support existed and if a deprecation warning should be added when parsing configs.


Positive Observations

  • Breaking change properly documented in CHANGELOG with clear migration implication
  • New tests verify both the removal of the flag and the L3 enforcement
  • README updated consistently
  • All existing tests updated to use L3

Overall Assessment

Ready to merge: With fixes

Fix the test mock inconsistency (Important #1). The other findings are optional improvements for maintainability.

@zipsonken
zipsonken merged commit af75fcb into main Nov 5, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant