diff --git a/docs/openup/analysis-design/design-wi-2-architecture-cleanup.md b/docs/openup/analysis-design/design-wi-2-architecture-cleanup.md new file mode 100644 index 0000000..b2000fd --- /dev/null +++ b/docs/openup/analysis-design/design-wi-2-architecture-cleanup.md @@ -0,0 +1,664 @@ +# WI-2 Design — Architecture Cleanup + OpenUP Work Set (skara-brae-server#2, PSP1) + +Status: DESIGN (interface-first; implementation by psp-coder). +Baseline verified 2026-10-03 on `wi-2-architecture-cleanup` @ `899fde8`: `cargo check --workspace` green, 45/45 sb-core tests green, working tree clean. +Standards: "Clean Architecture & OpenUP A&D — Standards" (Joplin `414f323e04b448f18e2ea39ecef57b1f`), WI-1 design conventions (`design-wi-1-injectable-rng.md`). + +Hard constraint: **behavior-identical**. JSON field names, field order, status codes, game semantics, and RNG draw order are frozen. The current `json!` literals and handler code are the wire-format source of truth (§4.3). + +**Decision reversal record (2026-10-03, later same day — user decision):** ALL OpenUP docs are versioned in the **skara-brae-common repo** under `docs/openup/` — one canonical project-docs home, no split rule. This **supersedes** the earlier "OpenUP docs inside skara-brae-server" decision and §11.a's original in-repo recommendation. Consequently: the drafted WI-2 artifacts currently sit **untracked** in `skara-brae-server/docs/openup/` and will be **moved verbatim (no content changes) to `skara-brae-common/docs/openup/` by the coder in session 2**; the WI-1 artifacts at the old project-top `docs/openup/` migrate there as well. `roadmap.md` remains at the **project-top folder** per the earlier PM decision (the design never placed it in a docs tree) — that location is unversioned; if PM later wants it versioned, the pre-agreed fallback is the skara-brae-common repo root next to `docs/` (PM-confirmable default). + +--- + +## 1. Requirements traceability + +Source: Forgejo issue #2 (WI scope), acceptance per PM decision. + +| WI scope item | Design section | UC realization (see `uc-realization-mapping.md`) | +|---|---|---| +| 1. Typed responses (4 handlers, 6 `json!` sites) | §4.3 | UC-08 Adventure, UC-09 Temple Heal, UC-11 Study Magic | +| 2. `run_adventure` service extraction | §4.2 | UC-08 (all scenarios) | +| 3. Named constants (~25 magic numbers) | §4.4 | all gameplay UCs (cross-cutting) | +| 4. `unwrap()` cleanup + error enum | §4.1, §6 | all request-path UCs (cross-cutting) | +| 5. `cargo fmt` baseline | §9 | — (process) | +| 6. EA model + generator pipeline (scope **added at 1B rework**) | §11 | UC-01…UC-12 (all realizations) | + +Out of scope (recorded as follow-up candidates, §10): heal/spell-learn business logic still living in handlers — **roadmap.md candidate** per Q3 resolution (roadmap.md itself stays at the project-top folder, see the decision-reversal record above); migration of WI-1 docs and the drafted WI-2 docs into `skara-brae-common/docs/openup/` (the canonical home per the reversal; verbatim move, coder, session 2). + +--- + +## 2. Target module layout + +``` +crates/sb-core/src/ + lib.rs + pub mod adventure; + pub mod constants; + pub mod error; + adventure.rs NEW domain use case: Run Adventure (UC-08) + constants.rs NEW named game-balance constants (single audit home) + error.rs NEW DomainError enum (thiserror 2 — already a dependency) + character.rs MOD constants inlined from constants.rs; level_up -> DomainError + combat.rs MOD constants; no signature change + services.rs MOD train/equip/unequip/pick_enemy -> DomainError + spells.rs MOD + caster_spell_level(); constants + songs.rs unchanged + items.rs, data.rs production code unchanged (loader pin tests added, §6) + +crates/sb-shared/src/ + item.rs MOD + ItemType::as_str(), Quality::as_str() (+ parity tests) + +crates/sb-api/src/routes/ + mod.rs MOD handlers keep HTTP concerns only; + domain_status(), now_unix_ms() + responses.rs NEW typed response DTOs for the 4 untyped handlers (private mod) +``` + +Visibility rules: +- `sb_core::{adventure, constants, error}` — `pub` modules; consumed by sb-api. +- `sb_api::routes::responses` — private `mod responses;` inside `routes` (used only by handlers). Not `pub`; not in sb-shared. +- No new crate, no new external dependency. `thiserror = "2"` already in sb-core's Cargo.toml (verified). + +Placement decisions (designer decision records): +- **Response DTOs in sb-api, not sb-shared.** sb-core depends on sb-shared; putting HTTP-only DTOs there would make the domain crate transitively carry HTTP wire types it never uses. Existing sb-shared response types stay where they are (moving them is churn outside WI-2). See QUESTION Q2 for the future OpenAPI/utoipa consideration. +- **Constants centralized in `sb_core::constants`** even when single-use, so the "no magic numbers" audit is one grep in one module. Static *data tables* (`RACES`, `CLASSES`, `BARD_SONGS`, `TRAINING_GROUNDS`) are data, not tuning scalars — they stay in their modules. +- **`AdventureOutcome` derives nothing** (no serde) — pure domain output. The HTTP DTO maps from it in sb-api (`impl From for AdventureRunResponse`). This is the only new place domain output crosses to wire format. + +--- + +## 3. HTTP ↔ domain boundary rules + +**routes/ (sb-api) may import:** `sb_core` (incl. new `adventure`, `constants`, `error`), `sb_shared`, `sb_common`, `sb_db`, `sb_auth`, axum/tokio/tower, `rand` **only** via the WI-1 single construction point `request_rng()` (`StdRng::from_os_rng`; never `rand::rng()`). + +**sb-core must not reference:** axum, `http`, `StatusCode`, `serde_json::json!`, tokio, tracing, sb-db, sb-api. Serde derives on *existing* sb-shared types (e.g. `CombatLog`) are an accepted legacy trade-off — **not extended**: no new serde derives on new sb-core types (`AdventureOutcome`, `DomainError` carries only `thiserror::Error`, which is `Display`, not wire). + +**Status mapping lives in exactly one place:** `routes::domain_status` (§4.1). Handlers translate store misses/authz to 404/403/… themselves (they own the store); domain rule failures flow through `DomainError`. + +**Accepted legacy debt (unchanged by WI-2):** heal (routes/mod.rs:862–888) and spell learn/activate (routes/mod.rs:1107–1180) keep business logic in handlers. Typed responses only. Flagged for a follow-up WI (§10, Q3). + +--- + +## 4. Interface specifications (the contracts psp-coder implements) + +### 4.1 Domain error enum + central status mapping + +```rust +// crates/sb-core/src/error.rs +#[derive(Debug, Clone, Copy, PartialEq, Eq, thiserror::Error)] +pub enum DomainError { + #[error("character is incapacitated")] + CharacterIncapacitated, + #[error("not enough nerve")] + NotEnoughNerve, + #[error("not enough energy")] + NotEnoughEnergy, + #[error("not enough gold")] + NotEnoughGold, + #[error("not enough XP")] + NotEnoughXp, + #[error("already at max level")] + AlreadyAtMaxLevel, + #[error("invalid stat name")] + InvalidStat, + #[error("item not in inventory")] + ItemNotInInventory, + #[error("nothing equipped in slot")] + NothingEquippedInSlot, + #[error("encounter references unknown enemy")] + UnknownEncounterEnemy, +} +``` + +One enum at the domain boundary (standards: no per-function error types, no error-enum fragmentation). Fieldless + `Copy` so tests assert with `assert_eq!`. + +```rust +// crates/sb-api/src/routes/mod.rs +fn domain_status(error: sb_core::error::DomainError) -> axum::http::StatusCode +``` + +Mapping table (frozen to current behavior — each row cites today's code): + +| Variant | Status | Current source (routes/mod.rs) | +|---|---|---| +| `CharacterIncapacitated` | 400 BAD_REQUEST | L444–446 (`hp <= 0`) | +| `InvalidStat` | 400 BAD_REQUEST | L648 (`_ => BAD_REQUEST` in train) | +| `ItemNotInInventory` | 400 BAD_REQUEST | L365 (`map_err(\|_\| BAD_REQUEST)`) | +| `NothingEquippedInSlot` | 400 BAD_REQUEST | L400 | +| `NotEnoughNerve` | 422 UNPROCESSABLE_ENTITY | L447–449 | +| `NotEnoughEnergy` | 422 | L646 | +| `NotEnoughGold` | 422 | L647 (train), L1077–1079 (level_up: ALL errors 422), heal L880 (422, handler-side) | +| `NotEnoughXp` | 422 | L1077 | +| `AlreadyAtMaxLevel` | 422 | L1078 (level_up maps every `Err` to 422) | +| `UnknownEncounterEnemy` | 500 INTERNAL_SERVER_ERROR | new (see §6, pick_enemy) | + +The match in `domain_status` must be non-wildcard and exhaustive: adding a `DomainError` variant then fails compilation until a status is chosen (taxonomy designed once). + +**Signature conversions** (behavior-identical; only the error type changes): + +| Function (current) | New signature | +|---|---| +| `character::level_up(&mut Character) -> Result` | `Result` (AlreadyAtMaxLevel, NotEnoughXp, NotEnoughGold) | +| `services::train_stat(&mut Character, &str) -> Result` | `Result` (NotEnoughEnergy, InvalidStat, NotEnoughGold) | +| `services::equip_item(...) -> Result<(), &'static str>` | `Result<(), DomainError>` (ItemNotInInventory) | +| `services::unequip_item(...) -> Result` | `Result` (NothingEquippedInSlot) | +| `services::pick_enemy(...) -> EnemyDef` | `Result` (UnknownEncounterEnemy; also empty `encounters` / `total_weight <= 0`) | + +Test-assertion updates this forces (mechanical `Err("…")` → `Err(DomainError::…)`, no test logic changes): +- character.rs: `level_up_fails_at_max_level`, `level_up_fails_without_enough_xp`, `level_up_fails_without_enough_gold` +- services.rs: `train_stat_fails_without_energy`, `train_stat_fails_without_gold`, `train_stat_fails_invalid_stat`, `equip_item_fails_if_not_in_inventory`, `unequip_item_fails_empty_slot`, `pick_enemy_respects_weights` (now `unwrap()`s the Ok) + +### 4.2 Adventure service (UC-08) + +```rust +// crates/sb-core/src/adventure.rs +use rand::Rng; +use sb_shared::{Character, CombatLog, Item}; +use crate::data::{EnemyDef, ZoneDef}; +use crate::error::DomainError; + +/// Domain output of one adventure run. No serde derives — the HTTP DTO +/// (sb-api `AdventureRunResponse`) maps from this at the boundary. +pub struct AdventureOutcome { + pub zone_id: String, // echoes zone.id (== requested zone_id; lookup is by id) + pub enemy_name: String, + pub combat_log: CombatLog, + pub loot_item_names: Vec, // display names, in drop-roll order +} + +pub fn run_adventure( + rng: &mut impl Rng, + character: &mut Character, + zone: &ZoneDef, + enemies: &[EnemyDef], + items: &[Item], + now_ms: i64, +) -> Result; +``` + +**Behavior contract** (ordered; RNG draw order is normative — the two deliberate semantic deltas against routes/mod.rs:430–555 are recorded immediately after the list): + +1. `character::apply_regen(character, now_ms)` — regen happens **before** the guards (current L442). +2. `character.hp <= 0` → `Err(CharacterIncapacitated)`. +3. `character.nerve < zone.nerve_cost` → `Err(NotEnoughNerve)`. +4. `character.nerve -= zone.nerve_cost`. +5. `enemy_def = services::pick_enemy(rng, &zone.encounters, enemies)?` — draw #1: one `random_range(0..total_weight)`. On `Err` the nerve is already spent (matches today's ordering where the panic occurred after deduction; the 500 replaces a panic — see §6). +6. Character combat stats from the *mutated* character + `items`: `services::equipped_attack_type`, `services::equipped_damage` (ob_base), `services::total_defense` (db_base); hp = post-regen hp; `CombatStats { … }` as L460–471. +7. Enemy `CombatStats` per L473–484 (`parse_attack_type(&enemy_def.attack_type)`). +8. `spell_effect`: `character.active_spell` → `spells::find_spell` → `combat::SpellEffect { … }` per L486–494. +9. `song_effect`: `character.active_song` → `songs::BARD_SONGS` find → `.combat_effect` per L502–504. +10. `log = combat::resolve_combat_full(char_stats, enemy_stats, enemy_def.xp, enemy_def.gold_min, enemy_def.gold_max, song_effect, spell_effect.as_ref(), rng)` — draws #2..n: open-ended rolls, per-round spell damage, final gold roll, in engine order. +11. If `log.winner == character.name` (win): + - `character.xp += log.xp_gained as i64`; `character.gold += log.gold_gained`; + - `character.hp = log.rounds.iter().rfind(|r| r.defender == character.name).map(|r| r.defender_hp)` — last round in which the character was defender (i.e. the final enemy attack against the character), if any round exists; + - loot: for each `drop` in `enemy_def.loot` **in order**: `rng.random_bool(drop.chance)` (draw) **and** `items.iter().any(|i| i.id == drop.item_id)` → push `drop.item_id` into inventory and record it. + Else (loss): `character.hp = 0`, no xp/gold/loot. +12. Spell-point upkeep (regardless of win/loss, L542–547): if `character.active_spell.is_some()` and `spells::find_spell(active_spell)` → `character.spell_points = (spell_points - sp.spell_points).max(0)`. +13. `loot_item_names` = recorded ids mapped through `items` (id → `name`), preserving order. +14. Return `AdventureOutcome { zone_id: zone.id.clone(), enemy_name: enemy_def.name.clone(), combat_log: log, loot_item_names }`. + +Mutation semantics: `character` is mutated in place through step 1 even when an `Err` is returned (regen side effect stays in memory) — identical to today, where the handler applies regen, returns early, and never persists. **The handler persists only on `Ok`** (unchanged). + +**Decided semantic deltas vs the current handler (1B review finding 5 — recorded, wire-invisible):** +1. *Lock consolidation.* Today the handler acquires locks three times (authz read-lock; write-lock #1 for regen/nerve/enemy-pick; write-lock #2 for outcome application), computing combat stats in between. The refactored handler holds **one** write-lock span around the service call. The mid-request "character vanished between acquisitions" branch (today: second lookup misses → 200 with empty loot and no persist, routes/mod.rs:509–555) becomes structurally impossible. That branch is dead code today — no character-deletion endpoint exists and nothing else removes characters from the store — so no reachable behavior changes; the delta is recorded rather than claimed away. +2. *Enemy-pick failure.* See §6: impossible-data `UnknownEncounterEnemy` now yields a 500 where the current code panics (same in-memory side effects). + +**Handler after refactor (shape):** + +```rust +async fn run_adventure(/* extractors unchanged */) + -> Result, axum::http::StatusCode> +{ + let zone_id = query.zone_id.as_deref().unwrap_or(DEFAULT_ZONE_ID); + // authz read-guard block: unchanged (404 / 403) + let zone = state.zones.iter().find(|z| z.id == zone_id) + .ok_or(axum::http::StatusCode::NOT_FOUND)?; + let now_ms = now_unix_ms(); + let mut rng = request_rng(); + let (outcome, updated) = { + let mut chars = state.characters.write().await; + let character = chars.iter_mut().find(|c| c.id == char_id) + .ok_or(axum::http::StatusCode::NOT_FOUND)?; + let outcome = sb_core::adventure::run_adventure( + &mut rng, character, zone, &state.enemies, &state.items, now_ms) + .map_err(domain_status)?; + (outcome, character.clone()) + }; + persist_character(&state, &updated).await; + Ok(Json(outcome.into())) // From for AdventureRunResponse +} +``` + +The handler owns: extractors, authz guard, store lookup/locking, rng construction (WI-1 point), status mapping, persistence, DTO mapping. Nothing else. + +### 4.3 Typed responses (`crates/sb-api/src/routes/responses.rs`) + +**Wire-order rule (corrected at 1B review, finding 1 — blocker).** This workspace resolves `serde_json` v1.0.151 **without** the `preserve_order` feature (verified: `cargo tree -p serde_json` → `default,raw_value,std`). `json!` therefore builds `Map` backed by `BTreeMap`, and **every `json!` object in the current handlers serializes its keys in lexicographic (byte) order, regardless of the literal's key order**. + +Rule for the coder: **declare struct fields in lexicographic order of their JSON keys** — serde struct serialization follows declaration order, so lexicographic declaration order reproduces the current bytes exactly. The field orders below are already correct; do not "reorder for readability." + +Two consequences: +- **`json!` sorts keys at EVERY nesting level, not just the top level** (defect #14, found by psp-coder during implementation prep): `json!({"combat_log": &log})` first converts the struct via `to_value` into a `Value::Object(BTreeMap)`, so today's wire shows nested structs sorted too — e.g. `combat_log` emits `{"gold_gained":…, "loser":…, "rounds":[…], "winner":…, "xp_gained":…}` with each `CombatRound`/`CritResult`/`CritEntry` lexicographic. A DTO field typed as the struct would emit **declaration order** (serde derive) and change the wire bytes. Therefore `AdventureRunResponse.combat_log` is a `serde_json::Value` **bridge**, populated via `serde_json::to_value(&outcome.combat_log)` in the `From` impl (PM decision, option A) — reproducing today's sorted bytes exactly. `CombatLog` itself stays a typed struct in the domain and in `AdventureOutcome`; only the wire DTO field is the `Value` bridge. +- `learn_costs` uses `BTreeMap`: its string keys "2"…"7" sort lexicographically exactly as today's `json!` inner object emits them. + +```rust +use serde::Serialize; +use std::collections::BTreeMap; + +#[derive(Debug, Serialize)] +pub struct AdventureRunResponse { + pub combat_log: serde_json::Value, // Value bridge — see From impl below (defect #14) + pub enemy: String, + pub loot: Vec, + pub zone: String, +} +// Current actual emission (routes/mod.rs:561–566 under BTreeMap ordering): +// {"combat_log":…, "enemy":…, "loot":[…], "zone":…} +// (the literal writes zone, enemy, combat_log, loot — the wire bytes are sorted) + +#[derive(Debug, Serialize)] +pub struct HealResponse { + pub gold: i32, + pub healed: bool, + pub hp: i32, + pub max_hp: i32, +} +// Current actual emission (routes/mod.rs:891–896): {"gold":…, "healed":true, "hp":…, "max_hp":…} + +#[derive(Debug, Serialize)] +pub struct SpellEntryResponse { + pub code: String, + pub combat_damage_max: i32, + pub combat_damage_min: i32, + pub combat_effect: String, + pub description: String, + pub heal_max: i32, + pub heal_min: i32, + pub id: String, + pub known: bool, + pub name: String, + pub range: String, + pub spell_level: i32, + pub spell_points: i32, +} +// Current actual emission (routes/mod.rs:1015–1029) — sorted keys; the literal +// order (id, code, name, …) is NOT what the wire shows today. + +#[derive(Debug, Serialize)] +pub struct SpellbookResponse { + pub caster_class: String, + pub learn_costs: BTreeMap, + pub max_spell_points: i32, + pub spell_level: i32, + pub spell_points: i32, + pub spells: Vec, +} +// Current actual emission (routes/mod.rs:1031–1045); learn_costs keys exactly +// "2".."7" (spell_learn_cost per level), sorted like today. + +#[derive(Debug, Serialize)] +pub struct ActivateSpellResponse { + pub active_spell: Option, // None → JSON null (json! identical) + pub spell_points: i32, +} +// routes/mod.rs:1128–1131 — the only site whose literal order already was +// lexicographic; field order unchanged by this rule. + +#[derive(Debug, Serialize)] +pub struct LearnSpellsResponse { + pub gold: i32, + pub learned_count: usize, + pub max_spell_points: i32, + pub spell_points: i32, +} +// Current actual emission (routes/mod.rs:1183–1188): gold, learned_count, +// max_spell_points, spell_points. NOTE (behavior wart, kept): "learned_count" +// carries updated.known_spells.len() — the TOTAL known spells after learning, +// not the newly-learned count (Q1 resolution: fenced wart, keep verbatim). +``` + +Handler return types change from `Result, StatusCode>` to: +- `run_adventure` → `Result, StatusCode>` +- `heal_character` → `Result, StatusCode>` +- `get_spells` → `Result, StatusCode>` +- `learn_spell_level` → returns an **enum** (two different bodies): + +```rust +#[derive(Debug, Serialize)] +#[serde(untagged)] +pub enum LearnSpellResponse { + Activate(ActivateSpellResponse), // body present when request had spell_id + Learn(LearnSpellsResponse), // body present when request had spell_level +} +``` +`#[serde(untagged)]` serializes the inner struct — byte-identical to the two current `json!` shapes. Handler returns `Result, StatusCode>`. + +**Value bridge impl (defect #14 — populates the `combat_log` field):** + +```rust +// crates/sb-api/src/routes/responses.rs — mapping happens at the HTTP boundary +impl From for AdventureRunResponse { + fn from(outcome: sb_core::adventure::AdventureOutcome) -> Self { + AdventureRunResponse { + // to_value mirrors exactly what json! does to &log today: struct → + // Value::Object with BTreeMap ordering — nested keys sorted, byte-identical wire. + combat_log: serde_json::to_value(&outcome.combat_log) + .expect("CombatLog graph contains no map types; to_value is infallible"), + enemy: outcome.enemy_name, + loot: outcome.loot_item_names, + zone: outcome.zone_id, + } + } +} +``` + +The single `expect` is a §6 keep-with-reason invariant: `serde_json::to_value` fails only on non-string map keys, and the `CombatLog`/`CombatRound`/`CritResult`/`CritEntry` graph contains no map types (String, i32/u8/f64, Vec, Option only) — infallible by construction, and pinned by the W1 golden test. + +`Debug + Serialize` only (no `Deserialize`, no `Clone`) — these types are write-only at the HTTP edge. + +**Wire-identity proof obligation (1B review finding 1):** byte-comparison tests W1–W6 (§7) must serialize each DTO with fixed representative values and assert equality against golden strings recorded from the **current** `json!` expressions before the refactor — sorted-key bytes, not literal-order bytes. + +Known wire-format wart preserved deliberately (do **not** “fix”): shop/inventory `attack_type` renders Debug-form `"Slash"`/`"Crush"`/`"Puncture"` (routes/mod.rs:693, 708 via `format!("{:?}", at)`), while `item_type`/`quality` render snake_case (`"weapon"`, `"common"`). The new `as_str()` helpers (§4.5) must NOT be applied to those two `attack_type` sites. + +### 4.4 Constants module (`crates/sb-core/src/constants.rs`) + +All `pub const`, `SCREAMING_SNAKE_CASE`, with a doc comment carrying the origin. Value column is normative; origin = current file:line of the literal. + +**Combat (combat.rs):** + +| Name | Value | Origin | +|---|---|---| +| `ATTACK_ROLL_MIN` / `ATTACK_ROLL_MAX` | `1` / `100` | combat.rs:66 | +| `OPEN_END_HIGH_THRESHOLD` | `96` (re-roll upward) | combat.rs:68 | +| `OPEN_END_LOW_THRESHOLD` | `5` (re-roll downward) | combat.rs:70 | +| `CRIT_SEVERITY_A_MIN` | `21` | combat.rs:129 | +| `CRIT_SEVERITY_B_MIN` | `51` | combat.rs:127 | +| `CRIT_SEVERITY_C_MIN` | `101` | combat.rs:125 | +| `CRIT_SEVERITY_D_MIN` | `151` | combat.rs:123 | +| `CRIT_CAP_DISABLED_MAX_LEVEL_GAP` | `9` (gaps 1..=9 → no cap) | combat.rs:56 | +| `CRIT_CAP_TIER1_MIN_GAP` / `CRIT_CAP_TIER1_MAX_GAP` | `10` / `19` | combat.rs:57 | +| `CRIT_CAP_TIER2_MIN_GAP` / `CRIT_CAP_TIER2_MAX_GAP` | `20` / `49` | combat.rs:58 | +| `CRIT_CAP_TIER3_MIN_GAP` / `CRIT_CAP_TIER3_MAX_GAP` | `50` / `99` | combat.rs:59 | +| `CRIT_CAP_TIER4_MIN_GAP` | `100` (and above → 0.05 ratio) | combat.rs:60 | +| `CRIT_CAP_TIER1_RATIO` | `0.60` (gap 10–19) | combat.rs:57 | +| `CRIT_CAP_TIER2_RATIO` | `0.40` (gap 20–49) | combat.rs:58 | +| `CRIT_CAP_TIER3_RATIO` | `0.15` (gap 50–99) | combat.rs:59 | +| `CRIT_CAP_TIER4_RATIO` | `0.05` (gap ≥ 100) | combat.rs:60 | +| `DEFENSIVE_CON_BONUS_DIVISOR` | `2` | combat.rs:36 | +| `SONG_ACCURACY_OB_BONUS` | `15` | combat.rs:246 | +| `SONG_DAMAGE_OB_BONUS` | `20` | combat.rs:247 | +| `SONG_DEFENSE_DB_BONUS` | `10` | combat.rs:256 | +| `SONG_WEAKEN_OB_FACTOR` | `0.7` | combat.rs:257 | +| `BARD_SONG_HEAL_FRACTION` | `0.03` (of max HP per round) | combat.rs:230 | + +**Character (character.rs):** + +| Name | Value | Origin | +|---|---|---| +| `STAT_BONUS_NEUTRAL` | `20` | character.rs:59 | +| `STAT_BONUS_STEP` | `2` | character.rs:59 | +| `STAT_ROLL_MIN` / `STAT_ROLL_MAX` | `6` / `16` | character.rs:63 | +| `BASE_MAX_ENERGY` | `30` | character.rs:85 | +| `MAX_ENERGY_PER_BASE_HP_DIVISOR` | `3` | character.rs:85 | +| `BASE_MAX_NERVE` | `100` | character.rs:90 | +| `MAX_NERVE_PER_BASE_HP_DIVISOR` | `2` | character.rs:90 | +| `ENERGY_REGEN_INTERVAL_MS` | `180_000` | character.rs:94 | +| `NERVE_REGEN_INTERVAL_MS` | `300_000` | character.rs:98 | +| `HP_REGEN_INTERVAL_MS` | `300_000` | character.rs:102 | +| `HP_REGEN_FRACTION_PER_TICK` | `0.05` | character.rs:118 | +| `TRAINING_BASE_COST` | `5.0` | character.rs:126 | +| `TRAINING_GROWTH_FACTOR` | `1.3` | character.rs:126 | +| `TRAINING_STAT_SCALER` | `10.0` | character.rs:125 | +| `TRAINING_ENERGY_COST` | `1` (energy per training session) | services.rs:85, 100 | +| `STARTING_LEVEL` | `1` | character.rs:157 | +| `STARTING_GOLD` | `50` | character.rs:166 | +| `XP_CURVE_COEFFICIENT` | `100.0` | character.rs:196 | +| `LEVEL_UP_GOLD_PER_LEVEL` | `500` | character.rs:200 | + +(`MAX_LEVEL = 100`, `XP_FACTOR = 18` already exist as named consts — re-export/keep.) + +**Spells (spells.rs):** + +| Name | Value | Origin | +|---|---|---| +| `CASTER_LEVELS_PER_SPELL_LEVEL` | `3` ((level/3)+1 curve) | spells.rs:59 | +| `SPELL_POINTS_PER_SPELL_LEVEL` | `10` | spells.rs:62 | +| `SPELL_LEARN_COST_BY_LEVEL` | `[0, 1000, 2000, 4000, 7000, 10000, 20000]` (index = level−1) | spells.rs:44–52 | +| `SPELL_LEARN_COST_UNKNOWN_LEVEL` | `999_999` | spells.rs:52 | +| `MIN_SPELL_LEVEL` / `MAX_SPELL_LEVEL` | `1` / `7` — **learn-request validation only** (routes/mod.rs:1146); the `caster_spell_level` formula itself is uncapped | routes/mod.rs:1146 | + +Guard semantics for `spell_learn_cost` (1B review finding 9): the function stays **total**. It indexes `SPELL_LEARN_COST_BY_LEVEL` only when `MIN_SPELL_LEVEL <= level <= MAX_SPELL_LEVEL` (1..=7 — the range check gates the subtraction, so no `usize` underflow for level ≤ 0); any other input, including level ≤ 0 and level > 7, returns `SPELL_LEARN_COST_UNKNOWN_LEVEL` (999_999) — identical to today's catch-all match arm (spells.rs:52). + +**HTTP-layer policy constants (stay in sb-api, `routes/mod.rs` — request-validation policy, not game balance):** + +| Name | Value | Origin | +|---|---|---| +| `DEFAULT_ZONE_ID` | `"skara_brae_streets"` | routes/mod.rs:415 | +| `CHARACTER_NAME_MAX_LEN` | `32` | routes/mod.rs:289 | +| `REGISTER_NAME_MIN_LEN` / `REGISTER_NAME_MAX_LEN` | `3` / `32` | routes/mod.rs:1215 | +| `PASSWORD_MIN_LEN` | `6` | routes/mod.rs:1218 | +| `PARTIAL_HEAL_FRACTION` | `0.5` | routes/mod.rs:874 | +| `MIN_HEAL_COST` | `1` | routes/mod.rs:878 | +| `SPELL_LEARN_COST_DISPLAY_MIN/MAX` | `2` / `7` (learn_costs response keys) | routes/mod.rs:1037–1043 | + +### 4.5 Shared label helpers + small domain helpers + +```rust +// crates/sb-shared/src/item.rs — additions (mirror Race::as_str/Class::as_str precedent) +impl ItemType { pub fn as_str(&self) -> &'static str } // "weapon"|"armor"|"accessory"|"consumable"|"material" +impl Quality { pub fn as_str(&self) -> &'static str } // "common"|"uncommon"|"rare"|"epic"|"legendary" +``` +Parity unit tests (in sb-shared): for every variant, `serde_json::to_value(variant).unwrap() == serde_json::json!(variant.as_str())`. Kills the four `serde_json::to_value(&x).unwrap().as_str().unwrap()` chains (routes/mod.rs:688–689, 703–704). + +```rust +// sb-core/src/character.rs +pub const GYM_STAT_DISPLAY_ORDER: &[&str] = + &["strength", "intelligence", "dexterity", "constitution", "luck"]; +// 1B review finding 2: this sequence is PINNED to the handler's fixed array +// (routes/mod.rs:588–594) and deliberately differs from TRAINING_GROUNDS order +// (strength, dexterity, constitution, intelligence, luck). get_gym iterates +// GYM_STAT_DISPLAY_ORDER (NOT TRAINING_GROUNDS) and resolves the training +// ground and stat value per entry, so the wire order of GymResponse.stats +// is unchanged. + +pub fn stat_value(stats: &CharacterStats, stat: &str) -> Option; +// Strength/Intelligence/Dexterity/Constitution/Luck by name; None otherwise. +// Replaces the get_gym expect("valid stat") pattern (routes/mod.rs:596): +// unknown ground or stat → skip (unreachable: add test +// gym_display_order_covers_all_training_grounds asserting set equality between +// GYM_STAT_DISPLAY_ORDER entries and TRAINING_GROUNDS stats, each resolvable +// via stat_value). + +// sb-core/src/spells.rs +pub fn caster_spell_level(character_level: i32) -> i32; // (level / CASTER_LEVELS_PER_SPELL_LEVEL) + 1 +// Replaces the three inline copies: spells.rs:59, routes/mod.rs:1012, routes/mod.rs:1150. +// Uncapped by design (level 100 → 34); see use-case-model UC-11. +``` + +```rust +// sb-api/src/routes/mod.rs +fn now_unix_ms() -> i64 { + std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .map(|elapsed| elapsed.as_millis() as i64) + .unwrap_or(0) +} +// Replaces 4 unwrap() sites (routes/mod.rs:261, 326, 432, 635). Clock-before-epoch +// (unreachable on real hardware) yields 0 → apply_regen no-op, instead of panic. +``` + +### 4.6 Interface verification protocol (for psp-coder) + +This WI is docs-only in DES, so the interface crate cannot be `cargo check`ed here (per constraints: no src/ edits in design phase). Coder protocol, stated unambiguously: + +1. **No stubs, ever.** Never commit `todo!()`, `unimplemented!()`, or placeholder `panic!()` bodies. Code lands complete or not at all. +2. **Every commit compiles.** `cargo check --workspace` is green at each step of the §8 sequence; never leave the tree broken between commits. +3. **Land order per §8:** standalone-landable types first (`error.rs`, `constants.rs`, `responses.rs` — they compile unreferenced thanks to the existing crate-level `dead_code` allow), then signature conversions with their test updates in the same commit, then `adventure.rs` complete (body + tests A1–A8 in one commit), then the handler rewiring commit that finally references everything. +4. **Tests move with signatures.** A signature change and the updates to the tests exercising it are one commit — never split. + +--- + +## 5. Signature-conversion ledger + +See §4.1 table. Exactly **9** existing tests change text (3 `level_up` assertions in character.rs; `train_stat_fails_without_energy`, `train_stat_fails_without_gold`, `train_stat_fails_invalid_stat`, `equip_item_fails_if_not_in_inventory`, `unequip_item_fails_empty_slot`, and `pick_enemy_respects_weights` in services.rs — the last because `pick_enemy` becomes `Result`-returning and its call site gains a `.unwrap()` on the Ok path). No test gains or loses logic. All 45 tests must pass before and after every commit. + +--- + +## 6. unwrap/expect ledger (full classification) + +**Removed — sb-api (8 `unwrap()` + 1 `expect()`):** + +| Site | Replacement | +|---|---| +| routes/mod.rs:261, 326, 432, 635 (`SystemTime…unwrap()`) | `now_unix_ms()` (§4.5) | +| routes/mod.rs:688, 689, 703, 704 (`to_value(…).unwrap().as_str().unwrap()`) | `ItemType::as_str()` / `Quality::as_str()` (§4.5) | +| routes/mod.rs:596 (`expect("valid stat")`) | `stat_value()` + TRAINING_GROUNDS iteration (§4.5) | + +**Converted to `Result` — sb-core:** +- `services::pick_enemy` (services.rs:17, 21, 22): `expect`/`unwrap` on data-dependent lookups (zones.json → enemies.json referential integrity) → `DomainError::UnknownEncounterEnemy` → 500. Empty `encounters` (today: `random_range(0..0)` panics) → same `Err`. This is a deliberate defensive improvement on a currently-panicking, data-impossible path; all shipped zones have ≥6 encounters. + +**Kept with rationale + new pinning tests — sb-core (startup/static invariants, fail-fast is correct):** +- Static embedded-JSON parses behind `LazyLock` (combat.rs:11, data.rs:58/62, items.rs:14/18/22, spells.rs:25): compiled-in data must parse; a `LazyLock` cannot propagate `Result`. New tests `crit_tables_parse`, `enemies_json_parses`, `zones_json_parses`, `all_items_json_parses`, `spells_json_parses` pin the invariant. +- Enum→table lookups (character.rs:68, 79, 84, 89, 230, 234): `Race`/`Class` are enums; every variant has a table row. New tests `race_table_covers_all_variants`, `class_table_covers_all_variants`. +- Composition-root password-hash `expect`s (routes/mod.rs:1304, 1306): server cannot operate with a broken hashing config; fail-fast at startup. Documented, unchanged. + +**sb-db — 8× `conn.lock().unwrap()` (added at 1B review, finding 3; all KEEP-WITH-REASON):** + +Sites: lib.rs:20 (`run_migrations`, startup via `open`), 68 (`load_users`, startup/seed), 91 (`save_user`, request path via `persist_user` on register), 104 (`load_characters`, startup), 167 (`save_character`, request path on every mutating character action), 196 (`load_shop_stock`, startup), 210 (`save_shop_stock`, request path on buy/sell), 219 (`next_character_id`, startup). + +Rationale (uniform for all eight — `std::sync::Mutex` fails here only via *poisoning*, i.e. after another thread already panicked while holding the lock): +1. A poisoned connection mutex means the process is in an unrecoverable post-panic state; fail-fast is the correct posture (same invariant class as the startup expects above). +2. Folding the poison error into the existing `Result<_, rusqlite::Error>` is impossible without either a new sb-db error enum (violates the one-error-enum-per-boundary standard and CON-04's no-new-dependency rule — sb-db has no `thiserror`) or lossy mapping. +3. The realistic in-lock panic sources are already gone: `save_character` serializes equipment/inventory/known_spells with `unwrap_or_default()` (lib.rs:168–170), so no serde panic can occur under the lock; remaining sources are rusqlite-internal bugs. +4. The three request-path sites sit under `persist_*`, where the architecture already treats persistence as best-effort (NFR-06) — but a poison there still indicates global corruption, not a transient write failure. + +Rejected alternative (recorded): `lock().unwrap_or_else(|poisoned| poisoned.into_inner())` poison-recovery — it would convert request-path panics into continued operation, but silently reusing a `Connection` after an arbitrary mid-statement panic risks undefined SQLite statement state; rejected as corruption-over-availability. **PM note:** this extends the issue #2 item-4 *ledger* to sb-db/sb-auth for completeness; the classification requires **no code change** in those crates. If PM prefers active conversion over keep-with-reason, that is a scoped follow-up (needs an sb-db error type decision). + +**sb-auth — 3 `expect`s (added at 1B review, finding 3; all KEEP-WITH-REASON):** + +| Site | Path | Rationale | +|---|---|---| +| jwt.rs:22 `checked_add_signed(Duration::hours(24)).expect(…)` | request path (register/login) inside `create_token` | Returns `None` only when now+24 h exceeds `DateTime` bounds (year ≈ 262 000) — unreachable for any real clock. Converting would require changing `create_token`'s public error type (`jsonwebtoken::Error` cannot carry it) — new type for an impossible case. | +| hash.rs:8 `getrandom::fill(&mut salt_bytes).expect(…)` | request path (register) + startup seeding, inside `Result`-returning `hash_password` | Failure = OS entropy source unavailable — the environment can no longer service authentication at all; fail-fast matches the startup hashing expects. Mapping into `argon2::password_hash::Error` would fabricate a misleading variant. | +| hash.rs:9 `SaltString::encode_b64(&mut salt_bytes).expect(…)` | same | Statically total for the fixed 16-byte input (PHC b64 covers all byte values at this length); unreachable by construction. | + +**New designed site (defect #14, §4.3):** `responses.rs` From-impl `serde_json::to_value(&outcome.combat_log).expect(…)` — keep-with-reason: `to_value` fails only on non-string map keys and the CombatLog graph has no map types; infallible by construction; pinned by the W1 wire-identity golden. + +--- + +## 7. Test strategy + +**Existing coverage of moved code (must stay green — regression net):** +- `character.rs` tests (21): regen application/no-op, XP curve, level-up success/fail, training cost, deterministic creation (seeded StdRng — WI-1). +- `combat.rs` tests (9): open-ended roll bounds, OB/DB composition, crit caps, winner production. +- `services.rs` tests (15): pick_enemy weights, equipped damage/defense/attack type, train/equip/unequip. + +**New tests — `adventure.rs` (deterministic; `StdRng::seed_from_u64` per WI-1 pattern):** + +| # | Vector | Setup | Assertions | +|---|---|---|---| +| A1 | win path | seeded rng; strong char (str 50, hp 100, sword equipped) vs weak enemy def (hp 10, xp 5, gold 1–2, 1 loot drop chance 1.0 with item in catalog); nerve ≥ cost | `Ok`; `combat_log.winner == char.name`; `char.xp == before + 5`; `char.gold ∈ [before+1, before+2]` and `== before + log.gold_gained`; `char.nerve == before − zone.nerve_cost`; loot item in `char.inventory` and its name in `loot_item_names`; `enemy_name == enemy_def.name`; `zone_id == zone.id` | +| A2 | loss path | weak char vs overwhelming enemy (hp 500, damage 200) | `Ok`; `combat_log.winner == enemy name`; `char.hp == 0`; xp/gold/inventory unchanged; `loot_item_names` empty | +| A3 | incapacitated | `char.hp = 0` | `Err(CharacterIncapacitated)`; nerve unchanged | +| A4 | insufficient nerve | `char.nerve < zone.nerve_cost` | `Err(NotEnoughNerve)`; nerve unchanged | +| A5 | regen before guards | hp 40/80, `last_regen = 0`, `now_ms = 10 × HP_REGEN_INTERVAL_MS`; enemy fixture with `damage = 0` and a seed **verified at fixture-recording time to yield zero enemy hits** | (i) seed-independent core assertion: `character.last_regen == now_ms` (apply_regen ran with elapsed > 0; it advances `last_regen` only when it actually regenerates, character.rs:107/121); (ii) under the recorded no-hit seed additionally: `character.hp == min(40 + ceil(80·0.05)·10, 80) = 80`. Do NOT assert `hp > starting hp` alone — an enemy lucky roll can drop HP below the post-regen start (1B review finding 13). | +| A6 | spell upkeep on win and loss | caster char with `active_spell` pointing at spell with `spell_points > 0` | both paths: `spell_points == (before − sp).max(0)`; `active_spell` unchanged | +| A7 | loot existence filter | drop rolls true for id **not** in items catalog | drop skipped (no inventory entry, no name) | +| A8 | determinism | two runs, `seed_from_u64(42)`, identical inputs | identical `format!("{:?}")` of outcome fields + char (Character has no PartialEq — WI-1 D5 pattern) | + +Golden-value pinning (optional but recommended): run A1 once, record the full `CombatLog` JSON as a fixture string, assert `serde_json::to_string(&outcome.combat_log) == fixture` — pins RNG draw order permanently. Values are recorded by the coder when the test first runs; the assertion structure is fixed here. + +**New tests — wire identity W1–W6 (1B review finding 1; sb-api `#[cfg(test)]` in routes):** + +| # | Vector | Method | +|---|---|---| +| W1 | `AdventureRunResponse` | Build the DTO with fixed representative values; `assert_eq!(serde_json::to_string(&dto).unwrap(), GOLDEN)` where GOLDEN is recorded **before the refactor** by serializing the current `json!` expression (routes/mod.rs:561–566) with the same values. Golden keys must come out in lexicographic order (`combat_log`, `enemy`, `loot`, `zone`) — proving the BTreeMap-order reproduction, not the literal order. The golden's `combat_log` must additionally show **nested** sorted keys (`gold_gained`, `loser`, `rounds`, `winner`, `xp_gained`; each round/crit entry sorted likewise) — proving the §4.3 Value bridge (defect #14). | +| W2 | `HealResponse` | Same protocol vs routes/mod.rs:891–896; expected key order `gold, healed, hp, max_hp`. | +| W3 | `SpellEntryResponse` | Same protocol vs routes/mod.rs:1015–1029; expected sorted keys `code … spell_points` (13 keys). | +| W4 | `SpellbookResponse` | Same protocol vs routes/mod.rs:1031–1045; expected key order `caster_class, learn_costs, max_spell_points, spell_level, spell_points, spells`; `learn_costs` keys "2".."7" sorted. | +| W5 | `ActivateSpellResponse` | Same protocol vs routes/mod.rs:1128–1131 (already lexicographic — the regression guard is the point). | +| W6 | `LearnSpellsResponse` (via `LearnSpellResponse::Learn`) | Same protocol vs routes/mod.rs:1183–1188; expected key order `gold, learned_count, max_spell_points, spell_points`. | + +Recording procedure (coder): before touching the handler, add a temporary test that evaluates the exact current `json!` expression with the fixed values and `to_string`s it; paste the output as the golden constant; delete the temporary test. The comparison is **byte-for-byte**, covering names, order, and null/number formatting. + +**New tests — constants/helpers:** `as_str` parity for `ItemType`/`Quality` (10 variants), `gym_display_order_covers_all_training_grounds`, `race/class` table exhaustiveness, 5 loader-parse pins, `caster_spell_level` spot values (1→1, 2→1, 3→2, 6→3, 100→34), `spell_learn_cost` guard spot checks (level 0, −1, 8 → 999_999; levels 1..=7 → table values). + +**Not covered (gaps that remain — documented, out of scope):** heal and spell-learn/activate business logic remains handler-resident and unit-untested (§10 Q3); no HTTP-level integration tests exist (unchanged by WI-2). + +--- + +## 8. fmt / rollout order + +**Recommendation: fmt FIRST, as a standalone commit.** + +Justification: +- Every later WI-2 commit (constants, extraction, responses) touches the same lines fmt would touch. fmt-last mixes ~120 lines of whitespace churn into reviewable diffs and corrupts PROBE size accounting (M-churn inflated into B/D categories). +- The acceptance gate `cargo fmt --check` applies at every future commit anyway; establishing the baseline first means each subsequent commit is born compliant. +- git blame noise is bounded to one revertable commit; optionally record the fmt commit hash in `.git-blame-ignore-revs` (coder's choice). + +**Commit sequence (each leaves check/fmt/clippy/test green):** + +1. `fmt: repo-wide baseline` (pure churn, no semantic diff — verify with `git diff -w` empty). +2. `sb-core: constants module` — add `constants.rs`, replace literals (combat, character, spells). No signature changes. +3. `sb-core: DomainError + signature conversions` — `error.rs`, convert 5 signatures, update 8 test assertions, convert `pick_enemy` to `Result`. +4. `sb-core: adventure service` — new module + tests A1–A8; no handler change yet (dead_code allow already present at crate level; remove nothing). +5. `sb-api: responses module + handler rewiring` — `responses.rs`, `domain_status`, `now_unix_ms`, run_adventure delegates to service, 4 handlers typed. +6. `sb-api: label helpers wiring + gym/stat_value + policy constants` (may fold into 5). +7. Full gate: `cargo fmt --check && cargo clippy --all-targets -- -D warnings && cargo test`. +8. EA model + generator pipeline commits (scope added at 1B rework) — see §11 for the plan; these live in the **skara-brae-common repo** (`scripts/openup/`, `docs/openup/model/`) and never touch skara-brae-server at all. + +--- + +## 9. Size planning inputs (for psp-coder PLN) + +Deleted/added estimates derive from: routes/mod.rs 1356 lines (run_adventure body ~150 → handler ~35; 6 json! sites → ~90 lines of DTO structs); new sb-core modules ~140 lines + ~200 test lines. PROBE by the coder; this WI's plan (660 LOC, PI 607–1295) already carries the churn. + +--- + +## 10. Findings & decisions — 1B design review disposition + +**Resolved at the question gate (PM/user decisions, 2026-10-03):** + +- **Q1 — RESOLVED (keep verbatim):** `learned_count` (routes/mod.rs:1184) carries the **total** known-spell count after learning, not the newly-learned count. Fenced as a wire wart in §4.3; no semantic change permitted. +- **Q2 — RESOLVED (confirmed):** new response DTOs stay **sb-api-local** (`routes::responses`); migration to a shared/OpenAPI home is deferred to the utoipa work item. +- **Q3 — RESOLVED (roadmap):** extraction of heal and spell learn/activate business logic out of handlers becomes a **roadmap.md candidate**; not part of WI-2 (out-of-scope note in §1). + +**1B review findings 1–13 disposition:** all fixed in this revision — F1 wire-order rule inverted (§4.3 rewritten; W1–W6 added §7), F2 gym stat order pinned (§4.5), F3 sb-db/sb-auth ledger (§6), F4 test-count reconciliation to 9 (§4.1/§5/test-cases.md), F5 lock-consolidation delta recorded (§4.2), F6 regen call sites corrected (use-case-model.md), F7 spell-level cap claim removed (use-case-model.md, §4.4), F8 constants ledger completed (§4.4), F9 spell-learn-cost guard semantics (§4.4), F10 crit-severity citations corrected (§4.4), F11 items/data wording (§2), F12 coder protocol restated (§4.6), F13 A5 de-flaked (§7). + +**Defect #14 (type: Interface, inject: DES, remove: Code — found by psp-coder during implementation prep):** §4.3's "nested structs emit declaration order today" claim was wrong; `json!` sorts keys at **every nesting level**. Corrected per PM decision (coder's option A): `AdventureRunResponse.combat_log` is a `serde_json::Value` bridge populated via `serde_json::to_value(&outcome.combat_log)` in the `From` impl — byte-identical to today's sorted wire; `CombatLog` remains a typed struct in the domain/outcome. W1 golden as specified passes and now also pins the nested ordering. Applied in this revision; W2–W6 confirmed unaffected by the coder. + +**Open QUESTION (new, from the rework scope): none.** The `.eapx`-in-a-versioned-repo recommendation was affirmed by the 2026-10-03 decision reversal with its home transposed to **skara-brae-common** (§11.a); keyfob-station precedent supports committing it. + +Non-blocking notes (no question needed): nerve is spent before enemy pick, so the (data-impossible) `UnknownEncounterEnemy` 500 leaves nerve deducted — today's panic had the same side effect in memory; `now_unix_ms` returning 0 on clock-before-epoch differs from a panic only on hardware that cannot exist in deployment. + +--- + +## 11. EA model & generator pipeline (scope added at 1B rework) + +The full EA model plus generator pipeline (keyfob-station style) is **in WI-2 scope**. The interactive `.eapx` editing session is a **human gate**; everything around it — skeleton, scripts, checklist derivation, exports — is agent work assigned to **COD** (commits 8–9 in §8). **Everything in this section lives in the skara-brae-common repo** (per the 2026-10-03 decision-reversal record in the header); nothing here touches skara-brae-server or any `src/`. + +### 11.a Model file home — decision: commit the `.eapx` into the skara-brae-common repo + +- **Home:** version the binary model at `skara-brae-common/docs/openup/model/skara-brae.eapx`, alongside generated exports at `skara-brae-common/docs/openup/model/exports/` (JSON, later PDF/PNG diagram renders); generator scripts at `skara-brae-common/scripts/openup/`. +- Rationale (post-reversal): skara-brae-common is the one canonical, versioned project-docs home for all OpenUP artifacts (header decision-reversal record) — the `.eapx` and its exports belong beside the markdown they render. The earlier "project-top folder is not versioned" argument is obsolete and superseded. keyfob-station precedent still applies: it commits **both** model and exports inside a repo (`docs/openup/model/keyfob-station.eapx` + `exports/`). The markdown artifacts + builder scripts remain the content source of truth; the `.eapx` is the human-gated rendering whose change history we keep (one binary per gate session bounds diff noise). +- Alternative rejected: model outside any repo, only exports committed — loses model history and splits the canonical home; binary-diff unfriendliness is inherited either way. +- Revisit trigger: if the `.eapx` grows beyond a few MB (diagram renders accumulate), switch that path to git-lfs; not needed now. + +### 11.b Agent preparation steps (COD) + +1. Clone the reference read-only: `git clone http://192.168.1.10:3000/jan/keyfob-station.git` (token `~/.config/forgejo/moria-token` if prompted). Never modify or push it. +2. Study two scripts as the adaptation source: `scripts/add-wi19-uc11.ps1` (scenario-embedding pattern: `Name/Type/Notes/Steps` arrays with per-step `Actor`) and `scripts/export-model-json.ps1` (COM walk of Actors/UseCases/scenarios → JSON). +3. Create the skeleton: copy the **EA installation's standard empty-model template `EABase.eapx`** to `skara-brae-common/docs/openup/model/skara-brae.eapx`. Note: keyfob carries **no** `EABase.eapx` in-repo (verified — only its derived `keyfob-station.eapx`); do **not** derive the skeleton from keyfob's model, it carries keyfob content. +4. Derive the build checklist from the mapping content: package tree per `uc-realization-mapping.md` §1; 3 actors + 12 use cases with their Basic Path / Alternate / Exception scenario lists from `requirements/use-case-model.md`; 12 `UC-N Realization` packages with elements from the mapping table in `uc-realization-mapping.md` §2. + +### 11.c Generator pipeline (COD; scripts in the skara-brae-common repo) + +- `skara-brae-common/scripts/openup/build-model.ps1` (adapted from keyfob's builder pattern): EA COM automation (`New-Object -ComObject EA.App`); creates/rebuilds the `Requirements` and `Analysis-Design` package trees; scenario content is read from a machine-readable intermediate `docs/openup/model/exports/use-case-model.json` (common-repo-relative) so the hand-authored markdown stays the human-facing source and scripts consume JSON (keyfob embeds the content directly in PowerShell arrays — we keep that as fallback if the JSON step proves friction). +- `skara-brae-common/scripts/openup/export-model.ps1` (adapted from keyfob's `export-model-json.ps1`): walks Actors/UseCases/scenarios/realization packages and regenerates `exports/use-case-model.json` + `exports/uc-realizations.json`; additionally triggers EA's native **XMI/XML export** as a lossless, diffable backup of the model. +- Drift rule (standards): markdown and model must agree; exports are generated, never hand-edited; a mismatch is a defect fixed in the same cycle (stale docs are defects). +- Environment: Windows + PowerShell + installed EA (as keyfob); scripts are committed to the skara-brae-common repo so the human gate is reproducible. + +### 11.d Human-gate checklist (user, in EA on Windows; all paths in skara-brae-common) + +1. Run `scripts/openup/build-model.ps1` against `docs/openup/model/skara-brae.eapx` (agent prepared it; user executes — EA must be installed/licensed). +2. In the EA UI verify: model opens clean; `Requirements → Use Case Model` shows 3 actors + 12 use cases; every UC carries its Basic Path plus the alternates/exceptions with per-step actors; 12 `UC-N Realization` packages exist under `Analysis-Design` containing the mapping elements. +3. Spot-check two scenarios against `requirements/use-case-model.md` and one realization table against `uc-realization-mapping.md` §2. +4. Run `scripts/openup/export-model.ps1`; confirm exports regenerate without unexpected diff (or review the diff). +5. Save the model; in the **skara-brae-common repo** commit `.eapx` + `exports/` + `scripts/openup/` together as one commit ("openup: EA model WI-2 baseline"). + +Phase assignment summary: 11.b steps 1–4 and 11.c scripts = **COD** (after the code commits in §8, in the skara-brae-common repo); 11.d = **HUMAN GATE** inside COD (`.eapx` editing/verification is never agent-performed); no PD interaction, no changes to skara-brae-server. diff --git a/docs/openup/analysis-design/uc-realization-mapping.md b/docs/openup/analysis-design/uc-realization-mapping.md new file mode 100644 index 0000000..64913c8 --- /dev/null +++ b/docs/openup/analysis-design/uc-realization-mapping.md @@ -0,0 +1,80 @@ +# Skara Brae — Analysis & Design Model (text form) + +Version 1.0 (WI-2, 2026-10-03) · OpenUP artifact: `analysis-design/uc-realization-mapping.md` +This is the **content the EA model will carry** once the modeling pipeline exists. The EA repository itself (`.eapx`, keyfob-station style) is planned for a later work item — EA is a human-gated interactive tool; this document is the authoritative source any future EA builder script generates from. + +## 1. Model structure (planned EA packages) + +``` +Model: Skara Brae +├── Requirements +│ ├── Vision (from requirements/vision.md) +│ ├── Use Case Model +│ │ ├── Actors: Player, Administrator, Guest +│ │ └── Use Cases UC-01 … UC-12 (from requirements/use-case-model.md; +│ │ scenarios as Basic Path / Alternate / Exception with per-step Actor) +│ └── Supplementary Specifications (from requirements/supplementary-specifications.md) +└── Analysis-Design + ├── Architecture Overview (crate/layer diagram, dependency rule) + └── UC-01 Realization … UC-12 Realization (one package per UC: analysis classes, + design elements, realization table below) +``` + +## 2. UC realization mapping (analysis classes → design elements) + +Legend — **Analysis class** (OpenUP stereotype): `«controller»`, `«entity»`, `«boundary»`. **Design element**: crate::module::item (Rust). "WI-2" marks elements introduced or re-homed by this work item; everything else exists at 899fde8. + +| UC | Analysis class | Stereotype | Design element | Notes | +|---|---|---|---|---| +| UC-01 Register | AccountRegistry | controller | `sb-api::routes::{register, seed_users}` | argon2 hash via `sb-auth::hash`; JWT via `sb-auth::jwt` | +| | UserAccount | entity | `sb-shared::user::User`, `Role` | persisted via `sb-db::Database` | +| | AuthBoundary | boundary | `POST /api/auth/register`, `AuthResponse` | | +| UC-02 Authenticate | Authenticator | controller | `sb-api::routes::login`, `sb-auth::{hash::verify_password, jwt::create_token}` | | +| | TokenService | boundary | `POST /api/auth/login`, `AuthResponse`, `middleware::auth::auth_middleware` | middleware guards all UC-03…12 | +| UC-03 Manage Characters | CharacterFactory | controller | `sb-core::character::create_character` | takes injected rng (WI-1) | +| | Character | entity | `sb-shared::character::Character` (+ `Race`, `Class`, `CharacterStats`) | | +| | CharacterCatalog | boundary | `GET/POST /api/users/{id}/characters`, `GET /api/characters/{id}` | store: `AppState.characters` | +| | ResourceRegenerator | entity | `sb-core::character::apply_regen` (+ regen constants — WI-2 names them) | included sub-flow | +| UC-04 Browse Catalogue | CatalogueBrowser | boundary | `GET /api/{races,classes,items,enemies,adventures}`; response types `sb-shared::responses` | data from `sb-common` JSON via `sb-core::{character::RACES/CLASSES, data, items}` | +| | GameData | entity | `sb-core::data::{EnemyDef, ZoneDef, EncounterEntry, LootDrop}`, `items::load_all_items` | | +| UC-05 Train Stat | TrainingService | controller | `sb-core::services::train_stat` | error type `DomainError` (WI-2) | +| | TrainingGrounds | entity | `sb-core::character::TRAINING_GROUNDS`, `training_cost` (+ cost constants WI-2) | | +| | GymBoundary | boundary | `GET/POST /api/characters/{id}/gym`, `GymResponse`, `TrainStatResponse` | | +| UC-06 Manage Equipment | EquipmentService | controller | `sb-core::services::{equip_item, unequip_item}` | `DomainError` (WI-2) | +| | Equipment/Inventory | entity | `Character.equipment` / `.inventory`, `sb-shared::item::Item` | | +| | EquipmentBoundary | boundary | `POST /api/characters/{id}/{equip,unequip}` | | +| UC-07 Trade at Shop | ShopService | controller | `sb-api::routes::{shop_action, buy_item, sell_item}` (handler-side; extraction candidate, design note Q3) | prices: `sb-core::items::{item_price, item_sell_price}` | +| | ShopStock | entity | `AppState.shop_stock` | persisted via `sb-db` | +| | ShopBoundary | boundary | `GET/POST /api/characters/{id}/shop`, `ShopResponse`, `ShopActionResponse` | | +| UC-08 Run Adventure | **AdventureService** | controller | `sb-core::adventure::run_adventure` (**new, WI-2**) + `services::pick_enemy` → `Result` (WI-2) | takes injected rng; `DomainError` (WI-2) | +| | CombatEngine | entity | `sb-core::combat::{resolve_combat_full, resolve_attack, crit tables, open_ended_roll}` + combat constants (WI-2) | | +| | Combatant | entity | `sb-core::combat::CombatStats` | built from Character/EnemyDef | +| | SpellEffect / BardSong | entity | `sb-core::combat::SpellEffect`, `sb-core::spells`, `sb-core::songs` | | +| | AdventureBoundary | boundary | `GET /api/characters/{id}/adventure`; `AdventureRunResponse` (**new, WI-2**), `sb-shared::responses::CombatLog` | | +| UC-09 Heal at Temple | HealingService | controller | `sb-api::routes::heal_character` (handler-side; extraction candidate, Q3) | | +| | TempleBoundary | boundary | `POST /api/characters/{id}/heal`; `HealResponse` (**new, WI-2**) | | +| UC-10 Practice Bard Song | SongService | controller | `sb-api::routes::set_song`; `sb-core::songs::available_songs` | | +| | BardSong | entity | `sb-core::songs::BARD_SONGS`, `Character.active_song` | | +| | SongBoundary | boundary | `GET/POST /api/characters/{id}/songs`, `SongsResponse`, `SetSongResponse` | | +| UC-11 Study Magic | MagicService | controller | `sb-api::routes::{get_spells, learn_spell_level}`; `sb-core::spells::{spells_for_class, spell_learn_cost, max_spell_points, caster_spell_level (WI-2)}` | | +| | Spell | entity | `sb-core::spells::SpellDef`, `Character.{known_spells, active_spell, spell_points}` | | +| | MagicBoundary | boundary | `GET/POST /api/characters/{id}/spells`; `SpellbookResponse`, `LearnSpellResponse` (**new, WI-2**) | | +| UC-12 Level Up | ReviewBoard | controller | `sb-core::character::level_up` | `DomainError` (WI-2) | +| | Progression | entity | `sb-core::character::{xp_for_level, level_up_cost, MAX_LEVEL}` (+ curve constants WI-2) | | +| | ReviewBoundary | boundary | `POST /api/characters/{id}/level-up`, `LevelUpResponse` | | + +Cross-cutting design elements (WI-2): `sb-core::error::DomainError` + `sb-api::routes::domain_status` (status mapping table, design note §4.1) realize the error paths of UC-05/06/08/12; `sb-core::constants` realizes NFR "no magic numbers"; `sb-api::routes::responses` realizes NFR-19 (wire-format freeze). + +Dependency rule check (Clean Architecture): all controllers/entities above live in `sb-core`/`sb-shared` (domain side, no HTTP imports); all boundaries live in `sb-api`; the single current violations (ShopService, HealingService, parts of MagicService in sb-api) are recorded as debt with a follow-up question — the model must not paper over them (stale docs are defects). + +## 3. Research summary — keyfob-station EA pipeline (reference) + +Source: read-only clone `http://192.168.1.10:3000/jan/keyfob-station.git` (inspected 2026-10-03; no modifications). + +- **Model file:** `docs/openup/model/keyfob-station.eapx` (MS Access database — the EA native format; EABase-derived). Never hand-edited. +- **Structure:** Model → `Requirements` package → `Use Case Model` package containing `Actor` and `UseCase` elements; scenario lists are stored on each use case as named scenarios of type **Basic Path / Alternate / Exception**, each with ordered steps carrying a per-step **Actor** tag and notes. An `Analysis-Design` package holds **one `UC-N Realization` package per use case** (realization diagrams + design elements). +- **Builders are surgical scripts:** `scripts/add-*.ps1` (PowerShell driving the EA COM object `New-Object -ComObject EA.App`, `$repo.OpenFile($modelPath)`). Each script opens the model, adds/rebuilds exactly one package's contents, and closes it — because EA repository IDs and package rebuilds clobber manual edits, hand edits do not survive; all content lives in the scripts (scenario text is embedded as PowerShell arrays of `Name/Type/Notes/Steps`). This matches our standards note: *model additions are surgical; builders rebuild whole packages*. +- **Exports:** `scripts/export-model-json.ps1` walks the Use Case Model package and emits `exports/use-case-model.json` + `supplementary-requirements.json`; realization diagrams are exported to PDF/PNG into `docs/openup/analysis-design/`. The JSON export is the doc-machine readable form of the same content. +- **Windows-only:** the pipeline runs on Windows (EA is a Win32 app; scripts use `.ps1` + COM). For Skara Brae, the same pattern applies when the modeling WI lands: keep `docs/openup/*.md` (this file set) as the source the builder scripts embed, never the reverse. + +Adoption decision for Skara Brae (updated at 1B rework): the EA model + generator pipeline is **in WI-2 scope**. This document (§1–2) plus `requirements/use-case-model.md` are the content source; the model-home recommendation, agent preparation steps, generator scripts (`scripts/openup/build-model.ps1`, `export-model.ps1`), and the human-gate checklist are specified in `design-wi-2-architecture-cleanup.md` §11. The `.eapx` interactive editing session remains human-gated; everything around it is COD-phase agent work. diff --git a/docs/openup/requirements/supplementary-specifications.md b/docs/openup/requirements/supplementary-specifications.md new file mode 100644 index 0000000..f876da5 --- /dev/null +++ b/docs/openup/requirements/supplementary-specifications.md @@ -0,0 +1,51 @@ +# Skara Brae — Supplementary Specifications + +Version 1.0 (WI-2, 2026-10-03) · OpenUP artifact: `requirements/supplementary-specifications.md` +Non-functional requirements and constraints for the server MVP. Facts cite code where they are implemented; assumptions are marked. + +## 1. Performance / Scalability + +- **NFR-01 (in-memory state):** all game state (users, characters, shop stock) is held in process memory behind `tokio::sync::RwLock` (`crates/sb-api/src/routes/mod.rs:33–42`). Single-process deployment; persistence is write-through to SQLite, reads never hit the DB. +- **NFR-02 (lazy regeneration):** resource regeneration is computed on every character access from elapsed wall-clock time (`apply_regen`, character.rs:105–122) — no timers, no background ticks. Intervals: energy 180 s, nerve 300 s, HP 300 s (5% max HP per tick). +- **NFR-03 (client polling):** clients poll character state to observe regeneration; the assumed UI refresh cadence is ~15 s *(assumption carried from project planning; server-side behavior is poll-frequency-independent by design NFR-02)*. +- **NFR-04 (static catalogue):** items, enemies, zones, spells, and crit tables are compiled in via `include_str!` from `skara-brae-common` JSON and parsed once behind `LazyLock`; catalogue reads are lock-free after first access. + +## 2. Reliability / Correctness + +- **NFR-05 (deterministic domain logic):** all domain randomness is injected `StdRng` (WI-1); identical seeds produce identical outcomes. Request paths construct RNG only at `request_rng()`. +- **NFR-06 (persistence best-effort):** persistence failures are logged (`tracing::error`) and do not fail the request; in-memory state remains authoritative for the process lifetime. +- **NFR-07 (referential data integrity):** zone encounter tables must reference existing enemy ids and loot drops must reference existing item ids; violations are detected at first use (WI-2: `DomainError::UnknownEncounterEnemy` → 500 instead of a panic). + +## 3. Quality Gates (per-commit acceptance) + +- **NFR-08:** `cargo fmt --check` clean (fmt baseline established by WI-2). +- **NFR-09:** `cargo clippy --all-targets -- -D warnings` clean — zero-warning policy. +- **NFR-10:** `cargo test --workspace` fully green (45 sb-core tests at WI-2 start; suite only grows). +- **NFR-11 (WI-2-specific):** behavior-identical refactor — JSON field names/order, HTTP status codes, game semantics, and RNG draw order unchanged. +- **NFR-12:** no `unwrap()`/`expect()` on request paths outside the documented keep-with-reason invariants in the design-note §6 ledger (covers sb-api, sb-core, sb-db mutex poisoning, sb-auth time/entropy invariants). + +## 4. Data + +- **NFR-13 (shared data source):** all game content lives in the `skara-brae-common` repo as language-neutral JSON under `data/` — single source of truth consumed at build time by server (`include_str!` + serde) and client. Server must not duplicate or transform these definitions in code. +- **NFR-14 (persistence format):** optional SQLite via `SB_DB_PATH` (rusqlite; users, characters, shop stock with `next_character_id`). Absent the variable the server runs purely in-memory with seeded `admin`/`player` accounts. +- **NFR-15 (numeric headroom):** stats are `i64` (BIGINT) — “exponential growth, tiered display” is a product commitment; no narrowing conversions to smaller integers in new code. + +## 5. Security + +- **NFR-16:** passwords hashed with argon2; verification constant-time via the crate. +- **NFR-17:** JWT (HS256, `jsonwebtoken`), 24 h expiry; all game routes behind auth middleware; cross-user access requires the `admin` role (403 otherwise). +- **NFR-18:** no secrets in code; JWT secret and admin/player seed passwords come from environment/configuration. + +## 6. Constraints + +- **CON-01:** Rust, edition **2024**; workspace crates `sb-api` / `sb-core` / `sb-db` / `sb-auth` / `sb-shared` + external sibling `sb-common` (absolute path dependency; submodule migration planned). +- **CON-02:** HTTP framework **Axum 0.8** (ws + macros features), tokio, tower stack (CORS/trace/compression-gzip). +- **CON-03:** `rand 0.9`; domain functions take `rng: &mut impl Rng`; handlers obtain RNG only from `request_rng()` (WI-1 rule). +- **CON-04:** Error handling: `thiserror 2` in-domain; **no new error/framework dependencies**; one domain error enum (`sb_core::error::DomainError`) with a single central HTTP status mapping in sb-api. +- **CON-05:** Clean Architecture dependency rule: sb-core has zero knowledge of HTTP/axum/serde_json/tokio/tracing/sb-db; ports (domain traits) would live in sb-core, adapters outward. +- **CON-06:** OpenUP work products are versioned in the **skara-brae-common repo** under `docs/openup/` — one canonical project-docs home (decision reversed 2026-10-03 from the earlier skara-brae-server `docs/openup/` plan). The drafted WI-2 artifacts move there verbatim (coder, session 2); WI-1 artifacts at the old project-top folder migrate there as well. `roadmap.md` stays at the project-top folder per the earlier PM decision. + +## 7. Usability / API + +- **NFR-19:** REST JSON API; field names and order are a wire contract — refactors must preserve them byte-for-byte. The current API emits JSON object keys in **lexicographic order** (serde_json resolves without `preserve_order`; `json!` uses BTreeMap-backed maps), and the typed-response structs of WI-2 declare fields in that order to stay byte-identical. +- **NFR-20:** meaningful HTTP status codes: 400 malformed input, 401 auth failure, 403 foreign/non-permitted resource, 404 unknown resource, 409 name conflict, 410 shop stock exhausted, 422 game-rule violation (resources/requirements), 500 internal. diff --git a/docs/openup/requirements/use-case-model.md b/docs/openup/requirements/use-case-model.md new file mode 100644 index 0000000..79d05fb --- /dev/null +++ b/docs/openup/requirements/use-case-model.md @@ -0,0 +1,170 @@ +# Skara Brae — Use Case Model + +Version 1.0 (WI-2, 2026-10-03) · OpenUP artifact: `requirements/use-case-model.md` +Scope: the **existing implemented system** (server MVP as of `wi-2-architecture-cleanup` @ 899fde8). Every flow below is derived from actual handler code (`crates/sb-api/src/routes/mod.rs`) — status codes cited are the implemented behavior and are normative. + +## Actors + +| Actor | Description | +|---|---| +| **Player** | Authenticated user owning one or more characters (JWT role `player`). | +| **Administrator** | Player with elevated rights (`role = admin`); may view any user's and any character's resources read-only. | +| **Guest** | Unauthenticated caller; only register/login are open. | + +## Use Case Overview + +| ID | Use Case | Primary Actor | Includes / Extends | +|---|---|---|---| +| UC-01 | Register Account | Guest | — | +| UC-02 | Authenticate | Guest | — | +| UC-03 | Manage Characters | Player (Admin read) | — | +| UC-04 | Browse Game Catalogue | Player | — | +| UC-05 | Train Stat | Player | includes Regenerate Resources | +| UC-06 | Manage Equipment | Player | — | +| UC-07 | Trade at Shop | Player | — | +| UC-08 | Run Adventure | Player | includes Regenerate Resources, Resolve Combat | +| UC-09 | Heal at Temple | Player | — | +| UC-10 | Practice Bard Song | Player | — | +| UC-11 | Study Magic | Player | — | +| UC-12 | Level Up at Review Board | Player | — | + +Shared sub-flow **Regenerate Resources**: the system recomputes time-based regeneration **at specific call sites only** — list characters, view character, run adventure, and train stat (routes/mod.rs:264–266, 331, 442, 642). Energy +1 per 180 s, nerve +1 per 300 s, HP +5% of max per 300 s (only if alive and injured), capped at maxima; `last_regen` advances to now. It is **not** applied by equip/unequip, gym view, shop, heal, songs, spells, or level-up. + +--- + +## UC-01 Register Account + +**Precondition:** none. **Main success scenario:** +1. Guest submits name (3–32 chars) and password (≥ 6 chars). +2. System verifies the name is unused → 409 CONFLICT if taken. +3. System hashes the password (argon2), assigns the `player` role and the next id, persists the user. +4. System issues a JWT (HS256, `player` role) and returns `{token, user_id, name, role}`. + +**Alternate flows:** name length invalid → 400; password too short → 400; name taken → 409; hashing/token failure → 500. + +## UC-02 Authenticate + +**Main success scenario:** +1. Guest submits name and password. +2. System finds the user (401 if unknown) and verifies the password hash (401 on mismatch). +3. System issues a JWT carrying the user's real role (`admin`/`player`) and returns `{token, user_id, name, role}`. + +**Alternate flows:** unknown name or wrong password → 401 UNAUTHORIZED; token creation failure → 500. + +## UC-03 Manage Characters + +**Main success scenario (create):** +1. Player submits character name, race, class for their user id (Admin may act for any user). +2. System validates the target user exists (404) and the trimmed name is 1–32 chars (400). +3. System rolls stats (d6+5 base 6–16 per stat ± race modifiers), computes max HP/energy/nerve from class, grants starting gold (50) and level-1 class spells to casters. +4. System assigns the next character id, stores and persists the character; returns it fully. + +**Scenario (list/view):** Player lists their characters (Admin: any user's); the system applies Regenerate Resources to each returned character and returns them. Viewing one character applies regen then returns it. + +**Alternate flows:** foreign user/character without admin role → 403 FORBIDDEN; user/character not found → 404; invalid name → 400. + +## UC-04 Browse Game Catalogue + +**Main success scenario:** +1. Player requests the catalogue: races, classes, items (optional type/quality/slot filters), enemies, or adventure zones. +2. System returns static game data loaded from `skara-brae-common` JSON (class info is enriched with `is_caster`). + +**Alternate flows:** single item by unknown id → 404; filters may return an empty list (not an error). + +## UC-05 Train Stat + +**Precondition:** character exists and caller may act on it. **Main success scenario:** +1. Player selects a training ground (strength / dexterity / constitution / intelligence / luck) and submits the stat name. +2. System applies Regenerate Resources. +3. System checks energy ≥ 1 and gold ≥ training cost (cost grows ~1.3× per 10 stat points). +4. System deducts 1 energy and the gold cost, increments the stat; constitution training recomputes max HP (current HP clamped). +5. System persists and returns `{success, message, character}`. + +**Alternate flows:** foreign character → 403; character not found → 404; not enough energy → 422; not enough gold → 422; unknown stat name → 400. + +## UC-06 Manage Equipment + +**Main success scenario (equip):** +1. Player submits an item id from their inventory. +2. System moves the item from inventory to its slot; a previously equipped item in that slot returns to inventory. +3. System persists and returns the updated character. + +**Scenario (unequip):** Player submits a slot name; the system moves the equipped item back to inventory. + +**Alternate flows:** foreign character → 403; character/item not found → 404; item not in inventory → 400; nothing equipped in slot → 400. + +## UC-07 Trade at Shop + +**Main success scenario (buy):** +1. Player views the shop: always-stocked items plus rotating stock (with quantities) alongside their inventory. +2. Player submits `buy` + item id. +3. System checks gold ≥ price (422), and for non-always-in-stock items decrements the limited stock (410 GONE if 0). +4. System deducts gold, adds the item to inventory, persists character and stock; returns `{success, message, gold}`. + +**Scenario (sell):** Player submits `sell` + item id; system removes the first matching inventory item, pays half the item value, increments shop stock for non-permanent items. + +**Alternate flows:** unknown action or item not owned (sell) → 400; item not found → 404; insufficient gold → 422; stock exhausted → 410. + +## UC-08 Run Adventure + +**Precondition:** character exists and caller may act on it. **Main success scenario:** +1. Player requests an adventure for a zone (default `skara_brae_streets`). +2. System validates the zone (404) and applies Regenerate Resources. +3. System refuses incapacitated characters (HP ≤ 0 → 400) and characters with nerve below the zone cost (422); otherwise deducts the nerve cost. +4. System draws one enemy from the zone's weighted encounter table. +5. System resolves turn-based combat: open-ended d100 attack rolls, OB vs DB, RoleMaster crit tables with level-gap damage caps, active bard song effects, active spell damage/heal per round. +6. On player victory: XP and gold per the enemy definition, character HP set to the last round's defender HP, and loot dropped per-item chance rolls (only items existing in the catalogue). +7. On defeat: character HP set to 0; no rewards. +8. Spell-point upkeep is charged whenever a spell was active, win or lose. +9. System persists the character and returns `{zone, enemy, combat_log, loot}` (loot = item display names). + +**Alternate flows:** foreign character → 403; character/zone not found → 404; incapacitated → 400; insufficient nerve → 422. **Exception:** encounter references an unknown enemy (data error) → 500 (post WI-2; previously a panic). + +## UC-09 Heal at Temple + +**Main success scenario:** +1. Player requests healing, optionally `partial`. +2. System computes missing HP; full heals all, partial heals half (rounded up); cost = amount healed, minimum 1 gold. +3. System checks gold (422), deducts it, restores HP, persists; returns `{hp, max_hp, gold, healed: true}`. + +**Alternate flows:** foreign character → 403; not found → 404; already at full HP → 400; insufficient gold → 422. + +## UC-10 Practice Bard Song + +**Precondition:** character class is Bard. **Main success scenario:** +1. Player views available songs (level-gated, with combat effect descriptions) and the active song. +2. Player submits a song id; system validates level requirement (422) and sets it active; returns `{success, message, active_song}`. + +**Scenario (clear):** submitting no song id clears the active song. **Alternate flows:** non-Bard → 403; unknown song → 404; level too low → 422; foreign character → 403; not found → 404. + +## UC-11 Study Magic + +**Precondition:** character belongs to a caster class (Mage→magician, Wizard, Conjurer, Sorcerer). Spell level = (character level / 3) + 1 — **uncapped by code** (level 100 → spell level 34); learn *requests* are validated to 1–7, and the practical ceiling is emergent from the data (spells are defined only through level 7, so higher computed levels surface no new spells). + +**Scenario (view spellbook):** +1. Player requests their spellbook: caster class, current spell level, spell points, spells up to the achievable level (each marked known), and the learn-cost table for levels 2–7. + +**Scenario (learn spell level):** +1. Player submits a spell level (1–7). +2. System validates caster class (403), level range (400), level not above the character's current spell level (422), gold ≥ learn cost (422), and that new spells exist (400). +3. System deducts gold, adds all unknown spell ids of that level, recomputes and refills max spell points; returns `{learned_count, spell_points, max_spell_points, gold}`. + +**Scenario (activate/clear active spell):** +1. Player submits a spell id (or empty to clear). +2. System validates the spell is known (403), exists (404), and spell points suffice (422); deducts the spell's points and sets it active; returns `{active_spell, spell_points}`. + +## UC-12 Level Up at Review Board + +**Main success scenario:** +1. Player requests a level-up. +2. System validates: below max level 100, XP ≥ 18·100·(level)² for the next level (i.e. `xp_for_level(next)`), gold ≥ 500·next level. +3. System deducts the gold cost, increments the level, recomputes and fully restores HP/energy/nerve and max spell points; persists; returns `{success, message, character}`. + +**Alternate flows (all 422):** already at max level; not enough XP; not enough gold. Foreign character → 403; not found → 404. + +--- + +## Traceability Notes + +- UC-08 is the design driver for WI-2's service extraction (`sb_core::adventure`); its status mapping feeds `DomainError` (see `analysis-design/design-wi-2-architecture-cleanup.md` §4.1–4.2). +- Catalogue data backing UC-04 lives in `skara-brae-common/data/` (items, enemies, zones, spells, crit tables). diff --git a/docs/openup/requirements/vision.md b/docs/openup/requirements/vision.md new file mode 100644 index 0000000..080cc11 --- /dev/null +++ b/docs/openup/requirements/vision.md @@ -0,0 +1,41 @@ +# Skara Brae — Vision + +Version 1.0 (WI-2, 2026-10-03) · OpenUP artifact: `requirements/vision.md` · Canonical home: **skara-brae-common repo**, `docs/openup/` (decision reversed 2026-10-03; drafts move verbatim from skara-brae-server/docs/) + +## Problem / Opportunity + +Text-based browser MMORPGs have proven retention and monetization for two decades (Torn City, since 2004), but incumbents carry 2000s-era engines. There is room for a modern, API-first, type-safe implementation of the formula: deep numeric progression, player-driven economy, idle-friendly pacing, real-time feedback. + +## Product Vision + +**Skara Brae** is a fantasy text-based/idle MMORPG set in an ancient, labyrinthine city of stone and sorcery (The Bard's Tale I inspiration, dark-fantasy tone). It transplants **Torn City's proven loop** — energy-gated activities, training, economy, social systems — into that setting, adds **idle-RPG elements** (offline progression, hireable workers, pets; Idle Fantasy inspiration), and grounds combat in a **RoleMaster-inspired critical-hit system** (open-ended d100, crit tables with bleed/stun effects, level-gap damage caps). + +**Tagline:** *Set your hero to work. Close the app. Come back to loot.* + +## Stakeholders + +- **Owner/developer (solo)** — builds and operates the game on self-hosted infrastructure (Forgejo/Jenkins pipeline). +- **Players** — web/PWA users; the API-first contract also serves future community tools. + +## Key Features (committed MVP scope, Phase 1) + +1. Accounts (register/login, JWT) and character creation (5 races, 10 classes). +2. Stat training with escalating gold costs; energy/nerve/HP resources with lazy timed regeneration. +3. Adventures (energy/nerve-gated zone runs) with turn-based combat, crit tables, XP/gold/loot. +4. Economy: shop (buy/sell, stock), equipment slots, inventory. +5. Magic (class-gated spell levels, spell points) and bard songs (combat buffs). +6. Level-up review board; temple healing. +7. REST API (Rust/Axum) + Vue 3 client; WebSocket notifications. + +Later phases (not committed): PvP, guilds, properties, crafting, pets, blessings, offline workers, tower endgame. + +## Constraints & Quality Bars + +- Behavior continuity: refactors must be wire-format and game-semantics identical (WI-2 governs). +- Quality gates: `cargo fmt --check`, `cargo clippy --all-targets -- -D warnings`, full test suite — zero-warning policy. +- Game data lives in the shared `skara-brae-common` repo as language-neutral JSON (single source of truth for server, client, and a future generated wiki). +- BIGINT (`i64`) stat growth; stats only ever go up. + +## Success Criteria (current iteration) + +WI-1 delivered injectable RNG (deterministic domain tests). This iteration (WI-2) delivers an architecture cleanup that makes the codebase safe to grow on: typed responses, domain-extracted adventure service, named game-balance constants, one domain error taxonomy, fmt baseline — with all 45 existing tests green and zero behavior change. Longer term: a playable MVP per the feature list above. diff --git a/docs/openup/test/test-cases.md b/docs/openup/test/test-cases.md new file mode 100644 index 0000000..76b578a --- /dev/null +++ b/docs/openup/test/test-cases.md @@ -0,0 +1,60 @@ +# Skara Brae — Test Cases (existing coverage mapped to use cases) + +Version 1.0 (WI-2, 2026-10-03) · OpenUP artifact: `test/test-cases.md` +Baseline: 45 unit tests, all green on `wi-2-architecture-cleanup` @ 899fde8 (`cargo test --workspace`). + +## 1. Existing tests → use-case mapping + +| Test (file::name) | Verifies | UC | +|---|---|---| +| character::`stat_bonus_at_20_is_zero` / `_below_20_is_negative` / `_above_20_is_positive` / `_handles_large_values` | OB/DB stat-bonus curve incl. BIGINT headroom | UC-08, UC-05 | +| character::`xp_for_level_1_is_zero` / `_2_is_1800` / `_scales_subexponentially` / `_does_not_overflow` | XP curve 18·100·(level−1)² | UC-12 | +| character::`level_up_fails_at_max_level` / `_succeeds_with_enough_xp_and_gold` / `_fails_without_enough_xp` / `_fails_without_enough_gold` | review-board rules | UC-12 | +| character::`level_up_cost_scales` | 500·level cost | UC-12 | +| character::`calc_max_hp_depends_on_class_and_con` / `calc_max_energy_and_nerve_vary_by_class` | class tables | UC-03 | +| character::`training_cost_scales_with_stat` | 5·1.3^(stat/10) cost curve | UC-05 | +| character::`apply_regen_restores_resources` / `_no_change_if_no_time_passed` | lazy regen (180 s / 300 s / 300 s) | shared sub-flow Regenerate Resources | +| character::`roll_stats_returns_all_five` / `race_modifiers_apply` | character creation rolls ± race mods | UC-03 | +| character::`create_character_is_deterministic_under_seeded_rng` | WI-1 injectable RNG at creation | UC-03, NFR-05 | +| combat::`open_ended_roll_stays_in_range` | d100 open-ended re-rolls (≥96 / ≤5) | UC-08 | +| combat::`offensive_bonus_includes_stat_and_base` / `defensive_bonus_includes_stat_and_base` | OB/DB composition | UC-08 | +| combat::`crit_damage_cap_equal_level_is_none` / `_higher_defender_reduces` / `_scaling` | level-gap crit caps (0.60/0.40/0.15/0.05) | UC-08 | +| combat::`resolve_attack_can_hit_or_miss` | round structure and HP clamping | UC-08 | +| combat::`resolve_combat_produces_winner` | fight termination, winner/loser | UC-08 | +| combat::`parse_attack_type_maps_known_types` | slash/crush/puncture + default crush | UC-08 | +| services::`pick_enemy_respects_weights` | weighted encounter draw | UC-08 | +| services::`train_stat_increases_stat` / `_fails_without_energy` / `_fails_without_gold` / `_fails_invalid_stat` / `train_constitution_recals_hp` | gym rules incl. max-HP recompute | UC-05 | +| services::`equip_item_moves_from_inventory_to_slot` / `_swaps_previous` / `_fails_if_not_in_inventory` | equip semantics | UC-06 | +| services::`unequip_item_moves_to_inventory` / `_fails_empty_slot` | unequip semantics | UC-06 | +| services::`equipped_damage_sums_weapon_slots` / `total_defense_sums_all_slots` / `equipped_attack_type_returns_weapon_type` / `_defaults_to_crush` | equipment → combat stats | UC-08 | +| (WI-1) seeded determinism pattern across the above | RNG injection | NFR-05 | + +## 2. Coverage gaps the WI-2 refactor must not lose (and currently uncovered areas) + +**Covered code being moved** — the run_adventure handler body (routes/mod.rs:409–567) is currently **not directly tested**; its ingredients (regen, pick_enemy, equipped stats, combat resolution) are covered per §1. WI-2 therefore **adds** direct service-level tests while moving the code: + +| New test | Vector (design note §7) | UC | +|---|---|---| +| adventure::`run_adventure_win_applies_rewards_and_loot` | A1: seeded win → xp/gold/nerve/loot/inventory/exact outcome fields | UC-08 main | +| adventure::`run_adventure_defeat_sets_hp_to_zero` | A2: overwhelming enemy → no rewards | UC-08 alt | +| adventure::`run_adventure_rejects_incapacitated` | A3: hp = 0 → `CharacterIncapacitated`, nerve intact | UC-08 exception | +| adventure::`run_adventure_rejects_insufficient_nerve` | A4 → `NotEnoughNerve` | UC-08 exception | +| adventure::`run_adventure_applies_regen_before_guards` | A5: regen visible in combat HP | shared sub-flow | +| adventure::`run_adventure_charges_spell_upkeep_on_win_and_loss` | A6 both paths | UC-08 step 8 / UC-11 interplay | +| adventure::`run_adventure_filters_loot_to_catalog_items` | A7 drop for unknown item id skipped | UC-08 step 6 / NFR-07 | +| adventure::`run_adventure_is_deterministic_under_seeded_rng` (+ optional golden CombatLog fixture) | A8 two seeds-42 runs identical | NFR-05 | +| routes::`wire_identity_adventure_response` … `wire_identity_learn_response` (W1–W6) | byte-for-byte `serde_json::to_string` equality of each new DTO against golden strings recorded from the current `json!` expressions **before** the refactor; golden keys in lexicographic order **at every nesting level** (serde_json has no `preserve_order` — json! objects emit sorted keys; the adventure DTO's `combat_log` is the §4.3 Value bridge, defect #14) | UC-08/09/11, NFR-19 | + +**New helper/constant pins** (design note §4.4–4.5): `as_str` parity for `ItemType`/`Quality` (wire-format guarantee NFR-19), `gym_display_order_covers_all_training_grounds` (pins GymResponse.stats order to routes/mod.rs:588–594, which differs from TRAINING_GROUNDS order), `race/class` table exhaustiveness, embedded-JSON loader parses (5), `caster_spell_level` spot values, `spell_learn_cost` guard spot checks (0, −1, 8 → 999_999). + +**Known remaining gaps (documented, out of WI-2 scope — see design note Q3):** +- Heal cost logic (UC-09) and spell learn/activate rules (UC-11) live in handlers and have no unit tests; WI-2 only types their responses. +- Shop buy/sell stock transitions (UC-07), auth flows (UC-01/02), and level-up handler status mapping (UC-12 → all 422) have no HTTP-level integration tests. The exhaustive `domain_status` match (compile-time totality) is the WI-2 mitigation for the mapping itself. +- No test asserts JSON field **order**; the response structs declare fields in wire order and the golden CombatLog fixture pins the largest payload. + +## 3. Regression protocol for WI-2 (coder must run per commit) + +1. `cargo fmt --check` +2. `cargo clippy --all-targets -- -D warnings` +3. `cargo test --workspace` — 45 baseline tests green at every step; the only permitted test edits are the **9** mechanical assertion updates listed in design note §4.1 (3 `level_up`, 3 `train_stat`, 1 `equip_item`, 1 `unequip_item`, 1 `pick_enemy_respects_weights` — the last gains an Ok-path `.unwrap()` because `pick_enemy` becomes `Result`-returning) plus newly added tests. +4. Manual wire spot-check (recommended once, after commit 5): run the server, exercise `GET /api/characters/{id}/adventure`, `POST …/heal`, `GET …/spells`, `POST …/spells` (both modes) and diff responses **byte-for-byte** against pre-refactor output — field names, key **order** (lexicographic, since serde_json runs without `preserve_order`), null-ness, number formatting.