Files
chatbot_v3/docs/plans/002-point-crud-and-keyword-search.md

286 lines
14 KiB
Markdown
Raw Blame History

This file contains ambiguous Unicode characters
This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.
# 002. Point CRUD and keyword-search implementation plan
## Purpose
This plan covers the milestone named at the end of
[plan 001](001-ingestion-vertical-slice.md): direct, fine-grained management of
individual Qdrant points through `/v1/points`, plus filter/keyword search over
them. Ingestion (plan 001) writes points in bulk; this slice lets a human or
admin frontend read, edit, reorder, and soft-delete them one at a time, under
the same tenant isolation.
This is an implementation plan, not an Architecture Decision Record. The ADRs
explain why the collection schema, endpoints, and isolation rules are what they
are; this document defines order, scope, and verification criteria.
## Prerequisite
Plan 001 must be complete through **Phase 5** before Phase 3 of this plan
starts. Specifically this plan depends on: the `chunks` collection and its
payload indexes actually existing, API-key authentication and `AuthContext`
tenant derivation, the application-lifetime Qdrant client from the FastAPI
lifespan, and the request-lifetime `AsyncSession` wiring. Phases 1–2 below
(schemas and the read paths) can be built against the collection alone and do
not need the full ingestion path.
## Architecture baseline
| System | Responsibility |
|---|---|
| FastAPI | HTTP boundary, auth, tenant derivation, request/response schemas, scope checks. |
| Points application service | Point CRUD semantics: soft delete, ordering, neighbor relinking, optimistic concurrency. |
| Qdrant | The only store for point content, vectors, and payload. |
| Postgres | Audit of mutating operations. It does **not** hold a mirror of point state. |
The controlling ADRs are:
- [ADR-0001](../adr/0001-ingestion-pipeline-and-collection-schema.md): the
`chunks` collection, payload schema, payload indexes, deterministic point IDs,
`order_id` as a fractional float, and the
`previous_chunk_id`/`next_chunk_id` adjacency pointers.
- [ADR-0002](../adr/0002-chunk-crud-and-search-api.md): the CRUD operation set,
soft-delete-by-default, reorder semantics, keyword search vs. semantic search,
and `version`-based optimistic concurrency.
- [ADR-0008](../adr/0008-rest-api-and-fastapi-boundary.md): the `/v1/points`
REST surface, `GET /v1/files/{file_id}/points`,
`DELETE /v1/files/{file_id}`, and the `points:read`/`points:write` scopes.
- [ADR-0012](../adr/0012-application-resource-lifetime-and-dependency-ownership.md):
the Qdrant client is application-lifetime and injected, never constructed per
request.
- [ADR-0015](../adr/0015-modular-monolith-package-architecture.md): routes call
`application/points/`, which calls a port implemented in
`infrastructure/qdrant/`. Routers never build Qdrant filters or call the SDK.
- [ADR-0016](../adr/0016-testing-strategy-and-quality-gates.md): unit tests
against a fake point port; Testcontainers Qdrant for the adapter.
ADR-0001 through 0004 are Accepted; 0008, 0012, 0015, and 0016 are still
Proposed. Treat the proposed ones as the implementation baseline only once the
project owner accepts them, and update the ADR rather than diverging silently.
## Scope
### In scope
- `POST /v1/points`, `GET /v1/points/{point_id}`, `PUT /v1/points/{point_id}`,
`PATCH /v1/points/{point_id}/payload`, `DELETE /v1/points/{point_id}`.
- `GET /v1/points?file_id=...` (scroll, `order_by: order_id`, paginated),
`GET /v1/points/count`, `GET /v1/points/search?q=...`.
- `PATCH /v1/points/{point_id}/order` with correct neighbor relinking.
- `POST /v1/points/batch` for bulk multi-operation edits.
- `GET /v1/files/{file_id}/points` and `DELETE /v1/files/{file_id}`
(bulk soft delete of a file's points), from ADR-0008.
- Soft delete as the default for every delete path, with neighbor relinking.
- Optimistic concurrency on every mutating path via the `version` payload field.
- Audit rows in Postgres for mutating operations.
- Automated tests for tenant isolation, pointer integrity, concurrency
conflicts, and pagination.
### Explicitly out of scope
- Hybrid dense+sparse retrieval, RRF fusion, and late-interaction rerank
(ADR-0003/0005) — that is the next plan, 003. `GET /v1/points/search` here is
**keyword/filter matching only**; do not let it grow a semantic mode.
- Re-embedding a point on content edit (see the open decision below).
- Hard deletion / compliance purge endpoints.
- The LangGraph conversational API (ADR-0006/0007).
- A frontend.
## Required invariants
1. `tenant_id` comes from the authenticated context and is injected into every
filter server-side — on reads *and* writes, on every code path. A
`tenant_id`, or any payload key that would override it, appearing in a
request body or query string is rejected, never honored.
2. Cross-tenant access returns `404`, not `403` — the same rule plan 001 applies
to files. A caller must not be able to probe for the existence of another
tenant's point IDs.
3. Soft delete is the default: `is_active=false` + `deleted_at` via
`set_payload`. Points are never removed from Qdrant by any endpoint in this
slice.
4. Reads exclude inactive points unless the caller explicitly opts in.
5. Any operation that changes a point's position or removes it from the sequence
— insert, reorder, delete — updates the affected neighbors'
`previous_chunk_id`/`next_chunk_id` in the **same** `points/batch` request.
A partial relink is a defect: ADR-0003's context-window expansion walks these
pointers.
6. Point IDs stay derived from `file_id` + the immutable `chunk_index`.
Reordering changes `order_id` only, never the point ID.
7. Every mutating operation is guarded by the `version` payload field via
Qdrant's `update_filter`, and increments it. A stale write returns `409`,
it does not silently clobber.
8. `order_id` is a fractional float. Inserting or moving a point assigns a value
between its two new neighbors; it never renumbers siblings.
9. Routers contain no Qdrant SDK calls and no filter construction. The Qdrant
client is injected from the lifespan (ADR-0012).
## Decisions needed before the affected phase
### Re-embedding on content edit (blocks Phase 4)
`PUT /v1/points/{point_id}` can change `content`. The stored vectors then no
longer match the text. Three options, in order of preference:
1. **Re-embed inline** on content change, reusing plan 001's embedding ports and
bounds. Consistent, but puts embedder latency and `502`/`504` failure modes
on an admin edit path.
2. **Require caller-supplied vectors** when content changes, and reject the edit
otherwise. Simple and honest, but pushes model knowledge to the client.
3. **Mark the point stale** (a payload flag) and re-embed later. Needs
background work, which ADR-0017 currently rules out.
Default to (1) for parity with ingestion, with the same batch/semaphore bounds
and the same status codes. Record whichever is chosen in ADR-0002 before
implementing Phase 4 — this is a real behavioral contract, not an
implementation detail.
### Fractional-key exhaustion
ADR-0001 notes float keys eventually need renormalization. Decide now whether
this slice ships a renormalize path (an internal operation rewriting a file's
`order_id` values to `1000, 2000, 3000, ...`) or explicitly defers it with a
logged warning when the gap between neighbors falls under a threshold. Deferring
is acceptable; silently producing unrepresentable gaps is not.
### Batch semantics
`POST /v1/points/batch` must define, in the API schema and the tests: whether
operations are all-or-nothing, what happens when operation 3 of 5 fails a
version check, and the maximum operation count per request. Decide before
Phase 5; do not let the answer be "whatever Qdrant happened to do."
## Build order
### Phase 1: Point contracts and the port
1. Define the payload model in `src/application/points/` mirroring ADR-0001's
field list exactly, with the reserved/server-owned fields (`tenant_id`,
`version`, `chunk_index`, `deleted_at`) separated from caller-writable ones.
2. Define the point port: get, upsert, set payload, scroll, count, keyword
search, batch. Tenant filter is a required parameter on every method — not an
optional argument a caller can forget.
3. Implement the Qdrant adapter behind the port in
`src/infrastructure/qdrant/`, using the injected application-lifetime client.
4. Add a fake port implementation for unit tests, with ordering and version
behavior faithful enough to test the service against.
5. Define the Pydantic request/response schemas in `src/api/schemas/`. Vectors
are returned only when explicitly requested (ADR-0008).
**Exit criteria:** the service layer can be unit-tested end to end against the
fake; a Testcontainers Qdrant test confirms the adapter's filter construction
and `order_by` scroll match the fake's semantics.
### Phase 2: Read paths
1. `GET /v1/points/{point_id}` — tenant-filtered retrieve; `404` when the point
belongs to another tenant or does not exist.
2. `GET /v1/points?file_id=...` — scroll with `order_by: order_id`, stable
pagination, `is_active: true` implied.
3. `GET /v1/files/{file_id}/points` — same listing, addressed by file.
4. `GET /v1/points/count` — tenant/domain-filtered count.
5. `GET /v1/points/search?q=...` — full-text payload match on `content` plus
structured filters. Name the response and docstring so it cannot be mistaken
for semantic retrieval.
6. Enforce `points:read` scope on all of the above.
7. Tests: tenant isolation returns `404`; inactive points are excluded by
default and included on explicit opt-in; pagination does not skip or repeat
under a concurrent insert.
**Exit criteria:** a tenant can list and search only its own active points, in
display order, with correct paging.
### Phase 3: Soft delete and neighbor relinking
1. Implement the relinking primitive in the service: given a point leaving the
sequence, compute the neighbor payload updates and emit them with the
deactivation in one `points/batch` call.
2. `DELETE /v1/points/{point_id}` — soft delete plus relink.
3. `DELETE /v1/files/{file_id}` — bulk soft delete of a file's active points.
The whole file leaves the sequence, so the boundary pointers must end
consistent (typically all null within that file).
4. Deleting an already-inactive point is a no-op success, not a `404` and not a
second relink.
5. Tests: after deleting a middle point, its old neighbors point at each other;
after deleting the first point, the new first has a null
`previous_chunk_id`; a full traversal of a file's pointer chain after a
series of deletes visits every active point exactly once and never enters an
inactive one.
**Exit criteria:** no delete path can leave a stale or dangling pointer, and
none removes a point from Qdrant.
### Phase 4: Create, replace, and payload update
1. `POST /v1/points` — create one point. Server assigns `tenant_id`, `version`,
and the derived point ID; the caller supplies content, position, and
metadata. Insert relinks neighbors like a reorder does.
2. `PUT /v1/points/{point_id}` — upsert with `update_only`, guarded by
`version` via `update_filter`; `409` on a stale version. Apply the
re-embedding decision above.
3. `PATCH /v1/points/{point_id}/payload` — payload-only update. Reject attempts
to write server-owned fields.
4. Enforce `points:write` scope; write an audit row per mutation
(tenant, actor, point, operation, resulting version).
5. Tests: concurrent writers at the same version — one succeeds, one gets `409`
and does not mutate; a `tenant_id` or `version` in the request body is
rejected; a create lands in the right sequence position.
**Exit criteria:** manual edits and ingestion re-runs cannot silently clobber
each other; server-owned fields are unwritable through any endpoint.
### Phase 5: Reorder and batch
1. `PATCH /v1/points/{point_id}/order` — assign a new fractional `order_id`
between the new neighbors and relink up to four points in one
`points/batch`, per ADR-0002.
2. Handle the boundary moves (to first, to last) and the no-op move
(already in that position).
3. Implement the fractional-gap decision from above.
4. `POST /v1/points/batch` with the semantics decided above, including the
per-request operation cap.
5. Tests: a randomized sequence of inserts, moves, and deletes leaves the
`order_id` ordering and the pointer chain agreeing with each other after
every step — this property test is the main defense for this phase.
**Exit criteria:** display order derived from `order_id` and traversal order
derived from the pointer chain are identical for every file, after any sequence
of mutations.
### Phase 6: Integration tests, operations, and documentation
1. Testcontainers Qdrant integration tests for each endpoint's real filter and
ordering behavior, isolated per test by unique collection or tenant keys.
2. An end-to-end test crossing plan 001 and this slice: ingest a CSV, list its
points, reorder one, soft-delete another, re-upload the same file, and assert
the manual edits interact with re-ingestion exactly as ADR-0001/0002 specify.
If that interaction is not yet decided, this test is what forces the decision.
3. Structured logging at the mutation boundary with stable event names
(`points.updated`, `points.reordered`, `points.soft_deleted`) carrying
`request_id`, `tenant_id`, `file_id`, and the resulting version.
4. Extend the operator runbook: how to inspect a file's point sequence, how to
spot a broken pointer chain, and how to recover one.
5. Update the README and ADR-0002 with anything this implementation settled.
**Exit criteria:** every endpoint has an integration test against real Qdrant,
and an operator can diagnose an ordering problem from the logs and the runbook.
## Definition of done
This slice is done when, in local Compose and under automated test:
```text
ingest a CSV (plan 001)
-> GET /v1/points?file_id=... lists its active points in order_id order
-> PATCH .../order moves one point; order_id and the
previous/next chain still agree
-> PUT /v1/points/{id} edits content under a version guard;
a stale write gets 409
-> DELETE /v1/points/{id} soft-deletes and relinks neighbors
-> GET /v1/points/search?q=... keyword-matches content within the tenant
-> every one of the above returns 404, not 403, for another tenant
```
The next plan (003) is agent hybrid retrieval — three prefetches, RRF fusion,
late-interaction rerank, and context-window expansion over the pointer chain
this slice is responsible for keeping correct (ADR-0003, ADR-0005). Do not pull
any of it into this plan.