Skip to content

Extract doctor diagnostics boundary - #168

Merged
errfld merged 2 commits into
mainfrom
gh-164/doctor-diagnostics-module
Jun 23, 2026
Merged

Extract doctor diagnostics boundary#168
errfld merged 2 commits into
mainfrom
gh-164/doctor-diagnostics-module

Conversation

@errfld

@errfld errfld commented Jun 23, 2026

Copy link
Copy Markdown
Owner

Summary

  • Extract git smee doctor diagnostics models, report building, and text/JSON rendering into crates/git-smee-cli/src/doctor.rs.
  • Keep main.rs as the top-level command dispatcher for doctor, matching the status extraction pattern.
  • Add focused unit coverage for doctor status finalization while preserving existing CLI behavior and JSON fields.

Refs #164

Validation

  • Baseline before changes: cargo test -p git-smee-cli doctor (7 integration tests passed after initial cold-build timeouts were rerun to completion)
  • cargo fmt --all -- --check
  • cargo clippy --workspace --all-targets --all-features -- -D warnings
  • cargo test --workspace --all-targets --all-features
  • Manual smoke in a temporary Git repository:
    • git smee install
    • git smee doctor
    • git smee doctor --json
    • git smee status
    • git smee status --json
    • Parsed JSON status values were ok for doctor and status.

Notes

  • The local disk filled during full validation because several Rust worktree target/ directories were present. I reclaimed space using cargo clean only on rebuildable project artifacts, then reran full validation successfully.

Summary by CodeRabbit

New Features

  • Added git-smee doctor command to diagnose and validate repository setup and configuration. The command checks if the repository is properly configured, verifies hook configuration, validates managed hooks, and reports any issues found. Output can be formatted as JSON or human-readable text, with status reported as OK, Warning, or Error.

@coderabbitai

coderabbitai Bot commented Jun 23, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@errfld, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 52 minutes and 25 seconds. Learn how PR review limits work.

Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file).

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits.

🚦 How do rate limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan refill rate.

For paid Pro and Pro+ PR reviews, CodeRabbit uses rolling per-developer review limits. Reviews become available again as older review attempts age out of the rolling limit window.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 993ff79f-42fb-4c3b-9bbe-368c7bc3a3a1

📥 Commits

Reviewing files that changed from the base of the PR and between aa0d760 and 9864df6.

📒 Files selected for processing (2)
  • crates/git-smee-cli/src/doctor.rs
  • crates/git-smee-cli/tests/cli_integration.rs
📝 Walkthrough

Walkthrough

The inline doctor command implementation in main.rs (report structs, enums, helpers, ~218 lines) is extracted into a new doctor.rs module. The new module introduces DoctorReport/DoctorStatus, build_doctor_report, finish_doctor_report, output helpers, and unit tests. main.rs is updated to declare mod doctor and delegate to doctor::run_doctor.

Changes

Doctor subcommand extraction

Layer / File(s) Summary
DoctorReport model and run_doctor entrypoint
crates/git-smee-cli/src/doctor.rs
Defines DoctorReport and DoctorStatus types. run_doctor builds a report via build_doctor_report, finalizes status with finish_doctor_report, and prints either pretty JSON or human-readable sections. Returns Err when the report contains errors, Ok(()) otherwise.
Report construction and hook validation
crates/git-smee-cli/src/doctor.rs
build_doctor_report validates Git root, resolves the effective hooks directory, parses TOML config, derives expected normalized config and executable paths, then iterates configured hook phases checking wrapper file existence, type, readability, managed-marker presence, and config/executable staleness. Unit tests verify finish_doctor_report produces Ok with no findings and Error when both errors and warnings are present.
main.rs wiring
crates/git-smee-cli/src/main.rs
Drops MANAGED_FILE_MARKER import, adds mod doctor;, and updates the Doctor dispatch arm to call doctor::run_doctor. All previously inline doctor structs, enums, and helpers are removed.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related issues

Possibly related PRs

  • errfld/git-smee#162: Both PRs implement CLI reporting that inspects managed hook wrapper files and validates embedded config/executable path staleness, so the doctor checks directly overlap with the status drift/coverage logic introduced there.
  • errfld/git-smee#167: Both PRs apply the same pattern of refactoring main.rs to delegate a CLI subcommand (doctor here, status there) to a newly introduced module entrypoint.

Poem

🐇 Hoppity-hop, the doctor's set free,
No longer stuffed in main.rs with glee,
A module of its own, tidy and bright,
Each hook phase inspected with rabbit-sharp sight,
Errors and warnings now cleanly align —
This burrow of code is finally fine! 🐾

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 22.22% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'Extract doctor diagnostics boundary' accurately reflects the main change: refactoring the doctor command's diagnostics logic from main.rs into a dedicated module.
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
  • Commit unit tests in branch gh-164/doctor-diagnostics-module

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

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai 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.

Actionable comments posted: 1

🤖 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 `@crates/git-smee-cli/src/doctor.rs`:
- Around line 157-163: The check for whether a hook file is managed uses loose
substring matching with `content.contains(MANAGED_FILE_MARKER)` which does not
align with the actual behavior in the installer module. The installer's
`has_managed_header()` public API function properly validates that the marker
appears as a normalized line within the first 32 lines and 8 KB of the file,
whereas the substring check could incorrectly match the marker anywhere in the
file content. Replace the `content.contains(MANAGED_FILE_MARKER)` check at line
157 with a call to the installer's `has_managed_header(hook_path)` function to
ensure doctor's verdict matches install's actual behavior. Update the condition
logic and error handling accordingly to work with the function's Result return
type.
🪄 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: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 66f8aff3-538b-4dd2-98b4-c754216fc968

📥 Commits

Reviewing files that changed from the base of the PR and between 6e5be5c and aa0d760.

📒 Files selected for processing (2)
  • crates/git-smee-cli/src/doctor.rs
  • crates/git-smee-cli/src/main.rs

Comment thread crates/git-smee-cli/src/doctor.rs Outdated
@errfld
errfld merged commit 4fc8bcd into main Jun 23, 2026
28 checks passed
@errfld
errfld deleted the gh-164/doctor-diagnostics-module branch June 23, 2026 06:35
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.

2 participants