Skip to content

fix: Align dependency checks with cascade delete behavior#52

Merged
spuentesp merged 2 commits into
dl-1-cleanfrom
copilot/sub-pr-47-one-more-time
Dec 28, 2025
Merged

fix: Align dependency checks with cascade delete behavior#52
spuentesp merged 2 commits into
dl-1-cleanfrom
copilot/sub-pr-47-one-more-time

Conversation

Copy link
Copy Markdown

Copilot AI commented Dec 28, 2025

Addresses three inconsistencies in universe deletion logic identified in code review:

Missing Stories in dependency validation

  • Dependency check now counts Stories before blocking deletion
  • Prevents silent cascade deletion of unchecked Stories when force=True
# Before: Stories deleted but not checked
dependency_query = """
  OPTIONAL MATCH (u)-[:HAS_SOURCE]->(s:Source)
  OPTIONAL MATCH (u)-[:HAS_AXIOM]->(a:Axiom)
  RETURN count(DISTINCT s) AS sources, count(DISTINCT a) AS axioms
"""

# After: Complete dependency accounting
dependency_query = """
  OPTIONAL MATCH (u)-[:HAS_SOURCE]->(s:Source)
  OPTIONAL MATCH (u)-[:HAS_AXIOM]->(a:Axiom)
  OPTIONAL MATCH (u)-[:HAS_STORY]->(st:Story)
  RETURN count(DISTINCT s) AS sources, 
         count(DISTINCT a) AS axioms,
         count(DISTINCT st) AS stories
"""

Query construction pattern inconsistency

  • Update query now uses explicit string concatenation instead of f-strings
  • Aligns with security pattern documented in neo4j_list_universes

Chained OPTIONAL MATCH dependency

  • Scene/PlotThread matches now anchor to universe node instead of story variable
  • Eliminates NULL propagation when no stories exist: (u)-[:HAS_STORY]->(:Story)-[:HAS_SCENE]->(scene:Scene)

💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

…uery security, and fix chained OPTIONAL MATCH

Co-authored-by: spuentesp <112034353+spuentesp@users.noreply.github.com>
Copilot AI changed the title [WIP] Implement data layer for managing multiverse and universes fix: Align dependency checks with cascade delete behavior Dec 28, 2025
Copilot AI requested a review from spuentesp December 28, 2025 04:00
@spuentesp spuentesp marked this pull request as ready for review December 28, 2025 04:11
Copilot AI review requested due to automatic review settings December 28, 2025 04:11
@spuentesp spuentesp merged commit 107ae66 into dl-1-clean Dec 28, 2025
4 checks passed
@github-actions github-actions Bot added area/data-layer Data layer changes type/tests Tests touched labels Dec 28, 2025
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes three inconsistencies in universe deletion logic to ensure dependency validation matches cascade deletion behavior. The changes prevent silent data loss by ensuring Stories are counted before deletion and improve query reliability by fixing NULL propagation issues.

Key Changes:

  • Added Stories to dependency validation checks to match cascade deletion behavior
  • Updated query construction pattern from f-strings to explicit concatenation for security consistency
  • Fixed Scene/PlotThread matching to anchor on universe node, eliminating NULL propagation when no stories exist

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
packages/data-layer/src/monitor_data/tools/neo4j_tools.py Added Story counting to dependency validation query, updated error message to include stories, changed update query to use string concatenation, and fixed Scene/PlotThread matching to start from universe node instead of story variable
packages/data-layer/tests/test_tools/test_universe_tools.py Updated test mock data to include stories: 0 and stories: 1 fields to match the new dependency check response structure

After a thorough review of the code changes, I found no issues to report. The implementation is correct, well-tested, and follows established patterns in the codebase:

  • The dependency validation now properly checks for Stories before blocking deletion
  • Test mocks correctly reflect the updated query response structure
  • The query construction pattern aligns with security best practices documented in neo4j_list_universes
  • The Scene/PlotThread matching fix properly addresses NULL propagation by anchoring to the universe node

All changes are consistent, maintainable, and address the issues identified in the PR description.


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

@spuentesp spuentesp deleted the copilot/sub-pr-47-one-more-time branch April 25, 2026 00:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/data-layer Data layer changes type/tests Tests touched

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants