From 37172e51433ee05acf7b31b40e3732cb8354051c Mon Sep 17 00:00:00 2001 From: Siddhant Singh Date: Thu, 25 Jun 2026 13:16:41 +0530 Subject: [PATCH] adding the initial structure --- .claude/CLAUDE.md | 152 ++++++++++++++++++ .claude/README.md | 93 +++++++++++ .claude/agents/convention-reviewer.md | 141 ---------------- .claude/agents/crud-writer.md | 20 --- .claude/agents/feature-builder.md | 71 -------- .claude/agents/model-writer.md | 20 --- .claude/agents/route-writer.md | 21 --- .claude/agents/service-writer.md | 21 --- .../celery.md} | 24 +-- .claude/conventions/cross-cutting.md | 48 ++++++ .claude/conventions/crud.md | 2 +- .../error-handling.md} | 11 +- .../migration.md} | 31 ++-- .claude/conventions/model.md | 2 +- .claude/conventions/route.md | 4 +- .claude/conventions/service.md | 2 +- .../test-writer.md => conventions/test.md} | 12 +- .claude/skills/backend-conventions/SKILL.md | 42 +++++ .claude/skills/feature-builder/SKILL.md | 56 +++++++ .claude/skills/srd-creator/SKILL.md | 16 +- .../srd-creator/reference/srd-template.md | 13 ++ CLAUDE.md | 144 ----------------- README.md | 6 + docs/domain-map.md | 83 ++++++++++ 24 files changed, 537 insertions(+), 498 deletions(-) create mode 100644 .claude/CLAUDE.md create mode 100644 .claude/README.md delete mode 100644 .claude/agents/convention-reviewer.md delete mode 100644 .claude/agents/crud-writer.md delete mode 100644 .claude/agents/feature-builder.md delete mode 100644 .claude/agents/model-writer.md delete mode 100644 .claude/agents/route-writer.md delete mode 100644 .claude/agents/service-writer.md rename .claude/{agents/celery-task-writer.md => conventions/celery.md} (79%) create mode 100644 .claude/conventions/cross-cutting.md rename .claude/{agents/error-handling-humane-logging.md => conventions/error-handling.md} (95%) rename .claude/{agents/migration-writer.md => conventions/migration.md} (75%) rename .claude/{agents/test-writer.md => conventions/test.md} (90%) create mode 100644 .claude/skills/backend-conventions/SKILL.md create mode 100644 .claude/skills/feature-builder/SKILL.md delete mode 100644 CLAUDE.md create mode 100644 docs/domain-map.md diff --git a/.claude/CLAUDE.md b/.claude/CLAUDE.md new file mode 100644 index 000000000..b4601c10e --- /dev/null +++ b/.claude/CLAUDE.md @@ -0,0 +1,152 @@ +# CLAUDE.md + +This file provides guidance to Claude Code when working with code in this repository. + +## Project Overview + +Kaapi is an AI platform built with FastAPI and PostgreSQL, containerized with Docker. It provides AI capabilities including multi-provider LLM access (OpenAI, Anthropic, Google, plus speech providers like Sarvam/ElevenLabs — orchestrated via `litellm`), fine-tuning, document processing, collection management, and evaluation runs. Async work runs on Celery (RabbitMQ broker, Redis backend); observability via Langfuse, OpenTelemetry, and Sentry. + +## Key Commands + +### Development + +**The Python project root is `backend/`, not the repo root.** Every command below assumes `cd backend` first (`pyproject.toml`, `uv.lock`, `alembic.ini`, and `scripts/` all live there). Paths like `app/...` in this doc are relative to `backend/`. + +```bash +# Bring up the full stack (Postgres, RabbitMQ, Redis, backend, celery, Adminer, Flower) — from repo root +docker compose watch + +# --- the rest run from backend/ --- +cd backend + +# Start development server with auto-reload (stop the docker `backend` service first) +fastapi run --reload app/main.py + +# Run the Celery worker locally (separate terminal) +uv run celery -A app.celery.celery_app worker --loglevel=info + +# Lint / format (also wired into pre-commit) +bash scripts/lint.sh # ruff check + mypy strict +bash scripts/format.sh # ruff format + import sort +uv run pre-commit run --all-files + +# Generate database migration. +# Compute at runtime as the latest existing revision ID + 1, +# zero-padded to 3 digits (check the highest NNN in app/alembic/versions/NNN_*.py). +uv run alembic revision --autogenerate -m "Description" --rev-id +uv run alembic upgrade head + +# Seed database with test data +uv run python -m app.seed_data.seed_data + +# Internal admin/data CLI (entrypoint: app.cli.main:cli) +uv run ai-cli --help +``` + +### Testing + +Tests use `.env.test` for environment-specific configuration (env var `ENVIRONMENT=testing`). Run against a **real Postgres** — the suite runs `tests_pre_start.py` + `alembic upgrade head` before pytest; do not mock the DB session. + +```bash +# Full suite with coverage, from backend/ (applies test migrations, then pytest) +uv run bash scripts/tests-start.sh + +# A single test (after migrations are applied), from backend/ +uv run pytest app/tests/path/to/test_file.py::test_name -x + +# Dockerized run (from repo root): builds, brings up the stack, runs the suite, tears down +bash scripts/test.sh +``` + +## Architecture + +### Backend Structure + +The backend follows a layered architecture located in `backend/app/`: + +- **Models** (`models/`): SQLModel entities representing database tables and domain objects + +- **CRUD** (`crud/`): Database access layer for all data operations + +- **Routes** (`api/`): FastAPI REST endpoints organized by domain + +- **Core** (`core/`): Core functionality and utilities + - Configuration and settings + - Database connection and session management + - Security (JWT, password hashing, API keys) + - Cloud storage (`cloud/storage.py`) + - Document transformation (`doctransform/`) + - Fine-tuning utilities (`finetune/`) + - Langfuse observability integration (`langfuse/`) + - Exception handlers and middleware + +- **Services** (`services/`): Business logic services + - Response service (`response/`): OpenAI Responses API integration, conversation management, and job execution + +- **Celery** (`celery/`): Asynchronous task processing with RabbitMQ and Redis + - Task definitions (`tasks/`) + - Celery app configuration with priority queues + - Beat scheduler and worker configuration + + +### Authentication & Security + +- JWT-based authentication +- API key support for programmatic access +- Organization and project-level permissions + +## Environment Configuration + +The application uses different environment files: +- `.env` - Application environment configuration (use `.env.example` as template) +- `.env.test` - Test environment configuration + + +## Testing Strategy + +- Tests located in `app/tests/` +- Factory pattern for test fixtures +- Automatic coverage reporting + +## Code Standards + +- Python 3.12+ with type hints (`requires-python = ">=3.12"`; mypy `strict`, ruff target `py312`) +- Pre-commit hooks for linting and formatting + +## Coding Conventions + +Layer-specific conventions live in **`.claude/conventions/*.md`** — one doc per layer (`model`, `crud`, `service`, `route`, `migration`, `celery`, `test`, `error-handling`) plus `cross-cutting.md`. The **`backend-conventions`** skill indexes them, and the `feature-builder` skill loads each layer's doc when *building* a feature. Same source of truth, so design and code never drift. + +### Cross-cutting rules + +These are a terse summary for quick reference; `.claude/conventions/cross-cutting.md` is the authoritative, fuller version (update that file, not just this list). + +- **Type hints** on every parameter and return value. `-> Any` is not an annotation — narrow it or drop it. +- **Logging prefix:** every log line starts with the function name in square brackets. + ```python + logger.info(f"[function_name] Message | key: {value}") + ``` +- **`uv` is the runner**, not `pip`. Examples: `uv run pytest`, `uv run alembic ...`, `uv run pre-commit run --all-files`. +- **No magic values** in code — extract repeated literals to constants / `Enum` / settings. +- **Comments explain *why*, not *what*.** Don't restate what the code already says (`i += 1 # increment i`), don't narrate self-evident lines, and don't pad docstrings/migration descriptions with obvious recaps of the operations. A comment earns its place only by adding non-obvious context — rationale, a gotcha, a link, a constraint. When in doubt, delete it; clear code needs fewer comments, not more. +- **Naming:** `list_*` for plural fetch, `get_*` for singletons; snake_case funcs/vars, PascalCase classes, UPPER_SNAKE constants; `Enum` suffix on enum classes. +- **Timestamps** are `inserted_at` / `updated_at` (not `created_at`). + +## Skills (the feature lifecycle) + +Work is driven by skills, not specialist subagents. The lifecycle: + +| Stage | Skill | Output | +|---|---|---| +| Product spec | `start-prd` | `features//PRD.md` | +| Software spec | `srd-creator` | `features//SRD.md` (includes blast-radius impact analysis via `docs/domain-map.md`) | +| Execute SRD (build) | `feature-builder` | the code: model → crud → service → route, then migration / celery / tests | +| Review | `/pr-review` | convention / security / correctness gate on the diff | + +The `backend-conventions` skill is the conventions index that `feature-builder` loads per layer while building. + +### Building a feature + +The `feature-builder` skill walks the dependency spine **model → crud → service → route** in the current context, loading each layer's convention doc (via `backend-conventions`) right before writing that layer, then the migration / Celery / tests as needed, and finishes by running `/pr-review` on the diff. Only build the layers the feature touches; a single-layer change just loads that one doc. + +For a large feature where context size is a concern, you may dispatch a **general-purpose subagent per phase** (schema-spine → migration → tests) and pass only the artifacts forward (signatures, file paths, next migration rev-id) — the `feature-builder` skill works the same whether run inline or inside a dispatched subagent. diff --git a/.claude/README.md b/.claude/README.md new file mode 100644 index 000000000..ed4568752 --- /dev/null +++ b/.claude/README.md @@ -0,0 +1,93 @@ +# AI-assisted development (`.claude/`) + +How features get designed and built in kaapi-backend with Claude Code. The system is **skills + +convention docs** — no specialist subagents. Skills carry the *workflow*; convention docs carry +the *house style*; a domain map carries *impact analysis*. Every stage loads only what it needs. + +## The feature lifecycle + +A feature moves through four stages, each driven by a skill. Each stage consumes the previous +one's output — run them in order, but skip any that don't apply (a small change can go straight to +`feature-builder`). + +| Stage | Skill | Produces | Answers | +|---|---|---|---| +| 1. PRD (product spec) | `start-prd` | `features//PRD.md` | *why* are we building this, and for whom? (no tech) | +| 2. SRD (software spec) | `srd-creator` | `features//SRD.md` | *what* must the software do — endpoints, schema, scope, **+ blast radius** | +| 3. Execute SRD (build) | `feature-builder` | the code | model → crud → service → route, then migration / celery / tests | +| 4. Review | `/pr-review` | review notes | convention / security / correctness gate on the diff | + +Everything for one feature lives together in `features//` (PRD, SRD), reusing the same +kebab-case slug. + +## How to run it + +Invoke a skill by name (e.g. type `/srd-creator`), or just describe the task — the main agent +routes to the matching skill. A typical end-to-end run: + +1. **Discuss the idea**, then `/start-prd` → writes `features//PRD.md`. +2. `/srd-creator` → writes `features//SRD.md` (the testable spec). As part of this it reads + `docs/domain-map.md` and does **blast-radius analysis** — it stops and asks you about every + impacted surface the spec didn't address (in scope / deferred / out of scope), so a downstream + surface never gets silently dropped. `/grill-me` to stress-test the spec. +3. `/feature-builder` → **executes the SRD**: walks the dependency spine, loading each layer's + convention doc before writing that layer, then the migration / Celery tasks / tests. +4. `/pr-review` → the pre-merge gate. + +For a large feature you can dispatch a **general-purpose subagent per phase** (schema-spine → +migration → tests), passing forward only signatures, paths, and the next migration rev-id — the +`feature-builder` skill behaves the same inline or inside a subagent. + +## Code conventions + +All house style lives in **`.claude/conventions/*.md`** — one doc per layer. The +**`backend-conventions`** skill is the index: it maps each layer to its doc and tells you to load +the relevant ones *before* writing or reviewing that layer. `feature-builder` loads them while +building — **one source of truth, so the code never drifts from house style.** + +| Concern | Doc | Concern | Doc | +|---|---|---|---| +| Cross-cutting (always) | `cross-cutting.md` | Migrations | `migration.md` | +| Models | `model.md` | Celery tasks | `celery.md` | +| CRUD | `crud.md` | Tests | `test.md` | +| Services | `service.md` | Error handling | `error-handling.md` | +| Routes | `route.md` | | | + +The terse cross-cutting summary in `CLAUDE.md` defers to `cross-cutting.md` — edit the doc, not +just the summary. + +## The domain map (`docs/domain-map.md`) + +The source of truth for **blast-radius analysis**: which product surfaces consume which, so a +change to one surface doesn't silently break a downstream one. `srd-creator` traverses its +`Consumed by` edges (1-hop then 2-hop) while writing the SRD's **Impact / Blast Radius** section. +It's a dated snapshot — `srd-creator` reconciles it against live code and flags drift, but refresh +it as surfaces are added. + +## Directory map + +``` +.claude/ +├── README.md ← this file +├── CLAUDE.md project context loaded into every session (terse; defers to the docs) +├── skills/ +│ ├── start-prd/ PRD writer +│ ├── srd-creator/ SRD writer + blast-radius analysis (+ reference/ template & guide) +│ ├── backend-conventions/ conventions index/loader +│ ├── feature-builder/ the build workflow (executes an SRD) +│ └── grill-me/ stress-test a spec/design +└── conventions/ cross-cutting + per-layer code conventions (the source of truth) +docs/ +├── domain-map.md product surfaces + consumer edges (blast radius) +└── architecture/ deep-dive architecture docs per subsystem +features// PRD.md · SRD.md for each feature +``` + +## Maintaining the system + +- **Convention changed?** Edit the doc in `.claude/conventions/`. Then sync the `/pr-review` + checklist, which mirrors these docs as a self-contained review list. +- **New product surface?** Add it to `docs/domain-map.md` (surface + its consumes/consumed-by + edges) so future blast-radius analysis sees it. +- **New layer or skill?** Add the convention doc, register it in the `backend-conventions` index, + and reference it from `feature-builder`. diff --git a/.claude/agents/convention-reviewer.md b/.claude/agents/convention-reviewer.md deleted file mode 100644 index 3865a13ce..000000000 --- a/.claude/agents/convention-reviewer.md +++ /dev/null @@ -1,141 +0,0 @@ ---- -name: convention-reviewer -description: Use BEFORE committing or opening a PR to catch kaapi-backend convention violations early. Also use on demand when the user asks to "review", "check conventions", "lint my changes", or "see what /pr-review would flag". Read-only — never edits files. -tools: Read, Grep, Glob, Bash -model: sonnet ---- - -You are the local pre-commit gate for kaapi-backend. Your job is to run the same checklist that `/pr-review` runs at PR time, but on uncommitted or branch-local changes — so issues are caught before they become review comments. - -## How to gather the diff - -1. If the user supplied a PR number → `gh pr view ` + `gh pr diff `. -2. If the user said "branch" / "this branch" / "my changes" / supplied no argument → `git diff main...HEAD` + `git status` + `git log main..HEAD --oneline`. -3. If there are uncommitted changes that aren't in any of those, also inspect `git diff` (unstaged) and `git diff --cached` (staged). -4. `Read` full files at non-trivial change sites — judge in context, not from hunks. -5. `Grep` for duplication, reused literals, unused symbols. - -## What to check - -Skip any section in the output that has nothing notable. - -### Conventions -- Logs prefixed with `[function_name]`, levels matched to severity (`info`/`warning` for expected events, `error` only for genuine failures). -- Route descriptions via `description=load_description("/.md")`, never inline strings. `response_model` set; no untyped `dict` responses. -- DB columns get `sa_column_kwargs={"comment": "..."}` when purpose isn't obvious (status fields, JSON, foreign keys). -- Type hints on every parameter and return. `-> Any` is not an annotation — narrow it or drop it. -- `uv` is the runner, not `pip`. - -### Layering & duplication -- `HTTPException` belongs in routes (and is acceptable in `services/` for orchestration), **never** in `crud/`. CRUD returns data / `None` / raises domain errors. Third-party network calls also don't belong in `crud/` — that's DB-only. -- Routes thin, business logic in `services/`, DB access in `crud/`. -- Grep before approving: if a JWT pair, callback sender, or auth helper is duplicated across 2+ files, push for a single util. Before suggesting "extract a helper", confirm one doesn't already exist. -- Look for simplification — three near-identical functions (`_execute_text/_pdf/_image`) often collapse into one. - -### Magic values & config -- Repeated literals (provider names, status values, `"custom_id"`, route paths, magic numbers like `1_000_000`) → constant / Enum / config. Name the other location where it's reused. -- Hardcoded operational config (worker counts, model names, token limits, timeouts, retry counts) → env / config. Defaults lean toward smallest/cheapest, not most expensive. -- Dict crossing function boundaries where a Pydantic model belongs. - -### Naming -- `list_*` for plural fetch, `get_*` for singletons. Verb plurality matches return shape (`load_secrets_from_aws` if it returns multiple). Suffix `Enum` on enum classes. snake_case funcs/vars, PascalCase classes, UPPER_SNAKE constants. -- No leftover names from copy-paste of a sibling file. -- Alphabetical / grouped imports and route registrations, consistent with the rest of the repo. PEP 8 import order (stdlib first). -- Timestamp columns use `inserted_at` not `created_at` (per migration 060 cleanup). - -### Error handling -- `try` wraps *only* the line(s) that throw. Bloated try blocks are bugs waiting to happen. -- Nested `try/except`: trace the path. A raised `HTTPException(404)` caught by an outer `except Exception` becomes `500` and the intended status is lost. -- Concrete exception types, not `except Exception:` / `except:`. -- Status codes: `422` for "wrong shape" (bad CSV) over `400`; `409` for conflicts; `201`/`204` on create/delete. -- Validation at the Pydantic layer or via explicit ownership checks (`organization_id`, `project_id` belong to caller). `assert` is not validation in production code. -- Errors to the client must not leak internals (hashes, stack traces, paths, credentials). - -### Concurrency & data integrity -- "Compute next / check then write" patterns (`MAX(version)+1`, find-by-name-then-insert, increment counter) are races. Push for unique constraints, transactions, or DB-side sequences. -- JSON columns are fine for opaque metadata, not for fields you'll filter or sort on — push for first-class columns. -- Cross-codebase consistency: timestamp names (`inserted_at`), HTTP code choices, route shape (`/list` suffix is redundant). - -### API & response design -- Can the caller use this field? Is `data.id` the id of *what*? Are list responses missing fields the detail response populates (`signed_url`)? -- Swagger is a deliverable — generated docs must be unambiguous to an external client. -- All responses wrap in `APIResponse[T]` via `APIResponse.success_response(...)`. - -### FastAPI -- Router prefixes/tags/versioning consistent with the rest of `app/api/routes/`. -- `Depends(require_permission(...))` on every restricted endpoint, with the right `Permission` enum value. -- `SessionDep` / `AuthContextDep` for db + current user/org/project. -- Background tasks vs Celery: short fire-and-forget → `BackgroundTasks`; heavy or retryable → Celery in `app/celery/tasks/`. - -### Async correctness -- `async def` doesn't make blocking calls (sync DB drivers, `requests`, `time.sleep`, sync file I/O). -- `await` only on coroutines. CPU-bound work → threadpool / Celery / sync route. - -### Security -- No secrets / `.env` changes committed. -- Every endpoint has the right `Depends` and verifies `organization_id` / `project_id` ownership. -- API keys / hashes never returned raw — mask after a known prefix. -- **SSRF**: any URL the server fetches (callbacks, webhooks) needs scheme + private-IP validation, optionally an allowlist. -- File uploads enforce max size and content-type allowlist — required, not optional. -- DB / shell input parameterized (no f-string SQL, no `shell=True` with user input). - -### Performance -- N+1: loops issuing queries per row → `selectinload` / `joinedload` / batch fetch. -- New filter / FK columns → `index=True`. Pagination on list endpoints. - -### Pythonic idioms (small but recurring) -- Generators over materialized lists when iterated once. -- No redundant `str()` in f-strings; `x is None` over `not x` when None is what you mean; drop unneeded `return None`; no brackets when joining (`", ".join(p.value for p in Provider)`). -- Imports inside functions are a smell — usually a cycle that should be broken structurally. -- `setattr` on Pydantic / SQLModel objects → use `model_copy(update={...})` or `dataclasses.replace`. - -### Edge cases -For each new path, ask: input is `None`? list is empty? upstream call fails partway? what does the downgrade migration leave behind? - -### Migrations (treat as carefully as code) -- `--rev-id` = latest existing + 1; check `app/alembic/versions/`. Latest is `060` → next must be `061`. -- New tables include timestamps + indexes on FKs / common filters; nullability correct; no skipped seed IDs. -- `downgrade()` implemented and reversible — empty downgrade is a blocker. -- Backfills live in `upgrade()` SQL, not a separate manual script. - -### Cleanup -- Unused imports / functions / params / dead paths. -- Empty `__init__.py` for non-existent modules, scaffolding files no other file imports — ask "what reason was this added?" -- Commented-out blocks and `print(...)` debug removed. - -### Tests -- New behavior → test. Bug fix → regression test. Non-trivial code with zero tests → say so. -- Tests assert behavior, not implementation. Flag tautological / framework-only tests. -- Use the `app/tests/` factory pattern (`create_random_user`, `random_email`, `random_lower_string`) — no hardcoded `organization_id=1`. -- **Real DB only — no mocked database sessions.** This repo's `conftest.py` provides a transactional `db` fixture; tests must use it. -- Mocks match the real library's interface — prefer purpose-built mock libs over hand-rolled stubs. - -## How to write the findings - -- Cite `path:line`. Show the suggested change inline when short. -- **Name the failure mode**, not just the smell. Weak: "this try/except is too broad." Strong: "the `try` wraps the DB call too — if it raises, the handler returns 500 instead of the 404 you intended." -- **Pair criticism with a concrete fix**: a snippet, a library link, or a path in the repo that already does it right. -- **Question form** for judgment calls ("Why hardcode four workers?"). **Direct form** for unambiguous bugs. -- Hedge ("maybe", "I think") on judgment, not on correctness. -- Defer non-blocking work explicitly: "Not for this PR — worth a follow-up." Don't let style nits gate a merge. -- Tag severity: `VERY IMPORTANT:` / `MUST:` for security / data-loss / contract breaks; `nit:` for tiny cleanups. - -## Output format - -``` -## Summary -<1–3 sentences: what changed + verdict (clean / clean with nits / fix before commit).> - -## Blocking issues -- - -## Suggestions -- - -## Nits -- -``` - -Each item gets exactly one bullet — no item appears in more than one section. Use inline tags to mark domain when useful: `[migration]`, `[test]`, `[security]`, `[follow-up]`. Severity drives the section; the tag adds the domain colour. - -Drop empty sections. Don't pad. **Read-only — do not modify files during the review.** diff --git a/.claude/agents/crud-writer.md b/.claude/agents/crud-writer.md deleted file mode 100644 index f347e6b9d..000000000 --- a/.claude/agents/crud-writer.md +++ /dev/null @@ -1,20 +0,0 @@ ---- -name: crud-writer -description: Use when adding or modifying data-access functions under `app/crud/`. DB-only — never raises HTTPException, never makes external HTTP calls. Handles SQLModel/SQLAlchemy queries, eager loading to avoid N+1, and the canonical logging style. -tools: Read, Edit, Write, Bash, Grep, Glob -model: sonnet ---- - -You write CRUD functions for kaapi-backend, in `app/crud/` — the only place that talks directly to -the database via SQLModel/SQLAlchemy. - -**Before writing, Read `.claude/conventions/crud.md`** — it is the authoritative convention for this -layer (hard rules, canonical function shape, naming, performance, concurrency, error surface, and -what not to do). Apply it to the task. Then follow the handoff below. - -## After writing - -Tell the user: -1. The CRUD functions added (path + signature). -2. Any new domain exception type or relationship that the model needs. -3. Whether the route layer needs updating to call your new function. diff --git a/.claude/agents/feature-builder.md b/.claude/agents/feature-builder.md deleted file mode 100644 index b074dc43c..000000000 --- a/.claude/agents/feature-builder.md +++ /dev/null @@ -1,71 +0,0 @@ ---- -name: feature-builder -description: Use when building a full feature that spans multiple layers in kaapi-backend. Walks the dependency spine model -> crud -> service -> route in ONE context, pulling each layer's conventions on demand. For single-layer edits, use the matching standalone agent instead (model-writer / crud-writer / service-writer / route-writer). -tools: Read, Edit, Write, Bash, Grep, Glob -model: sonnet ---- - -You build complete features for kaapi-backend in a single context, walking the dependency spine -**model → crud → service → route**. You are Context 1 of a 4-context pipeline: - -``` -1. schema + code-spine ← YOU (model + crud + service + route) -2. migration → migration-writer -3. test → test-writer -4. review → convention-reviewer -``` - -## How you work - -1. **Scope the feature first.** Decide which layers it actually touches. A new entity touches all - four; a new endpoint over existing data may touch only service + route; a query-only change may - be crud + route. **Only build the layers the feature needs.** - -2. **Walk the spine in dependency order:** model → crud → service → route. Each downstream layer - depends on the one above it (route calls service calls crud uses model), so never build out of - order. - -3. **Before writing each layer, Read its convention doc and apply it** — these are the single source - of truth, shared with the standalone layer agents: - - model → `.claude/conventions/model.md` - - crud → `.claude/conventions/crud.md` - - service → `.claude/conventions/service.md` - - route → `.claude/conventions/route.md` - - **Lazy-load:** Read a doc only when you're about to write that layer. Skip docs for layers the - feature doesn't touch. - -4. **Do NOT do per-layer handoffs.** The standalone agents end each layer by telling the user "now - hand off to the next layer" — you don't. You ARE the next layer. Build the model, then keep going - straight into crud, then service, then route, all in this one context. - -5. **Stay within the spine.** Do NOT write the migration (Context 2), tests (Context 3), or Celery - tasks. If the feature needs background/async work, note it for `celery-task-writer`; don't build it. - -## Cross-cutting rules (apply at every layer) - -- Type hints on every parameter and return. `-> Any` is not an annotation. -- Logging: every line starts with `[function_name]`. Mask secrets with `mask_string` from `app.utils`. -- `uv` is the runner, not `pip`. -- No magic values — extract repeated literals to constants / `Enum` / settings. -- Comments explain *why*, not *what*. Don't restate what the code already says or pad docstrings with obvious recaps — a comment earns its place only by adding non-obvious context (rationale, gotcha, constraint). When in doubt, delete it. -- Naming: `list_*` plural fetch, `get_*` singleton; `Enum` suffix on enum classes. -- Timestamps are `inserted_at` / `updated_at`, never `created_at`. - -## After building the whole feature - -Emit ONE summary (not one per layer): - -1. The layers you built and the key signatures added (model variants; crud/service/route function - signatures + paths). -2. Any new `Permission` enum value, domain exception, or `.env.example` / settings key the user must add. -3. **The migration handoff:** if you added or changed any model field, state explicitly that a - migration is needed and give the next rev-id. Get it with - `ls app/alembic/versions/ | sort | tail -1` → next = that number + 1, zero-padded. Tell the user - to run Context 2 (`migration-writer`) with `--rev-id `. -4. What Context 3 (`test-writer`) should cover, and any external HTTP boundary it must mock. - -## If the feature is single-layer only - -Stop and tell the user a standalone agent is the better fit — `model-writer`, `crud-writer`, -`service-writer`, or `route-writer` — rather than spinning up the full spine for a one-layer change. diff --git a/.claude/agents/model-writer.md b/.claude/agents/model-writer.md deleted file mode 100644 index 18f3f7877..000000000 --- a/.claude/agents/model-writer.md +++ /dev/null @@ -1,20 +0,0 @@ ---- -name: model-writer -description: Use when adding or modifying SQLModel entities and their request/response variants under `app/models/`. Handles the Base/Create/Update/Public split, `sa_column_kwargs={"comment": "..."}` on every field, FK indexes, Enum naming, and first-class columns over JSON for filterable data. -tools: Read, Edit, Write, Bash, Grep, Glob -model: sonnet ---- - -You write SQLModel entities for kaapi-backend, in `app/models/`. - -**Before writing, Read `.claude/conventions/model.md`** — it is the authoritative convention for -this layer (structure, hard rules, naming, validation, indexes, and what not to do). Apply it to -the task. Then follow the handoff below. - -## After writing - -Tell the user: -1. The model variants added (Base, Create, Update, Public, table). -2. Which fields need indexes that aren't obvious. -3. **Explicitly:** "You now need a migration to add this to the DB. Hand off to `migration-writer` with `--rev-id `." Give them the next number by running `ls backend/app/alembic/versions/ | sort | tail -1`. -4. Whether `__init__.py` re-exports need updating so `from app.models import Foo` works. diff --git a/.claude/agents/route-writer.md b/.claude/agents/route-writer.md deleted file mode 100644 index c51cfbbeb..000000000 --- a/.claude/agents/route-writer.md +++ /dev/null @@ -1,21 +0,0 @@ ---- -name: route-writer -description: Use when adding or modifying FastAPI endpoints under `app/api/routes/`. Handles `response_model=APIResponse[T]`, `description=load_description(...)`, permission deps, status codes, HTTPException placement, and the matching swagger markdown in `app/api/docs/`. -tools: Read, Edit, Write, Bash, Grep, Glob -model: sonnet ---- - -You write FastAPI routes for kaapi-backend, in `app/api/routes/`. - -**Before writing, Read `.claude/conventions/route.md`** — it is the authoritative convention for this -layer (required endpoint ingredients, swagger markdown, layering rules, status codes, ownership -checks, background work, logging, and what not to do). Apply it to the task. Then follow the handoff -below. - -## After writing - -Tell the user: -1. The route file and line range you added. -2. The swagger markdown you created. -3. Any new `Permission` enum value or any CRUD function that the user (or `crud-writer` / `service-writer`) still needs to add. -4. A suggested `curl` or `httpie` invocation to smoke-test the endpoint. diff --git a/.claude/agents/service-writer.md b/.claude/agents/service-writer.md deleted file mode 100644 index bb9b7ab91..000000000 --- a/.claude/agents/service-writer.md +++ /dev/null @@ -1,21 +0,0 @@ ---- -name: service-writer -description: Use when adding or modifying business logic under `app/services/`. This is the only layer that combines DB access (via `app/crud/`) with external HTTP (OpenAI, Langfuse, S3, webhooks). Handles orchestration, SSRF guards, narrow try blocks, and domain-error translation. -tools: Read, Edit, Write, Bash, Grep, Glob -model: sonnet ---- - -You write business-logic services for kaapi-backend, in `app/services//`. Services -orchestrate — they call CRUD for DB work and external HTTP libraries for third-party APIs. - -**Before writing, Read `.claude/conventions/service.md`** — it is the authoritative convention for -this layer (what belongs here vs elsewhere, hard rules, SSRF/HTTP checklist, calling CRUD, config & -secrets, magic values, and what not to do). Apply it to the task. Then follow the handoff below. - -## After writing - -Tell the user: -1. The service function(s) added (path + signature). -2. Which CRUD functions you call and which you still need. -3. Any external HTTP boundary that the test layer should mock. -4. Any new env / settings key the user must add to `.env.example`. diff --git a/.claude/agents/celery-task-writer.md b/.claude/conventions/celery.md similarity index 79% rename from .claude/agents/celery-task-writer.md rename to .claude/conventions/celery.md index c5c346c01..6b94c4dc3 100644 --- a/.claude/agents/celery-task-writer.md +++ b/.claude/conventions/celery.md @@ -1,11 +1,9 @@ ---- -name: celery-task-writer -description: Use when adding or modifying Celery tasks under `app/celery/tasks/`. Handles queue/priority choice, retry policy, idempotency, OpenTelemetry trace propagation, and the gevent_timeout wrapper. -tools: Read, Edit, Write, Bash, Grep, Glob -model: sonnet ---- +# Celery task conventions (`app/celery/tasks/`) -You write Celery tasks for kaapi-backend. Tasks live in `app/celery/tasks/`. Celery uses RabbitMQ as broker and supports multiple priority queues. Read `app/celery/tasks/job_execution.py` before writing — it shows the full pattern (decorator + timeout + OTel propagation + delegation to a service). +Authoritative conventions for Celery tasks in kaapi-backend. Tasks live in `app/celery/tasks/`. +Celery uses RabbitMQ as broker and supports multiple priority queues. Read +`app/celery/tasks/job_execution.py` before writing — it shows the full pattern (decorator + +timeout + OTel propagation + delegation to a service). ## Canonical decorator stack @@ -28,7 +26,8 @@ def run_my_job(self, project_id: int, job_id: str, trace_id: str, **kwargs): ) ``` -`_set_trace`, `_run_with_otel_parent`, and `gevent_timeout` already exist in this module / `app/celery/utils.py` — reuse them, don't reinvent. +`_set_trace`, `_run_with_otel_parent`, and `gevent_timeout` already exist in this module / +`app/celery/utils.py` — reuse them, don't reinvent. ## Queue choice — be explicit @@ -73,12 +72,3 @@ Document the choice in a comment if it's not obvious. - Don't catch `Exception` and silently swallow — let it propagate so retries / failure handlers fire. - Don't run `.delay(...)` from another Celery task to chain — use a Celery `chain` / `chord` / `group` primitive if you need orchestration, or have the service return a result the next task picks up. - Don't use `time.sleep(...)` in a task to "wait for something" — schedule a follow-up task with `apply_async(countdown=...)`. - -## After writing - -Tell the user: -1. The task name(s) and the queue / priority chosen. -2. The service function it delegates to (path). -3. Whether Beat schedule needs an entry. -4. The idempotency strategy used. -5. How to invoke it locally for a smoke test (e.g., `uv run python -c "from app.celery.tasks.foo import run_my_job; run_my_job.delay(...)"`). diff --git a/.claude/conventions/cross-cutting.md b/.claude/conventions/cross-cutting.md new file mode 100644 index 000000000..a9e3ced22 --- /dev/null +++ b/.claude/conventions/cross-cutting.md @@ -0,0 +1,48 @@ +# Cross-cutting conventions (every layer) + +The rules that apply across **every** layer of kaapi-backend, regardless of which file you're +editing. This is the authoritative source; CLAUDE.md carries a terse summary that defers here, +and the layer agents/conventions reference it rather than restating it. + +## Rules + +- **Type hints** on every parameter and return value. `-> Any` is not an annotation — narrow it + or drop it. Use `|` unions (`str | None`), not `Optional[str]`. +- **Logging prefix:** every log line starts with the function (or `ClassName.method`) name in + square brackets: + ```python + logger.info(f"[function_name] Message | key: {value}") + ``` + Mask secrets / PII with `mask_string` from `app.utils`; never log raw payloads that may carry + sensitive data. Pick the log *level* by who is at fault, not merely "did it fail" — see + `error-handling.md` for the fault-based level table. +- **`uv` is the runner**, not `pip`: `uv run pytest`, `uv run alembic ...`, + `uv run pre-commit run --all-files`. +- **No magic values.** Extract repeated literals (provider names, status strings, route paths, + magic numbers like `1_000_000`) to a constant / `Enum` / settings. Operational config (worker + counts, model names, timeouts, retry counts) goes to env / config, not hardcoded. +- **Comments explain *why*, not *what*.** Don't restate what the code already says + (`i += 1 # increment i`), don't narrate self-evident lines, and don't pad docstrings / + migration descriptions with obvious recaps. A comment earns its place only by adding + non-obvious context — rationale, a gotcha, a link, a constraint. When in doubt, delete it. +- **Naming:** `list_*` for plural fetch, `get_*` for singletons (verb plurality matches the + return shape); snake_case funcs/vars, PascalCase classes, UPPER_SNAKE constants; `Enum` + suffix on enum classes. No leftover names from copy-pasting a sibling file. +- **Timestamps** are `inserted_at` / `updated_at`, never `created_at` (migration 060 renamed the + legacy stragglers — do not reintroduce them). + +## Layering (which layer may do what) + +- **Routes** are thin: parse/validate input, call a service, wrap the result in `APIResponse[T]`. + `HTTPException` belongs here. +- **Services** hold business logic and orchestration — the only layer that combines DB access + (via `crud/`) with external HTTP (OpenAI, Langfuse, S3, webhooks). `HTTPException` is + acceptable here for orchestration. +- **CRUD** is DB-only: returns data / `None` / raises domain errors. **Never** `HTTPException`, + **never** third-party network calls. +- **Models** are leaf nodes: pure data shape, no imports from `fastapi`, `app.crud`, or + `app.services`. + +Per-layer detail lives in the matching convention doc (`model.md`, `crud.md`, `service.md`, +`route.md`, `migration.md`, `celery.md`, `test.md`, `error-handling.md`) — load the one for the +layer you're touching. diff --git a/.claude/conventions/crud.md b/.claude/conventions/crud.md index 8d684a23a..189a47c17 100644 --- a/.claude/conventions/crud.md +++ b/.claude/conventions/crud.md @@ -44,7 +44,7 @@ Note: **keyword-only args** with `*` for anything more than `(session, id)`. Red ## Performance - **N+1 is a bug.** If you `list_` and the caller is going to access a relationship attribute, eager-load with `selectinload(...)` or `joinedload(...)`. Read the call sites before deciding. -- **Index any column you filter on.** That's a model-writer concern, but if you write a `get__by_` and the column has no index, flag it. +- **Index any column you filter on.** That's a model-layer concern (`model.md`), but if you write a `get__by_` and the column has no index, flag it. - **Pagination.** Any function that could return more than ~100 rows takes `limit: int` and `offset: int` (or `cursor`) — not "we'll add pagination later". ## Concurrency diff --git a/.claude/agents/error-handling-humane-logging.md b/.claude/conventions/error-handling.md similarity index 95% rename from .claude/agents/error-handling-humane-logging.md rename to .claude/conventions/error-handling.md index 88fcb4fa9..b1b2d9940 100644 --- a/.claude/agents/error-handling-humane-logging.md +++ b/.claude/conventions/error-handling.md @@ -1,11 +1,6 @@ ---- -name: exception-handling-error-message -description: Apply Kaapi's standardized error-handling pattern when adding or refactoring exception handling in code that calls external SDKs or makes raw HTTP requests. Use when the user asks to "add error handling", "improve error messages", "log and bubble errors", apply "the same pattern" across providers/CRUD layers, or asks why an error wasn't logged. Produces source-tagged ([KAAPI] vs []) descriptive errors with consistent logging. ---- +# Error-handling & humane-logging conventions -# Exception handling & error-message skill - -Apply this pattern when adding or refactoring error handling in: +Kaapi's standardized error-handling pattern. Apply when adding or refactoring error handling in: - LLM/AI provider wrappers (`app/services/llm/providers/*.py`) - CRUD layers that call external SDKs (`app/crud/**/*.py`) - Any code that calls an external SDK or makes raw HTTP requests @@ -195,7 +190,7 @@ Whenever the SDK exposes a per-request identifier (OpenAI/Anthropic's `request_i - ❌ Don't log every failure as `.warning` "because the operation failed." Pick level by **fault** (see Logging conventions table). Ops alerts fire on `.error` rate; everything-as-warning means real outages get buried. - ❌ Don't log every failure as `.error` either. `.error` on every 429 / 401 / 400 generates pager noise — those are the caller's fault. -## Workflow when applying this skill +## Workflow when applying this pattern 1. **Identify the provider's exception shape** — read the SDK's error module (e.g. `/errors/__init__.py` or `/_exceptions.py`) to enumerate available classes and what status codes they map to. 2. **Identify the Kaapi-side error sites** — every existing `return None, ""` and every `raise (...)` without a preceding `logger.*` call. diff --git a/.claude/agents/migration-writer.md b/.claude/conventions/migration.md similarity index 75% rename from .claude/agents/migration-writer.md rename to .claude/conventions/migration.md index b7ac28872..88689ddc8 100644 --- a/.claude/agents/migration-writer.md +++ b/.claude/conventions/migration.md @@ -1,17 +1,19 @@ ---- -name: migration-writer -description: Use when generating or hand-writing Alembic migrations under `app/alembic/versions/`. Handles --rev-id discipline, reversible downgrades, in-upgrade backfills, FK indexes, and CONCURRENTLY-built constraints. -tools: Read, Edit, Write, Bash, Grep, Glob -model: sonnet ---- +# Migration conventions (`app/alembic/versions/`) -You write Alembic migrations for kaapi-backend. The DB is PostgreSQL. Migration files live in `app/alembic/versions/` and follow a strict numeric ordering. +Authoritative conventions for Alembic migrations in kaapi-backend. The DB is PostgreSQL. +Migration files live in `app/alembic/versions/` and follow a strict numeric ordering. ## Before writing anything -1. `ls backend/app/alembic/versions/` and find the highest `NNN_*.py`. The new revision id is **that number + 1**, zero-padded to 3 digits. As of this writing the latest is `060` → next is `061`. Do not skip numbers. -2. If the change adds/removes/renames model fields, prefer `alembic revision --autogenerate -m "..." --rev-id ` (run via `uv`, not `pip`) and then hand-edit. For data-only changes (backfills, FK additions), write the migration by hand. -3. Read at least one recent migration (e.g., `060_v1_assorted_cleanups.py`) to match the project's docstring style and operation patterns. +1. `ls backend/app/alembic/versions/` and find the highest `NNN_*.py`. The new revision id is + **that number + 1**, zero-padded to 3 digits. (At time of writing the latest is `069` → next + is `070`; this number moves — always recompute from the directory, don't trust this line.) Do + not skip numbers. +2. If the change adds/removes/renames model fields, prefer + `alembic revision --autogenerate -m "..." --rev-id ` (run via `uv`, not `pip`) and then + hand-edit. For data-only changes (backfills, FK additions), write the migration by hand. +3. Read a recent migration (the highest-numbered file in that dir) to match the project's + docstring style and operation patterns. ## Required structure @@ -54,7 +56,7 @@ def downgrade(): - `inserted_at` (NOT `created_at`) and `updated_at` timestamps. Server default `NOW()` for backfill; the column comment should describe what the timestamp tracks. - `index=True` on every FK and every column commonly used in `WHERE` / `ORDER BY` / `GROUP BY`. - `sa.Column(..., comment="...")` for any column with a non-obvious purpose, matching the model's `sa_column_kwargs={"comment": "..."}`. -- **Adding a non-nullable column to a populated table**: add as nullable with `server_default=sa.text("...")`, backfill, then `ALTER COLUMN ... SET NOT NULL` and optionally drop the server default if the model has a `default_factory`. See `060_v1_assorted_cleanups.py` for the exact pattern. +- **Adding a non-nullable column to a populated table**: add as nullable with `server_default=sa.text("...")`, backfill, then `ALTER COLUMN ... SET NOT NULL` and optionally drop the server default if the model has a `default_factory`. See a recent migration for the exact pattern. - **Adding a unique constraint to a populated table**: dedupe first (`op.execute("DELETE ... USING ...")`), then `CREATE UNIQUE INDEX ... CONCURRENTLY` and `ALTER TABLE ... ADD CONSTRAINT ... USING INDEX` so the build doesn't take `AccessExclusiveLock`. - **Index builds on large tables**: use `CREATE INDEX CONCURRENTLY` via raw `op.execute(...)`. Note that CONCURRENTLY requires the migration to NOT run inside a transaction — set `transactional_ddl = False` if needed, or split the index build into its own migration. @@ -72,10 +74,3 @@ def downgrade(): - Don't skip the docstring. The docstring is what someone debugging at 2am will read. - Don't pad the docstring either — keep it to the non-obvious WHY. A step-by-step recap of the `op.*` calls (which the reader can already see in `upgrade()`) is noise. Same for inline comments: explain a tricky backfill or lock-avoidance trick, not `# add the column`. - Don't import from `app.models` to "save typing" — migrations must be model-independent so they still run after the model file is later renamed/deleted. - -## Output for the user - -When the migration is written, tell the user: -1. The new file path and revision id. -2. Any caller updates still needed (models, CRUD, tests). -3. The exact `uv run alembic ...` command(s) to apply and verify it. diff --git a/.claude/conventions/model.md b/.claude/conventions/model.md index e0e6371c2..e2467e4c4 100644 --- a/.claude/conventions/model.md +++ b/.claude/conventions/model.md @@ -101,7 +101,7 @@ class FoosPublic(SQLModel): ## What you DO NOT do -- Don't write the migration here — that's `migration-writer`. You write the model, then hand off (or tell the user) that migration `NNN+1` is needed. +- Don't write the migration here — that's the migration step (`migration.md`). You write the model, then note that migration `NNN+1` is needed. - Don't import from `fastapi`, `app.crud`, or `app.services` in a model file. Models are leaf nodes. - Don't use `setattr` on instances of these models. Use `model_copy(update={...})` or `sqlmodel_update(...)` (see `app/crud/user.py:update_user` for the pattern). - Don't put a `_status` private attr or computed property that hits the DB — model files are pure data shape. diff --git a/.claude/conventions/route.md b/.claude/conventions/route.md index 646f9b377..2733e6ea9 100644 --- a/.claude/conventions/route.md +++ b/.claude/conventions/route.md @@ -44,7 +44,7 @@ For every new endpoint, create `backend/app/api/docs//.md`. Keep - **Routes are thin.** Pull arguments, call a CRUD or service function, wrap the result. If your route has >20 lines of business logic, that logic belongs in `app/services//`. - **HTTPException is allowed here.** Use it when the caller-facing error needs a specific HTTP code (`404`, `403`, `409`, `422`). Catch domain exceptions from CRUD/services and translate. - **Never call third-party HTTP from a route.** That belongs in `app/services/`. -- **Never write SQL or `session.exec(select(...))` in a route.** Use a CRUD function. If one doesn't exist, ask the user whether to delegate creating it to `crud-writer`. +- **Never write SQL or `session.exec(select(...))` in a route.** Use a CRUD function. If one doesn't exist, add it following `crud.md`. ## Status codes (the ones to get right) @@ -70,7 +70,7 @@ Returning `404` instead of `403` for cross-tenant access is intentional — it d ## Background work - Short fire-and-forget (send an email, write an audit log) → `BackgroundTasks`. -- Heavy or retryable (LLM call, large doc transform, anything with timeouts) → Celery task in `app/celery/tasks/`. Hand off to `celery-task-writer`. +- Heavy or retryable (LLM call, large doc transform, anything with timeouts) → Celery task in `app/celery/tasks/`, following `celery.md`. ## Logging diff --git a/.claude/conventions/service.md b/.claude/conventions/service.md index c7c3819a7..4925fce5d 100644 --- a/.claude/conventions/service.md +++ b/.claude/conventions/service.md @@ -70,7 +70,7 @@ Delegation to `app.core.security` is the pattern — services orchestrate; primi - **Timeout** — every `httpx`/`requests` call has an explicit timeout. The default is too long. - **Retry policy** — idempotent GETs can retry with backoff. Mutations should retry only if you're certain the API is idempotent or you have an idempotency key. - **Error mapping** — `httpx.HTTPStatusError` → a domain exception or `HTTPException` with a sensible code (often 502 for upstream failures, NOT 500). -- **Mock at this boundary in tests** — `monkeypatch` the HTTP client, not the DB. (See `test-writer` agent.) +- **Mock at this boundary in tests** — `monkeypatch` the HTTP client, not the DB. (See `test.md`.) ## Calling CRUD diff --git a/.claude/agents/test-writer.md b/.claude/conventions/test.md similarity index 90% rename from .claude/agents/test-writer.md rename to .claude/conventions/test.md index fc19ec00b..8bd996c7f 100644 --- a/.claude/agents/test-writer.md +++ b/.claude/conventions/test.md @@ -1,11 +1,7 @@ ---- -name: test-writer -description: Use when writing or updating tests under `app/tests/` for kaapi-backend. Handles the factory pattern, transactional `db` fixture, real-DB testing (no mocked sessions), behavior-focused asserts, and seeded randomness. -tools: Read, Edit, Write, Bash, Grep, Glob -model: sonnet ---- - -You write pytest tests for kaapi-backend. Tests live under `app/tests/` and mirror the `app/` structure (`api/`, `crud/`, `services/`, `core/`, `models/`). +# Test conventions (`app/tests/`) + +Authoritative conventions for pytest tests in kaapi-backend. Tests live under `app/tests/` and +mirror the `app/` structure (`api/`, `crud/`, `services/`, `core/`, `models/`). ## Hard rules diff --git a/.claude/skills/backend-conventions/SKILL.md b/.claude/skills/backend-conventions/SKILL.md new file mode 100644 index 000000000..311be12a8 --- /dev/null +++ b/.claude/skills/backend-conventions/SKILL.md @@ -0,0 +1,42 @@ +--- +name: backend-conventions +description: Use when planning, designing, building, or reviewing kaapi-backend code and you need the house conventions for a layer — models, CRUD, services, routes, Alembic migrations, Celery tasks, tests, error handling, or the cross-cutting rules. Loads the authoritative per-layer convention docs on demand. +--- + +# Backend conventions + +The single index of kaapi-backend's code conventions. Each layer's authoritative rules live in a +doc under `.claude/conventions/`. This skill maps the layer you're touching to its doc so you +load only what you need. + +## How to use this + +1. Identify which layers your task touches (a full feature touches several; a single edit may + touch one). +2. **`Read` the cross-cutting doc first**, then **`Read` the doc for each layer you will write or + review.** Do this *before* writing or reviewing that layer's code — these docs are the source + of truth, not background reading. When planning a feature, read the docs for every layer the + plan's LLD will specify. +3. Apply each doc's rules to the task. If a doc names a canonical reference file in the repo + (e.g. `app/models/user.py`), read that too before writing. + +## Layer → convention doc + +| Layer / concern | Path under `app/` | Convention doc | Covers | +|---|---|---|---| +| **Cross-cutting** (always) | every file | `.claude/conventions/cross-cutting.md` | type hints, logging prefix + masking, `uv`, no magic values, comments, naming, timestamps, layering boundaries | +| **Models** | `app/models/` | `.claude/conventions/model.md` | Base/Create/Update/Public split, `sa_column_kwargs` comments, FK indexes, Enum naming, first-class columns over JSON | +| **CRUD** | `app/crud/` | `.claude/conventions/crud.md` | DB-only access, no `HTTPException`, eager loading to avoid N+1, logging style | +| **Services** | `app/services/` | `.claude/conventions/service.md` | orchestration (DB + external HTTP), SSRF guards, narrow try blocks, domain-error translation | +| **Routes** | `app/api/routes/` | `.claude/conventions/route.md` | `response_model=APIResponse[T]`, `load_description(...)`, permission deps, status codes, swagger markdown | +| **Migrations** | `app/alembic/versions/` | `.claude/conventions/migration.md` | `--rev-id` discipline, reversible downgrades, in-upgrade backfills, FK indexes, CONCURRENTLY constraints | +| **Celery tasks** | `app/celery/tasks/` | `.claude/conventions/celery.md` | queue/priority, retry policy, idempotency, OTel propagation, `gevent_timeout` | +| **Tests** | `app/tests/` | `.claude/conventions/test.md` | factory pattern, transactional `db` fixture, real-DB (no mocked sessions), behavior asserts | +| **Error handling** | provider wrappers, SDK-calling CRUD | `.claude/conventions/error-handling.md` | `[KAAPI]`/`[]` source tags, status→message templates, fault-based log levels | + +## Who else uses this + +- `feature-builder` loads each layer's doc right before writing that layer while executing a + feature's SRD — these docs are the single source of truth for code style. +- `/pr-review` is the review gate; it carries its own checklist mirroring these docs, so when a + convention doc changes, update that checklist to match. diff --git a/.claude/skills/feature-builder/SKILL.md b/.claude/skills/feature-builder/SKILL.md new file mode 100644 index 000000000..dd131a072 --- /dev/null +++ b/.claude/skills/feature-builder/SKILL.md @@ -0,0 +1,56 @@ +--- +name: feature-builder +description: Use when executing a kaapi-backend feature from its SRD (or a direct request) — writing models, CRUD, services, routes, Alembic migrations, Celery tasks, or tests. Walks the model→crud→service→route spine, loading each layer's house conventions before writing it. +--- + +# Feature Builder + +Build a feature by walking the dependency spine and loading the house conventions for each layer +*before* you write it. This runs **inline in the current context**. For a large feature you may +dispatch a general-purpose subagent per phase to keep context lean — pass forward only the +artifacts (function signatures, file paths, the next migration rev-id), never re-derive prior +reasoning. + +## Workflow + +1. **Start from the SRD, then scope the layers.** Read `features//SRD.md` — its endpoints, + schema, execution flow, and **Impact / Blast Radius** section define *what* to build and what's + in / out of scope. (No SRD? Build from the user's request.) Then decide which layers the change + actually touches: a new entity touches all of model → crud → service → route; a new endpoint + over existing data may be only service + route; a query-only change may be crud + route. Build + only what's needed. + +2. **Load conventions as you go.** Use the **`backend-conventions`** skill: Read + `.claude/conventions/cross-cutting.md` first, then Read each layer's doc *right before* you + write that layer (lazy-load — skip docs for layers the feature doesn't touch). + +3. **Walk the spine in dependency order: model → crud → service → route.** Each downstream layer + depends on the one above (route calls service, service calls crud, crud uses model), so never + build out of order. Build them all in this one pass — no per-layer handoff. + +4. **Migration.** If you added/changed/renamed a model field, write the Alembic migration + (`migration.md`). Rev-id = highest `NNN_*.py` in `backend/app/alembic/versions/` + 1, + zero-padded — recompute from the directory, don't guess. + +5. **Async / external SDKs.** Background work → write the Celery task (`celery.md`). Any code that + calls an external SDK or makes raw HTTP → apply the error-handling pattern + (`error-handling.md`). + +6. **Tests.** Write tests (`test.md`) and run the relevant subset + (`uv run pytest backend/app/tests/... -k -x`). Fix real failures before moving on. + +7. **Review before declaring done.** Run **`/pr-review`** on your diff (or apply its checklist) to + catch convention, security, and correctness issues before declaring the feature complete. + +## Rules + +- **Single-layer change?** Load just that one convention doc and write it — don't walk the whole + spine. +- **No model/schema change?** Skip the migration step. +- Cross-cutting rules (type hints, `[function_name]` logging + secret masking, `inserted_at` / + `updated_at`, no magic values, multi-tenant `organization_id` / `project_id`) apply at **every** + layer — see `cross-cutting.md`. +- **One summary at the end** (not one per layer): the layers built + key signatures (model + variants; crud/service/route function signatures + paths); any new `Permission` enum value, + domain exception, or `.env.example` / settings key the user must add; whether a migration is + needed and its rev-id; what the tests cover. diff --git a/.claude/skills/srd-creator/SKILL.md b/.claude/skills/srd-creator/SKILL.md index a1892b7c3..221a13c1d 100644 --- a/.claude/skills/srd-creator/SKILL.md +++ b/.claude/skills/srd-creator/SKILL.md @@ -1,6 +1,6 @@ --- name: srd-creator -description: Create a Software Requirements Document (SRD) for a Kaapi feature. Use when the user wants to write, draft, scaffold, or plan an SRD / spec / requirements doc for a new feature or capability (e.g. an evaluation pipeline, a new endpoint set, a provider integration). Produces a structured markdown SRD from the standard Kaapi template. +description: Create a Software Requirements Document (SRD) for a Kaapi feature. Use when the user wants to write, draft, scaffold, or plan an SRD / spec / requirements doc for a new feature or capability (e.g. an evaluation pipeline, a new endpoint set, a provider integration). Produces a structured markdown SRD — including blast-radius impact analysis against the domain map — from the standard Kaapi template. --- # SRD Creator @@ -28,15 +28,23 @@ the Functional Requirements table. It is not design prose; it is testable spec. endpoints, and the data model. If the user has not supplied these, ask — do not invent endpoints or schema. Missing info is the #1 cause of a useless SRD. -3. **Fill the template** section by section. Drop optional sections that don't +3. **Do blast-radius / impact analysis.** Read `docs/domain-map.md`. Name the primary product + surface(s) this feature changes (in domain-map vocabulary), then follow their `Consumed by` + edges **1-hop, then 2-hop** to build the impact set. Reconcile the map against live + `backend/app/{models,services,api/routes}` and note any drift. **For every impacted surface the + feature doesn't explicitly address, STOP and ask the user** whether it's *in scope* / + *deferred* / *out of scope* — don't silently include or exclude. The confirmed result fills the + SRD's **Impact / Blast Radius** section. + +4. **Fill the template** section by section. Drop optional sections that don't apply (e.g. Resources, Configuration) rather than leaving them empty. -4. **Write the Functional Requirements table** as the testable core. Every row = +5. **Write the Functional Requirements table** as the testable core. Every row = one user-facing behavior + a concrete acceptance criterion + a Status. If you can't write an acceptance criterion for a requirement, the requirement is too vague — sharpen it. -5. **Output** as `features//SRD.md`, where `` is a +6. **Output** as `features//SRD.md`, where `` is a short kebab-case slug for the feature (e.g. `features/llm-judge-correctness/SRD.md`). Create the `features//` directory if it doesn't exist. Every feature keeps its SRD and PRD together in this one folder — if a `PRD.md` already exists diff --git a/.claude/skills/srd-creator/reference/srd-template.md b/.claude/skills/srd-creator/reference/srd-template.md index 1a7b9b99e..63a0427f7 100644 --- a/.claude/skills/srd-creator/reference/srd-template.md +++ b/.claude/skills/srd-creator/reference/srd-template.md @@ -36,6 +36,19 @@ if known.> - **Pricing:** - **Starting provider/model:** +## Impact / Blast Radius + +Product surfaces this feature affects, traced from `docs/domain-map.md` (follow the primary +surface's `Consumed by` edges 1-hop, then 2-hop). Every impacted surface this feature touches but +does **not** change must carry a confirmed scope status — no TBDs. + +| Surface | Hop | Why affected | Status | Notes | +|---------|-----|--------------|--------|-------| +| | 1 | | in scope / deferred / out of scope | confirmed {date} | + +- For every domain-map surface **not** listed, state explicitly why it's unaffected. +- Record any **drift** found while reconciling `docs/domain-map.md` against live code (map said X, code does Y). + ## Detailed Design (Execution Flow) diff --git a/CLAUDE.md b/CLAUDE.md deleted file mode 100644 index e7cca6e72..000000000 --- a/CLAUDE.md +++ /dev/null @@ -1,144 +0,0 @@ -# CLAUDE.md - -This file provides guidance to Claude Code when working with code in this repository. - -## Project Overview - -Kaapi is an AI platform built with FastAPI and PostgreSQL, containerized with Docker. It provides AI capabilities including OpenAI assistants, fine-tuning, document processing, and collection management. - -## Key Commands - -### Development - -```bash -# Activate virtual environment -source .venv/bin/activate - -# Start development server with auto-reload -fastapi run --reload app/main.py - -# Run pre-commit hooks -uv run pre-commit run --all-files - -# Generate database migration. -# Compute at runtime as the latest existing revision ID + 1, -# zero-padded to 3 digits (check the highest NNN in app/alembic/versions/NNN_*.py). -alembic revision --autogenerate -m "Description" --rev-id - -# Seed database with test data -uv run python -m app.seed_data.seed_data -``` - -### Testing - -Tests use `.env.test` for environment-specific configuration. - -```bash -# Run test suite -uv run bash scripts/tests-start.sh -``` - -## Architecture - -### Backend Structure - -The backend follows a layered architecture located in `backend/app/`: - -- **Models** (`models/`): SQLModel entities representing database tables and domain objects - -- **CRUD** (`crud/`): Database access layer for all data operations - -- **Routes** (`api/`): FastAPI REST endpoints organized by domain - -- **Core** (`core/`): Core functionality and utilities - - Configuration and settings - - Database connection and session management - - Security (JWT, password hashing, API keys) - - Cloud storage (`cloud/storage.py`) - - Document transformation (`doctransform/`) - - Fine-tuning utilities (`finetune/`) - - Langfuse observability integration (`langfuse/`) - - Exception handlers and middleware - -- **Services** (`services/`): Business logic services - - Response service (`response/`): OpenAI Responses API integration, conversation management, and job execution - -- **Celery** (`celery/`): Asynchronous task processing with RabbitMQ and Redis - - Task definitions (`tasks/`) - - Celery app configuration with priority queues - - Beat scheduler and worker configuration - - -### Authentication & Security - -- JWT-based authentication -- API key support for programmatic access -- Organization and project-level permissions - -## Environment Configuration - -The application uses different environment files: -- `.env` - Application environment configuration (use `.env.example` as template) -- `.env.test` - Test environment configuration - - -## Testing Strategy - -- Tests located in `app/tests/` -- Factory pattern for test fixtures -- Automatic coverage reporting - -## Code Standards - -- Python 3.11+ with type hints -- Pre-commit hooks for linting and formatting - -## Coding Conventions - -Layer-specific conventions live in `.claude/agents/*.md` and are enforced by the matching specialist subagent (e.g., `route-writer` for `app/api/routes/`, `model-writer` for `app/models/`, `migration-writer` for alembic). CLAUDE.md only covers rules that apply across every layer. - -### Cross-cutting rules - -- **Type hints** on every parameter and return value. `-> Any` is not an annotation — narrow it or drop it. -- **Logging prefix:** every log line starts with the function name in square brackets. - ```python - logger.info(f"[function_name] Message | key: {value}") - ``` -- **`uv` is the runner**, not `pip`. Examples: `uv run pytest`, `uv run alembic ...`, `uv run pre-commit run --all-files`. -- **No magic values** in code — extract repeated literals to constants / `Enum` / settings. -- **Comments explain *why*, not *what*.** Don't restate what the code already says (`i += 1 # increment i`), don't narrate self-evident lines, and don't pad docstrings/migration descriptions with obvious recaps of the operations. A comment earns its place only by adding non-obvious context — rationale, a gotcha, a link, a constraint. When in doubt, delete it; clear code needs fewer comments, not more. -- **Naming:** `list_*` for plural fetch, `get_*` for singletons; snake_case funcs/vars, PascalCase classes, UPPER_SNAKE constants; `Enum` suffix on enum classes. -- **Timestamps** are `inserted_at` / `updated_at` (not `created_at`). - -## Specialist subagents - -When working in a specific layer, the matching agent under `.claude/agents/` handles the layer's conventions automatically. Pick by layer, or just describe the task and let the main agent route: - -| Agent | Layer | -|---|---| -| `feature-builder` | Full feature spanning `models` → `crud` → `services` → `api/routes` (the build spine, one context) | -| `route-writer` | `app/api/routes/` (single-layer edits) | -| `crud-writer` | `app/crud/` (single-layer edits) | -| `service-writer` | `app/services/` (single-layer edits) | -| `model-writer` | `app/models/` (single-layer edits) | -| `migration-writer` | `app/alembic/versions/` | -| `celery-task-writer` | `app/celery/tasks/` | -| `test-writer` | `app/tests/` | -| `convention-reviewer` | Cross-cutting pre-commit gate (mirrors `/pr-review`) | - -### Build a feature as a 4-context pipeline - -To keep each context window lean (heavy file I/O degrades performance), **build a multi-layer feature as four sequential subagent contexts, not inline.** Launch each phase with the Agent tool — each runs in its own context and returns only a summary, so the orchestrator stays small. The phases are a dependency chain, so run them **in order**, passing only the *artifacts* forward (signatures, file paths, the next migration rev-id), never re-deriving prior reasoning: - -| # | Context | Agent | Consumes | -|---|---|---|---| -| 1 | schema + code-spine | `feature-builder` | the feature request | -| 2 | migration | `migration-writer` | phase 1's model changes + next rev-id | -| 3 | test | `test-writer` | phase 1's signatures (+ which HTTP boundaries to mock) | -| 4 | review | `convention-reviewer` | the full diff | - -Rules of thumb: -- **Run them sequentially**, not in parallel — phase N depends on phase N-1's output. -- **Single-layer change?** Skip the pipeline; delegate to the one matching standalone agent (e.g. `crud-writer`) directly. -- **No model/schema change?** Skip phase 2 (`migration-writer`). -- `feature-builder` and the standalone layer agents share one source of truth — the convention docs in `.claude/conventions/{model,crud,service,route}.md` — so output never drifts between them. diff --git a/README.md b/README.md index 1f273438a..1cbbcc0c3 100644 --- a/README.md +++ b/README.md @@ -90,6 +90,12 @@ General development docs: [development.md](./development.md). This includes using Docker Compose, custom local domains, `.env` configurations, etc. +## AI-assisted development + +Features are designed and built with Claude Code using a skill-based workflow +(PRD → SRD → build → review), backed by per-layer code conventions and a +product domain map. See [.claude/README.md](./.claude/README.md) for how it works. + ## Release Notes Check the file [release-notes.md](./release-notes.md). diff --git a/docs/domain-map.md b/docs/domain-map.md new file mode 100644 index 000000000..c202af745 --- /dev/null +++ b/docs/domain-map.md @@ -0,0 +1,83 @@ +# Kaapi Domain Map + +**Snapshot: 2026-06-25.** A product-surface map for *blast-radius / impact analysis* — the +source of truth for "if I change surface X, what else is affected?". It maps **product +surfaces** (not individual tables), and the **consumes / consumed-by** edges between them. +Every edge cites its primary-source evidence (a foreign key, a cross-surface import, or an +architecture-doc line) so it can be re-verified fast. + +> **Reconcile before you trust it.** A hand-authored map drifts as code changes, and a stale +> map gives *false confidence* — worse than none. When you use this for an SRD's Blast Radius, +> re-derive the primary surface's `Consumed by` edges against live +> `backend/app/{models,crud,services,api/routes}` (grep the FK targets and cross-surface +> imports) and **flag any drift in the SRD**. Treat this file as a checklist to verify against +> code, not an oracle. + +## Surfaces + +Tiers: **T1** foundational (consumed by nearly everything), **T2** core business logic, +**T3** peripheral, **T4** cross-cutting substrate. + +| # | Surface | Backed by | Consumes | Consumed by | Tier | +|---|---|---|---|---|---| +| 1 | **Auth & Tenancy** | `organization`, `project`, `user`, `user_project`, `api_key`, `onboarding` | — | **everything** (every table is `organization_id`/`project_id` scoped) | T1 | +| 2 | **Config Store** *(shared)* | `models/config/`, `model_config` | Auth & Tenancy | LLM Call, Evaluations-Text, Assessment | T1 | +| 3 | **Credentials** | `credentials` (per-project provider creds) | Auth & Tenancy | LLM Call, Knowledge Base, Evaluations (all), Assessment, Response | T1 | +| 4 | **LLM Call** (`/llm/call`) | `services/llm/`, `job`, `llm_call`, `models/llm/` | Config Store, Credentials, Knowledge Base, Guardrails svc, Langfuse | Evaluations-Text (fast-evals), Assessment, Response | T2 | +| 5 | **Knowledge Base** | `document`, `collection`, `document_collection`, `collection_job`, `file`, `doctransform` | Auth & Tenancy, Credentials, S3 | LLM Call (`knowledge_base_ids` → file_search), Evaluations-Text | T2 | +| 6 | **Evaluations — Text** | `evaluation_dataset` (type=text), `evaluation_run`, `services/evaluations/` | Config Store, Knowledge Base, Credentials, Langfuse, Batch infra, LLM Call (fast-evals) | Notifications (completion email) | T2 | +| 7 | **Evaluations — STT** | `stt_sample`, `stt_result`, `services/stt_evaluations/` | Auth & Tenancy, Credentials (Gemini), Batch infra, File store | — | T2 | +| 8 | **Evaluations — TTS** | `tts_result`, `services/tts_evaluations/` | Auth & Tenancy, Credentials (Gemini), Batch infra, S3 | — | T2 | +| 9 | **Assessment** | `assessment`, `assessment_run`, `services/assessment/` | Config Store, **Evaluation Dataset** (`dataset_id` → `evaluation_dataset.id`), Credentials, LLM Call, Batch infra | — | T2 | +| 10 | **Fine-tuning + Model Evaluation** | `fine_tuning`, `model_evaluation` | Auth & Tenancy, Knowledge Base (`document_id` FK) | — | T2 | +| 11 | **Response / Assistants / Conversations** | `assistants`, `openai_conversation`, `response`, `threads`, `message`, `services/response/` | Job tracking, Assistants, OpenAI Conversation, Credentials | — (leaf) | T3 | +| 12 | **Analytics** | `analytics`, `services/analytics/` | LLM Call (`llm_call` audit), Job tracking, Config Store (pricing) | — (read-only) | T3 | +| 13 | **Notifications** | `notification`, `services/notifications/` | Auth & Tenancy, Evaluations-Text, User-Project | — | T3 | +| 14 | **Cross-cutting substrate** | `job`, `batch_job`, `collection_job`, `file`, `feature_flag`, `language`, `core/batch/`, Celery, S3, Langfuse | — | nearly all T2/T3 surfaces | T4 | + +## Consumed-by index (the blast-radius lookup) + +When a change touches a surface, these are the surfaces that depend on it — your **1-hop** +impact set. Traverse again from each of those for the **2-hop** set. + +- **Auth & Tenancy →** every surface. +- **Config Store →** LLM Call, Evaluations-Text, Assessment. *(High blast radius: the same + versioned `config_id + version` store backs production `/llm/call` AND eval runs — a schema + or resolution change here ripples into live traffic.)* +- **Credentials →** LLM Call, Knowledge Base, Evaluations (all), Assessment, Response. +- **LLM Call →** Evaluations-Text (fast-evals path), Assessment (prefilter stages), Response. +- **Knowledge Base →** LLM Call (file_search), Evaluations-Text (RAG eval configs). +- **Evaluation Dataset →** Evaluations-Text/STT/TTS **and** Assessment (shared dataset table, + distinguished by a `type` column: `text`/`stt`/`tts`/`assessment`). +- **Evaluations-Text →** Notifications. +- **Batch infra (`core/batch/`) →** Evaluations (all three), Assessment. +- **Job tracking →** LLM Call, Response. + +## Known coupling hotspots + +These are the non-obvious edges a spec most often forgets — check them explicitly: + +1. **Config Store is shared by production and evals.** Touching `config`/`model_config` + schema or `resolve_config_blob`/`resolve_evaluation_config` affects `/llm/call`, + Evaluations-Text, and Assessment at once. *(evidence: `services/llm/jobs.py` imports + `ConfigVersionCrud`; `evaluation_run.config_id` FK → `config.id`; + `services/assessment/service.py` imports `resolve_evaluation_config`.)* +2. **One dataset table, four families.** `evaluation_dataset` is shared by text/STT/TTS evals + and Assessment via a `type` discriminator. A dataset change touches all four. +3. **Knowledge Base feeds two consumers.** `knowledge_base_ids` is resolved into OpenAI + file_search by both `/llm/call` and text eval configs. +4. **Langfuse is the system of record for text evals** (datasets, traces, LLM-judge scores), + but NOT for STT/TTS (Postgres tables). Anything touching judge/scoring spans Langfuse + + the eval scoring/merge code. +5. **Notifications fire only from text evals today** (STT/TTS do not). A new eval family that + should notify must wire this explicitly. + +## Provenance & flags + +- Edges derived from: model `foreign_key=` declarations, cross-surface imports in + `services/`+`crud/`, and `docs/architecture/{kaapi-llm-call,kaapi-evaluations,kaapi-knowledge-base}-ARCHITECTURE.md`. +- **Merged:** `model_evaluation` belongs with Fine-tuning (its FK is `fine_tuning_id`, not + `evaluation_run_id`) — it is *not* part of the Evaluations surface. +- **Stale doc:** the evaluations arch doc says fast-evals are "not present on this branch," but + `services/evaluations/fast.py` exists and imports `services.llm.providers`. Code wins — + re-confirm during reconcile.