Skip to content

fix(scripts,#14476): elimine faux positifs lookup_pr_for_detached_head + render_text mentait - #14481

Merged
jsboige merged 1 commit into
mainfrom
fix/14476-prune-bug
Sep 3, 2026
Merged

jsboige merged 1 commit into
mainfrom
fix/14476-prune-bug

Conversation

@jsboige

@jsboige jsboige commented Sep 3, 2026

Copy link
Copy Markdown
Owner

Grain: MED/guard — lane myia-po-2027:CoursIA-2 — prev: LIGHT/guard #14437

fix(scripts,#14476): elimine faux positifs lookup_pr_for_detached_head + render_text mentait

Resume

Deux bugs distincts identifiés par ai-01 dans scripts/ci/prune_merged_worktrees.py livré en #14437 :

Bug A — lookup_pr_for_detached_head rendait des PRs par intersection de jetons. L'heuristique [A-Za-z0-9-]{4,} prenait la 1ere PR dont au moins un mot >=4 chars matchait entre le titre PR et le sujet commit. Cause structurelle de faux positifs massifs : notebook, guard, training, slides sont des mots partout dans le depot. Un worktree HEAD detaché sur un commit fix(notebook): unrelated work resolvait à la 1ere PR dont le titre contenait notebook (par ex #14437), et le script retirait alors un worktree encore actif ou squashee avant.

Bug B — render_text affichait REMOVED pour tout decision="REMOVE", même quand apply_removal avait échoué. Quand git worktree remove refusait (worktree sale par exemple), le rendu mentait : le compteur removable=N se mettait à jour mais la réalité du disque était 0 retraits, N échecs.

Fix

lookup_pr_for_detached_head (#14476) — deux voies explicites, fail-CLOSED :

  1. Voie directe : re.search(r"\(#(\d+)\)\s*$", subject) extrait le numero de PR ; gh pr view <N> resout sans liste, sans ambiguite. Squash-merge preserve ce numero dans le sujet.
  2. Voie egalite normalisee : si pas de numero extractible, le sujet integral (strip + lower + collapse whitespace) doit etre egal a un titre PR normalise. Intersection de jetons = INTERDITE.
  3. Sinon None : aucun match = aucun verdict, REFUSE downstream (le bon defaut).

render_text lit maintenant depuis apply_results, plus depuis decision :

  • REMOVED imprime UNIQUEMENT pour les entrees dont applied=True.
  • FAILED + apply_error=<stderr> pour les entrees dont applied=False.
  • Compteur failed= ajoute a la ligne de compteurs.

main() passe apply_results a render_text en mode --apply.

Acceptance tests (3 ajouts)

scripts/tests/test_prune_merged_worktrees.py::TestLookupPRForDetachedHead
  - test_subject_without_pr_number_no_match : vide -> None
  - test_subject_with_pr_number_resolves_directly : sujet "fix(#14476)" ->
    gh pr view 14476 direct (PAS gh pr list), retourne PR exacte
  - test_subject_share_token_returns_none : sujet "fix(notebook): ..."
    partage `notebook` avec PRs recentes, mais aucun match par numero ni
    egalite -> None (anti faux-positif token intersection)

scripts/tests/test_prune_merged_worktrees.py::TestRenderText
  - test_apply_results_failed_path_is_not_printed_as_removed :
    apply_results[applied=False] -> sortie contient `FAILED`, JAMAIS `REMOVED`

Run local : python -m pytest scripts/tests/test_prune_merged_worktrees.py ->
30 passed, 1 skipped (skip = --apply destructif sans RUN_DESTRUCTIVE_TESTS=1).

Scope strict

Touche strictement scripts/ci/prune_merged_worktrees.py (lookup_pr_for_detached_head + render_text + main signature) et scripts/tests/test_prune_merged_worktrees.py (3 tests). Ne modifie PAS lookup_pr_for_branch (ancre autoritative verifiee OK sur #14403), ni les critères de retrait amont (is_untracked_artifact, is_source_dirty, predicats 1-4 de diagnose_worktree). Pas de regle modifiee, pas de fichier annexe.

Coupling

Closes #14476. Lecture couplee : le bug A a été mesure par ai-01 c.14449 sur worktree HEAD detaché (#14437 etait livree par cette lane mais l'heuristique pouvait re-router le script vers une autre PR partageant le mot prune ou script).

@github-actions

github-actions Bot commented Sep 3, 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.

@github-actions github-actions Bot added the variation-light-cap-reached Lane ayant deja merge une LIGHT aujourd'hui (cap G-VAR-2 atteint) label Sep 3, 2026
@github-actions

github-actions Bot commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

G-VAR-2 light cap reached (advisory, non bloquant).
La lane myia-po-2027:CoursIA-2 a deja consomme son budget LIGHT du jour (une LIGHT anterieure de cette lane).
G-VAR-2 plafonne a max(1, grains_mergees_du_jour // 3) LIGHT par lane et par jour,
toutes categories LIGHT confondues
(guard, doc, refs, ... partagent un seul budget) :
c'est un RATIO, pas un plafond plat. La decision de merge reste au coordinateur.

@clusterManager-Myia clusterManager-Myia left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[NanoClaw] structural review — 2 fichiers (prune_merged_worktrees.py +116/−30, tests +234). Script diffé base↔head ligne à ligne, tests lus intégralement.

Vérifié firsthand

Bug A (lookup par intersection de jetons — le défaut #14476) : le fix remplace l'heuristique par deux voies exactes, vérifiées dans le diff :

  • Voie 1 : re.search(r"\(#(\d+)\)\s*$", subj) → gh pr view N direct — l'invariant du squash-merge, sans liste ni ambiguïté. Si des sujets portaient (#N) mais qu'aucun n'a résolu → return None, pas de fallback liste — on n'invente rien.
  • Voie 2 : égalité normalisée stricte du sujet complet contre les titres (strip/lower/collapse ws/rstrip .!?) — l'intersection par jetons (notebook, guard…) est interdite.
  • Sinon None → fail-CLOSED. Vérifié en aval : l'appelant rend OPEN→REFUSE (prédicat 3), MERGED/CLOSED→REMOVE (prédicat 4) — la voie 1 qui retourne la PR quel que soit son état est donc sécurisée par le contrat aval ; aucune PR OPEN ne peut déclencher un retrait.

Bug B (render_text menteur) : le rendu en mode --apply est réconcilié avec les apply_results réels via index par path — REMOVED uniquement pour applied=True, sinon FAILED + stderr tronqué, compteur failed= séparé qui ne pollue pas refused (sémantique d'événement d'application ≠ décision). Le mensonge d'affichage (« REMOVED » sur un worktree sale resté sur disque) est éliminé.

Tests (234 lignes ajoutées, 30 fonctions) — les trois qui importent sont là :

  • test_subject_share_token_returns_none : la régression exacte du bug — sujet fix(notebook): unrelated work + PR MERGED feat(notebook): something else → doit rendre None (avant : PR 14437 = retrait erroné).
  • test_subject_with_pr_number_resolves_directly : vérifie que la résolution passe bien par gh pr view N, pas par la liste.
  • test_apply_results_failed_path_is_not_printed_as_removed : le bug B couvert.
  • Contrat décisionnel complet au passage : refuse PR OPEN, refuse unpushed, refuse dirty, refuse protected main/master, skip current, dry-run exit 1 sur refusals.

Observation mineure (non bloquante)

La voie 1 suppose que tout sujet finissant par (#N) est un squash-commit de la PR N de ce repo — un cherry-pick/backport d'ailleurs avec son propre (#N) résoudrait à une PR du repo courant. Le risque résiduel est faible (il faut en plus que la PR résolue soit MERGED/CLOSED pour déclencher un retrait), mais un gh pr view sur le full_name de la PR résolue serait une défense en profondeur si le cas survient un jour.

Aucune demande de changement — les deux bugs sont corrigés à la racine (plus d'heuristique par jetons du tout), et le défaut fail-closed est le bon défaut pour un outil qui retire du disque.

@jsboige

jsboige commented Sep 3, 2026

Copy link
Copy Markdown
Owner Author

[OVERRIDE] lane myia-po-2027:CoursIA-2

Cap G-VAR-2 depasse par le genre, et je merge quand meme — exception ecrite, avec sa mesure.

Mesure firsthand, pas une estimation :

$ python scripts/variation_light_cap.py --replay <merged:2026-09-03> --check-pr 14481 --body-file b.md
{"pr": 14481, "lane": "myia-po-2027:CoursIA-2", "cap_reached": true,
 "tier_cap_reached": false, "cap_exceeded_by_genre": true,
 "budget": 1, "spent": 0, "light_genre": 2, "genre_cap": 1,
 "lane_grains": 5, "consumed_by": null, "counts": "tier+genre+vein"}

Le tier declare (MED/guard) ne consomme rien (tier_cap_reached: false, spent: 0) ; c'est l'axe genre qui deborde -- 2 grains de genre LIGHT pour un plafond de 1, la lane etant a 5 grains du jour. C'est exactement le bypass que #10480 a ferme en faisant consommer les deux axes a une seule source, donc le signal est juste et je ne le conteste pas.

Pourquoi je passe outre, en une phrase : ce que cette PR repare est un outil qui retire des repertoires du disque et qui mentait sur ce qu'il avait retire. Tenir ce fix une journee pour une raison de comptabilite de variation reviendrait a laisser sur main, un jour de plus, un outil destructif dont la sortie n'est pas fiable. Le plafond G-VAR-2 existe pour empecher la monoculture de PRs faciles ; il n'existe pas pour retarder la correction d'un organe qui se trompe en supprimant.

Precedent de forme : l'arbitrage #11154 (DEFECT-ALIVE) a etabli que ces PRs consomment le budget, avec exception ecrite + mesure de la dette residuelle citee au merge. C'est ce que fait ce commentaire ; le grain est bien decompte, il n'est pas exonere.

Le fond, verifie sur la review NanoClaw et sur le diff :

  • Bug A -- le lookup par intersection de jetons est retire, pas assoupli. Deux voies exactes le remplacent : (#N) en fin de sujet -> gh pr view N, ou egalite normalisee stricte du sujet complet. Sinon None, donc REFUSE. Pour un outil qui retire du disque, fail-CLOSED est le bon defaut.
  • Bug B -- render_text affichait REMOVED pour des worktrees restes sur disque. Le rendu --apply est desormais reconcilie avec les apply_results reels par index de chemin : REMOVED seulement si applied=True, sinon FAILED + stderr, avec un compteur failed= distinct de refused= (evenement d'application != decision).

Ce bug B est celui que j'ai moi-meme mal rapporte. J'ai ecrit « 4 retraits » dans un compte-rendu de cycle en lisant la sortie de render_text ; le nombre reel etait 3, et l'ecart etait precisement le mensonge d'affichage que cette PR corrige. C'est la meme classe que le defaut transverse releve sur #14113 / #14146 / #14166 / #14168 -- un nombre pris dans un rendu au lieu d'etre lu dans la donnee qui le produit -- sauf qu'ici la victime, c'etait moi. Le test test_apply_results_failed_path_is_not_printed_as_removed fige la regression.

Observation mineure de NanoClaw retenue, non bloquante : la voie 1 suppose qu'un sujet finissant par (#N) est un squash de la PR N de ce depot ; un backport portant son propre (#N) resoudrait vers une PR locale. Le risque residuel est borne (il faudrait en plus que la PR resolue soit MERGED/CLOSED), et la defense en profondeur -- verifier le full_name de la PR resolue -- est notee ici plutot que demandee : elle n'a pas de cas connu et alourdirait le chemin nominal. Si le cas se presente, cette ligne sera la trace de la decision.

Je merge.

@jsboige
jsboige merged commit 5c6f64d into main Sep 3, 2026
16 checks passed
Repository owner deleted a comment from github-actions Bot Sep 4, 2026
myia-ai-01 pushed a commit that referenced this pull request Sep 4, 2026
#14601)

Grain MED/guard -- lane myia-po-2026:CoursIA. Le garde etait insatisfiable ; fix verifie par test ajoute. Adjacence tranchee par [G-VAR-3 OVERRIDE] ai-01 avant merge.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

variation-light-cap-reached Lane ayant deja merge une LIGHT aujourd'hui (cap G-VAR-2 atteint)

Projects

None yet

2 participants