feat(data-layer): DL-24 - Implement Turn Resolution Management#102
Conversation
There was a problem hiding this comment.
Pull request overview
This pull request implements a comprehensive turn resolution management system (DL-24) for tracking mechanical resolution of actions during gameplay. The implementation adds new schemas, MongoDB CRUD operations, auth middleware updates, and comprehensive test coverage.
Key Changes
- New resolution schemas defining data contracts for action resolutions including dice rolls, contested checks, card draws, and effect tracking
- MongoDB CRUD tools for creating, retrieving, updating, and deleting resolution records with validation against scenes, turns, and stories
- Auth middleware configuration with appropriate authority assignments for resolution operations
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/data-layer/src/monitor_data/schemas/resolutions.py | New schema file defining enums (ActionType, ResolutionType, SuccessLevel, EffectType) and data structures (Modifier, RollResult, ContestedRoll, CardDraw, Mechanics, Effect) along with CRUD and query schemas |
| packages/data-layer/src/monitor_data/tools/mongodb_tools.py | Added 5 new resolution CRUD functions with proper validation, a helper conversion function, and updated combat tools to use consistent Neo4j client patterns |
| packages/data-layer/src/monitor_data/schemas/init.py | Exported all new resolution-related schemas and classes |
| packages/data-layer/src/monitor_data/middleware/auth.py | Added authority matrix entries for resolution operations with appropriate role restrictions |
| packages/data-layer/tests/test_tools/test_resolution_tools.py | Comprehensive test suite with 19 tests covering all CRUD operations, validation scenarios, and resolution types |
| packages/data-layer/tests/test_tools/test_combat_tools.py | Added story validation test and updated Neo4j client mocking to use execute_read pattern |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| effect_type: EffectType | ||
| target_id: UUID = Field(description="Entity affected by this effect") | ||
| magnitude: int = Field(description="Numeric magnitude (damage, healing, etc.)") |
There was a problem hiding this comment.
The magnitude field in the Effect schema is required and has no default value, but its usage varies by effect_type. For some effect types like CONDITION or POSITION_CHANGE, a numeric magnitude may not be meaningful or necessary (e.g., applying "grappled" condition, or moving to a new position). Consider making magnitude optional with a default value of 0, or adding validation to ensure magnitude is only required for effect types where it makes sense (DAMAGE, HEALING, RESOURCE_CHANGE, STAT_CHANGE).
| magnitude: int = Field(description="Numeric magnitude (damage, healing, etc.)") | |
| magnitude: int = Field(default=0, description="Numeric magnitude (damage, healing, etc.)") |
| description="Dice kept after keep/drop logic (may equal raw_rolls)", | ||
| ) | ||
| total: int = Field(description="Final total after modifiers") | ||
| natural: int = Field( |
There was a problem hiding this comment.
The natural field in RollResult is marked as required but doesn't have a default value, while other fields like critical and fumble have defaults. This creates an inconsistency where some optional roll data (critical/fumble) has defaults, but the natural field doesn't.
For contested rolls where only the opponent_roll is being created without modifiers, this could cause issues. Consider making natural have a default value of 0, or if it should always be calculated from kept_rolls, consider making it a computed property or ensuring it's always provided during construction.
| natural: int = Field( | |
| natural: int = Field( | |
| default=0, |
* Fix combat creation Neo4j validation * Add combat story validation test --------- Co-authored-by: spuentesp <spuentesp@gmail.com>
Addressed both code review comments from Copilot: 1. **P1: Make magnitude optional** - Added default=0 to Effect.magnitude field since numeric magnitude doesn't apply to all effect types (e.g., CONDITION for "grappled", POSITION_CHANGE for movement) 2. **P2: Add default to natural field** - Added default=0 to RollResult.natural for consistency with other optional fields like critical and fumble These changes improve schema flexibility and consistency. All 256 tests still passing ✅ 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
7fe70ef to
b3034b6
Compare
Added trailing comma to function parameter to match black 25.12.0 formatting rules used in CI. This is a formatting-only change with no functional impact. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Summary
Implements comprehensive turn resolution storage system for tracking mechanical resolution of actions during gameplay, including dice rolls, contested checks, card draws, and effect tracking.
Schemas (resolutions.py)
New Enums:
ActionType: combat, skill, social, exploration, magic, otherResolutionType: dice, card, narrative, deterministic, contestedSuccessLevel: critical_success, success, partial_success, failure, critical_failureEffectType: damage, healing, condition, buff, debuff, resource_change, stat_change, position_change, otherData Structures:
Modifier: source, value, reason for modifiersRollResult: raw/kept rolls, total, natural, critical/fumble flagsContestedRoll: opponent data with margin of victoryCardDraw: cards drawn, total value, special resultsMechanics: formula, modifiers, target, roll/contested/card dataEffect: effect type, target, magnitude, duration, descriptionCRUD Schemas:
ResolutionCreate/Update/Response: Full resolution managementResolutionFilter/ListResponse: Flexible queryingMongoDB Tools
New Tools:
mongodb_create_resolution: Create resolution with validationmongodb_get_resolution: Retrieve resolution by IDmongodb_list_resolutions: Filter by scene, turn, actor, action_type, success_levelmongodb_update_resolution: Update effects, description, gm_notesmongodb_delete_resolution: Remove resolution recordAuth Middleware
Authority Checks:
mongodb_create_resolution: Orchestrator, CanonKeepermongodb_get_resolution: *mongodb_list_resolutions: *mongodb_update_resolution: Orchestrator, CanonKeepermongodb_delete_resolution: CanonKeeperTests
Added 19 comprehensive tests:
All 256 tests passing ✅
Architecture Decisions
✅ Pure data storage - Dice rolling/evaluation logic lives in agents layer
✅ Turn validation - Validates turn exists in scene before creating resolution
✅ Multi-resolution support - Dice, contested, card, narrative, deterministic types
✅ Flexible mechanics - Structure accommodates different game systems
✅ Effects tracking - Array tracks all changes resulting from resolution
✅ GM notes - Field for secret information
Implements: DL-24
Blocks Unblocked: P-4 (Player Action), P-9 (Dice Roll Mechanics)
🤖 Generated with Claude Code
Co-Authored-By: Claude Sonnet 4.5 noreply@anthropic.com