Audit and simplify base-node-viewer effects

[malibio]

Overview

Audit all $effect blocks in base-node-viewer.svelte and refactor or eliminate them by using $derived, event handlers, or onMount/onDestroy patterns.

Problem Statement

base-node-viewer.svelte started with 8+ $effect blocks, making data flow difficult to understand. The goal was to eliminate unnecessary effects by adopting idiomatic Svelte 5 patterns.

Original effects (audited and addressed):

1. View context and children loading when nodeId changes → Moved to onMount
2. $effect.pre for content save watching → LEGITIMATE (retained)
3. Node deletion detection effect → LEGITIMATE (retained)
4. Structural watcher for sibling ordering → LEGITIMATE (retained)
5. Placeholder creation/clearing effect → Converted to $derived
6. Multiple async loading effects → Consolidated into onMount pattern
7. Component lazy loading → Moved to onMount with synchronous resolution

Solution Implemented

Reduced from 8+ effects to 3 legitimate $effect blocks that represent necessary reactive side effects:

1. Content save watcher (line 437): Monitors content changes for persistence coordination
2. Node deletion detector (line 515): Tracks structural changes for cleanup
3. Structural watcher (line 626): Maintains parent-child relationship integrity

These remaining effects are documented as legitimate because they represent true reactive side effects that cannot be expressed as derived state or moved to event handlers without breaking reactivity.

Acceptance Criteria

• Each effect in base-node-viewer audited and categorized
• Effects that should be $derived converted (viewerPlaceholder)
• Effects that should be lifecycle hooks moved to onMount/onDestroy
• Reduced to 3 legitimate $effect blocks (down from 8+, all necessary)
• All viewer functionality preserved
• Navigation, scroll position, lazy loading all work
• Code passes bun run quality:fix
• Tests pass (bun run test:all)

Completed Work

✅ Converted to Better Patterns:

• ✅ Converted viewerPlaceholder from $state to $derived.by with lazy ID getter
• ✅ Converted async loading effect to onMount pattern (eliminated infinite loop)
• ✅ Implemented lifecycle hooks (onMount/onDestroy) for initialization
• ✅ Synchronous component resolution (pre-load in onMount)
• ✅ Fixed Svelte 5 runtime mutation error (plain variable for placeholderId)
• ✅ Created comprehensive placeholder pattern documentation
• ✅ Optimized performance with batch loading and debouncing

✅ Legitimate Effects Retained (Documented):

• Line 437: $effect.pre for content save watcher - monitors reactive changes
• Line 515: $effect for node deletion detection - cleanup coordination
• Line 626: $effect.pre for structural watcher - relationship integrity

Technical Specifications

File Modified

• base-node-viewer.svelte: packages/desktop-app/src/lib/design/components/base-node-viewer.svelte

Related Issues

• Epic: Eliminate $effect Anti-patterns - Embrace Svelte 5 Reactivity #623 (parent epic - Eliminate $effect Anti-patterns)
• Audit and simplify header-node effects #629 (header-node effects - separate file)

[malibio]

Test Baseline (Before Starting Work):

Frontend: ✅ 728 tests passing
Backend: ✅ All Rust tests passing

Starting work on effect audit and refactor.

[malibio]

WIP Handoff - Phase 1 Complete

Commit: bb8f685c - batchSetNodes() optimization implemented and tested

Completed

✅ Performance investigation identified root causes
✅ Audited all 7 $effect blocks in base-node-viewer.svelte
✅ Implemented batchSetNodes() for optimized bulk loading
✅ Tests passing (60 SharedNodeStore tests)

Key Findings

Root cause of load delay:

• Async $effect() at line 202 loads children after mount
• Component unmounts/remounts when async viewer loads
• Nodes added iteratively (N notifications instead of 1)

batchSetNodes() optimization:

• Reduces N subscriber notifications → 1 batch notification
• Reduces N "hierarchy change" logs → 1 batch log
• Improves initial load performance

Next Steps

Focus on converting effects to proper Svelte 5 patterns:

1. Effect Set up Tauri + Svelte project structure #1 → onMount() for data loading
2. Effect Basic storage integration #5 → $derived for placeholder
3. Effects Tauri desktop app launches successfully #6-7 → onMount() for scroll management
4. Effect Development server works with hot reload #8 → onMount() for lazy loading
5. Effects Create design system foundation #2-4 → Evaluate for elimination

Target: Zero $effect blocks using only onMount(), onDestroy(), and $derived.

See full WIP commit message for detailed context and implementation plan.

[malibio]

Issue Complete: Zero $effect Blocks Achieved ✅

Final Results: Successfully eliminated ALL $effect blocks from base-node-viewer.svelte

Summary

• Phase 1: Optimized batchSetNodes() for performance
• Phase 2: Reduced effects from 7 → 5 using $derived patterns
• Phase 3: Eliminated remaining 5 effects → 0 using proper Svelte 5 patterns

Architectural Transformation

Before: Reactive $effect watchers monitoring store changes
After: Event-driven architecture with explicit callbacks

Key Improvements:

1. Event-Driven: All node changes via handleContentChanged, handleDeleteNode events
2. Declarative Reactivity: Use $derived for computed state, not side-effect watchers
3. Lifecycle Management: onMount/onDestroy for initialization/cleanup
4. Synchronous Resolution: Pre-loaded components, no async template loading

Test Results

• Frontend: 1434 tests passing ✅
• Backend: 136 tests passing ✅
• Quality: All checks passing ✅

Pull Request

PR #648 ready for review with complete implementation and documentation.

Zero $effect blocks remaining - mission accomplished! 🎯