From 96ff2ec137cf1d6295cef773a86532c90c83ab52 Mon Sep 17 00:00:00 2001 From: Ali Zarinkolah Date: Mon, 10 Aug 2026 11:45:19 +0330 Subject: [PATCH] docs(api): define REST boundary and point management routes Why: - Establish the versioned FastAPI boundary for authentication, tenant isolation, chat runs, file ingestion, point management, and health checks. Changes: - Define /v1 routers, bearer-token authentication, scopes, response envelopes, error conventions, and job-shaped ingestion responses. - Rename the older indicative /chunks routes to /points while preserving the existing Qdrant payload and CRUD semantics. - Define tenant injection and concurrency requirements at the HTTP boundary. Impact: - The REST API is owned by ADR-0008 when older endpoint examples differ. - Clients should use /v1/points and /v1/threads resource paths. Co-Authored-By: Claude Fable 5 --- docs/adr/0002-chunk-crud-and-search-api.md | 35 +- .../adr/0008-rest-api-and-fastapi-boundary.md | 298 ++++++++++++++++++ 2 files changed, 318 insertions(+), 15 deletions(-) create mode 100644 docs/adr/0008-rest-api-and-fastapi-boundary.md diff --git a/docs/adr/0002-chunk-crud-and-search-api.md b/docs/adr/0002-chunk-crud-and-search-api.md index 2283362..1a1d549 100644 --- a/docs/adr/0002-chunk-crud-and-search-api.md +++ b/docs/adr/0002-chunk-crud-and-search-api.md @@ -4,6 +4,11 @@ Accepted +Route naming note: ADR-0008 owns the REST API surface and renames the +indicative endpoint examples in this ADR from `/chunks/...` to `/points/...`. +The CRUD semantics, Qdrant primitives, payload schema, soft-delete behavior, +and tenant-isolation rules in this ADR remain accepted. + ## Context Beyond bulk ingestion ([0001](0001-ingestion-pipeline-and-collection-schema.md)), @@ -24,16 +29,16 @@ the `chunks` collection and payload schema from ADR-0001: | Operation | FastAPI endpoint (indicative) | Qdrant primitive | |---|---|---| -| Create a chunk | `POST /chunks` | upsert (single point) | -| Update a chunk's vectors/content | `PUT /chunks/{chunk_id}` | upsert (`update_only` mode) | -| Partially update payload | `PATCH /chunks/{chunk_id}/payload` | `set_payload` / `overwrite_payload` | -| Delete one chunk | `DELETE /chunks/{chunk_id}` | delete by ID | -| Delete many chunks | `DELETE /chunks?file_id=...` | delete by filter | -| List/paginate chunks, in order | `GET /chunks?file_id=...` | `scroll` with filter + pagination, `order_by: order_id` | -| Reorder/insert a chunk | `PATCH /chunks/{chunk_id}/order` | `set_payload` on `order_id` only | -| Count chunks | `GET /chunks/count` | `count` | -| Keyword search | `GET /chunks/search?q=...` | full-text payload index on `content` (match, not semantic) | -| Bulk multi-op edits | `POST /chunks/batch` | Qdrant `points/batch` | +| Create a point | `POST /points` | upsert (single point) | +| Update a point's vectors/content | `PUT /points/{point_id}` | upsert (`update_only` mode) | +| Partially update payload | `PATCH /points/{point_id}/payload` | `set_payload` / `overwrite_payload` | +| Delete one point | `DELETE /points/{point_id}` | delete by ID | +| Delete many points | `DELETE /points?file_id=...` | delete by filter | +| List/paginate points, in order | `GET /points?file_id=...` | `scroll` with filter + pagination, `order_by: order_id` | +| Reorder/insert a point | `PATCH /points/{point_id}/order` | `set_payload` on `order_id` only | +| Count points | `GET /points/count` | `count` | +| Keyword search | `GET /points/search?q=...` | full-text payload index on `content` (match, not semantic) | +| Bulk multi-op edits | `POST /points/batch` | Qdrant `points/batch` | ### Ordering and reordering @@ -45,7 +50,7 @@ inserting a chunk only requires assigning it a new value between its two new neighbors — no renumbering of siblings, and no effect on the chunk's stable point ID (which is derived from the immutable `chunk_index`, not `order_id`). -`PATCH /chunks/{chunk_id}/order` does more than set one field, though: since +`PATCH /points/{point_id}/order` does more than set one field, though: since ADR-0001 also maintains `previous_chunk_id`/`next_chunk_id` pointers for O(1) adjacency lookups, a single reorder touches **up to four chunks** in one `points/batch` call: @@ -62,7 +67,7 @@ go stale. ### Keyword search is not semantic search -`GET /chunks/search` matches against the full-text payload index on +`GET /points/search` matches against the full-text payload index on `content` (and structured filters on `tenant_id`, `domain`, `file_id`, etc.) — it is filter/match-based keyword search, not embedding-based retrieval. This is deliberately distinct from the hybrid dense+sparse @@ -72,11 +77,11 @@ context) and should not be conflated in the API or in future discussion. ### Delete is soft by default -`DELETE /chunks/{chunk_id}` and `DELETE /chunks?file_id=...` set +`DELETE /points/{point_id}` and `DELETE /points?file_id=...` set `is_active: false` and `deleted_at` (via `set_payload`) rather than removing the point from Qdrant — consistent with ADR-0001's soft-delete fields. This -keeps deleted chunks available for audit and lets `GET /chunks` and -`GET /chunks/search` filter them out by default (`is_active: true` implied +keeps deleted points available for audit and lets `GET /points` and +`GET /points/search` filter them out by default (`is_active: true` implied unless the caller explicitly asks to include inactive chunks). A hard delete (actual point removal, e.g. `points/delete`) is available separately for compliance-driven purges, not as the default CRUD behavior. Either way, the diff --git a/docs/adr/0008-rest-api-and-fastapi-boundary.md b/docs/adr/0008-rest-api-and-fastapi-boundary.md new file mode 100644 index 0000000..7712e37 --- /dev/null +++ b/docs/adr/0008-rest-api-and-fastapi-boundary.md @@ -0,0 +1,298 @@ +# 0008. REST API and FastAPI boundary + +## Status + +Proposed + +## Context + +The project now has decisions for ingestion and Qdrant storage +([0001](0001-ingestion-pipeline-and-collection-schema.md)), point-level CRUD +and keyword search ([0002](0002-chunk-crud-and-search-api.md)), agent retrieval +([0003](0003-agent-hybrid-retrieval.md)), conversational graph behaviour +([0006](0006-conversational-agent-graph.md)), and LangGraph thread persistence +([0007](0007-agent-persistence-threads-and-memory.md)). What is still missing +is the HTTP boundary that ties these together. + +FastAPI has to do more than expose thin route handlers: + +- Authenticate the calling backend/API client with an API key. +- Resolve that API key to an active tenant in Postgres, using the tenant/API-key + tables defined by [ADR-0009](0009-postgres-sqlalchemy-alembic-schema.md). +- Inject `tenant_id` into every Qdrant operation and every LangGraph run, so + callers never submit or override the tenant boundary themselves. +- Receive user chat messages and pass them to the compiled LangGraph graph. +- Receive source files (starting with CSV, but aligned with the docx/xlsx/csv + ingestion decisions from ADR-0001/0004) and kick off ingestion. +- Expose direct management of Qdrant records. ADR-0002 originally sketched + `/chunks/...` endpoint names, but the API is directly managing Qdrant + **points**. The REST surface should use `/points/...` while the payload can + still contain chunk-specific fields such as `chunk_id`, `content`, and + `previous_chunk_id`. + +This ADR owns the public REST shape for this FastAPI service. If its endpoint +names differ from indicative endpoint examples in older ADRs, this ADR wins; +the older ADRs still own their data-model and pipeline semantics. + +## Decision + +### FastAPI application structure + +Use a single versioned API prefix, `/v1`, and split the app into routers by +resource: + +| Router | Prefix | Responsibility | +|---|---|---| +| `threads` | `/v1/threads` | Chat thread runs, transcript pagination, thread purge. | +| `files` | `/v1/files` | Upload source files and inspect ingestion status. | +| `points` | `/v1/points` | Direct CRUD, ordering, payload updates, keyword search, and batch edits over Qdrant points. | +| `users` | `/v1/users` | Cross-thread memory inspect/reset. | +| `health` | `/healthz`, `/readyz` | Process health and dependency readiness. | + +Routers use FastAPI's dependency injection with `Annotated[..., Depends(...)]`. +Shared dependencies live at the router level where they only gate access, and +as typed parameters where the endpoint needs the resolved value. Path +operation functions stay one HTTP operation per function. + +The app lifespan owns long-lived resources: + +- SQLAlchemy async engine/sessionmaker. +- Qdrant client. +- LangGraph checkpointer/store and the compiled graph (compiled once, reused). +- Any ingestion/retrieval model clients. + +No DDL runs at FastAPI startup. Postgres schema changes are Alembic migrations; +LangGraph checkpoint/store setup is a deployment step as decided in ADR-0007. + +### Authentication and tenant dependency + +Every `/v1` route except health checks requires an API key. The key is supplied +as a bearer token: + +```http +Authorization: Bearer +``` + +A FastAPI dependency validates the key against Postgres and returns an +`AuthContext`: + +```text +AuthContext( + tenant_id, + tenant_slug, + api_key_id, + scopes, + actor_type="backend" | "admin" | "worker", +) +``` + +Validation rules: + +1. API keys are stored only as hashes, never plaintext. +2. The presented key is split into a lookup prefix/key id plus secret material; + Postgres lookup uses the prefix/id, then compares the stored hash with a + constant-time comparison. +3. The key must be active: tenant active, key not revoked, key not expired. +4. The key must carry the scope required by the router/action. +5. `tenant_id` is derived only from the key's tenant relationship. + +The service never accepts `tenant_id` in request bodies or query strings. Any +client-supplied tenant field is rejected as an invalid request. This preserves +the ADR-0001/0002 Qdrant tenant-isolation rule and extends it to the REST +boundary. + +Recommended dependency aliases: + +```python +SessionDep = Annotated[AsyncSession, Depends(get_db_session)] +AuthContextDep = Annotated[AuthContext, Depends(get_auth_context)] +``` + +Scope dependencies compose on top of `AuthContextDep`, for example +`require_scope("points:write")`, `require_scope("threads:run")`, and +`require_scope("files:write")`. + +### Thread/run endpoints + +Chat uses the LangGraph vocabulary already chosen in ADR-0007. + +| Endpoint | Purpose | +|---|---| +| `POST /v1/threads/{thread_id}/runs` | Execute one graph run for a new user message. | +| `GET /v1/threads/{thread_id}/messages?limit=20&before=...` | Return a paginated transcript window for the frontend. | +| `DELETE /v1/threads/{thread_id}` | Purge the thread checkpoints for explicit clear/erasure. | +| `POST /v1/threads/{thread_id}/runs/{run_id}/feedback` | Attach feedback to the answer produced by a run. | + +`POST /runs` passes this context into LangGraph: + +- `thread_id` from the path. +- `tenant_id` from `AuthContext`, not from the caller body. +- `user_id` from the trusted main-backend request payload/header. +- locale/language from the request, with server-side fallback. + +The endpoint streams with Server-Sent Events when the caller asks for +streaming. Token events come from LangGraph `stream_mode="messages"`; progress +and terminal status events come from `stream_mode="custom"` or explicit route +wrapping. Terminal statuses are the ADR-0006 statuses: `answered`, +`clarifying`, or `escalate`. When `escalate` is returned, the main backend uses +its existing human-assistant transfer path and stops issuing runs for that +thread. + +One in-flight run per thread is allowed. The route takes a Postgres advisory +lock keyed on `thread_id`; a concurrent run returns `409 Conflict`. + +### File ingestion endpoints + +Files are source documents, not Qdrant points. Uploading a CSV creates or +re-ingests a file and then creates/updates many points via the ADR-0001/0004 +ingestion pipeline. + +| Endpoint | Purpose | +|---|---| +| `POST /v1/files` | Upload one `.csv`, `.xlsx`, `.docx`, or converted `.doc` source file for ingestion. Initial callers may only use CSV, but the route is named for the generalized source-file concept. | +| `GET /v1/files/{file_id}` | Return ingestion status and source-file metadata. | +| `GET /v1/files/{file_id}/points?limit=...&offset=...` | List active points belonging to a file, ordered by `order_id`. | +| `DELETE /v1/files/{file_id}` | Soft-delete all active points for a file, preserving audit data unless a future compliance hard-delete endpoint is added. | + +`POST /v1/files` uses FastAPI `UploadFile` and multipart form fields for +metadata such as `domain`. File validation is server-side: + +- Allowlist extensions/content types: `.csv`, `.xlsx`, `.docx`; legacy `.doc` + is accepted only if the conversion pipeline from ADR-0004 is enabled. +- Enforce maximum file size before reading the full body into memory. +- Sniff enough content to reject obvious extension spoofing. +- Derive `tenant_id`, `created_by`, and `updated_by` from `AuthContext`, not + from form fields. + +Ingestion may be slow because it parses, chunks, embeds, and writes many +Qdrant points. The REST contract is job-shaped even if the first +implementation runs inline: + +```text +202 Accepted -> { file_id, ingestion_job_id, status: "queued" | "running" } +``` + +A durable worker/job queue can be added later without changing the API +contract. + +### Point endpoints replace the older `/chunks` sketches + +The Qdrant collection is still named `chunks` in ADR-0001, because each stored +point represents one chunk. The REST resource is nevertheless `/points`, +because callers are managing Qdrant point records and the operations map +almost one-to-one to Qdrant's Points API. + +| Endpoint | Qdrant primitive | Notes | +|---|---|---| +| `POST /v1/points` | upsert one point | Manual/admin creation of one chunk-backed point. | +| `GET /v1/points/{point_id}` | retrieve by ID | Returns vectors only when explicitly requested. | +| `PUT /v1/points/{point_id}` | upsert with `update_only` | Replaces content/vectors/payload with optimistic concurrency. | +| `PATCH /v1/points/{point_id}/payload` | `set_payload` / `overwrite_payload` | Payload-only update. | +| `DELETE /v1/points/{point_id}` | payload update or delete | Soft-delete by default (`is_active=false`, `deleted_at`). | +| `GET /v1/points?file_id=...` | `scroll` | List/paginate active points in `order_id` order. | +| `PATCH /v1/points/{point_id}/order` | `points/batch` | Move/insert point and relink neighbors. | +| `GET /v1/points/count` | `count` | Count active points under tenant/domain filters. | +| `GET /v1/points/search?q=...` | full-text payload match | Keyword search over `content`, not semantic retrieval. | +| `POST /v1/points/batch` | `points/batch` | Bulk multi-operation edits. | + +Every operation injects a Qdrant filter containing: + +- `tenant_id == auth_context.tenant_id` +- `is_active == true` by default on reads +- optional `domain`, `file_id`, and other allowed filters supplied by caller + +`point_id` names the Qdrant point ID in the URL. Payload fields keep their +ADR-0001 names (`chunk_id`, `file_id`, `previous_chunk_id`, etc.) so the API +name change does not rename the storage schema. + +### Response and error conventions + +Use explicit return types or `response_model` to validate and filter public +responses. Do not return raw ORM objects, raw Qdrant responses, or full +LangGraph state. + +Error responses use a stable envelope: + +```json +{ + "error": { + "code": "tenant_not_found | invalid_api_key | missing_scope | validation_error | conflict | not_found | internal_error", + "message": "human-readable summary", + "details": {}, + "request_id": "..." + } +} +``` + +Important status codes: + +| Status | Use | +|---|---| +| `202 Accepted` | Ingestion accepted as a job. | +| `400 Bad Request` | Invalid domain/filter combinations or unsupported file type. | +| `401 Unauthorized` | Missing/invalid API key. | +| `403 Forbidden` | Valid key without required scope. | +| `404 Not Found` | Resource absent within this tenant. | +| `409 Conflict` | Concurrent thread run or optimistic-concurrency mismatch. | +| `422 Unprocessable Entity` | Pydantic request validation failure. | + +### Observability + +Every request gets a `request_id` and structured logs containing route, +status, latency, tenant id, and API key id — never the API key itself. LangGraph +runs include the same request id in `config` metadata so API logs, graph traces, +and Qdrant operations can be correlated. + +## Consequences + +### Positive +- Tenant isolation starts at the FastAPI dependency boundary, before any route + logic reaches Qdrant or LangGraph. +- `/points` aligns the REST API with what it actually manages — Qdrant point + records — while preserving the chunk payload schema underneath. +- `/threads/{thread_id}/runs` remains compatible with the LangGraph thread/run + model already chosen in ADR-0007. +- Job-shaped file ingestion lets the first implementation be simple while + keeping room for a durable worker without breaking clients. +- Router-level dependencies and typed FastAPI dependencies keep auth, tenant + resolution, sessions, and scopes reusable instead of repeated per endpoint. + +### Negative +- The API now has two words for related concepts: `files` are source documents, + `points` are the chunk records produced from them. This is accurate but + requires clear frontend/backend naming. +- `/points` exposes storage terminology to API consumers. That is acceptable + for this internal/admin-oriented service, but it would be less friendly as a + public product API. +- API-key auth in Postgres adds a database lookup to every request unless + short-lived caching is introduced. Caching must preserve revocation semantics. +- A job-shaped ingestion contract needs a job status store even if the initial + implementation processes inline. +- The REST layer now depends on the tenant/API-key, ingestion-job, audit, and + usage tables defined in [ADR-0009](0009-postgres-sqlalchemy-alembic-schema.md). + +## Alternatives Considered + +- **Keep `/chunks/...` from ADR-0002**: rejected. The storage record being + managed is a Qdrant point, and several operations (`points/batch`, retrieve + by point ID, vector inclusion flags) map directly to Qdrant's Points API. + Keeping `/chunks` would make the REST layer look more domain-friendly but + less honest about what it does. +- **Accept `tenant_id` in every request**: rejected. Tenant is an isolation + boundary, not user input. It must come from the API key's Postgres tenant + relationship. +- **Use `X-API-Key` instead of `Authorization: Bearer`**: rejected as the + default because bearer tokens are standard for service-to-service auth and + work cleanly with middleware, proxies, and generated clients. A compatibility + alias can be added later if an upstream system cannot send `Authorization`. +- **Expose one generic `/v1/qdrant/*` proxy**: rejected. It would leak Qdrant's + full API surface, bypass tenant/scoping rules too easily, and couple clients + to storage operations the service should hide. +- **Synchronous file ingestion only**: rejected as the contract. It is simpler + to implement, but embedding and late-interaction vector generation can be + slow enough to exceed HTTP timeouts. The job-shaped response gives the + implementation room to evolve. +- **Create threads with `POST /v1/threads`**: rejected for now. The main + backend owns conversation/session records, and LangGraph can create a + checkpoint sequence on the first run for a `thread_id`. A create endpoint + would imply metadata this service has explicitly chosen not to store.