From 645b7f3a6b13dfdc880a90351ebe6932d140c273 Mon Sep 17 00:00:00 2001 From: Jayme Klein Date: Mon, 20 Jul 2026 11:39:43 -0300 Subject: [PATCH 1/3] docs(claude): split CLAUDE.md into a lean index + path-scoped .claude/rules CLAUDE.md becomes a lean, always-loaded index (the change loop, the gate, required skills, and a rule-file table). The detailed working agreement moves into path-scoped .claude/rules/*.md so each area loads into context only when its code is edited, keeping the always-on context small. One rule file per area: code-standards, architecture, configuration, api, permissions, ingestion, mcp, database, vector-db, testing, and skills (the required review/verify skills). --- .claude/rules/api.md | 57 +++++++++++ .claude/rules/architecture.md | 42 +++++++++ .claude/rules/code-standards.md | 53 +++++++++++ .claude/rules/configuration.md | 49 ++++++++++ .claude/rules/database.md | 30 ++++++ .claude/rules/ingestion.md | 83 ++++++++++++++++ .claude/rules/mcp.md | 54 +++++++++++ .claude/rules/permissions.md | 60 ++++++++++++ .claude/rules/skills.md | 59 ++++++++++++ .claude/rules/testing.md | 29 ++++++ .claude/rules/vector-db.md | 35 +++++++ CLAUDE.md | 162 +++++++++++--------------------- 12 files changed, 607 insertions(+), 106 deletions(-) create mode 100644 .claude/rules/api.md create mode 100644 .claude/rules/architecture.md create mode 100644 .claude/rules/code-standards.md create mode 100644 .claude/rules/configuration.md create mode 100644 .claude/rules/database.md create mode 100644 .claude/rules/ingestion.md create mode 100644 .claude/rules/mcp.md create mode 100644 .claude/rules/permissions.md create mode 100644 .claude/rules/skills.md create mode 100644 .claude/rules/testing.md create mode 100644 .claude/rules/vector-db.md diff --git a/.claude/rules/api.md b/.claude/rules/api.md new file mode 100644 index 0000000..a4173f4 --- /dev/null +++ b/.claude/rules/api.md @@ -0,0 +1,57 @@ +--- +paths: + - "api/routers/**" + - "api/schemas/**" + - "api/main.py" + - "api/middleware.py" + - "api/dependencies.py" +--- + +# HTTP API + +FastAPI app built in `api/main.py` (`root_path="/api"` — nginx strips the prefix). Layering is strict and +one-way: **`router → service → (adapter | db)`** (see [`architecture.md`](architecture.md)). + +## Routers stay thin +`api/routers/*.py` do path registration + `Depends(...)` + an authorization check, then a **single +delegating call** to a service. No business logic, no raw SQL, no schema declarations. The routers: + +| Router | Base | Auth | Responsibility | +|--------|------|------|----------------| +| `health` | `/healthz`, `/metrics` | none | liveness snapshot | +| `workspaces` | `/workspaces` | router `require_master` | workspace CRUD | +| `collections` | `/workspaces/{ws}/collections` | router `require_master` | collection CRUD + API-key mgmt | +| `documents` | nested + flat `/documents…` | per-route `require_auth` + `can_access` | upload / list / status / delete / download / reprocess | +| `tags` | `/workspaces/{ws}` | router `require_master` | tag CRUD, merge, assignment | +| `graph` | `/workspaces/{ws}` | router `require_master` | tag-correlation graph | +| `search` | `POST /search` | per-route `require_master` | multi-collection hybrid search | +| `indexing` | `/indexing/…` | mixed master/auth | BM25 (re)index status + enqueue | +| `jobs` | `/ingestion/jobs…` | `require_master` | ingestion-queue list, stats, retry-failed | +| `config` | `/config` | router `require_master` | live config GET/PUT, ollama-models, reload-status | +| `ws` | `WS /ws` | `?key=` query param | Redis pub/sub → WebSocket (ingestion progress) | +| `mcp` | mounted `/mcp` | in middleware | MCP ASGI sub-app, mounted **last** ([`mcp.md`](mcp.md)) | + +## Adding an endpoint +1. **Route** → the matching `api/routers/.py`; handler resolves/authorizes then `return await .(...)`. +2. **Model** → CRUD request bodies go in `api/schemas/.py`; richer domain/response/query models and + config go in `api/models/.py`. (`schemas/` = per-endpoint DTOs; `models/` = shared domain/config + contracts reused across layers. `api/tables/` is persistence, not Pydantic.) +3. **Logic + DB** → new function in `api/services/.py` (`select(...)` on the injected `AsyncSession`; + raise `HTTPException` on domain errors — [`database.md`](database.md)). +4. **Register** in `api/main.py` (`include_router`); keep the MCP mount last. +5. **Inject deps** from `api/dependencies.py`: `db = Depends(get_db)`; auth `require_master` / `require_auth` + ([`permissions.md`](permissions.md)); adapters `Depends(require_embedding_adapter | require_vector_store)` + (503 if not ready) or `Depends(get_reranker)` (optional, may be `None`); config `Depends(get_search_config | get_app_config)`. + +## Errors, middleware, lifespan +- Services raise `fastapi.HTTPException(status, detail)` → `{"detail": …}`. Codes in use: 401 (bad/missing key), + 403 (wrong-collection / master-required), 404, 409 (conflict), 415 (unsupported file type), 422 (validation), + 503 (backend/queue not ready). No custom global handlers — FastAPI defaults + the request-id middleware. +- **Graceful degradation is a hard rule**: an optional stage that lacks a model / errors falls back and + **never 500s** a search or ingest. +- Middleware (`api/middleware.py`): `RequestIDMiddleware` (outermost) mints a `uuid4` request id, times the + request, sets `X-Request-ID`, emits one structured log line; CORS inside it. +- Lifespan (`api/main.py`): structlog → load `config.yaml` → `init_db()` (migrations) → **background** + `_warm_up_adapters` (embedding + vector-store + reranker load off the startup path, so uvicorn serves + immediately; getters return `None` and `/healthz` reports not-loaded until ready) → Redis → run the MCP + session manager for the app's lifetime. diff --git a/.claude/rules/architecture.md b/.claude/rules/architecture.md new file mode 100644 index 0000000..12364dc --- /dev/null +++ b/.claude/rules/architecture.md @@ -0,0 +1,42 @@ +--- +paths: + - "api/**" + - "worker/**" +--- + +# Architecture conventions — so "follow the pattern" is concrete + +The principles behind these are in [`code-standards.md`](code-standards.md). This file is the +"how, in this repo." + +## Adapters — the extension seam +`api/adapters/{embeddings,parsers,reranker,vector_store}` (also `tagging`, `llm_chat`): + +- A `runtime_checkable` **`Protocol` in `base.py`**, **one provider per file**, resolved by a + **`get_*(config)` factory** that dispatches on `config.provider` (or file type) with **lazy imports + inside each branch** and **raises `ValueError` on an unknown provider**. +- A disabled optional stage's factory returns **`None`** so callers skip it with `is not None` + (see `get_reranker`). +- **Adding a backend = a new file under `api/adapters//` + one branch in that kind's factory.** + Callers depend on the Protocol, never on a concrete class. Do not edit callers to add a backend. +- Keep the Protocols minimal (ISP). Optional capabilities stay *off* the Protocol — e.g. a parser's + `on_progress(current, total)` is detected by signature inspection in the worker, not declared on + `ParserAdapter`. + +## Graceful degradation — a hard rule +Any stage that lacks a model, times out, or errors **falls back to prior behaviour and never turns a +search / ingest into a 500.** Concretely: a missing reranker → RRF-only ranking; adjacency expansion is +best-effort; `/healthz` surfaces the degrade. **A new stage that can 500 the request is a bug.** + +## Layering — one direction only +`router → service → (adapter | db)`. Never the reverse; adapters/db never import services/routers. +- **Routers** (`api/routers/`) are *routing-only*: path + `Depends(...)`, an authorization check, then a + single delegating call to a service. No business logic, no raw SQL, no schema declarations there. +- **Services** (`api/services/`) own business logic + persistence; they raise `HTTPException` for domain errors. +- Details of the HTTP layer: [`api.md`](api.md). + +## Where the specifics live +- **Config** (pydantic models, secrets, hot-reload): [`configuration.md`](configuration.md). +- **Metadata DB** (ORM-only, tables, Alembic): [`database.md`](database.md). +- **Vector store** (the one raw-asyncpg exception): [`vector-db.md`](vector-db.md). +- **Tests / fakes / DI-for-testability**: [`testing.md`](testing.md). diff --git a/.claude/rules/code-standards.md b/.claude/rules/code-standards.md new file mode 100644 index 0000000..9951d03 --- /dev/null +++ b/.claude/rules/code-standards.md @@ -0,0 +1,53 @@ +--- +paths: + - "api/**" + - "worker/**" + - "tests/**" +--- + +# Code standards — DRY · KISS · YAGNI · SOLID + +The bar is **make it work, simply, without repeating yourself — and prove it.** These are the +principles; the concrete repo conventions that implement them live in +[`architecture.md`](architecture.md). + +## DRY — one fact, one place +- **Search for an existing implementation before writing a new one**, and reuse it. Duplication + discovered later is a defect, not a style nit. +- **Shared test doubles live in `tests/unit/fakes.py` and `tests/unit/fake_redis.py`** — import them + (`from tests.unit.fakes import FakeStore, FakeEmbedder`). **Never** re-declare `FakeStore` / + `FakeEmbedder` / `FakeRedis` inside a test module. A new reusable double goes in those modules, + not inline. (More in [`testing.md`](testing.md).) +- Shared domain logic belongs in a service/adapter/module — never copy-pasted across routers, tasks, or tests. +- A literal used in more than one place becomes a named constant (see `api/constants.py`). +- **Fixing a DRY violation means finding *every* copy**, not only the one that was pointed out. + +## KISS — the simplest thing that works +- Prefer a function to a class, a plain call to a framework, a straight line to a clever one. + Optimise for the next reader. +- No premature abstraction: the **second** concrete case justifies an interface, the first does not. +- If a change needs a paragraph to explain why it's clever, it's probably wrong. + +## YAGNI — build what's needed now +- No config knobs, parameters, hooks, or "for later" branches without a **current** caller. + The roadmaps in `plans/` and `docs/` are staged on purpose — don't pull a future phase forward. + +## SOLID — where it earns its keep +- **SRP** — one reason to change per unit. Parsing ≠ chunking ≠ embedding ≠ storing; keep them + separate, mirroring `api/services/` and `api/adapters/`. +- **OCP** — extend by adding, not by editing callers. A new embedding / reranker / parser backend is + **a new file under `api/adapters//` plus one branch in that kind's `get_*()` factory** — + callers depend on the factory, never on a concrete class. (Pattern in [`architecture.md`](architecture.md).) +- **LSP** — a new adapter honours its Protocol's full contract, including graceful degradation. +- **ISP** — keep the Protocols in `api/adapters/base.py` minimal. Optional capabilities stay *off* the + Protocol (see the `on_progress` note on `ParserAdapter`) so implementations that lack them still conform. +- **DIP** — depend on the `Protocol`, inject the dependency. `_run_ingestion` takes + `embedder` / `vector_store` / `storage` as parameters — that's *why* it is testable with the fakes. + Follow that shape. + +## Libraries — code against *current* docs +Before writing or debugging code against a library / framework / SDK / API / CLI, pull its **current** +docs via the **Context7 MCP** — don't code from memory, training data lags real releases. This repo pins +and verifies real versions (e.g. the `# verified against … via Context7` note atop +`api/adapters/vector_store/pgvector.py`); keep that habit for new adapters and upgrades. +Canonical rule: `CLAUDE.md` § Libraries & docs. diff --git a/.claude/rules/configuration.md b/.claude/rules/configuration.md new file mode 100644 index 0000000..890328a --- /dev/null +++ b/.claude/rules/configuration.md @@ -0,0 +1,49 @@ +--- +paths: + - "api/models/config.py" + - "api/services/config_service.py" + - "api/services/config_env.py" + - "api/services/config_reload.py" + - "worker/config.py" + - "worker/config_reload.py" + - "config.yaml" + - "config.example.yaml" + - ".env" + - ".env.example" +--- + +# Configuration + +Config is **pydantic models** — `AppConfig` and its sub-configs in `api/models/config.py` +(`embedding`, `vector_store`, `reranker`, `parsers`, `chunking`, `storage`, `mcp`, `search`, …). +`config.yaml` and `.env` are **gitignored — never commit secrets.** The read/apply service is +`api/services/config_service.py`; it backs the editable settings page. + +## Secrets — mask on GET, preserve on PUT +- Write-only secret fields are listed in **`SECRET_PATHS`** in `config_service.py` and masked with + **`SECRET_MASK` (`"__SECRET_SET__"`)** by `get_masked_config()`. Currently: `embedding.api_key`, + `vector_store.password`, `reranker.api_key`. +- **Adding a new secret field?** Add its path to `SECRET_PATHS` (or, for user-named backends, extend + `_storage_secret_paths()` — S3 `access_key_id` / `secret_access_key` are computed per-config because + backend names are dynamic). Otherwise a whole-config save from the UI round-trips it in the clear. +- On PUT, `_merge_secrets()` restores any field the client echoed back as `SECRET_MASK` from the live config. + +## The save flow — build-then-commit, then fan out +`apply_config()`: +1. **Merge secrets** → **validate + dry-run build the adapters** (`_validate_and_build`). Building is the + dry run: a bad embedding model / unreachable backend fails **here (422)** before anything is persisted. + A rate-limited dimension probe → **503** (retry), not a rejection; the reranker degrades to `None` if + its model can't load (an unknown *provider* still raises). +2. **Persist** `config.yaml` **in place** (not rename-swap — a rename splits Docker/WSL bind-mount views), + fsynced, with a `.bak` taken only when the current file is non-empty **and** parses (so a crash-truncated + file can't clobber the last-good backup — the boot recovery source in `config_env.load_config_data`). +3. **Hot-swap** the API's live singletons (`_swap_live`), then **propagate to workers** over Redis and wait + for acks; a worker rejection triggers **rollback** (restore `.bak`, revert adapters, republish) and **409**. + No Redis (single-process/dev) → an API-local status record. + +## Gotchas +- The config load path is `api/main.py::_load_app_config` with `.bak` crash recovery + env overlays + (`api/services/config_env.py`, e.g. `S3____*`). Startup sets `app.state.config` + `set_app_config`. +- Worker-side reload lives in `worker/config_reload.py` / `worker/tasks.py::reload_adapters` (rebuilds the + worker's embedder/vector-store singletons on a new version). +- Adapters are built from config via the `get_*()` factories — see [`architecture.md`](architecture.md). diff --git a/.claude/rules/database.md b/.claude/rules/database.md new file mode 100644 index 0000000..9e0a525 --- /dev/null +++ b/.claude/rules/database.md @@ -0,0 +1,30 @@ +--- +paths: + - "api/db.py" + - "api/tables/**" + - "api/alembic/**" + - "worker/db.py" +--- + +# Metadata DB — ORM only + +The metadata DB (**workspaces, collections, documents, tags, api_keys, job_records**) goes through the +**SQLAlchemy 2.0 async ORM**. The `chunks` table is **not** here — it's the vector store's, raw asyncpg, +see [`vector-db.md`](vector-db.md). + +## The rules +- **Engine + session factory live in `api/db.py`** (`engine`, `AsyncSessionLocal`, `init_db`). Table objects + live in `api/tables/` and are re-exported from `api.db`, so `from api.db import documents` stays valid. +- **Build queries with `select(...)` / `insert` / `update` / `delete` and the table objects on the injected + `AsyncSession`.** **Do not hand-write raw SQL against the metadata DB.** +- **Every schema change is an Alembic migration** in `api/alembic/versions/`. `init_db()` runs + `alembic upgrade head` at startup (safe on every boot) from a thread executor. +- Services own the queries and commit explicitly; routers stay thin (see [`api.md`](api.md)). The request + gets its `AsyncSession` via `Depends(get_db)` — unhandled exceptions auto-rollback. + +## Good to know +- URL comes from `DATABASE_URL` (e.g. `postgresql+psycopg://…`); the dev fallback is + `sqlite+aiosqlite:///…`. SQLite-only pragmas (WAL, `foreign_keys=ON`) are set on the sync connection at + connect time — don't apply them to Postgres. +- Hierarchy is `workspaces → collections → (documents, api_keys, job_records)` with FK `CASCADE`. +- `expire_on_commit=False`, `autoflush=False` — flush/commit intentionally explicit. diff --git a/.claude/rules/ingestion.md b/.claude/rules/ingestion.md new file mode 100644 index 0000000..05cdbed --- /dev/null +++ b/.claude/rules/ingestion.md @@ -0,0 +1,83 @@ +--- +paths: + - "worker/tasks.py" + - "worker/celery_app.py" + - "api/services/ingestion.py" + - "api/services/indexing.py" + - "api/services/upload.py" + - "api/services/storage.py" + - "api/services/documents.py" + - "api/services/jobs.py" + - "api/services/tasks.py" + - "api/services/realtime.py" + - "api/adapters/parsers/**" + - "api/routers/documents.py" + - "api/routers/indexing.py" + - "api/routers/jobs.py" +--- + +# Ingestion pipeline + +Flow: **upload → parse → chunk → embed → store**. The DI seam is `worker/tasks.py::_run_ingestion`. + +## Sync (API) vs async (worker) +- **API** (`api/routers/documents.py::upload_document`): reject unsupported ext (415), stream to storage with + a size guard (`api/services/upload.py`, `.tmp` + atomic `os.replace`), insert `documents` + `job_records`, + enqueue. The **API never imports the worker package** — `api/services/tasks.py::enqueue_ingest` dispatches by + task-name string over Redis, keeping heavy parser/embedding deps out of the API image. +- **Worker** (`worker/tasks.py::ingest_document → _run_ingestion`): fetch bytes to a local temp + (`storage.fetch_to_temp`), **parse** (`get_parser(...)`), **chunk** (`api/services/ingestion.py::sliding_window`, + tiktoken `cl100k_base`, called inside the txt/markdown parsers), **embed + store** in resumable batches + (`_embed_and_store` → `embedder.embed_batch` → `vector_store.upsert`), mark done. +- **BM25 is not a stage** — it's the stored `chunks.text_tsv` maintained by the upsert ([`vector-db.md`](vector-db.md)). + +## Jobs, status, progress +- **`job_records` table.** Status: `pending → processing → done | failed | rate_limited`. `_claim_job` atomically + flips pending→processing; a **Redis heartbeat** `ingest:hb:{job_id}` (TTL 600s) is refreshed on each progress event. +- **Progress**: the `emit()` closure publishes via `api/services/realtime.py` to two pub/sub topics + (`ingestion:{col}` for the Documents view, `ingestion-queue` for the Queue tab) with `snapshot_key=document_id` + so late joiners get replayed the latest per-document state. Phases: `parsing/embedding/storing/done/failed/rate_limited`. + PDF page callbacks are coalesced to ~1 frame/percent. +- **Queue read side** (master key): `api/routers/jobs.py` — `GET /ingestion/jobs` (paginated/filtered), + `GET /ingestion/jobs/stats`. + +## DI shape (why it's testable) +`_run_ingestion(job_id, file_path, collection_id, document_id, file_type, *, session_factory=None, +embedder=None, vector_store=None, redis_client=None, config=None, storage=None)`. Each `None` resolves to a +lazy prod singleton; unit tests pass fakes ([`testing.md`](testing.md)). Keep new stages inject-everything. + +## Retries / failure / resume +- `ingest_document` is `bind=True, max_retries=3, retry_backoff=True`. Except ladder: `SoftTimeLimitExceeded` + first (fail, no retry) → **rate limit** (`_is_rate_limit`: typed `RateLimitError` / httpx 429 / narrow phrase — + deliberately not bare "429"/"quota") → other (fail + `self.retry`). +- **Rate limit is not a failure**: `_pause_embedding(delay)` sets a **global** TTL key `EMBEDDING_PAUSE_KEY`, + job → `rate_limited`, returns None (acks). One 429 pauses **all** ingestion so N queued docs don't each + re-hit the exhausted quota (circuit breaker); `/ingestion/jobs/stats` surfaces remaining backoff. +- **Resume-aware, never restart from chunk 0**: `_embed_and_store` skips chunks already in + `vector_store.document_chunk_ids_at_model(col, doc, model)`; per-batch upsert persists immediately. + Deterministic `make_chunk_id` makes re-parse yield the same ids; a model change re-embeds only stale chunks. +- **Beat sweeps** (`worker/celery_app.py`, embedded `-B`, every 300s): `retry_rate_limited_ingests` requeues + `rate_limited` + stale-heartbeat `processing` + orphaned `pending` via `_requeue_by_status` — re-enqueued as + **fresh** Celery tasks (uncapped lineage), **same `job_id`** (so `_claim_job` dedups), bounded batch. Gated + off while paused. `purge_expired_documents` cleans temp docs. +- **Retry all failed**: `POST /ingestion/jobs/retry-failed` → `api/services/jobs.py::reprocess_failed_documents` + re-enqueues every **currently**-failed doc (latest attempt failed) matching the queue's active filters, each + via `documents.py::reprocess_document` (idempotent). **Single retry**: `POST /documents/{id}/reprocess`. +- The ingestion-queue **refetch-storm fix is frontend-only** (`ui/src/realtime/useIngestionQueue.ts` debounces + invalidation) — no backend change; don't look for it in `api/`/`worker/`. + +## Changing a stage +Stage → file: parse = `api/adapters/parsers/.py` (+ registry in `parsers/__init__.py`); chunk = +`api/services/ingestion.py::sliding_window`; embed = `api/adapters/embeddings/.py`; store + BM25 = +`api/adapters/vector_store/pgvector.py`; storage backend = `api/services/storage.py`. All resolve through +`get_*()` factories — add a backend = new file + one factory branch ([`architecture.md`](architecture.md)). +Knobs live in `api/models/config.py` ([`configuration.md`](configuration.md)). + +## Hard rules / gotchas +- **Graceful degradation**: progress/status reporting is best-effort and wrapped — it must never mask the + original exception or 500 an ingest. Heartbeat/Redis reads **fail open** (`_job_alive` assumes alive on error, + so a blip can't cause double-processing). Misclassifying a 429 as a hard failure loses the infinite-resume path. +- **Idempotency**: `_claim_job` skips `done` and live `processing`; requeue/reprocess keep the same `job_id`; + delete is idempotent. **Storage key is single-source-of-truth** — import `document_key`, never re-encode it. +- RPM throttle: module-level `_RpmLimiter` (single-process — valid because worker `--concurrency=1`). +- Effective tags (workspace→collection→document union) are folded into each chunk's metadata at embed time. diff --git a/.claude/rules/mcp.md b/.claude/rules/mcp.md new file mode 100644 index 0000000..c10f414 --- /dev/null +++ b/.claude/rules/mcp.md @@ -0,0 +1,54 @@ +--- +paths: + - "api/services/mcp/**" + - "api/routers/mcp.py" +--- + +# MCP server (embedded) + +EmbedBase embeds a **FastMCP** server inside the FastAPI app so an external agent (Claude Code/Desktop, +Cursor, Zed) can drive the store. It exposes **five tools**, no resources/prompts: +`list_workspaces`, `search_documents`, `ingest_document` (container-local file path), `list_documents`, +`delete_document`. + +## Key files +- `api/services/mcp/server.py` — builds the `FastMCP` server, **declares the tools**, resolves runtime deps, + builds + mounts the ASGI app. The central file. +- `api/services/mcp/tools.py` — framework-agnostic tool **implementations**; thin wrappers over existing services. +- `api/services/mcp/middleware.py` — raw ASGI middleware: API-key auth + rate limit on `/mcp`. +- `api/services/mcp/rate_limit.py` — per-key `TokenBucketRateLimiter`. +- `api/routers/mcp.py` — routing-only shim; `mount_mcp()` delegates to the service. +- `MCPConfig` (`enabled`, `rate_limit_rpm`, `max_results`) in `api/models/config.py`; mounted last in `api/main.py`. + +## Adding / changing a tool — two layers, always both +1. **Implement** in `tools.py` as an `async def`, keyword-only, that **delegates to an existing service** and + returns a JSON-serialisable `dict`. It receives deps (`db`, `embedder`, `vector_store`, `reranker`, + `principal`) as kwargs — it never resolves them itself and never opens a DB session. +2. **Declare** in `server.py::_register_tools()` with `@server.tool()`. The wrapper's **signature = the tool's + input schema** and its **docstring = the description the LLM reads** — both are load-bearing. The wrapper + resolves singletons via `_require(get_embedding_adapter(), …)` / `get_vector_store()` / `get_reranker()` + from `api/dependencies.py`, opens `async with AsyncSessionLocal() as db:`, then calls the `tools.*` function. + +Don't put business logic in the wrapper or the router; don't resolve deps in `tools.py`. + +## Transport & mounting (already plumbed — don't re-plumb) +- `FastMCP("embedbase", stateless_http=True, json_response=True)`, `streamable_http_path="/"` — **stateless + streamable HTTP** (verified against `mcp==1.28.1`). Chosen because SSE died on `uvicorn --reload`; **do not + reintroduce SSE / stateful sessions.** +- `mount_app()` owns the enablement decision (`if not config.enabled: return`), mounts at `/mcp`, stashes the + server on `app.state.mcp_server`. `api/main.py` runs `mcp_server.session_manager.run()` for the app's + lifetime (`nullcontext` when disabled), and mounts MCP **last** so `/mcp` never shadows REST routes. +- External URL is `/api/mcp/` (app `root_path="/api"` + sub-app at `/mcp`); the trailing slash avoids a 307. + +## Auth & degradation +- Every request must carry the **master key** (`Authorization: Bearer` or `X-API-Key`), constant-time compared + in `middleware.py`; missing/wrong → 401. There is **no per-collection scoping** — tools run as + `MASTER_PRINCIPAL` ([`permissions.md`](permissions.md)). Over-limit → 429. +- `_require()` raises `RuntimeError(" backend not ready")` if the embedding/vector-store singleton hasn't + warmed up yet; the reranker is optional (`None` just skips rerank). `search_documents` clamps `top_k` to + `[1, mcp.max_results]` and emits a `more_available` + `notice` saturation hint when relevant chunks exceed the cut. + +## Delegates to (point here, don't duplicate) +`workspaces.list_workspace_tree`, `search.multi_collection_search`, `documents.{ingest_local_path,list_documents, +delete_document,resolve_document_collection}`, `auth.Principal`. (Distinct from the standalone REST reference at +`/api/reference` — don't conflate them.) diff --git a/.claude/rules/permissions.md b/.claude/rules/permissions.md new file mode 100644 index 0000000..ba2c8e1 --- /dev/null +++ b/.claude/rules/permissions.md @@ -0,0 +1,60 @@ +--- +paths: + - "api/services/auth.py" + - "api/tables/api_keys.py" + - "api/services/collections.py" + - "api/services/mcp/middleware.py" + - "api/services/mcp/rate_limit.py" + - "api/routers/collections.py" + - "api/routers/ws.py" +--- + +# Permissions — API-key auth + +**API keys only — no sessions/cookies/JWT.** The core is `api/services/auth.py`. Authorization is binary: +**master vs single-collection** (no roles/RBAC/scopes). + +## Two credential types +- **Master key** — the one required secret `MASTER_API_KEY` (`api/settings.py`; app won't start without it). + Grants access to every workspace/collection. Matched with `secrets.compare_digest` (constant-time). +- **Collection key** — an `eb_`-prefixed token scoped to exactly one collection. Stored in the `api_keys` + table as `key_prefix` (chars `raw[3:11]`, indexed) + **bcrypt** `key_hash`. Auth = narrow by prefix, then + `bcrypt.checkpw` the full key. The raw key and plaintext are **never** stored. +- Header: `X-API-Key` (preferred) or `Authorization: Bearer `. WebSocket can't set headers → `?key=` + query param (`api/routers/ws.py`). Auth resolves to a frozen `Principal(is_master, collection_id, api_key_id)`. + +## Protecting a route +Two FastAPI deps in `auth.py`: +- `require_master` → `Principal`, 403 unless `is_master`. +- `require_auth` → `Principal` (master **or** collection key); **does not** enforce collection scope. + +Patterns: +- **Admin/management planes** → router-level gate: `APIRouter(..., dependencies=[Depends(require_master)])` + (collections, workspaces, tags, config, graph). This exists so a later-added endpoint can't be accidentally + unauthenticated. +- **Collection-scoped route** → the canonical shape: + ```python + principal: Principal = Depends(require_auth) + await doc_svc.resolve_collection(db, col_id, ws_id) # 404 if not in ws + if not principal.can_access(col_id): + raise HTTPException(403, "API key not valid for this collection") + ``` + Forgetting the `can_access` check after `require_auth` leaves a collection key able to touch other + collections — the scope check is your responsibility per route. +- Note: **search is master-only**; collection keys manage their own collection's documents but can't run search. + +## Key lifecycle (`api/services/collections.py`) +- **Mint**: `raw = "eb_" + secrets.token_urlsafe(32)`; store `key_prefix` + `bcrypt.hashpw(raw, gensalt(rounds=12))`. + The raw key is returned **once** and never retrievable again. +- **List/mask**: metadata only — the query never selects `key_hash`; clients see only `key_prefix`. +- **Revoke**: hard-deletes the row (immediate; no soft-delete/disabled flag). + +## Hard rules / gotchas +- **Never return or log `key_hash` or a raw key.** Hashing is fixed: bcrypt, `rounds=12`; prefix is only a + candidate filter — never authenticate on prefix alone. Auth errors are coarse (401 vs 403) — don't leak which part failed. +- **MCP** runs as master: its transport is gated by `MCPAuthRateLimitMiddleware` (master key only + per-key + token-bucket rate limit → 429), and tools use `MASTER_PRINCIPAL` ([`mcp.md`](mcp.md)). +- **Known gap**: `record_key_use` (updates `last_used_at`) is currently only called in tests — no router/dep + invokes it, so `last_used_at` is effectively never updated in the running app. Don't assume usage tracking works. +- Config secrets are masked by a separate mechanism ([`configuration.md`](configuration.md)) — same + "never expose secrets" rule, different code path. Request-id logging is in `api/middleware.py` (not auth). diff --git a/.claude/rules/skills.md b/.claude/rules/skills.md new file mode 100644 index 0000000..fc60d79 --- /dev/null +++ b/.claude/rules/skills.md @@ -0,0 +1,59 @@ +--- +paths: + - "api/**" + - "worker/**" + - "tests/**" +--- + +# Required skills — how a change earns "done" + +The gate proves the code is *mechanically* sound; these skills prove it's *actually* done — that it +follows the working agreement and doesn't ship a bug. Run them at the end of every implementation, +before you report the task complete or offer to commit. `/standards-check` is this repo's own skill +(`.claude/skills/standards-check/`); the rest are built in. + +## The order (and why) +1. **The gate** — `ruff` + `mypy` + `pytest` green (commands in `CLAUDE.md`). Prerequisite: don't run the + review skills over code that doesn't lint / type / pass. The toolchain lives in the WSL virtualenv, so + from a Windows host wrap commands with `wsl.exe -e bash -lc '…'`. +2. **`/standards-check`** — conventions. Cheap and structural; catches DRY / layering / secret-masking / + ORM issues that would otherwise waste a review pass. Fixing them can move code, so do it *before* the + deep review. +3. **`/code-review`** — correctness. The final bug hunt, on now-convention-clean code. It's the last thing + between you and "done". + +**Fix-and-re-run:** after non-trivial fixes from either skill, run it again — a fix can introduce a new issue. + +## `/standards-check` (this repo's skill) +- **What** — audits the current diff against `CLAUDE.md` + the path-scoped `.claude/rules/*.md`, emitting a + pass / warn / fail checklist with `file:line` and concrete fixes. Report-first: it won't edit unless asked. +- **When** — after the gate is green, before `/code-review`; or any time you want to confirm a change + follows the project's standards. +- **How** — `/standards-check` audits the work in progress. Pass a scope if needed: a path, or a range like + `main...HEAD`. Fix every ❌ (fix or justify each ⚠️), then re-run until the verdict is clean. + +## `/code-review` (built-in) +- **What** — reviews the diff for correctness bugs plus reuse / simplification / efficiency cleanups. +- **When** — last, before calling the work finished or offering to commit. **Mandatory** — don't report a + task complete without it. +- **How** — `/code-review` (default effort). Raise depth with `high` / `max`, or `ultra` for a deep + multi-agent cloud review (user-triggered, billed — you can't launch it yourself). `--fix` applies findings + to the working tree; `--comment` posts them on a PR. Address every finding; re-review if fixes are non-trivial. + +## Situational — run when they apply +- **`/security-review`** — a security pass over the pending branch. Run whenever a change touches auth / + permissions (`api/services/auth.py`, API keys), secrets / `SECRET_PATHS`, raw SQL (the pgvector adapter), + file upload or other external input, or the MCP surface. +- **`/verify`** — exercises a change end-to-end to confirm it does what it should (not just that tests pass). + Run for a nontrivial behavioural change — a new/changed endpoint, pipeline stage, or search behaviour — + before committing. +- **`/simplify`** — quality-only cleanup (reuse / simplification / efficiency), applied. Use when you want + the cleanup without the bug hunt; `/code-review` already covers this ground, so it's optional on top. + +## Rules of engagement +- **Address every finding** — fix it, or state explicitly why it's a non-issue. An unaddressed ❌ means the + change isn't done; a skill's output is not advisory noise. +- **Don't stop at the first pass** — re-run after non-trivial fixes. +- **Don't report complete or offer to commit** until `/standards-check` and `/code-review` are clean. +- These skills *verify*; they don't excuse skipping step 1 of the loop (understand + reuse first). A clean + review of duplicated code is still a DRY violation — see [`code-standards.md`](code-standards.md). diff --git a/.claude/rules/testing.md b/.claude/rules/testing.md new file mode 100644 index 0000000..3f62e04 --- /dev/null +++ b/.claude/rules/testing.md @@ -0,0 +1,29 @@ +--- +paths: + - "tests/**" +--- + +# Tests + +Tests live in `tests/{unit,integration,smoke}`. **Every behavioural change ships with a test.** +`tests/` is linted and type-checked to the same bar as shipping code (the gate in `CLAUDE.md`). + +## Unit tests touch nothing external +No network, no DB, no Redis. Use the **shared fakes** — never spin up a real model or pgvector: +- `tests/unit/fakes.py` — `FakeEmbedder` (deterministic 3-dim vectors) and `FakeStore` (in-memory + vector store that records `upserts` and answers `document_chunk_ids_at_model`, the resume set). +- `tests/unit/fake_redis.py` — `FakeRedis`. + +## The DRY rule for doubles (do not violate) +- **Import** the shared doubles: `from tests.unit.fakes import FakeStore, FakeEmbedder`. +- **Never re-declare** `FakeStore` / `FakeEmbedder` / `FakeRedis` inside a test module. +- A **new reusable** double goes in `fakes.py` / `fake_redis.py`, not inline — one contract, one place, + so every test sees the same behaviour and a change is made once. + +## Why the pipeline is testable +`_run_ingestion` (and `_embed_and_store`) take `embedder` / `vector_store` / `storage` / `redis_client` +as injected params — that's the DIP shape ([`code-standards.md`](code-standards.md)) that lets the whole +ingestion flow run against the fakes with no infrastructure. When you add a stage, keep its deps injected +so a unit test can substitute a fake. Pipeline details: [`ingestion.md`](ingestion.md). + +Run: `.venv/bin/python -m pytest tests/unit tests/integration -q`. diff --git a/.claude/rules/vector-db.md b/.claude/rules/vector-db.md new file mode 100644 index 0000000..5d62bcc --- /dev/null +++ b/.claude/rules/vector-db.md @@ -0,0 +1,35 @@ +--- +paths: + - "api/adapters/vector_store/**" +--- + +# Vector store — the one sanctioned raw-asyncpg path + +`api/adapters/vector_store/pgvector.py`. The **`chunks` table has no ORM model by design** and is accessed +with **raw, parameterised asyncpg** — the single documented exception to the ORM-only rule +([`database.md`](database.md)). Everything else about the adapter pattern still applies +([`architecture.md`](architecture.md)): it's resolved by `get_vector_store(config)`. + +## Why raw asyncpg (don't "fix" this to ORM) +- It needs operators the ORM can't express: **pgvector `<=>`** (cosine distance) and **ParadeDB `pg_search`** + (`|||` match-any-term, `pdb.score(id)` = Okapi BM25). +- asyncpg is async-only but the `VectorStoreAdapter` Protocol is **sync** and called from both the worker + (no loop) and the API (inside a loop). Each adapter owns a **background thread + persistent event loop** + (`_AsyncRunner`); sync methods submit coroutines to it. asyncpg pools are **loop-bound**, so the pool + binds to that one loop — a request's `AsyncSession` can't share it. + +## Conventions for new `chunks` queries +- **Always parameterised** (`$1, $2, …`) — never interpolate values. The `{filter}` placeholder is the + only composed fragment (a metadata AND-chain from `_metadata_filter_sql`, pushed into the WHERE for + pre-filter pushdown); build it the same way, with bound params. +- **Simple queries inline; complex ones as module-level `_*_SQL` constants** (`_UPSERT_SQL`, `_SEARCH_SQL`, + `_BM25_SQL`, `_HYBRID_SQL`, `_LIST_SQL`). Follow that sibling pattern. +- **Hybrid search is one round-trip**: `_HYBRID_SQL` fuses semantic + BM25 via **Reciprocal Rank Fusion in + SQL** (CTE + `RANK()` + `UNION ALL`), weighted by `alpha`. Don't move fusion back into Python. It surfaces + `from_bm25` (so the caller can degrade to semantic-only) and `vscore` (real cosine, kept for that case). +- **BM25 / lexical is not a pipeline stage** — it's the stored `chunks.text_tsv` / `pg_search` index, + maintained by the upsert. `index_document` / `index_collection` are no-ops kept for API compatibility. +- Each collection gets a **lazily-built partial HNSW index** (`WHERE collection_id = …`) once it crosses + `index_min_rows`, built with `CREATE INDEX CONCURRENTLY` so it never locks the table. +- Chunk ids are deterministic (`make_chunk_id`), so upsert (`ON CONFLICT (id)`) is re-runnable — the basis + for ingestion resume ([`ingestion.md`](ingestion.md)). diff --git a/CLAUDE.md b/CLAUDE.md index b94b17e..bd8167d 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -3,110 +3,31 @@ Binding for every change, Claude and humans alike. The bar is not "make it work." The bar is **make it work, simply, without repeating yourself — and prove it.** -This file exists because standards get skipped when they're implicit. They are not -implicit here. +This file is the **index**. The detail lives in [`.claude/rules/`](.claude/rules/) — each +file there carries a `paths:` front-matter glob, so it loads into context **automatically +when you edit the code it governs**, and stays out of the way otherwise. Read the matching +rule file *before* you touch an area; don't rediscover conventions that are already written down. --- ## The loop — every change, no exceptions -1. **Understand before writing.** Search the repo for an existing pattern, helper, - constant, config field, adapter, or test double that already does what you need, - and reuse or extend it. About to add a `class`/`def`/constant? `grep` its name - and its siblings **first** — if a sibling exists, follow it; if a copy exists, - you're creating duplication, stop. -2. **Implement to the standards below** — DRY · KISS · YAGNI · SOLID. -3. **Pass the gate** — ruff + mypy + pytest all green (commands at the bottom). - A change that fails the gate is not done. -4. **Run `/code-review`.** **Mandatory at the end of every implementation**, before - you call the work finished or offer to commit. Address every finding; re-review - if the fixes are non-trivial. Do not report a task complete without it. +1. **Understand before writing.** Search the repo for an existing pattern, helper, constant, + config field, adapter, or test double that already does what you need, and reuse or extend it. + About to add a `class`/`def`/constant? `grep` its name and its siblings **first** — a sibling + means follow it; a copy means stop, you're creating duplication. +2. **Implement to the standards** — DRY · KISS · YAGNI · SOLID + (see [`code-standards.md`](.claude/rules/code-standards.md)). +3. **Pass the gate** — ruff + mypy + pytest all green (below). A change that fails the gate is not done. +4. **Run the required skills** — `/standards-check` (conventions), then `/code-review` (bugs) — at the + end of every implementation, before you call the work finished or offer to commit. Address every + finding; re-run if the fixes are non-trivial. Don't report a task complete without them. + Details & order: [Required skills](#required-skills) → [`skills.md`](.claude/rules/skills.md). Skipping step 1 or step 4 is the exact failure mode this document prevents. --- -## Code standards - -### DRY — one fact, one place -- **Search for an existing implementation before writing a new one**, and reuse it. - Duplication discovered later is a defect, not a style nit. -- **Shared test doubles live in [`tests/unit/fakes.py`](tests/unit/fakes.py) and - [`tests/unit/fake_redis.py`](tests/unit/fake_redis.py)** — import them - (`from tests.unit.fakes import FakeStore, FakeEmbedder`). **Never** re-declare - `FakeStore` / `FakeEmbedder` / `FakeRedis` inside a test module. A new reusable - double goes in those modules, not inline. -- Shared domain logic belongs in a service/adapter/module — never copy-pasted across - routers, tasks, or tests. -- A literal used in more than one place becomes a named constant (see - [`api/constants.py`](api/constants.py)). -- **Fixing a DRY violation means finding *every* copy**, not only the one that was - pointed out. - -### KISS — the simplest thing that works -- Prefer a function to a class, a plain call to a framework, a straight line to a - clever one. Optimise for the next reader. -- No premature abstraction: the **second** concrete case justifies an interface, the - first does not. -- If a change needs a paragraph to explain why it's clever, it's probably wrong. - -### YAGNI — build what's needed now -- No config knobs, parameters, hooks, or "for later" branches without a **current** - caller. The roadmaps in [`plans/`](plans/) and [`docs/`](docs/) are staged on - purpose — don't pull a future phase forward. - -### SOLID — where it earns its keep -- **SRP** — one reason to change per unit. Parsing ≠ chunking ≠ embedding ≠ storing; - keep them separate, mirroring `api/services/` and `api/adapters/`. -- **OCP** — extend by adding, not by editing callers. A new embedding / reranker / - parser backend is **a new file under `api/adapters//` plus one branch in - that kind's `get_*()` factory** — callers depend on the factory, never on a - concrete class. -- **LSP** — a new adapter honours its Protocol's full contract, including graceful - degradation (below). -- **ISP** — keep the Protocols in [`api/adapters/base.py`](api/adapters/base.py) - minimal. Optional capabilities stay *off* the Protocol (see the `on_progress` note - on `ParserAdapter`) so implementations that lack them still conform. -- **DIP** — depend on the `Protocol`, inject the dependency. `_run_ingestion` takes - `embedder` / `vector_store` / `storage` as parameters — that's *why* it is testable - with the fakes above. Follow that shape. - ---- - -## Architecture conventions (so "follow the pattern" is concrete) - -- **Adapters** (`api/adapters/{embeddings,parsers,reranker,vector_store}`): - a `runtime_checkable` `Protocol` in `base.py`, one provider per file, resolved by a - `get_*(config)` factory that dispatches on `config.provider` with **lazy imports - inside each branch** and **raises `ValueError` on an unknown provider**. A disabled - optional stage returns `None` so callers skip it with `is not None` - (see [`get_reranker`](api/adapters/reranker/__init__.py)). -- **Graceful degradation is a hard rule.** Any stage that lacks a model, times out, - or errors **falls back to prior behaviour and never turns a search/ingest into a - 500.** A new stage that can 500 the request is a bug. -- **Config** is pydantic models in [`api/models/config.py`](api/models/config.py). - `config.yaml` and `.env` are **gitignored — never commit secrets.** A new secret - field must be added to `SECRET_PATHS` in - [`api/services/config_service.py`](api/services/config_service.py) so it is masked - on GET and preserved on PUT. -- **Database access — ORM only for the metadata DB.** The metadata DB (workspaces, - collections, documents, tags, api_keys, job_records) goes through the **SQLAlchemy - 2.0 async ORM**: engine + session in [`api/db.py`](api/db.py), table objects in - [`api/tables/`](api/tables/), every schema change an Alembic migration in - `api/alembic/versions/`. **Do not hand-write raw SQL against the metadata DB** — - build queries with `select(...)` and the table objects on the injected - `AsyncSession`. The **one** sanctioned exception is the pgvector vector store - ([`api/adapters/vector_store/pgvector.py`](api/adapters/vector_store/pgvector.py)): - the `chunks` table has no ORM model and is accessed with raw, **parameterised** - asyncpg, because it needs pgvector (`<=>`) and ParadeDB `pg_search` (`|||`, - `pdb.score`) operators the ORM can't express, on a loop-bound pool the request - `AsyncSession` can't share. New `chunks` queries follow that adapter's raw-asyncpg - sibling pattern (simple ones inline, complex ones as module-level `_*_SQL` constants). -- **Tests** live in `tests/{unit,integration,smoke}`. Unit tests touch no network, - DB, or Redis — use the shared fakes. **Every behavioural change ships with a test.** - ---- - ## The gate — run before every "done" The toolchain lives in the WSL virtualenv. From the repo root: @@ -117,24 +38,53 @@ The toolchain lives in the WSL virtualenv. From the repo root: .venv/bin/python -m pytest tests/unit tests/integration -q ``` -All three green, **then `/code-review`.** This mirrors CI -([`.github/workflows/ci.yml`](.github/workflows/ci.yml)): ruff rules `E,F,I,UP,B,SIM` -(line-length 100, target py312), mypy on `api/ worker/`, the unit and integration -suites, and the docker build smoke test. Ruff now lints `tests/` too — test code is -held to the same standard as shipping code. +All three green, **then the required skills** (see [Required skills](#required-skills)). This mirrors CI ([`.github/workflows/ci.yml`](.github/workflows/ci.yml)): +ruff `E,F,I,UP,B,SIM` (line-length 100, py312), mypy on `api/ worker/`, the unit + integration +suites, and the docker-build smoke test. `tests/` is linted too — test code meets the same bar. + +--- + +## Required skills + +At the end of every implementation — once the gate is green — run these and address every finding +before calling the work done or offering to commit. Full when / how / order in +[`skills.md`](.claude/rules/skills.md): + +- **`/standards-check`** — audits the diff against this working agreement (`CLAUDE.md` + `.claude/rules/`). + Run it first; fix every ❌ and re-run until clean. +- **`/code-review`** — hunts correctness bugs and reuse / simplification issues. Run it last; address + findings, re-review if the fixes are non-trivial. + +Situational: **`/security-review`** when touching auth, secrets, SQL, or external input; **`/verify`** +when the change has runtime behaviour worth exercising end-to-end. + +--- + +## Rule index — read the file for the area you're touching + +| Area | Rule file | Read it when you're working on… | +|------|-----------|--------------------------------| +| Code standards | [`code-standards.md`](.claude/rules/code-standards.md) | any code — DRY/KISS/YAGNI/SOLID, reuse-first, the shared test doubles | +| Architecture | [`architecture.md`](.claude/rules/architecture.md) | adapters, the `get_*()` factory pattern, graceful degradation, router→service→adapter layering | +| Configuration | [`configuration.md`](.claude/rules/configuration.md) | `AppConfig` pydantic models, secrets & masking (`SECRET_PATHS`), the config save / hot-reload / rollback flow | +| API | [`api.md`](.claude/rules/api.md) | adding an endpoint, routers, middleware, lifespan, `schemas/` vs `models/`, error/status conventions | +| Permissions | [`permissions.md`](.claude/rules/permissions.md) | API-key auth, `require_auth` / `require_master`, collection scoping, minting & revoking keys | +| Ingestion | [`ingestion.md`](.claude/rules/ingestion.md) | upload→parse→chunk→embed→store, Celery jobs, retries / rate-limit pause, retry-all-failed, progress events | +| MCP | [`mcp.md`](.claude/rules/mcp.md) | the embedded MCP server, adding or changing an MCP tool, its transport & auth | +| Metadata DB | [`database.md`](.claude/rules/database.md) | the SQLAlchemy ORM, `api/tables/`, Alembic migrations — the metadata DB | +| Vector DB | [`vector-db.md`](.claude/rules/vector-db.md) | the `chunks` table, raw asyncpg, pgvector / ParadeDB operators, hybrid-search SQL | +| Tests | [`testing.md`](.claude/rules/testing.md) | test layout, the shared fakes, what a unit test may touch | --- ## Commits (only when explicitly asked) -- **Conventional Commits**: `type(scope): summary` (`feat`, `fix`, `refactor`, - `docs`, `test`, `chore`, …). Same format for PR titles. -- **No AI attribution** — commit under the user's identity. No `Co-Authored-By: - Claude`, no "Generated with" trailer. +- **Conventional Commits**: `type(scope): summary` (`feat`, `fix`, `refactor`, `docs`, `test`, `chore`, …). + Same format for PR titles. +- **No AI attribution** — commit under the user's identity. No `Co-Authored-By: Claude`, no "Generated with" trailer. - Don't commit or push unless asked; branch off `main` first when needed. ## Libraries & docs -Use the **Context7 MCP** for any library / framework / SDK / API / CLI question -(syntax, config, migrations, debugging) *before* answering from memory — training -data lags real releases. +Use the **Context7 MCP** for any library / framework / SDK / API / CLI question (syntax, config, +migrations, debugging) *before* answering from memory — training data lags real releases. From a1c02cfd2a7945d635eba15d883efa5e92bfc4e8 Mon Sep 17 00:00:00 2001 From: Jayme Klein Date: Mon, 20 Jul 2026 11:39:43 -0300 Subject: [PATCH 2/3] feat(claude): add standards-check skill to audit changes against the working agreement The /standards-check skill maps each changed file to the rule files that govern it (via their paths: globs), reads CLAUDE.md + those rules live, and reports a per-standard pass/warn/fail checklist with file:line references and fixes. Report-first; complements /code-review (bugs) and the ruff/mypy/pytest gate (mechanical). --- .claude/skills/standards-check/SKILL.md | 127 ++++++++++++++++++++++++ 1 file changed, 127 insertions(+) create mode 100644 .claude/skills/standards-check/SKILL.md diff --git a/.claude/skills/standards-check/SKILL.md b/.claude/skills/standards-check/SKILL.md new file mode 100644 index 0000000..48c4806 --- /dev/null +++ b/.claude/skills/standards-check/SKILL.md @@ -0,0 +1,127 @@ +--- +name: standards-check +description: >- + Audit the current change set against this repo's own documented standards — CLAUDE.md (the change + loop, the gate, commits) and the path-scoped rule files in .claude/rules/*.md — and report a + per-standard pass/warn/fail checklist with file:line references and concrete fixes. Use this + whenever the user wants to check, verify, or confirm that changes follow the project's standards, + conventions, rules, or working agreement (phrasings like "do these follow our standards", "check + this against the rules", "is this up to standard", "standards check", "conventions review"), and as + the recommended compliance pass after implementing a change and before committing or opening a PR. + It complements /code-review (which hunts correctness bugs) and the ruff/mypy/pytest gate (mechanical + checks) by verifying adherence to the written project conventions specifically. It maps each changed + file to the rule files that govern it and reads them live, so it never goes stale. +--- + +# Standards Check + +Verify that the current changes actually honour **this repo's own written standards**, and report +where they don't with enough precision to fix it. The standards are not in this skill — they live in +`CLAUDE.md` and the path-scoped rule files under `.claude/rules/`. **Read them at runtime and check +against them.** Copying the rules in here would rot the moment someone edits a rule file; reading them +live keeps this check correct by construction (and honours the project's own DRY rule). + +This is the *conventions* pass. It does **not** replace: +- **`/code-review`** — finds correctness bugs and generic reuse/simplification issues. +- **the gate** — `ruff` + `mypy` + `pytest`, the mechanical checks. + +Run those too; this one answers a different question: *does the diff follow the documented working agreement?* + +## Process + +### 1. Scope the change set +Default to the work in progress — staged + unstaged + untracked. If the working tree is clean, fall +back to the branch's diff vs `main`. The user may narrow it (a path, or a ref/range like `main...HEAD`). + +### 2. Map changed files → the standards that govern them +For each changed file (from step 1), read every `.claude/rules/*.md` whose `paths:` front-matter glob +matches its path — those rule files are the standards in scope. `CLAUDE.md`, `code-standards.md`, and +`architecture.md` **always** apply (they're universal). Claude Code already auto-attaches a rule file +when you open a file its glob matches, so reading the changed files pulls most of the relevant rules +into context for free — but read the matching rule files explicitly too, so nothing is missed. + +### 3. Read from the rules, not from memory +Include `CLAUDE.md` alongside the rule files from step 2 — together they're your checklist. Audit from +what they actually say: the rules are specific (e.g. exact `SECRET_PATHS`, the ORM-vs-asyncpg split, the +adapter-factory shape) and that specificity is the whole point. + +### 4. Audit the diff against each applicable rule +Go hunk by hunk. Judge the change against the concrete criteria in the rule files, plus the universal +loop from `CLAUDE.md`. Cover at least these categories (each cites where its criteria live): + +- **Reuse-first / DRY** (`code-standards.md`) — did the change reuse an existing helper/constant/service, + or duplicate one? `grep` the new symbol and its siblings to be sure. A **re-declared shared test double** + (`FakeStore`/`FakeEmbedder`/`FakeRedis`) or a copied literal/logic is a fail. Find *every* copy. +- **KISS / YAGNI** (`code-standards.md`) — premature abstraction, unused params, config knobs or branches + with no current caller, cleverness that needs a paragraph to justify. +- **Architecture / SOLID** (`architecture.md`) — a new backend must be **a new file under + `api/adapters//` + one branch in that kind's `get_*()` factory**, with callers depending on the + Protocol; **graceful degradation** preserved (no new path that can 500 a search/ingest); layering kept + (routers thin, logic in services, no raw SQL on the metadata DB). +- **Area rules** — whichever applied in step 2: config secrets added to `SECRET_PATHS` + (`configuration.md`); metadata DB via ORM/`select()` only, schema change ⇒ Alembic migration + (`database.md`); new `chunks` queries raw **and parameterised** in the pgvector adapter (`vector-db.md`); + `require_auth` routes still enforce `can_access`, secrets never logged (`permissions.md`); ingestion stays + resume-safe / injects its deps / degrades gracefully (`ingestion.md`); MCP tools keep the two-layer split + (`mcp.md`); endpoints stay thin with schemas vs models placed correctly (`api.md`). +- **Tests** (`CLAUDE.md`, `testing.md`) — every behavioural change ships a test; unit tests touch no + network/DB/Redis and reuse the shared fakes. +- **Libraries** (`CLAUDE.md`) — new or upgraded library usage verified against current docs via Context7 / + pinned, not coded from memory. +- **Commits** (`CLAUDE.md`) — only if a commit/PR is in scope: Conventional Commits format, **no AI + attribution** (no `Co-Authored-By: Claude` / "Generated with" trailer). + +For every finding, give: **`file:line`**, the **rule** (name the rule file and quote the clause), +**why** the change violates it, and the **concrete fix**. Vague findings help no one — be specific enough +that someone could act on it without re-reading the diff. + +### 5. Check the gate +Report whether `ruff` / `mypy` / `pytest` were run and are green — "a change that fails the gate is not +done." If their status is unknown, offer to run them: + +```bash +.venv/bin/ruff check api/ worker/ tests/ +.venv/bin/mypy api/ worker/ --ignore-missing-imports --explicit-package-bases +.venv/bin/python -m pytest tests/unit tests/integration -q +``` + +### 6. Report +Use the template below. Rank findings worst-first. End with an ordered must-fix list and an offer to fix. + +## Report format +Use this exact shape so results read consistently: + +``` +## Standards Check — @ · file(s) +**Scope:** · **Audited:** a.py, b.py, … + +### Verdict: ✅ Compliant | ⚠️ Compliant with warnings | ❌ Not compliant + +### Findings +❌ ** — ** · `path/to/file.py:42` + Rule (.md): "" + → + +⚠️ ** — ** · `path/to/file.py:88` + Rule (.md): + → + +✅ **** — (only list the notable passes) + +### Gate +ruff <✅|⚠️ not run> · mypy <…> · pytest <…> + +### Must-fix (in order) +1. +Then re-run this check. +``` + +## Rules of engagement +- **Report-first.** Do not edit code as part of the check. After presenting findings, offer to fix; only + touch code if the user says so, then re-run the check to confirm. +- **Earn a clean bill.** A ✅ must mean you read the applicable rules and the diff and found nothing — never + a rubber stamp. If you didn't read a rule file, you can't pass its category; say so. +- **Docs-only or config-only changes are fine** — say the code categories are N/A and check only what + applies (e.g. commit format, no committed secrets, docs consistency). +- **Large diff?** You may fan out a subagent per rule area for independence, then merge — but the explicit + read-the-rules checklist above is what guarantees coverage, not the parallelism. From cffbf88899b7b23ec692c676ff6bc0800840647e Mon Sep 17 00:00:00 2001 From: Jayme Klein Date: Mon, 20 Jul 2026 11:39:43 -0300 Subject: [PATCH 3/3] chore(git): ignore local Claude settings, keep shared .claude committed Ignore .claude/settings.local.json in-repo (it was only excluded via a machine-global gitignore, so a fresh clone could leak it), while CLAUDE.md and .claude/{rules,skills}/ stay committed and travel with the project. --- .gitignore | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/.gitignore b/.gitignore index 37f22f6..5d1c715 100644 --- a/.gitignore +++ b/.gitignore @@ -32,9 +32,10 @@ ui/dist/ .DS_Store Thumbs.db -# AI assistant local session data. CLAUDE.md is intentionally committed — it is the -# repo's working agreement / coding standard, enforced for everyone (see the file). -.claude/* +# AI assistant data. CLAUDE.md and .claude/{rules,skills}/ are intentionally committed — +# they are the repo's working agreement, standards, and shared skills, enforced for everyone +# and carried with the project wherever it's cloned. Only personal/local settings stay out. .vscode/launch.json .vscode/settings.json .playwright-mcp/ +.claude/settings.local.json