diff --git a/plan/15-pokemon-demo-handover.md b/plan/15-pokemon-demo-handover.md new file mode 100644 index 0000000..7ab8344 --- /dev/null +++ b/plan/15-pokemon-demo-handover.md @@ -0,0 +1,117 @@ +# Pokémon League Championship demo: playtest handover + +Read this before you resume work on the demo scenario. It covers the current +state of `05-league-championship.json`, what a live 10-turn playtest on +production confirmed, and two bugs the playtest found. + +**Last updated: 2026-08-28.** + +> **Superseded in part on 2026-08-28.** Both bugs below were investigated against the +> real data and **both root causes named here are wrong**. See +> `plan/16-world-state-refusals.md` for what was actually happening and what was +> changed. The playtest record and the "Confirmed working" section still stand. + +--- + +## Where things stand + +The scenario is live at `https://ai-dnd-1gmp.onrender.com` as +**"[Demo] League Championship: Round One"**, seeded from +`backend/app/seed_data/05-league-championship.json`. It replaced an earlier, +weaker draft titled "Road to the Champion" — that old scenario and its stale +test adventure (adventure id 42) are still in the production database. Delete +them by hand from `/scenarios` and `/adventures` when convenient; a delete +click froze the browser tab during this session behind what looked like a +native confirm dialog, so budget time for that if you try again. + +The schema nests all five of the player's Pokémon and Milo's Pokémon under +`npcs`, not flattened into `player._hp` fields. Each npc entry carries +its own `stats` map (`hp`, `status`, and for Milo, `active_pokemon`, +`active_hp`, `active_status`, `pokemon_left`). `player.active_pokemon` is a +text stat that names whichever of the player's Pokémon is currently out. This +design survived a real playtest: see "Confirmed working" below. + +Settings on the test account now point at the user's own OpenRouter key, +endpoint `https://openrouter.ai/api/v1`, model `deepseek/deepseek-v4-flash-0731`, +reasoning budget `-1`. The shared demo key's model +(`google/gemma-4-26b-a4b-it:free`) was hitting persistent 429s from OpenRouter +capacity, not from the app's own rate limiter — switch back to it only after +confirming that model isn't still rate-limited. + +## Confirmed working: a live 10-turn playtest + +Adventure id 43, played turn by turn against DeepSeek V4 Flash on production. +Milo's Graveler and Onix both fainted; Kabutops came out third. Across ten +turns: + +- HP tracked correctly on both sides, including sandstorm chip damage each + turn once `sandstorm_active` flipped on. +- `status` stayed correctly independent per Pokémon (all `none` throughout + this run — no status move was tried). +- `player.active_pokemon` and `npc.milo.active_pokemon` both switched + correctly as Pokémon were sent out or fainted (Pidgeotto → Wartortle → + Machoke; Graveler → Onix → Kabutops). +- `player.potions` decremented correctly on use (3 → 2) and the healed + Pokémon's HP rose by the expected amount. +- The World State sidebar reflected every one of these changes live, without + a manual refresh, after the model's reply finished streaming. + +This confirms the nested-`npcs` redesign from earlier in the session was the +right fix for "why did you flatten then" — parallel entities with independent +stats work as npcs, not as flattened player fields. + +## Two bugs the playtest found + +**1. The model sometimes skips the trailing `state` block entirely.** + +> **Unconfirmed.** Truncation at `max_output_tokens` removes the block too, and it +> was never ruled out here. See `plan/16`. +On the very first turn of this run, DeepSeek V4 Flash narrated a Wartortle HP +drop but never appended the ` ```state ` block the engine parses. The engine +correctly left the state untouched — this is model non-compliance, not an +engine bug — but the drop was silent: no error, no visible sign in the UI +beyond "the numbers didn't move." A `Retry` on that same turn produced the +delta correctly. Confirmed by reading the raw action row over +`/api/adventures/{id}/actions` — `hasState` came back `false` on the first +attempt and `true` on the retry. If this happens often during your own play, +it is worth an authors'-note reminder or a stronger trailing-instruction +nudge in `engine.py`'s prompt scaffolding, not a schema change. + +**2. The model reliably forgets `npc.milo.pokemon_left` and milestones on a +faint, despite an explicit instruction to update both.** + +> **Wrong on both halves.** The model emitted `pokemon_left` at *both* faints; the +> engine clamped it to nothing and reported it as applied. And it never emitted a +> milestone because the milestone ids were absent from the prompt entirely. Neither +> was an attention problem. See `plan/16`. `ai_instructions` in +the scenario file already says: *"decrement `npc.milo.pokemon_left` when one +of his faints"* and *"Mark milestones as they happen."* Across two separate +faints in this session (Graveler, then Onix), the model correctly reset +`active_pokemon`/`active_hp`/`active_status` for the incoming Pokémon every +time, but never once touched `pokemon_left` (stuck at `3/3` through both +faints) and never checked off "Knock out Milo's lead Graveler," even though +that milestone was unambiguously satisfied. This looks like the faint +instruction is buried inside a longer bulleted list the model is only +partially attending to. Worth trying: pull the faint-handling instructions +into their own short paragraph, or add a stat-guide line for `pokemon_left` +and the milestones that makes them as visually prominent as `hp`/`status`. + +**Separately, not necessarily a bug:** `world.turn` (a `counter` stat defined +in the schema) stayed at `0` for all ten turns. `ai_instructions` never tells +the model to increment it — the instructions cover HP, status, potions, +active_pokemon, and pokemon_left, but not turn. If you want the counter to +mean something, add an explicit line telling the model to bump +`world.turn` by 1 every reply. + +## Suggested next steps + +1. Decide whether to patch `ai_instructions` for the two gaps above, then + redeploy and play a few more turns to confirm faints correctly decrement + `pokemon_left` and flip milestones. +2. Clean up the stale "Road to the Champion" scenario and adventure 42. +3. Try a status-condition move (Ivysaur's Poison Powder or similar) — this + playtest never exercised the `status` stat changing away from `none`, so + it is unverified in practice even though the schema supports it. +4. If DeepSeek keeps skipping state blocks more than rarely, consider the + trailing-reminder wording in `engine.py` (`build_state_reminder` or + equivalent) rather than switching models — the schema itself is sound. diff --git a/plan/16-world-state-refusals.md b/plan/16-world-state-refusals.md new file mode 100644 index 0000000..b3b9ee8 --- /dev/null +++ b/plan/16-world-state-refusals.md @@ -0,0 +1,164 @@ +# World-state refusals: what the engine throws away, and who gets told + +Read this before you play the Pokémon demo again. It records why two bugs in +`plan/15-pokemon-demo-handover.md` were diagnosed wrongly, what the engine was +actually doing, and what changed. Everything here is merged and green. **None of +it has been driven in a browser.** + +**Last updated: 2026-08-28.** + +--- + +## The one sentence version + +`apply_delta` records three outcomes for every change the model sends — +`applied`, `clamped`, `rejected` — and everything downstream read only +`applied`. A refused change therefore reached the player as an ordinary chip and +reached the model, on the next turn, as a change that had succeeded. + +## What was actually wrong + +`plan/15` recorded two bugs and named a cause for each. Both causes were wrong, +and the investigation is worth keeping because the same reasoning trap is easy +to repeat: **the visible evidence was "the number did not move", and the natural +reading of that is that the model never tried.** + +### `pokemon_left` was emitted every time + +Adventure 43's action rows carry `milo pokemon_left` in `world_changes` at both +faints, each with `"delta": 0, "value": 3`. The model saw the faint and wrote +the path. It was not forgetting anything. + +`old == new == 3` is reachable only from a **positive** value. `pokemon_left` +was `min 0, max 3, initial 3, max_delta_per_turn 1`, so `+2` capped to `+1`, +reached 4, and clamped back to the ceiling of 3. Net zero. + +So the model sent the remaining count as an absolute — "two left" — instead of +a delta of `-1`. Milo's four stats alternate between the two conventions: + +| stat | convention | +|---|---| +| `active_pokemon` | text, absolute | +| `active_hp` | number, delta | +| `active_status` | text, absolute | +| `pokemon_left` | number, delta | + +HP survives because damage is naturally phrased as a change. A count is +naturally phrased as a state, so it got text semantics. + +**The general rule this produces:** a numeric stat whose `initial` equals the +boundary it moves away from turns every wrong-signed change into a silent +no-op. Every `hp` in the scenario has that shape (`initial == max`). It has +never fired only because damage is phrased as a decrease by luck of language. + +### Milestones were never emitted at all + +Zero milestone changes across nine AI turns. `EMIT_RULE` asks for +`"milestones.": true` and `apply_delta` matches `` against the schema +key, but `render_state_section` printed only the description, and +`render_reference` skipped the milestones section entirely +(`STAT_SECTIONS = ("world", "player")`). The string `graveler_defeated` was +nowhere in the prompt. + +The same playtest is its own control: `sandstorm_active` is a flag, flags +*are* printed by name, and it worked. + +### The replay was teaching the model to repeat itself + +Found while deciding whether to feed refusals forward, and the most damaging of +the three. `_history_text` re-attached each past turn's state block from +`world_delta["delta"]` — **what the model sent**, not what was applied. So the +turn after the faint contained the model's own block claiming +`"npc.milo.pokemon_left": 2`, directly above a live values line reading +`pokemon_left 3/3`, with nothing to say which was true. + +That is a per-turn lesson that sending `2` is correct. The identical mistake at +the second faint is what that lesson predicts. + +## What changed + +Five changes, on `fix-silent-clamps-and-milestone-ids`. + +1. **`Action.world_changes` reports refusals** (`models.py`). Reads `clamped` + and `rejected` beside `applied`. Accepted stats carry `clamped`; refusals + become `kind: "rejected"` entries. The `fix` key is present only when the + engine wrote one — it is empty for every accepted change, and this property + runs for every action of every list response. +2. **The UI distinguishes three outcomes** (`Play.jsx`, `index.css`). Clamped to + a standstill reads `no change — at its limit` on a dashed chip; a partial + clamp is marked `(limited)`; a rejection carries its reason. Dashed and + dimmed rather than red: the rules refusing a change is them working. +3. **Milestones are named to the model** (`engine.py`). The goals line is now + `Goals (mark with milestones.): graveler_defeated — Knock out Milo's lead + Graveler; …`, the same treatment NPCs get with `(npc.milo)`. +4. **Refusals carry a generated correction** (`engine.py`). Each rejection, and + each clamp that moved nothing, builds a `fix` string from the stat definition + at the point of refusal, so it quotes the real limits and lists the real + names. `render_refusals()` renders them into the prompt directly above + `EMIT_REMINDER`, for the previous AI turn only. +5. **History replays what was accepted** (`builder.py`). `applied_delta()` + rebuilds the block from `report["applied"]`, dropping any numeric entry where + `new == old` so a change that moved nothing cannot be copied as a zero. + +Plus the demo scenario: `pokemon_left` became `pokemon_fainted` +(`type: counter`, `initial: 0`), which puts a wrong sign on the counter rule +where it is rejected out loud instead of absorbed. The faint instruction moved +into its own paragraph, and a `world.turn` line was added — it sat at 0 for the +whole playtest because nothing ever told the model to move it. + +### The design call worth not re-litigating + +**A clamp that reduced a change but still moved the value says nothing.** Only +total losses are reported. If you tell a model its 80 damage became 30, it can +treat the shortfall as a debt and send the remaining 50 next turn — which is the +swing `max_delta_per_turn` exists to prevent. A rejection has no partial credit +to chase. `test_a_clamp_that_still_moved_the_value_says_nothing` pins this. + +## How to test it + +530 backend tests pass and the frontend builds. **The whole of the UI work is +unverified** — this project has no frontend test runner, which is the standing +reason its UI bugs are found by hand. + +Run the backend from `backend/` with +`.venv/Scripts/python.exe -m pytest tests/`. The new file is +`tests/test_change_visibility.py` (21 tests). Each of the three mechanisms fails +its own test when disabled; that was checked by sabotage, not assumed. + +To drive it, re-seed the scenario and play the demo: + +1. **A refusal chip.** Open the World State drawer, use its ✎ edit mode to put + Milo's `active_hp` at full, then play a turn where he takes no damage but the + model tries to heal him. Easier and more reliable: send a deliberately wrong + block by editing an AI turn. What you are looking for is a dashed chip + reading `no change — at its limit`, not a `+0`. +2. **The milestone.** Knock out Graveler. `graveler_defeated` should tick, and + a `✓ graveler defeated` chip should appear. This is the single clearest + pass/fail in the whole change — it never once happened before. +3. **The faint counter.** At the same faint, `pokemon_fainted` should go 0 → 1. + If the model sends an absolute again, it is now refused rather than absorbed, + and the refusal note should appear in the *next* turn's prompt. Read it under + Insights → the turn's context snapshot, section `world_state_refusals`. +4. **The replay.** In the same snapshot, check the replayed history: a past + turn's `state` block should carry only the changes that were accepted. +5. **`world.turn`** should now advance by 1 per reply. + +Check the narrow layout too. The chips grew longer text, and `.chg` is inside +the story column with nothing to scroll sideways — `overflow-wrap: anywhere` is +doing the work, and it was not re-checked at 390 px. Chrome clamps its minimum +window width to ~500 px, so relaunch with `--window-size=` rather than trying to +resize a maximized window. + +## Still open + +- **The missing `state` block from `plan/15` bug 1 is unexplained.** Truncation + at `max_output_tokens` removes the block, which `LENGTH_HEADROOM` exists to + prevent, and it was never ruled out. The distinguishing evidence is whether + the narration ends mid-sentence with `finish_reason: length`. +- **Every `hp` stat still has the `initial == max` shape.** Now visible when it + bites, rather than silent, but not designed out. +- **The stale "Road to the Champion" scenario and adventure 42** are still on + production. Deleting them is hand-work and was deliberately not automated. +- **The Bandit Camp demo (`04-rpg-world-state.json`) was not checked** for the + same milestone problem. Its milestones were equally unnamed to the model + before this change, so it is worth asking whether one has ever fired there. diff --git a/plan/STATUS.md b/plan/STATUS.md index 940a0b8..f404cbc 100644 --- a/plan/STATUS.md +++ b/plan/STATUS.md @@ -3,7 +3,7 @@ Read this first when picking the project back up. Updated at the end of a working session; the per-phase plan files hold the detail, this holds the thread. -**Last updated: 2026-08-22.** +**Last updated: 2026-08-28.** --- @@ -78,10 +78,21 @@ needed; nothing requires reading a row of anyone's story. ## Pick up here -**`plan/14-phase-story-tree.md`, SP9 — merge it.** SP7 shipped, PR #6 merged, and the tree -went live on 2026-08-18. It was then driven by hand and **found unusable**, which is what -SP9 exists to fix. SP9 is written, green (**433 tests**) and **driven by hand** on branch -`sp7b-take-pager`; it is **not merged and not deployed**. +**`plan/14-phase-story-tree.md`, SP8 — drop the legacy columns.** SP8 was gated on the +tree being proven live, and it now is: SP9 merged, and production answers `/api/health` +with the tree schema in place. SP8 drops `index`, `variants`, `variant_index`, +`variant_count`, the two legacy cursors and the two `*_before` snapshots. Check that +nothing still reads `variant_count`/`variant_index` before dropping them, and note this +is the migration shape that rewrites toasted values, so it owes one `VACUUM FULL actions;` +on the direct (non-`-pooler`) endpoint afterwards. + +Ahead of that, `plan/16-world-state-refusals.md` records a set of world-state fixes that +are merged but **never driven in a browser**. See that file for what to test. + +**SP9 and SP10 are both on `main`, despite what earlier notes here said.** The branches +`sp7b-take-pager` and `sp10-memory-bank-eviction` still exist and still read as unmerged +to `git branch --no-merged`, because the work landed as squashes. Check the code, not the +branch list: `actions.parent_id` in `models.py` is SP9, and commit `c0cd6fa` is SP10. **Driving it found two bugs the suite could not have.** The pager did not appear until the page was reloaded — a retry's reply is the second take of its turn, and the SSE stream