# NEO-49 — Implementation plan

## Story reference

| Field | Value |
|--------|--------|
| **Key** | NEO-49 |
| **Title** | E2.M3: perk_unlock telemetry hook sites |
| **Linear** | https://linear.app/neon-sprawl/issue/NEO-49/e2m3-perk-unlock-telemetry-hook-sites |
| **Module** | [E2.M3 — MasteryAndPerkUnlocks](../decomposition/modules/E2_M3_MasteryAndPerkUnlocks.md) · Epic 2 Slice 4 |
| **Branch** | `NEO-49-perk-unlock-telemetry-hook-sites` |
| **Blocked by** | [NEO-47](https://linear.app/neon-sprawl/issue/NEO-47) — **Done** on `main` (`PerkUnlockEngine`, `PerkUnlockEvent`). May land in parallel with [NEO-48](https://linear.app/neon-sprawl/issue/NEO-48) (perk HTTP). |

## Kickoff clarifications

| Topic | Question / note | Resolution |
|--------|-----------------|------------|
| **Hook runtime behavior** | Comments-only vs dev `ILogger` (same as NEO-40)? | **User:** **comments-only** — no runtime emit, no production logging. |
| **Hook anchor** | Single site in `TryUnlockPerks` vs duplicate markers at callers? | User asked for recommendation → **Agent recommendation (adopted):** **single anchor** in `PerkUnlockEngine.TryUnlockPerks` immediately after `PerkUnlockEvent` instances are built (covers branch-pick unlocks, path-auto on pick, and level-up reevaluation). **Do not** duplicate at `SkillProgressionGrantOperations` or future NEO-48 HTTP — those call the engine and already receive `Events`; E9.M1 wiring can subscribe at the engine or via a future `IPerkUnlockEventSink` (NEO-47 plan note). |
| **Branch-select without unlock** | Hook on successful `TrySelectBranch` when `Events` is empty? | **No** — catalog event is **`perk_unlock`** only when a perk id is newly persisted; tier-1 pick-without-perk is not an unlock event. |
| **Deny paths** | `perk_branch_select_denied` hooks? | **Out of scope** (Linear); only **`perk_unlock`**. |

## Goal, scope, and out-of-scope

**Goal:** Document and place **comment-only hook sites** on the server-authoritative perk unlock path for future E9.M1 catalog event **`perk_unlock`**. Align with Epic 2 Slice 4 telemetry vocabulary ([epic_02 — Slice 4](../decomposition/epics/epic_02_skills_and_progression.md)).

**In scope (from Linear + E2M3-05):**

- Hook location(s) in unlock / branch-select path documented in **code comments** with **`TODO(E9.M1)`**.
- **`server/README.md`** + [E2_M3 module](../decomposition/modules/E2_M3_MasteryAndPerkUnlocks.md) note hook placement.
- Manual QA checklist per repo convention.

**Out of scope (from Linear):**

- E9.M1 catalog, ingest, dashboards.
- `ILogger` / metrics / dev-only log lines (rejected at kickoff).
- Optional `IPerkUnlockEventSink` implementation (deferred; comments-only this story).

## Acceptance criteria checklist

- [x] Hook location(s) in unlock / branch-select path documented in code comments.
- [x] `TODO(E9.M1)` alignment; no production logging calls.
- [x] Module doc + server README note hook placement.

## Technical approach

1. **Authoritative unlock surface:** `PerkUnlockEngine.TryUnlockPerks` — the only place new perk ids are persisted and `PerkUnlockEvent` records are created. All paths funnel here:
   - **`TrySelectBranch`** → explicit pick tier perks + `ApplyPathAutoTiersAtOrBelowLevel` (may emit zero events on tier-1-only pick).
   - **`ReevaluateAfterLevelUp`** → path-auto tiers after skill level-up (called from `SkillProgressionGrantOperations` and any future callers).

2. **Hook site — `perk_unlock`:** After building the `events` list (one `PerkUnlockEvent` per newly unlocked `perkId`), add a **comment block** before `return true`:
   - Name future E9.M1 event **`perk_unlock`**.
   - **`TODO(E9.M1): catalog emit`** (match NEO-30 / NEO-40 style).
   - Document payload fields from `PerkUnlockEvent`: `playerId`, `perkId`, `skillId`, `tierIndex`, `branchId`, `source` (`BranchPick` | `LevelUp`).
   - Note: emit **once per unlocked perk** in the batch (idempotent reevaluation produces no events when already unlocked).

3. **Cross-references (minimal):**
   - One-line pointer in `PerkUnlockEvent.cs` xml summary → hook site in `TryUnlockPerks`.
   - **`server/README.md`:** new short subsection under mastery (after NEO-46 block) for perk unlock engine + **NEO-49 telemetry hook** anchor.
   - **`E2_M3_MasteryAndPerkUnlocks.md`:** replace “telemetry in NEO-49” backlog wording with **NEO-49 landed** line when implementation completes.
   - **`documentation_and_implementation_alignment.md`:** E2.M3 row cites NEO-49 hook sites.

4. **NEO-48 readiness:** When perk HTTP lands, it will call `TrySelectBranch` and surface `Events`; no extra hook comments required at the API layer if the engine anchor exists.

## Files to add

| Path | Purpose |
|------|---------|
| `docs/manual-qa/NEO-49.md` | Manual QA: verify comment blocks + no behavior/API change. |

## Files to modify

| Path | Rationale |
|------|-----------|
| `server/NeonSprawl.Server/Game/Mastery/PerkUnlockEngine.cs` | **NEO-49:** comment-only `perk_unlock` hook site in `TryUnlockPerks` when events are created. |
| `server/NeonSprawl.Server/Game/Mastery/PerkUnlockEvent.cs` | Cross-reference hook anchor in type summary (no behavior change). |
| `server/README.md` | Document perk unlock telemetry hook placement (NEO-49). |
| `docs/decomposition/modules/E2_M3_MasteryAndPerkUnlocks.md` | Implementation snapshot: NEO-49 telemetry hooks landed. |
| `docs/decomposition/modules/documentation_and_implementation_alignment.md` | E2.M3 row: NEO-49 hook sites (still no E9.M1 ingest). |

## Tests

| Path | Change |
|------|--------|
| **None required** | Comments-only; existing `PerkUnlockEngineTests` and grant integration tests already cover unlock paths. |

**Manual verification:** `docs/manual-qa/NEO-49.md` — open `PerkUnlockEngine.cs`, confirm hook cites **`perk_unlock`**, **`TODO(E9.M1)`**, payload fields, and no new log statements; run `dotnet test` on server solution for regression sanity.

## Open questions / risks

**None.** NEO-47 blocker is complete; comments-only avoids production logging risk. If E9.M1 later needs caller context (HTTP request id), prefer `IPerkUnlockEventSink` at call sites without moving the primary hook documentation away from `TryUnlockPerks`.