From 75a2db087b86e32a391f5165369518a921ca3802 Mon Sep 17 00:00:00 2001 From: Pier-Jean Malandrino Date: Thu, 7 May 2026 10:49:23 +0200 Subject: [PATCH] docs(#256): flesh out design doc with chunk service + 9 routes plan --- docs/design/256-doc-tab-chunk-mode-404.md | 369 +++++++++++++++++++--- 1 file changed, 319 insertions(+), 50 deletions(-) diff --git a/docs/design/256-doc-tab-chunk-mode-404.md b/docs/design/256-doc-tab-chunk-mode-404.md index 7beb912..c22f9f4 100644 --- a/docs/design/256-doc-tab-chunk-mode-404.md +++ b/docs/design/256-doc-tab-chunk-mode-404.md @@ -32,9 +32,9 @@ the linked issue. Everything else is on the author. - **Date:** 2026-05-07 - **Status:** Draft - **Target milestone:** 0.6.0 — Doc-centric ingest -- **Impacted layers:** · | shared | app> · · -- **Audit dimensions likely touched:** -- **ADR spawned?:** *(write an ADR when choosing a library, moving a boundary, or deciding **not** to do something — see `docs/architecture/adr-guide.md`)* +- **Impacted layers:** backend: api · services · (persistence read-only — déjà en place) · frontend: features/chunks (zéro changement attendu, déjà aligné sur le contrat) · e2e: nouveau scénario Karate +- **Audit dimensions likely touched:** Hexagonal Architecture · DDD · Decoupling · Tests · Documentation +- **ADR spawned?:** no — le modèle canonical `Chunk` (port + repository) existe déjà depuis #205, on ne fait que combler une couche service + API absente. --- @@ -47,9 +47,13 @@ Two or three short paragraphs is usually enough. If you can't state the problem in plain language, you are not ready to design a solution. --> -Dans l'onglet **Doc**, l'activation du mode **chunk** déclenche une réponse **HTTP 404** côté front. Le mode chunk est donc inutilisable. +Dans l'onglet **Doc** (`DocWorkspacePage` → `DocChunksTab`), l'activation du mode **chunk** déclenche un **HTTP 404**. -TODO: ajouter logs / capture / endpoint exact qui 404. +L'investigation a montré que ce n'est pas un endpoint isolé qui manque, mais **9 routes complètes côté `/api/documents/{id}/...`** qui sont appelées depuis le front mais n'existent pas côté backend. La couche frontend a été développée sur la cible 0.6.0 (doc-centric) avant que la couche API correspondante ne soit posée. + +Le domain et la persistence du modèle canonical `Chunk` (entité de première classe, audit log via `ChunkEdit`, snapshots `ChunkPush`) existent **déjà** depuis l'issue #205 (`document-parser/domain/models.py`, `domain/ports.py`, `persistence/chunk_repo.py`, `persistence/chunk_edit_repo.py`). Ce qui manque, c'est la couche **service** et la couche **API** qui les exposent au front. + +Note importante: le mode **OCR Debug** de `StudioPage` (`/api/analyses/*`) doit rester intact. Il opère sur des `AnalysisJob` éphémères avec pipeline options variables, ce qui est conceptuellement différent des chunks canonical d'un document. Les deux flux partagent le service de chunking sous-jacent (port `DocumentChunker`) mais exposent des ressources distinctes — pas de duplication d'endpoints. ## 2. Goals @@ -59,9 +63,13 @@ checkboxes here; the design is "done" when all are satisfied. Keep the list small — five or fewer goals is a good smell. --> -- [ ] TODO: étapes précises de reproduction (doc utilisé, séquence UI) -- [ ] TODO: identifier l'endpoint backend qui 404 -- [ ] TODO: correctif + non-régression +- [ ] Mode chunk de l'onglet Doc renvoie la liste des chunks canonical sans 404 (endpoint `GET /api/documents/{id}/chunks` opérationnel). +- [ ] Toutes les actions d'édition fonctionnent: add, edit, delete, split, merge, rechunk (endpoints `POST/PATCH/DELETE` correspondants). +- [ ] Le push vers un store et le diff par store fonctionnent (`POST /push`, `GET /diff?store=…`). +- [ ] L'arbre structurel s'affiche sur l'onglet Inspect (`GET /tree`). +- [ ] Le mode OCR Debug (`StudioPage` → `/api/analyses/*`) reste totalement fonctionnel — non-régression. +- [ ] Promotion automatique: à la première analyse réussie d'un document, ses chunks deviennent canonical (peuplent `chunks` table via `ChunkRepository`). +- [ ] Tests: unit Vitest sur les nouveaux appels, pytest sur le service + router, e2e Karate sur le flow `Doc tab → mode chunk`. ## 3. Non-goals @@ -73,41 +81,75 @@ prevents scope creep. If you leave this empty, reviewers will fill it in for you, badly. --> -- ... -- ... +- **Pas de refonte du modèle de données.** Le domain `Chunk` / `ChunkEdit` / `ChunkPush` est déjà posé (#205). On ne touche ni aux dataclasses ni au schéma SQLite. +- **Pas de refonte de StudioPage / OCR Debug.** Le flux `/api/analyses/*` reste tel quel; aucun port ni service de l'AnalysisService n'est modifié au-delà du hook de promotion canonical. +- **Pas de fusion `features/chunking/` ↔ `features/chunks/` côté front.** Ce sont deux features sémantiquement distinctes (acte de chunking debug vs état canonical persistant). +- **Pas de refactor des fichiers > 300 lignes** identifiés dans l'audit (StudioPage, GraphView, ChunksEditor) — sera traité dans des issues séparées du milestone 0.7.0. +- **Pas d'ajout de nouvelle table** ni de nouvelles colonnes — tout est déjà présent. +- **Pas de versioning d'API** — première exposition de ces routes, pas de breaking change. ## 4. Context & constraints - + +- Pas de migration SQLite (tables existent déjà). +- Pas de breaking change API (premières routes exposées). +- Performance: les opérations chunk lisent/écrivent ≤ N chunks pour un doc (typiquement < 200). Aucune requête non-bornée. Le rechunk lance Docling chunker en thread (`asyncio.to_thread`), comme `analysis_service.rechunk` actuel. ## 5. Proposed design @@ -162,18 +204,118 @@ valuable than pseudocode. ### 5.1 Domain +**Aucun changement.** Tout est en place depuis #205: + +- `Chunk(id, document_id, sequence, text, headings, source_page, bboxes, doc_items, token_count, created_at, updated_at, deleted_at)` +- `ChunkEdit(id, document_id, chunk_id, action ∈ INSERT|UPDATE|DELETE|MERGE|SPLIT, actor, at, before, after, parents, children, reason)` +- `ChunkPush(id, document_id, store_id, chunkset_hash, chunk_ids, …)` +- Ports: `ChunkRepository`, `ChunkEditRepository`, `ChunkPushRepository`, `DocumentChunker`. + ### 5.2 Persistence +**Aucun changement.** `chunk_repo.py` et `chunk_edit_repo.py` couvrent CRUD + audit. Le schéma SQLite existe déjà (vérifier `database.py`). Si une colonne ou un index manque pour les nouvelles requêtes (par ex. lookup chunks d'un doc ordonnés par `sequence`), ajouter dans la PR via un script idempotent. + ### 5.3 Infra adapters +**Aucun changement.** `DocumentChunker` est déjà implémenté (le même qui sert `analysis_service.rechunk`). + ### 5.4 Services +#### Nouveau: `ChunkService` — `document-parser/services/chunk_service.py` + +Orchestre toutes les opérations sur le chunkset canonical d'un document. Délègue aux ports existants. Écrit les `ChunkEdit` en transaction avec chaque mutation. + +```python +class ChunkService: + def __init__( + self, + chunk_repo: ChunkRepository, + chunk_edit_repo: ChunkEditRepository, + chunker: DocumentChunker, + document_repo: DocumentRepository, + analysis_service: AnalysisService, # pour le rechunk + ): ... + + async def list_chunks(self, doc_id: str) -> list[Chunk]: ... + async def add_chunk(self, doc_id: str, text: str, after_id: str | None) -> Chunk: ... + async def update_chunk(self, doc_id: str, chunk_id: str, *, text: str | None, headings: list[str] | None) -> Chunk: ... + async def delete_chunk(self, doc_id: str, chunk_id: str) -> None: ... + async def split_chunk(self, doc_id: str, chunk_id: str, cursor_offset: int) -> list[Chunk]: ... + async def merge_chunks(self, doc_id: str, ids: list[str]) -> Chunk: ... + async def rechunk_document(self, doc_id: str, opts: ChunkingOptions | None) -> list[Chunk]: ... + async def get_tree(self, doc_id: str) -> list[DocTreeNode]: ... + async def diff_against_store(self, doc_id: str, store: str) -> list[ChunkDiff]: ... + async def push_to_store(self, doc_id: str, store: str) -> ChunkPush: ... +``` + +Règles d'invariants: +- `sequence` des chunks reste dense ascendant. `add_chunk` insère et incrémente les suivants. `delete_chunk` est soft (marque `deleted_at`) pour préserver l'audit; les listings filtrent. +- Toute mutation produit **une ligne `ChunkEdit`** atomiquement. +- `rechunk_document`: appelle `analysis_service.rechunk_for_document(doc_id, opts)` (helper public à exposer dans `AnalysisService` si pas déjà), récupère la liste de chunks, **remplace** le chunkset canonical (delete soft de l'ancien + insert nouveau), enregistre un `ChunkEdit` `RESET`. +- `get_tree`: dérive de la dernière `AnalysisJob` réussie (lecture de `pages_json` / `document_json`). Pas de calcul nouveau. +- `push_to_store` et `diff_against_store`: délèguent à `ingestion_service` / `store_service` existants. Aucune logique dupliquée; le service expose juste l'interface "doc-centric". + +#### Hook de promotion canonical — `AnalysisService.create` + +À la fin d'une analyse réussie, si le document n'a pas encore de chunkset canonical (`chunk_repo.count_for_document(doc_id) == 0`), peupler depuis `job.chunks_json`: + +```python +async def create(self, ...) -> AnalysisJob: + job = await self._run_analysis(...) + if job.status == AnalysisStatus.COMPLETED and job.chunks_json: + if await self._chunk_repo.count_for_document(job.document_id) == 0: + await self._promote_to_canonical(job) # nouveau private method + return job +``` + +Promotion = parse `chunks_json` → instancie `Chunk(document_id=…)` → `chunk_repo.bulk_insert(chunks)` → `chunk_edit_repo.insert(ChunkEdit(action=INSERT, actor='system:initial-analysis'))`. + +C'est le **seul** point qui croise les deux contextes (analyse → doc canonical). Reste local à `AnalysisService`. + ### 5.5 API +#### Nouveau router: `document-parser/api/document_chunks.py` + +Préfixe `/api/documents/{doc_id}`, tag `documents`. Toutes les routes injectent `ChunkService` via `Depends`. + +```python +@router.get("/{doc_id}/chunks", response_model=list[ChunkResponse]) +@router.post("/{doc_id}/chunks", response_model=ChunkResponse) # body: AddChunkRequest +@router.patch("/{doc_id}/chunks/{chunk_id}", response_model=ChunkResponse) # body: UpdateChunkRequest +@router.delete("/{doc_id}/chunks/{chunk_id}", status_code=204) +@router.post("/{doc_id}/chunks/{chunk_id}/split", response_model=list[ChunkResponse]) # body: SplitChunkRequest { cursorOffset } +@router.post("/{doc_id}/chunks/merge", response_model=ChunkResponse) # body: MergeChunksRequest { ids } +@router.post("/{doc_id}/rechunk", response_model=list[ChunkResponse]) # body: RechunkRequest (optionnel) +@router.get("/{doc_id}/tree", response_model=list[DocTreeNodeResponse]) +@router.get("/{doc_id}/diff", response_model=list[ChunkDiffResponse]) # query: store +@router.post("/{doc_id}/chunks/push", response_model=PushSummaryResponse) # body: PushRequest { store } +``` + +Note: deux URLs présentes côté front pour push (`/{id}/push` dans `document/api.ts` et `/{id}/chunks/push` dans `chunks/api.ts`). Une seule route doit exister côté backend pour respecter la directive "no duplicate". **Choix**: garder `/{id}/chunks/push` (sémantiquement plus juste — c'est bien des chunks qu'on pousse). Mettre à jour `features/document/api.ts:34-39` pour cibler la même URL et virer la duplication. + +DTOs Pydantic: tous camelCase (alias_generator), `chunkId` au lieu de `chunk_id` côté wire, `sourcePage` etc. + +Erreurs: +- `404` si doc inexistant ou chunk inexistant +- `409` si `merge_chunks` reçoit des ids non contigus (invariant `sequence`) +- `400` si `split` reçoit un `cursorOffset` hors range + ### 5.6 Frontend — feature module +**Zéro changement attendu.** `features/chunks/api.ts` et `features/document/api.ts` sont déjà alignés. + +Une seule correction à apporter pour respecter "no duplicate endpoints": modifier `features/document/api.ts:35` pour pointer sur `/api/documents/${id}/chunks/push` au lieu de `/api/documents/${id}/push`. Ensuite cette fonction `pushDocumentToStore` peut être supprimée et le caller (`document/store.ts`) bascule sur `pushChunksToStore` de `features/chunks/api.ts`. + +Bug latents identifiés par l'audit, à corriger dans la même PR (scope #256): +- `ChunksEditor.vue:235` — `saveTimer` non clearé sur `onUnmounted`. Ajouter `onBeforeUnmount(() => clearTimeout(saveTimer))`. +- `ChunksEditor.vue:215` — remplacer `alert(...)` natif par le pattern toast partagé. + ### 5.7 Cross-cutting (feature flags, i18n, shared types) +- **Feature flags**: aucun. Le mode chunk est un flow standard du doc-centric, pas optionnel. +- **i18n**: aucune nouvelle clé requise; les strings existent déjà côté `chunks.*`. +- **Shared types**: `DocChunk`, `ChunkDiff`, `PushSummary`, `DocTreeNode` existent déjà dans `frontend/src/shared/types.ts`. Vérifier la correspondance camelCase avec les nouveaux DTOs Pydantic; aucun champ nouveau attendu. + ## 6. Alternatives considered -### Alternative A — +### Alternative A — Recâbler le front sur `/api/analyses/*` -- **Summary:** -- **Why not:** +- **Summary:** Côté front, résoudre `docId → latest analysis jobId` dans `DocWorkspacePage`, puis appeler les endpoints `/api/analyses/{jobId}/...` existants (rechunk, chunks/{idx}, etc.). Aucun nouveau endpoint backend. +- **Why not:** Conceptuellement faux. Une analyse est une run éphémère avec ses propres pipeline options; un document a un état chunk canonical persistant édité dans le temps. Identifier les deux casserait le modèle 0.6.0 doc-centric et empêcherait d'avoir plusieurs analyses parallèles d'un doc (ce que StudioPage permet) sans muter l'état canonical. Casse aussi l'audit log par chunk (`ChunkEdit`) qui est par doc, pas par job. -### Alternative B — +### Alternative B — Fusion `Analysis` ↔ `Document chunks` -- **Summary:** -- **Why not:** +- **Summary:** Supprimer `analysis_jobs.chunks_json`, faire que toute analyse écrive directement dans la table `chunks` du doc. StudioPage devient une UI de re-chunking d'un doc. +- **Why not:** Casse l'usage debug de StudioPage (plusieurs runs avec pipeline options ≠ ne peuvent pas coexister). Refactor majeur, hors scope d'un bug fix. Pourrait être envisagé en 0.7.0+ si l'usage de StudioPage évolue. ## 7. API & data contract +### Endpoints (tous nouveaux, additifs) + +| Method | Path | Request | Response | Breaking? | +|--------|------|---------|----------|-----------| +| GET | `/api/documents/{id}/chunks` | — | `ChunkResponse[]` | additive | +| POST | `/api/documents/{id}/chunks` | `AddChunkRequest` `{text, afterId?}` | `ChunkResponse` | additive | +| PATCH | `/api/documents/{id}/chunks/{chunkId}` | `UpdateChunkRequest` `{text?, headings?}` | `ChunkResponse` | additive | +| DELETE | `/api/documents/{id}/chunks/{chunkId}` | — | 204 | additive | +| POST | `/api/documents/{id}/chunks/{chunkId}/split` | `SplitChunkRequest` `{cursorOffset}` | `ChunkResponse[]` | additive | +| POST | `/api/documents/{id}/chunks/merge` | `MergeChunksRequest` `{ids}` | `ChunkResponse` | additive | +| POST | `/api/documents/{id}/rechunk` | `RechunkRequest` `{chunkingOptions?}` | `ChunkResponse[]` | additive | +| GET | `/api/documents/{id}/tree` | — | `DocTreeNodeResponse[]` | additive | +| GET | `/api/documents/{id}/diff?store={name}` | — | `ChunkDiffResponse[]` | additive | +| POST | `/api/documents/{id}/chunks/push` | `PushRequest` `{store}` | `PushSummaryResponse` | additive | + +Sérialisation **camelCase** côté wire (Pydantic `alias_generator`), snake_case interne. `pages_json` reste l'exception documentée si réutilisé. + +### Persistence schema + +```sql +-- Aucune migration. Tables existantes (issue #205): +-- chunks(id, document_id, sequence, text, headings_json, source_page, bboxes_json, +-- doc_items_json, token_count, created_at, updated_at, deleted_at) +-- chunk_edits(id, document_id, chunk_id, action, actor, at, before_json, after_json, +-- parents_json, children_json, reason) +-- chunk_pushes(id, document_id, store_id, chunkset_hash, chunk_ids_json, ...) +``` + +### Env vars / config + +| Name | Default | Allowed | Notes | +|------|---------|---------|-------| +| _(aucune)_ | | | Le `ChunkService` ne lit aucune nouvelle config. | + +### Breaking changes + +**Aucun.** Toutes les routes sont additives, premier passage live. Côté front, suppression de l'alias `pushDocumentToStore` (URL `/push`) au profit de `pushChunksToStore` (URL `/chunks/push`) — pas exposé externement. + - -- ... -- ... +- Le port `DocumentChunker` accepte-t-il `cursor_offset` pour `split`, ou faut-il l'implémenter manuellement dans `ChunkService.split_chunk` (split textuel basique sans repasser dans Docling) ? À vérifier au moment de l'implémentation. +- Faut-il enregistrer un `ChunkEdit` `RESET` lors du `rechunk_document`, ou un `DELETE` par chunk + `INSERT` par nouveau chunk ? Décision: une seule entrée `RESET` (lisibilité audit), avec `parents = [old_ids]`, `children = [new_ids]`. +- `get_tree`: lit-on `pages_json` ou `document_json` ? À confirmer en lisant `analysis_service.find_by_id` — probablement `document_json` qui contient la structure docling complète. ## 12. References