ttrpg-tools/docs/variable-store-plan.md

Variable store — outstanding fixes

Notes from an audit of the client-side variable / tag reactivity system:

• src/components/journal/var-reactivity.ts — engine (dependency graph, base/mod separation, threshold-gated tag activation, cascade).
• src/components/journal/variable-expression.ts — expression evaluator.
• src/components/journal/command-dispatcher.ts — /set, /rolltag, cascade.
• src/components/journal/completions.ts — init + seed.
• src/components/stores/journalStream.ts — replay + hydration rebuild.
• src/components/journal/types/var.tsx — var message + reducer.

Already fixed (context)

Commit a26d53c (fix(journal): keep variable bases in the stream):

• Hydration double-counted active tag modifiers because the stream carried only combined values. var messages now carry base alongside value, the store keeps variableBases, and rebuildReactivityFromStore restores exact bases.
• Seeding ran at import against an empty store (clobbering persisted values and appending duplicate var messages on every load). It now runs after hydration and only for keys absent from the store.

The items below are still open.

Open issues

1. Modifier and declaration errors are swallowed — ✅ fixed

• Where: computeInitialValues (catch {}, "skip failed evaluations at init time") and applyTagMapActivations (catch {}, "skip failed modifier") in var-reactivity.ts; initReactivity throws are reduced to console.warn in completions.ts.
• Symptom: a typo'd declaration or modifier expression silently yields a wrong value (often 0) with no author-facing feedback.
• Fixed: the engine now collects ReactivityDiagnostic entries for failed declarations (computeInitialValues, reevaluateDependents), failed modifier expressions (applyTagMapActivations), and circular declarations (initReactivity). They are deduped, logged via console.warn with key/expression/tag context, cleared on each initReactivity, and bridged into a signal exposed as useJournalCompletions().diagnostics(). VariableView 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 — ✅ 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.
• 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 — ✅ fixed

• Where: bootstrapReactivity in completions.ts runs initReactivity once; nothing re-runs it when declare blocks change or a session/document is switched.
• Also: DispatchContext.declarations / tagModifiers (command-dispatcher.ts) are declared but never read — a vestige of a per-dispatch init that was never wired up.
• Symptom: editing role=declare content requires a full reload; stale graph/expressions persist.
• 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 — ✅ fixed

• Where: EvalContext.lookup documents "Resolve $var", but parseFactor calls ctx.lookup(varName.slice(1)) (bare name), and every caller re-adds $.
• Symptom: a caller following the doc would key variables with $ and silently get 0s.
• Fixed: EvalContext.lookup now receives the full $var name (including $), consistent with every other variable API (getCombined, setBase, getMods, resolveVariable). Callers no longer add or strip $.

5. Undefined variables evaluate to 0 — ✅ fixed (strict for commands)

• Where: parseFactor (variable-expression.ts) returns a numeric 0 for an unresolved $var.
• Symptom: /set $hp $cn*5 (typo) silently sets 0 instead of failing.
• Fixed: parseFactor now throws Unknown variable: $x whenever lookup returns undefined, and resolveVariable() distinguishes unknown from a real 0. /set and /roll use the strict resolver, so typos fail loudly via dispatchError. Declarations and tag modifiers stay lenient on purpose: they resolve unknown references to 0 through getCombined, because the documented pattern ($hp = $con*5+$mod_hp) relies on external/undeclared variables defaulting to 0.

6. Tokenizer accepts malformed numbers — ✅ fixed

• Where: the number branch of tokenize consumes [0-9.] greedily, so 1.2.3 becomes one "number" and parseFloat truncates it.
• Fixed: the tokenizer now validates the scanned chunk against ^\d+(\.\d+)?$ and throws Invalid number literal: "…" instead of pushing a silently-truncated value. The error propagates through the diagnostics channel (#1) for declarations/modifiers and through dispatchError for /set and /roll.

7. topoSortAffected re-evaluates untouched declarations — low

• Where: in var-reactivity.ts, the visit condition affected.has(dep) || declExprs.has(dep) pulls declared dependencies that are not in affected into the result.
• Symptom: reevaluateDependents re-evaluates and re-publishes variables that did not change. Mostly harmless, but noisy.
• Fix: only visit/topologically order keys in affected.

8. Failed initReactivity leaves partial state — ✅ fixed

• Where: initReactivity populates depGraph / declExprs / tagModMap before checkCircular() throws; inFlight is never cleared on re-init.
• 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

• Base fallback: rebuildReactivityFromStore falls back to the combined value when a message has no base, which double-counts mods. In-repo emitters always send base; only hand-crafted/external var messages hit this.
• Tagmap targets: rebuild derives activations from each source's base tagmap (this fixed a feedback loop where a tagmap-valued target re-triggered its own modifier on every rebuild). If a variable's effective count crosses a threshold purely via mods, that activation is not reconstructed on hydration. Revisit if tagmap-valued modifier targets come into use.

Suggested order

1. #3 (re-init) and #2 (del) — correctness of engine lifecycle.
2. #1 (diagnostics) — authoring feedback.
3. #4–#8 — cleanup and hardening.
4. #9 — only if the tagmap-target pattern is adopted.