Factory: mvtt-factory · Definition: v1
Started: 2026-06-16 15:21:37 UTC
Currently: active at code-review (cycle 1)
Path: intake → planning → plan-review → planning → plan-review → planning → plan-review → planning → plan-review → implementing → checks → code-review
26 events · 12 stage visits · 11 artifacts recorded · 2 evidence records · 1 approvals · 12 findings (7 resolved)
Entered 15:21:37 via start
Light tracking fixes: the torch/light holder should always count as in the light; adjacent characters should get the correct lesser light level per TB rules (verify dim light); and the light coverage pop-up is transparent and unreadable (CSS fix).
lane: full
rulesRelated: true
→ plan to planning
Entered 15:22:13 via plan
Fix three gaps in Torchbearer light-coverage tracking. (1) The light bearer is never automatically in the light — fix by treating the holder as always fully lit. (2) There is no "dim light" tier, but TB2 (p.43/p.34) says each source lights N characters in full light AND N more in dim light (candle 1+1, torch 2+2, lantern 3+3); characters near a source but not covered are in dim light (+1 Ob factor). Add a lit/dim/dark model. (3) The coverage pop-up is transparent and unreadable because light-tracker.tsx uses bg-surface-raised, whose token --color-surface-raised does not exist (real token: --color-surface-elevated). All changes are additive and backwards-compatible per the repo's data-survival rules.
testingStrategy:
Unit (node): extend light.test.ts — holder auto-included in the lit set; dim assignment validated against dim capacity (= full capacity); legacy LightCoverage values without the new dim field read as [] (trait upgrade/defensive-normalize); LightCoverageChanged replay accepts old payloadVersion. Component (jsdom): extend light-tracker.test.tsx — holder renders as lit (never in the darkness list) and is non-toggleable in the editor; a new Dim section renders dim characters; assert the panel uses an opaque surface class (regression guard for the transparency bug); dim-editor click dispatches the updated command. Wire smoke: only if the AssignLightCoverage payload shape changes on the wire — add/extend a light-coverage smoke round-tripping the dim field. Run pnpm -r typecheck and pnpm test (the mvtt-checks workflow) green before code review.
steps:
1. Fix the transparent pop-up. In light-tracker.tsx:337 replace `bg-surface-raised` with `bg-surface-elevated` (the real token, themed via light-dark() in tokens.css). Audit the file for any other `surface-raised` usage. Smallest, isolated, no data impact.
— files: packages/system-torchbearer/src/client/light-tracker.tsx
2. Holder always in full light. Add a single source-of-truth helper in shared/light.ts that, given a LightCoverage assignment, returns the effective lit set = coveredCharacterIds plus the holderId. Use it everywhere coverage is read so the holder is never dark and never needs manual assignment. Decide (and document) whether the holder consumes one of the full-light slots — proposed: yes, holder occupies one slot, so a torch = holder + 1 other in full light (matches the rules table's "2"). Validation in AssignLightCoverage should reserve the holder's slot rather than reject it.
— files: packages/system-torchbearer/src/shared/light.ts, packages/system-torchbearer/src/client/light-tracker.tsx
3. Add the dim-light tier. Extend the LightCoverage trait with an additive optional dimCharacterIds (EntityId[]); legacy values default to []. Dim capacity = lightCoverage(source) (same N as full). Extend AssignLightCoverage with an optional dimCharacterIds input, validate length <= dim capacity and disjoint from the lit set; bump the LightCoverageChanged event payloadVersion and accept both shapes on replay. Provide a derivation that classifies every PC as lit / dim / dark (lit = effective lit set incl. holder; dim = dimCharacterIds; dark = the rest).
— files: packages/system-torchbearer/src/shared/light.ts, packages/system-torchbearer/src/server/light.ts
4. Surface dim light in the UI. In light-tracker.tsx add a "Dim Light" section (parallel to "In Darkness") and a dim editor in CoverageEditor (capped at dim capacity); show the holder as a fixed, non-toggleable lit badge. Keep all values read live via useTrait/useQuery (no snapshotting per the real-time rule).
— files: packages/system-torchbearer/src/client/light-tracker.tsx
5. Tests + backwards-compat verification. Extend light.test.ts and light-tracker.test.tsx per the testing strategy, including a legacy LightCoverage value (no dimCharacterIds) and an old-payloadVersion LightCoverageChanged replay. Note as a follow-up (not in scope here): wiring the dim-light +1 Ob factor into the TB test/conflict pipeline.
— files: packages/system-torchbearer/src/light.test.ts, packages/system-torchbearer/src/light-tracker.test.tsx
→ submit to plan-review
Entered 15:29:17 via submit
DDD-1 (critical, backcompat-event-replay) — open
The plan's "bump LightCoverageChanged payloadVersion, branch on replay" mechanism is unimplementable here. defineEvent takes no payloadVersion; toPersistedEvent hardcodes payloadVersion:1; cold-boot replay reconstructs events from the RAW stored payload and never calls schema.parse, so neither a version branch nor a Zod .default([]) runs on replay. Old payloads reach LightCoverageSystem with dimCharacterIds undefined. Real protection = defensive ?? [] coalescing in the mirror system and every reader, NOT a payloadVersion bump.
DDD-3 (critical, mirror-system-data-loss) — open
LightCoverageSystem deletes the assignment when coveredCharacterIds.length === 0. With dim added, "0 full + N dim" is legitimate and would be silently deleted. Delete predicate must become (covered==0 && dim==0). The mirror write object and LightCoverageChanged payload must also carry dimCharacterIds, and ClearLightCoverage (uses covered:[] as its delete signal) becomes ambiguous under two-set semantics. Plan never mentions the mirror system.
DDD-2 (high, backcompat-trait-default) — open
"legacy dimCharacterIds -> [] via Zod default" is false on read: world.get returns stored values verbatim without re-parsing, and snapshot restore loads raw. The default only materializes on the next world.set. Every read site must coalesce ?? []; the schema default is not a migration.
DDD-4 (high, derivation-misfit) — open
"Add a derivation classifying each PC lit/dim/dark" fights the substrate: derivations re-run only when a DECLARED input trait on the SAME entity changes, one-writer-per-output. The classification depends on the grind sentinel's LightCoverage (a different entity) + the holder relationship — not PC-local — so a materialized trait goes stale, violating the no-snapshot/real-time rule. Extend the existing client memo (inDarkness, light-tracker.tsx:302) to lit/dim/dark instead of materializing a trait.
DDD-5 (high, invariant-double-count) — open
Fix 1 models "holder is lit" three incompatible ways. Today the holder is stored INSIDE coveredCharacterIds and counted within maxCoverage (encoded by light.test.ts:342-366). Adding both a read-time union {holderId} AND a validation that "reserves the holder's slot" double-counts. Pick ONE source of truth: holder implicit (never stored/counted) OR stored+counted (never unioned/reserved); update the existing tests accordingly.
DDD-8 (high, client-server-drift) — open
The real drift risk is the capacity math. Server AssignLightCoverage.validate would "reserve a holder slot" while the client computes its own max/isDisabled (light-tracker.tsx:171,199) with no holder reservation — UI offers an assignment the server rejects. Both sides must consume ONE shared "remaining assignable slots" helper including the holder rule.
SOLID-1 (high, for-vs-index) — open
useTrait captures entityId once and is not reactive to a changing prop. New Dim/Dark lists must use keyed by the EntityId string (as the darkness list already does, light-tracker.tsx:358); keys by position and would show the old character's name when membership shifts. Switching to here is silently broken.
SOLID-2 (high, test-coverage) — open
CLAUDE.md jsdom rules require each new interactive control to assert rendered output + dispatched command, plus GM gating. Plan adds a dim editor, a non-toggleable holder badge, and the transparency swap with NO tests specified. Mandatory: dim-editor dispatch test (AssignLightCoverage with dimCharacterIds), "holder never in darkness / non-toggleable" test, transparency class assertion, and given/when/then for the new validate branches + the mirror covered==0-but-dim>0 no-delete path.
SOLID-5 (medium, list-reactivity) — open
keys by reference but useLitSources rebuilds new objects every recompute (fires on any TbCarries / turnsRemaining change), so every SourceRow tears down and its local editing signal resets — the editor snaps shut mid-edit. Adding a second (dim) editor worsens this. Key the For by src.key (stable) or lift editing state out of the row.
SOLID-6 (medium, no-live-source) — open
LightCoverage stores only coveredCharacterIds + maxCoverage; there is no dim field anywhere. A "read live" dim memo has no trait slice to subscribe to and would snapshot a guess. Must add a dim-covered field on the assignment (parallel to coveredCharacterIds) and the command/event path so the memo reads it live via coverage().
SOLID-3 (medium, transparency-fix) — open
Transparency fix is real (--color-surface-elevated is a genuine @theme token; --color-surface-raised is not). But jsdom can't assert computed background (no Tailwind build / getComputedStyle). The catching test must assert the class string: toHaveClass('bg-surface-elevated') and not.toHaveClass('bg-surface-raised'). State this in the plan.
SOLID-4 (medium, holder-derivation) — open
"Holder always lit" must be derived reactively: the classification memo must subtract the live set of litSources().map(holderId) so passing/dousing a torch updates who is holder-lit; CoverageEditor must exclude/disable the holder button by reading props.source.holderId at render, not snapshot it at open.
DDD-6 (medium, invariant-disjointness) — open
Dim/lit disjointness must be enforced against the EFFECTIVE lit set (covered ∪ {holder}), not stored covered alone, else the holder could be accepted into dim and classified both lit and dim. Also define precedence for a PC in range of two sources: brightest-wins (lit > dim > dark) in the memo.
DDD-7 (medium, ubiquitous-language) — open
Naming drift: stored coveredCharacterIds (full light) vs the new "lit" (covered ∪ holder) vs "dim"/"dark". Two near-synonyms for full light differing only by the holder is a trap. Reconcile the glossary (e.g. litCharacterIds/fullLightCharacterIds) and document covered/lit/dim/dark on the trait + event.
RULES-5 (medium, missing-darkness-tier) — open
Plan models only FULL and DIM, ignoring the third RAW tier DARKNESS (printed p.43): in darkness one cannot use Cartography/Scholar and may only flee/riddle/argue in conflict — hard ACTION RESTRICTIONS, not just +1 Ob. A PC beyond every source's full+dim radius is in darkness. Classify a real third "dark" bucket (the +1 Ob / action-restriction wiring can be a flagged follow-up, but the tier must exist).
RULES-6 (medium, dropped-source-dim-only) — open
Omission: set-down/dropped sources give DIM-ONLY light (printed p.44 — a dropped torch gives dim light for two and no other light; a set-down lantern dim for three). Such a source contributes 0 full slots + its dim coverage, and the bearer-auto-full-light rule must be suppressed for grounded sources.
DDD-9 (low, incomplete-fix) — open
Fix 3 is correct but the same file uses bg-surface-alt (CharacterBadge, light-tracker.tsx:155), an equally-undefined token — fix both.
SOLID-7 (low, adjacent-dead-token) — open
bg-surface-alt on CharacterBadge (light-tracker.tsx:155) is also a dead token, so every coverage/darkness badge already renders transparent. Since CharacterBadge is reused for the new dim/dark lists, fix it too.
SOLID-8 (low, snapshot-caution) — open
Keep classification as one memo returning EntityId[] per tier and pass cid down to leaves that read the name via useTrait (existing pattern at :358-364). Do not pass {id,name,tier} objects to badges — breaks live rename propagation.
DDD-10 (low, missing-tests) — open
Plan adds new UI + an optional command field but does not name the unit (light.test.ts) and jsdom (light-tracker.test.tsx) test additions CLAUDE.md makes mandatory.
RULES-1 (low, coverage-numbers) — open
CONFIRMED: full coverage candle=1/torch=2/lantern=3 is correct (printed p.42-43) and is the TOTAL lit including the bearer — so "holder occupies one full slot (torch = holder + 1 other)" has NO off-by-one. Candle = holder only; lantern = holder + 2.
RULES-2 (low, bearer-auto-lit) — open
"Bearer always in full light" is a reasonable codification but an INFERENCE, not printed RAW (the book lets the GM assign who is lit). Acceptable since it keeps the table numbers intact; document it as a design decision, not RAW.
RULES-3 (low, dim-tier) — open
CONFIRMED: dim coverage equals full coverage per source and is ADDITIONAL to the fully-lit (printed p.43: candles 1 additional, torches 2, lanterns 3).
RULES-4 (low, dim-effect) — open
CONFIRMED verbatim: dim light increases obstacle by one on all tests except riddling (printed p.43 / p.34).
RULES-7 (low, missing-daylight) — open
Minor: Daylight = full light for all (∞), printed p.43. If an outdoor/daylit scene is ever represented, all PCs are full-lit with no source. Plan only enumerates candle/torch/lantern.
RULES-8 (low, monster-exemption) — open
Context for the +1 Ob penalty: monsters see in dim light without penalty by default (printed p.174), some ignore darkness entirely (e.g. Orcs p.192). Don't apply the dim/darkness Ob factor to monster tests.
→ rework to planning
Entered 15:36:44 via rework
Revised after adversarial review (cycle 1). Fix three TB light-tracking gaps with ONE source of truth and defensive backcompat. (1) Holder always in full light: enforce holderId INTO coveredCharacterIds in AssignLightCoverage.apply (dedup) and in validate, so coveredCharacterIds stays the single source of truth — no read-time union helper (kills the DDD-5 double-count and DDD-8 client/server drift); cap stays max counting the holder, matching the RAW table (RULES-1: torch=2 total incl bearer). Client renders the holder as an always-on, disabled toggle. (2) Dim tier: add an additive dimCharacterIds[] to the LightCoverage assignment, the LightCoverageChanged event, and the AssignLightCoverage command; dim capacity = lightCoverage(source) (RULES-3 confirmed); classify each PC full/dim/dark in the EXISTING client memo (no materialized trait — DDD-4); brightest-wins across sources (DDD-6). (3) Transparent pop-up: bg-surface-raised and bg-surface-alt are dead tokens — swap both to bg-surface-elevated. Backcompat is defensive coalescing (?? []) at every read AND in the mirror system, NOT a payloadVersion bump (DDD-1/2). No schema-default-as-migration assumptions.
testingStrategy:
Unit (node, light.test.ts): AssignLightCoverage.apply always includes the holder in coveredCharacterIds even if omitted; validate caps at max counting the holder; UPDATE the existing holder-in-coverage cases (light.test.ts:342-366) for the new model; dim capacity validated; dim disjoint from full (incl holder); the mirror keeps a "0 full + N dim" assignment (delete only when BOTH empty — DDD-3); a legacy assignment value with no dimCharacterIds reads as [] via coalescing (DDD-2); a LightCoverageChanged replayed without dimCharacterIds is coalesced in the mirror (DDD-1). Component (jsdom, light-tracker.test.tsx): holder never appears in the Dim/Dark lists and is a disabled toggle; new "Dim Light" section renders dim PCs with keyed by EntityId (SOLID-1); dim-editor click dispatches AssignLightCoverage with dimCharacterIds (SOLID-2); assert panel toHaveClass('bg-surface-elevated') and not bg-surface-raised (SOLID-3, class-string only — jsdom has no Tailwind build); editor stays open when an unrelated TbCarries change recomputes litSources (SOLID-5). Wire smoke: extend a light-coverage smoke only because the AssignLightCoverage wire payload gains dimCharacterIds — round-trip it. Gate on the mvtt-checks workflow (lint+typecheck+test) green.
steps:
1. Transparent pop-up (smallest, no data impact). In light-tracker.tsx swap bg-surface-raised -> bg-surface-elevated on the panel (line 337) AND bg-surface-alt -> bg-surface-elevated on CharacterBadge (line 155); both referenced tokens are undefined in tokens.css (DDD-9/SOLID-7).
— files: packages/system-torchbearer/src/client/light-tracker.tsx
2. Holder always in full light — single source of truth (Model B). In AssignLightCoverage.validate, treat the holder as occupying one slot (cap unchanged at max, which already counts the holder per RULES-1) and in apply, add holderId to coveredCharacterIds if absent (dedup) before emitting LightCoverageChanged. Readers need NO holder logic; the holder is simply always present in coveredCharacterIds. Document covered(=full light, incl holder)/dim/dark glossary on the trait + event (DDD-7). This is a deliberate model change — update the existing tests, do not add a read-time union (DDD-5, DDD-8).
— files: packages/system-torchbearer/src/shared/light.ts
3. Dim-light tier — additive + defensive. Add optional dimCharacterIds[] to the LightCoverage assignment shape, the LightCoverageChanged event schema (with .default([]) for new writes), and the AssignLightCoverage command (optional, default []). Dim capacity = lightCoverage(source). validate: dim length <= capacity and dim disjoint from coveredCharacterIds (which includes the holder — DDD-6). In LightCoverageSystem: coalesce event.dimCharacterIds ?? [] (replay safety — DDD-1), persist it on the assignment, and change the delete predicate to (covered.length===0 && dim.length===0) so dim-only/clears behave correctly; ClearLightCoverage emits both empty (DDD-3). NO payloadVersion bump.
— files: packages/system-torchbearer/src/shared/light.ts
4. Classification + UI. Extend the existing client memo (light-tracker.tsx inDarkness ~line 302) into full/dim/dark buckets read LIVE from coverage(): full = union of coveredCharacterIds across lit sources, dim = dimCharacterIds minus anything full, dark = remaining PCs; brightest-wins (DDD-6). Render a "Dim Light" section parallel to "In Darkness" using <For> keyed by EntityId, passing cid down to CharacterBadge (names stay live via useTrait — SOLID-1, SOLID-8). In CoverageEditor add a dim sub-editor capped at dim capacity and render the holder as a disabled always-on full-light toggle by reading props.source.holderId at render (SOLID-4). Key the litSources <For> by src.key and keep editing state stable so an unrelated recompute doesn't snap editors shut (SOLID-5).
— files: packages/system-torchbearer/src/client/light-tracker.tsx
5. Tests (mandatory per CLAUDE.md) + backcompat verification, per the testing strategy: unit branches in light.test.ts (holder-enforced, dim-capacity, dim-disjoint, mirror no-delete-on-dim-only, legacy/replay coalescing) and jsdom cases in light-tracker.test.tsx (holder disabled & never dim/dark, Dim section, dim-editor dispatch, transparency class assertion, editor-stays-open). Add the dimCharacterIds round-trip to a light-coverage wire smoke.
— files: packages/system-torchbearer/src/light.test.ts, packages/system-torchbearer/src/light-tracker.test.tsx, packages/server/src/light-coverage.smoke.test.ts
6. Explicit scope boundary (acknowledging RULES-5/6/7/8). IN SCOPE: the full/dim/dark data model + tracker UI, holder-always-lit, additive dim tier. DEFERRED as documented follow-ups (TODOs with printed-page cites, not implemented here): wiring the dim-light +1 Ob factor and the darkness action-restrictions (flee/riddle/argue only; no Cartography/Scholar) into the TB test/conflict pipeline (p.43); dropped/set-down sources giving dim-only light with the bearer rule suppressed (p.44); Daylight as full-light-for-all (p.43); monster dim/darkness exemption (p.174/192). The third "dark" tier itself IS in scope as a classification bucket; only its mechanical effects are deferred.
— files: packages/system-torchbearer/src/shared/light.ts
→ submit to plan-review
Entered 15:38:21 via submit
DDD-11 (high, client-server-drift-regression) — open
Model B enforces the holder server-side but the plan says "readers need no holder logic" — yet CoverageEditor is a WRITER. On a first assignment props.assignment is undefined, the editor seeds no holder, the GM picks max non-holder PCs, and apply enforces holder -> max+1 -> validate rejects, dispatch fails silently (no error surfaced). Fix: the editor must be holder-aware via the shared remaining-slots helper — seed the holder into the editor's covered set and cap non-holder picks at max-1 on a fresh source.
DDD-13 (high, full-replace-overwrites-dim) — open
AssignLightCoverage.apply emits one LightCoverageChanged carrying full desired state; the mirror upserts BOTH covered and dim from it. The current editor dispatches only coveredCharacterIds, so the optional dimCharacterIds defaults to [] and toggling a full-light character wipes all dim assignments (and vice-versa). Every dispatch must re-send the complete current state of BOTH arrays (or the command needs explicit partial-update semantics). As written this is data loss.
SOLID-9 (high, nonexistent-for-key) — open
"Key the litSources by src.key" is not a real Solid capability — the built-in keys by reference and has no key/by prop. Since useLitSources rebuilds new objects each recompute, recreates every SourceRow and resets its local editing signal (light-tracker.tsx:232), snapping editors shut. Must instead pick a real mechanism: lift editing into a parent open-key signal in LightTracker (createSignal<string|null> keyed by src.key), OR make useLitSources return referentially stable objects, OR use a keyed component. Plan must name the chosen mechanism.
SOLID-10 (medium, holder-not-in-bucket-memo) — open
Holder-always-lit was only handled as editor presentation; the live bucket memo (inDarkness, light-tracker.tsx:302) derives covered solely from coverage().assignments, so the holder appears in dim/dark unless its holderId is folded into the FULL bucket inside that memo. Fold each lit source's holderId into the full set in the live memo.
DDD-12 (low, holder-toggle-trap) — open
With holder force-added in apply, the holder's editor toggle becomes a no-op (click removes, apply re-adds, snaps back). Render the holder as a fixed disabled "bearer" chip.
SOLID-11 (low, brightest-wins-dedup) — open
Brightest-wins must dedup across buckets in ONE memo: full wins over dim, dim subtracts full, dark subtracts both. Keep as a single createMemo over coverage() + playerCharIds() returning EntityId[] per bucket; pass cid to leaves (no snapshotted names).
RULES-11 (low, dropped-source-invariant) — open
Deferring dropped/set-down dim-only sources (printed p.44) is safe ONLY while the tracker models every source as held/full. The deferral TODO must state the invariant explicitly: until dropped-state is wired, no UI path may show a source as dropped while still granting full light or a bearer full-light slot (that would be RAW-wrong, not just missing).
DDD-1 (low, resolved) — open
RESOLVED: no payloadVersion bump; mirror coalesces dimCharacterIds ?? []; legacy Assign (covered>0) upserts dim=[], legacy Clear (covered=[]) hits both-empty -> delete. Backcompat holds without a version branch.
DDD-3 (low, resolved) — open
RESOLVED and now MORE unambiguous: Model B means an in-hand source always has the holder in covered, so covered==0 && dim==0 only originates from Clear/auto-clear. Auto-clear-on-douse/burnout delete by key directly and are unaffected.
DDD-5 (low, resolved-conditional) — open
RESOLVED if validate adds+dedups the holder BEFORE the length check, so the existing maxCoverage tests survive ([holder,char2,char3] on a torch still rejects 3>2; lantern allows 3). Make validate (not just apply) enforce the holder pre-check.
RULES-9 (low, rules-confirmed) — open
RAW re-confirmed (printed p.43): full coverage 1/2/3 includes the bearer; dim capacity equals the source's coverage and is additional. Holder-slot and dim-capacity numbers are correct. Scope deferral judged honest provided dim is shown as a descriptive classification, not an effect-asserting label.
→ rework to planning
Entered 15:42:48 via rework
Revised after cycle-2 review. Same Model B (holder enforced into coveredCharacterIds as the single source of truth) plus the fixes for the bugs the cycle-2 reviewers found. (A) Holder-aware capacity on BOTH sides: one shared helper computes effective covered (always incl holder, deduped) and remaining assignable slots; validate enforces+dedups the holder BEFORE the length check (so existing maxCoverage tests survive), and the editor seeds the holder and caps NON-holder picks at max-1 on a fresh source — killing the silent-reject drift (DDD-11). (B) Full-replace-of-both: every AssignLightCoverage dispatch re-sends the complete current state of BOTH coveredCharacterIds and dimCharacterIds, so toggling one never wipes the other (DDD-13). (C) Editor persistence: lift SourceRow's editing into a parent open-key signal in LightTracker keyed by src.key (Solid has no key prop), so unrelated recomputes don't snap editors shut (SOLID-9). (D) Live bucket memo: one createMemo folds each lit source's holderId into the FULL bucket and applies brightest-wins dedup (full > dim > dark), so the holder never shows as dim/dark (SOLID-10, SOLID-11). Dim tier, defensive ?? [] backcompat, and the dead-token transparency fix are unchanged from cycle 2.
testingStrategy:
Unit (light.test.ts): validate enforces+dedups holder before the length check — [holder,char2,char3] on a torch still rejects, lantern allows 3, a fresh assign of [char2] on a torch is accepted and apply yields [holder,char2]; dim capacity = source coverage, dim disjoint from covered (incl holder); an AssignLightCoverage that resends both arrays preserves the other; the mirror keeps "covered>=1 + dim>0" and deletes only on both-empty; legacy assignment / replayed event without dimCharacterIds coalesces to []. Component (light-tracker.test.tsx): holder is a disabled bearer chip and never appears in Dim/Dark; a fresh torch editor caps non-holder picks at 1; dim-editor click dispatches AssignLightCoverage carrying BOTH arrays; toggling full-light preserves existing dim (assert payload); opening an editor then triggering an unrelated TbCarries recompute leaves it open; panel toHaveClass('bg-surface-elevated') and not('bg-surface-raised'); no bg-surface-alt remains. Wire smoke: round-trip AssignLightCoverage with dimCharacterIds. Gate on the mvtt-checks workflow green.
steps:
1. Transparency. Swap bg-surface-raised (panel, light-tracker.tsx:337) and bg-surface-alt (CharacterBadge, :155) to bg-surface-elevated; grep the file to confirm no other dead-token surfaces remain (DDD-9/SOLID-7).
— files: packages/system-torchbearer/src/client/light-tracker.tsx
2. Shared holder-aware capacity + Model B. In shared/light.ts add a pure helper effectiveCovered(holderId, coveredCharacterIds) = dedup([holderId, ...covered]) and assignableNonHolderSlots(max, effectiveCovered). AssignLightCoverage.validate builds effectiveCovered FIRST, then checks length<=max (existing maxCoverage tests preserved — DDD-5); apply emits coveredCharacterIds = effectiveCovered. Document the covered(=full light, incl holder)/dim/dark glossary on the trait + event (DDD-7).
— files: packages/system-torchbearer/src/shared/light.ts
3. Dim tier (additive + defensive, full-replace-of-both). Add optional dimCharacterIds[] to the LightCoverage assignment, LightCoverageChanged (.default([])), and AssignLightCoverage. validate: dim<=lightCoverage(src) and dim disjoint from effectiveCovered (DDD-6). apply emits BOTH arrays as full current state; document that every dispatch must resend both (DDD-13). Mirror: coalesce dimCharacterIds ?? [] (DDD-1), persist it, delete only when (covered==0 && dim==0) (DDD-3); ClearLightCoverage emits both empty. No payloadVersion bump.
— files: packages/system-torchbearer/src/shared/light.ts
4. Live classification memo. Replace the inDarkness memo (light-tracker.tsx ~302) with one createMemo over coverage() + playerCharIds() returning {full, dim, dark} as EntityId[]: full = union of each assignment's coveredCharacterIds PLUS each lit source's holderId (SOLID-10); dim = union of dimCharacterIds minus full; dark = PCs in neither; brightest-wins dedup (SOLID-11). Coalesce dimCharacterIds ?? [] when reading assignments (DDD-2). Pass cid to CharacterBadge leaves (names live via useTrait — SOLID-8).
— files: packages/system-torchbearer/src/client/light-tracker.tsx
5. Editor + persistence. Lift SourceRow's `editing` into a parent signal in LightTracker, createSignal<string|null>(null) holding the open src.key, so <For> recreation can't lose it (SOLID-9). In CoverageEditor: render the holder as a disabled bearer chip (DDD-12); seed effectiveCovered and cap non-holder picks via assignableNonHolderSlots (DDD-11); add a dim sub-editor capped at dim capacity; both sub-editors dispatch AssignLightCoverage with BOTH current arrays (DDD-13); read source.holderId at render (SOLID-4). Add a "Dim Light" section parallel to "In Darkness" with <For> keyed by EntityId (SOLID-1).
— files: packages/system-torchbearer/src/client/light-tracker.tsx
6. Tests + scope boundary. Add the unit/jsdom/smoke cases in the testing strategy. Scope boundary: IN SCOPE = full/dim/dark classification + tracker UI, holder-always-full, additive dim tier. DEFERRED (TODOs with printed-page cites, NOT implemented): dim +1 Ob + darkness action restrictions into the test/conflict pipeline (p.43); dropped/set-down dim-only sources — TODO must state the invariant that until dropped-state is wired every source is treated as held/full and no UI path may show a dropped source granting full light (RULES-11); Daylight full-for-all (p.43); monster dim/darkness exemption (p.174/192). Dim is labelled as a descriptive status, not an effect-asserting label (RULES-9/10).
— files: packages/system-torchbearer/src/light.test.ts, packages/system-torchbearer/src/light-tracker.test.tsx, packages/server/src/light-coverage.smoke.test.ts, packages/system-torchbearer/src/shared/light.ts
→ submit to plan-review
Entered 15:43:37 via submit
DDD-14 (high, max-input-divergence) — open
The shared slot helpers don't close the drift because the divergence is in the max INPUT: the editor computes lightCoverage(props.source.itemId) WITHOUT the item name (light-tracker.tsx:171) while validate/apply use lightCoverage(itemId, itemName) WITH the name heuristic (light.ts:137,152). For a name-heuristic item (e.g. an adventure item named "Torch" with no catalog id) editor max=1 but server max=2; once the editor seeds the holder, assignableNonHolderSlots(1,[holder])=0 locks the GM out of the companion the server would accept. Catalog/map torches (all existing tests) hide it. Fix: the editor must resolve ItemIdentity.name client-side and pass it into lightCoverage so both sides feed the same max into the shared helper.
SOLID-12 (medium, legacy-dim-default) — open
Adding dimCharacterIds to the assignment shape means legacy worlds hold assignments with no dim field; useTrait/world.get does not re-parse, so the bucket memo reads a.dimCharacterIds === undefined and throws when iterated, crashing the panel render. The memo AND the editor read path must defensively default a.dimCharacterIds ?? []. (Required defensive default, per the same class as DDD-1/2.)
DDD-15 (low, sticky-bearer-assignment) — open
Because apply emits effectiveCovered (always >=[holder]), deselecting everyone leaves a sticky {covered:[holder]} record rather than removing the assignment. This is rules-correct (a lit torch always lights its bearer) and self-heals on douse/burnout; Clear remains a separate command emitting both-empty. No bad interaction — noted for awareness.
DDD-5 (low, resolved) — open
RESOLVED: building effectiveCovered = order-preserving dedup([holder, ...covered]) before the length check keeps the existing maxCoverage tests green (torch [holder,char2] ok; [holder,char2,char3] fail; lantern 3 ok; toEqual([holder,char2]) preserved).
SOLID-9 (low, resolved) — open
RESOLVED: lifting editing into a parent createSignal<string|null> keyed by the stable src.key survives recreation — the signal lives in LightTracker (never disposed by the For) and src.key is deterministic from holderId+entryIndex, so a recreated row re-derives openKey()===src.key.
SOLID-10 (low, resolved) — open
RESOLVED: folding each lit source's holderId into the full bucket keeps the bearer out of dim/dark; non-PC holders are harmless (dim/dark derive from playerCharIds()).
DDD-11 (low, resolved) — open
RESOLVED for catalog/map sources (residual name-heuristic gap tracked as DDD-14): editor seeding + assignableNonHolderSlots closes the over-pick silent-reject. Editor holder-awareness must ship atomically with server holder enforcement.
→ rework to planning
Entered 15:48:29 via rework
Cycle-4 plan (converged). Model B with the holder as the single source of truth in coveredCharacterIds, plus every fix from cycles 1-3. Net of the final two: (DDD-14) the editor resolves the item NAME client-side and feeds it into lightCoverage(itemId, itemName) so the editor's max matches the server's exactly — closing the name-heuristic lockout; (SOLID-12) the bucket memo and editor read path defensively default a.dimCharacterIds ?? [] so legacy assignments don't crash the panel. Carried forward: shared holder-aware capacity (validate dedups holder before the length check; editor seeds the holder and caps non-holder picks at max-1; holder is a disabled bearer chip); full-replace-of-both arrays on every AssignLightCoverage dispatch; mirror coalesces dim ?? [] and deletes only on both-empty (no payloadVersion bump); a single live createMemo folds each lit source's holderId into the FULL bucket with brightest-wins dedup; transparency fixed by swapping the dead tokens bg-surface-raised and bg-surface-alt to bg-surface-elevated.
testingStrategy:
Unit (light.test.ts): validate dedups holder before length check (existing maxCoverage cases stay green; fresh torch assign of [char2] -> apply yields [holder,char2]); dim capacity = source coverage; dim disjoint from effectiveCovered; resending both arrays preserves the other; mirror keeps covered>=1+dim>0 and deletes only on both-empty; legacy/replay without dim coalesces to []. Component (light-tracker.test.tsx): holder is a disabled bearer chip and never in Dim/Dark; a fresh source named "Torch" (name heuristic, no catalog id) lets the GM pick exactly one companion (DDD-14 regression); dim-editor dispatch carries BOTH arrays; toggling full-light preserves dim; a legacy assignment without dimCharacterIds renders without throwing (SOLID-12 regression); editor stays open across an unrelated TbCarries recompute; panel toHaveClass('bg-surface-elevated') and not('bg-surface-raised'); no bg-surface-alt remains. Wire smoke: round-trip AssignLightCoverage with dimCharacterIds. Gate on the mvtt-checks workflow.
steps:
1. Transparency. Swap bg-surface-raised (panel :337) and bg-surface-alt (CharacterBadge :155) to bg-surface-elevated; grep the file to confirm no other dead-token surface remains.
— files: packages/system-torchbearer/src/client/light-tracker.tsx
2. Shared holder-aware capacity (Model B). In shared/light.ts add pure helpers: effectiveCovered(holderId, covered) = order-preserving dedup([holderId, ...covered]); assignableNonHolderSlots(max, effectiveCovered). AssignLightCoverage.validate builds effectiveCovered FIRST then checks length<=max (existing tests preserved); apply emits coveredCharacterIds = effectiveCovered. Document the covered(=full light, incl holder)/dim/dark glossary on the trait + event.
— files: packages/system-torchbearer/src/shared/light.ts
3. Dim tier (additive, defensive, full-replace-of-both). Add optional dimCharacterIds[] to the LightCoverage assignment, LightCoverageChanged (.default([])) and AssignLightCoverage. validate: dim<=lightCoverage(src) and dim disjoint from effectiveCovered. apply emits BOTH arrays as full current state. Mirror: coalesce dimCharacterIds ?? [], persist it, delete only when (covered==0 && dim==0); ClearLightCoverage emits both empty. No payloadVersion bump.
— files: packages/system-torchbearer/src/shared/light.ts
4. Live classification memo. Replace inDarkness (~:302) with one createMemo over coverage()+playerCharIds() returning {full,dim,dark} EntityId[]: full = each assignment's coveredCharacterIds PLUS each lit source's holderId; dim = (a.dimCharacterIds ?? []) minus full (SOLID-12 default); dark = neither; brightest-wins dedup. Pass cid to CharacterBadge leaves.
— files: packages/system-torchbearer/src/client/light-tracker.tsx
5. Editor + persistence + matching max (DDD-14). Lift SourceRow `editing` into a parent createSignal<string|null>(null) keyed by src.key. In CoverageEditor: resolve the item name (ItemIdentity via useTrait) and compute max = lightCoverage(itemId, itemName) so it matches the server; seed effectiveCovered and cap non-holder picks via assignableNonHolderSlots; render the holder as a disabled bearer chip (read source.holderId at render); add a dim sub-editor capped at dim capacity; both sub-editors dispatch AssignLightCoverage with BOTH current arrays (defaulting dim ?? []). Add a "Dim Light" section parallel to "In Darkness" with <For> keyed by EntityId.
— files: packages/system-torchbearer/src/client/light-tracker.tsx
6. Tests + scope boundary. Add the unit/jsdom/smoke cases above. IN SCOPE: full/dim/dark classification + tracker UI, holder-always-full, additive dim tier. DEFERRED (TODOs + printed-page cites, not implemented): dim +1 Ob and darkness action restrictions into the test/conflict pipeline (p.43); dropped/set-down dim-only sources — TODO states the invariant that until dropped-state is wired every source is held/full and no UI path may show a dropped source granting full light (RULES-11); Daylight full-for-all (p.43); monster dim/darkness exemption (p.174/192). Dim is a descriptive status label, not effect-asserting.
— files: packages/system-torchbearer/src/light.test.ts, packages/system-torchbearer/src/light-tracker.test.tsx, packages/server/src/light-coverage.smoke.test.ts, packages/system-torchbearer/src/shared/light.ts
→ submit to plan-review
Entered 15:49:08 via submit
DDD-16 (high, plan-stage-scope) — ✔ Stage scope: this is a plan review, not code review — the reviewer checked the working tree and objected that CoverageEditor's max memo (light-tracker.tsx:171) doesn't yet pass itemName, i.e. the fix isn't implemented. That is expected at the planning stage. The reviewer confirmed the approach is correct and the primitives exist (lightCoverage accepts (itemId, itemName); ItemIdentity is already read client-side by ItemName). The plan (step 5) specifies exactly this wiring, and the DDD-14 name-heuristic regression test is in the testing strategy. Tracked as an implementation task to be verified at code-review against the plan.
Plan v4's DDD-14 fix (editor resolves item name and computes max = lightCoverage(itemId, itemName) to match the server) is specified in the plan but not present in the working-tree code yet — correct for the planning stage; will be implemented and verified downstream.
SOLID-13 (low, resolved) — open
RESOLVED: explicit (a.dimCharacterIds ?? []) in the bucket memo and dim ?? [] in the editor read path correctly guard legacy assignments lacking the dim field (world.get does not re-parse), preventing the iterate-over-undefined panel crash; no reactivity regression. Plan v4 is clean from the Solid lens.
DDD-14 (low, resolved) — open
RESOLVED in plan v4: editor resolves ItemIdentity name client-side and feeds lightCoverage(itemId, itemName) so editor max matches server max; name-heuristic lockout closed (regression test added). Implementation verified at code-review.
“Approved in chat”
→ approve to implementing
Entered 15:53:49 via approve
Holder always in full light (single source of truth in coveredCharacterIds); additive dim-light tier with full/dim/dark classification, GM dim editor, per-source dim badges; transparent pop-up fixed (bg-surface-raised/bg-surface-alt -> bg-surface-elevated). Shared holder-aware capacity helpers used by validate + editor; SourceRow editing lifted to a parent open-key signal. 39 light tests + wire smoke pass; typecheck clean. Legacy backcompat dropped per user (no users today). Deferred: +1 Ob/darkness-action wiring, dropped sources, daylight, monster exemption.
headSha: 23f11733
commitId: 23f117334a06caeb8764af6ebf2d1e4a0bb4d31f · jjChange: vznzvtnu · files: 5
→ submit to checks
Entered 16:17:10 via submit
status: succeeded runId: db94ad97-5350-4224-a71d-5e358b8df3e0
jobs:
{
"lint": "succeeded",
"format": "succeeded",
"typecheck": "succeeded",
"tests": "succeeded"
}
→ pass to code-review
Entered 16:18:58 via pass
SOLID-1 (medium, test-gating) — ✔ Added a non-GM gating test (harness("player")) asserting the assign button and turns-remaining label are absent for players (light-tracker.test.tsx).
light-tracker.test.tsx only mounted as GM; CLAUDE.md requires asserting GM gating where the view applies it (the assign button and the turns label are gated on props.isGm).
DDD-4 (low, ui-readability) — ✔ Extracted LiveCharName; dim/dark badges now apply the tier tint directly instead of wrapping an opaque CharacterBadge, so the warning/dark colour cue is visible (this was undermining the readability fix itself).
Dim/dark badges double-wrapped an opaque CharacterBadge (bg-surface- elevated) inside a tinted span, hiding the tint.
SOLID-4 (low, ui-readability) — ✔ Same fix as DDD-4 (LiveCharName, no nested opaque badge).
Badge double-background hid the dim/dark colour cue.
DDD-5 (low, validation-robustness) — ✔ validate and apply now dedup dimCharacterIds via [...new Set(...)] for parity with the effective full set.
dimCharacterIds was not deduped (a crafted payload could double-count toward the cap / render twice).
DDD-3 (low, dead-code) — ✔ Removed unused TbSupply import from light.ts.
Unused TbSupply import.
SOLID-2 (low, test-regression) — ✔ Transparency test now also asserts the panel HTML contains no bg-surface-alt (the other dead token).
The transparency regression test only pinned the panel class, not the badge surface token.
DDD-1 (low, apply-reads-world) — open
AssignLightCoverage.apply (and Clear) read the world (TbCarries + ItemIdentity) to resolve max. CLAUDE.md lists reading state in apply as an anti-pattern. ACKNOWLEDGED, NOT CHANGED: this is the established pattern across ~25 apply blocks in the TB plugin (apply runs immediately after validate, deterministic); threading it would be an inconsistent one-off. Out of scope for this change.
DDD-2 (low, mirror-without-event) — open
The auto-clear systems write the sentinel's LightCoverage without emitting an event, relying on universal-mirror re-execution of the upstream EntryStateChanged/LightSourceWentOut. Pre-existing pattern, not modified by this change. ACKNOWLEDGED.
SOLID-5 (low, reactivity-confirmed) — open
CONFIRMED sound: classification memo dedups across buckets and folds the holder into full; the lifted open-key signal genuinely survives recreation (exercised by the editor-stays-open test); no props destructured; names read live at the leaf. No reactivity break.