diff --git a/docs/plans/NEO-109-implementation-plan.md b/docs/plans/NEO-109-implementation-plan.md new file mode 100644 index 0000000..05e4cd8 --- /dev/null +++ b/docs/plans/NEO-109-implementation-plan.md @@ -0,0 +1,136 @@ +# NEO-109 — Implementation plan + +## Story reference + +| Field | Value | +|--------|--------| +| **Key** | NEO-109 | +| **Title** | E5M3-10: Slice 3 encounter telemetry hook sites | +| **Linear** | https://linear.app/neon-sprawl/issue/NEO-109/e5m3-10-slice-3-encounter-telemetry-hook-sites | +| **Module** | [E5.M3 — EncounterAndRewardTables](../decomposition/modules/E5_M3_EncounterAndRewardTables.md) · Epic 5 Slice 3 · backlog **E5M3-10** | +| **Branch** | `NEO-109-slice-3-encounter-telemetry-hook-sites` | +| **Blocked by** | [NEO-106](https://linear.app/neon-sprawl/issue/NEO-106) — combat wiring (**Done** on `main`); [NEO-107](https://linear.app/neon-sprawl/issue/NEO-107) — event record (**Done** on `main`) | +| **Pattern** | [NEO-49](NEO-49-implementation-plan.md) / [NEO-64](NEO-64-implementation-plan.md) — comment-only engine anchors; [NEO-84](NEO-84-implementation-plan.md) — reserved `encounter_start` in Slice 1 combat | +| **Blocks** | None — client capstone [NEO-110](https://linear.app/neon-sprawl/issue/NEO-110) / [NEO-111](https://linear.app/neon-sprawl/issue/NEO-111) do not depend on telemetry | +| **Client counterpart** | None — server-only comments; player-visible HUD is [NEO-110](https://linear.app/neon-sprawl/issue/NEO-110) | + +## Kickoff clarifications + +| Topic | Question | Agent recommendation | Answer | +|--------|----------|----------------------|--------| +| **`reward_attribution` shape** | Once per completion vs per grant vs skip? | **Once per successful completion** — batch payload after all grants + completion mark commit; `item_created` remains at inventory layer (**NEO-56**). | **Adopted** — once per completion | +| **`encounter_complete` anchor** | Single engine anchor vs dual wiring + ops? | **`EncounterCompletionOperations.TryCompleteAndGrant` only** — remove duplicate in **`EncounterCombatWiring`** (**NEO-49** / **NEO-64** precedent). | **Adopted** — completion ops only | +| **Manual QA doc** | `docs/manual-qa/NEO-109.md`? | **Yes** (NEO-64 pattern). | **Adopted** — **skip** (NEO-108 server-only precedent per user) | +| **`encounter_complete_denied`** | Keep deny hook on `Deny`? | **Keep** — already landed from NEO-105 review; **NEO-56** deny precedent; not in epic vocabulary but comments-only. | **Adopted** (implicit — no change) | +| **Runtime behavior** | Comments-only vs dev `ILogger`? | **Comments-only** — `TODO(E9.M1)`; no production logging (**NEO-49** precedent). | **Adopted** (repo precedent; not asked) | + +## Goal, scope, and out-of-scope + +**Goal:** Document and place **comment-only hook sites** on the server-authoritative encounter funnel for Epic 5 Slice 3 product telemetry **`encounter_start`**, **`encounter_complete`**, and **`reward_attribution`**, plus consolidate anchors so E9.M1 wiring has one documented surface per event. + +**In scope (from Linear + [E5M3-10](E5M3-prototype-backlog.md#e5m3-10--slice-3-encounter-telemetry-hook-sites)):** + +- **`TODO(E9.M1)`** markers at encounter start, completion commit, and **once** reward attribution on successful grant batch. +- **`server/README.md`** encounter telemetry subsection; epic Slice 3 vocabulary cross-link. +- Normalize partial NEO-109 stubs from NEO-104/105/106 into full **NEO-64**-style comment blocks (event name, payload fields, no ingest). +- Update **NEO-84** reserved **`encounter_start`** pointer in **`CombatOperations`** to E5.M3 anchor (comments only). +- E5M3 backlog + alignment register + module snapshot when implementation completes. + +**Out of scope (from Linear + backlog):** + +- Production ingest, dashboards (**E9.M1**). +- `ILogger` / metrics / dev-only log lines. +- Godot client, Bruno changes, API/wire contract changes. +- `docs/manual-qa/NEO-109.md` (kickoff: skip). +- Duplicate hook comments in **`AbilityCastApi`** or **`EncounterCombatWiring`** for **`encounter_complete`** / **`reward_attribution`**. + +## Acceptance criteria checklist + +- [ ] Hook names match [epic_05 Slice 3](../decomposition/epics/epic_05_pve_combat.md#epic-5-slice-3) vocabulary (`encounter_complete`, reward attribution / **`reward_attribution`**). +- [ ] No runtime behavior change beyond comments. + +## Technical approach + +### Current state (on `main`) + +Partial **NEO-109** tags already exist from precursor stories: + +| Event | File | Status | +|-------|------|--------| +| **`encounter_start`** | `EncounterProgressOperations.TryActivateOnFirstEngagement` | Stub present — expand to full block | +| **`encounter_complete`** | `EncounterCompletionOperations` success path | Thin `TODO` — expand; **remove** `EncounterCombatWiring` duplicate | +| **`encounter_complete_denied`** | `EncounterCompletionOperations.Deny` | Block present — keep | +| **`reward_attribution`** | — | **Missing** — add on completion success path | + +### Hook sites (after implementation) + +1. **`encounter_start`** — **`EncounterProgressOperations.TryActivateOnFirstEngagement`**, immediately before **`progressStore.TryActivate`** when activation will proceed (not when already started/completed/no encounter): + - Comment block: future E9.M1 event **`encounter_start`**. + - **`TODO(E9.M1): catalog emit`** — once per player+encounter on first activation. + - Payload: `playerId`, `encounterId`, `npcInstanceId` (engagement target), optional `requiredNpcInstanceIds` count. + - **NEO-84:** update class-level reserved comment in **`CombatOperations`** to point here (Slice 3 landed). + +2. **`encounter_complete`** — **`EncounterCompletionOperations.TryCompleteAndGrant`**, after **`TryMarkCompleted`** + **`TryRecord`** succeed (authoritative commit): + - Replace thin TODO with full block naming **`encounter_complete`**. + - Payload: `playerId`, `encounterId`, `rewardTableId`, `completedAt`, `idempotencyKey`, `grantedItems` summary from **`EncounterCompleteEvent`**. + - **Remove** duplicate hook block at end of **`EncounterCombatWiring.TryProcessCastOutcome`** (wiring only calls completion ops). + +3. **`reward_attribution`** — same success path, **once per completion** (kickoff decision): + - Comment block naming **`reward_attribution`** (snake_case catalog name; epic prose “reward attribution”). + - **`TODO(E9.M1): catalog emit`** — once when reward table grants commit for encounter completion. + - Payload: `playerId`, `encounterId`, `rewardTableId`, `grantedItems[]` (batch from **`EncounterGrantApplied`** / event store snapshot). + - Note in comment: inventory layer still emits **`item_created`** per stack via **`PlayerInventoryOperations`** (**NEO-56**); **`reward_attribution`** is encounter-scoped attribution for E7.M2 / loot analytics. + +4. **`encounter_complete_denied`** — keep existing **`Deny`** block ( **`reasonCode`** from **`EncounterCompletionReasonCodes`** ). + +5. **Cross-references:** + - **`EncounterCompletionResult`** / **`EncounterCompleteEvent`** xml summaries → telemetry anchors in **`EncounterCompletionOperations`**. + - **`EncounterProgressOperations`** class summary → **`encounter_start`** anchor. + - **`server/README.md`:** new **Encounter telemetry hooks (NEO-109)** subsection (after NEO-108 or NEO-106 block) listing all three epic events + deny + **`CombatOperations`** reserved pointer. + - **`E5_M3_EncounterAndRewardTables.md`**, **`documentation_and_implementation_alignment.md`**, **`module_dependency_register.md`**, **`E5M3-prototype-backlog.md`:** E5M3-10 landed note when complete. + +### NEO-56 / NEO-64 analogy + +| Layer | Inventory / gather | Encounter Slice 3 | +|-------|-------------------|-------------------| +| Low-level item mutation | **`item_created`** (**NEO-56**) | Same on each **`TryAddStack`** during grants | +| Orchestration / funnel | **`resource_gathered`** (**NEO-64**) | **`encounter_complete`** + **`reward_attribution`** (**NEO-109**) at completion commit | +| Engagement | — | **`encounter_start`** at first activation | + +## Files to add + +None. + +## Files to modify + +| Path | Rationale | +|------|-----------| +| `server/NeonSprawl.Server/Game/Encounters/EncounterCompletionOperations.cs` | Expand **`encounter_complete`** block; add **`reward_attribution`** once on success; keep **`encounter_complete_denied`** on **`Deny`**. | +| `server/NeonSprawl.Server/Game/Encounters/EncounterCombatWiring.cs` | Remove duplicate **`encounter_complete`** hook; one-line pointer to **`EncounterCompletionOperations`**. | +| `server/NeonSprawl.Server/Game/Encounters/EncounterProgressOperations.cs` | Expand **`encounter_start`** block to NEO-64 style; class summary cross-ref. | +| `server/NeonSprawl.Server/Game/Encounters/EncounterCompletionResult.cs` | Xml summary → telemetry hook anchor (no behavior change). | +| `server/NeonSprawl.Server/Game/Encounters/EncounterCompleteEvent.cs` | Xml summary → completion + attribution hook anchor (no behavior change). | +| `server/NeonSprawl.Server/Game/Combat/CombatOperations.cs` | Update reserved **`encounter_start`** comment → E5.M3 **`EncounterProgressOperations`** anchor. | +| `server/README.md` | **NEO-109** encounter telemetry hooks subsection; fix NEO-104 vs NEO-109 label on **`encounter_start`** in NEO-106 table if needed. | +| `docs/decomposition/modules/E5_M3_EncounterAndRewardTables.md` | NEO-109 telemetry hooks landed line. | +| `docs/decomposition/modules/documentation_and_implementation_alignment.md` | E5.M3 row: NEO-109 hook sites. | +| `docs/decomposition/modules/module_dependency_register.md` | E5.M3 note — E5M3-10 complete when landed. | +| `docs/plans/E5M3-prototype-backlog.md` | E5M3-10 acceptance checkboxes + landed note when complete. | + +## Tests + +| Test file | What it covers | +|-----------|----------------| +| *(none added or changed)* | Comments-only; no behavior or wire contract change. Regression: `dotnet test NeonSprawl.sln` (existing **`EncounterCompletionOperationsTests`**, **`EncounterProgressOperationsTests`**, cast integration tests unchanged). | + +**Manual verification (no checklist doc):** grep `NEO-109` / `encounter_complete` / `reward_attribution` / `encounter_start` under `server/NeonSprawl.Server/Game/Encounters/`; confirm **`EncounterCombatWiring`** has no **`encounter_complete`** emit comment; confirm no new **`ILogger`** or ingest calls. + +## Open questions / risks + +| Question / risk | Agent recommendation | Status | +|-----------------|----------------------|--------| +| **Overlap with NEO-56 `item_created`** | Keep both — **`item_created`** per stack; **`reward_attribution`** once per encounter completion for attribution context. | **adopted** (kickoff) | +| **`encounter_complete_denied` not in epic list** | Keep deny hook — **NEO-56** precedent; zero runtime cost. | **adopted** | +| **Precursor partial stubs** | Treat as draft; NEO-109 normalizes style and removes wiring duplicate. | **adopted** | + +None blocking.