NEO-93: Address code review — AAA tests, single-call advance, windup fix.

Co-authored-by: Cursor <cursoragent@cursor.com>

server/NeonSprawl.Server.Tests/Game/Npc/NpcRuntimeOperationsTests.cs

@@ -35,8 +35,8 @@ public sealed class NpcRuntimeOperationsTests
         var (runtime, threat, behavior) = CreateFixture();
         // Act
         Advance(T0, runtime, threat, behavior);
-        runtime.TryGet(PrototypeNpcRegistry.PrototypeNpcMeleeId, out var snapshot);
         // Assert
+        runtime.TryGet(PrototypeNpcRegistry.PrototypeNpcMeleeId, out var snapshot);
         Assert.Equal(NpcBehaviorState.Idle, snapshot.BehaviorState);
         Assert.Null(snapshot.ActiveTelegraph);
     }
@@ -49,8 +49,8 @@ public sealed class NpcRuntimeOperationsTests
         SetHolder(threat, PrototypeNpcRegistry.PrototypeNpcMeleeId);
         // Act
         Advance(T0, runtime, threat, behavior);
-        runtime.TryGet(PrototypeNpcRegistry.PrototypeNpcMeleeId, out var snapshot);
         // Assert
+        runtime.TryGet(PrototypeNpcRegistry.PrototypeNpcMeleeId, out var snapshot);
         Assert.Equal(NpcBehaviorState.Aggro, snapshot.BehaviorState);
         Assert.Equal(T0, snapshot.PhaseStartedUtc);
         Assert.Null(snapshot.ActiveTelegraph);
@@ -64,8 +64,8 @@ public sealed class NpcRuntimeOperationsTests
         runtime.LastAdvancedUtc = T0;
         // Act
         Advance(T0.AddSeconds(10), runtime, threat, behavior);
-        runtime.TryGet(PrototypeNpcRegistry.PrototypeNpcMeleeId, out var snapshot);
         // Assert
+        runtime.TryGet(PrototypeNpcRegistry.PrototypeNpcMeleeId, out var snapshot);
         Assert.Equal(NpcBehaviorState.Idle, snapshot.BehaviorState);
         Assert.Null(snapshot.ActiveTelegraph);
     }
@@ -79,8 +79,8 @@ public sealed class NpcRuntimeOperationsTests
         Advance(T0, runtime, threat, behavior);
         // Act
         Advance(T0.AddSeconds(3), runtime, threat, behavior);
-        runtime.TryGet(PrototypeNpcRegistry.PrototypeNpcMeleeId, out var snapshot);
         // Assert
+        runtime.TryGet(PrototypeNpcRegistry.PrototypeNpcMeleeId, out var snapshot);
         Assert.Equal(NpcBehaviorState.TelegraphWindup, snapshot.BehaviorState);
         Assert.Equal(T0.AddSeconds(3), snapshot.PhaseStartedUtc);
         Assert.NotNull(snapshot.ActiveTelegraph);
@@ -95,13 +95,13 @@ public sealed class NpcRuntimeOperationsTests
         Advance(T0, runtime, threat, behavior);
         // Act
         Advance(T0.AddSeconds(2.9), runtime, threat, behavior);
-        runtime.TryGet(PrototypeNpcRegistry.PrototypeNpcMeleeId, out var snapshot);
         // Assert
+        runtime.TryGet(PrototypeNpcRegistry.PrototypeNpcMeleeId, out var snapshot);
         Assert.Equal(NpcBehaviorState.Aggro, snapshot.BehaviorState);
     }
 
     [Fact]
-    public void AdvanceAll_ShouldCompleteMeleeWindupDuration_BeforeAttackExecute()
+    public void AdvanceAll_ShouldRemainTelegraphWindup_BeforeMeleeWindupCompletes()
     {
         // Arrange
         var (runtime, threat, behavior) = CreateFixture();
@@ -110,14 +110,27 @@ public sealed class NpcRuntimeOperationsTests
         Advance(T0.AddSeconds(3), runtime, threat, behavior);
         // Act
         Advance(T0.AddSeconds(4.4), runtime, threat, behavior);
-        runtime.TryGet(PrototypeNpcRegistry.PrototypeNpcMeleeId, out var duringWindup);
-        Advance(T0.AddSeconds(4.5), runtime, threat, behavior);
-        runtime.TryGet(PrototypeNpcRegistry.PrototypeNpcMeleeId, out var afterWindup);
         // Assert
-        Assert.Equal(NpcBehaviorState.TelegraphWindup, duringWindup.BehaviorState);
-        Assert.Equal(NpcBehaviorState.Recover, afterWindup.BehaviorState);
-        Assert.Equal(T0.AddSeconds(4.5), afterWindup.PhaseStartedUtc);
-        Assert.Null(afterWindup.ActiveTelegraph);
+        runtime.TryGet(PrototypeNpcRegistry.PrototypeNpcMeleeId, out var snapshot);
+        Assert.Equal(NpcBehaviorState.TelegraphWindup, snapshot.BehaviorState);
+    }
+
+    [Fact]
+    public void AdvanceAll_ShouldEnterRecover_WhenMeleeWindupCompletes()
+    {
+        // Arrange
+        var (runtime, threat, behavior) = CreateFixture();
+        SetHolder(threat, PrototypeNpcRegistry.PrototypeNpcMeleeId);
+        Advance(T0, runtime, threat, behavior);
+        Advance(T0.AddSeconds(3), runtime, threat, behavior);
+        Advance(T0.AddSeconds(4.4), runtime, threat, behavior);
+        // Act
+        Advance(T0.AddSeconds(4.5), runtime, threat, behavior);
+        // Assert
+        runtime.TryGet(PrototypeNpcRegistry.PrototypeNpcMeleeId, out var snapshot);
+        Assert.Equal(NpcBehaviorState.Recover, snapshot.BehaviorState);
+        Assert.Equal(T0.AddSeconds(4.5), snapshot.PhaseStartedUtc);
+        Assert.Null(snapshot.ActiveTelegraph);
     }
 
     [Fact]
@@ -131,8 +144,24 @@ public sealed class NpcRuntimeOperationsTests
         Advance(T0.AddSeconds(4.5), runtime, threat, behavior);
         // Act
         Advance(T0.AddSeconds(7.5), runtime, threat, behavior);
-        runtime.TryGet(PrototypeNpcRegistry.PrototypeNpcMeleeId, out var snapshot);
         // Assert
+        runtime.TryGet(PrototypeNpcRegistry.PrototypeNpcMeleeId, out var snapshot);
+        Assert.Equal(NpcBehaviorState.TelegraphWindup, snapshot.BehaviorState);
+        Assert.Equal(T0.AddSeconds(7.5), snapshot.PhaseStartedUtc);
+        Assert.NotNull(snapshot.ActiveTelegraph);
+    }
+
+    [Fact]
+    public void AdvanceAll_ShouldRunFullMeleeCycle_ToSecondTelegraph_InSingleAdvanceCall()
+    {
+        // Arrange
+        var (runtime, threat, behavior) = CreateFixture();
+        SetHolder(threat, PrototypeNpcRegistry.PrototypeNpcMeleeId);
+        Advance(T0, runtime, threat, behavior);
+        // Act
+        Advance(T0.AddSeconds(7.5), runtime, threat, behavior, maxDeltaSeconds: 10);
+        // Assert
+        runtime.TryGet(PrototypeNpcRegistry.PrototypeNpcMeleeId, out var snapshot);
         Assert.Equal(NpcBehaviorState.TelegraphWindup, snapshot.BehaviorState);
         Assert.Equal(T0.AddSeconds(7.5), snapshot.PhaseStartedUtc);
         Assert.NotNull(snapshot.ActiveTelegraph);
@@ -149,16 +178,35 @@ public sealed class NpcRuntimeOperationsTests
         _ = threat.TryClearHolder(PrototypeNpcRegistry.PrototypeNpcMeleeId);
         // Act
         Advance(T0.AddSeconds(3.5), runtime, threat, behavior);
-        runtime.TryGet(PrototypeNpcRegistry.PrototypeNpcMeleeId, out var snapshot);
         // Assert
+        runtime.TryGet(PrototypeNpcRegistry.PrototypeNpcMeleeId, out var snapshot);
         Assert.Equal(NpcBehaviorState.Idle, snapshot.BehaviorState);
         Assert.Null(snapshot.ActiveTelegraph);
     }
 
+    [Theory]
+    [InlineData("prototype_npc_ranged", 4.0)]
+    [InlineData("prototype_npc_elite", 5.0)]
+    public void AdvanceAll_ShouldEnterTelegraph_AtCatalogCooldownForArchetype(
+        string npcInstanceId,
+        double attackCooldownSeconds)
+    {
+        // Arrange
+        var (runtime, threat, behavior) = CreateFixture();
+        SetHolder(threat, npcInstanceId);
+        Advance(T0, runtime, threat, behavior);
+        // Act
+        Advance(T0.AddSeconds(attackCooldownSeconds), runtime, threat, behavior);
+        // Assert
+        runtime.TryGet(npcInstanceId, out var snapshot);
+        Assert.Equal(NpcBehaviorState.TelegraphWindup, snapshot.BehaviorState);
+        Assert.Equal(T0.AddSeconds(attackCooldownSeconds), snapshot.PhaseStartedUtc);
+    }
+
     [Theory]
     [InlineData("prototype_npc_ranged", 4.0, 2.0)]
     [InlineData("prototype_npc_elite", 5.0, 2.5)]
-    public void AdvanceAll_ShouldUseCatalogTimings_ForEachArchetype(
+    public void AdvanceAll_ShouldEnterRecover_AfterCatalogWindupForArchetype(
         string npcInstanceId,
         double attackCooldownSeconds,
         double telegraphWindupSeconds)
@@ -167,22 +215,15 @@ public sealed class NpcRuntimeOperationsTests
         var (runtime, threat, behavior) = CreateFixture();
         SetHolder(threat, npcInstanceId);
         Advance(T0, runtime, threat, behavior);
-        // Act
         Advance(T0.AddSeconds(attackCooldownSeconds), runtime, threat, behavior);
-        runtime.TryGet(npcInstanceId, out var telegraphSnapshot);
-        Advance(
-            T0.AddSeconds(attackCooldownSeconds + telegraphWindupSeconds),
-            runtime,
-            threat,
-            behavior);
-        runtime.TryGet(npcInstanceId, out var recoverSnapshot);
+        // Act
+        Advance(T0.AddSeconds(attackCooldownSeconds + telegraphWindupSeconds), runtime, threat, behavior);
         // Assert
-        Assert.Equal(NpcBehaviorState.TelegraphWindup, telegraphSnapshot.BehaviorState);
-        Assert.Equal(T0.AddSeconds(attackCooldownSeconds), telegraphSnapshot.PhaseStartedUtc);
-        Assert.Equal(NpcBehaviorState.Recover, recoverSnapshot.BehaviorState);
+        runtime.TryGet(npcInstanceId, out var snapshot);
+        Assert.Equal(NpcBehaviorState.Recover, snapshot.BehaviorState);
         Assert.Equal(
             T0.AddSeconds(attackCooldownSeconds + telegraphWindupSeconds),
-            recoverSnapshot.PhaseStartedUtc);
+            snapshot.PhaseStartedUtc);
     }
 
     [Fact]
@@ -192,10 +233,10 @@ public sealed class NpcRuntimeOperationsTests
         var (runtime, threat, behavior) = CreateFixture();
         SetHolder(threat, PrototypeNpcRegistry.PrototypeNpcMeleeId);
         Advance(T0, runtime, threat, behavior);
-        // Act — 10s gap with 5s cap should not reach second telegraph (needs 7.5s from aggro start)
+        // Act
         Advance(T0.AddSeconds(10), runtime, threat, behavior, maxDeltaSeconds: 5);
-        runtime.TryGet(PrototypeNpcRegistry.PrototypeNpcMeleeId, out var snapshot);
         // Assert
+        runtime.TryGet(PrototypeNpcRegistry.PrototypeNpcMeleeId, out var snapshot);
         Assert.Equal(NpcBehaviorState.Recover, snapshot.BehaviorState);
         Assert.Equal(T0.AddSeconds(4.5), snapshot.PhaseStartedUtc);
     }

server/NeonSprawl.Server/Game/Npc/NpcRuntimeOperations.cs

@@ -171,19 +171,13 @@ public static class NpcRuntimeOperations
                     }
 
                     cursor = phaseEnd;
-                    WriteState(
-                        runtimeStore,
-                        npcInstanceId,
-                        NpcBehaviorState.AttackExecute,
-                        cursor,
-                        activeTelegraph: null);
                     WriteState(
                         runtimeStore,
                         npcInstanceId,
                         NpcBehaviorState.Recover,
                         cursor,
                         activeTelegraph: null);
-                    // telemetry: telegraph_fired / npc_state_transition (NEO-96); NEO-95 applies attackDamage at windup complete
+                    // telemetry: telegraph_fired / npc_state_transition (NEO-96); logical attack_execute → recover (NEO-95 applies attackDamage)
                     continue;
                 }