Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 7 additions & 7 deletions .claude/rules/pr-review-discipline.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,7 @@ S'applique à **tous les reviewers**, humains et bots (clusterManager-Myia, jsbo

## Émission du verdict — un point qui tient le merge porte un marqueur (HARD, #14682)

Toute review (bot ou humaine) dont le corps formule **un point tenant le merge** porte un **marqueur reconnu** : préfixe de verdict (`[Hermes] COMMENT_WITH_CONCERNS`, `CHANGES_REQUESTED`) ou glyphe de sévérité (🟡, 🔴). L'organe B.0 (`scripts/check_unaddressed_nits.py`, `CONCERN_MARKERS`) ne lit **que** ces marqueurs : une réserve bloquante posée en prose libre **sans** marqueur lui est invisible et rend `rc=0` — instance fondatrice **#14658** (réserve qualifiée « le seul point bloquant pour un LGTM plein », prose française sans marqueur, `rc=0`).

La réciproque est tranchée par mesure (**#14682**, scan de 80 PRs mergées) : **ne pas élargir** `CONCERN_MARKERS` à des mots de prose (« bloquant », « à corriger », « est faux ») — un tel filet sur-accuse d'un facteur 5 (4 des 5 détections : de la prose qui *décrit* un blocage de job ou de garde, pas qui *pose* une réserve). Le contrat est côté émission, pas côté filet.
Toute review (bot ou humaine) formulant **un point tenant le merge** porte un **marqueur reconnu** : préfixe de verdict (`[Hermes] COMMENT_WITH_CONCERNS`, `CHANGES_REQUESTED`) ou glyphe de sévérité (🟡, 🔴). L'organe B.0 (`scripts/check_unaddressed_nits.py`, `CONCERN_MARKERS`) ne lit **que** ces marqueurs : une réserve bloquante posée en prose libre **sans** marqueur lui est invisible et rend `rc=0`. **Ne pas élargir** le filet à des mots de prose : il sur-accuse d'un facteur 5 (mesure #14682) — le contrat est côté émission, pas côté filet. Instance #14658 + scan 80 PRs : [pr-review-context.md](../../docs/reference/pr-review-context.md).

## Critères CHANGES_REQUESTED obligatoires (HARD)

Expand All @@ -29,7 +27,7 @@ Un reviewer **DOIT** poster `state: CHANGES_REQUESTED` (pas COMMENTED, pas APPRO

Toute PR touchant `*.lean` ou `agent_tests/prover/` **DOIT** inclure dans le body :

1. Compte de `sorry` **réel** avant/après — `python scripts/lean/count_code_sorry.py --json`, champ `distinct_code_sorry`. **Pas `grep -c sorry`** : il compte la prose (docstrings, `-- commentaires`, feuilles de route), et les modules Lean du dépôt documentent précisément leur propre absence de `sorry`. Mesuré le 2026-08-14 sur les 21 lakes : **484 naïfs pour 21 réels (23×)**, dont **9 lakes à 0 réel** — un reviewer appliquant `grep` à la lettre exigerait la justification de 68 `sorry` dans `grothendieck_lean`, qui n'en a aucun. Le gate CI mesure déjà juste (`sorry-filter-mode: real` de `lean-axiom.yml`) : c'est le texte de cette règle qui pointait le mauvais instrument.
1. Compte de `sorry` **réel** avant/après — `python scripts/lean/count_code_sorry.py --json`, champ `distinct_code_sorry`. **Pas `grep -c sorry`** : il compte la prose, pas les preuves (mesure du 2026-08-14 : 484 naïfs pour 21 réels sur les 21 lakes, [pr-review-context.md](../../docs/reference/pr-review-context.md)). Le gate CI mesure déjà juste (`sorry-filter-mode: real`) : c'est le texte de cette règle qui pointait le mauvais instrument.
2. Lien vers `Lake build SUCCESS` (CI ou commit local prouvable)
3. Lien vers `Proof integrity SUCCESS` (job CI `proof-integrity` → `LeanVerifier.check_axioms(module, fail_on_sorry=True)`)
4. Si refactor du prover Python : justifier pourquoi il est nécessaire au claim Lean (sinon split)
Expand All @@ -38,11 +36,11 @@ Toute PR touchant `*.lean` ou `agent_tests/prover/` **DOIT** inclure dans le bod

**Un `proof-integrity SUCCESS` antérieur au 2026-07-28 ne prouve PAS l'absence de `native_decide`** (parser aveugle aux noms longs wrappés ; corrigé #8740). Ne pas ré-invoquer un vert plus ancien comme preuve.

**B.3 se lit « non applicable » — et doit être ÉCRIT tel quel dans le body** dans deux cas, jamais sauté en silence : (a) le job n'est pas câblé sur le lake de la PR (#8677) ; (b) il l'est, mais ses `target-modules` n'atteignent pas le module modifié, ni directement ni par clôture d'imports (#8782) — un vert hors-cible est indiscernable d'un vert sur cible dans le rollup. Le job advisory `target-coverage` rend l'écart lisible. Câblage = **exactement** les workflows appelant `lean-axiom.yml` (`grep -ln 'lean-axiom' .github/workflows/*.yml`, moins le fichier lui-même) — mesure mécanique, pas un compte recopié. Triage par lake : [lean-axiom-coverage.md](../../docs/reference/lean-axiom-coverage.md) ; incidents : [pr-review-context.md](../../docs/reference/pr-review-context.md).
**B.3 se lit « non applicable » — et s'ÉCRIT tel quel dans le body** dans deux cas, jamais sauté en silence : (a) le job n'est pas câblé sur le lake de la PR (#8677) ; (b) il l'est, mais ses `target-modules` n'atteignent pas le module modifié (#8782) — un vert hors-cible est indiscernable d'un vert sur cible dans le rollup (job advisory `target-coverage`). Câblage = **exactement** les workflows appelant `lean-axiom.yml` (`grep -ln 'lean-axiom' .github/workflows/*.yml`, moins le fichier lui-même). Triage : [lean-axiom-coverage.md](../../docs/reference/lean-axiom-coverage.md) ; incidents : [pr-review-context.md](../../docs/reference/pr-review-context.md).

### C. ML : multi-seed obligatoire

Toute PR claim « BEATS » / « improvement » sur métriques ML/trading **DOIT** inclure : (1) walk-forward 5-fold ; (2) **≥4 seeds** parmi 0/1/7/42/99 ; (3) **la conjonction** edge ≥ 2σ cross-seed **ET** Diebold-Mariano `dm_p_median < 0.05` — les deux, pas σ seul (σ mesure la dispersion inter-seeds, pas la significativité) — le DM portant sur une **perte de précision** (`loss_fn="mse"` ou `"mae"` dans `MyIA.AI.Notebooks/QuantConnect/ML-Training-Pipeline/scripts/dm_test.py`) ; `loss_fn="linear"` est un **contrôle de biais**, jamais la jambe de la conjonction — mesuré sur #10956/#10961, `d_mean = mean(e_a) − mean(e_b) = biais_a − biais_b`, aveugle à la dispersion : un modèle strictement plus précis peut « perdre » le test face à une baseline plus biaisée ; (4) comparaison à majority baseline + coûts de transaction (5bps SPY, 10bps crypto) ; (5) **pas de FAANG/Mag7** en training ; (6) verdict honnête « BEATS » / « NO BEATS » / « INCONCLUSIVE » — jamais « promising » ; (7) **rapport de biais par modèle** dans le body (`mean(e)` signé ou biais OOS, modèle ET baseline) — le contrôle qui aurait fait apparaître `har_bias_oos = −0.227` (#10938) avant qu'une lecture soit construite dessus ; un edge porté par le biais (pas par la précision) se déclare comme tel.
Toute PR claim « BEATS » / « improvement » sur métriques ML/trading **DOIT** inclure : (1) walk-forward 5-fold ; (2) **≥4 seeds** parmi 0/1/7/42/99 ; (3) **la conjonction** edge ≥ 2σ cross-seed **ET** Diebold-Mariano `dm_p_median < 0.05` — les deux, pas σ seul (σ mesure la dispersion inter-seeds, pas la significativité) — le DM portant sur une **perte de précision** (`loss_fn="mse"` ou `"mae"`) ; `loss_fn="linear"` est un **contrôle de biais**, jamais la jambe de la conjonction (aveugle à la dispersion : un modèle plus précis peut « perdre » face à une baseline plus biaisée, #10961 CE1) ; (4) comparaison à majority baseline + coûts de transaction (5bps SPY, 10bps crypto) ; (5) **pas de FAANG/Mag7** en training ; (6) verdict honnête « BEATS » / « NO BEATS » / « INCONCLUSIVE » — jamais « promising » ; (7) **rapport de biais par modèle** dans le body (`mean(e)` signé ou biais OOS, modèle ET baseline) — un edge porté par le biais (pas par la précision) se déclare comme tel.

Les trois contre-exemples inscrits qui fondent (3) — `+19.97σ` avec `DM p = 0.236` ; `dm_stat` bit-identique sous `mse` pour `e` et `-e` ; et le modèle 11× plus précis « BEATEN » sous `linear` face à une baseline biaisée (#10961, CE1) — sont mesurés dans [pr-review-context.md §C](../../docs/reference/pr-review-context.md).

Expand All @@ -59,7 +57,9 @@ Single-seed ou single-fold = **CHANGES_REQUESTED** sauf flag explicite `[POC]` d

**Refus si :** le body **ne contient pas** `## Diagnostic dérive` (citer #8364 en label ne suffit pas) · verdict `CAUSE_DOCUMENTED_ONLY` **sans** issue fille traitant la cause (= « jambe de bois repeinte ») · la valeur ré-alignée est un **nombre de perf/timing/accuracy/coût** ET le notebook est **re-exécutable localement** (règle F) : elle doit venir d'une **re-exécution fraîche**, jamais d'un byte-surgical markdown-align — enshriner un nombre qui changera au prochain passage kernel *est* la dérive que C.4 interdit. Si une re-exec est **déjà due** : **folder** l'alignement dedans (incident #8479, [détail](../../docs/reference/pr-review-context.md)).

6. **PRs notebook : vérifier le verdict du check-run `Output-failure ratchet (base vs PR)`** (organe `scripts/notebook_tools/check_output_failure_text.py`, enregistré **bloquant** dans `scripts/ci/fast_lane_registry.py`). Le check-run DOIT être `success`. `TOOL_FAILURE` (bannières « `program is not installed` » ou rendu d'échec) ou `MACHINE_PATH` (chemin machine dans une sortie) qui **augmente** sur la PR (N → N+k, type `0 → 21`) = **régression → `CHANGES_REQUESTED`**, même si les points 1-3 passent : les bannières d'échec ne sont **pas** des exceptions Python, elles ne déclenchent ni `exec_count` nul ni `grep -nE "raise NotImplementedError|assert False|1/0"` — une PR qui **remplace** un rendu SVG de factor-graph par une bannière est le dégât exact de #3473/#11685, pas un « ça tourne ». Récurrence : #13517 (PR #13036 LDA — bannières 0→21, `MACHINE_PATH` 0→14, approuvée par Hermes alors que le garde rend rc=1) ; juin → #3473 (~15 filles) ; 18/08 → #11693.
6. **PRs notebook : vérifier le verdict du check-run `Output-failure ratchet (base vs PR)`** (organe `scripts/notebook_tools/check_output_failure_text.py`, **bloquant** dans `scripts/ci/fast_lane_registry.py`) — il DOIT être `success`. `TOOL_FAILURE` (bannières « `program is not installed` ») ou `MACHINE_PATH` qui **augmente** sur la PR (ex. `0 → 21`) = **régression → `CHANGES_REQUESTED`**, même si les points 1-3 passent : les bannières ne déclenchent ni `exec_count` nul ni le grep de 2 — remplacer un rendu SVG par une bannière est le dégât exact de #3473/#11685, pas un « ça tourne ». Récurrences (dont #13517 LDA, approuvée par Hermes malgré `rc=1`) : [pr-review-context.md](../../docs/reference/pr-review-context.md).

7. **PRs notebook : lire le check-run ADVISORY `Output-collapse ratchet (base vs PR, advisory)`** (organe `scripts/notebook_tools/check_output_collapse.py`, enregistré `blocking=False` dans `scripts/ci/fast_lane_registry.py`, #15327). Conclusion neutre par design — le signal vit dans le détail du check-run. Un finding `SIGNATURE` (sortie base substantielle remplacée par « `Execution sautee (API non configuree)` » et consœurs) = **re-exécution sans les clés → `CHANGES_REQUESTED`** : les cellules se sont « exécutées avec succès » en dégradation gracieuse (`if api_ok:`), c'est le contournement de C.2 par la porte de secours. Contre-exemple mesuré (fondateur) : #15209, `Lean-7b-Examples.ipynb` `6b327a9bf` → `56d98429a` — 11 → 11 cellules, 0 erreur, `execution_count` réels partout, et **10637 → 2985** caractères de sortie (cellules `2195 → 147`, `2568 → 38`, `2074 → 42`) : tous les organes verts, la perte réelle. Un finding `MAGNITUDE` (perte d'un ordre de grandeur par cellule, non couvert par les exemptions automatiques contenu-déplacé/purge-diagnostic) exige une **justification dans le body** (allègement déclaré, au même titre que les autres ratchets) — sans elle : `CHANGES_REQUESTED`.

**Advisory `.NET execution_count` ≠ outputs vides autorisés (#5214).** L'advisory autorise à sauter la ré-exécution **CI** (pas de kernel .NET en CI), **pas** à committer des sorties vides : `.NET Interactive` s'exécute **localement** sur chaque worker → une cellule .NET committée **DOIT** porter `execution_count != null`. `validate_pr_notebooks.py` FAIL sur `.NET` + `null`, et ne tolère `null` que là où l'exécution locale est aussi impossible (QC Cloud, Lean). Verdict attendu dans le body : `EXEC_PROVED` vs `STRUCTURAL_ONLY` (refus).

Expand Down
Loading
Loading