feat: replaced vulture with grimp - better deadcode detection#242
Merged
feat: replaced vulture with grimp - better deadcode detection#242
Conversation
📝 WalkthroughWalkthroughThis PR consolidates backend import paths by moving repository and producer imports from deeper modules ( Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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.
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
Pull request overview
This PR replaces Vulture-based dead-code detection with a Grimp-based “orphan module” check in the backend CI/tooling, and refactors backend imports to use app.db / app.events re-export facades.
Changes:
- Swap Vulture CI/docs/pre-commit configuration for a Grimp orphan-module check, implemented via a new backend script.
- Remove the Vulture whitelist/config and the (now-unused)
consumer_group_monitormodule. - Refactor many backend imports to use
from app.db import …andfrom app.events import UnifiedProducer.
Reviewed changes
Copilot reviewed 33 out of 34 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| docs/operations/cicd.md | Updates CI diagram/docs to reference Grimp and the new local command. |
| backend/vulture_whitelist.py | Removes Vulture whitelist file (no longer needed). |
| backend/uv.lock | Adds grimp and removes vulture from the lockfile/groups. |
| backend/scripts/check_orphan_modules.py | Adds Grimp-based orphan module detection script used by CI/pre-commit. |
| backend/pyproject.toml | Replaces vulture dependency/config with grimp and removes vulture-specific exclusions. |
| backend/app/services/user_settings_service.py | Switches repository import to app.db facade. |
| backend/app/services/sse/sse_service.py | Switches repository import to app.db facade. |
| backend/app/services/saved_script_service.py | Switches repository import to app.db facade. |
| backend/app/services/saga/saga_service.py | Switches repository imports to app.db facade. |
| backend/app/services/saga/saga_orchestrator.py | Switches repository imports to app.db facade and producer import to app.events. |
| backend/app/services/saga/execution_saga.py | Switches repository import to app.db facade and producer import to app.events. |
| backend/app/services/runtime_settings.py | Switches repository import to app.db facade. |
| backend/app/services/result_processor/processor.py | Switches repository import to app.db facade and producer import to app.events. |
| backend/app/services/notification_service.py | Switches repository import to app.db facade. |
| backend/app/services/notification_scheduler.py | Switches repository import to app.db facade. |
| backend/app/services/kafka_event_service.py | Switches producer import to app.events. |
| backend/app/services/k8s_worker/worker.py | Switches producer import to app.events. |
| backend/app/services/execution_service.py | Switches repository import to app.db facade and producer import to app.events. |
| backend/app/services/event_service.py | Switches repository import to app.db facade. |
| backend/app/services/event_replay/replay_service.py | Switches repository import to app.db facade and producer import to app.events. |
| backend/app/services/auth_service.py | Switches repository import to app.db facade and producer import to app.events. |
| backend/app/services/admin/admin_user_service.py | Switches repository import to app.db facade. |
| backend/app/services/admin/admin_settings_service.py | Switches repository import to app.db facade. |
| backend/app/services/admin/admin_execution_service.py | Switches repository import to app.db facade. |
| backend/app/services/admin/admin_events_service.py | Switches repository import to app.db facade. |
| backend/app/events/core/producer.py | Switches repository import to app.db facade. |
| backend/app/events/consumer_group_monitor.py | Removes unused module (consistent with orphan-module cleanup). |
| backend/app/dlq/manager.py | Switches repository import to app.db facade. |
| backend/app/db/repositories/init.py | Refactors admin repository imports via app.db.repositories.admin. |
| backend/app/db/init.py | Expands/maintains app.db re-export surface used by updated imports. |
| backend/app/core/providers.py | Switches imports to app.db and app.events facades. |
| AGENTS.md | Updates developer commands/docs from Vulture to the new Grimp script. |
| .pre-commit-config.yaml | Replaces Vulture hook with Grimp orphan-module check hook. |
| .github/workflows/grimp.yml | Adds Grimp workflow to run the orphan-module check in CI. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
backend/pyproject.toml (1)
146-146: Consider pinning grimp to a specific version for reproducible builds.The dependency uses
grimp>=3.14which allows automatic patch and minor version upgrades. Other lint dependencies in this file (mypy, ruff) are pinned to exact versions (e.g.,mypy==1.19.1,ruff==0.14.10). For consistency and to prevent unexpected breaking changes in CI, consider usinggrimp==3.14instead.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/pyproject.toml` at line 146, Replace the loose dependency spec "grimp>=3.14" in pyproject.toml with a pinned exact version to ensure reproducible builds; update the entry "grimp>=3.14" to "grimp==3.14" so it matches the pinning style used for other linters (e.g., mypy==1.19.1, ruff==0.14.10).backend/scripts/check_orphan_modules.py (1)
21-22: Add Google-style docstrings for function compliance.
_is_empty_init()andmain()should include Google-style docstrings with explicitArgs/Returnssections.As per coding guidelines, "Use Google-style docstrings with Args/Returns/Raises sections for all functions and classes".
Also applies to: 35-35
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/scripts/check_orphan_modules.py` around lines 21 - 22, Add Google-style docstrings to both _is_empty_init(module: str) and main() describing purpose, parameters, return values and exceptions; for _is_empty_init include an Args section documenting module: str and a Returns section indicating bool, and mention any raised exceptions in Raises (e.g., IOError/FileNotFoundError if you open files); for main include Args (if any), Returns (e.g., exit code or None) and Raises for uncaught exceptions. Ensure the docstrings follow Google style with a short summary line, a blank line, then Args/Returns/Raises sections and place them immediately under the function definitions for _is_empty_init and main.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@backend/scripts/check_orphan_modules.py`:
- Around line 27-32: The helper _is_empty_init currently treats any string
literal lines as meaningful which misclassifies docstring-only __init__.py
files; update _is_empty_init to parse the source with ast.parse and treat a
module as empty if, after removing the module-level docstring (an ast.Expr of
ast.Str/ast.Constant containing a string) there are no remaining significant
nodes; specifically, in _is_empty_init use ast.parse(source) and consider the
module empty when all nodes in module.body are either the docstring Expr or are
pass/empty constructs (or simply check that no node remains that is not a
module-level docstring), ensuring comments/whitespace are ignored and only
executable statements mark the file as non-empty.
---
Nitpick comments:
In `@backend/pyproject.toml`:
- Line 146: Replace the loose dependency spec "grimp>=3.14" in pyproject.toml
with a pinned exact version to ensure reproducible builds; update the entry
"grimp>=3.14" to "grimp==3.14" so it matches the pinning style used for other
linters (e.g., mypy==1.19.1, ruff==0.14.10).
In `@backend/scripts/check_orphan_modules.py`:
- Around line 21-22: Add Google-style docstrings to both _is_empty_init(module:
str) and main() describing purpose, parameters, return values and exceptions;
for _is_empty_init include an Args section documenting module: str and a Returns
section indicating bool, and mention any raised exceptions in Raises (e.g.,
IOError/FileNotFoundError if you open files); for main include Args (if any),
Returns (e.g., exit code or None) and Raises for uncaught exceptions. Ensure the
docstrings follow Google style with a short summary line, a blank line, then
Args/Returns/Raises sections and place them immediately under the function
definitions for _is_empty_init and main.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
backend/uv.lockis excluded by!**/*.lock
📒 Files selected for processing (33)
.github/workflows/grimp.yml.pre-commit-config.yamlAGENTS.mdbackend/app/core/providers.pybackend/app/db/__init__.pybackend/app/db/repositories/__init__.pybackend/app/dlq/manager.pybackend/app/events/consumer_group_monitor.pybackend/app/events/core/producer.pybackend/app/services/admin/admin_events_service.pybackend/app/services/admin/admin_execution_service.pybackend/app/services/admin/admin_settings_service.pybackend/app/services/admin/admin_user_service.pybackend/app/services/auth_service.pybackend/app/services/event_replay/replay_service.pybackend/app/services/event_service.pybackend/app/services/execution_service.pybackend/app/services/k8s_worker/worker.pybackend/app/services/kafka_event_service.pybackend/app/services/notification_scheduler.pybackend/app/services/notification_service.pybackend/app/services/result_processor/processor.pybackend/app/services/runtime_settings.pybackend/app/services/saga/execution_saga.pybackend/app/services/saga/saga_orchestrator.pybackend/app/services/saga/saga_service.pybackend/app/services/saved_script_service.pybackend/app/services/sse/sse_service.pybackend/app/services/user_settings_service.pybackend/pyproject.tomlbackend/scripts/check_orphan_modules.pybackend/vulture_whitelist.pydocs/operations/cicd.md
💤 Files with no reviewable changes (2)
- backend/app/events/consumer_group_monitor.py
- backend/vulture_whitelist.py
|
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@backend/scripts/check_orphan_modules.py`:
- Around line 22-27: Add full Google-style docstrings for both _is_empty_init
and main: for _is_empty_init(module: str) include a short summary, an Args
section documenting module (path or module name), a Returns section stating it
returns bool indicating whether the module __init__.py contains only an optional
module docstring and no other statements, and a Raises section if you
expect/handle exceptions (e.g., SyntaxError from ast.parse) or omit Raises if
none. For main(), add a summary and Args (if it accepts CLI args or uses none),
a Returns section describing the exit code or None, and a Raises section for any
exceptions that propagate; ensure both docstrings follow Google-style headings
exactly ("Args:", "Returns:", "Raises:") and reference behavior found in the
existing function logic (use of AST inspection in _is_empty_init and script
entry behavior in main).
- Around line 28-31: The code builds init_path relative to the current working
directory which makes detection flaky; instead resolve package paths relative to
the script location. Change the logic that computes init_path (currently using
Path(module.replace(".", "/")) / "__init__.py") to base the path off the script
directory (e.g. Path(__file__).parent.resolve()) and then join
module.replace(".", "/") and "__init__.py"; keep using init_path.is_file() and
init_path.read_text() unchanged. This ensures init_path and the empty-package
detection are deterministic in CI/pre-commit runs.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by cubic
Switched dead code detection from Vulture to Grimp to catch orphan modules and reduce false positives. Added an orphan-check script, updated CI and pre-commit, and refactored imports to keep the module graph accurate.
New Features
Refactors
Written for commit f7a261c. Summary will update on new commits.
Summary by CodeRabbit
Chores
Refactor