Fix cantrip/spell selection regression + add source-suffix toggle#27
Open
codeGlaze wants to merge 8 commits into
Open
Fix cantrip/spell selection regression + add source-suffix toggle#27codeGlaze wants to merge 8 commits into
codeGlaze wants to merge 8 commits into
Conversation
The plugin-classes sub mutated class :name to "Cleric (Source)" for display, but downstream spell-selection key derivation re-derived identity from the mutated :name via name-to-kw. This shifted selection keys to :cleric-source-cantrips-known and orphaned saved entities holding the canonical :cleric-cantrips-known. Decouple display from key derivation: revert the mutation, plumb plugin-source through option-cfg as a distinct ::plugin-source slot, and compute the suffixed display string at the dropdown render site under a new user-pref toggle (::show-class-source-suffix, default off). Add a load-time reconciler (reconcile-spell-selection-keys) wired into :set-character that heals regression-window orphan keys when a single suffix-match candidate exists in the class's expected set. https://claude.ai/code/session_01RJNbubXUAhDenYSGUo3c7f
Adds key-vs-name-separation.md documenting the :key/:name design rule, the cantrip regression case study with all four leak sites, the benign-vs-leak heuristic for name-to-kw(:name) audits, and the plugin-load race verification trail. Saves future reviewers from re-running this investigation. https://claude.ai/code/session_01RJNbubXUAhDenYSGUo3c7f
Three one-liners at the speculation-prone sites: the plugin-classes sub (why :name is canonical), the reconciler's within-class branch (why it looks unreachable but isn't dead), and the set-character invocation (why db :plugins are trusted to be hydrated). https://claude.ai/code/session_01RJNbubXUAhDenYSGUo3c7f
console.info on the :rewrote list when set-character heals orphan keys, so the rebind is visible in the browser console rather than silent. From review feedback on the phase 1 reconciler. https://claude.ai/code/session_01RJNbubXUAhDenYSGUo3c7f
Captures: phase 1 commits already on branch, the mid-thread architectural pivot from convention-defended to architecture-enforced identity, the planned phase 2 (key-based kw derivation) + phase 3 (import-rename abbreviation in name+key) + relink UI, why ':parked' was vapor and got dropped, why the plugin-load race is a non-issue, and the reasoning traps to avoid. https://claude.ai/code/session_01RJNbubXUAhDenYSGUo3c7f
Removes :name from the identity pipeline for spell-selection keys outside the class editor. Previously kw came from name-to-kw(title) where title was built from class-name; now kw derives from class-key directly via the new opt5e/spell-selection-key helper. Class-name still feeds title for display. Built-in classes produce byte-identical keys (e.g. :cleric-cantrips-known). Conflict-renamed homebrew now produces source-aware keys (:artificer-kibbles-tasty-cantrips-known instead of the previously name-collapsed :artificer-cantrips-known); saved characters with the older shape get auto-rewritten by the reconciler at load via suffix match. Reconciler updates: - class->expected-spell-keys computes from class-key, matching the new derivation - input is now a flat set of known class keys (built-ins + plugins) rather than a seq of class data; events.cljs assembles that set - :parked accumulator dropped — never fired in practice and the existing missing-content banner already handles the user-facing case - builtin-classes made public so events.cljs can build the known set - tests updated for new fixture shape and rewritten orphan scenario Dead helpers removed: - options/class-key-name (latent leak fallback, no callers anywhere) ?prepare-spell-count at template_base.cljc:275 remains the one outstanding name-to-kw site. Surgical fix requires propagating :class-key through spell-data + spells-known callers — its own follow-up PR. Phase 1 keeps :name canonical, so active behavior is correct. Code comment + kb doc flag the latent risk. https://claude.ai/code/session_01RJNbubXUAhDenYSGUo3c7f
events.cljs and content_reconciliation.cljs both had their own copy of the 12 SRD class keys. Consolidate to one def in classes.cljc (the semantic home for class data); update both call sites to reference it via the class5e alias. content-recon/builtin-classes is removed entirely; the builtin? predicate now consults class5e/base-class-keys directly. events.cljs's loaded-class-keys helper does the same. If a 13th SRD class is ever added, there is now exactly one place to update. https://claude.ai/code/session_01RJNbubXUAhDenYSGUo3c7f
…d findings Concise session handoff for an agent picking up this branch in a fresh web context. Covers: the bug + root cause, what's shipped vs. pending (phase 1 + phase 2 + dedupe are landed; phase 3 import-rename abbreviation and the relink UI are not), the architectural decision (class-key-based identity outside the editor, editor-save carve-out), verified findings (plugin-load race is a non-issue; :parked branch was vapor; base-class-keys was duplicated; built-in content registry is genuinely missing), open architectural concern (shim-shadow shim sets for every built-in content type), and reasoning traps from prior sessions to avoid repeating. Explicitly flags the stale-local-checkout trap that previously led to incorrect claims about branch state. https://claude.ai/code/session_01RJNbubXUAhDenYSGUo3c7f
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The plugin-classes sub mutated class :name to "Cleric (Source)" for
display, but downstream spell-selection key derivation re-derived
identity from the mutated :name via name-to-kw. This shifted selection
keys to :cleric-source-cantrips-known and orphaned saved entities
holding the canonical :cleric-cantrips-known.
Decouple display from key derivation: revert the mutation, plumb
plugin-source through option-cfg as a distinct ::plugin-source slot,
and compute the suffixed display string at the dropdown render site
under a new user-pref toggle (::show-class-source-suffix, default off).
Add a load-time reconciler (reconcile-spell-selection-keys) wired into
:set-character that heals regression-window orphan keys when a single
suffix-match candidate exists in the class's expected set.
https://claude.ai/code/session_01RJNbubXUAhDenYSGUo3c7f