fix: use deep merge for update operations to preserve statics by adnaan · Pull Request #20 · livetemplate/client

adnaan · 2026-02-01T06:02:56Z

Summary

Fixes a bug where the client was using shallow spread instead of deep merge when applying update operations to range items, causing statics to be lost.

The Bug

When the server sends a partial update to a range item field:

{"5": {"0": "new text"}}

The old item had statics cached:

{"5": {"s": ["<span>", "</span>"], "0": "old text"}}

Before (shallow spread):

currentItems[updateIndex] = { ...currentItems[updateIndex], ...changes };
// Result: {"5": {"0": "new text"}} ← Statics lost!

After (deep merge):

currentItems[updateIndex] = this.deepMergeTreeNodes(currentItems[updateIndex], changes, ...);
// Result: {"5": {"s": ["<span>", "</span>"], "0": "new text"}} ✓

Discovery

This bug was discovered by the fuzz testing framework in the livetemplate core repo. The TypeScript oracle (using this client code) diverged from expected behavior when conditional branches changed within range items.

Changes

state/tree-renderer.ts: Changed both applyDifferentialOpsToRange and applyDifferentialOpsToRangeMap to use deepMergeTreeNodes instead of shallow spread for update operations.
oracle-server.js: Added persistent Node.js server for the Go fuzz testing framework. This server applies diffs using the production client code, serving as the source of truth for diff correctness validation.

Test plan

Existing tests pass
Fuzz tests in livetemplate core pass with this fix
CI passes

🤖 Generated with Claude Code

The previous implementation only checked for range structures directly on a node using isRangeNode(). This missed cases where the range is nested inside another structure, such as: {{if .ShowList}} <ul> {{range .Items}}...{{end}} </ul> {{else}} List is hidden {{end}} When ShowList changes from true to false: - Position 0 changes from {"0": {range...}, "s": ["<ul>", "</ul>"]} - To {"s": ["List is hidden"]} The nested range at position 0.0 was being preserved during merge, causing orphaned data in the tree state. Fix: - Add hasRangeAnywhere() to recursively check for nested ranges - Add shouldFullReplace() to detect all structure transitions - Update deepMergeTreeNodes to use shouldFullReplace instead of just isRangeNode Tests: - Add 3 new test cases for nested range→non-range transitions - Test deeply nested ranges (3 levels deep) - Test transition back from else to nested range Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

When applying update operations to range items, the client was using shallow spread ({...old, ...changes}) which replaced entire sub-objects. This caused statics to be lost when the server sent partial updates. For example, when updating field "5" in an item: - Server sends: {"5": {"0": "new text"}} - Old item had: {"5": {"s": ["", ""], "0": "old text"}} - Shallow spread result: {"5": {"0": "new text"}} ← Lost statics! - Deep merge result: {"5": {"s": ["", ""], "0": "new text"}} ✓ This fix changes both applyDifferentialOpsToRange and applyDifferentialOpsToRangeMap to use deepMergeTreeNodes instead of shallow spread. Also adds oracle-server.js for the Go fuzz testing framework. This persistent Node.js server applies diffs using the production client code, serving as the source of truth for diff correctness validation. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

claude · 2026-02-01T06:03:52Z

Review

Critical Issue in oracle-server.js:73
The uncaughtException handler uses console.error() which writes to stderr. Since the protocol expects line-delimited JSON on stdout, this will break the protocol. Should be:

console.log(JSON.stringify({ html: "", tree: null, error: err.message }));

Code Quality - tree-renderer.ts
✅ Good work on hasRangeAnywhere() for detecting nested ranges
✅ Deep merge fix in differential ops prevents static loss
✅ shouldFullReplace() logic handles structure transitions correctly

Tests
✅ Excellent coverage of nested range edge cases

Overall strong improvement to range handling. Just fix the stderr issue in the oracle server.

Copilot

Pull request overview

This PR fixes a bug where shallow spread operations in range item updates caused cached statics to be lost, and adds a persistent Node.js oracle server for Go fuzz testing.

Changes:

Replaced shallow spread with deep merge in applyDifferentialOpsToRange and applyDifferentialOperations to preserve nested statics when updating range items
Added hasRangeAnywhere and shouldFullReplace helper functions to detect nested range structures and determine when full replacement vs. merging is needed
Added comprehensive tests for range-to-non-range transitions, including nested scenarios
Introduced oracle-server.js for persistent fuzz testing with line-delimited JSON protocol

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.

File	Description
state/tree-renderer.ts	Core bug fix: replaced shallow spread with deep merge for range item updates; added helper functions for nested range detection and structural transition logic
tests/tree-renderer.test.ts	Added tests for range-to-non-range transitions (direct and nested), though missing coverage for the specific update operation bug
oracle-server.js	New persistent Node.js server for Go fuzz testing framework with line-delimited JSON protocol

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

Copilot · 2026-02-01T06:07:31Z

oracle-server.js

+
+// Handle errors gracefully
+process.on("uncaughtException", (err) => {
+  console.error(JSON.stringify({ html: "", tree: null, error: err.message }));


The console.error call bypasses the line-delimited JSON protocol. According to the server's design (lines 8-11), all output should be on stdout as JSON responses. This error call writes directly to stderr without following the JSON format, which could interfere with the Go fuzz testing framework parsing. Consider changing this to write a JSON error response to stdout instead, similar to the error handling in the 'line' event handler (lines 57-63).

Suggested change

console.error(JSON.stringify({ html: "", tree: null, error: err.message }));

const output = {

html: "",

tree: null,

error: err instanceof Error ? err.message : String(err),

};

console.log(JSON.stringify(output));

Copilot · 2026-02-01T06:07:31Z

oracle-server.js

+
+// Handle errors gracefully
+process.on("uncaughtException", (err) => {
+  console.error(JSON.stringify({ html: "", tree: null, error: err.message }));


After handling an uncaught exception, the server continues running without exiting. This could lead to undefined behavior since the state after an uncaught exception is unpredictable. Consider adding process.exit(1) after line 74 to ensure the server exits cleanly. Alternatively, if the intention is to keep the server running, the error handling should be more robust to ensure the server state is still valid.

Suggested change

console.error(JSON.stringify({ html: "", tree: null, error: err.message }));

console.error(JSON.stringify({ html: "", tree: null, error: err.message }));

process.exit(1);

Copilot · 2026-02-01T06:07:32Z

tests/tree-renderer.test.ts

+  describe("render - NESTED range to non-range transition", () => {
+    /**
+     * This tests the case where a range is nested inside another structure:
+     *
+     * Template:
+     *   {{if .ShowList}}
+     *     <ul>
+     *     {{range .Items}}
+     *       <li id="{{.ID}}">{{.Text}}</li>
+     *     {{end}}
+     *     </ul>
+     *   {{else}}
+     *     <p>List is hidden</p>
+     *   {{end}}
+     *
+     * When ShowList goes from true to false:
+     * - Position 0 changes from {0: {range...}, s: ["<ul>", "</ul>"]}
+     * - To {s: ["<p>List is hidden</p>"]}
+     *
+     * The nested range at position 0.0 must be removed, not preserved.
+     */
+    it("should replace nested range structure with else clause", () => {
+      // Initial state: ShowList=true with items
+      // Structure: position 0 contains a UL wrapper with nested range at 0.0
+      const initialUpdate = {
+        s: ["<div>", "</div>"],
+        0: {
+          // The if-branch: UL wrapper containing the range
+          0: {
+            // The range (nested at position 0.0)
+            d: [
+              { 0: "item-1", 1: "Apple", _k: "item-1" },
+              { 0: "item-2", 1: "Banana", _k: "item-2" },
+            ],
+            s: ['<li id="', '">', "</li>"],
+          },
+          s: ["<ul>", "</ul>"], // UL wrapper statics
+        },
+      };
+      const initialResult = renderer.applyUpdate(initialUpdate);
+
+      expect(initialResult.html).toContain("<ul>");
+      expect(initialResult.html).toContain('<li id="item-1">Apple</li>');
+      expect(initialResult.html).toContain('<li id="item-2">Banana</li>');
+      expect(initialResult.html).toContain("</ul>");
+
+      // Update: ShowList=false, show else clause
+      // The nested range should be completely replaced
+      const elseUpdate = {
+        0: {
+          // The else-branch: just a paragraph
+          s: ["<p>List is hidden</p>"],
+        },
+      };
+      const elseResult = renderer.applyUpdate(elseUpdate);
+
+      // Verify the tree state doesn't contain orphaned range data
+      const treeState = renderer.getTreeState();
+      expect(treeState[0]).not.toHaveProperty("d");
+      expect(treeState[0][0]).toBeUndefined(); // Nested range should be gone
+
+      // Should NOT contain old items
+      expect(elseResult.html).not.toContain("Apple");
+      expect(elseResult.html).not.toContain("Banana");
+      expect(elseResult.html).not.toContain("<ul>");
+      expect(elseResult.html).not.toContain("<li");
+
+      // Should contain else content
+      expect(elseResult.html).toContain("<p>List is hidden</p>");
+    });
+
+    it("should handle transition back from else to nested range", () => {
+      // Start with else clause (ShowList=false)
+      const elseUpdate = {
+        s: ["<div>", "</div>"],
+        0: {
+          s: ["<p>List is hidden</p>"],
+        },
+      };
+      renderer.applyUpdate(elseUpdate);
+
+      // Transition to ShowList=true with items
+      const rangeUpdate = {
+        0: {
+          0: {
+            d: [
+              { 0: "item-1", 1: "New Item", _k: "item-1" },
+            ],
+            s: ['<li id="', '">', "</li>"],
+          },
+          s: ["<ul>", "</ul>"],
+        },
+      };
+      const rangeResult = renderer.applyUpdate(rangeUpdate);
+
+      // Should show the list now
+      expect(rangeResult.html).toContain("<ul>");
+      expect(rangeResult.html).toContain('<li id="item-1">New Item</li>');
+      expect(rangeResult.html).not.toContain("List is hidden");
+    });
+
+    it("should handle deeply nested range to non-range transition", () => {
+      // Initial: deeply nested structure with range at position 0.0.0
+      const initialUpdate = {
+        s: ["<main>", "</main>"],
+        0: {
+          s: ["<section>", "</section>"],
+          0: {
+            s: ["<div>", "</div>"],
+            0: {
+              // Range nested 3 levels deep
+              d: [
+                { 0: "deep-1", 1: "Deep Item 1", _k: "deep-1" },
+                { 0: "deep-2", 1: "Deep Item 2", _k: "deep-2" },
+              ],
+              s: ['<span id="', '">', "</span>"],
+            },
+          },
+        },
+      };
+      const initialResult = renderer.applyUpdate(initialUpdate);
+
+      expect(initialResult.html).toContain('<span id="deep-1">Deep Item 1</span>');
+      expect(initialResult.html).toContain('<span id="deep-2">Deep Item 2</span>');
+
+      // Replace the entire nested structure with simple content
+      const simpleUpdate = {
+        0: {
+          s: ["<p>No content</p>"],
+        },
+      };
+      const simpleResult = renderer.applyUpdate(simpleUpdate);
+
+      // All nested range data should be gone
+      expect(simpleResult.html).not.toContain("Deep Item");
+      expect(simpleResult.html).not.toContain("<span");
+      expect(simpleResult.html).toContain("<p>No content</p>");
+
+      // Verify tree state is clean
+      const treeState = renderer.getTreeState();
+      expect(treeState[0]).not.toHaveProperty("d");
+      expect(treeState[0][0]).toBeUndefined();
+    });
+  });


These tests validate the shouldFullReplace logic for range-to-non-range transitions, but they don't test the actual bug described in the PR summary. The PR describes a bug where UPDATE operations (the "u" differential operation) on range items lose statics when using shallow spread instead of deep merge. These tests should include a scenario that uses differential operations like ["u", "item-key", {"5": {"0": "new text"}}] to verify that statics are preserved when updating a nested field within a range item. Without this, the core fix in applyDifferentialOpsToRange and applyDifferentialOpsToRangeMap (lines 353-361 and 676-682 in state/tree-renderer.ts) remains untested.

Copilot · 2026-02-01T06:07:32Z

state/tree-renderer.ts

+            currentItems[updateIndex] = this.deepMergeTreeNodes(
+              currentItems[updateIndex],
+              changes,
+              `${statePath || ""}.item`


This use of variable 'statePath' always evaluates to true.

The oracle server communicates via line-delimited JSON on stdout. Using console.error() (stderr) for uncaught exceptions would break the protocol since the Go client only reads from stdout. Addresses review feedback on PR #20. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

claude · 2026-02-01T06:13:15Z

Review

This PR fixes a subtle but critical bug where shallow spread was losing statics during range item updates. The fix and approach look solid.

Key Issues Found

1. Potential infinite recursion in hasRangeAnywhere ⚠️

The function in state/tree-renderer.ts:56-79 only checks numeric keys but doesn't have depth limits or cycle detection. If a malformed tree has circular references, this could cause a stack overflow.

Consider adding a depth limit or visited set:

function hasRangeAnywhere(node: any, depth = 0, maxDepth = 10): boolean {
  if (depth > maxDepth || node == null || typeof node \!== "object" || Array.isArray(node)) {
    return false;
  }
  // ... rest of logic with depth + 1 in recursive call
}

2. Missing error handling in oracle-server.js ⚠️

The uncaughtException handler at line 73 logs the error but continues running. After an uncaught exception, Node.js process state may be corrupted. The server should exit gracefully:

process.on("uncaughtException", (err) => {
  console.log(JSON.stringify({ html: "", tree: null, error: err.message }));
  process.exit(1);  // Add this
});

Minor Observations

The deep merge changes at state/tree-renderer.ts:356-361 and state/tree-renderer.ts:676-682 correctly preserve statics and prevent data loss
shouldFullReplace logic is well-reasoned and handles nested ranges properly
Test coverage is comprehensive with good edge cases for nested structures

Verdict

The core fix is correct and solves the bug. The two issues above should be addressed before merging to prevent potential runtime issues in the fuzz testing environment.

- Add process.exit(1) after uncaughtException in oracle-server.js to prevent continued operation with potentially corrupted state - Add depth limit (50) to hasRangeAnywhere() as defensive measure against deeply nested or malformed tree structures Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

claude · 2026-02-01T15:56:24Z

Review Summary

Overall: Good changes that fix nested range replacement bugs and improve performance. Minor issues to address:

Critical Issues

oracle-server.js:74-76 - Uncaught exception handler exits without cleaning up readline interface. Should call rl.close() before process.exit(1).

Code Quality

tree-renderer.ts:68-70 - The shouldFullReplace function has overlapping conditions with hasRangeAnywhere. Consider simplifying: if hasRangeAnywhere(existing) is true and update has statics, that already implies structure change.
tree-renderer.ts:358-362 - Duplicate deep merge logic in two locations (lines 358 and 681). Consider extracting to a helper method like mergeRangeItem(item, changes, path) to reduce duplication.

Tests

The test coverage for nested range transitions is excellent and covers the edge cases well.

Security

No security issues found. The MAX_DEPTH limit (line 59) properly prevents stack overflow attacks.

Performance

The persistent oracle server is a solid optimization (300ms → 20-50ms per request).

- Close readline interface before process.exit(1) in oracle-server.js - Extract mergeRangeItem() helper to reduce duplication in update handlers Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

claude · 2026-02-01T16:05:22Z

Review Summary

Overall: Strong improvements to tree merge logic and test coverage. No security issues found.

Key Changes

New oracle server (oracle-server.js): Reduces request overhead from ~300ms to ~20-50ms by maintaining persistent process
Improved merge detection (tree-renderer.ts): Better handling of nested range→non-range transitions
Comprehensive tests: 3 new test cases covering edge cases

Issues Found

Critical - Potential infinite loop (tree-renderer.ts:60)

hasRangeAnywhere() recursion lacks base case for circular references
MAX_DEPTH=50 only protects against deep nesting, not cycles
Fix: Track visited nodes using WeakSet

Moderate - Error handling (oracle-server.js:73-77)

Uncaught exceptions exit after single error response, but stdin may have more requests queued
Consider: Keep server alive if error is recoverable

Minor - Code quality

tree-renderer.ts:233: Update comment to reflect new shouldFullReplace() logic (still says "range→non-range")
oracle-server.js:23: Consider exposing logger level via env var for debugging

Recommendations

Add WeakSet to prevent circular reference issues
Consider graceful degradation for oracle server errors
Update inline comments to match new function names

Good work on the test coverage and performance optimization!

adnaan and others added 2 commits January 29, 2026 20:52

Copilot AI review requested due to automatic review settings February 1, 2026 06:02

Copilot started reviewing on behalf of adnaan February 1, 2026 06:03 View session

Copilot AI reviewed Feb 1, 2026

View reviewed changes

fix: address review comments

04edb69

- Close readline interface before process.exit(1) in oracle-server.js - Extract mergeRangeItem() helper to reduce duplication in update handlers Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

adnaan merged commit a71c25d into main Feb 1, 2026
6 checks passed

adnaan deleted the fix/deep-merge-update-operations branch February 1, 2026 18:17

This was referenced Feb 2, 2026

chore: update livetemplate to v0.8.2 livetemplate/lvt#84

Merged

chore: update livetemplate to v0.8.2 livetemplate/examples#27

Merged

chore: update livetemplate to v0.8.2 livetemplate/tinkerdown#124

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: use deep merge for update operations to preserve statics#20

fix: use deep merge for update operations to preserve statics#20
adnaan merged 5 commits intomainfrom
fix/deep-merge-update-operations

adnaan commented Feb 1, 2026

Uh oh!

claude bot commented Feb 1, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Feb 1, 2026

Uh oh!

Copilot AI Feb 1, 2026

Uh oh!

Copilot AI Feb 1, 2026

Uh oh!

Copilot AI Feb 1, 2026

Uh oh!

claude bot commented Feb 1, 2026

Uh oh!

claude bot commented Feb 1, 2026

Uh oh!

claude bot commented Feb 1, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

	console.error(JSON.stringify({ html: "", tree: null, error: err.message }));
	console.error(JSON.stringify({ html: "", tree: null, error: err.message }));
	process.exit(1);

Conversation

adnaan commented Feb 1, 2026

Summary

The Bug

Discovery

Changes

Test plan

Uh oh!

claude bot commented Feb 1, 2026

Review

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Feb 1, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 1, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 1, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 1, 2026

Choose a reason for hiding this comment

Uh oh!

claude bot commented Feb 1, 2026

Review

Key Issues Found

Minor Observations

Verdict

Uh oh!

claude bot commented Feb 1, 2026

Review Summary

Critical Issues

Code Quality

Tests

Security

Performance

Uh oh!

claude bot commented Feb 1, 2026

Review Summary

Key Changes

Issues Found

Recommendations

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant