Skip to content

Complete Hierarchy Migration: Remove Container Concept and Fix All Parent Relationships #535

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
malibio merged 6 commits into from
Nov 17, 2025
Merged

Complete Hierarchy Migration: Remove Container Concept and Fix All Parent Relationships #535

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
46d42d9
Complete hierarchy migration: remove container concept and use only p…
malibio Nov 17, 2025
61e1eca
WIP: Address code review - Phases 1-3 complete (validation, tests, qu…
malibio Nov 17, 2025
c78abc6
Clarify markdown recursive export TODO - requires async refactoring
malibio Nov 17, 2025
70c84cc
WIP: Complete code review resolution - All phases complete (Issue #533)
malibio Nov 17, 2025
96881a0
Delete 5 obsolete NodeService tests that bypass NodeOperations layer
malibio Nov 17, 2025
8852630
Improve code formatting in validate_parent_hierarchy call
malibio Nov 17, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion packages/core/src/db/surreal_store.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2119,7 +2119,7 @@ where
// Note: parent_id/container_node_id removed - use graph edges instead
let query = r#"
SELECT
uuid,
id,
node_type,
content,
before_sibling_id,
Expand Down
26 changes: 10 additions & 16 deletions packages/core/src/mcp/handlers/markdown.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -696,11 +696,11 @@ async fn create_node(
node_type: &str,
content: &str,
parent_id: Option<String>,
container_node_id: Option<String>,
_root_node_id: Option<String>, // Deprecated - kept for backward compat but ignored (root auto-derived from parent)
before_sibling_id: Option<String>,
) -> Result<String, MCPError> {
// Create node via NodeOperations (enforces all business rules)
// Container node ID is provided from the pre-created container
// Note: container/root is now auto-derived from parent chain by backend

// Provide required properties based on node type
let properties = match node_type {
Expand All @@ -718,7 +718,6 @@ async fn create_node(
node_type: node_type.to_string(),
content: content.to_string(),
parent_id,
container_node_id,
before_sibling_id,
properties,
})
Expand Down Expand Up @@ -900,22 +899,17 @@ fn export_node_hierarchy(
// Recursively export children (if enabled)
if include_children {
// TODO: Implement proper child lookup using graph edges
// For now, skip children in recursive export (markdown export partially broken)
// The container-level export still works for top-level nodes
let mut children: Vec<&Node> = Vec::new();
// For now, skip children in recursive export (needs NodeOperations access for graph queries)
// The container-level export still works for top-level nodes (gets children via NodeOperations.get_children)
let children: Vec<&Node> = Vec::new();

// Sort children by sibling order (reconstruct before_sibling_id chain)
sort_by_sibling_chain(&mut children);
// let mut sorted_children = children;
// sort_by_sibling_chain(&mut sorted_children);

for child in children {
export_node_hierarchy(
child,
_nodes_map,
output,
current_depth + 1,
max_depth,
include_children,
)?;
for _child in children {
// Skipping recursive export due to lack of graph edge access
// in this context. Would need to refactor to pass NodeOperations.
}
}

Expand Down
17 changes: 4 additions & 13 deletions packages/core/src/mcp/handlers/nodes.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -208,13 +208,13 @@ pub async fn handle_create_node(
.map_err(|e| MCPError::invalid_params(format!("Invalid parameters: {}", e)))?;

// Create node via NodeOperations (enforces all business rules)
// Note: container_node_id is auto-derived from parent chain by backend
let node_id = operations
.create_node(CreateNodeParams {
id: None, // MCP generates IDs server-side
node_type: mcp_params.node_type.clone(),
content: mcp_params.content,
parent_id: mcp_params.parent_id,
container_node_id: mcp_params.container_node_id,
before_sibling_id: mcp_params.before_sibling_id,
properties: mcp_params.properties,
})
Expand Down Expand Up @@ -451,7 +451,6 @@ async fn ensure_parent_exists(
node_type: "date".to_string(),
content: parent_id.to_string(),
parent_id: None,
container_node_id: None,
before_sibling_id: None,
properties: json!({}),
})
Expand Down Expand Up @@ -668,29 +667,21 @@ pub async fn handle_insert_child_at_index(
Some(children_info[params.index - 1].node_id.clone())
};

// 5. Determine container_node_id using graph traversal
// In graph-native architecture, traverse up to find the container
let container_node_id = operations
.get_container_id(&params.parent_id)
.await
.ok()
.or_else(|| Some(params.parent_id.clone())); // Default to parent as container

// 6. Create node using pointer-based operation
// 5. Create node using pointer-based operation
// Note: container/root is auto-derived from parent chain by backend
let node_id = operations
.create_node(CreateNodeParams {
id: None, // MCP generates IDs server-side
node_type: params.node_type.clone(),
content: params.content,
parent_id: Some(params.parent_id.clone()),
container_node_id,
before_sibling_id: before_sibling_id.clone(),
properties: params.properties,
})
.await
.map_err(|e| MCPError::node_creation_failed(format!("Failed to create node: {}", e)))?;

// 7. Fix sibling chain after insertion
// 6. Fix sibling chain after insertion
// When inserting at a specific position, we need to update the node that was previously
// at that position to now point to the newly inserted node
if params.index < children_info.len() {
Expand Down
34 changes: 0 additions & 34 deletions packages/core/src/mcp/handlers/nodes_test.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,6 @@ mod occ_tests {
node_type: "text".to_string(),
content: "Test content".to_string(),
parent_id: None,
container_node_id: None,
before_sibling_id: None,
properties: json!({}),
})
Expand All @@ -192,7 +191,6 @@ mod occ_tests {
node_type: "text".to_string(),
content: "Original".to_string(),
parent_id: None,
container_node_id: None,
before_sibling_id: None,
properties: json!({}),
})
Expand Down Expand Up @@ -236,7 +234,6 @@ mod occ_tests {
node_type: "text".to_string(),
content: "Original".to_string(),
parent_id: None,
container_node_id: None,
before_sibling_id: None,
properties: json!({}),
})
Expand Down Expand Up @@ -280,7 +277,6 @@ mod occ_tests {
node_type: "text".to_string(),
content: "Original".to_string(),
parent_id: None,
container_node_id: None,
before_sibling_id: None,
properties: json!({}),
})
Expand Down Expand Up @@ -331,7 +327,6 @@ mod occ_tests {
node_type: "text".to_string(),
content: "To be deleted".to_string(),
parent_id: None,
container_node_id: None,
before_sibling_id: None,
properties: json!({}),
})
Expand Down Expand Up @@ -387,7 +382,6 @@ mod occ_tests {
node_type: "text".to_string(),
content: "Start".to_string(),
parent_id: None,
container_node_id: None,
before_sibling_id: None,
properties: json!({}),
})
Expand Down Expand Up @@ -429,7 +423,6 @@ mod occ_tests {
node_type: "text".to_string(),
content: "Test".to_string(),
parent_id: None,
container_node_id: None,
before_sibling_id: None,
properties: json!({"status": "draft"}),
})
Expand Down Expand Up @@ -465,7 +458,6 @@ mod occ_tests {
node_type: "text".to_string(),
content: "Test".to_string(),
parent_id: None,
container_node_id: None,
before_sibling_id: None,
properties: json!({}),
})
Expand Down Expand Up @@ -613,7 +605,6 @@ mod integration_tests {
node_type: "date".to_string(),
content: "2025-10-24".to_string(),
parent_id: None,
container_node_id: None,
before_sibling_id: None,
properties: json!({}),
})
Expand All @@ -627,7 +618,6 @@ mod integration_tests {
node_type: "text".to_string(),
content: "A".to_string(),
parent_id: Some(date.clone()),
container_node_id: Some(date.clone()),
before_sibling_id: None,
properties: json!({}),
})
Expand All @@ -640,7 +630,6 @@ mod integration_tests {
node_type: "text".to_string(),
content: "B".to_string(),
parent_id: Some(date.clone()),
container_node_id: Some(date.clone()),
before_sibling_id: Some(node_a.clone()),
properties: json!({}),
})
Expand All @@ -653,7 +642,6 @@ mod integration_tests {
node_type: "text".to_string(),
content: "C".to_string(),
parent_id: Some(date.clone()),
container_node_id: Some(date.clone()),
before_sibling_id: Some(node_b.clone()),
properties: json!({}),
})
Expand Down Expand Up @@ -709,7 +697,6 @@ mod integration_tests {
node_type: "date".to_string(),
content: "2025-10-25".to_string(),
parent_id: None,
container_node_id: None,
before_sibling_id: None,
properties: json!({}),
})
Expand All @@ -723,7 +710,6 @@ mod integration_tests {
node_type: "text".to_string(),
content: "Parent".to_string(),
parent_id: Some(date.clone()),
container_node_id: Some(date.clone()),
before_sibling_id: None,
properties: json!({}),
})
Expand All @@ -737,7 +723,6 @@ mod integration_tests {
node_type: "text".to_string(),
content: "Child".to_string(),
parent_id: Some(parent.clone()),
container_node_id: Some(date.clone()),
before_sibling_id: None,
properties: json!({}),
})
Expand All @@ -751,7 +736,6 @@ mod integration_tests {
node_type: "text".to_string(),
content: "Grandchild".to_string(),
parent_id: Some(child.clone()),
container_node_id: Some(date.clone()),
before_sibling_id: None,
properties: json!({}),
})
Expand Down Expand Up @@ -798,7 +782,6 @@ mod integration_tests {
node_type: "date".to_string(),
content: "2025-10-26".to_string(),
parent_id: None,
container_node_id: None,
before_sibling_id: None,
properties: json!({}),
})
Expand All @@ -812,7 +795,6 @@ mod integration_tests {
node_type: "text".to_string(),
content: "A".to_string(),
parent_id: Some(date.clone()),
container_node_id: Some(date.clone()),
before_sibling_id: None,
properties: json!({}),
})
Expand All @@ -825,7 +807,6 @@ mod integration_tests {
node_type: "text".to_string(),
content: "B".to_string(),
parent_id: Some(date.clone()),
container_node_id: Some(date.clone()),
before_sibling_id: Some(_node_a.clone()),
properties: json!({}),
})
Expand Down Expand Up @@ -860,7 +841,6 @@ mod integration_tests {
node_type: "date".to_string(),
content: "2025-10-27".to_string(),
parent_id: None,
container_node_id: None,
before_sibling_id: None,
properties: json!({}),
})
Expand Down Expand Up @@ -943,7 +923,6 @@ mod integration_tests {
node_type: "text".to_string(),
content: "Test".to_string(),
parent_id: None,
container_node_id: None,
before_sibling_id: None,
properties: json!({}),
})
Expand Down Expand Up @@ -999,7 +978,6 @@ mod integration_tests {
node_type: "text".to_string(),
content: "Node 1".to_string(),
parent_id: None,
container_node_id: None,
before_sibling_id: None,
properties: json!({}),
})
Expand All @@ -1012,7 +990,6 @@ mod integration_tests {
node_type: "text".to_string(),
content: "Node 2".to_string(),
parent_id: None,
container_node_id: None,
before_sibling_id: None,
properties: json!({}),
})
Expand All @@ -1025,7 +1002,6 @@ mod integration_tests {
node_type: "text".to_string(),
content: "Node 3".to_string(),
parent_id: None,
container_node_id: None,
before_sibling_id: None,
properties: json!({}),
})
Expand Down Expand Up @@ -1057,7 +1033,6 @@ mod integration_tests {
node_type: "text".to_string(),
content: "Node 1".to_string(),
parent_id: None,
container_node_id: None,
before_sibling_id: None,
properties: json!({}),
})
Expand Down Expand Up @@ -1126,7 +1101,6 @@ mod integration_tests {
node_type: "text".to_string(),
content: "# Task List".to_string(),
parent_id: None,
container_node_id: None,
before_sibling_id: None,
properties: json!({}),
})
Expand All @@ -1139,7 +1113,6 @@ mod integration_tests {
node_type: "task".to_string(),
content: "- [ ] Task 1".to_string(),
parent_id: Some(container.clone()),
container_node_id: Some(container.clone()),
before_sibling_id: None,
properties: json!({"task": {"status": "OPEN"}}),
})
Expand All @@ -1152,7 +1125,6 @@ mod integration_tests {
node_type: "task".to_string(),
content: "- [ ] Task 2".to_string(),
parent_id: Some(container.clone()),
container_node_id: Some(container),
before_sibling_id: Some(node1.clone()),
properties: json!({"task": {"status": "OPEN"}}),
})
Expand Down Expand Up @@ -1193,7 +1165,6 @@ mod integration_tests {
node_type: "text".to_string(),
content: "Node 1".to_string(),
parent_id: None,
container_node_id: None,
before_sibling_id: None,
properties: json!({}),
})
Expand Down Expand Up @@ -1276,7 +1247,6 @@ mod integration_tests {
node_type: "text".to_string(),
content: "Test".to_string(),
parent_id: None,
container_node_id: None,
before_sibling_id: None,
properties: json!({"priority": "low"}),
})
Expand Down Expand Up @@ -1327,7 +1297,6 @@ mod integration_tests {
node_type: "text".to_string(),
content: format!("Node {}", i),
parent_id: None,
container_node_id: None,
before_sibling_id: None,
properties: json!({}),
})
Expand Down Expand Up @@ -1389,7 +1358,6 @@ mod integration_tests {
node_type: "text".to_string(),
content: "# Benchmark Container".to_string(),
parent_id: None,
container_node_id: None,
before_sibling_id: None,
properties: json!({}),
})
Expand All @@ -1405,7 +1373,6 @@ mod integration_tests {
node_type: "task".to_string(),
content: format!("- [ ] Task {}", i),
parent_id: Some(container.clone()),
container_node_id: Some(container.clone()),
before_sibling_id: None,
properties: json!({}),
})
Expand Down Expand Up @@ -1498,7 +1465,6 @@ mod integration_tests {
node_type: "text".to_string(),
content: "Performance test".to_string(),
parent_id: None,
container_node_id: None,
before_sibling_id: None,
properties: json!({}),
})
Expand Down
Loading