chore: update embedded client with large integer fix

[adnaan]

Summary

Updates the embedded client library to include the fix for preserving large integers (like UnixNano timestamps) as strings to prevent JavaScript precision loss.

Related

• Depends on: fix: preserve large integers as strings to prevent precision loss client#19

Test plan

• E2E tests in progressive-enhancement example pass

🤖 Generated with Claude Code

[Copilot]

Copilot wasn't able to review any files in this pull request.

[claude]

Pull Request Review - PR #83

Summary

This PR updates the embedded client library (testing/livetemplate-client.browser.js) to include the fix for large integer precision loss from livetemplate/client#19.

---

✅ Code Quality & Best Practices

APPROVED - The change follows best practices:

• Uses the standard Number.MAX_SAFE_INTEGER constant (2^53-1) for precision checking
• Maintains backward compatibility by only affecting integers that exceed safe limits
• Properly handles the edge case of UnixNano timestamps which commonly exceed JavaScript's safe integer range

---

✅ Functionality Review

APPROVED - The fix correctly addresses the precision loss issue:

The Problem

JavaScript's Number type uses IEEE 754 double-precision floating-point, which can only safely represent integers up to 2^53-1 (9,007,199,254,740,991). Go's time.Now().UnixNano() returns nanosecond timestamps that exceed this limit:

• Example timestamp: 1,769,358,878,696,557,000 (> MAX_SAFE_INTEGER)
• After precision loss: 1,769,358,878,696,557,056 (corrupted!)

The Solution

The updated parseValue() function now checks:

parseValue(e) {
  let t = parseFloat(e);
  return !isNaN(t) && e.trim() === t.toString() 
    ? Number.isInteger(t) && Math.abs(t) > Number.MAX_SAFE_INTEGER 
      ? e  // Keep as string if too large
      : t  // Convert to number if safe
    : e === "true" ? !0 
    : e === "false" ? !1 
    : e;
}

This ensures large integers are preserved as strings, allowing server-side GetStringOk() to handle them correctly.

---

✅ Potential Issues

NONE IDENTIFIED - The implementation is sound:

• No breaking changes to existing functionality
• Only affects edge case values (very large integers)
• Boolean parsing remains unchanged
• Small integer handling remains unchanged

---

⚠️ Test Coverage

ACCEPTABLE - According to the PR description, E2E tests in the progressive-enhancement example pass. However, I recommend:

1. Future enhancement: Add explicit test cases in this repository that verify:

   • Large integer values (> MAX_SAFE_INTEGER) are preserved as strings
   • Small integers continue to work as numbers
   • UnixNano timestamp handling specifically

2. Documentation: Consider adding a comment in the test utilities about this edge case for future maintainers.

---

✅ Performance Considerations

APPROVED - No performance concerns:

• The additional check (Math.abs(t) > Number.MAX_SAFE_INTEGER) is O(1)
• Only executes when value is numeric and an integer
• Negligible overhead for the vast majority of values

---

🔒 Security Concerns

NONE IDENTIFIED - The change is security-neutral:

• No new attack vectors introduced
• No changes to validation or sanitization logic
• Proper type checking maintained

---

📋 Final Recommendation

✅ APPROVE AND MERGE

This is a well-targeted fix for a real precision loss bug. The change:

• Solves the UnixNano timestamp corruption issue
• Maintains backward compatibility
• Has minimal performance impact
• Is properly tested (via E2E tests mentioned)

Minor Suggestions (Non-blocking):

1. Consider adding explicit test coverage for this edge case in the future
2. Document the MAX_SAFE_INTEGER limitation in any relevant developer guides

---

Great work on identifying and fixing this subtle but important bug! 🎉

[claude]

Pull Request Review: Update Embedded Client with Large Integer Fix

Summary

This PR updates the embedded browser client library (testing/livetemplate-client.browser.js) to include a fix for preserving large integers (like UnixNano timestamps) as strings to prevent JavaScript precision loss.

---

Strengths

1. Addresses Real JavaScript Limitation: JavaScripts Number.MAX_SAFE_INTEGER (2^53-1) means timestamps in nanoseconds (like UnixNano which can exceed this) lose precision. Preserving them as strings is the correct approach.

2. Proper Dependency Management: The PR correctly references the upstream dependency (fix: preserve large integers as strings to prevent precision loss client#19), showing good traceability.

3. Test Coverage Verified: The PR description indicates E2E tests in the progressive-enhancement example pass, which is appropriate for a client library update.

4. Appropriate Scope: This is a focused, single-file update to an embedded asset - exactly what a chore: commit should be.

---

Observations and Questions

1. Embedded File Update Process

• The changed file is an embedded minified JavaScript file (56KB, single line)
• Question: Is there a documented process for updating this embedded client?
• Suggestion: Consider adding a comment block indicating source version, build process, and verification method

2. Build Reproducibility

• Question: How can reviewers verify that this minified file matches the source from fix: preserve large integers as strings to prevent precision loss client#19?
• Recommendation: Consider adding a checksum or version tag that can be cross-referenced

3. Test Coverage
While E2E tests pass, consider:

• Are there specific tests that exercise large integer handling?
• Does the progressive-enhancement example specifically test UnixNano timestamp preservation?
• Suggestion: Add a test case comment or reference to document what specific scenario this fixes

4. Integration Point
The file is embedded into the testing package via go:embed directive. Used by:

• E2E tests for browser automation
• Testing framework for LiveTemplate apps

Question: Are there other places this client library is used or embedded that might also need updating?

---

Security Considerations

Low Risk:

• This is a testing asset, not production code shipped to end users
• The file is embedded at build time, not downloaded at runtime
• Changes are from a trusted upstream repository

Verification Needed:

• Since this is a minified file, manual review of the code is impractical
• Recommend: Establish a process to verify the integrity of embedded third-party code

---

Performance Considerations

• File size: 56,872 bytes (no significant change indicated)
• This is a testing dependency, so performance impact is minimal
• The fix itself (string handling for large integers) should have negligible performance impact

---

Code Quality and Best Practices

Follows Conventions:

• Commit message follows Conventional Commits format (chore:)
• PR template properly filled out with summary and test plan
• References upstream dependency properly

Documentation:

• Could benefit from more context about the update process
• Consider documenting in CONTRIBUTING.md how to update embedded client libraries

---

Testing

Adequate for this change:

• E2E tests pass
• The testing framework itself uses this file, so any breakage would be caught

Recommendations:

1. Document which specific E2E tests cover large integer handling
2. Consider adding a unit test that explicitly validates UnixNano timestamp preservation
3. Add a test that verifies large integers are not silently truncated

---

Recommendations for Approval

This PR can be approved with minor documentation improvements:

Before Merge (Nice to have):

1. Add source version/checksum comment to the embedded file
2. Document the update process for this file in CONTRIBUTING.md or a README

Future Improvements (Follow-up):

1. Automate client library updates with version checking
2. Add explicit test coverage for large integer scenarios
3. Consider adding a testing/README.md explaining the embedded assets

---

Verdict: APPROVE

This is a clean, focused update that addresses a real precision issue. The change is well-scoped, properly tested, and correctly tracks the upstream dependency. The suggestions above are primarily about improving maintainability and documentation, not blocking issues.

Reviewed by: Claude Code Assistant
Review Date: 2026-01-25

[claude]

PR Review: Update Embedded Client with Large Integer Fix

Summary

This PR updates the embedded browser client (testing/livetemplate-client.browser.js) to incorporate critical fixes from livetemplate/client#19 that prevent JavaScript precision loss when handling large integers like UnixNano timestamps.

---

✅ Code Quality & Best Practices

Strengths:

• Proper dependency update: The PR follows the correct workflow of updating the client library dependency after the upstream fix is merged
• Clear commit messages: All three commits follow conventional commit format and explain the "why" behind each change
• Incremental fixes: The three-commit approach shows iterative improvement based on code review feedback
• Good documentation: PR description clearly explains the problem, solution, and references the upstream PR

Observations:

• The file is a minified JavaScript bundle, so direct code review is limited
• The change is a binary substitution of the entire client library (4 additions, 4 deletions)

---

🐛 Potential Issues & Concerns

Critical Fix Validation:
The upstream PR fixes a critical bug where the `MAX_SAFE_INTEGER` check was unreachable due to precision loss during `parseFloat()`. The fix sequence was:

1. Initial fix: Added `MAX_SAFE_INTEGER` check
2. Review fix: Trimmed whitespace once, added unit tests
3. Logic fix: Moved range check BEFORE string equality comparison (critical!)

The final fix ensures that:
```javascript
// Before: Check failed because parseFloat() already lost precision
parseFloat("1769358878696557000") // → 1769358878696557056 (wrong!)

// After: Check range before conversion
Number.isInteger(n) && Math.abs(n) > Number.MAX_SAFE_INTEGER
? keepAsString // ✓ Correct
: convertToNumber
```

Recommendation: ✅ The logic is sound and addresses a real precision issue with UnixNano timestamps

---

⚡ Performance Considerations

No significant performance concerns:

• The added check (`Math.abs(n) > Number.MAX_SAFE_INTEGER`) is O(1) and executes only during value parsing
• The overhead is negligible compared to the WebSocket/HTTP communication cost
• Keeping large integers as strings prevents downstream issues that would be more expensive to debug

---

🔒 Security Concerns

No security issues identified:

• The change is defensive: preserving precision prevents data corruption
• No new attack surface introduced
• The `parseValue()` function already handles type coercion safely
• String preservation for large integers is the correct security posture (prevents silent data corruption)

---

🧪 Test Coverage

Upstream test coverage (from client#19):

• ✅ E2E tests in progressive-enhancement example pass
• ✅ Unit tests added for:
  • Values at/under `MAX_SAFE_INTEGER` → parse to numbers
  • Values over `MAX_SAFE_INTEGER` → stay as strings
  • Whitespace handling consistency
• ✅ Toggle and delete operations verified with large integer IDs

LVT-specific testing:
The PR description mentions E2E tests in the progressive-enhancement example pass, which is appropriate for an embedded library update.

---

📋 Suggestions & Recommendations

1. ✅ Approved: The fix is well-tested and addresses a real bug
2. Version tracking (optional): Consider documenting the client library version/commit hash in a comment for easier tracking
3. Future consideration: If this file is frequently updated, consider automating the update process

---

🎯 Overall Assessment

Verdict: ✅ LGTM (Looks Good To Me)

This is a high-quality dependency update that:

• Fixes a critical precision bug with large integers (UnixNano timestamps)
• Follows proper development workflow (upstream fix → downstream update)
• Has good test coverage
• Maintains backward compatibility
• Poses no security or performance risks

The three-commit iteration shows good engineering practice: initial fix → review improvements → logic correction.

Recommendation: Approve and merge once CI passes.

---

📚 References

• Upstream PR: fix: preserve large integers as strings to prevent precision loss client#19
• JavaScript `MAX_SAFE_INTEGER`: 2^53-1 = 9007199254740991
• Issue: UnixNano timestamps (~1.7e18) exceed safe integer range