skara-brae-common/docs/openup/analysis-design/design-wi-2-architecture-cleanup.md

# 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<AdventureOutcome> 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<i32, &'static str>` | `Result<i32, DomainError>` (AlreadyAtMaxLevel, NotEnoughXp, NotEnoughGold) |
| `services::train_stat(&mut Character, &str) -> Result<i64, &'static str>` | `Result<i64, DomainError>` (NotEnoughEnergy, InvalidStat, NotEnoughGold) |
| `services::equip_item(...) -> Result<(), &'static str>` | `Result<(), DomainError>` (ItemNotInInventory) |
| `services::unequip_item(...) -> Result<String, &'static str>` | `Result<String, DomainError>` (NothingEquippedInSlot) |
| `services::pick_enemy(...) -> EnemyDef` | `Result<EnemyDef, DomainError>` (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<String>, // 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<AdventureOutcome, DomainError>;
```

**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<Json<responses::AdventureRunResponse>, 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<AdventureOutcome> 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<String, Value>` 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<AdventureOutcome>` 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<String, i32>`: 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<String>,
    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<String, i32>,
    pub max_spell_points: i32,
    pub spell_level: i32,
    pub spell_points: i32,
    pub spells: Vec<SpellEntryResponse>,
}
// 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<String>,   // 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<Json<serde_json::Value>, StatusCode>` to:
- `run_adventure` → `Result<Json<AdventureRunResponse>, StatusCode>`
- `heal_character` → `Result<Json<HealResponse>, StatusCode>`
- `get_spells` → `Result<Json<SpellbookResponse>, 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<Json<LearnSpellResponse>, 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<sb_core::adventure::AdventureOutcome> 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<i64>;
// 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<AdventureOutcome>` 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.