diff --git a/docs/plans/NEO-136-implementation-plan.md b/docs/plans/NEO-136-implementation-plan.md new file mode 100644 index 0000000..e8a95b7 --- /dev/null +++ b/docs/plans/NEO-136-implementation-plan.md @@ -0,0 +1,152 @@ +# NEO-136 — E7M3-04: ReputationOperations (apply ReputationDelta) + +**Linear:** [NEO-136](https://linear.app/neon-sprawl/issue/NEO-136) +**Branch:** `NEO-136-e7m3-reputation-operations-apply-delta` +**Backlog:** [E7M3-pre-production-backlog.md](E7M3-pre-production-backlog.md) — **E7M3-04** +**Module:** [E7_M3_FactionReputationLedger.md](../decomposition/modules/E7_M3_FactionReputationLedger.md) +**Pattern:** [NEO-105](NEO-105-implementation-plan.md) / `EncounterCompletionOperations` — static ops + reason codes + compensating rollback; [NEO-127](NEO-127-implementation-plan.md) — orchestrate store writes in order +**Precursor:** [NEO-135](https://linear.app/neon-sprawl/issue/NEO-135) **`Done`** — `IFactionStandingStore`, `IReputationDeltaStore`, in-memory + Postgres `V009`/`V010` (**landed on `main`**) +**Blocks:** [NEO-138](https://linear.app/neon-sprawl/issue/NEO-138) (reward bundle rep wiring), [NEO-141](https://linear.app/neon-sprawl/issue/NEO-141) (telemetry hook sites) + +## Goal + +Server-internal **`ReputationOperations.TryApplyDelta`** orchestrates standing mutation + append-only audit so game code never calls **`IFactionStandingStore.TryApplyStandingDelta`** alone. Apply clamps to faction min/max, records one **`ReputationDelta`** row on success, and returns structured outcomes. + +## Kickoff clarifications + +**No clarifications needed.** Linear AC, [E7M3-04 backlog scope](E7M3-pre-production-backlog.md#e7m3-04--reputationoperations-apply-reputationdelta), NEO-135 handoff notes, and NEO-62/NEO-105/NEO-127 precedents settle: + +| Topic | Decision | Evidence | +|-------|----------|----------| +| Orchestration shape | **Static `ReputationOperations`** with injected stores + `TimeProvider` | `CraftOperations`, `RewardRouterOperations`, `EncounterCompletionOperations` precedent | +| Faction validation | **Delegate to `IFactionStandingStore`** (already fail-closed via `IFactionDefinitionRegistry`) | Backlog “validates faction id via registry”; store implements that at boundary — no duplicate registry inject | +| `invalid_delta` | **Deny `deltaAmount == 0`** with reason code **`invalid_delta`** at ops layer before store | NEO-135 deferred constant to NEO-136; `SkillProgressionGrantOperations` denies `amount <= 0` | +| Audit row id | **Caller supplies non-empty `deltaId`** (UUID string) | NEO-135 adopted decision table: “UUID string generated by caller (NEO-136)” | +| Source attribution | **Require non-empty `sourceKind` + `sourceId`**; prototype constant `quest_completion` from `ReputationDeltaSourceKinds` | NEO-135 audit row shape; NEO-138 passes quest id as `sourceId` | +| Commit order | **Standing apply first, audit append second** | `IReputationDeltaStore` XML + `server/README.md` Postgres FK remark | +| Audit append failure | **Compensating standing rollback** — inverse delta via `TryApplyStandingDelta` when `TryAppend` returns `false` | NEO-62/NEO-69/NEO-105 compensating rollback precedent; avoid standing without audit | +| Result type | **New `ReputationApplyOutcome`** (success, reason, before/after standing, optional persisted audit row) | Store outcomes lack ops-level codes (`invalid_delta`, `audit_append_failed`) | +| HTTP / quest wiring / Godot | **Out of scope** | Backlog E7M3-04; NEO-138 wires `RewardRouterOperations` | +| Bruno / manual QA | **None** — server engine only | Backlog client counterpart: none | + +## Scope and out-of-scope + +**In scope (from Linear + backlog):** + +- **`ReputationOperations.TryApplyDelta`** in `server/NeonSprawl.Server/Game/Factions/`. +- **`ReputationApplyOutcome`**, **`ReputationApplyReasonCodes`** (`invalid_delta`, `invalid_delta_id`, `invalid_source`, `audit_append_failed`; passthrough `unknown_faction`, `player_not_writable` from standing store). +- Add **`invalid_delta`** to **`FactionStandingReasonCodes`** only if store layer needs it later — **prefer ops-only `ReputationApplyReasonCodes`** to keep store contract unchanged. +- Unit tests (AAA): happy path (+15 from 0, audit row queryable), clamp at min/max via ops, unknown faction deny, `deltaAmount == 0` deny, duplicate `deltaId` audit deny with standing unchanged (rollback), empty `deltaId` / source deny. +- Host DI smoke: resolve stores + one ops apply through factory services (extend existing faction store host test or dedicated ops test). +- `server/README.md` **`ReputationOperations`** section (callers must use ops, not standing store alone). + +**Out of scope:** + +- Quest bundle wiring via **`RewardRouterOperations`** (NEO-138 / E7M3-06). +- Quest accept gate evaluation (NEO-137). +- HTTP GET standing (NEO-139), telemetry hook comments (NEO-141), Godot (NEO-142+). +- Postgres-specific integration test for ops (store persistence already covered by NEO-135; ops tests use in-memory stores). + +**Client counterpart:** none (server engine). + +## Acceptance criteria checklist + +- [ ] Delta apply updates standing once and records audit row. +- [ ] Standing never exceeds faction min/max after apply. +- [ ] `dotnet test` covers ops happy path, clamp, unknown faction deny, and invalid-delta deny. + +## Technical approach + +### 1. Types (`Game/Factions/`) + +| Type | Role | +|------|------| +| `ReputationApplyReasonCodes` | `invalid_delta`, `invalid_delta_id`, `invalid_source`, `audit_append_failed`; passthrough store codes | +| `ReputationApplyOutcome` | `Success`, `ReasonCode`, `PreviousStanding`, `NewStanding`, optional `AuditRow` (`ReputationDeltaRow`) | + +### 2. `ReputationOperations.TryApplyDelta` + +Signature (static, mirror other ops): + +```csharp +public static ReputationApplyOutcome TryApplyDelta( + string playerId, + string factionId, + int deltaAmount, + string deltaId, + string sourceKind, + string sourceId, + IFactionStandingStore standingStore, + IReputationDeltaStore auditStore, + TimeProvider timeProvider) +``` + +**Flow:** + +1. Normalize ids via **`FactionStandingIds`** (reuse store normalization rules). +2. **Pre-flight denies** (no store mutation): + - `deltaAmount == 0` → `invalid_delta` + - empty `deltaId` → `invalid_delta_id` + - empty `sourceKind` or `sourceId` → `invalid_source` +3. **`standingStore.TryApplyStandingDelta(playerId, factionId, deltaAmount)`** — on deny, return outcome with store **`ReasonCode`** and zeroed standings. +4. Build **`ReputationDeltaRow`** with caller `deltaId`, applied delta, `NewStanding` from mutation outcome, normalized source fields, **`timeProvider.GetUtcNow()`**. +5. **`auditStore.TryAppend(row)`** — on `true`, return success with audit row. +6. On append `false`: **compensating rollback** — `standingStore.TryApplyStandingDelta(playerId, factionId, -deltaAmount)`; return deny with **`audit_append_failed`** (standing restored to pre-apply if rollback succeeds; document prototype risk if rollback fails — extremely unlikely after fresh apply). + +XML remarks: consumed by **`RewardRouterOperations`** (NEO-138) and future admin tools; not HTTP directly. NEO-141 telemetry hook site on successful apply (comment-only in NEO-141). + +### 3. Tests + +**`ReputationOperationsTests`** (in-memory stores + prototype faction registry helper mirroring `InMemoryFactionStandingStoreTests`): + +| Test | Covers | +|------|--------| +| `TryApplyDelta_ShouldApplyStandingAndAppendAudit_WhenHappyPath` | +15 from 0; audit row fields; `GetDeltasForPlayer` | +| `TryApplyDelta_ShouldClampToMax_WhenDeltaExceedsBand` | Standing 95 + 50 → 100; audit `ResultingStanding` 100 | +| `TryApplyDelta_ShouldClampToMin_WhenDeltaBelowBand` | Standing -95 + (-50) → -100 | +| `TryApplyDelta_ShouldDenyUnknownFaction_WithStructuredReason` | No standing change; no audit rows | +| `TryApplyDelta_ShouldDenyInvalidDelta_WhenAmountZero` | `invalid_delta` | +| `TryApplyDelta_ShouldDenyInvalidDeltaId_WhenEmpty` | `invalid_delta_id` | +| `TryApplyDelta_ShouldDenyInvalidSource_WhenEmpty` | `invalid_source` | +| `TryApplyDelta_ShouldRollbackStanding_WhenAuditAppendFails` | Pre-append duplicate id; standing unchanged after deny | +| `TryApplyDelta_ShouldDenyNonWritablePlayer` | `player_not_writable` passthrough | + +**Host DI smoke:** one test resolving stores from `InMemoryWebApplicationFactory` and applying via `ReputationOperations` (can live in same test file). + +Manual QA: **none** (infrastructure-only). + +## Files to add + +| Path | Purpose | +|------|---------| +| `server/NeonSprawl.Server/Game/Factions/ReputationApplyReasonCodes.cs` | Ops-level stable deny codes including `invalid_delta` | +| `server/NeonSprawl.Server/Game/Factions/ReputationApplyOutcome.cs` | Structured apply result with optional audit row | +| `server/NeonSprawl.Server/Game/Factions/ReputationOperations.cs` | `TryApplyDelta` orchestration + compensating rollback | +| `server/NeonSprawl.Server.Tests/Game/Factions/ReputationOperationsTests.cs` | AAA unit + host DI smoke tests | + +## Files to modify + +| Path | Rationale | +|------|-----------| +| `server/README.md` | Document `ReputationOperations.TryApplyDelta`, commit order, rollback policy, and “do not call standing store alone for game apply” | +| `docs/decomposition/modules/E7_M3_FactionReputationLedger.md` | Status line: E7M3-04 in progress / landed when shipped | +| `docs/decomposition/modules/documentation_and_implementation_alignment.md` | Register NEO-136 plan + shipped ops when complete | + +## Tests + +| File | Action | Coverage | +|------|--------|----------| +| `server/NeonSprawl.Server.Tests/Game/Factions/ReputationOperationsTests.cs` | **Add** | Happy path, clamp min/max, unknown faction, invalid delta/id/source, audit-append rollback, non-writable player, host DI smoke | + +No new Postgres integration tests — NEO-135 already covers store persistence; ops layer is store-agnostic. + +## Open questions / risks + +| Question / risk | Agent recommendation | Status | +|-----------------|----------------------|--------| +| Standing rollback fails after audit append deny | Document as prototype invariant violation (same as NEO-62 depletion edge); no cross-store transaction | `deferred` | +| Negative `deltaAmount` for future rep loss | **Allow** signed delta at ops layer; content grants are positive only | `adopted` | +| Caller-generated `deltaId` vs auto UUID | **Caller supplies id** per NEO-135; NEO-138 can derive from delivery key + faction | `adopted` | +| Duplicate `deltaId` on retry after rollback | Append fails → rollback → caller may retry with **new** id; quest idempotency at delivery store prevents double call in NEO-138 | `adopted` | + +**NEO-138** (`RewardRouterOperations` rep grants) and **NEO-141** (telemetry comments) depend on this orchestration landing first.