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>
248 lines
13 KiB
Markdown
248 lines
13 KiB
Markdown
# 0002. Chunk CRUD and payload/keyword search API
|
||
|
||
## Status
|
||
|
||
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)),
|
||
we need FastAPI endpoints for direct, fine-grained management of individual
|
||
chunks: create a single chunk, update a chunk's content/vectors/payload,
|
||
delete chunks (by ID or in bulk by filter), and list/search chunks by keyword
|
||
and payload metadata (tenant, domain, file, etc.) — independent of the
|
||
semantic hybrid retrieval used by the AI agent (ADR-0003).
|
||
|
||
This pipeline operates on the same collection and payload schema defined in
|
||
ADR-0001, and Qdrant's Points API already provides the primitives needed —
|
||
there's no need for a custom data layer on top.
|
||
|
||
## Decision
|
||
|
||
Expose a thin FastAPI layer directly over Qdrant's Points API, operating on
|
||
the `chunks` collection and payload schema from ADR-0001:
|
||
|
||
| Operation | FastAPI endpoint (indicative) | Qdrant primitive |
|
||
|---|---|---|
|
||
| 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
|
||
|
||
Chunks belonging to a `file_id` are listed via `scroll` ordered by the
|
||
mutable `order_id` payload field (ADR-0001) — this is what lets the frontend
|
||
display and let a backend user modify chunk order. Because `order_id` is a
|
||
fractional float key rather than a sequential integer position, moving or
|
||
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 /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:
|
||
1. The moved chunk: new `order_id`, new `previous_chunk_id`/`next_chunk_id`.
|
||
2. Its old neighbors: re-link their `previous_chunk_id`/`next_chunk_id` to
|
||
skip over the moved chunk.
|
||
3. Its new neighbors: re-link them to point at the moved chunk.
|
||
|
||
The same fix-up applies to insert (new chunk) and delete (soft-deleted chunk
|
||
— see below) operations: any operation that changes a chunk's position or
|
||
removes it from the sequence must atomically update the neighbors' pointers
|
||
in the same `points/batch` request, or `previous_chunk_id`/`next_chunk_id`
|
||
go stale.
|
||
|
||
### Keyword search is not semantic search
|
||
|
||
`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
|
||
retrieval used by the AI agent in ADR-0003; the two "search" concepts serve
|
||
different callers (a human/admin managing chunks vs. an agent retrieving
|
||
context) and should not be conflated in the API or in future discussion.
|
||
|
||
### Delete is soft by default
|
||
|
||
`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 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
|
||
deleted chunk's `previous_chunk_id`/`next_chunk_id` neighbors are relinked to
|
||
point at each other in the same `points/batch` call, so context-window
|
||
expansion (ADR-0003) never walks into a deactivated or removed chunk.
|
||
|
||
### Tenant isolation
|
||
|
||
Every endpoint (read and write) has its `tenant_id` filter injected
|
||
server-side from the authenticated request context — never accepted as
|
||
client-supplied input in the request body or query string. This is
|
||
non-negotiable: it's the same isolation boundary ADR-0001 relies on for
|
||
multitenancy, and it must hold for every code path that touches the
|
||
collection, not just ingestion.
|
||
|
||
### Concurrency
|
||
|
||
Updates use the `version` payload field (from ADR-0001) together with
|
||
Qdrant's `update_filter`, giving an optimistic-concurrency-style guard
|
||
against races between a concurrent ingestion re-run (ADR-0001) and a manual
|
||
edit through this API.
|
||
|
||
### Re-ingestion versus manual edits
|
||
|
||
A file can be re-uploaded after someone has hand-edited one of its points
|
||
through this API. **The newly ingested file wins.** Ingestion is authoritative
|
||
for the content of the file it ingested; a manual edit is a correction that
|
||
survives only until the source document is replaced.
|
||
|
||
Concretely:
|
||
|
||
- A point that still exists in the new version (same `file_id` +
|
||
`chunk_index`, hence the same deterministic point ID) is **overwritten in
|
||
place**. Ingestion performs a read-check-write so `version` is incremented
|
||
from whatever the manual edit left it at, rather than reset to `1`.
|
||
- A point from the previous ingestion that is **absent** from the new version
|
||
is flagged `is_active: false` with `deleted_at` set. It is never removed
|
||
from Qdrant — the soft-delete rule above applies to re-ingestion exactly as
|
||
it applies to `DELETE`.
|
||
- A manually created point (`POST /points`) is assigned a `chunk_index` past
|
||
the ingested range, so the same sweep deactivates it on the next upload of
|
||
its file. This is the intended consequence of "the new file wins", not an
|
||
accident of the sweep's bounds.
|
||
|
||
Because the point ID is derived from the immutable `chunk_index`, an
|
||
overwritten point cannot hold both the manual edit and the new file's content.
|
||
The clobbered content is therefore recorded in `point_audit_events`
|
||
(ADR-0009) as a `reingest_overwrite` operation carrying `before_version`, so
|
||
the edit is recoverable from the audit trail even though it is no longer a
|
||
live point.
|
||
|
||
Rejected alternative: preserving manual edits by having ingestion skip points
|
||
with `version > 1`. It breaks the guarantee that a successful upload leaves
|
||
Qdrant matching the uploaded document, and it needs a second, separate rule
|
||
for edited points that no longer exist in the new version — two divergent
|
||
notions of authority over one file.
|
||
|
||
### Re-embedding on content edit
|
||
|
||
`PUT /points/{point_id}` can change `content`, which leaves the stored
|
||
vectors stale unless they're recomputed. When `content` changes, the point
|
||
is **re-embedded inline**, reusing the same async embedding ports and
|
||
batch/semaphore bounds ingestion uses ([0017](0017-synchronous-ingestion-in-the-request-path.md)),
|
||
for parity between the two write paths. When `content` is unchanged, the
|
||
edit applies only the supplied vector/payload fields and skips re-embedding
|
||
entirely. Failure modes on this path reuse ingestion's status codes: `502`
|
||
on embedder failure, `504` if the edit's embedding step exceeds the same
|
||
timeout budget class as ingestion. The `version` guard (`update_filter`)
|
||
still applies to the write — re-embedding happens before the guarded write,
|
||
not instead of it, so a stale-version edit still fails with `409` rather
|
||
than re-embedding for nothing.
|
||
|
||
Rejected alternatives: requiring the caller to supply vectors when content
|
||
changes (pushes model knowledge onto the client, and is easy to get subtly
|
||
wrong); marking the point stale for background re-embedding later (needs
|
||
background work, which ADR-0017 currently rules out for this slice).
|
||
|
||
### `order_id` gap exhaustion
|
||
|
||
Repeatedly inserting into the same gap between two neighbors eventually
|
||
exhausts float precision (ADR-0001's known limitation). This slice does
|
||
**not** ship a renormalize endpoint. Instead, any operation that assigns a
|
||
new fractional `order_id` between two neighbors (insert, reorder) computes
|
||
the resulting gap and:
|
||
|
||
- logs a structured warning (`points.order_id.gap_low`) with `file_id` and
|
||
the two neighbor point IDs once the gap falls under a defined safety
|
||
threshold, so the condition is observable before it becomes uninsertable;
|
||
- **rejects** the write with `409` and a distinct error code if the
|
||
computed gap is no longer representable (would collapse to one of the two
|
||
neighbor values), instead of silently applying an imprecise value.
|
||
|
||
Recovering from an exhausted gap is a manual data-fix operation covered by
|
||
the operator runbook, not an endpoint this slice builds — deferring the
|
||
renormalize primitive is acceptable, silently producing an unrepresentable
|
||
gap is not.
|
||
|
||
### `POST /points/batch` semantics
|
||
|
||
Batch requests are **all-or-nothing**, capped at **100 operations per
|
||
request**. The service layer validates every operation's `version`
|
||
precondition before applying any of them; if any operation's precondition
|
||
fails, the whole request is rejected with `409` and nothing is applied — no
|
||
partially-applied batch ever reaches Qdrant. This follows directly from the
|
||
`version`-guard rule above applied at the batch level, and from the
|
||
pointer-relinking rule (a reorder/insert/delete's neighbor updates must land
|
||
in the same `points/batch` call, and a partial relink is a defect): partial
|
||
application of a batch is exactly the failure mode that would produce a
|
||
stale pointer chain. The 100-operation cap is independent of ADR-0001's
|
||
64–256-point bulk-ingestion batch sizing — that number is about upload
|
||
throughput; this one bounds an admin/manual edit request to something that
|
||
comfortably finishes inside a normal request timeout.
|
||
|
||
## Consequences
|
||
|
||
### Positive
|
||
- Reuses the exact collection/schema from ADR-0001 — no parallel data model
|
||
to keep in sync.
|
||
- Thin mapping to Qdrant primitives keeps the API predictable and easy to
|
||
extend as new filter/sort needs arise.
|
||
- Clear separation from ADR-0003's semantic search avoids API consumers
|
||
confusing "find chunks matching this filter" with "retrieve chunks relevant
|
||
to this query."
|
||
|
||
### Negative
|
||
- Keyword search here is limited to what Qdrant's full-text payload index
|
||
supports — no stemming/synonym handling beyond what that index offers.
|
||
- Optimistic concurrency via `version` requires every writer (ingestion and
|
||
this API) to consistently read-check-write; a writer that skips this can
|
||
silently clobber concurrent edits.
|
||
- Inline re-embedding puts embedder latency and `502`/`504` failure modes on
|
||
an admin content edit, not just on ingestion — an edit that only intended
|
||
to fix a typo pays the same embedding cost as a fresh chunk.
|
||
- Deferring the `order_id` renormalize endpoint means a file whose gaps are
|
||
genuinely exhausted has no automated recovery in this slice; an operator
|
||
must intervene by hand until that endpoint exists.
|
||
- All-or-nothing batch semantics mean one stale operation in a 100-operation
|
||
batch fails the entire request, even when the other 99 operations are
|
||
independent and would have succeeded on their own.
|
||
|
||
## Alternatives Considered
|
||
|
||
- **Separate keyword-search infrastructure (e.g. Elasticsearch)**: rejected —
|
||
Qdrant's native full-text payload index already covers filter-style keyword
|
||
search on `content`, and adding a second search system would duplicate data
|
||
and infrastructure for no clear benefit at current scale.
|
||
- **Client-supplied `tenant_id` in request body**: rejected — trusting
|
||
client input for the isolation boundary is a direct multitenancy security
|
||
risk; it must come from server-side auth context.
|
||
- **Caller-supplied vectors on content edit**: rejected — pushes embedding
|
||
model knowledge onto the client and makes it easy to silently desync
|
||
vectors from content.
|
||
- **Mark-stale-and-re-embed-later on content edit**: rejected for this
|
||
slice — needs background work, which ADR-0017 currently rules out.
|
||
- **Renormalize `order_id` automatically within this slice**: rejected —
|
||
nothing in current scope has hit gap exhaustion; building the primitive
|
||
now is speculative. Revisit if the logged warning starts firing in
|
||
practice.
|
||
- **Partial-success batch semantics (per-operation status)**: rejected —
|
||
a partially-applied batch is exactly the failure mode that leaves the
|
||
pointer chain (`previous_chunk_id`/`next_chunk_id`) inconsistent, which
|
||
this ADR treats as a defect, not a degraded-but-acceptable outcome.
|