sts2-bot/docs/research/05-failure-modes.md

# 05 — Failure modes

Every entry below was observed **live**, caused a real infinite loop or a
stalled run, and has a fix in the code. None are theoretical.

The general lesson: this interface is full of actions that report
`{"status": "ok"}` while doing nothing. **Never trust `ok`. Trust the state.**

---

## 1. `claim_reward(index=0)` forever

`rewards.items[]` is rebuilt and **re-indexed from 0 after every claim**.
Claiming index 0 repeatedly reclaims the same slot forever.

Observed: 65 consecutive rejected `claim_reward(index=0)` calls.

**Fix:** claim right-to-left, `items[-1]`.

## 2. A potion reward with full slots silently vanishes

When all potion slots are full, `claim_reward` on a potion reward returns
`ok` and is **silently dropped**. The item never leaves `items[]`, so the loop
never terminates.

Observed:

```
potions: 3 of max_potion_slots 3
claim_reward(1) -> ok | Claiming reward: potion (Energy Potion)
   items now: [gold, potion]        <- unchanged
claim_reward(0) -> ok | Claiming reward: gold (19)
   items now: [potion]              <- re-indexed
```

**Fix:** if the last item is a potion and `len(potions) >= max_potion_slots`,
discard the weakest potion first, then claim. Weakness comes from a small
`POTION_VALUE` table in `brain.py`.

## 3. `select_card` twice on one index freezes the screen

On grid screens `select_card` **toggles**. The second call deselects, so the
state never changes and the loop stalls.

Observed: `select_card(index=0)` twice, then 10 unchanged reads.

**Fix:** when `preview_showing` and `can_confirm` are true, the action is
`confirm_selection`, never another `select_card`.

## 4. A stale preview makes `confirm_selection` a no-op

`confirm_selection` returns `ok` while changing nothing, when the preview was
left over from a desynchronised state.

Observed:

```
confirm_selection -> ok  "Confirming selection from preview"
state_type before/after: card_select -> card_select
sig changed: False
```

**Fix:** if the identical `card_select` state is seen twice in a row, the
confirm did not take effect. Send `cancel_selection` to reset, then re-select.
Verified working:

```
cancel_selection -> preview False
select_card(0)   -> preview True
confirm_selection-> state_type becomes 'event'
```

## 5. `rest_site` options use `name`, not `title`

Reading `options[].title` yields `None` for every option, so the handler
concluded there were no options and fell through to a rejected `proceed`.

**Fix:** read `name` (plus `id`), and honour `is_enabled`.

## 6. `treasure` claims a relic before the chest opens

The chest auto-opens. During opening the response has no `relics` key and no
`can_proceed`, so `claim_treasure_relic` is rejected repeatedly.

Observed: 4 consecutive rejections.

**Fix:** wait while `relics` is absent; claim only when present.

## 7. Acting during transitions

The loop re-decided faster than the game animated:

- `choose_map_node` fired **3 times in a row** during a single travel.
- `end_turn` fired repeatedly during the enemy turn.

**Fix, two parts:**

- `__wait__` when `battle.is_play_phase` is false.
- An **unchanged-state guard** in `run.py`: if the state signature repeats and
  the **last action succeeded**, wait. If the last action was **rejected**,
  re-decide instead, so a different action can be tried.

The second condition matters: waiting on a rejected action would prevent
recovery and stall until the stuck counter fires.

## 8. `unknown` and `overlay` treated as dead ends

Both are transitions, not terminal states. Returning "stop" ended the session
while the game was mid-transition into combat.

**Fix:** wait and re-observe. The unchanged-state guard bounds this, so a
genuine dead end still stops after 10 reads.

## 9. The confidence gate rejected a correct answer

Not a loop, but a wrong decision, and worth recording because the bug was in
**our** gate rather than in the model.

Measured: 5 cards offered. Jev picked `Bash` at **0.61** probability with
`Defend` at 0.29, and reported `confidence 0.50`. A fixed 0.55 floor rejected
it and fell back to a heuristic that chose to block instead.

`confidence` measures peakedness, so it falls as the option count rises:
`(5 × 0.61 − 1) / 4 = 0.50`.

**Fix:** gate a `Choice` on margin over the runner-up
(`top >= 0.45 and top - runner >= 0.20`), which is scale-free. See
[02](02-system-one-jev.md).

---

## Checklist for a new `state_type`

1. Capture the real shape. Do not code from documentation.
2. Check which fields are **absent** in some states.
3. Ask: what does this action return when it is a no-op?
4. Prefer `__wait__` over stopping for anything that looks transitional.
5. Add a guard so a repeated identical state does not loop forever.

## Checklist for a new action

1. Does it report `ok` on a no-op? (Most do.)
2. Does it re-index the collection it acts on?
3. Is there a precondition the state exposes (`can_confirm`, `can_proceed`,
   `is_stocked`, `can_afford`) that should be checked first?
4. Does it toggle?

---

# Session 2 additions

## 10. An event option ended the run, and the gate allowed it

Measured on a deciphering event:

```
[102] jev chose Keep Deciphering    conf=0.28
[104] jev chose Lose Everything     conf=0.49
```

"Lose Everything" **set the player's max HP to 1**. The margin gate passed it,
because the gate only measured how *decisive* the answer was, never how
*consequential* the action was.

**Fix, in two parts:**

1. A stricter gate for events: `top >= 0.60 AND margin >= 0.30`.
2. A deterministic safety net. When the model is not confident, choose the
   option with the lowest `event_safety_rank`, which counts risk words
   ("everything", "keep", "continue", "gamble", "lose") minus stop words
   ("stop", "leave", "refuse", "decline", "take what").

**A "does this risk losing the run?" Noul was tried and REMOVED.** Measured on
the same event:

| Option | Risk Noul |
|---|---|
| "Lose Everything" | **0.46** |
| "Keep Deciphering" | **0.52** |
| "Stop" | 0.38 |

It ranked the run-ending option as *less* risky than a moderate one. A
misleading signal is worse than no signal, so danger is detected by keywords
instead. Keep the model for the confident case; use code for the dangerous one.

## 11. `hp=1/1` was reported as "healthy"

An effect reduced max HP to 1. `_hp_bucket` bucketed by percentage alone, so
`1/1` was 100% and returned `healthy`. The bot walked into a normal fight at
1 HP and died.

**Fix:** absolute HP is now part of the bucket. `hp <= 5` is always `critical`.

```python
def _hp_bucket(pct, hp=None):
    if hp is not None and hp <= 5:
        return HP_CRITICAL
    ...
```

## 12. `hand_select` blindly selected index 0

`combat_select_card(index=0)` failed with:

```
Card index 0 out of range (0 selectable cards)
```

`hand_select` has `cards` (still selectable) and `selected_cards` (already
chosen). When `cards` is empty the only useful action is
`combat_confirm_selection`.

**Fix:** confirm when nothing remains selectable; otherwise give up basic
Strikes first, then Defends.

## 13. `bundle_select` has the same preview trap as `card_select`

```
A bundle preview is already open - confirm or cancel it first
```

Same shape (`preview_showing`, `can_confirm`) and same fix: confirm when a
preview is showing, and reset with `cancel_bundle_selection` if the identical
state repeats.

## 14. Error text lives in `error`, not `message`

`ActionResult` read only `message`, so every rejection printed as
`action rejected:` with nothing after it. This hid four separate bugs for a
whole session.

**Fix:** `message = data.get("message") or data.get("error") or ""`.

**Lesson:** make failures loud before chasing them. A blank error message is
worse than no error handling.

## 15. `can_proceed` is not reliable

For shops, `shop.can_proceed` was `false` while `proceed()` worked and moved the
game to the map. Waiting on that flag stalls forever.

**Fix:** do not gate an exit on `can_proceed`. Attempt `proceed` and let the
rejection counter bound the retries.

## 16. The unchanged-state guard was count-based

A boss death animation plus the rewards transition exceeded 10 reads, so the
guard declared STUCK while the game was still animating.

**Fix:** the guard is now time-based (`--stuck-seconds`, default 25 s).

## 17. Transient rejections are normal

`proceed` at a rest site right after a heal is rejected for a moment and then
succeeds. The retry loop was too impatient.

**Fix:** on rejection, back off 3x the normal pause, and allow 6 attempts.

---

# Session 3 — the programmatic audit

Prompted by observing that the bot "was only upgrading common attack cards".
These are usage bugs, not decision-quality issues, and are listed separately
from the tuning items.

## 18. `card_select` fallback was hardcoded to `cards[0]`

The upgrade screen fell back to the first card in the list, and the list is
ordered with basic Strikes first. So every fallback upgraded a Strike.

Measured across runs:

```
[156] [fallback] select_card(index=0)  # select the first card
[263] [fallback] select_card(index=0)  # low confidence; select the first
[378] [fallback] select_card(index=0)  # low confidence; select the first
```

The fallback fired often because a single `Choice` over a 13+ card deck
dilutes, exactly like the shop.

**Fix, two parts:**

1. Re-ranking: one absolute `Noul` per card, argmax in code.
2. Screen-aware deterministic fallback (`upgrade_rank`, `removal_rank`) that
   never picks index 0 blindly:
   - upgrade prefers a non-basic card, then Bash, then Strikes, then Defends,
     and never an already-upgraded card
   - remove/transform invert the order: shed basic Strikes and Defends first,
     and never target an upgraded card

Verified live after the fix:

```
[197] jev chose Perfected Strike (noul=0.67)
[205] jev chose Bludgeon        (noul=0.66)
[212] jev chose Rampage         (noul=0.67)
[303] jev chose Bash            (noul=0.62)
```

## 19. `hand_select` fed good cards to a "choose any number" prompt

The give-up ranking returned `2` for anything that was not a Strike or Defend,
so once the basics ran out it started offering real cards:

```
[023] give up Uppercut
[086] give up Stomp
[171] give up Bash        <- the deck's only Vulnerable source
```

**Fix:** only basic Strikes and Defends are candidates. When none remain,
confirm and keep the good cards. A selection is forced only when
`can_confirm` is false.

## 20. `card_reward` had a redundant gate that skipped almost everything

A `want_any` Noul ("does this deck want any of these?") gated the whole
decision. When it was merely *uncertain* (0.54–0.59) the bot skipped, so it
skipped nearly every card reward and ran a 10-card deck.

**Fix:** the per-card Nouls ARE the signal. Skip only when no card clears
`CARD_PICK_THRESHOLD`.

## 21. A skipped card reward is NOT consumed — infinite loop

```
[409] skip_card_reward()          # no card cleared 0.6
[410] claim_reward(index=2)
[411] skip_card_reward()
[412] claim_reward(index=2)       ... forever
```

Skipping returns to the rewards screen with the card **still listed**. Claiming
it again reopens the card screen, and the cycle repeats.

Verified directly: `skip_card_reward` -> `ok`, then the rewards list still
contains `[2] card: Add a card to your deck.`

**Fix:** record that a card reward was skipped, and ignore card rewards on the
rewards screen afterwards. `rewards` and `card_reward` are declared one
**screen group** so the flag survives the hop between them — otherwise it is
cleared on every transition and the loop returns.

## 22. Module-level guards leaked across screens

Found by the new `test_brain.py`. A fresh fake-merchant shop was reported as
"unchanged after a purchase" because a shop signature from an earlier screen
was still set.

**Fix:** `_reset_screen_guards()` clears all per-screen state whenever the
screen group changes. Also made the shop guard precise: it now only fires when
we actually purchased from that exact shop state.

## 23. `fake_merchant` nests its inventory one level deeper

The shop is at `fake_merchant.shop.items`, not `shop.items`. Reading only
`obs["shop"]` made every fake-merchant shop look empty, so the bot left
immediately without buying.

**Fix:** resolve `obs["shop"]`, else `obs["fake_merchant"]["shop"]`, else
`obs["fake_merchant"]`.

## 24. `embark` is rejected without a character selected

```
action rejected: Embark button not available — select a character first
```

An earlier version assumed `embark` defaults to the first unlocked character.
It only worked once because a character happened to be selected already.

**Fix:** read the game's own `message`:
- `"Select a character."` -> select one
- `"Selected The Ironclad. Use 'confirm' to embark."` -> embark

## 25. `relic_select` used a diluted Choice and ignored `can_skip`

Verified shape: `relic_select.relics[]` with `index/id/name/description/rarity`
plus `can_skip`. It now uses the same re-ranking pattern, honours `can_skip`,
and falls back to the rarest relic rather than index 0.

## New: `test_brain.py`

A structural regression suite, deliberately separate from decision quality.
It asserts:

1. **Every `state_type` produces an action that is LEGAL for that state.**
   Compares against `sts2.LEGAL_ACTIONS`. This is what would have caught a
   handler emitting `end_turn` during someone else's turn.
2. **Every action the decision layer can emit is declared somewhere**, so a
   typo cannot silently produce an invalid action.
3. **Every fallback respects its inputs** — removal does not target an
   upgraded card, upgrade does not target a basic one, card reward takes the
   rarest rather than index 0, `hand_select` gives up a Strike rather than
   Bash.

54 assertions, runs offline with `client=None`, no model calls, no game.
Run it before every session.

---

# Session 4 — the toggle and transition traps

Found by running the bot for long stretches. Every one of these was a stall or a
crash, not a decision-quality issue.

## 26. `combat_select_card` TOGGLES — re-selecting deselects

The same trap as `card_select`, in a different action. Measured: **18
consecutive** `combat_select_card(card_index=0) # give up Defend` with the state
never changing, because selecting an already-selected card deselects it.

`hand_select.selected_cards` is the authoritative list of what is already
chosen. Exclude those from the candidate list.

**Watch out:** `cards[].index` and `selected_cards[].index` are **different
index spaces** (different arrays, independent counters). Names are the only
reliable way to match between them.

## 27. `hand_select` has a mode that only needs confirming

Measured state:

```
mode         = "upgrade_select"
prompt       = "Confirm Card to Upgrade"
cards        = [(0, "Defend")]
selected     = []
can_confirm  = true
```

Sending `combat_select_card` is a silent no-op here. The card is already picked;
`combat_confirm_selection` closes the screen and moves to `monster`.

**Fix:** mode-aware.

| mode | action |
|---|---|
| `upgrade_select` | `combat_confirm_selection` |
| `simple_select` | select cards, then confirm when nothing selectable remains |

## 28. Character select: no indicator, and flaky embark

Two problems at once:

1. **The state carries no "selected" indicator.** The mod hardcodes
   `result["message"] = "Select a character."` whatever is chosen. Verified in
   `AddCharacterSelectMenuState`. So the message cannot be used to tell whether
   a character is picked.
2. **Embarking right after selecting is flaky.** Measured three consecutive
   `Embark button not available - select a character first` rejections, because
   the selection had not registered yet.

**Fix:** ALTERNATE — select, embark, select, embark. A rejected embark is always
followed by a fresh select, so the sequence is self-correcting regardless of
timing.

An earlier attempt used signature comparison ("if the screen is unchanged, a
character was already selected"). It failed because the signature keeps changing
during the embark transition, restarting the cycle.

## 29. The run loop's guard blocked the brain's own recovery

`run.py` waited whenever the state was unchanged after a successful action, and
it did so **before calling `brain.decide`**. That made it impossible for any
handler to notice a repeated state and react differently — which is exactly how
`character_select` and multi-line dialogue work.

**Fix:** move the check to AFTER deciding, and suppress only a **repeated
action**, never acting in general. Proposing a *different* action is always
allowed.

## 30. A bounded retry is required

Even the corrected guard stalled on Ancient dialogue: one click was issued, the
state had not updated yet, and the guard then refused to retry forever.

**Fix:** `--max-duplicate-waits` (default 3). After that many suppressions on an
unchanged state, re-execute the action. Some actions legitimately need repeating
and some transitions are just slow.

## 31. `unknown` and `overlay` are transitions, not screens

Resetting the per-screen guards on them wiped state mid-flow. During embark the
state flickers through `unknown`, which cleared the character-select guard and
restarted the select/embark cycle.

**Fix:** `_reset_screen_guards` returns early for `unknown` and `overlay`.

## 32. `menu_screen` must be part of the screen key

`main`, `singleplayer`, `character_select` and `tutorial_prompt` all share
`state_type == "menu"` but have completely different valid actions. Keying only
on `state_type` meant moving from the main menu to character select did not
reset anything.

**Fix:** `_screen_group()` returns `menu:<menu_screen>` for the menu state.

## 33. `RemoteDisconnected` crashed a run

```
http.client.RemoteDisconnected: Remote end closed connection without response
```

`RemoteDisconnected` is an `http.client.HTTPException`, **not** a `URLError`, so
it escaped `jev.py`'s transport handler and killed the process mid-fight.

**Fix, both layers:**

- `jev.py` also catches `http.client.HTTPException` and `OSError`, and retries.
- `run.py` wraps `brain.decide` in a broad `except Exception`, falls back to the
  deterministic handlers, and keeps playing.

## 34. Known, self-correcting: stale-state rejections

Occasionally the game rejects a card we chose:

```
Card 'Infection' cannot be played: HasUnplayableKeyword
Card 'Evil Eye' cannot be played: EnergyCostTooHigh
```

`can_play` comes from the game's own `card.CanPlay()`, so the state was correct
when read. A Jev call takes ~0.75 s, so the state we decided on can be about a
second stale by the time the action lands.

This is a **race, not a logic bug**, and it self-corrects: the rejection sets
`last_ok = False`, so the loop re-decides immediately against a fresh state
instead of waiting. Cost is one wasted action. No revalidation pass is needed.

## Test coverage after this session

`test_brain.py`: **78 assertions**. New this session:

- indices must come from the data, never from list position (hostile-index
  cases where `index` deliberately differs from array position)
- `hand_select` skips already-chosen cards, including duplicate names
- `hand_select` confirms for `upgrade_select`
- character select alternates select/embark and resets correctly
- a `StubClient` makes the model paths testable offline and deterministically

---

## 35. Combat had no defense policy at all (the biggest one)

Found by mining the 37 run files, not by reading the code.

**Symptom.** 81% of runs (30/37) died in Act 1. `THE_KIN_BOSS` alone killed 9.
Every one of those fights ran **5-10 turns** and cost **44-80 HP**, i.e. ~10-13 a
turn, with block cards in hand the entire time.

```
damage taken: [44, 50, 50, 53, 63, 64, 70, 75, 80]
turns:        [6, 9, 10, 6, 6, 10, 9, 7, 5]
```

**Cause A.** `facts.py` classes a hit of `<= 15% of max HP` as `THREAT_CHIP`.
At 80 max HP that is **12**. `_fallback_combat` then required

```python
must_respect = threat in (HEAVY, SEVERE, LETHAL) or hp_bucket in (WOUNDED, CRITICAL)
```

so at 74/80 HP (HEALTHY) a 12-damage hit was **ignored** and the bot attacked.
The boss's main attack sits exactly on that boundary. Measured:

```
  hp  incoming   threat  hp_bucket   old play
  74        12     chip    healthy   Bash (dmg)
  60        12     chip    healthy   Bash (dmg)
  45        12     chip    wounded   Defend     <- only now, 29 HP already gone
```

**Cause B.** `_jev_combat` asked a `should_defend` Noul on **every combat turn
and never read it**. `grep -rn should_defend *.py` returned exactly one line —
the one that created it. Pure latency cost.

**Cause C.** With that question dead, defense fell entirely to
`choice("Which single play best advances winning this fight?")` — a phrasing
biased to damage, in the `Choice` shape already measured as diluting with
option count. Measured against the real Kin state, Jev answered **Bash at 0.42
confidence**. 0.42 is below the 0.45 gate, so it fell through to the fallback,
which also chose damage. **Both paths agreed on the wrong answer.**

**Fix.** Blocking is arithmetic, so it is decided in code before Jev is asked:

- `facts.CombatFacts.turns_to_kill` — remaining fight length from this turn's
  reachable damage.
- `facts.CombatFacts.projected_incoming` — `turns_to_kill * incoming_damage`.
- `facts.CombatFacts.affordable_loss` — `hp - 30% of max_hp`.
- `facts.CombatFacts.must_block` — `projected_incoming > affordable_loss`.
- `combat_decision` now forces a block when `block_urgent` and a blocker is in
  hand, **after** the lethal check, **before** Jev.
- `should_defend` deleted.

**Verification.** Replayed all 600 real combat captures. The rule changes
**8 of 68** in-play turns (11.8%), and stays silent on short fights and when
nothing is incoming:

```
live_162_combat.json    80/80  12 in   6 turns  proj 72 > afford 56  -> Defend
live_238_combat.json    78/80  17 in  20 turns  proj 340 > afford 54 -> Rage
trivial (15 HP enemy)                                           -> still attacks
```

Tests: 50 in `test_facts.py`, 131 in `test_brain.py` (the two lethal-search
regressions in [09](09-typesafe-best-practice.md) §5 are included).

**Lesson.** The bug was invisible from the code and obvious from the data. Nine
runs died the same way, and the run files recorded the damage, the turn count
and the potions spent. Mine the history before theorising about the meta.

## 36. `relic_select` asked one question id and read another

`relic_select_decision` builds its ranking questions keyed `good_relicN`:

```python
keys = [f"relic{r.get('index', 0)}" for r in relics]
questions = {f"good_{k}": noul(...) for k in keys}
```

but then read the answers under the unprefixed ids:

```python
best_key, best_noul = best_by_noul(response, keys, CARD_PICK_THRESHOLD)
```

`best_by_noul` looks each key up with `response.get(key)`, so every lookup
returned `None`, `ranked` was empty, and the function returned `(None, 0.0)`
**for every state**. The relic path could therefore only ever take the rarest
relic — Jev's answer was silently discarded on every boss and elite relic
offer. The same shape is correct in `card_reward_decision` and
`card_select_decision`, which keep the `good_` prefix end to end; relic_select
was the one that did not.

Nothing in the trace caught it, because no session in `decisions.jsonl` ever
reached a `relic_select` state. Found by auditing every `best_by_noul` call
site against the ids it asked for, not by a run.

Fix: rank `[f"good_{k}" for k in keys]` and map the winner back with the same
prefix. Regression test in `test_brain.py`: the highest-rated relic is
deliberately the **Common** one, so the rarity fallback cannot produce the
expected answer by accident — the test fails before the fix and passes after.

**Lesson.** A question id that does not match its answer id fails silently and
looks exactly like "the model had nothing to say". Every `best_by_noul` call
site must be checked against the ids it actually asked for.