Merge pull request #139 from scub-france/fix/clean-code-audit

fix(clean-code): English mode strings, SRP & getter rename

document-parser/infra/local_converter.py

@@ -114,7 +114,7 @@ def _build_docling_converter(options: ConversionOptions) -> DoclingConverter:
     )
 
 
-def _get_default_converter() -> DoclingConverter:
+def _ensure_default_converter() -> DoclingConverter:
     global _default_converter
     if _default_converter is None:
         try:
@@ -127,7 +127,7 @@ def _get_default_converter() -> DoclingConverter:
 
 def _select_converter(options: ConversionOptions) -> DoclingConverter:
     if options.is_default():
-        return _get_default_converter()
+        return _ensure_default_converter()
     return _build_docling_converter(options)
 
 

document-parser/services/analysis_service.py

@@ -268,6 +268,67 @@ class AnalysisService:
                 job_id, file_path, filename, pipeline_options, chunking_options
             )
 
+    def _build_conversion_options(self, pipeline_options: dict | None) -> ConversionOptions:
+        """Build ConversionOptions, applying default table_mode if not specified."""
+        opts_dict = pipeline_options or {}
+        if "table_mode" not in opts_dict:
+            opts_dict = {**opts_dict, "table_mode": self._config.default_table_mode}
+        return ConversionOptions(**opts_dict)
+
+    async def _run_conversion(
+        self,
+        job_id: str,
+        file_path: str,
+        options: ConversionOptions,
+    ) -> ConversionResult | None:
+        """Run batched or single conversion. Returns None if the job was deleted mid-batch."""
+        total_pages = _count_pdf_pages(file_path)
+        batch_size = self._config.batch_page_size
+
+        if batch_size > 0 and total_pages > batch_size:
+            return await self._run_batched_conversion(
+                job_id, file_path, options, total_pages, batch_size
+            )
+        return await asyncio.wait_for(
+            self._converter.convert(file_path, options),
+            timeout=self._conversion_timeout,
+        )
+
+    async def _finalize_analysis(
+        self,
+        job_id: str,
+        result: ConversionResult,
+        chunking_options: dict | None,
+    ) -> None:
+        """Serialize results, optionally chunk, mark job completed, update page count."""
+        pages_json = json.dumps([asdict(p) for p in result.pages])
+
+        chunks_json = None
+        if chunking_options and self._chunker and result.document_json:
+            chunk_opts = ChunkingOptions(**chunking_options)
+            chunks = await self._chunker.chunk(result.document_json, chunk_opts)
+            chunks_json = json.dumps([_chunk_to_dict(c) for c in chunks])
+            logger.info("Chunking produced %d chunks for job %s", len(chunks), job_id)
+
+        # Re-read the job so we don't lose progress_current/progress_total
+        # written to the DB during batched conversion.
+        job = await self._analysis_repo.find_by_id(job_id)
+        if not job:
+            return
+        job.mark_completed(
+            markdown=result.content_markdown,
+            html=result.content_html,
+            pages_json=pages_json,
+            document_json=result.document_json,
+            chunks_json=chunks_json,
+        )
+        await self._analysis_repo.update_status(job)
+
+        if result.page_count:
+            await self._document_repo.update_page_count(job.document_id, result.page_count)
+
+        logger.info("Analysis completed: %s (%d pages)", job_id, result.page_count)
+
     async def _run_analysis_inner(
         self,
         job_id: str,
@@ -287,55 +348,12 @@ class AnalysisService:
             await self._analysis_repo.update_status(job)
             logger.info("Analysis started: %s (file: %s)", job_id, filename)
 
-            opts_dict = pipeline_options or {}
+            options = self._build_conversion_options(pipeline_options)
-            if "table_mode" not in opts_dict:
+            result = await self._run_conversion(job_id, file_path, options)
-                opts_dict = {**opts_dict, "table_mode": self._config.default_table_mode}
+            if result is None:
-            options = ConversionOptions(**opts_dict)
+                return  # job was deleted mid-batch
 
-            total_pages = _count_pdf_pages(file_path)
+            await self._finalize_analysis(job_id, result, chunking_options)
-            batch_size = self._config.batch_page_size
-
-            if batch_size > 0 and total_pages > batch_size:
-                result = await self._run_batched_conversion(
-                    job_id,
-                    file_path,
-                    options,
-                    total_pages,
-                    batch_size,
-                )
-                if result is None:
-                    return  # job was deleted mid-batch
-            else:
-                result = await asyncio.wait_for(
-                    self._converter.convert(file_path, options),
-                    timeout=self._conversion_timeout,
-                )
-
-            pages_json = json.dumps([asdict(p) for p in result.pages])
-
-            chunks_json = None
-            if chunking_options and self._chunker and result.document_json:
-                chunk_opts = ChunkingOptions(**chunking_options)
-                chunks = await self._chunker.chunk(result.document_json, chunk_opts)
-                chunks_json = json.dumps([_chunk_to_dict(c) for c in chunks])
-                logger.info("Chunking produced %d chunks for job %s", len(chunks), job_id)
-
-            # Re-read the job so we don't lose progress_current/progress_total
-            # written to the DB during batched conversion.
-            job = await self._analysis_repo.find_by_id(job_id) or job
-            job.mark_completed(
-                markdown=result.content_markdown,
-                html=result.content_html,
-                pages_json=pages_json,
-                document_json=result.document_json,
-                chunks_json=chunks_json,
-            )
-            await self._analysis_repo.update_status(job)
-
-            if result.page_count:
-                await self._document_repo.update_page_count(job.document_id, result.page_count)
-
-            logger.info("Analysis completed: %s (%d pages)", job_id, result.page_count)
 
         except TimeoutError:
             logger.error("Analysis timed out after %ds: %s", self._conversion_timeout, job_id)

document-parser/tests/test_pipeline_options.py

@@ -146,7 +146,7 @@ class TestBuildConverter:
 class TestConvertDocumentRouting:
     """Verify convert_document uses default converter for default opts, custom otherwise."""
 
-    @patch("infra.local_converter._get_default_converter")
+    @patch("infra.local_converter._ensure_default_converter")
     @patch("infra.local_converter._build_docling_converter")
     def test_uses_default_converter_with_all_defaults(self, mock_build, mock_get_default):
         mock_conv = MagicMock()
@@ -164,7 +164,7 @@ class TestConvertDocumentRouting:
         mock_get_default.assert_called_once()
         mock_build.assert_not_called()
 
-    @patch("infra.local_converter._get_default_converter")
+    @patch("infra.local_converter._ensure_default_converter")
     @patch("infra.local_converter._build_docling_converter")
     def test_uses_custom_converter_when_ocr_disabled(self, mock_build, mock_get_default):
         mock_conv = MagicMock()
@@ -182,7 +182,7 @@ class TestConvertDocumentRouting:
         mock_build.assert_called_once()
         mock_get_default.assert_not_called()
 
-    @patch("infra.local_converter._get_default_converter")
+    @patch("infra.local_converter._ensure_default_converter")
     @patch("infra.local_converter._build_docling_converter")
     def test_uses_custom_converter_when_table_mode_fast(self, mock_build, mock_get_default):
         mock_conv = MagicMock()
@@ -200,7 +200,7 @@ class TestConvertDocumentRouting:
 
         mock_build.assert_called_once_with(opts)
 
-    @patch("infra.local_converter._get_default_converter")
+    @patch("infra.local_converter._ensure_default_converter")
     @patch("infra.local_converter._build_docling_converter")
     def test_uses_custom_converter_when_code_enrichment_on(self, mock_build, mock_get_default):
         mock_conv = MagicMock()
@@ -218,7 +218,7 @@ class TestConvertDocumentRouting:
 
         mock_build.assert_called_once_with(opts)
 
-    @patch("infra.local_converter._get_default_converter")
+    @patch("infra.local_converter._ensure_default_converter")
     @patch("infra.local_converter._build_docling_converter")
     def test_uses_custom_converter_when_formula_enrichment_on(self, mock_build, mock_get_default):
         mock_conv = MagicMock()
@@ -235,7 +235,7 @@ class TestConvertDocumentRouting:
 
         mock_build.assert_called_once()
 
-    @patch("infra.local_converter._get_default_converter")
+    @patch("infra.local_converter._ensure_default_converter")
     @patch("infra.local_converter._build_docling_converter")
     def test_uses_custom_converter_when_picture_options_on(self, mock_build, mock_get_default):
         mock_conv = MagicMock()
@@ -252,7 +252,7 @@ class TestConvertDocumentRouting:
 
         mock_build.assert_called_once()
 
-    @patch("infra.local_converter._get_default_converter")
+    @patch("infra.local_converter._ensure_default_converter")
     @patch("infra.local_converter._build_docling_converter")
     def test_uses_custom_converter_when_generate_images_on(self, mock_build, mock_get_default):
         mock_conv = MagicMock()
@@ -269,7 +269,7 @@ class TestConvertDocumentRouting:
 
         mock_build.assert_called_once()
 
-    @patch("infra.local_converter._get_default_converter")
+    @patch("infra.local_converter._ensure_default_converter")
     @patch("infra.local_converter._build_docling_converter")
     def test_uses_custom_converter_when_images_scale_changed(self, mock_build, mock_get_default):
         mock_conv = MagicMock()
@@ -287,7 +287,7 @@ class TestConvertDocumentRouting:
 
         mock_build.assert_called_once_with(opts)
 
-    @patch("infra.local_converter._get_default_converter")
+    @patch("infra.local_converter._ensure_default_converter")
     @patch("infra.local_converter._build_docling_converter")
     def test_forwards_all_options_to_build_converter(self, mock_build, mock_get_default):
         mock_conv = MagicMock()

document-parser/tests/test_robustness.py

@@ -159,7 +159,7 @@ import infra.local_converter as lc_mod  # noqa: E402
 
 build_converter = lc_mod._build_docling_converter
 convert_sync = lc_mod._convert_sync
-get_default_converter = lc_mod._get_default_converter
+get_default_converter = lc_mod._ensure_default_converter
 
 # ---------------------------------------------------------------------------
 # C1 — document_timeout in PdfPipelineOptions
@@ -317,7 +317,7 @@ class TestConvertSyncLimits:
 
 
 class TestGetDefaultConverterReset:
-    """Verify _get_default_converter resets on failure and retries on next call."""
+    """Verify _ensure_default_converter resets on failure and retries on next call."""
 
     @pytest.fixture(autouse=True)
     def reset_default_converter(self):

frontend/src/features/history/navigation.test.ts

@@ -125,7 +125,7 @@ describe('History → Studio navigation', () => {
   })
 
   describe('Full restore flow (store-level integration)', () => {
-    it('restores completed analysis: selects analysis + document + verifier mode', async () => {
+    it('restores completed analysis: selects analysis + document + verify mode', async () => {
       const { fetchAnalysis } = await import('../analysis/api')
       fetchAnalysis.mockResolvedValue({
         id: 'a1',
@@ -147,12 +147,12 @@ describe('History → Studio navigation', () => {
       docStore.select(analysis.documentId)
       expect(docStore.selectedId).toBe('d1')
 
-      // Mode would be set to 'verifier' since status is COMPLETED
+      // Mode would be set to 'verify' since status is COMPLETED
-      const mode = analysis.status === 'COMPLETED' ? 'verifier' : 'configurer'
+      const mode = analysis.status === 'COMPLETED' ? 'verify' : 'configure'
-      expect(mode).toBe('verifier')
+      expect(mode).toBe('verify')
     })
 
-    it('restores failed analysis: selects analysis + document, stays in configurer mode', async () => {
+    it('restores failed analysis: selects analysis + document, stays in configure mode', async () => {
       const { fetchAnalysis } = await import('../analysis/api')
       fetchAnalysis.mockResolvedValue({
         id: 'a2',
@@ -171,8 +171,8 @@ describe('History → Studio navigation', () => {
       docStore.select(analysis.documentId)
       expect(docStore.selectedId).toBe('d2')
 
-      const mode = analysis.status === 'COMPLETED' ? 'verifier' : 'configurer'
+      const mode = analysis.status === 'COMPLETED' ? 'verify' : 'configure'
-      expect(mode).toBe('configurer')
+      expect(mode).toBe('configure')
     })
 
     it('handles missing analysis gracefully', async () => {

frontend/src/pages/StudioPage.vue

@@ -23,8 +23,8 @@
           <button
             class="toggle-btn"
             data-e2e="toggle-btn"
-            :class="{ active: mode === 'configurer' }"
+            :class="{ active: mode === 'configure' }"
-            @click="mode = 'configurer'"
+            @click="mode = 'configure'"
           >
             <svg class="toggle-icon" viewBox="0 0 20 20" fill="currentColor">
               <path
@@ -38,8 +38,8 @@
           <button
             class="toggle-btn"
             data-e2e="toggle-btn"
-            :class="{ active: mode === 'verifier' }"
+            :class="{ active: mode === 'verify' }"
-            @click="mode = 'verifier'"
+            @click="mode = 'verify'"
             :disabled="!analysisStore.currentAnalysis"
           >
             <svg class="toggle-icon" viewBox="0 0 20 20" fill="currentColor">
@@ -55,8 +55,8 @@
             v-if="chunkingEnabled"
             class="toggle-btn"
             data-e2e="toggle-btn"
-            :class="{ active: mode === 'preparer' }"
+            :class="{ active: mode === 'prepare' }"
-            @click="mode = 'preparer'"
+            @click="mode = 'prepare'"
             :disabled="!analysisStore.currentAnalysis"
           >
             <svg class="toggle-icon" viewBox="0 0 20 20" fill="currentColor">
@@ -82,7 +82,7 @@
           data-e2e="run-btn"
           :disabled="analysisStore.running"
           @click="runAnalysis"
-          v-if="mode === 'configurer'"
+          v-if="mode === 'configure'"
         >
           <div v-if="analysisStore.running" class="spinner-sm" />
           <svg v-else viewBox="0 0 20 20" fill="currentColor" class="btn-icon">
@@ -226,7 +226,7 @@
               @load="onPdfImageLoad"
             />
             <BboxOverlay
-              v-if="(visualMode || mode === 'preparer') && hasAnalysisResults"
+              v-if="(visualMode || mode === 'prepare') && hasAnalysisResults"
               ref="bboxOverlayRef"
               :image-el="pdfImageRef"
               :page-data="currentPageData"
@@ -246,7 +246,7 @@
       <!-- Right: Config or Results panel -->
       <div class="right-panel" :style="{ width: rightPanelWidth + 'px' }">
         <!-- CONFIGURER MODE -->
-        <div v-if="mode === 'configurer'" class="config-panel" data-e2e="config-panel">
+        <div v-if="mode === 'configure'" class="config-panel" data-e2e="config-panel">
           <div class="config-section">
             <label class="config-label">
               {{ t('config.model') }}
@@ -429,7 +429,7 @@
         </div>
 
         <!-- VERIFIER MODE -->
-        <div v-if="mode === 'verifier'" class="verify-panel">
+        <div v-if="mode === 'verify'" class="verify-panel">
           <ResultTabs
             :current-page="currentPage"
             :highlighted-index="highlightedElementIndex"
@@ -438,7 +438,7 @@
         </div>
 
         <!-- PREPARER MODE (feature-flipped) -->
-        <div v-if="mode === 'preparer' && chunkingEnabled" class="prepare-panel">
+        <div v-if="mode === 'prepare' && chunkingEnabled" class="prepare-panel">
           <ChunkPanel
             :current-page="currentPage"
             @highlight-bboxes="highlightedChunkBboxes = $event"
@@ -470,7 +470,7 @@ const analysisStore = useAnalysisStore()
 const { t } = useI18n()
 const chunkingEnabled = useFeatureFlag('chunking')
 
-const mode = ref('configurer')
+const mode = ref('configure')
 const currentPage = ref(1)
 const visualMode = ref(false)
 const highlightedElementIndex = ref(-1)
@@ -573,12 +573,12 @@ watch(currentPage, () => {
   highlightedChunkBboxes.value = []
 })
 
-// Auto-switch to verifier when analysis completes + refresh document data (pageCount)
+// Auto-switch to verify when analysis completes + refresh document data (pageCount)
 watch(
   () => analysisStore.currentAnalysis?.status,
   (status) => {
     if (status === 'COMPLETED') {
-      mode.value = 'verifier'
+      mode.value = 'verify'
       documentStore.load()
     }
   },
@@ -596,7 +596,7 @@ onMounted(async () => {
     if (analysis) {
       documentStore.select(analysis.documentId)
       if (analysis.status === 'COMPLETED') {
-        mode.value = 'verifier'
+        mode.value = 'verify'
       }
     }
     // Clean query param from URL