neon-sprawl/docs/plans/NEO-107-implementation-plan.md

156 lines
13 KiB
Markdown
Raw Permalink Blame History

This file contains ambiguous Unicode characters!

This file contains ambiguous Unicode characters that may be confused with others in your current locale. If your use case is intentional and legitimate, you can safely ignore this warning. Use the Escape button to highlight these characters.

# NEO-107 — Implementation plan
## Story reference
| Field | Value |
|--------|--------|
| **Key** | NEO-107 |
| **Title** | E5M3-09: EncounterCompleteEvent + E7.M2 quest-credit hook stub |
| **Linear** | https://linear.app/neon-sprawl/issue/NEO-107/e5m3-09-encountercompleteevent-e7m2-quest-credit-hook-stub |
| **Module** | [E5.M3 — EncounterAndRewardTables](../decomposition/modules/E5_M3_EncounterAndRewardTables.md) · Epic 5 Slice 3 · backlog **E5M3-09** |
| **Branch** | `NEO-107-encounter-complete-event-e7m2-hook` |
| **Precursor** | [NEO-105](https://linear.app/neon-sprawl/issue/NEO-105) — `EncounterCompleteEvent` result payload + `TryCompleteAndGrant` (**landed on `main`**) · [NEO-106](https://linear.app/neon-sprawl/issue/NEO-106) — combat wiring invokes completion (**landed on `main`**) |
| **Pattern** | [NEO-104](NEO-104-implementation-plan.md) — split in-memory stores + per-key locks; [NEO-105](NEO-105-implementation-plan.md) — event type + idempotency key already defined |
| **Blocks** | [NEO-110](https://linear.app/neon-sprawl/issue/NEO-110) — client encounter HUD (indirect; needs event record for downstream parity) |
| **Client counterpart** | None this story — server-only event record + hook stub; player-visible HUD is [NEO-110](https://linear.app/neon-sprawl/issue/NEO-110) via [NEO-108](https://linear.app/neon-sprawl/issue/NEO-108) |
## Kickoff clarifications
**No blocking clarifications needed.** NEO-105 kickoff adopted the split: NEO-105 returns `EncounterCompleteEvent` in `EncounterCompletionResult`; NEO-107 persists the record and expands the E7.M2 hook. E5M3 backlog defaults fix in-memory storage, once-per-player+encounter idempotency, and comment-only E7.M2 behavior (no `QuestRewardBundle` router). NEO-106 wires all completion through `TryCompleteAndGrant` — a single record site preserves atomicity with `TryMarkCompleted`.
| Topic | Question | Agent recommendation | Answer |
|--------|----------|----------------------|--------|
| **Event store shape** | Dedicated store vs extend `IEncounterCompletionStore`? | **Dedicated `IEncounterCompleteEventStore`** — completion flag is boolean+timestamp; event carries grant summary + idempotency key (NEO-104 split-store precedent). | **Adopted** (backlog + NEO-105) |
| **Record site** | Inside `TryCompleteAndGrant` vs caller after success? | **Inside `TryCompleteAndGrant` after `TryMarkCompleted` succeeds** — same completion commit boundary; NEO-106 / cast path need no duplicate orchestration. | **Adopted** (NEO-105 deferral note) |
| **Storage backend** | In-memory vs Postgres? | **In-memory only** — E5M3 kickoff default (NEO-104). | **Adopted** |
| **E7.M2 runtime** | Stub router call vs comments only? | **Comments only** — document `QuestRewardBundle` / `RewardDeliveryEvent` consumer contract; no quest state machine ([E5M3-09](E5M3-prototype-backlog.md#e5m3-09--encountercompleteevent--e7m2-quest-credit-hook-stub)). | **Adopted** |
| **HTTP / Godot** | Expose event on GET or client? | **Defer** — [NEO-108](https://linear.app/neon-sprawl/issue/NEO-108) projects grant summary from stores; this story records only. | **Adopted** (backlog out-of-scope) |
## Goal, scope, and out-of-scope
**Goal:** Persist **`EncounterCompleteEvent`** exactly once per player+encounter when completion grants commit, and expand the **E7.M2** quest-credit hook stub documenting the future **`QuestRewardBundle`** consumer contract.
**In scope (from Linear + [E5M3-09](E5M3-prototype-backlog.md#e5m3-09--encountercompleteevent--e7m2-quest-credit-hook-stub)):**
- **`IEncounterCompleteEventStore`** + **`InMemoryEncounterCompleteEventStore`** — idempotent **`TryRecord(EncounterCompleteEvent)`** keyed by **`(playerId, encounterId)`** / **`IdempotencyKey`**; **`TryGet`** for tests and NEO-108 read path.
- Wire store into **`EncounterCompletionOperations.TryCompleteAndGrant`** — record on success after completion mark; replay / deny paths do not record.
- Expand **`// TODO(E7.M2)`** hook in completion path with **`reward_delivery`** vocabulary and pointer to [E7_M2_RewardAndUnlockRouter](../decomposition/modules/E7_M2_RewardAndUnlockRouter.md) **`QuestRewardBundle`** / **`RewardDeliveryEvent`** consumer contract (prototype: quest credit beyond **`contract_handoff_token`** item loot).
- Pass **`IEncounterCompleteEventStore`** through **`EncounterCombatWiring`** → **`AbilityCastApi`** DI chain.
- Unit tests (AAA): first completion records event once; second completion does not duplicate record; deny paths leave store empty.
- `server/README.md` encounter-complete-event section; E5.M3 module snapshot + E7.M2 implementation anchor.
**Out of scope (from Linear + backlog):**
- Production **`QuestRewardBundle`** apply, quest state machine, **`RewardDeliveryEvent`** type implementation ([E7.M2](../decomposition/modules/E7_M2_RewardAndUnlockRouter.md) full router).
- **`GET /game/players/{id}/encounter-progress`** projection ([NEO-108](https://linear.app/neon-sprawl/issue/NEO-108)).
- Telemetry **`encounter_complete`** ingest ([NEO-109](https://linear.app/neon-sprawl/issue/NEO-109)).
- Godot HUD — **client counterpart [NEO-110](https://linear.app/neon-sprawl/issue/NEO-110)**.
- Postgres persistence, Bruno, `docs/manual-qa/NEO-107.md`.
## Acceptance criteria checklist
- [x] Completion path records **`EncounterCompleteEvent`** exactly once per player+encounter.
- [x] Hook documents E7.M2 **`QuestRewardBundle`** consumer contract (comments + module cross-link).
- [x] No runtime behavior beyond event record + comments.
## Implementation reconciliation (shipped)
- **Store:** `IEncounterCompleteEventStore` + `InMemoryEncounterCompleteEventStore` — idempotent `TryRecord` / `TryGet` keyed by `(playerId, encounterId)`.
- **Wiring:** `EncounterCompletionOperations.TryCompleteAndGrant` records event after successful completion mark; `EncounterCombatWiring` + `AbilityCastApi` pass store through DI.
- **Hook:** Expanded E7.M2 `reward_delivery` TODO comments; [E7_M2](../decomposition/modules/E7_M2_RewardAndUnlockRouter.md) implementation anchor.
- **Tests:** `InMemoryEncounterCompleteEventStoreTests`; extended `EncounterCompletionOperationsTests`, `EncounterCombatWiringTests`, `EncounterDefinitionRegistryTests`.
- **Docs:** `server/README.md`; E5.M3 module snapshot + alignment register + backlog + module dependency register updated.
## Technical approach
1. **`IEncounterCompleteEventStore`**
- **`bool TryRecord(EncounterCompleteEvent completeEvent)`** — `true` on first insert for normalized **`(playerId, encounterId)`**; `false` if already recorded (idempotent no-op).
- **`bool TryGet(string playerId, string encounterId, out EncounterCompleteEvent completeEvent)`** — read back stored event for tests / future HTTP.
2. **`InMemoryEncounterCompleteEventStore`**
- Key: **`EncounterProgressIds.MakeProgressKey(playerId, encounterId)`** (same as progress/completion stores).
- Value: full **`EncounterCompleteEvent`** struct (including **`GrantedItems`** snapshot at record time).
- Thread-safe: **`ConcurrentDictionary`** + per-key locks (mirror **`InMemoryEncounterCompletionStore`**).
3. **`EncounterCompletionOperations.TryCompleteAndGrant`**
- Add parameter **`IEncounterCompleteEventStore completeEventStore`**.
- After **`TryMarkCompleted`** succeeds and **`EncounterCompleteEvent`** is built:
- Call **`completeEventStore.TryRecord(completeEvent)`**.
- If **`TryRecord`** returns `false` (unexpected after fresh mark — invariant violation / race), **do not rollback** grants or completion flag; treat as idempotent success (event already present for this key). Document in README.
- Replace terse TODO with expanded E7.M2 hook block:
```csharp
// TODO(E7.M2): reward_delivery — QuestRewardBundle router consumes EncounterCompleteEvent
// (playerId, encounterId, idempotencyKey, grantedItemSummary) → idempotent RewardDeliveryEvent.
// Prototype quest credit: contract_handoff_token item loot + this event record; full router: E7.M2.
```
4. **DI**
- Register in **`EncounterCatalogServiceCollectionExtensions.AddEncounterAndRewardCatalogs`**:
- **`services.AddSingleton<IEncounterCompleteEventStore, InMemoryEncounterCompleteEventStore>();`**
5. **Call chain updates**
- **`EncounterCombatWiring.TryProcessCastOutcome`** — resolve **`IEncounterCompleteEventStore`** from deps; pass to **`TryCompleteAndGrant`**.
- **`AbilityCastApi`** — inject **`IEncounterCompleteEventStore`** into wiring call (alongside existing encounter/inventory deps).
6. **Docs**
- **`server/README.md`** — new subsection under encounter completion: event store, idempotency, E7.M2 deferral.
- **`E5_M3_EncounterAndRewardTables.md`** — implementation snapshot for E5M3-09.
- **`E7_M2_RewardAndUnlockRouter.md`** — implementation anchor: **`EncounterCompleteEvent`** producer + future consumer hook.
- Backlog / alignment register / module dependency register — note NEO-107 when landed.
### Prototype completion → event flow (frozen)
| Step | Expected |
|------|----------|
| First **`TryCompleteAndGrant`** success | Completion store marked; **`TryRecord`** stores event with scrap ×10 + token ×1 summary |
| Second **`TryCompleteAndGrant`** | Deny **`already_completed`** before record; event store unchanged |
| Inventory deny / **`not_ready`** | No completion mark; event store empty for that encounter |
| Cast path (NEO-106) | Third defeat → same record via **`TryCompleteAndGrant`** |
## Files to add
| Path | Purpose |
|------|---------|
| `server/NeonSprawl.Server/Game/Encounters/IEncounterCompleteEventStore.cs` | Injectable event persistence contract. |
| `server/NeonSprawl.Server/Game/Encounters/InMemoryEncounterCompleteEventStore.cs` | Thread-safe in-memory implementation (NEO-107). |
| `server/NeonSprawl.Server.Tests/Game/Encounters/InMemoryEncounterCompleteEventStoreTests.cs` | AAA store idempotency + TryGet tests. |
| `docs/plans/NEO-107-implementation-plan.md` | This plan. |
## Files to modify
| Path | Rationale |
|------|-----------|
| `server/NeonSprawl.Server/Game/Encounters/EncounterCompletionOperations.cs` | Accept event store; **`TryRecord`** on success; expand E7.M2 hook comments. |
| `server/NeonSprawl.Server/Game/Encounters/EncounterCompleteEvent.cs` | Update XML — persistence landed on NEO-107. |
| `server/NeonSprawl.Server/Game/Encounters/EncounterCombatWiring.cs` | Pass **`IEncounterCompleteEventStore`** into **`TryCompleteAndGrant`**. |
| `server/NeonSprawl.Server/Game/AbilityInput/AbilityCastApi.cs` | Inject event store for wiring call. |
| `server/NeonSprawl.Server/Game/Encounters/EncounterCatalogServiceCollectionExtensions.cs` | Register **`IEncounterCompleteEventStore`** singleton. |
| `server/NeonSprawl.Server.Tests/Game/Encounters/EncounterCompletionOperationsTests.cs` | Assert event recorded once; replay / deny paths leave store empty; update **`ResolveDependencies`**. |
| `server/NeonSprawl.Server.Tests/Game/Encounters/EncounterCombatWiringTests.cs` | Assert event store populated after third-defeat wiring completion. |
| `server/NeonSprawl.Server.Tests/Game/Encounters/EncounterDefinitionRegistryTests.cs` | Host DI resolves **`IEncounterCompleteEventStore`** (if not already covered). |
| `server/README.md` | Document event store + E7.M2 hook stub; remove NEO-107 deferral notes. |
| `docs/decomposition/modules/E5_M3_EncounterAndRewardTables.md` | Implementation snapshot — E5M3-09 event persistence. |
| `docs/decomposition/modules/E7_M2_RewardAndUnlockRouter.md` | Implementation anchor — **`EncounterCompleteEvent`** producer hook. |
| `docs/decomposition/modules/documentation_and_implementation_alignment.md` | E5.M3 row — note NEO-107 event record when landed. |
| `docs/decomposition/modules/module_dependency_register.md` | E5.M3 note — NEO-107 event store when landed. |
| `docs/plans/E5M3-prototype-backlog.md` | Mark E5M3-09 AC when complete. |
## Tests
| Test file | What it covers |
|-----------|----------------|
| `InMemoryEncounterCompleteEventStoreTests.cs` | **Arrange:** prototype **`EncounterCompleteEvent`**. **Act/Assert:** first **`TryRecord`** → `true` + **`TryGet`** round-trip; second **`TryRecord`** same key → `false`; empty key / invalid ids fail closed. |
| `EncounterCompletionOperationsTests.cs` | **Extend existing grant-once test:** assert **`TryGet`** returns event matching **`CompleteEvent`**; **`already_completed`** / **`not_ready`** / **`inventory_full`** paths assert store still empty (or unchanged count). |
| `EncounterCombatWiringTests.cs` | **Extend third-defeat completion test:** assert event store has record after wiring success. |
| `EncounterDefinitionRegistryTests.cs` | **Assert:** host startup resolves **`IEncounterCompleteEventStore`** as **`InMemoryEncounterCompleteEventStore`**. |
## Open questions / risks
| Question / risk | Agent recommendation | Status |
|-----------------|----------------------|--------|
| **Record after mark ordering** | Record only after **`TryMarkCompleted`** succeeds — event mirrors committed completion. | **adopted** |
| **TryRecord false after fresh mark** | Treat as idempotent (event already there); no grant rollback — same key invariant as completion store. | **adopted** |
| **NEO-108 grant summary** | HTTP read may use event store **`GrantedItems`** or completion + inventory delta — defer projection design to NEO-108; store exposes **`TryGet`**. | **deferred** |
| **Postgres follow-on** | Defer — session persistence migration is a future issue. | **deferred** |