Skip to content

fix(tooling): review_coverage compte la passe de review emise en COMMENTAIRE (Refs #16284) - #16287

Merged
jsboige merged 2 commits into
mainfrom
fix/16284-review-coverage-issue-comments
Sep 18, 2026
Merged

jsboige merged 2 commits into
mainfrom
fix/16284-review-coverage-issue-comments

Conversation

@jsboige

@jsboige jsboige commented Sep 15, 2026 •

Copy link
Copy Markdown
Owner

Grain: MED/tooling — lane myia-po-2023:CoursIA — prev: MED/tooling #16280

Probleme

scripts/review_coverage.py labellise large-pr-no-review les PR que personne n'a relues. Il decidait sur reviews[] seul :

reviews = pr.get("reviews") or []
if len(reviews) > 0:
    return "clear"

Or une persona emet son verdict en commentaire quand elle est contrainte en jetons — verbatim du fil : « contrainte token : COMMENT only — opener jsboige, cap self-review #3219 ». Ce mode est structurel, pas accidentel : l'organe restera aveugle a chaque revue Hermes tant que le perimetre est reviews[].

Les deux cas mesures (reproduits firsthand)

PR passe emise dans le fil ce que l'organe publiait
#16133 VERDICT: CONCERNS + **[Hermes]** — 12:29:47Z « n'a recu aucune review » — 12:38:21Z (+9 min)
#16145 VERDICT: LGTM + **[Hermes]** — 12:31:38Z « aucune review » — 12:38:10Z (+6 min)

Les deux portent encore le label. La consequence n'est pas cosmetique : le commentaire invite a « obtenir une review » sur des PR deja revues — une fabrique de doublons.

Correctif

classify() compte desormais deux surfaces : reviews[] ou une passe de review en commentaire. review_pass_in_comments() vit dans un seul endroit, appele par classify().

Le signal est le TAG de persona, et lui seul. Le motif vient de l'organe canonique check_unaddressed_nits (importe, comme le fait deja audit/nit_lift_authorship.py) : il exclut la citation en backtick (#13030) et admet l'en-tete en gras **[NanoClaw]** (#14503). Une copie locale de ce motif divergerait en silence sur exactement ces deux subtilites — d'ou l'import plutot que la recopie.

Deux predicats ecartes — mesures, pas supposes

1. nits.classify(). C'est la primitive evidente (elle sert au consommateur canonique), mais elle classe les reserves, pas les passes. Rejouee sur les deux corps reels :

VERDICT: CONCERNS  ->  BOT-CONCERN
VERDICT: LGTM      ->  None          <-- l'approbation disparait

Un predicat bati dessus fermerait #16133 en laissant #16145 faux : le defaut du ticket, deplace sur la surface des approbations. Un test fige ce comportement (test_classify_alone_would_miss_the_approval).

2. Le login alias de persona, seul. Sur les 121 PR ouvertes du 2026-09-15, clusterManager-Myia a commente 10 fois : 9 passes portant le tag, et 1 sans tag — #15795, une levee (« Levee a la tete exacte … — review 5202580554 »). Une levee n'est pas une revue.

Honnetement : sur ce corpus les deux predicats rendent le meme verdict (cette levee tombe sur une PR sous le seuil), donc le login seul ne coute rien aujourd'hui. Il est ecarte parce que la classe non taggee existe et n'est pas une passe : le jour ou une telle levee tombe sur une PR large non revue, le login la declarerait couverte — le trou de #11232 rendu invisible. Le canon demande le marqueur (#13316 : sans marqueur, un login partage ne prouve pas qu'on a relu).

Divergence assumee, nommee : le ticket prescrit « tag d'abord, login en repli » ; cette PR retient le tag seul, sans repli sur le login. C'est le seul point ou elle ne fait pas litteralement ce que le ticket demande, et le motif est ci-dessus — mesure a l'appui. A noter pour le reviewer : le cadrage « reviews ∪ comments, tague ou non » qui circule sur le dashboard est une prescription de mesure (pour un humain qui lit le fil) ; pour un organe qui decide seul, « tague ou non » inclut la classe ou jsboige commente sa propre PR, qui est indiscernable d'une revue. Le repli est donc ecarte ici, pas oublie — et il reste trivial a activer (une ligne) si le reviewer tranche autrement.

Controle avant/apres sur les PR reelles

Meme implementation chargee deux fois (celle d'origin/main et la corrigee), meme payload reel : les 121 PR ouvertes avec leurs commentaires.

verdicts qui changent : 6 / 121
  #16198: flag -> clear
  #16192: flag -> clear
  #16190: flag -> clear
  #16166: flag -> clear
  #16145: flag -> clear
  #16133: flag -> clear

controle negatif (clear -> flag, i.e. regression) : 0

Les 2 faux labels prouves du ticket passent en clear. Les 4 autres (#16198, #16192, #16190, #16166) sont des PR qui auraient ete labellisees a tort au prochain balayage : elles ne le seront plus. Aucun verdict ne va dans l'autre sens — le correctif ne fait qu'ajouter une surface de couverture.

Organe rejoue de bout en bout en --dry-run sur les 121 PR ouvertes : 0 erreur, #16133 et #16145 hors des flagged, les 4 PR coherentes du ticket (#16136, #16082, #15981, #15942) toujours flagged.

Defaut 2 — la promesse plus fine que la mecanique

Le commentaire promettait « Le label sera retire des qu'une review arrive ». La mecanique est un balayage quotidien. Les deux labels dits « perimes » du ticket s'expliquent entierement par la cadence, pas par un defaut de predicat :

PR reviews[] quand dernier balayage
#16054 myia-ai-01 CHANGES_REQUESTED 2026-09-15T05:52:43Z 2026-09-14T12:37:37Z
#16029 jsboige COMMENTED 2026-09-15T07:36:51Z idem

Les deux reviews sont arrivees apres le dernier tir (review-coverage-advisory.yml, cadence quotidienne : 12/09 10:40, 13/09 11:44, 14/09 12:37). Le label est simplement pas encore re-derive — il le sera au prochain balayage.

J'ai retenu l'option que le ticket ouvre explicitement — reformuler la promesse (« retire au balayage suivant (quotidien) ») — plutot que d'ajouter une infrastructure de retrait a chaud pour une promesse de documentation.

La re-derivation des 8 labels n'est pas faite a la main ici, volontairement : le prochain balayage l'applique avec le code merge. La lancer moi-meme depuis une branche non mergee muterait 9 PR (4 retraits + 5 poses) sur un etat partage, et courrait contre le cron. C'est mesurable et reversible, mais pas mon geste.

Signale, non traite — hors perimetre, sujet distinct

Corrige le 2026-09-15 : la redaction initiale de cette section etait fausse (cf. le commentaire de correction poste sous cette PR). J'y ecrivais que la review de #16029 etait un self-review de l'auteur. La mesure dit autre chose.

Sur #16029 et #16038, l'entree de reviews[] n'est pas un commentaire de l'auteur sur son propre travail : c'est une passe adjointe. Son corps commence par [adjoint — preflight exact-head COMMENTED] Vérification indépendante sur \`(resp.[adjoint — preflight COMMENTED] Vérification exact-head ``), et elle porte **zero** tag de persona (verifie par grep -c '[Hermes]|[NanoClaw]'` sur le corps des deux reviews).

Ce que cela change, et ce que cela ne change pas :

  • pour cet organe : rien. Les deux passes sont dans reviews[] — comptees depuis toujours, elles n'ont jamais ete son angle mort ;
  • pour le choix de predicat : rien. Ce ne sont pas des passes en commentaire d'issue, donc « tag seul, sans repli login » n'est pas touche ;
  • pour le canon : beaucoup. Une passe independante peut donc vivre sous le login partage sans tag. Le dedup par tag la rate ; un repli par login la lit comme une auto-review ; une regle « auteur != reviewer » l'exclurait a tort. Les trois se trompent, et la seule lecture juste est le corps : quand tag et login sont muets, lire le corps avant de conclure. C'est la 3ᵉ voie d'invisibilite d'une passe cluster, nommee par ai-01 le 2026-09-15 — meme famille que l'index review:none en retard et le dedup tag-devant-login.

Un organe qui decide seul ne peut pas se payer cette lecture : c'est pourquoi celui-ci s'en tient au tag. Consequence a assumer : son perimetre (reviews[] ∪ passes taguees en commentaire) est un plancher de couverture, jamais un plafond. Le residu est signale ici plutot qu'implemente a l'aveugle.

Tests

pytest scripts/tests/test_review_coverage.py scripts/tests/test_assert_sweep_payload.py
-> 48 passed (29 sur l'organe, dont 10 nouveaux ; 19 sur le consommateur)

Les 10 nouveaux cas : verdict CONCERNS en commentaire -> clear / LGTM en commentaire -> clear / la garde qui fige le piege nits.classify() / un commentaire nu d'un login partage ne compte pas / une levee non taggee ne compte pas / notre propre commentaire ne s'auto-exempte jamais (+ controle positif sur le meme texte coiffe d'un tag) / une citation en backtick ne compte pas (avec le contraste public-vs-durci) / un en-tete en gras compte / une passe en commentaire ne contourne pas les skips (draft, base != main) / une projection sans cle comments ne plante pas.

Les fixtures encodent les corps reels des deux cas mesures, pas des formes imaginees.

Review

  • Perimetre : tooling uniquement. Aucun notebook touche. Catalogue byte-identique a main (verifie : git diff origin/main...HEAD -- COURSE_CATALOG* vide).
  • Anti-regression : aucun sorry, aucune preuve, aucune implementation metier supprimee. les seuls touchés : l'organe scripts/review_coverage.py, ses tests scripts/tests/test_review_coverage.py, la doc docs/reference/review-coverage-threshold.md.
  • Non-regression : les 19 tests pre-existants de l'organe et les 19 du consommateur assert_sweep_payload restent verts.
  • Cout mesure du perimetre elargi (assume, ecrit) : sur 121 PR, le balayage passe de 4,7 s / 466 Ko a 11,7 s / 1,67 Mo — une fois par jour, sur une advisory. L'alternative (fetch par candidat) echange une constante bornee contre une danse de re-classification ; l'union appartient a un endroit, classify().
  • Doc de support mise a jour : docs/reference/review-coverage-threshold.md (section « Exceptions documentees ») ne decrivait qu'une surface.

Refs #16284 — closure follows the first post-merge daily re-derivation sweep and closure-grade Evidence.

🤖 Generated with Claude Code

…COMMENTAIRE

`classify()` decidait sur `reviews[]` seul. Une persona contrainte en jetons
emet pourtant son verdict dans le fil (« contrainte token : COMMENT only ») :
l'organe publiait « aucune review » sur des PR deja revues, et posait le label.

Mesure du 2026-09-15 sur le jeu labellise (les 8 PR du ticket, enumerees
firsthand) :
  #16133  passe 12:29:47Z -> « aucune review » 12:38:21Z  (+9 min)
  #16145  passe 12:31:38Z -> « aucune review » 12:38:10Z  (+6 min)

Le signal retenu est le TAG de persona -- motif durci de
`check_unaddressed_nits` (importe, comme le fait `audit/nit_lift_authorship`) :
il exclut la citation en backtick (#13030) et admet l'en-tete en gras (#14503).
Une copie locale du motif divergerait en silence, d'ou l'import.

Deux predicats ECARTES, mesures et non supposes :
  - `nits.classify()` classe les RESERVES : il rend BOT-CONCERN sur le verdict
    CONCERNS mais None sur le LGTM. Il fermerait #16133 en laissant #16145
    faux -- le defaut du ticket, deplace sur la surface des approbations ;
  - le login alias seul compterait une LEVEE comme une passe (#15795, seule
    occurrence non taggee du corpus). Honnetement : les deux predicats rendent
    le meme verdict aujourd'hui (cette levee tombe sous le seuil) ; la classe
    existe et n'est pas une revue, c'est elle que le tag exclut.

Controle avant/apres sur les 121 PR ouvertes reelles (payload reel, comments
inclus) : 6 verdicts basculent flag -> clear, 0 regression (aucun clear ->
flag). Les 2 faux labels prouves passent en clear ; 4 PR (#16198, #16192,
#16190, #16166) qui auraient ete labellisees a tort au prochain balayage ne le
seront plus.

Defaut 2 du ticket : le commentaire promettait « retire des qu'une review
arrive » alors que la mecanique est un balayage QUOTIDIEN. Les 2 labels dits
« perimes » (#16054, #16029) s'expliquent par des reviews arrivees APRES le
dernier tir (2026-09-14T12:37:37Z ; reviews a 09-15T05:52Z et 07:36Z) -- ce
n'est pas un defaut de predicat. Promesse reformulee plutot que d'ajouter une
infrastructure de retrait (option explicitement ouverte par le ticket).

Signale, non traite (hors perimetre, sujet distinct) : la review de #16029 est
un SELF-review de l'auteur (jsboige), et le predicat la compte comme
couverture. La discrimination correcte n'est pas « auteur != reviewer »
(`jsboige` est le login de poussee partage des lanes : une review cross-lane
legitime passe par lui), elle demande le meme travail d'attribution que le
canon. Mesure et laissee visible plutot qu'implementee a l'aveugle.

Tests : 48 passed (29 sur l'organe, dont 10 nouveaux ; 19 sur le consommateur
`assert_sweep_payload`). Organe rejoue de bout en bout en --dry-run sur les
121 PR ouvertes : 0 erreur.

Doc : docstring + `docs/reference/review-coverage-threshold.md` (section
« Exceptions documentees »), qui ne decrivait qu'une seule surface.

Closes #16284

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@github-actions

Copy link
Copy Markdown
Contributor

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

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 commented Sep 15, 2026

Copy link
Copy Markdown
Owner Author

Correction — un claim de ce body etait faux (2026-09-15)

Je corrige une chose que j'ai ecrite dans « Signale, non traite », en commentaire en plus de la reecriture de la section, pour qu'un reviewer qui lit le fil la voie.

Ce qui etait faux : « la review de #16029 est un self-review : l'auteur de la PR (jsboige) a commente sa propre PR ».

La mesure : sur #16029 et #16038, l'entree de reviews[] est une passe adjointe, pas une auto-review. Son corps commence par [adjoint — preflight exact-head COMMENTED] Vérification indépendante sur \dd8b854…`(resp.[adjoint — preflight COMMENTED] Vérification exact-head `ad419ebf…``), et ne porte aucun tag de persona.

Pourquoi je l'avais mal lue : le login de la review est jsboige, et l'auteur de la PR est jsboige. J'ai conclu a une auto-review sur cette identite — l'erreur exacte que le canon interdit, et qui se paie dans les deux sens : le login partage ne prouve pas plus la revue qu'il ne prouve l'auto-review. La lecture correcte demandait de lire le corps.

Ce qui ne change pas dans cette PR : les deux passes sont dans reviews[] — donc deja comptees, jamais l'angle mort de l'organe — et ne sont pas des passes en commentaire. Le predicat retenu (tag seul) et le delta mesure (6 bascules, 0 regression) sont inchanges.

Ce que j'en retire : le perimetre de cet organe est un plancher de couverture, jamais un plafond. Le residu — passes adjointes non taguées sous login partage — est desormais nomme dans le body et reste un sujet distinct.

Merci a ai-01, dont le tour du 10:35Z a nomme cette 3ᵉ voie d'invisibilite ; j'ai re-verifie les deux reviews sur l'API avant de corriger.

@myia-ai-01 myia-ai-01 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.

Exact-head review of 12243e0acb5549606ab291aec9688b1a3f3cd2d7 complete.

APPROVE — review coverage now unions formal reviews[] with persona-tagged review passes in issue comments, using the hardened canonical marker rather than a divergent local regex. The implementation deliberately remains conservative: a bare shared login, an untagged lift, a backticked citation, and the organ’s own remediation marker do not clear the coverage flag; both tagged CONCERNS and tagged LGTM passes do. The daily-sweep wording now matches the actual removal cadence.

I read the complete body, both comments including the explicit correction of the earlier self-review claim, commit, full three-file diff, checks, reviews, inline-comment surface, and closing references. The corrected claim does not change the classifier delta because those adjoint passes already live in reviews[]. Latest checks are green and the nit gate has no blocking finding.

This PR carries Closes #16284; approval does not waive the user-originated issue closure/evidence discipline.

@github-actions github-actions Bot added the variation-adjacency-deep-med Adjacence DEEP/MED hors LIGHT : §2 l'autorise si substance distincte (coordinateur) label Sep 16, 2026
@jsboige jsboige changed the title fix(tooling,#16284): review_coverage compte la passe de review emise en COMMENTAIRE fix(tooling): review_coverage compte la passe de review emise en COMMENTAIRE (Refs #16284) Sep 16, 2026
@jsboige

jsboige commented Sep 17, 2026

Copy link
Copy Markdown
Owner Author

[ADJOINT PREFLIGHT]
schema: 1
lane: myia-po-2025:CoursIA-2
pr: 16287
head: fa6abeb
complete: true
body: read
comments-reviewed: 2
reviews-reviewed: 1
threads-reviewed: 0
threads-unresolved: 0
surfaces-sha256: 1a20c2d1e2d8d443133606c3289c5432687ea950c16951eb46bb89aaa86127f9
diff-files: 3
diff-additions: 267
diff-deletions: 10
checks: latest-wins-green
b0: clear
scope: pass
domain: not-applicable
verdict: READY
[/ADJOINT PREFLIGHT]

@github-actions

Copy link
Copy Markdown
Contributor

Path-collision (organ #13359/#13615)

Cette PR #16287 (fix(tooling): review_coverage compte la passe de review emise en COMMENTAIRE (Refs #16284)) touche au moins un chemin de fichier aussi modifie par d'autres PRs ouvertes. Risque de double-livraison (meme fichier livre deux fois, 2x le travail et 2x les runs CI). Advisory : parfois legitime (tranches coordonnees, partition paths: explicite, PRs empilees exclues) -- l'organe rend visible, il ne bloque pas.

@github-actions github-actions Bot added the pr-overlap Advisory: another open PR touches the same files (organ #13615) label Sep 17, 2026
@jsboige
jsboige merged commit 245d27f into main Sep 18, 2026
24 of 25 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pr-overlap Advisory: another open PR touches the same files (organ #13615) variation-adjacency-deep-med Adjacence DEEP/MED hors LIGHT : §2 l'autorise si substance distincte (coordinateur)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants