Add shared ast-grep rules, GitHub Actions step, and dev-command docs#437
Add shared ast-grep rules, GitHub Actions step, and dev-command docs#437nossila wants to merge 2 commits into
Conversation
Consuming projects (baseapp-backend-template) are adding ast-grep rules (.ast-grep/rules/) that statically enforce the code-guideline skills. List the scan/test commands alongside the other lint commands so agents translate intent into the correct Docker Compose invocation. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Updates the repository’s canonical “how to run dev commands” references to include ast-grep so consuming projects/templates can run ast-grep-based convention checks inside the web Docker service.
Changes:
- Add
docker compose … ast-grep scantoAGENTS.mdcode-quality command list. - Add
ast-grep scanandast-grep testto therun-development-commandsagent skill’s “Lint & Format” block. - Bump
run-development-commandsskill version from1.1.0to1.2.0.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| AGENTS.md | Adds an ast-grep scan entry to the documented “Code quality” Docker Compose commands. |
| .agents/skills/run-development-commands/SKILL.md | Bumps skill version and adds ast-grep scan/test to the Lint & Format command patterns. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
WalkthroughThis PR adds an .ast-grep ruleset with rule definitions, tests, and snapshots; updates the run-development-commands skill (v1.1.0→1.2.0) with Changesast-grep Integration
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
.agents/skills/run-development-commands/SKILL.md (1)
114-114: 💤 Low valueConsider mentioning ast-grep in Rule 1 for consistency.
Rule 1 explicitly lists tools that must run inside the container (Python, pytest, Black, Flake8, manage.py, uv). Now that ast-grep is part of the standard lint workflow, you might add it to this list for completeness, though the general principle "never run on the host" already covers it.
🤖 Prompt for 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. In @.agents/skills/run-development-commands/SKILL.md at line 114, Update Rule 1's sentence ("Never run on the host. All Python, pytest, Black, Flake8, manage.py, and uv commands run inside the container.") to also mention ast-grep so it reads like the other listed tools (e.g., add "ast-grep" to the comma-separated list), ensuring the rule explicitly includes ast-grep in the set of commands that must run inside the container.
🤖 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.
Nitpick comments:
In @.agents/skills/run-development-commands/SKILL.md:
- Line 114: Update Rule 1's sentence ("Never run on the host. All Python,
pytest, Black, Flake8, manage.py, and uv commands run inside the container.") to
also mention ast-grep so it reads like the other listed tools (e.g., add
"ast-grep" to the comma-separated list), ensuring the rule explicitly includes
ast-grep in the set of commands that must run inside the container.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: bae3cb05-a6c1-414a-b7fa-9f5dbeccab40
📒 Files selected for processing (2)
.agents/skills/run-development-commands/SKILL.mdAGENTS.md
Rules move here from baseapp-backend-template so they can be shared the same way the agent skills are: consuming projects symlink .ast-grep to this directory and keep their own sgconfig.yml. - 15 rules in .ast-grep/rules/ enforcing the backend code guidelines; severity error blocks CI, warning/hint are informational. - django-admin-use-unfold is scoped to apps/** — baseapp_* packages intentionally register stock admins that consuming projects override with Unfold. - Rule test suite (.ast-grep/rule-tests/ + snapshots) runs via `ast-grep test`; ast-grep-cli added to the dev dependency group. - New ast-grep step in the lint chain of github-actions.yml runs `ast-grep test` + `ast-grep scan` (currently 0 errors repo-wide). - run-development-commands skill: template invocations exclude the submodule via --globs '!baseapp-backend' (linted here instead). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
.ast-grep/rule-tests/drf-validation-error-format-test.yml (1)
8-11: ⚡ Quick winAdd invalid fixtures for
gettext(...)andgettext_lazy(...)forms.The rule explicitly matches those variants, but this test file currently exercises only plain string and
_()cases. Adding both variants here will prevent silent regressions in that part of the rule contract.🤖 Prompt for 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. In @.ast-grep/rule-tests/drf-validation-error-format-test.yml around lines 8 - 11, Add two invalid fixtures that mirror the existing ones but use gettext(...) and gettext_lazy(...) variants so the rule covers those forms; specifically add cases where raise serializers.ValidationError(gettext("This email is already in use.")) and raise serializers.ValidationError(gettext_lazy("This email is already in use.")) to the test set (the rule targets serializers.ValidationError and the gettext/gettext_lazy call shapes).
🤖 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 @.agents/skills/run-development-commands/SKILL.md:
- Around line 81-82: Update the two ast-grep invocations so they run via the uv
wrapper used elsewhere: change the commands that call "docker compose <run> web
ast-grep scan --globs '!baseapp-backend'" and "docker compose <run> web ast-grep
test" to invoke "uv run ast-grep" inside the web container (i.e., prepend "uv
run" before "ast-grep") so the web container uses the dev dependency via uv like
the CI and .ast-grep/README.md do.
In
@.ast-grep/rule-tests/__snapshots__/django-property-instead-of-annotation-snapshot.yml:
- Around line 9-12: The snapshot's labels.source blocks over-indent the function
declaration under the `@property` decorator causing mismatches; update the YAML
snapshot so the def line is aligned directly under the `@property` (i.e., remove
the extra leading spaces before "def favourites_count(self) -> int:") for the
block containing favourites_count and apply the same dedent fix to the other
labels.source occurrence (the second block around lines 22-25) so both snapshots
exactly match the actual matched code.
In @.ast-grep/rule-tests/django-use-timezone-now-test.yml:
- Line 8: The test currently treats the snippet "when =
datetime.now(dt.timezone.utc)" as valid but the rule message requires using
timezone.now(); update the test to make this a failing case: move this example
from the valid set into the invalid set and expand the invalid matcher to detect
calls to datetime.now(...) with timezone arguments (match the expression
"datetime.now(dt.timezone.utc)" or pattern "datetime.now(*)"); reference the
test example named "when" and the rule that warns to use timezone.now() so the
test and message stay consistent.
---
Nitpick comments:
In @.ast-grep/rule-tests/drf-validation-error-format-test.yml:
- Around line 8-11: Add two invalid fixtures that mirror the existing ones but
use gettext(...) and gettext_lazy(...) variants so the rule covers those forms;
specifically add cases where raise serializers.ValidationError(gettext("This
email is already in use.")) and raise
serializers.ValidationError(gettext_lazy("This email is already in use.")) to
the test set (the rule targets serializers.ValidationError and the
gettext/gettext_lazy call shapes).
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 8516b7c8-3bd7-4ede-bb43-46cf4f16dc14
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (50)
.agents/skills/run-development-commands/SKILL.md.ast-grep/README.md.ast-grep/rule-tests/__snapshots__/django-admin-use-unfold-snapshot.yml.ast-grep/rule-tests/__snapshots__/django-property-instead-of-annotation-snapshot.yml.ast-grep/rule-tests/__snapshots__/django-related-name-required-snapshot.yml.ast-grep/rule-tests/__snapshots__/django-translation-model-fields-lazy-snapshot.yml.ast-grep/rule-tests/__snapshots__/django-translation-no-module-level-eager-gettext-snapshot.yml.ast-grep/rule-tests/__snapshots__/django-translation-use-gettext-lazy-snapshot.yml.ast-grep/rule-tests/__snapshots__/django-use-timezone-now-snapshot.yml.ast-grep/rule-tests/__snapshots__/drf-filters-in-viewset-snapshot.yml.ast-grep/rule-tests/__snapshots__/drf-validation-error-format-snapshot.yml.ast-grep/rule-tests/__snapshots__/drf-validation-in-viewset-snapshot.yml.ast-grep/rule-tests/__snapshots__/exception-handling-without-logging-snapshot.yml.ast-grep/rule-tests/__snapshots__/graphql-use-django-object-type-snapshot.yml.ast-grep/rule-tests/__snapshots__/graphql-use-relay-model-snapshot.yml.ast-grep/rule-tests/__snapshots__/graphql-use-relay-mutation-snapshot.yml.ast-grep/rule-tests/__snapshots__/python-typing-required-snapshot.yml.ast-grep/rule-tests/django-admin-use-unfold-test.yml.ast-grep/rule-tests/django-property-instead-of-annotation-test.yml.ast-grep/rule-tests/django-related-name-required-test.yml.ast-grep/rule-tests/django-translation-model-fields-lazy-test.yml.ast-grep/rule-tests/django-translation-no-module-level-eager-gettext-test.yml.ast-grep/rule-tests/django-translation-use-gettext-lazy-test.yml.ast-grep/rule-tests/django-use-timezone-now-test.yml.ast-grep/rule-tests/drf-filters-in-viewset-test.yml.ast-grep/rule-tests/drf-validation-error-format-test.yml.ast-grep/rule-tests/drf-validation-in-viewset-test.yml.ast-grep/rule-tests/exception-handling-without-logging-test.yml.ast-grep/rule-tests/graphql-use-django-object-type-test.yml.ast-grep/rule-tests/graphql-use-relay-model-test.yml.ast-grep/rule-tests/graphql-use-relay-mutation-test.yml.ast-grep/rule-tests/python-typing-required-test.yml.ast-grep/rules/django-admin-use-unfold.yml.ast-grep/rules/django-property-instead-of-annotation.yml.ast-grep/rules/django-related-name-required.yml.ast-grep/rules/django-translation-model-fields-lazy.yml.ast-grep/rules/django-translation-no-module-level-eager-gettext.yml.ast-grep/rules/django-translation-use-gettext-lazy.yml.ast-grep/rules/django-use-timezone-now.yml.ast-grep/rules/drf-filters-in-viewset.yml.ast-grep/rules/drf-validation-error-format.yml.ast-grep/rules/drf-validation-in-viewset.yml.ast-grep/rules/exception-handling-without-logging.yml.ast-grep/rules/graphql-use-django-object-type.yml.ast-grep/rules/graphql-use-relay-model.yml.ast-grep/rules/graphql-use-relay-mutation.yml.ast-grep/rules/python-typing-required.yml.github/workflows/github-actions.ymlpyproject.tomlsgconfig.yml
✅ Files skipped from review due to trivial changes (26)
- .ast-grep/rules/drf-validation-in-viewset.yml
- sgconfig.yml
- .ast-grep/rule-tests/graphql-use-relay-model-test.yml
- .ast-grep/rules/django-translation-use-gettext-lazy.yml
- .ast-grep/rules/django-translation-no-module-level-eager-gettext.yml
- .ast-grep/rules/django-use-timezone-now.yml
- .ast-grep/rule-tests/snapshots/graphql-use-relay-model-snapshot.yml
- .ast-grep/rules/graphql-use-django-object-type.yml
- .ast-grep/rules/drf-filters-in-viewset.yml
- .ast-grep/rule-tests/django-translation-use-gettext-lazy-test.yml
- .ast-grep/rule-tests/django-translation-model-fields-lazy-test.yml
- .ast-grep/rules/django-admin-use-unfold.yml
- .ast-grep/rules/graphql-use-relay-model.yml
- .ast-grep/rule-tests/snapshots/django-translation-model-fields-lazy-snapshot.yml
- .ast-grep/rule-tests/django-translation-no-module-level-eager-gettext-test.yml
- .ast-grep/rules/django-property-instead-of-annotation.yml
- .ast-grep/rules/exception-handling-without-logging.yml
- .ast-grep/rules/django-related-name-required.yml
- .ast-grep/rules/graphql-use-relay-mutation.yml
- .ast-grep/rule-tests/snapshots/drf-validation-in-viewset-snapshot.yml
- .ast-grep/rules/django-translation-model-fields-lazy.yml
- .ast-grep/rule-tests/snapshots/python-typing-required-snapshot.yml
- .ast-grep/rule-tests/drf-validation-in-viewset-test.yml
- .ast-grep/rule-tests/snapshots/graphql-use-relay-mutation-snapshot.yml
- .ast-grep/rule-tests/snapshots/django-translation-use-gettext-lazy-snapshot.yml
- .ast-grep/rule-tests/snapshots/graphql-use-django-object-type-snapshot.yml
| docker compose <run> web ast-grep scan --globs '!baseapp-backend' # code-guideline rules (shared from baseapp-backend/.ast-grep/); the submodule is linted in its own CI | ||
| docker compose <run> web ast-grep test # test the ast-grep rules themselves |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 1) Show SKILL.md lines around 81-82
echo "===== SKILL.md (lines 60-100) ====="
nl -ba .agents/skills/run-development-commands/SKILL.md | sed -n '60,110p'
# 2) Search for github-actions workflow ast-grep and uv run
echo "===== Search workflows for ast-grep ====="
ls -la .github/workflows || true
rg -n "ast-grep" .github/workflows -S || true
echo "===== Search workflows for uv run ====="
rg -n "uv run" .github/workflows -S || true
# 3) Read .ast-grep/README.md if exists
echo "===== .ast-grep/README.md (first 200 lines) ====="
if [ -f .ast-grep/README.md ]; then
nl -ba .ast-grep/README.md | sed -n '1,200p'
else
echo "Missing .ast-grep/README.md"
fi
# Also search for "docker compose" and "uv run" in .ast-grep/README.md
echo "===== Search in .ast-grep/README.md for uv run ====="
if [ -f .ast-grep/README.md ]; then
rg -n "uv run" .ast-grep/README.md -S || true
rg -n "docker compose" .ast-grep/README.md -S || true
fiRepository: silverlogic/baseapp-backend
Length of output: 168
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Helper: print file with line numbers without nl
print_lines() { awk -v start="$1" -v end="$2" 'NR>=start && NR<=end {printf "%6d\t%s\n", NR, $0}' "$3"; }
# 1) Show SKILL.md lines around 81-82 (and a bit more)
echo "===== SKILL.md (lines 70-95) ====="
print_lines 70 95 .agents/skills/run-development-commands/SKILL.md
# 2) List workflows and search for ast-grep/uv run
echo "===== Files in .github/workflows ====="
ls -ლა .github/workflows || true
echo "===== Search workflows for ast-grep ====="
rg -n "ast-grep" .github/workflows -S || true
echo "===== Search workflows for uv run ====="
rg -n "uv run" .github/workflows -S || true
# 3) Read .ast-grep/README.md if exists
echo "===== .ast-grep/README.md (first 200 lines) ====="
if [ -f .ast-grep/README.md ]; then
print_lines 1 200 .ast-grep/README.md
else
echo "Missing .ast-grep/README.md"
fi
# Also search for uv run / docker compose in README
echo "===== Search in .ast-grep/README.md for uv run and docker compose ====="
if [ -f .ast-grep/README.md ]; then
rg -n "uv run" .ast-grep/README.md -S || true
rg -n "docker compose" .ast-grep/README.md -S || true
fiRepository: silverlogic/baseapp-backend
Length of output: 10244
Add uv run wrapper to ast-grep commands in the run-development-commands skill
The ast-grep invocations in .agents/skills/run-development-commands/SKILL.md are missing uv run, while both CI (.github/workflows/github-actions.yml) and .ast-grep/README.md run ast-grep through uv because ast-grep-cli is a uv dev dependency inside the web container.
🔧 Proposed fix
-docker compose <run> web ast-grep scan --globs '!baseapp-backend' # code-guideline rules (shared from baseapp-backend/.ast-grep/); the submodule is linted in its own CI
-docker compose <run> web ast-grep test # test the ast-grep rules themselves
+docker compose <run> web uv run ast-grep scan --globs '!baseapp-backend' # code-guideline rules (shared from baseapp-backend/.ast-grep/); the submodule is linted in its own CI
+docker compose <run> web uv run ast-grep test # test the ast-grep rules themselves📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| docker compose <run> web ast-grep scan --globs '!baseapp-backend' # code-guideline rules (shared from baseapp-backend/.ast-grep/); the submodule is linted in its own CI | |
| docker compose <run> web ast-grep test # test the ast-grep rules themselves | |
| docker compose <run> web uv run ast-grep scan --globs '!baseapp-backend' # code-guideline rules (shared from baseapp-backend/.ast-grep/); the submodule is linted in its own CI | |
| docker compose <run> web uv run ast-grep test # test the ast-grep rules themselves |
🧰 Tools
🪛 SkillSpector (2.1.1)
[warning] 1: [TR2] Shadow Command Trigger: Shadow Command Trigger: 'run tests' conflicts with built-in command 'run'
Remediation: Choose triggers that do not conflict with built-in commands or other skills. Prefix with a unique namespace if necessary.
(Trigger Abuse (TR2))
[warning] 2: [TR2] Shadow Command Trigger: Shadow Command Trigger: 'run server' conflicts with built-in command 'run'
Remediation: Choose triggers that do not conflict with built-in commands or other skills. Prefix with a unique namespace if necessary.
(Trigger Abuse (TR2))
[warning] 5: [TR2] Shadow Command Trigger: Shadow Command Trigger: 'run lint' conflicts with built-in command 'run'
Remediation: Choose triggers that do not conflict with built-in commands or other skills. Prefix with a unique namespace if necessary.
(Trigger Abuse (TR2))
[warning] 6: [TR2] Shadow Command Trigger: Shadow Command Trigger: 'format code' conflicts with built-in command 'format'
Remediation: Choose triggers that do not conflict with built-in commands or other skills. Prefix with a unique namespace if necessary.
(Trigger Abuse (TR2))
[warning] 7: [TR2] Shadow Command Trigger: Shadow Command Trigger: 'open shell' conflicts with built-in command 'open'
Remediation: Choose triggers that do not conflict with built-in commands or other skills. Prefix with a unique namespace if necessary.
(Trigger Abuse (TR2))
[warning] 12: [TR2] Shadow Command Trigger: Shadow Command Trigger: 'install package' conflicts with built-in command 'install'
Remediation: Choose triggers that do not conflict with built-in commands or other skills. Prefix with a unique namespace if necessary.
(Trigger Abuse (TR2))
🤖 Prompt for 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.
In @.agents/skills/run-development-commands/SKILL.md around lines 81 - 82,
Update the two ast-grep invocations so they run via the uv wrapper used
elsewhere: change the commands that call "docker compose <run> web ast-grep scan
--globs '!baseapp-backend'" and "docker compose <run> web ast-grep test" to
invoke "uv run ast-grep" inside the web container (i.e., prepend "uv run" before
"ast-grep") so the web container uses the dev dependency via uv like the CI and
.ast-grep/README.md do.
| - source: |- | ||
| @property | ||
| def favourites_count(self) -> int: | ||
| return self.favourites.filter(active=True).count() |
There was a problem hiding this comment.
Fix snapshot source indentation to match the actual matched code.
Both labels.source blocks over-indent the def line compared to the snippet under test. This can cause snapshot mismatch/noise in ast-grep test.
Suggested patch
: labels:
- source: |-
`@property`
- def favourites_count(self) -> int:
- return self.favourites.filter(active=True).count()
+ def favourites_count(self) -> int:
+ return self.favourites.filter(active=True).count()
@@
: labels:
- source: |-
`@property`
- def is_favourited(self):
- return self.favourites.filter(user=self.user).exists()
+ def is_favourited(self):
+ return self.favourites.filter(user=self.user).exists()Also applies to: 22-25
🤖 Prompt for 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.
In
@.ast-grep/rule-tests/__snapshots__/django-property-instead-of-annotation-snapshot.yml
around lines 9 - 12, The snapshot's labels.source blocks over-indent the
function declaration under the `@property` decorator causing mismatches; update
the YAML snapshot so the def line is aligned directly under the `@property` (i.e.,
remove the extra leading spaces before "def favourites_count(self) -> int:") for
the block containing favourites_count and apply the same dedent fix to the other
labels.source occurrence (the second block around lines 22-25) so both snapshots
exactly match the actual matched code.
|
|
||
| created_at = timezone.now() | ||
| - | | ||
| when = datetime.now(dt.timezone.utc) |
There was a problem hiding this comment.
Align valid case with the rule message to avoid policy drift.
Line 8 currently treats datetime.now(dt.timezone.utc) as valid, but the rule message says to use timezone.now() instead of datetime.now(...). Either move this case to invalid and expand the matcher, or narrow the message so behavior and tests are consistent.
🤖 Prompt for 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.
In @.ast-grep/rule-tests/django-use-timezone-now-test.yml at line 8, The test
currently treats the snippet "when = datetime.now(dt.timezone.utc)" as valid but
the rule message requires using timezone.now(); update the test to make this a
failing case: move this example from the valid set into the invalid set and
expand the invalid matcher to detect calls to datetime.now(...) with timezone
arguments (match the expression "datetime.now(dt.timezone.utc)" or pattern
"datetime.now(*)"); reference the test example named "when" and the rule that
warns to use timezone.now() so the test and message stay consistent.
nossila
changed the title
What
.ast-grep/— 15 ast-grep rules enforcing the BaseApp backend code guidelines, with a full rule test suite (rule-tests/+ snapshots) andsgconfig.yml. Rules live here so they can be shared with consuming projects the same way the agent skills are: the template symlinks.ast-grep→baseapp-backend/.ast-grep.ast-grepstep in the lint chain (after black) runsast-grep test+ast-grep scan. The scan currently reports 0 errors repo-wide (~2.2k warnings/hints are informational; onlyseverity: errorrules fail the build).ast-grep-cliadded to the dev dependency group (pyproject + uv.lock, re-locked with the pinned uv 0.10.9).run-development-commandsskill / AGENTS.md — ast-grep commands added to the lint command reference (template invocations exclude this submodule via--globs '!baseapp-backend', since it's linted here).Notes
django-admin-use-unfold(error) is scoped tofiles: apps/**— thebaseapp_*packages intentionally register stock admins that consuming projects unregister and re-register with Unfold.backend-conventions/backend-patternsskill reference files that live in the consuming template;.ast-grep/README.mdexplains the layout, both CI surfaces, and how to contribute rules.feature/ast-grepin baseapp-backend-template (JenkinsAstGreplint stage +.ast-grepsymlink). The template's submodule pointer should be bumped after this merges.🤖 Generated with Claude Code