From 986cf55e737bd836d4ee255e74178da7488b9b2a Mon Sep 17 00:00:00 2001 From: VinPropane Date: Sun, 31 May 2026 18:22:57 -0400 Subject: [PATCH] NEO-107: Add kickoff implementation plan for EncounterCompleteEvent persistence. --- docs/plans/NEO-107-implementation-plan.md | 147 ++++++++++++++++++++++ 1 file changed, 147 insertions(+) create mode 100644 docs/plans/NEO-107-implementation-plan.md diff --git a/docs/plans/NEO-107-implementation-plan.md b/docs/plans/NEO-107-implementation-plan.md new file mode 100644 index 0000000..133a00a --- /dev/null +++ b/docs/plans/NEO-107-implementation-plan.md @@ -0,0 +1,147 @@ +# 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 + +- [ ] Completion path records **`EncounterCompleteEvent`** exactly once per player+encounter. +- [ ] Hook documents E7.M2 **`QuestRewardBundle`** consumer contract (comments + module cross-link). +- [ ] No runtime behavior beyond event record + comments. + +## 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();`** + +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** |