From ba737de9b438582727160cdb9622b7851fc20300 Mon Sep 17 00:00:00 2001 From: JesseMarkowitz Date: Tue, 1 Sep 2026 16:11:38 -0400 Subject: [PATCH] Add Phase 0B local validation findings and recommendation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Validates the three finalists by clone, build, test run and live local Ollama inference, then answers the fork question with measurements rather than static review. Recommendation: fork AI-DnD, confidence high. The Phase 0A call holds, but it was wrong that AI-DnD's undo is non-destructive — retry preserves the replaced take, undo hard-deletes it. A follow-up spike fixed that in 3 files (+130/-31): undo now moves a head cursor, redo round-trips, writing below a moved-back head forks and keeps the abandoned line, branch-scoped memory isolation survives, suite 627/632 with all 5 failures asserting the deleted-row behaviour that was replaced. Findings that change the plan: - AI-DnD cannot take a turn air-gapped as shipped; tiktoken fetches its encoding from a CDN. Proven on an internal Docker network, proven fixed by vendoring the file. - ai-adventure needs zero code for Ollama — two config lines — and its turn/head/checkpoint schema is the target model to build to. - Open Dungeon has zero automated tests and a positional summary watermark, making its branch retrofit larger than Phase 0A costed. - The world-state referee takes relative deltas; a 3B model sent absolute values under full context, so a wounded player ended at full health. Validation cannot catch this, so prefer ai-adventure's typed-event vocabulary when generalising narrative state. - Export/import recomputes head depth, so a round-trip silently undoes an undo. Must be fixed alongside the undo work. Docs only; no production code. Working tree from the runs stays untracked under phase0b/. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_015gUPLuxLs8wypxZPEmccJu --- planning/PHASE-0B-RECOMMENDATION.md | 500 ++++++++++++++++++ .../reports/PHASE-0B-AI-ADVENTURE-OLLAMA.md | 119 +++++ .../reports/PHASE-0B-AI-DND-EXPERIMENT.md | 146 +++++ planning/reports/PHASE-0B-BASELINE.md | 39 ++ planning/reports/PHASE-0B-FOLLOWUP-CHECKS.md | 164 ++++++ planning/reports/PHASE-0B-OFFLINE-NETWORK.md | 106 ++++ .../reports/PHASE-0B-OPEN-DUNGEON-HISTORY.md | 105 ++++ planning/reports/PHASE-0B-UNDO-SPIKE.md | 214 ++++++++ 8 files changed, 1393 insertions(+) create mode 100644 planning/PHASE-0B-RECOMMENDATION.md create mode 100644 planning/reports/PHASE-0B-AI-ADVENTURE-OLLAMA.md create mode 100644 planning/reports/PHASE-0B-AI-DND-EXPERIMENT.md create mode 100644 planning/reports/PHASE-0B-BASELINE.md create mode 100644 planning/reports/PHASE-0B-FOLLOWUP-CHECKS.md create mode 100644 planning/reports/PHASE-0B-OFFLINE-NETWORK.md create mode 100644 planning/reports/PHASE-0B-OPEN-DUNGEON-HISTORY.md create mode 100644 planning/reports/PHASE-0B-UNDO-SPIKE.md diff --git a/planning/PHASE-0B-RECOMMENDATION.md b/planning/PHASE-0B-RECOMMENDATION.md new file mode 100644 index 0000000..9ab3843 --- /dev/null +++ b/planning/PHASE-0B-RECOMMENDATION.md @@ -0,0 +1,500 @@ +# Phase 0B — Local Validation Findings and Recommendation + +**Date:** 2026-09-01 +**Status:** Complete, and updated after the follow-up undo/redo spike. +Stop condition reached; no production work started. +**Method:** Clean clones, pinned SHAs, documented installs, full test runs, real +local Ollama inference, and targeted disposable experiments. + +> **Update — the recommended spike was run and it passed.** Section E previously +> recommended proving non-destructive Undo/Redo in AI-DnD before committing to +> the fork. That spike is now complete: 3 files, +130/-31 lines, undo deletes +> zero rows, redo round-trips exactly, writing below a moved-back head forks and +> preserves the abandoned line, branch-scoped memory isolation survives, and the +> suite goes 627/632 with all 5 failures asserting the deleted-row behavior that +> was deliberately replaced. Section E has been rewritten accordingly, and the +> open questions it resolved are marked in section F. Full detail: +> `reports/PHASE-0B-UNDO-SPIKE.md`. +> +> **Update 2 — four follow-up checks run.** The world-state referee was exercised +> against local models for the first time, and export/import, story-card lineage +> safety and Postgres removability were settled. One new defect found (a +> round-trip silently undoes an undo) and one design conclusion that changes what +> we should build (§H). Detail: `reports/PHASE-0B-FOLLOWUP-CHECKS.md`. + +Pinned commits (cloned 2026-09-01): + +| Repo | SHA | Last commit | +|---|---|---| +| AI-DnD | `d72f7c1bda0f34fccd84afb7a25c34eb01c901de` | 2026-08-31 | +| Open Dungeon | `b0a79f96bf852be7b4e53908dff6a7f7c179da23` | 2026-07-06 | +| ai-adventure | `873ea9180d5b611576cddb155921fc16a17ae88b` | 2026-07-21 | + +Inference used for every live test: Ollama in Docker, `qwen2.5:0.5b` and +`qwen2.5:3b-instruct` for narration, `nomic-embed-text` for embeddings. +CPU only, no GPU. + +--- + +## A. Executive Recommendation + +**Fork AI-DnD as the production base. Confidence: high.** + +**The Phase 0A recommendation held up, but for partly different reasons than it +gave, and it was wrong about one important thing.** + +What held: AI-DnD really does implement the expensive correctness work, and it +survived every entanglement test Phase 0A worried about. The RPG machinery is +optional, the scripting sandbox is removable, and the branch-scoped memory +isolation — the single hardest requirement in the specification — works +correctly against real local embeddings. + +What did not hold: **Phase 0A's claim that AI-DnD's undo is non-destructive is +false.** `POST /undo` hard-deletes rows. Retry is non-destructive; undo is not. +There is no Redo. This is a direct conflict with `DECISIONS/005-branch-preserving-history.md` +and specification §4. + +That discovery does not change the decision, because the comparison is relative: +AI-DnD needs one bounded change at a chokepoint that every read already passes +through. Open Dungeon needs the entire non-destructive history subsystem built +from nothing, under a 3,991-line component, with **zero automated tests**. + +**That "one bounded change" is no longer an estimate.** The follow-up spike +implemented it in 3 files and +130/-31 lines, and it works: undo deletes nothing, +redo restores exactly, the abandoned tail becomes an ordinary branch, and memory +isolation holds. Confidence in the fork decision moves from "high on the evidence" +to "high and demonstrated". + +The decision gate from `reports/PRELIMINARY-RECOMMENDATION.md` said to select +AI-DnD unless a blocker is confirmed. All four were tested and none is a blocker: + +| Phase 0A blocker | Result | +|---|---| +| Branching/state inseparable from RPG mechanics | **No.** A scenario-less adventure with empty world state played, branched, and retrieved memories correctly. | +| Removing hosted/scripting destabilizes many tests | **No.** 19 of 632 (3%), and every one fails for instrumentation reasons, not coupling. | +| Local-only still needs external services | **Partly — but both are packaging fixes.** `tiktoken` downloads its encoding from a Microsoft CDN; the SPA fetches Google Fonts. | +| Dependency/security burden worse than expected | **No.** 80 packages total vs Open Dungeon's 335 with 6 high-severity advisories. | + +--- + +## B. What We Learned About Each Candidate + +### AI-DnD — recommended base + +**What worked** + +- `632 passed` in 236s, first try, no fixes needed. +- Docker image builds clean; `docker compose` path is real. +- Default model endpoint is already `http://localhost:11434/v1`. Connection test + and model discovery against Ollama worked with no code change. +- Streamed a full multi-turn story over SSE against local Ollama. +- **Genre-neutrality is real.** An adventure created with `scenario_id: null` + gets an empty world state and no RPG scaffolding, and plays normally. Noir + prose, no fantasy code paths, no stats. +- **Retry is genuinely non-destructive.** Retrying kept the replaced attempt as a + sibling take at the same coordinate (rows 6 and 7 both present; both listed by + `/variants`). +- **Forking is genuinely non-destructive.** Adding an alternate take at a passed + turn created branch 2 while every row of branch 1 survived, with + `parent_branch_id` and `fork_depth` recorded. +- **Branch-scoped memory isolation is correct.** Two memories were written on + branch 1 (depths 5 and 11) from real `nomic-embed-text` embeddings. On branch 2, + retrieval returned `used: []` and none of `brass key`, `revolver`, `lockbox`, + `ashtray` or `bullets` appeared anywhere in the assembled prompt. The negative + control passed: switching back to branch 1 returned both memories with + similarity scores (0.8087) and the abandoned facts reappeared. Retrieval is + working *and* correctly scoped. +- **Prompt transparency is already there.** The context endpoint returns labeled + sections with token costs: `narrator 57, ai_instructions 9, persona 15, + plot_essentials 24, history 988, used_memories 143, length_hint 46`. + +**What failed** + +- **Undo hard-deletes.** Measured: rows 7 → 4, deleting ids 5, 6 **and 7** — that + is, both the player action, the live AI take, *and the alternate take the + earlier retry had preserved*. `delete_turn` also calls `memorybank.forget_node`. + A retry's preserved alternative does not survive a later undo. There is no Redo + endpoint. **Resolved in the follow-up spike** — see section G. +- **Fails offline on a clean install.** First turn in a network-isolated container + died with `NameResolutionError` for + `openaipublic.blob.core.windows.net/encodings/cl100k_base.tiktoken`. `tiktoken` + fetches its BPE encoding on first use. Seeding the 1.7 MB file into the image + made a full turn generate with no internet at all. Packaging fix, not + architecture — but a hard blocker until fixed, and static review missed it. +- **Runtime remote asset.** The built SPA still requests Google Fonts; the CSP + explicitly allows `fonts.googleapis.com` and `fonts.gstatic.com`. + +**Strongest reusable pieces** + +Immutable parent-linked story tree with lineage entries; alternate takes; branch +fork/switch/rename/delete; per-node world-state and script-state snapshots; +branch-scoped memories with local embeddings; the Insights prompt snapshot; +`ai-dnd-adventure-v2` whole-tree export; the 632-test suite; SSE streaming. + +**Major architectural problems** + +Destructive undo and no redo. No named checkpoints (branch names are the nearest +thing). Multi-user/hosted concerns are spread across ten modules. `netguard.py` +is the *inverse* of what we need — it blocks non-public endpoints in hosted mode +and does nothing locally; we want a warning on non-loopback endpoints. + +**Adaptation required** + +Bounded and mostly subtractive. The one genuinely additive change is +non-destructive undo/redo, now measured at 3 files and +130/-31 lines (section G). + +### Open Dungeon — not recommended as base + +**What worked** + +Installed and ran on first try. Plays a real story against Ollama through its +"custom" OpenAI-compatible provider. Clean browser UX. No runtime Google Fonts +(`next/font` self-hosts at build time). Local SQLite, local image generation, +character portrait continuity. + +**What failed** + +- **Every history operation is destructive, confirmed by measurement.** + - Edit is an in-place `UPDATE messages SET content = ?`. After editing, the + original text `brass key from the ashtray` was **gone from every column of + every table**. + - Retry and Erase both call `DELETE /messages/{id}?after=1`, which runs + `DELETE FROM messages WHERE chat_id = ? AND (created_at > ? OR ...)`. Measured + rows 5 → 4, with no archive anywhere. + - The database has exactly four tables — `chats`, `messages`, `characters`, + `app_settings`. No parent pointer, no branch, no take, no checkpoint. +- **Zero automated tests.** No `test` script, and no test framework in + `devDependencies` at all. Phase 0A listed this as "BUILD/VERIFY"; it is zero. +- **The local provider hard-codes a five-model Gemma 4 allowlist** + (`src/lib/text-models.ts`). Arbitrary local Ollama models are only reachable + through the "custom" OpenAI-compatible path. +- Next.js telemetry is on by default and printed its notice on first start. +- 335 npm packages, 6 high-severity advisories. + +**Cost of the branch retrofit — the Experiment B answer** + +Larger than Phase 0A estimated, because of a dependency Phase 0A did not identify: + +1. `messages` needs `parent_id`, `branch_id`, `depth`, and a live/take flag. +2. Of 20 exported `db.ts` functions, the four history ones need rewriting, and + `getChat` must stop returning a flat list and become a lineage query. +3. `story-prompt.ts` windows history by **array index** (`messages.slice(dropped)`, + `messages.length - 1`). All of it becomes an ancestry walk. +4. **The rolling summary watermark is positional.** `chats.story_summary_count` + means "how many of the chat's oldest messages the summary already covers", and + `route.ts` compares `evicted.length > stored.coveredCount`. On a branch, "the + first N messages" has no meaning. Summaries must be re-anchored to a turn id + and made per-lineage. **Phase 0A did not identify this.** +5. `page.tsx` is a single 3,991-line component holding retry/erase/edit as array + slicing, and there is no take stepper, branch panel, or tree view to extend. +6. All of the above lands with no test suite underneath it. + +**Worth reusing as reference:** the reading UI, inline scene images, the +character-portrait-as-reference-image continuity idea, and the scene/media +architecture. None of it ports directly — Open Dungeon is Next.js and AI-DnD is +React + FastAPI. + +### ai-adventure — not the base, but promote it to the architectural reference + +**What worked — this is the strongest result of the round** + +- `Ran 76 tests ... OK` in 1.9s. +- **The Ollama "adapter" is zero code.** `LMStudioBackend` is a generic + OpenAI-compatible client hitting `/v1/chat/completions` and `/v1/models`. I + changed two lines of `world.toml` — `base_url` and model `name` — and the + doctor reported `[PASS] LM Studio reachable: http://127.0.0.1:11434` and + `[PASS] Configured model visible: qwen2.5:3b-instruct`. A real turn then + committed to the database with `status='committed'`, zero validation errors, + and a `model_calls` row recording backend, model and attempt. **Phase 0A's + "MODIFY — LM Studio primary provider" was wrong; it is a config change.** +- **Its history model is exactly what the specification asks for.** Verified + independently, not just by trusting its tests: + - Undo deleted **0 rows**, rolled `has_brass_key` back from `False` to `True`, + and the abandoned turn was still in the database. + - Branching from the restored point produced `gate_open` on the branch and + **did not leak it into the original session**. + - Checkpoint restore rolled state back with the later turn still present. + - Total turns in the database never decreased. +- Schema is the target data model: `turns.parent_turn_id`, `status`, + append-only `state_events`, `state_cache` keyed by `head_turn_id`, + `model_calls` with request/response hashes, `named_checkpoints`, `summaries` + anchored by `through_turn_id`, and lore in FTS5. +- **The cleanest privacy posture by a wide margin.** Exactly one outbound call + site in the entire codebase (`lm_studio.py`'s `urlopen`), no hardcoded remote + host anywhere, and **one** third-party dependency (`pydantic`). It is the only + candidate that already warns when `model.base_url` is not loopback — which is + specification §12, implemented. + +**What failed** + +- With `qwen2.5:0.5b` the turn failed validation and **nothing was saved**. That + is the model-proposes/app-validates discipline working correctly, but it means + the typed-event contract needs a reasonably capable model; the freeform-prose + candidates tolerate a weak one. `qwen2.5:3b-instruct` succeeded. +- Product distance is real and large: terminal only, worlds authored as + hand-written TOML and Markdown files rather than set up in-app, no streaming, + no embeddings/semantic memory, no media, no import of user documents. + +**Adaptation required:** building essentially the entire product on top of a +correct core. + +--- + +## C. Key Technical Findings + +| Requirement | AI-DnD | Open Dungeon | ai-adventure | +|---|---|---|---| +| History/undo model | Tree + takes; **undo deletes**, no redo (both fixed in the spike, §G) | Flat list; **retry/edit/erase all destroy** | **Parent-linked, head pointer, undo deletes nothing** | +| State rollback | Per-node snapshots, verified | None | **Event replay, verified** | +| Memory isolation across branches | **Verified correct, with negative control** | N/A (no branches) | **Verified correct** | +| Local Ollama | Default endpoint; verified streaming | Verified via "custom" provider; local provider allowlists 5 Gemma builds | **Verified, zero code change** | +| Offline | **Blocked by tiktoken CDN fetch** until vendored; then fully offline | Runtime clean; build needs network; Next telemetry on by default | **Verifiably clean — 1 outbound call site, 1 dependency** | +| Browser suitability | React SPA + FastAPI, already there | Next.js, already there | **None** | +| Prompt inspection | **Per-action snapshot with labeled sections and token costs** | Not exposed | Prompt hashes; `store_prompts` off by default | +| Imported knowledge | Story cards (keyword-triggered) — closest starting point | None | **Local FTS5 lore, deterministic** | +| Named checkpoints | Absent (branch names only) | Absent | **Present** | +| Future media | Absent | **Present and working** | Absent | +| Tests | **632** | **0** | 76 | + +--- + +## D. Important Surprises + +Ordered by how much they should change the plan. + +1. **AI-DnD's undo is destructive, and it destroys retry's preserved + alternatives too.** `reports/PRELIMINARY-RECOMMENDATION.md` and + `REUSE-MATRIX.md` both credit AI-DnD with non-destructive rollback. Retry + earns that; undo does not. Measured: undo deleted the player action, the live + take, and the sibling take a prior retry had kept. The follow-up spike has + since replaced it with a head-cursor undo and added Redo (§G), so this is a + corrected planning assumption rather than an outstanding defect. +2. **AI-DnD cannot take a single turn on an air-gapped machine as shipped.** + `tiktoken` fetches `cl100k_base` from a Microsoft CDN on first use. Proven by + running it on a Docker `--internal` network, and proven fixed by seeding the + file. +3. **ai-adventure needs no Ollama adapter at all.** Two config lines. The + "LM Studio" name is a misnomer for a generic OpenAI-compatible client. +4. **Open Dungeon has zero automated tests** — not few, none, and no framework + installed. +5. **Open Dungeon's summary watermark is positional**, which adds a summary + redesign to its branch retrofit that Phase 0A did not cost. +6. **Open Dungeon's "local" provider only offers five Gemma 4 QAT builds.** A + genre-agnostic app that lets the user pick any installed Ollama model needs + that lifted. +7. **AI-DnD fetches Google Fonts at runtime** and its CSP is written to allow it, + violating specification §12's "no remote fonts". +8. **AI-DnD's `netguard` is the opposite of our requirement** — it protects a + hosted deployment from SSRF and is inert locally. ai-adventure has the + loopback warning we actually want. +9. Repo maturity differs sharply: AI-DnD is 172 commits / 3 authors, Open Dungeon + 45 / 1, ai-adventure 20 / 1. All three are young. Since we fork, this matters + less than it would for a dependency, but there is no upstream to lean on. + +--- + +## E. Recommendation for Next Step + +**Fork AI-DnD and begin the controlled strip-down.** The blocking uncertainty is +gone: the spike this section previously asked for has been run and passed +(section G). + +Recommended order, cheapest and most load-bearing first: + +1. **Close the two offline blockers.** Vendor the `tiktoken` encoding and + self-host the three font families, then re-run the air-gapped test. Until this + is done the application does not satisfy specification §2, and it is a day's + work. +2. **Strip the hosted surface.** Remove multi-user/auth, the demo key, analytics, + `render.yaml`, the Postgres path, and the quickjs sandbox. Re-instrument the + ~19 tests that use JS hooks to observe state, moving them onto the + `world_state_after` snapshots that already exist. +3. **Land the undo/redo work properly**, promoting the spike from a proof to a + feature: fix the four rough edges it left (undo floor at the opening node, + retry/`add_take` while behind the head, marking abandoned turns disposable, + and the frontend Redo control and branch labelling) — **and add `headDepth` to + the export bundle in the same change**, or a round-trip silently undoes every + undo (§H). +4. **Add named checkpoints**, using ai-adventure's + `named_checkpoints(session_id, turn_id, name)` as the model. With an explicit + head that can sit anywhere, a checkpoint is now just a named head position. +5. **Add the loopback-endpoint warning**, modeled on ai-adventure's + `_endpoint_warnings`. +6. **Then** imported knowledge, and only after that, media. + +Treat **ai-adventure as the written specification for state and history +semantics** throughout. Its schema is the target shape and its 76 tests are the +behavioral contract worth copying — the spike already reproduced its central +property (undo that moves a head instead of deleting) and benefited from having +that reference to aim at. §H extends this: when generalizing AI-DnD's world state +into genre-neutral narrative state, take ai-adventure's **typed-event vocabulary** +rather than AI-DnD's relative-delta vocabulary, because the delta protocol has a +failure mode that validation cannot catch and that worsens as context grows. + +One addition to the ordering above: whatever narrative-state engine we build must +be **exercised at realistic context length against the models users will actually +run**, not only in a clean harness. That is where the referee's weakness showed +up, and it would not have appeared in a unit test. + +Do not merge repositories. Open Dungeon remains a UX and media reference only. + +## F. Open Questions + +**Resolved by the undo/redo spike (§G):** + +- ~~Does capping `Path.clause` by head depth break the memory and summary + cursors?~~ **No.** `test_memory_settling`, `test_memory_nodes`, + `test_memory_retrieval`, `test_memory_rewrite` and `test_history_window` all + passed unchanged, and memory retrieval was verified end-to-end against real + embeddings with a control. +- ~~How should named checkpoints sit on top of takes and branches?~~ **Largely + answered.** The spike introduces a head that can sit behind the tip, so a + checkpoint becomes a named `(branch_id, depth)` position. Still needs a + decision on whether restoring one forks immediately or on first write — the + spike chose "on first write" for undo, and consistency argues for the same. + +**Resolved by the follow-up checks (§H):** + +- ~~What is the model capability floor?~~ **Characterized, not a blocker.** The + delta protocol is within a 3B model's reach in a clean prompt (3/3 correct) but + degraded to absolute values under the full application context. `0.5b` cannot + do it at all. See §H for the design consequence. +- ~~Can `psycopg`/Postgres be dropped cleanly?~~ **Yes.** Three places, no + blockers: one branch in `database.py`, two migration helpers, one import inside + analytics. +- ~~Do story cards become imported knowledge, or is a new table needed?~~ + **A new table.** `StoryCard` carries no `branch_id`/`depth`, so it is not + lineage-scoped and inherits neither branch nor undo isolation. +- ~~Does the export format survive the history rework?~~ **No — and this is now a + known defect to fix, not a question.** See §H. + +**Still open:** + +1. **When does abandoned history get cleaned up?** Nothing marks it disposable + yet, and undone turns now accumulate rather than being deleted. This is the + intended trade, but it needs a retention story. +2. **Is Open Dungeon's image path portable at all?** It is Next.js calling a local + FLUX worker over HTTP. The worker protocol may be reusable even though none of + the UI is. +3. **What is the real model recommendation for users?** The probe establishes a + floor and a failure mode, not a ranking. A proper capability matrix across the + models a user would actually run belongs in the build phase, measured at + realistic context length. +4. **Not tested in any round:** import/export round-trip *behavior* beyond the + head-depth defect found by reading, multi-hour long-story context behavior, or + any concurrency beyond the single-turn lock. + +## G. Follow-Up Spike: Non-Destructive Undo/Redo — Passed + +Run after the main round, on a disposable copy of AI-DnD `d72f7c1b`. Full detail +in `reports/PHASE-0B-UNDO-SPIKE.md`. + +**The change is three files, +130 / -31 lines**, roughly a fifth of it comments: + +| File | Change | What | +|---|---|---| +| `context/lineage.py` | +26 / -3 | Read an uncapped lineage entry as capped at `self.tip` (the head), in `clause` and `contains`; add `uncapped()`. | +| `routers/adventures/takes.py` | +74 / -27 | `undo_turn` moves `head_depth` instead of deleting; new `redo_turn`. | +| `routers/adventures/turns.py` | +30 / -1 | `fork_if_behind_head` calls the existing `tree.branch_at` when a write lands below the head. | + +It is this small because the pieces already existed: every read funnels through +`lineage.path_of()`, `adventures.head_depth` was already a stored head, and +`tree.branch_at` already created a branch that leaves a path at a depth without +touching the line it leaves. + +**Results against the success criteria:** + +| Criterion | Result | +|---|---| +| Undo deletes zero rows | **Pass.** 6 → 6 rows across three undos; separately, 11 consecutive undos on a 23-action story with 23 actions still present. Clears the "at least five, unlimited preferred" requirement. | +| Abandoned tail stays reachable as a branch | **Pass.** Undo-then-write forked branch 2 at fork depth 3; branch 1 kept all six of its rows, live and readable, and the branch switcher reaches it. | +| Redo restores | **Pass.** Three redos returned the head from -1 to 5 and the transcript to all six actions, exactly. | +| Branch-scoped memory isolation holds | **Pass, with control.** An embedded memory at depth 5 was retrieved at the tip (similarity 0.689), returned `used: []` once the head moved to depth 3, and became eligible again on redo — without being deleted. None of seven terms unique to the hidden turns appeared in the assembled prompt. | +| Suite stays green apart from delete-behavior tests | **Pass.** `627 passed, 5 failed` (0.8%). All five assert deleted rows or pruned memories. The state assertions inside those same tests still pass — `assert adv.script_state == {"gold": 0}` succeeds in both `test_state_revert` failures, and the fork-boundary guard in `test_undo_stops_at_the_fork` is untouched. | + +**Rough edges left for the real implementation** (none architectural): undo can +walk past a user-written opening to an empty transcript; retry and `add_take` +were not routed through the fork check; abandoned turns are not yet marked +disposable; and the frontend has no Redo control. + +**What this changes about the recommendation:** nothing about the choice, and the +one thing about the plan — the strip-down can start now rather than after another +investigation. + +## H. Follow-Up Checks — Referee, Export, Cards, Postgres + +Run after the spike. Detail in `reports/PHASE-0B-FOLLOWUP-CHECKS.md`. + +### The finding that changes what we build + +AI-DnD's world-state referee asks the model for **relative deltas** +(`new = old + delta`, then clamped). Playing the seeded RPG scenario with +`qwen2.5:3b-instruct`, the model sent **absolute values** — `{"player.hp": 92}` +meaning "hp is now 92". The engine read `+92`, clamped to `+35`, then to the +ceiling of 100: + +``` +player.hp 100 -> 100 "did not move. It is already at its maximum of 100" +``` + +**The player took an arrow to the shoulder and finished the turn at full health.** + +The referee did its job — nothing was corrupted, and it fed corrective text back. +But no validator can catch this, because `+92` is a legal proposal. The +application stayed authoritative while the authoritative state silently stopped +tracking the narration. That is the exact divergence specification §2 exists to +prevent. + +An isolated probe using AI-DnD's own `EMIT_RULE` and parser shows the protocol is +**not** beyond a small model — `qwen2.5:3b-instruct` and `qwen2.5:7b-instruct` +both got 3/3 correct relative deltas, and `qwen2.5:0.5b` emitted no block at all. +The difference is context load: in a short prompt the 3B model follows the rule, +and under the full application prompt it drifts to absolutes. + +**Consequence for the design:** when we generalize AI-DnD's world state into +genre-neutral narrative state, take **ai-adventure's typed-event vocabulary** +(`set_flag key=… value=…` — explicit and absolute) rather than AI-DnD's delta +vocabulary. The delta protocol has a failure mode that is invisible to validation +and worsens with context length; the event protocol cannot express the mistake. +This is the second time in this round that ai-adventure's core has turned out to +be the right specification to build to. + +### One new defect + +**Export/import silently redoes an undone story.** `bundle.py:_point_the_head` +recomputes `head_depth = max(depths)` on import, and the bundle format carries +`headBranch` but not the head depth. That was correct when undo deleted rows; with +the spike's non-destructive undo, a round-trip restores every abandoned turn. Fix +is one optional `headDepth` field plus honouring it on import, falling back to +`max(depths)` so existing `v2` files still load. It must land with the undo work, +not after it. + +### Two smaller answers + +- **Story cards are not lineage-scoped.** `StoryCard` has no `branch_id` or + `depth`, unlike `Memory`. Harmless today because cards are authored rather than + derived — but any imported-knowledge source that can be created from story + content must carry those coordinates and be read through `lineage.Path.clause`, + or it inherits neither branch nor undo isolation. Argues for a new table rather + than extending `story_cards`. +- **Postgres is cleanly removable.** One branch in `database.py`, two migration + helpers, one import inside analytics. `psycopg[binary]` then drops out. + +## Supporting Evidence + +- `reports/PHASE-0B-BASELINE.md` — installs, test runs, ports, storage, versions. +- `reports/PHASE-0B-AI-DND-EXPERIMENT.md` — branch/undo/retry measurements, + memory isolation with negative control, scripting removal. +- `reports/PHASE-0B-OPEN-DUNGEON-HISTORY.md` — destructive-history proof and + retrofit cost map. +- `reports/PHASE-0B-AI-ADVENTURE-OLLAMA.md` — zero-change Ollama run and the + fixture checks. +- `reports/PHASE-0B-OFFLINE-NETWORK.md` — offline runs and egress inventory. +- `reports/PHASE-0B-UNDO-SPIKE.md` — the non-destructive undo/redo spike: the + diff, the measurements, and the rough edges it left. +- `reports/PHASE-0B-FOLLOWUP-CHECKS.md` — the world-state referee under local + models, the export/import head defect, story-card lineage safety, Postgres. + +Working tree, scripts and databases from both rounds are under `phase0b/` +(untracked scratch, safe to delete). The spike itself is `phase0b/spike/`, a +disposable copy of the AI-DnD backend — it is a proof, not a branch to merge. diff --git a/planning/reports/PHASE-0B-AI-ADVENTURE-OLLAMA.md b/planning/reports/PHASE-0B-AI-ADVENTURE-OLLAMA.md new file mode 100644 index 0000000..c590072 --- /dev/null +++ b/planning/reports/PHASE-0B-AI-ADVENTURE-OLLAMA.md @@ -0,0 +1,119 @@ +# Phase 0B — Experiment C: ai-adventure + +## C1. Tests + +``` +python -m unittest discover -s tests → Ran 76 tests in 1.9s OK +``` + +## C2. Provider abstraction — the Ollama adapter is zero code + +`llm/backend.py` defines a `ModelBackend` Protocol with a single `generate` +method plus typed `ModelRequest`/`ModelResponse`. `llm/lm_studio.py` is not +LM-Studio-specific at all: it POSTs `/v1/chat/completions` and reads `/v1/models` +— the same OpenAI-compatible surface Ollama serves. `llm/scripted.py` is a +deterministic fake used by the tests. + +The only change needed was two lines of `world.toml`: + +```toml +base_url = "http://127.0.0.1:11434" # was 127.0.0.1:1234 +name = "qwen2.5:3b-instruct" # was "replace-with-a-local-model" +``` + +``` +python -m local_adventure doctor --world + [PASS] SQLite FTS5 available + [PASS] Database schema at version 2 + [PASS] World valid: Ember Hollow + [PASS] LM Studio reachable: http://127.0.0.1:11434 + [PASS] Configured model visible: qwen2.5:3b-instruct +``` + +A real turn then committed: + +``` +TURN: (30ad56d4…, 1, 'committed', 'I search the observatory for the brass k', + 'You rummage through the dusty shelves, your fingers brushing away + decades of cobwebs...') +MODELCALL: ('lm_studio', 'qwen2.5:3b-instruct', attempt 1, 832 bytes, errors=False) +``` + +`ModelSettings.backend` is typed `Literal["lm_studio"]`, so adding a second +backend means widening one literal and one factory call in `cli.py` — the +transport itself already works. Phase 0A's "MODIFY — LM Studio primary provider" +overstated this. + +**Caveat:** with `qwen2.5:0.5b` the same turn failed with "The model response +could not be validated; no turn was saved." That is the validation contract +working as designed — the engine refuses to commit an unvalidated turn — but the +typed-event contract needs a reasonably capable model, where the freeform-prose +candidates tolerate a weak one. + +## C3. Undo / branch / checkpoint / replay — verified independently + +Run directly against `GameService` with a temporary database, not by trusting the +project's own tests: + +``` +after 2 turns: turns=2 events=2 has_brass_key=False + +UNDO + turns 2 → 2 (DELETED 0) + state after undo: has_brass_key=True + abandoned turn id_3 still in DB: True + +BRANCH from the restored point, then play on the branch + branch state: has_brass_key=True gate_open=True + ORIGINAL session leaked 'gate_open'? False + total turns in DB: 3 (nothing destroyed) + +CHECKPOINT RESTORE ("before_gate") + restored has_brass_key=True + later turn id_3 still present: True + turns in DB after restore: 3 +``` + +This is the specification's history model, working. `undo` sets +`session.head_turn_id` to the parent; `branch` inserts a new session sharing the +same head; `restore_checkpoint` replays ancestry; `_replay_to` walks +`turns.ancestry(head)` and reduces `state_events`. + +## C4. Schema — the target data model + +``` +sessions(session_id, ..., head_turn_id, initial_state_json) +turns(turn_id, session_id, parent_turn_id, turn_number, + player_input, narration, status CHECK IN ('committed','failed'), model_call_id) +state_events(event_id, turn_id, sequence_number, event_type, payload_json) -- append-only +state_cache(session_id, head_turn_id, state_json) -- replay cache +model_calls(..., request_hash, response_hash, parsed_response_json, + validation_errors_json, prompt_eval_count, eval_count, duration_ms) +named_checkpoints(checkpoint_id, session_id, turn_id, name, UNIQUE(session_id,name)) +summaries(summary_id, session_id, through_turn_id, kind CHECK IN ('scene','campaign')) +lore_documents + lore_documents_fts (FTS5) +``` + +Note `summaries.through_turn_id` — anchored to a turn, not a count. That is +exactly the property Open Dungeon lacks. + +## C5. Coupling to the CLI + +The layering is clean: `cli.py` → `app/commands.py` → `app/game_service.py` / +`app/turn_service.py` → `storage/repositories.py`. Authoritative state lives in +`state/` (events, reducer, validator) and context assembly in `context/`. None of +that imports the CLI. A browser/API layer could sit beside `cli.py` and call the +same services without moving state logic. + +What it does **not** have, and would all be new work: any HTTP layer, streaming, +in-app campaign setup (worlds are hand-authored TOML + Markdown on disk), +embeddings or semantic memory, document import, media, and prompt inspection +beyond stored hashes. + +## C6. Lore / FTS — reusable + +`lore/indexer.py` + `lore_documents_fts` (FTS5) with content hashing and +`modified_ns` for incremental reindex, scoped by `world_id`, and a `kind` column +already carrying a classification axis. This is a good starting shape for the +Canon/Reference/Inspiration tiers in `IMPORTED-KNOWLEDGE-DESIGN.md`, and it is +deterministic and offline. diff --git a/planning/reports/PHASE-0B-AI-DND-EXPERIMENT.md b/planning/reports/PHASE-0B-AI-DND-EXPERIMENT.md new file mode 100644 index 0000000..31eac96 --- /dev/null +++ b/planning/reports/PHASE-0B-AI-DND-EXPERIMENT.md @@ -0,0 +1,146 @@ +# Phase 0B — Experiment A: AI-DnD + +All results are from a live server on `127.0.0.1:8321` with a fresh SQLite +database, generating against local Ollama. + +## A1. Runs with Ollama locally — yes + +Default `endpoint_url` is `http://localhost:11434/v1`. After setting only the +model name: + +``` +POST /api/settings/test → {"ok":true,"models":["qwen2.5:0.5b"]} +``` + +Three streamed turns produced 101/162/113 SSE events with `player`, `chunk` and +`done` types and a coherent transcript. + +## A2. RPG state can be left empty — yes + +`schemas.AdventureCreate.scenario_id` is optional. With it null, +`crud.create_adventure` sets `world_state={}` and creates no story cards, no +scripts and no opening action, but still calls `tree.head_branch(...)` so the +story tree exists from the start. A noir campaign created this way played, +branched, summarized and retrieved memories normally. The RPG referee is +opt-in per scenario, not a core dependency. + +## A3. Retry is non-destructive; Undo is destructive + +Measured on the same adventure, reading rows straight from SQLite: + +``` +baseline rows: 6 ids [1,2,3,4,5,6] + +RETRY → new action id 7; rows 7 ids [1..7] + /variants at the tip lists BOTH take 0 (id 6) and take 1 (id 7) + +UNDO → rows 7 → 4; DELETED ids = [5, 6, 7] +``` + +Undo removed the player action (5), the live AI take (6) **and the alternate +take that the retry had just preserved (7)**. `nodes.delete_turn` deletes every +attempt in the group on that branch and calls `memorybank.forget_node`. There is +no redo endpoint anywhere in `routers/`. + +This contradicts `reports/PRELIMINARY-RECOMMENDATION.md`, which credits AI-DnD +with "non-destructive retry" (true) and generalizes it to rollback (false). + +## A4. Branching is non-destructive + +Adding an alternate take at a passed turn (`POST /actions/3/takes`) forked +automatically: + +``` +before: ids 1-8, all branch 1, depths 0-7 +after : ids 1-8 unchanged on branch 1 + id 9 branch 2 depth 2 (the alternate player action) + id 10 branch 2 depth 3 (its AI continuation) + +/branches → [ {id:1, parent:null, fork_depth:null, own_actions:8}, + {id:2, parent:1, fork_depth:1, own_actions:2, is_head:true} ] +``` + +Not one row of branch 1 was touched. Note the semantics: `after_id` pointing at a +node that is *live and on the path* is a no-op, not a fork. A fork happens only +when writing below a take the story has moved past (`nodes.stand_on`). So +"restore to an arbitrary earlier turn and branch" is not directly expressible +today — it must go through creating a take at that turn. + +## A5. Memory isolation across branches — correct, with negative control + +Memory bank enabled, summary model `qwen2.5:0.5b`, embeddings +`nomic-embed-text`, `memory_top_k=5`. Three more turns played on branch 1 +establishing possessions (brass key, silver revolver, bullets). + +``` +memories stored: (id 1, branch 1, depth 5) + (id 2, branch 1, depth 11) + +on branch 2 (forked at depth 1): + memories retrieved = {"used": [], "error": null} + leak probes over the FULL assembled prompt: + 'brass key' False | 'revolver' False | 'lockbox' False + 'ashtray' False | 'bullets' False + +NEGATIVE CONTROL — switch back to branch 1: + memories retrieved = 2, top similarity 0.8087 + 'revolver' present in prompt = True +``` + +The empty result on branch 2 is genuine isolation, not a dead retrieval path. +`memorybank.retrieve_memories` applies `lineage.path_of(db, adventure).clause(models.Memory)`, +and `Memory` rows carry `branch_id` plus source depths. Eviction deliberately +ignores the branch clause, which is correct — it is a capacity concern. + +History is likewise lineage-scoped: branch 2's prompt contained only its own +text. + +## A6. Insights / prompt transparency — already sufficient + +`GET /adventures/{id}/context` returns the exact next prompt broken into labeled +sections with token costs: + +``` +narrator 57 | ai_instructions 9 | persona 15 | plot_essentials 24 +history 988 | used_memories 143 | length_hint 46 +``` + +Per-action snapshots are stored on `Action.context_snapshot` and served by +`GET /actions/{id}/context`. This satisfies specification §11 as-is. + +## A7. Scripting/QuickJS can be removed cheaply + +`quickjs` is imported in exactly one file (`scripting/engine.py`). The whole +feature is reachable through two symbols (`run_hook`, `ScriptPipeline`) across +four call sites plus `routers/scripts.py`. + +Disposable experiment: made `quickjs` unimportable via a `meta_path` blocker and +replaced `run_hook` with a pass-through stub. + +``` +app.main imports OK with scripting stubbed and quickjs absent +19 failed, 613 passed (3.0% of the suite) +``` + +Every failure is in `test_turn_flow_integration`, `test_take_state`, +`test_story_tree_baseline` or `test_retry_variants`, and every one fails for the +same reason: those tests **use a JS `output_js` hook as the instrument** to make +state observable, e.g. `test_play_then_undo_reverts_gold` seeds a `GOLD_SCRIPT` +and asserts `script_state == {"gold": 10}` then `{}` after undo. The behavior +under test (per-node state rollback) is intact; only the measuring device is +gone. Re-instrumenting against `world_state_after`, which is already snapshotted +per node, is the fix. + +## A8. Hosted/cloud features + +`MULTI_USER` is off by default and gates ten modules +(`auth`, `limits`, `netguard`, `cleanup`, `security`, `analytics`, +`routers/{auth,analytics,debug,story_cards}`). `analytics.py` is a *self-hosted* +counter writing to two local tables — no third-party tracker and no outbound +call — so it is removable rather than dangerous. `render.yaml`, the demo-key +path and `psycopg` are the hosted leftovers. + +`netguard.py` is worth flagging: it refuses endpoints that resolve to +**non-public** addresses, and only in multi-user mode. That is SSRF protection +for a hosted deployment. Our requirement is the opposite — warn on endpoints that +are **not loopback**. See ai-adventure's `_endpoint_warnings` for the shape we want. diff --git a/planning/reports/PHASE-0B-BASELINE.md b/planning/reports/PHASE-0B-BASELINE.md new file mode 100644 index 0000000..61c691a --- /dev/null +++ b/planning/reports/PHASE-0B-BASELINE.md @@ -0,0 +1,39 @@ +# Phase 0B — Baseline Results + +**Date:** 2026-09-01. Host: Linux, 4 cores, 15 GB RAM, no GPU, Python 3.12.3, Node 22.23.1. +Inference: Ollama in Docker (`ollama/ollama:latest`) on `127.0.0.1:11434`, models +`qwen2.5:0.5b`, `qwen2.5:3b-instruct`, `nomic-embed-text`. + +| | AI-DnD | Open Dungeon | ai-adventure | +|---|---|---|---| +| SHA | `d72f7c1b` | `b0a79f96` | `873ea918` | +| License | MIT | MIT | Apache-2.0 | +| Repo age / commits / authors | created 2026-07-20, 172, 3 | 2026-06-12, 45, 1 | 2026-07-20, 20, 1 | +| Code size | ~30.7k LOC | ~9.1k LOC | ~4.5k LOC | +| Stack | FastAPI + SQLAlchemy + React/Vite | Next.js 16 + React 19 | Python stdlib + pydantic, CLI | +| Storage | SQLite (Postgres option via `AIDND_DATABASE_URL`) | SQLite (`better-sqlite3`) | SQLite + FTS5 | +| Install | `pip install -r requirements*.txt`, `npm install` — clean | `npm install` — clean, 7 advisories (6 high) | `pip install -r requirements.txt` — clean | +| Dependencies | 41 pip + 39 npm | 335 npm | **6 pip (1 direct: pydantic)** | +| Tests | **632 passed / 236s** | **none — no script, no framework** | **76 passed / 1.9s** | +| Ports | 8000 (prod/docker), 5173 + 8000 (dev) | 3000 (app), 7869 (FLUX worker), 8188 (ComfyUI) | none (CLI) | +| Model assumption | any OpenAI-compatible; **defaults to `http://localhost:11434/v1`** | Ollama native API, or OpenAI-compatible "custom" | OpenAI-compatible; base_url in `world.toml` | +| Docker build | `docker build` succeeds (3-stage, builds SPA) | not attempted (npm path used) | n/a | +| Runtime failures | none once running; **offline blocker, see offline report** | none | typed-event validation fails on a weak model (by design) | +| Cloud/hosted surface | `render.yaml`, multi-user auth, demo key, analytics, Postgres, OpenRouter default | OpenRouter preset, Next telemetry on by default, donation links | **none** | + +## Notes + +- **AI-DnD** ran its whole suite green on the first attempt with no fixes. Its + default settings already point at Ollama; `POST /api/settings/test` returned + `{"ok":true,"models":["qwen2.5:0.5b"]}` with no configuration beyond selecting + a model name. +- **Open Dungeon** has `lint` and several Windows/image smoke scripts, but no + unit or integration tests of any kind. Its `local` provider only offers five + hard-coded Gemma 4 QAT builds (`src/lib/text-models.ts`), so a non-Gemma local + model must go through the "custom" OpenAI-compatible provider; that is how it + was driven here. +- **ai-adventure** is the only candidate whose full dependency closure is one + third-party package. `python -m local_adventure doctor` is a genuine + environment self-check (Python version, writable runtime dir, SQLite version, + FTS5 availability, schema version, world validity, endpoint reachability, + model visibility). diff --git a/planning/reports/PHASE-0B-FOLLOWUP-CHECKS.md b/planning/reports/PHASE-0B-FOLLOWUP-CHECKS.md new file mode 100644 index 0000000..e7bc742 --- /dev/null +++ b/planning/reports/PHASE-0B-FOLLOWUP-CHECKS.md @@ -0,0 +1,164 @@ +# Phase 0B — Follow-Up Checks + +Four open questions settled after the undo/redo spike. Ordered by how much each +changes the plan. + +--- + +## 1. The world-state referee under a local model + +**Why it mattered:** the referee is the closest thing AI-DnD has to +specification §2's "the model proposes, the application owns authoritative +state", and §7's structured narrative state. It was never exercised in the main +round, because the RPG path was deliberately left empty to prove genre-neutrality. +If it only works with a cloud-class model, the principle the architecture rests +on is at risk locally. + +**How it works.** `EMIT_RULE` asks for a fenced ` ```state ` block of **relative +deltas**, with an example (`{"player.hp": -15, ...}`) and a recency reminder +appended at the end of the prompt. `apply.py` computes `new = old + delta`, then +clamps by `max_delta_per_turn`, then by `min`/`max`, and reports every change as +applied / clamped / rejected with corrective text fed back to the model. + +### Result A — in the full application, `qwen2.5:3b-instruct` sent absolute values + +Playing the seeded "Bandit Camp (RPG world state)" scenario: + +``` +turn 1 proposed: {"player.hp": 92, "npc.gwen.trust": 25, + "player.outfit": "...arrow in shoulder", "npc.gwen.health": 90} + applied : player.hp 100 -> 100 "did not move. It is already at its + maximum of 100 ... at most 35 per turn" + gwen.trust 20 -> 40 (clamped from +25 to +20) + gwen.health 100 -> 100 (clamped) + outfit updated correctly + +turn 2 proposed: {"npc.gwen.trust": 30} + applied : gwen.trust 40 -> 60 (clamped from +30 to +20) +``` + +The model meant "hp is now 92". The engine read `+92`, clamped it to `+35`, then +to the ceiling of 100. **The player took an arrow to the shoulder and finished +the turn at full health**, with the prose describing a wound. + +This is the failure mode that matters: it is not corruption, and no validator can +catch it, because `+92` is a perfectly legal proposal. The application stayed +authoritative — which is the point of the referee — but the authoritative state +silently stopped tracking the narration. + +The free-text stat (`player.outfit`) was handled correctly, because it is marked +`(free text)` and takes a whole value rather than a delta. + +### Result B — in isolation, the same model gets it right + +An isolated probe using AI-DnD's own `EMIT_RULE` and `extract_delta` verbatim, so +that only the model varies. A short stat guide, live values, one wound scene, +3 samples each: + +| Model | Emitted a parsable state block | Correct relative (negative) hp | +|---|---|---| +| `qwen2.5:0.5b` | 0/3 | 0/3 | +| `qwen2.5:3b-instruct` | 3/3 | **3/3** (−20, −20, −15) | +| `qwen2.5:7b-instruct` | 3/3 | **3/3** (−15, −30, −15) | + +So the delta protocol is **not** beyond a 3B model. The difference between A and +B is context: the full application prompt carries scenario prose, persona, a +stat guide, live values, story cards and history, and under that load the 3B +model degraded to absolute values. In a short prompt it followed the same +instruction perfectly. + +> Recorded because it nearly became a wrong finding: the first run of this probe +> reported 0/3 for both models. That was a bug in the probe, not the models — +> `extract_delta` returns `(clean_text, delta)` and the harness was reading +> element 0. The table above is the corrected run. + +Note also `qwen2.5:7b-instruct` sample 2 proposing `npc.gwen.trust: +75`, which +the referee would clamp to +20. Even a capable model proposes disproportionate +values, which is precisely what the clamp is for. + +### What this means for the design + +1. **Not a blocker for the fork.** The referee is per-scenario and opt-in, and we + are generalizing it into narrative state rather than adopting it as-is. +2. **Prefer an unambiguous state vocabulary.** A relative-delta protocol has a + failure mode that validation cannot detect, and it degrades with context + length. ai-adventure's typed events (`set_flag key=… value=…`) are explicit + and absolute, so the same mistake is impossible to express. **This is a + concrete argument for taking ai-adventure's event vocabulary rather than + AI-DnD's delta vocabulary when generalizing to genre-neutral narrative state.** +3. **Keep the emission rule close to generation.** AI-DnD already appends + `EMIT_REMINDER` at the end of the prompt for exactly this reason; whatever we + build should keep that property and be tested at realistic context length, + not just in a clean harness. +4. **Sample sizes are small** (3 per model; 2 parsed in-app turns). Enough to + establish the failure mode exists and that it is context-dependent, not enough + to rank models. A proper capability matrix belongs in the build phase. + +--- + +## 2. Export/import silently redoes an undone story + +**Status: a real defect the spike introduces into the export path. One-field fix.** + +`bundle.py:_point_the_head` sets the head on import by recomputing it: + +```python +depths = [n["depth"] for n in story["nodes"] if n["branch"] == head] +if depths: + adventure.head_depth = max(depths) +``` + +The export format carries `headBranch` (line 120) but **not** the head depth. + +With the destructive undo this was correct — undone rows did not exist, so +`max(depths)` was the head. With the spike's non-destructive undo, an adventure +exported while its head sits behind the tip comes back with every abandoned turn +restored. The undo is silently undone by a round-trip. + +**Fix:** add `headDepth` to the bundle (optional field, or a `v3`), and honour it +in `_point_the_head`, falling back to `max(depths)` when absent so that existing +`ai-dnd-adventure-v2` files still import. Small, but it must land in the same +change as the non-destructive undo, not after it. + +--- + +## 3. Story cards are not lineage-scoped + +**Status: a design constraint for imported knowledge, not a present defect.** + +`models.StoryCard` is keyed by `scenario_id` **or** `adventure_id`, and carries +**no `branch_id` and no `depth`** — unlike `Memory`, which carries both and is +therefore filtered by the same capped lineage clause that gives memories their +branch isolation. + +`context.builder.match_cards` is pure keyword matching over the adventure's whole +card list; `routers/story_cards.py` contains no lineage reference at all. + +This is harmless today, because cards are authored by the user or copied from a +scenario — nothing derives a card from story content, so an abandoned branch +cannot invent one. + +It stops being harmless the moment imported knowledge or any derived-card feature +lands. **Implication for `IMPORTED-KNOWLEDGE-DESIGN.md`:** any knowledge source +that can be created or updated from story content must carry `(branch_id, depth)` +like `Memory` does, and be read through `lineage.Path.clause`. Do that and it +inherits both the branch isolation and the undo isolation for free — which is the +strongest argument for a new table rather than extending `story_cards`. + +--- + +## 4. Postgres/psycopg is cleanly removable + +**Status: resolved, low effort.** + +The entire Postgres surface is three places: + +- `database.py` — the `AIDND_DATABASE_URL` / `DATABASE_URL` branch and + `_normalize_url`. Deleting the branch leaves the SQLite path, which is already + the default when neither variable is set. +- `migrations.py` — two helpers that tolerate SQLite returning a raw JSON string + where psycopg returns a parsed list. They simplify rather than disappear. +- `analytics.py` — `from sqlalchemy.dialects.postgresql import insert as pg_insert`, + which goes with analytics, already scheduled for removal. + +`psycopg[binary]` then drops out of `requirements.txt`. No blockers. diff --git a/planning/reports/PHASE-0B-OFFLINE-NETWORK.md b/planning/reports/PHASE-0B-OFFLINE-NETWORK.md new file mode 100644 index 0000000..ffacaa0 --- /dev/null +++ b/planning/reports/PHASE-0B-OFFLINE-NETWORK.md @@ -0,0 +1,106 @@ +# Phase 0B — Offline and Network Behavior + +Method: a Docker network created with `--internal` (no NAT, no DNS to the +outside). The Ollama container was attached to it so the app could reach a model +while having no path to the Internet. Isolation was verified from inside the +app container before testing: + +``` +blocked ('1.1.1.1', 443) OSError +blocked ('openrouter.ai', 443) gaierror +blocked ('fonts.googleapis.com', 443) gaierror +``` + +## AI-DnD — fails offline as shipped; fine once one file is vendored + +First turn on the isolated network died. The SSE stream emitted the `player` +event and stopped. Container log: + +``` +requests.exceptions.ConnectionError: HTTPSConnectionPool( + host='openaipublic.blob.core.windows.net', port=443): + Max retries exceeded with url: /encodings/cl100k_base.tiktoken + (NameResolutionError ... Temporary failure in name resolution) +``` + +`tiktoken` downloads its BPE encoding on first use, and AI-DnD calls it for +context budgeting on every turn. On the host this was invisible, because the +file had already been cached to `/tmp/data-gym-cache/9b5ad71b…` during an +earlier online run. + +After copying that 1.7 MB file into the container: + +``` +OFFLINE TURN GENERATED: "I am Vale, an explorer. I journey alone through the +darkened depths of the lighthouse..." +``` + +So: a hard blocker on a clean air-gapped install, and a packaging fix — vendor +the encoding (or pre-seed `TIKTOKEN_CACHE_DIR`, or replace the tokenizer). Worth +stressing that **static analysis could not have found this**; it took an actually +isolated run. + +Other AI-DnD network surface: + +- **Google Fonts at runtime.** The built SPA's `index.html` still contains + `fonts.googleapis.com/css2?family=Cinzel...&family=Crimson+Pro...&family=Inter...`, + and `main.py`'s CSP explicitly allows `fonts.googleapis.com` / + `fonts.gstatic.com`. Violates specification §12. Fix: self-host three families. +- Only other remote host in backend Python is `https://openrouter.ai/api/v1`, + used as a default endpoint constant and a header-attribution host check. Both + removable with the cloud-provider path. +- `analytics.py` is a **self-hosted** counter writing to two local tables. No + third-party script, no outbound request. It records no IP, no user agent, and + hashes the user id with HMAC. Removable, and not a telemetry leak in the + meantime. +- Connection test and turns honour the configured endpoint only; no automatic URL + retrieval or content fetching was observed. + +## Open Dungeon — runtime clean, build and telemetry are not + +- **Next.js telemetry is on by default** and printed its notice on first start. + Needs `NEXT_TELEMETRY_DISABLED=1` or `next telemetry disable` in the + production configuration. +- **Fonts are fine at runtime.** `layout.tsx` uses `next/font/google`, which + downloads at *build* time and self-hosts. The served page contains no + `fonts.googleapis.com` reference. The trade-off is that the build needs + network access. +- Remote hosts referenced in `src/`: `openrouter.ai` (preset provider URL and a + model list), plus `ko-fi.com` and `github.com/sponsors` donation links in the + UI. Everything else is `127.0.0.1` / `localhost` (13 occurrences). +- Local story play needs no cloud service: Ollama for text, a local FLUX worker + or the user's own ComfyUI for images. +- 335 npm packages with 6 high-severity advisories is the largest supply-chain + surface of the three. + +## ai-adventure — verifiably local-only + +The strongest posture by a wide margin, and the easiest to audit: + +- **Exactly one outbound call site in the whole codebase**: `urlopen` in + `llm/lm_studio.py`. Nothing else in `local_adventure/` opens a socket. +- **No hardcoded remote host anywhere.** The only `http` string in the package is + the scheme check in `content/models.py`. +- **One direct dependency** (`pydantic`), six packages in the closure. +- It is the only candidate that already implements specification §12's + non-local-endpoint warning: + +```python +def _endpoint_warnings(config): + hostname = urlparse(config.model.base_url).hostname + if hostname not in {"127.0.0.1", "localhost", "::1"}: + return ["model.base_url is not loopback; game prompts and content will + be sent to that endpoint. Enable API authentication."] +``` + +- `audit.store_prompts` defaults to `false`, with prompt *hashes* stored instead. +- Tests, world validation, session creation, replay, branching and export are all + offline by construction. + +## Summary + +| | Local play works offline | Cloud service required | Telemetry / remote assets | Removal difficulty | +|---|---|---|---|---| +| AI-DnD | **Only after vendoring the tiktoken encoding** | No | Google Fonts at runtime; local-only analytics tables | Low — one vendored file, three self-hosted fonts, delete analytics | +| Open Dungeon | Yes at runtime; build needs network | No | Next.js telemetry on by default | Low — one env var; fonts already self-hosted | +| ai-adventure | **Yes, unconditionally** | No | **None** | Nothing to remove | diff --git a/planning/reports/PHASE-0B-OPEN-DUNGEON-HISTORY.md b/planning/reports/PHASE-0B-OPEN-DUNGEON-HISTORY.md new file mode 100644 index 0000000..366817d --- /dev/null +++ b/planning/reports/PHASE-0B-OPEN-DUNGEON-HISTORY.md @@ -0,0 +1,105 @@ +# Phase 0B — Experiment B: Open Dungeon history model + +Driven live on `127.0.0.1:3111` against Ollama through the "custom" +OpenAI-compatible provider (`http://127.0.0.1:11434/v1`, `qwen2.5:0.5b`), with +its own SQLite file. + +## B1. Schema — no lineage exists + +`src/lib/db.ts` creates four tables: `chats`, `messages`, `characters`, +`app_settings`. The message row is: + +```sql +CREATE TABLE messages ( + id TEXT PRIMARY KEY, + chat_id TEXT NOT NULL REFERENCES chats(id) ON DELETE CASCADE, + role TEXT CHECK (role IN ('user','assistant')), + content TEXT NOT NULL, + attachments_json ..., image_request_json ..., generated_image_json ..., + created_at TEXT NOT NULL +); +CREATE INDEX idx_messages_chat_created ON messages(chat_id, created_at); +``` + +No `parent_id`, no branch, no take, no depth, no checkpoint table. Order is +`created_at` (a TEXT timestamp), with ties broken by id. + +## B2. Retry / Edit / Erase — all destructive, measured + +``` +BEFORE (5 rows) + a504cb34 assistant "Oh, Vale, you're just sitting in that rundown ..." + e349d8b9 user "I pocket a brass key from the ashtray." + 9984692b assistant "You find a worn, faded brass key on the tabletop..." + bb1ceb3f user "I drive to the address on its tag." + a6e11115 assistant "Yes, you keep going. There, you can call your ..." + +EDIT e349d8b9 in place → PATCH /api/chats/{id}/messages/{id} + original text "brass key from the ashtray" still anywhere in DB? False + +DELETE a6e11115?after=1 (what BOTH Retry and Erase call) + rows 5 → 4 + +Scan of every column of every table for the deleted tail +or the pre-edit text: none +``` + +- **Edit** is `UPDATE messages SET content = ? WHERE id = ?`. The prior text is + unrecoverable. +- **Retry** (`retryLastTurn`, `page.tsx`) deletes the last assistant message and + everything after it, then regenerates. The discarded attempt is gone. +- **Erase** (`eraseLastTurn`) deletes the last exchange and the tail. +- Old future turns are therefore always deleted, never retained. + +## B3. What depends on the linear model + +1. **`db.ts`** — 20 exported functions. `addMessage`, `updateMessageContent`, + `deleteMessageAndAfter` and `getChat` are the history four. + `getChat` returns a flat `messages` array and must become a lineage query. +2. **`story-prompt.ts`** — windows by array index: + `for (let i = messages.length - 1; ...)`, `messages.slice(dropped)`, + `messages.slice(0, dropped)`. Becomes an ancestry walk. +3. **The summary watermark is positional — the cost Phase 0A missed.** + `chats.story_summary_count` records "how many of the chat's oldest messages + the summary already covers", and `api/story/route.ts` compares + `evicted.length > stored.coveredCount`, then summarizes + `evicted.slice(stored.coveredCount)`. "The first N messages" is meaningless on + a branch. Summaries must be re-anchored to a turn id and made per-lineage. +4. **`page.tsx` is 3,991 lines** in one component; `api/story/route.ts` is 1,196. + Retry/erase/edit live there as array slicing (`messages.slice(0, cutFrom)`). + There is no take stepper, branch panel or tree view to build on. +5. **No tests.** All of the above would be changed with no regression net. + +## B4. Invasiveness estimate + +To reach parent-linked turns, an active head, retained abandoned history, named +checkpoints and lineage-safe summaries, Open Dungeon needs: a schema migration, +a rewrite of its persistence read/write path, a rewrite of context assembly, a +redesign of summarization, and new branch UI — with a test suite written first +to make any of it safe. This is a from-scratch implementation of the hardest +part of the specification, not a retrofit. + +By comparison AI-DnD already has all of it except a non-destructive undo, and +its reads funnel through one chokepoint (`lineage.Path.clause`). + +## B5. Worth reusing as reference + +- The reading experience: serif prose, prose-size control, the composer's + Do/Say/Story modes. +- Inline scene images driven by a narrator tool call (`generate_image`), with a + local FLUX worker on `127.0.0.1:7869` or a user's ComfyUI on `8188`. +- Character portraits reused as reference images for visual continuity, and fed + back to the narrator as vision context. This is the most valuable idea here for + `MEDIA-EXTENSION-CONTRACT.md`. +- The worker HTTP protocol may be portable even though none of the UI is — + Open Dungeon is Next.js, AI-DnD is React + FastAPI. + +## B6. Other findings + +- The `local` provider hard-codes five Gemma 4 QAT builds + (`src/lib/text-models.ts`, `LOCAL_TEXT_MODELS`) and `/api/health` reports only + those as installed. Any other local Ollama model must be reached through the + "custom" provider. A genre-agnostic app wanting "pick any installed model" + must lift this. +- Next.js telemetry is enabled by default and printed its notice on first start. +- 335 npm packages; `npm audit` reports 6 high, 1 low. diff --git a/planning/reports/PHASE-0B-UNDO-SPIKE.md b/planning/reports/PHASE-0B-UNDO-SPIKE.md new file mode 100644 index 0000000..04d298f --- /dev/null +++ b/planning/reports/PHASE-0B-UNDO-SPIKE.md @@ -0,0 +1,214 @@ +# Phase 0B — Spike: Non-Destructive Undo/Redo in AI-DnD + +**Date:** 2026-09-01 +**Base:** AI-DnD `d72f7c1b`, disposable copy at `phase0b/spike/` +**Verdict:** **Passed on every criterion.** The retrofit is small, centralized, +and costs 5 of 632 tests — all of which assert the deleted-row behavior that was +deliberately replaced. + +## The question + +Can undo move a head cursor backward without deleting rows, does writing below a +moved-back head fork a branch, and does Redo work — without breaking +branch-scoped memory isolation? + +## What the change turned out to be + +Three files, **+130 / -31 lines**, and roughly a fifth of the additions are +comments and the new endpoint's docstring. + +| File | Change | +|---|---| +| `app/context/lineage.py` | +26 / -3 | +| `app/routers/adventures/takes.py` | +74 / -27 | +| `app/routers/adventures/turns.py` | +30 / -1 | + +### 1. Read path — cap the lineage at the head + +The stored lineage records the newest branch entry **uncapped** (`max_depth = +None`, meaning "through to the tip"), and older entries capped at their fork +depths. `Path.clause` and `Path.contains` now read an uncapped entry as capped at +`self.tip`, which is `adventure.head_depth`: + +```python +def _cap(self, max_depth: int | None) -> int | None: + return self.tip if max_depth is None else max_depth +``` + +This is the whole read-side change. It works because **every read already funnels +through `lineage.path_of()`**, so one substitution moves the entire application — +transcript paging, context assembly, `attempts.preceding`, and memory retrieval — +onto a head that can sit behind the deepest node. + +A `Path.uncapped()` helper was added for the two callers that must deliberately +look past the head (redo, and the fork check). + +`prefix_covering` already computed `top = self.tip if max_depth is None else +max_depth`, so the windowing arithmetic was consistent with this reading before +the change; nothing there needed touching. + +### 2. Undo — move the head instead of deleting + +`undo_turn` keeps its turn lock, its "nothing to undo" guard and its +fork-boundary guard, keeps `attempts.preceding` + `attempts.restore_state` for +the state rollback, and replaces the two `delete_turn` calls plus +`tree.refresh_head` with one assignment: + +```python +adventure.head_depth = (first_removed.depth or 0) - 1 +``` + +No `db.delete`. No `memorybank.forget_node`. + +### 3. Write path — fork when writing below the head + +`turns.fork_if_behind_head` runs in `create_action` beside the existing +`_move_to_after`. If any live node on the path sits deeper than the head, it calls +the **already-existing** `tree.branch_at(db, adventure, adventure.head_depth)`, +which creates an empty branch leaving the path at that depth and does not touch +the branch being left. When the head is at the tip it does nothing, so a story +that is never undone forks exactly as often as before. + +### 4. Redo — walk the head forward + +New `POST /adventures/{id}/redo`. Reads the path *uncapped*, takes the next node +deeper than the head, and advances over the whole turn (player action + AI reply) +rather than half of it. 400 when there is nothing to redo. + +## Results + +### Undo deletes nothing; redo round-trips exactly + +Six actions across three turns, live Ollama: + +``` +AFTER 3 TURNS head=(branch 1, depth 5) rows 1..6 all live + visible: [1 story, 2 ai, 3 do, 4 ai, 5 do, 6 ai] + +UNDO x1 rows 6 -> 6 DELETED=0 head=(1, 3) + visible: [1 story, 2 ai, 3 do, 4 ai] + +UNDO x2 more head=(1, -1) + visible: [] rows still 6 + +REDO x3 head=(1, 5) rows 6 + visible: [1 story, 2 ai, 3 do, 4 ai, 5 do, 6 ai] +``` + +Elsewhere in the run, **11 consecutive undos** were performed on a 23-action +story with 0 rows deleted, which clears the specification's "at least five undo +operations, unlimited preferred" comfortably. + +### Writing below a moved-back head forks, and the abandoned line survives + +``` +UNDO once, then write a different continuation: + + 1 story br1 d0 live 5 do br1 d4 live <- the abandoned tail, + 2 ai br1 d1 live 6 ai br1 d5 live still live, still on br1 + 3 do br1 d2 live + 4 ai br1 d3 live 7 do br2 d4 live <- the new line + 8 ai br2 d5 live + +branches: {id 1, parent null, fork_depth null, own_actions 6, is_head False} + {id 2, parent 1, fork_depth 3, own_actions 2, is_head True} + +visible on the new line: [1, 2, 3, 4, 7, 8] +switch to branch 1 : [1, 2, 3, 4, 5, 6] +``` + +This is `DECISIONS/005-branch-preserving-history.md` behavior: the abandoned +future is retained as an alternate branch rather than erased, and it is reachable +through the existing branch switcher with no new UI concept. + +### Memory isolation still holds — the important regression check + +A 23-action story with a real memory embedded via `nomic-embed-text` at depth 5. +Probed with terms unique to the turns an undo hides, plus terms from the turns it +keeps: + +``` +AT TIP (control) head_depth=22 memories_used=1 (similarity 0.689) + hidden-turn terms leaking: ['six bullets','warehouse nine','iron stairs', + 'ledger','docks','office desk','inside my coat'] + (correct — nothing is hidden at the tip) + visible-turn terms present: ['rainy city','desk drawer'] + +11 undos, 0 rows deleted (actions still 23) + +AFTER UNDO head_depth=3 memories_used=0 + hidden-turn terms leaking: NONE + visible-turn terms present: ['rainy city','desk drawer'] + +REDO back to tip head_depth=22 memories_used=1 (similarity 0.689) +``` + +The memory at depth 5 stops being retrieved when the head moves behind it and +becomes eligible again on redo — **without being deleted**. That is the property +the destructive undo bought by calling `forget_node`, recovered for free, because +`Memory` rows carry `branch_id` and `depth` and retrieval already goes through +the same capped clause. + +> One false alarm worth recording: an early probe reported `revolver` leaking. It +> was legitimate visible history — actions 22 and 23 sit at depths 2 and 3, at or +> below the head. A second false alarm came from probing the context *after* the +> script had already redone to the tip. Both were resolved by re-probing inside a +> single script with an explicit control, which is why the table above reports the +> control run alongside the result. + +### Test suite: 627 passed, 5 failed + +``` +5 failed, 627 passed in 184s (0.8% of the suite) + +tests/test_attempt_siblings.py::test_undo_takes_every_attempt_with_it +tests/test_branch_forking.py::test_undo_stops_at_the_fork +tests/test_state_revert.py::test_undo_reverts_state_to_before_the_turn +tests/test_state_revert.py::test_undo_of_bare_continue_uses_the_node_in_front +tests/test_state_revert.py::test_undo_prunes_memory_covering_removed_actions +``` + +Every one asserts that rows or memories were **deleted**: + +- `assert [a.type for a in adv.actions] == ["start"]` (three of them) +- `assert len(_rows(...)) == rows_before - 1` +- `assert texts == {"k"}` — the pruned-memory set + +Crucially, the *state* assertions inside those same tests still pass. In both +`test_state_revert` cases, `assert adv.script_state == {"gold": 0}` succeeds and +only the row-count line fails — state rollback is intact. And +`test_undo_stops_at_the_fork` still enforces its real subject: the guard +refusing to undo into a parent branch is untouched and still returns 400. + +Also worth noting for the open question raised in the recommendation: the four +memory suites — `test_memory_settling`, `test_memory_nodes`, +`test_memory_retrieval`, `test_memory_rewrite` — and `test_history_window` all +**passed unchanged**. The cursor arithmetic did not need re-deriving. + +## Rough edges found, not fixed + +1. **Undo can empty the story.** The original guard refuses when the newest node + is `type == "start"`, but an adventure opened with a user-written `story` + action has no `start` node, so undo walks to `head_depth = -1` and the + transcript renders empty. Redo recovers it, but the floor should be the + opening node rather than `-1`. +2. **Retry and `add_take` were left on the old path.** The spike only routed the + ordinary write through `fork_if_behind_head`. Retrying while the head is + behind the tip is not yet defined and needs a decision — most likely the same + fork. +3. **No pruning story yet.** Abandoned turns now accumulate. That matches the + specification ("retained but marked disposable; cleanup later"), but nothing + marks them disposable and no cleanup exists. +4. **Frontend untouched.** There is no Redo button and the branch panel does not + distinguish a line abandoned by undo from one forked deliberately. + +## Conclusion + +The retrofit lands where the recommendation predicted: one chokepoint +(`lineage.Path.clause`), one stored head that already existed +(`adventures.head_depth`), and one branch primitive that already existed +(`tree.branch_at`). The unknown was whether the depth cap would disturb the +memory and summary cursors, and it does not. + +The fork decision is sound. The remaining work on this axis is the four rough +edges above plus named checkpoints, not a redesign.