Fix cantrip/spell selection regression + add source-suffix toggle

docs/kb/README.md

@@ -9,6 +9,7 @@ direct inspection of code, logs, or authoritative references. Speculation is mar
 | Document | Topic | Source quality |
 |----------|-------|---------------|
 | [datomic-crash-analysis.md](datomic-crash-analysis.md) | Datomic transactor crashes — root cause, frequency, fix options | High — direct log analysis from `logs/datomic.{1,2,3}.log` |
+| [key-vs-name-separation.md](key-vs-name-separation.md) | Class config `:key`/`:name` separation, cantrip regression case study, `name-to-kw(:name)` audit pattern, plugin-load race verification | High — direct code trace |
 
 ## Contribution rules
 

docs/kb/key-vs-name-separation.md

@@ -0,0 +1,87 @@
+# Class config: `:key` is identity, `:name` is display
+
+**Date:** 2026-05-04
+**Source:** Direct code analysis of `spell_subs.cljs:467-485`, `options.cljc:469`, `template_base.cljc:275`, `options.cljc:635, 1060, 2904`, `template.cljc:74-88`, `db.cljs:267-305`, `events.cljs:208-228`, `content_reconciliation.cljs`, plus session reasoning trail dated 2026-05-04.
+
+## TL;DR design rule
+
+`:key` is identity. `:name` is display. Display strings never bleed into keys.
+
+- *Inside the editor:* renaming a class moves the key with the name (e.g. `gambit` → `charlatan`). Editor flow owns key changes and the rebinding of dependent selections.
+- *Outside the editor:* `:key` is read, never re-derived. `name-to-kw(:name)` outside the editor/import path is the bug pattern — apparent in the cantrip regression case study below.
+- Built-in classes (`:cleric`, `:wizard`, ...) are source-code constants — never rename, never collide.
+
+## Case study: cantrip/spell selection regression (2026)
+
+### Symptom
+Homebrew Cleric/Druid replacements lose cantrip/spell selections after a UX change to suffix the source name onto class display labels. Saved entity still has `:cleric-cantrips-known`; UI shows nothing selected. Setting the orcbrew source name to blank temporarily "fixes" it.
+
+### Root cause
+`spell_subs.cljs:475-478` (`::classes5e/plugin-classes` sub) mutated class `:name` to `"Cleric (Source)"` for display. Class `:key` was untouched. The mutation contract on `:479-480` claimed *"`:name` is display-only"* — but downstream code violated that contract by re-deriving identity via `common/name-to-kw` from the mutated `:name`.
+
+### The four downstream leak sites
+
+| # | Site | What leaked |
+|---|------|-------------|
+| 1 | `options.cljc:469` (`spell-selection`) | mutated `name` → `name-to-kw` → `::t/key` (the cantrip bug) |
+| 2 | `template_base.cljc:275` (`?prepare-spell-count`) | mutated `class-name` → `name-to-kw` → lookup miss against canonical `?spell-slot-factors` |
+| 3 | `options.cljc:2951` (`mods/map-mod ?prepares-spells`) | feeds #2; same root |
+| 4 | `options.cljc:635` (`class-key-name` fallback) | latent; current callers pass `class-key`, but the fallback is wrong |
+
+### Fix
+Reverted the `:name` mutation. Plumbed `:plugin-source` through `option-cfg` as a distinct `::plugin-source` slot. Display suffix computed at the dropdown render site, gated by a new `::show-class-source-suffix` user pref. All four leaks heal from the single revert.
+
+A load-time reconciler (`reconcile-spell-selection-keys` in `content_reconciliation.cljs`) heals regression-window orphans on character load when a single suffix-match candidate exists.
+
+## When `name-to-kw(:name)` is benign vs. a leak
+
+Common false-alarm pattern during reviews — `name-to-kw(:name)` shapes can look identical but differ in safety.
+
+**Benign (constructor / minting time):**
+- `template.cljc:option-cfg` / `:selection-cfg` — destructured `name` kwarg, derived once at template-build time
+- `options.cljc:feat-option` (`:1060`) — operates on feat configs whose `:name` is not mutated upstream
+- `common.cljc:add-keys` — applied to static seed data (equipment, weapons, conditions, monsters, spells)
+- Import path slugification (`import_validation.cljs`)
+- Homebrew duplicate-detection (`views.cljs:6404+`) — operates on user-typed names in the editor
+
+The shared property: `:name` is stable at the time of derivation. The key gets minted once from a canonical name.
+
+**Leak shape:**
+- `:name` is *mutated upstream* of the identity derivation (e.g., a sub appends a source label to `:name` for display, then a downstream consumer derives a key from it)
+- The derived key drifts away from the canonical key the rest of the pipeline expects
+- Saved data referencing the canonical key is orphaned
+
+**Audit heuristic:** for any `name-to-kw(:name)` site, trace `:name`'s upstream. If any code rewrites `:name` for display purposes, you have a leak. If `:name` is the canonical name from source code or stable config, you don't.
+
+## Plugin-load race: verified non-issue
+
+**✅ VERIFIED 2026-05-04 — direct trace of `db.cljs` and `events.cljs`.**
+
+A reviewer worried that `:set-character` could fire before plugins hydrate, leaving any reconciliation logic with empty `:plugins` in `db`. Confirmed not the case:
+
+- `::e5/plugins` is registered via `reg-local-store-cofx` at `db.cljs:302` — synchronous localStorage read.
+- `:initialize-db` at `events.cljs:208` injects `:local-store-character` and `::e5/plugins` as cofx in the same handler.
+- The handler writes both atomically into `db` via a single `cond->` (`events.cljs:222-228`).
+- Subsequent `:set-character` dispatches see `db` with plugins already populated.
+
+The "user opened a character on a machine without the originating plugin imported" case is real but distinct — that's correctly the missing-content / parked-orphan case, not a race. Reconciler treats it conservatively (no silent rewrite).
+
+## Reconciler approach: transient parked state
+
+The reconciler returns `{:rewrote [...] :parked [...]}`:
+- `:rewrote` — orphan keys with a single unambiguous canonical candidate; rewritten in place at character load.
+- `:parked` — orphans with multiple candidates (multiclass overlap) or zero candidates (originating class not loaded). Data preserved on the entity; held in transient db state for UI banner.
+
+**Design call:** parked entries are NOT persisted to the entity as a separate field. They're re-derived every character load from the entity's orphan keys × currently-loaded plugins. This avoids a schema slot that could drift from the underlying data.
+
+Resolution UI lives in `views.cljs` (banner + resolution view); resolution event mutates the entity and re-runs the reconciler.
+
+## Source comments worth leaving
+
+For sites where future reviewers are most likely to speculate about behavior:
+
+1. **`spell_subs.cljs` `::classes5e/plugin-classes`** — note that `:name` was historically mutated for display and the mutation was reverted; refer to this kb doc.
+2. **`content_reconciliation.cljs:reconcile-class-entry-options`** — note that the "multiple candidates within one class entry" branch is unreachable today (single suffix per class) but the cross-class accumulator in `reconcile-spell-selection-keys` populates `:parked` for multiclass overlap.
+3. **`events.cljs` reconciler invocation site** — one-liner that the reconciler trusts `db` plugins to be hydrated because `::e5/plugins` is a sync cofx at `:initialize-db`.
+
+Comments should be one-liners pointing here, not duplications of this doc.

src/cljc/orcpub/dnd/e5/classes.cljc

@@ -27,6 +27,13 @@
 
 (spec/def ::homebrew-boon (spec/keys :req-un [::name ::key ::option-pack]))
 
+(def base-class-keys
+  "SRD built-in class keys. Source-code constants; never rename, never collide.
+   Canonical location for this set; other namespaces should reference it from
+   here rather than redefining."
+  #{:barbarian :bard :cleric :druid :fighter :monk
+    :paladin :ranger :rogue :sorcerer :warlock :wizard})
+
 (defn class-level [levels class-kw]
   (get-in levels [class-kw :class-level]))
 

src/cljc/orcpub/dnd/e5/options.cljc

@@ -461,12 +461,26 @@
    #(memoized-spell-option spells-map spellcasting-ability class-name % prepend-level? qualifier)
    (sort spells)))
 
-(defn spell-level-title [class-name level]
+(defn spell-level-title
+  "Display title for a class's spell selection at a given level."
+  [class-name level]
   (str class-name (if (and level (zero? level)) " Cantrips Known" (str " Spells Known" (when level (str " " level))))))
 
+(defn spell-selection-key
+  "Identity-derived selection key for a class's spell selection at a given
+   level. Mirrors the shape spell-level-title produces but rooted in
+   :class-key, not :name. See docs/kb/key-vs-name-separation.md."
+  [class-key level]
+  (keyword (str (name class-key)
+                (if (and level (zero? level))
+                  "-cantrips-known"
+                  (str "-spells-known" (when level (str "-" level)))))))
+
 (defn spell-selection [spell-lists spells-map {:keys [title class-key level spellcasting-ability class-name num prepend-level? spell-keys options min max exclude-ref? ref]}]
+  ;; Identity (kw) derives from :class-key. :class-name still feeds title
+  ;; for display. See docs/kb/key-vs-name-separation.md.
   (let [title (or title (spell-level-title class-name level))
-        kw (common/name-to-kw title)
+        kw (spell-selection-key class-key level)
         ref (or ref (when (not exclude-ref?) [:class class-key kw]))]
      (t/selection-cfg
       {:name title
@@ -629,14 +643,6 @@
      spell-keys)
     spell-keys))
 
-(defn class-key-name [cls-key cls-nm]
-  (if cls-key
-    (name cls-key)
-    (common/name-to-kw cls-nm)))
-
-(defn spell-selection-key [cls-key-nm]
-  (keyword (str cls-key-nm "-spells-known")))
-
 
 (defn spells-known-selections [spell-lists
                                spells-map
@@ -677,17 +683,14 @@
                             filtered-keys)))
                        all-spells))]
          (assoc m cls-lvl
-                [(let [cls-key-nm (class-key-name (:key cls-cfg) (:name cls-cfg))
-                       kw (spell-selection-key cls-key-nm)
-                       cls-nm (:name cls-cfg)]
-                   (spell-selection
-                    spell-lists
-                    spells-map
-                    {:class-key class-key
-                     :class-name cls-nm
-                     :min num
-                     :max (when (not acquire?) num)
-                     :options options}))])))
+                [(spell-selection
+                  spell-lists
+                  spells-map
+                  {:class-key class-key
+                   :class-name (:name cls-cfg)
+                   :min num
+                   :max (when (not acquire?) num)
+                   :options options})])))
    {}
    spells-known))
 
@@ -2867,6 +2870,7 @@
                             help
                             hit-die
                             plugin?
+                            plugin-source
                             profs
                             levels
                             ability-increase-levels
@@ -2904,6 +2908,7 @@
     (t/option-cfg
      {:name name
       :key kw
+      :plugin-source plugin-source
       :help [:div.p-t-5.p-l-10.p-r-10
              (class-help hit-die save-profs weapon-profs armor-profs)
              [:div.m-t-10 help]]

src/cljc/orcpub/dnd/e5/template_base.cljc

@@ -272,6 +272,14 @@
                           (?class-level class-kw)
                           (get ?spell-slot-factors class-kw)))
     ?prepare-spell-count (fn [class-name]
+                           ;; Latent name-to-kw on class-name. Active risk is
+                           ;; bounded — callers pass class-name from spell entries'
+                           ;; :class field (sourced from canonical hardcoded source
+                           ;; strings or class config :name, which phase 1 keeps
+                           ;; canonical). Propagating class-key through spell-data
+                           ;; and the ~100 mod5e/spells-known call sites is its own
+                           ;; PR; tracked in docs/TODO.md. See
+                           ;; docs/kb/key-vs-name-separation.md.
                            (let [class-kw (common/name-to-kw class-name)
                                  slot-factor (get ?spell-slot-factors class-kw)
                                  spell-mods ?spell-modifiers

src/cljc/orcpub/template.cljc

@@ -71,7 +71,7 @@
    ::prereq-fn func
    ::hide-if-fail? hide-if-fail?})
 
-(defn option-cfg [{:keys [:db/id name key help selections modifiers associated-options prereqs order ui-fn icon select-fn edit-event] :as cfg}]
+(defn option-cfg [{:keys [:db/id name key help selections modifiers associated-options prereqs order ui-fn icon select-fn edit-event plugin-source] :as cfg}]
   {::id id
    ::name name
    ::key (or key (common/name-to-kw name))
@@ -84,7 +84,8 @@
    ::ui-fn ui-fn
    ::select-fn select-fn
    ::icon icon
-   ::edit-event edit-event})
+   ::edit-event edit-event
+   ::plugin-source plugin-source})
 
 (declare make-modifier-map-from-selections)
 (declare make-plugin-map-from-selections)

src/cljs/orcpub/character_builder.cljs

@@ -194,12 +194,18 @@
 
 (def levels-selection #(when (= :levels (::t/key %)) %))
 
+(defn class-option-display-name [name plugin-source show-suffix?]
+  (if (and show-suffix? plugin-source)
+    (str name " (" plugin-source ")")
+    name))
+
 (defn class-level-selector []
   (let [expanded? (r/atom false)]
     (fn [i key selected-class options unselected-classes-set built-char]
       (let [options-map (make-options-map options)
             class-template-option (options-map key)
-            path [:class-levels key]]
+            path [:class-levels key]
+            show-suffix? @(subscribe [::subs5e/show-class-source-suffix])]
         [:div.m-b-5
          {:class (when @expanded? "b-1 b-rad-5 p-5")}
          [:div.flex.align-items-c
@@ -208,13 +214,14 @@
             :on-change (set-class i options-map)}
            (doall
             (map
-             (fn [{:keys [::t/key ::t/name] :as option}]
+             (fn [{:keys [::t/key ::t/name ::t/plugin-source] :as option}]
                (let [failed-prereqs (when (pos? i) (prereq-failures option built-char))]
                  ^{:key key}
                  [:option.builder-dropdown-item
                   {:value key
                    :disabled (seq failed-prereqs)}
-                  (str name (when (seq failed-prereqs) (str " (" (s/join ", " failed-prereqs) ")")))]))
+                  (str (class-option-display-name name plugin-source show-suffix?)
+                       (when (seq failed-prereqs) (str " (" (s/join ", " failed-prereqs) ")")))]))
              (sort-by
               ::t/name
               (filter
@@ -270,6 +277,7 @@
   (let [options (::t/options selection)
         built-char @(subscribe [:built-character])
         selected-classes @(subscribe [::char5e/levels])
+        show-suffix? @(subscribe [::subs5e/show-class-source-suffix])
         unselected-classes (remove
                             (set (keys selected-classes))
                             (map ::t/key options))
@@ -281,6 +289,9 @@
                               (entity/meets-prereqs? option built-char)))
                            options)]
     [:div
+     [:div.m-b-5
+      {:on-click #(dispatch [::events5e/toggle-class-source-suffix])}
+      [views5e/labeled-checkbox "Show homebrew source on class names" show-suffix?]]
      [:div
       (doall
        (map-indexed