diff --git a/CLAUDE.md b/CLAUDE.md index 2439b87..254f0f1 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -37,7 +37,7 @@ driving `tests/e2e/test_compose_smoke.py` against a real uvicorn process, which skips itself unless `SMOKE_BASE_URL` is set. That maps to plan 001 Phases 1-6 done. -Plan 002 (`/v1/points` CRUD and keyword search) is **Phases 1-2 done**. Phase 1 +Plan 002 (`/v1/points` CRUD and keyword search) is **Phases 1-3 done**. Phase 1 landed the `PointRepository` port (`src/application/ports/point_repository.py`) with its `Point` read model (`src/application/points/point.py`), the Qdrant adapter (`src/infrastructure/qdrant/point_repository.py`), request/response schemas @@ -75,15 +75,59 @@ with `normalize_persian_text` before matching, because ingestion letter-folds content and an unfolded Arabic-keyboard query would return an empty result set silently rather than erroring (ADR-0002). +Phase 3 added **soft delete**: `DELETE /v1/points/{point_id}` and +`DELETE /v1/files/{file_id}`, over `src/application/points/deletion.py` (with +the pure relinking primitive in `src/application/points/relinking.py`) and +`src/application/files/deletion.py`. Both are gated on `points:write` — the +file route included, since the data it destroys is points. Nothing is ever +removed from Qdrant. + +Four rules there are load-bearing, and three of them look like complications +until the concurrency is taken seriously: + +- `patches_for_removal` computes **what is still missing between the state just + read and the desired end state**, not "the patches a delete implies". That is + what makes a normal delete, a second delete of an already-inactive point (a + no-op success, never `404`), and recovery from a half-applied batch one code + path. Rewriting it as a straight-line "deactivate, patch prev, patch next" + breaks all three. +- Qdrant has no multi-point transaction and reports success for a filtered + `set_payload` that matched nothing, so a batch whose second operation loses a + version race applies its first anyway. `soft_delete_point` therefore re-plans + and re-applies up to three times, verifying by read-back, and only then raises + `PointVersionConflictError` (`409`). A single-shot delete would be able to + leave a stale pointer, which ADR-0002 calls a defect. +- A soft-deleted point **keeps its own** `previous_chunk_id`/`next_chunk_id`; + only the surviving neighbours are rewritten. Those pointers are unreachable + rather than stale, they are the only record of where the point sat, and the + retry re-plans from them. The whole-file sweep follows from the same rule: + every point leaves at once, so no survivor can dangle and no pointer is + touched at all. +- `DELETE /v1/files/{file_id}` marks the `source_files` row `soft_deleted` + **after** the point sweep, in its own short transaction (no session is held + across the Qdrant work). Order matters: a half-finished sweep leaves the row + `active` and a retried `DELETE` finishes it, and retiring the row is what + makes a later re-upload of the same bytes re-ingest instead of matching + `find_active_by_content_hash` and returning a file whose points are gone. + +Audit rows are still Phase 4/6 work; Phase 3 emits log events only +(`points.soft_deleted`, `files.soft_deleted`, `points.relink.neighbour_missing`, +and the two `*.conflict` warnings). The completion and conflict events carry +ADR-0011's `duration_ms` plus `rounds`, and the pair is what makes them +diagnostic: relinking itself is O(1) (that is what the adjacency pointers buy), +so a single-point delete costs a fixed ~5 Qdrant round trips and a `rounds` +above 1 means contention, not a slow store. The whole-file sweep is the one +whose cost scales — two round trips per 100-point page. + Also worth knowing before touching the points tests: `tests/support/point_contract.py` holds **one** scenario suite run against both `FakePointRepository` (unit) and `QdrantPointRepository` (integration), so new repository behaviour belongs there rather than in one of the two runners — that is what keeps the fake from drifting more permissive than the real store. -Not built yet: plan 002 Phases 3-6 — soft delete with neighbour relinking, -create/replace/patch, reorder and batch, and the -`api_request_logs`/`point_audit_events` tables — and `src/agent/`. +Not built yet: plan 002 Phases 4-6 — create/replace/patch, reorder and batch, +the `api_request_logs`/`point_audit_events` tables, and the runbook section on +inspecting and repairing a file's pointer chain — and `src/agent/`. Architecture decisions live in `docs/adr/` (18 ADRs plus the 0000 template; 0001–0004 are `Accepted` — 0004 amended by 0018; 0014 is `Superseded by 0017`;