From c360d639f2c43774da78e01cb49a28b8d2ca38b1 Mon Sep 17 00:00:00 2001
From: gamer147 <gamer147@gmail.com>
Date: Wed, 3 Jun 2026 18:26:07 -0400
Subject: [PATCH] refactor(battle-node): address final-review minor notes
 (comments + test backfill)

- PlayActionsHandler doc: drop the phantom 'with a debug log' (handlers are
  stateless singletons with no logger); say token plays degrade silently.
- KnownListBuilder.ExtractMoveTo doc: note first-match-wins semantics and the
  send-side==recv-side 'to' assumption pending recv-capture confirmation.
- KnownListBuilderTests: add multi-move first-match coverage and the
  in-deck-but-no-matching-move null branch for BuildPlayedCard.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
---
 .../Dispatch/Handlers/PlayActionsHandler.cs   |  4 +--
 .../Sessions/Dispatch/KnownListBuilder.cs     |  7 ++--
 .../Sessions/KnownListBuilderTests.cs         | 35 +++++++++++++++++++
 3 files changed, 42 insertions(+), 4 deletions(-)

diff --git a/SVSim.BattleNode/Sessions/Dispatch/Handlers/PlayActionsHandler.cs b/SVSim.BattleNode/Sessions/Dispatch/Handlers/PlayActionsHandler.cs
index 9f22097..6d76df4 100644
--- a/SVSim.BattleNode/Sessions/Dispatch/Handlers/PlayActionsHandler.cs
+++ b/SVSim.BattleNode/Sessions/Dispatch/Handlers/PlayActionsHandler.cs
@@ -5,8 +5,8 @@ namespace SVSim.BattleNode.Sessions.Dispatch.Handlers;
 
 /// <summary>PvP PlayActions translator (vanilla deck-card slice). Synthesizes the opponent-facing
 /// knownList from the sender's idx->cardId map + the orderList move op, renames targetList ->
-/// oppoTargetList, drops orderList, consumes keyAction. Token plays (idx>deck) degrade to
-/// {playIdx,type} with a debug log. Scripted/Bot drop (no rule).</summary>
+/// oppoTargetList, drops orderList, consumes keyAction. Token plays (idx>deck) degrade silently to
+/// {playIdx,type} (no knownList). Scripted/Bot drop (no rule).</summary>
 internal sealed class PlayActionsHandler : IFrameHandler
 {
     public IReadOnlyList<DispatchRoute> Handle(FrameDispatchContext ctx)
diff --git a/SVSim.BattleNode/Sessions/Dispatch/KnownListBuilder.cs b/SVSim.BattleNode/Sessions/Dispatch/KnownListBuilder.cs
index 7735d8f..c0e3084 100644
--- a/SVSim.BattleNode/Sessions/Dispatch/KnownListBuilder.cs
+++ b/SVSim.BattleNode/Sessions/Dispatch/KnownListBuilder.cs
@@ -20,8 +20,11 @@ internal static class KnownListBuilder
         return new KnownCardEntry(Idx: playIdx, CardId: cardId, To: to.Value, Spellboost: 0, AttachTarget: "");
     }
 
-    /// <summary>The <c>to</c> place-state of the <c>move</c> op whose <c>idx</c> list contains
-    /// <paramref name="playIdx"/>, or null if absent.</summary>
+    /// <summary>The <c>to</c> place-state of the FIRST <c>move</c> op whose <c>idx</c> list contains
+    /// <paramref name="playIdx"/> (the played card's own move; later add/alter ops are the deferred
+    /// token slice), or null if absent. NOTE: the sender-side <c>to</c> is passed through verbatim —
+    /// for the vanilla slice we assume send-side and recv-side place-state codes match, pending
+    /// recv-capture confirmation.</summary>
     public static int? ExtractMoveTo(object? orderList, int playIdx)
     {
         if (orderList is not IEnumerable<object?> ops) return null;
diff --git a/SVSim.UnitTests/BattleNode/Sessions/KnownListBuilderTests.cs b/SVSim.UnitTests/BattleNode/Sessions/KnownListBuilderTests.cs
index 796eea2..d983c6a 100644
--- a/SVSim.UnitTests/BattleNode/Sessions/KnownListBuilderTests.cs
+++ b/SVSim.UnitTests/BattleNode/Sessions/KnownListBuilderTests.cs
@@ -33,6 +33,41 @@ public class KnownListBuilderTests
         Assert.That(KnownListBuilder.ExtractMoveTo(null, playIdx: 3), Is.Null);
     }
 
+    [Test]
+    public void ExtractMoveTo_returns_first_matching_move_op()
+    {
+        // A real PlayActions can carry several move ops; the played card's move comes first,
+        // later ops (token add/alter) target other idxs. Confirm first-match-wins, not last.
+        var orderList = new List<object?>
+        {
+            new Dictionary<string, object?>
+            {
+                ["move"] = new Dictionary<string, object?>
+                {
+                    ["idx"] = new List<object?> { 3L }, ["isSelf"] = 1L, ["from"] = 10L, ["to"] = 30L,
+                }
+            },
+            new Dictionary<string, object?>
+            {
+                ["move"] = new Dictionary<string, object?>
+                {
+                    ["idx"] = new List<object?> { 31L, 32L }, ["isSelf"] = 1L, ["from"] = 0L, ["to"] = 40L,
+                }
+            },
+        };
+        Assert.That(KnownListBuilder.ExtractMoveTo(orderList, playIdx: 3), Is.EqualTo(30));
+        Assert.That(KnownListBuilder.ExtractMoveTo(orderList, playIdx: 31), Is.EqualTo(40));
+    }
+
+    [Test]
+    public void BuildPlayedCard_returns_null_for_deck_card_with_no_matching_move_op()
+    {
+        // idx is in the deck, but the orderList has no move op for it → can't synthesize.
+        var deckMap = new Dictionary<int, long> { [3] = 128821011L };
+        var entry = KnownListBuilder.BuildPlayedCard(deckMap, playIdx: 3, orderList: OrderListMove(7, 10, 20));
+        Assert.That(entry, Is.Null);
+    }
+
     [Test]
     public void BuildPlayedCard_synthesizes_entry_for_deck_card()
     {