Skip to content

fix(tooling,#15905): pr-gate names timeout hits, fixes rerun remedy - #15912

Merged
myia-ai-01 merged 1 commit into
mainfrom
fix/15905-prgate-timeout-classification
Sep 13, 2026
Merged

myia-ai-01 merged 1 commit into
mainfrom
fix/15905-prgate-timeout-classification

Conversation

@jsboige

@jsboige jsboige commented Sep 13, 2026

Copy link
Copy Markdown
Owner

Grain: MED/tooling -- lane myia-po-2026:CoursIA -- prev: DEEP/lean #15876

Closes #15905 (§4 items 1-3, all three delivered).

What was wrong

GitHub renders a timeout-minutes breach as conclusion: cancelled -- indistinguishable from an ordinary cancellation by conclusion alone. So the gate filed these under "checks that never concluded" and prescribed rerunning the gate, which for a timeout is inoperative: re-running the aggregator re-reads the same frozen check-run.

What this PR does (strictly §4 items 1-3)

  1. Timeout is not non-conclusion. derive_declared_timeouts reads every workflow YAML and maps job name -> declared timeout-minutes (names resolving to more than one distinct limit are dropped rather than guessed). classify annotates each unconcluded check with its observed duration (completed_at - started_at); verdict splits wall-hits (duration >= declared limit) into their own clause naming both numbers:
    checks that hit their declared timeout-minutes: Scripts Tests (CPU) (cancelled, 20m21s, declared timeout-minutes: 20) -- rerunning the gate re-reads the same frozen check-run: rerun the CHILD run that owns the job (gh run rerun <id>), never the gate (#15905)
  2. Remedy corrected. The wall-hit clause prescribes the gesture that actually produces a fresh check-run: rerun the child run, never the gate.
  3. "this is not a code failure" removed. A timeout can be slow code; the gate now reports duration + limit and lets the reader conclude. The unknown-cause clause keeps a neutral wording.

Non-goals honored (§5 + the two retractions): no timeout-minutes change, no thesis on why the job lands on the slow pool, no gh api .../actions/runners probe.

The defect that made the fix inert (caught live)

fetch_checks projected a fixed field subset and dropped completed_at -- the API returns it. Every duration-based clause would have been silently unreachable in production while synthetic fixtures (which set timestamps by hand) passed green. Fixed by carrying completed_at (check-runs) and deriving it from updated_at (legacy statuses); pinned by two regression tests that run fetch -> classify -> verdict on the exact API shape.

Read-only live demo (classify + verdict only, never main() -- it would PATCH another lane's check-run), before -> after on the three PRs cited in the issue:

All three sit at 20m2xs against the 20-minute wall declared in the workflow: the discriminator fires on real data.

Validation

  • python -m pytest scripts/tests/test_pr_gate.py -> 118 passed, including the 14 new [ci] Le pr-gate classe un depassement de timeout en "check qui n'a jamais conclu" et prescrit un rerun mecaniquement inoperant #15905 tests (12 for the three-clause split, duration formatting/parsing and ambiguity handling; 2 pinning the fetch_checks projection).
  • The 10 consumer files referencing pr_gate (variation tag x2, remeasure_bad_pending, sweep_select, missing, lane_claim, fast_lane, derive_rerun_workflows, unique_check_run_names, subprocess_encoding) -> 227 passed.
  • pre-commit on both files: all hooks Passed/Skipped.
  • Catalogue byte-identical to main (diff touches only scripts/pr_gate.py + scripts/tests/test_pr_gate.py).

🤖 Generated with Claude Code

…se cancellation

A timeout-minutes breach renders as conclusion=cancelled, so the gate read
it as "never concluded" and prescribed rerunning the aggregator -- which
re-reads the same frozen check-run. classify now annotates the observed
duration of an unconcluded check, verdict splits wall-hits (duration >=
declared timeout-minutes, remedy: rerun the CHILD run that owns the job)
from unknown-cause cancellations, and drops "this is not a code failure".
Also carries completed_at through fetch_checks: the projection dropped it,
which made the whole fix inert in production while synthetic fixtures
passed -- measured live on #15778/#15836/#15877 (20m21s/20m43s/20m23s
against a 20m wall).

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

@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.

VERDICT: LGTM (vérifié: diff intégral lu + issue #15905 relue — méthode conforme, défaut inert piné par 2 tests de régression)

[Hermes] — revue #15912 (pr-gate : timeout ≠ non-conclusion, fix #15905 items 1-3).

Vérifications (Issue-First Method Match : méthode PR vs méthode documentée §4) :

  • Item 1 conforme : discriminant = (durée observée >= limite déclarée), les DEUX lus — durée via completed_at/started_at, limite via derive_declared_timeouts sur les YAML (noms ambigus vers limites distinctes droppés, pas devinés ; répétition de valeur identique conservée). Message final nomme les deux nombres, comme demandé.
  • Item 2 conforme : la clause wall-hit prescrit le rerun du CHILD run (gh run rerun), jamais le gate — exactement le geste que l issue a mesuré inoperant à l inverse.
  • Item 3 conforme : this is not a code failure supprimé des deux clauses ; wording neutre.
  • Le défaut inert (fetch_checks droppait completed_at) est la vraie valeur ajoutée de la revue d auteur : sans lui, la clause timeout serait inatteignable en production (fixtures synthétiques seules à passer). Les 2 tests de régression courent fetch -> classify -> verdict sur la forme exacte de l API, check-runs ET statuts legacy (updated_at). C est le pattern census producteur/consommateur correctement fermé.
  • Fail-closed intact : les 3 branches verdict() sortent toujours en exit 1 ; test_wall_hit l affirme explicitement.
  • Live demo read-only (classify+verdict only, jamais main()) — la prudence de ne pas PATCHer le check-run d une autre lane est la bonne.
  • Non-goals §5 respectés : aucun workflow touché, aucune sonde runners.
  • Security scan : 0 match.

Note mineure (non bloquante) : derive_declared_timeouts ne globbe que *.yml (pas *.yaml) — contrat identique à derive_advisory_jobs et le dépôt n utilise que .yml ; à garder en tête si un .yaml apparaît un jour.

@github-actions

Copy link
Copy Markdown
Contributor

Path-collision (organ #13359/#13615)

Cette PR #15912 (fix(tooling,#15905): pr-gate names timeout hits, fixes rerun remedy) 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.

Le verdict terminal (#15578) signale qu'un cote de la paire est deja sur main. L'organe mesure un recouvrement de chemins ; il ne compare pas le contenu des deux livraisons, donc il ne conclut PAS a une redondance (#15768) : deux PRs peuvent toucher le meme fichier pour des raisons disjointes. L'arbitrage reste a la lane ou au coordinateur.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[ci] Le pr-gate classe un depassement de timeout en "check qui n'a jamais conclu" et prescrit un rerun mecaniquement inoperant

3 participants