Files
chatbot_v3/docs/plans/002-point-crud-and-keyword-search.md
Ali Zarinkolah ac3810182d docs(adr): record the re-ingestion rule and close plan 002's open decisions
Why:
- ADR-0002 already answered three of the four questions plan 002 listed as
  "decisions needed"; the fourth -- what happens to a manually edited point when
  its file is re-uploaded -- was left for a Phase 6 test to force. Deciding it in
  code rather than in the ADR would invert this repo's rule.

Changes:
- ADR-0002 gains "Re-ingestion versus manual edits": the new file wins,
  surviving points are overwritten with an incremented version, absent points
  are flagged inactive rather than removed, and manually created points sit past
  the ingested chunk_index range so the existing sweep covers them.
- Plan 002's stale decisions section becomes a pointer table; its audit scope is
  pinned to both ADR-0009 tables.

Impact:
- Clobbered edits are recoverable from point_audit_events, not from Qdrant: the
  deterministic point ID cannot hold both versions.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
2026-08-22 13:09:15 +03:30

271 lines
15 KiB
Markdown
Raw Permalink 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 is complete through Phase 6, so this prerequisite is satisfied. It
required plan 001 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: both ADR-0009 tables,
`api_request_logs` (one row per API call, written from the request middleware)
and `point_audit_events` with the real `api_request_log_id` foreign key.
- 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 resolved before implementation
An earlier revision of this plan listed three open decisions here. All are now
settled, and one further question this plan deferred to a Phase 6 test has been
settled too. They are recorded in the ADRs — these lines are a pointer, not a
second source of truth.
| Question | Resolution | Recorded in |
|---|---|---|
| Re-embedding on content edit | Re-embed inline, reusing ingestion's ports and bounds and its `502`/`504` codes. The re-embed happens *before* the version-guarded write, so a stale edit still `409`s rather than re-embedding for nothing. | ADR-0002, "Re-embedding on content edit" |
| Fractional-key exhaustion | No renormalize endpoint in this slice. Log `points.order_id.gap_low` under a safety threshold; reject with `409` and a distinct error code if the gap would collapse onto a neighbor value. Recovery is a runbook operation. | ADR-0002, "`order_id` gap exhaustion" |
| Batch semantics | All-or-nothing, capped at 100 operations. Every operation's `version` precondition is validated before any is applied; one failure rejects the whole request and nothing reaches Qdrant. | ADR-0002, "`POST /points/batch` semantics" |
| Re-ingestion versus manual edits | The newly uploaded file wins. Surviving points are overwritten in place with an incremented `version`; points absent from the new version are flagged inactive, never removed; manually created points sit past the ingested `chunk_index` range and are swept by the same rule. Clobbered content is recorded in `point_audit_events` as `reingest_overwrite`. | ADR-0002, "Re-ingestion versus manual edits" |
Phase 6's cross-slice end-to-end test therefore *verifies* the re-ingestion rule
rather than forcing the decision.
## 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-0002's
"Re-ingestion versus manual edits" specifies.
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.