Slow down spacetime diagram animation for readable km indicator

[Copilot]

The spacetime intervals diagram animation was too fast, causing the km indicator at the bottom right to appear as a blur.

Changes

• Increased LOOP_DURATION from 4s to 16s in minkowski.ts frame animation

const LOOP_DURATION = 16000; // 16 seconds total loop (slower for readable km indicator)

Screenshot

Original prompt

---

This section details on the original issue you should resolve

<issue_title>Spacetime diagram animates too quickly</issue_title>
<issue_description>The JavaScript tab spacetime intervals diagram animates too quickly, and the km indicator at the bottom right is just a blur. Animate the diagram more slowly so we can clearly see the km indicator changing, perhaps in increments of 1000km</issue_description>

Comments on the Issue (you are @copilot in this section)

• Fixes Spacetime diagram animates too quickly #46

---

✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

[claude]

Pull Request Review: Slow down spacetime diagram animation

Overview

This PR addresses issue #46 by slowing down the spacetime diagram animation to make the km indicator readable. The changes are focused and appropriate for the stated goal.

---

✅ Code Quality & Best Practices

Strengths:

• Focused scope: Changes are minimal and directly address the issue without unnecessary modifications
• Clear documentation: The updated constant includes an inline comment explaining the rationale
• Consistent formatting: Changes to formatSignificant calls maintain consistency across the codebase
• Proper handling of zero decimal places: The new logic in formatSignificant correctly returns just the integer part when significantDecimalPlaces === 0

Minor observations:

• The change from 4s to 16s (4x slower) is significant. Consider if this could feel too slow for some users. However, given the issue description requesting visible 1000km increments, this seems reasonable.
• Line 656 in minkowski.ts: The formatSignificant signature shows d.deltaXPrimeDecimal exists, which is good for precision preservation (per your Decimal.js architecture in CODE_REVIEW.md)

---

🐛 Potential Bugs or Issues

No critical bugs identified, but some considerations:

1. formatSignificant edge case handling: The new early return for significantDecimalPlaces === 0 (line 605-607 in relativity_lib.ts) is correct, but ensure it handles:

   • Negative numbers: ✅ Works correctly (returns parts[0] which includes the minus sign)
   • Very large numbers: ✅ The scientific notation check above handles this
   • The function properly returns parts[0] before checking decimal length (line 627)

2. Simultaneity.ts precision: Line 960 uses Math.round(Math.abs(deltaXPrime)).toLocaleString() instead of going through Decimal. This is acceptable for display formatting with thousands separators, but verify that deltaXPrime values won't exceed JavaScript's safe integer range (±2^53). For astronomical distances in km, this could theoretically be an issue (e.g., 1 light-year ≈ 9.46 × 10^12 km).

---

⚡ Performance Considerations

Positive impacts:

• The animation slows from 4s to 16s, which reduces CPU usage by decreasing the frame update frequency needed to appear smooth
• No additional computational overhead introduced
• The formatSignificant early return for zero decimal places slightly improves performance by avoiding unnecessary string processing

No performance concerns identified.

---

🔒 Security Concerns

No security issues identified.

The changes are purely presentational (animation timing and number formatting) with no:

• User input handling modifications
• External data fetching
• DOM manipulation changes
• Execution context changes

---

🧪 Test Coverage

Observations:

• No test files found in the repository (*.test.ts or *.spec.ts patterns return no results)
• No test script in package.json (only dev, build, type-check, lint, preview)

Recommendations:

1. Manual testing checklist (since no automated tests exist):

   • Verify km indicator is now readable during animation (not blurred)
   • Confirm values update in ~1000km increments as requested in issue Spacetime diagram animates too quickly #46
   • Check that zero decimal place formatting doesn't show ".0" or trailing decimals
   • Test with various velocity values to ensure formatting works across ranges
   • Verify the simultaneity diagram distance formatting with very large values

2. Future consideration: Add unit tests for formatSignificant given its critical role in display precision:

   // Example test cases
   formatSignificant(new Decimal("1234.5678"), "0", 0) // Should return "1234"
   formatSignificant(new Decimal("0.0009999"), "9", 2) // Should return "0.9999"
   formatSignificant(new Decimal("-1234567.89"), "0", 0) // Should return "-1234567"

---

📋 Specific Code Review Notes

relativity_lib.ts (lines 604-607, 627)

// If 0 decimal places requested, return just the integer part
if (significantDecimalPlaces === 0) {
    return parts[0];
}

✅ Good: Early return avoids unnecessary processing
✅ Correct: Properly handles the zero decimal case before the loop
⚠️ Minor: Line 627's conditional could be simplified given the early return, but current logic is still correct

minkowski.ts (line 845)

const LOOP_DURATION = 16000; // 16 seconds total loop (slower for readable km indicator)

✅ Good: Clear comment explaining the change
✅ Good: Named constant (not a magic number buried in code)

Multiple files: formatSignificant(..., "0", 1) → formatSignificant(..., "0", 0)

All changes from 1 decimal place to 0 decimal places are consistent and appropriate for showing whole kilometers.

---

✨ Recommendations

1. Consider adding user controls: Since animation speed is now a UX concern (some users might find 16s too slow), consider adding a speed control slider in a future PR. This could range from 4s to 32s.

2. Test the change: Please manually verify:

   • The km indicator is now readable (primary goal)
   • The 16s duration feels appropriate (not too slow)
   • Zero decimal formatting displays correctly without trailing ".0"

3. Documentation: Consider adding this timing constant to your code documentation as a tunable parameter for future UX adjustments.

4. Simultaneity precision: For the simultaneity.ts change (line 960), verify that distances displayed won't exceed JavaScript's integer precision limits for your expected use cases.

---

📊 Summary Score

Criteria	Rating	Notes
Code Quality	9/10	Clean, focused changes with good documentation
Bug Risk	9/10	Very low risk; only potential issue is large number handling in simultaneity
Performance	10/10	Actually improves performance by reducing animation frequency
Security	10/10	No security implications
Test Coverage	4/10	No automated tests (project-wide issue, not PR-specific)

Overall: Approve with minor testing recommendations

This is a well-executed fix for the reported issue. The changes are minimal, focused, and maintain the codebase's existing precision architecture. The main recommendation is manual testing to ensure the 16s duration provides the desired UX improvement.

---

Review completed per repository CODE_REVIEW.md standards - focusing on runtime safety, type safety, and precision preservation.