feat(agents,skills): add django-build-resolver, django-reviewer, and django-celery skill#1310
feat(agents,skills): add django-build-resolver, django-reviewer, and django-celery skill#1310mrigank2seven wants to merge 3 commits intoaffaan-m:mainfrom
Conversation
…django-celery skill
|
ECC bundle files are already tracked in this repository. Skipping generation of another bundle PR. |
📝 WalkthroughWalkthroughThis PR updates documentation counts (agents 47→49, skills 181→182) and adds three new markdown specs: two Django-focused agent specs and one Django+Celery skill guide. Changes
Sequence Diagram(s)(No sequence diagrams generated — documentation additions only.) Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 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 |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
AGENTS.md (1)
17-45: Add the new Django agents to the agent table.The PR introduces
django-revieweranddjango-build-resolver, but they are missing from the agent table. Other language-specific reviewers (python-reviewer, java-reviewer, cpp-reviewer) and build resolvers (java-build-resolver, kotlin-build-resolver, go-build-resolver) are listed here. For consistency and discoverability, add:| python-reviewer | Python code review | Python projects | +| django-reviewer | Django code review | Django projects | +| django-build-resolver | Django/pip/migration errors | Django build failures | | java-reviewer | Java and Spring Boot code review | Java/Spring Boot projects |Insert them after
python-reviewerto group Django (a Python framework) near its parent language.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@AGENTS.md` around lines 17 - 45, The AGENTS.md table is missing the new agents django-reviewer and django-build-resolver; update the Markdown table by inserting rows for "django-reviewer" (Purpose: Django code review, When to Use: Django projects, framework-specific reviews) and "django-build-resolver" (Purpose: Django build/CI resolver, When to Use: Django build failures/CI errors) immediately after the existing "python-reviewer" row so Django agents are grouped with their parent language; ensure the row formatting matches the existing table style and naming conventions used for other language-specific reviewers and build-resolvers.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@AGENTS.md`:
- Around line 17-45: The AGENTS.md table is missing the new agents
django-reviewer and django-build-resolver; update the Markdown table by
inserting rows for "django-reviewer" (Purpose: Django code review, When to Use:
Django projects, framework-specific reviews) and "django-build-resolver"
(Purpose: Django build/CI resolver, When to Use: Django build failures/CI
errors) immediately after the existing "python-reviewer" row so Django agents
are grouped with their parent language; ensure the row formatting matches the
existing table style and naming conventions used for other language-specific
reviewers and build-resolvers.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 4ca03255-1abc-4a8e-aea5-825015ddba33
📒 Files selected for processing (5)
AGENTS.mdREADME.mdagents/django-build-resolver.mdagents/django-reviewer.mdskills/django-celery/SKILL.md
Greptile SummaryThis PR adds two Django-specific agents ( Confidence Score: 5/5Safe to merge — all P0/P1 findings from previous review rounds are resolved; only minor P2 style suggestions remain. All remaining findings are P2: a naive datetime in a code example and a note about a deprecated Celery setting. Neither causes a runtime error in the repo itself (these are documentation/skill files). CI validator confirms all 49 agents and 182 skills pass schema validation. skills/django-celery/SKILL.md has two minor P2 suggestions (timezone-aware datetime example, deprecated CELERY_TASK_ALWAYS_EAGER note); no files require blocking attention. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
User([Developer]) -->|Django project| Trigger{Task type?}
Trigger -->|Build/startup error| BResolver[django-build-resolver]
Trigger -->|Code review| Reviewer[django-reviewer]
Trigger -->|Async tasks| CelerySkill[skill: django-celery]
BResolver --> B1[pip / Poetry dep check]
BResolver --> B2[Migration conflict fix]
BResolver --> B3[Settings / import repair]
BResolver --> B4[collectstatic / runserver fix]
B1 & B2 & B3 & B4 --> BCheck[python manage.py check]
BCheck --> BDone([FIXED / FAILED report])
Reviewer --> R1[Security: SQL injection, mark_safe, CSRF]
Reviewer --> R2[ORM: N+1, atomic, bulk_create]
Reviewer --> R3[Migration: backward-compat, RunPython]
Reviewer --> R4[DRF: permissions, pagination, serializer fields]
R1 & R2 & R3 & R4 --> RVerdict{Issues?}
RVerdict -->|CRITICAL/HIGH| Block([Block merge])
RVerdict -->|MEDIUM only| Warn([Warn — can merge])
RVerdict -->|None| Approve([Approve])
CelerySkill --> C1[Project setup: broker, results, beat]
CelerySkill --> C2[Task design: retryable / idempotent]
CelerySkill --> C3[Canvas: chain / group / chord]
CelerySkill --> C4[Error handling: DLQ + Sentry]
CelerySkill --> C5[Testing: unit + ALWAYS_EAGER]
C1 & C2 & C3 & C4 & C5 --> CDone([Production-ready Celery setup])
Reviewer -.->|references| CelerySkill
BResolver -.->|references| DjangoPatterns[skill: django-patterns]
Reviewer -.->|references| DjangoPatterns
Reviewer -.->|chains with| PythonReviewer[agent: python-reviewer]
Reviews (3): Last reviewed commit: "fix(agents,docs): resolve PR review comm..." | Re-trigger Greptile |
There was a problem hiding this comment.
2 issues found across 5 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="skills/django-celery/SKILL.md">
<violation number="1" location="skills/django-celery/SKILL.md:349">
P2: The test example patches `apps.notifications.tasks.EmailService`, but `EmailService` is imported inside the task function from `apps.notifications.services`. This patch target won’t exist on the tasks module and won’t intercept the import, so the example would fail or not mock the dependency.</violation>
</file>
<file name="agents/django-reviewer.md">
<violation number="1" location="agents/django-reviewer.md:15">
P2: The agent is instructed to begin review immediately, but team process requires waiting for automated checks/CI to pass before starting review. This should gate review on green checks.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review, or fix all with cubic.
- Fix sentry_sdk.capture_exception to use new_scope() instead of unsupported extras= kwarg (P2, greptile) - Remove stray self parameter from standalone pytest function test_task_retries_on_connection_error (P1, greptile) - Fix patch target to apps.notifications.services.EmailService so mock correctly intercepts the import (P2, cubic) - Gate django-reviewer on CI checks passing before review begins (P2, cubic)
|
ECC bundle files are already tracked in this repository. Skipping generation of another bundle PR. |
There was a problem hiding this comment.
🧹 Nitpick comments (2)
agents/django-reviewer.md (2)
8-16: Consider explicit composition with python-reviewer.The PR objectives describe this agent as "extending python-reviewer checks," but the workflow doesn't reference or compose with the python-reviewer agent. This may lead to gaps if python-reviewer isn't separately invoked for Django projects.
Options:
- Add explicit composition: Insert a step to invoke python-reviewer first, then add Django-specific checks on top.
- Document the relationship: Add a note clarifying that this agent focuses solely on Django-specific concerns and assumes general Python review happens separately.
💡 Example explicit composition
When invoked: +0. Run `@python-reviewer` to verify general Python quality first 1. Run `git diff -- '*.py'` to see recent Python file changes 2. Run `python manage.py check` if a Django project is presentOr add a note:
When invoked: +**Note**: This agent focuses on Django-specific concerns. Ensure python-reviewer has been invoked for general Python quality checks. 1. Run `git diff -- '*.py'` to see recent Python file changesBased on learnings, use code-reviewer agent immediately after writing or modifying code to address CRITICAL and HIGH severity issues.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@agents/django-reviewer.md` around lines 8 - 16, Update the django-reviewer workflow to explicitly compose with the python-reviewer agent: insert a first step that invokes python-reviewer (e.g., "run python-reviewer" or call the python-reviewer agent) before running Django-specific checks, then proceed with the existing steps (git diff, manage.py check, ruff, mypy); alternatively add a clear note in the header that python-reviewer must run separately and that this agent only covers Django-specific concerns; also add a final step to run code-reviewer after fixes to catch CRITICAL/HIGH issues.
8-16: Clarify CI gating pattern.Line 15 instructs the agent to "Wait for CI checks to pass before beginning review," but the PR commit message indicates this gating should happen at the orchestration level. This creates ambiguity about responsibility.
Options:
- Orchestration-level gating (recommended): If the agent should only be invoked after CI passes, rephrase line 15 to assume CI has already passed:
"Assume CI checks have passed (orchestration gated); if CI status needs verification, use gh CLI to confirm green status before proceeding"- Agent-level gating: If the agent should actively wait for CI, add explicit instructions on how to query CI status using the available Bash tool (e.g.,
gh pr checks).✏️ Suggested clarification for orchestration-level gating
When invoked: 1. Run `git diff -- '*.py'` to see recent Python file changes 2. Run `python manage.py check` if a Django project is present 3. Run `ruff check .` and `mypy .` if available 4. Focus on modified `.py` files and any related migrations -5. Wait for CI checks to pass before beginning review; if checks are still running, note which checks are pending and proceed only after green +5. Verify CI status with `gh pr checks` if needed; assume CI has passed if invoked via orchestration gate🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@agents/django-reviewer.md` around lines 8 - 16, Replace the ambiguous instruction "Wait for CI checks to pass before beginning review" with an orchestration-level gating statement such as "Assume CI checks have passed (orchestration gated); if CI status needs verification, use gh CLI (e.g., gh pr checks) to confirm green before proceeding" so the agent treats CI as pre-gated by default; alternatively, if you want agent-level gating, add explicit steps showing how to query CI via the Bash tool (e.g., run `gh pr checks` and wait for a green status) and mention which command to use to re-check pending checks.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@agents/django-reviewer.md`:
- Around line 8-16: Update the django-reviewer workflow to explicitly compose
with the python-reviewer agent: insert a first step that invokes python-reviewer
(e.g., "run python-reviewer" or call the python-reviewer agent) before running
Django-specific checks, then proceed with the existing steps (git diff,
manage.py check, ruff, mypy); alternatively add a clear note in the header that
python-reviewer must run separately and that this agent only covers
Django-specific concerns; also add a final step to run code-reviewer after fixes
to catch CRITICAL/HIGH issues.
- Around line 8-16: Replace the ambiguous instruction "Wait for CI checks to
pass before beginning review" with an orchestration-level gating statement such
as "Assume CI checks have passed (orchestration gated); if CI status needs
verification, use gh CLI (e.g., gh pr checks) to confirm green before
proceeding" so the agent treats CI as pre-gated by default; alternatively, if
you want agent-level gating, add explicit steps showing how to query CI via the
Bash tool (e.g., run `gh pr checks` and wait for a green status) and mention
which command to use to re-check pending checks.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f3bdcc39-a99c-4563-9475-f722a8a1499d
📒 Files selected for processing (2)
agents/django-reviewer.mdskills/django-celery/SKILL.md
✅ Files skipped from review due to trivial changes (1)
- skills/django-celery/SKILL.md
- Add django-reviewer and django-build-resolver to AGENTS.md table after python-reviewer - Add explicit python-reviewer composition note to django-reviewer agent - Clarify CI gating to use orchestration-level pattern with gh pr checks
|
ECC bundle files are already tracked in this repository. Skipping generation of another bundle PR. |
There was a problem hiding this comment.
🧹 Nitpick comments (2)
agents/django-reviewer.md (2)
17-17: Consider clarifying the CI gating instruction.The phrasing "Assume CI checks have passed (orchestration gated); if CI status needs verification, run
gh pr checks" is somewhat contradictory. It's unclear whether the agent should assume passing or verify.Consider rewording to make the default action clearer, for example:
- "Trust orchestration-level CI checks unless you need to verify status, in which case run
gh pr checks"- Or: "Run
gh pr checksto verify orchestration-level CI status before proceeding"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@agents/django-reviewer.md` at line 17, The sentence "Assume CI checks have passed (orchestration gated); if CI status needs verification, run `gh pr checks`" is ambiguous; replace it with a single clear directive such as either "Trust orchestration-level CI checks by default; if you need to confirm status, run `gh pr checks`" or "Before proceeding, run `gh pr checks` to verify orchestration-level CI is green" so readers know the default behavior and the explicit verification step; locate and update the exact phrase in the document (the line containing the quoted instruction) to one of these clearer variants.
148-148: Consider nuancing the idempotency requirement for Celery tasks.The requirement "Tasks must be idempotent" is excellent best practice, but some tasks are inherently non-idempotent (e.g., sending email notifications, creating audit logs, triggering external webhooks). Consider softening this to:
- "Tasks should be idempotent where possible" or
- "Tasks must handle duplicate execution safely (idempotency or guards)"
This aligns with the idempotent pattern shown in the django-celery skill (context snippet 1) which uses database guards rather than strict idempotency.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@agents/django-reviewer.md` at line 148, Update the "Celery" bullet that currently reads "Tasks must be idempotent" to a softened, more accurate guidance: change it to either "Tasks should be idempotent where possible" or "Tasks must handle duplicate execution safely (idempotency or guards)"; keep the existing implementation hint about using bind=True + self.retry() and add a short note pointing to database-guard patterns (as shown in the django-celery skill/example) for inherently non-idempotent work like emails/webhooks.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@agents/django-reviewer.md`:
- Line 17: The sentence "Assume CI checks have passed (orchestration gated); if
CI status needs verification, run `gh pr checks`" is ambiguous; replace it with
a single clear directive such as either "Trust orchestration-level CI checks by
default; if you need to confirm status, run `gh pr checks`" or "Before
proceeding, run `gh pr checks` to verify orchestration-level CI is green" so
readers know the default behavior and the explicit verification step; locate and
update the exact phrase in the document (the line containing the quoted
instruction) to one of these clearer variants.
- Line 148: Update the "Celery" bullet that currently reads "Tasks must be
idempotent" to a softened, more accurate guidance: change it to either "Tasks
should be idempotent where possible" or "Tasks must handle duplicate execution
safely (idempotency or guards)"; keep the existing implementation hint about
using bind=True + self.retry() and add a short note pointing to database-guard
patterns (as shown in the django-celery skill/example) for inherently
non-idempotent work like emails/webhooks.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 17671b67-1549-4dc0-a385-fb769a492b54
📒 Files selected for processing (2)
AGENTS.mdagents/django-reviewer.md
🚧 Files skipped from review as they are similar to previous changes (1)
- AGENTS.md
|
@affaan-m : Please review and merge |
Summary
agents/django-build-resolver.md— Django/Python build error resolver covering pip/Poetry dependency failures, migration conflicts (InconsistentMigrationHistory, merge conflicts),ImproperlyConfiguredsettings errors, circular imports, database connection failures,collectstaticerrors, andrunserverfailures. Follows the same diagnostic → fix → verify pattern as the existingjava-build-resolver,go-build-resolver, etc.agents/django-reviewer.md— Django-specific code reviewer that goes deeper than the existingpython-reviewer. Covers ORM N+1 queries, missingtransaction.atomic()for multi-step writes, backward-incompatible migrations, DRFfields = '__all__'data leaks, missing pagination,mark_safeXSS, missingpermission_classes,save()withoutupdate_fields, and service layer violations.skills/django-celery/SKILL.md— Celery is the de-facto async task layer in Django — not previously covered. Covers project setup, task design patterns (basic/retryable/idempotent/time-limited),apply_asyncinvocation, Beat scheduling (code-defined + database-driven), canvas workflows (chain/group/chord), dead-letter error handling, testing withCELERY_TASK_ALWAYS_EAGER, Flower monitoring, and a production checklist.README / AGENTS / docs count updates — Updated agent (47→49) and skill (181→182) counts in all doc files to keep the CI validator green.
Type
Testing
node tests/run-all.js)tests/ci/validators.test.js) passes: 152/152Checklist
node tests/run-all.js— all passWhen to ActivatesectionsSummary by cubic
Adds two Django-focused agents and a
django-celeryskill to speed up error recovery, tighten reviews, and standardize async tasks in Django apps. Updates docs to 49 agents and 182 skills, with fixes for Sentry, tests, and CI gating.New Features
agents/django-build-resolver— Fixes dependency, migration, settings/import, DB, andcollectstatic/runservererrors with a diagnose → fix → verify flow.agents/django-reviewer— Django-aware reviews: flags ORM N+1s, missingtransaction.atomic(), unsafe migrations, DRF permission/pagination/serializer issues,mark_saferisks, and perf pitfalls. Composes withpython-reviewerfor general Python checks.skills/django-celery— Setup withdjango-celery-results/django-celery-beat, task patterns (retryable/idempotent/time limits), canvas (chain/group/chord), monitoring (Flower), testing withCELERY_TASK_ALWAYS_EAGER, and a prod checklist.AGENTS.mdtable.Bug Fixes
skills/django-celery: Usesentry_sdk.new_scope()withcapture_exception; fix test examples to patchapps.notifications.services.EmailService; remove strayselfintest_task_retries_on_connection_error.agents/django-reviewer: Gate review on passing CI viagh pr checksbefore starting.Written for commit e008d81. Summary will update on new commits.
Summary by CodeRabbit