Merge pull request #91 from ViPro-Technologies/NEO-56-item-created-transfer-failure-telemetry-hooks

NEO-56: E3.M3 item_created / inventory_transfer_denied telemetry hook sites
pull/93/head
VinPropane 2026-05-24 13:45:19 -04:00 committed by GitHub
commit 597c3873d4
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
11 changed files with 248 additions and 7 deletions

View File

@ -12,12 +12,22 @@ When the user starts work on a **Linear issue** (e.g. issue key `NEO-5`, phrases
- During kickoff and while drafting the plan, **ask the user** about anything **unclear**, **underspecified**, or that **needs a decision** (scope, behavior, acceptance criteria, priorities, trade-offs, or what is out of scope). Do not guess or pick silent defaults when the issue, Linear notes, or repo context do not settle it.
- Use **concrete questions**, often with short options when helpful; group related questions so the user can answer in one pass.
- For **every** question or blocking decision presented to the user, state the **agents recommendation**: which option to pick (or what to do) and a **one-line rationale** tied to repo precedent, Linear AC, or decomposition docs. Do not list options without a stated preference unless there is genuinely no basis to recommend—in that case, say why.
- When the Cursor session supports it (**Agent** mode), prefer **`AskQuestion`** multiple-choice for blocking decisions (same pattern as [story-end](story-end.md) §4a for **Linear state** after “end story”) instead of only long prose lists. Put the **agent recommendation** in the chat message that introduces the questions (`AskQuestion` has no recommendation field).
- When the Cursor session supports it (**Agent** mode), prefer **`AskQuestion`** multiple-choice for blocking decisions (same pattern as [story-end](story-end.md) §4a for **Linear state** after “end story”) instead of only long prose lists. **`AskQuestion` has no recommendation field** — recommendations are **never** optional and **never** deferred to the plan alone.
### AskQuestion + recommendations (required order)
When you use **`AskQuestion`** for kickoff blocking decisions, follow **all** of this in the **same assistant turn**:
1. **Chat first (mandatory):** In the user-visible message, include a **Recommend:** line (or compact table) for **each** topic — **before** any `AskQuestion` tool call. Format: topic → recommended option → one-line rationale (repo precedent, Linear AC, or decomposition doc).
2. **Then `AskQuestion`:** Invoke the tool only after step 1 is written in chat. A turn that calls **`AskQuestion` without step 1 is a kickoff violation** — do not draft or commit the implementation plan in that turn; fix the message and re-ask.
3. **Label backup:** Mark the recommended option in the **`AskQuestion` option label** as well (e.g. `Engine only (recommended)`). This does **not** replace step 1; it is a fallback if the preamble is truncated.
**Invalid (do not do):** `AskQuestion` alone with bare option lists and no **Recommend:** preamble. **Invalid:** recommendations only inside `docs/plans/{KEY}-implementation-plan.md` before the user has answered.
### Clarifications gate (hard requirement)
- **Do not** create or update `docs/plans/{KEY}-implementation-plan.md` until clarification questions have been asked and the user has answered (or explicitly says no questions are needed).
- If at least one **blocking decision** exists in planning, use **`AskQuestion`** (Agent mode) instead of prose-only questions.
- If at least one **blocking decision** exists in planning, use **`AskQuestion`** (Agent mode) per **AskQuestion + recommendations (required order)** above — not prose-only lists, and not `AskQuestion` without a **Recommend:** preamble.
- If kickoff has zero questions, explicitly record why in the plan under a **Kickoff clarifications** section with evidence from Linear + repo context.
- **Do not commit** a kickoff plan until the clarification step above is complete.

View File

@ -21,7 +21,7 @@ Optional **personas** for Cursor. Enable by **@ mentioning** the rule in chat or
| **Code review** | [`.cursor/rules/code-review-agent.md`](.cursor/rules/code-review-agent.md) | PR / diff / pre-merge review; **always writes** `docs/reviews/YYYY-MM-DD-{slug}.md` with **Documentation checked** vs `docs/plans/` and `docs/decomposition/modules/`; **when suggestions are fixed, strike through those bullets + `Done.` in that file** ([planning-implementation-docs](.cursor/rules/planning-implementation-docs.md)); short chat pointer |
| **Docs review** | [`.cursor/rules/docs-review-agent.md`](.cursor/rules/docs-review-agent.md) | Coherence / links / dev-guide fitness for `docs/` (especially `docs/game-design/` + decomposition); **writes** `docs/reviews/YYYY-MM-DD-{slug}.md`; **when suggestions are fixed, strike through + `Done.` in that file** ([planning-implementation-docs](.cursor/rules/planning-implementation-docs.md)); use when working in **documents** or @ mention |
Project-wide conventions live under [`.cursor/rules/`](.cursor/rules/) (many are `alwaysApply`). **Planning / implementation decisions** must be written into `docs/plans/` (and related docs)—[`.cursor/rules/planning-implementation-docs.md`](.cursor/rules/planning-implementation-docs.md). **Manual QA checklists** for user-visible ticketed work are generated during implementation at `docs/manual-qa/{KEY}.md` (same rule). **Automated tests (AAA, mandatory):** **C#** — every new or changed **`[Fact]` / `[Theory]`** method in **`*Tests.cs`** must use full **Arrange → Act → Assert**: the three **`// Arrange` / `// Act` / `// Assert`** labels, **Act** = behavior under test only, **Assert** = all expectations including **`ReadFromJsonAsync`** (etc.) for verification (no blank line required after the phase comments) — [csharp-style](.cursor/rules/csharp-style.md#unit-and-integration-tests-arrange-act-assert), [testing-expectations](.cursor/rules/testing-expectations.md); snippets **`xut`** / **`xutc`**. **GdUnit (`client/test/`)** — same AAA with **`# Arrange` / `# Act` / `# Assert`** — [gdscript-style](.cursor/rules/gdscript-style.md#gdunit-test-layout-aaa). **Godot client layout** (thin `main.gd`, split by concern): [`.cursor/rules/godot-client-script-organization.md`](.cursor/rules/godot-client-script-organization.md). **Git:** on Linear/story work, agents **commit as they go** on the story branch (kickoff plan, coherent batches)—**do not wait** for the user to say “commit”; **never** `git push` unless the user asks — [`.cursor/rules/commit-and-review.md`](.cursor/rules/commit-and-review.md). **Open PR:** when the user asks, use the **GitHub MCP** **`create_pull_request`** tool (`user-github`) with a title starting **`NEO-123:`** ([commit-and-review](.cursor/rules/commit-and-review.md)); **do not use `gh` CLI for GitHub operations in this repo**. **Story lifecycle:** kickoff [`.cursor/rules/story-kickoff.md`](.cursor/rules/story-kickoff.md); after merge / end of story — `checkout main`, `pull`, delete local story branch — [`.cursor/rules/story-end.md`](.cursor/rules/story-end.md). **Linear:** on **end story**, use **`AskQuestion`** multiple choice for **In Test** / **Done** / **Skip** when Agent mode supports it ([story-end](.cursor/rules/story-end.md) §4a); otherwise ask in prose. Wait for the choice (or the same message naming the state) **before** `save_issue`; do not guess. **PR / push text:** no “Made-with: Cursor” boilerplate (same file).
Project-wide conventions live under [`.cursor/rules/`](.cursor/rules/) (many are `alwaysApply`). **Planning / implementation decisions** must be written into `docs/plans/` (and related docs)—[`.cursor/rules/planning-implementation-docs.md`](.cursor/rules/planning-implementation-docs.md). **Manual QA checklists** for user-visible ticketed work are generated during implementation at `docs/manual-qa/{KEY}.md` (same rule). **Automated tests (AAA, mandatory):** **C#** — every new or changed **`[Fact]` / `[Theory]`** method in **`*Tests.cs`** must use full **Arrange → Act → Assert**: the three **`// Arrange` / `// Act` / `// Assert`** labels, **Act** = behavior under test only, **Assert** = all expectations including **`ReadFromJsonAsync`** (etc.) for verification (no blank line required after the phase comments) — [csharp-style](.cursor/rules/csharp-style.md#unit-and-integration-tests-arrange-act-assert), [testing-expectations](.cursor/rules/testing-expectations.md); snippets **`xut`** / **`xutc`**. **GdUnit (`client/test/`)** — same AAA with **`# Arrange` / `# Act` / `# Assert`** — [gdscript-style](.cursor/rules/gdscript-style.md#gdunit-test-layout-aaa). **Godot client layout** (thin `main.gd`, split by concern): [`.cursor/rules/godot-client-script-organization.md`](.cursor/rules/godot-client-script-organization.md). **Git:** on Linear/story work, agents **commit as they go** on the story branch (kickoff plan, coherent batches)—**do not wait** for the user to say “commit”; **never** `git push` unless the user asks — [`.cursor/rules/commit-and-review.md`](.cursor/rules/commit-and-review.md). **Open PR:** when the user asks, use the **GitHub MCP** **`create_pull_request`** tool (`user-github`) with a title starting **`NEO-123:`** ([commit-and-review](.cursor/rules/commit-and-review.md)); **do not use `gh` CLI for GitHub operations in this repo**. **Story lifecycle:** kickoff [`.cursor/rules/story-kickoff.md`](.cursor/rules/story-kickoff.md) — blocking decisions need a **Recommend:** preamble in chat **before** `AskQuestion`, plus **(recommended)** on the chosen option label; `AskQuestion` alone is a violation. After merge / end of story — `checkout main`, `pull`, delete local story branch — [`.cursor/rules/story-end.md`](.cursor/rules/story-end.md). **Linear:** on **end story**, use **`AskQuestion`** multiple choice for **In Test** / **Done** / **Skip** when Agent mode supports it ([story-end](.cursor/rules/story-end.md) §4a); otherwise ask in prose. Wait for the choice (or the same message naming the state) **before** `save_issue`; do not guess. **PR / push text:** no “Made-with: Cursor” boilerplate (same file).
**Commits tied to a Linear issue** must start the subject with the **issue id** and a colon (e.g. `NEO-8: …`). Branch naming and exceptions (`chore:` when there is no ticket) — [`.cursor/rules/linear-git-naming.md`](.cursor/rules/linear-git-naming.md).

View File

@ -72,6 +72,8 @@ Epic 3 **Slice 1** — MVP inventory; `item_created`, transfer failures.
**Player inventory HTTP (NEO-55):** **`GET` / `POST /game/players/{id}/inventory`** — versioned snapshot + **`mutationKind`** add/remove backed by **`PlayerInventoryOperations`**; Bruno `bruno/neon-sprawl-server/inventory/`. Plan: [NEO-55 implementation plan](../../plans/NEO-55-implementation-plan.md).
**Inventory telemetry hooks (NEO-56):** comment-only **`item_created`** (successful add) and **`inventory_transfer_denied`** + **`reasonCode`** anchors in **`PlayerInventoryOperations`**; [server README — Player inventory (NEO-56)](../../../server/README.md#player-inventory-neo-54-store-neo-55-http). Plan: [NEO-56 implementation plan](../../plans/NEO-56-implementation-plan.md); manual QA [`NEO-56`](../../manual-qa/NEO-56.md).
**Linear backlog (decomposed):** [E3M3-prototype-backlog.md](../../plans/E3M3-prototype-backlog.md) — **E3M3-01** [NEO-50](https://linear.app/neon-sprawl/issue/NEO-50) (content + CI) through **E3M3-07** [NEO-56](https://linear.app/neon-sprawl/issue/NEO-56) (telemetry hooks).
## Source anchors

View File

@ -54,7 +54,7 @@ Rows appear when work starts; default for unlisted modules is **Planned** / not
| E2.M3 | In Progress | **NEO-45 landed:** prototype **`salvage`** mastery catalog + CI gates (see [NEO-45 plan](../../plans/NEO-45-implementation-plan.md)). **NEO-46 landed:** fail-fast server load under `server/NeonSprawl.Server/Game/Mastery/``MasteryCatalogLoader`, `IMasteryCatalogRegistry`, cross-check vs `ISkillDefinitionRegistry`, Slice 4 + **`tierIndex`** 1..N gate; see [NEO-46 plan](../../plans/NEO-46-implementation-plan.md). **NEO-47 landed:** `PerkUnlockEngine`, `IPlayerPerkStateStore` + **`V004`**, level-up hook in skill XP grants; see [NEO-47 plan](../../plans/NEO-47-implementation-plan.md). **NEO-49 landed:** comment-only **`perk_unlock`** telemetry hook site in [`PerkUnlockEngine.TryUnlockPerks`](../../../server/NeonSprawl.Server/Game/Mastery/PerkUnlockEngine.cs) ([NEO-49 plan](../../plans/NEO-49-implementation-plan.md), [`NEO-49` manual QA](../../manual-qa/NEO-49.md)); [server README — Perk unlock telemetry (NEO-49)](../../../server/README.md#perk-unlock-engine-and-telemetry-hooks-neo-47-neo-49). **NEO-48 landed:** **`GET`/`POST /game/players/{id}/perk-state`** — `PerkStateApi` + DTOs in `Game/Mastery/` ([NEO-48](../../plans/NEO-48-implementation-plan.md), [`NEO-48` manual QA](../../manual-qa/NEO-48.md)); [server README — Perk state (NEO-48)](../../../server/README.md#perk-state-neo-48); Bruno `bruno/neon-sprawl-server/perk-state/`. | [NEO-45](../../plans/NEO-45-implementation-plan.md), [NEO-46](../../plans/NEO-46-implementation-plan.md), [NEO-47](../../plans/NEO-47-implementation-plan.md), [NEO-48](../../plans/NEO-48-implementation-plan.md), [NEO-49](../../plans/NEO-49-implementation-plan.md), [E2M3-pre-production-backlog](../../plans/E2M3-pre-production-backlog.md), [E2_M3](E2_M3_MasteryAndPerkUnlocks.md) |
| E2.M2 | In Progress | **NEO-37 landed:** versioned **`GET /game/players/{id}/skill-progression`** ([NEO-37](../../plans/NEO-37-implementation-plan.md)) — read model for every registered skill; known-player gate via `IPositionStateStore`; [server README — Skill progression snapshot (NEO-37)](../../../server/README.md#skill-progression-snapshot-neo-37); manual QA [`NEO-37`](../../manual-qa/NEO-37.md). **NEO-38 landed:** **`POST`** same path — grant apply, persistence (`IPlayerSkillProgressionStore`, `V003` migration), structured denies + **`levelUps`** ([NEO-38](../../plans/NEO-38-implementation-plan.md)); manual QA [`NEO-38`](../../manual-qa/NEO-38.md); Bruno `bruno/neon-sprawl-server/skill-progression/`; [server README — Skill progression grant (NEO-38)](../../../server/README.md#skill-progression-grant-neo-38). **NEO-39 landed:** data-driven `LevelCurve` content + schema + CI validation (`*_level_curve.json`) with fail-fast startup schema checks; progression GET/POST level resolution now uses `ISkillLevelCurve` content-backed thresholds ([NEO-39](../../plans/NEO-39-implementation-plan.md)); manual QA [`NEO-39`](../../manual-qa/NEO-39.md). **NEO-40 landed:** comment-only telemetry hook sites in [`SkillProgressionSnapshotApi.cs`](../../../server/NeonSprawl.Server/Game/Skills/SkillProgressionSnapshotApi.cs) on **`POST …/skill-progression`** for future **`xp_grant`** / **`level_up`** ([NEO-40](../../plans/NEO-40-implementation-plan.md), [`NEO-40` manual QA](../../manual-qa/NEO-40.md)). **NEO-41 landed:** gather prototype — **`POST …/interact`** with **`kind: resource_node`** grants **`salvage`** + **`activity`** (10 XP) via [`SkillProgressionGrantOperations`](../../../server/NeonSprawl.Server/Game/Skills/SkillProgressionGrantOperations.cs) ([NEO-41](../../plans/NEO-41-implementation-plan.md), [`NEO-41` manual QA](../../manual-qa/NEO-41.md)); [server README — Interaction](../../../server/README.md#interaction-neo-9). **NEO-42 landed (prep):** **`RefineActivitySkillXpGrant`** / **`RefineSkillXpConstants`** + deny-registry factory + tests — same NEO-38 grant stack; **E3.M2** must invoke on craft/refine success ([NEO-42](../../plans/NEO-42-implementation-plan.md), [`NEO-42` manual QA](../../manual-qa/NEO-42.md)); [server README — Craft / refine hook (NEO-42)](../../../server/README.md#craft--refine-hook--skill-xp-neo-42). **NEO-43 landed (prep):** **`MissionRewardSkillXpGrant`** / **`MissionRewardSkillXpConstants`** + deny-registry factory + tests — same NEO-38 grant stack with fixed **`sourceKind: mission_reward`**; **E7.M2** must invoke from quest hand-in ([NEO-43](../../plans/NEO-43-implementation-plan.md), [`NEO-43` manual QA](../../manual-qa/NEO-43.md)); [server README — Mission / quest reward (NEO-43)](../../../server/README.md#mission--quest-reward--skill-xp-neo-43). **Slice 3 still open:** [NEO-44](https://linear.app/neon-sprawl/issue/NEO-44). See [epic_02 — E2.M2 + Slice 3](../epics/epic_02_skills_and_progression.md). | [NEO-37](../../plans/NEO-37-implementation-plan.md), [NEO-38](../../plans/NEO-38-implementation-plan.md), [NEO-39](../../plans/NEO-39-implementation-plan.md), [NEO-40](../../plans/NEO-40-implementation-plan.md), [NEO-41](../../plans/NEO-41-implementation-plan.md), [NEO-42](../../plans/NEO-42-implementation-plan.md), [NEO-43](../../plans/NEO-43-implementation-plan.md), [E2_M2](E2_M2_XpAwardAndLevelEngine.md); label **`E2.M2`** on NEO-37NEO-41, NEO-42NEO-44 |
| E3.M1 | In Progress | **NEO-41 landed (prototype):** `POST …/interact` success on **`resource_node`** (`prototype_resource_node_alpha`) applies **`salvage`** skill XP (**`sourceKind: activity`**, 10 XP) via shared NEO-38 grant operations. **Still planned:** `GatherResult`, yields, inventory per [E3_M1](E3_M1_ResourceNodeAndGatherLoop.md). | [NEO-41](../../plans/NEO-41-implementation-plan.md), [E3_M1](E3_M1_ResourceNodeAndGatherLoop.md); `server/NeonSprawl.Server/Game/Interaction/`, `Game/Skills/` |
| E3.M3 | In Progress | **NEO-50 landed:** frozen prototype six-item catalog in [`content/items/prototype_items.json`](../../../content/items/prototype_items.json); [`item-def.schema.json`](../../../content/schemas/item-def.schema.json); PR gate + [`validate_content.py`](../../../scripts/validate_content.py). **NEO-51 landed:** fail-fast server load of `content/items/*_items.json` at startup — `server/NeonSprawl.Server/Game/Items/` ([NEO-51](../../plans/NEO-51-implementation-plan.md)); [server README — Item catalog](../../../server/README.md#item-catalog-contentitems-neo-51). **NEO-52 landed:** injectable **`IItemDefinitionRegistry`** + lookup tests ([NEO-52](../../plans/NEO-52-implementation-plan.md)). **NEO-53 landed:** **`GET /game/world/item-definitions`** — `ItemDefinitionsWorldApi` + DTOs in `Game/Items/` ([NEO-53](../../plans/NEO-53-implementation-plan.md), [`NEO-53` manual QA](../../manual-qa/NEO-53.md)); [server README — Item definitions (NEO-53)](../../../server/README.md#item-definitions-neo-53); Bruno `bruno/neon-sprawl-server/item-definitions/`. **NEO-54 landed:** **`IPlayerInventoryStore`** + **`PlayerInventoryOperations`** — 24 bag + 1 equipment slots, stack rules, `V005` migration ([NEO-54](../../plans/NEO-54-implementation-plan.md)). **NEO-55 landed:** **`GET`/`POST /game/players/{id}/inventory`** — `PlayerInventoryApi` + DTOs in `Game/Items/` ([NEO-55](../../plans/NEO-55-implementation-plan.md)); [server README — Player inventory (NEO-54 store, NEO-55 HTTP)](../../../server/README.md#player-inventory-neo-54-store-neo-55-http); Bruno `bruno/neon-sprawl-server/inventory/`. **Still planned:** telemetry hook sites (NEO-56). | [NEO-50](../../plans/NEO-50-implementation-plan.md), [NEO-51](../../plans/NEO-51-implementation-plan.md), [NEO-52](../../plans/NEO-52-implementation-plan.md), [NEO-53](../../plans/NEO-53-implementation-plan.md), [NEO-54](../../plans/NEO-54-implementation-plan.md), [NEO-55](../../plans/NEO-55-implementation-plan.md), [E3M3-prototype-backlog](../../plans/E3M3-prototype-backlog.md), [E3_M3](E3_M3_ItemizationAndInventorySchema.md) |
| E3.M3 | In Progress | **NEO-50 landed:** frozen prototype six-item catalog in [`content/items/prototype_items.json`](../../../content/items/prototype_items.json); [`item-def.schema.json`](../../../content/schemas/item-def.schema.json); PR gate + [`validate_content.py`](../../../scripts/validate_content.py). **NEO-51 landed:** fail-fast server load of `content/items/*_items.json` at startup — `server/NeonSprawl.Server/Game/Items/` ([NEO-51](../../plans/NEO-51-implementation-plan.md)); [server README — Item catalog](../../../server/README.md#item-catalog-contentitems-neo-51). **NEO-52 landed:** injectable **`IItemDefinitionRegistry`** + lookup tests ([NEO-52](../../plans/NEO-52-implementation-plan.md)). **NEO-53 landed:** **`GET /game/world/item-definitions`** — `ItemDefinitionsWorldApi` + DTOs in `Game/Items/` ([NEO-53](../../plans/NEO-53-implementation-plan.md), [`NEO-53` manual QA](../../manual-qa/NEO-53.md)); [server README — Item definitions (NEO-53)](../../../server/README.md#item-definitions-neo-53); Bruno `bruno/neon-sprawl-server/item-definitions/`. **NEO-54 landed:** **`IPlayerInventoryStore`** + **`PlayerInventoryOperations`** — 24 bag + 1 equipment slots, stack rules, `V005` migration ([NEO-54](../../plans/NEO-54-implementation-plan.md)). **NEO-55 landed:** **`GET`/`POST /game/players/{id}/inventory`** — `PlayerInventoryApi` + DTOs in `Game/Items/` ([NEO-55](../../plans/NEO-55-implementation-plan.md)); [server README — Player inventory (NEO-54 store, NEO-55 HTTP)](../../../server/README.md#player-inventory-neo-54-store-neo-55-http); Bruno `bruno/neon-sprawl-server/inventory/`. **NEO-56 landed:** comment-only **`item_created`** / **`inventory_transfer_denied`** telemetry hook sites in [`PlayerInventoryOperations`](../../../server/NeonSprawl.Server/Game/Items/PlayerInventoryOperations.cs) ([NEO-56](../../plans/NEO-56-implementation-plan.md), [`NEO-56` manual QA](../../manual-qa/NEO-56.md)); no E9.M1 ingest. | [NEO-50](../../plans/NEO-50-implementation-plan.md), [NEO-51](../../plans/NEO-51-implementation-plan.md), [NEO-52](../../plans/NEO-52-implementation-plan.md), [NEO-53](../../plans/NEO-53-implementation-plan.md), [NEO-54](../../plans/NEO-54-implementation-plan.md), [NEO-55](../../plans/NEO-55-implementation-plan.md), [NEO-56](../../plans/NEO-56-implementation-plan.md), [E3M3-prototype-backlog](../../plans/E3M3-prototype-backlog.md), [E3_M3](E3_M3_ItemizationAndInventorySchema.md) |
---

View File

@ -48,7 +48,7 @@ Fleshed-out scope, contracts, and integration notes live in **per-module documen
| E3.M4 | SinkAndDurabilityLifecycle | E3.M3, E8.M3 | DurabilityState, ItemSinkEvent, RepairCostRule | Pre-production | Planned |
| E3.M5 | EconomyBalancePolicy | E3.M4, E9.M2 | EconomyPolicy, PriceBandRule, FaucetSinkRatio | Pre-production | Planned |
**E3.M3 note:** Epic 3 **Slice 1** backlog in Linear ([Epic 3 — Crafting, Gathering, and Itemization Economy](https://linear.app/neon-sprawl/project/epic-3-crafting-gathering-and-itemization-economy-65785ed05bc2)): [NEO-50](https://linear.app/neon-sprawl/issue/NEO-50) → [NEO-56](https://linear.app/neon-sprawl/issue/NEO-56); label **`E3.M3`**. See [E3M3-prototype-backlog.md](../../plans/E3M3-prototype-backlog.md), [E3_M3_ItemizationAndInventorySchema.md](E3_M3_ItemizationAndInventorySchema.md). **NEO-50** (content + CI), **NEO-51** (server fail-fast load), **NEO-52** (`IItemDefinitionRegistry` + DI), **NEO-53** (`GET /game/world/item-definitions`), **NEO-54** (inventory store + stack/slot rules engine), and **NEO-55** (`GET`/`POST /game/players/{id}/inventory`) moved the register row to **In Progress**; later slices update the alignment table as they land.
**E3.M3 note:** Epic 3 **Slice 1** backlog in Linear ([Epic 3 — Crafting, Gathering, and Itemization Economy](https://linear.app/neon-sprawl/project/epic-3-crafting-gathering-and-itemization-economy-65785ed05bc2)): [NEO-50](https://linear.app/neon-sprawl/issue/NEO-50) → [NEO-56](https://linear.app/neon-sprawl/issue/NEO-56); label **`E3.M3`**. See [E3M3-prototype-backlog.md](../../plans/E3M3-prototype-backlog.md), [E3_M3_ItemizationAndInventorySchema.md](E3_M3_ItemizationAndInventorySchema.md). **NEO-50** (content + CI), **NEO-51** (server fail-fast load), **NEO-52** (`IItemDefinitionRegistry` + DI), **NEO-53** (`GET /game/world/item-definitions`), **NEO-54** (inventory store + stack/slot rules engine), **NEO-55** (`GET`/`POST /game/players/{id}/inventory`), and **NEO-56** (comment-only `item_created` / `inventory_transfer_denied` telemetry hook sites in `PlayerInventoryOperations`) — Epic 3 Slice 1 backlog **E3M3-01****E3M3-07** complete; register row **In Progress** until Slice 2+; later slices update the alignment table as they land.
### Epic 4 — World Topology

View File

@ -0,0 +1,35 @@
# Manual QA — NEO-56 (inventory telemetry hook sites)
Reference: [implementation plan](../plans/NEO-56-implementation-plan.md), [NEO-54 implementation plan](../plans/NEO-54-implementation-plan.md) (inventory engine), [NEO-55 implementation plan](../plans/NEO-55-implementation-plan.md) (inventory HTTP).
## Preconditions
- Run `NeonSprawl.Server` (`dotnet run` from `server/NeonSprawl.Server`; default `http://localhost:5253`) if exercising Bruno; not required for code review only.
- This story adds **comments only**; inventory behavior matches NEO-54/NEO-55.
## Code review check
- [ ] Open `server/NeonSprawl.Server/Game/Items/PlayerInventoryOperations.cs`.
- [ ] **`TryAddStack`:** confirm **`item_created`** comment block immediately before **`Applied`** return (successful add only).
- [ ] **`TryAddStack` / `TryRemoveStack`:** confirm **`inventory_transfer_denied`** hook on post-mutation **`Denied`** returns (`inventory_full`, `insufficient_quantity`).
- [ ] **`Deny`:** confirm **`inventory_transfer_denied`** hook covers early validation denies (`invalid_quantity`, `invalid_item`).
- [ ] Confirm **`TODO(E9.M1)`** markers and planned payload fields; **no** new `ILogger` or metrics calls.
- [ ] Confirm **no** duplicate hook comments in `PlayerInventoryApi.cs`.
- [ ] Open `PlayerInventoryReasonCodes.cs` — summary cross-references NEO-56 hook anchor.
- [ ] Open [server README — Player inventory (NEO-54 store, NEO-55 HTTP)](../../server/README.md#player-inventory-neo-54-store-neo-55-http) — NEO-56 telemetry subsection present.
## Regression (optional)
```bash
cd server && dotnet test NeonSprawl.sln
```
Expect all tests pass (comments-only change).
## Bruno (optional sanity)
From `bruno/neon-sprawl-server/inventory/` against a running server:
- **Get inventory** — 200 empty grid for `dev-local-1`.
- **Post inventory add**`applied: true` unchanged.
- **Post inventory add deny invalid item**`applied: false`, `reasonCode: invalid_item` unchanged.

View File

@ -0,0 +1,116 @@
# NEO-56 — Implementation plan
## Story reference
| Field | Value |
|--------|--------|
| **Key** | NEO-56 |
| **Title** | E3.M3: item_created / transfer-failure telemetry hook sites |
| **Linear** | https://linear.app/neon-sprawl/issue/NEO-56/e3m3-item-created-transfer-failure-telemetry-hook-sites |
| **Module** | [E3.M3 — ItemizationAndInventorySchema](../decomposition/modules/E3_M3_ItemizationAndInventorySchema.md) · Epic 3 Slice 1 (E3M3-07) |
| **Branch** | `NEO-56-item-created-transfer-failure-telemetry-hooks` |
| **Precursors** | [NEO-54](https://linear.app/neon-sprawl/issue/NEO-54) — `PlayerInventoryOperations` (**Done**); [NEO-55](https://linear.app/neon-sprawl/issue/NEO-55) — inventory GET/POST HTTP (**Done**) |
| **Pattern** | [NEO-49](https://linear.app/neon-sprawl/issue/NEO-49) / [NEO-40](https://linear.app/neon-sprawl/issue/NEO-40) — comment-only engine anchors; [NEO-30](https://linear.app/neon-sprawl/issue/NEO-30) — deny event + `reasonCode` |
## Kickoff clarifications
| Topic | Question | Agent recommendation | Answer |
|--------|----------|----------------------|--------|
| **Hook anchor** | Engine only vs engine + HTTP? | **Engine only**`PlayerInventoryOperations` (NEO-49 single-anchor precedent; NEO-55 deferred hooks here). | **User:** engine only. |
| **Deny event shape** | Per-reason events vs single deny + `reasonCode`? | **`inventory_transfer_denied` + `reasonCode`** (NEO-30 `ability_cast_denied`; epic `inventory_full` maps to a reason code). | **User:** single deny + `reasonCode`. |
| **`item_created` scope** | Add success only vs add + remove? | **Successful add only** (`TryAddStack` → `Applied`). | **User:** add success only. |
| **Runtime behavior** | Comments-only vs dev `ILogger`? | **Comments-only**`TODO(E9.M1)`; no production logging. | **User:** comments-only. |
| **Manual QA doc** | `docs/manual-qa/NEO-56.md`? | **Yes** — NEO-49 verification pattern. | **User:** yes. |
## Goal, scope, and out-of-scope
**Goal:** Document and place **comment-only hook sites** on the server-authoritative inventory mutation path for future E9.M1 catalog events **`item_created`** (successful add) and **`inventory_transfer_denied`** (transfer failures with stable `reasonCode`). Align with Epic 3 Slice 1 telemetry vocabulary ([epic_03 — Slice 1](../decomposition/epics/epic_03_crafting_economy.md)).
**In scope (from Linear + [E3M3-07](E3M3-prototype-backlog.md#e3m3-07--item_created--transfer-failure-telemetry-hook-sites)):**
- Hook placement in **`PlayerInventoryOperations`** (`TryAddStack` / `TryRemoveStack`) with **`TODO(E9.M1)`** comments.
- **`server/README.md`** inventory subsection + [E3_M3 module](../decomposition/modules/E3_M3_ItemizationAndInventorySchema.md) hook note.
- **`docs/manual-qa/NEO-56.md`** — verify comments + no behavior/API change.
**Out of scope (from Linear):**
- Telemetry ingest, dashboards (**E9.M1**).
- `ILogger` / metrics / dev-only log lines.
- Duplicate hook comments in **`PlayerInventoryApi`** (HTTP calls engine; NEO-55 plan deferred hooks to NEO-56).
- **`item_created`** on successful **remove** (kickoff decision).
- **`StoreMissing`** hooks (not a transfer failure — HTTP maps to **404**).
## Acceptance criteria checklist
- [x] Hook sites documented in code (`PlayerInventoryOperations`) with **`TODO(E9.M1)`**.
- [x] **`item_created`** on successful add; **`inventory_transfer_denied`** + `reasonCode` on rule denies (including early validation denies).
- [x] `server/README.md` documents hook placement.
- [x] Matches Epic 3 Slice 1 vocabulary (`item_created`, `inventory_full` / transfer failures via `reasonCode`).
## Technical approach
1. **Authoritative surface:** `PlayerInventoryOperations` — all inventory add/remove rules and outcomes. Callers (`PlayerInventoryApi` POST, future gather/craft) funnel here; **no** duplicate markers at HTTP.
2. **Hook site A — `item_created`:** In **`TryAddStack`**, immediately before returning **`PlayerInventoryMutationKind.Applied`** (after successful `TryMutateSnapshot` with no `denyReason`):
- Comment block names future E9.M1 event **`item_created`**.
- **`TODO(E9.M1): catalog emit`** (NEO-49 / NEO-30 style).
- Planned payload fields: `playerId`, `itemId`, `quantity`, optional `mutationKind`/`source` when Slice 2+ callers exist.
- **Not** on **`TryRemoveStack`** success (kickoff).
3. **Hook site B — `inventory_transfer_denied`:** On every **`Denied`** return path in **`TryAddStack`** and **`TryRemoveStack`**:
- Early **`Deny(...)`** (e.g. `invalid_quantity`, `invalid_item` before store write).
- Post-mutation deny when `denyReason` is set (`inventory_full`, `insufficient_quantity`).
- Single comment pattern: catalog event **`inventory_transfer_denied`** + **`reasonCode`** from [`PlayerInventoryReasonCodes`](../../../server/NeonSprawl.Server/Game/Items/PlayerInventoryReasonCodes.cs) (`inventory_full`, `invalid_item`, `insufficient_quantity`, `invalid_quantity`).
- Epic Slice 1 **`inventory_full`** is the flagship transfer-failure code; others share the same deny event shape (NEO-30 precedent).
4. **No hook on `StoreMissing`:** Outcome is missing player/store — not an inventory transfer rule deny.
5. **Cross-references (minimal):**
- One-line pointer in `PlayerInventoryMutationOutcome.cs` or `PlayerInventoryReasonCodes.cs` xml summary → hook sites in `PlayerInventoryOperations`.
- **`server/README.md`:** extend **Player inventory (NEO-54 store, NEO-55 HTTP)** with **NEO-56 telemetry hooks** subsection (anchor file + event names).
- **`E3_M3_ItemizationAndInventorySchema.md`:** **NEO-56 landed** line under Related implementation slices.
- **`documentation_and_implementation_alignment.md`:** E3.M3 row cites NEO-56 hook sites (still no E9.M1 ingest).
6. **Regression:** Run `dotnet test` on server solution; existing `PlayerInventoryOperationsTests` / `PlayerInventoryApiTests` unchanged (comments-only).
## Files to add
| Path | Purpose |
|------|---------|
| `docs/manual-qa/NEO-56.md` | Manual QA: verify hook comment blocks + no API/behavior change. |
## Files to modify
| Path | Rationale |
|------|-----------|
| `server/NeonSprawl.Server/Game/Items/PlayerInventoryOperations.cs` | **NEO-56:** `item_created` on add `Applied`; `inventory_transfer_denied` on all `Denied` paths in add/remove. |
| `server/NeonSprawl.Server/Game/Items/PlayerInventoryReasonCodes.cs` | Cross-reference deny `reasonCode` values → transfer-denied hook anchor (no behavior change). |
| `server/README.md` | Document inventory telemetry hook placement (NEO-56). |
| `docs/decomposition/modules/E3_M3_ItemizationAndInventorySchema.md` | Implementation snapshot: NEO-56 telemetry hooks. |
| `docs/decomposition/modules/documentation_and_implementation_alignment.md` | E3.M3 row: NEO-56 hook sites. |
## Tests
| Path | Change |
|------|--------|
| **None required** | Comments-only; existing `PlayerInventoryOperationsTests` and `PlayerInventoryApiTests` cover add/remove/deny paths. |
**Manual verification:** `docs/manual-qa/NEO-56.md` — confirm **`item_created`** and **`inventory_transfer_denied`** comments, **`TODO(E9.M1)`**, payload field notes, no new log statements; `dotnet test` regression.
## Open questions / risks
| Question / risk | Agent recommendation | Status |
|-----------------|----------------------|--------|
| **Future gather/craft callers** | Hooks stay in engine; callers pass `source`/`mutationKind` in E9.M1 payload when catalog exists — document in hook comments as optional fields. | `adopted` |
| **HTTP request correlation** | Defer `requestId` to E9.M1 / future sink; do not add API-layer hooks (kickoff: engine only). | `adopted` |
| **Partial stack placement on add** | NEO-54 all-or-nothing add — single `item_created` emit for full requested quantity on `Applied`. | `adopted` |
None blocking.
## Decisions (post-review)
| Topic | Decision |
|--------|----------|
| **PR scope** | Branch includes chore `3d43b00` (kickoff `AskQuestion` + recommendation rule hardening). Intentional tooling hygiene on the story branch; call out in PR description if reviewers want story-only diff vs `main`. |
| **Post-mutation deny hooks** | Expanded to multi-line NEO-49-style blocks with payload fields (`mutationKind=add` / `remove`) per code review. |
| **Dependency register** | E3.M3 footnote extended with NEO-56 to match alignment table. |

View File

@ -0,0 +1,56 @@
# Code review — NEO-56 inventory telemetry hook sites
**Date:** 2026-05-24
**Scope:** Branch `NEO-56-item-created-transfer-failure-telemetry-hooks` · commits `786513b``dc5141e` vs `origin/main` (includes chore `3d43b00` kickoff rule hardening)
**Base:** `origin/main`
## Verdict
**Approve with nits**
## Summary
NEO-56 adds **comment-only** E9.M1 telemetry anchor sites in `PlayerInventoryOperations`: **`item_created`** before successful add (`Applied`), and **`inventory_transfer_denied`** + **`reasonCode`** on every rule **`Denied`** path (centralized early validation in **`Deny`**, inline markers on post-mutation denies). No runtime behavior, API, or test changes; docs (plan, manual QA, README, E3.M3 slice, alignment table) match kickoff decisions (engine-only, add-success only, single deny event shape, comments-only). Risk is very low for merge. The branch also carries an unrelated **kickoff rule** hardening commit (`story-kickoff.md`, `AGENTS.md`) — fine if intentional, otherwise consider splitting before PR.
## Documentation checked
| Document | Result |
|----------|--------|
| [`docs/plans/NEO-56-implementation-plan.md`](../plans/NEO-56-implementation-plan.md) | **Matches** — engine-only hooks, kickoff clarifications recorded, acceptance checklist complete; comments-only regression noted. |
| [`docs/decomposition/modules/E3_M3_ItemizationAndInventorySchema.md`](../decomposition/modules/E3_M3_ItemizationAndInventorySchema.md) | **Matches** — NEO-56 telemetry bullet under **Related implementation slices** with README + manual QA links. |
| [`docs/decomposition/modules/documentation_and_implementation_alignment.md`](../decomposition/modules/documentation_and_implementation_alignment.md) | **Matches** — E3.M3 row cites NEO-56 landed hook sites; no E9.M1 ingest. |
| [`docs/decomposition/modules/module_dependency_register.md`](../decomposition/modules/module_dependency_register.md) | **Matches** — E3.M3 footnote includes NEO-56 telemetry hooks; Slice 1 backlog E3M3-0107 noted complete. |
| [`docs/decomposition/epics/epic_03_crafting_economy.md`](../decomposition/epics/epic_03_crafting_economy.md) | **Matches** — Slice 1 vocabulary `item_created`, `inventory_full` / transfer failures via stable `reasonCode`. |
| [`docs/manual-qa/NEO-56.md`](../manual-qa/NEO-56.md) | **Matches** — code-review checklist, regression command, optional Bruno sanity. |
| [`server/README.md`](../../server/README.md) | **Matches** — NEO-56 telemetry subsection under Player inventory. |
| [`docs/decomposition/modules/client_server_authority.md`](../decomposition/modules/client_server_authority.md) | **Matches** — hooks remain on server-authoritative engine; no HTTP-layer duplication. |
Register/tracking: alignment table, E3.M3 module page, and dependency register footnote updated for NEO-56.
## Blocking issues
None.
## Suggestions
1. ~~**Dependency register footnote (optional)** — Extend the **E3.M3 note** in [`module_dependency_register.md`](../decomposition/modules/module_dependency_register.md) to mention **NEO-56** (telemetry hook sites) alongside NEO-50NEO-55, matching the alignment table and NEO-55 review pattern.~~ **Done.**
2. ~~**PR scope clarity (optional)** — Commit `3d43b00` (kickoff `AskQuestion` + recommendation rule) is unrelated to NEO-56 inventory hooks. If the PR should be story-scoped only, cherry-pick or split; if bundled tooling hygiene is intentional, call it out in the PR description.~~ **Done.** — documented in [NEO-56 plan](../plans/NEO-56-implementation-plan.md) **Decisions (post-review)**; mention in PR body when opening.
## Nits
- Nit: Plan allowed a cross-reference in `PlayerInventoryMutationOutcome.cs` **or** `PlayerInventoryReasonCodes.cs`; only ReasonCodes was updated — acceptable (“or” satisfied).
- ~~Nit: Post-mutation **`Denied`** hooks (lines ~54, ~124 in `PlayerInventoryOperations.cs`) use a one-line marker; **`Deny`** and **`item_created`** use fuller multi-line blocks (NEO-49 style). Consistent enough with NEO-30 deny precedent; optional payload-field notes on post-mutation paths would mirror **`Deny`** for E9.M1 implementers.~~ **Done.** — multi-line blocks with payload fields on add/remove post-mutation denies.
- ~~Nit: **`Deny`** hook comments list planned payload fields `itemId`, `quantity`, `mutationKind`, but the method signature does not carry those today — fine for comments-only; E9.M1 will likely extend **`Deny`** or pass a context struct when wiring emit.~~ **Done.** — comment clarifies emit call sites supply itemId/quantity/mutationKind.
## Verification
```bash
cd server
dotnet test NeonSprawl.Server.Tests/NeonSprawl.Server.Tests.csproj 2>/dev/null || dotnet test NeonSprawl.sln
dotnet test NeonSprawl.Server.Tests/NeonSprawl.Server.Tests.csproj --filter "FullyQualifiedName~PlayerInventory"
```
Manual: walk [`docs/manual-qa/NEO-56.md`](../manual-qa/NEO-56.md) code-review checklist (confirm no hooks in `PlayerInventoryApi.cs`, `TODO(E9.M1)` present, no new logging).

View File

@ -1,6 +1,6 @@
namespace NeonSprawl.Server.Game.Items;
/// <summary>Server-authoritative inventory add/remove rules (NEO-54).</summary>
/// <summary>Server-authoritative inventory add/remove rules (NEO-54). NEO-56 telemetry hook sites: <see cref="TryAddStack"/>, <see cref="TryRemoveStack"/>, and <see cref="Deny"/>.</summary>
public static class PlayerInventoryOperations
{
/// <summary>
@ -51,9 +51,17 @@ public static class PlayerInventoryOperations
if (denyReason is not null)
{
// --- Telemetry hook site (NEO-56): future E9.M1 catalog event `inventory_transfer_denied` ---
// TODO(E9.M1): catalog emit — reasonCode (e.g. inventory_full). Planned payload: playerId, itemId, quantity,
// reasonCode, mutationKind=add; optional source when Slice 2+ callers exist. No ingest or ILogger here (comments-only).
return new PlayerInventoryMutationOutcome(PlayerInventoryMutationKind.Denied, denyReason, result);
}
// --- Telemetry hook site (NEO-56): future E9.M1 catalog event `item_created` ---
// TODO(E9.M1): catalog emit — successful add only (not remove). Planned payload: playerId, itemId, quantity;
// optional mutationKind/source when gather/craft callers exist. No ingest or ILogger here (comments-only).
return new PlayerInventoryMutationOutcome(PlayerInventoryMutationKind.Applied, null, result);
}
@ -116,6 +124,10 @@ public static class PlayerInventoryOperations
if (denyReason is not null)
{
// --- Telemetry hook site (NEO-56): future E9.M1 catalog event `inventory_transfer_denied` ---
// TODO(E9.M1): catalog emit — reasonCode (e.g. insufficient_quantity). Planned payload: playerId, itemId, quantity,
// reasonCode, mutationKind=remove; optional source when Slice 2+ callers exist. No ingest or ILogger here (comments-only).
return new PlayerInventoryMutationOutcome(PlayerInventoryMutationKind.Denied, denyReason, result);
}
@ -124,6 +136,11 @@ public static class PlayerInventoryOperations
private static PlayerInventoryMutationOutcome Deny(string playerId, IPlayerInventoryStore store, string reasonCode)
{
// --- Telemetry hook site (NEO-56): future E9.M1 catalog event `inventory_transfer_denied` ---
// TODO(E9.M1): catalog emit — reasonCode from PlayerInventoryReasonCodes (invalid_quantity, invalid_item, etc.).
// Planned payload: playerId, reasonCode; itemId, quantity, mutationKind (add|remove) at emit call sites (not on Deny signature today).
// Optional source when Slice 2+ callers exist. No ingest or ILogger here (comments-only).
if (store.TryGetSnapshot(playerId, out var snapshot))
{
return new PlayerInventoryMutationOutcome(PlayerInventoryMutationKind.Denied, reasonCode, snapshot);

View File

@ -1,6 +1,9 @@
namespace NeonSprawl.Server.Game.Items;
/// <summary>Stable deny reason codes for inventory mutations (NEO-54).</summary>
/// <summary>
/// Stable deny reason codes for inventory mutations (NEO-54).
/// NEO-56: each code maps to <c>inventory_transfer_denied</c> telemetry at hook sites in <see cref="PlayerInventoryOperations"/>.
/// </summary>
public static class PlayerInventoryReasonCodes
{
public const string InventoryFull = "inventory_full";

View File

@ -89,6 +89,8 @@ curl -sS -X POST "http://localhost:5253/game/players/dev-local-1/inventory" \
**Persistence:** same **[NEO-29-style](#position-persistence-neo-8) split** as skill progression — **`player_inventory`** in PostgreSQL when **`ConnectionStrings:NeonSprawl`** is set (DDL [`V005__player_inventory.sql`](../db/migrations/V005__player_inventory.sql), sparse occupied-slot rows), otherwise in-memory fallback seeding the configured dev player. Store/engine plan: [NEO-54 implementation plan](../../docs/plans/NEO-54-implementation-plan.md).
**NEO-56 telemetry hooks:** comment-only hook sites in **`PlayerInventoryOperations`** for future E9.M1 catalog events — **`item_created`** on successful **`TryAddStack`** (`Applied`); **`inventory_transfer_denied`** + stable **`reasonCode`** on every rule **`Denied`** path (early validation via **`Deny`** and post-mutation denies such as **`inventory_full`** / **`insufficient_quantity`**). **`TODO(E9.M1)`** — no production ingest. HTTP **`POST …/inventory`** calls the engine only (no duplicate API-layer hooks). Manual QA: [`docs/manual-qa/NEO-56.md`](../../docs/manual-qa/NEO-56.md); plan: [NEO-56 implementation plan](../../docs/plans/NEO-56-implementation-plan.md).
## Mastery catalog (`content/mastery`, NEO-46)
After the skill catalog loads, the host loads every **`*_mastery.json`** under the mastery directory, validates each file against **`content/schemas/mastery-catalog.schema.json`**, cross-checks track **`skillId`** values against **`ISkillDefinitionRegistry`**, and enforces the same post-schema gates as **`scripts/validate_content.py`** (unknown perk references, duplicate perk ids, branch-set equality tier-to-tier, unreferenced perks, prototype **Slice 4** single **`salvage`** track). The server also requires **`tierIndex`** values per track to be **unique and sequential 1..N** (stricter than CI today). Invalid data **exits during startup**—no silent fallback.