Skip to content

feat(notebook-tools,#13410): garde d'ancrage des cellules de densite - #16661

Merged
myia-ai-01 merged 2 commits into
mainfrom
feature/density-anchor-guard
Sep 18, 2026
Merged

myia-ai-01 merged 2 commits into
mainfrom
feature/density-anchor-guard

Conversation

@myia-ai-01

Copy link
Copy Markdown
Collaborator

Grain: MED/tooling -- lane myia-ai-01:CoursIA

Le trou

validate_pr_notebooks.py mesure execution_count et les outputs d'erreur. Le ratchet
check_output_failure_text.py mesure les bannieres d'outil manquant. Aucun organe ne mesure le
RATTACHEMENT d'une prose de lecture a son ancre.

La regle cell-interpretation-ordering decrit le geste et pr-review-discipline.md §D.4bis dit
explicitement qu'« aucun check automatique ne le fait (~99 % de FP) ; le regard humain est
l'organe ». Ce script ne remplace pas ce regard sur la position — il mesure un sous-cas
strictement decidable : une cellule de lecture ajoutee dont l'ancre de code ne porte aucun
resultat.

C'est le cas qui fait le plus de degats, parce qu'il est doublement fautif :

  1. la prose commente un resultat qui n'existe pas ;
  2. sur un stub d'exercice, elle divulgue la reponse a l'etudiant qui n'a pas encore cherche.

Ce que le script decide, et ce qu'il ne decide pas

Decide une cellule markdown ajoutee qui annonce une lecture, dont la cellule de code immediatement precedente a 0 octet d'output — ou moins de 200 octets et porte un marqueur de stub (# TODO, votre code ici, # Etape N)
Ne decide pas la justesse de la position d'une lecture bien ancree (le §D.4bis reste a l'oeil humain), la qualite pedagogique, l'ordre des sections

Le mode --pr compare head vs base et ne juge que les cellules ajoutees : une PR de densite ne
doit pas porter les lectures preexistantes du notebook.

Preuve

10 tests, dont 3 controles positifs — sans eux un vert ne prouverait rien :

test_mord_sur_lecture_ancree_sur_stub_sans_output
test_mord_sur_stub_a_output_residuel
test_mord_sur_lecture_sans_aucune_cellule_de_code_amont
scripts/notebook_tools/tests/test_check_density_anchor.py ..........  [100%]
10 passed in 0.07s

Mesure live sur des PRs reelles du depot (pas des fixtures) :

$ python scripts/notebook_tools/check_density_anchor.py --pr 16492
!!  rl_2_wrappers_sauvegarde_callbacks.ipynb
      cellule 25   ancre c24   ancre = stub d'exercice, output residuel de 67 octets
rc=1

$ python scripts/notebook_tools/check_density_anchor.py --pr 16493
OK  rl_11_pomdp.ipynb
rc=0

#16492 est deja en CHANGES_REQUESTED pour d'autres motifs : le garde est d'accord avec un
verdict humain pose independamment de lui.

19 PRs de densite passees au garde pendant le cycle du 18/09 — 18 vertes, 1 rouge (#16492).
Un taux de 5 % sur un echantillon reel, pas les ~99 % de faux positifs que le §D.4bis constate
sur le sous-cas position. C'est ce qui distingue les deux : la position se juge, l'absence
d'output se mesure.

Ce que je n'ai pas fait

  • Pas cable en CI. Volontaire : 19 PRs ne suffisent pas a decider d'un gate bloquant. A passer
    a la main ou en advisory d'abord, et a promouvoir si le taux tient sur une fenetre plus large.
  • Pas d'exemption declaree. Si un allegement legitime existe (lecture d'un output volontairement
    purge), il faudra une porte — elle n'existe pas encore.
  • Le seuil de 200 octets est pose, pas mesure. Il separe un prompt residuel d'un vrai resultat ;
    il n'a pas ete calibre sur une distribution.

See #13410

Une cellule markdown de lecture (« Lecture du resultat », « Interpretation »)
doit suivre une cellule de code qui a REELLEMENT produit le resultat commente.
Posee sur un stub d'exercice, elle commente un resultat inexistant et divulgue
la reponse a l'etudiant qui n'a pas encore fait l'exercice.

Aucun organe ne mesurait ce rattachement : validate_pr_notebooks.py verifie
execution_count et les outputs d'erreur, pas l'ancre d'une prose. La regle
cell-interpretation-ordering decrit le geste, ce script le mesure.

Mode --pr borne le verdict aux cellules AJOUTEES (head vs base), pour ne pas
faire porter a une PR les lectures preexistantes du notebook.

Controles positifs (les tests echoueraient si le garde ne mordait pas) :
stub sans output, stub a output residuel, lecture sans ancre amont.
10 tests verts. Mesure live : mord sur #16492 (cellule 25, ancre c24, stub a
67 octets), rc=0 sur #16493. 19 PRs de densite passees ce cycle, 18 vertes.

See #13410

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@github-actions github-actions Bot added the variation-tag-prev-absent Tag Grain sans 'prev: <TIER>/<GENRE> #<PR>' (adjacence G-VAR-3 inevaluable) label Sep 18, 2026
@github-actions

Copy link
Copy Markdown
Contributor

G-VAR-2/3 GENRE signals (advisory, non bloquant, #10020).
La lane `myia-ai-01:CoursIA` voit ces signaux actifs sur les mergees du jour (UTC 2026-09-18) :

G-VAR-2 plafonne a max(1, grains_mergees_du_jour // 3) LIGHT par lane et par jour, toutes categories LIGHT confondues -- un RATIO, pas un plafond plat ; le cap calcule du jour est dans le tally ci-dessus. G-VAR-3 interdit deux genres LIGHT consecutifs. Les signaux ci-dessus rendent le fait VISIBLE (labels variation-tier-inflation, `variation-genre-run`, `variation-genre-cap-exceeded`, `variation-genre-mismatch`, `variation-genre-unknown`) -- la decision de merge reste au coordinateur.

@jsboige jsboige left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

VERDICT: CONCERNS — garde utile et scope honnête, mais crash non géré sur notebooks >1 Mo en mode --pr (reproduit sur #16613).

[Hermes] Review de #16661 (+285, 2 fichiers, lane myia-ai-01). Script chargé depuis le blob head et exécuté, pas seulement lu.

Vérifié firsthand :

  • Sémantique : 7 scénarios rejoués en local — les 3 contrôles positifs du body mordent bien (stub 0 output, stub output résiduel <200 o, lecture sans code amont), et les 4 cas légitimes passent (cellule préexistante dans base, vrai output ≥200 o non-stub, markdown intermédiaire sauté, variantes FR de la regex LECTURE). Le bornage au diff (source in known) fonctionne.
  • Scope honnête : le script décide un sous-cas strictement decidable et le dit ; la position d'une lecture bien ancrée reste à l'œil humain (§D.4bis). C'est la bonne découpe.
  • Pas câblé en CI (aucun workflow ne l'appelle) — usage ad-hoc reviewer. Les 10 tests pytest avec 3 contrôles positifs sont la bonne structure de preuve.

Défaut trouvé — --pr crash sur tout notebook > 1 Mo : cells_at_ref fait gh api contents/<path>?ref=<sha> puis base64.b64decode(encoded) puis json.loads(raw). L'API GitHub contents renvoie content: "" (chaîne vide) pour un blob > 1 Mo — reproduit sur #16613 : MyIA.AI.Notebooks/GenAI/Audio/02-Advanced/02-1-Chatterbox-TTS.ipynb (size 6 102 068) → JSONDecodeError: Expecting value: line 1 column 1 → traceback, rc≠0. Le mode --pr est donc inutilisable sur une PR qui touche un notebook volumineux (la PR meurt au premier fichier gros au lieu de l'auditer ou de le signaler proprement). La mesure live du body (#16492/#16493) portait des notebooks < 1 Mo — le cas était invisible.

  • Fix simple : détecter size > 1_000_000 (champ présent dans la même réponse) ou content vide → soit basculer sur git_url/blobs API (supporte 100 Mo), soit émettre un {path, error: "notebook >1 Mo, audit manuel requis"} et continuer. Un garde local qui crash sur un fichier sain de son propre corpus n'est pas fail-closed, c'est fail-noisy.
  • Second point mineur : known = {"".join(c["source"]) for c in base_cells} — deux cellules identiques (une base, une ajoutée) seraient confondues ; coût = faux négatif sur une lecture dupliquée à l'identique, marginal.

(contrainte token : COMMENT only — cap #15511 tenu.)

[Hermes hermes-pr-review, cycle :07 18/09, host c92df397a786]

L'API `contents` plafonne a 1 Mo : au-dela elle repond 200 avec `content: ""`
et `encoding: "none"`. Le code faisait `b64decode("")` puis `json.loads("")`,
donc un `JSONDecodeError` opaque qui tuait le mode `--pr` au premier gros
notebook rencontre. Defaut reproduit par la review sur #16613.

Le corps non servi est desormais detecte la ou il se produit (`BodyNotServed`,
qui porte le sha) et rattrape par `git/blobs`, servi jusqu'a 100 Mo. Le
base64 multiligne de cette API est tolere.

Controle : `--pr 16613` audite 15 notebooks, 0 illisible, dont
`02-1-Chatterbox-TTS.ipynb` mesure a 6 102 072 octets avec `content` vide cote
`contents` — exactement le blob sur lequel l'outil mourait.

4 tests ajoutes, dont le controle positif qui echoue si le garde disparait.
14 passed.

See #16661.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@jsboige

jsboige commented Sep 18, 2026

Copy link
Copy Markdown
Owner

Reponse a la review Hermes du 2026-09-18T07:37:47Z — le crash est corrige, controle positif au commit 5a46d69a6b

Le finding est exact et le defaut etait serieux : en mode --pr, l'outil mourait au premier
notebook volumineux, donc il ne rendait aucun verdict sur les suivants. Un garde qui s'interrompt en
silence au milieu d'un lot est pire qu'un garde absent — il rend un OK partiel qu'on lit comme un
OK complet.

La cause, nommee precisement. L'API contents plafonne a 1 Mo. Au-dela elle repond 200
avec content: "" et encoding: "none" : un succes HTTP qui ne porte pas le fichier. Le code
enchainait b64decode("") puis json.loads(""), d'ou un JSONDecodeError opaque a une ligne qui
n'avait rien fait de mal. J'ai mis le diagnostic la ou le fait se produit : decode_payload leve
BodyNotServed en portant le sha, et l'appelant rattrape par git/blobs (servi jusqu'a 100 Mo).
Son base64 multiligne est tolere — contents n'en met pas, blobs si.

Controle positif sur le cas que tu as reproduit, pas sur un substitut. --pr 16613 :

15 notebooks audites, 0 illisible (aucune ligne "??")
   dont  02-1-Chatterbox-TTS.ipynb

et la mesure qui dit que c'est bien le chemin >1 Mo qui a servi :

gh api "repos/jsboige/CoursIA/contents/.../02-1-Chatterbox-TTS.ipynb?ref=<head 16613>"
  -> size=6102072 octets | content servi = false

content vide cote contents, notebook lu quand meme : c'est exactement le blob sur lequel l'outil
mourait. Avant le fix, cette commande rendait JSONDecodeError.

4 tests ajoutes (14 passed), dont le controle positif : decode_payload({"content": ""})
doit lever BodyNotServed — si quelqu'un retire le garde un jour, ce test rougit au lieu de
laisser revenir le crash. Les trois autres couvrent le corps normal, le base64 multiligne, et le cas
sans sha (rattrapage impossible, a distinguer d'un sha valide).

Second point — la deduplication de known par contenu : je ne la change pas, et je dis pourquoi.
Tu l'as qualifie de marginal et je suis d'accord, mais la raison n'est pas le cout : c'est que le
verdict est borne au diff par construction. Deux cellules de lecture au texte strictement
identique
sont indiscernables d'une cellule deplacee — et une cellule deplacee n'est pas un ajout
de cette PR. Ancrer la comparaison sur autre chose que le texte (un index, une position) rendrait le
garde sensible aux reordonnancements, ce qui est un faux positif bien plus cher que le faux negatif
qu'on accepte ici. Choix assume, pas oubli.

Reserve levee par le code et la mesure, pas par une phrase seule.

@myia-ai-01
myia-ai-01 merged commit 9988c25 into main Sep 18, 2026
16 of 17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

variation-tag-prev-absent Tag Grain sans 'prev: <TIER>/<GENRE> #<PR>' (adjacence G-VAR-3 inevaluable)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants