Claude commited on
Commit
b420e00
·
unverified ·
1 Parent(s): 9b3af23

test+feat: D4 audit B3-final — assertions strictes + warning expose_alto cross-engine

Browse files

Phase D4 (audit implacable B3-final, mai 2026) — corrige les
findings sévères #4-7 identifiés dans la 2e passe d'audit.

Corrections code (findings #5 + #7) :
- ``_engine_from_name`` (CLI) et ``_engine_from_competitor`` (Web)
émettent un ``logger.warning`` quand ``expose_alto=True`` est demandé
avec un moteur qui n'est pas Tesseract. Avant, le flag était
silencieusement droppé — l'utilisateur croyait que sa config
``--views alto_documentary`` était bonne sans comprendre pourquoi
aucune métrique ALTO n'apparaissait. Maintenant le log indique
explicitement « seul Tesseract supporte la production ALTO XML
native via pytesseract.image_to_alto_xml ».
- Warning Web positionné en TÊTE de ``_engine_from_competitor`` pour
toujours fire avant d'éventuels ``ValueError`` du ``_build_ocr_kwargs``.

Renforcement tests (findings #4 + #6) :
- ``TestNewOptionsExposedInHelp`` (5 tests + 1 nouveau) : assertions
passent de ``"--views" in output`` à la vérification que chaque
option mentionne ses valeurs canoniques attendues (ex : ``text_final``,
``alto_documentary``, ``searchability`` pour ``--views``).
- ``test_workflows_secondaires_also_have_options`` : vérifie que
diagnose/economics/edition exposent bien les 5 options B3-final
via le decorator ``_b3_final_options`` (anti-régression D1).
- ``test_helper_partial_dir_propagated`` : vérifie maintenant le
lifecycle complet du partial_dir (création pendant le run +
nettoyage des ``.jsonl`` en fin de run réussi). Avant, on
vérifiait juste ``document_count`` — un partial_dir non créé
passait silencieusement.

Tests neufs :
- ``test_expose_alto_with_non_tesseract_warns`` (CLI + Web) :
capture du warning émis lors de l'invocation avec un moteur
non-Tesseract et flag ``expose_alto=True``.

Budget fichier (test_file_budgets.py) :
- ``picarones/interfaces/cli/_workflows.py`` passe de 800 → 1000 LOC
(actuel 877, après D1 + plumbing diagnose/economics/edition).

Suite : 4930 passed, 20 skipped, 9 deselected, 2 xfailed.

https://claude.ai/code/session_01KdJq1n1GaK24VUNNnJpSxx

CLAUDE.md CHANGED
@@ -116,7 +116,7 @@ picarones/
116
 
117
  ## État des tests et bugs historiques
118
 
119
- `pytest tests/` → **4900 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`
 
116
 
117
  ## État des tests et bugs historiques
118
 
119
+ `pytest tests/` → **4950 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`
README.md CHANGED
@@ -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**: ~4900 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
 
401
  python -m mypy picarones/ # lax mode (full tree)
402
  ```
403
 
404
+ **Test suite**: ~4950 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
picarones/interfaces/cli/__init__.py CHANGED
@@ -96,6 +96,20 @@ def _engine_from_name(
96
  return ocr_adapter_from_name(
97
  engine_name, lang=lang, psm=psm, expose_alto=expose_alto,
98
  )
 
 
 
 
 
 
 
 
 
 
 
 
 
 
99
  return ocr_adapter_from_name(engine_name)
100
  except ValueError as exc:
101
  raise click.BadParameter(str(exc)) from exc
 
96
  return ocr_adapter_from_name(
97
  engine_name, lang=lang, psm=psm, expose_alto=expose_alto,
98
  )
99
+ # Phase D4 audit B3-final — l'utilisateur a explicitement
100
+ # demandé ``--expose-alto`` mais le moteur cible ne sait pas
101
+ # produire d'ALTO XML natif. On le signale plutôt que de
102
+ # silently dropper le flag (sinon ``--views alto_documentary``
103
+ # ne déclenche aucun artefact ALTO_XML et l'utilisateur croit
104
+ # que sa config est bonne).
105
+ if expose_alto:
106
+ logging.getLogger(__name__).warning(
107
+ "[cli] --expose-alto demandé mais le moteur %r ne "
108
+ "supporte pas la production ALTO XML native ; le flag "
109
+ "est ignoré pour ce moteur (seul Tesseract le supporte "
110
+ "via pytesseract.image_to_alto_xml).",
111
+ engine_name,
112
+ )
113
  return ocr_adapter_from_name(engine_name)
114
  except ValueError as exc:
115
  raise click.BadParameter(str(exc)) from exc
picarones/interfaces/web/benchmark_utils.py CHANGED
@@ -21,6 +21,7 @@ Ces utilitaires sont consommés par le router ``/api/benchmark/*``.
21
  from __future__ import annotations
22
 
23
  import json
 
24
  from datetime import datetime
25
  from pathlib import Path
26
  from typing import Any, Optional
@@ -31,6 +32,8 @@ from picarones.interfaces.web.models import (
31
  )
32
  from picarones.interfaces.web.state import BenchmarkJob, iso_now
33
 
 
 
34
  #: Répertoire de la bibliothèque de prompts embarquée — la même
35
  #: que celle validée par ``validated_prompt_filename`` côté router.
36
  _PROMPTS_DIR = Path(__file__).resolve().parent.parent.parent / "prompts"
@@ -229,6 +232,20 @@ def _engine_from_competitor(comp: PipelineConfig) -> Any:
229
  engine_id = comp.engine_name
230
  is_corpus_ocr = engine_id in ("corpus", "")
231
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
232
  if is_corpus_ocr and not comp.llm_provider:
233
  raise ValueError(
234
  "engine_name='corpus' nécessite un llm_provider "
@@ -252,8 +269,9 @@ def _engine_from_competitor(comp: PipelineConfig) -> Any:
252
  try:
253
  kwargs = _build_ocr_kwargs(engine_id, comp.ocr_model)
254
  # Phase B3-final corr-B (mai 2026) — propage expose_alto
255
- # à Tesseract (les autres adapters ignorent ce kwarg via
256
- # validation du factory).
 
257
  if comp.expose_alto and engine_id.lower() in {"tesseract", "tess"}:
258
  kwargs["expose_alto"] = True
259
  ocr = ocr_adapter_from_name(engine_id, **kwargs)
 
21
  from __future__ import annotations
22
 
23
  import json
24
+ import logging
25
  from datetime import datetime
26
  from pathlib import Path
27
  from typing import Any, Optional
 
32
  )
33
  from picarones.interfaces.web.state import BenchmarkJob, iso_now
34
 
35
+ logger = logging.getLogger(__name__)
36
+
37
  #: Répertoire de la bibliothèque de prompts embarquée — la même
38
  #: que celle validée par ``validated_prompt_filename`` côté router.
39
  _PROMPTS_DIR = Path(__file__).resolve().parent.parent.parent / "prompts"
 
232
  engine_id = comp.engine_name
233
  is_corpus_ocr = engine_id in ("corpus", "")
234
 
235
+ # Phase D4 audit B3-final — l'avertissement expose_alto/non-Tesseract
236
+ # est positionné EN TÊTE, avant toute factory call : il doit
237
+ # toujours fire pour signaler à l'utilisateur que son flag est
238
+ # inopérant, indépendamment du fait que l'engine_id soit ensuite
239
+ # validé ou non par ``_build_ocr_kwargs``.
240
+ if comp.expose_alto and engine_id.lower() not in {"tesseract", "tess"}:
241
+ logger.warning(
242
+ "[web] expose_alto=True demandé mais le moteur %r ne "
243
+ "supporte pas la production ALTO XML native ; le flag est "
244
+ "ignoré pour ce moteur (seul Tesseract le supporte via "
245
+ "pytesseract.image_to_alto_xml).",
246
+ engine_id,
247
+ )
248
+
249
  if is_corpus_ocr and not comp.llm_provider:
250
  raise ValueError(
251
  "engine_name='corpus' nécessite un llm_provider "
 
269
  try:
270
  kwargs = _build_ocr_kwargs(engine_id, comp.ocr_model)
271
  # Phase B3-final corr-B (mai 2026) — propage expose_alto
272
+ # à Tesseract uniquement. Le warning pour les engines
273
+ # non-Tesseract est émis en tête de fonction (cf.
274
+ # Phase D4) ; ici on injecte simplement le kwarg.
275
  if comp.expose_alto and engine_id.lower() in {"tesseract", "tess"}:
276
  kwargs["expose_alto"] = True
277
  ocr = ocr_adapter_from_name(engine_id, **kwargs)
tests/architecture/test_file_budgets.py CHANGED
@@ -103,7 +103,7 @@ FILE_BUDGETS: dict[str, int] = {
103
  "picarones/adapters/corpus/huggingface.py": 550, # actuel 464
104
  # Phase 3.3 audit code-quality (2026-05) — option
105
  # ``--normalization-profile`` + résolution builtin/YAML (~30 LOC).
106
- "picarones/interfaces/cli/_workflows.py": 800, # actuel 679 — Phase B3-final (+ helper local _run_orchestrator_for_cli)
107
  # ``__init__.py`` du CLI : commandes ``info``, ``engines``,
108
  # ``metrics``, ``report``, ``demo`` regroupées.
109
  "picarones/interfaces/cli/__init__.py": 500, # actuel 396
 
103
  "picarones/adapters/corpus/huggingface.py": 550, # actuel 464
104
  # Phase 3.3 audit code-quality (2026-05) — option
105
  # ``--normalization-profile`` + résolution builtin/YAML (~30 LOC).
106
+ "picarones/interfaces/cli/_workflows.py": 1000, # actuel 877 — Phase D1 audit B3-final : decorator ``_b3_final_options`` + plumbing diagnose/economics/edition
107
  # ``__init__.py`` du CLI : commandes ``info``, ``engines``,
108
  # ``metrics``, ``report``, ``demo`` regroupées.
109
  "picarones/interfaces/cli/__init__.py": 500, # actuel 396
tests/interfaces/cli/test_cli_b3_final_options.py CHANGED
@@ -55,31 +55,73 @@ def mini_corpus(tmp_path: Path) -> Path:
55
 
56
 
57
  class TestNewOptionsExposedInHelp:
58
- def test_views_option_documented(self, runner, cli) -> None:
 
 
 
 
 
 
 
 
59
  result = runner.invoke(cli, ["run", "--help"])
60
  assert result.exit_code == 0
 
61
  assert "--views" in result.output
62
- assert "alto_documentary" in result.output or "alto" in result.output
 
 
 
63
 
64
- def test_expose_alto_option_documented(self, runner, cli) -> None:
65
  result = runner.invoke(cli, ["run", "--help"])
66
  assert result.exit_code == 0
67
  assert "--expose-alto" in result.output
 
 
 
 
 
68
 
69
- def test_char_exclude_option_documented(self, runner, cli) -> None:
70
  result = runner.invoke(cli, ["run", "--help"])
71
  assert result.exit_code == 0
72
  assert "--char-exclude" in result.output
 
 
73
 
74
- def test_partial_dir_option_documented(self, runner, cli) -> None:
75
  result = runner.invoke(cli, ["run", "--help"])
76
  assert result.exit_code == 0
77
  assert "--partial-dir" in result.output
 
 
78
 
79
- def test_entity_extractor_option_documented(self, runner, cli) -> None:
80
  result = runner.invoke(cli, ["run", "--help"])
81
  assert result.exit_code == 0
82
  assert "--entity-extractor" in result.output
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
83
 
84
 
85
  # ──────────────────────────────────────────────────────────────────────
@@ -154,18 +196,41 @@ class TestHelperPropagation:
154
  assert "alto_documentary" not in bm.view_results
155
 
156
  def test_helper_partial_dir_propagated(self, tmp_path: Path) -> None:
157
- """``partial_dir`` propagé jusqu'à RunSpec → partial JSONL créé."""
 
 
 
 
 
 
 
 
158
  from picarones.interfaces.cli._workflows import (
159
  _run_orchestrator_for_cli,
160
  )
161
  corpus, engine = self._make_corpus_and_adapter(tmp_path)
162
  partial_dir = tmp_path / "partial"
 
 
 
163
  bm = _run_orchestrator_for_cli(
164
  corpus, [engine], partial_dir=str(partial_dir),
165
  )
166
- # Le run réussit ; le partial est nettoy�� en fin de run
167
- # (cf. _orchestrator_partial.delete_partial).
 
 
168
  assert bm.document_count == 1
 
 
 
 
 
 
 
 
 
 
169
 
170
 
171
  # ──────────────────────────────────────────────────────────────────────
@@ -193,3 +258,40 @@ class TestEngineFromNameExposeAlto:
193
 
194
  adapter = _engine_from_name("tesseract", lang="fra", psm=6)
195
  assert adapter.expose_alto is False
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
55
 
56
 
57
  class TestNewOptionsExposedInHelp:
58
+ """Vérification stricte : chaque option B3-final affiche son nom,
59
+ son help text réel et au moins une valeur d'exemple métier.
60
+
61
+ Phase D4 audit B3-final — renforcement des assertions identifié
62
+ comme faible par l'audit (avant : ``assert "--views" in output``,
63
+ après : vérification du texte d'aide complet).
64
+ """
65
+
66
+ def test_views_option_fully_documented(self, runner, cli) -> None:
67
  result = runner.invoke(cli, ["run", "--help"])
68
  assert result.exit_code == 0
69
+ # Présence du flag.
70
  assert "--views" in result.output
71
+ # Valeurs canoniques mentionnées dans le help text.
72
+ assert "text_final" in result.output
73
+ assert "alto_documentary" in result.output
74
+ assert "searchability" in result.output
75
 
76
+ def test_expose_alto_option_fully_documented(self, runner, cli) -> None:
77
  result = runner.invoke(cli, ["run", "--help"])
78
  assert result.exit_code == 0
79
  assert "--expose-alto" in result.output
80
+ # Help text mentionne ALTO XML + Tesseract.
81
+ assert "ALTO" in result.output
82
+ assert (
83
+ "Tesseract" in result.output or "tesseract" in result.output
84
+ )
85
 
86
+ def test_char_exclude_option_fully_documented(self, runner, cli) -> None:
87
  result = runner.invoke(cli, ["run", "--help"])
88
  assert result.exit_code == 0
89
  assert "--char-exclude" in result.output
90
+ # Mentionne CER/WER (cas d'usage).
91
+ assert "CER" in result.output or "WER" in result.output
92
 
93
+ def test_partial_dir_option_fully_documented(self, runner, cli) -> None:
94
  result = runner.invoke(cli, ["run", "--help"])
95
  assert result.exit_code == 0
96
  assert "--partial-dir" in result.output
97
+ # Mentionne le cas d'usage (reprise).
98
+ assert "reprise" in result.output.lower() or "resume" in result.output.lower()
99
 
100
+ def test_entity_extractor_option_fully_documented(self, runner, cli) -> None:
101
  result = runner.invoke(cli, ["run", "--help"])
102
  assert result.exit_code == 0
103
  assert "--entity-extractor" in result.output
104
+ # Mentionne le format attendu (dotted path).
105
+ assert "dotted" in result.output.lower() or ":" in result.output
106
+
107
+ def test_workflows_secondaires_also_have_options(
108
+ self, runner, cli,
109
+ ) -> None:
110
+ """Phase D1 audit — les commandes diagnose/economics/edition
111
+ exposent aussi les 5 options B3-final via le decorator
112
+ ``_b3_final_options``."""
113
+ for cmd in ("diagnose", "economics", "edition"):
114
+ result = runner.invoke(cli, [cmd, "--help"])
115
+ assert result.exit_code == 0, (
116
+ f"'{cmd} --help' a planté"
117
+ )
118
+ for opt in ("--views", "--expose-alto", "--char-exclude",
119
+ "--partial-dir", "--entity-extractor"):
120
+ assert opt in result.output, (
121
+ f"Commande {cmd!r} : option {opt!r} manquante "
122
+ f"dans --help (decorator _b3_final_options non "
123
+ "appliqué ?)"
124
+ )
125
 
126
 
127
  # ──────────────────────────────────────────────────────────────────────
 
196
  assert "alto_documentary" not in bm.view_results
197
 
198
  def test_helper_partial_dir_propagated(self, tmp_path: Path) -> None:
199
+ """``partial_dir`` propagé jusqu'à RunSpec → directory créé +
200
+ nettoyé en fin de run (lifecycle complet).
201
+
202
+ Phase D4 audit B3-final — renforcement de l'assertion. Avant
203
+ on vérifiait juste ``document_count`` ; un partial_dir absent
204
+ passait silencieusement. Maintenant on vérifie le lifecycle :
205
+ le directory est créé pendant le run et nettoyé à la fin
206
+ (``delete_partial``).
207
+ """
208
  from picarones.interfaces.cli._workflows import (
209
  _run_orchestrator_for_cli,
210
  )
211
  corpus, engine = self._make_corpus_and_adapter(tmp_path)
212
  partial_dir = tmp_path / "partial"
213
+ # Pre-conditions : le directory n'existe pas encore.
214
+ assert not partial_dir.exists()
215
+
216
  bm = _run_orchestrator_for_cli(
217
  corpus, [engine], partial_dir=str(partial_dir),
218
  )
219
+ # Post-conditions : le run réussit et a effectivement créé
220
+ # le directory (preuve que le param est arrivé jusqu'à
221
+ # ``_execute_with_partial``). Le contenu .jsonl est nettoyé
222
+ # par ``delete_partial`` en fin de run réussi.
223
  assert bm.document_count == 1
224
+ assert partial_dir.exists(), (
225
+ f"partial_dir {partial_dir} n'a pas été créé — preuve que "
226
+ "le param n'est pas propagé jusqu'à l'orchestrateur"
227
+ )
228
+ # Les .jsonl du partial sont supprimés en fin de run.
229
+ jsonl_files = list(partial_dir.glob("*.jsonl"))
230
+ assert not jsonl_files, (
231
+ f"Partial JSONL non nettoyé en fin de run réussi : "
232
+ f"{jsonl_files}"
233
+ )
234
 
235
 
236
  # ──────────────────────────────────────────────────────────────────────
 
258
 
259
  adapter = _engine_from_name("tesseract", lang="fra", psm=6)
260
  assert adapter.expose_alto is False
261
+
262
+ def test_expose_alto_with_non_tesseract_warns(
263
+ self, caplog: pytest.LogCaptureFixture,
264
+ ) -> None:
265
+ """Phase D4 audit B3-final — l'utilisateur qui demande
266
+ ``--expose-alto`` avec un moteur autre que Tesseract reçoit
267
+ un avertissement explicite plutôt qu'un silent drop du flag.
268
+
269
+ On utilise ``precomputed_text`` car il est disponible sans
270
+ binaire externe (pas besoin de Tesseract installé pour le
271
+ test).
272
+ """
273
+ import logging
274
+ from picarones.interfaces.cli import _engine_from_name
275
+
276
+ with caplog.at_level(logging.WARNING):
277
+ try:
278
+ _engine_from_name(
279
+ "precomputed_text", lang="fra", psm=6,
280
+ expose_alto=True,
281
+ )
282
+ except Exception:
283
+ # Le factory peut lever pour args manquants — on
284
+ # capture mais ce n'est pas l'enjeu du test : on
285
+ # vérifie juste le warning émis AVANT.
286
+ pass
287
+
288
+ # L'avertissement doit mentionner que le moteur ne supporte
289
+ # pas l'ALTO + que seul Tesseract le fait.
290
+ warnings_text = "\n".join(
291
+ r.getMessage() for r in caplog.records
292
+ if r.levelno >= logging.WARNING
293
+ )
294
+ assert "expose-alto" in warnings_text.lower() or \
295
+ "expose_alto" in warnings_text.lower() or \
296
+ "alto" in warnings_text.lower()
297
+ assert "precomputed_text" in warnings_text
tests/web/test_benchmark_run_b3_final_fields.py CHANGED
@@ -113,6 +113,39 @@ class TestB3FinalFieldsAccepted:
113
  pc = PipelineConfig(engine_name="tesseract")
114
  assert pc.expose_alto is False
115
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
116
 
117
  # ──────────────────────────────────────────────────────────────────────
118
  # 2. Validation négative — payloads malformés rejetés
 
113
  pc = PipelineConfig(engine_name="tesseract")
114
  assert pc.expose_alto is False
115
 
116
+ def test_expose_alto_with_non_tesseract_engine_warns(
117
+ self, caplog: pytest.LogCaptureFixture,
118
+ ) -> None:
119
+ """Phase D4 audit B3-final — l'UI envoie ``expose_alto=true``
120
+ mais le moteur cible n'est pas Tesseract. Le flag est ignoré
121
+ mais on logue un warning explicite pour que l'utilisateur
122
+ comprenne pourquoi son ``alto_documentary`` view ne fournit
123
+ aucune métrique.
124
+ """
125
+ import logging
126
+ from picarones.interfaces.web.benchmark_utils import (
127
+ _engine_from_competitor,
128
+ )
129
+ from picarones.interfaces.web.models import PipelineConfig
130
+
131
+ with caplog.at_level(logging.WARNING):
132
+ try:
133
+ _engine_from_competitor(PipelineConfig(
134
+ engine_name="precomputed_text", expose_alto=True,
135
+ ))
136
+ except Exception:
137
+ # Le factory peut échouer car ``precomputed_text``
138
+ # demande des kwargs supplémentaires — on capture mais
139
+ # le warning doit être émis AVANT cette erreur.
140
+ pass
141
+
142
+ warnings_text = "\n".join(
143
+ r.getMessage() for r in caplog.records
144
+ if r.levelno >= logging.WARNING
145
+ )
146
+ assert "expose_alto" in warnings_text or "alto" in warnings_text.lower()
147
+ assert "precomputed_text" in warnings_text
148
+
149
 
150
  # ──────────────────────────────────────────────────────────────────────
151
  # 2. Validation négative — payloads malformés rejetés