diff --git a/docs/variable-store-plan.md b/docs/variable-store-plan.md index d5dc978..f35f0b1 100644 --- a/docs/variable-store-plan.md +++ b/docs/variable-store-plan.md @@ -42,17 +42,19 @@ The items below are still open. renders them as a warning banner. A bad declaration no longer aborts the rest of a cascade. -### 2. `var del` does not reach the engine — medium +### 2. `var del` does not reach the engine — ✅ fixed (dropped) - **Where:** reducer in `types/var.tsx` supports `action: "del"`; the parser (`command-parser.ts`) has no `/del`; `var-reactivity.ts` never clears `baseValues` and `rebuildReactivityFromStore` never removes stale keys. - **Symptom:** a deleted variable still resolves locally via `getCombined`, because it reads `baseValues` first. -- **Fix:** either wire a `/del` command and clear engine state (base + mods) on - delete, or drop `del` from the schema/reducer if it is intentionally unused. +- **Fixed:** `/del` was never reachable (no parser emits it, and replay does not + validate), so the `del` action was removed from the `var` schema, render, and + reducer. The wire type is now `action: "set"` only; a legacy `del` message in + an old stream is a no-op on replay rather than resurrecting a deleted key. -### 3. Reactivity is initialized once, at import — medium +### 3. Reactivity is initialized once, at import — ✅ fixed - **Where:** `bootstrapReactivity` in `completions.ts` runs `initReactivity` once; nothing re-runs it when declare blocks change or a session/document is @@ -62,8 +64,16 @@ The items below are still open. per-dispatch init that was never wired up. - **Symptom:** editing `role=declare` content requires a full reload; stale graph/expressions persist. -- **Fix:** expose a re-init entry point keyed to content/session changes, and - either use or remove the unused context fields. +- **Fixed:** `bootstrapReactivity` is now `applyReactivity` behind an idempotent + `ensureReactivity()` that reads the live completions signal, compares a + signature, and re-initializes only when the declaration set changes. + `dispatchCommand` calls it before every dispatch, so a content rescan takes + effect on the next command without a reload. The unused + `DispatchContext.declarations` / `tagModifiers` fields were removed (they were + mount-time snapshots and could not be trusted). Session switches were already + covered by the hydration rebuild, which now also passes + `journalStreamState.variableBases` so bases are restored exactly instead of + falling back to combined values. ### 4. `lookup` contract disagrees with its docs — low @@ -102,12 +112,14 @@ The items below are still open. that did not change. Mostly harmless, but noisy. - **Fix:** only visit/topologically order keys in `affected`. -### 8. Failed `initReactivity` leaves partial state — low +### 8. Failed `initReactivity` leaves partial state — ✅ fixed - **Where:** `initReactivity` populates `depGraph` / `declExprs` / `tagModMap` before `checkCircular()` throws; `inFlight` is never cleared on re-init. -- **Fix:** build into locals and commit on success, or roll back on failure. - Clear `inFlight` in `initReactivity`. +- **Fixed:** `initReactivity` now builds the declaration map, dependency graph, + and tag-modifier map into locals and commits them only after `checkCircular` + passes, so a failed (re-)init leaves the previous engine intact. Runtime state + (`baseValues`, mods, `sourceActivations`) and `inFlight` are reset on commit. ### 9. Residual base / tagmap-target edges — low diff --git a/src/cli/completions/variable-system.test.ts b/src/cli/completions/variable-system.test.ts index 2575aac..4de48b8 100644 --- a/src/cli/completions/variable-system.test.ts +++ b/src/cli/completions/variable-system.test.ts @@ -517,6 +517,33 @@ describe("var-reactivity", () => { }), ).not.toThrow(); }); + + test("keeps the previous configuration when a re-init fails", () => { + const warn = jest.spyOn(console, "warn").mockImplementation(() => {}); + initReactivity({ + declarations: [{ key: "$hp", expression: "10 + 5" }], + tagModifiers: [], + }); + expect(computeInitialValues(store()).find((r) => r.key === "$hp")?.value).toBe( + "15", + ); + + expect(() => + initReactivity({ + declarations: [ + { key: "$a", expression: "$b + 1" }, + { key: "$b", expression: "$a + 1" }, + ], + tagModifiers: [], + }), + ).toThrow(); + + // The failed re-init did not clobber the working configuration. + const results = computeInitialValues(store()); + expect(results.find((r) => r.key === "$hp")?.value).toBe("15"); + expect(results.find((r) => r.key === "$a")).toBeUndefined(); + warn.mockRestore(); + }); }); describe("computeInitialValues", () => { diff --git a/src/components/CommandLinkManager.tsx b/src/components/CommandLinkManager.tsx index de6ac56..c09d097 100644 --- a/src/components/CommandLinkManager.tsx +++ b/src/components/CommandLinkManager.tsx @@ -33,8 +33,6 @@ export const CommandLinkManager: Component = () => { command, sparkTables: comp.data.sparkTables, variables: stream.variables, - declarations: comp.data.declarations, - tagModifiers: comp.data.tagModifiers, }).then((result) => { if (!result.ok) { setDispatchError(result.error); diff --git a/src/components/journal/JournalInput.tsx b/src/components/journal/JournalInput.tsx index 25b1cef..7f48c63 100644 --- a/src/components/journal/JournalInput.tsx +++ b/src/components/journal/JournalInput.tsx @@ -109,8 +109,6 @@ export const JournalInput: Component = () => { command: raw, sparkTables: comp.data.sparkTables, variables: stream.variables, - declarations: comp.data.declarations, - tagModifiers: comp.data.tagModifiers, }); // dispatchCommand already set the shared error if it failed. diff --git a/src/components/journal/command-dispatcher.ts b/src/components/journal/command-dispatcher.ts index 6eb1a58..d99d4b1 100644 --- a/src/components/journal/command-dispatcher.ts +++ b/src/components/journal/command-dispatcher.ts @@ -12,7 +12,7 @@ import { resolveRollPayload } from "./types/roll"; import { resolveSparkPayload } from "./types/spark"; import { evaluateExpression, exprValueToString } from "./variable-expression"; import { computeCascade, getCombined, setBase } from "./var-reactivity"; -import type { VarDeclaration, TagModifier } from "./declare-parser"; +import { ensureReactivity } from "./completions"; // Tagmap pattern: "#warrior:1;#druid:2". Also accepts bare "#tag" as shorthand. const BARE_TAG_PATTERN = /^#[a-zA-Z_][a-zA-Z0-9_]*$/; @@ -58,10 +58,6 @@ export interface DispatchContext { }[]; /** Current runtime variable values */ variables: Record; - /** Variable declarations (from role=declare blocks) */ - declarations: VarDeclaration[]; - /** Tag modifiers (from role=declare blocks) */ - tagModifiers: TagModifier[]; } /** @@ -74,6 +70,10 @@ export async function dispatchCommand( const raw = ctx.command.trim(); if (!raw) return { ok: false, error: "Empty command" }; + // Make sure the engine reflects the currently loaded declarations; this + // re-initializes only when they changed since the last dispatch. + ensureReactivity(); + // Observers can only send chat if (ctx.role === "observer") { const result = sendMessage("chat", { text: raw }); diff --git a/src/components/journal/completions.ts b/src/components/journal/completions.ts index c71c126..91f08dc 100644 --- a/src/components/journal/completions.ts +++ b/src/components/journal/completions.ts @@ -150,23 +150,59 @@ async function scanClientSide(): Promise { // may land first, so seeding is driven by whichever becomes ready last. let _reactivityReady = false; +/** Signature of the declarations/tag modifiers last applied to the engine. */ +let _reactivitySignature: string | null = null; + +function reactivitySignature( + declarations: VarDeclaration[], + tagModifiers: TagModifier[], +): string { + return JSON.stringify({ declarations, tagModifiers }); +} + /** - * Initialize the reactivity engine from the loaded declarations. If the store - * has already been hydrated, rebuild from it and seed missing values now. + * Initialize the reactivity engine from the given declarations, then (if the + * store is hydrated) rebuild it from the store and seed missing values. + * Rebuilding uses both the combined values and their exact bases so tag + * modifiers are not double-counted. */ -function bootstrapReactivity( +function applyReactivity( declarations: VarDeclaration[], tagModifiers: TagModifier[], ): void { initReactivity({ declarations, tagModifiers }); _reactivityReady = true; + _reactivitySignature = reactivitySignature(declarations, tagModifiers); if (hasHydrated()) { - rebuildReactivityFromStore(journalStreamState.variables); + rebuildReactivityFromStore( + journalStreamState.variables, + journalStreamState.variableBases, + ); seedDeclaredVariables(); } } +/** + * Ensure the engine matches the currently loaded completions, re-initializing + * only when the declaration set has changed since the last apply. No-op while + * completions are still loading. Called at bootstrap and before each dispatch, + * so a content rescan takes effect without a full reload. + */ +export function ensureReactivity(): void { + const s = completionsState(); + if (s.status !== "loaded") return; + + const signature = reactivitySignature(s.data.declarations, s.data.tagModifiers); + if (_reactivityReady && _reactivitySignature === signature) return; + + try { + applyReactivity(s.data.declarations, s.data.tagModifiers); + } catch (e) { + console.warn("[completions] reactivity init error:", e); + } +} + // Seeding runs after every hydration snapshot so declared variables are // computed against the real session store rather than an empty one. setHydrationListener(() => { @@ -187,11 +223,7 @@ const _initPromise: Promise = (async () => { // spark-table CSVs are resolved through this registry at roll time. await tryServerRegistry(); setCompletionsState({ status: "loaded", data: serverData }); - try { - bootstrapReactivity(serverData.declarations, serverData.tagModifiers); - } catch (e) { - console.warn("[completions] reactivity init error:", e); - } + ensureReactivity(); return; } @@ -200,11 +232,7 @@ const _initPromise: Promise = (async () => { const data = await scanClientSide(); if (data.dice.length > 0 || data.links.length > 0) { setCompletionsState({ status: "loaded", data }); - try { - bootstrapReactivity(data.declarations, data.tagModifiers); - } catch (e) { - console.warn("[completions] reactivity init error:", e); - } + ensureReactivity(); } else { setCompletionsState({ status: "empty" }); } diff --git a/src/components/journal/index.ts b/src/components/journal/index.ts index 1a4eb02..a37c7e3 100644 --- a/src/components/journal/index.ts +++ b/src/components/journal/index.ts @@ -48,7 +48,7 @@ export { StreamMessageCard } from "./StreamMessage"; export { ComposePanel } from "./ComposePanel"; export { JournalInput } from "./JournalInput"; export { DynamicForm } from "./DynamicForm"; -export { useJournalCompletions } from "./completions"; +export { useJournalCompletions, ensureReactivity } from "./completions"; export { dispatchCommand } from "./command-dispatcher"; export type { DispatchContext, DispatchResult } from "./command-dispatcher"; export { parseInput } from "./command-parser"; diff --git a/src/components/journal/types/var.tsx b/src/components/journal/types/var.tsx index 1371f5e..8160326 100644 --- a/src/components/journal/types/var.tsx +++ b/src/components/journal/types/var.tsx @@ -5,8 +5,8 @@ * Command: /set $key expression * * Variable declarations and tag modifiers are authored in ```csv role=declare - * code blocks in markdown documents. The stream carries set/del mutations - * that build up the runtime variable store additively. + * code blocks in markdown documents. The stream carries `set` mutations that + * build up the runtime variable store additively. */ import { z } from "zod"; @@ -15,7 +15,7 @@ import { journalSetState } from "../../stores/journalStream"; import { produce } from "solid-js/store"; const schema = z.object({ - action: z.enum(["set", "del"]), + action: z.literal("set"), key: z.string().min(1), value: z.string().optional(), /** The raw base value (before tag modifiers). The store's `variables` map @@ -33,16 +33,6 @@ registerMessageType({ schema, defaultPayload: () => ({ action: "set", key: "", value: "" }), render: (p) => { - if (p.action === "del") { - return ( -
- 🔢 - - {p.key} - -
- ); - } return (
🔢 @@ -55,10 +45,7 @@ registerMessageType({ reducer: (p) => { journalSetState( produce((s) => { - if (p.action === "del") { - delete s.variables[p.key]; - delete s.variableBases[p.key]; - } else if (p.value !== undefined) { + if (p.value !== undefined) { s.variables[p.key] = p.value; if (p.base !== undefined) { s.variableBases[p.key] = p.base; diff --git a/src/components/journal/var-reactivity.ts b/src/components/journal/var-reactivity.ts index 1f3ebf6..1efa101 100644 --- a/src/components/journal/var-reactivity.ts +++ b/src/components/journal/var-reactivity.ts @@ -216,44 +216,46 @@ export function onReactivityDiagnostics( * Initialize (or re-initialize) the reactivity engine from declarations * and tag modifiers parsed from role=declare blocks. * + * The new state is built into locals and only committed once validation + * passes, so a throw (e.g. a circular dependency) leaves the previously + * initialized engine intact rather than half-populated. + * * Throws if a circular dependency is detected. */ export function initReactivity(state: VarReactivityState): void { - depGraph = new Map(); - declExprs = new Map(); - tagModMap = new Map(); - baseValues.clear(); - numericMods.clear(); - tagMapMods.clear(); - sourceActivations.clear(); - clearReactivityDiagnostics(); + const nextTagModMap = new Map< + string, + Array<{ target: string; expression: string; threshold: number }> + >(); + const nextDeclExprs = new Map(); + const nextDepGraph = new Map>(); // Index tag modifiers for (const tm of state.tagModifiers) { - let list = tagModMap.get(tm.tag); + let list = nextTagModMap.get(tm.tag); if (!list) { list = []; - tagModMap.set(tm.tag, list); + nextTagModMap.set(tm.tag, list); } list.push({ target: tm.target, expression: tm.expression, threshold: tm.threshold }); } // Index declarations and build dependency graph for (const decl of state.declarations) { - declExprs.set(decl.key, decl.expression); + nextDeclExprs.set(decl.key, decl.expression); const deps = extractDependencies(decl.expression); for (const dep of deps) { - let dependents = depGraph.get(dep); + let dependents = nextDepGraph.get(dep); if (!dependents) { dependents = new Set(); - depGraph.set(dep, dependents); + nextDepGraph.set(dep, dependents); } dependents.add(decl.key); } } try { - checkCircular(); + checkCircular(nextDeclExprs); } catch (e) { reportDiagnostic({ kind: "init", @@ -261,6 +263,17 @@ export function initReactivity(state: VarReactivityState): void { }); throw e; } + + // Validation passed — commit the new configuration and reset runtime state. + depGraph = nextDepGraph; + declExprs = nextDeclExprs; + tagModMap = nextTagModMap; + baseValues.clear(); + numericMods.clear(); + tagMapMods.clear(); + sourceActivations.clear(); + inFlight.clear(); + clearReactivityDiagnostics(); } /** Extract $var names from an expression string. */ @@ -789,10 +802,8 @@ function topoSortAffected(affected: Set): string[] { return result; } -function checkCircular(): void { - if (!declExprs) return; - - const allKeys = [...declExprs.keys()]; +function checkCircular(exprs: Map): void { + const allKeys = [...exprs.keys()]; const state = new Map(); for (const k of allKeys) state.set(k, "unvisited"); @@ -812,11 +823,11 @@ function checkCircular(): void { state.set(key, "visiting"); path.push(key); - const expr = declExprs!.get(key); + const expr = exprs.get(key); if (expr) { const deps = extractDependencies(expr); for (const dep of deps) { - if (declExprs!.has(dep)) { + if (exprs.has(dep)) { dfs(dep); } }