dm-pal/docs/repo-review.md

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<LlmConfig>; 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 email and Team ID (and both docs quote them). 5-minute fix, do it before the next push. Note: the values are already in git history from earlier commits — scrubbing stops future copies, not the past.

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 (<dataDir>/<campaign>/, 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.