Improve CI checks and contributor guidance by mihow · Pull Request #1336 · RolnickLab/antenna

mihow · 2026-06-09T23:06:01Z

Summary

This bundles several CI and contributor-guidance improvements that all aim at the same thing: catching problems automatically or at authoring time, instead of in review.

Two are small CI fixes. First, the concurrency groups in the backend and frontend test workflows are now namespaced per workflow, so a new push cancels only the same workflow's in-progress run rather than cancelling an unrelated one that happens to share the branch. Second, CI now fails when a model change is missing its migration, a class of mistake that is easy to miss in review.

The larger part restructures the agent/contributor instructions (.agents/AGENTS.md, symlinked as CLAUDE.md) so they actively prevent the issues that recur in this repo's PR review history. The guidance comes from reviewing roughly fifty merged PRs over the last two years and grouping the recurring feedback — from human reviewers and the review bots — into categories, then writing checklists and rules targeted at the most frequent ones: 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, params returning a 500 instead of a 400, PII through nested serializers, single-row test fixtures that can't catch N+1s, and forgotten migrations.

No application behavior changes except the two CI steps and the one migration the migration-gate required.

List of Changes

#	Change (what it does)	How
1	A push cancels only the same workflow's running job, not an unrelated one on the same branch	Namespaced the `concurrency` group by `github.workflow` in `test.backend.yml` and `test.frontend.yml`
2	CI now fails when a model change is missing its migration	Added a `makemigrations --check --dry-run` step to `test.backend.yml`
3	Includes the state-only migration that `Job.job_type_key` requires	Its `choices` are built from `VALID_JOB_TYPES`, so the registered job types and the field definition must stay in sync; `makemigrations --check` requires this migration to be present
4	Agents get a "definition of done" before opening a PR, mapped to the most frequent review findings	New checklists in `AGENTS.md` for endpoint/queryset changes, model changes, and a pre-review pass (permission-matrix tests, default filters, param validation → 400, no PII in serializers, `assertNumQueries` with multi-row fixtures, self-review for debris)
5	Agents are warned about silent-bug classes that don't appear in a diff	New "Domain Invariants & Correctness Rules" section: one feature-extractor per clustering query, `timezone.now()` vs `datetime.now()`, the ML-backend schema boundary, raise-don't-return, no `assert` in prod, migration-in-same-PR
6	The always-loaded instructions are ~325 lines shorter and faster to load	Moved the DB schema table, query guide, and QuerySet catalog to `docs/claude/reference/query-patterns.md`; worktree-vs-main-stack testing to `docs/claude/reference/worktree-testing.md`; chaos-testing to a pointer to the existing runbook
7	Agents can find and reuse existing helpers instead of reinventing them	New `docs/claude/reference/canonical-patterns.md` — reuse table with file:line refs (SingleParamSerializer, ProjectMixin, permission classes, stats pattern, fixtures)
8	The frontend has its own conventions doc	New `ui/AGENTS.md` (symlinked as `ui/CLAUDE.md`, matching the repo-root convention): strings through the translation layer, no `any` in data-services models, disable mutation buttons in-flight, naming, and which lint rules are actually configured (there is no Stylelint config, so bot Stylelint findings should be ignored)
9	Agent docs are discoverable	New `docs/claude/INDEX.md` indexing all reference docs, runbooks, and plans
10	Agents handle review-bot feedback better	New section: verify bot findings against the actual lint configs, push back with evidence, split PRs over ~150 files (CodeRabbit skips them)
11	Agents calibrate confidence in comments and PR text to evidence	New "Writing Comments, Docstrings, and PR Text" section in `AGENTS.md`: keep the load-bearing fact and cut unverified mechanism, label hunches ("best-guess", "not measured"), keep comments short and link to the PR/issue for depth, and distinguish measured from inferred in PR descriptions

Detailed Description

The agent-doc guidance is evidence-based: each checklist item and invariant corresponds to a category of feedback that recurred across multiple PRs and reviewers. The intent is to shift that feedback to authoring time or 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 drafted but is being held back for team review, since it's more strategic than mechanical.

How to Test the Changes

Concurrency: cosmetic CI-config change; verifiable by observing that a second push to a branch no longer cancels the other workflow's run.
Migration gate: python manage.py makemigrations --check --dry-run exits 0 on this branch. Verified locally against the CI compose file.
Everything else is documentation.

Checklist

I have tested these changes appropriately.
I have added and/or modified relevant tests. (N/A — CI + docs; the migration is state-only)
I updated relevant documentation or comments.
I have verified that this PR follows the project's coding standards.
Any dependent changes have already been merged to main.

🤖 Generated with Claude Code

The `test.backend.yml` and `test.frontend.yml` workflows used the same concurrency group (`head_ref || run_id`), so a push that touched both `ui/**` and non-`ui/**` paths would race the two workflows against the same group, and `cancel-in-progress: true` killed the one that started second. Observed on PR #1306 head `0d05fb47`: Backend Tests succeeded, Frontend Tests got cancelled three seconds after starting. The FE run was re-run manually and passed. Namespace the group by `${{ github.workflow }}` so each workflow has its own group. Cancel-on-new-push within the same workflow stays as before. Co-Authored-By: Claude <noreply@anthropic.com>

…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>

Add makemigrations --check --dry-run step before the test run. Forgotten migrations have repeatedly shipped to main and required standalone follow-up PRs (#722, #897, #946); this catches them at PR time. Co-Authored-By: Claude <noreply@anthropic.com>

netlify · 2026-06-09T23:06:06Z

✅ Deploy Preview for antenna-preview canceled.

Name	Link
🔨 Latest commit	`4e6a57a`
🔍 Latest deploy log	https://app.netlify.com/projects/antenna-preview/deploys/6a28a4d500db540008a93fce

netlify · 2026-06-09T23:06:06Z

✅ Deploy Preview for antenna-ssec canceled.

Name	Link
🔨 Latest commit	`4e6a57a`
🔍 Latest deploy log	https://app.netlify.com/projects/antenna-ssec/deploys/6a28a4d5c511c80007487265

coderabbitai · 2026-06-09T23:06:09Z

Warning

Review limit reached

@mihow, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 7 minutes and 31 seconds. Learn how PR review limits work.

Your organization has run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 7290f656-f03a-4f18-a82f-21aa50f6f6d6

📥 Commits

Reviewing files that changed from the base of the PR and between 13203e4 and 4e6a57a.

📒 Files selected for processing (10)

.agents/AGENTS.md
.github/workflows/test.backend.yml
.github/workflows/test.frontend.yml
ami/jobs/migrations/0023_alter_job_job_type_key.py
docs/claude/INDEX.md
docs/claude/reference/canonical-patterns.md
docs/claude/reference/query-patterns.md
docs/claude/reference/worktree-testing.md
ui/AGENTS.md
ui/CLAUDE.md

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch fix/ci-concurrency-per-workflow

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.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

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>

Copilot

Pull request overview

This PR improves CI reliability and contributor/agent guidance by (1) isolating workflow concurrency to avoid cross-workflow cancellations, (2) enforcing a “missing migrations” gate in backend CI, and (3) restructuring/adding agent reference docs (plus a frontend-specific conventions doc) to prevent common review-time issues.

Changes:

Namespaced GitHub Actions concurrency.group per workflow for backend/frontend test workflows.
Added a backend CI step to fail if Django detects missing migrations, plus a state-only migration fixing Job.job_type_key choices.
Added/reorganized agent documentation (index + reference docs) and introduced ui/CLAUDE.md for frontend conventions.

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
`.github/workflows/test.backend.yml`	Namespaced concurrency group; added migration-check gate step.
`.github/workflows/test.frontend.yml`	Namespaced concurrency group to avoid cross-workflow cancellation.
`ami/jobs/migrations/0023_alter_job_job_type_key.py`	State-only migration to align `job_type_key` choices with `VALID_JOB_TYPES`.
`.agents/AGENTS.md`	Added domain invariants + checklists; moved bulky reference material into docs and linked to it.
`docs/claude/INDEX.md`	Added an index for agent-oriented docs (reference/runbooks/plans).
`docs/claude/reference/canonical-patterns.md`	Canonical “reuse” table pointing to existing helpers/patterns.
`docs/claude/reference/query-patterns.md`	Extracted DB/query reference and QuerySet method catalog into a load-on-demand doc.
`docs/claude/reference/worktree-testing.md`	Extracted worktree testing procedures into a dedicated doc.
`ui/CLAUDE.md`	Added frontend conventions (i18n, types, async/mutations, lint/format reality).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

- ui/CLAUDE.md: React Query is pinned to v4, so mutation in-flight state is isLoading, not the v5 isPending. - docs/claude/INDEX.md: fix stray space in the title heading. - docs/claude/reference/worktree-testing.md: use repo-relative paths instead of developer-specific absolute paths in the override example. - docs/claude/reference/canonical-patterns.md: correct the url_boolean_param usage reference to ami/jobs/views.py:165. - .github/workflows/test.backend.yml: add --noinput to makemigrations --check so a model change needing a default fails deterministically instead of blocking on a prompt. Co-Authored-By: Claude <noreply@anthropic.com>

…ructions Adds a "Writing Comments, Docstrings, and PR Text" section: keep the load-bearing fact and cut unverified mechanism, label hunches as hunches, keep comments short and link to the PR/issue for depth, and distinguish measured from inferred in PR descriptions. A confident but wrong comment is read as fact and built on, so confidence should match evidence. Co-Authored-By: Claude <noreply@anthropic.com>

Match the repo-root convention (CLAUDE.md -> .agents/AGENTS.md): the frontend conventions file is now ui/AGENTS.md, with ui/CLAUDE.md a symlink to it. Other agent tools read AGENTS.md; Claude Code auto-loads the nested CLAUDE.md symlink when editing under ui/. Updated references in the instructions and index. Co-Authored-By: Claude <noreply@anthropic.com>

…r-workflow

After merging main, VALID_JOB_TYPES includes regroup_events (added with the configurable session-gap work). Regenerated 0023 so the job_type_key field choices in the migration match the current registry; makemigrations --check passes against the merged tree. Co-Authored-By: Claude <noreply@anthropic.com>

mihow and others added 3 commits June 9, 2026 15:30

Copilot AI review requested due to automatic review settings June 9, 2026 23:06

Copilot started reviewing on behalf of mihow June 9, 2026 23:06 View session

mihow changed the title ~~Make CI catch more problems: per-workflow concurrency, a migration gate, and review checklists in the agent docs~~ Improve CI checks and contributor guidance Jun 9, 2026

mihow force-pushed the fix/ci-concurrency-per-workflow branch from 0fcc968 to bf85ca5 Compare June 9, 2026 23:09

Copilot AI reviewed Jun 9, 2026

View reviewed changes

mihow and others added 5 commits June 9, 2026 16:13

Merge remote-tracking branch 'origin/main' into fix/ci-concurrency-pe…

a96f184

…r-workflow

mihow merged commit 3f7f276 into main Jun 10, 2026
9 checks passed

mihow deleted the fix/ci-concurrency-per-workflow branch June 10, 2026 00:01

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improve CI checks and contributor guidance#1336

Improve CI checks and contributor guidance#1336
mihow merged 9 commits into
mainfrom
fix/ci-concurrency-per-workflow

mihow commented Jun 9, 2026 •

edited

Loading

Uh oh!

netlify Bot commented Jun 9, 2026 •

edited

Loading

Uh oh!

netlify Bot commented Jun 9, 2026 •

edited

Loading

Uh oh!

coderabbitai Bot commented Jun 9, 2026 •

edited

Loading

Review limit reached

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

mihow commented Jun 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

List of Changes

Detailed Description

How to Test the Changes

Checklist

Uh oh!

netlify Bot commented Jun 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Deploy Preview for antenna-preview canceled.

Uh oh!

netlify Bot commented Jun 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Deploy Preview for antenna-ssec canceled.

Uh oh!

coderabbitai Bot commented Jun 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review limit reached

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

mihow commented Jun 9, 2026 •

edited

Loading

netlify Bot commented Jun 9, 2026 •

edited

Loading

netlify Bot commented Jun 9, 2026 •

edited

Loading

coderabbitai Bot commented Jun 9, 2026 •

edited

Loading