fix(tree-renderer): handle range→non-range transitions in deepMergeTreeNodes

[adnaan]

Summary

• Fix bug where transitioning from range structure to non-range (else clause) incorrectly preserved old range items
• When deepMergeTreeNodes merges an update, if existing node has d (range items) but update doesn't, the old d was preserved
• This caused the renderer to iterate old items with new statics, showing wrong content

Problem

When searching for non-existent terms in a list view:

No posts found matching "Post Title 1"
No posts found matching "Post Title 2"

Instead of:

No posts found matching "search query"

Root Cause

In deepMergeTreeNodes:

const merged: any = { ...existing };  // Copies OLD d array!

for (const [key, value] of Object.entries(update)) {
  // Only updates keys present in update
  // Since update has no 'd', old 'd' is preserved!
}

Fix

Detect range→non-range transition and replace instead of merge:

const existingIsRange = Array.isArray(existing.d) && Array.isArray(existing.s);
const updateIsRange = Array.isArray(update.d) && Array.isArray(update.s);

if (existingIsRange && !updateIsRange) {
  return update;  // Full replacement
}

Test plan

• Added unit tests for deepMergeTreeNodes range→non-range transitions
• All 193 existing tests pass
• Manual browser testing confirmed fix works

🤖 Generated with Claude Code

[claude]

Review

Looks good overall! The fix correctly handles the range→non-range transition bug. The root cause analysis is clear and the solution is appropriate.

Key strengths:

• Properly detects when a range structure is being replaced with non-range content
• Comprehensive test coverage for the edge case
• Avoids incorrect merging that preserved stale range items

Minor suggestions:

1. Type safety: Consider using a type guard function instead of inline checks:

   const isRangeNode = (node: any): boolean => 
     Array.isArray(node?.d) && Array.isArray(node?.s);

   This would make the code more maintainable and reusable.

2. Edge case: What happens if existing.d exists but existing.s is undefined/malformed? The current check might miss this. Consider whether stricter validation is needed.

3. Test naming: The test file could include a comment explaining what "range" vs "non-range" means in this context for future maintainers unfamiliar with the template system.

Recommendation: Approve with optional improvements above.

[Copilot]

Pull request overview

This PR fixes a bug in the tree renderer where transitioning from a range structure (list items) to a non-range structure (else clause) incorrectly preserved old range items, causing the renderer to display wrong content with outdated data.

Key Changes:

• Added range-to-non-range transition detection in deepMergeTreeNodes to replace instead of merge when appropriate
• Introduced comprehensive test suite covering state transitions and rendering behavior for range structures

Reviewed changes

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

File	Description
state/tree-renderer.ts	Implements range→non-range transition detection by checking if existing node has range structure (d and s arrays) while update doesn't, triggering full replacement instead of merge to prevent stale data preservation
tests/tree-renderer.test.ts	Adds comprehensive test coverage for range transitions including basic range-to-non-range replacement, range-to-range preservation, nested transitions, and HTML rendering validation

---

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

[claude]

Review ✅

Summary: Clean fix for the range→non-range transition bug. The implementation correctly detects structural mismatches and replaces instead of merging.

Strengths:

• Root cause correctly identified and fixed at the right layer (deepMerge)
• Comprehensive test coverage including nested scenarios
• Clear documentation explaining range vs non-range structures
• Debug logging for troubleshooting

Minor observations:

1. The isRangeNode() function could potentially use a type guard return type if you define a RangeNode interface later, but current implementation is fine
2. Consider if similar transitions exist elsewhere (e.g., non-range→range, though the current logic handles that via normal merge)

Security/Bugs: None identified. The null checks in isRangeNode() prevent runtime errors.

LGTM - ready to merge once tests pass.