DL-6: Comprehensive Story Outlines & Plot Threads#97
Merged
Conversation
github-actions
Bot
added
area/data-layer
| \nTests not detected in this PR. Please add/confirm coverage where applicable. |
There was a problem hiding this comment.
Pull request overview
This PR implements comprehensive narrative engine support for DL-6 (Manage Story Outlines & Plot Threads), introducing story beat progression tracking, mystery mechanics, foreshadowing/payoff patterns, and automatic pacing metrics. The implementation spans MongoDB for flexible story outline planning and Neo4j for canonical plot thread tracking with relationship management.
Key changes:
- Added comprehensive story outline schemas with beat progression, mystery structures, and pacing metrics stored in MongoDB
- Implemented plot thread management in Neo4j with 5 relationship types and status transition validation
- Introduced 9 new enums for narrative tracking including thread types, priorities, and clue visibility states
Reviewed changes
Copilot reviewed 4 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/data-layer/src/monitor_data/tools/neo4j_tools.py | Fixed Cypher WHERE clause by adding parentheses around OR condition for proper label filtering |
| packages/data-layer/src/monitor_data/tools/mongodb_tools.py | Enhanced beat manipulation operations with order preservation, comprehensive validation for reorder operations, and null-check for mystery structure |
| packages/data-layer/src/monitor_data/schemas/story_outlines.py | Updated datetime handling to use timezone-aware UTC timestamps and made description field optional with empty string default |
| packages/data-layer/src/monitor_data/schemas/base.py | Reordered ClueVisibility enum values alphabetically (hidden, discovered, revealed) |
| .gitignore | Added MCP plugin workspace directory and uv.lock to ignored files |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+1071
to
+1080
| if len(params.reorder_beats) != len(beats_by_id): | ||
| raise ValueError( | ||
| f"reorder_beats must include all {len(beats_by_id)} beat IDs. " | ||
| f"Got {len(params.reorder_beats)} IDs instead." | ||
| ) | ||
| reordered: list[StoryBeat] = [] | ||
| for beat_id in params.reorder_beats: | ||
| beat_id_str = str(beat_id) | ||
| if beat_id_str in beats_by_id: | ||
| beat = beats_by_id[beat_id_str] | ||
| beat.order = len(reordered) | ||
| reordered.append(beat) | ||
| if beat_id_str not in beats_by_id: | ||
| raise ValueError(f"Beat ID {beat_id} not found in current beats") |
There was a problem hiding this comment.
The validation logic for checking if all beat IDs are included in the reorder operation is missing test coverage. Consider adding test cases for:
- Attempting to reorder with fewer beat IDs than currently exist (should raise ValueError)
- Attempting to reorder with an invalid beat ID that doesn't exist in current beats (should raise ValueError)
These tests would ensure the new validation behaves correctly when users provide incomplete or incorrect reorder lists.
Comment on lines
+1103
to
+1106
| if mystery is None: | ||
| raise ValueError( | ||
| "Cannot mark clue as discovered: story outline has no mystery structure" | ||
| ) |
There was a problem hiding this comment.
The validation logic that checks if mystery structure is None when marking a clue as discovered is missing test coverage. Consider adding a test case that attempts to mark a clue as discovered when the story outline has mystery_structure field present in the document but set to None. This would verify that the ValueError is correctly raised in this edge case.
Addressed all 9 review comments from Copilot code review: ## Critical Fixes 1. **P1: Guard clue discovery** - Added None check for mystery_structure to prevent AttributeError when marking clues discovered on outlines without mystery structures 2. **Validate reorder_beats** - Added validation to ensure reorder_beats includes all beat IDs to prevent silent data loss 3. **Preserve beat order** - Fixed update_beats to preserve original order by iterating and replacing in place instead of using dictionary 4. **Fix deprecated datetime** - Replaced datetime.utcnow() with datetime.now(timezone.utc) in story_outlines.py for Python 3.12+ compatibility ## Code Improvements 5. **Clarify WHERE clause** - Added parentheses around OR condition in neo4j_get_plot_thread for clarity 6. **Make description optional** - Added default empty string to StoryBeat.description for flexibility during planning 7. **Reorder enum values** - Reordered ClueVisibility enum to follow logical progression: HIDDEN → DISCOVERED → REVEALED 8. **Document beat operations** - Added comprehensive documentation of beat operation order and potential conflicts in docstring All 194 tests passing ✅ 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Added tests for new validation logic introduced in review fixes: - test_update_story_outline_mark_clue_discovered_no_mystery_structure: Validates error when trying to mark clue discovered without mystery structure - test_update_story_outline_reorder_beats_incomplete: Validates error when reorder_beats doesn't include all beat IDs These tests cover the guard conditions added in the review fixes commit. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
spuentesp
force-pushed
the
feature/DL-6-story-outlines-plot-threads
branch
from
f23fa80 to
b4ba9d1
Compare
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.
Summary
Implements DL-6: Manage Story Outlines & Plot Threads with comprehensive narrative engine support including beat progression tracking, mystery mechanics, foreshadowing/payoff patterns, and automatic pacing metrics.
Story Outlines (MongoDB)
Schemas
MongoDB Tools
mongodb_create_story_outline: Create outline with automatic pacing calculationmongodb_get_story_outline: Retrieve outline with all beats/mystery structuremongodb_update_story_outline: Partial updates supporting beat manipulation, mystery updates, clue discoveryPlot Threads (Neo4j)
Schemas
Neo4j Tools
neo4j_create_plot_thread: Creates thread node + 5 relationship types (HAS_THREAD, ADVANCED_BY, INVOLVES, FORESHADOWS, REVEALS)neo4j_get_plot_thread: Retrieve with all relationship collectionsneo4j_update_plot_thread: Update with status transition validationneo4j_list_plot_threads: Filter by story_id, type, status, priority, entity_idBase Schemas
Added 9 new enums for narrative tracking:
PlotThreadType,PlotThreadStatus,BeatStatusStoryStructureType,ArcTemplateThreadPriority,ThreadUrgencyClueVisibility,PayoffStatusTests
All 194 tests passing ✅
Architecture Highlights
Use Cases Implemented
🤖 Generated with Claude Code