NEO-109: Address code review hook-anchor findings.

Move encounter_start comments after successful TryActivate; clarify
encounter_complete_denied caller context; strike through review items.

docs/plans/NEO-109-implementation-plan.md

@@ -51,7 +51,7 @@
 
 ## Implementation reconciliation (shipped)
 
-- **`encounter_start`:** full comment block in **`EncounterProgressOperations.TryActivateOnFirstEngagement`**; NEO-84 **`CombatOperations`** reserved pointer updated.
+- **`encounter_start`:** full comment block in **`EncounterProgressOperations.TryActivateOnFirstEngagement`** after successful **`TryActivate`** (anchor matches once-per-activation emit); NEO-84 **`CombatOperations`** reserved pointer updated.
 - **`encounter_complete`** + **`reward_attribution`:** full comment blocks in **`EncounterCompletionOperations.TryCompleteAndGrant`** after event record; duplicate removed from **`EncounterCombatWiring`** (delegate pointer only).
 - **`encounter_complete_denied`:** retained on **`Deny`**.
 - **Docs:** `server/README.md` encounter telemetry subsection; E5.M3 module snapshot; alignment register; module dependency register; E5M3-10 backlog landed note.

docs/reviews/2026-05-31-NEO-109.md

@@ -31,13 +31,13 @@ None.
 
 ## Suggestions
 
-1. **`encounter_start` hook anchor vs emit condition** — The comment block sits **before** `progressStore.TryActivate`, but the TODO correctly says emit **when `TryActivate` returns true**. On re-engagement after activation, execution still passes the comment then `TryActivate` returns `false`. For E9.M1 wiring clarity, consider moving the block to immediately **after** a successful `TryActivate` (or wrapping with `if (progressStore.TryActivate(...)) { /* hook */ return true; }`) so the anchor matches the once-per-activation contract. Comments-only today; non-blocking.
+1. ~~**`encounter_start` hook anchor vs emit condition** — The comment block sits **before** `progressStore.TryActivate`, but the TODO correctly says emit **when `TryActivate` returns true**. On re-engagement after activation, execution still passes the comment then `TryActivate` returns `false`. For E9.M1 wiring clarity, consider moving the block to immediately **after** a successful `TryActivate` (or wrapping with `if (progressStore.TryActivate(...)) { /* hook */ return true; }`) so the anchor matches the once-per-activation contract. Comments-only today; non-blocking.~~ **Done.** Hook block moved after successful **`TryActivate`**; method returns **`true`** on first activation (same outward behavior).
 
-2. **`encounter_complete_denied` payload scope** — The **`Deny`** hook comment lists `playerId`, `encounterId`, `reasonCode`, but **`Deny(string reasonCode)`** has no player/encounter context. Pre-existing from NEO-105; E9.M1 will need call-site context or a signature change. Worth a one-line note in the comment (“wire at caller with playerId + encounterId”) to avoid implementer confusion.
+2. ~~**`encounter_complete_denied` payload scope** — The **`Deny`** hook comment lists `playerId`, `encounterId`, `reasonCode`, but **`Deny(string reasonCode)`** has no player/encounter context. Pre-existing from NEO-105; E9.M1 will need call-site context or a signature change. Worth a one-line note in the comment (“wire at caller with playerId + encounterId”) to avoid implementer confusion.~~ **Done.** Comment notes wire at each **`TryCompleteAndGrant`** **`Deny()`** call site.
 
 ## Nits
 
-- Nit: Plan lists optional **`requiredNpcInstanceIds` count** in **`encounter_start`** payload; the comment block omits it. Fine for prototype; add when E9.M1 shapes the catalog schema.
+- ~~Nit: Plan lists optional **`requiredNpcInstanceIds` count** in **`encounter_start`** payload; the comment block omits it. Fine for prototype; add when E9.M1 shapes the catalog schema.~~ **Done.** Payload comment includes **`requiredNpcInstanceIdsCount`**.
 
 - Nit: **`EncounterCombatWiring`** removed `return;` after the deny log block — behavior unchanged (method ends immediately after the delegate comment). Harmless; no action needed.
 

server/NeonSprawl.Server/Game/Encounters/EncounterCompletionOperations.cs

@@ -178,7 +178,8 @@ public static class EncounterCompletionOperations
     {
         // --- Telemetry hook site (NEO-109): future E9.M1 catalog event `encounter_complete_denied` ---
         // TODO(E9.M1): catalog emit — structured deny with reasonCode (EncounterCompletionReasonCodes).
-        // Planned payload: playerId, encounterId, reasonCode. No ingest or ILogger here (comments-only).
+        // Planned payload: playerId, encounterId, reasonCode — wire at each TryCompleteAndGrant Deny() call site
+        // (this helper has reasonCode only; callers hold playerId + encounterId). No ingest or ILogger here.
 
         return new EncounterCompletionResult(
             Success: false,

server/NeonSprawl.Server/Game/Encounters/EncounterProgressOperations.cs

@@ -53,7 +53,7 @@ public static class EncounterProgressOperations
         IEncounterProgressStore progressStore,
         IEncounterCompletionStore completionStore)
     {
-        if (!TryResolveEncounterForNpc(npcInstanceId, encounterRegistry, out var encounterId, out _))
+        if (!TryResolveEncounterForNpc(npcInstanceId, encounterRegistry, out var encounterId, out var definition))
         {
             return false;
         }
@@ -63,12 +63,18 @@ public static class EncounterProgressOperations
             return false;
         }
 
+        if (!progressStore.TryActivate(playerId, encounterId))
+        {
+            return false;
+        }
+
         // --- Telemetry hook site (NEO-109): future E9.M1 catalog event `encounter_start` ---
-        // TODO(E9.M1): catalog emit — when TryActivate returns true (first activation for player+encounter).
-        // Planned payload: playerId, encounterId, npcInstanceId (engagement target). No ingest or ILogger here
+        // TODO(E9.M1): catalog emit — once per player+encounter on first successful TryActivate (anchor matches emit).
+        // Planned payload: playerId, encounterId, npcInstanceId (engagement target),
+        // requiredNpcInstanceIdsCount (catalog requiredNpcInstanceIds for encounterId). No ingest or ILogger here
         // (comments-only). EncounterCombatWiring calls this on first damaging hit vs a required NPC (NEO-106).
 
-        return progressStore.TryActivate(playerId, encounterId);
+        return true;
     }
 
     /// <summary>

server/README.md

@@ -212,7 +212,7 @@ Comment-only hook sites for Epic 5 **Slice 3** catalog events ([epic_05 Slice 3]
 
 | Event | Anchor |
 |-------|--------|
-| **`encounter_start`** | **`EncounterProgressOperations.TryActivateOnFirstEngagement`** — emit when **`TryActivate`** returns true (first engagement). Activates NEO-84 reserved **`encounter_start`** pointer in **`CombatOperations`**. |
+| **`encounter_start`** | **`EncounterProgressOperations.TryActivateOnFirstEngagement`** — hook anchor after successful **`TryActivate`** (first engagement only). Activates NEO-84 reserved **`encounter_start`** pointer in **`CombatOperations`**. |
 | **`encounter_complete`** | **`EncounterCompletionOperations.TryCompleteAndGrant`** — once per successful grant + completion mark commit. |
 | **`reward_attribution`** | Same success path — once per completion (batch **`grantedItems`**); per-stack **`item_created`** remains in **`PlayerInventoryOperations`** ([NEO-56](#player-inventory-neo-54-store-neo-55-http)). |
 | **`encounter_complete_denied`** | **`EncounterCompletionOperations.Deny`** — structured deny + **`EncounterCompletionReasonCodes`**. |