fix(charsheet): Spell Save DC + Spell Attack Bonus header & class features on Abilities tab - #416
Merged
Merged
Conversation
…and class features (#optimizer) Two read-model/display data-depth gaps an optimizer persona flagged as critical in a built-.app playtest. Both are DISPLAY gaps — the engine has the data, the character screen did not surface it. Read-model + viewer only; no engine writes. Bug 1 — Spells tab never showed the character-level Spell Save DC / Spell Attack Bonus. PR #410 resolved per-spell saveDc, but there was no once-at-the-top caster summary. Added _casting_ability/_spell_attack_bonus/_character_spellcasting to viewer/server.py (reusing the #410 DC formula; mirrors engine server.spell_save_dc: DC = 8 + prof + mod, attack = prof + mod), exposed as hero.spellcasting on the character surface, and rendered a SpellcastingHeader at the top of SpellsTab. Non-caster -> None -> header omitted (no fabricated DC 0). Bug 2 — Abilities tab read "No active abilities recorded" for a L3 Wizard. The engine DOES populate class/subclass features as NAMES (Character.features via srd_tables.features_through), already surfaced by the read-model as classFeatures — but AbilitiesTab only read hero.abilities (always []) and class features were only shown on the Feats tab. AbilitiesTab now renders hero.classFeatures + honest class/subclass/level context. Names only — the engine does not model feature DESCRIPTIONS or racial traits, so those stay absent (never fabricated). Tests: viewer/tests/test_charsheet_depth.py (8) — caster DC/attack present, non-caster omitted cleanly, DC/attack track prof+ability, class-feature names surfaced + empty-when-engine-has-none. Full viewer suite: 231 passed. Do NOT close on merge — verify on the next 5-persona sweep (optimizer >=7, 0 critical).
📝 WalkthroughWalkthroughThe PR extends the character sheet to surface spellcasting summary data (Save DC and attack bonus derived from casting ability and proficiency) and displays class features alongside abilities. Backend helpers compute casting ability once and reuse it across DC and attack calculations; non-casters return ChangesSpellcasting Surface and Class Features
Sequence DiagramsequenceDiagram
participant Client
participant CharacterSurface as /character-surface
participant CharacterSheet as _character_sheet
participant Helpers as Spellcasting Helpers
participant UI as Frontend AbilitiesTab
Client->>CharacterSurface: GET ?campaign=wizard
CharacterSurface->>CharacterSheet: _character_sheet(character_dict)
CharacterSheet->>Helpers: _character_spellcasting(ch)
Helpers->>Helpers: _casting_ability(ch) → "intelligence"
Helpers->>Helpers: _spell_save_dc(ch) → 15
Helpers->>Helpers: _spell_attack_bonus(ch) → 5
Helpers-->>CharacterSheet: {abilityKey: "intelligence", spellSaveDc: 15, spellAttackBonus: 5}
CharacterSheet-->>CharacterSurface: {spellcasting: {...}, classFeatures: [...], abilities: [...]}
CharacterSurface-->>Client: 200 JSON response
Client->>UI: render hero with spellcasting + classFeatures
UI-->>Client: AbilitiesTab displays spellcasting block + class features list
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@viewer/openworlds/screen-character.jsx`:
- Around line 709-713: The message shown when {nothing && (...) } incorrectly
claims no "racial features" without checking hero.raceTraits; update the
conditional that builds the string (the JSX block using classLine inside the
{nothing && (...) } render) to also verify hero.raceTraits (or equivalent
prop/state) before mentioning racial features, so the fallback text only
references racial features when hero.raceTraits is empty/absent — adjust the
ternary that uses classLine to include a check like classLine &&
!hero.raceTraits (or include hero.raceTraits in the overall `nothing`
calculation) so the copy accurately reflects available race traits.
In `@viewer/server.py`:
- Around line 3635-3638: The lookup in _casting_ability currently only uses
cl.get("name") via _text(...) so rows that use "class_name" are missed; update
the lookup (in the block that assigns a =
_CASTING_ABILITY.get(_text(cl.get("name")).lower())) to fallback to
cl.get("class_name") when "name" is not present or empty (e.g., compute a key
from _text(cl.get("name") or cl.get("class_name")).lower() or attempt both
keys), then use that key to query _CASTING_ABILITY so casters are correctly
detected and spellcasting is emitted.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: e6a47ee3-0e2a-4801-bb9b-6b3eeecf22c8
📒 Files selected for processing (3)
viewer/openworlds/screen-character.jsxviewer/server.pyviewer/tests/test_charsheet_depth.py
Comment on lines
+709
to
+713
| {nothing && ( | ||
| <p className="body-sm muted" style={{ margin: 0 }}> | ||
| No active abilities recorded — this hero's edge is in their feats and class features. | ||
| {classLine | ||
| ? `No class, subclass, or racial features are recorded for this ${classLine} yet.` | ||
| : "No active abilities recorded — this hero's edge is in their feats and class features."} |
There was a problem hiding this comment.
Don't mention racial features here unless you check hero.raceTraits.
Line 711 only proves hero.abilities and hero.classFeatures are empty, so a hero can still have projected racial traits and get told none are recorded.
✏️ Suggested copy fix
- ? `No class, subclass, or racial features are recorded for this ${classLine} yet.`
+ ? `No class or subclass features are recorded for this ${classLine} yet.`🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@viewer/openworlds/screen-character.jsx` around lines 709 - 713, The message
shown when {nothing && (...) } incorrectly claims no "racial features" without
checking hero.raceTraits; update the conditional that builds the string (the JSX
block using classLine inside the {nothing && (...) } render) to also verify
hero.raceTraits (or equivalent prop/state) before mentioning racial features, so
the fallback text only references racial features when hero.raceTraits is
empty/absent — adjust the ternary that uses classLine to include a check like
classLine && !hero.raceTraits (or include hero.raceTraits in the overall
`nothing` calculation) so the copy accurately reflects available race traits.
Comment on lines
3635
to
+3638
| a = _CASTING_ABILITY.get(_text(cl.get("name")).lower()) | ||
| if a: | ||
| ability = a | ||
| break | ||
| return a | ||
| return None |
There was a problem hiding this comment.
Support class_name fallback in _casting_ability.
Line 3635 only reads cl.get("name"). If a snapshot class row uses class_name (already handled elsewhere in this file), casters are misclassified as non-casters and spellcasting is omitted.
Proposed fix
def _casting_ability(ch: dict) -> str | None:
@@
classes = ch.get("classes") if isinstance(ch.get("classes"), list) else []
for cl in classes:
if isinstance(cl, dict):
- a = _CASTING_ABILITY.get(_text(cl.get("name")).lower())
+ class_name = _text(cl.get("name") or cl.get("class_name")).lower()
+ a = _CASTING_ABILITY.get(class_name)
if a:
return a
return None🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@viewer/server.py` around lines 3635 - 3638, The lookup in _casting_ability
currently only uses cl.get("name") via _text(...) so rows that use "class_name"
are missed; update the lookup (in the block that assigns a =
_CASTING_ABILITY.get(_text(cl.get("name")).lower())) to fallback to
cl.get("class_name") when "name" is not present or empty (e.g., compute a key
from _text(cl.get("name") or cl.get("class_name")).lower() or attempt both
keys), then use that key to query _CASTING_ABILITY so casters are correctly
detected and spellcasting is emitted.
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
1 participant
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.
What & why
Two character-sheet data-depth gaps an optimizer persona flagged as critical in a built-
.appplaytest. Both are DISPLAY / read-model gaps — the engine already has the data; the character screen didn't surface it. Scope is strictlyviewer/server.py+viewer/openworlds/screen-character.jsx+viewer/tests/(no engine write-paths, no wire contracts touched).Bug 1 — Spell Save DC + Spell Attack Bonus not shown (caster planning was blind)
Engine has it / display showed it: The engine computes both (
servers/engine/server.py:4747spell_save_dc()returns{spell_save_dc: 8+prof+mod, spell_attack_bonus: prof+mod}), and PR #410 already added per-spellsaveDcto the read model (_spell_save_dc). But the Spells tab had no once-at-the-top character-level Spell Save DC / Spell Attack Bonus (the way D&D Beyond shows them once). A caster couldn't plan.Fix:
viewer/server.py: extracted_casting_ability(ch)(shared with the existing_spell_save_dc), added_spell_attack_bonus(ch)and_character_spellcasting(ch)— all reusing the fix(viewer): strip internal routing tag leaking into the chronicle (#410); verify roster/label reports #410 / engine formula (DC = 8 + prof + casting-mod,attack = prof + casting-mod). Exposed ashero.spellcasting({ability, abilityShort, spellSaveDc, spellAttackBonus}) on the character surface.screen-character.jsx: newSpellcastingHeaderrendered prominently at the top of SpellsTab.None→ the header is omitted entirely. No fabricatedDC 0.Bug 2 — Abilities tab empty for a L3 Wizard
Class-features finding — engine HAS (names), engine LACKS (descriptions + racial traits):
Character.features(models.py:673, filled fromsrd_tables.features_through/features_at), and the read model already projects them ashero.classFeatures/hero.proficiencies._character_sheethardcodes"abilities": [], andAbilitiesTabonly readhero.abilities(always empty) → "No active abilities recorded…". The populatedclassFeatureswere only ever rendered on the Feats tab.featuresislist[str]of names;detailis always"") and has no racial-traits field at all (Charactercarries noracial_traits).Fix:
screen-character.jsx:AbilitiesTabnow rendershero.classFeatures(names) + honest class/subclass/level context (Level 3 · Wizard · School of Evocation).detailshows only if the data carries it. Racial traits / feature prose stay absent.data/srd/class_features.json) and a racial-traits field. Minor: class features now appear on both Abilities and Feats tabs; de-duping is a UX follow-up.How verified
viewer/tests/test_charsheet_depth.py(8 tests, mirrorstest_readmodel_surfaces.py): caster DC/attack present (Wizard L3 INT16 → DC 13, atk +5); non-caster (Fighter) omitted cleanly (spellcasting is None); DC/attack track prof+ability (L5 INT18 → DC 15, atk +7); engine class-feature names surfaced; empty list when the engine has none (no fabrication).screen-character.jsxcompiles viaesbuild(exit 0).Summary by CodeRabbit
Release Notes
New Features
Tests