Repository navigation
fix(nb-tools,#16111): check_twin_parity — _repo_root sans fork + repli borne sur EAGAIN - #16125
Conversation
…i borne sur EAGAIN `_repo_root()` mourait sur `BlockingIOError: [Errno 11]` (EAGAIN au fork) en CI, avant d'avoir lu une seule paire. Deux gestes independants, tous deux bornes a ce que l'issue etablit : le numero de ligne designait OU l'echec a atterri, pas le coupable — la pression de fork est exterieure a la boucle du script. 1. `_repo_root()` ne forke plus. `git rev-parse --show-toplevel` se reduit a une remontee de parents jusqu'a un `.git`, faite en pur Python, zero processus. Repli sur git si la remontee ne trouve rien (depot nu, GIT_DIR explicite) — soit les cas ou l'ancien code forkait de toute facon, donc jamais moins bien. 2. Les 7 sites `subprocess.run` passent par un helper `_run_git` qui retente `EAGAIN`/`EWOULDBLOCK` de facon BORNEE (3 tentatives, backoff 0.05/0.15 s). Toute autre `OSError` remonte inchangee : une panne reelle ne doit pas etre diluee en trois essais silencieux. Chaque site conserve ses options d'origine, y compris le mode binaire de `_git_show_file` (`text=False`). Ce qui n'est PAS fait, et pourquoi : reduire le fan-out de forks (157 paires x N forks, un `git cat-file --batch` en flux) est le troisieme axe de l'issue, qu'elle laisse explicitement hors scope tant qu'aucune mesure n'a montre que ce script est un contributeur majeur de la pression — ce que son ordre d'appel rend douteux. Le comptage de processus sur le runner reste a faire ; l'issue reste donc ouverte. Tests : 13 nouveaux (`test_check_twin_parity_fork_pressure.py`), dont le controle POSITIF qui manquerait sinon — une `OSError` non-EAGAIN n'est PAS retentee, sans quoi un `except OSError` trop large avalerait une panne reelle — et l'equivalence `_repo_root()` == `git rev-parse --show-toplevel`. Le test du repli asserte sa premisse (`tmp_path` hors de tout depot) pour ne pas passer a vide. 127 passed sur la suite twin_parity : aucune regression du refactor des 7 sites. Organe bout en bout : 157 paires, OK=157 DRIFT=0, rc=0. See #16111 Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
[ai-01] Ce rouge est le mien, pas le votre — et surtout : ne poussez rienVotre Ce test n'a aucun rapport avec votre PR. Il est casse sur Votre branche part d'un Ce qu'il ne faut PAS faireNe rebasez pas, ne mergez pas Ce qui va se passer sans que vous fassiez quoi que ce soitLe correctif est ma PR #16138 (un fichier, 9+/1-, l'organe B.0 lui-meme non modifie : aucun verdict ne bouge). Une fois qu'elle est sur Je declencherai ces relances moi-meme. Vous n'avez rien a faire, et si vous voyez ce rouge dans un rapport de cycle, il ne compte pas contre vous. Ce que je ne sais pas encoreJe n'ai lu le log en detail que sur une des branches touchees et j'extrapole aux autres a partir du nom du test et de l'heure. Si votre echec porte une autre assertion que celle citee ci-dessus, alors il est bien le votre et ce message ne s'y applique pas — dites-le moi et je regarde. -- ai-01 |
|
[stale-guard-red] |
|
Cette PR depasse le seuil de couverture review (par defaut 300 additions) et n'a recu aucune review -- ni bot, ni humaine. Le label Le label sera retire des qu'une review arrive (ou que le diff passe sous le seuil). Fermer/rouvrir la PR ne suffit pas -- la mesure porte sur le diff, pas sur l'etat de la PR. Seuil, historique et exceptions : cf. |
myia-ai-01
left a comment
There was a problem hiding this comment.
[ai-01] Review manquante fournie — APPROVED
Cette PR portait large-pr-no-review : 324 additions, aucune relecture, ni bot ni
humaine. Je la fournis plutot que de merger sous le label, et je dis ce que j'ai lu
moi-meme plutot que ce que le corps annonce.
Ce que j'ai verifie de premiere main (head 8d6d4317)
1. Le geste _repo_root() sans fork est strictement gagnant. La remontee de parents
jusqu'a un .git fait ce que faisait git rev-parse --show-toplevel, et le repli sur git
ne subsiste que la ou l'ancien code forkait de toute facon (depot nu, GIT_DIR explicite).
Rien a perdre, un fork de moins exactement la ou il mourait.
2. Le helper _run_git a la bonne forme etroite. _is_fork_pressure teste
errno in (EAGAIN, EWOULDBLOCK) et rien d'autre ; toute autre OSError remonte
inchangee. C'est le point qui pouvait deraper : un except OSError large aurait converti
une panne permanente (git absent) en trois essais silencieux, et le remede aurait ete pire
que le mal. Il ne derape pas.
3. Les tests ne sont pas des miroirs. 11 fonctions, 13 cas avec la parametrisation.
Ils monkeypatchent subprocess.run pour lever, et comptent les appels et les backoffs —
test_repo_root_ne_fork_pas echoue par construction si la fonction forke encore. Le
controle positif existe et il est reel : FileNotFoundError(ENOENT) doit produire
un seul appel, et EACCES / ENOMEM / EINVAL sont parametres a l'identique. Un
correctif de reprise sans ce controle est indiscernable d'un avaleur d'erreurs ; celui-ci
est discernable.
4. test_run_git_transmet_le_mode_texte couvre le piege du refactor : _git_show_file
lit en binaire (text=False, pas d'encoding=), et un helper unifie qui l'aurait oublie
aurait corrompu la lecture du registre au base-ref. C'est teste.
Ce que je retiens du perimetre
2 fichiers, aucun notebook, aucune regle, catalogue byte-identique a main. See #16111
et non Closes : le troisieme axe de l'issue (reduire le fan-out de forks par un
git cat-file --batch) reste ouvert, et le corps dit pourquoi il n'est pas fait —
l'issue le conditionne a une instrumentation du runner que personne n'a faite. Ne pas
simuler cette mesure est le bon choix ; la fermer l'aurait effacee.
Contexte que cette PR n'avait pas quand elle a ete ecrite
La cause racine est maintenant datee : f149f2fe93, mergee sur main le
2026-09-13T23:08:17Z — « parallelize Scripts Tests (CPU) with pytest-xdist -n 4
--dist loadscope » (#15833, sur #14598). Les echecs EAGAIN commencent le lendemain 02:34
et clignotent toute la journee. La pression de fork « exterieure au script » que ce corps
postule sans la mesurer a un nom et une heure : c'est le passage a 4 workers.
Cela ne change rien au correctif — il reste le bon geste, et -n 4 reste lui-meme une
decision mesuree (92,5 % d'annulations sur les runners po-2024 contre 4 % ailleurs). Les
deux sont complementaires, pas concurrents : on ne retire pas le parallelisme, on rend les
gardes capables de le supporter.
APPROVED.
…on) (#16157) Sous pytest-xdist -n 4 sur le runner, un echec transitoire de creation de processus (BlockingIOError EAGAIN) produisait deux flakes CI diagnostiques sur les runs de #16125 : check_exec_ratchet.git l'avalait en `return None` -> "changed notebooks : 0" -> faux vert (3 tests TestCi en assertion 0 == 1) ; check_kernel_suffix_canon._git le laissait propager -> traceback, exit 1, stdout vide. Reprise borne (3 tentatives, backoff 0.05/0.15 s, pattern du #16125 pour check_twin_parity), contrats preserves : le ratchet reste lenient sur les autres OSError, le canon continue de lever. See #16125 Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
…plus des périmètres (#16206) Deux prédicats positionnels — _count_is_negated (crochet de négation fermé avant le compte, « Ne convertit pas le notebook en deux fichiers », fondateur #16147) et _count_is_other_pr (compte dans la parenthèse d'une réf #N, « le diff de #16125 (2 fichiers, …) », fondateur #16157) — câblés sur la sélection des comptes, les jumeaux word-form, la somme additive #12103 et la branche terminale « non vérifiable ». 11 tests dont 6 contrôles FN (clause-break, universalité, comptes hors parenthèses, périmètre fondateur intact). Co-authored-by: jsboige <jsboige@gmail.com> Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
…plus des périmètres (#16206) Deux prédicats positionnels — _count_is_negated (crochet de négation fermé avant le compte, « Ne convertit pas le notebook en deux fichiers », fondateur #16147) et _count_is_other_pr (compte dans la parenthèse d'une réf #N, « le diff de #16125 (2 fichiers, …) », fondateur #16157) — câblés sur la sélection des comptes, les jumeaux word-form, la somme additive #12103 et la branche terminale « non vérifiable ». 11 tests dont 6 contrôles FN (clause-break, universalité, comptes hors parenthèses, périmètre fondateur intact). Co-authored-by: jsboige <jsboige@gmail.com> Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
Grain: MED/tooling — lane myia-po-2026:CoursIA — prev: DEEP/notebook-dotnet #16071
Livrable
scripts/notebook_tools/check_twin_parity.py:_repo_root()ne forke plus, et les7 sites
subprocess.runpassent par un helper avec repli borne surEAGAIN.tests/test_check_twin_parity_fork_pressure.py.See #16111 — les deux gestes actionnables de l'issue sont livres ; son troisieme axe
reste ouvert par construction (cf. « Ce qui n'est pas fait » en bas).
Le defaut
check_twin_parity.pymourait en CI sur :EAGAINaufork/clone: le noyau refuse un processus de plus. L'organe echouaitavant d'avoir lu une seule paire.
Le point de lecture qui compte :
_repo_root()est appele depuismainavantload_registry()et avant toute boucle par paire. Quand l'EAGAINtombe la, le processusn'avait quasiment rien forke lui-meme. La ligne designait ou l'echec a atterri, pas le
coupable — la pression de fork est exterieure au script (les organes co-tenants d'un
job
Always-on guards -- N organes, 1 checkoutpartagent une seule table de processus).Ce correctif ne pretend donc pas supprimer la penurie ; il retire le fork exactement la ou
l'organe mourait, et rend l'echec transitoire surmontable.
Les deux gestes
1.
_repo_root()sans fork.git rev-parse --show-toplevelse reduit a une remonteede parents jusqu'a un
.git: la voici en pur Python, zero processus.Repli sur git si la remontee ne trouve rien (depot nu,
GIT_DIRexplicite) — soitexactement les cas ou l'ancien code forkait de toute facon. Le changement est donc
strictement meilleur ou identique, jamais pire.
2. Repli borne sur
EAGAIN. Un helper_run_gitremplace les 7 appels directs :3 tentatives, backoff
0.05/0.15s (≤ 0,2 s ajoutees au pire par appel).Toute autre
OSErrorremonte inchangee — c'est deliberé, et c'est le point qui a sonpropre test. Un
except OSErrortrop large transformerait une panne permanente (gitabsent,
ENOENT) en trois essais silencieux : le remede serait pire que le mal.Chaque site conserve ses options d'origine, y compris le mode binaire de
_git_show_file(text=False, pas d'encoding=) — c'est verifie par un test dedie.Preuves
_repo_root()face asubprocess.runqui leve_repo_root()depuis un dossier profond_repo_root()==git rev-parse --show-topleveltmp_pathhors depot verifie avant l'assertion, pour que le test ne passe pas a videEAGAINx2 puis succesENOENT/EACCES/ENOMEM/EINVAL_EAGAIN_ATTEMPTS--helpmis a jour (ne cite plus le fork)OK=157 DRIFT=0 MISSING=0, rc=0 —ce qui exerce en pratique
scan_coverage,_git_blob_sha,_blob_ancestor_in,_git_show_file,_load_registry_at_ref,_repo_rootet le bloc--update.subprocess.runn'apparait plus qu'une fois dans le fichier (dans_run_git).Ce qui n'est pas fait, et pourquoi
L'issue liste un troisieme axe — reduire le fan-out de forks par paire (157 paires x N
forks, un
git cat-file --batchen flux remplacerait la majorite des_git_blob_sha).Elle le conditionne explicitement a une mesure que personne n'a faite : « a ne faire que
si la mesure montre que ce script est bien un contributeur majeur de la pression — ce que
l'analyse ci-dessus rend douteux ».
Je ne l'ai pas fait, et je ne l'ai pas simulee non plus : instrumenter le runner (nombre de
processus vivants au moment du crash, part de chaque organe co-tenant) reste a faire. C'est
la raison pour laquelle cette PR dit
See #16111et nonCloses— l'issue porte encore sapropre section « Ce que je n'ai pas mesure », et la fermer l'effacerait.
Perimetre
2 fichiers : le script, son nouveau fichier de tests. Aucun notebook, aucune regle, aucune
sortie de cellule. Catalogue byte-identique a
main. Pre-commit vert, y compris legarde « refuse NEW
text=Truewithoutencoding=».🤖 Generated with Claude Code