Skip to content

docs: trim AGENTS.md to the non-derivable core and add CLAUDE.md - #1385

Open
mattbodle wants to merge 3 commits into
mParticle:mainfrom
mattbodle:docs/trim-agents-md
Open

docs: trim AGENTS.md to the non-derivable core and add CLAUDE.md#1385
mattbodle wants to merge 3 commits into
mParticle:mainfrom
mattbodle:docs/trim-agents-md

Conversation

@mattbodle

@mattbodle mattbodle commented Aug 28, 2026

Copy link
Copy Markdown
Contributor

Why

AGENTS.md had grown to 556 lines / 21.1 KiB, past the point where agents reliably follow it. It is now 161 lines / 8.2 KiB.

Every major agent vendor now publishes a budget for instruction files, and this one blew through them:

Consumer Published budget Before After
Claude Code target under 200 lines; longer files "consume more context and reduce adherence" 556 lines 161
Codex project_doc_max_bytes = 32 KiB for the whole merged root→cwd chain 21.1 KiB (66%) 8.2 KiB (26%)
Copilot "no longer than 2 pages" ~11 pages ~3

The failure mode isn't an error, it's dilution: past the budget the rules that matter get lost among the ones that don't, and Codex starts silently dropping nested, more-specific files once the chain fills.

CLAUDE.md symlink — the part that unblocks something

Claude Code reads CLAUDE.md, not AGENTS.md. There was no CLAUDE.md, so all 556 lines were invisible to it — while the repo ships seven .claude/skills/ definitions, i.e. it is clearly being used with Claude Code.

Anthropic's documented fix is a symlink or an @AGENTS.md import. This adds the symlink (create mode 120000), as ROKT/sdk-web does. Copilot independently accepts a root CLAUDE.md as a substitute for AGENTS.md, so one file serves every consumer with nothing to keep in sync.

The rule applied to every line

If a config file already states it, delete it. The config can't go stale; this file can.

That is also Anthropic's /doctor trim rule — cut directory layouts, dependency lists and architecture overviews; keep pitfalls, rationale, and conventions that differ from tool defaults.

Deleted because a config file says it:

  • the commit-type → version-bump table → release.config.js
  • every .prettierrc value → .prettierrc, and .eslintrc already sets "prettier/prettier": "error"
  • tsconfig target / lib / moduleResolutiontsconfig.json
  • the .nvmrc version, and every file count (60 .ts / 1 .js, 34 / 7, ~30 kits, ~58 src files) — all drift on the next migration PR
  • the ~40-line src/ tree, the 13-row Key Files table, the 13-term glossary, the private-module listing, five ASCII flow diagrams (ARCHITECTURE.md has the real ones), and the per-skill descriptions that ls .claude/skills/ already shows
  • the generic "read first, modify later" / "check existing tests" steps

Kept: the commands whose names don't tell you what they cover, the conventions no config encodes (naming — verified that no lint rule enforces it), and the traps.

Two things the sweep caught

1. The version-bump table was wrong. It claimed docs, test, refactor, perf, style, chore, ci, build and revert produce "no release". release.config.js gives ci, docs, test, refactor, style, build, chore and revert a patch release. Hand-copying a config into prose is how that happens, which is the argument for deleting it rather than fixing it.

2. Nothing checks the TypeScript. The first pass repeated the old file's claim that ".ts uses ESLint + GTS". Measured, that is false:

  • npm run lint is eslint src/ test/src/ with no --ext, so ESLint reports .js files only and never a .ts one (measured by running it)
  • npm run prettier globs "**/*.js"
  • gts:check exists as a script but no hook or workflow calls it
  • build:types runs tsc -p tsconfig.types.json || true — the || true swallows type errors
  • the pre-commit hook is "pre-commit": ["lint"], so it inherits the same .js-only scope

So neither CI nor the pre-commit hook catches a type error or a formatting slip in a .ts file, in a src/ that is now TypeScript throughout. That is trap 6, and npm run build:ts / npm run gts:check are surfaced in the commands table as the manual gates they are. Worth a look independently of this PR.

Other stale claims fixed

Was Now
"Create PRs against the master branch" main. Check PR for semantic target branch accepts only main and build/*. GitHub still offers master as the default base, and CONTRIBUTING.md:8 still says master.
"npm run test — Full suite (Karma + Jest)" npm test is Karma only; Jest is npm run test:jest.
tree listing identity.js, events.js, forwarders.js, persistence.js src/ is TypeScript throughout apart from the stub entry point (#1370 finished that migration).
tree had no kits/ at all kits/ is a top-level directory of vendored integrations, each with its own build and tests.
"Check ARCHITECTURE.md if available" It exists. Hedge dropped.
Only /verify listed All 7 skills named.

Traps added

  1. npm test needs a Firefox binarytest/karma.config.js runs ['ChromeHeadless', 'FirefoxHeadless']; without it the launcher crashes and takes karma-server with it.
  2. Karma's exit code can lie when the launcher dies — read the TOTAL: line, not $?.
  3. Both runners need a built dist/ — Karma serves ../dist/mparticle.js, Jest loads it via setupFiles. npm test builds first; npm run test:jest and a bare karma start do not, so Jest fails on a fresh clone for reasons unrelated to your diff.
  4. dist/ is committed and every build rewrites it — git checkout -- dist/ before committing.
  5. Prettier is pinned at 1.18.2 (exact, no caret), which predates optional chaining: x?.[key] makes it report Parsing error: Expression expected and silently stop formatting the whole file.
  6. Nothing checks your TypeScript — see above.
  7. Jest skips two kits by designkits/adobe (kit-level Jest) and kits/rokt (Vitest, whose .spec.ts names Jest would falsely match).

Verification

  • Every path and config claim was checked against the tree at 8288cc96, and the ESLint scope was measured by running it rather than inferred.
  • Docs and one symlink only, no behaviour change. npm run prettier globs **/*.js and there is no markdown linter in CI.
  • No bare @path tokens, so Claude Code won't treat anything as an unintended import when reading through the symlink.

For the SDK team to decide

  1. The COMMON SECTION is hand-synced across SDK repos and its banner was stamped Last Updated: 2026-02-16. I trimmed within the markers and kept them, but I can only see this repo — the trim needs mirroring, or the convention wants replacing with a shared file. I also moved the branch-target rule out of the shared block, since master was wrong here precisely because it lived in the block every SDK copies.
  2. CONTRIBUTING.md:8 still says "PR into the mParticle SDK master branch". Left alone as out of scope; happy to fix here if preferred.
  3. .github/workflows/reusable-workflows.yml still passes base_branch: "development" to security-checks.yml@stable — another master-era leftover.
  4. The TypeScript gap in finding 2 is a CI question, not a docs one. Documented as a trap so agents compensate, but the real fix is --ext .ts on the lint script, dropping the || true, or wiring up gts:check.

AGENTS.md was 556 lines / 21.5 KB, the point at which agents start
ignoring it. Claude Code targets under 200 lines per instruction file and
Codex caps the whole merged instruction chain at 32 KiB, so this file
alone consumed 66% of that budget. It is now 199 lines / 9.5 KB.

Cut the content an agent can derive by reading the repo: the annotated
src/ tree, the Key Files table, the glossary, the private-module listing,
and the ASCII flow diagrams (ARCHITECTURE.md already holds the real
diagrams). Kept the commands, the conventions that differ from tool
defaults, and the gotchas.

Fixed things that had gone stale or were wrong:

- Open PRs against main, not master. The semantic target-branch gate
  accepts only main and build/*, master is 51 commits behind main, and
  development is a legacy alias at master's commit.
- npm test runs Karma only. Jest is a separate npm run test:jest.
- src/ is TypeScript now, not a JS/TS mix; mParticle#1370 finished that migration.
- The src/ tree omitted kits/ entirely, ~30 vendored integrations.
- ARCHITECTURE.md exists, so drop the "if available" hedge.
- Only 1 of the 7 skills in .claude/skills/ was listed.

Added the traps that cost real debugging time and are invisible from the
script names: npm test needs a Firefox binary and Karma's exit code can
lie; both runners need a built dist/ but only npm test builds first;
dist/ is committed and every build rewrites it; Prettier is pinned to
1.18.2 and silently stops formatting a file containing ?.[; Prettier
checks only .js; Jest skips kits/adobe and kits/rokt.

CLAUDE.md is a symlink to AGENTS.md because Claude Code reads CLAUDE.md
and not AGENTS.md, so these instructions were invisible to it even though
the repo ships seven .claude/skills. sdk-web uses the same symlink.
@mattbodle
mattbodle requested a review from a team as a code owner August 28, 2026 21:06
@cursor

cursor Bot commented Aug 28, 2026

Copy link
Copy Markdown

PR Summary

Low Risk
Documentation and symlink only; no runtime, build, or SDK behavior changes.

Overview
Shrinks AGENTS.md from ~556 to ~161 lines so agent instruction files stay within vendor context budgets. The doc now states upfront that it only keeps pitfalls and conventions not already in repo config, and points architecture/API detail to ARCHITECTURE.md and public docs.

Adds root CLAUDE.md that references AGENTS.md, so Claude Code (and similar tools) load the same instructions without maintaining a second copy.

Replaces long generic SDK/web architecture content (glossary, src/ tree, flow diagrams, duplicated commit-type tables, lint/tsconfig listings) with web-specific essentials: PR rules (base main, branch naming, fork CI noise), a compact layout/commands table, seven “command traps” (Firefox/Karma, lying exit codes, dist/ build + commit churn, old Prettier vs optional chaining, TypeScript not gated by CI/lint), instance/DI model, gotchas, and skills directory pointer. The shared COMMON SECTION is trimmed and re-dated; logging/PII and commit guidance are tightened (e.g. version bumps defer to release.config.js).

Fixes several stale instructions removed with the old prose (e.g. PRs against master, npm test including Jest, .js-heavy src/ tree).

Reviewed by Cursor Bugbot for commit 7375b4a. Bugbot is set up for automated code reviews on this repo. Configure here.

Second pass, applying one test to every line: if a config file already
states it, delete it, because the config cannot go stale and this file
can. That removed another 38 lines and caught two errors in the first
pass.

Deleted because a config file says it:

- the whole commit-type -> version-bump table. It was also WRONG:
  release.config.js gives ci, docs, test, refactor, style, build, chore
  and revert a *patch* release, not "no release". Replaced with a pointer
  to that file and a warning that most types cut a release here.
- every .prettierrc value (tabWidth, singleQuote, trailingComma).
  .eslintrc sets "prettier/prettier": "error", so JavaScript formatting
  is already enforced and documenting the values only creates drift.
- tsconfig target, lib and moduleResolution. Kept only that
  strictNullChecks and noImplicitAny are off, since that one contradicts
  what an agent assumes about a TypeScript project.
- exact file counts (60 .ts / 1 .js, 34 / 7, ~30 kits, ~58 src files) and
  the .nvmrc version. All drift on the next migration PR.
- the annotated layout rows and skill descriptions that ls already shows.

Corrected: the first pass claimed ".ts uses ESLint + gts" and
"formatting is fully automated". Both false. Measured it - npm run lint
passes no --ext, so ESLint lints 21 .js files and zero .ts. npm run
prettier globs "**/*.js". gts:check exists but no hook or workflow calls
it. And build:types runs `tsc -p tsconfig.types.json || true`, which
swallows type errors.

So nothing in CI or the pre-commit hook checks TypeScript at all. That is
now trap 6, with npm run build:ts and npm run gts:check surfaced in the
commands table as the manual gates they are - a more useful fact than any
of the config values this pass deleted.

AGENTS.md is now 161 lines / 8.4 KB, down from 556 / 21.5 KB.
The trap said 'measured: 21 .js files, zero .ts'. That 21 came from a
different branch's working tree, and a count in an instruction file is
the staleness bomb this PR is supposed to remove - on this branch it
would be 9. The conclusion does not depend on the number: eslint is
invoked with no --ext, so it reports .js only and never a .ts file
whatever the file set. State that instead.
@sonarqubecloud

Copy link
Copy Markdown

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant