diff --git a/docs/decomposition/modules/module_dependency_register.md b/docs/decomposition/modules/module_dependency_register.md index 6b59763..d770858 100644 --- a/docs/decomposition/modules/module_dependency_register.md +++ b/docs/decomposition/modules/module_dependency_register.md @@ -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 diff --git a/docs/plans/NEO-56-implementation-plan.md b/docs/plans/NEO-56-implementation-plan.md index ef4dab0..adc1d6c 100644 --- a/docs/plans/NEO-56-implementation-plan.md +++ b/docs/plans/NEO-56-implementation-plan.md @@ -106,3 +106,11 @@ | **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. | diff --git a/docs/reviews/2026-05-24-NEO-56.md b/docs/reviews/2026-05-24-NEO-56.md index 2092aa1..b7b7660 100644 --- a/docs/reviews/2026-05-24-NEO-56.md +++ b/docs/reviews/2026-05-24-NEO-56.md @@ -19,13 +19,13 @@ NEO-56 adds **comment-only** E9.M1 telemetry anchor sites in `PlayerInventoryOpe | [`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) | **Partially matches** — E3.M3 note lists NEO-50→NEO-56 range but footnote still stops at NEO-55 for “moved the register row” (see Suggestion 1). | +| [`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-01–07 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 and E3.M3 module page updated for NEO-56; dependency register footnote could add NEO-56 for parity. +Register/tracking: alignment table, E3.M3 module page, and dependency register footnote updated for NEO-56. ## Blocking issues @@ -33,17 +33,17 @@ 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-50–NEO-55, matching the alignment table and NEO-55 review pattern. +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-50–NEO-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. +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. +- ~~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. +- ~~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 diff --git a/server/NeonSprawl.Server/Game/Items/PlayerInventoryOperations.cs b/server/NeonSprawl.Server/Game/Items/PlayerInventoryOperations.cs index 38145e1..d6b4bd4 100644 --- a/server/NeonSprawl.Server/Game/Items/PlayerInventoryOperations.cs +++ b/server/NeonSprawl.Server/Game/Items/PlayerInventoryOperations.cs @@ -51,7 +51,10 @@ public static class PlayerInventoryOperations if (denyReason is not null) { - // NEO-56 telemetry hook site: `inventory_transfer_denied` + reasonCode (TODO(E9.M1): catalog emit). + // --- 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); } @@ -121,7 +124,10 @@ public static class PlayerInventoryOperations if (denyReason is not null) { - // NEO-56 telemetry hook site: `inventory_transfer_denied` + reasonCode (TODO(E9.M1): catalog emit). + // --- 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); } @@ -132,8 +138,8 @@ public static class PlayerInventoryOperations { // --- 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); optional source when Slice 2+ callers exist. - // No ingest or ILogger here (comments-only). + // 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)) {