fix(journal): re-init reactivity and drop var del
Make the reactivity engine re-initialize when the loaded declarations change instead of only at import. ensureReactivity() reads the live completions signal, compares a signature, and rebuilds from the store (with exact bases) only when needed; dispatchCommand calls it before each dispatch. Remove the unused DispatchContext.declarations/tagModifiers fields, which were mount-time snapshots and could not be trusted. Build initReactivity's maps into locals and commit only after the circular check passes, so a failed re-init leaves the previous engine intact; clear inFlight on commit. Drop the unreachable var "del" action from the schema, render, and reducer.
This commit is contained in:
@@ -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
|
||||
|
||||
|
||||
@@ -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", () => {
|
||||
|
||||
@@ -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);
|
||||
|
||||
@@ -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.
|
||||
|
||||
@@ -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<string, string>;
|
||||
/** 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 });
|
||||
|
||||
@@ -150,23 +150,59 @@ async function scanClientSide(): Promise<JournalCompletions> {
|
||||
// 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<void> = (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<void> = (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" });
|
||||
}
|
||||
|
||||
@@ -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";
|
||||
|
||||
@@ -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<VarPayload>({
|
||||
schema,
|
||||
defaultPayload: () => ({ action: "set", key: "", value: "" }),
|
||||
render: (p) => {
|
||||
if (p.action === "del") {
|
||||
return (
|
||||
<div class="flex items-center gap-1.5">
|
||||
<span class="text-lg">🔢</span>
|
||||
<span class="font-mono text-sm text-gray-500 line-through">
|
||||
{p.key}
|
||||
</span>
|
||||
</div>
|
||||
);
|
||||
}
|
||||
return (
|
||||
<div class="flex items-center gap-1.5">
|
||||
<span class="text-lg">🔢</span>
|
||||
@@ -55,10 +45,7 @@ registerMessageType<VarPayload>({
|
||||
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;
|
||||
|
||||
@@ -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<string, string>();
|
||||
const nextDepGraph = new Map<string, Set<string>>();
|
||||
|
||||
// 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>): string[] {
|
||||
return result;
|
||||
}
|
||||
|
||||
function checkCircular(): void {
|
||||
if (!declExprs) return;
|
||||
|
||||
const allKeys = [...declExprs.keys()];
|
||||
function checkCircular(exprs: Map<string, string>): void {
|
||||
const allKeys = [...exprs.keys()];
|
||||
const state = new Map<string, "unvisited" | "visiting" | "visited">();
|
||||
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);
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user