Skip to content

fix(notebook-tools,#14297): ratchet — appariement cellule par id nbformat, 0 FP sur enrichissement - #14319

Merged
jsboige merged 1 commit into
mainfrom
feature/14297-ratchet-id-pairing
Sep 2, 2026
Merged

jsboige merged 1 commit into
mainfrom
feature/14297-ratchet-id-pairing

Conversation

@jsboige

@jsboige jsboige commented Sep 2, 2026

Copy link
Copy Markdown
Owner

Grain: MED/tooling -- lane myia-po-2023:CoursIA -- prev: MED/notebook-python #14316

Summary

Fix de #14297 : check_source_output_ratchet.py (classify_cells) apparie les cellules code base/head par index positionnel brut. Une PR qui insère des cellules décale tout ce qui suit ; l'organe compare alors head[i] à base[i] — deux cellules sans rapport — et rend STALE_OUTPUT chaque fois que leurs sorties sont byte-identiques. Ce n'est pas rare : les stubs conformes C.1 impriment tous print("Exercice a completer") → 3/3 faux positifs sur les PR d'enrichissement *-density (cellules déplacées, pas modifiées).

Geste

  • Appariement par id nbformat : les cellules portent un id stable (nbformat 4.5+) ; on construit base_by_id et on apparie par id quand la base en porte. Une cellule insérée (id frais) → UNPAIRED (clean), une cellule décalée conserve son identité → UNCHANGED/EXECUTED correct.
  • Fallback positionnel conservé pour les notebooks legacy sans id sur la base (comportement historique inchangé).
  • Docstring du module mise à jour (le caveat « Index pairing caveat » décrivait exactement ce défaut).

Tests

  • 3 tests de régression ajoutés dans scripts/tests/test_check_source_output_ratchet.py :
    1. insertion entre deux cellules à sorties identiques → pas de STALE_OUTPUT fabriqué (before : 1, after : 0)
    2. stub inséré (id frais) → UNPAIRED
    3. même id, source changée, sorties identiques → STALE_OUTPUT toujours détecté (le fix ne masque pas la vraie règle C.2)
  • 22/22 tests passent (test_check_source_output_ratchet.py), 15/15 test_check_papermill_ratchet.py inchangés.
  • Smoke test CLI sur la branche AIRL : check_source_output_ratchet.py origin/main → stale cells: 0 (charge réelle, noté dans le commit de cette PR — indépendant, 1 notebook re-exécuté).

Test plan

See #14297

…E_OUTPUT FPs on enrichment PRs

classify_cells matched base/head code cells positionally, so a PR that
inserts cells mispaired every shifted cell: two foreign cells with
byte-identical outputs (conforming C.1 stubs print the same marker)
were read as source-changed/outputs-identical -> STALE_OUTPUT. Now
cells pair by their stable nbformat `id` when the base carries ids
(fresh-id cells are UNPAIRED, shifted originals keep identity); legacy
id-less notebooks keep positional pairing. 3 regression tests: the
insertion case (no fabricated stale pair), fresh-id stub (UNPAIRED),
and same-id real stale output still flagged. 22/22 tests pass.

Co-Authored-By: Claude-Code <noreply@anthropic.com>
@jsboige

jsboige commented Sep 2, 2026

Copy link
Copy Markdown
Owner Author

[Hermes] — #14319 check_source_output_ratchet : appariement par id nbformat (fix #14297). Tests rejoués firsthand — OK.

Vérification réelle (tests exécutés, pas lecture seule) : récupération des fichiers au head SHA 3f6e4500, reconstruction de l'environnement de test, python3 -m unittest → 22/22 OK en 1.05s, dont les 3 nouveaux :

  • test_insertion_with_ids_does_not_fabricate_stale_pair → [UNCHANGED, UNPAIRED, UNCHANGED], 0 régression
  • test_shifted_cells_pair_by_id_after_conforming_insertion → [UNCHANGED, UNPAIRED]
  • test_id_pairing_still_flags_real_stale_output → STALE_OUTPUT + regression=True (le garde-fou anti-masquage : un vrai stale output reste détecté même pairé par id)

Lecture du code : l'appariement par id est sain — base_by_id construit uniquement sur les cellules avec id ; cellule head sans id → repli positionnel (legacy) ; cellule head avec id frais → UNPAIRED (clean, pas de faux stale). Le cas use_ids mixte (base partiellement avec ids) est cohérent : une cellule head sans id mais dont la position a shifté retombe sur le repli positionnel — comportement documenté, acceptable pour les notebooks legacy. Sécurité : aucun secret/token dans le diff.

Verdict : fix de cause racine (pairing positionnel → pairing sémantique), tests ciblés qui couvrent le cas #14297 ET le cas anti-masquage. (contrainte token : COMMENT only)

@github-actions

github-actions Bot commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Bash Syntax Advisory — shebang / executable-bit warnings

See the Shebang + dry-run advisory job log for the per-file ::warning:: lines. Non-blocking.

@jsboige
jsboige merged commit 011260f into main Sep 2, 2026
15 checks passed

@jsboige jsboige left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[COMMENT] Firsthand check sur head 3f6e4500a — worktree + unittest réels : 22/22 tests OK (dont les 3 nouveaux : test_insertion_with_ids_does_not_fabricate_stale_pair, test_shifted_cells_pair_by_id_after_conforming_insertion, test_id_pairing_still_flags_real_stale_output).

Cohérence avec l'issue #14297 :

  • Le bug décrit (appariement positionnel → 3/3 STALE_OUTPUT sur PRs d'enrichissement) est adressé à la source : classify_cells() apparie par nbformat id quand la base en porte, cellule insérée (id frais) → UNPAIRED, cellule décalée → identité conservée.
  • Fallback positionnel préservé pour notebooks legacy sans id — pas de régression sur la doc.
  • Anti-régression présent : test_id_pairing_still_flags_real_stale_output prouve que le pairing par id ne masque PAS un vrai stale (même id, source changée, outputs identiques → STALE_OUTPUT).

Rien à redire. LGTM (contrainte token : COMMENT only).

[REVIEW] par Hermes — 1er passage sur ce SHA.

myia-ai-01 pushed a commit that referenced this pull request Sep 2, 2026
…14378)

The id pairing of #14319 covers nbformat 4.5+ bases; legacy bases
without cell ids still paired positionally, so an enrichment insertion
shifted indices and confronted unrelated C.1 stubs whose uniform
outputs are byte-identical - the exact 3/3 FP class the issue measured.

Two passes over canonical sources: exact matches first (a
moved-unmodified cell pairs with its own base copy -> UNCHANGED), then
a difflib fuzzy pass (ratio >= 0.75, greedy descending) so a
moved-AND-modified cell still confronts its own base version
(STALE_OUTPUT). A head cell matching nothing stays UNPAIRED.

Acceptance #14297 sec3 pinned: moved-only -> UNCHANGED,
moved-and-modified -> STALE_OUTPUT, modified stub next to an identical
-output sibling pairs to its own base. Founding #13550 positive control
survives untouched (26/26).

Co-authored-by: Claude-Code <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant