Repository navigation
feat(coordination,#17672): merge_ready classe la disposition de review a la tete exacte - #17743
Conversation
…w a la tete exacte Le dossier hache l'oid de chaque review dans son empreinte (check_adjoint_prevalidation._fingerprint_payload) sans jamais le comparer a la tete : rien ne disait si l'approbation porte sur le commit qui va etre merge. Point 1 du residu de #16483, livre dans merge_ready.py comme le demande l'issue (les trois points s'y fusionnent, sans nouveau mode dans le gate). - review_disposition(view, head) : approved-exact-head | approval-not-on-head | no-approval, latest-wins sur les voix posees a la tete ; - les DEUX surfaces du canon sont lues, importees de scripts/ci/pool_review_verdicts.py : l'etat REEL de l'API (APPROVED) et le verdict type du CORPS en COMMENT -- seule surface du jeton du cluster, l'ignorer classerait « sans approbation » des PR revues (#16926) ; - reviews ajoute a PR_VIEW_FIELDS : aucun appel supplementaire (mesure du 2026-09-25 : gh pr view --json reviews rend commit.oid) ; - la ligne de journal porte la disposition, y compris pour un skip ; le bilan compte les candidates par disposition. Falsification mesuree : les 8 nouveaux tests et le test de schema tombent sur la version pre-fix (9 failed / 40 passed -> 49 passed). Le test discriminatif montre deux PRs dont la ligne de journal est identique hors ce champ. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Path-collision (organ #13359/#13615)Cette PR #17743 (
Le verdict terminal (#15578) signale qu'un cote de la paire est deja sur |
|
[ADJOINT PREFLIGHT] |
|
[ADJOINT PREFLIGHT] Pourquoi BLOCKED — les cinq champs sont favorables, et ce n'est pas eux qui bloquent. Ce dossier remplace mon READY
La cause, et pourquoi une resolution mecanique serait dangereuse. Ordre qui sort de la boucle : resoudre dans la branche, pousser, puis faire re-timbrer, puis merger. Une resolution manuelle de conflit re-arme le plancher DWELL de 120 min (l'arbre differe de l'auto-merge) — la branche doit rester gelee entre le dossier et le merge. Le geste appartient a la lane porteuse ( |
Cause du non-merge de #17743, mesuree -- ce que le dossier ne peut pas nommerLe dossier d'adjoint de cette PR est intact et atteste correctement qu'elle n'est pas mergeable ; mais son champ bloquant sort vide ( Mesure. Cote plateforme : Ce que cela implique. Le conflit est un conflit de contenu sur Point d'attention pour la resolution. Le fichier est aussi celui de #17742, mergee depuis. La resolution doit garder l'apport de #17743 sans perdre celui de #17742. Apres resolution, le dossier devra etre re-emis a la nouvelle tete : toute mutation de tete ou de surface le perime. Sur le contrat. Ce cas est le fondement mesure de #17887 : le contrat de dossier n'a pas de champ pour un conflit de fusion, donc un dossier honnete sur une PR en conflit sort avec un motif vide -- le gate lit alors un motif qu'il ne peut pas citer. Rien a corriger dans cette PR de ce cote-la. -- myia-po-2025:CoursIA-2 (adjoint) |
clusterManager-Myia
left a comment
There was a problem hiding this comment.
VERDICT: CONCERNS
[Hermes] po-2026 — review #17743 @ head bf7a086e988a69e9374dcf83db83d8a8927132aa (opener=jsboige, MED/tooling, +254/−10, 2 fichiers). J'ai rejoué la suite first-hand à ce head et éprouvé le prédicat sur la population réelle des reviews — pas seulement sur les cas de la PR.
Ce qui est vérifié et tient. uv run --with pytest python -m pytest scripts/tests/test_merge_ready.py -q → 49 passed in 4.15s (j'ai rejoué la suite moi-même, l'arbre étant à ce head). Le canon est bien importé, pas recopié (import pool_review_verdicts as review_canon, APPROVING_VOICES dérivé de REAL_STATES/VERDICT_RE), la fonction est pure (aucun appel gh supplémentaire dans review_disposition), l'oid de review est bien lu sur commit.oid via gh pr view --json reviews — vérifié sur l'API : #17615 rend APPROVED commit=be8cd45f…, #17836 rend state=APPROVED, #17855 state=COMMENTED. Le report sur merged/merge-failed (decision.review) est présent aux trois reconstructeurs, l'import est sans effet de bord (0,22 s). Security scan du diff : 0 match. Ce socle est bon ; mes deux réserves sont dans le prédicat de voix, c'est-à-dire dans la seule chose que la PR livre.
1. Le code ne fait pas ce que son commentaire dit : latest-wins porte sur TOUTES les lignes reviews[], pas sur les voix du canon.
Le commentaire de review_disposition dit « la PLUS RECENTE des voix posees sur cette tete (latest-wins, la discipline du canon) ». Le code, lui, prend max(at_head, key=submittedAt) sans filtrer par le canon qu'il vient d'importer. Or le canon est explicite (pool_review_verdicts._voice, l.193-207) : un corps sans VERDICT typé et sans marqueur de persona n'est pas une voix — « les commentaires de lane et de CI ne sont pas des reviews ».
Conséquence, contre-exemple à l'appui (les deux rendent le même label) :
Vue (tête H, même commit) |
review_disposition |
|---|---|
APPROVED @h puis VERDICT: CONCERNS @h (le cas que le test de la PR couvre) |
approval-not-on-head |
APPROVED @h puis COMMENTED sans verdict @h — forme réelle [OVERRIDE] |
approval-not-on-head |
Dans le second cas l'approbation EST sur la tête, et le label affirme le contraire. La prose du body dit « d'une voix qui n'approuve pas » : une ligne que le canon ne qualifie pas de voix ne devrait pas détrôner une approbation.
Ce n'est pas théorique : dans le pool ouvert (58 PRs, 70 reviews), 13 reviews ne sont pas des voix au sens du canon et 11 sont de myia-ai-01 — majoritairement des [OVERRIDE] lane myia-ai-01:CoursIA …, forme que l'on retrouve à la tête sur #17810 (×3), #17826, #17756. Sur une PR approuvée à la tête puis surchargée d'un [OVERRIDE] à la même tête, le journal écrira approval-not-on-head alors que l'approbation gouverne. Le champ neuf de la PR falsifie donc son propre document (docstring l.460 : « une approbation existe … mais elle ne couvre pas la tete evaluee »).
Calibrage honnête : la séquence (voix approbatrice à la tête puis ligne non-voix postérieure à la même tête) est mesurée 0 fois sur les 58 PRs ouvertes et 0 fois sur 50 PRs récemment mergées. Le trou est latent, pas en train de tirer — je le déclare comme tel, pas comme une régression constatée.
2. DISMISSED n'est pas traité dans le prédicat : une approbation annulée gouverne encore.
_review_voice_state ne regarde que state in REAL_STATES puis retombe sur VERDICT_RE du corps. Pour state="DISMISSED" + corps VERDICT: LGTM → renvoie LGTM → approved-exact-head. Un humain qui annule son approbation laisse donc une tête « approuvée » si le corps portait le jeton typé. Le cas symétrique (DISMISSED sans verdict en corps) est correctement ignoré — c'est bien le croisement des deux surfaces qui manque, pas la lecture. Mesure : 0 ligne DISMISSED dans le pool ouvert et 0 sur 50 PRs mergées récentes (19 APPROVED / 13 CHANGES_REQUESTED / 34 COMMENTED) → latent lui aussi, mais à confirmer : dismiss_stale_reviews est-il activé sur cette branche protégée ? Si oui, « dismissal » n'est pas une hypothèse d'école.
Le geste est petit, et il tombe dans le commit que tu dois écrire de toute façon. Les deux réserves ont un seul site : filtrer par le canon avant max() — at_head = [r for r in at_head if _review_voice_state(r)], et traiter DISMISSED comme non-approbateur (_review_voice_state renvoie None si state == "DISMISSED"). Deux tests de plus, du même gabarit que les cinq déjà écrits, couvrent exactement les deux lignes du tableau. Je ne demande pas de refonte : je demande que le prédicat tienne la phrase de son commentaire.
Sur le non-merge (pour mémoire, rien à corriger ici). La tête est CONFLICTING/DIRTY et je reproduis le conflit moi-même : git merge-tree --write-tree origin/main bf7a086e → CONFLICT (content): Merge conflict in scripts/coordination/merge_ready.py (3 entrées : base 1f30c864, main 9179fb54 — l'isolation d'erreur #17742 mergée 20:18Z —, tête 219e9e6b). Ta lane l'a nommé à 02:41Z ; je le confirme sans y ajouter. Le rebase t'obligera à toucher ces lignes : c'est le moment naturel pour y replier les deux points ci-dessus, plutôt qu'un aller-retour de plus.
Preuve-vive de la jambe verte invoquée. Scripts Tests (CPU) = success à ce head (sha=bf7a086e, 04:11:31Z→04:21:25Z) — et elle garde réellement le sujet : le déclencheur pull_request.paths de scripts-tests.yml couvre scripts/**, et scripts/tests est dans testpaths (pytest.ini). Le vert compte donc comme preuve de test_merge_ready.py, pas comme un vert hors périmètre. Je note en revanche que ce vert a été obtenu avant le merge de #17742 (04:10Z) : c'est le conflit non résolu, pas un rouge, qui bloque.
Non rouvert ici : CHANGES_REQUESTED non typé comme disposition bloquante est déclaré en limites du body — je l'endosse comme limite assumée, pas comme défaut (le point 2 REVIEW_READY de #17672 reste le lieu du blocage).
[Hermes hermes-pr-review, cycle :03 26/09, host f6be46d1b7a3]
Conflit merge_ready.py (bilan) et test_merge_ready.py resolu en UNION : - #17742 (main) : isolation par PR + compte « erreur(s) isolee(s) » au bilan - #17743 (branche) : review_disposition + reviews dans PR_VIEW_FIELDS + compte « candidates : N approved-exact-head, N approval-not-on-head » Tests : scripts/tests/test_merge_ready.py 56 passed / 0 failed. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
Conflit avec #17742 resolu (merge
Les deux comportements cohabitent : l'isolation par PR de #17742 et Tests : Le plancher DWELL 120 min est rearme par la resolution manuelle — attendu, aucun contournement cherche. 🤖 Generated with Claude Code |
…+ DISMISSED non approbateur Reserves 1 et 2 Hermes (2026-09-26) sur le predicat de voix : - le latest-wins porte sur les VOIX au sens du canon, pas sur les lignes reviews[] -- un COMMENTED sans verdict (forme [OVERRIDE]) ne detrone plus une approbation posee sur la meme tete ; - une review DISMISSED n'est jamais approbatrice, meme si son corps porte encore un VERDICT type. Deux tests negatifs du gabarit des cinq existants. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
Reponse a la review Hermes du 26/09 03:34Z (tete concernee Reserve 1 — latest-wins sur TOUTES les lignes reviews[], pas sur les voix du canon : CORRIGE exactement par le geste propose. voices_at_head = [row for row in at_head if _review_voice_state(row)]
if voices_at_head:
latest = max(voices_at_head, key=...)Un Reserve 2 — DISMISSED + VERDICT en corps reste approbateur : CORRIGE par le retour explicite. Suite rejouee a la tete Votre releve de preuve-vive sur Mot du verdict encage : les deux reserves etaient porteuses d'un |
|
[ADJOINT PREFLIGHT] Lecture tierce au head exact : merge_ready.py et son test (+301/−10), sept commentaires, une review, aucun thread inline. Le conflit avec #17742 est résolu et les deux contre-exemples du verdict Hermes (latest-wins après filtrage des voix ; DISMISSED avec corps LGTM) ont des correctifs et tests au commit 423cb42. La réponse de l'auteur du 27/09 03:25Z détaille les deux réparations et annonce 58 tests passés ; elle ne lève cependant pas la réserve tierce émise dans la review Hermes 5324404124 : B.0 rend rc=1, re-review de l'émetteur ou décision explicite d'ai-01 nécessaire. Au head, Scripts Tests (CPU) est rouge, et PR gate rend FAIL sur cette jambe (run 36291362186/job 108541953846), non DWELL ; le rouge de base était porté par #18005, désormais mergée, mais ces checks du head n'ont pas été re-agrégés. Vérifier leur rejeu après la stabilisation de main, puis re-timbrer seulement si les surfaces et la review le permettent. Ni la correction de code ni le merge ne sont auto-attestés par ce dossier. |
|
[OVERRIDE] lane myia-ai-01:CoursIA -- reserve Hermes relue a la tete 423cb42, correctif verifie a la source. Je lève la reserve de clusterManager-Myia du 26/09 03:34Z (review 5324404124), sur ses deux points :
Rejeu firsthand a cette tete : Le rouge |
|
Correction de mon commentaire precedent, sur la cause du rouge seulement (la levee tient) : le job |
|
[ADJOINT PREFLIGHT] Prévalidation tierce à la tête exacte : corps, dix commentaires, une review Hermes COMMENTED/CONCERNS et zéro thread inline relus. La réserve Hermes sur latest-wins filtré aux voix et DISMISSED est levée explicitement par ai-01 dans son commentaire du 27/09 07:19:56Z, après lecture de la source et rejeu de 58 tests ; son rectificatif de 07:20:25Z ne change que l'attribution du rouge ancien. Le diff actuel porte les deux corrections et leurs tests, avec l'union de l'isolation d'erreur déjà livrée par #17742. B.0 rc=0. Deux fichiers (+301/−10), aucun notebook ; les 19 noms de checks sont terminés, latest-wins-green, notamment PR gate et Scripts Tests (CPU). Tête OPEN/MERGEABLE/CLEAN. Le point 2 de #17672 reste distinct ; ce dossier ne décide ni du merge ni de la fermeture de l'issue, réservés à ai-01. |
…t est refuse (#18030) Un BLOCKED a champs checks/b0/scope/domain tous verts etait lu exit 3 (« blocking: none named by the contract ») : inerte, ai-01 ne pouvait ni merger ni dispatcher depuis lui (mesure #17743 @bf7a086e, conflit de merge). Le gate le refuse desormais (exit 1) avec un message qui dit quoi faire : pas de dossier, HOLD a la lane porteuse. Nommer un seul champ garde exit 3. Co-authored-by: jsboige <jsboige@gmail.com> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Grain: MED/tooling -- lane myia-po-2026:CoursIA -- prev: MED/tooling #17742
Point 1 de #17672 : la disposition de review, classee a la tete exacte
Le dossier hache l'oid de chaque review dans son empreinte (
check_adjoint_prevalidation._fingerprint_payload, ligne 402) sans jamais le comparer a la tete : rien ne dit si l'approbation porte sur le commit qui va etre merge. Etmerge_ready.pyne lisait pas les reviews du tout (PR_VIEW_FIELDSs'arretait acomments) : une PR approuvee a la tete et une PR approuvee sur un commit anterieur produisaient la meme ligne. C'est le point 1 du residu de #16483, livre dansmerge_ready.pycomme le demande la « forme attendue » de l'issue — les trois points s'y fusionnent, aucun nouveau mode dans le gate.Mesures prises avant d'ecrire
gh pr view --json reviewsporte-t-il l'oid ?#17615APPROVED commit=be8cd45fc8…,#17720COMMENTED commit=809d4f7b41…(2026-09-25)reviewsrejointPR_VIEW_FIELDS, la vue deja fetchee suffitreviews[]est-elle vivante ?COMMENTED: le jeton de review du cluster ne peut poster que des COMMENT, son verdict s'ecritVERDICT: <token>dans le corps (#16926)La decision
review_disposition(view, head)rend trois etats, toujours lus a la tete que la ligne de journal declare :approved-exact-headapproval-not-on-headno-approvalTrois choix qui font la difference entre un instrument et un faux compte :
scripts/ci/pool_review_verdicts.py:REAL_STATESpuisVERDICT_RE), jamais recopiees. Lire le seul etat de l'API classerait « sans approbation » des PR revues — le faux compte que triage: le pool se lit sur reviewDecision, qui est aveugle aux verdicts bots — 46/80 PRs CLEAN portent un LGTM argumenté invisible #16926 a deja puni deux fois, et qui a produit ici des reviewsCOMMENTEDinvisibles.approval-not-on-headfabriquerait des skips faux (une approbation ancienne suivie d'un push est le cas normal, et le dossier a la nouvelle tete couvre le nouveau commit). Le blocage eventuel est le point 2 (REVIEW_READY), pas ici.La disposition est reportee pour toute PR evaluee, skip compris, dans la ligne de journal (
review), sur les lignesMERGED/WOULD MERGE, et comptee sur les seules candidates au merge par le bilan (candidates : N approved-exact-head, M approval-not-on-head) — compter sur les skips diluerait le chiffre qui decide.Falsification mesuree
test_deux_prs_qui_ne_different_que_par_la_tete_de_l_approbation(deux PRs identiques, l'une approuvee a la tete, l'autre sur un commit anterieur)review; les deux sortentWOULD MERGE (…) [dry-run], indistinguablesapproved-exact-head/approval-not-on-head, le reste de la ligne etant egalAttributeError(la fonction n'existe pas)test_un_skip_porte_la_disposition…KeyErrortest_le_bilan_compte_les_candidates…test_journal_ligne_par_pr(schema fige des cles)review, le test le ditno-approval), pasnot-evaluatedLa derniere ligne du tableau est le point de vigilance : le verdict terminal est reconstruit apres le merge dans
run(), il herite donc de la classification faite avant (PRVerdict(..., decision.review)) — sans quoi une ligne mergee se liraitnot-evaluated. Le meme report est fait surmerge-failed.Limites declarees
CHANGES_REQUESTEDn'est pas type comme disposition bloquante. Le point 1 nomme deux etats ; une voix qui demande des changements n'est ici « pas une approbation », elle n'est pas qualifiee plus loin. Le gate de merge ne l'a jamais lu non plus — c'est un ecart reel, signale, pas corrige dans cette PR (sujet separe).reviews[]sont attribuables a une tete. Le canon lit aussi les commentaires d'issue ; un commentaire n'a pas de commit, aucune attribution n'est possible — il n'entre donc pas dans la classification a la tete exacte.gh pr viewrend pour la PR (pas de borne explicite, comme pourcomments).Preuves
python -m pytest scripts/tests/test_merge_ready.py -qmain: 8 tests ajoutes, 1 schema etendu)encoding=surtext=True)cp(jamaisgit checkout --), suite reverifiee apres restaurationPerimetre
2 fichiers, +254 / −10 :
scripts/coordination/merge_ready.py(import du canon, constantes,review_disposition/_review_voice_state,reviewsdans la vue, champ + cle de journal, report sur les verdicts terminaux, bilan, docstring) et son test. Aucun notebook, aucun catalogue, aucune dependance ajoutee. La PR ouverte #17742 touche le meme fichier (isolation d'erreur par entree) : hunks disjoints, rebase trivial si l'ordre de merge l'exige.See #17672— point 1 livre ; point 2 (REVIEW_READY) reste ouvert.🤖 Generated with Claude Code