fix: preserve large integers as strings to prevent precision loss

[adnaan]

Summary

• Modified parseValue() to check Number.MAX_SAFE_INTEGER
• Large integers (like UnixNano timestamps) are kept as strings to preserve precision
• Fixes an issue where IDs generated from time.Now().UnixNano() lost precision when converted to JavaScript numbers

Problem

JavaScript's Number.MAX_SAFE_INTEGER is 2^53-1 (9007199254740991). UnixNano timestamps exceed this limit, causing precision loss when the client's parseValue() function converted them to numbers.

For example:

• Server stored: 1769358878696557000
• Client sent: 1769358878696557056 (different due to float64 precision loss!)

Solution

The parseValue() function now checks if an integer exceeds Number.MAX_SAFE_INTEGER and keeps it as a string, allowing the server-side GetStringOk() to handle it correctly.

Test plan

• E2E tests pass with large integer IDs
• Toggle and delete operations work correctly

🤖 Generated with Claude Code

[claude]

Review

Approved ✓

The fix correctly addresses JavaScript's floating-point precision limitation for large integers (>2^53-1).

Code Quality

• Logic is sound: checks if value is an integer and exceeds Number.MAX_SAFE_INTEGER
• Good use of Number.isInteger() to ensure only integers are affected
• Preserves backward compatibility for normal-sized numbers

Minor Observation

The condition value.trim() === num.toString() on line 520 may not catch edge cases where large integers already fail the equality check due to precision loss during parsing. However, the current implementation should work fine for the UnixNano timestamp use case since the MAX_SAFE_INTEGER check happens after confirming round-trip equality.

No blocking issues found. The fix is targeted and solves the precision loss problem effectively.

[Copilot]

Pull request overview

Updates the client-side value parsing to avoid precision loss when handling large integer strings (e.g., UnixNano IDs) by preserving them as strings instead of converting to JavaScript numbers.

Changes:

• Updated parseValue() to only coerce numeric strings to number when doing so won’t lose integer precision.
• Added a safe-integer-range guard (Number.MAX_SAFE_INTEGER) for integer parsing.

---

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

[Copilot]

When the numeric string is outside MAX_SAFE_INTEGER, this returns the original value (which may include leading/trailing whitespace). Since the guard condition uses value.trim(), a value like " 176935... " would now be preserved (including spaces) and sent to the server, which is inconsistent with the number-parsing path and can break ID matching. Consider trimming once (e.g., const trimmed = value.trim()) and returning the trimmed string for this branch.

Suggested change

	// Try to parse as number, but only if it's safe (won't lose precision)
	const num = parseFloat(value);
	if (!isNaN(num) && value.trim() === num.toString()) {
	// Check if the number is within JavaScript's safe integer range
	// Large integers (like UnixNano timestamps) lose precision as float64
	if (Number.isInteger(num) && Math.abs(num) > Number.MAX_SAFE_INTEGER) {
	// Keep as string to preserve precision
	return value;
	const trimmed = value.trim();
	// Try to parse as number, but only if it's safe (won't lose precision)
	const num = parseFloat(trimmed);
	if (!isNaN(num) && trimmed === num.toString()) {
	// Check if the number is within JavaScript's safe integer range
	// Large integers (like UnixNano timestamps) lose precision as float64
	if (Number.isInteger(num) && Math.abs(num) > Number.MAX_SAFE_INTEGER) {
	// Keep as string (trimmed) to preserve precision while matching numeric parsing
	return trimmed;

[Copilot]

This change introduces important new parsing behavior (integers > Number.MAX_SAFE_INTEGER remain strings) but there doesn’t appear to be a unit test covering it. Since Jest coverage is collected for livetemplate-client.ts, please add tests that assert: (1) values at/under MAX_SAFE_INTEGER parse to numbers, (2) values over it stay strings, and ideally (3) behavior with surrounding whitespace.

[claude]

Code Review

Overall: Good fix for the precision loss issue with large integers. The approach is sound and well-tested.

Issues Found

🔴 Critical Logic Bug (livetemplate-client.ts:526)
The condition trimmed === num.toString() will fail for large integers that you're trying to catch:

• For "1769358878696557000", parseFloat() converts to 1769358878696557056 (precision loss)
• Then 1769358878696557056 !== "1769358878696557000" is true
• So it returns the string early (line 539) and never reaches your MAX_SAFE_INTEGER check (line 526)

The MAX_SAFE_INTEGER check is unreachable for values that need it. The logic should check the range before comparing string equality.

Suggested fix:

const num = parseFloat(trimmed);
if (!isNaN(num)) {
  if (Number.isInteger(num) && Math.abs(num) > Number.MAX_SAFE_INTEGER) {
    return trimmed;
  }
  if (trimmed === num.toString()) {
    return num;
  }
}

Minor Issues

1. Line 532 comment: "to preserve precision while matching server expectations" - this is unclear about what server expectations are. Consider: "to preserve precision for server-side GetStringOk()"

2. Test file line 108: Comment says "Boolean parsing uses exact match" but the test expects " true " to remain as-is (with spaces). This contradicts line 117 where "true" becomes boolean. Consider clarifying or fixing the test expectation.

Positive Notes

• Comprehensive test coverage including edge cases
• Clear problem statement and solution approach
• Trimming optimization is good (only once)

[claude]

Review Summary

The fix correctly addresses precision loss for large integers. The logic is sound and well-tested.

Bug Found

Boolean parsing inconsistency (line 537-538): The boolean checks use the original value instead of trimmed, which is inconsistent with the numeric parsing path. This means strings like " true " won't parse as booleans even though they're trimmed for numeric checks.

Fix:

if (trimmed === "true") return true;
if (trimmed === "false") return false;

Code Quality

• ✅ Logic correctly identifies integers > MAX_SAFE_INTEGER and preserves as strings
• ✅ Comprehensive test coverage including edge cases
• ✅ Clear comments explaining the precision issue
• ✅ Proper use of Number.isInteger() and Math.abs()

The fix solves a real data corruption issue. Just fix the boolean trimming inconsistency and this is ready to merge.