Repository navigation
feat(character): clarify class-aware spell progression - #107
Conversation
|
📋 At a glance Files & modules (2)
✅ Health gate: passed 📌 Before you merge
🔎 More signals (3)🗺️ Change map flowchart LR
subgraph PR ["Changed in this PR (3 with dependents)"]
f_src_engine_magic_spell_validator_ts[".../magic/spell-validator.ts 🔥"]:::changed
f_src_server_consolidated_character_manage_ts[".../consolidated/character-manage.ts 🔥"]:::changed
f_src_server_consolidated_contracts_ts[".../consolidated/contracts.ts 🔥"]:::changed
end
f_src_engine_magic_spell_resolver_ts[".../magic/spell-resolver.ts"]
f_src_engine_magic_spell_validator_ts --> f_src_engine_magic_spell_resolver_ts
f_src_server_consolidated_rest_manage_ts[".../consolidated/rest-manage.ts"]
f_src_engine_magic_spell_validator_ts --> f_src_server_consolidated_rest_manage_ts
f_src_server_handlers_combat_handlers_ts[".../handlers/combat-handlers.ts"]
f_src_engine_magic_spell_validator_ts --> f_src_server_handlers_combat_handlers_ts
f_scripts_seed_bastion_ts["scripts/seed-bastion.ts"]
f_src_server_consolidated_character_manage_ts --> f_scripts_seed_bastion_ts
f_src_server_consolidated_batch_manage_ts[".../consolidated/batch-manage.ts"]
f_src_server_consolidated_character_manage_ts --> f_src_server_consolidated_batch_manage_ts
f_src_server_consolidated_index_ts[".../consolidated/index.ts"]
f_src_server_consolidated_character_manage_ts --> f_src_server_consolidated_index_ts
f_src_server_consolidated_npc_manage_ts[".../consolidated/npc-manage.ts"]
f_src_server_consolidated_character_manage_ts --> f_src_server_consolidated_npc_manage_ts
f_src_server_consolidated_contracts_ts --> f_src_server_consolidated_index_ts
t_tests_engine_magic_spell_validator_test_ts(["✅ .../magic/spell-validator.test.ts"]):::guard
t_tests_engine_magic_spell_validator_test_ts -.-> f_src_engine_magic_spell_validator_ts
t_tests_server_consolidated_character_origin_test_ts(["✅ .../consolidated/character-origin.test.ts"]):::guard
t_tests_server_consolidated_character_origin_test_ts -.-> f_src_server_consolidated_character_manage_ts
classDef changed fill:#dbeafe,stroke:#1d4ed8,color:#1e3a5f
classDef warn fill:#fef3c7,stroke:#b45309,color:#78350f
classDef guard fill:#dcfce7,stroke:#15803d,color:#14532d
Solid arrows: code that imports the changed files (7 direct dependents, from the last indexed snapshot). Dashed: history/tests. 🔥 Hotspots touched (4)
1 more
💀 Dead code (10 findings)
7 more
📊 See the full report for this PR |
|
Warning Review limit reachedNext included review available in 49 minutes. View limit detailsLimit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthroughCharacter management now enforces levels from 1 through 20 and documents class-specific spell fields, wizard spellbooks, and spell-slot validation. Descriptor keywords and tests cover the new spell metadata and level boundaries. ChangesCharacter management
Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk: ⚪ Minimal · up to This PR clarifies character level and class-aware spell fields without introducing an actionable merge-blocking risk; it is merge-ready after normal checks and review. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Full details: Docstring CoverageExplanation No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 3 files. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e3bd696dcb
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
Addressed in
Focused tests, build, and full engine CI pass locally; hosted Node checks are running on this head. |
Supports Mnehmos/rpg-mcp-live#301.
Risk
R1/R2 — character-tool schema and MCP discovery contract. The engine's existing authoritative progression and spell persistence remain the source of truth.
Scope
Make the character tool's level range and class-aware spellcasting fields explicit to the DM/model. The contract now documents known-spell, prepared-spell, and Wizard spellbook semantics, and the create/update/outer schemas describe the durable spell fields and enforce levels 1 through 20.
Acceptance criteria
character_manageadvertises class-aware known/prepared/spellbook semantics.Verification
npx vitest run tests/server/consolidated/character-manage.test.ts --pool=forks— 74 passed.npm run build— passed.npm run test:ci— 2,277 passed, 7 skipped.Persistence or migration
None.
Rollback
Revert this PR; the prior schema descriptions and level lower-bound-only validation return.
Explicit deferrals
No new player-side rules or alternate spell database is introduced.
Summary by CodeRabbit