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

Summary

Complete the graph-native hierarchy migration by:

1. Removing the obsolete "container" concept (replaced by root node detection)
2. Fixing all remaining parentId references to work with has_child graph edges
3. Removing overly-strict validation that breaks tests

This builds on the partial fix from #528 which addressed placeholder promotion.

Context from #528

Issue #528 discovered that PR #523 broke parent-child edge creation by removing parentId/containerNodeId from the HTTP API. The fix introduced:

1. Transient fields: _parentId and _containerId for backend edge creation
2. Dev-proxy updates: Re-added hierarchy parameters to call NodeOperations
3. Children cache: Updated when promoting placeholders

However, #528 only fixed one code path (placeholder promotion in base-node-viewer). Many other paths still need the same treatment.

Additionally, #528's strict parent-container validation exposed architectural problems:

• Tests create nodes with explicit container_node_id that doesn't match actual tree root
• Validation had to be commented out to unblock development
• Reveals that "container" concept is confusing and error-prone

Current Problems

Problem 1: Incomplete Parent Relationship Migration

Found: 128 references to parentId in frontend (excluding tests)

Broken code paths (need verification and fixes):

• ❓ Node splitting (creates sibling without parent edge?)
• ❓ Indent/outdent (changes parent but edge not updated?)
• ❓ Enter key (creates sibling/child - edge created?)
• ❓ Slash command nodes (created with parent edge?)
• ❓ Mention autocomplete creating referenced nodes
• ❓ Shared-node-store persistence (5+ createNode calls)
• ❓ Base-node split operations
• ❓ Hierarchy-service tree manipulations

Problem 2: Container Concept is Obsolete and Confusing

What "container" was:

• Denormalized container_node_id field pointing to tree root
• Used for scoping queries (WHERE container_node_id = $id)
• Optimization to avoid recursive parent traversal

Why it's obsolete in graph-native architecture:

• Root node detected by: !EXISTS(<-has_child<-node) (no parent edge)
• Tree scoping via graph traversal: ->has_child->node.{..}
• No denormalized field needed

Why it's confusing:

• Terminology overlap with "parent" (is container the parent? root? scope?)
• Can be set incorrectly (tests prove this - explicit container ≠ actual root)
• _containerId transient field duplicates _parentId in most cases
• MCP tool names use "container" (update_container_from_markdown)

Problem 3: Validation Inconsistencies

From #528 investigation:

// Overly strict validation (commented out in #528)
if let Some(ref parent_id_str) = params.parent_id {
    // get_container_id() traverses parent chain to root
    let actual_container = get_container_id(parent_id_str).await?;

    // But params.container_node_id might be explicitly set to something else!
    if actual_container != params.container_node_id {
        return Err(...); // Validation failure
    }
}

This validation is correct - it caught inconsistent test data. The real fix is to:

• Remove container_node_id parameter entirely
• Calculate container by traversing to root
• Never let callers specify it (source of bugs)

Proposed Solution

Part 1: Remove Container Concept

Backend changes:

// BEFORE
pub struct CreateNodeParams {
    pub parent_id: Option<String>,
    pub container_node_id: Option<String>,  // ❌ REMOVE
    // ...
}

// AFTER
pub struct CreateNodeParams {
    pub parent_id: Option<String>,  // Used to create has_child edge
    // container derived automatically by traversing parent chain
    // ...
}

Frontend changes:

// BEFORE (from #528 fix)
const promotedNode = {
    ...node,
    _parentId: parentId,
    _containerId: parentId  // ❌ REMOVE - redundant
};

// AFTER
const promotedNode = {
    ...node,
    _parentId: parentId  // Backend derives container from this
};

Graph queries replace container_id:

-- OLD: Get all nodes in container
SELECT * FROM nodes WHERE container_node_id = $id;

// NEW: Get all descendants of root
SELECT ->has_child->node.{..} FROM node:⟨$root_id⟩;

Part 2: Fix All Remaining Parent Relationships

Apply the #528 fix pattern to all node creation paths:

Audit template (for each createNode call):

Location: file.ts:123
Function: splitNodeAt()
Current: await createNode({ nodeType, content })
Category: A (needs _parentId)
Fix: await createNode({ nodeType, content, _parentId: currentParent })
Priority: HIGH

Files to fix (from #530):

• base-node.svelte - Split/Enter operations
• shared-node-store.ts - 5+ createNode calls (lines 625, 634, 816, 823, 1926)
• node-reference-service.ts - Mention creation (line 729)
• hierarchy-service.ts - Tree operations
• Any keyboard command handlers
• Any slash command implementations

Part 3: Update Backend to Auto-Derive Container

Remove container_node_id parameter:

impl NodeOperations {
    pub async fn create_node(&self, params: CreateNodeParams) -> Result<String> {
        // BEFORE: Trust params.container_node_id
        // AFTER: Derive container from parent chain

        let container_id = if let Some(parent) = &params.parent_id {
            self.get_root_of_tree(parent).await?  // Traverse to root
        } else {
            None  // This node IS the root
        };

        // Use derived container_id instead of params.container_node_id
    }

    async fn get_root_of_tree(&self, start_node: &str) -> Result<Option<String>> {
        // Traverse parent chain until no parent exists
        // Using has_child edges
        let result = self.store.query(r#"
            SELECT id FROM node:⟨$start⟩
            WHERE !EXISTS(<-has_child<-node)  -- This node has no parent (is root)
            UNION
            SELECT <-has_child<-node.{..} FROM node:⟨$start⟩
            WHERE !EXISTS(<-has_child<-node)  -- Find ancestor with no parent
            LIMIT 1;
        "#).await?;

        Ok(result)
    }
}

Part 4: Terminology Cleanup

Replace everywhere:

• "container node" → "root node"
• "container_id" → "root_id" or just use "parent" where appropriate
• get_container_id() → get_root_of_tree()
• update_container_from_markdown → update_tree_from_markdown

Implementation Plan

Step 1: Remove Container from Backend (3-4 hours)

Files:

• packages/core/src/models/node.rs - Remove container_node_id from structs
• packages/core/src/operations/mod.rs - Remove container param, add get_root_of_tree()
• packages/core/src/mcp/handlers/*.rs - Update MCP tool names
• packages/desktop-app/src-tauri/src/bin/dev-proxy.rs - Remove container from API

Changes:

• Remove container_node_id parameter from CreateNodeParams
• Add get_root_of_tree() helper
• Derive container by graph traversal
• Re-enable strict validation (now it will pass!)

Step 2: Remove Container from Frontend (2-3 hours)

Files:

• packages/desktop-app/src/lib/types/node.ts - Remove container references
• packages/desktop-app/src/lib/services/*.ts - Remove _containerId usage
• packages/desktop-app/src/lib/design/components/base-node-viewer.svelte - Simplify

Changes:

• Remove _containerId transient field (only use _parentId)
• Update function names (getContainer → getRoot)
• Simplify logic (one hierarchy field instead of two)

Step 3: Fix All Parent Relationships (4-6 hours)

Systematic audit:

1. Search for all createNode() calls
2. Categorize (needs _parentId vs already correct)
3. Add _parentId where needed
4. Verify edge creation

Template for fixes:

// BEFORE
await nodeService.createNode({ nodeType, content });

// AFTER
await nodeService.createNode({
    nodeType,
    content,
    _parentId: getCurrentParent()  // Add parent context
});

Step 4: Update All Tests (3-4 hours)

Rust tests:

• Remove explicit container_node_id parameters
• Let backend derive container from parent chain
• Verify validation now passes

TypeScript tests:

• Update test helpers to not use container
• Add tests for root node detection
• Verify parent-child edges in integration tests

Acceptance Criteria

Functional

• All node creation paths establish parent edges correctly
• Tree structure is correct (can query children)
• No orphan nodes created (except root nodes)
• Root nodes detected via graph queries (!EXISTS(<-has_child<-node))
• No container terminology in codebase
• Reactivity works (UI updates with hierarchy changes)

Technical

• container_node_id removed from all structs and parameters
• _containerId removed from frontend (only _parentId used)
• Backend derives container by graph traversal
• All 128+ parentId references audited and fixed
• Terminology updated (container → root)
• MCP tools renamed (with backward compat aliases if needed)

Testing

• All Rust tests pass (strict validation re-enabled)
• All TypeScript tests pass
• Manual testing: typing, tree operations, edges persist
• No regressions in existing functionality

Validation

• Parent-container consistency validation ENABLED (not commented out)
• Validation passes because container is derived correctly
• Clear error messages when validation fails

Related Issues

• Bug: Placeholder node loses focus immediately after typing, preventing text input #528: Placeholder typing fix (foundation for this work) - READ THE INVESTIGATION
• Remove parent_id/container_node_id from Node API - Complete Graph Migration #514: Removed parent_id fields (started the migration)
• SurrealDB Schema Refactor: Graph-Native Architecture #511: Graph-native architecture (root cause)
• Implement Atomic Transactional Node Operations #532: Atomic operations (can be implemented alongside)

Key Insights from #528

1. Root cause was backend, not just frontend cache
2. Transient fields pattern (_parentId) works for HTTP API
3. Dev-proxy must use NodeOperations (not NodeService directly)
4. Strict validation is good - it caught inconsistent test data
5. Container is redundant - always derivable from parent

Estimated Effort

• Backend cleanup (remove container): 3-4 hours
• Frontend cleanup (remove _containerId): 2-3 hours
• Fix remaining parentId paths: 4-6 hours
• Update all tests: 3-4 hours
• Testing and verification: 2-3 hours
• Total: 14-20 hours (2-3 days)

Priority

CRITICAL - Combines two urgent issues:

• Fix broken node creation (Audit and Fix: Complete Parent Relationship Migration for Graph-Native Architecture #530)
• Remove confusing container concept (Remove Container Concept - Use Root Node Detection Instead #531)

Doing them together is more efficient than separately since they touch the same code.

Implementation Notes

Don't Just Add _containerId Everywhere

The #528 fix used both _parentId and _containerId, but this perpetuates the confusion. Instead:

Correct approach:

// Frontend sends only parent
{ _parentId: "parent-123" }

// Backend derives root
let root_id = get_root_of_tree("parent-123");  // Traverses to find root

// Backend uses both for edge creation
RELATE node:⟨parent-123⟩->has_child->node:⟨new⟩;  // Parent edge (explicit)
// Root is derived, not stored as field

This eliminates the source of inconsistency that #528's validation caught.

Backward Compatibility for MCP

// MCP handlers - maintain old names temporarily
"update_container_from_markdown" => {
    tracing::warn!("'update_container_from_markdown' is deprecated, use 'update_tree_from_markdown'");
    handle_update_tree_from_markdown(ops, args).await
}
"update_tree_from_markdown" => handle_update_tree_from_markdown(ops, args).await,

Remove deprecated names in 1-2 releases after migration is stable.