Claude commited on
Commit
9d06a2f
·
unverified ·
1 Parent(s): 5c1eff9

docs(sprint-D): audit complet du retrait de measurements/runner/

Browse files

Sprint D.0 du plan v2.0 — pré-requis du sprint le plus gros
(8-10 jours d'effort estimé). Ce document détaille
exhaustivement les gaps entre ``measurements.runner.run_benchmark``
(legacy, 1319 LOC sur 7 fichiers) et
``app.services.BenchmarkService`` (rewrite, déjà canonique).

Contenu
-------
1. Surface du retrait : 7 modules legacy, 4 call-sites de
production (cli/_workflows : 2, web/benchmark_utils : 2).
2. API legacy ``run_benchmark`` : 13 paramètres, 12 features
identifiées (parallélisme, timeout, reprise, annulation,
progress, NER, profile, etc.).
3. API rewrite ``BenchmarkService.run`` : 9 paramètres, features
natives (manifest, views, run_id) ; **gaps** identifiés et
chiffrés sub-phase par sub-phase.
4. Plan ordonné D.1 → D.6 :
- D.1 — adapter de compat ``run_benchmark_via_service`` (2-3 j)
- D.2 — combler 6 gaps (3-4 j)
- D.3 — migrer ``web/benchmark_utils.run_benchmark_thread_v2`` (0.5 j)
- D.4 — migrer ou supprimer ``run_benchmark_thread`` legacy (0.3-1 j)
- D.5 — migrer 5 commandes CLI ``cli/_workflows`` (1.5 j)
- D.6 — suppression ``measurements/runner/`` (0.5 j)
5. Risques et mitigations (6 risques chiffrés).
6. Critères d'acceptation (10 checkpoints binaires).
7. Non-objectifs explicites (hors-scope Sprint D).

Découverte
----------
``fixtures.py`` ne consomme PAS ``run_benchmark`` directement —
il fabrique un ``BenchmarkResult`` synthétique en pur Python.
Le plan maître l'avait listé comme caller à migrer ; ce n'est
en fait pas nécessaire. La liste des callers réels passe donc
de 3 (annoncés) à 2 packages (4 sites).

Ce document est la **source de vérité** pour le Sprint D. Toute
déviation du plan doit être documentée en commit message
``docs(sprint-D): ajustement <raison>``.

https://claude.ai/code/session_011XQZNitg1rCgia8ZD1a2hP

Files changed (1) hide show
  1. docs/migration/sprint-D-audit.md +287 -0
docs/migration/sprint-D-audit.md ADDED
@@ -0,0 +1,287 @@
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
1
+ # Sprint D — Audit du retrait de `measurements/runner/`
2
+
3
+ **Sprint D du plan v2.0** — migration du runner legacy
4
+ (`measurements/runner/`) vers `app/services/BenchmarkService`.
5
+ Préparation à la suppression du sous-package, qui débloque
6
+ ensuite Phase 9 (Web → `interfaces/web/`) et Phase 10 (CLI →
7
+ `interfaces/cli/`).
8
+
9
+ Ce document est le pré-requis du Sprint D — il identifie
10
+ exhaustivement les **gaps** entre les deux services et le
11
+ **plan ordonné** des sub-phases D.1 à D.6.
12
+
13
+ ## 1. Surface du retrait
14
+
15
+ ### 1.1 — Modules à supprimer (`measurements/runner/`)
16
+
17
+ | Fichier | LOC | Rôle |
18
+ |---|---:|---|
19
+ | `__init__.py` | 103 | Re-exports |
20
+ | `orchestration.py` | 545 | `run_benchmark()` + boucle principale |
21
+ | `document.py` | 200 | `_compute_document_result()` |
22
+ | `partial.py` | 140 | Reprise sur interruption |
23
+ | `workers.py` | 116 | Pool process/thread |
24
+ | `aggregation.py` | 82 | Agrégation EngineReport |
25
+ | `ner_attach.py` | 133 | Calcul NER (Sprint 40) |
26
+ | **Total** | **1 319** | |
27
+
28
+ ### 1.2 — Callers à migrer
29
+
30
+ | Caller | Sites | Bloquant |
31
+ |---|---:|---|
32
+ | `cli/_workflows.py` | 2 (run_cmd, compare_cmd) | bloque Phase 10 |
33
+ | `web/benchmark_utils.py` | 2 (run_benchmark_thread + thread_v2) | bloque Phase 9 |
34
+ | `picarones/__init__.py` | 1 (docstring) | cosmétique |
35
+ | `picarones/measurements/__init__.py` | 1 (docstring) | supprimé avec measurements/ |
36
+ | `picarones/adapters/corpus/iiif.py` | 1 (docstring) | cosmétique |
37
+
38
+ **Total : 4 call-sites de production à migrer.** `fixtures.py` ne
39
+ consomme PAS `run_benchmark()` (il fabrique un `BenchmarkResult`
40
+ synthétique en pur Python).
41
+
42
+ ## 2. API legacy `run_benchmark()`
43
+
44
+ ### 2.1 — Signature
45
+
46
+ ```python
47
+ def run_benchmark(
48
+ corpus: Corpus, # legacy
49
+ engines: list[BaseOCREngine], # legacy (avec OCRLLMPipeline)
50
+ output_json: Optional[str | Path] = None, # I/O
51
+ show_progress: bool = True, # tqdm
52
+ progress_callback: Optional[callable] = None, # SSE web
53
+ char_exclude: Optional[frozenset] = None, # filtre
54
+ max_workers: int = 4, # parallélisme
55
+ timeout_seconds: float = 60.0, # timeout/doc
56
+ partial_dir: Optional[str | Path] = None, # reprise
57
+ cancel_event: Optional[threading.Event] = None, # annulation web
58
+ entity_extractor: Optional[callable] = None, # NER opt-in
59
+ profile: str = "standard", # profil mesures
60
+ normalization_profile: Optional[str] = None, # normalisation
61
+ ) -> BenchmarkResult:
62
+ ```
63
+
64
+ ### 2.2 — Features fournies
65
+
66
+ | Feature | Module | Mécanisme |
67
+ |---|---|---|
68
+ | Boucle (engines × corpus) | `orchestration.py` | imbriquée séquentielle |
69
+ | Parallélisme intra-engine | `workers.py` | `Process` ou `ThreadPoolExecutor` selon `engine.execution_mode` |
70
+ | Timeout par document | `workers.py` | Future + `concurrent.futures.wait` |
71
+ | Reprise interruption | `partial.py` | JSON par engine, document-par-document |
72
+ | Annulation propre | `orchestration.py` | `threading.Event` propagé |
73
+ | Progress bar | `orchestration.py` | tqdm + callback |
74
+ | OCR confidences | `document.py` | `EngineResult.token_confidences` |
75
+ | OCR+LLM metadata | `orchestration.py:520` | duck-typing `is_pipeline` (Sprint C.1 ✅) |
76
+ | Over-normalization | `document.py` | `detect_over_normalization(gt, ocr_int, hyp)` |
77
+ | NER calculations | `ner_attach.py` | Optionnel via `entity_extractor` |
78
+ | Profil normalisation | `orchestration.py` | `validate_profile` + `normalization_profile` |
79
+ | Aggregation EngineReport | `aggregation.py` | per-engine corpus stats |
80
+ | Fail-if-CER seuil | callers (CLI) | post-hoc sur `BenchmarkResult` |
81
+
82
+ ## 3. API rewrite `BenchmarkService`
83
+
84
+ ### 3.1 — Signature
85
+
86
+ ```python
87
+ class BenchmarkService:
88
+ def run(
89
+ self,
90
+ *,
91
+ corpus: CorpusSpec, # rewrite
92
+ pipelines: Iterable[PipelineSpec], # rewrite (déclaratif)
93
+ views: Iterable[EvaluationView],
94
+ ground_truth_factory: GroundTruthFactory,
95
+ pipeline_inputs_factory: PipelineInputsFactory,
96
+ context_factory: ContextFactory,
97
+ run_id: str | None = None,
98
+ dependencies_lock: dict[str, str] | None = None,
99
+ adapter_kwargs: dict[str, dict[str, Any]] | None = None,
100
+ metadata: dict[str, str] | None = None,
101
+ ) -> RunResult:
102
+ ```
103
+
104
+ ### 3.2 — Features fournies
105
+
106
+ | Feature | Disponible ? | Source |
107
+ |---|---|---|
108
+ | Parallélisme intra-pipeline | ✅ | `pipeline.runner.CorpusRunner` (max_in_flight) |
109
+ | Timeout par document | ✅ | `CorpusRunner.timeout_seconds_per_doc` |
110
+ | Annulation propre | ✅ | `threading.Event` (CorpusRunner) |
111
+ | `RunManifest` provenance | ✅ | `RunResult.manifest` |
112
+ | `EvaluationView` (S13+) | ✅ | natif |
113
+ | Run ID stable | ✅ | `run_id` arg |
114
+
115
+ ### 3.3 — Features manquantes
116
+
117
+ | Feature | Effort | Sub-phase |
118
+ |---|---|---|
119
+ | Progress callback compat web | Faible | D.2.a |
120
+ | tqdm progress bar | Faible | D.2.a |
121
+ | Reprise sur interruption (`partial.py`) | Moyen | D.2.b |
122
+ | `output_json` sérialisation directe | Faible | D.2.c |
123
+ | Conversion `BenchmarkResult` → `RunResult` | Élevé | D.3 |
124
+ | `over_normalization` aggregation | Moyen | D.2.d (déjà migré au volet 1) |
125
+ | NER attach via `entity_extractor` | Moyen | D.2.e |
126
+ | `profile` validation | Faible | D.2.f |
127
+ | `normalization_profile` | Faible | D.2.f |
128
+ | `char_exclude` filter | Faible | D.2.f |
129
+ | `fail_if_cer` (callers) | Faible | côté caller |
130
+
131
+ ## 4. Plan ordonné
132
+
133
+ ### D.0 — Audit (ce document) — fait ✅
134
+
135
+ ### D.1 — Adapter de compat `run_benchmark_via_service`
136
+
137
+ Fonction qui présente l'API legacy (`Corpus`, `engines`,
138
+ `output_json`, etc.) et construit en interne :
139
+
140
+ 1. `CorpusSpec` à partir du `Corpus` legacy (mapping
141
+ `Document` → `DocumentRef`).
142
+ 2. `PipelineSpec` à partir de chaque `BaseOCREngine` :
143
+ - OCR seul → spec mono-step via le builder
144
+ (`adapter_name=engine.name`, params=engine.config).
145
+ - `OCRLLMPipeline` → utilise déjà `make_ocr_llm_pipeline_spec`
146
+ en interne via Sprint B.
147
+ 3. Adapter resolver (`name → instance`) qui retrouve les engines
148
+ par leur `name`.
149
+ 4. Factories par défaut (ground_truth, pipeline_inputs, context).
150
+ 5. Lance `BenchmarkService.run(...)`.
151
+ 6. Convertit `RunResult` → `BenchmarkResult` legacy
152
+ (mapping inverse : `RunDocumentResult` → `DocumentResult`,
153
+ `pipeline_results` → `EngineReport`).
154
+
155
+ **Effort** : 2-3 j. **Risque** : la conversion bidirectionnelle
156
+ ``Corpus ↔ CorpusSpec`` et ``RunResult ↔ BenchmarkResult`` est la
157
+ partie délicate (les structures sont différentes par design).
158
+
159
+ ### D.2 — Combler les gaps `BenchmarkService`
160
+
161
+ | Sub-phase | Gap | Effort |
162
+ |---|---|---|
163
+ | D.2.a | progress callback + tqdm | 0.5 j |
164
+ | D.2.b | reprise interruption | 1 j |
165
+ | D.2.c | `output_json` sérialisation | 0.3 j |
166
+ | D.2.d | over_normalization aggregation | 0.5 j |
167
+ | D.2.e | NER attach via entity_extractor | 0.5 j |
168
+ | D.2.f | profile + normalization + char_exclude | 0.5 j |
169
+
170
+ **Total D.2** : ~3.3 j.
171
+
172
+ ### D.3 — Migrer `web/benchmark_utils.py:run_benchmark_thread_v2`
173
+
174
+ Le caller le plus simple à migrer (le plus récent, code propre) :
175
+ remplacer `run_benchmark(...)` par `run_benchmark_via_service(...)`.
176
+ Tests `tests/web/test_sprint28_ux_save_compare.py` doivent rester
177
+ verts.
178
+
179
+ **Effort** : 0.5 j.
180
+
181
+ ### D.4 — Migrer `web/benchmark_utils.py:run_benchmark_thread` (legacy)
182
+
183
+ Cette fonction est plus ancienne et utilise un format de competitor
184
+ configuration différent. Probablement redondante avec `_v2` —
185
+ candidat à la **suppression pure** plutôt qu'à la migration.
186
+
187
+ **Effort** : 0.3 j (suppression) ou 1 j (migration si conservé).
188
+
189
+ ### D.5 — Migrer `cli/_workflows.py`
190
+
191
+ 5 commandes : `run`, `diagnose`, `economics`, `edition`, `compare`.
192
+ Toutes appellent `run_benchmark()` directement. Migration par
193
+ commande, en commençant par la plus simple (`run`).
194
+
195
+ **Effort** : 1.5 j.
196
+
197
+ ### D.6 — Suppression `measurements/runner/`
198
+
199
+ Une fois tous les callers migrés :
200
+
201
+ ```bash
202
+ rm -r picarones/measurements/runner/
203
+ ```
204
+
205
+ Plus mise à jour des tests qui importaient depuis `runner` (51
206
+ fichiers) et des baselines architecturaux.
207
+
208
+ **Effort** : 0.5 j.
209
+
210
+ ## 5. Ordre d'enchaînement et durée
211
+
212
+ ```
213
+ D.0 (audit) ✅ fait
214
+
215
+ D.1 (adapter) ←─────┐
216
+ ↓ │
217
+ D.2 (gaps) ←──────┤ parallélisable
218
+ ↓ │
219
+ D.3 (web v2) │
220
+ ↓ │
221
+ D.4 (web v1, opt.) │
222
+ ↓ │
223
+ D.5 (CLI) │
224
+ ↓ │
225
+ D.6 (suppression) ←──────┘
226
+ ```
227
+
228
+ | Sub-phase | Effort |
229
+ |---|---:|
230
+ | D.1 | 2-3 j |
231
+ | D.2 | 3-4 j |
232
+ | D.3 | 0.5 j |
233
+ | D.4 | 0.3-1 j |
234
+ | D.5 | 1.5 j |
235
+ | D.6 | 0.5 j |
236
+ | **Total Sprint D** | **8-10 j** |
237
+
238
+ ## 6. Risques et mitigations
239
+
240
+ | Risque | Probabilité | Mitigation |
241
+ |---|---|---|
242
+ | Conversion `RunResult ↔ BenchmarkResult` perd des champs | Élevée | tests round-trip détaillés en D.1 |
243
+ | Performance dégradée du runner rewrite | Moyenne | benchmark de comparaison sur fixtures |
244
+ | Reprise sur interruption manque dans rewrite | Élevée | D.2.b prioritaire |
245
+ | Tests Sprint 15 (warnings LLM vide) cassent | Faible | Sprint B a déjà préservé les warnings |
246
+ | Web SSE callback signature incompatible | Moyenne | D.2.a en premier |
247
+ | CLI fail-if-cer logique côté caller | Faible | Reste côté CLI, ne touche pas le runner |
248
+
249
+ ## 7. Critères d'acceptation Sprint D
250
+
251
+ À l'issue de D.6 :
252
+
253
+ - [ ] `picarones/measurements/runner/` n'existe plus.
254
+ - [ ] `from picarones.measurements.runner import run_benchmark`
255
+ → ImportError.
256
+ - [ ] `web/benchmark_utils.py` consomme `BenchmarkService` (ou son
257
+ adapter).
258
+ - [ ] `cli/_workflows.py` consomme `BenchmarkService` (ou son
259
+ adapter).
260
+ - [ ] Tests CLI (Sprint 9, 11) verts.
261
+ - [ ] Tests Web (Sprint 6, 28) verts.
262
+ - [ ] Tests metrics (Sprint 3, 15) verts.
263
+ - [ ] Performance : pas de régression > 10 % sur fixtures
264
+ (corpus de 5 documents, 1 engine Tesseract).
265
+ - [ ] Reprise sur interruption : test `test_partial_resume.py`
266
+ vert (à créer).
267
+ - [ ] Phase 9 (Web → `interfaces/web/`) débloquée — plus aucun
268
+ import `measurements.runner` dans `web/`.
269
+ - [ ] Phase 10 (CLI → `interfaces/cli/`) débloquée — plus aucun
270
+ import `measurements.runner` dans `cli/`.
271
+
272
+ ## 8. Non-objectifs (hors-scope Sprint D)
273
+
274
+ - ❌ Refactor de `app/services/run_orchestrator.py` (déjà
275
+ canonique).
276
+ - ❌ Migration des métriques `measurements/*.py` (Sprint E).
277
+ - ❌ Migration des routes web (Sprint F).
278
+ - ❌ Migration des commandes CLI (Sprint G).
279
+ - ❌ Suppression de `OCRLLMPipeline` (Sprint D.6 inclura sa
280
+ suppression car `pipelines/_executor_runner.py` n'aura plus
281
+ d'utilité — mais c'est un effet de bord, pas l'objectif).
282
+
283
+ ---
284
+
285
+ **Document de référence** pour le Sprint D. Toute déviation du
286
+ plan ci-dessus doit être documentée en commit message
287
+ `docs(sprint-D): ajustement <raison>`.