reafactor: data model#93
Merged
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Add Folder layout section to Standards. Types live with their domain (Character type in src/character/, Equipment type in models/common/gear/, etc.), helpers fold into model projections, and standalone files go in named per-domain folders. Accepted non-domain folders: data/, services/, lib/. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Restore API symmetry with the rest of the models tier — both had
get()/list() but no find(). Add find({id}) returning T | undefined,
matching species/backgrounds.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Shuffle files that were sitting in models/common but weren't actually
model modules:
- character-storage → src/services/ (I/O, not a catalog)
- icons + GOLD_ICON → src/lib/ (low-level primitive; GOLD_ICON also
moves off gear/equipment)
- character.ts + character-stats.ts → src/character/ (Character domain
has its own folder now, pairing the type with its derived stats)
- compute fns extracted from abilities.ts + skills.ts → src/character/
modifiers.ts (computeModifier/formatModifier/computeProficiencyBonus/
computeSkillModifier — shared d20 math, not per-entity)
- damage-types.ts → src/models/common/damage/{damage-types,index}.ts
(damage is shared by weapons + spells; subfolder leaves room to grow)
- gear/* → src/models/common/gear/* (regroup under common, which was
always the right home)
Also inline recommendedScores into class-details.json and expose via
classes.get({id}).recommendedScores; promote standardArray to its own
JSON in src/data/class/. Drops the redundant recommended-scores module.
No API changes — this is pure relocation. Models whose files moved keep
the same exported object/type shape. All 398 existing tests still pass.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Both actions and weapons used to export multiple model objects from one
file, violating the one-entity-per-file rule in src/models/CLAUDE.md.
- src/models/common/actions.ts → actions/{combat,class,exploration}.ts
plus actions/index.ts barrel re-exporting combatActions, classActions,
explorationActions (and their types). Tests split to match.
- src/models/common/gear/weapons.ts → weapons/weapon-properties/
weapon-mastery.ts. Added gear/index.ts barrel re-exporting armor,
weapons, weaponProperties, weaponMastery, plus their types and
Equipment. Tests split to match.
External consumers now import from the folder barrel (e.g. `import
{ Equipment, armor } from "src/models/common/gear"`). Existing deep
paths like src/models/common/gear/armor still work and are used by
siblings inside the gear folder.
Weapons keeps its {id}|{name} dual lookup for now — step 4 drops the
name variant.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
armor and weapons previously accepted { id: string } | { name: string }
criteria, breaking the rule that criteria mirror entity fields (id).
The dual lookup also meant their id types were just `string`, not narrow
unions like other models (CharacterClass, Species, ArmorCategory...).
- Add `ArmorId` (13 literals) and `WeaponId` (38 literals) narrow unions.
- Drop the { name } variant from armor.get/find and weapons.get/find;
criteria is now { id }.
- Equipment gains `armorId?: ArmorId`. Starting-equipment populates it
when creating armor/shield items, letting character-stats look armor
up by id instead of by display name — the one runtime call site that
depended on name lookup.
- Drop the corresponding { name } tests from armor.test.ts and
weapons.test.ts. Update character-stats.test.ts fixtures to include
the new armorId field.
Tests: 398 → 388 (name-lookup tests removed), all green. Build green.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Equipment resolution is tightly coupled to class data — every
StartingEquipmentItem lives on a ClassDetails. It belongs as a
projection on the classes model, not a standalone helper file.
- Move the toEquipment / resolveStartingEquipment logic into classes.ts.
- Expose classes.startingEquipment({id}) returning Equipment[].
- Delete src/models/common/gear/starting-equipment.ts + its test.
- Migrate character-creation.tsx and step-equipment.tsx consumers.
- Port the test cases to classes.test.ts as classes.startingEquipment()
describes, plus a new assertion that each resolved armor piece
carries its armorId (from the step-4 change).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
spells was missing get/find/list — only findAll/limit for class-scoped
queries. spell-timing was a standalone helper that transformed Spell
arrays but didn't belong outside the model.
- Add spells.get({id}), spells.find({id}), spells.list() for
per-spell lookup across the full cantrip + level-1 catalog.
- Fold spellCastingTiming → spells.timing(spell) projection.
- Fold groupSpellsByTiming → spells.groupByTiming(spells) projection.
- Delete src/models/spells/spell-timing.ts + its test; port the timing
cases into spells.test.ts with new coverage for get/find/list.
- Migrate the one consumer (action-bar.tsx).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…jections
class-resources was a pile of standalone functions over class-resource
data. Reshape it into a proper model plus projections on the classes
model, and split runtime rest logic into the character domain.
- class-resources.ts: `classResources` model (get/find/list) over a flat
catalog of ResourceDefinition (id, name, description, icon —
class-agnostic; resetOn lives per class-resource entry, not on the
definition). Internal helpers `resolveResourcesForLevel` and
`resolveResourceResetOn` stay here, consumed by the classes model.
- classes.ts: add `classes.resources({id, level, abilityScores})` →
CharacterResource[] (runtime state for that class+level) and
`classes.resourceResetOn({id, resourceId})` → RestType. Both back the
old standalone helpers without losing class-specific reset semantics
(same resource id can reset differently per class, e.g. cleric vs
paladin channel-divinity).
- src/character/rest.ts: applyRest moves here as a pure state transform
over CharacterResource[]. Uses classes.resourceResetOn internally.
- Migrate consumers: character-creation → classes.resources; character-
sheet → applyRest from src/character/rest; resource-tracker →
classResources.find + classes.resourceResetOn.
- Tests: class-resources.test.ts keeps resolve/classResources coverage;
applyRest tests move to src/character/rest.test.ts.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- Move data/gear/* to data/common/gear/* so the data tree mirrors src/models/common/gear/ (which regrouped under common in step 2). - Extract the inline DATA arrays from abilities.ts (6 entries) and skills.ts (18 entries) into data/common/abilities.json and data/common/skills.json. The narrow id unions, types, and the model object stay in the .ts file; the tables are just catalog data. src/data/ and src/models/ now share the same folder shape. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Bird's-eye view of every model in the folder: what lives where, plus a minified DTO for each with the main fields only. Complements CLAUDE.md (conventions) with a quick reference for what's catalogued. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Use actual samples drawn from the JSON data so the README is self-illustrating — barbarian, rage, longsword, leather-armor, acolyte, alarm, etc. — instead of bare field lists. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Refactors the data model and domain boundaries by moving general-purpose logic into src/character/, extracting JSON catalogs, and reorganizing “models” into per-domain modules with consistent get/find/list APIs.
Changes:
- Introduces new domain modules (actions, gear, spells, class resources) with barrels and symmetrized
get/find/listAPIs. - Extracts inline tables into JSON catalogs (
abilities,skills, gear data) and updates consumers. - Moves character-derived logic (modifiers, rest, stats) into
src/character/and storage intosrc/services/.
Reviewed changes
Copilot reviewed 83 out of 89 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/models/spells/spells.ts | Adds get/find/list and timing projections; builds ID index |
| src/models/spells/spells.test.ts | Adds coverage for new spells API + timing/grouping |
| src/models/spells/spell-timing.ts | Removes standalone timing helpers (folded into spells model) |
| src/models/spells/spell-timing.test.ts | Removes tests for deleted spell-timing module |
| src/models/origin/backgrounds.ts | Switches icon resolver import to src/lib/icons |
| src/models/gear/weapons.ts | Removes old weapons model (moved under common/gear) |
| src/models/gear/weapons.test.ts | Removes tests for deleted weapons model |
| src/models/gear/starting-equipment.ts | Removes standalone starting equipment resolver (moved to classes projection) |
| src/models/gear/starting-equipment.test.ts | Removes tests for deleted starting equipment resolver |
| src/models/gear/equipment.ts | Updates equipment types (damage import + optional armorId) |
| src/models/gear/armor.ts | Removes old armor model (moved under common/gear) |
| src/models/gear/armor.test.ts | Removes tests for deleted armor model |
| src/models/common/skills.ts | Loads skills from JSON; removes computeSkillModifier helper |
| src/models/common/skills.test.ts | Updates tests after removing computeSkillModifier |
| src/models/common/rest-actions.ts | Adds find() to restActions model |
| src/models/common/rest-actions.test.ts | Adds tests for restActions.find() |
| src/models/common/recommended-scores.ts | Removes old recommended scores module |
| src/models/common/proficiency-details.ts | Adds find() to proficiencyDetails model |
| src/models/common/proficiency-details.test.ts | Adds tests for proficiencyDetails.find() |
| src/models/common/icons.ts | Adds GOLD_ICON export alongside icon variant + path resolver |
| src/models/common/gear/weapons.ts | Adds new weapons model keyed by WeaponId |
| src/models/common/gear/weapons.test.ts | Adds tests for new common/gear weapons model |
| src/models/common/gear/weapon-properties.ts | Adds weapon properties model under common/gear |
| src/models/common/gear/weapon-properties.test.ts | Adds tests for weapon properties model |
| src/models/common/gear/weapon-mastery.ts | Adds weapon mastery model under common/gear |
| src/models/common/gear/weapon-mastery.test.ts | Adds tests for weapon mastery model |
| src/models/common/gear/index.ts | Adds barrel exports for common gear models/types |
| src/models/common/gear/armor.ts | Adds new armor model keyed by ArmorId |
| src/models/common/gear/armor.test.ts | Adds tests for new common/gear armor model |
| src/models/common/damage/index.ts | Adds barrel export for DamageType |
| src/models/common/character.ts | Updates Character type to import Equipment from common/gear |
| src/models/common/character-storage.ts | Switches Character type import to src/character/character |
| src/models/common/character-storage.test.ts | Updates Character type import for storage tests |
| src/models/common/character-stats.ts | Moves modifier logic to src/character/modifiers and uses armorId lookup |
| src/models/common/character-stats.test.ts | Updates AC tests to include armorId in equipment fixtures |
| src/models/common/actions/index.ts | Adds actions barrel to replace deleted single-file actions module |
| src/models/common/actions/exploration.ts | Splits exploration actions into dedicated module |
| src/models/common/actions/exploration.test.ts | Adds tests for exploration actions module |
| src/models/common/actions/combat.ts | Splits combat actions into dedicated module |
| src/models/common/actions/combat.test.ts | Adds tests for combat actions module |
| src/models/common/actions/class.ts | Splits class actions into dedicated module |
| src/models/common/actions/class.test.ts | Adds tests for class actions module |
| src/models/common/actions.ts | Removes old combined actions module |
| src/models/common/actions.test.ts | Removes tests for deleted combined actions module |
| src/models/common/abilities.ts | Loads abilities from JSON; removes modifier/proficiency helpers |
| src/models/common/abilities.test.ts | Updates tests after moving modifier/proficiency helpers out |
| src/models/class/classes.ts | Adds projections (startingEquipment/resources/resourceResetOn) and inlines recommended scores |
| src/models/class/classes.test.ts | Adds tests for classes.startingEquipment() projection |
| src/models/class/class-resources.ts | Refactors resource catalog into model + exposes reset lookup + list |
| src/models/class/class-resources.test.ts | Updates tests for new classResources model + projections |
| src/models/README.md | Documents new model tree and API “shape map” |
| src/elements/elements-showcase.tsx | Updates icon resolver import to src/lib/icons |
| src/elements/ability-card/ability-card.tsx | Switches modifier helpers to src/character/modifiers |
| src/data/common/skills.json | Adds JSON catalog for skills |
| src/data/common/abilities.json | Adds JSON catalog for abilities |
| src/data/class/standard-array.json | Adds JSON for standard array |
| src/data/class/recommended-scores.json | Removes old combined standard/recommended scores JSON |
| src/data/class/class-details.json | Adds per-class recommendedScores and re-formats equipment entries |
| src/components/proficiency-grid/proficiency-grid.tsx | Updates icon resolver import to src/lib/icons |
| src/components/inventory/inventory.tsx | Updates icon/money imports and Equipment type import |
| src/components/character-sheet/spell-cards/spell-cards.tsx | Updates icon resolver import to src/lib/icons |
| src/components/character-sheet/spell-book/spell-book.tsx | Updates icon resolver import to src/lib/icons |
| src/components/character-sheet/rest-bar/rest-bar.tsx | Updates icon resolver import to src/lib/icons |
| src/components/character-sheet/resource-tracker/resource-tracker.tsx | Switches to classes projections + classResources model + new icon resolver |
| src/components/character-sheet/exploration-bar/exploration-bar.tsx | Updates icon resolver import; uses actions barrel |
| src/components/character-sheet/combat-stats/combat-stats.tsx | Uses src/character stat types and modifier formatting; updates icon resolver |
| src/components/character-sheet/combat-stats/combat-stats.test.tsx | Updates stat type import path |
| src/components/character-sheet/character-sheet.tsx | Switches stats/rest/storage to src/character + src/services; updates Character type |
| src/components/character-sheet/character-overview/character-overview.tsx | Updates Character type import path |
| src/components/character-sheet/action-bar/action-bar.tsx | Uses spells model projection for grouping + new icon resolver |
| src/components/character-list/character-list.tsx | Updates Character type import path |
| src/components/character-creation/step-equipment/step-equipment.tsx | Uses classes.startingEquipment() instead of removed resolver |
| src/components/character-creation/step-abilities/use-ability-scores.ts | Loads standard array JSON and uses classes.recommendedScores |
| src/components/character-creation/step-abilities/step-abilities.tsx | Switches modifier helpers to src/character/modifiers |
| src/components/character-creation/character-creation.tsx | Switches to classes projections + src/character + src/services imports |
| src/components/cast-spell-grid/cast-spell-grid.tsx | Updates icon resolver import to src/lib/icons |
| src/character/rest.ts | Adds applyRest to character domain using classes resource reset projection |
| src/character/rest.test.ts | Adds tests for applyRest behavior |
| src/character/modifiers.ts | Adds modifier/proficiency/skill modifier helpers to character domain |
| src/character/modifiers.test.ts | Adds tests for moved modifier/proficiency/skill modifier helpers |
| src/app.tsx | Uses new storage module and Character type import |
| README.md | Updates coverage badge |
| AGENTS.md | Documents the new folder layout conventions |
Comments suppressed due to low confidence (3)
src/models/class/class-resources.ts:1
BY_IDis aMap<ResourceId, ResourceDefinition>, soBY_ID.get(id)is a type error becauseidisstringhere. Align with other models by either (a) changing the signature tofind({ id }: { id: ResourceId })or (b) castingid as ResourceIdwhen callingBY_ID.get(...).
src/models/spells/spells.ts:1spells.get()andspells.find()take different ID types (SpellIdvsstring), which makes the API asymmetric and harder to use consistently. Consider using the same parameter type for both (e.g.{ id: SpellId }), and only acceptstringvia an explicit escape hatch if needed.
src/models/spells/spells.ts:1spells.get()andspells.find()take different ID types (SpellIdvsstring), which makes the API asymmetric and harder to use consistently. Consider using the same parameter type for both (e.g.{ id: SpellId }), and only acceptstringvia an explicit escape hatch if needed.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+5
to
+11
| function res( | ||
| resourceId: string, | ||
| current: number, | ||
| max: number, | ||
| ): CharacterResource { | ||
| return { resourceId, current, max }; | ||
| } |
There was a problem hiding this comment.
CharacterResource.resourceId is typed as ResourceId (not string) in src/models/class/class-resources.ts, so this helper widens string literals and no longer type-checks. Update the helper signature to resourceId: CharacterResource[\"resourceId\"] (or import ResourceId and use that type) so the returned object is assignable to CharacterResource.
4c8173c to
1714b1c
Compare
…storage errors - Equipment reshaped as discriminated union on `type` with per-variant required fields; callers narrow via `item.type === "..."`. - classes.toEquipment fills weaponId, drops armor quantity, throws on unknown gear id via GENERIC_GEAR_IDS allow-list. - class-resources: new AbilityModKey union replaces `string` in ProgressionValue; resolveMax / resolveResourcesForLevel / resolveResourceResetOn throw on unknown instead of silent fallback. - spells: export SpellId and new SpellLevel (0–9); inline helper renamed parseCastingTiming. - character-storage: LoadResult gains `unavailable` flag; per-entry validation returns typed reason and logs dropped entries; save/delete re-throw on write failure. - character.weaponMasteries typed as WeaponMasteryName[]. - Tests updated for new Equipment shape; new cases for applyRest (empty, non-mutation, unknown id) and storage (unavailable, write failure). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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
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.
recommendedScores.