chore(agents): Trim derivable content from AGENTS.md - #121311
Conversation
`AGENTS.md` is loaded into the context of every session in this repo (via the root `CLAUDE.md` `@AGENTS.md` import), so every line costs tokens on every turn. This removes content a session can reconstruct on its own and moves one task-specific workflow behind lazy loading. Removed as derivable from the repo: - Frontend command fences (`pnpm run dev`, `dev-ui`, `typecheck`, `lint:js`, `fix`, `test-ci`) — all are `package.json` scripts. Collapsed to prose that keeps the parts that are *not* derivable: that `typecheck` takes no file paths, that `lint:js` does, and the two dev-server URLs. - Standard Django/Make invocations (`sentry django migrate`, `makemigrations`, `make reset-db`). Kept `./bin/update-migration`, which is a repo-specific script and not guessable. - Duplicated `## Backend` / `## Frontend` pointer sections, already covered by the "Context-Aware Loading" table above them. - A filler sentence under "Command Execution Guide". Moved to lazy loading: - The five-step FlagPole workflow is now the `feature-flags` skill. Only its description stays resident; the body loads on invocation. `AGENTS.md` keeps a one-line stub so the *policy* (new features should be gated) is still always in context, and `src/AGENTS.md` now points at the skill. Also fixes an inconsistent invocation: the typecheck line said `pnpm typecheck` where `package.json` defines the script as `pnpm run typecheck`. Net: AGENTS.md 8,623 -> 7,243 chars (~2,155 -> ~1,810 est. tokens), minus ~108 est. tokens for the new skill's resident description.
| # Proxies API requests to production sentry.io | ||
| pnpm run dev-ui | ||
| ``` | ||
| `pnpm run dev` starts the full development server (requires `devservices up`). |
There was a problem hiding this comment.
The derivability test I applied throughout: could a session working in this repo reconstruct this line by reading the code?
Every command in the fences I removed here is a package.json script, so a session that needs one can run jq .scripts package.json. Paying for them in every session context is pure duplication — and worse, it means a script rename silently leaves stale instructions in the always-loaded file.
I converted these to prose rather than deleting outright, because the non-derivable facts were interleaved with the derivable commands. pnpm run typecheck is recoverable from the manifest; "it does not accept file paths" is not — that is a gotcha you only learn by trying it and getting confused. Same for lint:js accepting paths, and both dev-server URLs, which appear nowhere in the manifest.
| @@ -85,32 +83,17 @@ For backend-scoped changes, prioritize running the individual relevant pytest fi | |||
| #### Database Operations | |||
|
|
|||
| ```bash | |||
There was a problem hiding this comment.
This block is where the derivability test cuts both ways, so it is worth showing the line I drew.
Cut: sentry django migrate and makemigrations are the standard Django management commands — any session already knows them. make reset-db is discoverable with make -n reset-db.
Kept: ./bin/update-migration <migration_name_or_number> <app_label> stays, and it is the clearest keep in the whole diff. Nothing in the repo layout tells you that script exists, its two positional arguments are not guessable, and the comment explaining what it handles (renaming, dependencies, lockfile) is real institutional knowledge. A session that did not know about it would hand-edit the migration and the lockfile and get it subtly wrong.
Rule of thumb this encodes: standard tool invocations go, repo-specific scripts stay.
| @@ -177,49 +142,9 @@ Skills under `.agents/skills/` should follow the same current-practice conventio | |||
| - Keep skill descriptions aligned with natural user requests like PR review, branch audit, and Warden follow-up. | |||
There was a problem hiding this comment.
Two separate things happen in this hunk — the largest deletion in the diff — and they had different reasoning.
1. The ## Backend / ## Frontend pointer sections are gone as redundant. They said "see src/AGENTS.md" and "see static/AGENTS.md", which is exactly what the Context-Aware Loading table ~15 lines above them already says, in table form. Pointers are normally a keep — a session cannot derive that static/AGENTS.md is where design-system rules live — but there is no reason to pay for the same pointer twice.
2. The FlagPole workflow moved to a skill, and the stub that replaced it is the deliberate part.
The migration trade is honest but not free: ~230 est. tokens of always-resident body became ~108 est. tokens of always-resident skill description. Net saving is only ~120 tokens. What actually justifies it is that the 250-token body now loads only when someone is genuinely adding a flag, instead of sitting in context during every unrelated frontend task.
The stub is the important design decision. I kept the heading and one line — "New features should be gated behind a feature flag" — because migrating the whole section would have made the policy lazy along with the mechanics. A session that never invokes the skill would then never learn that gating is expected, and would ship an ungated feature. The rule I followed: mechanics can be lazy, obligations cannot.
The skill body is byte-identical to what was removed here — verified with diff, no content lost or reworded.
| @@ -0,0 +1,39 @@ | |||
| --- | |||
| name: feature-flags | |||
| description: Gate a Sentry feature behind a FlagPole feature flag. Use when adding a feature flag, registering a flag in temporary.py, checking a flag from Python or the frontend, enabling a flag in tests, or asking where FlagPole rollout config lives. Trigger on "add a feature flag", "gate this behind a flag", "register a flag", "features.has", "api_expose", "OrganizationFeature", "ProjectFeature", "FlagPole". | |||
There was a problem hiding this comment.
The description frontmatter is the only part of this file that stays in context permanently — the body below it loads on invocation. That makes the description the load-bearing line, so I wrote it deliberately rather than as a one-line summary.
I front-loaded the identifiers someone would actually have on screen or in their prompt when they need this: features.has, api_expose, OrganizationFeature, ProjectFeature, FlagPole, temporary.py. The reason is that a lazily-loaded skill that never gets routed to is strictly worse than the always-loaded prose it replaced — you pay the description cost and get none of the content. Trigger-richness is what makes the migration pay off rather than just hide the guidance.
Two things I verified against the codebase before copying the body over, since a moved file is easy to leave stale:
src/sentry/features/temporary.pyexists, and themanager.add(..., FeatureHandlerStrategy.FLAGPOLE, api_expose=True)shape in step 1 matches real registrations in it.ProjectFeatureis genuinely used there (22 occurrences), so the project-scoped note is accurate.
Body content is byte-identical to what left AGENTS.md — this is a move, not a rewrite.
| ### Feature Flags | ||
|
|
||
| See **Feature Flags (FlagPole)** in `/AGENTS.md` for registration, the `features.has(...)` check, and test usage. | ||
| See the **feature-flags** skill (`.agents/skills/feature-flags/`) for registration, the `features.has(...)` check, and test usage. |
There was a problem hiding this comment.
This one-line change is the cleanup that makes the migration safe rather than lossy.
This was the only inbound reference to the moved section anywhere in the repo — I grepped for Feature Flags (FlagPole) across all markdown to confirm before touching it. Left alone, it would have pointed backend readers at /AGENTS.md for "registration, the features.has(...) check, and test usage", where they would now find only a one-line stub and no mechanics.
Worth noting for reviewers that this file is itself lazily loaded (it applies when working under src/), which is why the cross-reference existed in the first place rather than the content being duplicated here.
billyvg
left a comment
There was a problem hiding this comment.
Review: reasoning behind each change
I've left inline comments on each hunk explaining the specific call. This summary covers the framework they share and the judgment calls a reviewer might want to push back on.
The test applied throughout
Every line of AGENTS.md is in context for every session in this repo, so the question per section was: could a session working here reconstruct this by reading the code? If yes, it's duplication that also rots — a renamed package.json script leaves stale instructions in an always-loaded file that nobody thinks to update.
What that cut: package.json script echoes, standard Django/Make invocations, duplicated pointers, one filler sentence.
What it deliberately kept:
- Gotchas — "
typecheckdoes not accept file paths" is not in any manifest; you learn it by being confused. - Repo-specific scripts —
./bin/update-migrationand its two non-obvious positional args. - Non-derivable facts — the two dev-server URLs.
- Obligations — the FlagPole stub (see below).
The judgment call worth scrutinising
The FlagPole migration is the least clear-cut change here, and I want to be straight about the numbers: ~230 est. tokens of resident body became ~108 est. tokens of resident skill description. Net saving is only ~120 tokens — small. What justifies it is that the 250-token body now loads only when someone is actually adding a flag, rather than during every unrelated frontend task.
If a reviewer thinks that trade isn't worth an extra file and an indirection, reverting just that part is clean: restore the section in AGENTS.md, revert the src/AGENTS.md line, delete the skill directory. The other cuts stand on their own.
The related decision I'd defend more firmly is the stub. Migrating the whole section would have made the policy lazy along with the mechanics — a session that never routes to the skill would never learn that gating is expected, and would ship an ungated feature. Hence: mechanics can be lazy, obligations cannot.
Verification performed
- Every
package.jsonscript named in the removed fences confirmed present before cutting. ./bin/update-migrationconfirmed to exist;make -n reset-dbconfirmed the target resolves.src/sentry/features/temporary.py, themanager.add(...)call shape, andProjectFeatureusage all confirmed before copying the skill body.- Migrated body confirmed byte-identical to the removed section via
diff— a move, not a rewrite. - Grepped all markdown for inbound references to the moved section; found exactly one (
src/AGENTS.md) and updated it. prek runpasses.
Scope note
This PR is documentation-only, so the frontend/backend split rule doesn't apply — but note it touches both AGENTS.md and src/AGENTS.md, neither of which is deployed code.
Two things I found but deliberately did not change:
devservicescontradiction. My local~/.claude/CLAUDE.mdtells agents never to rundevservices(it pegs the CPU on my machine);AGENTS.mdsays to run it. That's a machine-specific constraint and the checked-in guidance is correct for the team, so it stays.AGENTS.mdis still ~1,810 est. tokens. Everything remaining passed the test — the.venvrequirement, the customer-info prohibition, the PR-split rule, thedev.logtip. Further reduction would mean moving universal constraints into lazy files, which is the wrong trade.
| @@ -0,0 +1,39 @@ | |||
| --- | |||
| name: feature-flags | |||
|
|
||
| ## Backend | ||
|
|
||
| For backend development patterns, security guidelines, and architecture, see `src/AGENTS.md`. |
There was a problem hiding this comment.
nice cleanup, this routing was duplicated lol
…121469) Recommend reviewing commit-by-commit to see exact path of content shifting from system prompt -> skills, see #121311 for another recent trim ## Summary Audit and refactor of the repo's `AGENTS.md` / `CLAUDE.md` setup to reduce always-loaded agent context and defer task-specific procedures to on-demand skills, following current prompt-engineering guidance (progressive disclosure; "AGENTS.md = README for agents, reference don't copy"; keep always-on files short). Our own `agents-md` skill targets ≤100 lines per file and Claude's memory docs target <200 lines; the files had drifted well past both. Deep, task-specific how-to (RTL testing guide, TanStack Query essays, logging/tracing/metrics walkthroughs) was paying full context cost on every unrelated task. This moves that content into skills that load only when relevant. ## Result Always-loaded AGENTS.md context reduced ~64% (1381 → 495 lines); every file now under the 200-line target: | File | Before | After | |------|--------|-------| | `AGENTS.md` (root) | 234 | 140 | | `src/AGENTS.md` | 449 | 170 | | `static/AGENTS.md` | 592 | 77 | | `tests/AGENTS.md` | 106 | 108 | No information lost — deep content moved into skills (progressive disclosure) with one-line pointers left behind. ## New skills - **`react-testing`** — full RTL guide (extracted from `static/`) - **`backend-conventions`** — logging (`LOG005`/`LOG011`), tracing/spans, metrics tag cardinality, options system (from `src/`) - **`frontend-data-fetching`** — TanStack Query / `apiOptions`, type-inference rules, response headers (from `static/`) - **`feature-flags`** — FlagPole register/check/test/rollout (from root) ## Deferred to existing skills (pointers, no new skills) - IDOR/scoping → `django-access-review`, `sentry-security` - Serializer N+1, composite indexes → `django-perf-review` - Silo/RPC/outbox → `hybrid-cloud-*`, `cell-architecture` - Migrations, DB reset, per-worktree env → `generate-migration`, `setup-dev` - Design-system worked examples (avatars/disclosure/icons/images) → `design-system` - API docs → `document-api-endpoint`
I ran
claude doctorand these were the suggestionsSlop
AGENTS.mdis loaded into every session's context in this repo (the rootCLAUDE.mdis just@AGENTS.md), so every line costs tokens on every turn. This trims content a session can reconstruct on its own, and moves one task-specific workflow behind lazy loading.Net:
AGENTS.md8,623 → 7,243 chars (~2,155 → ~1,810 est. tokens), minus ~108 est. tokens for the new skill's resident description.Removed as derivable from the repo
dev,dev-ui,typecheck,lint:js,fix,test-ciare allpackage.jsonscriptssentry django migrate/makemigrations/make reset-db## Backend/## Frontendpointer sectionsThe frontend sections became prose rather than being deleted, because the non-derivable facts were interleaved with the derivable commands — that
typecheckaccepts no file paths, thatlint:jsdoes, and the two dev-server URLs. Those all stay../bin/update-migrationalso stays: it's a repo-specific script with non-obvious arguments, so it fails the derivability test.Moved to lazy loading
The five-step FlagPole workflow is now the
feature-flagsskill. Only its description stays resident; the body loads when the model routes to it. The body is byte-identical to what was removed.AGENTS.mdkeeps a one-line stub under the same heading, so the policy — new features should be gated — is still always in context even though the mechanics aren't.src/AGENTS.mdhad the only inbound reference and now points at the skill.Drive-by fix
The typecheck line said
pnpm typecheck;package.jsondefines the script aspnpm run typecheck.Verification
prek runpasses (format, whitespace, EOF, case conflicts).diff.package.jsonscript named in the removed fences verified present before cutting.What was deliberately not changed
The
devservicesguidance stays as-is. My local~/.claude/CLAUDE.mdtells agents never to run it (it pegs the CPU on my machine), which contradictsAGENTS.md— but that's a machine-specific constraint and the checked-in guidance is correct for the team.