Skip to content

fix(guards): check_unaddressed_nits — toute réserve est levée par n'importe quel commentaire de la lane (nit_author == pr_author == jsboige) #12319

Closed

Description

@myia-ai-01

scripts/check_unaddressed_nits.py rend exit 0 sur des PRs portant une réserve Hermès explicitement rédigée « à corriger avant merge », jamais levée. Mesuré aujourd'hui sur 4 PRs du même lot : #12271, #12306, #12274, #12265.

C'est l'organe de la règle B.0. Tant qu'il est aveugle sur cette classe, la règle n'est plus portée par rien : les lanes attestent [READY-FOR-MERGE] en citant son exit 0, et le coordinateur merge en le citant aussi.

Le mécanisme

Deux lignes, prises ensemble :

# ~L770 — AUCUNE exigence de LIFT_MARKER ici
lift_events = [
    (ts(c["createdAt"]), (c.get("author") or {}).get("login", ""), c.get("body", "") or "")
    for c in (pr_data.get("comments") or []) if can_lift(c)
]

# ~L793
def _lift_eligible(lift_author, nit_author, lift_body=""):
    if lift_author in (nit_author, pr_author):
        return True

explicit_lifts exige bien un LIFT_MARKER — mais il ne sert qu'à la branche review:CHANGES_REQUESTED. Toutes les autres réserves (dont chaque review Hermès, qui est COMMENTED par self-review cap) passent par la branche elif qui consulte lift_events, où tout commentaire suffit.

Et la borne d'auteur ne borne rien ici : Hermès poste sous jsboige (self-review cap) et toutes les lanes poussent sous jsboige. Donc nit_author == pr_author == "jsboige" sur pratiquement chaque PR du dépôt, et _lift_eligible rend True pour n'importe quel commentaire ultérieur de la lane.

Résultat : une lane éteint la réserve de son propre reviewer en postant un [READY-FOR-MERGE SELF attestation] qui ne la mentionne pas. C'est littéralement le geste que B.0 interdit — « un commit poussé après la remarque ne la lève PAS à lui seul » — transposé du commit au commentaire de forme.

La borne #11145 n'est pas fautive : elle a été écrite pour un monde à logins distincts, et sa mesure sur 850 PRs (SELF 59,5 % / PR_AUTHOR 16,2 % / BYSTANDER 24,3 %) était juste dans ce monde-là. Ce qui a changé, c'est que Hermès s'est mis à poster sous l'identité de l'auteur. L'hypothèse d'identité distincte est devenue fausse sans que personne ne la re-teste.

Reproduction

python scripts/check_unaddressed_nits.py 12271        # OK  — aucun nit non leve.   <-- faux
gh pr view 12271 --json reviews --jq '.reviews[].body' | grep -c "avant merge"   # 1
gh pr view 12271 --json commits --jq '.commits[-1].committedDate'                # 02:02Z
gh pr view 12271 --json reviews --jq '.reviews[0].submittedAt'                   # 02:30Z — apres le dernier commit

Le classifieur voit pourtant tout ce qu'il faut — c'est la levée qui est fausse, pas la détection :

m.classify("jsboige", body)                       # 'BOT-CONCERN'
m.has_live_marker(stripped, m.CONCERN_MARKERS)    # True
m.has_marker(attestation_body, m.LIFT_MARKERS)    # False   <-- et pourtant ca leve

Correctif proposé

Exiger un LIFT_MARKER partout, pas seulement sur la branche CHANGES_REQUESTED — c'est-à-dire faire consulter explicit_lifts (et non lift_events) à la branche elif. B.0 le dit déjà en toutes lettres : « ce qui lève une remarque est une phrase, pas un SHA » — ni un SHA, ni un commentaire de protocole.

lift_events garde son rôle propre pour les re-reviews APPROVED (état natif GitHub, qui est une phrase de levée au sens fort).

Contrôle négatif à préserver (sans lui, on remplace un faux négatif par un faux positif) :

  • une levée écrite réelle (« je lève », « levée », « LGTM », « est adressé ») continue de lever ;
  • une re-review APPROVED continue de lever ;
  • un [OVERRIDE] lane … du coordinateur continue de lever ;
  • CONDITIONAL_LIFT (« corrige X et je lève ») continue de ne pas lever.

Contrôles positifs (doivent bloquer après le fix) : #12271 et #12306 — les deux sont ouvertes et CHANGES_REQUESTED par moi à l'instant, elles servent de fixtures vivantes.

Portée — ce qui a été mergé sous ce régime

À vérifier par un --audit après le fix. Sur le lot d'aujourd'hui, les deux PRs mergées qui portaient une réserve vivante (#12274, #12265) étaient en réalité couvertes : leurs nits sont corrigés par #12310 et #12309, que j'ai vérifiés arithmétiquement contre les outputs avant merge, et Hermès qualifiait celui de #12265 de « non bloquant ». Le trou n'a donc pas produit de dégât mesurable ce coup-ci — il a produit un OK qui ne valait rien, et c'est ça qu'on corrige.

Un --audit --limit 400 re-passé après le fix dira si des merges antérieurs sont concernés.

Réf. : B.0 (CLAUDE.md §B), #11145 (borne d'auteur), #11222 (état natif), #10761 (incident fondateur), #12314 (autre correctif en cours sur CONCERN_MARKERS).

Activity

  1. added a commit that references this issue on Aug 22, 2026
  2. added a commit that references this issue on Aug 22, 2026
  3. added a commit that references this issue on Aug 22, 2026
  4. added
    candidate-deliveredReferenced by a merged PR with no post-merge activity -- candidate for close triage (#10466)
    on Aug 23, 2026
  5. jsboige commented on Aug 24, 2026

    @jsboige
    Owner

    Retrait du label candidate-delivered (misfire, verifie firsthand 2026-08-24).

    Le bug #12319 est TOUJOURS PRESENT sur origin/main d2be9da : check_unaddressed_nits.py:925-928 passe encore par la branche elif lift_events — rend True pour nit_author == pr_author == jsboige (L834-837), donc une reserve Hermes postee sous jsboige est levee par n'importe quel commentaire ulterieur de la lane. explicit_lifts (L852) existe mais ne sert qu'a la branche approved_rereview/LIFT_MARKER (L916-922) ; le fallback elif lift_events reste l'echappatoire.

    Les PRs merges qui referencent #12319 (#12336, #12306, #12344) ne touchent pas check_unaddressed_nits.py — le fix demande (consulter explicit_lifts sur la branche elif) n'a pas ete livre. Issue reste ouverte.

  6. removed
    candidate-deliveredReferenced by a merged PR with no post-merge activity -- candidate for close triage (#10466)
    on Aug 24, 2026
  7. jsboige commented on Aug 24, 2026

    @jsboige
    Owner

    [Re-post propre — corrige le micro-gap de la version precedente.]
    Verification candidat-delivered (misfire, G.9) : le bug #12319 est TOUJOURS
    PRESENT sur origin/main d2be9da.

    check_unaddressed_nits.py:925-928 passe encore par la branche elif lift_events.
    _lift_eligible (L834-837) rend True pour nit_author == pr_author == jsboige,
    donc une reserve Hermes postee sous jsboige est levee par n'importe quel
    commentaire ulterieur de la lane. explicit_lifts (L852) existe mais ne sert
    qu'a la branche approved_rereview/LIFT_MARKER (L916-922) ; le fallback
    elif lift_events reste l'echappatoire.

    Les PRs merges qui referencent #12319 (#12336, #12306, #12344) ne touchent pas
    check_unaddressed_nits.py — le fix demande n'a pas ete livre. Label
    candidate-delivered retire ; issue reste ouverte.

  8. jsboige commented on Aug 24, 2026

    @jsboige
    Owner

    [CLAIMED] lane myia-po-2027:CoursIA-2 -- paths: scripts/check_unaddressed_nits.py, scripts/tests/test_check_unaddressed_nits.py -- correctif propose: branche elif -> explicit_lifts (LIFT_MARKER partout), lift_events garde les re-reviews APPROVED natifs. Controles negatifs preserves, fixtures 12271/12306, audit post-fix.

    Grain: MED/guard — lane myia-po-2027:CoursIA-2 — prev: MED/ledger #12815

  9. jsboige commented on Aug 24, 2026

    @jsboige
    Owner

    [RELEASED] lane myia-po-2027:CoursIA-2 — correctif livré en PR #12819 (branche fix/12319-nits-lift-marker, commit 09914fe). Validation: 134 passed (5 tests de regression #12319), fixtures #12271/#12306 BLOCKED post-fix, audit --limit 30 -> 0 flagged. Audit --limit 400 en cours, resultat poste sur la PR.

  10. added 2 commits that reference this issue on Aug 25, 2026
  11. added 2 commits that reference this issue on Aug 29, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions