Skip to content

Commit 9359a12

Browse files
W9 — Variables and Consequences: typed set/increment/decrement, clamp-once (#41)
* W9 — Variables and Consequences: typed set/increment/decrement, clamp-once Opens src/engine/src/kinds/story-graph/ — the first kind-specific module, exactly where eslint.config.js's dependency-arrow rule already expected it. - variables.ts: VarType/VariableDecl/VariableSchema/VarValue/Consequence (03 §2, §5); buildInitialVariables seeds runtime variables from a schema's declared initial values, walking keys sorted rather than in declaration order; applyConsequences runs a transition's typed effects against declared variables, accumulating raw values and clamping ints exactly once at the end so a +5 then -5 nets to zero instead of clipping at a bound it only transiently crossed. - Undeclared-variable and mistyped writes throw rather than returning a ValidationError — 03 §11 assigns that check to Tier 1 load-time validation (W14, which doesn't depend on W9), so this guard is the runtime backstop for content that should be structurally unreachable, the same precedent weightedPick already sets. - Returns one coalesced StateChange per touched variable (not per op), sorted by variable name, carrying previous/final value and mirroring the variable's own visible declaration. The StateChange shape (path/op/reason) isn't pinned by 04-core.md/03-story-graph-kind.md — same category of gap as W8's achievement-unlock convention — so it's recorded in TODO.md's Known Open Items pending a real caller (W11) and formal codification. Full reasoning in plans/16-w9-variables-and-consequences.md. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * Address Qodo review: finite-int guards and null-prototype variable maps - increment/decrement now validate both the running value and by are finite integers before doing arithmetic, so corrupted kindState or a bad delta can't produce a NaN/Infinity that later crashes canonical serialization. buildInitialVariables validates decl.initial the same way a set consequence would, so a malformed schema can't seed invalid runtime state either. - variables/next are built as null-prototype objects (Object.create(null)) and requireDecl uses Object.hasOwn, so a schema-declared variable literally named __proto__ can't have its writes silently swallowed by the prototype accessor, and an inherited Object.prototype member name can't be misread as a declared variable. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
1 parent ec34508 commit 9359a12

4 files changed

Lines changed: 617 additions & 0 deletions

File tree

docs/docs/engine/TODO.md

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -479,6 +479,16 @@ in [SubZeroDev.SunTrap](https://github.com/The-Running-Dev/SubZeroDev.SunTrap) (
479479
kind has implemented it yet to check the two sides against each other. Needs formal
480480
codification in `04-core.md` §12 once `story-graph` (W9–14) or `simulation` actually
481481
emits one. See `plans/15-w8-profile-store.md`, Decision 1.
482+
- [ ] **The consequence-applied `StateChange` shape is also an invented convention.** 03
483+
§5/§8.4 describe the `consequence.applied` *event* (`data: variable, op, clamped`)
484+
but not the `StateChange` a variable write should produce. W9 built against one
485+
coalesced `StateChange` per touched variable per batch —
486+
`{ path: "var.<name>", op: "set", value: <final>, previous: <before>,
487+
reason: "consequence_applied", visible: <decl.visible> }` — reusing 03 §6's
488+
`var.<name>` condition-field name, `op: "set"` regardless of which typed ops ran,
489+
and W8's `reason` literal-string precedent. Needs the same formal codification in
490+
`04-core.md` §12 once a caller (W11's `submitChoice`) actually attaches these to an
491+
`AdvanceResult`. See `plans/16-w9-variables-and-consequences.md`, Decision 3.
482492
- [ ] `wisdom` attribute has no consumer in the simulation kind — needs one to earn its
483493
place (`games/04-engine-specification.md` §8.4).
484494
- [ ] Provisional numbers across the simulation kind (drift rates, scenario economics,
Lines changed: 242 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,242 @@
1+
# W9 — Variables and Consequences
2+
3+
**Status:** Draft — implementing immediately after this document (user directive: "get
4+
the next milestone, create branch, work, create PR, watch comments").
5+
6+
**Unit:** [`docs/docs/engine/TODO.md`](../docs/docs/engine/TODO.md) — W9
7+
8+
**Scope:** `VariableSchema`, typed `set` / `increment` / `decrement`, clamp-after-all-
9+
effects, sorted iteration of state-affecting records (03 §2, §5, §8.1).
10+
11+
**Depends on:** W1 — done, merged (`f7d8f59`).
12+
13+
## What's Actually Left to Build
14+
15+
This is the first unit under "The Story-Graph Kind" — nothing kind-specific exists yet.
16+
`grep`ing `src/engine/src` for `VariableSchema`/`Consequence`/`VarValue` returns nothing;
17+
W1 scaffolded only the core's kind-agnostic types. `src/engine/eslint.config.js` already
18+
anticipates a `src/kinds/` tree (its dependency-arrow rule bans `**/kinds/*` imports from
19+
`src/core/**`), so that's the directory this unit opens: `src/engine/src/kinds/story-graph/`.
20+
21+
Everything here is new:
22+
23+
1. The typed vocabulary from 03 §2 (`VarType`, `VariableDecl`, `VariableSchema`,
24+
`VarValue`) and §5 (`Consequence`).
25+
2. `buildInitialVariables(schema)` — turns a schema's declared `initial` values into the
26+
runtime `variables: Record<string, VarValue>` that seeds `StoryGraphKindState` (§8.1),
27+
built later in W11's `initialState`.
28+
3. `applyConsequences(schema, variables, consequences)` — the one place a story-graph
29+
game is allowed to mutate a variable. Runs a batch of typed effects (a choice's,
30+
auto-node's, or random-transition's `effects` array) against declared variables,
31+
clamping `int`s once at the end, and returns both the new `variables` and an audit
32+
trail of `StateChange`s for the caller to attach to `AdvanceResult.changes` (W11).
33+
34+
## Decisions
35+
36+
### 1. Undeclared/mistyped writes throw — they don't produce a `ValidationError`
37+
38+
03 §5's "validation checks" (var declared; op suits type; `set` value matches type/enum)
39+
read, at first glance, like something that should return the engine's existing
40+
`ValidationError`/`ReasonCode` vocabulary (`kernel/reasons.ts`), the way a rejected choice
41+
does (03 §8.2 step 2).
42+
43+
They're not the same kind of rejection, and 03 §11 confirms it: "every variable in a
44+
consequence... is declared" and "every consequence op suits its variable's type" are
45+
listed as **Tier 1, load-time** checks (W14's `validateCampaign`, which depends on W5 and
46+
W11 — not W9). In a campaign that has passed Tier 1, this function's guard is
47+
structurally unreachable; it exists as the runtime backstop for a bug (bad content that
48+
slipped past validation, or a caller misusing the typed API), not a gameplay outcome a
49+
well-formed campaign can trigger. 03 §11 sets exactly this precedent already:
50+
`weightedPick` "throws... so this is a load-time rule, not a runtime crash" for the same
51+
reason (an all-zero-weight `random` node is a Tier 1 error; the runtime function still
52+
guards it defensively). `applyConsequences` follows the same pattern — `throw new
53+
Error("story-graph variables: ...")`, matching the plain-`Error`, module-prefixed style
54+
already used for this class of guard (`persistence/canonical.ts`, `determinism/pcg32.ts`,
55+
`session/store.ts`).
56+
57+
Reserving `ValidationError`/`ReasonCode` for rejections a *validated* campaign can still
58+
produce during play (`unknown_action`, `requirement_unmet`, and 03 §8.3's kind-specific
59+
three, all added in W11/W12) keeps that vocabulary meaning "the player did something the
60+
content legitimately disallows," not "the content is broken."
61+
62+
This is exactly why W9's done-criterion ("undeclared and mistyped writes are rejected")
63+
is tested directly against this module in isolation — W14 doesn't exist yet, has no
64+
dependency on W9, and checks the whole node graph statically rather than by exercising
65+
this function.
66+
67+
### 2. Clamp once, after the whole batch — raw accumulation in between
68+
69+
"`+5` then `-5` on a clamped int nets to zero rather than clipping" only holds if the
70+
value is *not* clamped between the two ops. Worked example: `money: int, min: 0, max: 3`,
71+
current value `2`, effects `[increment by 5, decrement by 5]`.
72+
73+
- **Wrong (clamp-per-op):** `2 + 5 = 7 → clamp → 3`; `3 - 5 = -2 → clamp → 0`. Net: `2 → 0`
74+
— a `+5/-5` pair that should cancel instead *loses* 2, purely from where the ceiling sat.
75+
- **Right (clamp-once):** raw accumulation `2 + 5 = 7`; `7 - 5 = 2`; clamp the final `2`
76+
once → `2` (no clamping needed). Net: unchanged, as `+5/-5` should be.
77+
78+
So `applyConsequences` accumulates a raw (unclamped) running value per touched variable
79+
across the whole input array — `set` replaces the running value outright,
80+
`increment`/`decrement` add/subtract from it — and clamps exactly once per variable,
81+
after every consequence in the call has been folded in. Clamping only ever applies to
82+
`int` (03 §5); `bool`/`enum` writes pass through unclamped (there is nothing to clamp).
83+
84+
### 3. `StateChange`s are coalesced per variable, not emitted per consequence
85+
86+
A `StateChange` is 04 §12's audit record of what actually landed in state, not a log of
87+
operations attempted (`kernel/reasons.ts`'s header: "emitted by a typed reducer — never
88+
the mutation mechanism"). Emitting one raw `StateChange` per input `Consequence` would
89+
mean showing the unclamped intermediate from Decision 2's `+5` step even though it never
90+
actually took effect — misleading, and it duplicates what
91+
`kind.story-graph.consequence.applied` (03 §8.4, a separate operational event, out of
92+
scope here — that's W11/W12's `ctx.emit` wiring) already exists to log per-op.
93+
94+
Instead, `applyConsequences` returns exactly one `StateChange` per variable **touched by
95+
at least one consequence in the batch**, carrying the final (post-clamp) value and the
96+
value the variable held before the batch started:
97+
98+
```typescript
99+
{ path: `var.${name}`, op: "set", value: <final>, previous: <before>, reason: "consequence_applied", visible: <decl.visible ?? false> }
100+
```
101+
102+
- `path: var.<name>` reuses 03 §6's condition-field namespace verbatim — the same string
103+
a `Condition` would read back, rather than inventing a second name for the same thing.
104+
- `op: "set"` regardless of which ops ran — the record describes the net landing value,
105+
not the arithmetic that produced it (03 §5's "no arbitrary path write" framing is about
106+
the write being typed and audited, not about preserving op history here).
107+
- `reason: "consequence_applied"` follows the literal-string convention W8 established
108+
for `achievement_unlocked` (`plans/15-w8-profile-store.md` Decision 1) — a stable,
109+
descriptive reason a session store or client can match on, not a `BASE_REASON_CODES`
110+
entry (it isn't kind-agnostic, and it isn't a rejection).
111+
- `visible` mirrors the variable's own `visible` declaration — an audit record for a
112+
hidden variable must stay hidden, same as the variable itself never leaking into
113+
projection or text interpolation (03 §3.1, §9).
114+
115+
A variable whose net change is zero (this unit's own `+5`/`-5` example) still emits a
116+
`StateChange` — it was touched, even though `previous === value`. Whether a zero-delta
117+
change is worth suppressing is a projection/UX concern for later units, not this one.
118+
119+
### 4. Sorted iteration, applied at the two points this unit actually has a `Record` to iterate
120+
121+
The kind's Record fields are "subject to the core's sorted-iteration rule... a `Record`
122+
iterated in a state-affecting way is sorted first, or a save/load round trip can
123+
diverge" (03 §8.1, citing the core). `persistence/canonical.ts` already sorts object keys
124+
on every `serialize()`, so a save/load round trip through the engine's own persistence
125+
is covered regardless of insertion order. What canonical serialization does *not* cover
126+
is any code in this unit that walks a `Record`'s keys directly and lets that order affect
127+
its own output — insertion order is language-guaranteed for string keys, but two
128+
call sites building structurally-equal `variables` objects in different key orders should
129+
still be indistinguishable to anything downstream that isn't `canonicalStringify`.
130+
131+
Two places this unit touches a `Record` in a state-affecting way, both made to iterate
132+
`Object.keys(...).sort()` rather than declaration/insertion order:
133+
134+
- `buildInitialVariables(schema)` — builds `variables` by walking the schema's keys
135+
sorted, not in authoring order.
136+
- `applyConsequences`'s returned `changes` — coalesced per Decision 3 from a `Map` keyed
137+
by variable name, emitted sorted by name rather than by first-touch order in the input
138+
`consequences` array.
139+
140+
`consequences` itself is a plain array (a transition's effects, authored in the order
141+
they should apply — Decision 2 depends on that order) — sorting does not apply there;
142+
only `Record` iteration is in scope for this rule.
143+
144+
## Design
145+
146+
### New files
147+
148+
| File | Contents |
149+
|---|---|
150+
| `kinds/story-graph/variables.ts` **(new)** | `VarType`, `VariableDecl`, `VariableSchema`, `VarValue`, `Consequence`; `buildInitialVariables`; `applyConsequences`. |
151+
| `kinds/story-graph/variables.test.ts` **(new)** | Coverage below. |
152+
153+
No existing file changes — this unit adds a self-contained module with no wiring into
154+
`kernel/engine.ts` yet (that's W11/W12, once `Kind.advance` for `story-graph` exists to
155+
call it).
156+
157+
### Signatures
158+
159+
```typescript
160+
export type VarType = "bool" | "int" | "enum";
161+
export type VarValue = boolean | number | string;
162+
163+
export interface VariableDecl {
164+
type: VarType;
165+
initial: VarValue;
166+
values?: string[]; // enum only
167+
min?: number; // int only
168+
max?: number; // int only
169+
visible?: boolean;
170+
labelKey?: LocKey;
171+
}
172+
173+
export type VariableSchema = Record<string, VariableDecl>;
174+
175+
export type Consequence =
176+
| { op: "set"; var: string; value: VarValue }
177+
| { op: "increment"; var: string; by: number }
178+
| { op: "decrement"; var: string; by: number };
179+
180+
export function buildInitialVariables(schema: VariableSchema): Record<string, VarValue>;
181+
182+
export function applyConsequences(
183+
schema: VariableSchema,
184+
variables: Readonly<Record<string, VarValue>>,
185+
consequences: readonly Consequence[],
186+
): { variables: Record<string, VarValue>; changes: StateChange[] };
187+
```
188+
189+
`applyConsequences` never mutates its `variables` input — returns a new object (matching
190+
the codebase's existing pure-reducer style, e.g. `AdvanceResult.state`).
191+
192+
### Guard conditions (throw, per Decision 1)
193+
194+
Checked per consequence, in input order, before any accumulation:
195+
196+
- `schema[c.var]` missing → undeclared variable.
197+
- `op` is `increment`/`decrement` but `decl.type !== "int"` → op/type mismatch.
198+
- `op` is `set` and:
199+
- `decl.type === "bool"` but `typeof value !== "boolean"`,
200+
- `decl.type === "int"` but `typeof value !== "number"` or `!Number.isInteger(value)`,
201+
- `decl.type === "enum"` but `typeof value !== "string"` or `!decl.values?.includes(value)`
202+
→ mistyped/invalid write.
203+
204+
### Test Plan
205+
206+
Against TODO's W9 done-criteria directly:
207+
208+
- [ ] Writing to a variable name absent from the schema throws, for `set`, `increment`,
209+
and `decrement`.
210+
- [ ] `set` with a value of the wrong JS type for the declared `VarType` throws (bool
211+
given a number, int given a string, enum given a non-member string, int given a
212+
non-integer number).
213+
- [ ] `increment`/`decrement` against a `bool` or `enum` variable throws.
214+
- [ ] `[increment by 5, decrement by 5]` on an `int` with `min: 0, max: 3` starting at `2`
215+
nets to `2` (unchanged) — proving clamp-once, not clamp-per-op (Decision 2's worked
216+
example, asserted directly).
217+
- [ ] A single `increment` past `max` (or `decrement` past `min`) clamps to the bound.
218+
- [ ] `buildInitialVariables` reproduces every schema's declared `initial` value, and
219+
produces key-identical (via `canonicalStringify`) output regardless of the order
220+
keys appear in the source `VariableSchema` object literal — the sorted-iteration
221+
claim, tested by constructing two schemas with the same entries in different
222+
declaration order.
223+
- [ ] `applyConsequences`'s `changes` are sorted by variable name regardless of the input
224+
`consequences` array's touch order (multiple variables, deliberately out-of-order
225+
effects).
226+
- [ ] A touched variable's `StateChange` carries the correct `previous`/final `value`,
227+
`path: var.<name>`, and `visible` mirroring the declaration — including a `visible:
228+
false` (or omitted) variable producing `visible: false`.
229+
- [ ] `applyConsequences` does not mutate its `variables` input (reference/identity check
230+
plus a value check after the call).
231+
232+
### Explicit Non-Goals
233+
234+
- No `Condition` evaluator or requirement/gating logic — that's W10.
235+
- No node graph, `enter`, settle loop, or `turn` counter — that's W11. This unit has no
236+
concept of a "transition" beyond "one array of consequences passed in one call."
237+
- No wiring into `Kind.advance`, `kernel/engine.ts`, or `ctx.emit` — nothing calls this
238+
module yet; W11/W12 do.
239+
- No achievements (W13) or Tier 1/2 `validateCampaign` (W14) — Decision 1 explains why
240+
this unit's guard is deliberately not that check.
241+
- No text interpolation (03 §3.1) — reads `visible` off a `VariableDecl` but does not
242+
touch node text.

0 commit comments

Comments
 (0)