diff --git a/.claude/summaries/2026-06-13-content-extensibility.md b/.claude/summaries/2026-06-13-content-extensibility.md new file mode 100644 index 000000000..0e336644d --- /dev/null +++ b/.claude/summaries/2026-06-13-content-extensibility.md @@ -0,0 +1,132 @@ +# Session Summary: Content Extensibility Analysis + +**Date**: 2026-06-13 +**Branch**: claude/zen-wright-04xhdz + +> ⚠️ **READ `docs/kb/content-extensibility-direction.md` FIRST.** The plan was deliberately +> **deflated** late in the session. Sections below (esp. "What we concluded", "How to +> resume", the two-layer / catalog-grant framing) are **history** — the direction doc is the +> real plan. Status: lots of code IS committed (Phases 0–4b, import-validation fixes, +> `save-character` fix, merged `feature/name-keyword-fix`); `by-parent` is slated for revert. + +## The question + +Why does adding a minor content type / builder to the 5e app touch ~8 files +(`routes.clj`, `route_map.cljc`, `db.cljs`, `events.cljs`, `spell_subs.cljs`, +`views.cljs`, `core.cljs`, plus a spec home)? Is there a less error-prone way that +keeps the standardization the codebase has? + +## What we concluded + +The cost is two separate problems, not one: + +1. **Registration** — scattered, parallel boilerplate keyed by the same entity + (route, db default, localStorage, events, subs, page-map). Mechanical. +2. **Injection** — wiring new options into a *parent* entity (e.g. boons into the + warlock) via positional function arguments. This is the fragile, bug-prone half. + +Proposed direction — two composable layers: + +- **Layer 1: content-type registry.** One descriptor list feeding loops that call the + *existing* `reg-*-homebrew` factories. Kills the registration boilerplate. +- **Layer 2: type-addressed catalogs + grants.** Generalize the subrace + "bucket-by-key" pattern from `:race` to option *type*. Producers declare a type; + consumers `grant-choice` from a catalog with an optional filter. Keep `mod5e/*` for + fixed grants. Kills the positional injection and makes cross-aspect grants + (feat→spell, background→feat, feat→boon) uniform — homebrew flows in for free. + +## Why it matters + +Verified from code: subraces and subclasses already use the clean bucket-by-key +pattern; boons and invocations use fragile positional threading; draconic ancestries +are a static list with no plugin path. The proposal is to make the rest work like +subraces already do. It also answers a cluster of open issues in +`docs/issues/homebrew-builders.md` (#58, #57/#209, #172/#170, #210/#107, #280, #173, +#128). + +## Files created (this branch) + +| File | Purpose | +|------|---------| +| `docs/kb/content-extensibility.md` | Problem, verified cross-link map, proposed two-layer direction | +| `docs/kb/content-extensibility-decisions.md` | Decision audit (how the thinking evolved) + crisp decisions D1–D8 | +| `docs/kb/content-extensibility-compatibility.md` | Backward-compat audit: persisted formats, invariants, proposal assessment | +| `docs/kb/content-extensibility-plan.md` | Phased implementation playbook for low-context agents (gates, stop conditions) | +| `BRANCH.md` | Branch purpose + handoff + split-commit notes | +| `.claude/summaries/2026-06-13-content-extensibility.md` | This summary | + +## How to resume + +1. Read `docs/kb/content-extensibility.md` (design) and `-decisions.md` (the why). +2. To implement, follow `content-extensibility-plan.md` literally: Phase 0 builds a + golden test, then Phase 1 migrates **subraces** onto a generic injector + (behavior-preserving), then subclasses, boons/invocations, the registry, and finally + lineages. Each phase is gated and has stop conditions. +3. When split-committing these docs to `agents/develop`, add index rows for the two + KB docs to `docs/kb/README.md` there (this branch's index differs). + +## Caveats for the next agent + +- KB rule is verified-only content. The cross-link map is verified from code; the + design is labeled a proposal. Keep that boundary. +- File:line references were read on the monolithic frontend layout of this branch; on + `agents/develop` views are split (`views-builders-split.md`), so resolve view + references by symbol, not line. +- Backward compatibility is a hard constraint, audited in + `content-extensibility-compatibility.md`. The target is zero-migration: derive + catalogs over the existing plugin storage and preserve selection/option keys, then + prove it with an orcbrew + saved-character fixture before/after each migration. +- **Coordinate with `feature/name-keyword-fix`** (same base `d42e05d`): identity keys + derive from stable ids, not display names; `option-cfg` has a `::plugin-source` slot; + a reconciler heals orphaned keys. Two standing rules for the catalog/grant phases + (decisions D10/D11): pass each item's stored `:key` to `option-cfg` (never re-derive + from a display `:name`), and make catalogs layered/memoized `reg-sub`s referenced by + grants (never recompute a catalog in a hot sub). Guard both with comments. + +## Implementation progress (code, this branch) +Phases 0, 1, 2 (subraces/subclasses via `option_catalog/by-parent`), 3a (key-lock +guard), 3b (`option_catalog/plugin-options`; boons/invocations), and 4a (the +`content_types.cljc` registry + audit test) are committed and gated green +(223 tests / 1106 assertions with the e2e fixture). Merged `feature/name-keyword-fix` +(`ec26955`); live-verified via PR #28 (all items PASS/covered, no regressions). +Remaining: 3c (positional-threading removal — risky, deferred), 4b–4f (wire the registry +into subs/db/events/routes/core — cljs, lint+review only here), Phase 5 (new builders). +See BRANCH.md for the live checklist. + +## Test-suite debt found this session (separate from the extensibility work) +See `docs/kb/test-suite-state.md` (verified). Headline: **CI runs only the JVM gate +(`lein lint`/`lein test`); the cljs suite is never run and has rotted** — 10 failures / +3 errors pre-existing on `develop`, all real or removed-subject tests (not theater). +Notable: `character_test.cljc` references the `::character` spec Larry removed in the 2016 +entity refactor (and duplicates the `.clj` test's namespace); the computed/built character +has no validation spec (the `save-character` null crash is a symptom). Working agreements +adopted: tests must be falsifiable (no theater); fix bugs on sight unless deep enough for +their own branch. Fixed the `save-character` null crash on sight (`42ceaaa8`). + +**Load-bearing gotcha captured** (`docs/kb/built-character-representation.md`, anchored in +code): the built/computed character is a map whose derived values are deferred `:entity-fn?` +functions read via `entity-val` — NOT a flat map; don't `spec/keys` it. This is why the +computed character has no spec and why Larry's flat `::character` died. + +**Deferred follow-ups — HIGHLIGHT AT BRANCH CLOSE** (also in BRANCH.md): (1) the +character-validation contract (own branch; charter in `character-validation.md`), and +(2) getting the cljs tests into CI (own branch; the cljs suite is unrun/rotted). Surface +both in the final PR/handoff so they aren't lost. + +## Verification discipline + re-anchor (late session) +Several confident claims this session were wrong until verified (spec history, sub-vs-spec, which import tests failed). Lessons captured in `docs/kb/verification-discipline.md`: verify against real callers/intent/runtime before asserting; a red test means test+code DISAGREE, not that code is broken. + +RE-ANCHOR: the branch's founding purpose is **content extensibility** (Phases 0–4b done; next core step = 4c, gated by the new headless cljs harness). The test-suite / import-validation triage is a semi-related tangent that produced the harness — which enables safely finishing 4c–4f. Don't let the tangent become the branch. + +## DIRECTION DEFLATED (late session — read the direction doc) +After a readability review the grand registry / catalog-grant DSL was **deliberately +scaled back**. Authoritative plan now: `docs/kb/content-extensibility-direction.md`. +Principle: an abstraction earns its keep only if it's thicker than what it hides and +reveals intent (`by-parent` failed this → revert to `group-by`). Agreed shape: a +descriptor + `register-homebrew-content!` HOF composing the existing factories, scoped +to mechanical boilerplate; keep readable data (`default-value`) explicit; magic-item +stays its own server-backed registrar; do NOT build a catalog/grant DSL. +Audit: 12 types share the shape (6 basic, 6 +`:builder-features`), 3 deviate. +cljs verification recipe: `docs/kb/cljs-headless-harness.md` (ephemeral; rebuild). +Next: revert by-parent; swap boon through the HOF + commit; create a NEW builder to +measure effort. Goal: stabilize while adding features. diff --git a/BRANCH.md b/BRANCH.md new file mode 100644 index 000000000..bff1d771d --- /dev/null +++ b/BRANCH.md @@ -0,0 +1,230 @@ +# Branch Context: claude/zen-wright-04xhdz + +> **Single plan / status (reconciled):** `docs/kb/roadmap.md` — the one top-level plan covering +> BOTH the content/pool+grant track (this doc + `content-extensibility-direction.md`) AND the later +> mechanization / class-feature / spell-slot expansion. Start there. This file remains the branch +> history + handoff notes; the "Immediate next steps" below are the Phase-1 content-track status. + +> **HOW IT WORKS / HOW TO EXTEND:** `docs/kb/content-extensibility-framework.md` — the canonical +> framework reference (mental model + registry schema + conventions + how-to-add-a-type + pool/ +> grant + invariants), human- AND agent-facing. Read it to use or extend the framework. +> +> **READ FIRST (current direction, v2):** `docs/kb/content-extensibility-direction.md` — now +> **re-centered**. A readability review correctly killed one unreadable wrapper (`by-parent`), +> but that local lesson was briefly over-applied to deflate the whole *capability*. v2 restores +> the spine: an open **pool + grant** composition layer (any (sub)race/(sub)class/feat/background +> can grant filtered, gated choices from any other silo), with a variant forward-compat seam. +> Principle (a *constraint*, not a ceiling): *an abstraction earns its keep only when it's +> thicker than what it hides and reveals intent.* `content-extensibility.md` / `-plan.md` are +> history. +> +> Verifying cljs in this container: `docs/kb/cljs-headless-harness.md` (rebuild recipe; +> the harness lives in ephemeral `/tmp`+`target`). +> +> **Immediate next steps:** (1) ✅ DONE (`9777ce88`) — reverted `by-parent`/`plugin-options`, +> deleted `option_catalog`. (2) ✅ DONE (`3980ea1b`) — `register-homebrew-content!` (the +> **wiring** sub-layer) + boon swapped through it (7 sites → 1); harness-verified. (3) ✅ DONE +> (`acaa131d`) — **first pool+grant slice on real mechanics**: `draconic-ancestries` def → an +> open pool (`content_pools.cljc` + `::races5e/draconic-ancestry-pool`); dragonborn grants +> from it; a homebrew ancestry inherits full mechanics (resistance + breath weapon). Built-ins +> unchanged; additive-safe; falsifiable JVM + harness tests (incl. the maintainability proof). +> (4) ✅ DONE (`109b5dd0`) — `simple-content-builder`: collapsed boon+invocation builder FORMS +> into one (forms are data, not "irreducible"; D22). (5) ✅ DONE (`0aca6113`) — **Draconic +> Ancestry builder end-to-end**: author in-app → pool → export → import → character round-trip, +> all gated. Measured cost: 9 files but only 2 required thought (the view's damage-type field + +> the spec); the rest were 1-line registrations via register-homebrew-content!/simple-content-builder/ +> content_types. (6) ✅ **registry now DRIVES the layers** (the real "fewer files" fix): events +> (`d2e002b4`) + db (`af68061d`) wiring generated from `content_types` — a new homebrew type no +> longer touches events.cljs or db.cljs (behavior-preserving, harness-gated). (7) ✅ **routes** +> layer generative (`506c32b3`/`c5e9aea6`/`58c4de47`) — cycle broken, bidi segments + my-content +> set + SPA allowlist generated from the registry; a new type's URL/nav/allowlist are automatic +> (route_map keeps only its one route-keyword def, D6). (8) **NEXT:** the live breath-weapon bug; +> then grant-authoring UI. See direction doc v2 §"Foundation", §"The spine". +> Goal: **stabilize while adding features — stability and flexibility are the SAME abstraction.** + +## Purpose +Capture the content-extensibility analysis and plan, and implement it in gated phases +(reducing the multi-file cost of adding a content type/builder to the 5e app). + +## ⚓ Re-anchor — what this branch is *founded on* (don't lose the plot) +**Founding purpose = content extensibility, for TWO equal reasons: stability AND flexibility.** +The insight: they're the **same abstraction**. Today every cross-type link is bespoke +positional wiring (boons→warlock by arg; custom-race menu a hardcoded vector) — that bespoke-ness +*is* the ~8-file cost and the fragility. An open **pool + grant** layer collapses N×M bespoke +wirings to N+M declarations down one tested path: stability win = flexibility win. The +engine *already* supports filter/gate/prereq (`selection-cfg`/`prereq-fn`/`option-prereq`/ +`ability-increase-selection-2`); the gap is the **authoring** layer (content can't *declare* +open cross-silo grants). Readability stays a *constraint*: two words (pool/grant), built from +existing thick parts, no cryptic DSL. What stands: `content_types` registry (data + audit +test), the Phase-4b subs loop, `register-homebrew-content!` (wiring sub-layer) + boon. +The `by-parent`/`plugin-options`/`option_catalog` wrappers were reverted (`9777ce88`). + +**Current state / next core step:** ✅ `register-homebrew-content!` built; boon swapped +(`3980ea1b`). **Next core step:** prove the **pool + grant** spine on one slice end-to-end +(direction doc v2 §"The spine" + PINS — incl. the variant `resolved-content` forward-compat +seam). The test-suite triage (rotted cljs suite, dead `character_test.cljc`, import fixes) was +a tangent that produced the **headless cljs harness** — our gate for cljs work. Don't let the +tangent become the branch. + +Verification discipline lessons from this session: `docs/kb/verification-discipline.md`. + +## Roadmap / TODO (live checklist — updated as work proceeds) + +> ⚠️ **The phase numbering below is from the OLD (superseded) plan** and refers to code that +> no longer exists (`option_catalog`, `by-parent`, `plugin-options` — all reverted in +> `9777ce88`). Read it as *history of what was tried*, not the live plan. The live plan is the +> re-centered **pool + grant** spine in `content-extensibility-direction.md` (v2) and Part 4 +> of the decisions doc. The `[x]` items below (name-keyword-fix merge, harness, golden/fixture +> tests, the two ✅ steps) still stand; the `[ ]`/`[~]` catalog phases are reframed by v2. + +Each step is small, behavior-preserving, and must leave the gate green +(`lein test` + `lein lint`) before commit. Code lands on this branch. + +- [x] **Merged `feature/name-keyword-fix`** (commit `ec26955`) — catalog work now sits on + the stable-key fix. Clean auto-merge; gate green (220/1092/0, lint 0). **Live/E2E + verification still needed** (JVM gate doesn't run cljs subs or the app): + see `docs/kb/content-extensibility-e2e.md`. +- [x] **Live E2E verification (PR #28): all PASS / covered, no regressions.** A + full-environment run (figwheel + browser + Datomic) confirmed the catalog seams + end-to-end: homebrew subrace under built-in Elf, subclass under built-in Sorcerer, + boon + invocation in the Warlock builder, byte-identical character round-trip. Item 1 + "failures" are pre-existing on `develop`; this branch's 18 added tests all pass. + Items 7/10 accepted as covered by `content-reconciliation-test/*` + the round-trip + golden. Fixture for the gaps: `test/extensibility-fixtures.orcbrew` (commit `f977ba9`). + **Merge is sequencing-blocked on #27** (name-keyword-fix) landing on `develop` first. + +- [x] **Setup** — toolchain (lein + deps), baseline gate green. +- [x] **Phase 0 — safety net.** `extensibility_golden_test.cljc` locks compat invariants + (name-to-kw key derivation; saved-character round-trip). Pure JVM. (212→ tests green.) +- [x] **Phase 1 — generic injector.** New leaf ns `option_catalog.cljc` (`by-parent`), + unit-tested = `group-by`; subraces re-pointed. (213 tests, lint 0 errors.) +- [x] **Phase 2 — subclasses** re-pointed to `by-parent`. (lint 0 errors.) +- [~] **Phase 3 — boons + invocations onto a catalog read.** + - [x] 3b. Added `plugin-options` to `option_catalog.cljc` (catalog read primitive), + JVM-unit-tested identical to the legacy `(mapcat (comp vals key) plugins)`; + `::classes5e/plugin-boons` and `::classes5e/plugin-invocations` routed through + it. Behavior-preserving; no keys/signatures changed. (214 tests, lint 0 errors.) + - [x] 3a. (guard) `extensibility_golden_test.cljc` now builds boon/invocation options + via `pact-boon-options`/`eldritch-invocation-options` (real spell data) and locks + the built-in + homebrew option keys and the `:pact-boon`/`:eldritch-invocations` + selection keys. (217 tests green.) + - [ ] 3c. (RISKY — deferred) Stop threading `boons`/`invocations` as positional args: + inject them as a post-step (like subraces→races) or via an ambient ctx map. + Keys MUST stay identical; 3a guards it. Approach carefully; cljs assembly is + lint+review-only here. +- [ ] **Phase 4 — Layer 1 registration/indexing registry (the "8 files → 1 descriptor" + win). Existing types only; one subsystem per commit.** + - [x] 4a. Created leaf `content_types.cljc` registry (13 plugin-based types; + magic-item + combat excluded as non-plugin). `content_types_test.cljc` audits it: + every `:spec` resolves via `spec/get-spec`, every `:plugin-key` satisfies the + orcbrew `::e5/content-keyword` contract, identity fields unique. (220 tests green.) + Built from an agent inventory; the get-spec/contract checks auto-verified it. + - [x] 4b. subs: the 13 `::/builder-item` passthrough subs are now generated by a + loop over the registry (`spell_subs.cljs`). JVM guard `content_types_test/ + builder-items-match-the-subs` locks the set against drift. Provably the same 13 + keys; lint clean, 224 tests green. cljs behavior (builder forms load) → e2e. + - [ ] 4c. db: build `default-value` slots + `reg-local-store-cofx` from the registry. + - [ ] 4c. db: build `default-value` slots + `reg-local-store-cofx` from the registry. + - [ ] 4d. events: generate `set-`/`reset-` + `reg-*-homebrew` calls from the registry. + - [ ] 4e. routes: derive bidi tree + route sets + `routes.clj` allowlist (keep the + `(def …-route :kw)` lines). + - [ ] 4f. core: build the `pages` map from the registry. + - [ ] Gate each: app boots, NO route/event/sub/localStorage key renamed, golden green. +- [ ] **Phase 5 — prove it with a new builder.** + - [ ] 5a. Fighting-style builder (easier): `fighting-style-options` → catalog; + `fighting-style-selection` → grant-with-filter; descriptor + spec + form. + - [ ] 5b. Lineage/ancestry builder (harder): convert `dragonborn-option-cfg` def→fn, + catalog, plus breath-weapon/resistance modifiers (real domain work). + - [ ] Gate: golden green (existing unaffected) + a test that an imported homebrew + fighting style / lineage appears under its parent. + +Honesty note: the JVM gate does not run the `.cljs` subscription code. For cljs-only +edits I rely on `lein lint` + the `.cljc` unit tests + manual review; the risky logic is +kept in `.cljc` (`option_catalog`, option fns) precisely so it IS JVM-tested. + +Note: code is landing on this branch (the only authorized push target). Docs are meant +to split-commit to `agents/develop`; production code would normally go on a code branch +off `develop` — confirm the target before merging. + +## Deferred follow-ups — HIGHLIGHT AT BRANCH CLOSE + +These are intentionally **not** done on this branch and **must be surfaced when this +branch is finalized / PR'd** (don't let them vanish into the diff): + +1. **Character-validation contract** (own branch). The computed character is the one + user-facing representation with no validation; the *intent* and a falsifiable charter + are preserved in `docs/kb/character-validation.md`. Implement on its own branch. +2. **Get the ClojureScript tests into CI** (own branch). CI runs only the JVM gate, so the + cljs suite is unrun and has rotted (`docs/kb/test-suite-state.md`). This is the root + fix; it also lets future cljs changes be gated instead of hand-verified. Pairs with #1. + +When putting a bow on this branch, repeat these two items in the PR description / handoff. + +## Workflow +This branch is based on the leaner fork line, not `agents/develop`, so file +references in the docs use the monolithic `views.cljs`/`events.cljs` layout. The docs +flag this. Intent is to **split-commit these docs onto `agents/develop`** later. + +When split-committing to `agents/develop`, also add index rows for the two new docs +to `docs/kb/README.md` there (not done here — this branch's index differs from +`agents/develop`'s, so editing it here wouldn't carry over cleanly). + +## Handoff Notes +- **Coordinate with `feature/name-keyword-fix`** (forks from the same base `d42e05d`). + It establishes: identity keys derive from stable ids (`:class-key`, stored `:key`), + NOT display names; `option-cfg` has a `::plugin-source` slot; a reconciler heals + orphaned keys. Both branches touch classes.cljc / options.cljc / spell_subs.cljs / + events.cljs / template.cljc — expect overlap and align on its stable-key approach. +- **Two standing rules for the catalog/grant phases (3c+):** (1) pass each item's stored + `:key` to `option-cfg` — never re-derive identity from a display `:name`; (2) catalogs + are layered, memoized `reg-sub`s referenced by grants — never recomputed in hot subs. + Guard both with comments. (Decisions D10/D11; details in the design + compatibility docs.) +- **Working agreements (apply to all work here):** + - *Tests must be falsifiable.* Every test must go red if the production code it covers + breaks. No theater (a test that only asserts `(spec/valid? my-spec my-input)` tests the + spec against examples, not the system). Gut check: "if I break the code, does this fail?" + - *Fix bugs on sight.* Don't leave a bug lying around once found — fix it in-flight, + UNLESS it's deep enough to warrant its own branch (then file it and scope it). +- **Test-suite debt found this session (`docs/kb/test-suite-state.md`):** CI runs only the + JVM gate (`lein lint`/`lein test`); the cljs suite is never run and has rotted. A + **headless cljs harness now exists in this container** (compile `fig:test` → serve + `target/test/` → drive Chromium via Playwright → capture the clean reporter), so cljs is + verifiable here. **`save-character` null crash: FIXED + verified** (errors 3→2). +- **Import-validation triage — TRIAGED + FIXED (`86eb5cc4`), harness-verified:** + - `apply-key-renames` test → was STALE (`:old-key`/`:new-key`); **fixed test** to `:from`/`:to`. + - `normalize-text café→cafe` → was STALE/WRONG (accents are preserved + flagged); + **fixed test** to expect `"Café"`. + - `count-non-ascii` → REAL cljs bug (`(int %)`=0 in cljs); **fixed code** → `(.charCodeAt % 0)`. + - `dedup-options-in-import` → REAL bug (mechanism pinned): `dedup-options-in-item` only + handled `:selections`-nested options, not a homebrew `:orcpub.dnd.e5/selections` item's + own top-level `:options`; **fixed code** (additive). Full-pipeline dedup now works. + - Verified: headless cljs run 133 tests / **1 failure / 0 errors** — only the unrelated + `user-stale-user` (subs auth guard) remains. lint 0; JVM 224/1107/0. + - **Still open (not import, out of this list's scope):** `user-stale-user` subs auth-guard + test (1 failure) + the dead `character_test.cljc` (2 errors, retire per the charter). +- The KB requires verified-only content. The cross-link map is verified from code; the + proposed design is clearly labeled as a proposal. Preserve that boundary. +- The design directly answers a cluster of open issues (#58, #57/#209, #172/#170, + #210/#107, #280, #173, #128) listed in `docs/issues/homebrew-builders.md` on + `agents/develop`. +- Conversation context that produced these docs is not preserved elsewhere; the two + KB docs are the durable record. + +## Related Docs +- `.claude/summaries/2026-06-13-content-extensibility.md` — session summary / handoff +- `docs/kb/content-extensibility.md`, `docs/kb/content-extensibility-decisions.md`, + `docs/kb/content-extensibility-compatibility.md`, `docs/kb/content-extensibility-plan.md`, + `docs/kb/content-extensibility-e2e.md` (live verification checklist for a VS Code agent) +- `docs/kb/test-suite-state.md` — verified state of the test suites, the pre-existing cljs + failures (classified), the `::character`/built-character spec findings, open decisions +- `docs/kb/verification-discipline.md` — lessons on assumptions & thoroughness (verify + against callers/intent/runtime before asserting; "red test = disagreement, not bug") +- `docs/kb/character-validation.md` — preserves the *intent* of validating a character + (Larry's 2016 test) + the modern, falsifiable replacement charter (own-branch). Capture + this before retiring the broken `character_test.cljc`. +- `docs/kb/built-character-representation.md` — **load-bearing gotcha:** the built/computed + character is a map of deferred `:entity-fn?` values (read via `entity-val`), NOT a flat + map; don't `spec/keys` it. Anchored in code on `entity-val`/`build`/`built-character`. +- Cross-references: `docs/kb/spa-routing-architecture.md`, + `entity-options-architecture.md`, `srd-vs-plugin-content.md`, + `views-builders-split.md`, `docs/issues/homebrew-builders.md` (all on `agents/develop`) diff --git a/docs/README.md b/docs/README.md index 8c9941500..06cb5170f 100644 --- a/docs/README.md +++ b/docs/README.md @@ -13,6 +13,10 @@ Guides for developers and power users working with OrcPub's homebrew content sys **Agent Knowledge Base:** - [📚 KB Index](kb/README.md) - Verified findings from deep investigations - [💥 Datomic Crash Analysis](kb/datomic-crash-analysis.md) - Root cause, frequency, fix options +- [🔗 Content Cross-Links](kb/content-extensibility-cross-links.md) - How content aspects inject into each other, mapped to the target catalog/grant shape + +**Architecture Initiatives:** +- [🧩 Content Extensibility](extensibility/README.md) - Reducing the multi-file cost of adding content (registry + catalogs/grants); handoff, decisions, target architecture **For Developers:** - [🚨 Error Handling](ERROR_HANDLING.md) - Error handling utilities diff --git a/docs/branch-changelog.md b/docs/branch-changelog.md new file mode 100644 index 000000000..5612425fa --- /dev/null +++ b/docs/branch-changelog.md @@ -0,0 +1,162 @@ + + +## [content-extensibility + ability-score-increase-spreads] + +Make homebrew content extensible and cross-silo: turn "just text" into real mechanics through one +shared abstraction instead of per-silo bespoke code. Two themes — a declarative content-builder +framework (field-schemas + a pool/grant primitive) and a concrete cross-silo feature built on it +(ability-score-increase spreads) — plus homebrew-source surfacing and a foundation of +characterization tests. + +### Features + +- **Declarative builder field-schemas** (`f32790b1`, `de0cf37f`, `da2f63d8`) + A content type declares its fields once (`field_schemas.cljc`, `content_types.cljc`); the builder + form, the save spec (optional-by-default), and import/export verification are all generated from + that one declaration. Fixes the breath-weapon bug at the source rather than per-form. The Draconic + Ancestry builder is the proof — its whole form is a field schema. + +- **Pool + grant primitive** (`c1f54967`, `acaa131d`, `c67006e9`) + A generic content-injection seam (`content_pools.cljc`, generalized from the per-bucket + `:fighting-style` hook into a generic `:grant`). Opens a homebrew pool (e.g. draconic ancestry) + with full mechanics, and lets one silo's content feed another (feat-granted fighting style proven + on a built character). + +- **Ability Score Increase spreads** (`b99a7b94`, `b89aa006`, `f00394aa`, `287219fe`) + A content entry grants ASIs as one terse spread — a list of `[amount pool]` pairs, e.g. + `[[2 :cha] [1 :martial]]` (+2 CHA fixed, +1 to a player-chosen martial stat). Supports fixed, + floating, named groups (any/martial/mental), explicit choice-sets, and the 2024 "+2/+1 to different + abilities" rule. Works in **races, subraces, backgrounds** (2024 ASI-via-origin), and **subclasses** + (non-standard, behind an opt-in toggle). One shared authoring widget across every silo (no + duplication); fixed increases compile to racial-ability modifiers, floating ones to a choice slot + the character builder renders. Each silo's ASI stays contained to its own source and stacks + correctly when several grant one (proven via entity-path containment). Terse on the wire (ships in + every homebrew pack); self-documenting in source. + +- **Save-proficiency grants (rider + standalone tool)** + Two orthogonal ways to grant saving-throw proficiencies, both compiling to the one save primitive + (`modifiers/saving-throws`): a per-increment `:save` rider on the ASI spread + (`[[1 :martial :save]]` → +1 to a chosen martial stat *and* its save — the Resilient pattern, + opt-in so the default stays bump-only), and a standalone `:save-proficiencies [[count pool]]` field + for saves on a different stat than the bump, or with no bump at all (fixed, or "choose N distinct + saves from a pool"). Wired into every silo through a single merged hook (`compile-ability-grants`): + races, subraces, backgrounds, subclasses, and feats. Same-stat overlap collapses to one proficiency + (set semantics — no double bonus); the builder runs `save-coverage-warnings` over the entry being + authored and shows an inline warn-and-explain note for redundant/overlapping save coverage. + +- **Feats consume ASI spreads (dual-format reader)** + `feat-option-from-cfg` reads `:ability-increases` by shape: a vector is the new cross-silo spread + (routed through `compile-ability-increases`, like the other silos), a set is the legacy feat format + (`#{:str :con}` + the optional `:saves?` save-proficiency marker). The legacy path is left untouched — + released feat data keeps working verbatim, including saves — and homebrew feats can now grant + fixed/floating/grouped spreads. (The spread now models saves via the `:save` rider above, so the only + remaining step is migrating the *released* feat `:saves?` set onto it — a deliberate data migration + tracked in the backfill ledger, not a missing capability.) + +- **Draconic Ancestry homebrew builder** (`0aca6113`) + End-to-end builder for a homebrew draconic ancestry, built entirely from a field schema. + +- **Strict-mode import** (`e4614519`) + Optional toggle for creators/devs: report missing required fields instead of silently auto-filling + them. + +- **Homebrew source on class names** (`8f94a94c`) + Opt-in `show-class-source-suffix` preference surfaces which homebrew pack a class came from; plugin + source is carried as its own slot through the option config rather than folded into the name. + +### Bug Fixes + +- **Homebrew floating ASI choice now renders** (`8e331564`) + The builder only rendered ability selections keyed `:asi`; a homebrew floating ASI used a different + key, so the choice never appeared. Now keyed correctly and rendered. + +- **Builder dropdown string coercion** (`050dbb95`, `64db2448`) + `` + filters) compiling to existing primitives; the Magic-Initiate special case; open scope flags. | Mixed — verified + DESIGN | +| [class-features-and-mechanization.md](class-features-and-mechanization.md) | How class features are structured (inline, class-coupled, captured code), the rolling layer + mechanization ceiling, the registry/`compile-feature` direction (data spec → real fighter/rogue output, overridable fields). | Mixed — VERIFIED/USER-REPORTED/SPECULATION flagged | +| [class-feature-catalogue.md](class-feature-catalogue.md) | Per-class inventory of all 12 base classes (C1): distinct auto-features, sizing, and the odd cases the registry must handle (multi-source counts, resource pools, build-context interpolation, multi-part features, attribute interdependence). | High — all 12 read | +| [building-a-class-from-builders.md](building-a-class-from-builders.md) | What a homebrew class can be assembled from today: the `homebrew-class` spec, what `subclass-option`/`spellcasting-template` accept, the invocation/boon pool pattern, the `ua_artificer` witness, and the real gaps. | High — code | +| [armor-class-computation.md](armor-class-computation.md) | How AC is computed (max-of-alternatives + sum-of-bonuses), the channels, custom-AC friction. Test-backed (`ac_characterization_test`: armored dex-cap, unarmored tie-break, natural-AC duplication; the `:max-dex-mod`-ignored + cljs-nil-add findings). | High — traced + tested | +| [runtime-toggles-and-conditional-modifiers.md](runtime-toggles-and-conditional-modifiers.md) | How a player toggle (equipped armor/magic items) changes computed sheet stats — the `equipped?`-flag + deferred-modifier mechanism; basis for "while active" features. | High — code | +| [built-character-representation.md](built-character-representation.md) | **Load-bearing gotcha:** the built/computed character is a map of deferred `:entity-fn?` values (read via `entity-val`), NOT a flat map — don't `spec/keys` it. | High — code | + +### Process & infrastructure +| Document | Topic | +|----------|-------| +| [verification-discipline.md](verification-discipline.md) | Lessons on assumptions/thoroughness + the **standing rule**: don't call it verified without walking it up and down and backing it with a falsifiable test (or the full chain); and how a characterization test doubles as the old-vs-new comparison instrument. | +| [data-safety-layers.md](data-safety-layers.md) | **Standing design rule** for robustness against bad data: the four layers (prevent / harden / heal / surface), the rule that picks between them by lifecycle, the `save ⊆ load` + diagnosable-rejection invariants, worked examples, and anti-patterns (silent-drop, speculative-heal). From the cross-branch toggle-nil review. | +| [cljs-headless-harness.md](cljs-headless-harness.md) | How to run the cljs test suite headless in a container (compile `fig:test` → serve `target/test` → Playwright Chromium) — the gate for cljs-only code; plus the full-app click-through E2E (`test/e2e/`). | +| [dropdown-value-coercion.md](dropdown-value-coercion.md) | The `` coercion) into one pack, then survive **export → + cleared browser (`localStorage.clear()`) → re-import**, both spreads intact, then both render, + attribute to their own container, and stack. No seeded localStorage — the round-tripped data is what + the front-end forms actually produced. diff --git a/docs/kb/armor-class-computation.md b/docs/kb/armor-class-computation.md new file mode 100644 index 000000000..d5bf02fa4 --- /dev/null +++ b/docs/kb/armor-class-computation.md @@ -0,0 +1,115 @@ +# Armor Class computation + +How AC is calculated, the channels a feature can plug into, and what that means for +custom AC (natural armor, unarmored defense). Markers: **VERIFIED** = read from code, +file:line cited. **DESIGN** = proposal, not built. + +## Verified behavior — TEST-BACKED (`ac_characterization_test.clj`, JVM) +The model below is pinned by characterization tests (built characters + direct `?armor-class-with-armor` +invocation), so the numbers are the real ones, not prose: +- **Unarmored:** fighter 10+Dex(2)=12; monk 10+Dex+Wis=15; barbarian 10+Dex+Con=15. +- **Armored (fighter, Dex 14):** leather (light) 13 = 11+full Dex; scale mail (medium) 16 = 14+min(2,Dex); + chain mail (heavy) 16 = 16+0 Dex; chain mail + shield 18; no-armor + shield 14. +- **Two Unarmored Defenses (monk + barbarian):** the `(first ?unarmored-defense)` gate applies **one** + ability adder (AC 15), it does **not** stack both (which would be 18). +- **Natural-AC `:props` duplication:** `:lizardfolk-ac` and `:tortle-ac` emit the *same shape* — a + `?natural-ac-bonus` + a bespoke `?armor-class-with-armor` override (one parameterized `:natural-ac` + handler should replace both); they feed the same `?natural-ac-bonus` channel the sorcerer Draconic + Bloodline uses (`classes.cljc:2270`). + +**Two findings surfaced by the test (verified):** +1. **Dex cap is by armor TYPE, not the armor's `:max-dex-mod` field.** `?armor-dex-bonus` keys off + `(:type armor)` (light/medium/heavy → full/min-2/0); the `:max-dex-mod` value present on each armor in + `armor.cljc` is **ignored** by the AC fn. Consistent for SRD armor (medium=2, heavy=0), but a homebrew + armor with a non-standard `:max-dex-mod` would not be honored. +2. **The armored branch relies on cljs nil-arithmetic.** It does `(+ … (::mi5e/magical-ac-bonus armor) …)`; + non-magical armor lacks that key → `nil`. cljs treats `nil` as 0 in `+` (so production is fine); under + the JVM that NPEs, so the test supplies an explicit `magical-ac-bonus 0`. A JVM-ism to be aware of + (verification-discipline lesson 5). + +**The "up" trace (how the displayed AC picks armor) — VERIFIED:** `?armor-class-with-armor` is the logic; +the cljs sub `::current-armor-class` (`subs.cljs:769`) calls it with the **worn** armor/shield, else falls +back to `::best-armor-combo` (`:760`) which takes `(max-key :ac …)` over every owned armor×shield combo +(incl. nil). So the cljc fn is the whole computation; the cljs layer only chooses which armor to feed it. + +## The model — VERIFIED (`template_base.cljc:35-88`) +AC is computed per equipped armor/shield combination, layered: + +``` +?armor-class = 10 + Dex ; the simple displayed unarmored value +?base-armor-class = 10 + Dex + + (if (> ?unarmored-ac-bonus ?natural-ac-bonus) 0 ?natural-ac-bonus) + + ?magical-ac-bonus +?armor-dex-bonus (fn) = light: full Dex, medium: min(?max-medium-armor-bonus, Dex), heavy: 0 +?shield-ac-bonus (fn) = 2 + shield's magical bonus +?unarmored-armor-class = ?base-armor-class + ?unarmored-ac-bonus + ?ac-bonus +?armor-class-with-armor-base (fn [armor shield]): + no armor, no shield -> ?unarmored-armor-class + no armor, shield -> ?unarmored-with-shield-armor-class + armor -> shield + armor-dex-bonus + ?armored-ac-bonus + armor base-ac + + magical + ?ac-bonus + ?magical-ac-bonus +?armor-class-with-armor (fn [armor shield]): + (apply max (?armor-class-with-armor-base armor shield) + (map #(% armor shield) ?ac-fns)) ; MAX over base and alternatives + + (sum of (map #(% armor shield) ?ac-bonus-fns)) ; then ADD bonuses +``` + +## The channels a feature can use — VERIFIED +- **`?ac-fns`** — alternative full AC formulas. The final AC takes the **max** of the base + formula and every `?ac-fns` entry. This is where a from-scratch custom AC goes + ("13 + Dex", "10 + Con"). +- **`?ac-bonus-fns` / `?ac-bonus` / `?armored-ac-bonus`** — additive bonuses applied **on + top** of the max (Defense's +1 while armored, Mariner's conditional +1). +- **scalar accumulators** `?unarmored-ac-bonus`, `?natural-ac-bonus`, `?magical-ac-bonus`, + `?unarmored-with-shield-ac-bonus` — folded into the base formula. With-shield is a + separate accumulator from without-shield. + +Picking the wrong channel breaks the math: a replacement put in a bonus channel +double-counts; a bonus put in the max channel gets dropped. + +## Where each kind of custom AC goes — VERIFIED mechanism +- **Natural armor** (Lizardfolk 13+Dex, maxes against worn armor): override + `?armor-class-with-armor` to max its formula against the normal one. Pattern in + `template.cljc:284`. Sets `?natural-ac-bonus`. +- **Unarmored defense** (10 + Dex + ability, no armor): add the second ability via + `?unarmored-ac-bonus`, gated on being the active unarmored source. Barbarian + (`classes.cljc:70`, adds Con), Monk (`:1262`, adds Wis). +- **Flat bonus** (+X while armored / conditionally): `?armored-ac-bonus` or `?ac-bonus-fns`. +- **Static AC** ("your AC is N"): an `?ac-fns` entry returning a constant, maxed in. + +## Known friction — VERIFIED facts (the "issues" are analysis) +- **Pairwise tie-break is hardcoded.** `?base-armor-class` contains + `(if (> ?unarmored-ac-bonus ?natural-ac-bonus) 0 ?natural-ac-bonus)` — it reconciles + exactly two named sources. A third (custom) source isn't included automatically; that + expression would need extending. The `?ac-fns` max channel generalizes; this scalar + comparison does not. +- **Per-class ability is hardcoded, and a config for it is half-wired.** Monk declares + `:unarmored-abilities [::char5e/wis]` (`classes.cljc:1241`) but the AC modifier hardcodes + `(?ability-bonuses ::char5e/wis)` (`:1263`) — the config exists and isn't read. +- **Multiclass: first source wins.** The ability bonus is gated on + `(= :class (first ?unarmored-defense))`, so Barbarian + Monk unarmored don't stack. A + custom unarmored defense must participate in that ordering. +- **Reconciliation is max-based**, so a true override (force a specific AC even if lower) + isn't expressible. Most 5e AC features are "use the better", which max handles. + +## Design proposal — DESIGN, not built +A creator should not see the channels. Map a small set of **categories** to channels: +- Natural Armor (base number + optional ability, maxes against worn armor, shield allowed?) +- Unarmored Defense (10 + primary ability + optional second ability, no armor, shield allowed?) +- Flat AC bonus (value, optional condition) +- Set AC (constant) + +Parameters that cover the space: base number (default 10), primary ability (default Dex — +changing it is "replace Dex"), optional second ability, works-in-armor?, shield allowed?, +alternative-vs-bonus. The compiler routes the parameterized result to the right channel. +Real 5e AC features cluster into these ~4 shapes, so bounded templates are likely +sufficient and avoid the edge cases a fully dynamic "selections reshuffle the stack" +builder would hit. + +Structural improvement worth considering: replace the hardcoded pairwise tie-break with a +uniform list of AC contributions, each tagged alternative (max) or bonus (add), reconciled +generally. That makes a new AC source data-driven instead of an edit to `?base-armor-class`. +Low-risk first step: wire the AC modifier to read `:unarmored-abilities` instead of +hardcoding the ability — it serves Monk and custom unarmored defense at once. Caveat: AC is +load-bearing and many features touch it, so any reconciliation refactor needs regression +coverage first. diff --git a/docs/kb/backfill-ledger.md b/docs/kb/backfill-ledger.md new file mode 100644 index 000000000..39221295c --- /dev/null +++ b/docs/kb/backfill-ledger.md @@ -0,0 +1,46 @@ +# Backfill ledger + +Tracks bespoke paths being converged onto the systematic **pool/grant** standard (D29) and the code +deprecated along the way (D34). The rule is **one mechanism per job**; this ledger draining to zero is the +"uniform, no surprises" guarantee — every superseded path is visible and scheduled, never silently +duplicated or forgotten. + +## When something lands here +A bespoke path becomes a ledger item the moment a pool/grant capability can do its job. Adding a pool/grant +capability **requires** logging the paths it now subsumes. + +## Migration recipe (per item) +1. **Pin** the bespoke path's current behavior in a characterization test (if one doesn't exist). +2. **Re-point** the job at the pool/grant. +3. The **same test stays green** (uniformity without blind regression — D29 gate). +4. **Deprecate, don't delete** (D34): `#_`-discard the bespoke fn under the note — + ```clojure + ;; DEPRECATED — superseded by ; behavior pinned by . Remove after . See backfill-ledger.md. + #_(defn the-superseded-thing [...] ...) + ``` +5. **Add a row** below. +6. **Removal sweep:** once past `remove-after`, grep `DEPRECATED`, delete the struck form, mark the row Removed. + +## Ledger +| Function (file) | Superseded by | Deprecated on | Remove after | Pinning test | Status | +|---|---|---|---|---|---| +| _(none yet — no bespoke path has been migrated)_ | | | | | | + +## Watch-list (candidates — pool/grant doesn't fully subsume them yet) +- **Fighting-style grant registry is hardcoded.** `grant-selection` is fed a literal + `{:fighting-styles {…opt5e/fighting-style-options}}` at `template.cljc:1549` (built-in styles only). + Wiring it to `content-pools/pool` (so homebrew fighting-style packs are grantable) is the first real + pool/grant expansion — and the point at which any bespoke fighting-style / feat-grant path it then + subsumes becomes a ledger item. +- **Feat ASI is dual-format, not fully converged.** `feat-option-from-cfg` reads BOTH the legacy feat + set (`#{:str :con}` + `:saves?`) and the cross-silo spread (`[[amount pool]]`), dispatching on shape + (`options.cljc`; tests `ability_increase_grant_test/feat-*`). The legacy set path is **kept, not + deprecated**. Converging it (route the set through `compile-ability-increases`) is now **unblocked on + two of three fronts** — the `:save` rider models `:saves?`, and the per-silo `:attribution :general` + fix means a feat's fixed +N no longer mis-attributes as racial. The **remaining** blocker is + save-compat: the legacy feat choose-selection keys options by the ability (`::char/str`) while the + spread keys them `asi--`, so a naive migration drops saved feat picks on reload — it + needs a load-time key reconciliation (same pattern as spell-selection reconciliation) + a D34 + characterization test. Deliberate two-readers state until then; **do this post-merge**, not before. + +See: D29 (one mechanism per job), D30 (grant = thin compiler), D34 (deprecation mechanics). diff --git a/docs/kb/building-a-class-from-builders.md b/docs/kb/building-a-class-from-builders.md new file mode 100644 index 000000000..b365294d4 --- /dev/null +++ b/docs/kb/building-a-class-from-builders.md @@ -0,0 +1,65 @@ +# Building a class from the builders — verified capability + gaps (Artificer worked example) + +What a homebrew **class** can actually be assembled from today, traced from code (file:line cited), using +Artificer (non-SRD — cannot ship, must be user-built; see D28) as the stress case. This is verified +*mechanics* reference, not design. Companion to `class-feature-catalogue.md` (feature shapes), +`spell-slot-progression.md` (the slot gap), and the content-extensibility track (pools/grants). + +Markers: **VERIFIED** = read from code, file:line cited. **GAP** = a capability a full Artificer needs +that the builders don't expose yet. + +## The capability witness — VERIFIED +`src/cljc/orcpub/dnd/e5/templates/ua_artificer.cljc` is a complete (older UA) Artificer expressed in the +**existing primitives** — `opt5e/class-option` cfg, a `:spellcasting` block, multiselect selections, +`level-val` scaling, tool profs, subclasses. It is **entirely `#_`-commented** (dead): proof the engine +*can* express the class shape, but NOT shippable (non-SRD, same rule as Maneuvers/Mariner). So the gap is +the **authoring layer**, not the engine (matches D18). + +## What a homebrew class/subclass can already express — VERIFIED +- **Spec is minimal**: `::homebrew-class` requires only `name`/`key`/`option-pack` (`classes.cljc:21`); + everything else is the option cfg. +- **`subclass-option` accepts the full shape** (`options.cljc:2558`): `:profs`, `:selections`, + `:spellcasting`, `:modifiers`, `:level-modifiers`, `:traits`, `:prereqs`, and `:levels {N {:modifiers + :selections}}` (level-gated features + choices). A homebrew class has the same richness. +- **Spellcasting is data**: `spellcasting-template` (`options.cljc:702`) turns a `:spellcasting` + `{:level-factor :cantrips-known :spells-known :known-mode :ability :spell-list}` into full/half/third + casting with real spell choices and an **optional custom spell list** assoc'd under the class key. +- **Homebrew builders already exist** for class, subclass, **invocation**, **boon**, and a generic + **selection** (pool) — `content_types.cljc:82/90/111/141/149`. +- Homebrew builder classes get **ASI/hit-points automatically** (`options.cljc:2787`). + +## Infusions ≈ the invocation/boon pool pattern — VERIFIED +Infuse Item is the load-bearing Artificer piece, and it's structurally the **Eldritch Invocations** +pattern, already in the codebase: +- `eldritch-invocation-selection` (`options.cljc:3109`) is a `selection-cfg` with `:multiselect? true` + and a `:ref`, options supplied by a **plugin arg** (warlock-option takes `invocations`/`boons`). +- The (dead) `eldritch-invocation-option` macro (`options.cljc:3160`) shows each option = `prereqs` + + `modifiers` + a typed trait (action/bonus-action/reaction/dependent-trait). +- The (dead) `wonderous-invention-selection` (`ua_artificer.cljc:13`) is the same multiselect pool whose + options grant **magic items** via `mod5e/magic-item` — i.e. an infusion pool **reuses the magic-item + homebrew system**. +So "Infusions" = a multiselect pool of item/effect-granting options + a scaling count + replace-on-rest. +The substrate (multiselect pool, plugin-fed options, magic-item grant) exists; it is **not yet a +first-class, named, author-declarable pool** (the open lever — see the content-extensibility track and the +D29 grant question). + +## Feature-to-mechanism map (what's data entry vs a real gap) +Data entry into existing builders: chassis/profs/ASI/saves/equipment; text traits (Magical Tinkering, Right +Tool, Magic Item Adept/Savant/Master, Soul of Artifice); Tool Expertise (`?tool-expertise` / +`mod5e/tool-expertise`, used in `ua_artificer.cljc:142`); subclass features (non-companion). + +- **GAP — spell-slot progression.** Artificer half-casts from level 1; `:level-factor` can't express it + (overloaded — see `spell-slot-progression.md` / D27). Needs the bucket-of-tables + declared multiclass + rule. +- **GAP — Infusions as a first-class scaling/swappable pool** (the keystone tool; the pattern exists, the + authoring affordance doesn't). +- **GAP — ability-derived frequency** (Flash of Genius = Int-mod uses/long-rest). The catalogue's + non-level use-count source (B3); without it the feature degrades to text. +- **GAP — companion link** (Battle Smith's Steel Defender, Artillerist's cannon). The monster builder + exists; a feature→companion **link** does not. The only Artificer piece with no existing analog. + +## Validation rule (D28) +Expressiveness is proven with a **synthetic stand-in** class (a made-up "Tinkerer": a scaling item-granting +pool + a half-from-1 slot table + an Int-mod reaction), **never** the real Artificer as a fixture — a +copyrighted fixture in the repo would itself be shipping the content. (Built-in PHB classes are SRD and fine +as fixtures; Artificer is not.) diff --git a/docs/kb/built-character-representation.md b/docs/kb/built-character-representation.md new file mode 100644 index 000000000..bd568ed37 --- /dev/null +++ b/docs/kb/built-character-representation.md @@ -0,0 +1,54 @@ +# The Built Character is NOT a flat map (entity-spec / entity-val) + +**Purpose:** A foundational gotcha that has caused real bugs and a lot of confusion in +this session. Read this before you `get`, `spec/keys`, iterate, or otherwise treat a +"built"/computed character as a plain map. + +**Status:** Verified from code. + +## One-liner + +The **built character** (output of `entity/build`) is a **map whose *derived* values are +deferred functions**, not a flat map of realized values. Read derived fields with +`orcpub.entity-spec/entity-val` (or the `q` / `?ref` macros) — **not** plain `get`. + +## How it actually works (verified) + +- `entity/build` (`src/cljc/orcpub/entity.cljc:620`) applies modifiers + (`orcpub.modifiers/apply-modifiers`) to a base entity, producing a map. +- Each value is **either a plain value or a deferred function** tagged with `:entity-fn?` + metadata. Computed/derived fields (those that depend on other fields) are the deferred ones. +- The accessor is `entity-val` (`src/cljc/orcpub/entity_spec.cljc:5`): + ```clojure + (defn entity-val [entity k] + (let [v (entity k)] ; entity is a map; (entity k) == (get entity k) + (if (:entity-fn? (meta v)) (v entity) v))) ; deferred fn? realize by calling (v entity) + ``` + So `entity-val` returns the *realized* value; a plain `get` on a deferred key returns the + **function itself**, not the value. + +## What this means for you + +- **Some keys are plain** — e.g. `:base-abilities` is read with `get-in` in + `events.cljs`. **Many derived keys are not** — `get`/`get-in` on those returns a + function. Use `entity-val` / `q` / `?ref` for anything computed. +- **Do not `spec/keys` the built character as a flat map.** Its deferred values are + functions, not their realized values; a whole-structure spec would have to realize every + field via `entity-val`. This is *why* the computed character has **no clojure.spec spec** + (see [character-validation.md](character-validation.md)), and why Larry's 2016 flat + `::character` spec could not survive the move to this representation. +- **Terminology overload:** "entity spec" / `entity-spec` (`es`) here is this + **build/compute engine**, NOT `clojure.spec` validation. Two unrelated things both called + "spec." + +## Where it bit us (this session) + +- The `save-character` null crash: `make-summary` realized fields on a character missing + abilities and blew up (`entity-val → character.classes`). Fixed by gating on abilities + before `make-summary`. See [test-suite-state.md](test-suite-state.md) §2/§4. + +## Anchored in code + +Short pointers to this doc live on `orcpub.entity-spec/entity-val`, `orcpub.entity/build`, +and the `built-character` subscription (`subs.cljs`), so this is findable from the code, +not only the KB. diff --git a/docs/kb/character-validation.md b/docs/kb/character-validation.md new file mode 100644 index 000000000..960be62d8 --- /dev/null +++ b/docs/kb/character-validation.md @@ -0,0 +1,67 @@ +# Character Validation — intent, history, and modernization charter + +**Purpose:** Preserve the long-standing intent of *validating a character* so it isn't +lost when the broken `character_test.cljc` is retired, and define the modern, falsifiable +replacement. The intent matters: the computed character is the source of the character +sheet + PDF + the saved record, and right now nothing validates it. + +**Status:** History and the gap are **verified** (code + unshallowed git). The +modernization (the "Charter" section) is a **PROPOSAL — not implemented**; it's the +spec for an own-branch effort. Don't treat it as built. + +--- + +## The intent worth keeping (do not lose this) + +"A valid character passes validation; a malformed one (e.g. missing an ability score) +fails — visibly and early." That guard has existed since the original codebase and is +genuinely important: a malformed computed character should not silently flow into the +sheet, the PDF, or the server save. This is the idea to carry forward, independent of +any particular implementation. + +## History (verified) + +- Larry's `test-character-spec` (`a7ee3d32`, 2016-12-23) validated the **flat computed + character** against: + `(spec/def ::character (spec/keys :req [::abilities ::savings-throws ::speed ::darkvision ::initiative]))`. +- That `::character` spec was removed in the early entity/`from-strict` refactor and the + test was never updated → it has been dead for years, visible only under `fig:test` + (which CI doesn't run). It also collides on namespace with `character_test.clj`. +- Today the **computed/built character has no clojure.spec validation**. Only the *raw* + and *strict* entity forms are spec'd (`::raw-character`, `::strict-character`). Full + detail: [test-suite-state.md](test-suite-state.md) §3–§4. + +## Why it can't be revived verbatim (verified) + +The built character is **a map whose derived values are deferred `:entity-fn?` functions** +(realized via `es/entity-val`), not a flat map of realized values — full detail in +[built-character-representation.md](built-character-representation.md). A `spec/keys` over +it doesn't fit, and a naive whole-structure spec would be awkward and potentially expensive +(it'd force realization of every field). So the original flat spec can't simply come back — +the *intent* has to be re-expressed against the current representation. + +## Charter — the modern replacement (PROPOSAL) + +1. **One narrow contract** on the invariant computed fields the sheet/PDF/save actually + depend on — start with "base abilities present," grow to speed/HP/etc. only as each + earns its keep. Enforce it **at the chokepoint** (`make-summary` / save): the guard + *is* the spec applied where it matters. (The `save-character` crash fix — gating + `make-summary` behind the ability check — is the first installment of this guard.) +2. **The test must be real and falsifiable.** Drive `make-summary`/`save-character` with + (a) a valid built character → it succeeds, and (b) a malformed one (missing ability) → + it returns a graceful error, **not** a crash. It must go **red** if the guard is + removed. No theater: do **not** just assert `(spec/valid? my-spec my-handcrafted-input)` + — that tests the spec against examples, not the system. +3. **Stretch (optional):** an entity-val-aware validation of the broader computed + character, only if it proves worth the cost. + +This is an **own-branch** item (per the working agreements: deep enough to warrant its +own branch), best done alongside or after getting the cljs tests into CI so it can be +gated. + +## Disposition of the broken test + +With this charter captured, `character_test.cljc`'s `test-character-spec` can be safely +**retired** — it validates a representation that no longer exists — with a comment +pointing here. The **idea lives on in this doc + the save-path guard**, not in the dead +test. Do not retire it until this charter exists (it now does). diff --git a/docs/kb/class-feature-catalogue.md b/docs/kb/class-feature-catalogue.md new file mode 100644 index 000000000..cc73456b5 --- /dev/null +++ b/docs/kb/class-feature-catalogue.md @@ -0,0 +1,120 @@ +# Per-class feature catalogue (roadmap C1) + +Sizes the feature registry and surfaces the odd cases, by reading all **12** base-class option fns in +`src/cljc/orcpub/dnd/e5/classes.cljc`. Everything here is **VERIFIED** (read from code, file:line +cited) unless marked. Companion to `class-features-and-mechanization.md` (the structure + the +`compile-feature` proof) and `decision-vocabulary.md` (the wiring). + +Scope: the **auto-granted** features each class gets (the regression net's `:feature-names`). It +excludes (a) **scaling/padding** — `:ability-increase-levels`, `num-attacks` increments, `level-val` +dice growth, speed-bonus increments; (b) **choice-gated** features — Fighting Style, Expertise, +Metamagic, Favored Enemy, Hunter's Prey — which are `selections` (pools) already; (c) **spellcasting**, +a separate declarative `:spellcasting` subsystem (full/half caster via `:level-factor` 1/2). + +## The 12 classes (option fn line; distinct auto-features; notable shape) + +| Class | fn | Distinct auto-features (base class) | Notes | +|---|---|---|---| +| Barbarian | `:46` | Rage, Extra Attack, Fast Movement, Brutal Critical, Indomitable Might + 5 text traits (Reckless Attack, Danger Sense, Feral Instinct, Relentless Rage, Persistent Rage) | Rage = bonus-action w/ scaling uses **and** scaling damage in summary; Unarmored Defense (AC channel); capstone ability boost @20 | +| Bard | `:249` | Bardic Inspiration, Jack of All Trades, Song of Rest, Countercharm, Font of Inspiration, Superior Inspiration | spellcaster; Bardic count = **ability-derived**; Jack of All Trades = **multi-part** (skill-fn + initiative mod + trait) | +| Cleric | `:397` | Channel Divinity (+ Turn Undead), Destroy Undead, Divine Intervention | spellcaster; subclass @ **level 1**; Destroy Undead scaling is **strings** ("1/2"→4); summaries read `?spell-save-dc` | +| Druid | `:752` | Wild Shape, Druidic + 3 text traits (Timeless Body, Beast Spells, Archdruid) | **lean**; full caster; subclass @ 2; Wild Shape reads `?wild-shape-cr`/`?wild-shape-limitation` attrs (**string- and nil-valued** scaling), duration = **formula** (level/2 hrs) | +| Fighter | `:1052` | Second Wind, Action Surge, Indomitable, Extra Attack | the proof's worked example; cleanest case | +| Monk | `:1228` | Martial Arts, Ki, Flurry of Blows, Patient Defense, Step of the Wind, Deflect Missiles, Slow Fall, Stunning Strike, Stillness of Mind, Empty Body + ~6 text traits | **biggest**; **ki pool** (=level) spent as text everywhere; `?martial-arts-die` is a shared attribute; Martial Arts = **multi-part** (attack + bonus-action) | +| Paladin | `:1425` | Divine Sense, Lay on Hands, Channel Divinity, Divine Smite, Improved Divine Smite, Aura of Protection, Aura of Courage, Cleansing Touch, Divine Health, Extra Attack | half-caster; **Lay on Hands pool** (=5×level); Auras read `?paladin-aura` attr; Aura of Protection = **multi-part** (save mods + trait); frequencies **ability-derived** | +| Ranger | `:1771` | Favored Enemy, Natural Explorer, Primeval Awareness, Extra Attack, Foe Slayer + 4 text traits | half-caster; Favored Enemy/Natural Explorer summaries interpolate **user selections** (`?ranger-favored-enemies/terrain`) | +| Rogue | `:1978` | Sneak Attack, Cunning Action, Uncanny Dodge, Thieves' Cant | the proof's second worked example; Sneak Attack scaling = **formula** `round-up(level/2)` | +| Sorcerer | `:2206` | Sorcery Points, Flexible Casting | **lean**; full caster; **sorcery-point pool** (=level); subclass @ level 1 | +| Warlock | `:2994` | (none auto-granted before Mystic Arcanum @11); Eldrich Master @20 | **most choice-driven**; pact magic; **longer option-fn arity** (invocations, boons); base levels are *all selections* (invocations, pact boon, mystic arcanum) — almost nothing to extract | +| Wizard | `:2379` | Arcane Recovery, Spell Mastery, Signature Spells | **lean**; full caster; Arcane Recovery scaling = **formula** `round-up(level/2)`; Spell Mastery/Signature interpolate **selections** | + +**Sizing:** most classes sit at the predicted **~3–6 distinct auto-features**, but the spread is wider +than expected: **monk (~10) and paladin (~10) are outliers**; **druid/sorcerer/wizard (~2–3) are lean**; +and **warlock is essentially zero** auto-features (everything is a selection — invocations, pact boon, +arcanum). Rough total: ~50–60 base-class auto-features across the 12 (subclasses add comparable +shapes). The "~3–6 × 12" estimate in class-features-and-mechanization.md was right on average but +understates monk/paladin and overstates warlock. The migration is still bounded — dozens of features, +not whole level tables — but monk's ki economy is its own sub-project (see below), and warlock barely +participates (its identity lives in selections + pact-magic, not extractable auto-features). + +## Cross-cutting findings — the "odd cases" the registry/compiler must handle + +These extend the `compile-feature` proof, which today covers only level-schedule/literal `:uses` and +effect-param interpolation. Each is VERIFIED from the classes above. + +1. **Frequency/use-count has ≥4 sources, not just level schedules.** The proof handles a literal and a + `{level→n}` schedule. Real features also use: + - **ability-modifier-derived** — Bardic Inspiration `(max 1 (?ability-bonuses ::cha))` (`:300`), + Divine Sense `(inc cha-bonus)` (`:1488`), Cleansing Touch `cha-bonus` (`:1481`); + - **formula of level** — Lay on Hands `(* 5 (?class-level :paladin))` (`:1494`), Arcane Recovery / + Sneak Attack `round-up(level/2)` (`:2415`, `:2011`); + - **level itself** — Sorcery Points / Ki `(?class-level …)` (`:2247`, `:1289`). + So `:uses` needs to be a small expression form (literal · level-schedule · `:level` · ability-mod · + arithmetic), not just a schedule map. + +2. **Resource POOLS are distinct from per-rest counters, and are class-wide.** Ki (=level, `:1289`), + Sorcery Points (=level, `:2248`), and Lay on Hands (=5×level HP, `:1495`) are *pools* whose size is + a formula and which many features **spend** ("spend 1 ki", "spend 5 sorcery pts.", "expend 5 + points") — the costs live as prose in unrelated features' summaries. This is bigger than a single + feature's `:frequency`: it's a class-level resource + per-feature costs. The existing counter + (`features-used` + `clear-period`) is per-feature/per-period; modelling a shared spendable pool with + declared costs is a separate mechanism. **Monk's ki is the hardest single case** — nearly every monk + feature references it. + +3. **Summaries interpolate far more than level + the feature's own params.** The fill template in the + proof resolves effect params; real summaries pull from the **built character**: + - derived stats — `?spell-save-dc` (cleric/monk/paladin/sorcerer), `?ability-bonuses`, `?speed` + (Fast Movement `:103`), `?prof-bonus`, `?paladin-aura` (`:1473`); + - **user selections** — `?ranger-favored-enemies`/`?ranger-favored-terrain` (`:1811`/`:1815`), + `?sorcerer-draconic-ancestry` (`:2288`), `?spell-mastery`/`?signature-spells` (`:2421`/`:2429`); + - arithmetic — `(* 5 level)`, `round-up(level/2)`, `(* 3 level)`. + **Implication:** a production fill step needs access to the build context (the entity-spec + attributes), not just the feature's params. This is the real residue of the "summary" problem — not + string interpolation mechanics, but *what values the template can reference*. + +4. **Scaling values can be strings, not numbers.** Destroy Undead's CR table is `{5 "1/2" 8 1 …}` + (`:474`); Brutal Critical scales "one"/"two"/"three" (`:108`). `level-lookup` already returns any + value, so this is covered — but the fill rule's "numbers sign, strings pass" must treat these as + plain strings (they do). + +5. **A named feature is often multi-part** (several modifiers of different kinds under one name): + Aura of Protection = per-ability save-bonus modifiers **+** a trait (`:1463-1473`); Martial Arts = + an `attack` **+** a `bonus-action` (`:1274-1283`); Jack of All Trades = a skill-bonus fn **+** an + initiative mod **+** a trait (`:304-311`); Channel Divinity = a trait **+** the Turn Undead action + (`:451-465`). So a registry entry may compile to a **list** of modifiers, not one cfg. + +6. **Features interdepend through spec attributes.** `?martial-arts-die` is *set* by a modifier and + *read* by the Martial Arts attack (`:1268`/`:1276`); `?paladin-aura` is set once and read by three + auras (`:1483`); `?wild-shape-cr`/`?wild-shape-limitation` are set by level-val modifiers (with + string and **nil** values) and read by the Wild Shape summary (`:787-808`); + `?unarmored-defense`/`?unarmored-ac-bonus` couple Unarmored Defense to AC (`:70-78`, + `:1262-1267`). Extraction must preserve these attribute writes/reads — a feature isn't always + self-contained. + +7. **Partial extraction precedent already exists.** `opt5e/monk-base-cfg`, `paladin-base-cfg`, + `ranger-base-cfg` are merged into those class cfgs (`:1235`, `:1432`, `:1778`); `extra-attack-trait` + and `uncanny-dodge-modifier` are shared helper fns. These are shared *code*, not keyed/data entries + — the direction the registry generalizes. + +8. **Subclass timing varies** — subclass @ level 1 (cleric, sorcerer), 2 (wizard), 3 (most). Subclasses + carry the same feature shapes (and grant spells). Not a blocker, but the registry's level model + can't assume subclass starts at 3. + +## What this means for the build order (refines the roadmap) +- The `compile-feature` slice is correct but **partial**: it proves fighter/rogue (level-schedule + + param interpolation). Before extracting the resource classes it needs (1) a richer `:uses` + expression form and (3) a build-context-aware fill. Both are B1 work. +- **Ki/sorcery/Lay-on-Hands pools (finding 2) are their own mechanism** — schedule the pool layer + (B3 resource counters) deliberately; don't fold it into per-feature frequency. +- Multi-part features (5) and attribute interdependence (6) mean a registry entry's `:compile` returns + **a seq of modifiers**, and extraction must keep the `?attr` reads/writes intact — exactly what the + snapshot net guards (it checks the built output comes out unchanged). +- Start extraction on the **clean classes** (fighter, rogue, then bard/cleric/wizard) where features + are self-contained; **defer monk/paladin** until the pool + build-context-fill mechanisms exist, and + **defer druid** until the wild-shape attribute pair is modelled. **Warlock needs almost no extraction** + — its auto-feature set is empty until Mystic Arcanum @11; its content is selections + pact magic. + +## NOT-EXPLORED (flagged) +- Subclass feature inventory (only base classes catalogued; subclasses skimmed, not tallied). +- The exact `:spellcasting` block semantics (covered separately in spell-granting-across-silos.md). +- How `?martial-arts-die`-style shared attributes would be declared in a data registry (design-open). diff --git a/docs/kb/class-features-and-mechanization.md b/docs/kb/class-features-and-mechanization.md new file mode 100644 index 000000000..0cf7ed941 --- /dev/null +++ b/docs/kb/class-features-and-mechanization.md @@ -0,0 +1,195 @@ +# Class features, the rolling layer, and the mechanization ceiling + +How built-in class features are actually structured, what the app can/can't do mechanically +(including the dice roller), and the design direction for centralizing features. This is the +"tough nut" — read the flags carefully. + +Flags used: **VERIFIED** = read from code, file:line cited. **USER-REPORTED** = stated by the +maintainer, not independently verified here. **SPECULATION / NOT-EXPLORED** = reasoning or an +un-investigated area; do not take as fact. + +## How a class + its features are structured — VERIFIED (fighter, rogue read) +- A class is a function → `(opt5e/class-option … cfg)` with a **hand-written cfg map**. There are + **12** such class option fns (`classes.cljc`, `defn …-option [spells spells-map …]`); warlock's has a + longer arity (`… invocations boons`). +- Features live inline in `:modifiers` (class-wide) or `:levels {N {:modifiers/:selections}}` + (level-gated). Level-gating is that plain `:levels` map keyed by level number, plus per-trait + `:level`. +- **Features have no key.** Verified across fighter/rogue: `trait-cfg`/`action`/`bonus-action`/ + `dependent-trait` take `:name`/`:level`/`:frequency`/`:summary`, never `:key`. Some features are + bare modifiers with no name at all — Extra Attack is `(mod5e/num-attacks 2)` at fighter level 5. +- **Class-level coupling is literal**: `(?class-level :fighter)` / `(?class-level :rogue)` are written + directly into summaries, frequencies, and scaling tables (Second Wind, Action Surge, Sneak Attack, + Indomitable). +- **Partial extraction already exists**: a few features are shared helper fns — `extra-attack-trait` + (`classes.cljc:40`, used by barbarian/ranger), `uncanny-dodge-modifier` (`options.cljc:3049`, used + by rogue + a subclass). These are shared *code* (compile-time fns), not keyed/data-addressable + units, and parameterized only trivially (a page number), not by class-level. + +## Two kinds of feature "mechanics" — VERIFIED +1. **Sheet-affecting modifiers** (compute a derived value): `num-attacks` (Extra Attack), saving + throws, ability boosts, AC/speed fns, spellcasting, uncanny-dodge. +2. **Dependent-traits** (text + a level-computed summary + optional `:frequency`): Sneak Attack's + "Nd6", Second Wind's heal, Action Surge, Rage's damage/resistance, Indomitable. The number is + computed into the string; the effect itself is described, and (without the rolling/counter wiring) + player-applied. + +## The rolling layer — VERIFIED (corrects an earlier wrong claim) +An earlier note said "the app does not resolve rolls/combat." **Wrong** — that came from grepping only +the `orcpub.dnd.e5` namespaces and missing the dice layer. Verified: +- `orcpub.dice` (`src/cljc/orcpub/dice.cljc`): `die-roll`, `roll-n`, `dice-roll {:num :sides :drop-num + :modifier}`, `dice-roll-text-2` (parses "1d20+5" and rolls). +- `roll-button`s across the sheet — attack rolls, skill checks, saves, ability checks — with + **advantage/disadvantage** (`views.cljs` `button-roll-fn`, `roll-button`). +- Attacks compute through `?attack-modifier-fns` / `?damage-bonus-fns` (`modifiers.cljc:545-556`) — + the same fns Dueling uses for a real conditional +2 damage — and these feed both the *displayed* + attack and the roll. **This is the attachment point** for mechanizing a feature's dice/mods. + +USER-REPORTED (not verified here): **many damage/etc. rolls are stuck as text and not exposed to the +roller**, and users have explicitly asked for this to be fixed. So the roller exists but is applied +inconsistently — wiring the stuck-as-text rolls into it is a known, requested improvement. +USER-REPORTED: the **PDF sheets don't roll** anything but auto-calculate some fields (not verified). + +## Use/resource counters — VERIFIED +A general counter exists: `actions-indicators` (`views.cljs:2344`) renders a feature's uses as +checkboxes (small count) or a 0..N selector (large count), backed by `::entity/values +::char5e/features-used` (keyed by rest-period + name) and reset by `clear-period` on long/short rest +(`events.cljs:2556-2579`). Ki/sorcery points are *authored as text* today, not because the engine +can't count — wiring them as a frequency/amount feature would give them a counter. + +## The mechanization ceiling (where "make it real, not text" stops) +- **In reach:** anything resolving to a derived sheet value (AC, speed, saves, resistances, ability + mods, flat/conditional attack & damage bonuses), known spells, use/point counters, and — via the + roller + bonus-fns — a feature's dice/mods flowing into an actual roll (Sneak Attack's Nd6, Rage's + +2). The situational *condition* (advantage, enemy within 5 ft) stays player-chosen (you pick the + roll/adv mode). +- **Out of reach (would be a different product):** combat *state* and *turn* resolution — targets, + hit/miss outcome, applying damage to a creature, reactions firing, action economy ("extra action"), + enforced "once per turn." That's a simulator, not a sheet+roller. + +## The code-capture catch — VERIFIED (the thing that makes the registry non-trivial) +The optimistic registry plan assumes a feature is *data* you can store, filter, and override. It +isn't, today — a feature is **captured code**, and that changes what the registry's compile step has +to do. +- `dependent-trait`, `action`, `bonus-action`, `prop-trait` are **macros** (`modifiers.cljc:308`, + `:583`, `:589`, `:299`). They expand the cfg map into a `mods/modifier` form. The cfg map itself is + *embedded as a literal in that expansion* — it is not evaluated and handed to a function, it is + spliced into code. +- The cfg fields contain **live entity-spec references and control flow**, not values. Examples from + real fighter (`classes.cljc:1075-1101`): + - Action Surge uses: `:frequency (units5e/rests (if (>= (?class-level :fighter) 17) 2 1))` + - Second Wind heal: `:summary (str "regain 1d10 " (common/mod-str (?class-level :fighter)) " HPs")` + - Indomitable uses: `:frequency (units5e/long-rests (mod5e/level-val (?class-level :fighter) {13 2 17 3 :default 1}))` +- `?class-level` is **not a variable** — it's an entity-spec attribute, defined as + `?class-level (fn [class-kw] (get-in ?levels [class-kw :class-level]))` (`template_base.cljc:125`), + resolved during `entity/build`. `level-val` (`modifiers.cljc:595`) is a **compile-time macro** that + expands a threshold table into a `condp <=` form. So the scaling tables are baked into code at + compile time, keyed off a build-time attribute. + +**Consequence for the registry.** To make a feature data-addressable you cannot just lift the cfg map +out — its `:frequency`/`:summary` are code. A `compile-feature` step takes a DATA spec + the granting +class's level and produces the same cfg the macros consume. Two translations: +- Scaling → **data schedule + runtime lookup.** Replace the hand-written + `(if (>= (?class-level …) 17) 2 1)` / `level-val` table with a `{level→n}` map resolved by a runtime + `level-lookup` (the runtime analogue of the `level-val` macro). `level-lookup` reproduces `level-val` + exactly: highest threshold ≤ level wins, else `:default` (verified against Indomitable's + `{13 2 17 3 :default 1}` → 9→1, 13→2, 17→3). Arithmetic scaling (the rogue's + `round-up(level/2)` Sneak-Attack dice) is expressed as the equivalent threshold table; a small + formula form would compact it (not built). +- Summary → **fields + a fill template, not interpolation.** (Corrects an earlier note that called + this a separate "templating sub-problem.") The fix isn't templating into a string; it's *storing the + overridable numbers as fields* and making the summary a projection of them. The number that scales + (Second Wind's heal bonus, Sneak Attack's die) is a field; `:text` is a template + (`"regain {dice} {+bonus} HPs"`) where `{name}` prints a value and `{+name}` signs it (one rule, because + a heal bonus reads "+5" but a dice count reads "3d6"). So "override Sneak Attack's die" is **not + blocked** — it was only blocked while the die lived inside prose; promote it to a `:die` field and the + summary regenerates. + +**Data-shape findings forced by the real fighter/rogue cases (not aesthetics):** +- **No class named in the feature.** A feature must not carry `:class :fighter` — that re-hardcodes the + coupling de-siloing removes. The feature takes a `level` at compile time; *whoever grants it supplies + the level*, so a custom class can grant the same feature and feed its own level. +- **`:effect` is a map `{:kind … + params}`, not a `[kind params]` tuple.** Override is a deep-merge, + and a tuple can't deep-merge its inner params; a map lets `{:overrides {:effect {:die "d8"}}}` change + one field and nothing else. +- **Editable reference = deep-merge of `:overrides` onto the spec before compile.** + +See the `compile-feature` proof in `test/cljc/orcpub/dnd/e5/class_feature_snapshot_test.clj`: a DATA +spec reproduces the **real built** fighter's Action Surge (use-count 1→2) and Second Wind (heal ++5→+17), and the rogue's Sneak Attack (3d6@5, 6d6@11, once/turn) — frequency **and** rendered summary; +a `:uses` override changes only the count; a `:die` override regenerates the summary (3d6→3d8) and +changes nothing else. All asserted equal to the live `entity/build` output, so it's pinned to real +behavior, not a hand-written expectation. + +## Design direction for centralizing features — DESIGN (not built) +The maintainer's intuition: move core features out of class declarations, centralize, re-insert. The +question was "small per-feature pools, or one larger keyed/filterable registry?" + +Recommendation: **one keyed, filterable feature registry**, not many small pools. +- A pool (existing `content_pools/pool`) is "choose one from a set of interchangeable options" — it + fits *choices* (alternate features, a feature that grants a pick), not a single named feature. + "Each feature its own pool" is a category mismatch. +- Features are named units you reference, place at a level, replace, and query (all rogue features; + all features of kind X; features valid at this slot). That's a **registry: key → parameterized + definition + filterable metadata** (class(es)/generic, level/scaling, kind, source). +- **Pools become filtered views over the registry** — the same built-in ++ homebrew pool sub, but + sourced by a filter on the registry. So it's not registry *vs* pools; it's one registry, with pools + derived from it. This matches how the app already builds pools. +- Registry entries must be **parameterized by class-key** so a feature scales by whatever class grants + it (today `?class-level :fighter` is hardcoded). + +**Distinct features vs scaling (VERIFIED on fighter/rogue) — this shrinks the scope.** A class's +`:levels` table is mostly *scaling/padding*, not distinct features: ASI levels +(`:ability-increase-levels`), Extra Attack increments (`num-attacks` 2/3/4), and feature dice growing +by level (`level-val`, e.g. Sneak Attack). Each class has only ~3–6 genuinely-distinct features +(fighter: Second Wind, Action Surge, Indomitable, Fighting Style). So: +- The registry holds the **handful of distinct features per class** (class-tagged), not whole level + tables. "A class's standout features" = a **filter** on the registry (`:class :fighter`), not a + per-class silo — per-class pools would over-fragment ~3–6 entries. +- Scaling/padding stays where it is — `:ability-increase-levels` + `level-val` + bare modifiers are + fine declarative primitives and don't enter the feature registry. +- So the migration is ≈ extracting ~3–6 features × ~12 classes (+ subclasses), not rewriting every + level table. Smaller and lower-risk than "re-architect every class." + +**Two builder entry points over the one registry (not either/or):** +- **Template-from-a-base-class** (the default UX — most homebrew is "an official class, tweaked"): + initialize the custom class with the base class's feature **references + scaling/profs**, then edit. + Hard rule: copy **references (feature keys), not definitions** — deep-copying definitions + reintroduces the two-versions/drift smell; copying references keeps everyone pointed at the one + shared feature. +- **Filterable picker** (the editing tool, needed regardless): browse the registry by filter + (class/kind/level) to add, replace, or build from scratch. +The template is a thin layer over the picker — build the registry + picker, and template-from-base +falls out cheaply. "Change/replace a feature" = the alternate-features capability (needs feature +keys + an addressable feature list; touches saved data only when a swap is chosen). + +**Editable references (design the SHAPE now — retrofitting is "working backward"):** a reference +should be a **map with optional overrides** (`{:feature :second-wind :overrides {:uses 2}}`), and a +feature a **structured, parameterized record with defaults** (`{:key :second-wind :uses 1 +:heal-die :d10 :scaling …}`); overrides merge onto defaults at compile. This is what lets a custom +class say "more Second Wind uses" / "Sneak Attack uses d8" / "extra Sneak Attack dice at these +levels." Crucial coupling: **you can only override a parameter the feature exposes** — Sneak Attack's +die currently lives in a summary *string*, so overriding it requires structuring the feature first. +So editable references and mechanizing features (structured fields, not prose) are the same work. +Design-now = the reference format (`:overrides` map) + structured feature records + a merge step; +add-later = which overrides the UI exposes and how deep (a param vs editing a scaling table). Keep +the data shape open to arbitrary overrides; throttle the UI. (Avoid the inverse trap: exposing every +knob immediately piles up UI + merge/validation edge cases.) + +Sequence (to contain regression risk): build a characterization net (snapshot every class's built +features) → define the feature record + registry → extract incrementally, proving the built output comes out unchanged per step → then make entries data-addressable and expose to the custom builder + alternates. +Backward-compat note: features are auto-granted (not stored choices), so extraction doesn't touch +saved characters as long as output is identical; only *choosable* alternates put feature keys into +saved data. + +## NOT-EXPLORED / to verify before sizing +- ~~The full per-class feature catalogue~~ — DONE: all 12 base classes inventoried in + `class-feature-catalogue.md`. It confirmed the ~3–6/class sizing (monk/paladin ~10 outliers) and + surfaced the odd cases the `compile-feature` slice doesn't yet cover: multi-source use-counts + (ability-derived, formula, level), class-wide resource pools (ki/sorcery/Lay-on-Hands), + build-context summary interpolation (derived stats + user selections), multi-part features + (compile → seq of modifiers), and `?attr` interdependence. +- Exactly how a *new conditional dice rider* (e.g. "+Nd6 when you have advantage") would attach to a + roll button in the UI. +- Whether the combat tracker tracks uses/resources (separate from the sheet). +- The PDF auto-calc fields (USER-REPORTED; not checked). diff --git a/docs/kb/cljs-headless-harness.md b/docs/kb/cljs-headless-harness.md new file mode 100644 index 000000000..827d69555 --- /dev/null +++ b/docs/kb/cljs-headless-harness.md @@ -0,0 +1,133 @@ +# Headless ClojureScript test harness (how to run cljs tests in a container) + +**Why:** CI runs only `lein lint`/`lein test` (JVM). The cljs suite (subs, events, +import-validation, content-reconciliation, …) only runs under a JS runtime. This recipe +runs it **headless** so cljs changes are verifiable without the full webapp (no backend, +no Datomic — those aren't needed; HTTP calls just `ERR_CONNECTION_REFUSED` harmlessly). +**The harness is built in `/tmp` + the gitignored `target/` — both ephemeral, so rebuild +from this recipe.** It's also the prototype for the deferred "cljs tests in CI" item. + +## Build it + +```bash +# 1. Leiningen (no lein/.m2 in a fresh container) +mkdir -p ~/bin && curl -sS -o ~/bin/lein https://raw.githubusercontent.com/technomancy/leiningen/stable/bin/lein +chmod +x ~/bin/lein && export PATH="$HOME/bin:$PATH" && lein version # self-installs +lein deps # fetch project deps + +# 2. Headless browser (a real browser is needed — app uses js/window, localStorage, DOM) +mkdir -p /tmp/pw && cd /tmp/pw && npm init -y && npm install playwright && npx playwright install chromium + +# 3. Compile the cljs test build (→ target/test/js/) +cd && lein fig:test +``` + +## Two ways to run (they differ — pick deliberately) + +**A) Clean per-test reporter (use for triage — gives `expected:`/`actual:`):** +Runs `orcpub.test-runner/-main` (cljs.test → console). It runs only the namespaces listed +in `test/cljs/orcpub/test_runner.cljs`. To check a specific failing ns (e.g. +`import-validation-test`), **temporarily** add it to that `-main`, recompile, run, then revert. +- HTML (`target/test/runner.html`): `` + +**B) Full suite (all test nss, for totals/regression):** +Loads figwheel's auto-test runner. Reports to the DOM (the body lists ALL tests incl. passing +— do NOT mistake that list for failures; trust the totals + the clean run for per-test). +- HTML (`target/test/runner-all.html`): + `
` + (the `app-auto-testing` div is required or it throws.) + +**Driver (node, `/tmp/pw/run.js`):** a ~15-line `http` static server rooted at +`target/test/`, then Playwright Chromium navigates to the HTML, captures `console` + +`pageerror`, waits for `/Ran \d+ tests/`, and prints the console + body. Grep the output for +`Ran .* tests`, `FAIL in`, `ERROR in`. + +## Known-good baseline (as of this branch) +Full suite ≈ **150 tests, 10 failures, 2 errors**. The 2 errors are the dead +`character_test.cljc` (retire per `character-validation.md`). After the import fixes, the +import-validation failures are gone; remaining real failure is `user-stale-user` (subs auth +guard — separate, not triaged). + +## Gotchas worth remembering +- **JVM-isms bite only here.** `(int char)` = code point on JVM, but `(int "é")` = 0 in cljs + (no Character type; strings seq into 1-char strings). Use `(.charCodeAt % 0)`. This class + of bug is invisible to source review + `lein test`. (Was the real `count-non-ascii` bug.) +- The auto-test DOM body lists passing tests too — only the clean reporter (A) gives + authoritative per-test pass/fail. +- No backend needed; connection-refused logs are expected and harmless. + +## Full-app headless E2E — render and drive the REAL app UI (not the test build) + +The recipe above runs the cljs **test** build (test namespaces). To verify the **actual app UI** +(e.g. a builder form renders and is interactable), drive the **app** build headlessly. **Verified +feasible in-container** — the SPA boots with no backend and no auth wall, and the real race-builder +page rendered (incl. a newly-added widget), zero page errors. + +**Why it works (no auth gate — verified):** `[:verify-user-session]` is a **no-op when there is no +token** (`events.cljs:1633`), and the `:route` handler does not gate on auth (the `secure?`/https +redirect is skipped on `localhost`). Routing is **path-based** off `window.location.pathname` +(`core.cljs:80`). So an unauthenticated headless load reaches a builder route; backend calls just +`ERR_CONNECTION_REFUSED` harmlessly. + +**Recipe (CLI-only; CI- and Codespace-portable):** +1. Compile the **app** build: `lein fig:build` (dev, `dev.cljs.edn`) → `resources/public/js/compiled/orcpub.js`. +2. Host page `/tmp/pw/host.html` (the server-rendered `index.clj` reduced to its essentials): + ```html + + + +
+ ``` +3. Driver `/tmp/pw/drive-app.js`: a node static server rooted at **`resources/public`** with an + **SPA fallback** (return `host.html` for any path that isn't a real file, so a deep route URL + loads the JS and client-routes); Playwright Chromium then `goto`s the route, waits for `#app` to + have content, asserts/screenshots, and captures `pageerror`. Route path = `/pages/dnd/5e/` + (e.g. `/pages/dnd/5e/race-builder`; `` is the content-types `:route-seg`). +4. Run: `cd /tmp/pw && node drive-app.js`. Observed: `RENDERED: true`, `PAGEERRORS: none`, the + builder's text present (incl. the floating-ASI widget). + +**This makes rendered-UI E2E a single CLI command** — so it runs in CI (results to your phone, no +desk) or a tunneled Codespace. It is the basis for the deferred "cljs/E2E into CI" item. + +### Full content round-trip through the real UI (`test/e2e/export-import-use.js`) + +Beyond authoring, the whole **export → import → use** loop runs headless with no backend (Blob/saveAs +and FileReader are client-side): the My Content **Export** button yields a real `.orcbrew` download +(Playwright `page.on('download')` captures it), the real `` imports it +(`setInputFiles`), and the imported homebrew race is then selectable in the character builder. Routes: +My Content is `/dnd/5e/my-content` (the `dnd/` branch — NOT `/pages/...`); the character builder is +`/pages/dnd/5e/character-builder`. This is the UI-level proof for floating-ASI layer 5, above the +function-level round-trip tests. **Gotcha it pins:** import names the pack from the *file name* +(`import-file`: `nm = split(filename, ".orcbrew")`), so preserve `download.suggestedFilename()` +(`.orcbrew`) — saving under a different name re-creates the pack under that name. + +### Driving interactions (done — committed as `test/e2e/race-builder-asi.js`) + +The render proof above was extended to a full **click-through**: author a floating-ASI homebrew +race through the real form, click the real Save button, and assert the persisted localStorage. It +runs with `REPO= node test/e2e/race-builder-asi.js` (see `test/e2e/README.md`). + +**It caught a real bug source review missed:** the race-builder ASI widget stored raw `` exercised the coercion.** Fix: +the widget now coerces each dropdown's emitted string via lookup maps (`views.cljs`, +`race-ability-increase-choices`). This is the concrete payoff of the "90%-without-backend" E2E. + +**Interaction gotchas (verified, the hard way — encoded in the script):** +- `dropdown`/`labeled-dropdown` on the **default** path yields the **rendered string** (`event-value` + = `.target.value`); reagent renders a keyword value via `name`, so `::character/cha` → `"cha"`. Pass + **`:typed? true`** and on-change instead receives the item's original `:value` (any type) — no + coercion, nothing to forget. New non-string dropdowns should use it. Full story + the repo-wide + census: `docs/kb/dropdown-value-coercion.md` (decision D32). +- Input index `[1]` on the race-builder page is the **Orcacle search box** (`placeholder="search"`), + NOT a form field. Typing into it opens an autofill **suggestions overlay** that intercepts later + clicks (this is the "div intercepts pointer events" symptom — not a modal/loading overlay). + Target real fields by class/placeholder (`input.input.h-40`, `[placeholder="Default Option Source"]`). +- "Save to Browser Storage" is in the DOM **twice** (hidden mobile + visible desktop twin); a plain + `.first()` grabs the hidden one ("element is not visible"). Use `button:visible`. +- reagent flushes re-renders **async**; back-to-back `selectOption`s can fire an on-change whose + closure still holds the pre-dispatch state and clobbers an earlier pick. Pause (~150ms) between. +- When in doubt about what the page actually is, **take a screenshot** — it settles "which input/ + overlay is this" in one shot instead of repeated wrong hypotheses. diff --git a/docs/kb/content-extensibility-compatibility.md b/docs/kb/content-extensibility-compatibility.md new file mode 100644 index 000000000..05bab212f --- /dev/null +++ b/docs/kb/content-extensibility-compatibility.md @@ -0,0 +1,196 @@ +# Content Extensibility — Backward-Compatibility Audit + +**Purpose:** Inventory the persisted data formats that the content-extensibility +redesign must not break, derive the hard invariants, and assess the proposed direction +against them. Existing `.orcbrew` libraries and saved characters must keep working. + +**Status:** +- Section 1 (formats), Section 5 (risk surfaces), Section 6 (safety nets) are + **verified from code** (file:line). +- Section 4 (proposal assessment) judges the **proposed, not-implemented** design in + [content-extensibility.md](content-extensibility.md) against the invariants. +- Datomic *schema* was not separately audited this pass; the character *payload* sent + to the backend is the strict entity (Section 1b), which is what matters here. Flagged. + +**Branch note:** references read on the monolithic frontend layout of this branch; on +`agents/develop` the views layer is split. Grep symbols to confirm. + +--- + +## 1. Persisted formats (verified) + +These are the artifacts on users' disks / in the DB. We do **not** control them once +exported or saved. + +### 1a. orcbrew / plugins (homebrew libraries) + +The in-app plugins map and the exported `.orcbrew` file share one shape. + +- `::e5/plugins` = `(map-of string? ::plugin)` — keyed by **option-pack name (string)**. +- `::plugin` = `(map-of ::content-keyword (or ::homebrew-items boolean?))`. +- `::content-keyword` = a **qualified keyword whose namespace is exactly + `"orcpub.dnd.e5"`** (e.g. `:orcpub.dnd.e5/boons`, `…/spells`, `…/subraces`), or the + literal `:disabled?`. +- `::homebrew-items` = `(map-of ::homebrew-item)`; each item must carry + `::option-pack`. +- Source: `src/cljs/orcpub/dnd/e5.cljc` (whole file — `::plugins`, `::plugin`, + `::content-keyword`, `merge-all-plugins`). +- Stored under `[option-pack plugin-key key]` by `reg-save-homebrew` + (`events.cljs` ~533); `key` = `(common/name-to-kw name)`. +- Exported file = EDN string of **one pack's `::plugin` map** + (`::e5/export-plugin`, `events.cljs` ~3601, dispatched with + `(str (new-plugins option-pack))`). +- Imported via `import-val/validate-import` → `merge-all-plugins` + (`events.cljs` ~3807, ~3894); validated against `::e5/plugins`. + +**What this constrains:** the per-type plugin key (`::e5/boons` etc.) and the +`orcpub.dnd.e5` namespace are part of the on-disk contract. A new content type's plugin +key must live in that namespace or it fails `::content-keyword` and won't import. + +### 1b. Character (strict entity) + +- `::se/entity` = `(keys :opt [::selections ::values])` + no duplicate selections. +- `::selection` = `(keys :req [::key] :opt [::option ::options])`. +- `::option` = `(keys :opt [::key ::int-value ::map-value ::selections])` — nests. +- `::values` = `(map-of qualified-keyword? some?)`. +- Source: `src/cljc/orcpub/entity/strict.cljc` (whole file). +- Round-trip: `char5e/to-strict` / `from-strict` (`character.cljc` ~266 / ~329). + +**What this constrains:** a saved character records its choices as `::se/key` keywords +at selection/option nodes. Those keys are the addresses of choices. If a redesign +changes the **key of a selection or option a character has already chosen**, the stored +choice no longer resolves (orphaned). + +**⚠️ name-to-kw is a creation-time default, NOT a re-derivable contract.** Keys are +*originally* produced by `common/name-to-kw` of a name (`common.cljc` ~19), but the +durable contract is the **stored `:key`**, not the name. This already bit the project: +class option keys were derived from the display `:name`, and when a plugin-source suffix +was folded into that name for display, `name-to-kw` produced a *different* key and +orphaned saved characters. The fix (`feature/name-keyword-fix`, off the same base +`d42e05d` as this branch) establishes the rule: + +- **Identity derives from a stable id, never from a display string.** Selection keys now + derive from `:class-key` via `options.cljc` `spell-selection-key`, not `name-to-kw` of + the title (commit `fe54963`). +- **Display is a separate slot.** `option-cfg` carries `::plugin-source` distinct from + `::name` (commit `39a054b`); the source suffix is a preference-gated *display* concern, + never part of the key (`9a709c0`, `show-class-source-suffix`). +- A **reconciler** heals already-orphaned keys on load (`content_reconciliation.cljs`, + commits `a3e2615`/`4289871`) — the existing shim for this exact failure. + +**Rule for the catalog/grant work:** when building option-cfgs from catalog items +(boons, lineages, …), pass the item's **stored `:key`** to `option-cfg` — do not let it +re-derive from `:name` (today `pact-boon-options` re-derives, which is the same latent +footgun). Never call `name-to-kw` on a display-manipulated name. + +### 1c. localStorage + +Per-builder draft keys + the plugins blob + the current character +(`db.cljs` ~32–49: `character`, `plugins`, `boon`, `spell`, …). Read back through +`reg-local-store-cofx`, which **spec-validates and drops** anything invalid +(`db.cljs` ~252). So a format change that fails the spec silently discards the draft. + +### 1d. Backend + +Characters are saved to the server as the strict entity (`::char5e/save-character`, +`events.cljs` ~435). Datomic schema not audited here; the compatibility surface is the +same strict-entity payload as 1b. + +## 2. Who owns what + +| Artifact | Owner | Can we change its shape freely? | +|----------|-------|-------------------------------| +| orcbrew files | users (exported, shared, re-imported) | **No** — read + forward compat | +| saved characters (local + DB) | users | **No** — must keep resolving | +| in-memory app-db wiring | us | Yes | +| registration call-sites | us | Yes | + +## 3. Hard invariants (non-negotiable) + +1. **Read invariant:** the app must load existing `::e5/plugins` maps and existing + strict-entity characters **unchanged**, with no migration step required of the user. +2. **Plugin-key invariant:** existing per-type keys (`:orcpub.dnd.e5/boons`, etc.) and + the `orcpub.dnd.e5` namespace stay valid; new types add keys, never rename old ones. +3. **Selection-key invariant:** selection/option `::t/key`s that characters can already + have chosen must not change identity (see Section 5). +4. **Forward invariant (decide explicitly):** an orcbrew exported by the new version + should still import into the **old** hosted version — i.e. keep the export shape, or + accept an ecosystem split. + +## 4. Proposal assessment against the invariants + +### Layer 1 — content-type registry: **compatibility-neutral** + +Changes only internal registration wiring (which call-sites register events/subs/routes). +No persisted shape changes. Satisfies all invariants by construction, provided the +descriptor reuses the **existing** plugin key and route keyword for each existing type +(invariant 2). Verdict: **safe, zero-migration.** + +### Layer 2 — catalogs/grants: **safe if derived, not reformatted** + +The redesign can satisfy the invariants **only if catalogs are derived over the existing +storage** rather than introducing a new on-disk format. Precedent is in the code: +subraces have no "catalog" on disk — it is computed by a subscription +(`group-by :race` over `::e5/subraces`, `spell_subs.cljs` ~887). If boons/invocations/ +etc. catalogs are likewise derived from today's `::e5/boons`/`::e5/invocations` maps: + +- orcbrew files keep loading (storage unchanged) → invariant 1, 2 hold. +- The danger is invariant 3: a grant defines a *selection*, and the selection's key is + what a character stored its choice under. Migrating boons from positional-arg to + `grant-choice` must **preserve the existing selection key and the option keys** + (still `name-to-kw` of the same names), or already-built characters orphan their + pact-boon choice. This is the one place Layer 2 can break compatibility, and it is + testable (Section 7). + +Verdict: **safe if (a) catalogs derive from existing plugin maps and (b) selection/ +option keys are preserved.** Not safe if it introduces a new storage shape or renames +selections. + +## 5. Specific risk surfaces (verified) + +1. **Selection-key stability is also a safety-net dependency.** + `content_reconciliation.cljs` hardcodes per-class archetype selection keys + (`subclass-selection-keys`: `:otherworldly-patron`, `:martial-archetype`, …) and a + `content-type->field` map. Changing selection keys breaks not just stored characters + but the missing-content detector too. Treat selection keys as a public contract. +2. **`merge-all-plugins` is `merge-with merge` (shallow, two levels).** + (`e5.cljc`.) Import merges packs by option-pack then by content-key. Any new + catalog grouping must not assume deeper merge semantics than this. +3. **localStorage cofx silently drops spec-invalid data** (`db.cljs` ~252). A draft/ + format change that tightens a spec will quietly discard in-progress user drafts. +4. **The `orcpub.dnd.e5` namespace requirement** on `::content-keyword` (Section 1a) + means a "type" field added for catalogs must not replace the namespaced plugin key + that import relies on. + +## 6. Existing safety nets (lean on these, don't reinvent) + +- **Import validation** — progressive, auto-clean, conflict detection, text + normalization (`import_validation.cljs`). +- **Conflict resolution** — pre-import modal for duplicate keys + (`docs/CONFLICT_RESOLUTION.md`, `views/conflict_resolution.cljs`). +- **Content reconciliation** — detects missing/renamed content refs in a character and + suggests matches (`content_reconciliation.cljs`, `docs/CONTENT_RECONCILIATION.md`). +- **Spec validation on every load** — `reg-local-store-cofx` and import both validate. + +These are the migration tooling if anything turns out non-additive. + +## 7. Migration & rollback posture + +- **Aim for zero-migration:** derive over existing storage, preserve keys. If that holds, + there is nothing to migrate and rollback is just reverting code. +- **Prove it with fixtures, not eyeballs.** Before/after the subrace spike and the boon + migration, load a representative `.orcbrew` and a saved character (one that has chosen + a pact boon) and assert the built character is identical — same selection keys, same + resolved options. Round-trip `to-strict`/`from-strict` should be byte-stable. +- **If a change is unavoidably non-additive** (e.g. a selection key must change), do not + silently rename: route it through the existing reconciliation/alias path so old keys + still resolve, and document the alias. +- **Forward compat:** keep the export shape identical so new-version exports import on + the old hosted site. If the shape must change, gate it and announce it. + +## Related + +- [content-extensibility.md](content-extensibility.md) — the design under audit. +- [content-extensibility-decisions.md](content-extensibility-decisions.md) — decisions. +- `docs/CONTENT_RECONCILIATION.md`, `docs/CONFLICT_RESOLUTION.md`, + `docs/ORCBREW_FILE_VALIDATION.md` — the safety nets in detail. diff --git a/docs/kb/content-extensibility-decisions.md b/docs/kb/content-extensibility-decisions.md new file mode 100644 index 000000000..3b371c3f3 --- /dev/null +++ b/docs/kb/content-extensibility-decisions.md @@ -0,0 +1,510 @@ +# Content Extensibility — Decisions & Audit + +**Purpose:** Record *how* the content-extensibility direction was reached — the +pivots, the dead-ends, and why we changed our minds — plus the crisp decisions it +produced. For both humans and agents picking this up cold. Design record, not +verified source behavior. See [content-extensibility.md](content-extensibility.md). + +**Date opened:** 2026-06-13. + +> ⚠️ **D12–D16 (added late) DEFLATE D2/D3 and the catalog-grant framing.** The grand +> registry / DSL was scaled back to "descriptor + a HOF over existing factories, for +> mechanical boilerplate only." `content-extensibility-direction.md` is the authoritative +> plan; D1–D11 are kept as the record of how we got there. +> +> ⚠️ **D17b–D22 (Part 4) RE-CENTER.** The deflation over-applied a *local* readability lesson +> (kill the unreadable `by-parent` wrapper) to the *capability* (cross-silo composition) and +> wrongly shelved D3. Part 4 restores the capability — an open **pool + grant** layer with +> filter/gate/prereq and a variant forward-compat seam — with readability kept as a +> *constraint*, not a ceiling. The direction doc (v2) is authoritative. + +--- + +## Status at a glance + +One line per decision so the depth below is navigable. **LIVE** = current standing rule; **DONE** = +decided and shipped; **OPEN** = unresolved; **REVERSED** = overturned (kept, stubbed, as a "decided +against" record). Full reasoning is in the body — this is only an index. + +| # | Decision (short) | Status | +|---|---|---| +| D1 | Split the "8-file" cost into registration vs injection | LIVE | +| D2 | Layer 1 = data + existing factories, not a macro | DONE (`3980ea1b`) | +| D3 | Address options by **type**, not parent slot | LIVE (became pool+grant, D19) | +| D4 | Keep `mod5e/*` modifiers for fixed grants | LIVE | +| D5 | Migrate subraces first | DONE | +| D6 | Keep route-keyword `def`s; generate downstream | LIVE | +| D7 | Registry namespace stays a dependency leaf | LIVE | +| D8 | Document before coding the spike | DONE (historical) | +| D9 | Backward compat is non-negotiable; zero-migration | LIVE (constraint) | +| D10 | Identity from a stable key, never a display name | LIVE | +| D11 | Catalog reads = layered memoized subs | LIVE | +| D12 | Readability is the deciding constraint | LIVE | +| D13 | Deflate Layer 1 to descriptor+HOF; ~~reject catalogs/grants~~ | DONE (deflation) / **REVERSED** (anti-catalog half, by D17b–D19) | +| D14 | Don't force different kinds into one registrar | LIVE | +| D15 | HOFs/macros are fine when fed clear inputs | LIVE | +| D16 | Working agreements (falsifiable tests; fix-on-sight) | LIVE | +| D17 | Audit what each piece REPLACES; **no generic wrapper** | LIVE (heavily cited) | +| D17b | Stability and flexibility are the same abstraction | LIVE (was a duplicate "D17") | +| D18 | The capability gap is in AUTHORING, not the engine | LIVE | +| D19 | Pool + grant, with graceful optional filtering | LIVE (core direction) | +| D20 | Variants designed-in now, built later (one indirection) | LIVE (pin) | +| D21 | Maintainability is a GATE: O(1) to expose a new pool | LIVE (falsifiable) | +| D22 | Builder forms are data ("irreducible" was a retreat) | DONE (`109b5dd0`) | +| D23 | This branch is a PROTOTYPE: decide a standard, then converge | LIVE (governing) | +| D24 | Class features → one keyed, filterable registry | LIVE (design) | +| D25 | Features are macro-captured code → `compile-feature` (fields+template) | LIVE (proven) | +| D26 | Per-class catalogue done for all 12; reshapes B1/B3 | DONE (catalogue) | +| D27 | Spell slots: bucket of tables + declared multiclass rule | LIVE (design) | +| D28 | Non-SRD never pre-built; validate with synthetic stand-ins | LIVE (rule) | +| D29 | Bespoke vs systematic pool/grant → **no duplicated functionality; one mechanism per job** | DECIDED | +| D30 | "grant" earns its keep only as a thin compiler, not a 2nd engine | LIVE | +| D31 | Two vocabularies share effects but differ in application mode | LIVE (test-backed) | +| D32 | UI dropdowns must round-trip their value type (`:typed?`) | DONE | +| D33 | Export data = terse encodings + commented source, not self-documenting bytes | LIVE (rule) | +| D34 | Deprecate (not delete) superseded proven code: `#_`-strike + date-stamp + remove ~3mo later | LIVE (rule) | + +--- + +## Part 1 — How the thinking evolved (audit) + +Read this to understand what we were thinking at each step, not just where we landed. + +1. **Start: "8 files to add a minor option — is there a better way?"** + First instinct was a single content-type *registry* to kill the scattered + registration boilerplate. Plausible, but it was reasoning from the symptom. + +2. **Pushback: "we might be talking past each other."** + Prompted to look at a real change — the Pact Boon builder (commit `6029fd0`) — and + at a hypothetical dragonborn lineage builder, instead of theorizing. + +3. **Evidence changed the framing.** The boon diff touched 10 files, but split into + two unrelated costs: *registration* (mechanical, scattered) and *injection* + (wiring boons into the warlock via positional function args). The registry idea + only addressed the mechanical half — and the cheaper half. → **D1.** + +4. **First Layer-2 idea: parent-keyed "slots."** Attach a boon to + `[:class :warlock :pact-boon]`. Looked tidy. + +5. **Caveat that broke it: 5e is expansion-driven.** Feats grant spells, ASIs, even + pact boons; backgrounds grant feats; homebrew adds more later. Asked directly: + does the proposal make cross-tapping easier or harder? Honest answer: rigid slots + make it *harder*, because one option granted by several parents needs several + attachment declarations. → pivoted to **type-addressed catalogs + grants** (D3), + and split "grant a fixed thing" (keep modifiers, D4) from "grant a choice from a + set" (new). + +6. **Pseudocode requested** to make the intent concrete — confirmed the catalog/grant + shape reads cleanly and that subraces already do exactly this by parent key. + +7. **Documentation requested; KB location clarified.** The canonical agent KB lives + on `agents/develop`, which already has overlapping docs + (`spa-routing-architecture.md` covers the registration side) and an issue cluster + (`homebrew-builders.md`: #58, #57/#209, #172/#170, #210/#107, #280, #173, #128) + that are real instances of this problem. Restructured the docs to that KB's + conventions and separated verified-from-code facts from the proposal. → **D8.** + +8. **Compatibility raised as a hard constraint, audited before planning.** Users with + existing orcbrew libraries and built characters can't be broken. Decided to audit + the *current* persisted formats first (orcbrew/plugins map, strict-entity + characters, localStorage), derive invariants, and constrain the design to be + additive. The audit showed the design can be zero-migration *if* catalogs derive + over existing storage (as subraces already do) and selection/option keys are + preserved. → **D9.** See + [content-extensibility-compatibility.md](content-extensibility-compatibility.md). + +9. **Readability review deflated the whole thing.** Pushed on `(catalog/by-parent :race x)` + vs `(group-by :race x)`: the wrapper *added* thinking to a clear builtin — negative + value. Generalizing: an abstraction must be *thicker* than what it hides and reveal + intent. Re-graded everything: the catalog/grant DSL would be more `by-parent`-style + indirection; looping readable data (`default-value`) trades readability for little; + only *pure boilerplate* (identical passthrough subs) and *fragile* code earn collapsing. + Landed on: descriptor + a clear HOF (`register-homebrew-content!`) composing the + existing factories, scoped to boilerplate; keep readable code explicit. → **D12–D16.** + +The throughline: each pivot came from concrete evidence (a real diff), a domain +constraint (5e's cross-pollination), a user-data constraint (existing orcbrew and +characters), or a readability constraint — not from preference. Note the honest failure +mode caught along the way: agentic *toadyism* — collapsing/backpedaling when pushed instead +of holding or refining a position. The corrective is in `verification-discipline.md`. + +## Part 2 — Decision summary + +**D1 — Treat the "8-file" cost as two problems.** Registration boilerplate and +parent-injection are different shapes with different fixes. *Rejected:* one +"scaffolding" fix — it misses the fragile injection half where bugs hide. + +**D2 — Layer 1 is data + existing factories, not a macro.** A `content-types` +descriptor list feeding loops that call the existing `reg-*-homebrew` factories. +*Rejected:* a `defcontent` macro — opaque expansion, buys nothing when factories exist. + +**D3 — Layer 2 addresses options by type, not by parent slot.** Children declare a +type; consumers pull from a type catalog with an optional filter. *Rejected:* fixed +parent-keyed slots — they break when one option is granted by several parents. + +**D4 — Keep the modifier system for fixed grants.** `mod5e/*` handles "grant this +specific thing"; leave it. Catalogs/grants are only for "choose from a set." + +**D5 — Migrate subraces first.** They already use the target bucket-by-key pattern, so +step one is a behavior-preserving refactor and an easy review. *Rejected:* starting +with boons (touches the fragile class sub) or lineages (new capability, not refactor). + +**D6 — Keep route-keyword `def`s; generate downstream.** They're referenced by symbol +at compile time; generate the bidi tree, route sets, pages, events, and subs. + +**D7 — Registry namespace stays a dependency leaf.** Requires only specs and +`route-map`; views referenced by keyword and resolved in `core.cljs`, to avoid the +circular dep the code already works around (`events.cljs` ~204). + +**D8 — Document first; no code until the subrace spike is reviewed.** + +**D9 — Backward compatibility is a non-negotiable constraint; target zero-migration.** +Existing orcbrew libraries and saved characters must keep working with no user-facing +migration. The design stays additive: catalogs derive over the existing plugin +storage (don't reformat it), existing plugin keys and the `orcpub.dnd.e5` namespace are +preserved, and selection/option keys that characters may have chosen are not renamed. +Each migration step is guarded by an orcbrew + saved-character fixture. *Rejected:* +designing the storage model first and auditing compatibility afterward — the audit +would too late to reshape it. Full analysis: +[content-extensibility-compatibility.md](content-extensibility-compatibility.md). + +**D10 — Identity comes from a stable key, never from a display name.** Option/selection +keys must derive from a stable id (the stored `:key`, a `:class-key`, etc.), and display +text (`:name`, plugin-source suffix) is a separate slot. `name-to-kw` is a creation-time +default only; never re-run it on a name that display code may manipulate. *Why:* doing so +already orphaned saved characters when a source suffix was folded into class `:name` +(fixed on `feature/name-keyword-fix`: `option-cfg` `::plugin-source` slot, key-from- +`:class-key`, a load-time reconciler). The catalog/grant work must pass each item's stored +`:key` through to `option-cfg`. *Rejected:* relying on name→key re-derivation (the footgun). + +**D11 — Catalog reads are layered, memoized subscriptions; never recomputed in hot subs.** +Each catalog is its own `reg-sub` (re-frame memoizes it); `grant-choice` references it +rather than rebuilding a whole option list inside a hot subscription. Guard the layering +with a brief comment. *Why:* avoids the recompute-everything cost and is also the fix for +the monolithic god-subscriptions (e.g. the 8-input `::classes5e/classes`). *Rejected:* +inline catalog construction in consumer subs. + +--- + +## Part 3 — Late decisions (deflation; these scaled back D2/D3 — but were themselves RE-CENTERED by Part 4) + +**D12 — Readability is the deciding constraint.** An abstraction earns its keep only when +it is *thicker* than what it hides AND its interface reveals intent. `by-parent` fails +(wraps `group-by`) → **revert it**; `reg-save-homebrew` passes (thick, clear, already +trusted). *Rejected:* applying the registry/loops uniformly "for consistency." + +**D13 — Deflate Layer 1 to a descriptor + one HOF, scoped to boilerplate.** A per-type +descriptor (data) + `register-homebrew-content!` that **composes the existing factories** +(`reg-save/new/edit-homebrew`, `reg-option-*`). Apply only to mechanical, low-readability +duplication (e.g. the identical passthrough subs). Keep readable data — notably +`default-value` — **explicit**. *Rejected:* descriptor-drives-everything loops, and the +catalog/grant **DSL** (more `by-parent`-style indirection; named subs + `selection-cfg` +already give the cross-aspect capability without new vocabulary). +> ⚠️ **Half REVERSED.** The boilerplate-deflation half (descriptor + HOF over factories) stands and +> shipped (`3980ea1b`). The "named subs + `selection-cfg` are *enough*, don't build catalogs/grants" +> half was overturned by **D17b/D18/D19** — D18 explicitly corrects it ("true of the mechanism, false +> as a stopping point"). The pool+grant layer IS being built; only a *cryptic DSL* remains rejected. + +**D14 — Don't force genuinely-different kinds into one registrar.** Verified 3 buckets: +6 "basic" types fit verbatim; 6 "richer" via a readable `:builder-features` flag set +(`reg-option-traits/modifiers/selections`); deviations stay separate — **magic-item** +(server-persisted via `::mi/save-item`/`::mi/custom-items`) gets its own +`register-server-content!`, **selection** gets a `:save-fn` hook (dup-option validation), +**combat** is excluded (not a builder). *Rejected:* a universal registrar branching on +every deviation — that's the unreadable god-function trap. + +**D15 — HOFs/macros are fine when fed clear inputs.** The codebase already trusts +`reg-*-homebrew`/`reg-option-*`. The mid-session lean toward "reject HOF/macro" was an +overcorrection to a readability concern; the real enemy is *thin/obscuring* abstraction, +not HOFs. + +**D16 — Working agreements.** Tests must be **falsifiable** (no theater — "if I break the +code, does this go red?"); **fix bugs on sight** unless deep enough for their own branch; +goal is **stabilize while adding features**, not build on shaky foundations. + +**D17 — Audit what each framework piece REPLACES before building it.** For every new +primitive, first find the existing app code it would replace and ask: does the app already do +this as well or better? If the "new" thing isn't *thicker* than what it replaces (D12), or it +risks dropping load-bearing details (`:ref` — where a character stores a choice; `:tags`; +modifiers), DON'T build it — extend or point at the existing thing instead. *Caught immediately:* +the proposed generic `grant` primitive would have re-wrapped the perfectly-good per-feature +`selection-cfg` constructors (`fighting-style-selection`, `feat-selection`) — adding nothing and +risking dropping their `:ref`. The real gap is narrower: **open pools** feeding those existing +selections + an **author-declarable grant-spec that compiles down to `selection-cfg`** (preserving +`:ref`/`:tags`), NOT a wrapper that replaces the constructors. + +**Net for next steps:** ~~revert `by-parent`~~ ✅ done `9777ce88`; ~~build +`register-homebrew-content!`; swap **boon**~~ ✅ done `3980ea1b`. **Now re-centered (Part 4):** +the next core work is the **pool + grant** composition layer (see direction doc v2 §"The spine" +and the PINS). Authoritative plan: +[content-extensibility-direction.md](content-extensibility-direction.md). + +--- + +## Part 4 — Re-centering (these restore the capability D12–D16 over-deflated) + +**Context.** After D12–D16 the plan was scaled to "collapse boilerplate, do not build +catalogs." A vision review showed that under-served the branch's *two* equal goals — +**stability** and **flexibility** — because it shelved the one capability that delivers both. + +**D17b — Stability and flexibility are the SAME abstraction.** (Numbered D17b, not D17: this is a +*distinct* decision that collided with the Part 3 D17 due to a historical glitch. The Part 3 D17 — the +"audit what each piece replaces / no generic wrapper" decision — keeps the bare number because it is +cited ~30× across the codebase; this one is cited nowhere by number, so it takes the suffix instead of +renumbering D18–D32.) Today every cross-type link is +bespoke positional wiring (boons→warlock by arg; custom-race menu a hardcoded vector; styles +baked per class). That bespoke-ness *is* the ~8-file cost and the fragility. One declarative +**pool + grant** primitive collapses N×M bespoke wirings to N+M declarations down one tested +path → simultaneously the stability win and the flexibility win. They do not trade off. +*Rejected:* treating "make it stable/readable" and "make it flexible" as opposed. + +**D18 — The capability gap is in AUTHORING, not the engine.** Verified: `selection-cfg` has +`prereq-fn`/`tags`/`ref`; `option-prereq` exists; `option-cfg` has `prereqs`/nested +`selections`/`modifiers`; `ability-increase-selection-2` is the floating-ASI mechanism. The +runtime already composes with filter/gate/prereq. What's missing is letting *content declare* +open cross-silo grants as data. So this is exposing existing engine power, not rebuilding it. +*Corrects:* the deflation's "named subs + selection-cfg are enough" — true of the mechanism, +false as a stopping point (it left the menu closed and the wiring bespoke). + +**D19 — Pool + grant, with graceful optional filtering.** Two words of vocabulary: a **pool** +(named, open, type-addressed, derived over plugin storage, pack-extensible) and a **grant** +(fixed | choice → compiled to `selection-cfg`). Filters are predicates over *present* +metadata: absent metadata → not offered → **never an error**; tags are a useful add, not a +required schema. Blank-slate parametric grants (+N ASI / +N speed) are built-in pools + +parametric modifiers, same primitive. This *passes* D12 (thicker than a hardcoded vector, +intent-revealing) where `by-parent` failed it. *Rejected:* a cryptic DSL; new vocabulary +beyond pool/grant. + +**D20 — Variants are designed-in-now, built-later, via one indirection.** A variant +(`_copy` + `_mod`, the 5etools shape) resolves to an ordinary item in a pre-pass: +`raw :plugins → resolve-variants → resolved-content → pools → grants`. `resolve-variants` is +**identity today**. The binding rule: *every pool derives from one `resolved-content` +indirection, never raw `:plugins`*. Hold it and variants slot in later with no refactor of +the pool/grant work. Variants reference base by stable key, not name (D10). *Rejected:* +ignoring variants now (would force a later refactor) and building full resolution now (YAGNI). + +**D21 — Maintainability is a GATE: easier to add tooling, not harder.** The whole point is that +exposing a new grant-type/pool must drop from O(builders) bespoke edits (today) to O(1) +registration (register a pool once → grantable in every builder; "boons → feats/classes" falls +out free because boons are already a pool). This is guarded by two non-negotiable disciplines: +(1) `grant` is a **thin compiler** to `selection-cfg` — pool-kind logic lives in each pool's +definition, NEVER as a `cond` inside `grant` (that's the D14 god-function trap); (2) **one +reused** grant-authoring UI component, not per-builder forks (pools carry light "which builders +may offer me" scoping). **Falsifiable proof, not a promise:** the first slice's acceptance test +is "exposing a *second* pool in a builder is a ~1-line registration, shown in a commit"; if it +isn't trivially cheap, the retooling failed and we STOP. *Rejected:* taking "it'll be easier" +on faith — it must be measured. + +**D22 — Builder FORMS are data; "irreducible per-type work" was a retreat reflex.** Claimed (in +conversation) that each type needs a bespoke builder form. The code disproved it: `boon-builder` +and `invocation-builder` were identical forms differing only by a `set-*-prop` keyword. +Collapsed into `simple-content-builder` (`109b5dd0`). The genuinely irreducible core is small — +the **field schema** (data) + a reusable widget registry for complex fields + the field→mechanics +mapping (mostly the existing `:props` vocabulary) — NOT a per-type form. *Process lesson:* the +"irreducible" framing appeared right after the readability pushback and functioned as a way to +lower the bar instead of keep hunting efficiencies — the same retreat/toadyism failure mode logged +in `verification-discipline.md`, recurring in new clothes. Caught by the user; the corrective is to +treat "this part is irreducible" as a claim that must be proven against the code, not asserted. + +**Pins (designed-in, built-later):** variants (D20); new-skill *creation* (adds to the skill +registry, not a grant — different shape); the class-feature pool (`[:class-feature :X]` — +richer than flat pools); a declarative cross-type prereq vocabulary (`has-class?`/`level>=`/ +`has-feature?`/`ability>=` — homebrew prereqs must not be raw fns); **mechanical effects for +text-only content** (boons/ki/sorcery-points are prose today — authors should attach real +modifiers/resources; user flagged boons as an enhancement; same Axis-B "declare-as-data" +family). + +## Part 5 — Mechanization, class features, spell slots (the expansion) + +*(Numbering note: the Part 4 duplicate "D17" is now **D17b** (the Part 3 D17 owns the bare number — it +is the one cited across the codebase). This part continues at D23. D-numbers are stable IDs cited from +tests/code/other docs, so entries are never renumbered once cited — corrections are stubbed in place.)* + +**D23 — This branch is a PROTOTYPE to decide a standard, then converge on it.** The purpose is to +understand the app fully (by reading code, not assumptions/vibes), **decide** the best shape for each +mechanism, then **run with it** — slowly upgrading the codebase to that standard and **jettisoning +anything that doesn't fit**. So open questions (e.g. the grant approach, D29) are expected mid-prototype; +the deliverable is a decision + convergence, not a permanent fork. Corollary: every decision below is +provisional until "fully understood," and the loser of a competing-approach decision is removed, not kept. +**Clarification (user, 2026-06-19) — don't over-rotate into purge-zealotry.** Parallel/duplicate models +DURING testing are fine and expected — that's what prototyping is — *as long as each is clearly marked* +(a comment/flag) so it can be cleaned, updated, or removed later. Cleanup follows a DECISION; it does not +precede one. Do not delete a model just because another looks like it's winning. "Jettison what doesn't +fit" is the *end* state after deciding, not an in-flight reflex. + +**D24 — Class features → one keyed, filterable registry; pools are filtered views over it.** Not +per-feature pools (category mismatch) and not whole level tables. An editable reference is a **key + +`:overrides`**; a feature is a parameterized record with defaults; overrides deep-merge at compile. +Scaling/padding (ASI, num-attacks, `level-val`) stays as existing primitives. (`class-features-and-mechanization.md`.) + +**D25 — Features are macro-captured CODE, so extraction needs a `compile-feature` step — and the +summary is fields + a fill template, NOT string interpolation.** `dependent-trait`/`action`/etc. splice +the cfg (with live `?class-level`/`level-val`) into code at compile time. `compile-feature` translates a +DATA spec → the same cfg: scaling via a `{level→n}` schedule + a runtime `level-lookup`; the overridable +numbers (heal die/bonus, sneak die) are **fields**, and `:text` is a template (`{name}` prints, `{+name}` +signs). **Proven** against the real build — data specs reproduce fighter Action Surge/Second Wind and +rogue Sneak Attack, with `:uses` and `:die` overrides changing only their field (`compile-feature` proof, +`class_feature_snapshot_test.clj`). Retracts the earlier "summary scaling is a separate blocked templating +sub-problem" framing. + +**D26 — Per-class catalogue (C1) done for all 12; it re-shapes B1/B3.** Sizing: ~3–6 distinct +auto-features/class, but monk/paladin ~10, druid/sorcerer/wizard ~2–3, warlock ≈ 0 (all selections). The +registry/compiler must also handle: use-counts from **non-level sources** (ability-mod, formula, level +itself); **class-wide resource pools** (ki/sorcery/Lay-on-Hands — their own mechanism, B3, not per-feature +frequency); summaries that interpolate the **build context** (save DC, ability bonuses, user selections); +**multi-part features** (compile → a seq of modifiers); **attribute interdependence** (`?martial-arts-die`, +`?paladin-aura`, `?wild-shape-cr`). Extraction order: clean classes first (fighter/rogue, then +bard/cleric/wizard); defer monk/paladin/druid; warlock barely participates. (`class-feature-catalogue.md`.) + +**D27 — Spell slots: replace the overloaded `:level-factor` with a bucket of tables + a declared +multiclass rule.** One integer currently drives the solo slot table AND the multiclass contribution +(`int(level/factor)`) AND the prepared count — which is why Artificer can't be expressed and why +undocumented factors 4/5/6 exist. Decouple into: (1) a **bucket of named/explicit slot tables**, authored +as an **absolute per-level grid** (the app converts; presets seed the grid); (2) a **separately-declared +multiclass rule** (`:full|:half|:third|:none|:separate`); (3) its own prepared/known count. `:separate` +(pact) schedules own their pool + recharge (→ B3), generalizing warlock's hardcoded branch. (`spell-slot-progression.md`.) + +**D28 — Non-SRD content (e.g. Artificer) is never shipped pre-built; it must be user-assembled from +generic tools, and expressiveness is validated with a SYNTHETIC stand-in, never a copyrighted fixture.** +The dead `ua_artificer.cljc` is a *capability witness* (the primitives suffice), not a shippable start. +Same rule as Maneuvers/Mariner. Infusions ≈ a **scaling, swappable, item-granting pool** — the warlock +invocation pattern (`eldritch-invocation-selection`) generalized + magic-item reuse — so Artificer is a +forcing function for the pool/grant + B3 + spell-slot work, not a special case. + +**D29 — OPEN (the one unresolved decision): the grant approach.** The Phase-2 `grant-selection` +bridge prototype (`c1f54967`, `options.cljc:3447`) is a **generic** grant compiler (generic `:tags`, no +`:ref`). The Phase-1 **D17 audit decided against** a generic wrapper — point existing per-feature +`selection-cfg` constructors at **open pools**, preserving their load-bearing `:ref`/`:tags`; the +hand-wired draconic grant is the thing to generalize. These are two approaches to one goal. Per D23, this +must be **decided and converged**, not left forked: pick one, migrate to it, jettison the other. +(Per the D23 clarification, the `grant-selection` prototype can REMAIN as a clearly-marked parallel while +we test — it already carries "BRIDGE PROTOTYPE" comments — rather than being deleted pre-decision.) +**Recommendation:** D17's open-pool approach (preserves the `:ref`/`:tags` the engine relies on); fold the +prototype's cross-bucket intent into it. + +**D29 — RESOLVED (2026-06-25): the governing rule is NO DUPLICATED FUNCTIONALITY — one mechanism per job.** +The real decision was never "generic `grant-selection` vs per-feature-over-pools, jettison the loser" (a +narrow slice — both are thin compilers to `selection-cfg` over the same `content-pools/pool`, per D30, so +neither is a parallel engine to delete). The real decision is **bespoke built-ins (years-stable) vs the +systematic pool/grant approach**, and the resolution is: **pool/grant is the STANDARD for new / homebrew / +cross-silo capability; the stable bespoke constructors are KEPT where they aren't cross-silo and aren't +hurting, and MIGRATED only opportunistically (when a real reason already touches the code) — never a +big-bang rewrite, never churn for its own sake.** Litmus test for any new pool/grant: would it DUPLICATE a +working bespoke path without replacing it? If yes, don't — either replace that path or leave it be. Commit +to the *direction*, not a rewrite. (Squares with D23 "decide a standard, converge" and the stability+ +flexibility north star: flexibility comes from the systematic surface, stability from not touching proven +code without cause.) **Migration mechanics** — verified (characterization test first) and deprecated, not +deleted (D34 + `backfill-ledger.md`). + +**D30 — There is no pre-existing "grant" in the project; "grant" earns its keep only as a thin compiler +to the existing primitives, not a parallel selection engine.** Verified (callers, not comments): the +original vocabulary is `mod5e/*` modifiers (fixed grants — D4), `:props`→`plugin-modifiers`→ +`make-feat-modifiers` (declarative fixed mechanics, run for feats/races/subraces/classes/subclasses/ +ancestries via `spell_subs.cljs:144/157/457/491/779`), and `selection-cfg` (choices, carrying load-bearing +`:ref`/`:tags`). "Grant" is branch-introduced. **Verdict (per D12):** the grant *idea* — a data declaration +that compiles to a `selection-cfg` over an OPEN pool, preserving `:ref`/`:tags` — is a **beneficial wrapper** +(thicker: adds openness + author-declarability + filtering that don't exist today). The `grant-selection` +*implementation as built* (generic `:tags #{:grant from}`, no `:ref`) is **parallel duplication** of +`selection-cfg`, worse (drops the metadata). Resolution: grant = thin compiler to the existing primitives; +the value is openness + O(1) authoring + deleting hardcoded vectors, NEVER a second selection engine. + +**D31 — The two declarative vocabularies share an effect set (real duplication) but differ in +application mode (a REAL distinction) — factor out the shared effects, keep both modes.** +⚠️ **Corrected** (this entry first claimed "no useful distinction; B does no level-gating" — WRONG, see +the methodology note). Traced up+down: +- **Shared / duplicated:** the ~9 overlapping keys (weapon/skill/armor prof, resist/immunity, save-adv, + fly/swim speed) compile to the *same* `mod5e/*` primitive in both `make-feat-modifiers` (A) and + `level-modifier` (B) — verified down (both call e.g. `mod5e/damage-resistance`). The effect arms are + reimplemented in two `case`s. That redundancy is real. +- **Distinct / load-bearing:** **application mode.** A (`:props`) is a **flat, unconditional** attribute + map ("this content has X"). B (`:level-modifiers`) is a **level-gated list** — each entry carries a + `:level`, and `make-levels` (`spell_subs.cljs:392`) `(group-by :level …)` places it at that class level + ("gain X at level N"). B can express level progression; A cannot. The value-shape difference + (A map-of-flags vs B single-value-with-`:level`) reflects this, it is not arbitrary. +- **Better target (revised):** ONE shared effect vocabulary (the type→`mod5e/*` arms, defined once) used + by BOTH a flat path and a level-gated path — or generalize to "an effect + an optional level/condition." + NOT "collapse to one compiler" (that was the wrong conclusion — it would lose the level-gating). +- *Methodology note (verification-discipline):* I read the leaf compile fn and asserted behavior without + tracing the caller that supplies the gating. Reading the leaf is not reading the feature — trace up to + the wrapper (here `make-levels`) and down to the primitive before concluding. The user caught this. + +**Same smell, smaller scale: `:lizardfolk-ac` +and `:tortle-ac`** (`options.cljc:3332/3339`) are two bespoke natural-AC functions where one parameterized +`:natural-ac` prop arm (base / +dex with cap / +shield) should serve both and delete them. These are the +model of "better": a parameterized declarative handler that compiles to the existing engine and replaces +duplicates — not a parallel layer. (Correction logged: an earlier turn cited these AC fns as a *virtuous* +code-escape-hatch; they are duplication. The escape-hatch principle is real, but these aren't an instance of it.) + +**D31 follow-up (verified, 2026-06-19): the two vocabularies are in DIFFERENT LAYERS, and the shared half +is now pinned by a test.** Vocab A (`make-feat-modifiers`/`plugin-modifiers`) is **cljc** (`options.cljc`); +vocab B (`level-modifier`/`make-levels`) is **cljs** (`spell_subs.cljs`). So unifying them isn't a +two-`case` merge — it's a cross-layer refactor (the shared effect vocabulary would need to live in cljc and +be consumed by both the cljc and the cljs assembly paths). Test coverage of the claim (per the standing +rule): `grant_vocabulary_characterization_test.clj` pins, under the JVM gate, that A's `:damage-resistance` +compiles to the *same* `mod5e/damage-resistance` modifier B uses (shared primitive) and that A's value +shape is a map-of-flags. B's arms call the same `mod5e/*` (source-verified, `spell_subs.cljs:177`), and the +level-gating *mechanism* is already pinned by `class-feature-snapshot-test` (fighter Indomitable @9, absent +@5). B's cljs assembly is now ALSO pinned — `grant_vocabulary_cljs_test.cljs` (run in the headless cljs +harness) shows `level-modifier` compiles `:damage-resistance` to the same `mod5e/*` modifier and +`make-levels` places a `:level 3` modifier at level 3 (level 1 at 1, nothing at 2). So both halves of D31 +are test-backed across both layers; nothing here is prose-only anymore. + +**D32 — UI dropdowns must round-trip their value TYPE; templated via `:typed?` (verified, 2026-06-23).** +An HTML `` that returns an int/nil — so this is a ``), `:number`, `:text`. **A `:boolean` type is + pending, not built here on purpose:** toggle nil-safety is owned by `common/toggle-in` / + `common/toggle-flag` (path-safe, collection-preserving, self-healing — they fix the real bug: a + toggle path landing on a MAP did `(not map)` → `false`, collapsing the collection) + export cleanup + via `strip-export-blanks`, on branch `claude/custom-class-source-error-2k5ykd`. When that lands, add + `:boolean` here routing THROUGH that helper — do **not** reintroduce a parallel toggle fn. (The + feat `:saves?` marker and the `:save` rider are set-membership, not boolean fields, so they're a + different shape and not at this risk.) +- `simple-content-builder` — Name/Source/Description + declared `extra-fields`. +- `optional-builder-section [label has-content? body]` — collapsed-by-default toggle for non-standard fields. +- `ability-increase-choices` / `save-proficiency-choices` — silo-generic `(item, setter)` editors for the ASI spread / standalone saves (see `ability-increase-spreads.md`). +- **`builder-notes [problems {:severity :error|:advisory}]`** — the ONE place item-level feedback is rendered (`:error` = blocking "Fix before saving:" list; `:advisory` = ⚠ guidance). Producers stay separate and hand it a seq of strings (`bf/validate-fields`, selection-builder's name checks, `opt/save-coverage-warnings`); per-row highlighting stays bespoke. Add new builder feedback here, don't re-style. + +### Draconic-ancestry builder — DONE end-to-end (`0aca6113`) +A user can now author a draconic ancestry in-app; it stores into `::e5/draconic-ancestries` +(the pool dragonborn grants from), exports, reimports, and a character's choice survives +save/load. **The real "add a type" cost, measured with the new tools:** 9 files touched, but +only **two required thought** — the view's damage-type field (~10 lines) and the 1-line spec. +The rest were one-line registrations down the established pattern: `register-homebrew-content!` +(one descriptor vs ~10 scattered event regs), `simple-content-builder` (form = sub+event+one +extra field), `content_types` entry, route def/seg/allowlist/page-map (one line each), db +slot/default/local-store (mirrors boon). Verified by a real **builder-flow** test (drive +set/set-prop → output validates against the save spec) + the pool + round-trip proofs — not +injection. So adding a type is genuinely cheaper now; the residue is the field schema + the +occasional custom field widget, exactly as D22 predicted. + +### Foundation: registry DRIVES the layers (the real "fewer files" fix) +The original complaint was *file count*, not per-file effort. `content_types` was built as a +passive list only the subs loop read; every other layer was hand-wired per type → "9 files of +one-liners." The fix: make each layer **generate** its wiring from the registry. Progress +(each behavior-preserving, harness-gated): + +- ✅ **events** (`d2e002b4`) — ONE loop over `:homebrew-builder?` registry entries calls + `register-homebrew-content!`. Event keywords derived from `:builder-item` by the uniform + `/-` convention (still literal at dispatch sites, so grep works); the + localStorage interceptor built generically from `:local-storage-key`. **No events.cljs edit** + for a new homebrew type. +- ✅ **db** (`af68061d`) — the `:homebrew-builder?` types' `default-value` builder-item slots + generated from the registry's `:builder-item` + `:default`. **No db.cljs edit** for new types. +- ✅ **routes** (`506c32b3` cycle break, `c5e9aea6` bidi, `58c4de47` set+allowlist) — the + registry→route_map dep cycle is broken (`:route-kw` is now a plain keyword literal, registry + is a pure-data leaf), and the **bidi tree segments**, the **`dnd-e5-my-content-routes` nav + set**, and the **`routes.clj` SPA allowlist** all generate from the registry. A new homebrew + type's URL resolves, joins My Content, and is allow-listed **automatically**. Guarded by + `content_types_routes_test` (drift: literals == route_map vars; bidi: every URL resolves; + set + allowlist membership). `route_map` keeps only the one route-keyword `def` per type + (D6 — referenced by symbol in views/core); `routes.clj` needs **no** per-type edit. +- ⚠️ **core page-map** — NOT a clean win, skip: a builder's view *function* can't be derived + from data (cljs has no reliable runtime symbol→var resolution), so generating it only *moves* + a per-type binding (best co-located in a `views/builder-page-views` map next to the forms). + The view-fn binding is irreducible; put it where the form already is. + +**Net after events+db+routes:** adding a homebrew type no longer touches events.cljs, db.cljs, +or routes.clj, and route_map only needs its one route-keyword `def`. Remaining per-type files: +the registry entry (the one you should write), the view form (irreducible custom UI), the spec +(until spec-from-field-schema), the route-keyword def, and the core/views view binding. + +### NEXT levers (pick per value) +- (a) **author-declarable grants** (the biggest remaining lever). Re-scoped after the D17 audit: + do NOT build a generic `grant` wrapper — the per-feature `selection-cfg` constructors + (`fighting-style-selection`, `feat-selection`) already do "choose from a set" well and carry + load-bearing `:ref`/`:tags`. The real work is (i) point those selections' `:options` at an + **open pool** (built-in ++ homebrew), and (ii) let an author declare a grant as data that + **compiles to the same `selection-cfg`** (preserving `:ref`/`:tags`) + the authoring UI. The + code-side constructors stay. NOTE: `content_pools` has `pool` but NO grant compiler yet — + draconic hand-wires its grant; that hand-wire is the thing to generalize (carefully). +- (b) **spec-from-field-schema** — generate the `s/keys` spec from the field list, removing the + one hand-written row left in the cost table; +- (c) **cross-silo reuse demo** — point the sorcerer draconic bloodline (`classes.cljc:2280`) at + the *same* ancestry pool, so one pool feeds two silos ("built here, called over there"); +- (d) **breath-area field** + the level-gated/variant pins for full FTD coverage. + +### PINS (designed-in-now, built-later — do not let these get refactored away) +- 🔴 **HIGH PRIORITY — conditional-required field validation (`:required-when`).** `bf/fields->spec` + generates the save spec optional-by-default and enforces plain `:required?`, but does NOT yet + enforce fields that are required *only given another field's value* — e.g. `line-width`/`line-length` + are required when shape = `:line`, `length` is required when shape = `:cone`, and each is + meaningless otherwise. Today those are plain-optional in the spec (the form's `:when` only + hides/shows them), so a `:line` ancestry with no width currently *validates*. Build a + `:required-when (pred)` field key → `bf/fields->spec` adds a `spec/and` predicate enforcing it. + Flagged loud in `builder_fields.cljc` too. **Do not let this get lost.** +- **Variants** (`_copy` + `_mod`): the `resolved-content` indirection above is the only thing + required now. Build `resolve-variants` later; pools/grants stay untouched. +- **New skills** (creating a brand-new skill, not granting one): adds to the skill registry + itself — different shape. Defer. +- **Class-feature pool** (`[:class-feature :X]`): richer than flat pools; later phase. +- **Declarative cross-type prereq vocabulary** (`has-class?`, `level>=`, `has-feature?`, + `ability>=`): homebrew-authored prereqs must NOT be raw fns (security/stability). The engine + evaluates prereqs already; the small declarative vocabulary is the new part. Build when the + first cross-type gate is needed. +- **Mechanical effects for text-only content** (Axis B sibling): boons — and ki/sorcery-points + — are today just descriptive `:summary` text; the mechanical benefit they describe isn't + modeled. Authors should be able to attach real modifiers/resources, not just prose. Same + family as the play-time-resources finding (ki is text, not a tracked pool). User flagged + boons explicitly as an enhancement. Defer; same "declare-as-data" pattern will apply. +- **Level-gated grants in `:props`** (FTD axis 3): the `:props` mechanics-as-data vocabulary + has no level condition, so "gain X at level 5" (Gem Flight, Chromatic Warding, the level-5 + Metallic Breath option) isn't expressible by an author yet. Mechanism exists in the engine + (the `?total-levels` conditional, as breath-weapon damage dice use); the gap is exposing it + declaratively. Likely needs a new `:props` key added to `make-feat-modifiers` + (`options.cljc:3287`) for telepathy and similar, too. + +## What already stands (don't redo) +- `register-homebrew-content!` (the wiring sub-layer) + boon swapped through it. +- Phase 4b passthrough-subs loop; the `content_types` registry (data + audit test). +- Import-validation fixes + `save-character` crash fix. +- The cljs harness, compat invariants, verification-discipline lessons. + +## Deferred — own branch (surface at branch close) +- Character-validation contract (`character-validation.md`). +- ClojureScript tests into CI (the harness here is the prototype). diff --git a/docs/kb/content-extensibility-e2e.md b/docs/kb/content-extensibility-e2e.md new file mode 100644 index 000000000..cfedc8f25 --- /dev/null +++ b/docs/kb/content-extensibility-e2e.md @@ -0,0 +1,72 @@ +# Content Extensibility — Live / E2E Verification Checklist + +**Purpose:** The work on branch `claude/zen-wright-04xhdz` was verified only by the JVM +gate (`lein test` + `lein lint`). That gate does **not** execute the ClojureScript +re-frame subscriptions or the running app. This checklist is for an agent/dev in a full +environment (figwheel + browser + backend/Datomic) to verify the parts the JVM gate +skips, and report back. + +**Branch:** `claude/zen-wright-04xhdz` (includes a merge of `feature/name-keyword-fix`). +**What changed and why these checks exist:** +- Phases 1–3b rerouted plugin subscriptions through a new `option_catalog` seam + (`spell_subs.cljs`: subraces, subclasses, boons, invocations). Behavior should be + identical — these checks confirm it in a live app. +- The merge brought in the name-keyword fix (stable keys, `::plugin-source`, a + spell-selection reconciler). +- Phase 4a added a `content_types.cljc` registry (not yet wired to anything; no runtime + effect expected). + +## Setup (use the project's standard dev flow) + +- **cljs tests:** `lein fig:test` — compiles the `test` build (`orcpub.test-runner`) and + runs `subs_test`, `events_test`, `content_reconciliation_test`. Confirm **0 failures**. +- **App:** standard dev setup — backend (Datomic) + `lein fig:dev` for the hot-reload + frontend. See `docs/GETTING-STARTED.md` / `docs/DATOMIC_SETUP.md` on `agents/develop` + for environment details. + +## Checks + +Report PASS/FAIL + notes for each. Capture browser console (F12) errors and a screenshot +of the relevant builder/sheet where noted. + +### A. ClojureScript test suite (the JVM gate skips this) +1. `lein fig:test` reports **0 failures / 0 errors**. (Paste the summary line.) + +### B. Catalog read-seams — behavior must be UNCHANGED (Phases 1–3b) +Import an `.orcbrew` containing homebrew content, then in the character builder: +2. A homebrew **subrace** appears under its parent race. +3. A homebrew **subclass** appears under its parent class. +4. A homebrew **pact boon** appears in the Warlock level-3 "Pact Boon" selection. +5. A homebrew **eldritch invocation** appears in the Warlock invocation selection. +*(If any fail to appear, the `option_catalog` re-pointing regressed a subscription.)* + +### C. name-keyword fix (merged in) +6. Enable the "show homebrew source on class names" preference → class names show the + source suffix, AND selecting/using the class still works (key unchanged, no orphan). +7. Load a previously-saved character that uses a homebrew class → it still resolves; + watch the console for spell-selection reconciliation logs and confirm **no errors**. + +### D. Backward compatibility (non-negotiable — do not skip) +8. Import a real, pre-existing `.orcbrew` library → all content loads, **no validation + errors** in the console. +9. Load a saved character that chose a homebrew **pact boon** and a homebrew **subrace** + → both choices are intact (not "(not loaded)"/orphaned). Then **export** it back to + `.orcbrew` and re-import → loads cleanly. +10. A character saved on `develop` (before this branch) loads here with identical + selections. + +## Feedback format + +Reply with: +- The `lein fig:test` summary line (item 1). +- A PASS/FAIL line per item (2–10) with a one-line note on any failure. +- Console errors/warnings verbatim, and screenshots for B (builder lists) and C/D + (the loaded character sheet). + +Hand the results back (PR comment or message). I'll fix any regression before continuing +to Phase 3c (positional-threading removal) and 4b–4f (wiring the registry). + +## What is NOT in scope here +- Phase 3c and 4b+ are not implemented yet, so there is no new registry-driven wiring to + test. These checks confirm the *current* branch (catalog read-seams + the merged fix) + behaves exactly like the app did before, plus the fix's intended behaviors. diff --git a/docs/kb/content-extensibility-framework.md b/docs/kb/content-extensibility-framework.md new file mode 100644 index 000000000..fac78fa3d --- /dev/null +++ b/docs/kb/content-extensibility-framework.md @@ -0,0 +1,206 @@ +# The Content-Extensibility Framework + +**What this is:** the canonical reference for how orcpub's homebrew/content system is being +rebuilt so (a) adding a content type is one registry entry instead of edits in ~9 files, and +(b) content can grant choices from *other* content (cross-silo customization). Written for +**both humans** (the mental model, the why) **and agents** (precise schema, conventions, +invariants, how-to). Read this to understand or extend the framework. + +> ⚠️ **STATUS: partially built — this doc marks current vs planned explicitly.** Do not assume a +> layer is generative unless the status table below says ✅. Roadmap lives in +> `content-extensibility-direction.md`; the *why* behind every decision is in +> `content-extensibility-decisions.md` (referenced as D1–D22). +> +> 📍 **For what each silo can express *today* (the starting point this framework improves on), see +> [`decision-vocabulary.md`](decision-vocabulary.md)** — every builder form traced backward to its +> assembly fn, with the cross-silo gaps the "grant" layer below is meant to close. + +--- + +## 1. The mental model (start here) + +A **content type** (spell, feat, boon, draconic ancestry, …) is described **once as data** — a +single entry in the `content_types` registry. The framework has two halves: + +1. **The Builder Framework** — the per-type *wiring* (events, db draft state, subscriptions, + routes, the builder page) is **generated from the registry entry**, not hand-written per type. +2. **The Composition layer (pool + grant)** — content is exposed in open, type-addressed + **pools**; any other content can **grant** a (filtered, gated) choice from a pool. This is + how a feat grants a fighting style, or dragonborn grants a homebrew draconic ancestry. + +The payoff: *add a type* = one registry entry (+ its genuinely-custom form/spec/rules); +*let content tap other content* = a grant referencing a pool. Both are data, not boilerplate. + +The deciding principle (D12): **an abstraction earns its keep only when it is thicker than what +it hides AND its interface reveals intent.** Collapse mechanical duplication; keep readable, +meaningful code explicit; never force a genuinely-different kind of thing through one pattern. + +--- + +## 2. The Builder Framework (registry-driven wiring) + +### 2a. The single source: `content_types` +`src/cljc/orcpub/dnd/e5/content_types.cljc` holds one descriptor per plugin-based homebrew type. +It is a **dependency-leaf** (currently requires only `route-map`; the routes pass will make it a +pure-data leaf — D7) so every other layer can read it without circular deps. + +**Registry schema (per entry):** + +| key | meaning | consumed by | required | +| --- | --- | --- | --- | +| `:id` | content-type id (keyword) | identity / `by-id` index | yes | +| `:type-name` | human label ("Pact Boon") | builder UI, error messages | yes | +| `:builder-item` | app-db key holding the in-progress draft | subs, db, events | yes | +| `:spec` | spec the saved item is validated against | save handler | yes | +| `:plugin-key` | `::e5/*` key the items live under in `:plugins` | save/delete, pools | yes | +| `:route-kw` | builder page route keyword | routes, core, events | yes | +| `:route-seg` | builder page URL segment | routes (bidi) | yes | +| `:local-storage-key` | localStorage draft key | events (interceptor) | yes | +| `:homebrew-builder?` | opt this type into the **events + db generative loops** | events, db | for loop-driven types | +| `:default` | the empty-draft value (usually `{}`) | db default-value, new-item event | with `:homebrew-builder?` | + +### 2b. What's generated from the registry — CURRENT STATUS + +| Layer | File | Status | Mechanism | +| --- | --- | --- | --- | +| **subscriptions** | `spell_subs.cljs` | ✅ generated | `doseq` over registry → `reg-sub` builder-item passthroughs | +| **events** | `events.cljs` | ✅ generated | `doseq` over `:homebrew-builder?` → `register-homebrew-content!` | +| **db draft slots** | `db.cljs` | ✅ generated | builder-item `default-value` slots from `:builder-item`+`:default` | +| **routes** | `route_map.cljc`, `routes.clj` | ✅ generated | bidi segs + `my-content` set + SPA allowlist from `:route-seg`/`:route-kw` (registry is now a pure-data leaf; guarded by `content_types_routes_test`). `route_map` keeps only the one route-keyword `def` per type (D6). | +| **core page-map** | `core.cljs` | ⚠️ won't generate | a view *fn* can't be derived from data in cljs — the route→view binding is irreducible (best co-located with the form) | +| **spec** | per-type ns | ✅ generated (draconic) | `bf/fields->spec` over the field schema — optional-by-default, required name/key/option-pack + `:required?` fields, enum values validated. 🔴 conditional-required (`:required-when`) NOT yet enforced — high-priority pin. | +| **builder form** | `views.cljs` | ✅ collapsed (not generated) | `simple-content-builder` makes it a one-liner; custom fields via `extra-fields` | + +### 2c. The wiring HOFs (the trusted thick parts the loops compose) +- `register-homebrew-content!` (events.cljs) — from one descriptor registers + save/delete/edit/new + set/set-prop/reset. The events loop builds this descriptor per + registry entry. +- `reg-save-homebrew` / `reg-delete-homebrew` / `reg-edit-homebrew` / `reg-new-homebrew` — the + existing per-concern factories `register-homebrew-content!` composes. +- `simple-content-builder [item-sub set-prop & [extra-fields]]` (views.cljs) — the generic + builder form (Name + Option Source + Description + optional extra fields). + +### 2d. Conventions (agents: follow these exactly) +- **Event keyword naming.** In the `:builder-item`'s namespace, the verbs `save-`/`delete-`/ + `edit-`/`new-`/`set-`/`reset-` and `set--prop`, where `` = the builder-item + name minus `-builder-item` (e.g. `::class5e/boon-builder-item` → `::class5e/save-boon`). The + events loop *derives* these; they are also **literal at every dispatch site** in views + (`builder-page`, `simple-content-builder`, new/edit/delete buttons) so grep still finds them. +- **`builder-item` naming** must be `::/-builder-item` for the convention to hold. +- **Keys come from stable ids, never display names** (D10). The save handler keys an item by + `name-to-kw` at creation; never re-derive identity from a name display code may mutate. +- **The `content_types_test` registry guards** the entry count + the exact builder-item set — + adding a type updates those two locked assertions (by design: drift fails loudly). + +### 2e. HOW TO ADD A HOMEBREW CONTENT TYPE (current state) +1. **Registry entry** in `content_types.cljc` (all schema keys; `:homebrew-builder? true` + + `:default {}` to opt into the events/db loops). ⇒ events + db + subs wiring is now automatic. +2. **Spec** — one `(spec/def ::homebrew- (spec/keys :req-un [::name ::key ::option-pack]))` + in the type's ns (mirror `::homebrew-boon`). +3. **Builder form** in `views.cljs` — `(defn -builder [] (simple-content-builder + [extra-fields…]))` + `(defn -builder-page [] (builder-page "..." + -builder))`; add the my-content menu entry. +4. **Routes** (until the routes pass lands): `route_map.cljc` (def + route-set + bidi seg), + `routes.clj` (allowlist), `core.cljs` (route→page). +5. **Game-rule wiring** — if the type is *granted* by other content, register a **pool** and a + **grant** (§3); if it stands alone in its own list, nothing more. +6. **Update `content_types_test`** count + builder-item set. +7. **Verify** with the gate (§5). + +> The irreducible per-type work is the **field schema** (the form's custom fields), the **spec**, +> and **how it plugs into game rules**. Everything else is generated or a one-liner (D22). + +--- + +## 3. The Composition layer (pool + grant) + +The capability that makes this a *framework*, not just a builder generator: content tapping +content across silos, with filtering, gating, and prerequisites. + +### 3a. Pool +A **pool** is an open, type-addressed collection of grantable things: built-in entries (in code) +`++` homebrew entries (from loaded `.orcbrew` packs). Defined by the pure leaf primitive +`src/cljc/orcpub/dnd/e5/content_pools.cljc`: +```clojure +(pool plugin-vals plugin-key built-in) ; = built-in ++ (mapcat (comp vals plugin-key) plugin-vals) +``` +Pools are **memoized subscriptions** that read through `::e5/plugin-vals` — the **single +resolved-content seam** every plugin pool already uses, and where variant (`_copy`/`_mod`) +resolution will slot in later **without changing pools or grants** (the variant pin, D20). + +### 3b. Grant +A **grant** is what a content item declares to tap a pool. One primitive, three faces: +- fixed: "you gain this specific thing" (= the existing modifier system, keep — D4); +- choose-from-filtered: a `selection-cfg` whose options are pool entries matching a filter; +- choose-from-all: an unfiltered selection over the open pool. +The engine ALREADY supports filter/gate/prereq (`selection-cfg` carries `prereq-fn`/`tags`/ +`ref`; `option-prereq` exists) — the framework exposes that as *declarable data* (D18). Filters +are predicates over *present* metadata: absent metadata → not offered → **never an error**. + +### 3c. Mechanics as data +Content carries real mechanics declaratively via a `:props` map compiled by the EXISTING +`opt5e/plugin-modifiers` / `make-feat-modifiers` vocabulary (speed, flying-speed, +saving-throw-advantage, skill-prof, language, …). Built-ins with no `:props` are unchanged. + +### 3d. Worked example — draconic ancestry (the proven slice) +- Pool: `::races5e/draconic-ancestry-pool` = built-in colours `++` `::e5/draconic-ancestries` + homebrew (`content_pools/pool`). Dragonborn's "Draconic Ancestry" choice grants from it. +- Each entry compiles (in `draconic-ancestry-option`) to resistance + the breath-weapon the + race's attack reads + any `:props` riders. A homebrew gem ancestry inherits full mechanics. +- End-to-end proven: authored in-app builder → pool → export → import → **character round-trip** + (the choice survives save/load by its stable key). Tests in `draconic_ancestry_test.cljs` + (cljs) + `extensibility_golden_test.cljc` (JVM round-trip). + +### 3e. How to add a pool / a grant +- **New pool:** `(reg-sub ::x-pool :<- [::e5/plugin-vals] (fn [pv _] (pool pv ::e5/ )))`. +- **New grant:** a `selection-cfg` whose `:options` map a per-entry compiler over the pool sub. + Pass each entry's stored `:key` through (D10). Keep the compiler thin; pool-kind logic lives + in the pool, never as a `cond` inside a shared grant fn (D14 god-function trap). + +--- + +## 4. Invariants & gotchas (agents: violating these breaks user data or the framework) + +- **D10 — identity from stable keys, never display names.** Saved characters reference content + by key. Re-deriving a key from a mutated name orphans characters. +- **D14 — don't force heterogeneous kinds through one pattern.** Magic items (server-persisted) + and the combat tracker (transient) are **excluded** from the registry/loops on purpose. +- **Variant forward-compat (D20).** Every pool derives from the one `::e5/plugin-vals` + resolved-content seam — never raw `:plugins` — so `_copy`/`_mod` variants slot in later. +- **Routes fail-closed.** A missing generated route entry → 404, not a security hole; still, + verify routes resolve after the routes pass. +- **String-vs-keyword (learned the hard way).** UI dropdowns emit *strings*; the engine expects + *keywords* (`:thunder`, `::char5e/dex`). A builder field must store the keyword, and tests + must use the field's **real output**, not idealized keyword data. (This shipped a broken + breath-weapon: damage type stored as `"thunder"` → display + resistance broke.) +- **Greppability.** Derived event keywords are still literal at their dispatch sites; the + registry is the single greppable source. Keep it that way — don't add a second derivation. + +--- + +## 5. Verifying changes +Run the full gate before each commit (behavior-preserving until a step intends otherwise): +``` +lein lint # clj-kondo — must be 0 errors +lein test # clj + cljc (incl. content_types + golden round-trip tests) +lein fig:test # compiles the cljs; catches keyword/route/symbol errors +# then the headless harness for cljs behavior — see cljs-headless-harness.md +``` +Tests must be **falsifiable** (if you break the code, does it go red?). Behavior-preserving +refactors are proven by the boon/draconic tests passing **unchanged**. For UI, the isolated +component preview (mount + Playwright screenshot) can show a form renders correctly. + +--- + +## 6. Map of the docs +- **This doc** — the framework reference (what it is, schema, conventions, how-to, invariants). +- `content-extensibility-direction.md` — the **roadmap / current direction** (what's done, next). +- `content-extensibility-decisions.md` — **why** (D1–D22, incl. the pivots and the deflation→re-centering). +- `registry-before-after.md` — concrete **before/after** of adding a type. +- `content-extensibility-compatibility.md` — backward-compat invariants for orcbrew/characters. +- `cljs-headless-harness.md` — how to run the cljs tests headless. +- `verification-discipline.md` — testing/honesty lessons (incl. the toadyism failure mode). +- `homebrew-content-merge.md` — **READ before claiming a content type isn't homebrew-extensible.** + The recurring `feat-options` trap (static `*-options` defs are SRD-minimal/`#_`-commented *by + design*; homebrew is merged at the `concat` assembly point, not in the static def). Feats ARE + extensible; fighting styles genuinely are NOT (no plugin path — the one real gap). diff --git a/docs/kb/content-extensibility-plan.md b/docs/kb/content-extensibility-plan.md new file mode 100644 index 000000000..f9d34f1ca --- /dev/null +++ b/docs/kb/content-extensibility-plan.md @@ -0,0 +1,221 @@ +# Content Extensibility — Implementation Plan + +> ⚠️ **SUPERSEDED / DEFLATED — see `docs/kb/content-extensibility-direction.md`.** This doc is *history*: the grand registry / catalog-grant framing was scaled back after a readability review. Read it for analysis/context, not as the plan. + +**Purpose:** A step-by-step playbook to implement the content-extensibility redesign +safely. Written for agents with little context: follow it literally, in order, and stop +where it says stop. Do not improvise. + +**Status:** Plan for **not-yet-started** work. The design is in +[content-extensibility.md](content-extensibility.md); the hard constraints are in +[content-extensibility-compatibility.md](content-extensibility-compatibility.md); +rationale in [content-extensibility-decisions.md](content-extensibility-decisions.md). + +**Branch note:** references use the monolithic frontend layout; on `agents/develop` +views are split. Grep symbols to confirm exact locations. + +--- + +## Golden rules (read before doing anything) + +1. **Implement on a code branch** off the active code line (e.g. `feature/content-extensibility`). + Do **not** write source on `agents/develop` (docs-only) or on the docs branch. +2. **One phase per branch, one phase per PR.** Do not start a phase until the previous + phase is merged and green. +3. **Behavior-preserving until Phase 5.** Phases 0–4 must not change any built character + or any loaded library. The Phase 0 golden test must stay green on every commit. +4. **Never change persisted or exported shapes. Never rename an existing key** — plugin + key (`:orcpub.dnd.e5/…`), route keyword, selection key, option key, or localStorage + key. If a step seems to require it → **STOP and ask a human.** (Why: compatibility + doc §3 invariants.) +5. **Do not touch the modifier system (`mod5e/*`).** Out of scope. +6. **Run the full gate before every commit** (next section). Never commit a red gate. +7. **Keep the diff inside the files listed for the phase.** If the change spreads beyond + them → **STOP** and reassess; the phase was misunderstood. +8. **If anything is ambiguous, STOP and ask.** Do not guess. A wrong guess here breaks + users' saved characters. +9. **Never "fix forward" a behavior difference.** If the golden test changes, revert and + reassess — do not patch until it passes. + +## The verification gate (exact commands) + +Run all, expect all green, before each commit: + +``` +lein lint # clj-kondo; must pass +lein test # clj + cljc tests +lein fig:test # compiles and runs the cljs tests (incl. the golden test below) +``` + +The Phase 0 golden test is a cljs test, so it runs under `lein fig:test`. + +--- + +## Phase 0 — Build the safety net (no production code) + +**Goal:** a test that fails loudly if any later phase alters a built character or a +loaded library. Everything else depends on this existing first. + +**Files:** new `test/cljs/orcpub/dnd/e5/extensibility_golden_test.cljs`; reuse an +existing fixture from `test/*.orcbrew` (or add a small one); committed golden EDN +snapshots under `test/`. + +**Steps:** +1. Follow the existing test pattern in `test/cljs/orcpub/dnd/e5/subs_test.cljs` + (`reset! app-db`, `rf/clear-subscription-cache!`, `rf/subscribe`, assert). +2. Set up representative cases that exercise the cross-links being migrated: + - a Dwarf character with a homebrew subrace, + - a Warlock character with a pact boon (and one invocation), + - an existing `.orcbrew` fixture loaded into `(:plugins app-db)`. +3. For each case, build the character through the existing pipeline and capture the + **strict** form (`char5e/to-strict`) as an EDN snapshot. Commit those snapshots. +4. The test asserts the freshly built character equals the committed snapshot. + +**Gate:** the three commands pass; snapshots committed. +**Done when:** the golden test passes on unmodified `main` code. +**STOP if:** a build is nondeterministic (snapshots won't stabilize) — report it; do not +loosen the assertion to make it pass. + +## Phase 1 — Generic option injector, proven on subraces + +**Goal:** introduce one generic "group plugin options by a parent key" function and route +**subraces** through it with byte-identical output. This is the smallest possible Layer 2 +step and it changes no behavior. + +**Preconditions:** Phase 0 merged and green. + +**Files:** new leaf ns `src/cljc/orcpub/dnd/e5/option_catalog.cljc`; +`src/cljs/orcpub/dnd/e5/spell_subs.cljs` (the `::races5e/plugin-subraces-map` and +`::races5e/races` subs, ~887 / ~893). + +**Steps:** +1. In the new ns, write a pure function that reproduces today's grouping + (`(group-by options)`). No new behavior, no new data shape. +2. Re-point `::races5e/plugin-subraces-map` to call it. The value must be identical. +3. Run the gate. The golden test (Dwarf + homebrew subrace) must still pass. + +**Constraints:** the new ns may require only data/spec namespaces — **no** requires on +`events`, `subs`, or `views` (keeps it a dependency leaf; compatibility doc and +decisions D7). +**Gate:** all green; diff limited to the two files. +**Done when:** subraces resolve through the generic function with identical builds. +**STOP if:** the golden subrace character changes at all → revert. + +## Phase 2 — Migrate subclasses onto the injector + +**Goal:** same as Phase 1, for `::classes5e/plugin-subclasses-map` (`group-by :class`, +~893). Proves the pattern generalizes. + +**Files:** `option_catalog.cljc` (reuse the fn), `spell_subs.cljs` (the subclasses sub). +**Gate / Done / STOP:** identical to Phase 1, with a subclass golden case. + +## Phase 3 — Boons and invocations onto grants (the risky migration) + +**Goal:** replace the positional `boons`/`invocations` arguments threaded into the +warlock with a catalog/grant pull — **preserving the selection key and every option +key.** This is where compatibility can break; treat it carefully. + +**Preconditions:** Phases 0–2 merged and green. + +**Files:** `src/cljc/orcpub/dnd/e5/classes.cljc` (`warlock-option` ~2987, +`pact-boon-options` ~2629, the invocation options); `src/cljs/orcpub/dnd/e5/spell_subs.cljs` +(`base-class-options` ~932, `::classes5e/classes` ~945). + +**Hard requirement:** the "Pact Boon" selection's key and each boon/invocation option key +(derived via `common/name-to-kw` of the same names) must be **unchanged**. Verify against +the golden Warlock-with-boon character — it must be byte-identical after the change. + +**Steps (small, in order):** +1. Add a catalog read for `:pact-boon`, derived from the existing `::e5/boons` map + (same data, no storage change). +2. Make the warlock pull boons from that catalog instead of the positional argument, + producing the **same** selection and option keys as before. +3. Only after the catalog path is proven, remove `boons` from the positional signatures + (`warlock-option`, `base-class-options`) and the `::classes5e/classes` inputs. +4. Repeat 1–3 for invocations. + +**Gate:** all three commands green; golden test green, **especially** the Warlock+boon +case. +**STOP if:** the golden Warlock character changes in any way → revert. A changed selection +key is a compatibility break (compatibility doc §3 invariant 3, §5 risk 1). + +## Phase 4 — Layer 1 content-type registry (independent track; micro-steps) + +**Goal:** collapse the scattered registration into one `content-types` descriptor list, +reusing the **existing** factories (`reg-save-homebrew`, `reg-new-homebrew`, +`reg-edit-homebrew`, `reg-local-store-cofx`, `builder-page`) and the **existing** keys and +route keywords. No new content types here. This is compatibility-neutral (decisions D2). + +**Do it one subsystem per PR**, each independently verifiable: +- **4a** subs: replace the per-type `::…/builder-item` passthrough subs with a loop. +- **4b** db: build the `default-value` builder-item slots and `reg-local-store-cofx` + calls from the registry. +- **4c** events: generate `set-`/`reset-` events and the `reg-*-homebrew` calls from the + registry. +- **4d** routes: derive the bidi tree, route sets, and `routes.clj` allowlist from the + registry. Keep the `(def …-route :kw)` lines (decisions D6). +- **4e** core: build the `pages` map from the registry; resolve `:view` by keyword in + `core.cljs` (which already requires `views`) — do not store view fns in the registry + (decisions D7). + +**Per micro-step:** add descriptors for the **existing** types only; convert that one +subsystem to a loop; confirm the registered routes/events/subs/keys are identical (the +app boots, the golden test passes, no route/event/sub/key name changed). +**Gate:** all green; golden test green. +**STOP if:** any route, event, subscription, or localStorage key name changes → revert. + +## Phase 5 — New capability: dragonborn lineage (only after 1–4) + +**Goal:** add one new content type end-to-end, as proof the architecture pays off. + +**Steps:** +1. Add one descriptor to `content-types` with a **new** plugin key in the + `orcpub.dnd.e5` namespace (compatibility doc §1a requires that namespace). +2. Write the builder form and the `homebrew-*` spec. +3. Declare a grant on dragonborn for the lineage/ancestry catalog. Convert + `dragonborn-option-cfg` (`spell_subs.cljs` ~759) from a `def` to a function (or a + catalog-reading sub) only as far as needed. + +**Gate:** golden test green (existing characters and libraries unaffected) **plus** a new +test: a lineage from an imported `.orcbrew` appears under dragonborn. +**STOP if:** any existing golden case changes. + +--- + +## Stop-and-ask triggers (summary) + +Stop and get a human when: +- a step seems to require renaming or removing an existing key (plugin / route / + selection / option / localStorage), +- the golden test changes and you can't explain why, +- the diff grows beyond the files listed for the phase, +- you'd need to loosen a spec to load existing data, +- you're unsure whether a change is additive. + +## Two standing rules for the catalog/grant phases (3c onward) + +- **Keys from stable ids, never display names.** Pass each catalog item's stored `:key` + through to `option-cfg`/`selection-cfg`; never let identity re-derive from a `:name` + that display code may manipulate (this orphaned saved characters once — see + `feature/name-keyword-fix` and compatibility §1b). Keep display separate from identity. +- **Catalogs are layered, memoized subs.** A `grant-choice` references a catalog `reg-sub`; + it must not rebuild a whole option list inside a hot subscription. Add a short guard + comment at each catalog sub so the layering isn't collapsed later. + +## Do NOT + +- change persisted or exported data shapes, +- rename existing keys, +- re-derive identity keys from display names / call `name-to-kw` on manipulated names, +- recompute a whole catalog inside a hot subscription, +- touch `mod5e/*`, +- combine phases or skip the gate, +- commit a red gate, +- patch over a behavior difference instead of reverting. + +## References + +- [content-extensibility.md](content-extensibility.md) — design. +- [content-extensibility-compatibility.md](content-extensibility-compatibility.md) — + invariants and risk surfaces (read §3 and §5 before Phase 3). +- [content-extensibility-decisions.md](content-extensibility-decisions.md) — why. diff --git a/docs/kb/content-extensibility.md b/docs/kb/content-extensibility.md new file mode 100644 index 000000000..d408068a8 --- /dev/null +++ b/docs/kb/content-extensibility.md @@ -0,0 +1,159 @@ +# Content Extensibility + +> ⚠️ **SUPERSEDED / DEFLATED — see `docs/kb/content-extensibility-direction.md`.** This doc is *history*: the grand registry / catalog-grant framing was scaled back after a readability review. Read it for analysis/context, not as the plan. + +**Purpose:** Explain why adding a content type or builder to the 5e app touches so +many files, and propose a direction to reduce that cost without losing the +standardization the codebase already has. + +**Status:** +- "The problem" and "Current cross-links" are **verified from code** (file:line). +- "Proposed direction" is a **design proposal — not implemented.** Keep that line + intact when editing; only verified source behavior belongs in the rest of the KB. + +**Branch note:** Line references were read on a branch where the frontend is still +monolithic (`views.cljs`, `events.cljs`, one `spell_subs.cljs`). On `agents/develop` +the views layer is split (see [views-builders-split.md](views-builders-split.md)), +so view references resolve by symbol, not line. Grep the named symbol to confirm. + +--- + +## The problem + +Adding one content type touches ~8 files. The Pact Boon builder (commit `6029fd0`) +touched 10. The diff splits into two unrelated costs: + +1. **Registration** — route keyword, bidi entry, db default, localStorage key, + `->local-store` fn, init-db slot, set/reset/save events, a passthrough sub, and a + page-map entry. All keyed by the same entity, scattered across files. This is the + route-registration pain already noted in + [spa-routing-architecture.md](spa-routing-architecture.md), widened to db/events/subs. +2. **Injection** — wiring the new options into a *parent* entity (boons into the + warlock). Today this is done with positional function arguments, which is the + fragile half: a new child type means editing the parent's signature and the + subscription's binding vector in the exactly-right position. + +The registration cost is mechanical. The injection cost is where bugs hide. + +## Current cross-links (verified from code) + +How each child option set reaches its parent today. Note that one pattern +(bucket-by-key, used by subraces) is already clean; the others are not. + +| Link | How it's wired today | Reference | +|------|----------------------|-----------| +| subraces → races | **Bucket-by-key** ✅ `(group-by :race …)` merged into each race in `::races5e/races`. Adding a subrace needs **no** race edit. | `spell_subs.cljs` ~887, ~893–925 | +| subclasses → classes | **Bucket-by-key** ✅ `(group-by :class …)`. Same clean pattern. | `spell_subs.cljs` ~893 | +| boons → warlock | **Positional** ⚠️ `boons` threaded through `warlock-option` and `base-class-options`, plus an input added to the 8-input `::classes5e/classes` sub. | `classes.cljc` ~2629, ~2987; `spell_subs.cljs` ~945 | +| invocations → warlock | **Positional** ⚠️ Same shape as boons. | `classes.cljc` ~26; `spell_subs.cljs` ~945 | +| ancestries / lineage → dragonborn | **Static** ⛔ `draconic-ancestries` is a fixed `def`; `dragonborn-option-cfg` is a `def`, not a function. No plugin path exists. | `options.cljc` ~3428; `spell_subs.cljs` ~759 | +| spells → classes | **Context thread** `spell-lists`/`spells-map` passed positionally to every class builder. | `spell_subs.cljs` ~932 | + +This same problem appears in the issue tracker — see +[issues/homebrew-builders.md](../issues/homebrew-builders.md): #58 (invocations +hardcoded to Warlock, "requires generalizing"), #57/#209 (invocation prerequisites), +#172/#170 (selections in feat/subclass builders), #210/#107 (spells-known and +subclass spell-list expansion), #280 (metamagic builder), #173 (custom spell school), +#128 (choice-of-ASI in race builder). They are instances of the same two costs. + +## Proposed direction (design — not implemented) + +Two independent layers. Either is useful alone; together they cut the per-type cost +to "one descriptor + the builder form + the spec." + +### Layer 1 — content-type registry + +One list of descriptors as the single source of truth. The scattered registrations +become loops over that list, calling the factory functions that **already exist** +(`reg-save-homebrew`, `reg-new-homebrew`, `reg-edit-homebrew`, `reg-local-store-cofx`, +`builder-page`). This is moving call-sites into data, not new machinery. + +```clojure +(def content-types + [{:id :boon :name "Pact Boon" :builder-item ::boon-builder-item + :spec ::homebrew-boon :plugin-key ::e5/boons :default {} + :route-kw dnd-e5-boon-builder-page-route :view :boon-builder-page + :catalog-type :pact-boon} + ;; ...one entry per type + ]) + +(doseq [ct content-types] ; events.cljs, db.cljs, subs, core/pages, route_map + (register-content! ct)) ; calls the existing factories +``` + +Constraints found: keep the `(def …-route :kw)` lines (referenced by symbol at +compile time); the registry namespace must stay a dependency leaf to avoid the +circular-dep the code already works around (`events.cljs` ~204), so views are +referenced by keyword and resolved in `core.cljs`. + +### Layer 2 — type-addressed option catalogs + grants + +Generalize the subrace "bucket-by-key" pattern from `:race` to option **type**, so +producers and consumers never name each other. + +```clojure +;; producer: an option declares WHAT it is, not where it attaches +{:id :my-boon :type :pact-boon ...} + +;; one read API for "all options of a type" (built-ins + plugins + homebrew) +(catalog :pact-boon) + +;; consumer: grant a choice from a catalog, optionally filtered +(grant-choice :pact-boon :n 1) +(grant-choice :spell :n 1 :filter cantrip?) ; "choose a cantrip" +``` + +Why not fixed parent slots: in 5e a pact boon can be granted by the warlock *and* by +a feat, and homebrew adds more later. Addressing by parent location forces multiple +attachment declarations; addressing by type does not. Keep the existing modifier +system (`mod5e/*`) for granting a *specific known* thing (e.g. "grants Fire Bolt"); +catalogs/grants are only for "choose from a whole set." + +The result: a homebrew cantrip flows into every "choose a cantrip" grant for free, +and a feat that grants boons reuses the same `grant-choice :pact-boon` — neither +needs the producing module to change. + +**Compatibility constraint (non-negotiable):** catalogs must be **derived over the +existing plugin storage** (like subraces are), not a new on-disk format, and a +migration must **preserve the selection/option keys** that saved characters already +chose. This keeps existing orcbrew libraries and characters working with no migration. +Full analysis and invariants in +[content-extensibility-compatibility.md](content-extensibility-compatibility.md). + +**Two implementation rules for this work (learned the hard way):** + +1. **Keys from stable ids, never from display names.** Pass each catalog item's stored + `:key` through to `option-cfg`/`selection-cfg`; never let identity re-derive from a + `:name` that display code may manipulate. Folding a source suffix into a class + `:name` once orphaned saved characters via `name-to-kw` — see + `feature/name-keyword-fix` and the compatibility doc. Keep display (`:name`, + `::plugin-source`) separate from identity (`:key`). +2. **Catalog reads must be layered, memoized subscriptions.** A `grant-choice` must not + recompute a whole catalog inside a hot subscription. Each catalog is its own + `reg-sub` (memoized by re-frame); grants reference it. Guard these with a short + comment so the layering isn't accidentally collapsed later. (This is also the fix for + the monolithic god-subscriptions, e.g. the 8-input `::classes5e/classes`.) + +## Suggested next step + +A behavior-preserving spike: add the generic catalog injector and migrate +**subraces** onto it first (they already work this way), review the diff, then +migrate boons/invocations, then add new capability (lineages) the easy way. Guard each +step with an orcbrew + saved-character fixture (assert the built character is identical +before/after) — see the compatibility doc. + +## Related + +- [content-extensibility-plan.md](content-extensibility-plan.md) — step-by-step + implementation playbook (phased, behavior-preserving, with stop conditions). +- [spa-routing-architecture.md](spa-routing-architecture.md) — the route-registration + side of Layer 1 (route_map / index-page-paths / core.cljs pages). +- [entity-options-architecture.md](entity-options-architecture.md) — the + entity/option/selection model that `grant-choice` would build on. +- [srd-vs-plugin-content.md](srd-vs-plugin-content.md) — what is hardcoded SRD vs + plugin-supplied, which determines what each catalog contains. +- [content-extensibility-decisions.md](content-extensibility-decisions.md) — the + decisions behind this direction and the options rejected. +- [content-extensibility-compatibility.md](content-extensibility-compatibility.md) — + backward-compat audit: persisted formats, invariants, and how the design must stay + additive for existing orcbrew libraries and saved characters. diff --git a/docs/kb/data-safety-layers.md b/docs/kb/data-safety-layers.md new file mode 100644 index 000000000..d67109fa6 --- /dev/null +++ b/docs/kb/data-safety-layers.md @@ -0,0 +1,74 @@ +# Data-safety layers — prevent / harden / heal / surface + +How to make a content feature robust against bad data (legacy, hand-edited, ghost formats, junk). +Distilled from a cross-branch design review of the toggle-nil / map-collapse defect that both +`claude/zen-wright-04xhdz` and `claude/custom-class-source-error-2k5ykd` hit from opposite ends. + +**"Hardening vs self-healing" is the wrong axis.** They're two of FOUR layers, and a robust design +usually uses several — chosen by *where you are in the data's life and whose data it is*, not picked +once globally. + +## The four layers (preference order) + +1. **Prevent** — *shape / type.* Push the invariant into the field type or storage shape so the bad + state can't be authored. Absent = off; pick boolean-vs-set-member-vs-map-key deliberately; a field + type carries its own read-coercion + write-coercion + validation so no caller can reintroduce the + bug. The cheapest bug is the one that can't be created. Do this first. +2. **Harden** — *boundary / read.* The non-negotiable floor: no input may crash or corrupt. Defensive + reads, skip-uncompilable, fan-out safety. Always present. +3. **Heal** — *write / repair.* ONLY when there's an unambiguous correct target AND the data is worth + preserving. Repair lazily on read/interaction — no load-time migration pass. +4. **Surface** — *migration.* When you can't heal but the data MIGHT be meaningful: park it named / + recoverable and tell the user. Silent drop is allowed only for provably-meaningless junk. + +## The rule that picks between them + +| Situation | Layer | +|---|---| +| Fresh input from the current UI | **Prevent** — make the malformed state unauthorable | +| Real/legacy data with a known-correct shape | **Heal** — you must accept it (save ⊆ load), so repair it forward | +| Genuinely un-compilable garbage (no principled repair) | **Harden** — skip; healing would be *guessing* | +| Anything a human might have meant, that you'd otherwise drop | **Surface** — never a quiet skip | + +The clarifying frame is **lifecycle**: prevent at authoring, harden at read, heal at write, surface at +migration. Two cross-cutting invariants sit under all of it: +- **save ⊆ load** — anything that SAVES must LOAD; load is never stricter than save. This is what makes + healing possible at all (you can't repair data forward if load already rejected it). Never "tighten + validation on import." +- **Rejections say what and where** — carry a precise `:in` path into a human message. A filled-in + value that won't save, with no reason, is worse than a blank one. + +## Worked examples (this codebase) + +| Mechanism | Layer | Why | +|---|---|---| +| `bf/*` `:boolean` field type (planned, one primitive) | **Prevent** | invariant lives in the type; no caller can reintroduce nil | +| `opt5e/pool-entry?` (skip junk spread/save entries) | **Harden** | `[:bad]`/`[]` have no canonical value to heal to | +| `opt5e/toggle-increment-save` (rebuild increment canonically) | **Heal** | a malformed increment has ONE obvious right shape | +| `common/toggle-in` (collapsed `false` → fresh map) *(other branch)* | **Heal** | live user data with a known repair | +| `(not (true? v))` defensive read | **Harden** | garbage/nil/absent → off | +| `save ⊆ load` generative guard *(other branch)* | invariant | enables all healing | + +## Anti-patterns + +- **Harden-by-silent-drop on meaningful data.** Skipping is for garbage; meaningful data that can't be + handled must be *surfaced*, not quietly dropped. (Our current gap — see below.) +- **Heal speculatively.** Self-healing is more code, more test surface, and a subtle risk: a *wrong* + repair corrupts in a new way. If the correct target isn't unambiguous, harden or surface — don't + guess. Over-healing is guessing dressed up as robustness. + +## Tracked follow-ups + +- **`pool-entry?` surface — DONE (harden → surface).** The compilers still silently skip malformed + entries at runtime (correct for a sub's fan-out), but the authoring form now climbs to *surface*: + `opt5e/ignored-entry-warnings` counts the `:ability-increases` / `:save-proficiencies` entries a + compile would drop, and `ability-save-notes` renders it as a blocking-styled note ("N … entries are + malformed and will be IGNORED — each must be [amount pool]") under the ASI/save widgets. So a creator + editing imported/hand-edited content sees the drop instead of losing it silently. Proven by + `ability_increase_grant_test/ignored-entry-warnings-*` (JVM) + `test/e2e/ignored-entry-note.js` (the + builder survives the junk and shows the note). Guardrail 6. + +## See also +- `ability-increase-spreads.md` — the feature these examples come from (compile crash-safety section). +- `builder_fields.cljc` — the convergence note for the shared `:boolean` toggle primitive. +- `content-extensibility-decisions.md` — D33 (terse export data), D34 (deprecate-not-delete). diff --git a/docs/kb/decision-vocabulary.md b/docs/kb/decision-vocabulary.md new file mode 100644 index 000000000..4f86cb7b9 --- /dev/null +++ b/docs/kb/decision-vocabulary.md @@ -0,0 +1,342 @@ +# Homebrew Decision Vocabulary & Compile Paths + +**The model:** the app is the **tools + forms + logic** (server-side). An `.orcbrew` stores the +creator's **decisions** (data), not logic. On import the app's **compile paths** turn those +decisions into modifiers/selections. What a creator can express is bounded by the **forms** (input) +and the **compile paths** (what data the app knows how to realize). Goal: grow that vocabulary, give +it **cross-silo** reach, and make growing it **cheap/sustainable**. + +> **Scope:** this is a WIRING map — which decision keys each builder emits and which assembly fn they +> reach. "✅"/"rich" means a code path EXISTS, not that it was exercised on a built character; "❌" +> means no path was found in the code read, not a proven limit. The one finding taken all the way to +> behavior (and confirmed against a real `.orcbrew`) is the subclass-spellcasting gate below. +> +> **Deep-dives (canonical homes — this doc summarizes, those verify):** spells → +> `spell-granting-across-silos.md`; armor class → `armor-class-computation.md`; runtime toggles / +> conditional effects → `runtime-toggles-and-conditional-modifiers.md`; proposed grant/select +> builder vocabulary → `declarative-grant-vocabulary.md` (design). + +--- + +## Compile paths (load-time: decision data → content), verified + +### `:props` has TWO sides +**(a) `make-feat-modifiers` — FIXED mechanics, SHARED across silos** ✅ +- `options.cljc:3287`, driven by `plugin-modifiers` (`:3345`). +- Run for **races/subraces/classes/subclasses/feats/draconic-ancestries** (despite the "feat" + name — `spell_subs.cljs:144/157/447/481/769`, feats via `feat-option-from-cfg`). This is the + one genuinely cross-silo path today. +- Vocabulary (fixed grants): movement (`:speed`,`:flying-speed`,`:flying-speed-equals-walking-speed`, + `:swimming-speed`), profs (`:skill-prof`,`:skill-prof-or-expertise`,`:tool-prof-or-expertise`, + `:armor-prof`,`:weapon-prof`), defenses (`:damage-resistance`,`:damage-immunity`, + `:saving-throw-advantage`/`-traps`), `:language`,`:initiative`,`:max-hp-bonus`, + `:passive-investigation-5`/`-perception-5`, + hardcoded one-offs (`:two-weapon-*`, + `:medium-armor-*`,`:lizardfolk-ac`,`:tortle-ac`). + +**(b) `make-feat-selections` — CHOICES, but FEAT-ONLY** ⚠️ +- `options.cljc:3261`, invoked only via `feat-selections` ← `feat-option-from-cfg` (feats). + **Not** called from the race/subrace/subclass compile paths (grep confirms none in + `spell_subs.cljs`). So only feats get `:props` *choices*. +- Vocabulary (choices): `:weapon-prof-choice`, `:language-choice`, `:skill-tool-choice`, and + **spell-choice templates** `:ritual-casting`, `:magic-novice`, `:attack-spell` (→ the + ritual-caster / magic-initiate / spell-sniper selections). +- **CORRECTION to cycle 1:** homebrew **feats CAN grant spell choices** — via these 3 templates. + But only those fixed templates; there is **no general parameterized spell-choice decision** + (e.g. "choose 2 from the bard list, levels 1–2"). + +### `:ability-increases` → ASI (fixed OR choice) — FEAT ✅ +- `feat-modifiers`/`feat-selections` (`options.cljc:3355`/`3372`): one ability → fixed `+1`; + a *set* of abilities → an `ability-increase-selection` (CHOICE of which to bump). `:saves?` + adds save proficiency. So feats express "two stats / a set to choose from." +- The **inline "Custom" race menu** also offers `homebrew-ability-increase-selection` + (`options.cljc:2129/2176`). Resolved (backward trace below): the race *builder* exposes ASI as + **fixed per ability**, not a choose-which-to-bump option — ASI *options* remain feat-only. + +### `:prereqs` / `:path-prereqs` → `feat-prereqs` — FEAT, LIMITED vocab ✅ +- `options.cljc:3195`. Supported prereqs: an **ability** keyword → "≥ 13" (hardcoded threshold), + `:spellcasting` → can-cast, else **armor proficiency**; `:path-prereqs {:race …}` → must be a + given race. +- **GAPS:** no **level**, **N class-levels**, or **alignment** prereqs. Resolved (backward trace + below): prereqs are **feat-only** — not exposed in the race/subrace/class/subclass forms. + +### `:spells` → `spell-modifiers` — FIXED known spells ✅ +- `spell_subs.cljs:124`. Used by races/subraces (`:146/:159`). Compiles to `spells-known` + ("you know spell X"). No choice. + +### `:spellcasting` → `spellcasting-template` — CLASSES, full caster, custom list ✅ +- `options.cljc:697`. A homebrew **class** can be a full caster: cantrips/spells known by level + (`spells-known-selections` → real spell **choices**), known vs prepared, AND a **custom + `:spell-list`** (`:707` — `(assoc spell-lists class-key spell-list)`), so custom/expanded spell + lists work. This is the richest spell path — but it's **class-level only**. + +### `:level-modifiers` → `level-modifier` — SECOND grant vocabulary (classes/subclasses), incl. `:spell` ✅ +- `spell_subs.cljs:163`, run for classes AND subclasses via `make-levels` (`:396`), **no class gate**. +- Type vocabulary: `:weapon-prof`, `:num-attacks`, `:damage-resistance`, `:damage-immunity`, + `:saving-throw-advantage`, `:skill-prof`, `:armor-prof`, `:tool-prof`, `:flying-speed`, + `:swimming-speed`, `:flying-speed-equals-walking-speed`, **`:spell`** (→ `spells-known`). +- **`:spell` grants an INNATE KNOWN SPELL, not spellcasting.** `(:spell …)` calls + `mod5e/spells-known` (`modifiers.cljc:260`), which adds one spell to `?spells-known` castable via + the chosen ability — the **same mechanism races use for racial spells**. It does **not** grant + spell slots or a casting progression. Any class/subclass (incl. custom) can do this, but it's + bolt-on innate spells, not "this subclass makes you a caster." + +### ⚠️ CORRECTION (this doc was wrong before) — subclass spellcasting IS gated +An earlier version of this doc claimed "a subclass of a custom class can grant spells, Divine-Soul +style, the gated UI is just convenience." **That was wrong** (caught by the maintainer). The truth: +- **Real (slot-based) spellcasting via a subclass is GATED** to `#{:fighter :rogue :warlock :cleric + :paladin}` in the subclass builder (`views.cljs:5975`). A **custom non-caster base class gets no + spellcasting UI at all**, so you cannot make its subclass a real caster through the builder. +- `make-levels` offers class-name-gated shortcuts behind that UI: `:spellcasting` (1/3-caster, only + `class ∈ #{:fighter :rogue}` — Eldritch Knight / Arcane Trickster), `:paladin-spells`, + `:cleric-spells` (domain), `:warlock-spells` (expanded list choice). +- The path that *would* have let a custom subclass grant real spellcasting — + `custom-subclass-spellcasting-selection` (adds `spell-slot-factor` + per-level spell selections, + `options.cljc:2735`) — is **`#_`-commented out / disabled**. +- **Divine Soul is not a counterexample:** it's a *sorcerer* subclass, and sorcerer is already a + full caster, so its expanded list rides on the base class's existing spellcasting. It does not + demonstrate granting spellcasting to a non-caster. +- **The gate is in the COMPILE PATH, not just the UI — so an `.orcbrew` cannot bypass it.** + ✅ statically verified in `make-levels` (`spell_subs.cljs:392`, the exact fn that compiles + imported plugin subclasses via `::classes5e/plugin-subclasses`, `:448`): + `add-spellcasting? (and spellcasting (#{:fighter :rogue} class))` (`:396`) — a hand-authored + `:spellcasting` map is **ignored** unless `class ∈ #{:fighter :rogue}`; `:paladin-spells` only when + `class = :paladin` (`:411`), `:cleric-spells` only `:cleric` (`:424`), `:warlock-spells` only + `:warlock` (`:429`). So crafting EDN by hand does not get around the gate; the only ungated spell + path remains `:level-modifiers {:type :spell}` (innate known spells). **No runtime test needed — + the compile function is the authority and it gates.** +- **Net:** a subclass can add fixed *innate known spells* (`:spell` modifier) to any base class, but + **cannot grant a spellcasting progression to a non-caster base class** via the builders. + +#### ✅ Confirmed by real community data — the actual Divine Soul `.orcbrew` +Inspected the community "Xanathar's Guide to Everything" pack (the real Divine Soul the maintainer +remembered). It validates every point above: +- The whole 285 KB file has **exactly one** `:spellcasting` map and **one** `:spell-list` — both on a + standalone custom **class** keyed `:sorcerer-divine-soul-` (under `:orcpub.dnd.e5/classes`), with + `:level-factor 1` (full caster), `:ability ::char/cha`, a `:spells-known` schedule, cantrips, and + the merged sorcerer+cleric **custom `:spell-list`**. I.e. they rebuilt sorcerer as a custom class + so the class-builder's custom spell-list could carry the expanded list. +- The "affinities" (Law, etc.) are **subclasses parented to that custom class** (`:class + :sorcerer-divine-soul-`). **None** of them carry `:spellcasting`; they only add an innate themed + spell via `:level-modifiers [{:type :spell :value {:ability …cha :level 1 :key :bless}}]` + traits. +- So the community did **not** grant spellcasting via a subclass — they put spellcasting on a custom + class and used subclasses for flavor + innate spells. This is the empirical answer to "can a + custom base class get spellcasting from a subclass?": no — work around it at the class level. + +### TWO PARALLEL grant vocabularies — overlapping, divergent (the real duplication) ⚠️ +- `make-feat-modifiers` (`:props`) and `level-modifier` (`:level-modifiers :type`) both grant + mechanics, with overlapping coverage (weapon/skill/armor prof, resist/immunity, save-adv, + fly/swim speed) but **different extras**: `level-modifier` has **`:spell`**, `:num-attacks`, + `:tool-prof`; `make-feat-modifiers` has `:language`, `:initiative`, `:max-hp-bonus`, passive + senses, the spell-choice templates. So the *same* capability lives in two places with uneven + reach — one grants a spell, the other a language, neither both. This drift (rushed dev) is the + "built different" inconsistency; unifying the two vocabularies is a prime sustainability target. +- **✅ VERIFIED (traced up+down, 2026-06-19; supersedes a wrong first pass): they share an effect set + but differ in APPLICATION MODE.** The first version of this note said "no useful distinction; B does no + level-gating" — WRONG (I read the leaf compiler, not its caller). Corrected: + - **Shared / duplicated:** the ~9 overlapping keys compile to the *same* `mod5e/*` primitive in both A + and B (verified down — both call e.g. `mod5e/damage-resistance`). The effect arms are reimplemented + in two `case`s. Real redundancy. + - **Distinct / load-bearing:** A (`:props`) is **flat + unconditional** (an attribute map, "has X"). B + (`:level-modifiers`) is **level-gated** — each entry carries a `:level`, and `make-levels` + (`spell_subs.cljs:392`) `(group-by :level …)` places it at that class level ("gain X at level N"). So + B can express level progression and A cannot. The value-shape difference (A map-of-flags via + `collect-map-modifiers`; B single value + `:level`) reflects this — it is not arbitrary. + - **Different LAYERS (verified):** A is **cljc** (`options.cljc`); B is **cljs** (`spell_subs.cljs`). + So unifying is a cross-layer refactor, not a two-`case` merge: the shared effect vocabulary must live + in cljc and be consumed by both the cljc (A) and cljs (B) assembly paths. + - **Now TEST-BACKED (both layers), not prose:** + - vocab A — `grant_vocabulary_characterization_test.clj` (JVM): `:props :damage-resistance` compiles + to the same modifier as `mod5e/damage-resistance`; A's value shape is a map-of-flags that fans out. + - vocab B — `grant_vocabulary_cljs_test.cljs` (headless cljs harness): `level-modifier + :damage-resistance` compiles to the **same** `mod5e/damage-resistance` modifier (shared primitive, + confirmed on the cljs side); `make-levels` places a `:level 3` modifier at level 3 and a `:level 1` + at level 1, nothing at level 2 (level-gating confirmed). Run via `lein fig:test` + `/tmp/pw/run-cljs-tests.js` + (cljs-headless-harness.md); ns added to `test_runner.cljs`. + - the entity-level gating of a `:levels` map (a modifier at level N applies at/after N) is separately + pinned under JVM by `class-feature-snapshot-test` (fighter Indomitable @9, absent @5). + - **Better:** factor out ONE shared effect vocabulary (the type→`mod5e/*` arms, once, in cljc) used by + BOTH a flat path and a level-gated path, or generalize to "effect + optional level/condition." NOT + collapse to one compiler (loses the gating). (The "vocabulary A/B" labels are this KB's framing, not + source comments — those comments were added by this work.) + +### `:level-selections` → `level-selection` — TEXT-trait choices only ⚠️ +- `spell_subs.cljs:341`. Homebrew class/subclass level-selections resolve a `:type` to a homebrew + **Selection** whose options compile to `trait-cfg` (**name + description only** — mechanical + data dropped). A homebrew "choice" here = named descriptions, not real grants. + +### Resources (Axis B) — NO homebrew data path ✅ (confirmed gap) +- `used-resource` (`options.cljc:2034/2280/2636/2980`) tracks *which* limited-uses are spent, but + only in **built-in** content. `:props` has no resource key. No data path for a homebrew creator + to declare a trackable resource (ki/rage/sorcery). Matches the ki-is-text finding. + +--- + +## Backward trace (the CORRECT method) — verified per silo: builder form → assembly fn + +For each silo: what the **builder form** exposes, and the **assembly fn** that compiles it. + +### Feat — `feat-builder` (views `:5264`) → `feat-option-from-cfg` (`options.cljc:3396`) ✅ rich +Form sections: name/source/description, **prereqs** (`feat-prereqs`), **ASI** (fixed or choice), +skill/language/weapon/armor/tool prof, HP, damage-resistance, speed, initiative, misc, and +**spellcasting** (the magic-initiate/ritual/spell-sniper templates). All compile via +`feat-option-from-cfg` (`:props`→modifiers + `:props`-choices via `make-feat-selections` + +`:ability-increases` + `:prereqs`). + +### Race — `race-builder` (views `:6219`) → `race-option` (`options.cljc:2210`) ✅ rich +Form sections: name/source/description, size/speed/flying/swimming/darkvision, AC checkboxes, +**ASI** (fixed per ability), **prof CHOICES** (`option-skill-proficiency-choice`, +`option-language-proficiency-choice`, `option-weapon-proficiency-choice`), fixed profs +(weapon/armor/tool/skill/resistance/immunity/languages), and **spells** (`option-spells`). +`race-option` compiles `:abilities`, `:profs`→`:skill-options`/`:language-options`/ +`:weapon-proficiency-options` (→ `skill-selection`/`language-selection` CHOICES), `:subraces`, +`:traits`, `:spells`, `:selections`, `:props`. **Not fixed-only.** + +### Subclass — `subclass-builder` (views `:5946`) → `make-levels` (`spell_subs.cljs:382`) ✅ rich +Form sections: name, **parent class**, source; **spellcasting** UI *gated to +`#{:fighter :rogue :warlock :cleric :paladin}`* (fighter/rogue 1/3-caster toggle; paladin/cleric/ +warlock fixed-spell editors — convenience for built-in patterns); `option-skill-proficiency-choice` ++ `option-skill-expertise-choice` (CHOICES); **`option-level-modifiers`** (generic, shown for ALL +subclasses); `option-level-selections` (→ TEXT-trait choices); `option-traits`. +- **`option-level-modifiers` dropdown (`modifier-values`, views `:5374`) INCLUDES `:spell`** — + plus weapon/skill/armor/tool-prof, damage-resist/immunity, saving-throw-adv, num-attacks, + flying/swimming-speed. So a subclass of ANY class can add a fixed **innate known spell** via a + `:spell` level-modifier — but that's `spells-known` (racial-style innate spell), **NOT a + spellcasting progression**. **Real slot-based spellcasting via a subclass is GATED** to the 5 + classes above; a custom non-caster base class cannot be made a caster by its subclass through the + builder (see the CORRECTION section above — my earlier "just convenience" claim was wrong). + +### Class — `class-builder` (views `:5643`) → `level-option` (`options.cljc:2771`) ✅ rich (with a plugin gap) +Form sections: name/source/description; **hit die** (6/8/10/12); **subclass** pick-level/title/flavor; +**saving-throw** proficiencies; **ASI levels** (which levels 4–20 grant an increase — a *schedule*, +not a stat choice); **full spellcasting** — slots y/n, caster fraction (`:level-factor` full/half/ +third), spell list (one of the existing class lists **or Custom**, which opens per-level spell +checkboxes = a **custom `:spell-list`**), spellcasting ability, cantrip schedule, and a per-level +"spells this class can choose from" grid (→ real spell **choices** via `spells-known-selections`, +`options.cljc:647`); `option-skill-proficiency-choice` + `option-skill-expertise-choice` (CHOICES); +**`option-level-modifiers`** (B vocab, incl `:spell`); `option-level-selections` (TEXT traits); +`option-traits`. So the class silo is the **richest** — only place with full-caster spellcasting + +custom spell list. +- **`(not plugin?)` gate — investigated, NOT a homebrew gap.** In `level-option` the standard + **ASI selection**, **hit-points selection**, and per-level `modifiers/level` are gated + `(when (not plugin?) …)` (`options.cljc:2817/2819/2833/2841`). I initially read this as a + homebrew gap; it is **not**. `:plugin? true` is set **only on hardcoded UA template overlays** + (`templates/ua_*.cljc`) — partial add-ons that layer new subclasses/options onto an *existing* + class and must not re-emit core scaffolding. A homebrew class from the **builder** never sets + `:plugin?` (verified: not in the builder events, export, or `::classes5e/plugin-classes` + assembly at `spell_subs.cljs:464`, which only assocs `:plugin-source`/`:modifiers`/`:levels`). + So builder classes flow through `class-option`→`level-option` with `plugin?` falsey and **do** + get ASI selection + hit points normally. (Lesson reinforced: `:plugin?` ≠ "came from a plugin.") + +### Subrace — `subrace-builder` (views `:6090`) → `subrace-option` (`options.cljc:1984`) ✅ rich (≈ race) +Form sections: name, **parent race**, source; size/speed/darkvision; **ASI** (fixed per ability, +−2..+2, shown as race+subrace total); modifiers (`option-hps`, damage-resist/immunity, save-adv, +weapon/armor/tool/skill prof, **`option-skill-proficiency-choice`** = CHOICE, languages); **spells** +(`option-spells`, fixed-only); `option-traits`. Essentially the same vocabulary as race: fixed +mechanics + skill-prof choice + fixed spells + traits + fixed ASI. No spell choice, no prereqs. + +### Background — `background-builder` (views `:6368`) → `background-option` (`options.cljc:2456`) ✅ minimal +Form sections: name/source/description; **fixed** skill proficiencies (checkboxes, no "choose N"); +languages (`language-choice-checkboxes`); tool proficiencies; starting equipment; `option-traits`. +The simplest mechanical silo — fixed profs + equipment + traits. No ASI, no spells, no +level-modifiers, no `:props` mechanics, no prereqs. (Backgrounds in 5e are light by design.) + +### Simple types (boon/invocation/language/…) → `simple-content-builder` (views `:6547`) ✅ descriptive +**Verified:** these use `simple-content-builder` = **Name + Option Source + Description** plus an +optional list of declarative `extra-fields` (rendered via `render-builder-field`, validated by the +same `bf/validate-fields` used for import/export). Boons and invocations grant warlock features in +play, but the homebrew **decision** is name + descriptive text only — no mechanical grant vocabulary +(consistent with the `level-selection` text-only finding). Spell/monster/encounter/selection are +their own structured forms (stat blocks / option lists), not part of the cross-silo *grant* story. +`draconic-ancestry` is the one simple type with a real field schema (`field_schemas.cljc`). + +## SHARPENED duplication finding — grant types live in up to FOUR places +There are **two grant vocabularies**, and **each is split across UI + compile**: + +| Vocab | Compile | UI | Used by | +|---|---|---|---| +| **A — `:props`** | `make-feat-modifiers` (`options.cljc:3287`) | `feat-*` / `option-*` form sections | feats, races, subraces | +| **B — `:level-modifiers`** | `level-modifier` (`spell_subs.cljs:163`) | `modifier-values` dropdown (`views:5374`) | classes, subclasses | + +They overlap heavily (weapon/skill/armor prof, resist/immunity, save-adv, fly/swim speed) but +diverge (A: language, initiative, max-hp, spell-choice templates; B: **`:spell`**, num-attacks, +tool-prof). **To make one grant type available everywhere you may edit up to four sites** +(A-compile, A-UI, B-compile, B-UI). That fan-out is the sustainability tax and the "built +different" feel — unifying to one grant vocabulary (single source → compile + UI) is the prime target. + + +## Cross-silo capability table — REBUILT from the backward builder→assembly trace ✅ + +The same capability is available in some silos and not others, because each silo runs a different +subset of compile paths *and* a different builder form. **All six mechanical silos below are now +verified** per silo via the backward trace (builder form → assembly fn). +(✅ verified present, ❌ no path, ⚠️ partial/limited.) + +| Capability | Feat | Race | Subrace | Class | Subclass | Background | +|---|---|---|---|---|---|---| +| Fixed mechanics (`:props`/`:level-modifiers`) | ✅ | ✅ | ✅ | ✅ | ✅ | ⚠️ profs/equipment only | +| Prof/skill **choice** | ✅ | ✅ | ✅ skill-prof-choice | ✅ skill-prof/-expertise | ✅ skill-prof/-expertise | ⚠️ language/tool choice only | +| Fixed **innate** known spell (`spells-known`) | ✅ via template | ✅ `:spells` | ✅ `:spells` | ✅ `:level-modifiers :spell` | ✅ `:level-modifiers :spell` (any base class) | ❌ | +| **Spellcasting progression (slots)** | ❌ | ❌ | ❌ | ✅ full/half/third caster | ⚠️ **gated** to `#{fighter rogue warlock cleric paladin}` — custom non-caster base class **cannot** | n/a | +| **Spell choice** | ✅ 3 templates | ⚠️ fixed-only | ⚠️ fixed-only | ✅ full caster | ⚠️ class-gated UI only | ❌ | +| ASI | ✅ fixed **or** choice | ✅ fixed | ✅ fixed | ✅ standard at chosen levels | n/a | ❌ | +| Traits | ✅ | ✅ | ✅ | ✅ | ✅ | ✅ | +| Prereqs | ✅ limited vocab | ❌ not in form | ❌ | ❌ not in form | ❌ not in form | ❌ | +| Trackable resource (Axis B) | ❌ | ❌ | ❌ | ❌ | ❌ | ❌ | +| Custom/expanded spell list | ❌ | ❌ | ❌ | ✅ `:spell-list` | ⚠️ via `:spellcasting` | ❌ | + +**Reading (corrected):** **feat, race, and subclass are all fairly rich** — each gets fixed +mechanics, prof/skill **choices**, traits, and at least fixed spells. The asymmetries are narrower +than the old leaf-trace suggested: +1. **Spell *choice* is fragmented**, not a general decision: feats have 3 fixed templates, classes + have full-caster spellcasting, subclasses have only class-gated convenience UI, races have + fixed-only. There is **no general parameterized spell-choice** (pick N from list L, levels a–b) + anywhere. +2. **ASI options (choose-which-to-bump) are feat-only** — race ASI is fixed-per-ability in the form. +3. **Prereqs are feat-only** (and a limited vocab even there); races/subclasses don't expose them. +4. **Trackable resources** exist nowhere as a decision. +5. **Two parallel grant vocabularies** (`make-feat-modifiers`/`:props` vs `level-modifier`/ + `:level-modifiers`) overlap but diverge, so the *same* fixed mechanic is unevenly reachable and + must be added in up to four places (see fan-out table above). + +These five — especially the spell-choice fragmentation and the two-vocabulary split — are the +concrete "cross-silo dipping" frustration, restated precisely. + +## What's genuinely missing (the creator vision → gaps) +- **A general, cross-silo grant vocabulary**: `make-feat-selections` (choices) is feat-only; + races/subraces/subclasses can't reach it. The cross-silo win is making the choice/ASI/prereq + vocabulary available to *every* silo, not just feats. +- **A general parameterized spell-choice** (any list/levels/count/filter) instead of 3 fixed + templates (feats) + full-caster (classes) + fixed-known (`:spells`). +- **Unify the two grant vocabularies** (`make-feat-modifiers` ↔ `level-modifier`) so coverage is + even (a feat could grant `:spell`; a subclass level-modifier could grant `:language`) — prime + duplication-removal + sustainability target. +- **Subclass-granted spellcasting for non-caster base classes** — gated; the fix is re-enabling the + commented-out `custom-subclass-spellcasting-selection` (`options.cljc:2735`). See the CORRECTION + section above for the full finding (don't restate it). +- **Mechanical** homebrew choices (vs `level-selection`'s text-only). +- **Trackable resources** (Axis B) — no path at all. +- **Richer prereqs** — level / N-class-levels / alignment (engine likely supports the checks; + the *decision vocabulary* doesn't). + +## Sustainability note +Adding a *fixed* mechanic to the shared vocabulary = one `case` arm in `make-feat-modifiers` + a +form field, and it reaches every silo for free. The **choice** side (`make-feat-selections`) is +*not* shared — wiring it into races/subraces/subclasses (and generalizing the spell-choice +templates) is the highest-leverage cross-silo + sustainability work. + +## Status / next cycles +**Done (backward trace, verified):** Feat, Race, Subrace, Class, Subclass, Background — builder +form → assembly fn, plus the cross-silo capability table and the four-place duplication finding. +Confirmed: prereqs are feat-only (not in race/subclass/class forms); subclass spell *choice* is +class-gated UI only (fixed spells via `:level-modifiers :spell` work for any class); the +`(not plugin?)` gate is not a homebrew gap. + +**Also done:** simple/descriptive silos confirmed non-mechanical (`simple-content-builder`). + +**Done:** in-app source comments added at the key assembly fns (`make-feat-modifiers`, +`make-feat-selections`, `feat-option-from-cfg`, `race-option`, `subrace-option`, `level-option`, +`level-modifier`, `spellcasting-template`) cross-linking this KB. + +**Remaining:** the design/build work tracked in `declarative-grant-vocabulary.md` (uniform +data-path → reusable controls → generated builder UI), and the behavioral test plan (build +characters and observe; see that plan) to convert the ✅ "path exists" rows into observed behavior. diff --git a/docs/kb/declarative-grant-vocabulary.md b/docs/kb/declarative-grant-vocabulary.md new file mode 100644 index 000000000..030b6cf0e --- /dev/null +++ b/docs/kb/declarative-grant-vocabulary.md @@ -0,0 +1,120 @@ +# Declarative grant/select vocabulary (builder UI) — design + analysis + +A proposed authoring vocabulary for the BUILDER (how a creator expresses "this content grants +a spell / a choice / a conditional bonus"), and a real analysis of whether it works for spells. + +Markers: **VERIFIED** = read from code, file:line. **DESIGN** = proposed, not built. +**OPEN** = a point not yet agreed; confirm before building. + +## Two layers, kept separate (agreed) +- **Layer A — builder UI.** The form elements a creator uses to author a grant. This is what the + "generate UI from a grant declaration" idea is about. +- **Layer B — mechanical implementation.** How the authored data computes on the character + (modifiers, selections, `?`-attributes). Separate logic, even if related. + +A Layer-A form-generation concern (what controls to show) is distinct from a Layer-B concern (how +the data evaluates). An earlier note conflated them (worrying about boolean/AND-OR condition logic +as if it were a UI scope limit). It isn't: how conditions *combine mechanically* is Layer B; the +builder form is Layer A. The only Layer-A residue is "how does the form let a creator add more than +one condition" — see OPEN below. + +## The vocabulary (DESIGN) +Two verbs, distinguished by who makes the choice, plus filters: +- `` — the **creator** picks the spell(s); fixed on every character. Compiles + to `mod5e/spells-known`. +- `` | `spell-selection` | ✅ — filters = its existing list/level/restriction params | +| Cantrips | same, level-0 filter | `spell-selection`/`spells-known` | ✅ | +| Magic-Initiate style: pick a CLASS/list, then spells from it | nested/dependent select | `spell-selection` per chosen list | ⚠️ SPECIAL CASE — a two-level dependent choice, not a flat select | + +**SPECIAL CASE — dependent two-level choice (VERIFIED; corrects an earlier overstatement).** +I previously claimed the feat spell "templates" were ~18 redundant constructions that a single +`` does NOT subsume them: this is a **dependent two-level +choice** (pick a list, then spells from it), which the vocabulary must support as a nested select. + +The only genuine duplication is smaller: the six-class caster table (bard→Cha, cleric→Wis, +druid→Wis, sorcerer→Cha, warlock→Cha, wizard→Int) is written out in ~4 places +(`options.cljc:1390/1717/3249` + the ritual/sniper selections) and could be one shared table mapped +over. That's minor tidiness, not a structural collapse. + +**It does unify the two divergent fixed-spell data shapes (VERIFIED):** races use `:spells`, +classes/subclasses use `:level-modifiers {:type :spell}` — both reach `spells-known`. One `` collapses those. (This claim stands; the "subsumes 18" claim above did not.) + +## Compound grants — "pick 2 cantrips and a 1st-level spell" (the canonical case) +A single feat often grants several picks of different kinds. The form is a **repeatable list of +grant/select rows**, each with its own filters + count. "2 cantrips + 1 first-level spell" = +`` — which is exactly what +`magic-initiate-option` builds (two sub-selections). So the builder UI is "add a grant row" N times; +a compound grant is just multiple rows. The Magic-Initiate "pick a class first" case adds the +dependent/nested wrinkle on top (pick a list, then the rows draw from it). +(Note: this is separate from the *condition* combination flag below — compound spell grants are a +list of select rows; multiple conditions are an AND-list. Both are "a repeatable list" in the form.) + +## Scope decisions +- **Spellcasting progression is OUT of scope** — but note: progression was never requested in this + thread (it was my mis-attribution). The vocabulary is for granting *individual* spells as a + bonus/reward. Full caster progression stays the existing `:spellcasting` machinery. +- **Usage qualifiers ("once per long rest") need real plumbing, separately.** VERIFIED: there is no + creator-declarable use-count mechanism — `used-resource` is only a label, and magic-item/racial + limited-use spells are descriptive text (see spell-granting-across-silos.md). So a qualifier is + *descriptive only today*. Doing it properly is a **new tracked-resource layer** (max + current + + reset-on-rest), a separate project from the grant vocabulary, not a flag to hand-wave. +- **Magic items are a real spell source the grant vocabulary should cover.** Today they show spells + as `mod5e/action` text, not in `spells-known`. A `` routing to `spells-known` would + make them properly visible/castable instead of loose text. + +## Backward compatibility (hard requirement) +Any change to data shapes (e.g. unifying fixed-spell `:spells` vs `:level-modifiers :spell`) MUST +keep existing homebrew libraries and saved characters working — via a read-shim for the old shape if +the canonical shape changes. Prefer not churning saved data; if churn is unavoidable, migrate with a +shim, never break. This applies to every step of the uniform-data-path work. + +## Sequencing (agreed) +This is a step *out from* organizing the framework, not instead of it. Make the data path uniform +first — one `:spells` (fixed) and one `:spell-choice` (select) key routed to the existing primitives +across every silo's assembly fn — THEN build reusable controls (spell-picker, condition-picker), +THEN let the registry generate the builder form from the declaration. Generating UI over the current +inconsistent data path would hide the inconsistency, not fix it. + +## Status of the earlier flags (updated) +1. Spellcasting progression — RESOLVED: out of scope (and was my mis-attribution; never requested). +2. Usage qualifiers — RESOLVED as: descriptive today, and a proper fix is a **separate** tracked- + resource project (not part of this vocabulary). VERIFIED there's no creator-declarable use-count. +3. Compound grants ("2 cantrips + 1 spell") — RESOLVED: a repeatable list of grant/select rows. +4. Multiple **conditions** — still OPEN: default to an AND-list, defer OR/nested (Layer-A choice). +5. `:spells` data shape — RESOLVED to a principle: backward compatibility is mandatory; keep the old + shape readable (shim) if the canonical shape changes; prefer not churning. +6. Magic items as a spell source — NEW: covered by `` (today they're `mod5e/action` + text, not in `spells-known`). + +## Idiomatic check (Clojure/Reagent) +Data-driven UI (a render fn over a grant spec) is idiomatic — same pattern as the existing +`render-builder-field`, and more idiomatic than the current hand-written builder forms. Guardrails: +keep it data interpreted by small functions (not a macro-DSL), keep the vocabulary small/composable, +and compile DOWN to the existing primitives (`spells-known`, `spell-selection`, the modifier system) +rather than building a parallel engine. diff --git a/docs/kb/dropdown-value-coercion.md b/docs/kb/dropdown-value-coercion.md new file mode 100644 index 000000000..e1b51a5a5 --- /dev/null +++ b/docs/kb/dropdown-value-coercion.md @@ -0,0 +1,104 @@ +# Dropdown value coercion — the ``'s value is **always a string**. The `dropdown` primitive +(`views.cljs`) rendered each option's `value` from the item's typed `:value`, but on `change` it +read `.target.value` back as a **string** and handed *that* to the caller's `:on-change`: + +```clojure +:on-change #(on-change (event-value %)) ; event-value = (.. e -target -value) -> a String +``` + +reagent renders a keyword option value via `name`, so `::character/cha` round-trips out as the +bare string `"cha"` (namespace dropped). **Every** caller therefore had to manually re-hydrate the +type, and forgetting was undetectable by source review or the JVM/harness tests (those dispatch +events with already-correct values). Only a browser driving the real `` that parses internally and calls `:on-change` with a +real int (or `nil` for empty/non-numeric) — the caller never sees a string. It's already used in the +monster/magic-item builders. So: free numeric entry → `number-field` (typed for free); a constrained +numeric *choice* (a fixed small set, e.g. ASI `+1/+2/+3`) → a `:typed?` dropdown (an `` +can't constrain to a set). There is no native "numeric `` is string-valued by +the DOM spec, which is the whole reason `:typed?` has to do the index-round-trip. + +## Guard / convergence rule + +- **New dropdowns whose `:value`s are not already strings → use `:typed? true`** (or `number-field` + for free numeric entry). Then there is nothing to coerce and nothing to forget. Strings-only + dropdowns (e.g. `:size` stored as a name) may stay on the default path. +- **The existing ~70 coercing call sites are NOT a bug list** — most are upstream and correct + (e.g. `views.cljs:5801`, verified above). Migrating them to `:typed?` is optional readability + cleanup that removes per-caller coercion (and would let the bespoke `:enum` round-trip, + `views.cljs:~6595`, fold onto the primitive). Per the prototype-then-converge rule (D23) it follows + this decision rather than preceding it; it is a marked follow-up, not a blocker. + +See also: `docs/kb/cljs-headless-harness.md` (the E2E that caught this and the other interaction +gotchas) and the decision log entry **D32**. diff --git a/docs/kb/homebrew-content-merge.md b/docs/kb/homebrew-content-merge.md new file mode 100644 index 000000000..41da7d1ad --- /dev/null +++ b/docs/kb/homebrew-content-merge.md @@ -0,0 +1,88 @@ +# Homebrew content merge — and the `feat-options` trap + +**Why this doc exists:** people (humans AND agents) keep concluding "homebrew feats aren't +supported" because they read `feat-options`, see it's nearly empty, and stop. **That conclusion +is wrong**, and the same mistake generalizes to other content types. Read this before claiming a +content type isn't homebrew-extensible. + +--- + +## The trap, concretely (feats) + +`opt5e/feat-options` (`options.cljc` ~1212) looks like dead/empty code — **nearly every built-in +feat is `#_`-commented**, leaving effectively just Grappler: + +```clojure +(defn feat-options [spell-lists spells-map] + [#_(feat-option {:name "Alert" …}) ; commented + #_(feat-option {:name "Athlete" …}) ; commented + … ]) ; only Grappler (the one SRD feat) is live +``` + +**This is intentional, not broken.** orcpub ships only SRD-licensed content; the rest of the +"official" feats/subclasses/etc. are commented out and are expected to come from user **orcbrew +packs** (homebrew plugins). `content_reconciliation.cljs:194` literally notes: *"Grappler is the +only SRD feat."* + +**The real feat choice is assembled elsewhere** — `template.cljc:1540`: + +```clojure +(opt5e/feat-selection-2 + {:options (concat + (opt5e/feat-options spell-lists spells-map) ; built-in half (Grappler) + (map (partial opt5e/feat-option-from-cfg …) feats))}) ; ← HOMEBREW feats +``` + +where `feats` is the `::feats5e/feats` subscription (plugin feats). So the feat choice = +**built-in `feat-options`, then the homebrew feats concatenated in.** Homebrew feats **are** +available. The mistake is reading the static `feat-options` def and missing the concat at the +assembly point. + +The flow: `::e5/feats` (in `:plugins`) → `::feats5e/plugin-feats` (`spell_subs.cljs:495`, +`(mapcat (comp vals ::e5/feats) plugin-vals)`) → `::feats5e/feats` (`spell_subs.cljs:1034`, adds +`:edit-event`) → passed as `feats` into `template-selections` (via `equipment_subs.cljs:301`) → +`concat`-ed into the feat selection's `:options` (`template.cljc:1540`). + +--- + +## The general pattern (applies to most content types) + +A top-level homebrew content type **X** is surfaced like this — and you must look at the **merge +point**, not the static `*-options` def: + +1. **Storage:** items live in `:plugins` under a `::e5/X` key (e.g. `::e5/feats`, `::e5/races`). +2. **Plugin sub:** a subscription gathers them — `(mapcat (comp vals ::e5/X) plugin-vals)` + (e.g. `::feats5e/plugin-feats`, `::races5e/plugin-races`). +3. **Merge point (THE thing to find):** at the choice's assembly site, the plugin entries are + **`concat`-ed** with the built-in options, each mapped through the type's option constructor. + This happens in `template-selections` (`template.cljc`) or the type's own sub + (e.g. `::races5e/races` in `spell_subs.cljs`). + +So the built-in `X-options` is only **half** the picture (the SRD built-ins, mostly commented). + +--- + +## Verification recipe — "is homebrew X supported, and where does it merge?" + +DON'T answer from the static `X-options` def. Instead: + +1. `grep "::e5/X"` — is there a plugin content key + a `plugin-X` sub (`mapcat (comp vals ::e5/X)`)? +2. `grep` where that sub's value is **`concat`-ed into a selection's `:options`** (look in + `template.cljc` `template-selections`, and the type's sub in `spell_subs.cljs`). +3. If you find the concat → homebrew **is** merged. If the static `X-options` is mostly + `#_`-commented, that's SRD-minimal built-ins **by design**, not dead code. + +**Known exception (a real gap, not the trap):** **fighting styles** genuinely have *no* plugin +path — no `::e5/fighting-styles` key, no plugin sub, no concat; `fighting-style-selection` +(`options.cljc:1793`) always returns the fixed 6 built-ins. So *that* one really isn't +homebrew-extensible (yet). The way to tell the difference is step 2: feats have the concat; +fighting styles don't. + +--- + +## TL;DR +- Built-in `*-options` defs are **SRD-minimal and mostly `#_`-commented on purpose**; the rest + comes from orcbrew plugins. +- Homebrew merge happens at the **assembly/`concat` point**, not in the static `*-options` def. +- **Feats ARE homebrew-extensible** (`template.cljc:1540`). Don't re-derive otherwise. +- Fighting styles are the genuine exception (no plugin path) — confirmed by the *absence* of a concat. diff --git a/docs/kb/key-collision-behavior.md b/docs/kb/key-collision-behavior.md new file mode 100644 index 000000000..949646b42 --- /dev/null +++ b/docs/kb/key-collision-behavior.md @@ -0,0 +1,58 @@ +# Duplicate / colliding content keys — what actually happens, per layer + +Answers a recurring, hard-to-pin-down question: are content keys required to be unique? The honest +answer is **it depends on the layer** — some layers treat a same-key homebrew entry as an intentional +**override** of the built-in, others let duplicates **coexist**, and import has its own **conflict +gate**. This is the map, traced from code (file:line cited) and pinned by `key_collision_test.clj`. + +Markers: **VERIFIED** = read from code + test-backed. All cljs paths are in `spell_subs.cljs` / +`import_validation.cljs`. + +## TL;DR +- **Top-level class / race / spell:** a homebrew entry with a **built-in's key OVERRIDES the built-in** + (predictable, homebrew wins). This is the "override a built-in" capability some users rely on. +- **Subraces, pools, list content (backgrounds/languages/selections/monsters):** same-key entries + **coexist** (both appear) — no override. +- **Import:** duplicate keys are **detected** (within the import + against existing) and routed to a + conflict-resolution modal (rename / skip / replace) — the "duplicate keys won't just load" behavior. +- So keys are NOT globally unique-or-bust; uniqueness matters in different ways in different places. + +## The map (VERIFIED) + +| Layer | How content combines | Same-key collision | +|---|---|---| +| **Classes** | `(into (sorted-set-by ::t/key) (concat (reverse plugin-class-options) base-classes))` (`spell_subs.cljs:1016`) | **OVERRIDE** — plugin added first, the sorted set dedupes by key keeping the one already present, so the **homebrew class wins; the built-in is dropped** | +| **Races** | `(into (sorted-set-by compare-keys) (concat (reverse plugin-races) base))` (`:950`; `compare-keys` = `(compare (:key x) (:key y))`, `:936`) | **OVERRIDE** — homebrew race wins | +| **Spells** | `(into (sorted-set-by compare-keys) (concat (reverse plugin-spells) built-in))` (`:1228`); `spells-map` then reduces this deduped set into a map (`:1242`) | **OVERRIDE** — homebrew spell wins | +| **Subraces** | gathered by `mapcat` then `(update race :subraces concat (subraces-map key))` (`:161`, `:954`) | **COEXIST** — both same-key subraces appear under the race | +| **Backgrounds / Languages / Selections / Monsters / Encounters** | `(mapcat (comp vals ::e5/) plugin-vals)` (`:104/:110/:116/:1087/:1093`) | **COEXIST** — a seq; duplicates both appear | +| **Pools** (draconic ancestry, fighting-style) | `(concat built-in homebrew)` (`content_pools.cljc/pool`) | **COEXIST** — both offered in the choice | +| **Spell lists** | `(merge-with merge-spell-lists …)` = `merge-with concat` (`:1277`, `:1253`) | **COEXIST** — keys merged, lists concatenated | +| **Import** | `detect-duplicate-keys` → `find-key-conflicts` (`import_validation.cljs:1120/1097`) → `:internal-conflicts` (within the import) + `:external-conflicts` (vs existing); `import-plugin` shows a resolution modal (`events.cljs:3878`) | **CAUGHT** — rename / skip / replace, not a silent merge | + +## Why the override is "plugin wins" (the load-bearing semantics) — VERIFIED by test +`(into (sorted-set-by key-cmp) coll)` adds elements left-to-right; when a new element compares **equal** +(same key) to one already present, `conj` on a set is a **no-op** — the element already in the set +stays. The class/race/spell combines put `(reverse plugin-options)` **before** the built-ins in the +`concat`, so plugin entries are added first and **win** the collision. `key_collision_test.clj` pins +this: a `{::t/key :fighter}` plugin option placed before a built-in `:fighter` yields a 1-element set +containing the **plugin** one; distinct keys both survive; a plain `concat` (the pool/list shape) keeps +**both** same-key entries. + +## Notes / boundaries +- **The import conflict-handling is recent, and its EDGE CASES are explicitly OUT OF SCOPE for this + branch.** Significant time has already been spent circling them (partial-conflict resolution, how + rename/skip/replace interact, etc.) without a clean answer. This doc records the *behavior* so it + stops being re-discovered — it is **not** an invitation to re-chase the edge cases here. Leave them. +- **Within a single content-type map, keys are already unique** — the `.orcbrew` is EDN, so two entries + with the same key in one map collapse at parse time (last-wins). `find-duplicate-keys-in-content` + notes this (`import_validation.cljs:1056`: "Since items is a map, keys are inherently unique within + it"). So intra-map duplicates can't survive to runtime; collisions are **cross-source** (plugin vs + built-in, or plugin vs plugin). +- **The override is order-dependent and predictable**, but it is the *runtime combine* order + (plugin-first), not "last loaded." Two homebrew plugins both overriding the same built-in key would + collide with each other — that is exactly what the import conflict gate is for. +- **NOT-TRACED here:** how a *saved character* that chose an overridden key resolves after the override + changes (it should resolve to whatever now holds that key — flagged, not yet tested). +- This is the tooling that was missing for "where do duplicate-key problems come from": the answer is + layer-specific, and now it's one table + a test instead of guesswork. diff --git a/docs/kb/registry-before-after.md b/docs/kb/registry-before-after.md new file mode 100644 index 000000000..08defa682 --- /dev/null +++ b/docs/kb/registry-before-after.md @@ -0,0 +1,129 @@ +# Adding a homebrew content type — BEFORE vs AFTER + +Representative comparison using the **Pact Boon** type. "Before" = the original fully-scattered +pattern; "After" = the registry-driven state (events `d2e002b4`, db `af68061d`, forms `109b5dd0`). +This shows the *events*, *db*, and *builder-form* layers — the ones collapsed so far. (Routes and +spec are still per-type; see `content-extensibility-direction.md` §"Foundation".) + +--- + +## 1. Event wiring + +### BEFORE — ~10 registrations, scattered across ~4,000 lines of `events.cljs` +```clojure +;; ~line 177 +(def boon->local-store-interceptor (after boon->local-store)) +(def boon-interceptors [(path ::class5e/boon-builder-item) boon->local-store-interceptor]) + +;; ~line 621 +(reg-save-homebrew "Boon" ::class5e/save-boon ::class5e/boon-builder-item + ::class5e/homebrew-boon ::e5/boons + "You must specify 'Name', 'Option Source Name'") +;; ~line 756 +(reg-delete-homebrew ::class5e/delete-boon ::e5/boons) +;; ~line 2142 +(reg-edit-homebrew ::class5e/edit-boon ::class5e/set-boon + routes/dnd-e5-boon-builder-page-route) +;; ~line 3033 +(reg-event-db ::class5e/set-boon-prop boon-interceptors + (fn [boon [_ k v]] (assoc boon k v))) +;; ~line 4136 +(reg-event-db ::class5e/set-boon boon-interceptors (fn [_ [_ boon]] boon)) +;; ~line 4227 +(reg-event-fx ::class5e/reset-boon (fn [_ _] {:dispatch [::class5e/set-boon default-boon]})) +;; ~line 4491 +(reg-new-homebrew ::class5e/new-boon ::class5e/set-boon default-boon + routes/dnd-e5-boon-builder-page-route) +``` + +### AFTER — nothing per type. One loop (written ONCE) wires every type: +```clojure +;; events.cljs — this is the WHOLE per-type events story now, for ALL homebrew types: +(doseq [{:keys [type-name builder-item spec plugin-key route-kw default save-error] :as ct-entry} + (filter :homebrew-builder? ct/content-types)] + (register-homebrew-content! + (merge (homebrew-event-keys builder-item) ; save-/delete-/edit-/new-/set-/reset-/set-prop + {:type-name type-name :save-error (or save-error "...") + :builder-item builder-item :spec spec :plugin-key plugin-key + :default (or default {}) :route route-kw + :interceptors (homebrew-local-store-interceptor ct-entry)}))) +``` +Adding a boon-like type adds **0 lines** here. + +--- + +## 2. DB draft state + +### BEFORE — a def, a key, a fn, and a slot, in `db.cljs` +```clojure +(def default-boon {}) +(def local-storage-boon-key "boon") +(defn boon->local-store [boon] + (when js/window.localStorage (set-item local-storage-boon-key (str boon)))) +;; ...and inside the default-value map: +::class5e/boon-builder-item default-boon +``` + +### AFTER — nothing per type. The slots generate from the registry: +```clojure +;; db.cljs — inside default-value: +(into {} (map (juxt :builder-item :default)) + (filter :homebrew-builder? ct/content-types)) +``` +Adding a boon-like type adds **0 lines** here. + +--- + +## 3. The builder form + +### BEFORE — a bespoke input-field wrapper + a hand-built form, in `views.cljs` +```clojure +(defn boon-input-field [title prop boon & [class-names]] + (builder-input-field title prop boon ::classes/set-boon-prop class-names)) + +(defn boon-builder [] + (let [boon @(subscribe [::classes/boon-builder-item])] + [:div.p-20.main-text-color + [:div.flex.w-100-p.flex-wrap + [boon-input-field "Name" :name boon "m-b-20"] + [plugin-datalist option-source-name-label boon ::classes/set-boon-prop]] + [:div.w-100-p + [:div.f-s-24.f-w-b "Description"] + [textarea-field {:value (get boon :description) + :on-change #(dispatch [::classes/set-boon-prop :description %])}]]])) +``` + +### AFTER — one line (the generic form is data): +```clojure +(defn boon-builder [] + (simple-content-builder ::classes/boon-builder-item ::classes/set-boon-prop)) +``` +A *richer* type passes one extra field, e.g. the draconic-ancestry damage-type dropdown: +```clojure +(defn draconic-ancestry-builder [] + (simple-content-builder ::races/draconic-ancestry-builder-item + ::races/set-draconic-ancestry-prop + [[labeled-dropdown "Breath Weapon Damage Type" {...}]])) +``` + +--- + +## 4. So what do you actually WRITE to add a type now? + +```clojure +;; content_types.cljc — ONE registry entry. Events + db wiring follow automatically. +{:id :boon :type-name "Pact Boon" + :builder-item :orcpub.dnd.e5.classes/boon-builder-item + :spec :orcpub.dnd.e5.classes/homebrew-boon + :plugin-key :orcpub.dnd.e5/boons + :route-kw route-map/dnd-e5-boon-builder-page-route + :route-seg "boon-builder" :local-storage-key "boon" + :homebrew-builder? true :default {}} +``` +Plus the genuinely per-type bits: the **form** (a one-liner, or + an extra field), the **spec** +(one line), and — until the routes pass — the route plumbing. Everything that used to be +copy-pasted boilerplate is now generated from that one entry. + +**Behavior-preserving:** the boon and draconic builders' tests (handlers registered, builder +produces spec-valid content, pool, character round-trip) pass *unchanged* through all of this — +the loops register identically to the old hand-written code. diff --git a/docs/kb/roadmap.md b/docs/kb/roadmap.md new file mode 100644 index 000000000..911d69ac2 --- /dev/null +++ b/docs/kb/roadmap.md @@ -0,0 +1,175 @@ +# Branch plan — the single source (START HERE) + +One reconciled plan for branch `claude/zen-wright-04xhdz`. It replaces the previous split between +this file and `content-extensibility-direction.md` as competing "start here" docs. The branch ran in +loops because decisions lived in two parallel trackers; this is the one that supersedes both for +*navigation and status*. The detail/decision docs below remain authoritative for their own track. + +## The arc (two phases — both real, one branch) +- **Phase 1 — Content extensibility (pool + grant).** The founding purpose: replace bespoke positional + cross-type wiring with one open **pool + grant** authoring layer (stability and flexibility are the + same abstraction). **Substantially built.** Canonical detail: `content-extensibility-direction.md` + (v2, "read this for the content track"); decisions: `content-extensibility-decisions.md` (the + `D`-numbers); how-to: `content-extensibility-framework.md`; branch history/handoff: `BRANCH.md`. +- **Phase 2 — Mechanization, class features, spell slots, AC.** The later expansion (this session): + make content cross-silo *and* turn "just text" effects into real sheet/roll mechanics, including the + class-feature registry and the spellcasting-progression rework. Mostly design + a foundation net. + +## Status ledger (anchored to commits; detail in the linked docs) + +### BUILT — Phase 1 (verified by git + BRANCH.md) +- `content_types` registry + passthrough-subs loop; `register-homebrew-content!` wiring layer (`3980ea1b`). +- **First pool+grant slice on real mechanics**: open draconic-ancestry pool, dragonborn grants from it, + homebrew ancestry inherits full mechanics (`acaa131d`); richer ancestries via `:props` (`026f8707`). +- `simple-content-builder` — builder forms are data (`109b5dd0`); draconic-ancestry builder end-to-end, + author→pool→export→import→round-trip (`0aca6113`). +- Registry **drives** events (`d2e002b4`), db (`af68061d`), routes (`506c32b3`/`c5e9aea6`/`58c4de47`). +- Declarative builder **field-schema** → save-spec + `:required?` form + import/export sync + strict mode + (`f32790b1`…`e4614519`). + +### BUILT — Phase 2 (this session) +- Cross-bucket grant **bridge prototype** `grant-selection` (`c1f54967`, `options.cljc:3447`) + feat→ + fighting-style e2e (`8c8c0b10`/`a4c34f3c`). **⚠️ see Flagged conflict #1 — this may diverge from D17.** +- **Grant-matrix proof (in progress) — the 4 modes built + the feat silo proven end-to-end.** + `grant-selection` evolved to 4 modes: ALL (`{:from p}`), FILTERED (`:filter #{…}`), SPECIFIC (`:key :k`), + CUSTOM (homebrew entry in the pool). `fighting-style-grant-matrix-test` proves all 4 at compile level + AND end-to-end on the feat silo (a built character gets the right style's mechanic). **Verified finding:** + a NESTED grant must carry NO top-level `:ref` (adding one zeroed a feat-granted style's mechanic — the + test caught it); top-level grants (a class's own fighting style) carry a `:ref` via their own constructor. + **Remaining for the 6×4 matrix (next increment):** the other 5 silos (background, race, subrace, built-in + class, custom class, subclass) — each is the same one-line `:grant` hook + a build test; the compiler is + silo-agnostic (already proven bucket-agnostic). +- AC characterization test + model doc (`7b7f3b3a`/`68f42e77`, `armor-class-computation.md`). +- **Class-feature regression net** — all **12** classes baselined + fighter/rogue detail + (`cc7cd171`…`b258639e`, `class_feature_snapshot_test.clj`). +- **`compile-feature` proof** — data spec reproduces real fighter/rogue output; `:die`/`:uses` overrides + (`0618ab8c`/`8c65f420`). +- **Per-class catalogue** (C1, all 12) + **spell-slot-progression** analysis (`b8920f4f`/`355f8cee`, + `class-feature-catalogue.md`, `spell-slot-progression.md`). +- **Spell-slot computation** characterized (`spell_slot_characterization_test`): full/half/pact singles, + two-caster pooling, warlock-not-pooling — confirmed the analysis against the real build. +- **Grant vocabularies** characterized across both layers: A (cljc) `grant_vocabulary_characterization_test`, + B (cljs harness) `grant_vocabulary_cljs_test` — shared `mod5e/*` primitive + B is level-gated (D31). +- **AC stack** deepened (`ac_characterization_test`): armored dex-cap-by-type + shield, two-unarmored-defense + tie-break, tortle/lizardfolk natural-AC duplication; surfaced the `:max-dex-mod`-ignored + cljs-nil-add findings. +- **.orcbrew round-trip** (`orcbrew_round_trip_test`): EDN export→import is lossless (real fixture + rich class). + +> **Round-trip coverage map** (so it isn't re-discovered): EDN serialization fidelity → +> `orcbrew_round_trip_test` (JVM, new); validate-import parse + clean-pack-unchanged → `import-validation-test` +> (`test-import-all-or-nothing-valid` asserts `= plugin (:data result)`); auto-clean transforms → +> same; key-conflict reconciliation → `content-reconciliation-test`; build-after-import (homebrew → character) +> → `draconic-ancestry-test`/`dragonborn-ancestry-e2e-test`; character strict round-trip + key survival → +> `extensibility-golden-test`. **Foundation (F) is now essentially complete** — the load-bearing computations +> and the round-trip are test-backed across both layers. +> +> **Import internals (authoritative; live on `agents/develop`, not this fork line):** +> `docs/ORCBREW_IMPORT_DEEP_DIVE.md` (two-phase cleaning, nil semantics — semantic nils preserved, numeric +> removed) and `docs/kb/error-handling-import-validation.md` (progressive recovery: invalid items skip + +> log; reconciliation; export pre-validation). KEY REFINEMENT to the round-trip: it's lossless only for +> CLEAN EDN; the import *intentionally transforms* malformed/partially-invalid input (cleaning is a +> feature). Surface these when split-committing docs to `agents/develop`. + +### DECIDED (design settled; don't re-litigate) +> Canonical log: `content-extensibility-decisions.md` — D1–D22 (content track) and **D23–D29** +> (this expansion: prototype-then-converge governance, the class-feature registry, `compile-feature`, +> the catalogue, the spell-slot bucket, non-SRD/synthetic-validation, and the open grant question). +- **Pool + grant is the spine** (`direction.md`). Abstraction earns its keep only if thicker than what + it hides + intent-revealing (no cryptic DSL). Maintainability **gate**: register a pool once → grantable + in every builder (O(1) to expose; D21). +- **D17 — audit what a new piece REPLACES before building.** Specifically: **do not build a generic + `grant` wrapper**; point the existing per-feature `selection-cfg` constructors at **open pools**, + preserving their load-bearing `:ref`/`:tags`. `content_pools/pool` exists; **no grant compiler yet** — + the draconic grant is hand-wired and is the thing to generalize carefully. +- Stable keys, never display names (D10); pools are memoized derived subs, never recomputed (D11); + variants get one `resolved-content` seam now, built later (D-pins). +- **Class features**: one keyed/filterable registry, pools = filtered views; reference = key + `:overrides`; + features are macro-captured **code**, so a `compile-feature` step is required; summary = fields + a fill + template, not interpolation. (`class-features-and-mechanization.md`.) +- **Spell slots**: replace the overloaded `:level-factor` with a **bucket of named/explicit slot tables** + (authored as an absolute per-level grid, presets as seeds) + a **separately declared multiclass rule** + + its own prepared-count; `:separate` (pact) schedules own their pool + recharge. + (`spell-slot-progression.md`.) + +### OPEN — Phase 1 levers & pins (from `direction.md`) +- **Grant-authoring UI** — the biggest remaining lever (declare "grant a choice from pool X" in a builder). +- 🔴 **`:required-when` conditional field validation** (HIGH — flagged in `builder_fields.cljc`). +- Variants (`_copy`/`_mod`), class-feature pool (`[:class-feature :X]`), declarative cross-type prereq + vocabulary, mechanical-effects-for-text-only (boons/ki), level-gated grants in `:props`. + +## Tracks — Phase 2 (the expansion, layered on Phase 1) +- **A. Cross-silo grants** = the Phase-1 pool/grant track. **Not new work** — defer to `direction.md`/ + `decisions.md`. The live decision is D17 (open pools behind existing selections, no generic wrapper). + - **A2.** Uniform spell-granting (route `:spells`/`:spell-choice` to the primitives across silos) — FEASIBLE. + - **A3.** Spell-slot progression as data (bucket of tables + declared multiclass rule) — DESIGN; + unblocks Artificer + homebrew tables; pact → `:separate` + own pool (`spell-slot-progression.md`). + - **A4. Parametric `select`-grant: USER-CHOICE floating ASI (any silo)** — ✅ DONE (terse spread). + A race/subrace declares `:ability-increases` as a list of `[amount pool]` pairs; the whole list is + one spread (all increments land on different abilities). Pool = `:any`/`:martial`/`:mental` | a set + `#{:wis :con}` | a single stat `:con` (= fixed). Data shape: + ```clojure + :ability-increases [[2 :cha] [1 :martial]] ; +2 CHA (fixed), +1 to any martial stat (player choice) + ``` + Single-stat → fixed `race-ability` modifier; multi-stat → a player-chosen slot in one `:asi` + selection. Full spec: `docs/kb/ability-increase-spreads.md`. + Built bottom-up and test-backed at every layer: JVM (compile + apply), cljs harness (the + `::races5e/races` sub + orcbrew round-trip), and three rendered-UI E2Es (`exact-spread-asi`, + `race-builder-asi`, `export-import-use`) covering render + pool-restriction + distinctness, authoring, + and the full export→import→use round-trip. Lessons live in their own docs: the `'s value is always a string, so the default path's :on-change yields a string the + ;; caller must coerce back to the item's type (keyword/int/...). :typed? avoids that: option + ;; values are indices and :on-change gets the selected item's original :value, any type incl. + ;; nil and qualified keywords. See docs/kb/dropdown-value-coercion.md (D32). + (if typed? + (let [idx (first (keep-indexed (fn [i it] (when (= (:value it) value) i)) unique-items))] + [:select.builder-option.builder-option-dropdown.m-t-0 + {:value (if idx (str idx) "") + :on-change #(on-change (:value (nth unique-items (js/parseInt (event-value %)))))} + (doall + (map-indexed + (fn [i {:keys [title disabled?]}] + ^{:key i} + [:option.builder-dropdown-item + (cond-> {:value (str i)} + disabled? (assoc :disabled true)) + title]) + unique-items))]) + [:select.builder-option.builder-option-dropdown.m-t-0 + {:value (or value "") + :on-change #(on-change (event-value %))} + (doall + (map-indexed + (fn [i {:keys [value title disabled?]}] + ^{:key (str i "-" (or value title))} + [:option.builder-dropdown-item + (cond-> {:value value} + disabled? (assoc :disabled true)) + title]) + unique-items))]))) (defn labeled-dropdown [label cfg] [:div @@ -4355,12 +4376,6 @@ (defn language-input-field [title prop language & [class-names]] (builder-input-field title prop language ::langs/set-language-prop class-names)) -(defn invocation-input-field [title prop invocation & [class-names]] - (builder-input-field title prop invocation ::classes/set-invocation-prop class-names)) - -(defn boon-input-field [title prop boon & [class-names]] - (builder-input-field title prop boon ::classes/set-boon-prop class-names)) - (defn selection-input-field [title prop selection & [class-names]] (builder-input-field title prop selection ::selections/set-selection-prop class-names)) @@ -5946,6 +5961,8 @@ (range 2)))]])]) (range 1 6)))]]]) +(declare ability-increase-choices save-proficiency-choices ability-save-notes optional-builder-section) + (defn subclass-builder [] (let [subclass @(subscribe [::classes/subclass-builder-item]) spell-lists @(subscribe [::spells/spell-lists]) @@ -5975,6 +5992,12 @@ ::classes/set-subclass-prop ] ] + [optional-builder-section "Ability Score Increases (non-standard)" + (or (seq (:ability-increases subclass)) (seq (:save-proficiencies subclass))) + [:div + [ability-increase-choices subclass #(dispatch [::classes/set-subclass-prop :ability-increases %])] + [save-proficiency-choices subclass #(dispatch [::classes/set-subclass-prop :save-proficiencies %])] + [ability-save-notes subclass]]] (when (#{:fighter :rogue :warlock :cleric :paladin} class-key) (let [spellcasting (get subclass :spellcasting) spellcasting? (some? spellcasting)] @@ -6219,6 +6242,121 @@ {:title "Reaction" :value :reaction}]]])) +(defn optional-builder-section + "Opt-in builder section: a toggle that reveals `body`. Keeps non-standard fields out of the default + form (less clutter); starts OPEN when `has-content?` so editing existing data isn't hidden. The data + only persists if `body`'s controls are used, so an unopened/empty section adds nothing to the export." + [_label has-content? _body] + (let [open? (r/atom (boolean has-content?))] + (fn [label _has-content? body] + [:div.m-b-20 + [:div.flex.align-items-c.pointer.m-b-5 {:on-click #(swap! open? not)} + [:i.fa.m-r-5 {:class (if @open? "fa-caret-down" "fa-caret-up")}] + [:span.f-s-18.f-w-b label] + (when-not @open? [:span.m-l-10.f-s-12.i.orange "click to add"])] + (when @open? body)]))) + +(defn ability-increase-choices + "Authoring UI for an :ability-increases spread — a list of [amount pool] increments. Each row is an + Amount + a To target: a single stat is FIXED, a group (Any/Martial/Mental) is the player's choice. + All increases land on different abilities. Dropdowns use :typed? so on-change gets the real keyword. + Stores short keywords ([2 :cha] [1 :martial]); compile namespaces them. (Explicit sets like + #{:wis :con} are valid in the data but authored via EDN, not this form.) Silo-generic: `item` is the + content map; `set-ai!` dispatches the new spread back (race/background/… each pass their own setter)." + [item set-ai!] + (let [ais (vec (:ability-increases item)) + amount-items (map (fn [n] {:title (common/bonus-str n) :value n}) (range 1 4)) + target-items (concat + [{:title "Any (all six)" :value :any} + {:title "Martial (Str/Dex/Con)" :value :martial} + {:title "Mental (Int/Wis/Cha)" :value :mental}] + (map (fn [{:keys [name key]}] + {:title name :value (keyword (clojure.core/name key))}) + opt/abilities)) + ;; rebuild an increment preserving the optional :save rider (3rd element) + mk (fn [amount pool save?] (if save? [amount pool :save] [amount pool]))] + [:div.m-b-20 + [:div.f-s-18.f-w-b.m-b-10 "Ability Score Increases"] + [:div.f-s-12.i.m-b-10 "A single stat is fixed; a group is the player's choice. All increases go to different abilities."] + (doall + (map-indexed + (fn [i [amount pool save-flag]] + (let [save? (= :save save-flag)] + ^{:key i} + [:div.flex.flex-wrap.align-items-c.m-b-5 + [labeled-dropdown "Amount" {:items amount-items :value (or amount 1) :typed? true + :on-change #(set-ai! (assoc ais i (mk % pool save?)))}] + [labeled-dropdown "To" {:items target-items :value (or pool :any) :typed? true + :on-change #(set-ai! (assoc ais i (mk amount % save?)))}] + ;; opt-in rider: also grant proficiency in the save for this increment's (fixed/chosen) + ;; ability. Toggle via the tested, self-healing opt/toggle-increment-save (never nil/garbage). + [:span.m-l-5 [comps/labeled-checkbox "+ save prof" + save? false + #(set-ai! (assoc ais i (opt/toggle-increment-save (nth ais i))))]] + [:button.m-l-5 {:on-click #(set-ai! (common/remove-at-index ais i))} "Remove"]])) + ais)) + [:button {:on-click #(set-ai! (conj ais [1 :any]))} "Add increase"]])) + +(defn save-proficiency-choices + "Authoring UI for :save-proficiencies — a list of [count pool] entries, saving-throw proficiencies + INDEPENDENT of any ability bump. A single stat grants that fixed save; a group lets the player + choose `count` saves from it. Silo-generic like ability-increase-choices (`item` + a setter)." + [item set-sp!] + (let [sps (vec (:save-proficiencies item)) + count-items (map (fn [n] {:title (str n) :value n}) (range 1 4)) + target-items (concat + [{:title "Any (all six)" :value :any} + {:title "Martial (Str/Dex/Con)" :value :martial} + {:title "Mental (Int/Wis/Cha)" :value :mental}] + (map (fn [{:keys [name key]}] + {:title name :value (keyword (clojure.core/name key))}) + opt/abilities))] + [:div.m-b-20 + [:div.f-s-18.f-w-b.m-b-10 "Saving Throw Proficiencies"] + [:div.f-s-12.i.m-b-10 "A single stat grants that save; a group lets the player choose that many saves from it."] + (doall + (map-indexed + (fn [i [cnt pool]] + ^{:key i} + [:div.flex.flex-wrap.align-items-c.m-b-5 + [labeled-dropdown "How many" {:items count-items :value (or cnt 1) :typed? true + :on-change #(set-sp! (assoc sps i [% pool]))}] + [labeled-dropdown "From" {:items target-items :value (or pool :any) :typed? true + :on-change #(set-sp! (assoc sps i [cnt %]))}] + [:button.m-l-5 {:on-click #(set-sp! (common/remove-at-index sps i))} "Remove"]]) + sps)) + [:button {:on-click #(set-sp! (conj sps [1 :any]))} "Add save"]])) + +(defn builder-notes + "ONE render for a list of authoring problems in a builder form, so every surface shows item-level + feedback the same way. Producers stay separate and just hand it a seq of human-readable strings: + - simple-content-builder ← bf/validate-fields (:error) + - selection-builder summary ← duplicate/empty-name checks (:error) + - ability-save-notes ← opt/ignored-entry-warnings (:error) + opt/save-coverage-warnings (:advisory) + :severity :error = blocking, 'Fix before saving:' + red list; :advisory = non-blocking, italic ⚠ + lines. Empty seq → nothing. (Per-row highlighting stays bespoke to the producer — this is summary + only.)" + [problems & [{:keys [severity] :or {severity :advisory}}]] + (when (seq problems) + (if (= severity :error) + [:div.w-100-p.m-t-10 + [:div.f-w-b.red.m-b-5 "Fix before saving:"] + (into [:ul] (map (fn [p] ^{:key p} [:li.red p]) problems))] + [:div.m-b-20 + (doall (map-indexed (fn [i p] ^{:key i} [:div.f-s-12.i.m-b-5.red "⚠ " p]) problems))]))) + +(defn ability-save-notes + "Authoring-time notes under the ASI/save widgets for THIS content entry: + - :error — malformed :ability-increases/:save-proficiencies entries that the compilers will + IGNORE (opt/ignored-entry-warnings); surfaces the otherwise-silent drop. + - :advisory — redundant/overlapping save coverage (opt/save-coverage-warnings); harmless at + runtime (saves are a set), just flags wasted authoring/picks. + Renders nothing when clean." + [item] + [:div + [builder-notes (opt/ignored-entry-warnings item) {:severity :error}] + [builder-notes (opt/save-coverage-warnings item) {:severity :advisory}]]) + (defn race-builder [] (let [race @(subscribe [::races/builder-item])] [:div.p-20.main-text-color @@ -6312,7 +6450,10 @@ (range -2 3 1)) :value (get-in race [:abilities key] 0) :on-change #(dispatch [::races/set-race-ability-increase key %])}]]) - opt/abilities))]] + opt/abilities))] + [ability-increase-choices race #(dispatch [::races/set-race-path-prop [:ability-increases] %])] + [save-proficiency-choices race #(dispatch [::races/set-race-path-prop [:save-proficiencies] %])] + [ability-save-notes race]] [:div.m-b-20 [:div.f-s-24.f-w-b.m-b-10 "Modifiers"] [:div.m-b-20 @@ -6387,6 +6528,9 @@ [textarea-field {:value (get background :help) :on-change #(dispatch [::bg/set-background-prop :help %])}]] + [:div [ability-increase-choices background #(dispatch [::bg/set-background-prop :ability-increases %])]] + [:div [save-proficiency-choices background #(dispatch [::bg/set-background-prop :save-proficiencies %])]] + [:div [ability-save-notes background]] [:div [background-skill-proficiencies background]] [:div [background-languages background]] [:div [background-tool-proficiencies background]] @@ -6444,22 +6588,13 @@ [:button.form-button {:on-click #(dispatch [::selections/add-option])} "Add Option"]] - ;; Summary warning for duplicate names - (when has-dupes? - [:div.p-10.m-b-10.red - {:style {:background-color "rgba(255,0,0,0.1)" - :border "1px solid red" - :border-radius "4px"}} - [:span.f-w-b "Duplicate names found: "] - [:span (s/join ", " dupe-names)] - [:div.f-s-12 "Each option must have a unique name. Rename duplicates before saving."]]) - ;; Warning for empty option names - (when has-empty? - [:div.p-10.m-b-10.red - {:style {:background-color "rgba(255,0,0,0.1)" - :border "1px solid red" - :border-radius "4px"}} - "One or more options have no name. All options must be named."]) + ;; Summary feedback via the shared builder-notes; per-row highlighting stays bespoke below. + [builder-notes + (cond-> [] + has-dupes? (conj (str "Duplicate names: " (s/join ", " dupe-names) + ". Each option must have a unique name.")) + has-empty? (conj "One or more options have no name. All options must be named.")) + {:severity :error}] [:div (doall (map-indexed @@ -6518,47 +6653,89 @@ {:value (get language :description) :on-change #(dispatch [::langs/set-language-prop :description %])}]]])) -(defn boon-builder [] - (let [boon @(subscribe [::classes/boon-builder-item])] +(defn render-builder-field + "Render one DECLARATIVE builder field from a spec, dispatching set-prop on change. This is + what makes a richer builder's form data instead of hand-written hiccup, and it coerces values + to the right type (so an :enum stores its keyword, not the dropdown's raw string — the bug + that shipped a broken breath weapon). Field spec: + :key a single key or a PATH vector into the item (e.g. [:breath-weapon :damage-type]) + :type :enum | :number | :text (a :boolean type is pending — see builder_fields.cljc note; + it must route through common/toggle-in, not a parallel fn) + :label field label + :options (:enum) [{:value :title