Spaces:
Running
refactor(audit): Phase 7 — unifier doublons (PipelineMode + path helpers + engine list)
Browse filesTrois factorisations identifiées par l'audit code-quality.
**7.1 — ``PipelineMode`` Literal unifié**
Avant : 3 définitions identiques mais indépendantes :
- ``picarones.pipeline.llm_pipeline_config.OCRLLMMode``
- ``picarones.pipeline.llm_pipeline_builder.OCRLLMPipelineMode``
- ``picarones.interfaces.web.models.PipelineMode``
Risque : si l'une ajoute un mode (par exemple ``"hybrid"``), les
deux autres divergent silencieusement et un payload Pydantic
``PipelineConfig.pipeline_mode = "hybrid"`` passe l'API web mais
plante côté pipeline.
Refactor :
- Nouvelle source de vérité unique :
``picarones.domain.pipeline_spec.PipelineMode`` (couche 1).
- Les 3 sites historiques deviennent des **alias importés** —
préserve les imports existants sans casser l'API publique.
Test ajouté : ``tests/domain/test_pipeline_mode_single_source.py``
(4 tests) — vérifie la définition canonique, l'égalité des alias,
**et** scan AST pour interdire toute future redéfinition d'un
``Literal["text_only", "text_and_image", "zero_shot"]`` hors du
module canonique.
**7.2 — Validation de chemin utilisateur centralisée**
Pattern dupliqué dans 3 routers web :
.. code-block:: python
try:
resolved = validated_path(
user_path,
allowed_roots=compute_workspace_roots(UPLOADS_DIR),
must_exist=False,
)
except PathValidationError as exc:
raise HTTPException(status_code=400, detail=str(exc)) from exc
Factorisé en deux helpers dans
``picarones/interfaces/web/_path_helpers.py`` :
- ``validated_user_path(user_path, *, must_exist, must_be_dir) -> Path``
- ``validated_user_output_dir(user_path) -> str`` (alias sémantique
avec ``must_exist=False`` pour les ``output_dir``).
Sites migrés :
- ``routers/importers.py`` (helper local ``_validated_output_dir``
devient un alias vers ``validated_user_output_dir``).
- ``routers/history.py`` (validation inline → ``validated_user_path``).
- ``routers/benchmark.py`` conservé tel quel : il groupe 3
validations distinctes dans un seul try/except (corpus_path +
output_dir + prompt_file), la factorisation gagne peu.
Bénéfice : 4 imports d'utilitaires bas-niveau retirés
(``PathValidationError``, ``validated_path``, ``compute_workspace_roots``,
``UPLOADS_DIR``) dans ``importers.py`` et ``history.py``.
**7.3 — Liste engines : garde-fou d'alignement**
Le router ``/api/engines`` énumère manuellement les engines OCR
(``check_engine("tesseract", ...)`` + ``cloud_ocr_specs``). La
factory ``picarones.adapters.ocr.factory._SUPPORTED`` est la source
de vérité canonique. Risque de drift : un nouvel engine ajouté
côté factory sans MAJ du router → endpoint manque l'info ; ou
l'inverse → frontend affiche un engine non instanciable.
Refonte forcée non viable (le router consomme des métadonnées —
label humain, env var — que la factory ne fournit pas). À la
place, garde-fou actif :
Test ajouté : ``tests/web/test_engines_router_alignment.py``
(2 tests) — scan AST du router, extrait les ``engine_id`` cités
dans ``check_engine(...)`` et ``cloud_ocr_specs``, vérifie qu'ils
sont tous dans ``_SUPPORTED``. Vérifie aussi le contenu minimal
de ``_SUPPORTED`` (8 engines canoniques).
**Bilan**
Suite : 4 800 passed, 16 skipped, 8 deselected, 2 xfailed.
+6 tests (4 PipelineMode + 2 engine alignment). Ruff propre,
sync-counters CI vert.
Couvre 3 des findings « duplications » majeurs identifiés par
l'audit. Restent en Phase 7 : ``BaseOCRAdapter`` template-method
(8 adapters OCR qui se recopient ~250 LOC chacun) et ``BaseRenderer``
pour les 28 renderers HTML — chantiers plus ambitieux à traiter
dans des PRs dédiés.
- CLAUDE.md +1 -1
- README.md +1 -1
- picarones/app/services/benchmark_runner.py +1 -1
- picarones/domain/pipeline_spec.py +19 -1
- picarones/interfaces/web/_path_helpers.py +101 -0
- picarones/interfaces/web/models.py +5 -17
- picarones/interfaces/web/routers/history.py +7 -16
- picarones/interfaces/web/routers/importers.py +8 -23
- picarones/pipeline/llm_pipeline_builder.py +4 -4
- picarones/pipeline/llm_pipeline_config.py +5 -2
- tests/domain/test_pipeline_mode_single_source.py +127 -0
- tests/web/test_engines_router_alignment.py +105 -0
|
@@ -116,7 +116,7 @@ picarones/
|
|
| 116 |
|
| 117 |
## État des tests et bugs historiques
|
| 118 |
|
| 119 |
-
`pytest tests/` → **
|
| 120 |
(post-audit code-quality, mai 2026). Les deselected sont les markers
|
| 121 |
`live` (5 tests d'intégration contre vraie API/binaire) + `network`
|
| 122 |
(3 tests qui hit le réseau réel), opt-in en local via `pytest -m live`
|
|
|
|
| 116 |
|
| 117 |
## État des tests et bugs historiques
|
| 118 |
|
| 119 |
+
`pytest tests/` → **4800 passed, 16 skipped, 8 deselected, 2 xfailed, 0 failed**
|
| 120 |
(post-audit code-quality, mai 2026). Les deselected sont les markers
|
| 121 |
`live` (5 tests d'intégration contre vraie API/binaire) + `network`
|
| 122 |
(3 tests qui hit le réseau réel), opt-in en local via `pytest -m live`
|
|
@@ -401,7 +401,7 @@ python -m mypy picarones/domain/ # strict mode (Layer 1)
|
|
| 401 |
python -m mypy picarones/ # lax mode (full tree)
|
| 402 |
```
|
| 403 |
|
| 404 |
-
**Test suite**: ~
|
| 405 |
floor at 85% (currently ~87%). The `network` marker excludes tests
|
| 406 |
requiring live HTTP. A handful of tests depend on optional engines
|
| 407 |
(`pero-ocr`, `pytesseract`) and are skipped/fail gracefully when
|
|
|
|
| 401 |
python -m mypy picarones/ # lax mode (full tree)
|
| 402 |
```
|
| 403 |
|
| 404 |
+
**Test suite**: ~4800 tests, ~3 min on a modern laptop. Coverage
|
| 405 |
floor at 85% (currently ~87%). The `network` marker excludes tests
|
| 406 |
requiring live HTTP. A handful of tests depend on optional engines
|
| 407 |
(`pero-ocr`, `pytesseract`) and are skipped/fail gracefully when
|
|
@@ -1189,7 +1189,7 @@ def run_benchmark_via_service(
|
|
| 1189 |
# et ``_aggregate_ner_metrics`` restent ici comme alias pour ne pas
|
| 1190 |
# casser les appels internes (les autres fonctions du runner s'y
|
| 1191 |
# réfèrent) et les tests qui patchent ces symboles via monkeypatch.
|
| 1192 |
-
from picarones.app.services._benchmark_ner import (
|
| 1193 |
aggregate_ner_metrics as _aggregate_ner_metrics,
|
| 1194 |
attach_ner_metrics_to_benchmark as _attach_ner_metrics_to_benchmark,
|
| 1195 |
)
|
|
|
|
| 1189 |
# et ``_aggregate_ner_metrics`` restent ici comme alias pour ne pas
|
| 1190 |
# casser les appels internes (les autres fonctions du runner s'y
|
| 1191 |
# réfèrent) et les tests qui patchent ces symboles via monkeypatch.
|
| 1192 |
+
from picarones.app.services._benchmark_ner import ( # noqa: F401
|
| 1193 |
aggregate_ner_metrics as _aggregate_ner_metrics,
|
| 1194 |
attach_ner_metrics_to_benchmark as _attach_ner_metrics_to_benchmark,
|
| 1195 |
)
|
|
@@ -54,6 +54,7 @@ Anti-sur-ingénierie
|
|
| 54 |
from __future__ import annotations
|
| 55 |
|
| 56 |
import re
|
|
|
|
| 57 |
|
| 58 |
from pydantic import BaseModel, ConfigDict, Field, field_validator
|
| 59 |
|
|
@@ -64,6 +65,23 @@ from picarones.domain.artifacts import ArtifactType
|
|
| 64 |
#: lisible par un humain dans les logs et le rapport.
|
| 65 |
_STEP_ID_RE = re.compile(r"^[A-Za-z0-9_\-]+$")
|
| 66 |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| 67 |
#: Sentinel pour ``inputs_from`` qui désigne les artefacts initiaux
|
| 68 |
#: fournis au runner (typiquement ``IMAGE``).
|
| 69 |
INITIAL_STEP_ID = "__initial__"
|
|
@@ -178,4 +196,4 @@ class PipelineSpec(BaseModel):
|
|
| 178 |
return None
|
| 179 |
|
| 180 |
|
| 181 |
-
__all__ = ["
|
|
|
|
| 54 |
from __future__ import annotations
|
| 55 |
|
| 56 |
import re
|
| 57 |
+
from typing import Literal
|
| 58 |
|
| 59 |
from pydantic import BaseModel, ConfigDict, Field, field_validator
|
| 60 |
|
|
|
|
| 65 |
#: lisible par un humain dans les logs et le rapport.
|
| 66 |
_STEP_ID_RE = re.compile(r"^[A-Za-z0-9_\-]+$")
|
| 67 |
|
| 68 |
+
#: Modes canoniques d'un pipeline OCR+LLM. Source de vérité unique
|
| 69 |
+
#: depuis la Phase 7.1 audit code-quality (2026-05) — auparavant
|
| 70 |
+
#: dupliqué trois fois (``pipeline/llm_pipeline_config.py:25``,
|
| 71 |
+
#: ``pipeline/llm_pipeline_builder.py:61``,
|
| 72 |
+
#: ``interfaces/web/models.py:71``), avec un risque concret de
|
| 73 |
+
#: divergence si l'un des trois ajoutait un nouveau mode.
|
| 74 |
+
#:
|
| 75 |
+
#: Sémantique :
|
| 76 |
+
#:
|
| 77 |
+
#: - ``text_only`` — l'OCR amont produit un texte brut, le LLM le
|
| 78 |
+
#: corrige sans voir l'image (post-correction texte pur).
|
| 79 |
+
#: - ``text_and_image`` — l'OCR amont produit un texte ; le VLM le
|
| 80 |
+
#: corrige en s'appuyant sur l'image (post-correction multimodale).
|
| 81 |
+
#: - ``zero_shot`` — pas d'OCR amont ; un VLM transcrit l'image
|
| 82 |
+
#: directement.
|
| 83 |
+
PipelineMode = Literal["text_only", "text_and_image", "zero_shot"]
|
| 84 |
+
|
| 85 |
#: Sentinel pour ``inputs_from`` qui désigne les artefacts initiaux
|
| 86 |
#: fournis au runner (typiquement ``IMAGE``).
|
| 87 |
INITIAL_STEP_ID = "__initial__"
|
|
|
|
| 196 |
return None
|
| 197 |
|
| 198 |
|
| 199 |
+
__all__ = ["PipelineMode", "PipelineSpec", "PipelineStep", "INITIAL_STEP_ID"]
|
|
@@ -0,0 +1,101 @@
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| 1 |
+
"""Helpers de validation de chemin partagés par les routers FastAPI.
|
| 2 |
+
|
| 3 |
+
Phase 7.2 audit code-quality (2026-05) — le pattern :
|
| 4 |
+
|
| 5 |
+
.. code-block:: python
|
| 6 |
+
|
| 7 |
+
try:
|
| 8 |
+
resolved = validated_path(
|
| 9 |
+
user_path,
|
| 10 |
+
allowed_roots=compute_workspace_roots(UPLOADS_DIR),
|
| 11 |
+
must_exist=False,
|
| 12 |
+
)
|
| 13 |
+
except PathValidationError as exc:
|
| 14 |
+
raise HTTPException(status_code=400, detail=str(exc)) from exc
|
| 15 |
+
|
| 16 |
+
était dupliqué dans :
|
| 17 |
+
|
| 18 |
+
- ``routers/importers.py:45`` (``_validated_output_dir``)
|
| 19 |
+
- ``routers/history.py:53-60`` (inline)
|
| 20 |
+
- ``routers/benchmark.py:104-115`` (2 appels successifs)
|
| 21 |
+
|
| 22 |
+
Factorisé ici en deux fonctions :
|
| 23 |
+
|
| 24 |
+
- :func:`validated_user_path` — un chemin, retourne ``str``.
|
| 25 |
+
- :func:`validated_user_output_dir` — alias sémantique avec
|
| 26 |
+
``must_exist=False`` par défaut (le ``output_dir`` peut être créé
|
| 27 |
+
ultérieurement par le worker).
|
| 28 |
+
"""
|
| 29 |
+
|
| 30 |
+
from __future__ import annotations
|
| 31 |
+
|
| 32 |
+
from pathlib import Path
|
| 33 |
+
|
| 34 |
+
from fastapi import HTTPException
|
| 35 |
+
|
| 36 |
+
from picarones.app.services.path_security import (
|
| 37 |
+
PathValidationError,
|
| 38 |
+
validated_path,
|
| 39 |
+
)
|
| 40 |
+
from picarones.interfaces.web.security import compute_workspace_roots
|
| 41 |
+
from picarones.interfaces.web.state import UPLOADS_DIR
|
| 42 |
+
|
| 43 |
+
|
| 44 |
+
def validated_user_path(
|
| 45 |
+
user_path: str,
|
| 46 |
+
*,
|
| 47 |
+
must_exist: bool = False,
|
| 48 |
+
must_be_dir: bool = False,
|
| 49 |
+
) -> Path:
|
| 50 |
+
"""Valide un chemin utilisateur contre les racines workspace.
|
| 51 |
+
|
| 52 |
+
Wrapper FastAPI autour de :func:`validated_path` : convertit
|
| 53 |
+
``PathValidationError`` en ``HTTPException(400)`` pour que le
|
| 54 |
+
client reçoive un 400 lisible plutôt qu'un 500 stacktrace.
|
| 55 |
+
|
| 56 |
+
Parameters
|
| 57 |
+
----------
|
| 58 |
+
user_path:
|
| 59 |
+
Chaîne fournie par le client (corps JSON, query param,
|
| 60 |
+
form field, etc.).
|
| 61 |
+
must_exist:
|
| 62 |
+
Si ``True``, refuse un chemin qui n'existe pas. Utile pour
|
| 63 |
+
les inputs ; mettre à ``False`` pour les outputs (le worker
|
| 64 |
+
créera le répertoire).
|
| 65 |
+
must_be_dir:
|
| 66 |
+
Si ``True``, refuse un fichier (n'accepte que les dossiers).
|
| 67 |
+
|
| 68 |
+
Returns
|
| 69 |
+
-------
|
| 70 |
+
Path
|
| 71 |
+
Chemin canonique résolu (relatif à une racine workspace).
|
| 72 |
+
|
| 73 |
+
Raises
|
| 74 |
+
------
|
| 75 |
+
HTTPException(400)
|
| 76 |
+
Si le chemin n'est pas dans une racine autorisée, contient
|
| 77 |
+
``..`` ou un symlink hors workspace, etc.
|
| 78 |
+
"""
|
| 79 |
+
try:
|
| 80 |
+
return validated_path(
|
| 81 |
+
user_path,
|
| 82 |
+
allowed_roots=compute_workspace_roots(UPLOADS_DIR),
|
| 83 |
+
must_exist=must_exist,
|
| 84 |
+
must_be_dir=must_be_dir,
|
| 85 |
+
)
|
| 86 |
+
except PathValidationError as exc:
|
| 87 |
+
raise HTTPException(status_code=400, detail=str(exc)) from exc
|
| 88 |
+
|
| 89 |
+
|
| 90 |
+
def validated_user_output_dir(user_path: str) -> str:
|
| 91 |
+
"""Variante sémantique pour les ``output_dir`` des importers.
|
| 92 |
+
|
| 93 |
+
Équivalente à ``str(validated_user_path(user_path, must_exist=False))``.
|
| 94 |
+
Préserve la signature historique de ``_validated_output_dir``
|
| 95 |
+
qui retournait une ``str`` (les backends consommateurs n'attendent
|
| 96 |
+
pas un ``Path``).
|
| 97 |
+
"""
|
| 98 |
+
return str(validated_user_path(user_path, must_exist=False))
|
| 99 |
+
|
| 100 |
+
|
| 101 |
+
__all__ = ["validated_user_path", "validated_user_output_dir"]
|
|
@@ -68,23 +68,11 @@ Liste alignée sur ``measurements.normalization.NORMALIZATION_PROFILES``
|
|
| 68 |
répercutée ici sous peine de rejet Pydantic au niveau API web.
|
| 69 |
Sprint A14-S1 — alignement README ↔ web models ↔ runtime."""
|
| 70 |
|
| 71 |
-
|
| 72 |
-
|
| 73 |
-
|
| 74 |
-
|
| 75 |
-
|
| 76 |
-
|
| 77 |
-
Sémantique :
|
| 78 |
-
|
| 79 |
-
- ``text_only`` — l'OCR amont produit un texte brut, le LLM le corrige
|
| 80 |
-
sans voir l'image (post-correction texte).
|
| 81 |
-
- ``text_and_image`` — l'OCR amont produit un texte ; le VLM le corrige
|
| 82 |
-
en s'appuyant sur l'image (post-correction multimodale).
|
| 83 |
-
- ``zero_shot`` — pas d'OCR amont ; un VLM transcrit l'image directement.
|
| 84 |
-
|
| 85 |
-
Phase 2 du chantier post-rewrite : suppression du fallback silencieux
|
| 86 |
-
``mode_map.get(comp.pipeline_mode, 'text_only')`` qui acceptait toute
|
| 87 |
-
chaîne arbitraire et la mappait sur ``text_only``."""
|
| 88 |
|
| 89 |
|
| 90 |
# ``BenchmarkRequest`` (mode legacy à liste de moteurs plats) a été
|
|
|
|
| 68 |
répercutée ici sous peine de rejet Pydantic au niveau API web.
|
| 69 |
Sprint A14-S1 — alignement README ↔ web models ↔ runtime."""
|
| 70 |
|
| 71 |
+
# Phase 7.1 audit code-quality (2026-05) : ``PipelineMode`` est désormais
|
| 72 |
+
# importé depuis :data:`picarones.domain.pipeline_spec.PipelineMode`
|
| 73 |
+
# (source de vérité unique). L'alias local préserve l'API publique
|
| 74 |
+
# de ce module pour les callers historiques.
|
| 75 |
+
from picarones.domain.pipeline_spec import PipelineMode # noqa: E402
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| 76 |
|
| 77 |
|
| 78 |
# ``BenchmarkRequest`` (mode legacy à liste de moteurs plats) a été
|
|
@@ -23,13 +23,6 @@ from typing import Any, Optional
|
|
| 23 |
|
| 24 |
from fastapi import APIRouter, HTTPException, Query
|
| 25 |
|
| 26 |
-
from picarones.interfaces.web.security import (
|
| 27 |
-
PathValidationError,
|
| 28 |
-
compute_workspace_roots,
|
| 29 |
-
validated_path,
|
| 30 |
-
)
|
| 31 |
-
from picarones.interfaces.web.state import UPLOADS_DIR
|
| 32 |
-
|
| 33 |
router = APIRouter()
|
| 34 |
_logger = logging.getLogger(__name__)
|
| 35 |
|
|
@@ -50,15 +43,13 @@ async def api_history_regressions(
|
|
| 50 |
from picarones.evaluation.metrics.history import BenchmarkHistory
|
| 51 |
|
| 52 |
if db_path:
|
| 53 |
-
|
| 54 |
-
|
| 55 |
-
|
| 56 |
-
|
| 57 |
-
|
| 58 |
-
)
|
| 59 |
-
|
| 60 |
-
raise HTTPException(status_code=400, detail=str(exc)) from exc
|
| 61 |
-
effective_db_path: Optional[str] = str(resolved)
|
| 62 |
else:
|
| 63 |
env_db = os.environ.get("PICARONES_HISTORY_DB", "").strip()
|
| 64 |
effective_db_path = env_db or None
|
|
|
|
| 23 |
|
| 24 |
from fastapi import APIRouter, HTTPException, Query
|
| 25 |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| 26 |
router = APIRouter()
|
| 27 |
_logger = logging.getLogger(__name__)
|
| 28 |
|
|
|
|
| 43 |
from picarones.evaluation.metrics.history import BenchmarkHistory
|
| 44 |
|
| 45 |
if db_path:
|
| 46 |
+
# Phase 7.2 audit code-quality : helper centralisé pour la
|
| 47 |
+
# validation chemin → HTTPException 400.
|
| 48 |
+
from picarones.interfaces.web._path_helpers import validated_user_path
|
| 49 |
+
|
| 50 |
+
effective_db_path: Optional[str] = str(
|
| 51 |
+
validated_user_path(db_path, must_exist=False),
|
| 52 |
+
)
|
|
|
|
|
|
|
| 53 |
else:
|
| 54 |
env_db = os.environ.get("PICARONES_HISTORY_DB", "").strip()
|
| 55 |
effective_db_path = env_db or None
|
|
@@ -7,12 +7,6 @@ import os
|
|
| 7 |
from fastapi import APIRouter, HTTPException, Query
|
| 8 |
|
| 9 |
from picarones.interfaces.web.models import HTRUnitedImportRequest, HuggingFaceImportRequest
|
| 10 |
-
from picarones.interfaces.web.security import (
|
| 11 |
-
PathValidationError,
|
| 12 |
-
compute_workspace_roots,
|
| 13 |
-
validated_path,
|
| 14 |
-
)
|
| 15 |
-
from picarones.interfaces.web.state import UPLOADS_DIR
|
| 16 |
|
| 17 |
router = APIRouter()
|
| 18 |
|
|
@@ -42,23 +36,14 @@ def _htr_united_catalogue():
|
|
| 42 |
return HTRUnitedCatalogue.from_remote(timeout=5)
|
| 43 |
|
| 44 |
|
| 45 |
-
|
| 46 |
-
|
| 47 |
-
|
| 48 |
-
|
| 49 |
-
|
| 50 |
-
|
| 51 |
-
|
| 52 |
-
|
| 53 |
-
try:
|
| 54 |
-
resolved = validated_path(
|
| 55 |
-
user_path,
|
| 56 |
-
allowed_roots=compute_workspace_roots(UPLOADS_DIR),
|
| 57 |
-
must_exist=False,
|
| 58 |
-
)
|
| 59 |
-
except PathValidationError as exc:
|
| 60 |
-
raise HTTPException(status_code=400, detail=str(exc)) from exc
|
| 61 |
-
return str(resolved)
|
| 62 |
|
| 63 |
|
| 64 |
# ──────────────────────────────────────────────────────────────────────────
|
|
|
|
| 7 |
from fastapi import APIRouter, HTTPException, Query
|
| 8 |
|
| 9 |
from picarones.interfaces.web.models import HTRUnitedImportRequest, HuggingFaceImportRequest
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| 10 |
|
| 11 |
router = APIRouter()
|
| 12 |
|
|
|
|
| 36 |
return HTRUnitedCatalogue.from_remote(timeout=5)
|
| 37 |
|
| 38 |
|
| 39 |
+
# Phase 7.2 audit code-quality (2026-05) : la fonction
|
| 40 |
+
# ``_validated_output_dir`` (helper local historique) est désormais
|
| 41 |
+
# importée depuis ``interfaces/web/_path_helpers.py`` sous le nom
|
| 42 |
+
# ``validated_user_output_dir`` (factorisation de 3 sites web). Alias
|
| 43 |
+
# conservé pour ne pas casser les usages locaux.
|
| 44 |
+
from picarones.interfaces.web._path_helpers import (
|
| 45 |
+
validated_user_output_dir as _validated_output_dir,
|
| 46 |
+
)
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| 47 |
|
| 48 |
|
| 49 |
# ──────────────────────────────────────────────────────────────────────────
|
|
@@ -46,19 +46,19 @@ Le runtime résout les ``adapter_name`` en instances via le
|
|
| 46 |
|
| 47 |
from __future__ import annotations
|
| 48 |
|
| 49 |
-
from typing import Literal
|
| 50 |
-
|
| 51 |
from picarones.domain.artifacts import ArtifactType
|
| 52 |
from picarones.domain.errors import PicaronesError
|
| 53 |
from picarones.domain.pipeline_spec import (
|
| 54 |
INITIAL_STEP_ID,
|
|
|
|
| 55 |
PipelineSpec,
|
| 56 |
PipelineStep,
|
| 57 |
)
|
| 58 |
|
| 59 |
|
| 60 |
-
#
|
| 61 |
-
|
|
|
|
| 62 |
|
| 63 |
|
| 64 |
def make_ocr_llm_pipeline_spec(
|
|
|
|
| 46 |
|
| 47 |
from __future__ import annotations
|
| 48 |
|
|
|
|
|
|
|
| 49 |
from picarones.domain.artifacts import ArtifactType
|
| 50 |
from picarones.domain.errors import PicaronesError
|
| 51 |
from picarones.domain.pipeline_spec import (
|
| 52 |
INITIAL_STEP_ID,
|
| 53 |
+
PipelineMode as OCRLLMPipelineMode,
|
| 54 |
PipelineSpec,
|
| 55 |
PipelineStep,
|
| 56 |
)
|
| 57 |
|
| 58 |
|
| 59 |
+
# Phase 7.1 audit code-quality (2026-05) : ``OCRLLMPipelineMode`` est
|
| 60 |
+
# désormais un alias de ``picarones.domain.pipeline_spec.PipelineMode``
|
| 61 |
+
# importé en tête de fichier — plus de définition locale.
|
| 62 |
|
| 63 |
|
| 64 |
def make_ocr_llm_pipeline_spec(
|
|
@@ -20,9 +20,12 @@ Attributs exposés
|
|
| 20 |
from __future__ import annotations
|
| 21 |
|
| 22 |
from dataclasses import dataclass, field
|
| 23 |
-
from typing import Any
|
| 24 |
|
| 25 |
-
|
|
|
|
|
|
|
|
|
|
| 26 |
|
| 27 |
|
| 28 |
@dataclass(frozen=True)
|
|
|
|
| 20 |
from __future__ import annotations
|
| 21 |
|
| 22 |
from dataclasses import dataclass, field
|
| 23 |
+
from typing import Any
|
| 24 |
|
| 25 |
+
# Phase 7.1 audit code-quality (2026-05) : alias historique conservé
|
| 26 |
+
# pour les callers existants ; la source de vérité unique vit
|
| 27 |
+
# désormais dans :data:`picarones.domain.pipeline_spec.PipelineMode`.
|
| 28 |
+
from picarones.domain.pipeline_spec import PipelineMode as OCRLLMMode
|
| 29 |
|
| 30 |
|
| 31 |
@dataclass(frozen=True)
|
|
@@ -0,0 +1,127 @@
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| 1 |
+
"""Phase 7.1 audit code-quality (2026-05) — ``PipelineMode`` doit
|
| 2 |
+
avoir **une seule** définition canonique : :data:`picarones.domain.pipeline_spec.PipelineMode`.
|
| 3 |
+
|
| 4 |
+
Avant la Phase 7.1 :
|
| 5 |
+
|
| 6 |
+
- ``picarones.pipeline.llm_pipeline_config.OCRLLMMode`` :
|
| 7 |
+
``Literal["text_only", "text_and_image", "zero_shot"]``
|
| 8 |
+
- ``picarones.pipeline.llm_pipeline_builder.OCRLLMPipelineMode`` :
|
| 9 |
+
idem (redéfini)
|
| 10 |
+
- ``picarones.interfaces.web.models.PipelineMode`` : idem (redéfini)
|
| 11 |
+
|
| 12 |
+
Trois définitions identiques mais indépendantes — risque concret de
|
| 13 |
+
divergence si l'une ajoute un mode (par exemple ``"hybrid"``) sans
|
| 14 |
+
mettre à jour les deux autres.
|
| 15 |
+
|
| 16 |
+
Refactor : ``domain/pipeline_spec.py`` (couche 1) accueille la
|
| 17 |
+
définition canonique ; les trois alias historiques (OCRLLMMode,
|
| 18 |
+
OCRLLMPipelineMode, PipelineMode) deviennent des re-exports pour
|
| 19 |
+
préserver les imports existants.
|
| 20 |
+
"""
|
| 21 |
+
|
| 22 |
+
from __future__ import annotations
|
| 23 |
+
|
| 24 |
+
import ast
|
| 25 |
+
from pathlib import Path
|
| 26 |
+
from typing import get_args
|
| 27 |
+
|
| 28 |
+
REPO_ROOT = Path(__file__).resolve().parents[2]
|
| 29 |
+
SOURCE = REPO_ROOT / "picarones"
|
| 30 |
+
|
| 31 |
+
|
| 32 |
+
def test_pipeline_mode_canonical_definition_exists() -> None:
|
| 33 |
+
"""La définition canonique vit dans ``domain/pipeline_spec.py``."""
|
| 34 |
+
from picarones.domain.pipeline_spec import PipelineMode
|
| 35 |
+
|
| 36 |
+
args = set(get_args(PipelineMode))
|
| 37 |
+
assert args == {"text_only", "text_and_image", "zero_shot"}, (
|
| 38 |
+
f"PipelineMode canonique doit valoir exactement les 3 littéraux "
|
| 39 |
+
f"canoniques. Reçu : {args}"
|
| 40 |
+
)
|
| 41 |
+
|
| 42 |
+
|
| 43 |
+
def test_pipeline_mode_aliases_are_identical() -> None:
|
| 44 |
+
"""Chaque alias historique pointe vers la même valeur Literal."""
|
| 45 |
+
from picarones.domain.pipeline_spec import PipelineMode as canonical
|
| 46 |
+
from picarones.interfaces.web.models import PipelineMode as web_alias
|
| 47 |
+
from picarones.pipeline.llm_pipeline_builder import OCRLLMPipelineMode as builder_alias
|
| 48 |
+
from picarones.pipeline.llm_pipeline_config import OCRLLMMode as config_alias
|
| 49 |
+
|
| 50 |
+
canonical_args = set(get_args(canonical))
|
| 51 |
+
for alias_name, alias in [
|
| 52 |
+
("interfaces.web.models.PipelineMode", web_alias),
|
| 53 |
+
("pipeline.llm_pipeline_builder.OCRLLMPipelineMode", builder_alias),
|
| 54 |
+
("pipeline.llm_pipeline_config.OCRLLMMode", config_alias),
|
| 55 |
+
]:
|
| 56 |
+
alias_args = set(get_args(alias))
|
| 57 |
+
assert alias_args == canonical_args, (
|
| 58 |
+
f"Alias {alias_name} diverge du canonique : "
|
| 59 |
+
f"{alias_args} != {canonical_args}. Vérifier que l'alias "
|
| 60 |
+
f"importe bien ``PipelineMode`` depuis ``domain/pipeline_spec``."
|
| 61 |
+
)
|
| 62 |
+
|
| 63 |
+
|
| 64 |
+
def test_no_independent_literal_definition_in_codebase() -> None:
|
| 65 |
+
"""Aucun fichier du code source (hors ``domain/pipeline_spec``)
|
| 66 |
+
ne doit redéfinir ``Literal["text_only", "text_and_image", "zero_shot"]``.
|
| 67 |
+
|
| 68 |
+
Scan AST. Refuse toute occurrence d'une annotation de type
|
| 69 |
+
``Literal[...]`` qui contient ces 3 chaînes — sauf dans le module
|
| 70 |
+
canonique.
|
| 71 |
+
"""
|
| 72 |
+
canonical_strings = frozenset({"text_only", "text_and_image", "zero_shot"})
|
| 73 |
+
canonical_file = SOURCE / "domain" / "pipeline_spec.py"
|
| 74 |
+
|
| 75 |
+
offenders: list[tuple[str, int]] = []
|
| 76 |
+
for path in SOURCE.rglob("*.py"):
|
| 77 |
+
if path == canonical_file or "__pycache__" in path.parts:
|
| 78 |
+
continue
|
| 79 |
+
text = path.read_text(encoding="utf-8")
|
| 80 |
+
try:
|
| 81 |
+
tree = ast.parse(text)
|
| 82 |
+
except SyntaxError:
|
| 83 |
+
continue
|
| 84 |
+
for node in ast.walk(tree):
|
| 85 |
+
# Cherche ``X = Literal["a", "b", ...]`` ou
|
| 86 |
+
# ``foo: Literal["a", "b", ...]`` partout.
|
| 87 |
+
if not isinstance(node, ast.Subscript):
|
| 88 |
+
continue
|
| 89 |
+
val = node.value
|
| 90 |
+
if not (
|
| 91 |
+
(isinstance(val, ast.Name) and val.id == "Literal")
|
| 92 |
+
or (isinstance(val, ast.Attribute) and val.attr == "Literal")
|
| 93 |
+
):
|
| 94 |
+
continue
|
| 95 |
+
# Extraire les chaînes des arguments.
|
| 96 |
+
slice_node = node.slice
|
| 97 |
+
if isinstance(slice_node, ast.Tuple):
|
| 98 |
+
elts = slice_node.elts
|
| 99 |
+
else:
|
| 100 |
+
elts = [slice_node]
|
| 101 |
+
strings = {
|
| 102 |
+
e.value for e in elts
|
| 103 |
+
if isinstance(e, ast.Constant) and isinstance(e.value, str)
|
| 104 |
+
}
|
| 105 |
+
if canonical_strings.issubset(strings) and len(strings) == 3:
|
| 106 |
+
rel = path.relative_to(REPO_ROOT).as_posix()
|
| 107 |
+
offenders.append((rel, node.lineno))
|
| 108 |
+
|
| 109 |
+
if offenders:
|
| 110 |
+
sample = "\n".join(f" {p}:{ln}" for p, ln in offenders)
|
| 111 |
+
raise AssertionError(
|
| 112 |
+
"Définition indépendante de ``PipelineMode`` "
|
| 113 |
+
"(``Literal[\"text_only\", \"text_and_image\", \"zero_shot\"]``) "
|
| 114 |
+
"détectée hors du module canonique :\n"
|
| 115 |
+
+ sample
|
| 116 |
+
+ "\n\nLa Phase 7.1 audit code-quality impose une source de vérité "
|
| 117 |
+
"unique dans ``picarones/domain/pipeline_spec.py``. Remplacer "
|
| 118 |
+
"la définition locale par ``from picarones.domain.pipeline_spec "
|
| 119 |
+
"import PipelineMode``."
|
| 120 |
+
)
|
| 121 |
+
|
| 122 |
+
|
| 123 |
+
def test_pipeline_mode_in_domain_all() -> None:
|
| 124 |
+
"""``PipelineMode`` est exporté par ``picarones.domain.pipeline_spec``."""
|
| 125 |
+
from picarones.domain import pipeline_spec
|
| 126 |
+
|
| 127 |
+
assert "PipelineMode" in pipeline_spec.__all__
|
|
@@ -0,0 +1,105 @@
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| 1 |
+
"""Phase 7.3 audit code-quality (2026-05) — le router
|
| 2 |
+
``/api/engines`` doit lister uniquement des engines connus de la
|
| 3 |
+
factory ``picarones.adapters.ocr.factory``.
|
| 4 |
+
|
| 5 |
+
L'audit avait identifié un risque de drift :
|
| 6 |
+
|
| 7 |
+
- ``picarones.adapters.ocr.factory._SUPPORTED`` (source de vérité)
|
| 8 |
+
- ``picarones.interfaces.web.routers.engines._collect_engines_sync``
|
| 9 |
+
énumère manuellement ``tesseract``, ``pero_ocr``, ``kraken``,
|
| 10 |
+
``calamari``, puis ``cloud_ocr_specs = (mistral_ocr, google_vision,
|
| 11 |
+
azure_doc_intel)``.
|
| 12 |
+
|
| 13 |
+
Si quelqu'un ajoute un nouvel adapter OCR à la factory sans
|
| 14 |
+
mettre à jour le router, ``/api/engines`` mentirait au frontend.
|
| 15 |
+
Si l'inverse, le frontend afficherait un engine non instanciable.
|
| 16 |
+
|
| 17 |
+
Plutôt qu'une refonte (le router a besoin de métadonnées
|
| 18 |
+
supplémentaires — label humain, nom de variable d'env, type
|
| 19 |
+
``local``/``cloud`` — que la factory ne fournit pas), ce test
|
| 20 |
+
verrouille l'invariant minimal : tout engine ID listé par le
|
| 21 |
+
router doit exister dans ``_SUPPORTED`` de la factory.
|
| 22 |
+
"""
|
| 23 |
+
|
| 24 |
+
from __future__ import annotations
|
| 25 |
+
|
| 26 |
+
|
| 27 |
+
def test_router_engines_subset_of_factory_supported() -> None:
|
| 28 |
+
"""Chaque engine listé par le router est dans la factory."""
|
| 29 |
+
from picarones.adapters.ocr.factory import _SUPPORTED
|
| 30 |
+
|
| 31 |
+
# Inspection AST du router pour extraire les engine_ids cités.
|
| 32 |
+
import ast
|
| 33 |
+
from pathlib import Path
|
| 34 |
+
|
| 35 |
+
router_path = (
|
| 36 |
+
Path(__file__).resolve().parents[2]
|
| 37 |
+
/ "picarones" / "interfaces" / "web" / "routers" / "engines.py"
|
| 38 |
+
)
|
| 39 |
+
tree = ast.parse(router_path.read_text(encoding="utf-8"))
|
| 40 |
+
|
| 41 |
+
# Stratégie : extraire toutes les chaînes littérales qui
|
| 42 |
+
# ressemblent à un engine_id (snake_case court). Les chaînes
|
| 43 |
+
# d'engine candidates sont les premiers args de
|
| 44 |
+
# ``check_engine(name, ...)`` et le 1er tuple de
|
| 45 |
+
# ``cloud_ocr_specs = ((engine_id, ...), ...)``.
|
| 46 |
+
cited: set[str] = set()
|
| 47 |
+
for node in ast.walk(tree):
|
| 48 |
+
# ``check_engine("name", ...)``
|
| 49 |
+
if isinstance(node, ast.Call):
|
| 50 |
+
func = node.func
|
| 51 |
+
is_check = (
|
| 52 |
+
(isinstance(func, ast.Name) and func.id == "check_engine")
|
| 53 |
+
or (
|
| 54 |
+
isinstance(func, ast.Attribute)
|
| 55 |
+
and func.attr == "check_engine"
|
| 56 |
+
)
|
| 57 |
+
)
|
| 58 |
+
if is_check and node.args:
|
| 59 |
+
first = node.args[0]
|
| 60 |
+
if isinstance(first, ast.Constant) and isinstance(first.value, str):
|
| 61 |
+
cited.add(first.value)
|
| 62 |
+
# ``cloud_ocr_specs = (("id", "label", "ENV"), ...)``
|
| 63 |
+
if (
|
| 64 |
+
isinstance(node, ast.Assign)
|
| 65 |
+
and len(node.targets) == 1
|
| 66 |
+
and isinstance(node.targets[0], ast.Name)
|
| 67 |
+
and node.targets[0].id == "cloud_ocr_specs"
|
| 68 |
+
and isinstance(node.value, ast.Tuple)
|
| 69 |
+
):
|
| 70 |
+
for tup in node.value.elts:
|
| 71 |
+
if isinstance(tup, ast.Tuple) and tup.elts:
|
| 72 |
+
first = tup.elts[0]
|
| 73 |
+
if isinstance(first, ast.Constant) and isinstance(first.value, str):
|
| 74 |
+
cited.add(first.value)
|
| 75 |
+
|
| 76 |
+
supported = set(_SUPPORTED)
|
| 77 |
+
unknown = cited - supported
|
| 78 |
+
assert not unknown, (
|
| 79 |
+
f"Le router ``/api/engines`` mentionne des engines qui "
|
| 80 |
+
f"n'existent pas dans la factory : {sorted(unknown)}.\n"
|
| 81 |
+
f"Factory supportée : {sorted(supported)}.\n"
|
| 82 |
+
f"Soit ajouter ces engines à la factory, soit les retirer du router."
|
| 83 |
+
)
|
| 84 |
+
|
| 85 |
+
|
| 86 |
+
def test_factory_supported_at_least_canonical() -> None:
|
| 87 |
+
"""``_SUPPORTED`` contient les engines historiquement annoncés."""
|
| 88 |
+
from picarones.adapters.ocr.factory import _SUPPORTED
|
| 89 |
+
|
| 90 |
+
minimal = {
|
| 91 |
+
"tesseract",
|
| 92 |
+
"pero_ocr",
|
| 93 |
+
"kraken",
|
| 94 |
+
"calamari",
|
| 95 |
+
"mistral_ocr",
|
| 96 |
+
"google_vision",
|
| 97 |
+
"azure_doc_intel",
|
| 98 |
+
"precomputed",
|
| 99 |
+
}
|
| 100 |
+
actual = set(_SUPPORTED)
|
| 101 |
+
missing = minimal - actual
|
| 102 |
+
assert not missing, (
|
| 103 |
+
f"Engines canoniques absents de _SUPPORTED : {sorted(missing)}. "
|
| 104 |
+
f"Régression de la factory."
|
| 105 |
+
)
|