Claude commited on
Commit
b5bb4fa
·
unverified ·
1 Parent(s): 01bf885

fix(cli): garde-fou migration --fail-if-cer-above (Click callback)

Browse files

Suite à un audit de mon propre fix précédent : la sémantique fraction
introduite cassait silencieusement les callers externes qui utilisaient
l'ancienne sémantique pourcentage (15.0 = 15 %). Avec le seuil compris
comme fraction, 15.0 = 1500 % et la condition ne se déclenche jamais —
l'utilisateur croit que son CI est sain alors qu'il ne lève plus.

Solution : convertir la cassure silencieuse en cassure bruyante via
un Click callback de validation.

picarones/cli/_workflows.py
---------------------------
- Nouveau callback ``_validate_cer_threshold`` rejette à l'analyse
Click :
· valeur < 0 → BadParameter "doit être ≥ 0"
· valeur > 1.0 → BadParameter "doit être une fraction ∈ [0, 1]
(ex : 0.15 = 15 %), reçu N. Si vous utilisiez l'ancienne
sémantique pourcentage, divisez par 100 (ex : 15.0 → 0.15)."
- Validation à l'analyse, AVANT le chargement du corpus / Tesseract
/ OCR — un utilisateur qui se trompe découvre l'erreur en < 100 ms,
pas après 36 secondes.
- Suppression de la validation dupliquée dans le corps de la fonction
(redondante puisque le callback Click rejette en amont).

CHANGELOG.md
------------
Entrée [Unreleased] explicite la BREAKING CHANGE :
- L'option attend une fraction ∈ [0, 1], pas un pourcentage.
- Migration : 15.0 → 0.15.
- Le garde-fou Click callback rend la migration *bruyante* (le fix
ne peut pas être ignoré).

Tests S46 dédiés (4 ajoutés)
----------------------------
- TestMigrationGuard : 15.0 rejeté avec hint "divisez par 100",
-0.1 rejeté avec hint "≥ 0", 1.0 et 0.0 acceptés (bornes valides).

Tests : 14/14 dans tests/cli/test_fail_if_cer_above_semantics.py
+ 85 cli tests sans régression.
Lint : ruff check picarones/ tests/ → All checks passed.

Pourquoi cette deuxième passe
-----------------------------
Le premier commit (01bf885) faisait le bon fix conceptuel mais sans
filet de sécurité contre la migration. Un caller externe inconnu
(scripts BnF, HuggingFace Space CI, doc tierce) qui passait 15.0
aurait basculé silencieusement sur un seuil de 1500 %, jamais
déclenché — exactement le genre de comportement que la directive
"sans dette technique" exige d'éviter.

https://claude.ai/code/session_011XQZNitg1rCgia8ZD1a2hP

CHANGELOG.md CHANGED
@@ -7,6 +7,39 @@ La numérotation de version suit [Semantic Versioning](https://semver.org/lang/f
7
 
8
  ---
9
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
10
  ## [post-Sprint 97] — chantiers de consolidation — 2026-04 → ongoing
11
 
12
  > 6 chantiers de consolidation **sans suppression** sur la branche
 
7
 
8
  ---
9
 
10
+ ## [Unreleased] — fix CI perf_regression — 2026-05
11
+
12
+ ### ⚠️ BREAKING CHANGE — sémantique `--fail-if-cer-above`
13
+
14
+ L'option `picarones run --fail-if-cer-above` interprétait sa valeur
15
+ comme un **pourcentage** (ex : `15.0` = 15 %). Désormais elle attend
16
+ une **fraction** ∈ [0, 1] (ex : `0.15` = 15 %), cohérent avec la
17
+ représentation interne de `BenchmarkResult.ranking()[i]["mean_cer"]`.
18
+
19
+ **Migration** : si vous passiez `--fail-if-cer-above 15.0` (intention
20
+ « 15 % »), passez maintenant `--fail-if-cer-above 0.15`.
21
+
22
+ **Garde-fou** : un callback Click rejette à l'analyse toute valeur
23
+ > 1.0 avec un message de migration explicite — la cassure est
24
+ **bruyante**, pas silencieuse. Il est impossible de basculer
25
+ silencieusement sur l'ancienne sémantique.
26
+
27
+ **Pourquoi** : le job CI hebdomadaire `perf_regression.yml` passait
28
+ `0.15` en pensant fraction, mais la CLI le traitait comme 0.15 % et
29
+ échouait toujours. Le fix aligne la sémantique avec l'intention
30
+ documentée et avec la représentation interne de `mean_cer`.
31
+
32
+ **Tests anti-régression** (10) dans
33
+ `tests/cli/test_fail_if_cer_above_semantics.py` :
34
+
35
+ - Sémantique fraction (sous/au seuil/None/strict 1 %/lax 50 %).
36
+ - `perf_regression.yml` doit passer une valeur ∈ ]0, 1].
37
+ - Help texte mentionne explicitement « fraction ».
38
+ - Migration guard : `15.0` → `BadParameter` avec hint « divisez par 100 ».
39
+ - `1.0` et `0.0` acceptés (bornes valides).
40
+
41
+ ---
42
+
43
  ## [post-Sprint 97] — chantiers de consolidation — 2026-04 → ongoing
44
 
45
  > 6 chantiers de consolidation **sans suppression** sur la branche
picarones/cli/_workflows.py CHANGED
@@ -16,6 +16,38 @@ import click
16
 
17
  from picarones.cli import cli, _engine_from_name, _setup_logging
18
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
19
  # ---------------------------------------------------------------------------
20
  # picarones run
21
  # ---------------------------------------------------------------------------
@@ -54,6 +86,7 @@ from picarones.cli import cli, _engine_from_name, _setup_logging
54
  default=None,
55
  type=float,
56
  metavar="THRESHOLD",
 
57
  help=(
58
  "Quitte avec code 1 si CER moyen > THRESHOLD (usage CI/CD). "
59
  "THRESHOLD est une fraction ∈ [0, 1] (ex : 0.15 = 15 %)."
@@ -89,6 +122,10 @@ def run_cmd(
89
 
90
  Le corpus doit être un dossier contenant des paires
91
  <image>.<ext> + <image>.gt.txt (vérité terrain).
 
 
 
 
92
  """
93
  _setup_logging(verbose)
94
 
@@ -143,8 +180,7 @@ def run_cmd(
143
  click.echo(f"\nRésultats écrits dans : {output}")
144
 
145
  # Mode CI/CD : exit code non-zero si CER > seuil.
146
- # ``fail_if_cer_above`` est une fraction ∈ [0, 1] (ex : 0.15 = 15 %),
147
- # cohérent avec la représentation interne de ``mean_cer``.
148
  if fail_if_cer_above is not None:
149
  for entry in result.ranking():
150
  if (
 
16
 
17
  from picarones.cli import cli, _engine_from_name, _setup_logging
18
 
19
+
20
+ def _validate_cer_threshold(
21
+ ctx: click.Context, param: click.Parameter, value: float | None,
22
+ ) -> float | None:
23
+ """Callback Click qui valide ``--fail-if-cer-above`` à l'analyse.
24
+
25
+ Sémantique : fraction ∈ [0, 1] (ex : 0.15 = 15 %), cohérent avec
26
+ ``BenchmarkResult.ranking()[i]["mean_cer"]`` qui est aussi en
27
+ fraction.
28
+
29
+ Garde-fou migration : avant le fix de sémantique, le seuil était
30
+ interprété comme un pourcentage (15.0 = 15 %). Tout caller qui
31
+ passe encore une valeur > 1 vient de l'ancienne sémantique — on
32
+ échoue bruyamment plutôt que de muter silencieusement le
33
+ comportement (un seuil de 1500 % ne se déclencherait jamais et
34
+ l'utilisateur croirait que son CI est sain).
35
+ """
36
+ if value is None:
37
+ return None
38
+ if value < 0:
39
+ raise click.BadParameter(
40
+ f"doit être ≥ 0, reçu {value}.",
41
+ )
42
+ if value > 1.0:
43
+ raise click.BadParameter(
44
+ f"doit être une fraction ∈ [0, 1] (ex : 0.15 = 15 %), "
45
+ f"reçu {value}. Si vous utilisiez l'ancienne sémantique "
46
+ "pourcentage, divisez par 100 (ex : 15.0 → 0.15).",
47
+ )
48
+ return value
49
+
50
+
51
  # ---------------------------------------------------------------------------
52
  # picarones run
53
  # ---------------------------------------------------------------------------
 
86
  default=None,
87
  type=float,
88
  metavar="THRESHOLD",
89
+ callback=_validate_cer_threshold,
90
  help=(
91
  "Quitte avec code 1 si CER moyen > THRESHOLD (usage CI/CD). "
92
  "THRESHOLD est une fraction ∈ [0, 1] (ex : 0.15 = 15 %)."
 
122
 
123
  Le corpus doit être un dossier contenant des paires
124
  <image>.<ext> + <image>.gt.txt (vérité terrain).
125
+
126
+ ``--fail-if-cer-above`` est validé à l'analyse Click (cf.
127
+ ``_validate_cer_threshold``) — une valeur invalide est rejetée
128
+ avant toute opération coûteuse.
129
  """
130
  _setup_logging(verbose)
131
 
 
180
  click.echo(f"\nRésultats écrits dans : {output}")
181
 
182
  # Mode CI/CD : exit code non-zero si CER > seuil.
183
+ # ``fail_if_cer_above`` est déjà validé en tête de fonction (∈ [0, 1]).
 
184
  if fail_if_cer_above is not None:
185
  for entry in result.ranking():
186
  if (
tests/cli/test_fail_if_cer_above_semantics.py CHANGED
@@ -168,3 +168,69 @@ class TestCliEndToEnd:
168
  # Le message doit afficher les deux valeurs en pourcentage clair.
169
  assert "12.00%" in msg
170
  assert "5.00%" in msg
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
168
  # Le message doit afficher les deux valeurs en pourcentage clair.
169
  assert "12.00%" in msg
170
  assert "5.00%" in msg
171
+
172
+
173
+ # ──────────────────────────────────────────────────────────────────────
174
+ # Garde-fou migration : valeurs > 1.0 rejetées avec message clair
175
+ # ──────────────────────────────────────────────────────────────────────
176
+
177
+
178
+ class TestMigrationGuard:
179
+ """Avant le fix B, ``--fail-if-cer-above 15.0`` voulait dire 15 %
180
+ (sémantique pourcentage). Avec la nouvelle sémantique fraction,
181
+ un caller qui passe encore 15.0 par erreur doit obtenir une
182
+ erreur explicite plutôt qu'un comportement silencieusement faux
183
+ (seuil 1500 % qui ne se déclenche jamais)."""
184
+
185
+ def _invoke(
186
+ self, threshold: str, tmp_path: Path,
187
+ ) -> tuple[int, str]:
188
+ """Invoque ``picarones run --fail-if-cer-above THRESHOLD`` avec
189
+ un corpus tmp vide pour aller jusqu'à la validation du seuil
190
+ à l'analyse Click (callback ``_validate_cer_threshold``).
191
+ Une valeur invalide doit être rejetée à l'analyse, AVANT
192
+ toute opération coûteuse."""
193
+ from picarones.cli import cli
194
+ runner = CliRunner()
195
+ result = runner.invoke(cli, [
196
+ "run",
197
+ "--corpus", str(tmp_path),
198
+ "--engines", "tesseract",
199
+ "--output", str(tmp_path / "x.json"),
200
+ "--fail-if-cer-above", threshold,
201
+ ])
202
+ return result.exit_code, result.output + (result.stderr or "")
203
+
204
+ def test_value_greater_than_one_rejected_with_migration_hint(
205
+ self, tmp_path: Path,
206
+ ) -> None:
207
+ """Passer 15.0 (ancienne sémantique pourcentage) doit échouer
208
+ en early-validation avec un message qui pointe vers la
209
+ nouvelle sémantique."""
210
+ exit_code, output = self._invoke("15.0", tmp_path)
211
+ assert exit_code != 0
212
+ # Message doit contenir la valeur reçue ET la migration hint.
213
+ assert "15.0" in output
214
+ assert "fraction" in output.lower() or "0.15" in output
215
+ # Migration hint explicite.
216
+ assert "divisez" in output.lower() or "diviser" in output.lower()
217
+
218
+ def test_negative_value_rejected(self, tmp_path: Path) -> None:
219
+ exit_code, output = self._invoke("-0.1", tmp_path)
220
+ assert exit_code != 0
221
+ assert "≥ 0" in output or ">= 0" in output
222
+
223
+ def test_value_at_one_accepted(self, tmp_path: Path) -> None:
224
+ """1.0 est la borne haute valide (= 100 % de CER)."""
225
+ exit_code, output = self._invoke("1.0", tmp_path)
226
+ # Validation du seuil OK : pas de mention de "fraction" ou
227
+ # de migration hint. Le run échoue ensuite parce que le
228
+ # corpus est vide, mais c'est un autre problème.
229
+ assert "doit être une fraction" not in output
230
+ assert "divisez" not in output.lower()
231
+
232
+ def test_value_at_zero_accepted(self, tmp_path: Path) -> None:
233
+ """0.0 est valide (seuil zéro tolérance)."""
234
+ exit_code, output = self._invoke("0.0", tmp_path)
235
+ assert "doit être une fraction" not in output
236
+ assert "≥ 0" not in output