NEO-136: add implementation plan for ReputationOperations
parent
786a5b6d68
commit
e9abc8bf38
|
|
@ -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.
|
||||||
Loading…
Reference in New Issue