Igor/inventory + char status

[igor47]

we refactor our prompt to contain static info, fix Reed's personality, and provide way more tools to get character status and for item creation. lots of refactoring here to make things more consistent.

[Copilot]

Pull Request Overview

This PR refactors form handling to support complex data structures (arrays and objects) and adds new read-only character information tools for the AI assistant. The key changes include:

• Simplified AI system prompt to rely on tools for character state instead of embedding all data
• Added ArrayField and ObjectArrayField helper functions to handle form data parsing
• Refactored short rest to use array-based dice and arcane slot selection
• Refactored item creation/update to use object arrays for damage entries
• Added new read-only tools: character_status, character_traits, lookup_item_template, create_item
• Changed tool approval mechanism from requiresApproval boolean to presence of formatApprovalMessage
• Removed character_id from API schemas (now passed via service function parameters)

Reviewed Changes

Copilot reviewed 26 out of 26 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
test-schema-types.ts	Deleted test file (no longer needed)
src/tools.ts	Updated tool registration to use formatApprovalMessage pattern; added new character info and item tools
src/services/updateSpellSlots.ts	Removed character_id from schema, now using char.id directly
src/services/updateItem.ts	Refactored damage handling to use ObjectArrayField instead of individual numbered fields
src/services/shortRest.ts	Refactored to use ObjectArrayField for dice and ArrayField for arcane slots
src/services/shortRest.test.ts	Updated tests to use new array-based data structures
src/services/lookupSpell.ts	Removed formatApprovalMessage (read-only tool)
src/services/lookupItemTemplate.ts	New service for looking up item templates from SRD
src/services/createItemEffect.ts	Removed parsedToForm usage, return data directly
src/services/createItem.ts	Major refactor: damage as object array, new AI tool definition, removed parsedToForm
src/services/computeChat.ts	Updated to check formatApprovalMessage instead of requiresApproval
src/services/characterTraits.ts	New read-only tool returning character traits
src/services/characterStatus.ts	New read-only tool returning character status with spell slot summary
src/services/addTrait.ts	Removed character_id from schema, passed via function parameter
src/services/addLevel.ts	Removed character_id from schema
src/routes/character.tsx	Updated parseBody calls to use { all: true, dot: true } for complex form data
src/lib/formSchemas.ts	Added ArrayField and ObjectArrayField helpers for form data coercion
src/lib/formErrors.ts	Refactored zodToFormErrors to handle nested union errors and dot-notation paths
src/components/TraitEditForm.tsx	Removed hidden character_id input
src/components/ShortRestForm.tsx	Refactored to render dice and arcane slots as arrays using dot notation
src/components/EditItemForm.tsx	Refactored damage fields to use dot notation for object array
src/components/CreateItemForm.tsx	Refactored damage fields to use dot notation for object array
src/components/ChatBox.tsx	Fixed markdown renderer to properly parse inline tokens; hide approval UI for tools without formatters
src/ai/prompts.ts	Simplified system prompt to focus on character identity, delegating stats to character_status tool
src/ai/chat.ts	Updated approval checking to use formatApprovalMessage

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

The conversion logic here doesn't handle arrays correctly. The damage parameter is expected to be an array of objects (from CreateItemToolSchema), but calling .toString() on an array will produce a string like "[object Object],[object Object]" instead of preserving the array structure.

This will cause the createItem service to fail when trying to parse the damage field, as it expects an array of objects with num_dice, die_value, type, and versatile properties.

Consider preserving arrays and objects instead of converting everything to strings:

for (const [key, value] of Object.entries(parameters)) {
  if (value !== null && value !== undefined) {
    if (Array.isArray(value) || typeof value === 'object') {
      data[key] = value  // Preserve arrays and objects
    } else {
      data[key] = value.toString()
    }
  }
}

Or alternatively, handle the damage field specifically:

// Convert each parameter to string, except for damage which is an array
for (const [key, value] of Object.entries(parameters)) {
  if (value !== null && value !== undefined) {
    if (key === 'damage') {
      data[key] = value  // Keep as array
    } else {
      data[key] = value.toString()
    }
  }
}

Suggested change

	data[key] = value.toString()
	if (Array.isArray(value) || (typeof value === "object" && value !== null)) {
	data[key] = value; // Preserve arrays and objects
	} else {
	data[key] = value.toString();
	}

[Copilot]

The regex replacement logic for humanizing enum errors appears incorrect.

The first pass replaces "option"| with option|, which would transform:

• "option1"|"option2"|"option3" → option1|option2|option3|

The second pass replaces |"option" with , option, but after the first pass there are no more quoted options, so this second pass won't match anything.

The intended transformation is:

• Input: expected one of "option1"|"option2"|"option3"
• Output: expected one of option1, option2, option3

Consider this corrected implementation:

function humanizeEnumError(error: string): string {
  // Replace "option"| with option, (handles all but the last option)
  let result = error.replace(/"([^"]+)"\|/g, "$1, ")
  // Replace any remaining "option" (the last one) with option
  result = result.replace(/"([^"]+)"/g, "$1")
  return result
}

[Copilot]

Array mutation during map iteration. The code mutates selectedArcaneSlots array inside the map callback (line 172), which is used to track which slots have already been rendered. This pattern can lead to unexpected behavior.

The logic appears to be trying to handle duplicate spell slot levels (e.g., if the wizard has two level-1 used slots, render two separate checkboxes). However, mutating the tracking array during iteration makes the logic harder to understand and maintain.

Consider a clearer approach that doesn't mutate state during rendering:

{usedSpellSlots.map((level, index) => {
  // Count how many of this level have been rendered before this index
  const countBefore = usedSpellSlots.slice(0, index).filter(l => l === level).length
  // Check if this specific slot is selected
  const isChecked = selectedArcaneSlots.filter(l => l === level).length > countBefore
  const isDisabled = !isChecked && level > maxArcaneRecoveryLevelRemaining

  return (
    <div class="form-check" key={`${level}-${index}`}>
      {/* ... */}
    </div>
  )
})}

This approach tracks position by index rather than mutating the array.