From edeeb7a5cc18cf0a38196fa3cf00a5d2398999ef Mon Sep 17 00:00:00 2001 From: itsamejms Date: Sun, 6 Sep 2026 23:19:05 +0100 Subject: [PATCH] docs: repo review + master roadmap --- docs/repo-review.md | 211 ++++++++++++++++++++++++++++++++++++++++++++ docs/roadmap.md | 119 +++++++++++++++++++++++++ 2 files changed, 330 insertions(+) create mode 100644 docs/repo-review.md create mode 100644 docs/roadmap.md diff --git a/docs/repo-review.md b/docs/repo-review.md new file mode 100644 index 0000000..9cc65c2 --- /dev/null +++ b/docs/repo-review.md @@ -0,0 +1,211 @@ +# DM-Pal — Repo Review & Next Steps (2026-09-06) + +*A fresh whole-repo pass: shell, components, Rust backend, scripts, git history, +and the two existing docs. Findings are verified against the code, not copied +from `plan.md` — several items in `plan.md` §17 have since shipped, and a few +claims there are stale (noted below). Each finding ships with a concrete fix.* + +**Overall: healthy.** The architecture is clean (self-contained tools behind one +`renderView`, single shared `generate` path with lore injection, two SQLite +stores + a debounced UI-state store), the ponytail discipline is visible +everywhere, and the Rust side has real unit tests. The P0/P1 items from +`ui-ux-improvements.md` have genuinely landed. The biggest problems now are +one P0 functional bug (settings don't survive restart), one UX gap (streaming +is fake), and repo/distribution hygiene (CI, signing, stale docs). + +--- + +## 1. Findings — ranked + +### P0-1. LLM config is never persisted to disk — restart wipes Settings + +`set_llm_config` (`src-tauri/src/commands/llm_commands.rs`) only mutates an +in-memory `Mutex`; `lib.rs` setup always starts from +`LlmConfig::default()`. Nothing writes the config to the prefs store — +`dm-pal-prefs.json` only holds `dataDir`. So **every app restart silently +resets API URL, API key, model, embed model, and image server URL to +defaults**. Worse, the wizard-completed flag lives in `localStorage`, so after +restart the DM isn't even re-prompted — they just get a broken connection. + +**Fix (~30 lines):** in `set_llm_config`, `store.set("llmConfig", config)` on +the same `dm-pal-prefs.json` store used for `dataDir` (the store plumbing is +already there in `data_commands.rs`); in `lib.rs` setup, read it back before +`manage(AppState{...})`. The API key lands in plaintext in the OS app-data dir +— same trust level as every other local LLM client; fine for now, note it in +the README's data-locations table. + +### P0-2. PII still in `scripts/apple-signing.env.example` + +`plan.md` §17.1 marks "Scrub PII" as done. It isn't: the file still contains +the real Apple ID (`james.twose2711@gmail.com`) and the real Team ID +(`4759TW4SDC`). 5-minute fix, do it before the next push. + +### P1-1. "Streaming" is a single fake token — the biggest UX lever left + +`generate_stream` does a **non-streaming** call and emits the whole response as +one `Token` (see the comment in `llm_commands.rs`). SessionLogger's "streaming +AI summary" and every generator therefore show 10–20 s of nothing on a local +model. Both transport deps are **already installed**: `reqwest` (`stream` +feature) + `futures-util`. Ollama needs `"stream": true` + NDJSON line +parsing; OpenAI-compatible needs SSE `data:` line parsing. Emit +`LlmEvent::Token` per line; `inject_lore`, busy-balancing, and the frontend +Channel plumbing all stay as-is. SessionLogger's progressive display already +handles multi-token arrival. ~3 h, highest UX-per-line-of-code item in the repo. + +### P1-2. RAG: three real issues in the lore pipeline + +1. **UTF-8 chunk-drop bug** (`rag/mod.rs::chunk`): long paragraphs are split + with `para.as_bytes().chunks(MAX_CHUNK_CHARS)` then + `if let Ok(s) = std::str::from_utf8(chunk)` — when the 1000-byte boundary + lands mid multibyte character (any accented name, CJK, emoji), `from_utf8` + errors and the **whole chunk is silently dropped**. Fix: back the split + index off to a `char_boundary`, e.g. scan back while + `!chunk.is_char_boundary(i)` (~6 lines). Add a test with a long accented + paragraph. (Also: `MAX_CHUNK_CHARS` is actually bytes — rename or note.) +2. **Re-adding a source duplicates every chunk**: `add_document` always + INSERTs. Adding the same file twice doubles its embedding weight and skews + search. Fix: `DELETE FROM chunks WHERE source = ?` before the insert loop + (one line). +3. **Changing `embed_model` silently poisons the index**: old-model vectors + and new-model query vectors are compared by dot product — garbage results. + Fix options: store the embed model per source and warn/refuse on mismatch + in `list_sources`, or a "Reindex all" button in the Lore panel (needs + re-chunk from `text`, which is already stored — feasible). + +### P1-3. No CI, and the self-checks aren't aggregated + +The repo has `oxlint`, `tsc -b`, three `node scripts/check-*.ts` self-checks, +and `cargo test` — **nothing runs them automatically**. Two-part fix: + +1. Add to `package.json`: + `"check": "node scripts/check-dice.ts && node scripts/check-encounter-budget.ts && node scripts/check-worldmap.ts"` +2. A `.gitea/workflows/ci.yml` (release ships via `gitea-release.sh`, so + Gitea, not GitHub) running `npm ci`, `npm run lint`, `npm run build`, + `npm run check`, `cargo test` on push. Even lint-only beats none. + +**Gotcha:** the check scripts are bare `.ts` run via Node type-stripping — +that needs **Node ≥ 23.6** (22.6 with `--experimental-strip-types`), but the +README says "Node.js 20+". Bump the README prerequisite or note it. + +### P1-4. Version drift + stale docs + +- `tauri.conf.json` says `0.1.3`, `Cargo.toml` says `0.1.0`, `package.json` + says `0.1.3`. Pick one source (tauri.conf is what ships) and have + `gitea-release.sh` assert they match. +- `README.md` line 21 still says image generation is "(macOS, via Ollama)" — + the backend moved to stable-diffusion.cpp `sd-server` (cross-platform) and + the rest of the README says so. Fix the one line. +- `plan.md` §4.4 documents the old Ollama image path + "macOS-only" gating as + current design; §10 has the same. The ui-ux doc's image section and + `GeneratedImage` notes reference macOS gating too. One short + "superseded by sd-server (A1111 API)" banner at the top of each stale + section beats rewriting them. +- `public/icons.svg` is referenced by nothing (`favicon.svg` is the real icon) + — delete it, and the stale `dist/` copy with it. + +### P2-1. `csp: null` in `tauri.conf.json` + +All LLM/image HTTP calls happen in Rust (reqwest), so the WebView needs almost +nothing: a minimal CSP like +`default-src 'self'; img-src 'self' data:; style-src 'self' 'unsafe-inline'; connect-src 'self' ipc: http://ipc.localhost` +closes the standard Tauri warning. Verify against dev-mode HMR before +committing (Vite may need `ws:` under `connect-src` in dev). + +### P2-2. `is_ollama()` is URL-substring sniffing + +`url.contains("localhost:11434")` breaks the moment someone runs Ollama on a +custom port or behind a proxy (falls into the OpenAI branch and fails +confusingly). Cheapest robust fix: add a `provider: "ollama" | "openai"` +field to `LlmConfig` defaulted by URL sniff, selectable in Settings presets +(the ui-ux doc already prescribes provider presets). Alternatively probe +`/api/tags` then `/v1/models`. + +### P2-3. Small nits (batch into one cleanup PR) + +- `App.tsx` wizard effect re-checks `localStorage` on **every view change** — + harmless (modal blocks) but a one-time mount check reads cleaner. +- `HistoryView.tsx` is 806 lines — fine today; split only when the next + feature lands there, not before. +- `tokio` is pulled with `features = ["full"]`; only the async-runtime + subset is used. Harmless (Tauri drags tokio in anyway) — trim only if + compile time bothers you. +- Consider `store.get("llmConfig")` versioning (`configVersion: 1` field) so + future shape changes don't brick startup — cheap insurance while touching + P0-1. + +--- + +## 2. Over-engineering audit (ponytail pass) + +**Verdict: lean already, ship.** Specifically checked and *kept*: + +- Hand-rolled base64 encode/decode in `image_commands.rs` — no stdlib + equivalent in Rust; avoids a dep; has a roundtrip test. Correct call. +- `src/lib/bus.ts` (59 lines) — used by 3 components + (Encounter → Initiative/Soundboard push). Not dead. +- `sql_viewer.rs` read-only SQL passthrough — gated power-user tool with + proper read-only enforcement. Justified. +- `zustand` + `framer-motion` — both actually used (Toast/HistoryView); + framer-motion is only in `HistoryView`, but replacing working animation + with CSS is churn, not deletion. + +Only real delete: `public/icons.svg` (see P1-4) and the tracked-vs-reality +doc drift above. Net: ~0 lines to remove; the codebase discipline is good. + +--- + +## 3. Verified status of `plan.md` §17 (what actually shipped) + +Items marked done there that are **confirmed done in code**: README rewrite, +Greet deletion (marker comment in `lib.rs`), nav-map consolidation +(`TOOL_META` in `App.tsx`), dice/encounter/worldmap self-checks, lore +directory picker (`rag_add_directory` is in the invoke handler), first-run +wizard, `?` shortcut help, global busy indicator (`emit_busy` + +`GeneratingIndicator`), and the sd-server migration (cross-platform image gen, +macOS gate gone from the components). + +Still genuinely open from §17/§ui-ux (re-verified): Gitea CI, code signing + +notarization, auto-updater, quest branching graph, world hierarchy tree, real +ambience packs + sound import, campaign namespacing, image-gen into World +Builder (battle maps), handout renderer. Don't re-plan those here — this doc +just re-ranks them against the fresh findings. + +--- + +## 4. Next steps — one ranked list + +Merges fresh findings with the still-open roadmap items. Do them roughly in +order; #1–5 are one focused week. + +| # | Item | Effort | Impact | Tracked in | +|---|------|--------|--------|------------| +| 1 | Scrub PII from `apple-signing.env.example` | 5 min | High (before next push) | here | +| 2 | Persist `LlmConfig` to prefs store (P0-1) | 1 h | **Critical** | here | +| 3 | RAG: UTF-8 boundary fix + dedupe on re-add (P1-2.1–2) | 2 h | High | here | +| 4 | `npm run check` + Gitea Actions CI (P1-3) | 2 h | High | §17.2 | +| 5 | Real token streaming from Ollama/OpenAI (P1-1) | 3 h | **High** | here | +| 6 | Doc drift cleanup: README line 21, plan §4.4/§10 banners, version sync, delete `icons.svg`, Node ≥ 23.6 note (P1-4) | 1 h | Medium | here | +| 7 | Minimal CSP (P2-1) | 30 min | Medium | here | +| 8 | Provider select instead of URL sniff (P2-2) | 1 h | Medium | here | +| 9 | RAG embed-model mismatch warning / Reindex (P1-2.3) | 2 h | Medium | here | +| 10 | Code signing + notarization (env example + script already exist) | 3 h | High (distribution) | §17.2 | +| 11 | Campaign namespacing (`//`, title-bar selector) | 4 h | High (DMs run 2+ campaigns) | §17.4 | +| 12 | Quest branching graph | 4 h | High | §17.3 | +| 13 | World hierarchy tree (drill into region) | 3 h | High | §17.3 | +| 14 | Auto-updater (`tauri-plugin-updater`) | 3 h | Medium | §17.2 | +| 15 | Image-gen into World Builder maps + Encounter battle maps | 3 h | Medium | §16.10 | +| 16 | Real ambience packs (CC0) + custom sound import | 2 h | Medium | §17.3 | +| 17 | Handout renderer (Markdown → PDF) | 3 h | Low–Med | §15 | + +**Skipped deliberately:** i18n, player-facing web view, compendium/SRD stat +blocks, voice-to-text, draggable bento, session replay — all P3 in §17.4 and +correctly deferred. YAGNI until a real user asks. + +--- + +## 5. How to verify the P0 fix in 60 seconds + +1. `npm run tauri dev`, set a non-default model in Settings, quit, relaunch. +2. Settings shows the saved model (today: it shows `llama3.2` default). +3. `cat $APPDATA/../dm-pal-prefs.json`-equivalent (the prefs store next to + `dm-toolkit/`) now has an `llmConfig` key. \ No newline at end of file diff --git a/docs/roadmap.md b/docs/roadmap.md new file mode 100644 index 0000000..e68b3a4 --- /dev/null +++ b/docs/roadmap.md @@ -0,0 +1,119 @@ +# DM-Pal — Master Roadmap (every known gap) + +*Consolidates `docs/repo-review.md` (2026-09-06), the open items in +`plan.md` §15–17, the unchecked P2/P3 items in `ui-ux-improvements.md`, and +new feature proposals into one ordered plan. This is now the working list; +`plan.md` remains design history. Every item is verified as **not shipped** +as of 2026-09-06.* + +Source tags: **[R]** repo-review · **[P]** plan.md §17/§15 · +**[U]** ui-ux-improvements P2/P3 · **[N]** new proposal. + +--- + +## Phase 0 — Fix & harden → tag `v0.1.4` (~1.5 days) + +| # | Item | Effort | Notes | +|---|------|--------|-------| +| 0.1 | Scrub PII from `scripts/apple-signing.env.example` | 5 m | [R] real email + Team ID still present | +| 0.2 | Persist `LlmConfig` to prefs store; load on startup | 1 h | [R] **P0** — restart currently wipes Settings. Add a `configVersion` field for future shape changes | +| 0.3 | RAG: UTF-8 boundary fix in `chunk()` + test with accented text | 1 h | [R] multibyte paragraphs silently drop chunks today | +| 0.4 | RAG: `DELETE WHERE source = ?` before re-insert in `add_document` | 15 m | [R] re-adding a file doubles its chunks | +| 0.5 | RAG: embed-model mismatch guard (store model per source; refuse/warn) + "Reindex all" | 2 h | [R] changing embed model currently poisons cosine scores | +| 0.6 | `npm run check` + `.gitea/workflows/ci.yml` (lint, tsc, checks, `cargo test`) | 2 h | [R][P] | +| 0.7 | Doc drift: README line 21 ("macOS, via Ollama"), Node ≥ 23.6 prerequisite, version sync (Cargo vs tauri.conf), banner on stale plan.md §4.4/§10, delete `public/icons.svg` | 1 h | [R] | +| 0.8 | Minimal CSP in `tauri.conf.json` (self + `data:` img; check dev HMR) | 30 m | [R] | +| 0.9 | Replace `is_ollama()` URL sniff with `provider` field in `LlmConfig` (wizard presets set it) | 1.5 h | [R] breaks on custom Ollama ports | + +**Done when:** settings survive restart; CI green on push; re-indexed lore +returns sane results; no PII in the repo. + +## Phase 1 — Core UX enablers → tag `v0.2.0` (~1 week) + +| # | Item | Effort | Notes | +|---|------|--------|-------| +| 1.1 | Real token streaming: Ollama `stream:true` NDJSON + OpenAI SSE via `reqwest::bytes_stream` (dep already present) | 3 h | [R][N] unlocks visible generation everywhere; SessionLogger already renders progressive tokens | +| 1.2 | Model pull in first-run wizard: `POST /api/pull` NDJSON → Channel progress bar (same shape as image-gen poller) | 3 h | [N] wizard currently "hopes it exists" | +| 1.3 | `ragQuery` on Encounter, Quest, Session summary | 30 m | [N] NPC/Item/World already pass it | +| 1.4 | Auto-log session events: bus emits for dice rolls, initiative round/turn changes, encounter start → SessionLogger appends | 3 h | [N] makes the AI summary summarize the actual fight | +| 1.5 | NPC → Initiative push ("⚔ Add to tracker" on NPC card via `AddCombatants` bus event; stats already generated) | 30 m | [N] | +| 1.6 | Calendar "Next day" → session-log entry incl. rolled weather | 1 h | [N] ties calendar into the timeline | +| 1.7 | Dice macros: persisted named rolls (`usePersistentState`), button row + edit sheet | 2 h | [U P3] | +| 1.8 | Initiative condition durations (auto-decrement per round, expire) | 2 h | [U-adjacent] concentration/Hex is table-stakes | + +**Done when:** a brand-new user pulls a model in-wizard and watches it +progress; a combat logs itself; the AI summary streams and is grounded. + +## Phase 2 — Flagship features → tag `v0.3.0` (~2 weeks) + +| # | Item | Effort | Notes | +|---|------|--------|-------| +| 2.1 | **Ask the Oracle**: chat over the world bible — question → `rag_search` → `generate` → answer with source chips. New view, no backend change | 1 d | [N] the flagship for the RAG pipeline | +| 2.2 | Quest branching graph (`react-flow`): conditional edges, step status colors | 1 d | [P][U] | +| 2.3 | World hierarchy tree: continent → region → city; drill into a region to generate sub-regions | 1 d | [P][U] | +| 2.4 | NPC roster view: grid of generated NPCs w/ portraits, filter by race/alignment, click to open | 1 d | [U] reads generations.db | +| 2.5 | Item inventory view (rarity-coded grid) | 0.5 d | [U] | +| 2.6 | Quest roster view (status badges) | 0.5 d | [U] | +| 2.7 | Campaign namespacing: `//` + title-bar selector + migration of existing data; generalize the existing data-dir machinery | 1 d | [P] biggest data-model gap | +| 2.8 | Campaign backup/restore: `export_campaign` zip command (Rust `zip` crate) + import w/ confirm | 0.5 d | [N] no backup story today | +| 2.9 | Save portrait / right-click save image | 1 h | [U] | + +**Done when:** two campaigns coexist with separate lore/history; the cast +and inventory are browsable surfaces, not just history rows. + +## Phase 3 — Media & atmosphere → tag `v0.4.0` (~1.5 weeks) + +| # | Item | Effort | Notes | +|---|------|--------|-------| +| 3.1 | Real ambience packs: 2–3 CC0 loops in `public/sounds/` (synthesis stays as fallback) + custom sound import (drag MP3 onto tile) | 1 d | [P][U] | +| 3.2 | Image advanced params: negative prompt, seed, aspect ratio, steps, guidance → sd-server passthrough + cache key includes them | 0.5 d | [U] | +| 3.3 | Image-gen into World Builder (region map art per landmark) | 0.5 d | [P] §16.10 | +| 3.4 | Image-gen into Encounter (battle map tile + loot item art) | 0.5 d | [P] §16.10 | +| 3.5 | Handout renderer: Markdown → styled handout → export PNG/PDF | 1 d | [P][U] last §15 M6 item | +| 3.6 | Dice tumble animation (CSS 2.5D — no three.js) | 0.5 d | [N] | + +**Done when:** a full session can run without leaving the app: ambience, +maps, item art, handouts. + +## Phase 4 — Distribution → tag `v1.0.0` (~1 week) + +| # | Item | Effort | Notes | +|---|------|--------|-------| +| 4.1 | Cross-platform build matrix in Gitea CI (macOS/Windows/Linux) | 0.5 d | [P] | +| 4.2 | Finish code signing + notarization (macOS cert exists in `secrets/`; add Windows) | 1 d | [P] scripts + env already scaffolded | +| 4.3 | Auto-updater: `tauri-plugin-updater` + update manifest hosted beside Gitea releases | 1 d | [P] | +| 4.4 | i18n scaffolding: extract strings to `t()` (EN-only content for v1) | 1 d | [U] structure now, translations later | +| 4.5 | Final polish pass: glassmorphism consistency, `prefers-reduced-motion`, focus rings | 0.5 d | [P] §15 M6 | +| 4.6 | Tag `v1.0.0`, update README screenshots | 30 m | | + +**Done when:** a DM who downloads v1.0 gets a signed app that updates itself. + +## Phase 5 — Post-1.0 long tail (ordered by value) + +| # | Item | Effort | Notes | +|---|------|--------|-------| +| 5.1 | Compendium: local SRD 5e JSON → stat-block popovers in Encounter + Initiative import | 2 d | [U] | +| 5.2 | Player view: LAN read-only web page (initiative, dice, HP) from the DM's screen | 2–3 d | [U][P] | +| 5.3 | Console mode dashboard: embed 2–3 chosen tools as live-session cards | 1 d | [U] | +| 5.4 | Draggable bento (`react-grid-layout`), persisted per campaign | 1 d | [U] | +| 5.5 | Voice-to-text session notes (whisper.cpp sidecar or audio-capable model) | 2 d | [P][U] | +| 5.6 | Session replay: record the bus event stream → timeline replay | 2 d | [U] builds on 1.4's event logging | +| 5.7 | AI auto-tagging of session entries (#combat/#loot/#roleplay) | 0.5 d | [N] | +| 5.8 | Lore embedding visualization (t-SNE scatter) | 1 d | [N] | +| 5.9 | True 3D dice (react-three-fiber) — only if 3.6's CSS tumble proves insufficient | 2 d | [N] | +| 5.10 | Plugin system: dynamically registered commands/content packs | 3 d+ | [P] | +| 5.11 | Prompt library + fine-tuning on campaign data | research | [P] | + +--- + +## Ordering notes + +- **0.2 before 1.2** — the wizard can't save what it configures until config persists. +- **1.1 before 2.1** — the Oracle wants streaming; both ride the same Channel plumbing. +- **2.7 before 5.x** — campaign namespacing changes the data layout; rosters, + backup, and replay should key on campaign from day one. +- **1.4 before 5.6** — replay is just a recording of the event stream 1.4 introduces. +- Rust tests + self-checks grow with each phase; CI (0.6) runs them all from day one. + +**Totals to v1.0:** ~22 working days of focused effort +(1.5 + 5 + 6.5 + 5 + 4). Long tail adds ~15 days beyond that. \ No newline at end of file