diff --git a/docs/adr/0002-chunk-crud-and-search-api.md b/docs/adr/0002-chunk-crud-and-search-api.md index 5dcd328..8309837 100644 --- a/docs/adr/0002-chunk-crud-and-search-api.md +++ b/docs/adr/0002-chunk-crud-and-search-api.md @@ -105,6 +105,41 @@ 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 diff --git a/docs/plans/002-point-crud-and-keyword-search.md b/docs/plans/002-point-crud-and-keyword-search.md index 79fd813..5fb5d45 100644 --- a/docs/plans/002-point-crud-and-keyword-search.md +++ b/docs/plans/002-point-crud-and-keyword-search.md @@ -15,8 +15,9 @@ 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 +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 @@ -71,7 +72,9 @@ project owner accepts them, and update the ADR rather than diverging silently. (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. +- 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. @@ -113,40 +116,22 @@ project owner accepts them, and update the ADR rather than diverging silently. 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 +## Decisions resolved before implementation -### Re-embedding on content edit (blocks Phase 4) +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. -`PUT /v1/points/{point_id}` can change `content`. The stored vectors then no -longer match the text. Three options, in order of preference: +| 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" | -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." +Phase 6's cross-slice end-to-end test therefore *verifies* the re-ingestion rule +rather than forcing the decision. ## Build order @@ -251,8 +236,8 @@ of mutations. 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. + 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.