Claude commited on
Commit
a125a5f
·
unverified ·
1 Parent(s): 20e4ca7

fix(app): resume recalcule les view_results des docs repris du partial

Browse files

Bug découvert par le harnais de caractérisation : le partial store
persiste PipelineResult mais PAS ViewResult. Au resume,
_execute_with_partial peuplait vr_index uniquement depuis les
sub-runs frais, jamais depuis loaded_list (docs repris du partial)
→ pipeline_results.jsonl complet MAIS view_results.jsonl amputé des
docs repris → métriques agrégées (CER…) silencieusement faussées
après une reprise (linéaire/DAG ; non manifesté en multi-pipeline).

Fix B (recalcul, recommandé vs persister) : pour chaque PR rechargé
du partial, appel de bench._evaluate_document_in_views (entrée
d'éval canonique, même chemin que le run frais — fonction PURE de
pipeline_results + GT + profil). Avantages : zéro changement de
format de partial, vues toujours fraîches (cohérentes avec le code
d'éval courant, pas de vues périmées), contenu à _execute_with
_partial. Méthode privée appelée cross-module : couplage assumé et
commenté (entrée d'éval unique ; dupliquer la logique serait pire).

Le harnais a fait son office : 15/17 verts inchangés (doc_idx,
cancel, concurrence, goldens, complétude pipeline, non-dup) → fix
sûr ; 2 tests qui CARACTÉRISAIENT le défaut BASCULÉS pour
verrouiller le comportement CORRIGÉ (vues == pipeline == corpus,
toutes topologies). Parité resume existante (TestParityPartialDir)
+ CLI + golden : 296 verts.

https://claude.ai/code/session_01EmLiMPJJuB44QHEFzDWUvF

picarones/app/services/run_orchestrator.py CHANGED
@@ -646,10 +646,38 @@ class RunOrchestrator:
646
  # Map (doc_id, pipeline_name) → list[ViewResult]
647
  vr_index: dict[tuple[str, str], list[Any]] = {}
648
 
649
- # Charge les pipeline_results depuis les partials (rechargés).
 
 
 
 
 
 
 
 
 
 
 
 
 
650
  for pipeline_name, (_, loaded_list) in per_pipeline_state.items():
651
  for pr in loaded_list:
652
  pr_index[(pr.document_id, pipeline_name)] = pr
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
653
 
654
  # Charge les pipeline_results et view_results depuis les sub-runs.
655
  for sub_result in sub_run_results:
 
646
  # Map (doc_id, pipeline_name) → list[ViewResult]
647
  vr_index: dict[tuple[str, str], list[Any]] = {}
648
 
649
+ # Charge les pipeline_results depuis les partials (rechargés)
650
+ # ET recalcule leurs view_results.
651
+ #
652
+ # Fix défaut resume (harnais de caractérisation) : le partial
653
+ # store persiste ``PipelineResult`` mais PAS ``ViewResult``.
654
+ # Sans ce recalcul, les docs repris du partial sortaient avec
655
+ # leurs pipeline_results mais SANS view_results → métriques
656
+ # agrégées (CER…) silencieusement faussées après reprise.
657
+ # Les vues sont une fonction PURE de (pipeline_results + GT +
658
+ # profil) : les recalculer pour les docs repris est correct,
659
+ # ne change pas le format du partial, et garantit des vues
660
+ # fraîches (cohérentes avec le code d'éval courant) plutôt que
661
+ # potentiellement périmées si on les avait persistées.
662
+ doc_by_id = {d.id: d for d in corpus_spec.documents}
663
  for pipeline_name, (_, loaded_list) in per_pipeline_state.items():
664
  for pr in loaded_list:
665
  pr_index[(pr.document_id, pipeline_name)] = pr
666
+ doc = doc_by_id.get(pr.document_id)
667
+ if doc is None:
668
+ continue
669
+ # ``_evaluate_document_in_views`` est l'entrée d'éval
670
+ # canonique (même chemin que le run frais) ; appelée
671
+ # ici en isolation sur le PR rechargé.
672
+ for vr in bench._evaluate_document_in_views(
673
+ document=doc,
674
+ pipeline_results=[pr],
675
+ views=views,
676
+ ground_truth_factory=_default_gt_factory,
677
+ ):
678
+ vr_index.setdefault(
679
+ (pr.document_id, ""), [],
680
+ ).append(vr)
681
 
682
  # Charge les pipeline_results et view_results depuis les sub-runs.
683
  for sub_result in sub_run_results:
tests/golden/test_run_orchestrator_characterization.py CHANGED
@@ -368,22 +368,20 @@ class TestCrashResumeConsistency:
368
  on stoppe mi-run via cancel, puis on relance SANS cancel avec le
369
  même ``partial_dir``.
370
 
371
- ⚠️ DÉFAUT PRÉ-EXISTANT DÉCOUVERT PAR CE HARNAIS ⚠️
372
- Le partial store persiste ``PipelineResult`` mais PAS
373
- ``ViewResult``. Au resume, les documents rechargés du partial
374
- récupèrent leurs ``pipeline_results`` mais **pas** leurs
375
- ``view_results`` (jamais recalculés). Conséquence : après une
376
- reprise, ``view_results.jsonl`` est incomplet toute métrique
377
- agrégée (CER…) dérivée des vues est silencieusement faussée pour
378
- les documents repris.
379
-
380
- Ces tests CARACTÉRISENT le comportement ACTUEL (warts inclus)
381
- rôle d'un harnais de caractérisation pour que Phase B ne
382
- l'aggrave pas ET qu'une correction future du défaut soit
383
- consciente (le test échouera, forçant la revue). Le défaut
384
- lui-même est remonté à l'opérateur, pas corrigé furtivement ici
385
- (resume/views = changement stateful risqué, hors périmètre
386
- « construire le harnais »)."""
387
 
388
  def _persisted_doc_ids(self, results_dir: Path) -> tuple[list[str], list[str]]:
389
  pr = sorted({r["document_id"] for r in _jsonl(results_dir / "pipeline_results.jsonl")})
@@ -435,53 +433,37 @@ class TestCrashResumeConsistency:
435
  pr, _ = self._persisted_doc_ids(resumed)
436
  assert pr == ["doc01", "doc02", "doc03", "doc04", "doc05"]
437
 
438
- #: Relation ``view_results`` vs ``pipeline_results`` APRÈS resume,
439
- #: par topologie — comportement RÉEL observé (le défaut est
440
- #: topologie-dépendant : présent en linéaire/DAG, absent en
441
- #: multi-pipeline avec cette synchro d'interruption).
442
- _RESUME_VIEW_RELATION = {
443
- "single_linear": "strict_subset", # défaut : vues ⊊ pipeline
444
- "branching_dag": "strict_subset", # défaut idem
445
- "multi_pipeline": "equal", # pas de défaut ici
446
- }
447
-
448
  @pytest.mark.parametrize(
449
  "topo", ["single_linear", "multi_pipeline", "branching_dag"],
450
  )
451
- def test_resume_view_vs_pipeline_relation_DEFECT_characterized(
452
  self, tmp_path: Path, topo: str,
453
  ) -> None:
454
- """⚠️ CARACTÉRISE LE DÉFAUT (topologie-dépendant) ⚠️ : au
455
- resume, ``pipeline_results`` couvre tout le corpus, mais la
456
- relation ``view_results`` vs ``pipeline_results`` dépend de la
457
- topologie (cf. :data:`_RESUME_VIEW_RELATION`) :
458
-
459
- - ``single_linear`` / ``branching_dag`` : vues ⊊ pipeline —
460
- les vues des docs repris du partial ne sont jamais
461
- recalculées (métriques agrégées faussées après reprise).
462
- - ``multi_pipeline`` : vues == pipeline (le défaut ne se
463
- manifeste pas avec cette synchro d'interruption).
464
-
465
- Toute évolution de l'une de ces relations (Phase B, ou
466
- correction du défaut) fait échouer ce test et force une revue
467
- consciente."""
468
  _, resumed = self._interrupt_then_resume(
469
  tmp_path, 5, stop_after=2, topo=topo,
470
  )
471
  pr, vr = self._persisted_doc_ids(resumed)
472
  full = ["doc01", "doc02", "doc03", "doc04", "doc05"]
473
  assert pr == full, f"pipeline incomplet au resume ({topo}): {pr}"
474
- rel = self._RESUME_VIEW_RELATION[topo]
475
- if rel == "strict_subset":
476
- assert set(vr) < set(pr), (
477
- f"[{topo}] défaut resume/vues changé : pipeline={pr} "
478
- f"vues={vr}. Attendu : vues ⊊ pipeline."
479
- )
480
- else:
481
- assert set(vr) == set(pr), (
482
- f"[{topo}] relation resume/vues changée : pipeline={pr}"
483
- f" vues={vr}. Attendu : vues == pipeline."
484
- )
485
 
486
  def test_resume_does_not_duplicate_documents(
487
  self, tmp_path: Path,
 
368
  on stoppe mi-run via cancel, puis on relance SANS cancel avec le
369
  même ``partial_dir``.
370
 
371
+ HISTORIQUE DÉFAUT DÉCOUVERT PAR CE HARNAIS, PUIS CORRIGÉ.
372
+ Le partial store persistait ``PipelineResult`` mais PAS
373
+ ``ViewResult`` : au resume, les docs rechargés du partial
374
+ sortaient avec ``pipeline_results`` mais SANS ``view_results``
375
+ (jamais recalculés) ``view_results.jsonl`` incomplet →
376
+ métriques agrégées (CER…) silencieusement faussées après reprise
377
+ (linéaire/DAG ; non manifesté en multi-pipeline).
378
+
379
+ FIX : ``_execute_with_partial`` recalcule les vues des docs
380
+ repris via ``_evaluate_document_in_views`` (fonction pure de
381
+ pipeline_results + GT + profil ; aucun changement de format de
382
+ partial). Ces tests, qui caractérisaient le défaut, ont été
383
+ BASCULÉS pour verrouiller le comportement CORRIGÉ (= run propre)
384
+ toute régression Phase B refaisant l'incohérence échoue ici."""
 
 
385
 
386
  def _persisted_doc_ids(self, results_dir: Path) -> tuple[list[str], list[str]]:
387
  pr = sorted({r["document_id"] for r in _jsonl(results_dir / "pipeline_results.jsonl")})
 
433
  pr, _ = self._persisted_doc_ids(resumed)
434
  assert pr == ["doc01", "doc02", "doc03", "doc04", "doc05"]
435
 
 
 
 
 
 
 
 
 
 
 
436
  @pytest.mark.parametrize(
437
  "topo", ["single_linear", "multi_pipeline", "branching_dag"],
438
  )
439
+ def test_resume_view_results_complete_and_consistent(
440
  self, tmp_path: Path, topo: str,
441
  ) -> None:
442
+ """Régression-guard du FIX du défaut resume/vues.
443
+
444
+ Historique : ce harnais a découvert qu'au resume le partial
445
+ store rejouait ``pipeline_results`` mais PAS ``view_results``
446
+ des docs repris (linéaire/DAG : vues ⊊ pipeline → métriques
447
+ agrégées faussées après reprise). CORRIGÉ par recalcul des
448
+ vues au resume (``_execute_with_partial`` : ``_evaluate_
449
+ document_in_views`` sur les PR rechargés — fonction pure).
450
+
451
+ Invariant verrouillé désormais : après resume, sur TOUTES les
452
+ topologies, ``view_results`` couvre exactement le même
453
+ ensemble de docs que ``pipeline_results`` (= corpus complet),
454
+ comme un run propre. Si Phase B (ou un futur changement)
455
+ recasse l'égalité, ce test échoue."""
456
  _, resumed = self._interrupt_then_resume(
457
  tmp_path, 5, stop_after=2, topo=topo,
458
  )
459
  pr, vr = self._persisted_doc_ids(resumed)
460
  full = ["doc01", "doc02", "doc03", "doc04", "doc05"]
461
  assert pr == full, f"pipeline incomplet au resume ({topo}): {pr}"
462
+ assert vr == full, (
463
+ f"[{topo}] vues incomplètes au resume : pipeline={pr} "
464
+ f"vues={vr}. Le fix recalcule les vues des docs repris — "
465
+ "une régression ici refait des métriques faussées."
466
+ )
 
 
 
 
 
 
467
 
468
  def test_resume_does_not_duplicate_documents(
469
  self, tmp_path: Path,