feat(data-layer): DL-20 - Game Systems & Rules Management#108
Conversation
Implements MongoDB CRUD for game system definitions and rule overrides. Pure data storage - rule execution logic lives in agents layer. ## Game Systems (MongoDB) New schemas (game_systems.py): - CoreMechanicType, SuccessType, RuleOverrideScope enums - CoreMechanic: type, formula, success_type, critical rules - AttributeDefinition, SkillDefinition, ResourceDefinition - GameSystemCreate/Update/Response schemas - Built-in system protection (is_builtin flag) MongoDB tools (mongodb_tools.py): - mongodb_create_game_system: Create custom systems - mongodb_get_game_system: Retrieve system by ID - mongodb_list_game_systems: List with builtin filtering - mongodb_update_game_system: Update custom systems only - mongodb_delete_game_system: Delete custom systems only - _ensure_builtin_systems_seeded: Auto-seed on first access - _load_builtin_game_systems: Load from JSON seed file ## Rule Overrides (MongoDB) New schemas (game_systems.py): - RuleOverrideCreate/Update/Response - Scopes: one_time, scene, story, universe - Tracking: times_used, active status MongoDB tools (mongodb_tools.py): - mongodb_create_rule_override: Create scoped overrides - mongodb_get_rule_override: Retrieve by ID - mongodb_list_rule_overrides: Filter by scope/ID/active - mongodb_update_rule_override: Update usage/status - mongodb_delete_rule_override: Remove override ## Built-in Systems Seed Data (builtin_systems.json) Three pre-configured systems: 1. D&D 5e: d20 system with 6 attributes, 18 skills, HP/AC 2. Fate Core: Fudge dice (4dF) with 6 approaches, aspects 3. PbtA: 2d6+stat with 5 stats, partial success mechanics Protected from modification/deletion via is_builtin flag. ## Tests Added 23 comprehensive tests (test_game_system_tools.py): - Game system CRUD (11 tests): create, get, list, update, delete - Builtin protection (3 tests): prevent manual creation/edit/delete - Rule override CRUD (8 tests): full lifecycle + filtering - Seeding (2 tests): auto-seed on first access, prevent re-seed - All 357 tests passing ## Architecture - Pure data storage in MongoDB (no dice rolling/rule execution) - Rule interpretation logic reserved for agents layer - Built-in systems seeded lazily on first mongodb_get/list call - Custom systems fully mutable, builtins immutable Implements: DL-20, supports RS-1 (rules engine) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Adds a 4th built-in game system demonstrating fiction-first play: ## Narrative Tags System Key features: - Zero numeric attributes (attributes: []) - 10 descriptive tags as 'skills': Brave, Cunning, Strong-willed, Charming, Observant, Athletic, Scholarly, Connected, Stealthy, Empathetic - Pure narrative resolution via tag relevance + fiction quality - Resources: Resolve (5) and Momentum (3) for narrative economy Custom mechanics: - Oracle outcomes (yes_and, yes, yes_but, no_but, no, no_and) - Tag invocation (spend Resolve for advantage) - Momentum system (gain from risks, spend for narrative control) Demonstrates system flexibility: - Supports zero-stat systems - Skills work as tags without linked attributes - Narrative core mechanic type - custom_dice holds oracle tables and economy rules All 23 tests passing ✅ 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR implements MongoDB CRUD operations for game system definitions and rule overrides, establishing the data storage foundation for the rules engine. The implementation provides pure data layer functionality without rule execution logic, which is deferred to the agents layer.
Key Changes
- Game system schemas defining core mechanics, attributes, skills, and resources with support for custom dice systems
- MongoDB tools for full CRUD lifecycle of game systems and rule overrides with built-in system protection
- Automatic seeding of three pre-configured game systems (D&D 5e, Fate Core, PbtA) on first database access
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
packages/data-layer/src/monitor_data/schemas/game_systems.py |
New schemas for game systems and rule overrides with enums for mechanic types, success determination, and override scopes |
packages/data-layer/src/monitor_data/tools/mongodb_tools.py |
MongoDB CRUD operations for game systems and rule overrides, including seeding logic and built-in system protection |
packages/data-layer/src/monitor_data/schemas/__init__.py |
Export declarations for new game system schemas |
packages/data-layer/src/monitor_data/data/builtin_systems.json |
Seed data for three pre-configured game systems with comprehensive attributes, skills, and resources |
packages/data-layer/tests/test_tools/test_game_system_tools.py |
Comprehensive test suite covering all CRUD operations, built-in protection, and seeding behavior |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # Check if any builtin systems exist | ||
| existing_count = systems_collection.count_documents({"is_builtin": True}) | ||
| if existing_count > 0: | ||
| return # Already seeded | ||
|
|
||
| # Load and insert builtin systems | ||
| builtin_systems = _load_builtin_game_systems() | ||
| now = datetime.now(timezone.utc) | ||
|
|
||
| for system_data in builtin_systems: | ||
| system_id = uuid4() | ||
| doc = { | ||
| "system_id": str(system_id), | ||
| **system_data, | ||
| "created_at": now, | ||
| "updated_at": None, | ||
| } | ||
| systems_collection.insert_one(doc) |
There was a problem hiding this comment.
This seeding logic has a race condition. If multiple processes or threads call this function simultaneously, they may both detect existing_count == 0 and attempt to seed the systems, resulting in duplicate entries or conflicts. Consider using MongoDB's unique indexes or atomic operations to ensure only one process seeds the data.
| # Check if any builtin systems exist | |
| existing_count = systems_collection.count_documents({"is_builtin": True}) | |
| if existing_count > 0: | |
| return # Already seeded | |
| # Load and insert builtin systems | |
| builtin_systems = _load_builtin_game_systems() | |
| now = datetime.now(timezone.utc) | |
| for system_data in builtin_systems: | |
| system_id = uuid4() | |
| doc = { | |
| "system_id": str(system_id), | |
| **system_data, | |
| "created_at": now, | |
| "updated_at": None, | |
| } | |
| systems_collection.insert_one(doc) | |
| # Load and upsert builtin systems atomically to avoid race conditions | |
| builtin_systems = _load_builtin_game_systems() | |
| now = datetime.now(timezone.utc) | |
| for system_data in builtin_systems: | |
| # Use name (and is_builtin) to uniquely identify each builtin system. | |
| # The upsert ensures only one document is inserted per system, even if | |
| # multiple processes run this seeding code concurrently. | |
| filter_doc = { | |
| "is_builtin": True, | |
| "name": system_data.get("name"), | |
| } | |
| doc = { | |
| "system_id": str(uuid4()), | |
| "is_builtin": True, | |
| **system_data, | |
| "created_at": now, | |
| "updated_at": None, | |
| } | |
| systems_collection.update_one( | |
| filter_doc, | |
| {"$setOnInsert": doc}, | |
| upsert=True, | |
| ) |
| from monitor_data.schemas.game_systems import ( | ||
| CoreMechanic, | ||
| AttributeDefinition, | ||
| SkillDefinition, | ||
| ResourceDefinition, | ||
| ) |
There was a problem hiding this comment.
These schema imports are redundant since the schemas are already imported at the top of the file (lines 84-93). Importing them again inside this function is unnecessary and could cause confusion. The imports should be removed since they're already available in the module scope.
| from monitor_data.schemas.game_systems import ( | ||
| CoreMechanic, | ||
| AttributeDefinition, | ||
| SkillDefinition, | ||
| ResourceDefinition, | ||
| ) |
There was a problem hiding this comment.
These schema imports are redundant since the schemas are already imported at the top of the file (lines 84-93). The imports should be removed to avoid duplication.
| from monitor_data.schemas.game_systems import RuleOverrideScope | ||
|
|
There was a problem hiding this comment.
The RuleOverrideScope import is redundant since it's already imported at the top of the file (lines 84-93). Remove this import statement to avoid duplication.
| from monitor_data.schemas.game_systems import RuleOverrideScope |
| # Get all matching overrides (no pagination for now) | ||
| overrides_docs = overrides_collection.find(query).sort("created_at", -1) | ||
|
|
||
| from monitor_data.schemas.game_systems import RuleOverrideScope |
There was a problem hiding this comment.
The RuleOverrideScope import is redundant since it's already imported at the top of the file (lines 84-93). Remove this import statement to avoid duplication.
| with open(seed_file, "r") as f: | ||
| return json.load(f) |
There was a problem hiding this comment.
The function lacks error handling for file I/O operations. If the seed file is missing, corrupted, or contains invalid JSON, the function will raise an unhandled exception. Consider adding try-except blocks to provide clearer error messages for file operations.
| with open(seed_file, "r") as f: | |
| return json.load(f) | |
| try: | |
| with open(seed_file, "r", encoding="utf-8") as f: | |
| return json.load(f) | |
| except FileNotFoundError as exc: | |
| raise RuntimeError( | |
| f"Builtin game systems seed file not found: {seed_file}" | |
| ) from exc | |
| except json.JSONDecodeError as exc: | |
| raise RuntimeError( | |
| f"Builtin game systems seed file contains invalid JSON: {seed_file}" | |
| ) from exc | |
| except OSError as exc: | |
| raise RuntimeError( | |
| f"Error reading builtin game systems seed file: {seed_file}" | |
| ) from exc |
Addresses all review feedback from PR #108: 1. **Fix race condition in seeding** - Changed from count_documents check + insert_one to atomic upsert with $setOnInsert. Multiple processes can now safely seed simultaneously without creating duplicates. 2-5. **Remove redundant imports** - Removed 4 local imports of CoreMechanic, AttributeDefinition, SkillDefinition, ResourceDefinition, and RuleOverrideScope from within functions. Added them to module-level imports at line 85-89 instead. 6. **Add error handling for file I/O** - Added try-except blocks in _load_builtin_game_systems with specific error messages for FileNotFoundError, JSONDecodeError, and OSError. Test updates: - Updated test_builtin_systems_seeded to check update_one instead of insert_one - Renamed test_builtin_systems_not_reseeded to test_builtin_systems_use_atomic_upsert - New test verifies upsert=True and $setOnInsert usage All 23 tests passing ✅ 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com)
Addresses all review feedback from PR #108: 1. **Fix race condition in seeding** - Changed from count_documents check + insert_one to atomic upsert with $setOnInsert. Multiple processes can now safely seed simultaneously without creating duplicates. 2-5. **Remove redundant imports** - Removed 4 local imports of CoreMechanic, AttributeDefinition, SkillDefinition, ResourceDefinition, and RuleOverrideScope from within functions. Added them to module-level imports at line 85-89 instead. 6. **Add error handling for file I/O** - Added try-except blocks in _load_builtin_game_systems with specific error messages for FileNotFoundError, JSONDecodeError, and OSError. Test updates: - Updated test_builtin_systems_seeded to check update_one instead of insert_one - Renamed test_builtin_systems_not_reseeded to test_builtin_systems_use_atomic_upsert - New test verifies upsert=True and $setOnInsert usage All 23 tests passing ✅ 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com)
Resolved conflicts in: - packages/data-layer/src/monitor_data/schemas/__init__.py - packages/data-layer/src/monitor_data/tools/mongodb_tools.py Both branches added new imports and tools: - HEAD: game_systems schemas and tools - master: party_inventory, working_state schemas and tools Kept all additions from both branches.
| \nTests not detected in this PR. Please add/confirm coverage where applicable. |
Implements MongoDB CRUD for game system definitions and rule overrides. Pure data storage - rule execution logic lives in agents layer.
Game Systems (MongoDB)
New schemas (game_systems.py):
MongoDB tools (mongodb_tools.py):
Rule Overrides (MongoDB)
New schemas (game_systems.py):
MongoDB tools (mongodb_tools.py):
Built-in Systems Seed Data (builtin_systems.json)
Three pre-configured systems:
Protected from modification/deletion via is_builtin flag.
Tests
Added 23 comprehensive tests (test_game_system_tools.py):
Architecture
Implements: DL-20, supports RS-1 (rules engine)
🤖 Generated with Claude Code
Summary
Related Issues
Fixes #
Use Case(s)
Type of Change
Layer(s) Affected
packages/data-layer/)packages/agents/)packages/cli/)docs/)Checklist
Code Quality
Testing
Documentation
Authority Rules (if writing to Neo4j)
middleware/auth.pyTest Plan
Screenshots (if applicable)
Additional Notes