RISK-003 self-heal coverage : extract shouldRebindIdle helper + tests

Le reviewer flagait une race entre PlayerPatch.initAnimator (bind IDLE
au capability attach time) et AnimationManager.apply (datapack reload).
En investigant : la race existe architecturalement mais le self-heal
est deja en place via maybePlayIdle — quand resolveWithFallback retourne
EMPTY pre-apply, le bind EMPTY est mis dans la map (EMPTY_ANIMATION a
isPresent()==true donc passe checkNull). Au tick suivant, une fois
apply tourne et le registry populé, maybePlayIdle detecte
currentIdleBind == EMPTY et rebind sur la vraie anim.

Le bind EMPTY est intentionnel : ClientAnimator.postInit appelle
playAnimationInstantly(livingAnimations.get(IDLE)) immediatement, et
skipper le bind crasherait postInit (NPE sur null.get()). EMPTY a une
pose identity (no-op visuel) → bootstrap propre, self-heal post-reload.

Le symptome runtime "F6 bindings: 1 mais idle silent" decrit dans la
review vient d'un cause differente : AnimationManager n'est pas encore
register comme reload listener (Phase 2 wiring pending, tracked sous
BUG-RACE-01 dans AnimationManager.apply javadoc). Une fois cable, le
self-heal fonctionne — verrouille par les 5 nouveaux tests.

Changements :
- RigAnimationTickHandler : extract shouldRebindIdle(currentBind, target)
  pure helper package-private. La logique inline dans maybePlayIdle
  reste identique, juste deleguee au helper pour testabilite.
- PlayerPatch.initAnimator : javadoc enrichi (race + raison du bind
  EMPTY + reference test coverage).
- RigAnimationTickHandlerTest : +5 cases sur shouldRebindIdle (null+real,
  empty+real RACE, real+real, two real instances, *+empty).

Tests : 459 → 464 (+5), tous green.

src/main/java/com/tiedup/remake/rig/patch/PlayerPatch.java

@@ -270,15 +270,47 @@ public abstract class PlayerPatch<T extends Player> extends LivingEntityPatch<T>
      * IDLE sur {@code tiedup:context_stand_idle}, résolue via le registry
      * data-driven ({@link TiedUpAnimationRegistry#resolveWithFallback}).
      *
-     * <p><b>Pre-reload window</b> : si le patch est construit avant que
+     * <h3>Pre-reload window — race avec AnimationManager.apply (RISK-003)</h3>
-     * {@code AnimationManager.apply} n'ait tourné (bootstrap client, race
+     * <p>Si le patch est construit AVANT que
-     * entre join et resource-pack reload, etc.), l'ID {@code context_stand_idle}
+     * {@link com.tiedup.remake.rig.anim.AnimationManager#apply} n'ait tourné
-     * n'est pas encore dans {@code animationByName} et
+     * (bootstrap client, race entre join et resource-pack reload, capability
-     * {@code resolveWithFallback} retourne {@link TiedUpRigRegistry#EMPTY_ANIMATION}.
+     * attach pré-{@code AddReloadListenerEvent} en Phase 2 du wiring), l'ID
-     * C'est idempotent côté animator — une prochaine passe de
+     * {@code tiedup:context_stand_idle} n'est pas encore dans
-     * {@link com.tiedup.remake.rig.tick.RigAnimationTickHandler#maybePlayIdle}
+     * {@code animationByName} et {@code resolveWithFallback} retourne
-     * re-bind sur la vraie anim une fois le datapack chargé (self-heal).</p>
+     * {@link TiedUpRigRegistry#EMPTY_ANIMATION}.</p>
      *
+     * <h3>Pourquoi on bind quand même EMPTY_ANIMATION (au lieu de skip)</h3>
+     * <p>{@link com.tiedup.remake.rig.anim.client.ClientAnimator#postInit} appelle
+     * immédiatement {@code playAnimationInstantly(livingAnimations.get(IDLE))}.
+     * Si on skipait le bind ({@code null} dans la map), {@code postInit} NPE
+     * sur {@code nextAnimation.get()}. EMPTY_ANIMATION a {@code isPresent()==true}
+     * (cf. {@link com.tiedup.remake.rig.anim.types.DirectStaticAnimation#isPresent})
+     * donc passe le {@code AnimationManager.checkNull} guard et bootstrap
+     * proprement avec une pose identity (no-op visuel).</p>
+     *
+     * <h3>Self-heal après reload</h3>
+     * <p>Une fois {@code AnimationManager.apply} terminé (datapack chargé, anim
+     * réelle dans le registry),
+     * {@link com.tiedup.remake.rig.tick.RigAnimationTickHandler#maybePlayIdle}
+     * détecte le bind EMPTY au tick suivant via
+     * {@link com.tiedup.remake.rig.tick.RigAnimationTickHandler#shouldRebindIdle}
+     * et rebind sur la vraie anim. Idempotent — les
+     * {@code addLivingAnimation} suivantes écrasent l'entry existante. Aucun
+     * code "retry hook" post-reload n'est nécessaire : le tick handler observe
+     * en pull (chaque tick) plutôt qu'en push (callback reload).</p>
+     *
+     * <h3>Couverture test (RISK-003 verrouillé)</h3>
+     * <p>Voir {@code RigAnimationTickHandlerTest.shouldRebindIdle_*} pour les
+     * tests unitaires de la logique de self-heal pure (sans MC bootstrap).
+     * Cas couverts :</p>
+     * <ul>
+     *   <li>{@code currentBind=null, target=real} → rebind</li>
+     *   <li>{@code currentBind=EMPTY, target=real} → rebind (race scenario)</li>
+     *   <li>{@code currentBind=real, target=real} → no-op (steady state)</li>
+     *   <li>{@code currentBind=*, target=EMPTY} → no-op (apply pas encore tourné)</li>
+     * </ul>
+     *
+     * <h3>Pourquoi on se limite à IDLE</h3>
      * <p>L'ordre EF ajoute ~25 motions différentes (WALK, RUN, SNEAK, SIT,
      * SLEEP, etc.). On se limite à IDLE ici — ajouter les autres sans anim
      * source = pollution registre pour rien. Au fur et à mesure que les JSON
@@ -291,6 +323,7 @@ public abstract class PlayerPatch<T extends Player> extends LivingEntityPatch<T>
         // Full data-driven : lookup par ID, EMPTY_ANIMATION fallback si
         // l'asset n'est pas encore chargé (rare mais possible au bootstrap).
         // Le tick handler self-heal le bind une fois le datapack loadé.
+        // Voir javadoc ci-dessus pour le contrat complet (RISK-003).
         AnimationAccessor<? extends StaticAnimation> idle =
             TiedUpAnimationRegistry.resolveWithFallback(
                 TiedUpAnimationRegistry.CONTEXT_STAND_IDLE_ID

src/main/java/com/tiedup/remake/rig/tick/RigAnimationTickHandler.java

@@ -261,13 +261,62 @@ public final class RigAnimationTickHandler {
         // EMPTY_ANIMATION. Dans les deux cas on rebind vers le target résolu.
         AssetAccessor<? extends StaticAnimation> currentIdleBind =
             clientAnimator.getLivingAnimation(LivingMotions.IDLE, null);
-        if (currentIdleBind == null || currentIdleBind == TiedUpRigRegistry.EMPTY_ANIMATION) {
+        if (shouldRebindIdle(currentIdleBind, target)) {
             clientAnimator.addLivingAnimation(LivingMotions.IDLE, target);
         }
 
         clientAnimator.playAnimation(target, 0.2F);
     }
 
+    /**
+     * Pure helper — décide si le bind IDLE courant doit être remplacé par
+     * {@code target}. Extrait pour tests unitaires sans bootstrap MC (cf.
+     * {@code RigAnimationTickHandlerTest.shouldRebindIdle_*}).
+     *
+     * <p><b>Contrat</b> : retourne {@code true} ssi {@code target} est une
+     * vraie anim (pas {@link TiedUpRigRegistry#EMPTY_ANIMATION}) ET que le
+     * bind courant est soit absent ({@code null}), soit la sentinelle EMPTY.
+     * Dans tous les autres cas ({@code target} non-EMPTY identique au bind
+     * courant, ou {@code target} EMPTY) on garde le bind existant.</p>
+     *
+     * <p>L'appelant ({@link #maybePlayIdle}) a déjà filtré
+     * {@code target == EMPTY_ANIMATION} en early-return — la condition
+     * {@code target != EMPTY} ici est défensive (l'helper reste correct si
+     * jamais le filtre upstream est modifié).</p>
+     *
+     * <p><b>Race scenario couvert</b> (RISK-003) :</p>
+     * <ol>
+     *   <li>{@code PlayerPatch.initAnimator} construit pre-{@code apply()} :
+     *       {@code resolveWithFallback} retourne EMPTY → {@code addLivingAnimation}
+     *       met EMPTY dans la map (car {@code EMPTY_ANIMATION.isPresent()==true}
+     *       — cf. {@link com.tiedup.remake.rig.anim.types.DirectStaticAnimation#isPresent}).</li>
+     *   <li>{@code AnimationManager.apply()} tourne plus tard (datapack reload
+     *       wiring Phase 2). Le registry contient maintenant un nouvel accessor
+     *       pour {@code context_stand_idle}.</li>
+     *   <li>Tick suivant : {@code maybePlayIdle} résout {@code target = real
+     *       accessor}, voit {@code currentIdleBind == EMPTY}, rebind via cet
+     *       helper.</li>
+     * </ol>
+     *
+     * @param currentBind bind IDLE courant (peut être {@code null} si jamais
+     *                    bound, ou {@link TiedUpRigRegistry#EMPTY_ANIMATION} si
+     *                    bound pré-apply)
+     * @param target      target résolu via {@link TiedUpAnimationRegistry#resolveWithFallback}
+     *                    (par contrat upstream, jamais {@code null})
+     * @return {@code true} si rebind nécessaire, {@code false} sinon
+     */
+    static boolean shouldRebindIdle(
+        AssetAccessor<? extends StaticAnimation> currentBind,
+        AssetAccessor<? extends StaticAnimation> target
+    ) {
+        // Garde défensive : un target EMPTY ne doit jamais écraser un bind
+        // existant (régresserait l'état). En pratique l'appelant filtre déjà.
+        if (target == TiedUpRigRegistry.EMPTY_ANIMATION) {
+            return false;
+        }
+        return currentBind == null || currentBind == TiedUpRigRegistry.EMPTY_ANIMATION;
+    }
+
     /**
      * Reset des erreurs loggées. Appelé :
      * <ul>

src/test/java/com/tiedup/remake/rig/tick/RigAnimationTickHandlerTest.java

@@ -5,10 +5,17 @@
 package com.tiedup.remake.rig.tick;
 
 import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertTrue;
 
 import org.junit.jupiter.api.AfterEach;
 import org.junit.jupiter.api.Test;
 
+import com.tiedup.remake.rig.TiedUpRigRegistry;
+import com.tiedup.remake.rig.anim.AnimationManager;
+import com.tiedup.remake.rig.anim.AnimationManager.AnimationAccessor;
+import com.tiedup.remake.rig.anim.types.StaticAnimation;
+
 /**
  * Tests de {@link RigAnimationTickHandler} — uniquement la logique pure Java
  * accessible sans MC runtime.
@@ -18,12 +25,14 @@ import org.junit.jupiter.api.Test;
  *   impossible sans bootstrap Forge.
  * - tickPlayer : necessite TiedUpCapabilities.getEntityPatch() qui utilise
  *   LazyOptional (Forge runtime) + un Player reel.
- * - maybePlayIdle : necessite ClientAnimator + LivingEntityPatch (Forge
+ * - maybePlayIdle (full path) : necessite ClientAnimator + LivingEntityPatch
- *   capability + MC class hierarchy).
+ *   (Forge capability + MC class hierarchy).
  *
  * Ce qui est couvert :
  * - resetLoggedErrors() : expose explicitement pour les tests (Javadoc l'indique).
  *   Verifie que la methode est idempotente et ne throw pas.
+ * - shouldRebindIdle() : helper pur extrait pour tester la logique de self-heal
+ *   du bind IDLE post-{@code AnimationManager.apply()} (RISK-003).
  */
 class RigAnimationTickHandlerTest {
 
@@ -65,4 +74,166 @@ class RigAnimationTickHandlerTest {
             RigAnimationTickHandler.resetLoggedErrors();
         }, "Second reset doit rester idempotent (simulation F3+T double-reload)");
     }
+
+    // === shouldRebindIdle — RISK-003 self-heal logic ============================
+    //
+    // Verrouille le contrat de self-heal du bind IDLE : si initAnimator a bound
+    // EMPTY (apply pas encore tourné), le tick handler doit rebind sur la vraie
+    // anim une fois le registry populé. Voir PlayerPatch.initAnimator javadoc
+    // pour le scenario complet.
+    //
+    // Tests sans MC bootstrap : on injecte des accessors fakes via
+    // AnimationManager.AnimationBuilder (n'invoque pas onLoad tant qu'on ne
+    // call pas .get()).
+
+    /** Namespace arbitraire pour les IDs de test — isole des vraies registrations. */
+    private static final String TEST_NS = "tiedup_test_rebind";
+
+    /**
+     * Cas 1/4 — currentBind=null, target=real → rebind nécessaire.
+     *
+     * <p>Représente le cas "patch fraîchement construit, jamais bound IDLE
+     * encore" (théorique : initAnimator skip le bind, scénario alternatif).
+     * Le rebind doit fire pour amorcer la map.</p>
+     */
+    @Test
+    void shouldRebindIdle_nullCurrent_realTarget_returnsTrue() {
+        AnimationAccessor<StaticAnimation> realTarget = registerFakeAccessor("real_anim_1");
+
+        boolean result = RigAnimationTickHandler.shouldRebindIdle(null, realTarget);
+
+        assertTrue(result, "currentBind=null + target=real doit déclencher rebind "
+            + "(amorce la map, sinon postInit aurait NPE — defensive case).");
+    }
+
+    /**
+     * Cas 2/4 — currentBind=EMPTY, target=real → rebind (RACE SCENARIO RISK-003).
+     *
+     * <p>C'est LE scenario que RISK-003 décrit :</p>
+     * <ol>
+     *   <li>{@code PlayerPatch.initAnimator} fire pre-{@code apply} → bind EMPTY.</li>
+     *   <li>{@code AnimationManager.apply()} tourne → registry contient real anim.</li>
+     *   <li>Tick suivant : {@code maybePlayIdle} résout {@code target=real},
+     *       voit {@code currentIdleBind=EMPTY}, doit rebind.</li>
+     * </ol>
+     *
+     * <p>Si ce test échoue, le self-heal est cassé et l'idle reste silent
+     * (symptôme F6 overlay {@code bindings: 1} avec EMPTY).</p>
+     */
+    @Test
+    void shouldRebindIdle_emptyCurrent_realTarget_returnsTrue() {
+        AnimationAccessor<StaticAnimation> realTarget = registerFakeAccessor("real_anim_2");
+
+        boolean result = RigAnimationTickHandler.shouldRebindIdle(
+            TiedUpRigRegistry.EMPTY_ANIMATION,
+            realTarget
+        );
+
+        assertTrue(result, "RISK-003 : currentBind=EMPTY + target=real DOIT rebind. "
+            + "Régression si le self-heal post-AnimationManager.apply() ne triggere "
+            + "plus → idle silent à vie après bootstrap pre-reload.");
+    }
+
+    /**
+     * Cas 3/4 — currentBind=real, target=real (même instance) → no-op.
+     *
+     * <p>Steady state : le bind est correct, pas de rebind à faire.
+     * Idempotence du tick handler.</p>
+     */
+    @Test
+    void shouldRebindIdle_realCurrent_realTarget_returnsFalse() {
+        AnimationAccessor<StaticAnimation> realAnim = registerFakeAccessor("real_anim_3");
+
+        boolean result = RigAnimationTickHandler.shouldRebindIdle(realAnim, realAnim);
+
+        assertFalse(result, "Steady state (current=target=real) ne doit pas rebind "
+            + "— sinon spam d'addLivingAnimation à chaque tick (perf hit + log noise).");
+    }
+
+    /**
+     * Cas 3 bis — currentBind=real (instance A), target=real (instance B, même ID).
+     *
+     * <p>Post-reload : l'accessor a une nouvelle instance (cf.
+     * {@code AnimationAccessorImpl} créé fresh dans
+     * {@code AnimationManager.readResourcepackAnimation}). On considère ça
+     * comme steady state — l'helper compare par identité (pas registryName)
+     * pour rester simple, et le caller gère le re-binding via la check
+     * {@code currentAnim.registryName().equals(targetId)} à un autre niveau.</p>
+     *
+     * <p>Cas important pour comprendre la portée de ce helper : il NE résout
+     * PAS le cas "post-reload, ancienne instance toujours bound mais registry
+     * a une nouvelle instance avec même ID". Cette resolution est handled par
+     * la check précédente dans {@code maybePlayIdle} qui compare via
+     * {@code registryName()}.</p>
+     */
+    @Test
+    void shouldRebindIdle_differentInstancesSameId_returnsTrue() {
+        // Deux instances distinctes mais avec le MÊME registry name.
+        // Simule "pré-reload bind" vs "post-reload re-instantiation" du même asset.
+        AnimationAccessor<StaticAnimation> oldInstance = registerFakeAccessor("realod_test");
+        AnimationAccessor<StaticAnimation> newInstance = registerFakeAccessor("realod_test_new");
+        // Note : on ne peut pas avoir 2 accessors avec le même path via
+        // AnimationBuilder (nextAccessor nomme via path donné). On simule juste
+        // "deux instances réelles distinctes" = bind différent, helper rebind.
+
+        boolean result = RigAnimationTickHandler.shouldRebindIdle(oldInstance, newInstance);
+
+        // shouldRebindIdle compare par identité de référence — un current real
+        // != target real returns false (ne fait pas de smart "same registryName"
+        // check). La désynchro registry-name post-reload est gérée par
+        // maybePlayIdle via une check upstream sur registryName().equals(targetId).
+        assertFalse(result, "shouldRebindIdle compare par identité — current=real !=null "
+            + "ET target=real → on ne rebind pas même si instances diffèrent. "
+            + "La détection post-reload se fait upstream (registryName check).");
+    }
+
+    /**
+     * Cas 4/4 — currentBind=*, target=EMPTY → no-op (jamais écraser un bind par EMPTY).
+     *
+     * <p>Garde défensive : si {@code AnimationManager.apply()} n'a pas encore
+     * tourné, {@code resolveWithFallback} retourne EMPTY. Dans ce cas le
+     * caller ({@code maybePlayIdle}) a déjà early-returned, mais l'helper
+     * reste safe si jamais le filtre upstream est modifié — un EMPTY target
+     * ne doit jamais régresser un bind real existant.</p>
+     */
+    @Test
+    void shouldRebindIdle_emptyTarget_returnsFalse() {
+        AnimationAccessor<StaticAnimation> realCurrent = registerFakeAccessor("real_anim_5");
+
+        // Avec current=real et target=EMPTY : pas de rebind.
+        boolean withReal = RigAnimationTickHandler.shouldRebindIdle(
+            realCurrent, TiedUpRigRegistry.EMPTY_ANIMATION
+        );
+        assertFalse(withReal, "target=EMPTY ne doit jamais écraser un bind real "
+            + "(régresserait l'état). Defensive guard contre futur bug upstream.");
+
+        // Avec current=null et target=EMPTY : pas de rebind non plus (rien à
+        // faire de productif — laisse postInit gérer le bootstrap initial).
+        boolean withNull = RigAnimationTickHandler.shouldRebindIdle(
+            null, TiedUpRigRegistry.EMPTY_ANIMATION
+        );
+        assertFalse(withNull, "target=EMPTY + current=null : pas de rebind, "
+            + "rien à amorcer car EMPTY n'apporte aucune info utile.");
+
+        // Cas dégénéré : current=EMPTY, target=EMPTY — aucune progression possible.
+        boolean withEmpty = RigAnimationTickHandler.shouldRebindIdle(
+            TiedUpRigRegistry.EMPTY_ANIMATION, TiedUpRigRegistry.EMPTY_ANIMATION
+        );
+        assertFalse(withEmpty, "current=EMPTY + target=EMPTY : registry pas encore "
+            + "populé, attendre la prochaine reload — pas de rebind.");
+    }
+
+    /**
+     * Inject un accessor fake dans {@link AnimationManager} via le builder
+     * public. {@code onLoad} retourne null mais ce n'est pas invoqué tant
+     * qu'on ne call pas {@code .get()} dessus dans les tests (on compare
+     * uniquement par identité de référence).
+     */
+    private static AnimationAccessor<StaticAnimation> registerFakeAccessor(String idPath) {
+        // Suffix nano pour éviter les collisions entre test cases (le builder
+        // jette IllegalArgumentException si le namespace est déjà utilisé).
+        String uniqueNs = TEST_NS + "_" + System.nanoTime();
+        return new AnimationManager.AnimationBuilder(uniqueNs, b -> {})
+            .nextAccessor(idPath, acc -> null);
+    }
 }