From 80b0c44e8c4a39bbcf024ce1531bf6b17bb6af6c Mon Sep 17 00:00:00 2001 From: Pier-Jean Malandrino Date: Fri, 10 Apr 2026 19:44:38 +0200 Subject: [PATCH] feat(chunking): soft-delete chunk with confirmation dialog Closes #90 --- CHANGELOG.md | 1 + document-parser/api/analyses.py | 22 +++ document-parser/api/schemas.py | 1 + document-parser/services/analysis_service.py | 21 +++ .../tests/test_analysis_service.py | 61 ++++++++ document-parser/tests/test_chunking.py | 44 ++++++ frontend/src/features/chunking/api.test.ts | 16 +- frontend/src/features/chunking/api.ts | 6 + frontend/src/features/chunking/store.test.ts | 44 ++++++ frontend/src/features/chunking/store.ts | 17 +- .../src/features/chunking/ui/ChunkPanel.vue | 146 +++++++++++++++++- frontend/src/shared/i18n.ts | 8 + frontend/src/shared/types.ts | 1 + 13 files changed, 383 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index dfb0aab..7aab9c8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/), and this - Inline chunk text editing: double-click or edit button to modify chunk text, with save/cancel and "modified" badge - Docker Compose dev stack (`docker-compose.dev.yml`) with OpenSearch, Dashboards, hot-reload backend and Vite frontend +- Soft-delete chunks: delete button with confirmation dialog, chunks hidden from UI but preserved in data ### Fixed diff --git a/document-parser/api/analyses.py b/document-parser/api/analyses.py index f15c6e2..6c8a02a 100644 --- a/document-parser/api/analyses.py +++ b/document-parser/api/analyses.py @@ -128,6 +128,28 @@ async def update_chunk_text( token_count=c.get("tokenCount", 0), bboxes=[ChunkBboxResponse(page=b["page"], bbox=b["bbox"]) for b in c.get("bboxes", [])], modified=c.get("modified", False), + deleted=c.get("deleted", False), + ) + for c in chunks + ] + + +@router.delete("/{job_id}/chunks/{chunk_index}", response_model=list[ChunkResponse]) +async def delete_chunk(job_id: str, chunk_index: int, service: ServiceDep) -> list[ChunkResponse]: + """Soft-delete a chunk by index (marks it as deleted).""" + try: + chunks = await service.delete_chunk(job_id, chunk_index) + except ValueError as e: + raise HTTPException(status_code=400, detail=str(e)) from e + return [ + ChunkResponse( + text=c["text"], + headings=c.get("headings", []), + source_page=c.get("sourcePage"), + token_count=c.get("tokenCount", 0), + bboxes=[ChunkBboxResponse(page=b["page"], bbox=b["bbox"]) for b in c.get("bboxes", [])], + modified=c.get("modified", False), + deleted=c.get("deleted", False), ) for c in chunks ] diff --git a/document-parser/api/schemas.py b/document-parser/api/schemas.py index 1d77e73..42079a6 100644 --- a/document-parser/api/schemas.py +++ b/document-parser/api/schemas.py @@ -159,6 +159,7 @@ class ChunkResponse(_CamelModel): token_count: int = 0 bboxes: list[ChunkBboxResponse] = [] modified: bool = False + deleted: bool = False class UpdateChunkTextRequest(BaseModel): diff --git a/document-parser/services/analysis_service.py b/document-parser/services/analysis_service.py index e1a366a..809384e 100644 --- a/document-parser/services/analysis_service.py +++ b/document-parser/services/analysis_service.py @@ -182,6 +182,27 @@ class AnalysisService: return chunks + async def delete_chunk(self, job_id: str, chunk_index: int) -> list[dict]: + """Soft-delete a chunk by index. Returns the full updated chunks list.""" + job = await self._analysis_repo.find_by_id(job_id) + if not job: + raise ValueError(f"Analysis not found: {job_id}") + if job.status != AnalysisStatus.COMPLETED: + raise ValueError(f"Analysis is not completed: {job_id}") + if not job.chunks_json: + raise ValueError(f"No chunks available: {job_id}") + + chunks = json.loads(job.chunks_json) + if chunk_index < 0 or chunk_index >= len(chunks): + raise ValueError(f"Chunk index out of range: {chunk_index}") + + chunks[chunk_index]["deleted"] = True + + chunks_json = json.dumps(chunks) + await self._analysis_repo.update_chunks(job_id, chunks_json) + + return chunks + async def _run_batched_conversion( self, job_id: str, diff --git a/document-parser/tests/test_analysis_service.py b/document-parser/tests/test_analysis_service.py index 6fb1523..d79ae09 100644 --- a/document-parser/tests/test_analysis_service.py +++ b/document-parser/tests/test_analysis_service.py @@ -594,3 +594,64 @@ class TestUpdateChunkText: with pytest.raises(ValueError, match="No chunks available"): await service.update_chunk_text("j1", 0, "text") + + +class TestDeleteChunk: + """Tests for AnalysisService.delete_chunk.""" + + @pytest.mark.asyncio + async def test_delete_chunk_success(self): + chunks = [ + {"text": "chunk1", "headings": [], "sourcePage": 1, "tokenCount": 5, "bboxes": []}, + {"text": "chunk2", "headings": [], "sourcePage": 2, "tokenCount": 3, "bboxes": []}, + ] + job = MagicMock() + job.status = AnalysisStatus.COMPLETED + job.chunks_json = json.dumps(chunks) + + repo = MagicMock() + repo.find_by_id = AsyncMock(return_value=job) + repo.update_chunks = AsyncMock(return_value=True) + + service = _make_service(analysis_repo=repo) + result = await service.delete_chunk("j1", 0) + + assert result[0]["deleted"] is True + assert result[1].get("deleted", False) is False + repo.update_chunks.assert_called_once() + + @pytest.mark.asyncio + async def test_delete_chunk_not_found(self): + repo = MagicMock() + repo.find_by_id = AsyncMock(return_value=None) + service = _make_service(analysis_repo=repo) + + with pytest.raises(ValueError, match="Analysis not found"): + await service.delete_chunk("missing", 0) + + @pytest.mark.asyncio + async def test_delete_chunk_index_out_of_range(self): + chunks = [{"text": "only", "headings": [], "sourcePage": 1, "tokenCount": 5, "bboxes": []}] + job = MagicMock() + job.status = AnalysisStatus.COMPLETED + job.chunks_json = json.dumps(chunks) + + repo = MagicMock() + repo.find_by_id = AsyncMock(return_value=job) + service = _make_service(analysis_repo=repo) + + with pytest.raises(ValueError, match="out of range"): + await service.delete_chunk("j1", 5) + + @pytest.mark.asyncio + async def test_delete_chunk_no_chunks(self): + job = MagicMock() + job.status = AnalysisStatus.COMPLETED + job.chunks_json = None + + repo = MagicMock() + repo.find_by_id = AsyncMock(return_value=job) + service = _make_service(analysis_repo=repo) + + with pytest.raises(ValueError, match="No chunks available"): + await service.delete_chunk("j1", 0) diff --git a/document-parser/tests/test_chunking.py b/document-parser/tests/test_chunking.py index a1353fb..841ea05 100644 --- a/document-parser/tests/test_chunking.py +++ b/document-parser/tests/test_chunking.py @@ -342,6 +342,50 @@ class TestUpdateChunkTextEndpoint: assert resp.status_code == 400 +class TestDeleteChunkEndpoint: + def test_delete_chunk_success(self, client, mock_analysis_service): + updated_chunks = [ + { + "text": "chunk1", + "headings": [], + "sourcePage": 1, + "tokenCount": 10, + "bboxes": [], + "deleted": True, + }, + { + "text": "chunk2", + "headings": [], + "sourcePage": 2, + "tokenCount": 20, + "bboxes": [], + "deleted": False, + }, + ] + mock_analysis_service.delete_chunk = AsyncMock(return_value=updated_chunks) + + resp = client.delete("/api/analyses/j1/chunks/0") + assert resp.status_code == 200 + data = resp.json() + assert len(data) == 2 + assert data[0]["deleted"] is True + assert data[1]["deleted"] is False + + def test_delete_chunk_invalid_index(self, client, mock_analysis_service): + mock_analysis_service.delete_chunk = AsyncMock( + side_effect=ValueError("Chunk index out of range: 99"), + ) + resp = client.delete("/api/analyses/j1/chunks/99") + assert resp.status_code == 400 + + def test_delete_chunk_not_completed(self, client, mock_analysis_service): + mock_analysis_service.delete_chunk = AsyncMock( + side_effect=ValueError("Analysis is not completed: j1"), + ) + resp = client.delete("/api/analyses/j1/chunks/0") + assert resp.status_code == 400 + + class TestRechunkEndpoint: def test_rechunk_success(self, client, mock_analysis_service): mock_analysis_service.rechunk = AsyncMock( diff --git a/frontend/src/features/chunking/api.test.ts b/frontend/src/features/chunking/api.test.ts index 89d7bf7..fce35d3 100644 --- a/frontend/src/features/chunking/api.test.ts +++ b/frontend/src/features/chunking/api.test.ts @@ -1,5 +1,5 @@ import { describe, it, expect, vi, beforeEach } from 'vitest' -import { rechunkAnalysis, updateChunkText } from './api' +import { rechunkAnalysis, updateChunkText, deleteChunk } from './api' vi.mock('../../shared/api/http', () => ({ apiFetch: vi.fn(), @@ -40,4 +40,18 @@ describe('chunking API', () => { }) expect(result).toEqual(chunks) }) + + it('deleteChunk sends DELETE to chunk endpoint', async () => { + const chunks = [ + { text: 'chunk1', headings: [], sourcePage: 1, tokenCount: 10, bboxes: [], deleted: true }, + ] + apiFetch.mockResolvedValue(chunks) + + const result = await deleteChunk('job-1', 0) + + expect(apiFetch).toHaveBeenCalledWith('/api/analyses/job-1/chunks/0', { + method: 'DELETE', + }) + expect(result).toEqual(chunks) + }) }) diff --git a/frontend/src/features/chunking/api.ts b/frontend/src/features/chunking/api.ts index d15a835..7729829 100644 --- a/frontend/src/features/chunking/api.ts +++ b/frontend/src/features/chunking/api.ts @@ -14,3 +14,9 @@ export function updateChunkText(jobId: string, chunkIndex: number, text: string) body: JSON.stringify({ text }), }) } + +export function deleteChunk(jobId: string, chunkIndex: number): Promise { + return apiFetch(`/api/analyses/${jobId}/chunks/${chunkIndex}`, { + method: 'DELETE', + }) +} diff --git a/frontend/src/features/chunking/store.test.ts b/frontend/src/features/chunking/store.test.ts index 1ff9bba..9f55991 100644 --- a/frontend/src/features/chunking/store.test.ts +++ b/frontend/src/features/chunking/store.test.ts @@ -5,6 +5,7 @@ import { useChunkingStore } from './store' vi.mock('./api', () => ({ rechunkAnalysis: vi.fn(), updateChunkText: vi.fn(), + deleteChunk: vi.fn(), })) import * as api from './api' @@ -19,6 +20,7 @@ describe('useChunkingStore', () => { const store = useChunkingStore() expect(store.rechunking).toBe(false) expect(store.saving).toBe(false) + expect(store.deleting).toBe(false) expect(store.error).toBeNull() }) @@ -106,4 +108,46 @@ describe('useChunkingStore', () => { expect(store.saving).toBe(false) expect(store.error).toBe('save failed') }) + + it('deleteChunk calls API and returns chunks', async () => { + const chunks = [ + { text: 'chunk1', headings: [], sourcePage: 1, tokenCount: 5, bboxes: [], deleted: true }, + ] + vi.mocked(api.deleteChunk).mockResolvedValue(chunks) + + const store = useChunkingStore() + const result = await store.deleteChunk('j1', 0) + + expect(api.deleteChunk).toHaveBeenCalledWith('j1', 0) + expect(result).toEqual(chunks) + expect(store.deleting).toBe(false) + }) + + it('deleteChunk sets deleting during execution', async () => { + let resolve: (v: any) => void + vi.mocked(api.deleteChunk).mockImplementation( + () => + new Promise((r) => { + resolve = r + }), + ) + + const store = useChunkingStore() + const promise = store.deleteChunk('j1', 0) + + expect(store.deleting).toBe(true) + resolve!([]) + await promise + expect(store.deleting).toBe(false) + }) + + it('deleteChunk handles errors', async () => { + vi.mocked(api.deleteChunk).mockRejectedValue(new Error('delete failed')) + vi.spyOn(console, 'error').mockImplementation(() => {}) + + const store = useChunkingStore() + await expect(store.deleteChunk('j1', 0)).rejects.toThrow('delete failed') + expect(store.deleting).toBe(false) + expect(store.error).toBe('delete failed') + }) }) diff --git a/frontend/src/features/chunking/store.ts b/frontend/src/features/chunking/store.ts index 24a6bcc..b34b3c2 100644 --- a/frontend/src/features/chunking/store.ts +++ b/frontend/src/features/chunking/store.ts @@ -6,6 +6,7 @@ import * as api from './api' export const useChunkingStore = defineStore('chunking', () => { const rechunking = ref(false) const saving = ref(false) + const deleting = ref(false) const error = ref(null) async function rechunk(jobId: string, chunkingOptions: ChunkingOptions): Promise { @@ -40,5 +41,19 @@ export const useChunkingStore = defineStore('chunking', () => { } } - return { rechunking, saving, error, rechunk, updateChunkText } + async function deleteChunk(jobId: string, chunkIndex: number): Promise { + deleting.value = true + error.value = null + try { + return await api.deleteChunk(jobId, chunkIndex) + } catch (e) { + error.value = (e as Error).message || 'Failed to delete chunk' + console.error('Failed to delete chunk', e) + throw e + } finally { + deleting.value = false + } + } + + return { rechunking, saving, deleting, error, rechunk, updateChunkText, deleteChunk } }) diff --git a/frontend/src/features/chunking/ui/ChunkPanel.vue b/frontend/src/features/chunking/ui/ChunkPanel.vue index ba4dfbb..093452b 100644 --- a/frontend/src/features/chunking/ui/ChunkPanel.vue +++ b/frontend/src/features/chunking/ui/ChunkPanel.vue @@ -84,7 +84,7 @@
- {{ pagination.totalItems.value }} {{ t('chunking.chunks') }} + {{ activeChunks.length }} {{ t('chunking.chunks') }}
+
{{ h }} @@ -164,6 +178,30 @@
+ +
+
+

{{ t('chunking.deleteConfirm') }}

+
+ + +
+
+
+

{{ chunks.length ? t('chunking.noChunksOnPage') : t('chunking.noChunks') }} @@ -223,11 +261,29 @@ const isBatchedAnalysis = computed(() => { return props.analysisStatus === 'COMPLETED' && !props.hasDocumentJson }) -const pageChunks = computed(() => props.chunks.filter((c) => c.sourcePage === props.currentPage)) +const activeChunks = computed(() => props.chunks.filter((c) => !c.deleted)) +const pageChunks = computed(() => + activeChunks.value.filter((c) => c.sourcePage === props.currentPage), +) const pagination = usePagination(pageChunks, { pageSize: 20 }) function globalIndex(localIdx: number): number { - return (pagination.page.value - 1) * pagination.pageSize.value + localIdx + const pageLocalIdx = (pagination.page.value - 1) * pagination.pageSize.value + localIdx + const pageChunk = pageChunks.value[pageLocalIdx] + return props.chunks.indexOf(pageChunk) +} + +const deleteConfirmIdx = ref(-1) + +function confirmDelete(chunkIndex: number) { + deleteConfirmIdx.value = chunkIndex +} + +async function doDelete() { + if (!props.analysisId || deleteConfirmIdx.value === -1) return + await chunkingStore.deleteChunk(props.analysisId, deleteConfirmIdx.value) + deleteConfirmIdx.value = -1 + emit('rechunked') } const editingIdx = ref(-1) @@ -282,6 +338,7 @@ async function doRechunk() { flex-direction: column; height: 100%; overflow: hidden; + position: relative; } .chunk-config { @@ -529,6 +586,19 @@ async function doRechunk() { transition: opacity 0.15s; } +.chunk-delete-icon { + background: none; + border: none; + color: var(--text-secondary); + cursor: pointer; + padding: 2px; + border-radius: 4px; + display: flex; + align-items: center; + opacity: 0; + transition: opacity 0.15s; +} + .chunk-card:hover .chunk-edit-icon { opacity: 1; } @@ -538,6 +608,15 @@ async function doRechunk() { background: var(--bg-tertiary); } +.chunk-card:hover .chunk-delete-icon { + opacity: 1; +} + +.chunk-delete-icon:hover { + color: #ef4444; + background: rgba(239, 68, 68, 0.1); +} + .chunk-edit { display: flex; flex-direction: column; @@ -598,6 +677,67 @@ async function doRechunk() { color: var(--text); } +.chunk-confirm-overlay { + position: absolute; + inset: 0; + background: rgba(0, 0, 0, 0.4); + display: flex; + align-items: center; + justify-content: center; + z-index: 10; +} + +.chunk-confirm-dialog { + background: var(--bg); + border: 1px solid var(--border); + border-radius: var(--radius); + padding: 20px; + max-width: 300px; + width: 90%; + box-shadow: 0 4px 12px rgba(0, 0, 0, 0.15); +} + +.chunk-confirm-text { + font-size: 13px; + color: var(--text); + margin: 0 0 16px; + line-height: 1.5; +} + +.chunk-confirm-actions { + display: flex; + gap: 8px; + justify-content: flex-end; +} + +.chunk-confirm-btn { + padding: 6px 14px; + border: none; + border-radius: var(--radius-sm, 4px); + font-size: 12px; + font-weight: 500; + cursor: pointer; +} + +.chunk-confirm-btn.danger { + background: #ef4444; + color: white; +} + +.chunk-confirm-btn.danger:disabled { + opacity: 0.5; + cursor: not-allowed; +} + +.chunk-confirm-btn.cancel { + background: var(--bg-tertiary); + color: var(--text-secondary); +} + +.chunk-confirm-btn.cancel:hover { + color: var(--text); +} + .chunk-text { font-size: 12px; color: var(--text); diff --git a/frontend/src/shared/i18n.ts b/frontend/src/shared/i18n.ts index e8cfa70..d56832e 100644 --- a/frontend/src/shared/i18n.ts +++ b/frontend/src/shared/i18n.ts @@ -124,6 +124,10 @@ const messages: Messages = { 'chunking.saving': 'Enregistrement...', 'chunking.cancel': 'Annuler', 'chunking.modified': 'modifié', + 'chunking.delete': 'Supprimer', + 'chunking.deleting': 'Suppression...', + 'chunking.deleteConfirm': + 'Supprimer ce chunk ? Il sera marqué comme supprimé jusqu\u2019à la prochaine synchronisation.', 'chunking.batchNotice': 'Le chunking n\u2019est pas disponible pour cette analyse. Les documents volumineux trait\u00e9s par batch ne g\u00e9n\u00e8rent pas la structure interne n\u00e9cessaire au d\u00e9coupage.', @@ -255,6 +259,10 @@ const messages: Messages = { 'chunking.saving': 'Saving...', 'chunking.cancel': 'Cancel', 'chunking.modified': 'modified', + 'chunking.delete': 'Delete', + 'chunking.deleting': 'Deleting...', + 'chunking.deleteConfirm': + 'Delete this chunk? It will be marked as deleted until the next sync.', 'chunking.batchNotice': 'Chunking is not available for this analysis. Large documents processed in batch mode do not generate the internal structure required for chunking.', diff --git a/frontend/src/shared/types.ts b/frontend/src/shared/types.ts index 91173b1..e49b779 100644 --- a/frontend/src/shared/types.ts +++ b/frontend/src/shared/types.ts @@ -59,6 +59,7 @@ export interface Chunk { tokenCount: number bboxes: ChunkBbox[] modified?: boolean + deleted?: boolean } export interface PageElement {