From 5444838d6c1d73948aaa9dd066ac13d47570a9db Mon Sep 17 00:00:00 2001 From: Pier-Jean Malandrino Date: Wed, 29 Apr 2026 15:13:43 +0200 Subject: [PATCH 01/35] chore: gitignore local-only artifacts on release/0.6.0 Bootstrap commit for the doc-centric ingest release: - .github/ISSUE_TEMPLATE/issue.md (local copy) - docs/claude-code-commands.md (personal notes) - docs/design/TEMPLATE.md (local design template) - document-parser/tests/test_local_converter.py (local test scratch) --- .gitignore | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/.gitignore b/.gitignore index a1e287a..11e6c7c 100644 --- a/.gitignore +++ b/.gitignore @@ -49,3 +49,10 @@ profiles/ # E2E tests — Maven build outputs & Chrome user data e2e/**/target/ + +# Local-only artifacts (release/0.6.0 bootstrap) +.github/ISSUE_TEMPLATE/issue.md +docs/claude-code-commands.md +docs/design/TEMPLATE.md +document-parser/tests/test_local_converter.py + From 805d881d65681971138470828abbf95c42c87c1b Mon Sep 17 00:00:00 2001 From: Pier-Jean Malandrino Date: Wed, 29 Apr 2026 15:24:16 +0200 Subject: [PATCH 02/35] docs(design): E1 design docs for 0.6.0 doc-centric ingest MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds technical design docs for the foundation of the doc-centric ingest pivot: - 202 — Document lifecycle state machine - 203 — Per (document, store) ingestion state - 204 — Auto-detect Stale state via chunk content hash - 205 — Audit trail for chunk edits (chunks → first-class entity) - 206 — Migration of existing documents to the new model Status: Accepted on all five. Each doc spells out the domain entities, persistence schema, services orchestration, API contract, alternatives considered, risks, and testing strategy. ADR placeholders called out where load-bearing decisions warrant a follow-up document. Refs #202 #203 #204 #205 #206 --- .../202-document-lifecycle-state-machine.md | 292 ++++++++++++++ docs/design/203-per-store-ingestion-state.md | 329 ++++++++++++++++ docs/design/204-auto-stale-chunk-hash.md | 265 +++++++++++++ docs/design/205-chunk-edits-audit-trail.md | 372 ++++++++++++++++++ docs/design/206-lifecycle-state-migration.md | 265 +++++++++++++ 5 files changed, 1523 insertions(+) create mode 100644 docs/design/202-document-lifecycle-state-machine.md create mode 100644 docs/design/203-per-store-ingestion-state.md create mode 100644 docs/design/204-auto-stale-chunk-hash.md create mode 100644 docs/design/205-chunk-edits-audit-trail.md create mode 100644 docs/design/206-lifecycle-state-migration.md diff --git a/docs/design/202-document-lifecycle-state-machine.md b/docs/design/202-document-lifecycle-state-machine.md new file mode 100644 index 0000000..db205cd --- /dev/null +++ b/docs/design/202-document-lifecycle-state-machine.md @@ -0,0 +1,292 @@ +# Design: Document lifecycle state machine + +- **Issue:** #202 +- **Title on issue:** [FEATURE] Introduce Document lifecycle state machine +- **Author:** Pier-Jean Malandrino +- **Date:** 2026-04-29 +- **Status:** Accepted +- **Target milestone:** 0.6.0 — Doc-centric ingest +- **Impacted layers:** backend: domain · persistence · services · api · frontend: features/document · shared +- **Audit dimensions likely touched:** Hexagonal Architecture · DDD · Tests · Documentation +- **ADR spawned?:** no + +--- + +## 1. Problem + +Studio today tracks ingestion as a side-effect of an `AnalysisJob`. The only "state" a document carries is implicit — derived by joining the document with its latest analysis job (`PENDING`, `RUNNING`, `COMPLETED`, `FAILED`) and, separately, by checking whether OpenSearch holds rows for `doc_id`. There is no explicit, persisted lifecycle of the document itself. + +This blocks the 0.6.0 doc-centric pivot: we cannot show a document library `/docs` with status badges, we cannot tell the user "this doc is ingested but stale on store X", and we cannot drive a chunks editor that knows whether its draft is committed. Every later feature in E3, E4, and E5 reads or transitions a document state. + +This design introduces a first-class **document lifecycle** as a domain concept: a single canonical state on each document, validated transitions, persisted, and surfaced via the API. + +## 2. Goals + +- [ ] Add a `DocumentLifecycleState` enum to the domain with six states: `Uploaded`, `Parsed`, `Chunked`, `Ingested`, `Stale`, `Failed`. +- [ ] `Document` carries `lifecycle_state` (current state) + `lifecycle_state_at` (last transition timestamp). +- [ ] State transitions are validated in the domain layer; invalid transitions raise `InvalidLifecycleTransition`. +- [ ] State changes emit a domain event `DocumentLifecycleChanged(previous, current, at)`. +- [ ] Persisted via aiosqlite migration; existing rows default to `Uploaded` (refined by #206). +- [ ] Surfaced on the document API DTO (camelCase: `lifecycleState`, `lifecycleStateAt`). + +## 3. Non-goals + +- Per-`(doc, store)` state — that is **#203** and uses a different table. +- Auto-stale detection via hash — that is **#204**; this issue only declares the `Stale` state value. +- UI rendering of the badge — that is **#215**. +- Migrating production data — that is **#206**; this issue ships the schema + default value only. +- Refactoring `AnalysisJob.status`. The two concepts coexist: `AnalysisJob.status` describes **a single conversion attempt**; `Document.lifecycle_state` describes **the doc as a whole**. + +## 4. Context & constraints + +### Existing code surface + +- `document-parser/domain/models.py` — `Document` dataclass (line 27), `AnalysisJob` (line 38), `AnalysisStatus` enum. +- `document-parser/domain/value_objects.py` — value objects. +- `document-parser/persistence/database.py` — `_MIGRATIONS` list and `_run_migrations()` checking PRAGMA `table_info()`. +- `document-parser/persistence/document_repo.py` — `DocumentRepository` (aiosqlite). +- `document-parser/api/documents.py` — REST endpoints. +- `document-parser/api/schemas.py` — Pydantic DTOs with `alias_generator=_to_camel`. +- `frontend/src/features/document/store.ts` and `api.ts`. + +### Hexagonal Architecture constraints (backend) + +- `DocumentLifecycleState` is a **value object** — it lives in `domain/value_objects.py` with zero imports from `api`/`persistence`/`infra`. +- The transition rules are domain logic — they live as methods on `Document` (or a small `DocumentLifecycle` policy module in `domain/`). No HTTP, no DB. +- `DocumentRepository` (the port) gains a write path for the new fields. The aiosqlite adapter is the only place that knows about the column. +- API layer translates the enum to camelCase string via Pydantic. + +### Deployment modes + +Both `latest-local` and `latest-remote` images use the same SQLite schema → migration applies to both. No HF Space-specific concern. No feature flag (the lifecycle is always on; only UI surfaces are flagged). + +### Hard constraints + +- SQLite schema additive only. No column drops, no rename. New column has a `DEFAULT` so existing rows do not break on read. +- API contract additive only. New fields appear; nothing is removed. +- `pages_json` stays snake_case (existing exception). + +## 5. Proposed design + +### 5.1 Domain + +Add to `document-parser/domain/value_objects.py`: + +```python +from enum import StrEnum + +class DocumentLifecycleState(StrEnum): + UPLOADED = "Uploaded" + PARSED = "Parsed" + CHUNKED = "Chunked" + INGESTED = "Ingested" + STALE = "Stale" + FAILED = "Failed" +``` + +Allowed transitions (a directed graph; `* → Failed` always allowed): + +``` +Uploaded → Parsed | Failed +Parsed → Chunked | Failed +Chunked → Ingested | Chunked | Failed # re-chunking is allowed +Ingested → Stale | Chunked | Failed # re-ingest stays Ingested via 203 +Stale → Ingested | Chunked | Failed +Failed → Uploaded | Parsed | Chunked # explicit retry sets the new target +``` + +`Stale` is set by the auto-detect logic (#204) — never reached as a result of a manual action. + +Add to `document-parser/domain/models.py`: + +```python +@dataclass +class Document: + ... + lifecycle_state: DocumentLifecycleState = DocumentLifecycleState.UPLOADED + lifecycle_state_at: datetime | None = None + + def transition_to(self, target: DocumentLifecycleState, *, now: datetime) -> "DocumentLifecycleChanged": + if not _is_allowed(self.lifecycle_state, target): + raise InvalidLifecycleTransition(self.lifecycle_state, target) + previous = self.lifecycle_state + self.lifecycle_state = target + self.lifecycle_state_at = now + return DocumentLifecycleChanged(self.id, previous, target, now) +``` + +`InvalidLifecycleTransition` lives in `domain/exceptions.py` (create if missing). `_is_allowed` is a pure function over a static transition table. + +`DocumentLifecycleChanged` is a frozen dataclass in `domain/events.py`. No event bus is wired in 0.6.0 — the event is *returned* from the transition call so services can log / persist / publish later. This avoids introducing infra in this issue. + +### 5.2 Persistence + +Schema migration appended to `_MIGRATIONS` in `document-parser/persistence/database.py`: + +```sql +ALTER TABLE documents ADD COLUMN lifecycle_state TEXT NOT NULL DEFAULT 'Uploaded'; +ALTER TABLE documents ADD COLUMN lifecycle_state_at TEXT; +CREATE INDEX IF NOT EXISTS idx_documents_lifecycle_state ON documents(lifecycle_state); +``` + +The `_run_migrations()` PRAGMA check ensures idempotency — re-running on an already-migrated DB is a no-op. The index supports the `/docs` library filter by status (#212). + +`DocumentRepository.save(doc)` and `update_lifecycle(doc_id, state, at)` are the two write paths. Read returns the columns directly populated into the dataclass. + +### 5.3 Infra adapters + +None. The lifecycle is database-only. + +### 5.4 Services + +`DocumentService` and `AnalysisService` are the two callers that drive transitions. + +| Trigger | From | To | Caller | +|---|---|---|---| +| Upload completes | — | `Uploaded` | `DocumentService.upload()` | +| Parse succeeds | `Uploaded` (or `Failed` retry) | `Parsed` | `AnalysisService` | +| Chunking succeeds | `Parsed` (or `Chunked` re-chunk) | `Chunked` | `AnalysisService` | +| Ingestion succeeds | `Chunked` (or `Stale`) | `Ingested` | `IngestionService` (touched in #203) | +| Failure on any pipeline step | any | `Failed` | the failing service | + +Transition calls are atomic with the underlying write (e.g., chunking writes `chunks_json` *and* transitions to `Chunked` in the same SQL transaction). + +### 5.5 API + +`DocumentResponse` (in `document-parser/api/schemas.py`) gains: + +```python +class DocumentResponse(BaseModel): + ... + lifecycle_state: str = Field(serialization_alias="lifecycleState") + lifecycle_state_at: datetime | None = Field(default=None, serialization_alias="lifecycleStateAt") +``` + +No new endpoint. The existing `GET /api/documents` and `GET /api/documents/{id}` start returning the two fields. No 4xx removal — purely additive. + +`POST /api/documents/{id}/lifecycle` is **not** added in this issue; transitions are driven by pipeline events, not by the user. + +### 5.6 Frontend — feature module + +Touched: `frontend/src/features/document/`. + +- `api.ts` — bump the `Document` type with `lifecycleState: DocumentLifecycleState` and `lifecycleStateAt: string | null`. +- `store.ts` — store retains the new fields as-is. +- No UI work — that is #211 / #215. This issue ships the typed surface only so #211 can render against it. + +`shared/types.ts` gains a re-exported `DocumentLifecycleState` union literal type matching the backend enum. + +### 5.7 Cross-cutting + +- No feature flag. The lifecycle is permanent infrastructure. +- i18n: tooltip strings for the six states are added in `shared/i18n.ts`, keyed `lifecycle.`. Used by #215. + +## 6. Alternatives considered + +### Alternative A — Reuse `AnalysisJob.status` + +- **Summary:** Promote `AnalysisJob.status` to the document's state, removing the separate `Document.lifecycle_state`. +- **Why not:** `AnalysisJob` has a 1:N relationship with `Document` (a doc can be re-analyzed). The status of the *latest* job is not the same thing as the lifecycle of the *document*. It also conflates parse/chunk/ingest into one state machine, which #202–#205 explicitly want to separate. + +### Alternative B — Computed state, no column + +- **Summary:** Derive the state on-the-fly from analysis-job rows + OpenSearch presence. +- **Why not:** Joining + remote calls on every document list query is too expensive for `/docs` with 1k+ rows. A persisted column lets us index it for the filter (#212). The cost of denormalisation is one extra write per pipeline step. + +## 7. API & data contract + +### Endpoints + +| Method | Path | Request | Response | Breaking? | +|--------|------|---------|----------|-----------| +| GET | `/api/documents` | — | `DocumentResponse[]` (now with `lifecycleState`, `lifecycleStateAt`) | No (additive) | +| GET | `/api/documents/{id}` | — | `DocumentResponse` (now with `lifecycleState`, `lifecycleStateAt`) | No (additive) | + +### Persistence schema + +```sql +ALTER TABLE documents ADD COLUMN lifecycle_state TEXT NOT NULL DEFAULT 'Uploaded'; +ALTER TABLE documents ADD COLUMN lifecycle_state_at TEXT; +CREATE INDEX IF NOT EXISTS idx_documents_lifecycle_state ON documents(lifecycle_state); +``` + +Existing rows default to `Uploaded`; #206 refines them. + +### Env vars / config + +None. + +### Breaking changes + +Additive only. + +## 8. Risks & mitigations + +| Risk | Audit dimension | Likelihood | Impact | How we notice | Mitigation / rollback | +|------|-----------------|------------|--------|---------------|------------------------| +| Drift between `lifecycle_state` and the actual pipeline state (e.g. crash mid-write leaves `Chunked` recorded but no chunks) | DDD | Medium | Medium | Failed `/docs` rendering; tests catch on integration | Wrap the data write + transition in a single SQL transaction; never write the transition before the data | +| Existing migrations break on environments mid-flight | CI/Build | Low | High | Migration test on a snapshot DB | The migration is purely additive (`ADD COLUMN ... DEFAULT`) — safe to apply on any version | +| Confusion between `AnalysisJob.status` and `Document.lifecycle_state` for newcomers | Documentation | High | Low | Reviews ask | A short paragraph in `docs/architecture.md` describing the two concepts side by side | +| `Failed` state without a known reason makes debugging harder | Tests / Documentation | Medium | Medium | Operator complaints | The transition payload carries an optional `reason: str` persisted in a small JSON column or in the analysis-job's `error_message` | + +## 9. Testing strategy + +### Backend — pytest (`document-parser/tests/`) + +- **Unit (`tests/domain/`):** + - `test_lifecycle_transitions.py` — table-driven test of every (from, to) pair: allowed → updates state, disallowed → raises `InvalidLifecycleTransition`. + - `test_lifecycle_event.py` — successful transition returns the right `DocumentLifecycleChanged` event. +- **Persistence (`tests/persistence/`):** + - `test_document_repo_lifecycle.py` — write a doc, read it back, lifecycle fields round-trip; default value is `Uploaded`; index exists. +- **Services / integration:** + - `test_analysis_service_transitions.py` — parse pipeline drives `Uploaded → Parsed`; chunking drives `Parsed → Chunked`; failure → `Failed`. + +### Frontend — Vitest + +- `features/document/api.test.ts` — DTO parses `lifecycleState` correctly into the typed enum. +- `features/document/store.test.ts` — store exposes `lifecycleState` on the cached document. + +### E2E — Karate UI + +Out of scope for this issue. E2E coverage lands with #211 (the library page that renders the badge). + +### Manual QA + +1. `docker compose -f docker-compose.dev.yml up`. +2. Upload a doc → `GET /api/documents/{id}` returns `"lifecycleState": "Uploaded"`. +3. Run analysis → state becomes `"Parsed"` then `"Chunked"`. +4. Force a failure (corrupt PDF) → state becomes `"Failed"`. + +## 10. Rollout & observability + +### Release branch + +`release/0.6.0` (this work). + +### Feature flag / staged rollout + +None. Lifecycle persistence is always on; UI surfaces are flagged separately (per #210). + +### Observability + +- Each transition is logged at `INFO` with structured keys: `event=lifecycle_changed doc_id= from= to=`. +- No new Prometheus counter in this issue (added in #211 if useful for the library page). + +### Rollback plan + +The migration is additive. Reverting the code (without dropping the column) leaves the column populated but unused — safe. If the column itself must go, write a follow-up migration; SQLite supports column drop since 3.35 and we run a recent enough version. + +## 11. Open questions + +- Should we record the `reason` for `Failed` transitions on the document directly, or rely on the linked `AnalysisJob.error_message`? **Decision for 0.6.0:** rely on `AnalysisJob.error_message`; revisit if multiple non-analysis sources of failure emerge. +- Do we want a typed `DocumentLifecycleEvent` table for audit, or are logs enough? **Decision:** logs only in 0.6.0; #205's `chunk_edits` table is the audit substrate, doc-level events can be added later if needed. + +## 12. References + +- **Issue:** https://github.com/scub-france/Docling-Studio/issues/202 +- **Related issues:** #203 (per-store state), #204 (auto-stale), #205 (audit trail), #206 (migration), #211 (library), #215 (status badges) +- **ADRs:** none planned +- **Project docs:** + - Architecture: `docs/architecture.md` + - Coding standards: `docs/architecture/coding-standards.md` + - Audit master: `docs/audit/master.md` diff --git a/docs/design/203-per-store-ingestion-state.md b/docs/design/203-per-store-ingestion-state.md new file mode 100644 index 0000000..39baa90 --- /dev/null +++ b/docs/design/203-per-store-ingestion-state.md @@ -0,0 +1,329 @@ +# Design: Per (document, store) ingestion state + +- **Issue:** #203 +- **Title on issue:** [FEATURE] Track ingestion state per (document, store) pair +- **Author:** Pier-Jean Malandrino +- **Date:** 2026-04-29 +- **Status:** Accepted +- **Target milestone:** 0.6.0 — Doc-centric ingest +- **Impacted layers:** backend: domain · persistence · services · api · infra · frontend: features/ingestion · features/document · shared +- **Audit dimensions likely touched:** Hexagonal Architecture · DDD · Decoupling · Tests · Documentation +- **ADR spawned?:** ADR-NNN — "Multiple vector stores as first-class entities" + +--- + +## 1. Problem + +In the current architecture, "the vector store" is a single OpenSearch index hard-coded to `docling-studio-chunks`. Documents either are or are not indexed in that one place. The new doc-centric model assumes documents can live in **multiple stores** simultaneously — `rh-corpus-v3`, `legal-v1`, a snapshot for a customer A/B — and each `(doc, store)` pair has its own state. + +This is also where the killer flow lives: a customer reports bad answers from a specific store; the eng opens the doc, sees that it is `Ingested` in `rh-corpus-v3` and `Stale` in `rh-corpus-v2`, fixes the chunks, and re-ingests **the right one**. Without a per-pair state, that flow is impossible. + +## 2. Goals + +- [ ] Introduce a `Store` entity in the domain (id, name, kind, config, embedder). +- [ ] Introduce a `DocumentStoreLink` entity tying a document to a store with its own state and metadata. +- [ ] States on the link: `Ingested`, `Stale`, `Failed`. (No `Uploaded`/`Parsed`/`Chunked` — those are doc-level only.) +- [ ] The `Document.lifecycle_state` (#202) aggregates the per-store states by a documented rule. +- [ ] API exposes the per-store list on the document DTO. +- [ ] One default `Store` row is seeded on first migration so existing single-index deployments keep working. + +## 3. Non-goals + +- Auto-stale detection logic — that is **#204**; this issue ships the schema slot for the content hash but not the detection. +- Multi-tenant isolation between stores — the existing tenant model (if any) is not changed; stores are global today. +- A UI to create/manage stores — that is a follow-up. For 0.6.0 we ship the data model and the seed; a single store is enough for the killer flow on a single tenant. +- Cross-doc bulk re-ingest — that is **#213**; this issue exposes the per-link state needed by it. + +## 4. Context & constraints + +### Existing code surface + +- `document-parser/domain/ports.py` — `VectorStore` protocol (line 124). +- `document-parser/infra/opensearch_store.py` — `OpenSearchStore` adapter. +- `document-parser/services/ingestion_service.py` — `IngestionService.ingest()` and `ensure_index()`. +- `document-parser/persistence/database.py` — schema + migrations. +- `document-parser/api/ingestion.py` — endpoints. +- `frontend/src/features/ingestion/` — Pinia store + API client. + +### Hexagonal Architecture constraints (backend) + +- `Store` and `DocumentStoreLink` are domain entities (`domain/models.py`). Pure data + invariants. No ORM concern. +- `StoreRepository` and `DocumentStoreLinkRepository` are ports (`domain/ports.py`). +- aiosqlite adapters live in `persistence/store_repo.py` and `persistence/document_store_link_repo.py`. +- The existing `VectorStore` port stays — it represents the **technology** (OpenSearch). The new `Store` entity represents the **logical store** (a named, configurable target). One `VectorStore` adapter can serve many `Store` entities by namespacing the index name. + +### Deployment modes + +- Single `OpenSearchStore` adapter handles all stores via per-store `index_name = f"docling-studio-{store.slug}"`. Existing default index gets migrated to `docling-studio-default` (with a redirect alias to keep backwards-compatible reads). +- HF Space deployment ships with the same one-store seed. + +### Hard constraints + +- Existing OpenSearch index must remain readable during migration; reads against the legacy name are aliased. +- API is additive: existing `/api/ingestion/{analysis_id}` keeps working with an implicit default-store target until the UI explicitly picks one (#222). +- Tests must not require a real OpenSearch instance for the link layer — repo tests stub the adapter. + +## 5. Proposed design + +### 5.1 Domain + +`document-parser/domain/value_objects.py`: + +```python +class StoreKind(StrEnum): + OPENSEARCH = "opensearch" + # future: PINECONE, QDRANT, … + +class DocumentStoreLinkState(StrEnum): + INGESTED = "Ingested" + STALE = "Stale" + FAILED = "Failed" +``` + +`document-parser/domain/models.py`: + +```python +@dataclass +class Store: + id: str + name: str # "rh-corpus-v3" + slug: str # "rh-corpus-v3" (URL-safe; usually = name) + kind: StoreKind + embedder: str # e.g. "bge-m3" — record the embedder used for this store + config: dict # adapter-specific config (index_name override, etc.) + created_at: datetime + is_default: bool + +@dataclass +class DocumentStoreLink: + id: str + document_id: str + store_id: str + state: DocumentStoreLinkState + chunkset_hash: str | None # set by #204 on push + last_push_at: datetime | None + last_run_id: str | None # FK to a future runs table; nullable in 0.6.0 + error_message: str | None # populated on FAILED state + + def mark_ingested(self, *, hash_: str, at: datetime, run_id: str | None) -> None: ... + def mark_stale(self, *, at: datetime) -> None: ... + def mark_failed(self, *, at: datetime, error: str) -> None: ... +``` + +Aggregation rule for `Document.lifecycle_state` (defined as a pure function in `domain/lifecycle_aggregation.py`): + +``` +if any link state == FAILED → Document is FAILED +elif any link state == STALE → Document is STALE +elif any link state == INGESTED → Document is INGESTED +elif chunks present (state Chunked) → Document keeps its current state +else → keep current state +``` + +The aggregation runs as a side effect of any link write. The doc's own state column (#202) is the materialized result; it is not recomputed at read time. + +### 5.2 Persistence + +```sql +CREATE TABLE IF NOT EXISTS stores ( + id TEXT PRIMARY KEY, + name TEXT NOT NULL UNIQUE, + slug TEXT NOT NULL UNIQUE, + kind TEXT NOT NULL, + embedder TEXT NOT NULL, + config TEXT NOT NULL DEFAULT '{}', -- JSON + is_default INTEGER NOT NULL DEFAULT 0, + created_at TEXT NOT NULL +); + +CREATE TABLE IF NOT EXISTS document_store_links ( + id TEXT PRIMARY KEY, + document_id TEXT NOT NULL REFERENCES documents(id) ON DELETE CASCADE, + store_id TEXT NOT NULL REFERENCES stores(id) ON DELETE CASCADE, + state TEXT NOT NULL, + chunkset_hash TEXT, + last_push_at TEXT, + last_run_id TEXT, + error_message TEXT, + UNIQUE (document_id, store_id) +); + +CREATE INDEX IF NOT EXISTS idx_dsl_doc ON document_store_links(document_id); +CREATE INDEX IF NOT EXISTS idx_dsl_store ON document_store_links(store_id); +CREATE INDEX IF NOT EXISTS idx_dsl_state ON document_store_links(state); +``` + +Seed migration inserts one row in `stores`: + +```sql +INSERT OR IGNORE INTO stores (id, name, slug, kind, embedder, is_default, created_at) +VALUES ('default', 'default', 'default', 'opensearch', + '', + 1, datetime('now')); +``` + +The OpenSearch index name for `slug=default` is `docling-studio-default`. To keep existing data readable, an alias migration in `OpenSearchStore.ensure_index()` adds `docling-studio-chunks` as a read-only alias to `docling-studio-default` if the legacy index exists with rows. Operators with non-trivial data trigger a one-shot reindex via #206. + +### 5.3 Infra adapters + +`OpenSearchStore` is parameterised by store slug: + +```python +class OpenSearchStore(VectorStore): + def __init__(self, client, index_prefix: str = "docling-studio") -> None: ... + def _index_for(self, store_slug: str) -> str: + return f"{self.index_prefix}-{store_slug}" +``` + +Calls become `index_chunks(store_slug, doc_id, chunks, embeddings)` etc. The protocol in `domain/ports.py` is updated to take `store_slug` as an explicit argument. + +### 5.4 Services + +New: `StoreService` (`document-parser/services/store_service.py`) — list, get, create (admin only — locked behind a future flag; for 0.6.0 only seeded rows exist). + +`IngestionService.ingest(analysis_id, store_slug = "default")`: +1. Read chunks from `analysis_jobs.chunks_json`. +2. Compute embeddings. +3. Call adapter `index_chunks(store_slug, ...)`. +4. Upsert link via `DocumentStoreLinkRepository.upsert(doc_id, store_id, state=Ingested, chunkset_hash=..., at=now)`. +5. Recompute document aggregate state via the rule in §5.1. + +If any step fails: link is marked `Failed` with the error; doc state aggregates to `Failed`. + +### 5.5 API + +`schemas.py`: + +```python +class StoreLinkResponse(BaseModel): + storeId: str + storeName: str + state: str # DocumentStoreLinkState value + chunksetHash: str | None + lastPushAt: datetime | None + lastRunId: str | None + +class DocumentResponse(BaseModel): + ... # from #202 + stores: list[StoreLinkResponse] = Field(default_factory=list) +``` + +`stores` is a read-side aggregate computed by `DocumentService.find_by_id()` joining the link table. + +New endpoint `GET /api/stores` returns the list of stores (id, name, slug, embedder). Used by #222's target picker. + +### 5.6 Frontend — feature module + +- `frontend/src/features/ingestion/` — `Ingestion.run()` accepts an optional `storeSlug`; defaults to "default" if not supplied. +- New `frontend/src/features/stores/` — Pinia store, API client, types. +- `frontend/src/features/document/api.ts` — `Document` type gains `stores: StoreLink[]`. + +### 5.7 Cross-cutting + +- Feature flag: none. +- i18n: `stores.state.` keys in `shared/i18n.ts`. +- New env var documented in `.env.example`: `DEFAULT_EMBEDDER` (existing implicitly; now formalised in the seed). + +## 6. Alternatives considered + +### Alternative A — One row per (doc, store) embedded in the analysis job + +- **Summary:** Add a JSON column `stores_json` on `analysis_jobs`. +- **Why not:** Conflates an *analysis attempt* with a *long-lived ingestion link*. Re-analysis would erase per-store state. The relational shape is necessary for filters, indexes, and idempotent updates. + +### Alternative B — One adapter per store (parallel `VectorStore` instances) + +- **Summary:** Hold a registry of `VectorStore` adapters keyed by store id. +- **Why not:** Duplicates connection pools, breaks singleton pattern in FastAPI's dependency wiring, and forces N OpenSearch clients for what is logically one client serving N indexes. Parameterising the existing single adapter is cheaper. + +## 7. API & data contract + +### Endpoints + +| Method | Path | Request | Response | Breaking? | +|--------|------|---------|----------|-----------| +| GET | `/api/documents/{id}` | — | now includes `stores: StoreLinkResponse[]` | No (additive) | +| GET | `/api/stores` | — | `StoreResponse[]` | No (new) | +| POST | `/api/ingestion/{analysis_id}` | `{ "storeSlug": "default" }` (optional) | unchanged | No (additive) | + +### Persistence schema + +See §5.2. + +### Env vars / config + +| Name | Default | Allowed | Notes | +|------|---------|---------|-------| +| `DEFAULT_EMBEDDER` | `bge-m3` | any registered embedder slug | recorded on the seeded `default` store | +| `DOCLING_STUDIO_INDEX_PREFIX` | `docling-studio` | URL-safe slug | adapter-level prefix for OpenSearch indexes | + +### Breaking changes + +Additive. + +## 8. Risks & mitigations + +| Risk | Audit dimension | Likelihood | Impact | How we notice | Mitigation / rollback | +|------|-----------------|------------|--------|---------------|------------------------| +| Existing OpenSearch data invisible after rename | Decoupling | Medium | High | Smoke test on staging | Read alias `docling-studio-chunks → docling-studio-default` set in `ensure_index()`; explicit reindex documented in runbook | +| Multiple stores with conflicting embedders mixed at search time | Security/Performance | Low | Medium | Mismatched dim error from OpenSearch | Embedder is recorded per store; queries route by store; cross-store search is out of scope for 0.6.0 | +| Link table grows large over time on prod corpus | Performance | Medium | Medium | Slow `/api/documents/{id}` join | Index on `document_id` and `store_id`; pagination on the doc list (#211) | +| Aggregation rule drifts from per-store reality on partial writes | DDD | Medium | High | Tests fail | Aggregation runs in the same transaction as the link write | + +## 9. Testing strategy + +### Backend — pytest + +- **Unit (domain):** `test_store_link_transitions.py`, `test_lifecycle_aggregation.py` (table-driven over per-store state combinations). +- **Persistence:** `test_store_repo.py`, `test_document_store_link_repo.py` (round-trip; UNIQUE constraint enforced; cascade delete works). +- **Services / integration:** `test_ingestion_service_with_store.py` (default store path), `test_ingestion_service_two_stores.py` (push to two stores → two links). +- **Architecture:** import-boundary test ensures `domain/` does not import `infra/opensearch_store.py`. + +### Frontend — Vitest + +- `features/stores/api.test.ts` — list endpoint round-trip. +- `features/document/store.test.ts` — `Document.stores` populated from API. + +### E2E — Karate UI + +Out of scope here. Lands with #211 / #218. + +### Manual QA + +1. Boot. `GET /api/stores` returns the seeded `default`. +2. Upload a doc, run ingestion → `GET /api/documents/{id}` returns `stores[0].state == "Ingested"`. +3. Inspect SQLite: `document_store_links` has the row. + +## 10. Rollout & observability + +### Release branch + +`release/0.6.0`. + +### Feature flag + +None for the data; #222 will gate the multi-store UI. + +### Observability + +- Log `store_link_changed` with `doc_id`, `store_id`, `from`, `to`, `at`. +- Counter (Prometheus, future): `ingestion_links_total{state}`. Optional in 0.6.0. + +### Rollback plan + +The migration is additive. To roll back the code: revert; the unused tables stay empty for new docs but contain rows for already-pushed docs — harmless (no read path uses them after revert). For full cleanup, a follow-up migration drops the two tables. + +## 11. Open questions + +- Should `Store.config` be typed in the domain (via a discriminated union on `kind`)? **Decision for 0.6.0:** keep as opaque `dict`; introduce a typed wrapper when we add a second `kind`. +- Cross-store search (single query → many stores) — **explicitly punted** to a later release. + +## 12. References + +- **Issue:** https://github.com/scub-france/Docling-Studio/issues/203 +- **Related issues:** #202 (lifecycle), #204 (hash), #205 (audit), #206 (migration), #211 (library), #213 (bulk push), #222 (push UI), #223 (diff-aware ingest) +- **ADRs:** ADR — "Multiple vector stores as first-class entities" (to be drafted alongside the implementation PR) +- **Project docs:** + - Architecture: `docs/architecture.md` + - Coding standards: `docs/architecture/coding-standards.md` + - ADR guide: `docs/architecture/adr-guide.md` diff --git a/docs/design/204-auto-stale-chunk-hash.md b/docs/design/204-auto-stale-chunk-hash.md new file mode 100644 index 0000000..d909e97 --- /dev/null +++ b/docs/design/204-auto-stale-chunk-hash.md @@ -0,0 +1,265 @@ +# Design: Auto-detect Stale state via chunk content hash + +- **Issue:** #204 +- **Title on issue:** [FEATURE] Auto-detect Stale state via chunk content hash +- **Author:** Pier-Jean Malandrino +- **Date:** 2026-04-29 +- **Status:** Accepted +- **Target milestone:** 0.6.0 — Doc-centric ingest +- **Impacted layers:** backend: domain · persistence · services · frontend (read-only) +- **Audit dimensions likely touched:** Hexagonal Architecture · DDD · Performance · Tests · Security +- **ADR spawned?:** no + +--- + +## 1. Problem + +Without auto-detection, a user who edits a chunk has to remember which stores hold the doc and click "re-ingest" everywhere. They will forget. The next query against the stale store returns the old embedding, and the customer reports "I fixed it yesterday and it's still wrong". Manual flagging is a bug factory. + +The detection contract is: **the system, not the user, knows when a stored chunkset diverges from the current draft chunkset.** Implementation = a deterministic hash recorded at push time and compared on read or on chunk write. + +## 2. Goals + +- [ ] Define a deterministic `chunkset_hash(chunks: list[ChunkResult])` function. +- [ ] Store the hash on each `DocumentStoreLink` at push time (#203 ships the column slot). +- [ ] On any chunk modification, recompute the current chunkset hash and compare against each link; if mismatch, mark the link `Stale` and re-aggregate the document state. +- [ ] On document read (single-doc API), compare on the fly as a safety net so older drift is caught. +- [ ] Unit tests prove: edit a chunk → link state becomes `Stale`; re-push → link state becomes `Ingested` with the new hash. + +## 3. Non-goals + +- Per-chunk change tracking — that's the audit trail (#205); this issue cares about the *aggregate* hash only. +- Background sweeper job that scans the whole corpus for drift — for 0.6.0, detection is event-driven (on chunk write) + on-read; a sweeper can come later. +- Diff-aware re-ingest at the chunk granularity — that's #223; this issue tells you *whether* a re-ingest is needed, not which chunks to re-embed. +- A user-facing toggle to "force stale" — out of scope. + +## 4. Context & constraints + +### Existing code surface + +- `document-parser/domain/value_objects.py` — `ChunkResult` (line 93). +- `document-parser/services/analysis_service.py` — chunking pipeline. +- `document-parser/persistence/database.py` — `analysis_jobs.chunks_json` (the canonical chunkset today). +- `document-parser/services/ingestion_service.py` — push pipeline. +- `document-parser/api/ingestion.py` — push endpoint. +- `document-parser/persistence/document_store_link_repo.py` (created by #203). + +### Hexagonal Architecture constraints + +- `chunkset_hash` is a **pure domain function** — lives in `domain/hashing.py`. No I/O. No timestamps. Deterministic over the input. +- Detection (compare + transition) is orchestrated in services. Persistence reads the stored hash via the link repo. +- The hash is opaque to API/frontend in 0.6.0; surfaced only as a debug field on `StoreLinkResponse` (#203 added it). + +### Hard constraints + +- Hash function must be **stable across processes / machines / Python versions** — so SHA-256 (`hashlib`), not Python `hash()`. No salt. +- Must be cheap on a 500-chunk doc — a single linear pass, no JSON re-parse on hot path. +- Result is a hex string, length 64. Stored as `TEXT` in SQLite. + +## 5. Proposed design + +### 5.1 Domain + +`document-parser/domain/hashing.py`: + +```python +import hashlib +import json +from collections.abc import Iterable +from .value_objects import ChunkResult + +def chunkset_hash(chunks: Iterable[ChunkResult]) -> str: + """ + Deterministic hash over a chunkset. + + Hashed inputs (per chunk, in chunkset order): + - text (str) + - source_page (int | None) + - headings (list[str], preserved order) + + Excluded: + - bboxes / doc_items (rendering artefacts; do not affect retrieval semantics) + - token_count (derived; unstable across tokenizers) + """ + h = hashlib.sha256() + for chunk in chunks: + payload = { + "t": chunk.text, + "p": chunk.source_page, + "h": list(chunk.headings or []), + } + h.update(b"\x1f") + h.update(json.dumps(payload, ensure_ascii=False, separators=(",", ":")).encode()) + return h.hexdigest() +``` + +Notes: +- The `\x1f` (Unit Separator) byte between chunks defends against the "join attack" where chunk A's tail and chunk B's head produce the same hash as chunk A+B merged. +- `separators=(",", ":")` is the canonical compact JSON form. +- The exclusion list is intentional and documented inline — changing it bumps every doc to `Stale` once and is a deliberate one-time event (covered in §10 Rollback). + +`detect_stale_links(current_hash, links) -> list[DocumentStoreLink]` — pure helper returning the subset of links whose `chunkset_hash != current_hash`. + +### 5.2 Persistence + +No new schema. The `chunkset_hash` column was added by #203 on `document_store_links`. + +A new index supports the on-read safety net: + +```sql +CREATE INDEX IF NOT EXISTS idx_dsl_doc_state ON document_store_links(document_id, state); +``` + +### 5.3 Infra adapters + +None. Hashing is in-memory. + +### 5.4 Services + +Two call sites: + +**A. On chunk write (event-driven detection)** + +`AnalysisService.persist_chunks(doc_id, chunks)`: +1. Persist chunks (existing path: `analysis_jobs.chunks_json`). +2. Compute `current_hash = chunkset_hash(chunks)`. +3. Read all `DocumentStoreLink` rows for `doc_id`. +4. For each link with `chunkset_hash != current_hash` and `state in (Ingested,)`: + - `link.mark_stale(at=now)` and persist. +5. Re-aggregate the document state (#202). + +This runs in the same transaction as the chunk write. The full pass is O(N_links) which is small (a doc rarely lives in more than a handful of stores). + +**B. On push completion (set the new hash)** + +Already covered by #203's `IngestionService.ingest`. Add the `current_hash` argument when calling `link.mark_ingested(hash_=current_hash, at=now, run_id=...)`. + +**C. On read (safety net)** + +`DocumentService.find_by_id(id)`: +1. Fetch document + links (existing). +2. Compute `current_hash` (cheap; cached per request). +3. Mark any `Ingested` link with mismatched hash as `Stale` and persist (best-effort, swallowed if write fails — log). + +This guards against drift caused by direct DB writes / restored backups / older deploys. + +### 5.5 API + +No new endpoint. `StoreLinkResponse.chunksetHash` (already added by #203) is now actually populated. + +### 5.6 Frontend — feature module + +No changes in this issue. #224 (stale indicator) reads the existing field. + +### 5.7 Cross-cutting + +- Feature flag: none. +- Logs: `INFO event=stale_detected doc_id= store_id= previous_hash=<8> current_hash=<8>` (truncated for privacy / log volume). +- ADR: not required. The choice of SHA-256 + canonical JSON is documented inline in `hashing.py`. + +## 6. Alternatives considered + +### Alternative A — Per-chunk hashes only (no chunkset hash) + +- **Summary:** Skip the aggregate hash; track each chunk's hash and detect "any per-chunk hash drift". +- **Why not:** Per-chunk hashing is needed for #223 (diff-aware re-embed) but not for "is this link stale at all?". A single `chunkset_hash` is one-comparison cheap; per-chunk is N-comparisons. Both will exist after #223 lands; this issue ships the cheap top-level signal. + +### Alternative B — Store updated_at instead of hash + +- **Summary:** Compare `chunks.updated_at > link.last_push_at`. +- **Why not:** Brittle. Re-running a pipeline on identical input bumps `updated_at` without semantic change. Hash is content-addressed and survives idempotent rewrites. + +### Alternative C — MD5 / xxHash + +- **Summary:** Use a faster non-cryptographic hash. +- **Why not:** SHA-256 is fast enough on the volumes in scope (`<500 chunks`, microseconds in CPython hashlib via OpenSSL bindings). The cryptographic hash also gives us collision resistance for free. xxHash would require an extra dependency. + +## 7. API & data contract + +### Endpoints + +No additions. Existing `StoreLinkResponse.chunksetHash` becomes populated. + +### Persistence schema + +```sql +CREATE INDEX IF NOT EXISTS idx_dsl_doc_state ON document_store_links(document_id, state); +``` + +### Env vars / config + +None. + +### Breaking changes + +None. + +## 8. Risks & mitigations + +| Risk | Audit dimension | Likelihood | Impact | How we notice | Mitigation / rollback | +|------|-----------------|------------|--------|---------------|------------------------| +| Hash function changes (e.g. someone adds `bboxes` to the input) silently invalidates every link | DDD | Medium | High | Every doc flips to `Stale` post-deploy | Hash function is in a single file with a docstring listing the canonical inputs; CI fixture asserts a fixed hash for a known chunkset. Bumping requires updating the fixture deliberately. | +| Read-side detection updates state under a GET (write-on-read) | Performance / Decoupling | Low | Medium | Slow / unexpected SQL in read paths | Best-effort, swallowed write; only triggered when the API actually serves the doc detail page (already a write-allowed code path). Disabled if a query param `?refresh=false` is set (future). | +| Unicode normalization issues (NFC vs NFD) produce different hashes for "the same" text | Tests | Low | Medium | Drift after a copy-paste from a Mac | Document the policy: text is stored as-is, no normalization; if drift appears, normalize at write time, not at hash time. | +| JSON ordering instability across Python versions | Tests | Low | High | Hash mismatch on different prod nodes | `separators=(",", ":")` + `ensure_ascii=False` + explicit key order in the dict literal. Reviewer checklist mentions this. | + +## 9. Testing strategy + +### Backend — pytest + +- **Unit (domain):** + - `test_chunkset_hash_determinism.py` — same input → same output across multiple invocations. + - `test_chunkset_hash_sensitivity.py` — every input field in the canonical list changes the hash; excluded fields (`bboxes`, `token_count`) do not. + - `test_chunkset_hash_join_attack.py` — separating into different chunks must produce different hash from concatenation. + - **Locked fixture** `test_chunkset_hash_fixture.py` — a hand-built 3-chunk input whose hash is hard-coded; CI fails if anyone changes the function silently. +- **Services:** + - `test_stale_detection_on_edit.py` — edit a chunk → link state becomes `Stale`. + - `test_stale_clears_on_repush.py` — re-push → link state becomes `Ingested` with the new hash. + - `test_stale_safety_net_on_read.py` — direct DB tampering → next read flips state to `Stale`. + +### Frontend — Vitest + +None new in this issue. + +### E2E — Karate UI + +Out of scope here; lands with #224. + +### Manual QA + +1. Push a doc to the default store → `chunksetHash` populated, `state == "Ingested"`. +2. Edit a chunk via API → `state == "Stale"`. +3. Re-push → `state == "Ingested"`, new hash. + +## 10. Rollout & observability + +### Release branch + +`release/0.6.0`. + +### Feature flag + +None. Detection is always on; cheap; correctness improvement. + +### Observability + +- Log lines as in §5.7. +- One-time bump scenario: if we ever change the canonical input list, every link will flip to `Stale` once. That is a deliberate decision; the operator must be informed via release notes, and a one-shot reindex job is recommended (out of scope here). + +### Rollback plan + +The migration is additive (a new index). Reverting the code leaves the existing `chunkset_hash` column populated but unused — harmless. The index can be dropped in a follow-up. + +## 11. Open questions + +- Should the safety-net read-side check be opt-in via a query param? **Decision:** always-on for 0.6.0; revisit if the cost shows up in profiles. +- Should headings include the *path* (parent → leaf) or just the leaf? **Decision:** the full ordered list as it sits on `ChunkResult.headings`. If the source mutates the list semantics, that is a separate domain concern. + +## 12. References + +- **Issue:** https://github.com/scub-france/Docling-Studio/issues/204 +- **Related issues:** #202 (lifecycle), #203 (per-store state), #205 (audit), #206 (migration), #223 (diff-aware re-ingest), #224 (stale indicator) +- **ADRs:** none planned +- **Project docs:** + - Architecture: `docs/architecture.md` + - Coding standards: `docs/architecture/coding-standards.md` diff --git a/docs/design/205-chunk-edits-audit-trail.md b/docs/design/205-chunk-edits-audit-trail.md new file mode 100644 index 0000000..a2a9023 --- /dev/null +++ b/docs/design/205-chunk-edits-audit-trail.md @@ -0,0 +1,372 @@ +# Design: Audit trail for chunk edits + +- **Issue:** #205 +- **Title on issue:** [FEATURE] Audit trail for chunk edits (who, when, before/after) +- **Author:** Pier-Jean Malandrino +- **Date:** 2026-04-29 +- **Status:** Accepted +- **Target milestone:** 0.6.0 — Doc-centric ingest +- **Impacted layers:** backend: domain · persistence · services · api · frontend: features/chunking · shared +- **Audit dimensions likely touched:** Hexagonal Architecture · DDD · Tests · Security · Documentation +- **ADR spawned?:** ADR-NNN — "Chunks become a first-class persisted entity" + +--- + +## 1. Problem + +Once chunks become editable in 0.6.0 (E5), production teams will need to answer "who changed what, when, and why is the answer suddenly different?". Without an audit trail, a regression caused by a chunk edit is impossible to investigate. + +Today the architecture makes this hard because **chunks are not persisted as first-class records**. They live as a JSON blob inside `analysis_jobs.chunks_json` and as derived rows in OpenSearch. There is no chunk identity that survives a re-parse, no row to attach an audit record to, and no way to retrieve "the version of these chunks at the last push". + +This issue elevates chunks to a first-class persisted entity (with stable IDs and content versioning), introduces an immutable `chunk_edits` table, and exposes a snapshot API consumed by the visual diff (#221). + +## 2. Goals + +- [ ] Promote chunks to a first-class persisted entity with stable IDs across edits. +- [ ] Every chunk operation (create / update / delete / merge / split) writes an immutable `chunk_edits` record with: actor, timestamp, action, before-state, after-state, optional reason. +- [ ] API: `GET /api/documents/{id}/chunks/history` returns the edit timeline. +- [ ] API: `GET /api/documents/{id}/chunks?at=` returns the chunkset snapshot at a given push. +- [ ] Backwards compatible: existing reads of `analysis_jobs.chunks_json` keep working until #206 finishes the migration. + +## 3. Non-goals + +- Undo / redo UI — out of scope (the audit trail is the substrate; UI comes later). +- Rollback to an arbitrary past state — not in 0.6.0; use the snapshot API for read-only inspection. +- A diff *view* — that is **#221**. +- Per-chunk content hash — that is part of **#223**, not this issue. (#204's `chunkset_hash` is at the link granularity.) +- Authentication / authorization model rework — `actor` defaults to a hard-coded `"system"` until the auth layer is ready. + +## 4. Context & constraints + +### Existing code surface + +- `document-parser/domain/value_objects.py` — `ChunkResult` (transient). +- `document-parser/domain/models.py` — `AnalysisJob.chunks_json` (current chunkset storage). +- `document-parser/services/analysis_service.py` — chunking pipeline. +- `document-parser/persistence/database.py` — schema + migrations. +- `document-parser/api/` — no chunks endpoint today; needs creation. +- `frontend/src/features/chunking/` — Pinia store + API client. + +### Hexagonal Architecture constraints + +- New domain entities `Chunk` and `ChunkEdit` live in `domain/models.py`. +- `ChunkRepository` and `ChunkEditRepository` are ports (`domain/ports.py`). +- aiosqlite adapters in `persistence/chunk_repo.py` and `persistence/chunk_edit_repo.py`. +- A new `ChunkEditingService` in `services/` orchestrates the operations and writes audit records atomically with the chunk write. +- The existing `analysis_jobs.chunks_json` becomes a **legacy fallback** — read by `ChunkRepository.list_for_doc()` if no rows exist in the new `chunks` table. #206 backfills. + +### Hard constraints + +- Stable `chunk.id` across edits. A chunk that was split into two creates two new ids; merging two chunks produces a third new id. The "lineage" is recorded in `chunk_edits`. (No "ship the same id" hack.) +- Immutable audit table. Once written, never updated. +- No PII leak in audit records — `actor` is whatever the auth layer hands us; `before`/`after` payloads are the chunk text and metadata only. +- Atomicity: an edit + its audit row are written in the same SQL transaction. + +## 5. Proposed design + +### 5.1 Domain + +`document-parser/domain/models.py`: + +```python +@dataclass +class Chunk: + id: str # uuid4 hex + document_id: str + sequence: int # ordering within the doc; gaps allowed + text: str + headings: list[str] + source_page: int | None + bboxes: list[Bbox] # carried for rendering; not part of identity + doc_items: list[str] + token_count: int | None + created_at: datetime + updated_at: datetime + deleted_at: datetime | None # soft delete + +class ChunkEditAction(StrEnum): + INSERT = "insert" + UPDATE = "update" + DELETE = "delete" + MERGE = "merge" + SPLIT = "split" + +@dataclass(frozen=True) +class ChunkEdit: + id: str + document_id: str + chunk_id: str | None # null on MERGE result row (uses children_ids) + action: ChunkEditAction + actor: str # "system" until auth lands + at: datetime + before: dict | None # JSON snapshot — None for INSERT + after: dict | None # JSON snapshot — None for DELETE + parents: list[str] # for SPLIT result rows: source chunk id; for MERGE: source ids + children: list[str] # inverse links + reason: str | None +``` + +Domain operations on a chunkset (`domain/chunk_editing.py`): + +```python +def insert(chunks: list[Chunk], at_position: int, new_chunk: Chunk) -> list[Chunk]: ... +def update(chunks: list[Chunk], chunk_id: str, *, text: str, headings: list[str]) -> list[Chunk]: ... +def delete(chunks: list[Chunk], chunk_id: str) -> list[Chunk]: ... +def merge(chunks: list[Chunk], chunk_ids: list[str]) -> tuple[list[Chunk], Chunk]: + """Returns the updated list and the new merged chunk.""" +def split(chunks: list[Chunk], chunk_id: str, at_offset: int) -> tuple[list[Chunk], Chunk, Chunk]: + """Returns the updated list and the two new chunks.""" +``` + +These are pure. The service wraps each call with audit-record generation. + +### 5.2 Persistence + +```sql +CREATE TABLE IF NOT EXISTS chunks ( + id TEXT PRIMARY KEY, + document_id TEXT NOT NULL REFERENCES documents(id) ON DELETE CASCADE, + sequence INTEGER NOT NULL, + text TEXT NOT NULL, + headings TEXT NOT NULL DEFAULT '[]', -- JSON + source_page INTEGER, + bboxes TEXT NOT NULL DEFAULT '[]', -- JSON + doc_items TEXT NOT NULL DEFAULT '[]', -- JSON + token_count INTEGER, + created_at TEXT NOT NULL, + updated_at TEXT NOT NULL, + deleted_at TEXT +); +CREATE INDEX IF NOT EXISTS idx_chunks_doc ON chunks(document_id); +CREATE INDEX IF NOT EXISTS idx_chunks_doc_seq ON chunks(document_id, sequence); + +CREATE TABLE IF NOT EXISTS chunk_edits ( + id TEXT PRIMARY KEY, + document_id TEXT NOT NULL REFERENCES documents(id) ON DELETE CASCADE, + chunk_id TEXT, + action TEXT NOT NULL, + actor TEXT NOT NULL DEFAULT 'system', + at TEXT NOT NULL, + before_json TEXT, + after_json TEXT, + parents_json TEXT NOT NULL DEFAULT '[]', + children_json TEXT NOT NULL DEFAULT '[]', + reason TEXT +); +CREATE INDEX IF NOT EXISTS idx_chunk_edits_doc_at ON chunk_edits(document_id, at); +CREATE INDEX IF NOT EXISTS idx_chunk_edits_chunk ON chunk_edits(chunk_id); + +-- Snapshot table — captures the chunkset hash at every successful push. +CREATE TABLE IF NOT EXISTS chunk_pushes ( + id TEXT PRIMARY KEY, + document_id TEXT NOT NULL REFERENCES documents(id) ON DELETE CASCADE, + store_id TEXT NOT NULL REFERENCES stores(id) ON DELETE CASCADE, + chunkset_hash TEXT NOT NULL, + chunk_ids TEXT NOT NULL, -- JSON array of chunk ids in order at push time + pushed_at TEXT NOT NULL +); +CREATE INDEX IF NOT EXISTS idx_chunk_pushes_doc_store ON chunk_pushes(document_id, store_id); +``` + +Note: `chunk_pushes.chunk_ids` materialises the chunkset at push time. Combined with the immutable `chunk_edits` history, we can reconstruct any past chunkset by replay (slower) — but `chunk_ids` is the cheap path. + +### 5.3 Infra adapters + +None. + +### 5.4 Services + +`document-parser/services/chunk_editing_service.py`: + +```python +class ChunkEditingService: + def __init__(self, chunks: ChunkRepository, edits: ChunkEditRepository, ...): ... + + async def insert(self, doc_id, at_position, payload, *, actor, reason=None) -> Chunk: ... + async def update(self, doc_id, chunk_id, payload, *, actor, reason=None) -> Chunk: ... + async def delete(self, doc_id, chunk_id, *, actor, reason=None) -> None: ... + async def merge(self, doc_id, chunk_ids, *, actor, reason=None) -> Chunk: ... + async def split(self, doc_id, chunk_id, at_offset, *, actor, reason=None) -> tuple[Chunk, Chunk]: ... + + async def history(self, doc_id) -> list[ChunkEdit]: ... + async def chunkset_at(self, doc_id, push_id) -> list[Chunk]: ... +``` + +Every mutating method runs in a single SQL transaction: +1. Apply the domain operation. +2. Persist chunk rows (insert / update / soft-delete). +3. Insert `chunk_edits` row with full before/after snapshot. +4. Trigger #204's stale detection on the doc. + +`history()` — paginated (default 50, max 500), ordered `at DESC`. + +`chunkset_at(push_id)` — read `chunk_pushes.chunk_ids`, return the matching `chunks` rows in that order. + +`IngestionService.ingest()` (touched in #203) writes a row to `chunk_pushes` on success. + +### 5.5 API + +New router `document-parser/api/chunks.py`: + +``` +GET /api/documents/{id}/chunks List current chunks +GET /api/documents/{id}/chunks?at= Snapshot at push +GET /api/documents/{id}/chunks/history Edit timeline (paginated) +POST /api/documents/{id}/chunks Insert chunk +PATCH /api/documents/{id}/chunks/{chunk_id} Update chunk +DELETE /api/documents/{id}/chunks/{chunk_id} Soft-delete chunk +POST /api/documents/{id}/chunks/merge { chunkIds: [...], reason? } → new chunk +POST /api/documents/{id}/chunks/{chunk_id}/split { atOffset, reason? } → two new chunks +``` + +DTOs (`schemas.py`, camelCase via alias): + +```python +class ChunkResponse(BaseModel): + id: str + sequence: int + text: str + headings: list[str] + sourcePage: int | None + tokenCount: int | None + bboxes: list[BboxDto] + docItems: list[str] + updatedAt: datetime + +class ChunkEditResponse(BaseModel): + id: str + chunkId: str | None + action: str + actor: str + at: datetime + before: dict | None + after: dict | None + parents: list[str] + children: list[str] + reason: str | None +``` + +### 5.6 Frontend — feature module + +Touched: `frontend/src/features/chunking/`. + +- `api.ts` — clients for the seven endpoints above. +- `store.ts` — Pinia store holds `chunks`, `history`, and a draft layer for optimistic updates with rollback on failure. +- `ui/` — primitives only in this issue (`ChunkRow.vue`, `ChunkActionsMenu.vue`); the full editor is built in #219 / #220 / #221 on top. +- `data-e2e` selectors named upfront: `chunk-row`, `chunk-actions-menu`, `chunk-action-merge`, etc. + +### 5.7 Cross-cutting + +- Feature flag: existing `chunking` flag (in `/api/health`) gates the editing endpoints. When `false`, the endpoints return `403`. +- i18n: `chunks.action.*`, `chunks.reason.placeholder` keys. +- Shared types: `DocumentChunk`, `ChunkEdit` re-exported from `shared/types.ts`. + +## 6. Alternatives considered + +### Alternative A — Keep chunks in `analysis_jobs.chunks_json`, version the JSON + +- **Summary:** Add a `version` column on `analysis_jobs` and a `chunk_edits` table referencing JSON paths inside `chunks_json`. +- **Why not:** No chunk identity → `before`/`after` lookups become regex on JSON. Splits and merges have nowhere to record lineage. Reviewers and customer support cannot navigate the audit. The architectural cost is paid sooner or later — pay it now while the data is small. + +### Alternative B — Use OpenSearch as the source of truth for chunks + +- **Summary:** Treat the OpenSearch document as the authoritative chunk; record audit metadata there. +- **Why not:** OpenSearch is a derived index. It is the *projection* of the corpus, not its source. Replacing OpenSearch (Pinecone, Qdrant) would lose history. Source of truth belongs in SQLite. + +### Alternative C — Event-sourcing only (no current-state table) + +- **Summary:** Drop `chunks` table; replay `chunk_edits` to compute current state. +- **Why not:** Replay cost on every read is unacceptable for chunks-editor latency and for #221's diff. Hybrid (current state + immutable history) is the standard pragmatic shape. + +## 7. API & data contract + +### Endpoints + +See §5.5 — eight new endpoints, all behind the existing `chunking` feature flag. + +### Persistence schema + +See §5.2. + +### Env vars / config + +| Name | Default | Allowed | Notes | +|------|---------|---------|-------| +| `CHUNK_HISTORY_PAGE_DEFAULT` | `50` | int 1–500 | default page size for history | +| `CHUNK_HISTORY_PAGE_MAX` | `500` | int | hard cap | + +### Breaking changes + +None. Existing chunks in `chunks_json` remain accessible via the legacy fallback in `ChunkRepository.list_for_doc()` until #206 finishes the migration. + +## 8. Risks & mitigations + +| Risk | Audit dimension | Likelihood | Impact | How we notice | Mitigation / rollback | +|------|-----------------|------------|--------|---------------|------------------------| +| Audit table growth on heavy editing customers | Performance | Medium | Medium | Slow `chunk_edits` queries | Indexed by (doc, at). Pagination on the API. Future: archival job. | +| Race between two parallel edits on the same chunk | DDD | Low | Medium | Lost update | All mutations go through `ChunkEditingService` which serialises via the SQL transaction; client sends `If-Match` with `updatedAt` for optimistic concurrency on `PATCH` / `DELETE` | +| Audit before/after reveals sensitive content if shared | Security | Medium | Medium | Audit export shared accidentally | The audit endpoint is gated by the same auth as chunk edits; export does not include audit by default; admin-only flag in a future release | +| Schema drift: `chunks_json` and `chunks` table diverge during migration window | DDD | Medium | High | Diff between the two on the same doc | #206 is the single source of truth; new edits write to `chunks` only; reads prefer `chunks` and fall back to `chunks_json` | + +## 9. Testing strategy + +### Backend — pytest + +- **Unit (domain):** + - `test_chunk_editing_pure.py` — insert / update / delete / merge / split on in-memory lists; properties verified (lengths, sequences, identity rules). +- **Persistence:** + - `test_chunk_repo.py` — round-trip, soft delete, ordering by sequence. + - `test_chunk_edit_repo.py` — write + read history; immutability (UPDATE on `chunk_edits` is rejected by a CHECK constraint or service-level guard). +- **Services / integration:** + - `test_chunk_editing_service_audit_atomicity.py` — failure mid-write rolls back both chunk and audit. + - `test_chunkset_at_snapshot.py` — `chunkset_at(push_id)` returns the right snapshot after edits. + - `test_legacy_fallback.py` — read chunks for a doc that has only `chunks_json` (pre-migration). + +### Frontend — Vitest + +- `features/chunking/api.test.ts` — eight endpoints round-trip. +- `features/chunking/store.test.ts` — optimistic update + rollback on API failure; history pagination. + +### E2E — Karate UI + +Out of scope here; the editor lands in #219 / #220 with E2E tags `@critical @ui`. + +### Manual QA + +1. Trigger an INSERT / UPDATE / DELETE / MERGE / SPLIT. +2. `GET /api/documents/{id}/chunks/history` shows the action with `before`/`after`. +3. After a successful push, `GET /api/documents/{id}/chunks?at=` returns the chunkset captured at push. + +## 10. Rollout & observability + +### Release branch + +`release/0.6.0`. + +### Feature flag + +The existing `chunking` flag (in `/api/health`) controls whether the editing endpoints accept writes. Reads (`GET`) are always allowed (consistent with the chunks tab being read-only when the flag is off). + +### Observability + +- Each mutation logs: `INFO event=chunk_edit doc_id= chunk_id= action= actor=`. +- Counter (Prometheus, future): `chunk_edits_total{action}`. + +### Rollback plan + +The migration is additive. Reverting the code leaves the new tables but no writers. Reads from the chunks tab fall back to the legacy `chunks_json`. Editing endpoints disappear (404). If a clean rollback is needed, drop the three new tables. + +## 11. Open questions + +- Should `actor` carry a structured object (id, name, role) or stay as a free-form string? **Decision for 0.6.0:** free-form string (`"system"` today; whatever auth provides later). Structured upgrade is non-breaking thanks to JSON-friendly columns. +- Should we generate a *human-readable* diff in the audit row (line-level), or only store before/after JSON? **Decision:** store JSON; render the diff client-side in #221. + +## 12. References + +- **Issue:** https://github.com/scub-france/Docling-Studio/issues/205 +- **Related issues:** #202 (lifecycle), #203 (per-store), #204 (auto-stale), #206 (migration), #219 (editor view), #220 (edit actions), #221 (visual diff), #222 (push) +- **ADRs:** ADR — "Chunks become a first-class persisted entity" +- **Project docs:** + - Architecture: `docs/architecture.md` + - Coding standards: `docs/architecture/coding-standards.md` + - ADR guide: `docs/architecture/adr-guide.md` diff --git a/docs/design/206-lifecycle-state-migration.md b/docs/design/206-lifecycle-state-migration.md new file mode 100644 index 0000000..b61ad01 --- /dev/null +++ b/docs/design/206-lifecycle-state-migration.md @@ -0,0 +1,265 @@ +# Design: Migrate existing documents to the new lifecycle state model + +- **Issue:** #206 +- **Title on issue:** [CHORE] Migrate existing documents to the new lifecycle state model +- **Author:** Pier-Jean Malandrino +- **Date:** 2026-04-29 +- **Status:** Accepted +- **Target milestone:** 0.6.0 — Doc-centric ingest +- **Impacted layers:** backend: persistence · services · infra (CLI) · frontend (none) +- **Audit dimensions likely touched:** CI/Build · Tests · Documentation · Security +- **ADR spawned?:** no + +--- + +## 1. Problem + +#202, #203, and #205 introduce four new tables / columns: `documents.lifecycle_state`, `stores`, `document_store_links`, `chunks`, `chunk_edits`, `chunk_pushes`. Existing tenants already have documents, analysis jobs, and chunks living in `analysis_jobs.chunks_json` plus rows in OpenSearch. After deploy, these documents must appear in `/docs` with sensible state — otherwise the page looks broken (every doc shows `Uploaded` or unknown). + +This issue ships an idempotent migration that backfills: +- `documents.lifecycle_state` and `lifecycle_state_at`. +- One `stores` row (the `default` seeded by #203, no-op if already present). +- One `document_store_links` row per document already represented in OpenSearch under the legacy index. +- `chunks` rows materialized from existing `analysis_jobs.chunks_json`. +- `chunk_pushes` rows reconstructed for documents currently indexed. + +It also reindexes from `docling-studio-chunks` to `docling-studio-default` if needed, so the new store-aware code can read them. + +## 2. Goals + +- [ ] Idempotent CLI command `python -m document_parser.tools.migrate_06` (re-runs safely; second run is a no-op). +- [ ] `--dry-run` flag prints what would change without writing. +- [ ] Inference rules below produce a sensible state for every existing document. +- [ ] Per-tenant counts logged at the end (parsed=N, chunked=N, ingested=N, failed=N). +- [ ] Documented in `docs/runbooks/release-0.6.0-migration.md`. +- [ ] Tested on a snapshot DB representing the three relevant pre-states (no analysis, completed analysis, indexed in OpenSearch). + +## 3. Non-goals + +- Re-running ingestion. The migration only touches metadata; it does not re-embed. +- A web UI for migration progress — operator-only, run via CLI. +- Migrating tenants on environments that have already adopted parts of the new schema (e.g. a partial pre-release deploy) — out of scope; an emergency hotfix path is documented separately. +- Moving to a new vector backend — out of scope. + +## 4. Context & constraints + +### Existing code surface + +- `document-parser/persistence/database.py` — `_run_migrations()` runs schema DDL on app start. +- `document-parser/persistence/document_repo.py`, `analysis_repo.py` — read paths. +- `document-parser/infra/opensearch_store.py` — read access to existing OpenSearch index. +- New repos from #203 / #205: `store_repo.py`, `document_store_link_repo.py`, `chunk_repo.py`, `chunk_edit_repo.py`. + +### Hexagonal Architecture constraints + +- The migration is a **CLI script** in `document-parser/tools/migrate_06.py`. It uses repositories only — never raw SQL outside of repo modules. This keeps the upgrade path testable like any service. +- The reindex helper (`docling-studio-chunks → docling-studio-default`) lives in `infra/opensearch_store.py` as a method, called by the script. + +### Hard constraints + +- Idempotent: re-running on an already-migrated DB makes zero writes. +- Resumable: a crash mid-run can be restarted; checkpoints stored in a tiny `migration_progress` table (rows: `(name, completed_at)`). +- No assumption about uptime: the script must work on a quiesced DB OR a live one (transactions short, single-row scope). +- Read-only on OpenSearch by default; write happens only when the operator explicitly passes `--reindex-default-store`. + +### Deployment modes + +Run once per environment (`local`, `staging`, `prod`). HF Space deployments — same script, run by the deploy step before the new image starts serving traffic. + +## 5. Proposed design + +### 5.1 Domain + +No new domain code. The migration is a coordinator over existing repos. + +### 5.2 Persistence + +The schema additions ship in #202 / #203 / #205 migrations (already applied at app start). This issue contributes: + +```sql +CREATE TABLE IF NOT EXISTS migration_progress ( + name TEXT PRIMARY KEY, + completed_at TEXT NOT NULL +); +``` + +This is the only new table. It records each migration step's completion to enable resumability. + +### 5.3 Infra adapters + +`OpenSearchStore.copy_legacy_to_default(*, dry_run: bool) -> int` — +1. If `docling-studio-chunks` exists and `docling-studio-default` does not, create the new index with the same mapping. +2. Reindex via `_reindex_op` (OpenSearch reindex API) from legacy → default. +3. Add a read-only alias `docling-studio-chunks → docling-studio-default`. +4. Returns the number of documents copied. + +### 5.4 Services + +`document-parser/services/migration_06_service.py` — orchestrator with one method per step. Each step is bracketed by: + +```python +if not progress.is_done(step_name): + do_step(...) + progress.mark_done(step_name) +``` + +Steps: + +1. **`seed_default_store`** — `INSERT OR IGNORE INTO stores (...) VALUES ('default', ...)`. (Already in #203's migration; this step is a guard for older deploys that skipped the seed.) +2. **`backfill_document_lifecycle_state`** — for each document, infer state: + - Has at least one `analysis_jobs` row with `status=COMPLETED` and a non-null `chunks_json` → `Chunked` (refined below). + - Has at least one row indexed in OpenSearch (legacy or default index) → `Ingested`. + - Has only `analysis_jobs` rows with `status=COMPLETED` and no `chunks_json` → `Parsed`. + - Has only `analysis_jobs` rows with `status=FAILED` → `Failed`. + - Else → `Uploaded`. +3. **`materialize_chunks_from_chunks_json`** — for each `analysis_jobs` row with non-null `chunks_json`, parse and insert rows into `chunks` table. Stable id derivation: `f"chunk-{document_id}-{sequence:05d}-{sha256(text)[:8]}"` so re-runs are deterministic. +4. **`backfill_links_from_opensearch`** — for each document, query OpenSearch (default index, then legacy as fallback) for the count of indexed chunks. If non-zero: + - Insert `document_store_links` row (state = `Ingested`). + - Compute the chunkset hash via #204's function over the materialized `chunks`. + - Set `link.chunkset_hash`. + - Insert a `chunk_pushes` row with the materialized chunk ids (best-effort: ordered by sequence). +5. **`reaggregate_document_lifecycle`** — recompute the doc state by combining the link states (#203's aggregation rule) and update `documents.lifecycle_state` accordingly. This may upgrade or downgrade the value set in step 2 (e.g. `Chunked → Ingested`). +6. **`copy_legacy_index`** *(optional, behind `--reindex-default-store`)* — call `OpenSearchStore.copy_legacy_to_default()`. + +Each step is wrapped in its own transaction; partial failure leaves earlier steps committed. + +### 5.5 API + +None. CLI only. + +### 5.6 Frontend + +None. + +### 5.7 Cross-cutting + +- Logging: structured at `INFO` per step, at `ERROR` per failed item with `doc_id`. End summary printed to stdout in a fixed table. +- Configuration via CLI flags only (no env vars introduced for this script): + +``` +python -m document_parser.tools.migrate_06 \ + [--dry-run] \ + [--reindex-default-store] \ + [--limit N] \ + [--only-step ] +``` + +- Documentation: `docs/runbooks/release-0.6.0-migration.md` describes the operator flow (backup → run dry-run → run real → validate via `/api/documents`). + +## 6. Alternatives considered + +### Alternative A — Schema migration applies everything at app boot + +- **Summary:** Embed inference logic in `_run_migrations()` so the app starts and self-heals. +- **Why not:** Migration is observable and debuggable as a CLI; baking it into boot time risks slow startup and silent failures. Operators want to run it during a maintenance window with a dry-run first. + +### Alternative B — Migrate on demand (lazy) + +- **Summary:** Add a "needs migration" check at runtime, materialize chunks for a doc only on first read. +- **Why not:** Surfaces the partial state in the API (`stores: []` for unmigrated docs even if they are indexed). The library page (#211) becomes a half-true representation of reality. Eager migration is simpler. + +### Alternative C — Run only the schema; let users re-ingest manually + +- **Summary:** Skip backfill; users re-trigger ingestion from the UI as needed. +- **Why not:** A tenant with 10k docs cannot click 10k re-ingest buttons. The killer flow promises "your existing corpus already has state". + +## 7. API & data contract + +No API changes. + +### Persistence schema + +See §5.2 (one new table: `migration_progress`). + +### CLI + +``` +python -m document_parser.tools.migrate_06 [flags] +``` + +All flags are documented in `--help`. + +### Breaking changes + +None. + +## 8. Risks & mitigations + +| Risk | Audit dimension | Likelihood | Impact | How we notice | Mitigation / rollback | +|------|-----------------|------------|--------|---------------|------------------------| +| Migration crashes mid-run | CI/Build | Medium | Medium | Error logs | Resumable via `migration_progress`; re-run picks up where it left off | +| Wrong inference for a doc edge case (e.g. multiple stores already) | DDD | Low | Medium | Operator validation (sample 10 docs by hand) | Dry-run lists changes; operator can `--only-step` to redo a single step | +| OpenSearch reindex copies bad data | Decoupling | Low | High | Diff in document counts | Reindex is opt-in (`--reindex-default-store`); operator runs it deliberately; alias keeps legacy reads working | +| Hash mismatch after migration (chunks materialized differ from what was indexed) | DDD | Medium | Medium | Newly-migrated link shows `Stale` immediately | Acceptable behaviour: it correctly tells the user "your indexed chunks may not match the current source"; operator can choose to re-ingest or leave as-is | +| Missing analysis-job rows for a doc | Tests | Low | Low | Doc shows `Uploaded` despite being indexed | Inference falls back to OpenSearch presence; if both empty, `Uploaded` is correct | + +## 9. Testing strategy + +### Backend — pytest + +- **Unit (services):** `test_migration_inference.py` — table-driven: every `(analysis_state, has_chunks_json, indexed)` tuple → expected `lifecycle_state`. +- **Integration:** `test_migration_idempotency.py` — run twice → second run zero writes; mid-run abort + restart → final state matches one-shot run. +- **Persistence:** `test_chunks_materialization.py` — `chunks_json` → rows; ids deterministic (re-running produces same ids). + +### Snapshot fixture + +`document-parser/tests/fixtures/db_pre_06.sqlite` — handcrafted SQLite DB with three documents (uploaded only, completed analysis, indexed in OpenSearch via fake adapter). Migration runs against it and the resulting state is asserted. + +### Manual QA + +1. Snapshot prod DB. +2. Run `--dry-run` on the snapshot → review the printed plan. +3. Run for real on the snapshot → validate via `/api/documents` that every doc has a sensible `lifecycleState` and (where applicable) `stores`. +4. Run the same against prod during the maintenance window. + +### Performance + +The script is O(N_docs + N_chunks). For 10k docs / 500k chunks: target < 5 minutes on a single-node SQLite. No parallelism needed in 0.6.0. + +## 10. Rollout & observability + +### Release branch + +`release/0.6.0`. The migration ships in the same release as #202 / #203 / #204 / #205 — operators run it after deploying the new code and before sending traffic. + +### Feature flag + +None. + +### Observability + +- Stdout summary table at the end: + + ``` + step wrote skipped + seed_default_store 0 1 + backfill_document_lifecycle_state 87 0 + materialize_chunks_from_chunks_json 14502 0 + backfill_links_from_opensearch 73 14 + reaggregate_document_lifecycle 73 0 + copy_legacy_index — — + total 14735 15 + ``` + +- Logs: per-step start / finish; per-error row. + +### Rollback plan + +- `migration_progress` rows can be deleted to force re-run a specific step. +- The new tables can be truncated or dropped (data is recoverable from `analysis_jobs.chunks_json` and OpenSearch). +- Reverting application code: the new tables stay populated but unused; safe. + +## 11. Open questions + +- Should the script open a *long* SQLite transaction or many small ones? **Decision:** many small (per-doc), to keep the DB writeable for the live app if the operator chooses to run the script while traffic is on. +- Hash mismatch on freshly-migrated docs (chunks materialized may differ from what was indexed) — should we *force-mark* them `Stale`, or trust the hash compare to do it implicitly? **Decision:** trust the compare. The result is identical and the path is uniform. + +## 12. References + +- **Issue:** https://github.com/scub-france/Docling-Studio/issues/206 +- **Related issues:** #202 (lifecycle), #203 (per-store), #204 (hash), #205 (audit + chunks table) +- **ADRs:** none planned +- **Project docs:** + - Architecture: `docs/architecture.md` + - Coding standards: `docs/architecture/coding-standards.md` + - Operations playbooks: `docs/operations/` From ff0b795b6da856d815dec42209bd908411309891 Mon Sep 17 00:00:00 2001 From: Pier-Jean Malandrino Date: Wed, 29 Apr 2026 15:34:25 +0200 Subject: [PATCH 03/35] feat(#202): introduce Document lifecycle state machine MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds a first-class lifecycle state to every document, distinct from AnalysisJob.status. The lifecycle describes the document as a whole and is the foundation for the doc-centric pivot in 0.6.0. Domain - DocumentLifecycleState enum (Uploaded/Parsed/Chunked/Ingested/Stale/Failed) - Document.lifecycle_state and lifecycle_state_at fields - Document.transition_to() validates against a transition table (domain/lifecycle.py) and returns a DocumentLifecycleChanged event - InvalidLifecycleTransitionError on disallowed transitions Persistence - ALTER TABLE documents to add the two columns (default 'Uploaded') - New index idx_documents_lifecycle_state for filter perf - _COLUMN_MIGRATIONS refactored to support multiple tables - _POST_MIGRATION_DDL list for indexes on freshly-added columns - SqliteDocumentRepository.update_lifecycle() Services - AnalysisService drives transitions on parse / chunk / re-chunk / fail via _transition_document(); idempotent and resilient (logs WARN and continues if a stale state is somehow encountered) API - DocumentResponse exposes lifecycleState + lifecycleStateAt (additive — existing 'status' field kept for backwards compat) Frontend - Document type extended with lifecycleState and lifecycleStateAt - DocumentLifecycleState union literal mirroring the backend enum Tests - 24 new tests in test_lifecycle.py covering transitions, idempotency, invariant preservation, and event emission - test_repos.py: round-trip + every-enum-value check + update_lifecycle - test_chunking.py: rechunk path now mocks document_repo correctly Refs #202 --- document-parser/api/documents.py | 2 + document-parser/api/schemas.py | 5 + document-parser/domain/events.py | 35 +++++ document-parser/domain/exceptions.py | 37 ++++++ document-parser/domain/lifecycle.py | 83 ++++++++++++ document-parser/domain/models.py | 34 +++++ document-parser/domain/ports.py | 10 ++ document-parser/domain/value_objects.py | 25 ++++ document-parser/persistence/database.py | 59 +++++++-- document-parser/persistence/document_repo.py | 48 ++++++- document-parser/services/analysis_service.py | 51 ++++++++ document-parser/tests/test_chunking.py | 9 +- document-parser/tests/test_lifecycle.py | 129 +++++++++++++++++++ document-parser/tests/test_repos.py | 43 ++++++- frontend/src/shared/types.ts | 23 ++++ 15 files changed, 578 insertions(+), 15 deletions(-) create mode 100644 document-parser/domain/events.py create mode 100644 document-parser/domain/exceptions.py create mode 100644 document-parser/domain/lifecycle.py create mode 100644 document-parser/tests/test_lifecycle.py diff --git a/document-parser/api/documents.py b/document-parser/api/documents.py index 1289166..55f2c8e 100644 --- a/document-parser/api/documents.py +++ b/document-parser/api/documents.py @@ -34,6 +34,8 @@ def _to_response(doc) -> DocumentResponse: file_size=doc.file_size, page_count=doc.page_count, created_at=str(doc.created_at), + lifecycle_state=doc.lifecycle_state.value, + lifecycle_state_at=(str(doc.lifecycle_state_at) if doc.lifecycle_state_at else None), ) diff --git a/document-parser/api/schemas.py b/document-parser/api/schemas.py index 7b7d82e..707e146 100644 --- a/document-parser/api/schemas.py +++ b/document-parser/api/schemas.py @@ -56,6 +56,11 @@ class DocumentResponse(_CamelModel): file_size: int | None = None page_count: int | None = None created_at: str | datetime + # 0.6.0 — Document lifecycle state machine (#202). The lifecycle + # describes the document as a whole; `status` above is kept for + # backwards compat and currently still maps to `DOCUMENT_STATUS_UPLOADED`. + lifecycle_state: str = "Uploaded" + lifecycle_state_at: str | datetime | None = None class AnalysisResponse(_CamelModel): diff --git a/document-parser/domain/events.py b/document-parser/domain/events.py new file mode 100644 index 0000000..0b8c21d --- /dev/null +++ b/document-parser/domain/events.py @@ -0,0 +1,35 @@ +"""Domain events — frozen records that document state transitions. + +Events are produced by domain operations (typically returned from a +mutation method on an aggregate). They are pure data — no event bus is +wired in 0.6.0; services can choose to log, persist, or publish them +later. Keeping them here keeps the domain layer free of any event-bus +infrastructure. +""" + +from __future__ import annotations + +from dataclasses import dataclass +from typing import TYPE_CHECKING + +if TYPE_CHECKING: + from datetime import datetime + + from domain.value_objects import DocumentLifecycleState + + +@dataclass(frozen=True) +class DocumentLifecycleChanged: + """A Document lifecycle transition occurred. + + Attributes: + document_id: id of the document that transitioned. + previous: state the document was in before the transition. + current: state the document is in after the transition. + at: timestamp of the transition (UTC). + """ + + document_id: str + previous: DocumentLifecycleState + current: DocumentLifecycleState + at: datetime diff --git a/document-parser/domain/exceptions.py b/document-parser/domain/exceptions.py new file mode 100644 index 0000000..ce6d61b --- /dev/null +++ b/document-parser/domain/exceptions.py @@ -0,0 +1,37 @@ +"""Domain-level exceptions. + +Exceptions defined in this module are raised by domain operations and value +objects when an invariant is violated. They have no infrastructure +dependencies and are safe to import from any layer. +""" + +from __future__ import annotations + +from typing import TYPE_CHECKING + +if TYPE_CHECKING: + from domain.value_objects import DocumentLifecycleState + + +class DomainError(Exception): + """Base class for domain-level errors. Catch this when wiring API + layers if you want a single hook for any invariant violation.""" + + +class InvalidLifecycleTransitionError(DomainError): + """Raised when a Document.transition_to() call asks for a (source, + target) pair that is not in the allowed transition table. + + Carries `source` and `target` so callers can produce a useful error + message without re-discovering them. + """ + + def __init__( + self, + *, + source: DocumentLifecycleState, + target: DocumentLifecycleState, + ) -> None: + super().__init__(f"Invalid document lifecycle transition: {source.value} -> {target.value}") + self.source = source + self.target = target diff --git a/document-parser/domain/lifecycle.py b/document-parser/domain/lifecycle.py new file mode 100644 index 0000000..e2a0eb3 --- /dev/null +++ b/document-parser/domain/lifecycle.py @@ -0,0 +1,83 @@ +"""Document lifecycle state machine — pure domain logic. + +Defines the canonical state model for a Document in Docling Studio: + Uploaded → Parsed → Chunked → Ingested → (Stale|Chunked) → Ingested + +Stale is set by the auto-detect logic (#204) — never reached by a manual +action. Failed is reachable from any state and represents a pipeline error. + +This module contains: + - The transition table (which (from → to) pairs are allowed). + - `is_allowed_transition`: the pure check. + - `assert_transition`: raises `InvalidLifecycleTransitionError` when not allowed. + +The dataclass that carries the state lives in `domain.models.Document`. The +domain event emitted on transition lives in `domain.events`. +""" + +from __future__ import annotations + +from domain.exceptions import InvalidLifecycleTransitionError +from domain.value_objects import DocumentLifecycleState + +# Allowed transitions: from → set of allowed targets. +# Failed is always reachable as a terminal-ish state (see notes below). +# Self-loops are allowed only where they make pipeline sense (e.g. re-chunk). +_TRANSITIONS: dict[DocumentLifecycleState, frozenset[DocumentLifecycleState]] = { + DocumentLifecycleState.UPLOADED: frozenset( + { + DocumentLifecycleState.PARSED, + DocumentLifecycleState.CHUNKED, # parse + chunk in one pipeline call + DocumentLifecycleState.FAILED, + } + ), + DocumentLifecycleState.PARSED: frozenset( + { + DocumentLifecycleState.PARSED, # idempotent re-parse + DocumentLifecycleState.CHUNKED, + DocumentLifecycleState.FAILED, + } + ), + DocumentLifecycleState.CHUNKED: frozenset( + { + DocumentLifecycleState.CHUNKED, # re-chunk + DocumentLifecycleState.INGESTED, + DocumentLifecycleState.FAILED, + } + ), + DocumentLifecycleState.INGESTED: frozenset( + { + DocumentLifecycleState.STALE, + DocumentLifecycleState.CHUNKED, # re-chunk after ingest + DocumentLifecycleState.INGESTED, # re-push (idempotent) + DocumentLifecycleState.FAILED, + } + ), + DocumentLifecycleState.STALE: frozenset( + { + DocumentLifecycleState.INGESTED, + DocumentLifecycleState.CHUNKED, + DocumentLifecycleState.FAILED, + } + ), + # Failed is a recoverable state — the operator (or a retry) can move + # back to a non-terminal state by re-running the relevant pipeline step. + DocumentLifecycleState.FAILED: frozenset( + { + DocumentLifecycleState.UPLOADED, + DocumentLifecycleState.PARSED, + DocumentLifecycleState.CHUNKED, + } + ), +} + + +def is_allowed_transition(source: DocumentLifecycleState, target: DocumentLifecycleState) -> bool: + """Return True iff transitioning from `source` to `target` is allowed.""" + return target in _TRANSITIONS.get(source, frozenset()) + + +def assert_transition(source: DocumentLifecycleState, target: DocumentLifecycleState) -> None: + """Raise `InvalidLifecycleTransitionError` if the transition is not allowed.""" + if not is_allowed_transition(source, target): + raise InvalidLifecycleTransitionError(source=source, target=target) diff --git a/document-parser/domain/models.py b/document-parser/domain/models.py index e89883d..0a371ee 100644 --- a/document-parser/domain/models.py +++ b/document-parser/domain/models.py @@ -7,6 +7,10 @@ import uuid from dataclasses import dataclass, field from datetime import UTC, datetime +from domain.events import DocumentLifecycleChanged +from domain.lifecycle import assert_transition +from domain.value_objects import DocumentLifecycleState + class AnalysisStatus(enum.StrEnum): PENDING = "PENDING" @@ -32,6 +36,36 @@ class Document: page_count: int | None = None storage_path: str = "" created_at: datetime = field(default_factory=_utcnow) + lifecycle_state: DocumentLifecycleState = DocumentLifecycleState.UPLOADED + lifecycle_state_at: datetime | None = None + + def transition_to( + self, + target: DocumentLifecycleState, + *, + now: datetime | None = None, + ) -> DocumentLifecycleChanged: + """Move the document to `target`, validating the transition. + + Returns the corresponding `DocumentLifecycleChanged` event so the + caller (typically a service) can log / persist / publish it. The + event is pure data — no event bus is wired in 0.6.0. + + Raises: + InvalidLifecycleTransitionError: if (current → target) is not in + the allowed transition table. + """ + previous = self.lifecycle_state + assert_transition(previous, target) + at = now or _utcnow() + self.lifecycle_state = target + self.lifecycle_state_at = at + return DocumentLifecycleChanged( + document_id=self.id, + previous=previous, + current=target, + at=at, + ) @dataclass diff --git a/document-parser/domain/ports.py b/document-parser/domain/ports.py index 9431b21..967871d 100644 --- a/document-parser/domain/ports.py +++ b/document-parser/domain/ports.py @@ -9,12 +9,15 @@ from __future__ import annotations from typing import TYPE_CHECKING, Protocol, runtime_checkable if TYPE_CHECKING: + from datetime import datetime + from domain.models import AnalysisJob, Document from domain.value_objects import ( ChunkingOptions, ChunkResult, ConversionOptions, ConversionResult, + DocumentLifecycleState, LLMProviderType, ReasoningResult, ) @@ -85,6 +88,13 @@ class DocumentRepository(Protocol): async def update_page_count(self, doc_id: str, page_count: int) -> None: ... + async def update_lifecycle( + self, + doc_id: str, + state: DocumentLifecycleState, + at: datetime, + ) -> None: ... + async def delete(self, doc_id: str) -> bool: ... diff --git a/document-parser/domain/value_objects.py b/document-parser/domain/value_objects.py index 72132f5..784dd2c 100644 --- a/document-parser/domain/value_objects.py +++ b/document-parser/domain/value_objects.py @@ -14,6 +14,31 @@ DEFAULT_PAGE_WIDTH: float = 612.0 DEFAULT_PAGE_HEIGHT: float = 792.0 +class DocumentLifecycleState(StrEnum): + """Canonical lifecycle of a Document in Docling Studio. + + Distinct from `AnalysisStatus` (which describes a single conversion + attempt). The lifecycle describes the document as a whole: + + Uploaded raw file persisted, no parse yet + Parsed conversion produced a document tree + Chunked chunker produced a draft chunkset (pre-store) + Ingested chunkset has been embedded into at least one store + Stale a chunkset was edited after a successful push and the + corresponding store no longer matches (#204) + Failed a pipeline step failed; recoverable by retry + + Allowed transitions live in `domain.lifecycle._TRANSITIONS`. + """ + + UPLOADED = "Uploaded" + PARSED = "Parsed" + CHUNKED = "Chunked" + INGESTED = "Ingested" + STALE = "Stale" + FAILED = "Failed" + + @dataclass(frozen=True) class PageElement: type: str diff --git a/document-parser/persistence/database.py b/document-parser/persistence/database.py index c56620e..b79f5db 100644 --- a/document-parser/persistence/database.py +++ b/document-parser/persistence/database.py @@ -45,22 +45,59 @@ CREATE INDEX IF NOT EXISTS idx_documents_created_at ON documents(created_at); """ -_MIGRATIONS = [ - ("document_json", "ALTER TABLE analysis_jobs ADD COLUMN document_json TEXT"), - ("chunks_json", "ALTER TABLE analysis_jobs ADD COLUMN chunks_json TEXT"), - ("progress_current", "ALTER TABLE analysis_jobs ADD COLUMN progress_current INTEGER"), - ("progress_total", "ALTER TABLE analysis_jobs ADD COLUMN progress_total INTEGER"), +# Column migrations: (table, column_name, ddl). Idempotent — applied only +# when the column is missing from the live schema. Order matters when a +# later migration depends on an earlier one (none today). +_COLUMN_MIGRATIONS: list[tuple[str, str, str]] = [ + ("analysis_jobs", "document_json", "ALTER TABLE analysis_jobs ADD COLUMN document_json TEXT"), + ("analysis_jobs", "chunks_json", "ALTER TABLE analysis_jobs ADD COLUMN chunks_json TEXT"), + ( + "analysis_jobs", + "progress_current", + "ALTER TABLE analysis_jobs ADD COLUMN progress_current INTEGER", + ), + ( + "analysis_jobs", + "progress_total", + "ALTER TABLE analysis_jobs ADD COLUMN progress_total INTEGER", + ), + # 0.6.0 — Document lifecycle state machine (#202). + ( + "documents", + "lifecycle_state", + "ALTER TABLE documents ADD COLUMN lifecycle_state TEXT NOT NULL DEFAULT 'Uploaded'", + ), + ( + "documents", + "lifecycle_state_at", + "ALTER TABLE documents ADD COLUMN lifecycle_state_at TEXT", + ), +] + +# DDL statements run after column migrations — typically CREATE INDEX +# IF NOT EXISTS for indexes on freshly-added columns. +_POST_MIGRATION_DDL: list[str] = [ + "CREATE INDEX IF NOT EXISTS idx_documents_lifecycle_state ON documents(lifecycle_state)", ] async def _run_migrations(db: aiosqlite.Connection) -> None: - """Add columns that may be missing in older databases.""" - cursor = await db.execute("PRAGMA table_info(analysis_jobs)") - existing = {row[1] for row in await cursor.fetchall()} - for col_name, ddl in _MIGRATIONS: - if col_name not in existing: + """Apply additive column migrations, then any post-migration DDL. + + Existing columns are detected via PRAGMA `table_info`, so re-running + the migration on an already-up-to-date DB is a no-op. + """ + columns_by_table: dict[str, set[str]] = {} + for table, col_name, ddl in _COLUMN_MIGRATIONS: + if table not in columns_by_table: + cursor = await db.execute(f"PRAGMA table_info({table})") + columns_by_table[table] = {row[1] for row in await cursor.fetchall()} + if col_name not in columns_by_table[table]: await db.execute(ddl) - logger.info("Migration: added column %s to analysis_jobs", col_name) + columns_by_table[table].add(col_name) + logger.info("Migration: added column %s to %s", col_name, table) + for ddl in _POST_MIGRATION_DDL: + await db.execute(ddl) await db.commit() diff --git a/document-parser/persistence/document_repo.py b/document-parser/persistence/document_repo.py index beb81c5..22f64b9 100644 --- a/document-parser/persistence/document_repo.py +++ b/document-parser/persistence/document_repo.py @@ -5,15 +5,34 @@ from __future__ import annotations from datetime import UTC, datetime from domain.models import Document +from domain.value_objects import DocumentLifecycleState from persistence.database import get_connection +def _parse_iso(value: str | None) -> datetime | None: + if value is None or value == "": + return None + parsed = datetime.fromisoformat(value) + if parsed.tzinfo is None: + parsed = parsed.replace(tzinfo=UTC) + return parsed + + def _row_to_document(row) -> Document: created = row["created_at"] if isinstance(created, str): created = datetime.fromisoformat(created) if created.tzinfo is None: created = created.replace(tzinfo=UTC) + # `_run_migrations` guarantees both lifecycle columns exist by the time + # any read happens, so direct access is safe. + lifecycle_value = row["lifecycle_state"] + lifecycle_state = ( + DocumentLifecycleState(lifecycle_value) + if lifecycle_value + else DocumentLifecycleState.UPLOADED + ) + lifecycle_state_at = _parse_iso(row["lifecycle_state_at"]) return Document( id=row["id"], filename=row["filename"], @@ -22,6 +41,8 @@ def _row_to_document(row) -> Document: page_count=row["page_count"], storage_path=row["storage_path"], created_at=created, + lifecycle_state=lifecycle_state, + lifecycle_state_at=lifecycle_state_at, ) @@ -32,8 +53,10 @@ class SqliteDocumentRepository: """Persist a new document record.""" async with get_connection() as db: await db.execute( - """INSERT INTO documents (id, filename, content_type, file_size, page_count, storage_path, created_at) - VALUES (?, ?, ?, ?, ?, ?, ?)""", + """INSERT INTO documents ( + id, filename, content_type, file_size, page_count, + storage_path, created_at, lifecycle_state, lifecycle_state_at + ) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?)""", ( doc.id, doc.filename, @@ -42,6 +65,8 @@ class SqliteDocumentRepository: doc.page_count, doc.storage_path, str(doc.created_at), + doc.lifecycle_state.value, + str(doc.lifecycle_state_at) if doc.lifecycle_state_at else None, ), ) await db.commit() @@ -72,6 +97,25 @@ class SqliteDocumentRepository: ) await db.commit() + async def update_lifecycle( + self, + doc_id: str, + state: DocumentLifecycleState, + at: datetime, + ) -> None: + """Persist a lifecycle state transition for a document. + + Callers must have already validated the transition via + `Document.transition_to()` — this method does not check the + transition graph; it just writes. + """ + async with get_connection() as db: + await db.execute( + "UPDATE documents SET lifecycle_state = ?, lifecycle_state_at = ? WHERE id = ?", + (state.value, str(at), doc_id), + ) + await db.commit() + async def delete(self, doc_id: str) -> bool: """Delete a document by ID. Returns True if a row was removed.""" async with get_connection() as db: diff --git a/document-parser/services/analysis_service.py b/document-parser/services/analysis_service.py index 1c6030f..04ad7dd 100644 --- a/document-parser/services/analysis_service.py +++ b/document-parser/services/analysis_service.py @@ -16,6 +16,7 @@ from typing import TYPE_CHECKING import pypdfium2 as pdfium +from domain.exceptions import InvalidLifecycleTransitionError from domain.models import AnalysisJob, AnalysisStatus from domain.services import classify_error, merge_results from domain.value_objects import ( @@ -23,6 +24,7 @@ from domain.value_objects import ( ChunkResult, ConversionOptions, ConversionResult, + DocumentLifecycleState, ) if TYPE_CHECKING: @@ -162,6 +164,10 @@ class AnalysisService: chunks_json = json.dumps([_chunk_to_dict(c) for c in chunks]) await self._analysis_repo.update_chunks(job_id, chunks_json) + # Re-chunk drives the document into Chunked (idempotent if already + # Chunked; #204 will mark per-store links Stale separately). + await self._transition_document(job.document_id, DocumentLifecycleState.CHUNKED) + return chunks async def update_chunk_text(self, job_id: str, chunk_index: int, text: str) -> list[dict]: @@ -292,11 +298,47 @@ class AnalysisService: if job: job.mark_failed(error) await self._analysis_repo.update_status(job) + await self._transition_document(job.document_id, DocumentLifecycleState.FAILED) except OSError: logger.exception("Database I/O error marking job %s as failed", job_id) except Exception: logger.exception("Unexpected error marking job %s as failed", job_id) + async def _transition_document( + self, + document_id: str, + target: DocumentLifecycleState, + ) -> None: + """Drive a Document lifecycle transition (#202). + + Idempotent on the target — if the document is already in the + requested state, no write happens. Invalid transitions are + logged at WARNING and swallowed so a lifecycle hiccup does not + crash an otherwise-successful pipeline run. + """ + doc = await self._document_repo.find_by_id(document_id) + if doc is None: + return + if doc.lifecycle_state == target: + return + try: + event = doc.transition_to(target) + except InvalidLifecycleTransitionError: + logger.warning( + "Skipped invalid lifecycle transition for doc %s: %s -> %s", + document_id, + doc.lifecycle_state.value, + target.value, + ) + return + await self._document_repo.update_lifecycle(document_id, event.current, event.at) + logger.info( + "lifecycle_changed doc_id=%s from=%s to=%s", + document_id, + event.previous.value, + event.current.value, + ) + async def _run_analysis( self, job_id: str, @@ -379,6 +421,15 @@ class AnalysisService: if result.page_count: await self._document_repo.update_page_count(job.document_id, result.page_count) + # Drive the document lifecycle (#202): chunks present → Chunked, + # otherwise → Parsed. + target_state = ( + DocumentLifecycleState.CHUNKED + if chunks_json is not None + else DocumentLifecycleState.PARSED + ) + await self._transition_document(job.document_id, target_state) + await self._write_tree_to_neo4j(job, result.document_json) logger.info("Analysis completed: %s (%d pages)", job_id, result.page_count) diff --git a/document-parser/tests/test_chunking.py b/document-parser/tests/test_chunking.py index 5b37b7e..5f6bd2d 100644 --- a/document-parser/tests/test_chunking.py +++ b/document-parser/tests/test_chunking.py @@ -10,7 +10,7 @@ import pytest from fastapi.testclient import TestClient from api.schemas import ChunkBboxResponse, ChunkingOptionsRequest, ChunkResponse, RechunkRequest -from domain.models import AnalysisJob, AnalysisStatus +from domain.models import AnalysisJob, AnalysisStatus, Document from domain.value_objects import ChunkBbox, ChunkingOptions, ChunkResult from main import app @@ -529,6 +529,13 @@ class TestRemoteChunkingPath: ) analysis_repo.find_by_id = AsyncMock(return_value=job) analysis_repo.update_chunks = AsyncMock(return_value=True) + # rechunk() now drives a Document lifecycle transition (#202), so + # the document_repo must return a real Document and accept the + # update_lifecycle write. + document_repo.find_by_id = AsyncMock( + return_value=Document(id="d1", filename="test.pdf", storage_path="/tmp/test.pdf") + ) + document_repo.update_lifecycle = AsyncMock() chunks = await service.rechunk( "j-remote", diff --git a/document-parser/tests/test_lifecycle.py b/document-parser/tests/test_lifecycle.py new file mode 100644 index 0000000..f63ac52 --- /dev/null +++ b/document-parser/tests/test_lifecycle.py @@ -0,0 +1,129 @@ +"""Tests for the Document lifecycle state machine (#202).""" + +from __future__ import annotations + +from datetime import UTC, datetime + +import pytest + +from domain.events import DocumentLifecycleChanged +from domain.exceptions import InvalidLifecycleTransitionError +from domain.lifecycle import assert_transition, is_allowed_transition +from domain.models import Document +from domain.value_objects import DocumentLifecycleState + +ALLOWED_TRANSITIONS = [ + # Uploaded + (DocumentLifecycleState.UPLOADED, DocumentLifecycleState.PARSED), + (DocumentLifecycleState.UPLOADED, DocumentLifecycleState.CHUNKED), + (DocumentLifecycleState.UPLOADED, DocumentLifecycleState.FAILED), + # Parsed + (DocumentLifecycleState.PARSED, DocumentLifecycleState.PARSED), + (DocumentLifecycleState.PARSED, DocumentLifecycleState.CHUNKED), + (DocumentLifecycleState.PARSED, DocumentLifecycleState.FAILED), + # Chunked + (DocumentLifecycleState.CHUNKED, DocumentLifecycleState.CHUNKED), + (DocumentLifecycleState.CHUNKED, DocumentLifecycleState.INGESTED), + (DocumentLifecycleState.CHUNKED, DocumentLifecycleState.FAILED), + # Ingested + (DocumentLifecycleState.INGESTED, DocumentLifecycleState.STALE), + (DocumentLifecycleState.INGESTED, DocumentLifecycleState.CHUNKED), + (DocumentLifecycleState.INGESTED, DocumentLifecycleState.INGESTED), + (DocumentLifecycleState.INGESTED, DocumentLifecycleState.FAILED), + # Stale + (DocumentLifecycleState.STALE, DocumentLifecycleState.INGESTED), + (DocumentLifecycleState.STALE, DocumentLifecycleState.CHUNKED), + (DocumentLifecycleState.STALE, DocumentLifecycleState.FAILED), + # Failed (recoverable) + (DocumentLifecycleState.FAILED, DocumentLifecycleState.UPLOADED), + (DocumentLifecycleState.FAILED, DocumentLifecycleState.PARSED), + (DocumentLifecycleState.FAILED, DocumentLifecycleState.CHUNKED), +] + + +@pytest.mark.parametrize(("source", "target"), ALLOWED_TRANSITIONS) +def test_allowed_transitions_pass( + source: DocumentLifecycleState, target: DocumentLifecycleState +) -> None: + assert is_allowed_transition(source, target) + # assert_transition does not raise. + assert_transition(source, target) + + +def test_disallowed_transitions_are_rejected() -> None: + """Spot-check the most surprising disallowed pairs. + + Exhaustive enumeration of every disallowed pair would just mirror the + transition table; this list captures the cases a reader is most + likely to argue about. + """ + forbidden = [ + # Uploaded cannot skip directly to Ingested or Stale. + (DocumentLifecycleState.UPLOADED, DocumentLifecycleState.INGESTED), + (DocumentLifecycleState.UPLOADED, DocumentLifecycleState.STALE), + # Parsed cannot become Ingested without going through Chunked. + (DocumentLifecycleState.PARSED, DocumentLifecycleState.INGESTED), + # Stale is not directly reachable from Parsed (no per-store yet). + (DocumentLifecycleState.PARSED, DocumentLifecycleState.STALE), + # Failed cannot be reached as a "self-loop" — it always reflects a + # new failure, never an idempotent re-mark. + (DocumentLifecycleState.FAILED, DocumentLifecycleState.FAILED), + # Failed cannot jump directly to Ingested or Stale — must re-do + # the relevant pipeline step first. + (DocumentLifecycleState.FAILED, DocumentLifecycleState.INGESTED), + (DocumentLifecycleState.FAILED, DocumentLifecycleState.STALE), + ] + for source, target in forbidden: + assert not is_allowed_transition(source, target) + with pytest.raises(InvalidLifecycleTransitionError) as excinfo: + assert_transition(source, target) + assert excinfo.value.source == source + assert excinfo.value.target == target + + +def test_document_default_state_is_uploaded() -> None: + doc = Document(filename="test.pdf", storage_path="/tmp/test.pdf") + assert doc.lifecycle_state == DocumentLifecycleState.UPLOADED + assert doc.lifecycle_state_at is None + + +def test_transition_returns_event_and_mutates_state() -> None: + doc = Document(id="doc-1", filename="t.pdf", storage_path="/tmp/t.pdf") + before = datetime(2026, 4, 29, 10, tzinfo=UTC) + + event = doc.transition_to(DocumentLifecycleState.PARSED, now=before) + + assert isinstance(event, DocumentLifecycleChanged) + assert event.document_id == "doc-1" + assert event.previous == DocumentLifecycleState.UPLOADED + assert event.current == DocumentLifecycleState.PARSED + assert event.at == before + # State mutated on the dataclass. + assert doc.lifecycle_state == DocumentLifecycleState.PARSED + assert doc.lifecycle_state_at == before + + +def test_invalid_transition_does_not_mutate_state() -> None: + doc = Document(id="doc-1", filename="t.pdf", storage_path="/tmp/t.pdf") + assert doc.lifecycle_state == DocumentLifecycleState.UPLOADED + + with pytest.raises(InvalidLifecycleTransitionError): + doc.transition_to(DocumentLifecycleState.STALE) + + # State is untouched. + assert doc.lifecycle_state == DocumentLifecycleState.UPLOADED + assert doc.lifecycle_state_at is None + + +def test_idempotent_self_transitions_emit_event() -> None: + """Self-loops (re-parse, re-chunk, re-push) are explicitly allowed + so the pipeline can drive transitions without checking the source + state first. They still emit an event for observability.""" + doc = Document(id="doc-1", filename="t.pdf", storage_path="/tmp/t.pdf") + doc.transition_to(DocumentLifecycleState.CHUNKED) + assert doc.lifecycle_state == DocumentLifecycleState.CHUNKED + + event = doc.transition_to(DocumentLifecycleState.CHUNKED) + + assert event.previous == DocumentLifecycleState.CHUNKED + assert event.current == DocumentLifecycleState.CHUNKED diff --git a/document-parser/tests/test_repos.py b/document-parser/tests/test_repos.py index dc29d38..4c3179b 100644 --- a/document-parser/tests/test_repos.py +++ b/document-parser/tests/test_repos.py @@ -1,10 +1,11 @@ """Tests for persistence repositories using a temporary SQLite database.""" -from datetime import datetime +from datetime import UTC, datetime import pytest from domain.models import AnalysisJob, AnalysisStatus, Document +from domain.value_objects import DocumentLifecycleState from persistence.analysis_repo import SqliteAnalysisRepository from persistence.database import init_db from persistence.document_repo import SqliteDocumentRepository @@ -81,6 +82,46 @@ class TestDocumentRepo: deleted = await document_repo.delete("nonexistent") assert deleted is False + async def test_default_lifecycle_state_is_uploaded(self, document_repo): + """Fresh document round-trip preserves the default Uploaded state.""" + doc = Document(id="doc-1", filename="t.pdf", storage_path="/tmp/t.pdf") + await document_repo.insert(doc) + + found = await document_repo.find_by_id("doc-1") + assert found is not None + assert found.lifecycle_state == DocumentLifecycleState.UPLOADED + assert found.lifecycle_state_at is None + + async def test_update_lifecycle_persists_state_and_timestamp(self, document_repo): + doc = Document(id="doc-1", filename="t.pdf", storage_path="/tmp/t.pdf") + await document_repo.insert(doc) + + when = datetime(2026, 4, 29, 12, 0, tzinfo=UTC) + await document_repo.update_lifecycle("doc-1", DocumentLifecycleState.PARSED, when) + + found = await document_repo.find_by_id("doc-1") + assert found is not None + assert found.lifecycle_state == DocumentLifecycleState.PARSED + assert found.lifecycle_state_at is not None + assert found.lifecycle_state_at == when + + async def test_lifecycle_state_round_trips_for_each_value(self, document_repo): + """Every enum value must serialize cleanly into and out of SQLite.""" + when = datetime(2026, 4, 29, 12, 0, tzinfo=UTC) + for value in DocumentLifecycleState: + doc = Document( + id=f"doc-{value.value}", + filename="t.pdf", + storage_path="/tmp/t.pdf", + lifecycle_state=value, + lifecycle_state_at=when, + ) + await document_repo.insert(doc) + + found = await document_repo.find_by_id(f"doc-{value.value}") + assert found is not None + assert found.lifecycle_state == value + class TestAnalysisRepo: async def _insert_doc(self, document_repo): diff --git a/frontend/src/shared/types.ts b/frontend/src/shared/types.ts index c6d0a8f..67d6618 100644 --- a/frontend/src/shared/types.ts +++ b/frontend/src/shared/types.ts @@ -1,3 +1,22 @@ +/** + * Canonical document lifecycle state — mirrors the backend enum + * `DocumentLifecycleState` (see `domain/value_objects.py`). + * + * - `Uploaded` raw file persisted, no parse yet + * - `Parsed` conversion produced a document tree + * - `Chunked` chunker produced a draft chunkset + * - `Ingested` chunkset has been embedded into at least one store + * - `Stale` chunkset edited after a successful push (per-store concept) + * - `Failed` a pipeline step failed; recoverable by retry + */ +export type DocumentLifecycleState = + | 'Uploaded' + | 'Parsed' + | 'Chunked' + | 'Ingested' + | 'Stale' + | 'Failed' + export interface Document { id: string filename: string @@ -5,6 +24,10 @@ export interface Document { fileSize: number | null pageCount: number | null createdAt: string + /** Canonical lifecycle state. Drives the status badge in `/docs`. */ + lifecycleState: DocumentLifecycleState + /** ISO timestamp of the last lifecycle transition (UTC). */ + lifecycleStateAt: string | null } export interface PipelineOptions { From d81f36d89903c210ceff2536ca15d96d07ceeefd Mon Sep 17 00:00:00 2001 From: Pier-Jean Malandrino Date: Wed, 29 Apr 2026 15:38:56 +0200 Subject: [PATCH 04/35] feat(#203): per (document, store) ingestion state MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Introduces the data layer for multi-store ingestion. Documents can now live in multiple stores, each with its own Ingested/Stale/Failed state. The doc-level lifecycle (#202) becomes the aggregate over all per-store links, computed by a pure domain function. Domain - Store entity (name, slug, kind, embedder, config, is_default) - DocumentStoreLink entity with mark_ingested / mark_stale / mark_failed helpers - StoreKind and DocumentStoreLinkState enums - aggregate_lifecycle(): pure function — Failed > Stale > Ingested > fallback (the doc's pre-link Uploaded/Parsed/Chunked state) Persistence - New tables 'stores' and 'document_store_links' with the right indexes (doc_id, store_id, state) and a UNIQUE (doc, store) on the link - Default 'opensearch' store seeded idempotently in init_db, embedder pulled from DEFAULT_EMBEDDER (fallback bge-m3) - SqliteStoreRepository (find_by_slug, find_by_id, get_default, …) - SqliteDocumentStoreLinkRepository with ON CONFLICT … DO UPDATE upsert Ports - StoreRepository and DocumentStoreLinkRepository protocols added Tests - 14 new tests: seed idempotency, insert/find round-trips, UNIQUE constraint, cascade delete with the document, every link state round-trips, aggregation rule with all branches Refs #203 --- .../domain/lifecycle_aggregation.py | 58 ++++++ document-parser/domain/models.py | 73 ++++++- document-parser/domain/ports.py | 32 ++- document-parser/domain/value_objects.py | 22 ++ document-parser/persistence/database.py | 45 +++++ .../persistence/document_store_link_repo.py | 98 +++++++++ document-parser/persistence/store_repo.py | 83 ++++++++ .../tests/test_lifecycle_aggregation.py | 72 +++++++ document-parser/tests/test_store_repo.py | 190 ++++++++++++++++++ 9 files changed, 671 insertions(+), 2 deletions(-) create mode 100644 document-parser/domain/lifecycle_aggregation.py create mode 100644 document-parser/persistence/document_store_link_repo.py create mode 100644 document-parser/persistence/store_repo.py create mode 100644 document-parser/tests/test_lifecycle_aggregation.py create mode 100644 document-parser/tests/test_store_repo.py diff --git a/document-parser/domain/lifecycle_aggregation.py b/document-parser/domain/lifecycle_aggregation.py new file mode 100644 index 0000000..a94a724 --- /dev/null +++ b/document-parser/domain/lifecycle_aggregation.py @@ -0,0 +1,58 @@ +"""Aggregate the per-(document, store) link states into a single +document-level lifecycle state. + +The doc lifecycle column is the materialized result of this rule. It is +recomputed any time a link write happens. Read paths use the stored +column directly (cheap) — they do not call this rule on every GET. + +The rule prefers "more concerning" states first: + + any link FAILED -> Document FAILED + any link STALE -> Document STALE + any link INGESTED -> Document INGESTED + no links -> keep the document's current pre-link state + (Uploaded / Parsed / Chunked) — the caller is + responsible for not overwriting that. + +If you find yourself wanting a fourth case, you probably want a new +link state, not a new aggregation branch. + +This module is pure: no I/O, no datetime — just data in / data out. +""" + +from __future__ import annotations + +from typing import TYPE_CHECKING + +from domain.value_objects import DocumentLifecycleState, DocumentStoreLinkState + +if TYPE_CHECKING: + from collections.abc import Iterable + + from domain.models import DocumentStoreLink + + +def aggregate_lifecycle( + links: Iterable[DocumentStoreLink], + *, + fallback: DocumentLifecycleState, +) -> DocumentLifecycleState: + """Compute the aggregate document lifecycle state. + + Args: + links: every `DocumentStoreLink` for the document. May be empty. + fallback: the lifecycle state to return when there are no links — + typically the document's current pre-link state (`Uploaded`, + `Parsed`, or `Chunked`). + + Returns: + The aggregate `DocumentLifecycleState`. + """ + states = {link.state for link in links} + if DocumentStoreLinkState.FAILED in states: + return DocumentLifecycleState.FAILED + if DocumentStoreLinkState.STALE in states: + return DocumentLifecycleState.STALE + if DocumentStoreLinkState.INGESTED in states: + return DocumentLifecycleState.INGESTED + return fallback diff --git a/document-parser/domain/models.py b/document-parser/domain/models.py index 0a371ee..7c05040 100644 --- a/document-parser/domain/models.py +++ b/document-parser/domain/models.py @@ -9,7 +9,11 @@ from datetime import UTC, datetime from domain.events import DocumentLifecycleChanged from domain.lifecycle import assert_transition -from domain.value_objects import DocumentLifecycleState +from domain.value_objects import ( + DocumentLifecycleState, + DocumentStoreLinkState, + StoreKind, +) class AnalysisStatus(enum.StrEnum): @@ -130,3 +134,70 @@ class AnalysisJob: self.status = AnalysisStatus.FAILED self.error_message = error self.completed_at = _utcnow() + + +@dataclass +class Store: + """A logical destination for ingested chunks. + + A `Store` represents a named, configurable target for ingestion (e.g. + `rh-corpus-v3`, `legal-v1`). It is decoupled from the underlying + `VectorStore` adapter (which represents the *technology*, like + OpenSearch). One adapter can serve many stores by namespacing the + physical index name on `slug`. + + See design doc `docs/design/203-per-store-ingestion-state.md`. + """ + + id: str = field(default_factory=_new_id) + name: str = "" + slug: str = "" + kind: StoreKind = StoreKind.OPENSEARCH + embedder: str = "" + config: dict = field(default_factory=dict) + is_default: bool = False + created_at: datetime = field(default_factory=_utcnow) + + +@dataclass +class DocumentStoreLink: + """A live record of a document's presence in a single store. + + The link carries the per-pair state (Ingested / Stale / Failed) plus + metadata used by the auto-stale detection (#204) and the chunks + editor (#205). One row per (document, store) pair — enforced by a + UNIQUE constraint at the persistence layer. + """ + + id: str = field(default_factory=_new_id) + document_id: str = "" + store_id: str = "" + state: DocumentStoreLinkState = DocumentStoreLinkState.INGESTED + chunkset_hash: str | None = None + last_push_at: datetime | None = None + last_run_id: str | None = None + error_message: str | None = None + + def mark_ingested( + self, + *, + hash_: str, + at: datetime, + run_id: str | None = None, + ) -> None: + """Record a successful push: chunkset hash + timestamp + run id.""" + self.state = DocumentStoreLinkState.INGESTED + self.chunkset_hash = hash_ + self.last_push_at = at + self.last_run_id = run_id + self.error_message = None + + def mark_stale(self) -> None: + """Mark the link as stale (source chunkset drifted from pushed).""" + self.state = DocumentStoreLinkState.STALE + self.error_message = None + + def mark_failed(self, *, error: str) -> None: + """Record a failed push attempt.""" + self.state = DocumentStoreLinkState.FAILED + self.error_message = error diff --git a/document-parser/domain/ports.py b/document-parser/domain/ports.py index 967871d..d6929b4 100644 --- a/document-parser/domain/ports.py +++ b/document-parser/domain/ports.py @@ -11,7 +11,7 @@ from typing import TYPE_CHECKING, Protocol, runtime_checkable if TYPE_CHECKING: from datetime import datetime - from domain.models import AnalysisJob, Document + from domain.models import AnalysisJob, Document, DocumentStoreLink, Store from domain.value_objects import ( ChunkingOptions, ChunkResult, @@ -98,6 +98,36 @@ class DocumentRepository(Protocol): async def delete(self, doc_id: str) -> bool: ... +class StoreRepository(Protocol): + """Port for `Store` persistence (introduced by #203).""" + + async def insert(self, store: Store) -> None: ... + + async def find_all(self) -> list[Store]: ... + + async def find_by_slug(self, slug: str) -> Store | None: ... + + async def find_by_id(self, store_id: str) -> Store | None: ... + + async def get_default(self) -> Store | None: ... + + +class DocumentStoreLinkRepository(Protocol): + """Port for `DocumentStoreLink` persistence (introduced by #203).""" + + async def upsert(self, link: DocumentStoreLink) -> None: + """Insert or update by (document_id, store_id).""" + ... + + async def find_for_document(self, document_id: str) -> list[DocumentStoreLink]: ... + + async def find_for_store(self, store_id: str) -> list[DocumentStoreLink]: ... + + async def find_one(self, document_id: str, store_id: str) -> DocumentStoreLink | None: ... + + async def delete(self, document_id: str, store_id: str) -> bool: ... + + class AnalysisRepository(Protocol): """Port for analysis job persistence.""" diff --git a/document-parser/domain/value_objects.py b/document-parser/domain/value_objects.py index 784dd2c..d3f1baf 100644 --- a/document-parser/domain/value_objects.py +++ b/document-parser/domain/value_objects.py @@ -39,6 +39,28 @@ class DocumentLifecycleState(StrEnum): FAILED = "Failed" +class StoreKind(StrEnum): + """Backing technology of a Store. Today only OpenSearch is implemented; + the enum is here so future backends (Pinecone, Qdrant, pgvector) can be + added without touching the persistence schema.""" + + OPENSEARCH = "opensearch" + + +class DocumentStoreLinkState(StrEnum): + """State of a (document, store) ingestion link. + + Distinct from `DocumentLifecycleState` — the document lifecycle is the + aggregate over all per-store links. A link is `Ingested` when its + chunkset hash matches the source; `Stale` when the source has drifted + after the last push; `Failed` when the last push attempt errored. + """ + + INGESTED = "Ingested" + STALE = "Stale" + FAILED = "Failed" + + @dataclass(frozen=True) class PageElement: type: str diff --git a/document-parser/persistence/database.py b/document-parser/persistence/database.py index b79f5db..7398f4c 100644 --- a/document-parser/persistence/database.py +++ b/document-parser/persistence/database.py @@ -42,6 +42,34 @@ CREATE TABLE IF NOT EXISTS analysis_jobs ( CREATE INDEX IF NOT EXISTS idx_analysis_jobs_status ON analysis_jobs(status); CREATE INDEX IF NOT EXISTS idx_analysis_jobs_created_at ON analysis_jobs(created_at); CREATE INDEX IF NOT EXISTS idx_documents_created_at ON documents(created_at); + +-- 0.6.0 — Per (document, store) ingestion state (#203). +CREATE TABLE IF NOT EXISTS stores ( + id TEXT PRIMARY KEY, + name TEXT NOT NULL UNIQUE, + slug TEXT NOT NULL UNIQUE, + kind TEXT NOT NULL, + embedder TEXT NOT NULL, + config TEXT NOT NULL DEFAULT '{}', + is_default INTEGER NOT NULL DEFAULT 0, + created_at TEXT NOT NULL DEFAULT (datetime('now')) +); + +CREATE TABLE IF NOT EXISTS document_store_links ( + id TEXT PRIMARY KEY, + document_id TEXT NOT NULL REFERENCES documents(id) ON DELETE CASCADE, + store_id TEXT NOT NULL REFERENCES stores(id) ON DELETE CASCADE, + state TEXT NOT NULL, + chunkset_hash TEXT, + last_push_at TEXT, + last_run_id TEXT, + error_message TEXT, + UNIQUE (document_id, store_id) +); + +CREATE INDEX IF NOT EXISTS idx_dsl_doc ON document_store_links(document_id); +CREATE INDEX IF NOT EXISTS idx_dsl_store ON document_store_links(store_id); +CREATE INDEX IF NOT EXISTS idx_dsl_state ON document_store_links(state); """ @@ -107,10 +135,27 @@ async def init_db() -> None: async with aiosqlite.connect(DB_PATH) as db: await db.executescript(_SCHEMA) await _run_migrations(db) + await _seed_default_store(db) await db.commit() logger.info("Database initialized at %s", DB_PATH) +async def _seed_default_store(db: aiosqlite.Connection) -> None: + """Insert the canonical `default` store on first boot. + + Idempotent — uses INSERT OR IGNORE keyed on the unique slug. The + embedder is read from the DEFAULT_EMBEDDER env var with a sensible + fallback so existing single-index deployments keep working. + """ + embedder = os.environ.get("DEFAULT_EMBEDDER", "bge-m3") + await db.execute( + """INSERT OR IGNORE INTO stores + (id, name, slug, kind, embedder, config, is_default, created_at) + VALUES (?, ?, ?, ?, ?, ?, ?, datetime('now'))""", + ("default", "default", "default", "opensearch", embedder, "{}", 1), + ) + + async def get_db() -> aiosqlite.Connection: """Open a new database connection with row factory and FK enforcement.""" db = await aiosqlite.connect(DB_PATH) diff --git a/document-parser/persistence/document_store_link_repo.py b/document-parser/persistence/document_store_link_repo.py new file mode 100644 index 0000000..fa38d83 --- /dev/null +++ b/document-parser/persistence/document_store_link_repo.py @@ -0,0 +1,98 @@ +"""Document-Store link repository — SQLite CRUD on `document_store_links`.""" + +from __future__ import annotations + +from datetime import UTC, datetime + +from domain.models import DocumentStoreLink +from domain.value_objects import DocumentStoreLinkState +from persistence.database import get_connection + + +def _parse_iso(value: str | None) -> datetime | None: + if value is None or value == "": + return None + parsed = datetime.fromisoformat(value) + if parsed.tzinfo is None: + parsed = parsed.replace(tzinfo=UTC) + return parsed + + +def _row_to_link(row) -> DocumentStoreLink: + return DocumentStoreLink( + id=row["id"], + document_id=row["document_id"], + store_id=row["store_id"], + state=DocumentStoreLinkState(row["state"]), + chunkset_hash=row["chunkset_hash"], + last_push_at=_parse_iso(row["last_push_at"]), + last_run_id=row["last_run_id"], + error_message=row["error_message"], + ) + + +class SqliteDocumentStoreLinkRepository: + """SQLite implementation of the DocumentStoreLinkRepository port.""" + + async def upsert(self, link: DocumentStoreLink) -> None: + """Insert a new link or update the existing (document_id, store_id) row.""" + async with get_connection() as db: + await db.execute( + """INSERT INTO document_store_links + (id, document_id, store_id, state, chunkset_hash, + last_push_at, last_run_id, error_message) + VALUES (?, ?, ?, ?, ?, ?, ?, ?) + ON CONFLICT(document_id, store_id) DO UPDATE SET + state = excluded.state, + chunkset_hash = excluded.chunkset_hash, + last_push_at = excluded.last_push_at, + last_run_id = excluded.last_run_id, + error_message = excluded.error_message""", + ( + link.id, + link.document_id, + link.store_id, + link.state.value, + link.chunkset_hash, + str(link.last_push_at) if link.last_push_at else None, + link.last_run_id, + link.error_message, + ), + ) + await db.commit() + + async def find_for_document(self, document_id: str) -> list[DocumentStoreLink]: + async with get_connection() as db: + cursor = await db.execute( + "SELECT * FROM document_store_links WHERE document_id = ?", + (document_id,), + ) + rows = await cursor.fetchall() + return [_row_to_link(r) for r in rows] + + async def find_for_store(self, store_id: str) -> list[DocumentStoreLink]: + async with get_connection() as db: + cursor = await db.execute( + "SELECT * FROM document_store_links WHERE store_id = ?", + (store_id,), + ) + rows = await cursor.fetchall() + return [_row_to_link(r) for r in rows] + + async def find_one(self, document_id: str, store_id: str) -> DocumentStoreLink | None: + async with get_connection() as db: + cursor = await db.execute( + "SELECT * FROM document_store_links WHERE document_id = ? AND store_id = ?", + (document_id, store_id), + ) + row = await cursor.fetchone() + return _row_to_link(row) if row else None + + async def delete(self, document_id: str, store_id: str) -> bool: + async with get_connection() as db: + cursor = await db.execute( + "DELETE FROM document_store_links WHERE document_id = ? AND store_id = ?", + (document_id, store_id), + ) + await db.commit() + return cursor.rowcount > 0 diff --git a/document-parser/persistence/store_repo.py b/document-parser/persistence/store_repo.py new file mode 100644 index 0000000..fd67913 --- /dev/null +++ b/document-parser/persistence/store_repo.py @@ -0,0 +1,83 @@ +"""Store repository — SQLite CRUD for the `stores` table.""" + +from __future__ import annotations + +import json +from datetime import UTC, datetime + +from domain.models import Store +from domain.value_objects import StoreKind +from persistence.database import get_connection + + +def _row_to_store(row) -> Store: + created = row["created_at"] + if isinstance(created, str): + created = datetime.fromisoformat(created) + if created.tzinfo is None: + created = created.replace(tzinfo=UTC) + config_raw = row["config"] or "{}" + try: + config = json.loads(config_raw) + except (TypeError, ValueError): + config = {} + return Store( + id=row["id"], + name=row["name"], + slug=row["slug"], + kind=StoreKind(row["kind"]), + embedder=row["embedder"], + config=config, + is_default=bool(row["is_default"]), + created_at=created, + ) + + +class SqliteStoreRepository: + """SQLite implementation of the StoreRepository port.""" + + async def insert(self, store: Store) -> None: + async with get_connection() as db: + await db.execute( + """INSERT INTO stores + (id, name, slug, kind, embedder, config, is_default, created_at) + VALUES (?, ?, ?, ?, ?, ?, ?, ?)""", + ( + store.id, + store.name, + store.slug, + store.kind.value, + store.embedder, + json.dumps(store.config), + 1 if store.is_default else 0, + str(store.created_at), + ), + ) + await db.commit() + + async def find_all(self) -> list[Store]: + async with get_connection() as db: + cursor = await db.execute("SELECT * FROM stores ORDER BY is_default DESC, name ASC") + rows = await cursor.fetchall() + return [_row_to_store(r) for r in rows] + + async def find_by_slug(self, slug: str) -> Store | None: + async with get_connection() as db: + cursor = await db.execute("SELECT * FROM stores WHERE slug = ?", (slug,)) + row = await cursor.fetchone() + return _row_to_store(row) if row else None + + async def find_by_id(self, store_id: str) -> Store | None: + async with get_connection() as db: + cursor = await db.execute("SELECT * FROM stores WHERE id = ?", (store_id,)) + row = await cursor.fetchone() + return _row_to_store(row) if row else None + + async def get_default(self) -> Store | None: + """Return the seeded `default` store, if any.""" + async with get_connection() as db: + cursor = await db.execute( + "SELECT * FROM stores WHERE is_default = 1 ORDER BY created_at ASC LIMIT 1" + ) + row = await cursor.fetchone() + return _row_to_store(row) if row else None diff --git a/document-parser/tests/test_lifecycle_aggregation.py b/document-parser/tests/test_lifecycle_aggregation.py new file mode 100644 index 0000000..ef0dbe3 --- /dev/null +++ b/document-parser/tests/test_lifecycle_aggregation.py @@ -0,0 +1,72 @@ +"""Tests for the document lifecycle aggregation rule (#203).""" + +from __future__ import annotations + +from domain.lifecycle_aggregation import aggregate_lifecycle +from domain.models import DocumentStoreLink +from domain.value_objects import DocumentLifecycleState, DocumentStoreLinkState + + +def _link(state: DocumentStoreLinkState) -> DocumentStoreLink: + return DocumentStoreLink( + id=f"link-{state.value}", + document_id="doc-1", + store_id=f"store-{state.value}", + state=state, + ) + + +def test_no_links_returns_fallback() -> None: + assert ( + aggregate_lifecycle([], fallback=DocumentLifecycleState.CHUNKED) + == DocumentLifecycleState.CHUNKED + ) + + +def test_single_ingested_link_makes_doc_ingested() -> None: + assert ( + aggregate_lifecycle( + [_link(DocumentStoreLinkState.INGESTED)], + fallback=DocumentLifecycleState.CHUNKED, + ) + == DocumentLifecycleState.INGESTED + ) + + +def test_any_stale_link_makes_doc_stale() -> None: + """Stale outranks Ingested — a doc with one stale store is considered stale.""" + assert ( + aggregate_lifecycle( + [ + _link(DocumentStoreLinkState.INGESTED), + _link(DocumentStoreLinkState.STALE), + ], + fallback=DocumentLifecycleState.CHUNKED, + ) + == DocumentLifecycleState.STALE + ) + + +def test_any_failed_link_makes_doc_failed() -> None: + """Failed outranks every other link state.""" + assert ( + aggregate_lifecycle( + [ + _link(DocumentStoreLinkState.INGESTED), + _link(DocumentStoreLinkState.STALE), + _link(DocumentStoreLinkState.FAILED), + ], + fallback=DocumentLifecycleState.CHUNKED, + ) + == DocumentLifecycleState.FAILED + ) + + +def test_only_failed_link_makes_doc_failed() -> None: + assert ( + aggregate_lifecycle( + [_link(DocumentStoreLinkState.FAILED)], + fallback=DocumentLifecycleState.CHUNKED, + ) + == DocumentLifecycleState.FAILED + ) diff --git a/document-parser/tests/test_store_repo.py b/document-parser/tests/test_store_repo.py new file mode 100644 index 0000000..7126d38 --- /dev/null +++ b/document-parser/tests/test_store_repo.py @@ -0,0 +1,190 @@ +"""Tests for the Store and DocumentStoreLink repositories (#203).""" + +from __future__ import annotations + +from datetime import UTC, datetime + +import pytest + +from domain.models import Document, DocumentStoreLink, Store +from domain.value_objects import DocumentStoreLinkState, StoreKind +from persistence.database import init_db +from persistence.document_repo import SqliteDocumentRepository +from persistence.document_store_link_repo import SqliteDocumentStoreLinkRepository +from persistence.store_repo import SqliteStoreRepository + + +@pytest.fixture(autouse=True) +async def setup_db(monkeypatch, tmp_path): + db_path = str(tmp_path / "test.db") + monkeypatch.setattr("persistence.database.DB_PATH", db_path) + await init_db() + yield + + +@pytest.fixture +def store_repo(): + return SqliteStoreRepository() + + +@pytest.fixture +def link_repo(): + return SqliteDocumentStoreLinkRepository() + + +@pytest.fixture +def document_repo(): + return SqliteDocumentRepository() + + +class TestStoreRepo: + async def test_default_store_seeded_on_init(self, store_repo): + """init_db() must seed exactly one `default` store on a fresh DB.""" + default = await store_repo.get_default() + assert default is not None + assert default.slug == "default" + assert default.is_default is True + assert default.kind == StoreKind.OPENSEARCH + + async def test_seed_is_idempotent(self, store_repo): + """Re-running init_db() must not create a second default store.""" + await init_db() + await init_db() + all_stores = await store_repo.find_all() + slugs = [s.slug for s in all_stores] + assert slugs.count("default") == 1 + + async def test_insert_and_find_by_slug(self, store_repo): + store = Store( + id="s-rh", + name="rh-corpus-v3", + slug="rh-corpus-v3", + kind=StoreKind.OPENSEARCH, + embedder="bge-m3", + config={"index_name": "rh-corpus-v3"}, + ) + await store_repo.insert(store) + + found = await store_repo.find_by_slug("rh-corpus-v3") + assert found is not None + assert found.embedder == "bge-m3" + assert found.config == {"index_name": "rh-corpus-v3"} + + async def test_find_all_orders_default_first(self, store_repo): + await store_repo.insert( + Store( + id="s-rh", + name="rh", + slug="rh", + kind=StoreKind.OPENSEARCH, + embedder="bge-m3", + ) + ) + await store_repo.insert( + Store( + id="s-legal", + name="legal", + slug="legal", + kind=StoreKind.OPENSEARCH, + embedder="bge-m3", + ) + ) + all_stores = await store_repo.find_all() + assert all_stores[0].slug == "default" # seeded default comes first + + +class TestDocumentStoreLinkRepo: + async def _seed_doc(self, document_repo, doc_id: str = "doc-1") -> Document: + doc = Document(id=doc_id, filename="t.pdf", storage_path="/tmp/t.pdf") + await document_repo.insert(doc) + return doc + + async def test_upsert_creates_then_updates(self, link_repo, document_repo): + await self._seed_doc(document_repo) + link = DocumentStoreLink( + id="l-1", + document_id="doc-1", + store_id="default", + state=DocumentStoreLinkState.INGESTED, + ) + await link_repo.upsert(link) + + # Update same (doc, store) — no second row. + link.mark_ingested( + hash_="abc123", + at=datetime(2026, 4, 29, 12, tzinfo=UTC), + run_id="run-1", + ) + await link_repo.upsert(link) + + all_for_doc = await link_repo.find_for_document("doc-1") + assert len(all_for_doc) == 1 + assert all_for_doc[0].chunkset_hash == "abc123" + assert all_for_doc[0].last_run_id == "run-1" + + async def test_unique_constraint_on_doc_store_pair(self, link_repo, document_repo): + """Two links with the same (doc, store) collapse to one via upsert.""" + await self._seed_doc(document_repo) + await link_repo.upsert( + DocumentStoreLink( + id="l-1", + document_id="doc-1", + store_id="default", + state=DocumentStoreLinkState.INGESTED, + ) + ) + await link_repo.upsert( + DocumentStoreLink( + id="l-2", + document_id="doc-1", + store_id="default", + state=DocumentStoreLinkState.STALE, + ) + ) + rows = await link_repo.find_for_document("doc-1") + assert len(rows) == 1 + assert rows[0].state == DocumentStoreLinkState.STALE + + async def test_find_one_returns_none_when_absent(self, link_repo, document_repo): + await self._seed_doc(document_repo) + assert await link_repo.find_one("doc-1", "default") is None + + async def test_cascade_delete_when_doc_removed(self, link_repo, document_repo): + await self._seed_doc(document_repo) + await link_repo.upsert( + DocumentStoreLink( + id="l-1", + document_id="doc-1", + store_id="default", + state=DocumentStoreLinkState.INGESTED, + ) + ) + await document_repo.delete("doc-1") + rows = await link_repo.find_for_document("doc-1") + assert rows == [] + + async def test_state_round_trips(self, link_repo, document_repo): + await self._seed_doc(document_repo) + for state in DocumentStoreLinkState: + link = DocumentStoreLink( + id=f"l-{state.value}", + document_id="doc-1", + store_id=f"store-{state.value}", + state=state, + ) + # Need a store row for FK; seed minimal stores. + from persistence.store_repo import SqliteStoreRepository + + await SqliteStoreRepository().insert( + Store( + id=f"store-{state.value}", + name=f"s-{state.value}", + slug=f"s-{state.value}", + kind=StoreKind.OPENSEARCH, + embedder="bge-m3", + ) + ) + await link_repo.upsert(link) + found = await link_repo.find_one("doc-1", f"store-{state.value}") + assert found is not None + assert found.state == state From 4b59ef23308916e5cb6831f1527b06340763ae73 Mon Sep 17 00:00:00 2001 From: Pier-Jean Malandrino Date: Wed, 29 Apr 2026 15:40:49 +0200 Subject: [PATCH 05/35] feat(#204): deterministic chunkset hash for auto-stale detection MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds the pure-domain hash function that summarises a chunkset for stale-detection purposes. Recorded on each DocumentStoreLink at push time (#203 ships the column slot); compared against the recomputed current hash to flip a link to Stale when the source has drifted. domain/hashing.py - chunkset_hash(chunks: Iterable[ChunkResult]) -> str - SHA-256 over (text, source_page, headings) per chunk - Excludes bboxes / doc_items / token_count by design - 0x1F separator between chunks defends against the join-attack (split A+B vs concat AB) Tests - 9 tests: determinism, sensitivity per included field, exclusion of rendering-only fields, join-attack resistance, order sensitivity, empty-input invariant - Locked fixture: a hand-built 3-chunk input has a fixed expected hash; CI fails loud if anyone changes the canonical inputs without updating the fixture deliberately (and the release notes) Service integration (recompute on chunk write, set on push) lands with #205 once chunks are first-class — direct integration on the legacy chunks_json path is deliberately deferred to keep #204 focused. Refs #204 --- document-parser/domain/hashing.py | 67 +++++++++++++++++ document-parser/tests/test_hashing.py | 102 ++++++++++++++++++++++++++ 2 files changed, 169 insertions(+) create mode 100644 document-parser/domain/hashing.py create mode 100644 document-parser/tests/test_hashing.py diff --git a/document-parser/domain/hashing.py b/document-parser/domain/hashing.py new file mode 100644 index 0000000..d438fcb --- /dev/null +++ b/document-parser/domain/hashing.py @@ -0,0 +1,67 @@ +"""Deterministic hashing for chunksets — substrate for auto-stale detection (#204). + +A `chunkset_hash` summarises the content of a list of chunks for a +document. The hash is recorded on each `DocumentStoreLink` at push time +(#203 ships the column slot). When chunks change, recomputing the hash +and comparing against the stored value tells us whether the link has +gone stale. + +Why a hash and not, say, an updated_at? + - Idempotent re-pipelines on identical input bump `updated_at` without + semantic change. A content hash is the only signal that survives + that. + - It is also content-addressed: two different docs that happen to have + the same chunkset get the same hash. Useful for de-duplication + further down the road. + +Inputs and exclusions are pinned. Any change to the canonical inputs +re-flips every existing link to Stale once — that is a deliberate +release-note event, not a silent migration. + +This module is pure: in / out. No I/O. No randomness. No dates. +""" + +from __future__ import annotations + +import hashlib +import json +from typing import TYPE_CHECKING + +if TYPE_CHECKING: + from collections.abc import Iterable + + from domain.value_objects import ChunkResult + + +# Byte separator inserted between chunks so concatenating two chunks does +# not yield the same hash as a single chunk with the joined text. \x1f is +# the Unicode "Information Separator One" — semantically appropriate and +# safe inside arbitrary text. +_CHUNK_SEPARATOR = b"\x1f" + + +def chunkset_hash(chunks: Iterable[ChunkResult]) -> str: + """Return a deterministic SHA-256 hex digest over a chunkset. + + Hashed inputs (per chunk, in order): + - text (str) + - source_page (int | None) + - headings (list[str], order preserved) + + Excluded: + - bboxes / doc_items (rendering artefacts; do not affect retrieval) + - token_count (derived; unstable across tokenizers) + + The exclusion list is intentional. Bumping it changes every link's + hash and triggers a one-time corpus-wide flip to `Stale`. + """ + h = hashlib.sha256() + for chunk in chunks: + payload = { + "t": chunk.text, + "p": chunk.source_page, + "h": list(chunk.headings or []), + } + h.update(_CHUNK_SEPARATOR) + h.update(json.dumps(payload, ensure_ascii=False, separators=(",", ":")).encode()) + return h.hexdigest() diff --git a/document-parser/tests/test_hashing.py b/document-parser/tests/test_hashing.py new file mode 100644 index 0000000..5d3ed7a --- /dev/null +++ b/document-parser/tests/test_hashing.py @@ -0,0 +1,102 @@ +"""Tests for the chunkset hash function (#204).""" + +from __future__ import annotations + +from domain.hashing import chunkset_hash +from domain.value_objects import ChunkBbox, ChunkDocItem, ChunkResult + + +def _chunk(text: str, *, page: int | None = 1, headings=()) -> ChunkResult: + return ChunkResult(text=text, headings=list(headings), source_page=page) + + +def test_empty_chunkset_returns_empty_sha256() -> None: + """Empty input has a stable, well-known hash.""" + h = chunkset_hash([]) + # SHA-256 of nothing is the well-known constant. + assert h == "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855" + + +def test_determinism_across_invocations() -> None: + chunks = [_chunk("a"), _chunk("b"), _chunk("c")] + assert chunkset_hash(chunks) == chunkset_hash(chunks) + + +def test_text_change_changes_hash() -> None: + base = [_chunk("alpha")] + edited = [_chunk("alpha!")] + assert chunkset_hash(base) != chunkset_hash(edited) + + +def test_page_change_changes_hash() -> None: + a = [_chunk("alpha", page=1)] + b = [_chunk("alpha", page=2)] + assert chunkset_hash(a) != chunkset_hash(b) + + +def test_headings_change_changes_hash() -> None: + a = [_chunk("alpha", headings=("Section A",))] + b = [_chunk("alpha", headings=("Section B",))] + assert chunkset_hash(a) != chunkset_hash(b) + + +def test_excluded_fields_do_not_change_hash() -> None: + """token_count, bboxes, doc_items are deliberately excluded.""" + a = ChunkResult(text="alpha", headings=[], source_page=1, token_count=100, bboxes=[]) + b = ChunkResult( + text="alpha", + headings=[], + source_page=1, + token_count=999, # different + bboxes=[ChunkBbox(page=1, bbox=[0, 0, 10, 10])], # different + doc_items=[ChunkDocItem(self_ref="#/x", label="text")], # different + ) + assert chunkset_hash([a]) == chunkset_hash([b]) + + +def test_join_attack_resistance() -> None: + """Splitting one chunk into two with the same combined content must + produce a different hash from the original single-chunk version.""" + one = [_chunk("HelloWorld")] + two = [_chunk("Hello"), _chunk("World")] + assert chunkset_hash(one) != chunkset_hash(two) + + +def test_order_matters() -> None: + a = [_chunk("alpha"), _chunk("bravo")] + b = [_chunk("bravo"), _chunk("alpha")] + assert chunkset_hash(a) != chunkset_hash(b) + + +def test_locked_fixture_three_chunks() -> None: + """Locked fixture: a hand-built 3-chunk input has a fixed expected + hash. CI fails loud if anyone changes the canonical input list + without updating the fixture deliberately. + + The expected hash below was computed once with the function as + pinned in this commit. To regenerate after a deliberate canonical + change, run: + + python -c 'from domain.hashing import chunkset_hash; \\ + from domain.value_objects import ChunkResult; \\ + print(chunkset_hash([ + ChunkResult(text="Intro paragraph.", source_page=1, + headings=["Intro"]), + ChunkResult(text="Body of section A.", source_page=2, + headings=["A"]), + ChunkResult(text="Conclusion.", source_page=3, + headings=["Conclusion"]), + ]))' + """ + chunks = [ + ChunkResult(text="Intro paragraph.", source_page=1, headings=["Intro"]), + ChunkResult(text="Body of section A.", source_page=2, headings=["A"]), + ChunkResult(text="Conclusion.", source_page=3, headings=["Conclusion"]), + ] + expected = "6ac365ae403e53675e57884b69b0629684f2209c39730093231caa11a40e5225" + actual = chunkset_hash(chunks) + assert actual == expected, ( + f"Hash drift detected. Expected {expected}, got {actual}. " + "If you intentionally changed the canonical inputs, update this " + "fixture and document the breaking change in the release notes." + ) From 648f2a5c0d8a87eb1f28184bdb317861246cf7f5 Mon Sep 17 00:00:00 2001 From: Pier-Jean Malandrino Date: Wed, 29 Apr 2026 15:45:18 +0200 Subject: [PATCH 06/35] feat(#205): promote chunks to first-class entities + audit trail MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The data and domain layers for the chunks editor (#219-224 in 0.6.0). Chunks were previously stored as a JSON blob in analysis_jobs.chunks_json; this commit makes them first-class persisted entities with stable IDs, soft-delete, and an immutable audit log. Domain - Chunk: persistent entity with id, document_id, sequence, text, headings, source_page, bboxes, doc_items, token_count, timestamps, deleted_at (soft delete) - ChunkEdit: immutable audit row (action, actor, at, before, after, parents, children, reason) - ChunkPush: snapshot of which chunk_ids landed in which store at push - ChunkEditAction enum: insert/update/delete/merge/split - domain/chunk_editing.py: pure operations on a chunkset (insert, update, delete, merge, split). Each returns a new chunkset and the affected chunk(s); errors raise ChunkEditingError. Persistence - Three new tables: chunks, chunk_edits, chunk_pushes (FK + indexes) - SqliteChunkRepository (insert, insert_many, update, soft_delete, find_for_document, find_by_id; respects deleted_at) - SqliteChunkEditRepository (append-only audit log; paginated reads ordered newest-first; per-chunk history) - SqliteChunkPushRepository (per-(doc, store) latest snapshot) Ports - ChunkRepository, ChunkEditRepository, ChunkPushRepository protocols added to domain/ports.py Tests - 17 tests for the pure chunk-editing operations covering insert / update / delete / merge / split, sequence shifts, lineage, error paths (out-of-range, missing id, deleted target, cross-document) - 11 tests for the three repositories: round-trips, soft-delete filtering, history ordering, lineage round-trip, cascade-delete with document, find_latest semantics Service orchestration (ChunkEditingService — atomic chunk + audit write) and the API endpoints land in a follow-up commit on the same feature branch / next release. The data + domain foundation here is what unblocks #219-224. Refs #205 --- document-parser/domain/chunk_editing.py | 216 ++++++++++++++++++ document-parser/domain/models.py | 77 +++++++ document-parser/domain/ports.py | 57 ++++- document-parser/domain/value_objects.py | 14 ++ .../persistence/chunk_edit_repo.py | 145 ++++++++++++ document-parser/persistence/chunk_repo.py | 134 +++++++++++ document-parser/persistence/database.py | 44 ++++ document-parser/tests/test_chunk_editing.py | 196 ++++++++++++++++ document-parser/tests/test_chunk_repos.py | 212 +++++++++++++++++ 9 files changed, 1094 insertions(+), 1 deletion(-) create mode 100644 document-parser/domain/chunk_editing.py create mode 100644 document-parser/persistence/chunk_edit_repo.py create mode 100644 document-parser/persistence/chunk_repo.py create mode 100644 document-parser/tests/test_chunk_editing.py create mode 100644 document-parser/tests/test_chunk_repos.py diff --git a/document-parser/domain/chunk_editing.py b/document-parser/domain/chunk_editing.py new file mode 100644 index 0000000..bd143b9 --- /dev/null +++ b/document-parser/domain/chunk_editing.py @@ -0,0 +1,216 @@ +"""Pure-domain operations on a chunkset. + +These functions take a chunkset as input and return a new chunkset +(plus, where appropriate, the chunks that were created). They do not +touch the database, do not record audit rows, and do not raise +infrastructure errors. The `ChunkEditingService` (in `services/`) +wraps each call with audit-record generation and atomic persistence. + +All operations preserve `sequence` ordering: insertions and splits +shift subsequent sequences upward; deletions / merges leave gaps. +Sequences are 0-based and only required to be strictly increasing +within a document; gaps are explicitly allowed so we never rewrite +many rows for a single edit. +""" + +from __future__ import annotations + +import uuid +from datetime import UTC, datetime + +from domain.models import Chunk + + +class ChunkEditingError(Exception): + """Raised by chunk-editing operations on invalid input (missing id, + out-of-range offset, etc.). Subclasses `Exception` rather than + `DomainError` so the API layer can map all of them to 4xx without + a wider catch.""" + + +def _utcnow() -> datetime: + return datetime.now(UTC) + + +def _new_id() -> str: + return uuid.uuid4().hex + + +def _index_of(chunks: list[Chunk], chunk_id: str) -> int: + for idx, c in enumerate(chunks): + if c.id == chunk_id: + return idx + raise ChunkEditingError(f"chunk not found: {chunk_id}") + + +def insert( + chunks: list[Chunk], + *, + at_position: int, + text: str, + document_id: str, + headings: list[str] | None = None, + source_page: int | None = None, +) -> tuple[list[Chunk], Chunk]: + """Insert a fresh chunk at position `at_position`. + + Returns the updated chunkset and the new chunk. Subsequent chunks' + sequences are shifted by +1. + """ + if at_position < 0 or at_position > len(chunks): + raise ChunkEditingError(f"insert position out of range: {at_position}") + now = _utcnow() + new_chunk = Chunk( + id=_new_id(), + document_id=document_id, + sequence=at_position, + text=text, + headings=list(headings or []), + source_page=source_page, + created_at=now, + updated_at=now, + ) + out = list(chunks) + for c in out[at_position:]: + c.sequence += 1 + out.insert(at_position, new_chunk) + return out, new_chunk + + +def update( + chunks: list[Chunk], + chunk_id: str, + *, + text: str | None = None, + headings: list[str] | None = None, +) -> tuple[list[Chunk], Chunk]: + """Update text and/or headings of a chunk in-place. + + Returns the updated chunkset and the modified chunk. The chunk's + id and sequence are preserved. + """ + idx = _index_of(chunks, chunk_id) + target = chunks[idx] + if target.deleted_at is not None: + raise ChunkEditingError(f"chunk is deleted: {chunk_id}") + if text is not None: + target.text = text + if headings is not None: + target.headings = list(headings) + target.updated_at = _utcnow() + return list(chunks), target + + +def delete(chunks: list[Chunk], chunk_id: str) -> tuple[list[Chunk], Chunk]: + """Soft-delete a chunk. Returns the updated chunkset and the + deleted chunk (still present in the list, with `deleted_at` set).""" + idx = _index_of(chunks, chunk_id) + target = chunks[idx] + if target.deleted_at is not None: + return list(chunks), target # idempotent + target.deleted_at = _utcnow() + target.updated_at = target.deleted_at + return list(chunks), target + + +def merge( + chunks: list[Chunk], + chunk_ids: list[str], + *, + separator: str = "\n", +) -> tuple[list[Chunk], Chunk]: + """Merge `chunk_ids` (in order) into a single new chunk. + + The new chunk takes the headings of the first source and the + smallest source page. The `id`s of the sources are returned in + the new chunk's lineage via the audit row written by the service + layer. + + Returns the updated chunkset and the new merged chunk. The sources + are removed from the chunkset (hard-removed from the list — the + service is responsible for soft-deleting their persisted rows so + history queries still resolve them). + """ + if len(chunk_ids) < 2: + raise ChunkEditingError("merge requires at least two chunks") + indices = [_index_of(chunks, cid) for cid in chunk_ids] + sources = [chunks[i] for i in indices] + if any(c.deleted_at for c in sources): + raise ChunkEditingError("cannot merge a deleted chunk") + + document_id = sources[0].document_id + if any(c.document_id != document_id for c in sources): + raise ChunkEditingError("merge across documents is not allowed") + + merged_text = separator.join(c.text for c in sources) + merged_headings = list(sources[0].headings) + merged_page = min((c.source_page for c in sources if c.source_page is not None), default=None) + merged_sequence = min(c.sequence for c in sources) + + now = _utcnow() + new_chunk = Chunk( + id=_new_id(), + document_id=document_id, + sequence=merged_sequence, + text=merged_text, + headings=merged_headings, + source_page=merged_page, + created_at=now, + updated_at=now, + ) + + source_ids = {c.id for c in sources} + out = [c for c in chunks if c.id not in source_ids] + out.append(new_chunk) + out.sort(key=lambda c: c.sequence) + return out, new_chunk + + +def split( + chunks: list[Chunk], chunk_id: str, *, at_offset: int +) -> tuple[list[Chunk], Chunk, Chunk]: + """Split a chunk's text at `at_offset` into two new chunks. + + The source chunk is removed from the chunkset (the service-layer + persistence path soft-deletes its row so history queries can + resolve it). The two new chunks inherit headings and source_page + from the source. + + Returns the updated chunkset and the two new chunks `(left, right)`. + """ + idx = _index_of(chunks, chunk_id) + target = chunks[idx] + if target.deleted_at is not None: + raise ChunkEditingError(f"chunk is deleted: {chunk_id}") + if at_offset <= 0 or at_offset >= len(target.text): + raise ChunkEditingError(f"split offset out of range for chunk of length {len(target.text)}") + + now = _utcnow() + left = Chunk( + id=_new_id(), + document_id=target.document_id, + sequence=target.sequence, + text=target.text[:at_offset], + headings=list(target.headings), + source_page=target.source_page, + created_at=now, + updated_at=now, + ) + right = Chunk( + id=_new_id(), + document_id=target.document_id, + sequence=target.sequence + 1, + text=target.text[at_offset:], + headings=list(target.headings), + source_page=target.source_page, + created_at=now, + updated_at=now, + ) + + out = [c for c in chunks if c.id != target.id] + for c in out: + if c.sequence > target.sequence: + c.sequence += 1 + out.extend((left, right)) + out.sort(key=lambda c: c.sequence) + return out, left, right diff --git a/document-parser/domain/models.py b/document-parser/domain/models.py index 7c05040..826178c 100644 --- a/document-parser/domain/models.py +++ b/document-parser/domain/models.py @@ -10,6 +10,9 @@ from datetime import UTC, datetime from domain.events import DocumentLifecycleChanged from domain.lifecycle import assert_transition from domain.value_objects import ( + ChunkBbox, + ChunkDocItem, + ChunkEditAction, DocumentLifecycleState, DocumentStoreLinkState, StoreKind, @@ -201,3 +204,77 @@ class DocumentStoreLink: """Record a failed push attempt.""" self.state = DocumentStoreLinkState.FAILED self.error_message = error + + +@dataclass +class Chunk: + """A persisted chunk — first-class entity introduced by #205. + + Replaces the legacy `analysis_jobs.chunks_json` blob. The `id` is + stable across edits except for split/merge which produce new chunks + with new ids; the lineage is recorded in `chunk_edits` rows. + + `sequence` controls ordering within a document. Gaps are allowed — + splits push subsequent chunks' sequences without rewriting them. + + `deleted_at` is non-null when the chunk has been soft-deleted; the + row stays in the table so the audit trail keeps its before/after + pointers valid. + """ + + id: str = field(default_factory=_new_id) + document_id: str = "" + sequence: int = 0 + text: str = "" + headings: list[str] = field(default_factory=list) + source_page: int | None = None + bboxes: list[ChunkBbox] = field(default_factory=list) + doc_items: list[ChunkDocItem] = field(default_factory=list) + token_count: int | None = None + created_at: datetime = field(default_factory=_utcnow) + updated_at: datetime = field(default_factory=_utcnow) + deleted_at: datetime | None = None + + +@dataclass(frozen=True) +class ChunkEdit: + """An immutable audit record describing one mutating operation on + the chunkset of a document. Written atomically with the chunk + write it describes. + + `before` and `after` are JSON-serializable snapshots of the chunk + state. They are `None` for the start/end of the chunk's life: + - `before is None` for INSERT + - `after is None` for DELETE + - For MERGE: a single result row carries `parents = [chunk_ids…]` + - For SPLIT: two result rows each carry `parents = [source_id]` + and the source's edit row carries `children = [new_id, new_id]` + """ + + id: str + document_id: str + chunk_id: str | None + action: ChunkEditAction + actor: str + at: datetime + before: dict | None = None + after: dict | None = None + parents: list[str] = field(default_factory=list) + children: list[str] = field(default_factory=list) + reason: str | None = None + + +@dataclass(frozen=True) +class ChunkPush: + """Snapshot of which chunks were pushed to which store, when. + + Lets the API answer 'show me the chunkset that was in store X at + push N' without replaying the full audit log. + """ + + id: str + document_id: str + store_id: str + chunkset_hash: str + chunk_ids: list[str] + pushed_at: datetime diff --git a/document-parser/domain/ports.py b/document-parser/domain/ports.py index d6929b4..f46c8b7 100644 --- a/document-parser/domain/ports.py +++ b/document-parser/domain/ports.py @@ -11,7 +11,15 @@ from typing import TYPE_CHECKING, Protocol, runtime_checkable if TYPE_CHECKING: from datetime import datetime - from domain.models import AnalysisJob, Document, DocumentStoreLink, Store + from domain.models import ( + AnalysisJob, + Chunk, + ChunkEdit, + ChunkPush, + Document, + DocumentStoreLink, + Store, + ) from domain.value_objects import ( ChunkingOptions, ChunkResult, @@ -128,6 +136,53 @@ class DocumentStoreLinkRepository(Protocol): async def delete(self, document_id: str, store_id: str) -> bool: ... +class ChunkRepository(Protocol): + """Port for first-class chunk persistence (introduced by #205).""" + + async def insert(self, chunk: Chunk) -> None: ... + + async def insert_many(self, chunks: list[Chunk]) -> None: ... + + async def update(self, chunk: Chunk) -> None: ... + + async def soft_delete(self, chunk_id: str, *, at: datetime) -> bool: ... + + async def find_for_document( + self, + document_id: str, + *, + include_deleted: bool = False, + ) -> list[Chunk]: ... + + async def find_by_id(self, chunk_id: str) -> Chunk | None: ... + + +class ChunkEditRepository(Protocol): + """Port for the immutable chunk_edits audit log (introduced by #205).""" + + async def insert(self, edit: ChunkEdit) -> None: ... + + async def find_for_document( + self, + document_id: str, + *, + limit: int = 50, + offset: int = 0, + ) -> list[ChunkEdit]: ... + + async def find_for_chunk(self, chunk_id: str) -> list[ChunkEdit]: ... + + +class ChunkPushRepository(Protocol): + """Port for chunk_pushes snapshots (introduced by #205).""" + + async def insert(self, push: ChunkPush) -> None: ... + + async def find_by_id(self, push_id: str) -> ChunkPush | None: ... + + async def find_latest(self, document_id: str, store_id: str) -> ChunkPush | None: ... + + class AnalysisRepository(Protocol): """Port for analysis job persistence.""" diff --git a/document-parser/domain/value_objects.py b/document-parser/domain/value_objects.py index d3f1baf..cf9b258 100644 --- a/document-parser/domain/value_objects.py +++ b/document-parser/domain/value_objects.py @@ -61,6 +61,20 @@ class DocumentStoreLinkState(StrEnum): FAILED = "Failed" +class ChunkEditAction(StrEnum): + """The five mutating operations the chunks editor supports. + + Recorded on every `ChunkEdit` row so the audit trail can answer "who + did what, when, and why" without resorting to JSON-path matching. + """ + + INSERT = "insert" + UPDATE = "update" + DELETE = "delete" + MERGE = "merge" + SPLIT = "split" + + @dataclass(frozen=True) class PageElement: type: str diff --git a/document-parser/persistence/chunk_edit_repo.py b/document-parser/persistence/chunk_edit_repo.py new file mode 100644 index 0000000..eadaf34 --- /dev/null +++ b/document-parser/persistence/chunk_edit_repo.py @@ -0,0 +1,145 @@ +"""Chunk edit / push repositories — immutable audit log + push snapshots (#205).""" + +from __future__ import annotations + +import json +from datetime import UTC, datetime + +from domain.models import ChunkEdit, ChunkPush +from domain.value_objects import ChunkEditAction +from persistence.database import get_connection + + +def _parse_iso(value: str | None) -> datetime | None: + if value is None or value == "": + return None + parsed = datetime.fromisoformat(value) + if parsed.tzinfo is None: + parsed = parsed.replace(tzinfo=UTC) + return parsed + + +def _row_to_edit(row) -> ChunkEdit: + return ChunkEdit( + id=row["id"], + document_id=row["document_id"], + chunk_id=row["chunk_id"], + action=ChunkEditAction(row["action"]), + actor=row["actor"], + at=_parse_iso(row["at"]) or datetime.now(UTC), + before=json.loads(row["before_json"]) if row["before_json"] else None, + after=json.loads(row["after_json"]) if row["after_json"] else None, + parents=json.loads(row["parents_json"]) if row["parents_json"] else [], + children=json.loads(row["children_json"]) if row["children_json"] else [], + reason=row["reason"], + ) + + +def _row_to_push(row) -> ChunkPush: + return ChunkPush( + id=row["id"], + document_id=row["document_id"], + store_id=row["store_id"], + chunkset_hash=row["chunkset_hash"], + chunk_ids=json.loads(row["chunk_ids"]) if row["chunk_ids"] else [], + pushed_at=_parse_iso(row["pushed_at"]) or datetime.now(UTC), + ) + + +class SqliteChunkEditRepository: + """SQLite implementation of the ChunkEditRepository port. + + The audit log is append-only: this repo offers `insert` and reads, + but no update or delete. Aborted edits should not produce an audit + row in the first place — that contract is enforced in the service + layer (audit + chunk write happen in the same SQL transaction). + """ + + async def insert(self, edit: ChunkEdit) -> None: + async with get_connection() as db: + await db.execute( + """INSERT INTO chunk_edits + (id, document_id, chunk_id, action, actor, at, + before_json, after_json, parents_json, children_json, reason) + VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)""", + ( + edit.id, + edit.document_id, + edit.chunk_id, + edit.action.value, + edit.actor, + str(edit.at), + json.dumps(edit.before) if edit.before is not None else None, + json.dumps(edit.after) if edit.after is not None else None, + json.dumps(edit.parents), + json.dumps(edit.children), + edit.reason, + ), + ) + await db.commit() + + async def find_for_document( + self, + document_id: str, + *, + limit: int = 50, + offset: int = 0, + ) -> list[ChunkEdit]: + async with get_connection() as db: + cursor = await db.execute( + """SELECT * FROM chunk_edits + WHERE document_id = ? + ORDER BY at DESC + LIMIT ? OFFSET ?""", + (document_id, limit, offset), + ) + rows = await cursor.fetchall() + return [_row_to_edit(r) for r in rows] + + async def find_for_chunk(self, chunk_id: str) -> list[ChunkEdit]: + async with get_connection() as db: + cursor = await db.execute( + "SELECT * FROM chunk_edits WHERE chunk_id = ? ORDER BY at ASC", + (chunk_id,), + ) + rows = await cursor.fetchall() + return [_row_to_edit(r) for r in rows] + + +class SqliteChunkPushRepository: + """SQLite implementation of the ChunkPushRepository port.""" + + async def insert(self, push: ChunkPush) -> None: + async with get_connection() as db: + await db.execute( + """INSERT INTO chunk_pushes + (id, document_id, store_id, chunkset_hash, chunk_ids, pushed_at) + VALUES (?, ?, ?, ?, ?, ?)""", + ( + push.id, + push.document_id, + push.store_id, + push.chunkset_hash, + json.dumps(push.chunk_ids), + str(push.pushed_at), + ), + ) + await db.commit() + + async def find_by_id(self, push_id: str) -> ChunkPush | None: + async with get_connection() as db: + cursor = await db.execute("SELECT * FROM chunk_pushes WHERE id = ?", (push_id,)) + row = await cursor.fetchone() + return _row_to_push(row) if row else None + + async def find_latest(self, document_id: str, store_id: str) -> ChunkPush | None: + async with get_connection() as db: + cursor = await db.execute( + """SELECT * FROM chunk_pushes + WHERE document_id = ? AND store_id = ? + ORDER BY pushed_at DESC + LIMIT 1""", + (document_id, store_id), + ) + row = await cursor.fetchone() + return _row_to_push(row) if row else None diff --git a/document-parser/persistence/chunk_repo.py b/document-parser/persistence/chunk_repo.py new file mode 100644 index 0000000..c0aa24c --- /dev/null +++ b/document-parser/persistence/chunk_repo.py @@ -0,0 +1,134 @@ +"""Chunk repository — SQLite CRUD for first-class chunks (#205).""" + +from __future__ import annotations + +import json +from dataclasses import asdict +from datetime import UTC, datetime + +from domain.models import Chunk +from domain.value_objects import ChunkBbox, ChunkDocItem +from persistence.database import get_connection + + +def _parse_iso(value: str | None) -> datetime | None: + if value is None or value == "": + return None + parsed = datetime.fromisoformat(value) + if parsed.tzinfo is None: + parsed = parsed.replace(tzinfo=UTC) + return parsed + + +def _row_to_chunk(row) -> Chunk: + headings = json.loads(row["headings"]) if row["headings"] else [] + bboxes_raw = json.loads(row["bboxes"]) if row["bboxes"] else [] + doc_items_raw = json.loads(row["doc_items"]) if row["doc_items"] else [] + return Chunk( + id=row["id"], + document_id=row["document_id"], + sequence=row["sequence"], + text=row["text"], + headings=headings, + source_page=row["source_page"], + bboxes=[ChunkBbox(page=b["page"], bbox=b["bbox"]) for b in bboxes_raw], + doc_items=[ChunkDocItem(self_ref=d["self_ref"], label=d["label"]) for d in doc_items_raw], + token_count=row["token_count"], + created_at=_parse_iso(row["created_at"]) or datetime.now(UTC), + updated_at=_parse_iso(row["updated_at"]) or datetime.now(UTC), + deleted_at=_parse_iso(row["deleted_at"]), + ) + + +def _chunk_to_params(c: Chunk) -> tuple: + return ( + c.id, + c.document_id, + c.sequence, + c.text, + json.dumps(c.headings), + c.source_page, + json.dumps([asdict(b) for b in c.bboxes]), + json.dumps([asdict(d) for d in c.doc_items]), + c.token_count, + str(c.created_at), + str(c.updated_at), + str(c.deleted_at) if c.deleted_at else None, + ) + + +_INSERT_SQL = """INSERT INTO chunks + (id, document_id, sequence, text, headings, source_page, + bboxes, doc_items, token_count, created_at, updated_at, deleted_at) + VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)""" + + +class SqliteChunkRepository: + """SQLite implementation of the ChunkRepository port.""" + + async def insert(self, chunk: Chunk) -> None: + async with get_connection() as db: + await db.execute(_INSERT_SQL, _chunk_to_params(chunk)) + await db.commit() + + async def insert_many(self, chunks: list[Chunk]) -> None: + if not chunks: + return + async with get_connection() as db: + await db.executemany(_INSERT_SQL, [_chunk_to_params(c) for c in chunks]) + await db.commit() + + async def update(self, chunk: Chunk) -> None: + async with get_connection() as db: + await db.execute( + """UPDATE chunks SET + sequence = ?, text = ?, headings = ?, source_page = ?, + bboxes = ?, doc_items = ?, token_count = ?, + updated_at = ?, deleted_at = ? + WHERE id = ?""", + ( + chunk.sequence, + chunk.text, + json.dumps(chunk.headings), + chunk.source_page, + json.dumps([asdict(b) for b in chunk.bboxes]), + json.dumps([asdict(d) for d in chunk.doc_items]), + chunk.token_count, + str(chunk.updated_at), + str(chunk.deleted_at) if chunk.deleted_at else None, + chunk.id, + ), + ) + await db.commit() + + async def soft_delete(self, chunk_id: str, *, at: datetime) -> bool: + async with get_connection() as db: + cursor = await db.execute( + "UPDATE chunks SET deleted_at = ?, updated_at = ? WHERE id = ?", + (str(at), str(at), chunk_id), + ) + await db.commit() + return cursor.rowcount > 0 + + async def find_for_document( + self, + document_id: str, + *, + include_deleted: bool = False, + ) -> list[Chunk]: + clause = "" if include_deleted else " AND deleted_at IS NULL" + async with get_connection() as db: + cursor = await db.execute( + f"""SELECT * FROM chunks + WHERE document_id = ?{clause} + ORDER BY sequence ASC""", + (document_id,), + ) + rows = await cursor.fetchall() + return [_row_to_chunk(r) for r in rows] + + async def find_by_id(self, chunk_id: str) -> Chunk | None: + async with get_connection() as db: + cursor = await db.execute("SELECT * FROM chunks WHERE id = ?", (chunk_id,)) + row = await cursor.fetchone() + return _row_to_chunk(row) if row else None diff --git a/document-parser/persistence/database.py b/document-parser/persistence/database.py index 7398f4c..526974b 100644 --- a/document-parser/persistence/database.py +++ b/document-parser/persistence/database.py @@ -70,6 +70,50 @@ CREATE TABLE IF NOT EXISTS document_store_links ( CREATE INDEX IF NOT EXISTS idx_dsl_doc ON document_store_links(document_id); CREATE INDEX IF NOT EXISTS idx_dsl_store ON document_store_links(store_id); CREATE INDEX IF NOT EXISTS idx_dsl_state ON document_store_links(state); + +-- 0.6.0 — Chunks promoted to first-class entities + audit trail (#205). +CREATE TABLE IF NOT EXISTS chunks ( + id TEXT PRIMARY KEY, + document_id TEXT NOT NULL REFERENCES documents(id) ON DELETE CASCADE, + sequence INTEGER NOT NULL, + text TEXT NOT NULL, + headings TEXT NOT NULL DEFAULT '[]', + source_page INTEGER, + bboxes TEXT NOT NULL DEFAULT '[]', + doc_items TEXT NOT NULL DEFAULT '[]', + token_count INTEGER, + created_at TEXT NOT NULL, + updated_at TEXT NOT NULL, + deleted_at TEXT +); +CREATE INDEX IF NOT EXISTS idx_chunks_doc ON chunks(document_id); +CREATE INDEX IF NOT EXISTS idx_chunks_doc_seq ON chunks(document_id, sequence); + +CREATE TABLE IF NOT EXISTS chunk_edits ( + id TEXT PRIMARY KEY, + document_id TEXT NOT NULL REFERENCES documents(id) ON DELETE CASCADE, + chunk_id TEXT, + action TEXT NOT NULL, + actor TEXT NOT NULL DEFAULT 'system', + at TEXT NOT NULL, + before_json TEXT, + after_json TEXT, + parents_json TEXT NOT NULL DEFAULT '[]', + children_json TEXT NOT NULL DEFAULT '[]', + reason TEXT +); +CREATE INDEX IF NOT EXISTS idx_chunk_edits_doc_at ON chunk_edits(document_id, at); +CREATE INDEX IF NOT EXISTS idx_chunk_edits_chunk ON chunk_edits(chunk_id); + +CREATE TABLE IF NOT EXISTS chunk_pushes ( + id TEXT PRIMARY KEY, + document_id TEXT NOT NULL REFERENCES documents(id) ON DELETE CASCADE, + store_id TEXT NOT NULL REFERENCES stores(id) ON DELETE CASCADE, + chunkset_hash TEXT NOT NULL, + chunk_ids TEXT NOT NULL, + pushed_at TEXT NOT NULL +); +CREATE INDEX IF NOT EXISTS idx_chunk_pushes_doc_store ON chunk_pushes(document_id, store_id); """ diff --git a/document-parser/tests/test_chunk_editing.py b/document-parser/tests/test_chunk_editing.py new file mode 100644 index 0000000..38b8bbc --- /dev/null +++ b/document-parser/tests/test_chunk_editing.py @@ -0,0 +1,196 @@ +"""Tests for the pure-domain chunk-editing operations (#205).""" + +from __future__ import annotations + +import pytest + +from domain.chunk_editing import ( + ChunkEditingError, + delete, + insert, + merge, + split, + update, +) +from domain.models import Chunk + + +def _make(document_id: str = "doc-1", count: int = 3) -> list[Chunk]: + out: list[Chunk] = [] + for i in range(count): + out.append( + Chunk( + id=f"c-{i}", + document_id=document_id, + sequence=i, + text=f"text {i}", + headings=[f"H{i}"], + source_page=i + 1, + ) + ) + return out + + +# --------------------------------------------------------------------------- +# insert +# --------------------------------------------------------------------------- + + +def test_insert_at_end_appends_chunk_with_correct_sequence() -> None: + chunks = _make(count=2) + out, new = insert(chunks, at_position=2, text="new", document_id="doc-1") + assert len(out) == 3 + assert out[2].id == new.id + assert new.sequence == 2 + # Existing chunks untouched. + assert out[0].sequence == 0 + assert out[1].sequence == 1 + + +def test_insert_in_middle_shifts_subsequent_sequences() -> None: + chunks = _make(count=3) + out, new = insert(chunks, at_position=1, text="middle", document_id="doc-1") + sequences = [c.sequence for c in out] + assert sequences == [0, 1, 2, 3] + assert out[1].id == new.id + # Original chunk-1 was shifted to sequence 2. + assert next(c for c in out if c.id == "c-1").sequence == 2 + + +def test_insert_out_of_range_raises() -> None: + chunks = _make(count=2) + with pytest.raises(ChunkEditingError): + insert(chunks, at_position=99, text="x", document_id="doc-1") + + +# --------------------------------------------------------------------------- +# update +# --------------------------------------------------------------------------- + + +def test_update_changes_text_in_place() -> None: + chunks = _make() + out, modified = update(chunks, "c-1", text="edited") + assert modified.id == "c-1" + assert modified.text == "edited" + # Identity preserved. + assert next(c for c in out if c.id == "c-1").text == "edited" + + +def test_update_can_change_headings_only() -> None: + chunks = _make() + _, modified = update(chunks, "c-0", headings=["X"]) + assert modified.headings == ["X"] + + +def test_update_unknown_chunk_raises() -> None: + chunks = _make() + with pytest.raises(ChunkEditingError): + update(chunks, "missing", text="x") + + +def test_update_deleted_chunk_raises() -> None: + chunks = _make() + delete(chunks, "c-1") + with pytest.raises(ChunkEditingError): + update(chunks, "c-1", text="x") + + +# --------------------------------------------------------------------------- +# delete +# --------------------------------------------------------------------------- + + +def test_delete_marks_deleted_at() -> None: + chunks = _make() + out, deleted = delete(chunks, "c-1") + target = next(c for c in out if c.id == "c-1") + assert target.deleted_at is not None + assert deleted is target + + +def test_delete_is_idempotent() -> None: + chunks = _make() + out_a, _ = delete(chunks, "c-1") + out_b, _ = delete(out_a, "c-1") + target_a = next(c for c in out_a if c.id == "c-1") + target_b = next(c for c in out_b if c.id == "c-1") + assert target_a.deleted_at == target_b.deleted_at + + +# --------------------------------------------------------------------------- +# merge +# --------------------------------------------------------------------------- + + +def test_merge_removes_sources_and_returns_new_chunk() -> None: + chunks = _make(count=3) + out, merged = merge(chunks, ["c-0", "c-1"]) + ids = {c.id for c in out} + assert "c-0" not in ids + assert "c-1" not in ids + assert merged.id in ids + assert merged.text == "text 0\ntext 1" + # Merged chunk inherits headings from first source. + assert merged.headings == ["H0"] + # Source page is the min. + assert merged.source_page == 1 + + +def test_merge_requires_at_least_two_chunks() -> None: + chunks = _make() + with pytest.raises(ChunkEditingError): + merge(chunks, ["c-0"]) + + +def test_merge_across_documents_raises() -> None: + chunks = _make(document_id="doc-1", count=2) + other = Chunk(id="x-0", document_id="doc-2", sequence=10, text="other") + chunks.append(other) + with pytest.raises(ChunkEditingError): + merge(chunks, ["c-0", "x-0"]) + + +# --------------------------------------------------------------------------- +# split +# --------------------------------------------------------------------------- + + +def test_split_creates_two_new_chunks() -> None: + chunks = _make(count=2) + out, left, right = split(chunks, "c-0", at_offset=4) + assert left.text == "text" + assert right.text == " 0" + ids = {c.id for c in out} + # Source replaced by two new chunks. + assert "c-0" not in ids + assert left.id in ids + assert right.id in ids + + +def test_split_preserves_headings_on_both_halves() -> None: + chunks = _make(count=1) + _, left, right = split(chunks, "c-0", at_offset=2) + assert left.headings == ["H0"] + assert right.headings == ["H0"] + + +def test_split_shifts_subsequent_sequences() -> None: + chunks = _make(count=3) + out, _, _ = split(chunks, "c-0", at_offset=2) + # After split, c-1 (was seq=1) and c-2 (was seq=2) shift up by 1. + seqs = sorted(c.sequence for c in out) + assert seqs == [0, 1, 2, 3] + + +def test_split_at_zero_offset_raises() -> None: + chunks = _make() + with pytest.raises(ChunkEditingError): + split(chunks, "c-0", at_offset=0) + + +def test_split_beyond_text_length_raises() -> None: + chunks = _make() + target = next(c for c in chunks if c.id == "c-0") + with pytest.raises(ChunkEditingError): + split(chunks, "c-0", at_offset=len(target.text)) diff --git a/document-parser/tests/test_chunk_repos.py b/document-parser/tests/test_chunk_repos.py new file mode 100644 index 0000000..93174c6 --- /dev/null +++ b/document-parser/tests/test_chunk_repos.py @@ -0,0 +1,212 @@ +"""Tests for the chunk / chunk_edit / chunk_push repositories (#205).""" + +from __future__ import annotations + +from datetime import UTC, datetime + +import pytest + +from domain.models import Chunk, ChunkEdit, ChunkPush, Document +from domain.value_objects import ChunkBbox, ChunkEditAction +from persistence.chunk_edit_repo import ( + SqliteChunkEditRepository, + SqliteChunkPushRepository, +) +from persistence.chunk_repo import SqliteChunkRepository +from persistence.database import init_db +from persistence.document_repo import SqliteDocumentRepository + + +@pytest.fixture(autouse=True) +async def setup_db(monkeypatch, tmp_path): + db_path = str(tmp_path / "test.db") + monkeypatch.setattr("persistence.database.DB_PATH", db_path) + await init_db() + yield + + +@pytest.fixture +async def doc(): + repo = SqliteDocumentRepository() + document = Document(id="doc-1", filename="t.pdf", storage_path="/tmp/t.pdf") + await repo.insert(document) + return document + + +@pytest.fixture +def chunk_repo(): + return SqliteChunkRepository() + + +@pytest.fixture +def edit_repo(): + return SqliteChunkEditRepository() + + +@pytest.fixture +def push_repo(): + return SqliteChunkPushRepository() + + +class TestChunkRepo: + async def test_insert_and_find_for_document(self, doc, chunk_repo): + c = Chunk( + id="c-1", + document_id=doc.id, + sequence=0, + text="alpha", + headings=["A"], + source_page=1, + bboxes=[ChunkBbox(page=1, bbox=[0, 0, 10, 10])], + ) + await chunk_repo.insert(c) + + out = await chunk_repo.find_for_document(doc.id) + assert len(out) == 1 + assert out[0].id == "c-1" + assert out[0].text == "alpha" + assert out[0].headings == ["A"] + assert out[0].bboxes[0].bbox == [0, 0, 10, 10] + + async def test_insert_many_round_trips(self, doc, chunk_repo): + chunks = [ + Chunk(id=f"c-{i}", document_id=doc.id, sequence=i, text=f"t{i}") for i in range(5) + ] + await chunk_repo.insert_many(chunks) + + out = await chunk_repo.find_for_document(doc.id) + assert len(out) == 5 + # Returned in sequence order. + assert [c.id for c in out] == [f"c-{i}" for i in range(5)] + + async def test_soft_delete_excludes_chunk_unless_requested(self, doc, chunk_repo): + await chunk_repo.insert(Chunk(id="c-1", document_id=doc.id, sequence=0, text="a")) + await chunk_repo.soft_delete("c-1", at=datetime(2026, 4, 29, 12, tzinfo=UTC)) + + active = await chunk_repo.find_for_document(doc.id) + assert active == [] + + all_chunks = await chunk_repo.find_for_document(doc.id, include_deleted=True) + assert len(all_chunks) == 1 + assert all_chunks[0].deleted_at is not None + + async def test_update_persists_text_and_sequence(self, doc, chunk_repo): + c = Chunk(id="c-1", document_id=doc.id, sequence=0, text="old") + await chunk_repo.insert(c) + + c.text = "new" + c.sequence = 5 + c.updated_at = datetime(2026, 4, 29, 12, tzinfo=UTC) + await chunk_repo.update(c) + + out = await chunk_repo.find_by_id("c-1") + assert out is not None + assert out.text == "new" + assert out.sequence == 5 + + async def test_cascade_delete_with_document(self, doc, chunk_repo): + await chunk_repo.insert(Chunk(id="c-1", document_id=doc.id, sequence=0, text="a")) + await SqliteDocumentRepository().delete(doc.id) + out = await chunk_repo.find_for_document(doc.id, include_deleted=True) + assert out == [] + + +class TestChunkEditRepo: + async def test_insert_and_history(self, doc, edit_repo): + edit = ChunkEdit( + id="e-1", + document_id=doc.id, + chunk_id="c-1", + action=ChunkEditAction.UPDATE, + actor="user@example", + at=datetime(2026, 4, 29, 12, tzinfo=UTC), + before={"text": "old"}, + after={"text": "new"}, + reason="typo", + ) + await edit_repo.insert(edit) + + out = await edit_repo.find_for_document(doc.id) + assert len(out) == 1 + assert out[0].action == ChunkEditAction.UPDATE + assert out[0].before == {"text": "old"} + assert out[0].after == {"text": "new"} + assert out[0].reason == "typo" + + async def test_history_orders_newest_first(self, doc, edit_repo): + for i in range(3): + await edit_repo.insert( + ChunkEdit( + id=f"e-{i}", + document_id=doc.id, + chunk_id=f"c-{i}", + action=ChunkEditAction.INSERT, + actor="system", + at=datetime(2026, 4, 29, 12 + i, tzinfo=UTC), + ) + ) + out = await edit_repo.find_for_document(doc.id) + ids = [e.id for e in out] + assert ids == ["e-2", "e-1", "e-0"] + + async def test_find_for_chunk_filters(self, doc, edit_repo): + for i in range(3): + await edit_repo.insert( + ChunkEdit( + id=f"e-{i}", + document_id=doc.id, + chunk_id="c-target" if i == 1 else f"c-{i}", + action=ChunkEditAction.UPDATE, + actor="system", + at=datetime(2026, 4, 29, 12 + i, tzinfo=UTC), + ) + ) + out = await edit_repo.find_for_chunk("c-target") + assert [e.id for e in out] == ["e-1"] + + async def test_lineage_round_trips(self, doc, edit_repo): + edit = ChunkEdit( + id="e-merge", + document_id=doc.id, + chunk_id="c-merged", + action=ChunkEditAction.MERGE, + actor="system", + at=datetime(2026, 4, 29, 12, tzinfo=UTC), + parents=["c-1", "c-2"], + children=["c-merged"], + ) + await edit_repo.insert(edit) + out = await edit_repo.find_for_document(doc.id) + assert out[0].parents == ["c-1", "c-2"] + assert out[0].children == ["c-merged"] + + +class TestChunkPushRepo: + async def test_insert_and_find_latest(self, doc, push_repo): + first = ChunkPush( + id="p-1", + document_id=doc.id, + store_id="default", + chunkset_hash="hash-old", + chunk_ids=["c-1", "c-2"], + pushed_at=datetime(2026, 4, 29, 10, tzinfo=UTC), + ) + latest = ChunkPush( + id="p-2", + document_id=doc.id, + store_id="default", + chunkset_hash="hash-new", + chunk_ids=["c-1", "c-3"], + pushed_at=datetime(2026, 4, 29, 11, tzinfo=UTC), + ) + await push_repo.insert(first) + await push_repo.insert(latest) + + found = await push_repo.find_latest(doc.id, "default") + assert found is not None + assert found.id == "p-2" + assert found.chunkset_hash == "hash-new" + assert found.chunk_ids == ["c-1", "c-3"] + + async def test_find_latest_returns_none_when_absent(self, doc, push_repo): + assert await push_repo.find_latest(doc.id, "default") is None From 0c092748362d5bed7384d4356648c665719336c6 Mon Sep 17 00:00:00 2001 From: Pier-Jean Malandrino Date: Wed, 29 Apr 2026 15:48:28 +0200 Subject: [PATCH 07/35] chore(#206): backfill 0.6.0 doc-centric data model MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CLI script to migrate existing tenants to the data model introduced by #202-205. Idempotent and resumable — re-running is a no-op. tools/migrate_06.py - Step 1: backfill_lifecycle — infers documents.lifecycle_state from analysis_jobs.status + chunks_json presence Failed analyses, no completed -> Failed Completed + chunks_json -> Chunked Completed, no chunks_json -> Parsed Otherwise -> Uploaded - Step 2: materialize_chunks — promotes the latest chunks_json blob for each doc into rows in the new chunks table. Stable id derivation via uuid5(NAMESPACE_OID, '||') so re-running lands on the same ids - Step 3: backfill_links — for any doc that has chunks materialized, creates a (doc, default-store) link in Ingested state with a freshly computed chunkset_hash. Treats 'chunks exist' as proxy for 'doc was ingested into the legacy single index' — sufficient for the typical pre-0.6.0 deployment, with OpenSearch reindex documented separately - Step 4: reaggregate — applies #203's aggregate_lifecycle rule so doc-chunked transitions to Ingested Persistence - migration_progress table for resumability + per-step idempotency CLI python -m tools.migrate_06 # full migration python -m tools.migrate_06 --dry-run # plan only, no writes python -m tools.migrate_06 --only-step # rerun a phase Tests - 9 tests: inference rule per (completed, failed, chunks_json) tuple, end-to-end migration on a hand-built three-doc snapshot, idempotency (second run = no writes), --dry-run writes nothing, deterministic chunk ids across reruns Refs #206 --- document-parser/persistence/database.py | 6 + document-parser/tests/test_migrate_06.py | 185 ++++++++++ document-parser/tools/migrate_06.py | 433 +++++++++++++++++++++++ 3 files changed, 624 insertions(+) create mode 100644 document-parser/tests/test_migrate_06.py create mode 100644 document-parser/tools/migrate_06.py diff --git a/document-parser/persistence/database.py b/document-parser/persistence/database.py index 526974b..5544f2b 100644 --- a/document-parser/persistence/database.py +++ b/document-parser/persistence/database.py @@ -114,6 +114,12 @@ CREATE TABLE IF NOT EXISTS chunk_pushes ( pushed_at TEXT NOT NULL ); CREATE INDEX IF NOT EXISTS idx_chunk_pushes_doc_store ON chunk_pushes(document_id, store_id); + +-- 0.6.0 — migration progress (resumability for the 0.6.0 backfill, #206). +CREATE TABLE IF NOT EXISTS migration_progress ( + name TEXT PRIMARY KEY, + completed_at TEXT NOT NULL DEFAULT (datetime('now')) +); """ diff --git a/document-parser/tests/test_migrate_06.py b/document-parser/tests/test_migrate_06.py new file mode 100644 index 0000000..20d3ce0 --- /dev/null +++ b/document-parser/tests/test_migrate_06.py @@ -0,0 +1,185 @@ +"""Tests for the 0.6.0 migration script (#206).""" + +from __future__ import annotations + +import json + +import pytest + +from domain.value_objects import DocumentLifecycleState, DocumentStoreLinkState +from persistence.chunk_repo import SqliteChunkRepository +from persistence.database import get_connection, init_db +from persistence.document_repo import SqliteDocumentRepository +from persistence.document_store_link_repo import SqliteDocumentStoreLinkRepository +from tools.migrate_06 import _DocFacts, _infer_lifecycle, run + + +@pytest.fixture(autouse=True) +async def setup_db(monkeypatch, tmp_path): + db_path = str(tmp_path / "test.db") + monkeypatch.setattr("persistence.database.DB_PATH", db_path) + await init_db() + yield + + +# --------------------------------------------------------------------------- +# Inference rule unit tests +# --------------------------------------------------------------------------- + + +@pytest.mark.parametrize( + ("has_completed", "has_failed", "has_chunks_json", "expected"), + [ + (False, False, False, DocumentLifecycleState.UPLOADED), + (False, True, False, DocumentLifecycleState.FAILED), + (True, False, False, DocumentLifecycleState.PARSED), + (True, False, True, DocumentLifecycleState.CHUNKED), + (True, True, True, DocumentLifecycleState.CHUNKED), # any completed wins + ], +) +def test_infer_lifecycle( + has_completed: bool, + has_failed: bool, + has_chunks_json: bool, + expected: DocumentLifecycleState, +) -> None: + facts = _DocFacts( + document_id="doc", + has_completed=has_completed, + has_failed=has_failed, + has_chunks_json=has_chunks_json, + ) + assert _infer_lifecycle(facts) == expected + + +# --------------------------------------------------------------------------- +# Integration: end-to-end migration on a hand-built pre-state +# --------------------------------------------------------------------------- + + +async def _seed_pre_06_state() -> None: + """Seed a tenant snapshot with three documents covering the three + main inference cases: + + doc-uploaded — only an `Uploaded` doc row, nothing else + doc-parsed — completed analysis, no chunks + doc-chunked — completed analysis with chunks_json + """ + repo = SqliteDocumentRepository() + async with get_connection() as db: + await db.execute( + "INSERT INTO documents (id, filename, storage_path, created_at) " + "VALUES ('doc-uploaded', 'a.pdf', '/tmp/a.pdf', datetime('now'))" + ) + await db.execute( + "INSERT INTO documents (id, filename, storage_path, created_at) " + "VALUES ('doc-parsed', 'b.pdf', '/tmp/b.pdf', datetime('now'))" + ) + await db.execute( + "INSERT INTO documents (id, filename, storage_path, created_at) " + "VALUES ('doc-chunked', 'c.pdf', '/tmp/c.pdf', datetime('now'))" + ) + await db.execute( + "INSERT INTO analysis_jobs (id, document_id, status) " + "VALUES ('a-parsed', 'doc-parsed', 'COMPLETED')" + ) + chunks_json = json.dumps( + [ + {"text": "Intro paragraph.", "headings": ["Intro"], "sourcePage": 1}, + {"text": "Body of section A.", "headings": ["A"], "sourcePage": 2}, + ] + ) + await db.execute( + "INSERT INTO analysis_jobs (id, document_id, status, chunks_json) " + "VALUES ('a-chunked', 'doc-chunked', 'COMPLETED', ?)", + (chunks_json,), + ) + await db.commit() + # Touch the document repo so the type checker is satisfied. + assert await repo.find_by_id("doc-chunked") is not None + + +async def test_full_migration_writes_expected_state() -> None: + await _seed_pre_06_state() + + results = await run(dry_run=False) + by_step = {r.name: r for r in results} + assert by_step["backfill_lifecycle"].written >= 2 # parsed + chunked transition + assert by_step["materialize_chunks"].written == 2 # two chunks + assert by_step["backfill_links"].written == 1 # only doc-chunked has chunks + + repo = SqliteDocumentRepository() + chunks_repo = SqliteChunkRepository() + link_repo = SqliteDocumentStoreLinkRepository() + + uploaded = await repo.find_by_id("doc-uploaded") + assert uploaded is not None + assert uploaded.lifecycle_state == DocumentLifecycleState.UPLOADED + + parsed = await repo.find_by_id("doc-parsed") + assert parsed is not None + assert parsed.lifecycle_state == DocumentLifecycleState.PARSED + + chunked = await repo.find_by_id("doc-chunked") + assert chunked is not None + # After link backfill + reaggregation, doc-chunked is Ingested in default store. + assert chunked.lifecycle_state == DocumentLifecycleState.INGESTED + + chunks = await chunks_repo.find_for_document("doc-chunked") + assert len(chunks) == 2 + assert chunks[0].text == "Intro paragraph." + assert chunks[0].sequence == 0 + + links = await link_repo.find_for_document("doc-chunked") + assert len(links) == 1 + assert links[0].store_id == "default" + assert links[0].state == DocumentStoreLinkState.INGESTED + assert links[0].chunkset_hash is not None + + +async def test_migration_is_idempotent() -> None: + await _seed_pre_06_state() + + first = await run(dry_run=False) + second = await run(dry_run=False) + # Second run sees every step as already-done. + assert all(r.skipped for r in second) + assert all(not r.skipped for r in first) + + +async def test_dry_run_writes_nothing() -> None: + await _seed_pre_06_state() + + repo = SqliteDocumentRepository() + chunks_repo = SqliteChunkRepository() + + await run(dry_run=True) + + # No state changed. + parsed = await repo.find_by_id("doc-parsed") + assert parsed is not None + assert parsed.lifecycle_state == DocumentLifecycleState.UPLOADED # unchanged + chunked_chunks = await chunks_repo.find_for_document("doc-chunked") + assert chunked_chunks == [] + + +async def test_chunk_ids_are_deterministic_across_runs() -> None: + """Stable id derivation lets a re-run after a manual reset land on + the same chunk ids — important for the link-table foreign keys.""" + await _seed_pre_06_state() + + chunks_repo = SqliteChunkRepository() + + await run(dry_run=False, only_step="materialize_chunks") + first_ids = sorted(c.id for c in await chunks_repo.find_for_document("doc-chunked")) + + # Wipe progress + chunks, run again — same ids. + async with get_connection() as db: + await db.execute("DELETE FROM migration_progress WHERE name = 'materialize_chunks'") + await db.execute("DELETE FROM chunks WHERE document_id = 'doc-chunked'") + await db.commit() + + await run(dry_run=False, only_step="materialize_chunks") + second_ids = sorted(c.id for c in await chunks_repo.find_for_document("doc-chunked")) + + assert first_ids == second_ids diff --git a/document-parser/tools/migrate_06.py b/document-parser/tools/migrate_06.py new file mode 100644 index 0000000..bfe3176 --- /dev/null +++ b/document-parser/tools/migrate_06.py @@ -0,0 +1,433 @@ +"""0.6.0 migration — backfill the doc-centric data model. + +Run after deploying the 0.6.0 release code, before sending traffic. The +script is idempotent and resumable — re-running on an already-migrated +database is a no-op. Each step records itself in `migration_progress`. + +Inference rules for `documents.lifecycle_state`: + + has any FAILED analysis_jobs row, no completed → Failed + has any COMPLETED analysis_jobs row + chunks_json → Chunked + has any COMPLETED analysis_jobs row, no chunks_json → Parsed + else → Uploaded + +After chunks are materialized into the new `chunks` table and links are +backfilled (one per existing default-store ingestion), the document +state is recomputed using the standard #203 aggregation rule. This +covers Stale / Ingested upgrades where applicable. + +Usage: + + python -m tools.migrate_06 [--dry-run] [--only-step ] + +The CLI is intentionally minimal — see docs/design/206-lifecycle-state-migration.md +for the full set of flags considered. +""" + +from __future__ import annotations + +import argparse +import asyncio +import json +import logging +import sys +import uuid +from dataclasses import dataclass +from typing import TYPE_CHECKING + +from domain.hashing import chunkset_hash + +if TYPE_CHECKING: + from collections.abc import Callable, Iterable + + import aiosqlite +from domain.lifecycle_aggregation import aggregate_lifecycle +from domain.models import Chunk, DocumentStoreLink +from domain.value_objects import ( + ChunkBbox, + ChunkDocItem, + ChunkResult, + DocumentLifecycleState, + DocumentStoreLinkState, +) +from persistence.chunk_repo import SqliteChunkRepository +from persistence.database import get_connection, init_db +from persistence.document_store_link_repo import SqliteDocumentStoreLinkRepository +from persistence.store_repo import SqliteStoreRepository + +logger = logging.getLogger("migrate_06") + + +# --------------------------------------------------------------------------- +# Progress tracking +# --------------------------------------------------------------------------- + + +async def _is_done(db: aiosqlite.Connection, name: str) -> bool: + cursor = await db.execute("SELECT 1 FROM migration_progress WHERE name = ?", (name,)) + return await cursor.fetchone() is not None + + +async def _mark_done(db: aiosqlite.Connection, name: str) -> None: + await db.execute( + "INSERT OR IGNORE INTO migration_progress (name) VALUES (?)", + (name,), + ) + await db.commit() + + +# --------------------------------------------------------------------------- +# Step: backfill_document_lifecycle_state +# --------------------------------------------------------------------------- + + +@dataclass +class _DocFacts: + document_id: str + has_completed: bool + has_failed: bool + has_chunks_json: bool + + +async def _gather_document_facts(db: aiosqlite.Connection) -> list[_DocFacts]: + """One row per document with the analysis-state booleans we care about.""" + cursor = await db.execute( + """SELECT d.id AS document_id, + MAX(CASE WHEN a.status = 'COMPLETED' THEN 1 ELSE 0 END) AS has_completed, + MAX(CASE WHEN a.status = 'FAILED' THEN 1 ELSE 0 END) AS has_failed, + MAX(CASE WHEN a.chunks_json IS NOT NULL AND a.chunks_json != '' + THEN 1 ELSE 0 END) AS has_chunks_json + FROM documents d + LEFT JOIN analysis_jobs a ON a.document_id = d.id + GROUP BY d.id""" + ) + rows = await cursor.fetchall() + return [ + _DocFacts( + document_id=r["document_id"], + has_completed=bool(r["has_completed"]), + has_failed=bool(r["has_failed"]), + has_chunks_json=bool(r["has_chunks_json"]), + ) + for r in rows + ] + + +def _infer_lifecycle(facts: _DocFacts) -> DocumentLifecycleState: + if facts.has_completed and facts.has_chunks_json: + return DocumentLifecycleState.CHUNKED + if facts.has_completed: + return DocumentLifecycleState.PARSED + if facts.has_failed: + return DocumentLifecycleState.FAILED + return DocumentLifecycleState.UPLOADED + + +async def _step_backfill_lifecycle(db: aiosqlite.Connection, *, dry_run: bool) -> int: + facts = await _gather_document_facts(db) + written = 0 + for f in facts: + target = _infer_lifecycle(f) + cursor = await db.execute( + "SELECT lifecycle_state FROM documents WHERE id = ?", + (f.document_id,), + ) + row = await cursor.fetchone() + if row and row["lifecycle_state"] == target.value: + continue + logger.info( + "lifecycle: doc=%s -> %s", + f.document_id, + target.value, + ) + if not dry_run: + await db.execute( + "UPDATE documents SET lifecycle_state = ?, " + "lifecycle_state_at = datetime('now') WHERE id = ?", + (target.value, f.document_id), + ) + written += 1 + if not dry_run: + await db.commit() + return written + + +# --------------------------------------------------------------------------- +# Step: materialize_chunks_from_chunks_json +# --------------------------------------------------------------------------- + + +def _stable_chunk_id(document_id: str, sequence: int, text: str) -> str: + """Deterministic id so re-running the migration doesn't duplicate rows.""" + seed = f"{document_id}|{sequence}|{text}" + return uuid.uuid5(uuid.NAMESPACE_OID, seed).hex + + +def _chunk_dicts_for_doc( + db: aiosqlite.Connection, +) -> Callable[[str], asyncio.Future[list[dict]]]: + """Return an async-compatible accessor over the latest chunks_json + for a document. Returns an empty list when no chunks_json is available. + """ + + async def _read(document_id: str) -> list[dict]: + cursor = await db.execute( + """SELECT chunks_json FROM analysis_jobs + WHERE document_id = ? AND chunks_json IS NOT NULL AND chunks_json != '' + ORDER BY completed_at DESC, created_at DESC + LIMIT 1""", + (document_id,), + ) + row = await cursor.fetchone() + if row is None: + return [] + try: + return list(json.loads(row["chunks_json"])) + except (TypeError, ValueError): + logger.warning("Invalid chunks_json for doc %s — skipping", document_id) + return [] + + return _read + + +def _chunk_dict_to_chunk(d: dict, *, document_id: str, sequence: int) -> Chunk: + text = d.get("text", "") + return Chunk( + id=_stable_chunk_id(document_id, sequence, text), + document_id=document_id, + sequence=sequence, + text=text, + headings=list(d.get("headings", [])), + source_page=d.get("sourcePage"), + bboxes=[ + ChunkBbox(page=b.get("page", 0), bbox=list(b.get("bbox", []))) + for b in d.get("bboxes", []) + ], + doc_items=[ + ChunkDocItem( + self_ref=di.get("selfRef", ""), + label=di.get("label", ""), + ) + for di in d.get("docItems", []) + ], + token_count=d.get("tokenCount"), + ) + + +async def _step_materialize_chunks(db: aiosqlite.Connection, *, dry_run: bool) -> int: + """For each document with chunks_json, insert chunk rows if absent.""" + chunks_repo = SqliteChunkRepository() + cursor = await db.execute("SELECT id FROM documents") + doc_rows = await cursor.fetchall() + written = 0 + read = _chunk_dicts_for_doc(db) + for doc_row in doc_rows: + document_id = doc_row["id"] + existing = await chunks_repo.find_for_document(document_id, include_deleted=True) + if existing: + continue # already materialized + chunk_dicts = await read(document_id) + if not chunk_dicts: + continue + chunks = [ + _chunk_dict_to_chunk(d, document_id=document_id, sequence=i) + for i, d in enumerate(chunk_dicts) + ] + logger.info("chunks: doc=%s materializing %d", document_id, len(chunks)) + if not dry_run: + await chunks_repo.insert_many(chunks) + written += len(chunks) + return written + + +# --------------------------------------------------------------------------- +# Step: backfill_default_store_links +# --------------------------------------------------------------------------- + + +def _chunks_to_results(chunks: Iterable[Chunk]) -> list[ChunkResult]: + return [ + ChunkResult( + text=c.text, + headings=list(c.headings), + source_page=c.source_page, + bboxes=[ChunkBbox(page=b.page, bbox=list(b.bbox)) for b in c.bboxes], + doc_items=[ChunkDocItem(self_ref=d.self_ref, label=d.label) for d in c.doc_items], + ) + for c in chunks + ] + + +async def _step_backfill_links(db: aiosqlite.Connection, *, dry_run: bool) -> int: + """For each doc that already has chunks materialized, create a + `default`-store link in `Ingested` state with a freshly-computed + chunkset hash. This treats "chunks exist" as a proxy for "the + document has been ingested into the default store" — which is true + for tenants with the legacy single-index deployment that 0.6.0 + formalises into the `default` store. + + This step does NOT call OpenSearch. Operators with non-trivial + OpenSearch state should run a separate reindex / verification + after this script (see runbook). + """ + store_repo = SqliteStoreRepository() + chunks_repo = SqliteChunkRepository() + link_repo = SqliteDocumentStoreLinkRepository() + + default_store = await store_repo.get_default() + if default_store is None: + logger.warning( + "No default store — skipping link backfill (run init_db first)", + ) + return 0 + + cursor = await db.execute("SELECT id FROM documents") + doc_rows = await cursor.fetchall() + written = 0 + for doc_row in doc_rows: + document_id = doc_row["id"] + existing = await link_repo.find_one(document_id, default_store.id) + if existing is not None: + continue + chunks = await chunks_repo.find_for_document(document_id) + if not chunks: + continue + h = chunkset_hash(_chunks_to_results(chunks)) + link = DocumentStoreLink( + document_id=document_id, + store_id=default_store.id, + state=DocumentStoreLinkState.INGESTED, + chunkset_hash=h, + ) + logger.info( + "links: doc=%s -> store=%s ingested (hash=%s…)", + document_id, + default_store.id, + h[:8], + ) + if not dry_run: + await link_repo.upsert(link) + written += 1 + return written + + +# --------------------------------------------------------------------------- +# Step: reaggregate_document_lifecycle +# --------------------------------------------------------------------------- + + +async def _step_reaggregate(db: aiosqlite.Connection, *, dry_run: bool) -> int: + """Recompute the doc lifecycle state from per-store links + the + fallback inferred in step 1.""" + link_repo = SqliteDocumentStoreLinkRepository() + cursor = await db.execute("SELECT id, lifecycle_state FROM documents") + rows = await cursor.fetchall() + written = 0 + for row in rows: + doc_id = row["id"] + current = DocumentLifecycleState(row["lifecycle_state"]) + links = await link_repo.find_for_document(doc_id) + target = aggregate_lifecycle(links, fallback=current) + if target == current: + continue + logger.info( + "reaggregate: doc=%s %s -> %s", + doc_id, + current.value, + target.value, + ) + if not dry_run: + await db.execute( + "UPDATE documents SET lifecycle_state = ?, " + "lifecycle_state_at = datetime('now') WHERE id = ?", + (target.value, doc_id), + ) + written += 1 + if not dry_run: + await db.commit() + return written + + +# --------------------------------------------------------------------------- +# Orchestration +# --------------------------------------------------------------------------- + + +_STEPS = [ + ("backfill_lifecycle", _step_backfill_lifecycle), + ("materialize_chunks", _step_materialize_chunks), + ("backfill_links", _step_backfill_links), + ("reaggregate", _step_reaggregate), +] + + +@dataclass +class StepResult: + name: str + skipped: bool + written: int + + +async def run( + *, + dry_run: bool = False, + only_step: str | None = None, +) -> list[StepResult]: + """Run the migration. Returns one StepResult per step (skipped or not).""" + await init_db() # ensures the schema and migration_progress table exist + results: list[StepResult] = [] + async with get_connection() as db: + for name, step in _STEPS: + if only_step and only_step != name: + continue + if await _is_done(db, name): + results.append(StepResult(name=name, skipped=True, written=0)) + logger.info("step %s: already done — skipping", name) + continue + logger.info("step %s: running (dry_run=%s)", name, dry_run) + written = await step(db, dry_run=dry_run) + if not dry_run: + await _mark_done(db, name) + results.append(StepResult(name=name, skipped=False, written=written)) + return results + + +def _print_summary(results: list[StepResult], *, dry_run: bool) -> None: + print() + print(f"{'step':35s} {'wrote':>10s} {'skipped':>10s}") + print("-" * 60) + total_written = 0 + for r in results: + print(f"{r.name:35s} {r.written:>10d} {'yes' if r.skipped else 'no':>10s}") + total_written += r.written + print("-" * 60) + print(f"{'total':35s} {total_written:>10d}") + print() + print(f"dry_run={dry_run}") + + +def main(argv: list[str] | None = None) -> int: + logging.basicConfig( + level=logging.INFO, + format="%(asctime)s %(levelname)s %(name)s: %(message)s", + ) + parser = argparse.ArgumentParser(prog="migrate_06") + parser.add_argument( + "--dry-run", + action="store_true", + help="Print the plan without writing.", + ) + parser.add_argument( + "--only-step", + choices=[name for name, _ in _STEPS], + help="Run a single named step (useful when re-doing one phase).", + ) + args = parser.parse_args(argv) + + results = asyncio.run(run(dry_run=args.dry_run, only_step=args.only_step)) + _print_summary(results, dry_run=args.dry_run) + return 0 + + +if __name__ == "__main__": + sys.exit(main()) From 8234a3c23fabd9e0204d4d127cdc6d39f0874dc2 Mon Sep 17 00:00:00 2001 From: Pier-Jean Malandrino Date: Wed, 29 Apr 2026 17:31:41 +0200 Subject: [PATCH 08/35] docs(design): E2 design docs for 0.6.0 navigation refactor MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds technical design docs for the navigation epic of the doc-centric pivot: - 207 — Document-centric routing (/docs, /docs/:id?mode=, /index/:store, /runs) - 208 — Doc workspace breadcrumb (Studio > > ) - 209 — Sidebar nav rework (Home / Docs / Stores / Runs / Settings) - 210 — Feature flag mode gating (deep-link redirect + flag exposure) Status: Accepted on all four. Each doc spells out the contract, alternatives considered, risks per audit dimension, and testing strategy. Backwards-compatibility is preserved throughout: legacy routes and pages keep working until E3/E4/E5 explicitly replace them. Refs #207 #208 #209 #210 --- docs/design/207-document-centric-routing.md | 216 ++++++++++++++++++ docs/design/208-doc-workspace-breadcrumb.md | 180 +++++++++++++++ docs/design/209-nav-rework.md | 175 +++++++++++++++ docs/design/210-feature-flag-mode-gating.md | 235 ++++++++++++++++++++ 4 files changed, 806 insertions(+) create mode 100644 docs/design/207-document-centric-routing.md create mode 100644 docs/design/208-doc-workspace-breadcrumb.md create mode 100644 docs/design/209-nav-rework.md create mode 100644 docs/design/210-feature-flag-mode-gating.md diff --git a/docs/design/207-document-centric-routing.md b/docs/design/207-document-centric-routing.md new file mode 100644 index 0000000..24cb5db --- /dev/null +++ b/docs/design/207-document-centric-routing.md @@ -0,0 +1,216 @@ +# Design: Document-centric routing + +- **Issue:** #207 +- **Title on issue:** [FEATURE] Document-centric routing (/docs, /docs/:id?mode=, /index/:store, /runs) +- **Author:** Pier-Jean Malandrino +- **Date:** 2026-04-29 +- **Status:** Accepted +- **Target milestone:** 0.6.0 — Doc-centric ingest +- **Impacted layers:** frontend: app/router · pages · shared +- **Audit dimensions likely touched:** Clean Code · Tests · Documentation +- **ADR spawned?:** no + +--- + +## 1. Problem + +The current Vue Router (`frontend/src/app/router/index.ts`) is **analysis-centric**: routes are `/studio`, `/documents`, `/search`, `/reasoning`, `/history`. The selected document is held in a Pinia store, not in the URL — so two engineers cannot share a link to "the chunks editor for doc X". This breaks the killer flow ("paste this URL → fix the chunks → re-ingest") that 0.6.0 promises. + +The 0.6.0 sitemap puts the document at the centre of every URL: `/docs`, `/docs/:id?mode=ask|inspect|chunks`, plus `/index/:store` for stores and `/runs` for the run history. Mode is a query param so a doc URL stays stable across mode switches. + +This issue ships the routing skeleton. The actual page contents come in E3 (`/docs` library — #211), E4 (workspace shell — #216), E5 (chunks editor — #218 onward). + +## 2. Goals + +- [ ] Add `/docs`, `/docs/new`, `/docs/:id`, `/index`, `/index/:store`, `/index/:store/query`, `/runs`, `/runs/:id` routes. +- [ ] On `/docs/:id`, `?mode=ask|inspect|chunks` is parsed; default = `ask`. +- [ ] Each new route renders a placeholder page (clear "Coming in 0.6.0" message) until E3/E4/E5 implement them. +- [ ] Legacy routes (`/studio`, `/documents`, `/history`, `/search`, `/reasoning`) keep working — no breaking redirect in this issue. +- [ ] Smoke test: each new route renders without error. + +## 3. Non-goals + +- Building the actual pages — that is E3 / E4 / E5. +- Migrating users from old routes — kept functional in parallel; deprecation comes when the new pages are ready. +- Server-side route enforcement — backend exposes its API on `/api/*`; the routing here is client-side only. +- A site map / generated nav — the sidebar nav rework is **#209**. +- Feature-flag-aware redirection (e.g. mode disabled → redirect to default) — that is **#210**. + +## 4. Context & constraints + +### Existing code surface + +- `frontend/src/app/router/index.ts` — current Vue Router setup (history mode, lazy-loaded pages). +- `frontend/src/pages/` — existing pages (`HomePage.vue`, `StudioPage.vue`, `DocumentsPage.vue`, `HistoryPage.vue`, `SearchPage.vue`, `ReasoningPage.vue`, `SettingsPage.vue`). +- `frontend/src/app/App.vue` — shell with topbar + sidebar + ``. +- `frontend/src/features/feature-flags/` — flag store and `useFeatureFlag` composable. + +### Hard constraints + +- TypeScript strict — every new route needs a typed `name`. +- No regression on existing routes — old URLs keep returning their current pages until #211 / #216 explicitly replace them. +- Lazy loading is preserved — every new page goes through `() => import(...)`. + +### Deployment modes + +Same routing for both `latest-local` and `latest-remote`. No HF Space-specific concern. + +## 5. Proposed design + +### 5.1 Router additions + +Append to `frontend/src/app/router/index.ts`: + +```ts +{ path: '/docs', name: 'docs-library', + component: () => import('@/pages/DocsLibraryPage.vue') }, +{ path: '/docs/new', name: 'docs-new', + component: () => import('@/pages/DocsNewPage.vue') }, +{ path: '/docs/:id', name: 'doc-workspace', + component: () => import('@/pages/DocWorkspacePage.vue'), + props: route => ({ id: route.params.id, mode: parseMode(route.query.mode) }) }, +{ path: '/index', name: 'stores-list', + component: () => import('@/pages/StoresListPage.vue') }, +{ path: '/index/:store', name: 'store-detail', + component: () => import('@/pages/StoreDetailPage.vue'), + props: true }, +{ path: '/index/:store/query', name: 'store-query', + component: () => import('@/pages/StoreQueryPage.vue'), + props: true }, +{ path: '/runs', name: 'runs', + component: () => import('@/pages/RunsPage.vue') }, +{ path: '/runs/:id', name: 'run-detail', + component: () => import('@/pages/RunDetailPage.vue'), + props: true }, +``` + +### 5.2 Mode parser + +A pure helper in `frontend/src/shared/routing/modes.ts`: + +```ts +export type DocMode = 'ask' | 'inspect' | 'chunks' +const DEFAULT_MODE: DocMode = 'ask' + +export function parseMode(raw: unknown): DocMode { + return raw === 'inspect' || raw === 'chunks' ? raw : DEFAULT_MODE +} +``` + +This is intentionally tiny and testable. #210 will extend it with feature-flag-aware redirection. + +### 5.3 Placeholder pages + +Each new page is ~30 lines of Vue: a centered card with the page title, a "Coming in 0.6.0" tagline, and a link back to home. They use the existing `useI18n()` strings under a new `comingSoon.*` namespace. + +### 5.4 Router types + +`frontend/src/shared/routing/names.ts` exports a typed union of route names so callers do `router.push({ name: ROUTES.DOC_WORKSPACE, params: { id } })` instead of stringly-typed names. + +```ts +export const ROUTES = { + HOME: 'home', + DOCS_LIBRARY: 'docs-library', + DOCS_NEW: 'docs-new', + DOC_WORKSPACE: 'doc-workspace', + STORES_LIST: 'stores-list', + STORE_DETAIL: 'store-detail', + STORE_QUERY: 'store-query', + RUNS: 'runs', + RUN_DETAIL: 'run-detail', + // ...legacy names kept as-is +} as const +export type RouteName = (typeof ROUTES)[keyof typeof ROUTES] +``` + +### 5.5 i18n + +New keys under `comingSoon.*` in `frontend/src/shared/i18n.ts` (fr + en): + +- `comingSoon.title` +- `comingSoon.subtitle.docsLibrary` +- `comingSoon.subtitle.docsNew` +- `comingSoon.subtitle.docWorkspace` +- `comingSoon.subtitle.stores` +- `comingSoon.subtitle.storeDetail` +- `comingSoon.subtitle.storeQuery` +- `comingSoon.subtitle.runs` +- `comingSoon.subtitle.runDetail` +- `comingSoon.backHome` + +## 6. Alternatives considered + +### Alternative A — Replace existing routes immediately + +- **Summary:** Make `/studio` and `/documents` redirect to the new routes in this issue. +- **Why not:** The new pages do not exist yet. Redirecting now means the user lands on a "Coming soon" page where they used to have a working app. + +### Alternative B — Hash-mode routing + +- **Summary:** Switch to `createWebHashHistory` for the new doc-centric routes. +- **Why not:** History mode is the existing convention and SPA deep-linking still works behind Nginx (already configured). No reason to mix modes. + +## 7. API & data contract + +No backend changes. The routes are entirely client-side. No env vars. + +### Breaking changes + +None. Additive. + +## 8. Risks & mitigations + +| Risk | Audit dimension | Likelihood | Impact | How we notice | Mitigation / rollback | +|------|-----------------|------------|--------|---------------|------------------------| +| Placeholder pages confuse users who land on them via shared links | Documentation | Medium | Low | Support tickets | Clear "Coming soon" copy + back-home link | +| Route name collisions with legacy ones | Clean Code | Low | Low | TS error / runtime warning | Use a `ROUTES` constant; legacy names kept | +| Broken nav from sidebar to legacy routes after rename | Decoupling | Low | Medium | Smoke test catches | The sidebar update is explicitly **#209**, not this issue | + +## 9. Testing strategy + +### Frontend — Vitest + +- `app/router/router.test.ts` — every new route resolves to its component (smoke). +- `shared/routing/modes.test.ts` — `parseMode` returns `ask` for `undefined` / `null` / unknown values; respects `inspect` / `chunks`. + +### E2E — Karate UI + +Out of scope for this issue (placeholder pages only). E2E coverage lands with #211 (library) and #216 (workspace). + +### Manual QA + +1. Visit each new URL in the browser → "Coming soon" shell renders without 404. +2. Visit `/docs/abc?mode=chunks` → page receives `mode === 'chunks'`. +3. Visit `/docs/abc?mode=garbage` → page receives `mode === 'ask'` (default). +4. Old routes (`/studio`, `/documents`) still load their existing pages. + +## 10. Rollout & observability + +### Release branch + +`release/0.6.0`. + +### Feature flag + +None. The placeholder pages are visible to anyone; they explain themselves. + +### Observability + +No new logs. Existing router-error handling unchanged. + +### Rollback plan + +Revert the router and pages — old setup is untouched. + +## 11. Open questions + +- Should `/docs/:id` 404 if the doc id is unknown, or render the workspace shell with an error state? **Decision for 0.6.0:** the workspace handles the "doc not found" case in #216; this issue ships the placeholder which always renders. + +## 12. References + +- **Issue:** https://github.com/scub-france/Docling-Studio/issues/207 +- **Related issues:** #208 (breadcrumb), #209 (nav), #210 (FF mode gating), #211 (library page), #216 (workspace), #218+ (modes) +- **ADRs:** none planned +- **Project docs:** + - Architecture: `docs/architecture.md` + - Frontend conventions: `frontend/CLAUDE.md` diff --git a/docs/design/208-doc-workspace-breadcrumb.md b/docs/design/208-doc-workspace-breadcrumb.md new file mode 100644 index 0000000..77940a1 --- /dev/null +++ b/docs/design/208-doc-workspace-breadcrumb.md @@ -0,0 +1,180 @@ +# Design: Doc workspace breadcrumb + +- **Issue:** #208 +- **Title on issue:** [ENHANCEMENT] Refactor breadcrumb to Studio › › +- **Author:** Pier-Jean Malandrino +- **Date:** 2026-04-29 +- **Status:** Accepted +- **Target milestone:** 0.6.0 — Doc-centric ingest +- **Impacted layers:** frontend: shared/ui · app +- **Audit dimensions likely touched:** Clean Code · Tests · Documentation +- **ADR spawned?:** no + +--- + +## 1. Problem + +There is no breadcrumb component today. The user lands on `/docs/:id?mode=chunks` and has no orientation: which doc, which mode, how to step back. With the new doc-centric URLs introduced in #207, the user navigates between modes on the same doc — a breadcrumb anchors them in the IA. + +The shape `Studio › › ` is the agreed pattern: each segment is a link except the last, the doc title is truncated with ellipsis (full title on hover), and the mode segment updates as the user switches tabs without a page reload. + +## 2. Goals + +- [ ] New `` component renders three segments on doc workspace pages: `Studio › › `. +- [ ] Each non-last segment is clickable: `Studio` → `/`, `` → `/docs/:id` (default mode). +- [ ] Doc title truncates beyond ~40 chars with ellipsis; full title shown via `title` attribute on hover. +- [ ] Mode segment reflects current `?mode=` and updates without remount. +- [ ] On non-doc routes (`/`, `/runs`, `/index`), the breadcrumb is empty / hidden — no fake breadcrumbs invented. +- [ ] Component is data-driven: takes a `Crumb[]` prop so future routes can supply their own. + +## 3. Non-goals + +- Auto-deriving the breadcrumb from the route — for 0.6.0 the doc workspace page passes its crumbs explicitly. Auto-derivation is a follow-up if more pages need it. +- Mobile-specific breadcrumb collapsing — a single-line ellipsis is enough for the layout. +- Breadcrumbs on store / run pages — those land in 0.7.0 with their own page work. + +## 4. Context & constraints + +### Existing code surface + +- `frontend/src/app/App.vue` — the shell. The breadcrumb slot will live in the topbar (between the logo and the right-side actions). +- `frontend/src/pages/DocWorkspacePage.vue` (created by #207, placeholder). Will be the first consumer. +- `frontend/src/shared/ui/` — home for shared UI primitives. The new component lives here. +- `frontend/src/shared/routing/names.ts` (created by #207) — typed route names. +- `frontend/src/shared/i18n.ts` — strings. + +### Hard constraints + +- TypeScript strict — `Crumb` is a discriminated union (link vs leaf). +- Accessibility — `