Repository navigation
feat: disable repair command and simplify report output - #25
Conversation
- Disable 'inspect repair' command entry point - Remove score/grade from report section headers - Remove Impact/Fix columns from markdown report and terminal output - Align scanner prompts with pre-inspect-extract classification (Artifact/Carrier/Scaffolding/Reference, extraction tiers, completeness/evidence fields on tool/prompt findings) - Add completeness and evidence fields to Tool/Prompt content models - Update help panel, README, and tests
There was a problem hiding this comment.
🤖 Claude Code Review
PR #25 Code Review
Critical Issues
1. Dead Code: Disabled Repair Command
File: src/sanityops_cli/commands/inspect.py (line 280)
The inspect_repair function is commented out with # @inspect_app.command("repair") but the entire function remains. This creates dead code that will never be executed. Either:
- Remove the function entirely, or
- Add proper deprecation handling (raise a clear deprecation error when called)
Recommendation: Remove the function and all related helpers (_find_latest_report, _write_repairs_markdown) if they're no longer needed, or implement proper deprecation messaging.
Important Issues
2. Inconsistent Completeness Enum Values
File: src/sanityops_cli/agents/scanner_agent/models/finding.py (lines 117-125, 134-141)
ToolContent.completeness and PromptContent.completeness have different valid values:
- ToolContent:
full original | assembled | incomplete | summary | not inspectable - PromptContent:
full original | original · variable unexpanded | assembled | incomplete | summary | not inspectable
The original · variable unexpanded value is only documented for PromptContent, but prompts in prompts.py reference it inconsistently. Consider:
- Creating a shared
Completenessenum - Or documenting why they differ
3. Orphaned Test Code
File: tests/commands/test_inspect_repair.py (lines 27-72)
TestFindLatestReport tests _find_latest_report, which appears to be helper code for the disabled repair command. If repair is permanently removed, these tests are testing dead code.
4. Defect Fields Still Generated but Not Displayed
File: src/sanityops_cli/defect_checker/markdown_reporter.py (line 320)
The impact and fix_suggestion fields are removed from output, but defects may still contain these fields. The test helper _defect still generates them. Confirm that downstream systems no longer need this data, otherwise this is a breaking change.
Minor Issues
5. PEP 257 Blank Line After Docstring
File: src/sanityops_cli/agents/scanner_agent/models/finding.py (lines 31-32, 110-111)
Added blank lines after docstrings in Section and ToolContent classes. Per PEP 257, blank lines after docstrings are only needed when followed by additional code, but the style is inconsistent with other classes in the file.
6. Test Helper Contains Unused Fields
File: tests/unit/defect_checker/test_markdown_reporter.py (lines 44-57)
The _defect helper still includes impact and fix_suggestion parameters that are no longer displayed in reports. Consider removing these from the helper to align with the new output format.
Overall Assessment
Ready to merge: With fixes
The PR introduces valuable extraction fidelity tracking (completeness, evidence) and removes the repair workflow cleanly. However, it leaves behind dead code (disabled repair command function and its tests) that should either be removed or properly deprecated. The inconsistency in completeness field values between models should also be resolved.
Summary
inspect repaircommand entry point (decorator commented out, code kept)completenessandevidencefields toToolContent/PromptContentmodels (defaulted, backward compatible)Test plan
uv run pytest tests/ -q(356 passed)uv run ruff checkanduv run ruff format --checkclean on changed filesinspect --helpshows the 3-step Getting Started panel without repairinspect repair --helpfails with "No such command 'repair'"