Move entity rehydration to domain layer, fix tempHp gap
Rehydration functions (reconstructing typed domain objects from untyped JSON) lived in persistence adapters, duplicating domain validation. Adding a field required updating both the domain type and a separate adapter function — the adapter was missed for `level`, silently dropping it on reload. Now adding a field only requires updating the domain type and its co-located rehydration function. - Add `rehydratePlayerCharacter` and `rehydrateCombatant` to domain - Persistence adapters delegate to domain instead of reimplementing - Add `tempHp` validation (was silently dropped during rehydration) - Tighten initiative validation to integer-only - Exhaustive domain tests (53 cases); adapter tests slimmed to round-trip - Remove stale `jsinspect-plus` Knip ignoreDependencies entry Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
Lukas
238
docs/agents/research/2026-03-28-entity-rehydration.md
Normal file
238
docs/agents/research/2026-03-28-entity-rehydration.md
Normal file
@@ -0,0 +1,238 @@
|
||||
---
|
||||
date: 2026-03-28T01:35:07.925247+00:00
|
||||
git_commit: f4fb69dbc763fefe4a73b3491c27093bbd06af0d
|
||||
branch: main
|
||||
topic: "Entity rehydration: current implementation and migration surface"
|
||||
tags: [research, codebase, rehydration, persistence, domain, player-character, combatant]
|
||||
status: complete
|
||||
---
|
||||
|
||||
# Research: Entity Rehydration — Current Implementation and Migration Surface
|
||||
|
||||
## Research Question
|
||||
|
||||
Map all entity rehydration logic (reconstructing typed domain objects from untyped JSON) across the codebase. Document what validation each rehydration function performs, where it lives, how functions cross-reference each other, and what the domain layer already provides that could replace adapter-level validation. This research supports Issue #20: Move entity rehydration to domain layer.
|
||||
|
||||
## Summary
|
||||
|
||||
Entity rehydration currently lives entirely in `apps/web/src/persistence/`. Two primary rehydration functions exist:
|
||||
|
||||
1. **`rehydrateCharacter`** in `player-character-storage.ts` — validates and reconstructs `PlayerCharacter` from unknown JSON
|
||||
2. **`rehydrateCombatant`** (private) + **`rehydrateEncounter`** (exported) in `encounter-storage.ts` — validates and reconstructs `Combatant`/`Encounter` from unknown JSON
|
||||
|
||||
These are consumed by three call sites: localStorage loading, undo/redo stack loading, and JSON import validation. The domain layer already contains parallel validation logic in `createPlayerCharacter`, `addCombatant`/`validateInit`, and `createEncounter`, but the rehydration functions duplicate this validation with subtly different rules (rehydration is lenient/recovering; creation is strict/rejecting).
|
||||
|
||||
## Detailed Findings
|
||||
|
||||
### 1. PlayerCharacter Rehydration
|
||||
|
||||
**File**: `apps/web/src/persistence/player-character-storage.ts:25-65`
|
||||
|
||||
`rehydrateCharacter(raw: unknown): PlayerCharacter | null` performs:
|
||||
|
||||
| Field | Validation | Behavior on invalid |
|
||||
|-------|-----------|-------------------|
|
||||
| `id` | `typeof string`, non-empty | Return `null` (reject entire PC) |
|
||||
| `name` | `typeof string`, non-empty after trim | Return `null` |
|
||||
| `ac` | `typeof number`, integer, `>= 0` | Return `null` |
|
||||
| `maxHp` | `typeof number`, integer, `>= 1` | Return `null` |
|
||||
| `color` | Optional; if present, must be in `VALID_PLAYER_COLORS` | Return `null` |
|
||||
| `icon` | Optional; if present, must be in `VALID_PLAYER_ICONS` | Return `null` |
|
||||
| `level` | Optional; if present, must be integer 1-20 | Return `null` |
|
||||
|
||||
Constructs result via branded `playerCharacterId()` and type assertions for color/icon.
|
||||
|
||||
**Helper**: `isValidOptionalMember(value, validSet)` — shared check for optional set-membership fields (lines 18-23).
|
||||
|
||||
**Callers**:
|
||||
- `loadPlayerCharacters()` (same file, line 67) — loads from localStorage
|
||||
- `rehydrateCharacters()` in `export-import.ts:21-30` — filters PCs during import validation
|
||||
|
||||
### 2. Combatant Rehydration
|
||||
|
||||
**File**: `apps/web/src/persistence/encounter-storage.ts:67-103`
|
||||
|
||||
`rehydrateCombatant(c: unknown)` (private, no return type annotation) performs:
|
||||
|
||||
| Field | Validation | Behavior on invalid |
|
||||
|-------|-----------|-------------------|
|
||||
| `id` | Cast directly (`entry.id as string`) | No validation (relies on `isValidCombatantEntry` pre-check) |
|
||||
| `name` | Cast directly (`entry.name as string`) | No validation (relies on pre-check) |
|
||||
| `initiative` | `typeof number` or `undefined` | Defaults to `undefined` |
|
||||
| `ac` | Via `validateAc`: integer `>= 0` | Defaults to `undefined` |
|
||||
| `conditions` | Via `validateConditions`: array, each in `VALID_CONDITION_IDS` | Defaults to `undefined` |
|
||||
| `isConcentrating` | Strictly `=== true` | Defaults to `undefined` |
|
||||
| `creatureId` | Via `validateCreatureId`: non-empty string | Defaults to `undefined` |
|
||||
| `color` | String in `VALID_PLAYER_COLORS` | Defaults to `undefined` |
|
||||
| `icon` | String in `VALID_PLAYER_ICONS` | Defaults to `undefined` |
|
||||
| `playerCharacterId` | Non-empty string | Defaults to `undefined` |
|
||||
| `maxHp` / `currentHp` | Via `validateHp`: maxHp integer >= 1, currentHp integer 0..maxHp | Defaults to `undefined`; invalid currentHp falls back to maxHp |
|
||||
|
||||
**Key difference from PC rehydration**: Combatant rehydration is *lenient* — invalid optional fields are silently dropped rather than rejecting the entire entity. Only `id` and `name` are required (checked by `isValidCombatantEntry` at line 105-109 before `rehydrateCombatant` is called).
|
||||
|
||||
**Helper functions** (all private):
|
||||
- `validateAc(value)` — lines 24-28
|
||||
- `validateConditions(value)` — lines 30-37
|
||||
- `validateCreatureId(value)` — lines 39-43
|
||||
- `validateHp(rawMaxHp, rawCurrentHp)` — lines 45-65
|
||||
|
||||
### 3. Encounter Rehydration
|
||||
|
||||
**File**: `apps/web/src/persistence/encounter-storage.ts:111-140`
|
||||
|
||||
`rehydrateEncounter(parsed: unknown): Encounter | null` validates the encounter envelope:
|
||||
- Must be a non-null, non-array object
|
||||
- `combatants` must be an array
|
||||
- `activeIndex` must be a number
|
||||
- `roundNumber` must be a number
|
||||
- Empty combatant array → returns hardcoded `{ combatants: [], activeIndex: 0, roundNumber: 1 }`
|
||||
- All entries must pass `isValidCombatantEntry` (id + name check)
|
||||
- Maps entries through `rehydrateCombatant`, then passes to domain's `createEncounter` for invariant enforcement
|
||||
|
||||
**Callers**:
|
||||
- `loadEncounter()` (same file, line 142) — localStorage
|
||||
- `loadStack()` in `undo-redo-storage.ts:17-36` — undo/redo stacks from localStorage
|
||||
- `rehydrateStack()` in `export-import.ts:10-19` — import validation
|
||||
- `validateImportBundle()` in `export-import.ts:32-65` — import validation (direct call for the main encounter)
|
||||
|
||||
### 4. Import Bundle Validation
|
||||
|
||||
**File**: `apps/web/src/persistence/export-import.ts:32-65`
|
||||
|
||||
`validateImportBundle(data: unknown): ExportBundle | string` validates the bundle envelope:
|
||||
- Version must be `1`
|
||||
- `exportedAt` must be a string
|
||||
- `undoStack` and `redoStack` must be arrays
|
||||
- `playerCharacters` must be an array
|
||||
- Delegates to `rehydrateEncounter` for the encounter
|
||||
- Delegates to `rehydrateStack` (which calls `rehydrateEncounter`) for undo/redo
|
||||
- Delegates to `rehydrateCharacters` (which calls `rehydrateCharacter`) for PCs
|
||||
|
||||
This function validates the *envelope* structure. Entity-level validation is fully delegated.
|
||||
|
||||
### 5. Domain Layer Validation (Existing)
|
||||
|
||||
The domain already contains validation for the same fields, but in *creation* context (typed inputs, DomainError returns):
|
||||
|
||||
**`createPlayerCharacter`** (`packages/domain/src/create-player-character.ts:17-100`):
|
||||
- Same field rules as `rehydrateCharacter`: name non-empty, ac >= 0 integer, maxHp >= 1 integer, color/icon in valid sets, level 1-20
|
||||
- Returns `DomainError` on invalid input (not `null`)
|
||||
|
||||
**`validateInit`** in `addCombatant` (`packages/domain/src/add-combatant.ts:27-53`):
|
||||
- Validates maxHp (positive integer), ac (non-negative integer), initiative (integer)
|
||||
- Does NOT validate conditions, color, icon, playerCharacterId, creatureId, isConcentrating
|
||||
|
||||
**`createEncounter`** (`packages/domain/src/types.ts:50-71`):
|
||||
- Validates activeIndex bounds and roundNumber (positive integer)
|
||||
- Already used by `rehydrateEncounter` as the final step
|
||||
|
||||
**`editPlayerCharacter`** (`packages/domain/src/edit-player-character.ts`):
|
||||
- `validateFields` validates the same PC fields for edits
|
||||
|
||||
### 6. Validation Overlap and Gaps
|
||||
|
||||
| Field | Rehydration validates | Domain validates |
|
||||
|-------|----------------------|-----------------|
|
||||
| PC.id | Non-empty string | N/A (caller provides) |
|
||||
| PC.name | Non-empty string | Non-empty (trimmed) |
|
||||
| PC.ac | Integer >= 0 | Integer >= 0 |
|
||||
| PC.maxHp | Integer >= 1 | Integer >= 1 |
|
||||
| PC.color | In VALID_PLAYER_COLORS | In VALID_PLAYER_COLORS |
|
||||
| PC.icon | In VALID_PLAYER_ICONS | In VALID_PLAYER_ICONS |
|
||||
| PC.level | Integer 1-20 | Integer 1-20 |
|
||||
| Combatant.id | Non-empty string (via pre-check) | N/A (caller provides) |
|
||||
| Combatant.name | String type (via pre-check) | Non-empty (trimmed) |
|
||||
| Combatant.initiative | `typeof number` | Integer |
|
||||
| Combatant.ac | Integer >= 0 | Integer >= 0 |
|
||||
| Combatant.maxHp | Integer >= 1 | Integer >= 1 |
|
||||
| Combatant.currentHp | Integer 0..maxHp | N/A (set = maxHp on add) |
|
||||
| Combatant.tempHp | **Not validated** | N/A |
|
||||
| Combatant.conditions | Each in VALID_CONDITION_IDS | N/A (toggleCondition checks) |
|
||||
| Combatant.isConcentrating | Strictly `true` or dropped | N/A (toggleConcentration) |
|
||||
| Combatant.creatureId | Non-empty string | N/A (passed through) |
|
||||
| Combatant.color | In VALID_PLAYER_COLORS | N/A (passed through) |
|
||||
| Combatant.icon | In VALID_PLAYER_ICONS | N/A (passed through) |
|
||||
| Combatant.playerCharacterId | Non-empty string | N/A (passed through) |
|
||||
|
||||
Key observations:
|
||||
- Rehydration validates `id` (required for identity); domain creation functions receive `id` as a typed parameter
|
||||
- Combatant rehydration does NOT validate `tempHp` at all — it's silently passed through or ignored
|
||||
- Combatant rehydration checks `initiative` as `typeof number` but domain checks `Number.isInteger` — slightly different strictness
|
||||
- Domain validation for combatant optional fields is scattered across individual mutation functions, not centralized
|
||||
|
||||
### 7. Test Coverage
|
||||
|
||||
**Persistence tests** (adapter layer):
|
||||
- `encounter-storage.test.ts` — ~27 tests covering round-trip, corrupt data, AC validation, edge cases
|
||||
- `player-character-storage.test.ts` — ~17 tests covering round-trip, corrupt data, field validation, level
|
||||
|
||||
**Import tests** (adapter layer):
|
||||
- `validate-import-bundle.test.ts` — ~21 tests covering envelope validation, graceful recovery, PC filtering
|
||||
- `export-import.test.ts` — ~15 tests covering bundle assembly, round-trip, filename resolution
|
||||
|
||||
**Domain tests**: No rehydration tests exist in `packages/domain/` — all rehydration testing is in the adapter layer.
|
||||
|
||||
### 8. Cross-Reference Map
|
||||
|
||||
```
|
||||
loadPlayerCharacters() ──→ rehydrateCharacter()
|
||||
↑
|
||||
validateImportBundle() ──→ rehydrateCharacters() ──┘
|
||||
├─→ rehydrateEncounter() ──→ isValidCombatantEntry()
|
||||
│ ├─→ rehydrateCombatant() ──→ validateAc()
|
||||
│ │ ├─→ validateConditions()
|
||||
│ │ ├─→ validateCreatureId()
|
||||
│ │ └─→ validateHp()
|
||||
│ └─→ createEncounter() [domain]
|
||||
└─→ rehydrateStack() ───→ rehydrateEncounter() [same as above]
|
||||
|
||||
loadEncounter() ───────→ rehydrateEncounter() [same as above]
|
||||
|
||||
loadUndoRedoStacks() ──→ loadStack() ──→ rehydrateEncounter() [same as above]
|
||||
```
|
||||
|
||||
## Code References
|
||||
|
||||
- `apps/web/src/persistence/player-character-storage.ts:25-65` — `rehydrateCharacter` (PC rehydration)
|
||||
- `apps/web/src/persistence/player-character-storage.ts:18-23` — `isValidOptionalMember` helper
|
||||
- `apps/web/src/persistence/encounter-storage.ts:24-28` — `validateAc` helper
|
||||
- `apps/web/src/persistence/encounter-storage.ts:30-37` — `validateConditions` helper
|
||||
- `apps/web/src/persistence/encounter-storage.ts:39-43` — `validateCreatureId` helper
|
||||
- `apps/web/src/persistence/encounter-storage.ts:45-65` — `validateHp` helper
|
||||
- `apps/web/src/persistence/encounter-storage.ts:67-103` — `rehydrateCombatant` (combatant rehydration)
|
||||
- `apps/web/src/persistence/encounter-storage.ts:105-109` — `isValidCombatantEntry` (pre-check)
|
||||
- `apps/web/src/persistence/encounter-storage.ts:111-140` — `rehydrateEncounter` (encounter envelope rehydration)
|
||||
- `apps/web/src/persistence/export-import.ts:10-30` — `rehydrateStack` / `rehydrateCharacters` (collection wrappers)
|
||||
- `apps/web/src/persistence/export-import.ts:32-65` — `validateImportBundle` (import envelope validation)
|
||||
- `apps/web/src/persistence/undo-redo-storage.ts:17-36` — `loadStack` (undo/redo rehydration)
|
||||
- `packages/domain/src/create-player-character.ts:17-100` — PC creation validation
|
||||
- `packages/domain/src/add-combatant.ts:27-53` — `validateInit` (combatant creation validation)
|
||||
- `packages/domain/src/types.ts:50-71` — `createEncounter` (encounter invariant enforcement)
|
||||
- `packages/domain/src/types.ts:12-26` — `Combatant` type definition
|
||||
- `packages/domain/src/player-character-types.ts:70-83` — `PlayerCharacter` type definition
|
||||
|
||||
## Architecture Documentation
|
||||
|
||||
### Current pattern
|
||||
Rehydration is an adapter concern — persistence adapters validate raw JSON and construct typed domain objects. The domain provides creation functions that validate typed inputs for new entities, but no functions for reconstructing entities from untyped serialized data.
|
||||
|
||||
### Rehydration vs. creation semantics
|
||||
Rehydration and creation serve different purposes:
|
||||
- **Creation** (domain): Validates business rules for *new* entities. Receives typed parameters. Returns `DomainError` on failure.
|
||||
- **Rehydration** (adapter): Reconstructs *previously valid* entities from serialized JSON. Receives `unknown`. Returns `null` on failure. May be lenient (combatants drop invalid optional fields) or strict (PCs reject on any invalid field).
|
||||
|
||||
### Delegation chain
|
||||
`rehydrateEncounter` already delegates to `createEncounter` for encounter-level invariants. The entity-level rehydration functions (`rehydrateCharacter`, `rehydrateCombatant`) do NOT delegate to any domain function — they re-implement field validation inline.
|
||||
|
||||
### tempHp gap
|
||||
`Combatant.tempHp` is defined in the domain type but has no validation in the current rehydration code. It appears to be silently included or excluded depending on what `rehydrateCombatant` constructs (it's not in the explicit field list, so it would be dropped during rehydration).
|
||||
|
||||
## Open Questions
|
||||
|
||||
1. **Should `rehydrateCombatant` remain lenient (drop invalid optional fields) or become strict like `rehydrateCharacter` (reject on any invalid field)?** The current asymmetry is intentional: combatants can exist with minimal data (just id + name), while PCs always require ac/maxHp.
|
||||
|
||||
2. **Should `tempHp` be validated during rehydration?** It's currently missing from combatant rehydration but is a valid field on the type.
|
||||
|
||||
3. **Should `rehydrateEncounter` move to domain too, or only the entity-level functions?** The issue acceptance criteria says "validateImportBundle and rehydrateEncounter are unchanged" — but `rehydrateEncounter` currently lives alongside `rehydrateCombatant` and would need to import from domain instead of calling the local function.
|
||||
|
||||
4. **Should `isValidCombatantEntry` (the pre-check) be part of the domain rehydration or remain in the adapter?** It's currently the gate that ensures `id` and `name` exist before `rehydrateCombatant` is called.
|
||||
Reference in New Issue
Block a user