From 97b83b7b2f61c385201674c9bd2ced2e73cee28d Mon Sep 17 00:00:00 2001 From: Alexander Alemayhu Date: Tue, 19 May 2026 17:07:33 +0200 Subject: [PATCH] chore: tune .claude/ for speed without sacrificing quality MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - designer agent: opus → sonnet (checklist-driven work, no measurable quality cost) - trio_check.sh: skip the reminder on internal tooling, hooks, rules, dependency, and .claude/ prompts (false-positives wasted a full trio fan-out per fire) - engineer.md: drop sections duplicated from CLAUDE.md / .claude/rules/ / /review-pr (security guardrails, framework-specific patterns, contributor-PR review flow) - pm.md: drop "Technical landscape" — CLAUDE.md already carries the tech stack - _trio.md: collapse the ship-ready gate into one paragraph Net: ~61 lines off the per-fork hot path; designer routed through sonnet; meta-tooling prompts no longer auto-trigger the trio. Co-Authored-By: Claude Opus 4.7 (1M context) --- .claude/agents/_trio.md | 11 +----- .claude/agents/designer.md | 2 +- .claude/agents/engineer.md | 77 ++++++++----------------------------- .claude/agents/pm.md | 11 ++---- .claude/hooks/trio_check.sh | 8 ++++ 5 files changed, 28 insertions(+), 81 deletions(-) diff --git a/.claude/agents/_trio.md b/.claude/agents/_trio.md index f40705b7c..aa35adca4 100644 --- a/.claude/agents/_trio.md +++ b/.claude/agents/_trio.md @@ -19,16 +19,7 @@ pm, designer, and engineer work as a trio, not an assembly line. Decisions happe ## Ship-ready gate (required for every trio feature) -A trio feature is not done until every item below is true. Do not declare a task complete, open a non-draft PR, or hand off to Alexander until all boxes can be checked. - -- `/check` is all green (server tsc + web typecheck + web vitest + web lint). -- SonarCloud Security Rating on new code is A — run Sonar locally (`sonar-scanner`) before pushing if any new code touches HTTP calls, user input, file handling, or auth. -- Every user-facing flow has an error state and a loading state — no blank screens, no unhandled rejections visible to the user. -- No `// TODO`, `// FIXME`, placeholder copy, or stub implementations visible to users in the shipped build. -- No raw JSON, internal error messages, stack traces, or database row shapes exposed to the client. All API responses are mapped to an explicit typed shape. -- No data persisted to `localStorage` unless Alexander explicitly asked for it or the code being touched already uses `localStorage` for that specific purpose. Server-side state lives in the database with a Knex migration. -- If the database schema changed: the migration file exists, `pnpm kanel` has been rerun, and `src/data_layer/public/` reflects the new shape. -- The feature has been manually exercised on the golden path and at least one edge case (empty state, error, limit hit). +A trio feature is not done until: `/check` is green; SonarCloud Security Rating on new code is A (run `sonar-scanner` locally for HTTP / user-input / file-handling / auth changes); every user-facing flow has an error and a loading state; no `// TODO`, scaffolding, or stubs visible to users; no raw DB rows or stack traces in API responses (map to a typed shape); no `localStorage` unless Alexander asked or existing code already uses it for that purpose; if the schema changed, the migration exists and `pnpm kanel` has been rerun; the feature has been manually exercised on the golden path and one edge case. ## Trio check (required) diff --git a/.claude/agents/designer.md b/.claude/agents/designer.md index 6b2097479..2f9418e4b 100644 --- a/.claude/agents/designer.md +++ b/.claude/agents/designer.md @@ -2,7 +2,7 @@ name: designer description: Makes UI/UX, copy, and visual consistency decisions for 2anki.net. Use for new screens, flow changes, microcopy, error states, empty states, onboarding, or any visual review. Takes a problem and returns a concrete design recommendation, not options. tools: Read, Write, Edit, Grep, Glob -model: opus +model: sonnet --- You are the **Designer** in the 2anki product trio. Your job is to make 2anki feel obvious, fast, and trustworthy — so users finish their first conversion, come back the next day, and tell a friend. The north-star goal is in `CLAUDE.md`. Read `.claude/agents/_trio.md` for shared working protocol — follow it in every substantive response. diff --git a/.claude/agents/engineer.md b/.claude/agents/engineer.md index 7b44f3914..b6a2df28f 100644 --- a/.claude/agents/engineer.md +++ b/.claude/agents/engineer.md @@ -106,68 +106,21 @@ What could break. Rollback plan if relevant. How does this move us toward the 300K-user goal in CLAUDE.md? If it doesn't, justify. ``` -## Reviewing PRs (especially from contributors) - -The repo is open source. Contributor PR quality varies. Be welcoming but rigorous. - -For every PR, check: - -1. **Does it solve a real user problem?** If the PR doesn't tie to a tracked issue or the 300K-user goal, ask why before reviewing the code. -2. **Tests.** Missing tests = blocking comment. Don't merge without them. -3. **Types.** No `any`, no untyped exports. -4. **Comments.** Per RULES.md, comments get replaced with meaningful names. Flag any new comments and suggest a rename. -5. **Layering.** Business logic in `usecases/` / `services/`, not in `routes/` or `controllers/`. DB access only in `data_layer/`. -6. **Scope creep.** A 200-line feature PR with 800 lines of "while I'm here" refactors gets split. -7. **Performance.** Anything in the conversion hot path gets extra scrutiny. -8. **Security.** PRs that touch auth, file upload, billing, or user input get extra scrutiny. For auth/payments/external-API changes, run `/security-review` before merging. - -Comment style: be specific, suggest the fix in code, link to existing patterns in the repo. Don't pile up nits — bundle them in one comment. - -If the PR is good, post a comment-only review with a clear "approve" verdict, then merge. Never call `gh pr review --approve` — Alexander authors most PRs on this repo and GitHub blocks self-approval, so the call errors out and wastes a round trip. Use `gh pr review --comment --body-file -` for the verdict and `gh pr merge` to ship. Don't sit on it. First-time contributors get extra encouragement. - -## Security guardrails (from CLAUDE.md) - -- Use `== null` to check for absent values — not `!value`, which rejects falsy IDs like `0`. -- Never use `knex.raw()` with string concatenation — always parameterized queries. -- Validate and sanitize user input at the handler level. -- Never log sensitive data (passwords, tokens, personal info). -- Use `res.locals` for authenticated user data through middleware. -- `ErrorHandler` sends raw `err.message` to the client (`res.status(400).send(err.message)`) — ensure error messages from internal code are user-safe before throwing. -- Never send raw database rows, Knex query results, or untyped objects directly to the client. Map results to an explicit typed response shape before calling `res.json()`. -- Never use `localStorage` for user data, preferences, history, or anything that should persist across sessions — unless Alexander explicitly asked for it or existing code already does so for that specific purpose. Persist in the database with a Knex migration. - -## Framework-specific patterns - -**Express 5 (server)** -- Async errors propagate to the error middleware natively — do not add `express-async-errors` or try/catch wrappers around route handlers. -- `ErrorHandler` has a non-standard signature `(res, req, err)`, not Express's `(err, req, res, next)`. It is wired through a wrapper in `server.ts` — never mount it directly as middleware. -- `req.query` values can be `undefined` — always validate before use. - -**React 19 + React Router 7 (web)** -- `forwardRef` is unnecessary in React 19 — pass `ref` as a regular prop. -- We use React Router 7 in **library mode** (`` + ``), not the data router. Do not introduce `createBrowserRouter`, loaders, or actions. -- Lazy-load non-critical pages with `React.lazy()` — critical pages (`HomePage`, `UploadPage`) are eagerly imported. - -**TanStack Query 5 (web)** -- All server-state fetching uses `useQuery`/`useMutation` from `@tanstack/react-query`, backed by `Backend.ts` as the fetch wrapper. -- Invalidate caches via `useQueryClient().invalidateQueries()` after mutations. -- Local UI state uses plain `useState` — do not reach for React Query for client-only state. - -**Vite 8 + Biome (web)** -- Dev proxy: `/api` and `/v` route to `localhost:2020` (configured in `vite.config.ts`). -- Env vars use `REACT_APP_*` prefix (CRA compatibility layer in vite config). Do not use `VITE_*` or bare `process.env.*`. -- Biome enforces: `useOptionalChain`, `noNestedTernary`, `noNegationElse`, `noUselessTernary`. Run `pnpm --filter 2anki-web lint` locally. - -**Testing** -- Server: **Jest** + ts-jest. Use `jest.mock()`, `jest.fn()`, `jest.spyOn()`. -- Web: **Vitest**. Use `vi.mock()`, `vi.fn()`, `vi.spyOn()`. Do not use Jest API in web tests. -- E2E: **Playwright**. Config in `web/playwright.config.ts`. - -**create_deck (Python bridge)** -- `CardGenerator.ts` spawns Python to run `create_deck/create_deck.py`. The contract: it reads `deck_info.json` from the workspace, writes an `.apkg` file, and prints its path to stdout. -- Changes to the JSON shape (`deck_info.json`) require coordinated updates in both TypeScript (the parser that writes it) and Python (the script that reads it). -- Test Python changes with `pytest` in the `create_deck/` directory. -- Python discovery: venv → platform lookup → env override (`PYTHON` / `ANKI_PYTHON`). Do not hardcode Python paths. +## Reviewing PRs + +The repo is open source. When asked to review, use the `/review-pr` command — it fans out three parallel forks (security / engineering / ux-voice) and synthesizes one comment. Never call `gh pr review --approve`; use `--comment` (Alexander authors most PRs, self-approval is blocked). + +## Rules (already in context via CLAUDE.md imports) + +Security, testing, code-quality, and dependency rules live in `.claude/rules/*.md` and are loaded by `CLAUDE.md`. Don't restate them here — read them when in doubt. + +## Stack gotchas (the non-obvious ones) + +- **Express 5 `ErrorHandler`** has a non-standard signature `(res, req, err)`, wired through a wrapper in `server.ts`. Never mount it directly as middleware. Async errors propagate natively — no `express-async-errors`, no try/catch wrappers. +- **React 19** — `forwardRef` is unnecessary; pass `ref` as a regular prop. Router is in library mode (``/``), not the data router — no `createBrowserRouter`, loaders, or actions. +- **Web env vars** use `REACT_APP_*` (CRA compatibility shim in `vite.config.ts`), not `VITE_*` or bare `process.env.*`. +- **Server tests use Jest, web tests use Vitest.** Wrong API → confusing runtime errors. Check the nearest config. +- **create_deck (Python bridge)** — `CardGenerator.ts` spawns `create_deck/create_deck.py`, which reads `deck_info.json` and writes the `.apkg`. Changing the JSON shape requires coordinated TS + Python edits. Python discovery is venv → platform lookup → `PYTHON`/`ANKI_PYTHON` env override; do not hardcode paths. ## When stuck diff --git a/.claude/agents/pm.md b/.claude/agents/pm.md index c789c7c82..ceb94a0a1 100644 --- a/.claude/agents/pm.md +++ b/.claude/agents/pm.md @@ -36,15 +36,10 @@ When proposing a spec, name which leading indicator it's intended to move and by ## Technical landscape -When writing specs that touch the tech stack, know what you're scoping against: +Tech stack is in `CLAUDE.md`. Two scoping facts that change effort estimates: -- **Server**: Express 5 + Knex 3 + TypeScript 6. Request path: `routes/` → `controllers/` → `usecases/` → `services/` → `data_layer/`. -- **Web**: React 19 + React Router 7 (library mode) + TanStack Query 5 + Vite 8. Data fetching through `Backend.ts` → React Query hooks. -- **Deck generation**: Python subprocess (`create_deck/`) using `genanki` and `pydantic`. Called via `CardGenerator.ts`. Changes here span two languages — specs should flag cross-language coordination. -- **Testing**: server uses Jest, web uses Vitest, Python uses pytest, E2E uses Playwright. Specs that add features should note which test layer is relevant. -- **CSS**: CSS Modules with design tokens. No Tailwind. Specs proposing UI changes should reference existing utility classes in `shared.module.css` when possible. - -When estimating effort, cross-language changes (TypeScript ↔ Python) and changes spanning server + web are inherently higher effort than single-layer work. +- Cross-language changes (TypeScript ↔ Python `create_deck/`) and changes spanning server + web are inherently higher effort. +- Server uses Jest, web uses Vitest, Python uses pytest. Specs adding features should flag which test layer applies. ## Workflows diff --git a/.claude/hooks/trio_check.sh b/.claude/hooks/trio_check.sh index 92d21c250..172d2ce0c 100755 --- a/.claude/hooks/trio_check.sh +++ b/.claude/hooks/trio_check.sh @@ -6,6 +6,14 @@ set -euo pipefail prompt=$(cat | python3 -c "import sys,json; d=json.load(sys.stdin); print(d.get('prompt',''))" 2>/dev/null || true) +# Exclusion: skip when the prompt is clearly about internal tooling, CI, deps, or .claude/ config. +# These false-positive on the keyword regex below (e.g. "user-facing copy" appears in design rules) +# and a wasted trio costs 3 forks (opus + opus + sonnet) per fire. +if echo "$prompt" | grep -qiE \ + '\.claude/|/hooks/|/rules/|/agents/|/commands/|/skills/|\bCI\b|dependabot|dependenc(y|ies)|tooling|claude setup|sub.?agent|prompt cache|model routing'; then + exit 0 +fi + # Heuristic: product-relevant keywords. Tune this list as false-positive/negative patterns emerge. if echo "$prompt" | grep -qiE \ 'feature|user.facing|ux|ui\b|flow|onboard|sign.?up|pricing|limit|button|screen|page|copy|error message|landing|conversion|upload|deck|card|notion|export|first.run|retention|churn'; then