Repository navigation
Conversation
…reference files Slim CLAUDE.md (.agents/AGENTS.md) from 899 to ~600 lines and add the guidance that two years of PR review history shows is missing: - Definition-of-done checklists (endpoint/queryset, model change, pre-review) mapping 1:1 to the most frequent review findings: missing permission checks and default filters, 500s from unvalidated params, PII in serializers, N+1-blind single-row test fixtures, WIP debris, stale PR descriptions. - Domain invariants section: one feature extractor per clustering query, timezone.now() rule, ML-backend schema boundary, raise-don't-return, no assert in prod code, migration-in-same-PR rule. - Automated review bots section: verify bot findings against actual lint configs, push back with evidence, 150-file CodeRabbit limit. - Split bulk reference out of the always-loaded file: - DB schema table + query guide + QuerySet catalog -> docs/claude/reference/query-patterns.md - Worktree-vs-main-stack testing -> docs/claude/reference/worktree-testing.md - Chaos testing section -> pointer to existing docs/claude/debugging/chaos-scenarios.md - New docs/claude/reference/canonical-patterns.md: reuse-before-reinventing table with file:line refs (SingleParamSerializer, ProjectMixin, permissions, stats pattern, fixtures). - New ui/CLAUDE.md: frontend conventions (i18n via language.ts, no any in data-services models, mutation button states, naming, which lint rules are actually configured). - New docs/claude/INDEX.md: searchable index of all agent docs. Co-Authored-By: Claude <noreply@anthropic.com>
Job.job_type_key builds its choices from VALID_JOB_TYPES, so registry changes (post_processing and data_export job types added since the last migration) alter the field definition. The migration is state-only — no database schema change — but Django requires it, and the new makemigrations --check CI step fails without it. Documented the gotcha in the agent instructions. Co-Authored-By: Claude <noreply@anthropic.com>
✅ Deploy Preview for antenna-ssec ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for antenna-preview ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (8)
📝 WalkthroughWalkthroughThis PR establishes comprehensive documentation standards and pattern catalogs across backend and frontend, updates agent guidance to reference these materials, and enforces database field consistency through migration standardization and CI validation. ChangesDocumentation and Pattern Standards
Database Field Standardization and CI Enforcement
Estimated Code Review Effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly Related PRs
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 |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR restructures and expands the repository’s agent/contributor guidance by moving large reference material into docs/claude/, adding “definition of done” checklists + domain invariants, and introducing a CI gate to fail builds when model changes are missing migrations (plus adding the migration that gate detects on main).
Changes:
- Add a backend CI step to enforce “no missing migrations” (
makemigrations --check --dry-run) and include the missing state-only migration forJob.job_type_keychoices. - Split previously always-loaded guidance into focused “load on demand” reference docs under
docs/claude/(index + canonical patterns + query patterns + worktree testing). - Add a dedicated frontend conventions doc (
ui/CLAUDE.md) and expand.agents/AGENTS.mdwith invariants/checklists and pointers to the new docs.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
ui/CLAUDE.md |
Adds frontend-specific conventions and tooling guidance. |
docs/claude/reference/worktree-testing.md |
Extracts worktree testing instructions into a dedicated doc. |
docs/claude/reference/query-patterns.md |
Adds a query patterns + schema reference doc extracted from agent instructions. |
docs/claude/reference/canonical-patterns.md |
Documents reusable backend patterns with file:line pointers. |
docs/claude/INDEX.md |
Introduces an index for agent-oriented docs. |
ami/jobs/migrations/0023_alter_job_job_type_key.py |
Adds the state-only migration updating Job.job_type_key choices. |
.github/workflows/test.backend.yml |
Adds CI migration check step to fail on missing migrations. |
.agents/AGENTS.md |
Adds invariants/checklists and points to the new reference docs; removes bulky inline reference sections. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -0,0 +1,58 @@ | |||
| # docs/claude/ Index | |||
| | Pattern | Where | Use for | | ||
| |---|---|---| | ||
| | `SingleParamSerializer` | `ami/base/serializers.py:108` (usage: `ami/base/views.py:47`) | Validating individual GET params — returns 400 on bad input instead of a 500 | | ||
| | `url_boolean_param()` | `ami/utils/fields.py:5` (usage: `ami/jobs/views.py:126`) | Boolean query params, e.g. `?incomplete_only=true` | |
|
|
||
| #### Option A — Bind-mount worktree subdirs into main stack (preferred for code-only changes) | ||
|
|
||
| Main `docker-compose.yml` mounts `.:/app:z`. You **cannot** override the `/app` mount itself (Docker keeps both, the broader one wins for path resolution), but you **can** mount a deeper path on top of it. Add to `/home/michael/Projects/AMI/antenna/docker-compose.override.yml` (note: that file is a symlink to `docker-compose.override-example.yml` by default — break the symlink first by `rm`-ing it, then write a real file copying the example contents): |
| django: | ||
| volumes: | ||
| - ./compose/local/django/start:/start # keep existing entries from example | ||
| - /home/michael/Projects/AMI/antenna/.claude/worktrees/<branch>/ami:/app/ami:z |
Comment on lines
+28
to
+29
| - Disable mutation buttons while the request is in flight (React Query `isLoading` / | ||
| `isPending`). A fast double-click must not enqueue duplicate jobs. |
Comment on lines
48
to
55
| - name: Run DB Migrations | ||
| run: docker compose -f docker-compose.ci.yml run --rm django python manage.py migrate | ||
|
|
||
| - name: Check for missing migrations | ||
| run: docker compose -f docker-compose.ci.yml run --rm django python manage.py makemigrations --check --dry-run | ||
|
|
||
| - name: Run Django Tests | ||
| run: docker compose -f docker-compose.ci.yml run --rm django python manage.py test |
| ```python | ||
| # UNIQUE constraint - perfect for lookups | ||
| (deployment, group_by) | ||
| - **Always filter by `project` first** on Occurrence/SourceImage/Event queries — the composite indexes lead with it, and it enforces visibility. |
Collaborator
Author
|
Folding these commits into fix/ci-concurrency-per-workflow so the CI workflow changes ship together. Superseded by the new PR from that branch. |
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
This restructures the agent/contributor instructions (
.agents/AGENTS.md, symlinked asCLAUDE.md) so they actively prevent the kinds of issues that show up over and over in this repo's PR review history, instead of mostly documenting query patterns. The changes come from reviewing roughly fifty merged PRs across the last two years and grouping the recurring reviewer feedback — from both human reviewers and the review bots — into categories, then writing guidance (and one CI gate) targeted at the most frequent ones.The most common recurring findings were: new code paths missing a permission check or the default occurrence filters (bugs invisible in the diff), unbounded/Python-side query work that doesn't scale, query params that return a 500 instead of a 400, PII leaking through nested serializers, single-row test fixtures that can't catch N+1s, and forgotten migrations shipping to main and needing standalone follow-up PRs. Nothing here changes application behavior except the new CI step and the migration it required.
List of Changes
AGENTS.mdfor endpoint/queryset changes, model changes, and a pre-review pass (permissions matrix tests, default filters, param validation → 400, no PII in serializers,assertNumQuerieswith multi-row fixtures, self-review for debris)timezone.now()vsdatetime.now(), the ML-backend schema boundary, raise-don't-return, noassertin prod, migration-in-same-PRmakemigrations --check --dry-runstep totest.backend.ymlmainJob.job_type_keybuilds itschoicesfromVALID_JOB_TYPES; two job types were added without the (state-only) migration. The new CI gate caught itdocs/claude/reference/query-patterns.md; worktree-vs-main-stack testing todocs/claude/reference/worktree-testing.md; chaos-testing section to a pointer to the existing runbookdocs/claude/reference/canonical-patterns.md— reuse table with file:line refs (SingleParamSerializer, ProjectMixin, permission classes, stats pattern, fixtures)ui/CLAUDE.md: strings through the translation layer, noanyin data-services models, disable mutation buttons in-flight, naming, and which lint rules are actually configured (note: there is no Stylelint config, so bot Stylelint findings should be ignored)docs/claude/INDEX.mdindexing all reference docs, runbooks, and plansDetailed Description
The guidance is evidence-based: each checklist item and invariant corresponds to a category of feedback that recurred across multiple PRs and multiple reviewers. The intent is to shift that feedback from review-time to author-time (or to CI), and to make the always-loaded instruction file smaller and more rule-focused by moving bulk reference material into load-on-demand docs.
A separate, higher-level "why Antenna exists / design grounding" section was considered for this PR but is being held back for team review since it's more strategic than mechanical.
How to Test the Changes
python manage.py makemigrations --check --dry-runexits 0; without change Bump docker/setup-buildx-action from 1 to 2 #4 it exits 1. Verified locally against the CI compose file.Checklist
🤖 Generated with Claude Code
Summary by CodeRabbit
Bug Fixes / Tests
Documentation
Chores