Replace internalContent prop sync effects with $derived

[malibio]

Closes #693

[malibio]

Code Review: PR #696 - Replace internalContent prop sync effects with $derived

Summary

This PR removes redundant $effect blocks from three node components (code-block-node.svelte, ordered-list-node.svelte, quote-block-node.svelte) that were unnecessarily syncing the content prop to internalContent state.

Review Decision: APPROVE (with clarification notes)

---

Findings

Clarification on Acceptance Criteria Deviation

The implementation deviates from the original issue requirements (#693), but for valid technical reasons:

Issue #693 proposed:

// Replace $effect with $derived
const internalContent = $derived(content);

Actual implementation:

// Remove the $effect entirely, keep $state initialization
let internalContent = $state(content);
// NO $effect and NO $derived

Why this deviation is correct:

1. Mutable state requirement: These components modify internalContent within handleContentChange handlers (e.g., injecting language codes, adding prefixes). Using $derived(content) would make internalContent read-only, breaking the mutation logic.

2. Event-driven data flow: The pattern is:

   • User edits -> handleContentChange updates internalContent
   • handleContentChange dispatches contentChanged event to parent
   • Parent updates content prop
   • Component remount handles external changes

3. The $effect was truly redundant: The sync $effect(() => { internalContent = content; }) served no purpose because:

   • Initial sync: Already handled by $state(content) initialization
   • User edits: Flow through the event handlers, not prop changes
   • External changes: Rare and handled by component remount

calendar.svelte Correctly Excluded

The commit message correctly notes that calendar.svelte was NOT modified. The $effect in that component is a necessary workaround for bits-ui's Svelte 5 binding limitations (two-way binding bridge pattern), which is a fundamentally different use case.

---

Code Quality Assessment

Category	Status
Architecture	Pass - Correctly identifies and removes dead code
Correctness	Pass - Maintains existing data flow patterns
Security	N/A - No security-relevant changes
Maintainability	Pass - Better comments explain the pattern
Testing	Pass - All 1469 tests pass
Performance	Pass - Removes unnecessary reactive overhead

---

Positive Aspects

1. Clear comments: The new comments explain WHY the pattern works:

   // Internal reactive state - initialized from content prop
   // Note: Content updates flow through handleContentChange event, not prop sync
   // External changes are rare (only from parent re-renders) and handled by component remount

2. Conservative approach: Rather than over-engineering with $derived, the simpler solution of just removing the redundant $effect is the right call.

3. Thoughtful exclusion: Recognizing that calendar.svelte has a different pattern (library binding workaround) shows good judgment.

---

Acceptance Criteria Reconciliation

Criterion	Status	Notes
calendar.svelte $effect replaced with $derived	N/A	Correctly excluded - necessary workaround
code-block-node.svelte $effect replaced	Different	Effect removed, not replaced with $derived
ordered-list-node.svelte $effect replaced	Different	Effect removed, not replaced with $derived
quote-block-node.svelte $effect replaced	Different	Effect removed, not replaced with $derived
4 $effect blocks eliminated	Partial	3 eliminated (calendar correctly kept)
All node types render correctly	Pass	Tests pass
Edit mode works correctly	Pass	Tests pass
Tests pass	Pass	1469/1469 passing
Quality checks pass	TBD	Recommend running bun run quality:fix

---

Recommendation

Update issue #693 to reflect that:

1. Only 3 components needed modification (calendar.svelte is a different pattern)
2. The solution is $effect removal, not $derived replacement (due to mutability requirements)

The implementation is technically sound and the deviation from the original proposal is well-justified.

---

Reviewed by: Principal Engineer AI Reviewer

[malibio]

Review Status: APPROVE

The implementation correctly removes redundant $effect blocks while preserving the necessary mutable state pattern. The deviation from the original $derived proposal is technically justified due to the mutation requirements in handleContentChange handlers. Good documentation in comments explaining the pattern.

All tests pass (1469/1469). Ready to merge.