NEO-109: Add kickoff implementation plan.

Documents comment-only encounter_start, encounter_complete, and
reward_attribution hook sites with kickoff decisions adopted.
pull/148/head
VinPropane 2026-05-31 19:01:28 -04:00
parent 4aa7665e7c
commit e6debc2968
1 changed files with 136 additions and 0 deletions

View File

@ -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.