ci: enforce interrogate docstring coverage and restructure CI - #65
Conversation
|
Warning Review limit reached
Next review available in: 42 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the 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 reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (5)
📝 WalkthroughWalkthroughThe changes consolidate linting into the CI workflow, add interrogate configuration and prek integration, trigger checks for container-file changes, and add Dockerfile validation through hadolint. The separate lint workflow is removed. ChangesCI linting consolidation
Estimated code review effort: 3 (Moderate) | ~20 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #65 +/- ##
=======================================
Coverage 99.74% 99.74%
=======================================
Files 39 39
Lines 7144 7144
Branches 411 411
=======================================
Hits 7126 7126
Misses 13 13
Partials 5 5 ☔ View full report in Codecov by Harness. |
PR Summary by QodoCI: enforce interrogate docstring coverage and streamline workflows
AI Description
Diagram
High-Level Assessment
Files changed (3)
|
Code Review by Qodo
Context used✅ Compliance rules (platform):
31 rules 1. interrogate rationale undocumented
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
| dev = [ | ||
| "any-llm-sdk[all]>=1.19,<2", | ||
| "anyio>=4.11,<5", | ||
| "interrogate>=1.7,<2", |
There was a problem hiding this comment.
1. interrogate rationale undocumented 📘 Rule violation ⚙ Maintainability
A new third-party dev dependency interrogate was added without a documented rationale explaining its purpose and where it is used. This violates the dependency-rationale requirement and makes auditing/maintenance of dependency choices harder.
Agent Prompt
## Issue description
A new third-party dependency `interrogate` was added, but the repo does not document why it is needed and what feature/module uses it.
## Issue Context
Compliance requires each new dependency to have a documented rationale (purpose + where used). `interrogate` is introduced as a dev dependency and is wired into CI/prek, but there is no rationale recorded alongside the dependency declaration or in a canonical dependency rationale document.
## Fix Focus Areas
- pyproject.toml[28-35]
- docs/notes.md[139-142]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| - ".github/workflows/ci.yml" | ||
| - "tests/**" | ||
| - "weather_briefing/**" | ||
| - "Dockerfile" | ||
| - ".dockerignore" | ||
| - "prek.toml" | ||
| - "pyproject.toml" | ||
| - "uv.lock" |
There was a problem hiding this comment.
2. Ci checks bypassed by paths 🐞 Bug ⛨ Security
The CI workflow is now the only place running ruff/interrogate/gitleaks, but it only triggers for pull requests whose changed files match a small allowlist, so PRs changing other files can skip linting, docstring coverage, and secret scanning entirely.
Agent Prompt
## Issue description
`.github/workflows/ci.yml` only runs on pull requests that touch a limited set of paths. Since this workflow contains the repo’s `ruff`, `interrogate`, `prek`, and `gitleaks` steps, any PR that doesn’t touch one of the allowlisted paths will bypass these checks.
## Issue Context
This regression matters more now because `lint.yml` was deleted; checks that used to run on all PRs are now gated by `ci.yml`’s allowlist.
## Fix Focus Areas
- .github/workflows/ci.yml[4-18]
### Suggested fix options
- Remove `on.pull_request.paths` from `ci.yml` so the workflow runs on all PRs.
- Or switch to `paths-ignore` to only skip truly irrelevant files.
- Or split security-critical scanning (e.g., gitleaks) into a separate workflow that runs on all PRs/pushes regardless of changed paths.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| pull-requests: read | ||
| checks: write | ||
|
|
||
| on: |
There was a problem hiding this comment.
3. No lint on master push 🐞 Bug ☼ Reliability
After removing the dedicated lint workflow, the remaining CI workflow only runs on pushes to renovate/** (not master), so ruff/interrogate/prek/gitleaks won’t run on main-branch push events (including post-merge pushes).
Agent Prompt
## Issue description
The repo’s lint/security checks are in `.github/workflows/ci.yml`, but that workflow does not trigger on pushes to `master`. With `lint.yml` deleted, there’s no longer a push-to-master validation for ruff/interrogate/prek/gitleaks.
## Issue Context
This impacts direct pushes and the push event that occurs when PRs are merged into `master`.
## Fix Focus Areas
- .github/workflows/ci.yml[4-8]
### Suggested fix
Add `master` to `on.push.branches` in `ci.yml` (and optionally keep `renovate/**`). If you intentionally only want PR-time validation, document that decision and consider keeping a lightweight post-merge workflow for at least gitleaks/ruff to protect the main branch.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Move the Dockerfile hadolint step into the CI workflow with job-scoped permissions (pull-requests: read, checks: write) so the workflow keeps minimal grants. Extend the CI triggers to master pushes and Dockerfile and .dockerignore changes. Drop lint.yml; its ruff-action steps are redundant with the existing uv-run ruff check and the prek ruff hooks.
Drop pytest and the Python version matrix from the CI workflow; unit tests and coverage are the unittest workflow's responsibility. Run the remaining checks (lock, prek, ruff, gitleaks) on the latest Python 3.x and rename the job to lint to reflect its purpose.
Add interrogate 1.7 as a development dependency and configure it under [tool.interrogate] to mirror the ruff D rules: google style, ignore semiprivate, private, and nested functions, fail-under 100, and exclude tests and tooling directories. The config is verified to match ruff's --select D scope (both report full coverage of weather_briefing). Wire the check into both the prek hook suite (pass_filenames=false, explicit weather_briefing path) and the CI lint job. The hook env pins interrogate at the 1.7.0 tag while the dev dependency lets the explicit CI step and local uv run share the same configuration.
2013367 to
c287a0f
Compare
1 participant
Summary
Introduce
interrogatefor docstring coverage, sync its scope with the existing ruffDrules, and restructure the CI/Lint/Unittest workflows into clear, non-overlapping roles.Changes
ci: merge lint workflow into CIhadolintstep intoci.ymlas its own job with job-scopedpermissions: { pull-requests: read, checks: write }; the workflow keepspermissions: {}.masterpushes andDockerfile/.dockerignorechanges.lint.yml. Itsruff-actionsteps are redundant withuv run ruff checkand the prek ruff hooks.ci: confine pytest to unittest workflowpytestand the Python version matrix fromci.yml— unit tests + coverage areunittest.ymls job.python-version: "3.x"+check-latest: true) and rename the jobtest→lint.feat(ci): enforce docstring coverage with interrogateinterrogate>=1.7,<2dev dependency +[tool.interrogate]configured to mirror ruffD:style = "google", ignore semiprivate/private/nested functions,fail-under = 100, excludetests/tooling dirs.pass_filenames = false, explicitweather_briefingpath) and the CIlintjob (uv run interrogate weather_briefing).Verification
The interrogate config is verified to match
ruff --select Dexactly: both report 100% coverage ofweather_briefing(ruffDdoes not check private/semiprivate/nested functions ortests/**, and the interrogate flags mirror that).Local checks all green:
uv lock --check/uv sync --frozenuv run ruff check→ all checks passeduv run interrogate weather_briefing→ PASSED (100.0%, minimum 100.0)prek run --all-files→ all 16 hooks pass (incl. actionlint, TOML/YAML validation)uv run pytest→ 664 passedNotes
unittest.yml= unit tests + coverage (matrix 3.11–3.14 × ubuntu/macOS);ci.yml= lint/format/lock/prek/interrogate/gitleaks + hadolint.Summary by CodeRabbit