From a709b058b3e0935f0ca2d26ff0d89f73bdc9c695 Mon Sep 17 00:00:00 2001 From: Developer Date: Wed, 14 Oct 2026 14:45:41 -0400 Subject: [PATCH] fix(website): replay edge-case resilience and concurrent ride submission guards - Add replay regression test suite covering irregular/jittered timestamps, identical/zero dt, inverted timestamps, large gaps, pause/resume timing reconciliation, manual seek, speed changes, and large datasets (144k points). - Harden advancePlaybackIndex against non-finite/NaN budget and timestamp delta anomalies. - Reset lastTimeRef and cap frameDelta in WebTelemetryReplayModal to eliminate sudden jumps on speed switching or background tab resumes. - Guard group-ride creation with in-flight isSubmitting state and disabled submit button in CreateGroupRideModal; guarantee unique collision-free ride IDs across concurrent submissions. - Remove obsolete Vercel reference in website/.env.example in favor of Vultr VPS and Caddy; add npm test script. --- website/.env.example | 2 +- .../group-rides/group-rides-management.tsx | 23 +- .../components/web-telemetry-replay-modal.tsx | 4 +- website/lib/create-group-ride.ts | 10 +- website/lib/telemetry-track.ts | 7 +- website/package.json | 1 + website/tests/create-group-ride.test.ts | 32 +++ website/tests/group-rides-page.test.tsx | 37 +++ website/tests/telemetry-playback-sync.test.ts | 255 ++++++++++++++++++ 9 files changed, 359 insertions(+), 12 deletions(-) diff --git a/website/.env.example b/website/.env.example index 2ee1031..d621134 100644 --- a/website/.env.example +++ b/website/.env.example @@ -1,7 +1,7 @@ # MotorSignal Website Environment Configuration # # Copy this file to .env.local for local development, or configure these -# variables in your deployment environment (Vercel, Docker, VPS, etc.). +# variables in your deployment environment (Vultr VPS, Caddy, Docker, etc.). # # Production Firebase project: motoapp-d22e0 # Staging Firebase project: motorsignal-staging diff --git a/website/components/group-rides/group-rides-management.tsx b/website/components/group-rides/group-rides-management.tsx index f1394f1..50c4423 100644 --- a/website/components/group-rides/group-rides-management.tsx +++ b/website/components/group-rides/group-rides-management.tsx @@ -183,7 +183,7 @@ type MyRidesFilter = 'all' | 'organized' | 'joined'; interface CreateRideModalProps { isOpen: boolean; onClose: () => void; - onCreateRide: (ride: GroupRideSummary) => void; + onCreateRide: (ride: GroupRideSummary) => void | Promise; } function CreateGroupRideModal({ isOpen, onClose, onCreateRide }: CreateRideModalProps) { @@ -222,6 +222,7 @@ function CreateGroupRideModal({ isOpen, onClose, onCreateRide }: CreateRideModal const [participantLimit, setParticipantLimit] = useState(12); const [description, setDescription] = useState(''); const [errorMsg, setErrorMsg] = useState(null); + const [isSubmitting, setIsSubmitting] = useState(false); // Reset form when modal opens useEffect(() => { @@ -245,6 +246,7 @@ function CreateGroupRideModal({ isOpen, onClose, onCreateRide }: CreateRideModal setParticipantLimit(12); setDescription(''); setErrorMsg(null); + setIsSubmitting(false); } }, [isOpen, defaultDateValues]); @@ -288,8 +290,9 @@ function CreateGroupRideModal({ isOpen, onClose, onCreateRide }: CreateRideModal } }; - const handleSubmit = (e: React.FormEvent) => { + const handleSubmit = async (e: React.FormEvent) => { e.preventDefault(); + if (isSubmitting) return; if (!validate()) return; const newRide: GroupRideSummary & { @@ -321,8 +324,13 @@ function CreateGroupRideModal({ isOpen, onClose, onCreateRide }: CreateRideModal ...(rideType === 'club' && clubName.trim() ? { clubName: clubName.trim(), clubId: `club-${clubName.toLowerCase().replace(/\s+/g, '-')}` } : {}), }; - onCreateRide(newRide); - onClose(); + setIsSubmitting(true); + try { + await onCreateRide(newRide); + onClose(); + } finally { + setIsSubmitting(false); + } }; return ( @@ -708,9 +716,12 @@ function CreateGroupRideModal({ isOpen, onClose, onCreateRide }: CreateRideModal diff --git a/website/components/web-telemetry-replay-modal.tsx b/website/components/web-telemetry-replay-modal.tsx index a326c77..e736856 100644 --- a/website/components/web-telemetry-replay-modal.tsx +++ b/website/components/web-telemetry-replay-modal.tsx @@ -652,7 +652,8 @@ export function WebTelemetryReplayModal({ ride, onClose }: WebTelemetryReplayMod const animate = (timestamp: number) => { if (lastTimeRef.current == null) lastTimeRef.current = timestamp; - const frameDelta = timestamp - lastTimeRef.current; + const rawDelta = timestamp - lastTimeRef.current; + const frameDelta = Math.max(0, Math.min(rawDelta, 200)); lastTimeRef.current = timestamp; // Virtual recorded-time budget to consume this frame, at the selected speed. @@ -676,6 +677,7 @@ export function WebTelemetryReplayModal({ ride, onClose }: WebTelemetryReplayMod return () => { if (animationFrameRef.current) cancelAnimationFrame(animationFrameRef.current); + lastTimeRef.current = null; }; }, [isPlaying, playbackSpeed, totalPoints, points]); diff --git a/website/lib/create-group-ride.ts b/website/lib/create-group-ride.ts index f7879ef..aafc310 100644 --- a/website/lib/create-group-ride.ts +++ b/website/lib/create-group-ride.ts @@ -37,6 +37,12 @@ export interface CommitGroupRideDeps { * - unavailable: no Firestore handle * - commit-failed: the write was attempted but rejected (nothing persisted) */ +let rideIdCounter = 0; + +export function resetRideIdCounter(): void { + rideIdCounter = 0; +} + export async function commitGroupRide( newRide: WebGroupRideInput, deps: CommitGroupRideDeps @@ -46,7 +52,9 @@ export async function commitGroupRide( if (!deps.firestore) return { ok: false, reason: 'unavailable' }; const now = deps.now ?? new Date(); - const rideId = `ride_${now.getTime()}_${uid.substring(0, Math.min(6, uid.length))}`; + const seq = ++rideIdCounter; + const seqSuffix = seq > 1 ? `${seq}` : ''; + const rideId = `ride_${now.getTime()}${seqSuffix}_${uid.substring(0, Math.min(6, uid.length))}`; const payload = buildWebGroupRidePayload(newRide, { rideId, diff --git a/website/lib/telemetry-track.ts b/website/lib/telemetry-track.ts index aff3c4b..77f7baa 100644 --- a/website/lib/telemetry-track.ts +++ b/website/lib/telemetry-track.ts @@ -63,12 +63,13 @@ export function advancePlaybackIndex( ): { index: number; remainingMs: number } { if (points.length === 0) return { index: 0, remainingMs: 0 }; let i = Math.max(0, Math.min(fromIndex, points.length - 1)); - let budget = Math.max(0, budgetMs); + let budget = Number.isFinite(budgetMs) ? Math.max(0, budgetMs) : 0; while (i < points.length - 1) { const connected = areTelemetryPointsConnected(points[i], points[i + 1]); - const dt = connected - ? Math.max(0, Date.parse(points[i + 1].timestamp) - Date.parse(points[i].timestamp)) + const rawDt = connected + ? Date.parse(points[i + 1].timestamp) - Date.parse(points[i].timestamp) : 50; + const dt = Number.isFinite(rawDt) ? Math.max(0, rawDt) : 50; if (budget >= dt) { budget -= dt; i++; diff --git a/website/package.json b/website/package.json index c2d233a..e052b22 100644 --- a/website/package.json +++ b/website/package.json @@ -7,6 +7,7 @@ "build": "next build", "start": "next start", "lint": "next lint", + "test": "vitest run", "test:components": "vitest run" }, "dependencies": { diff --git a/website/tests/create-group-ride.test.ts b/website/tests/create-group-ride.test.ts index 979be66..b6cdac5 100644 --- a/website/tests/create-group-ride.test.ts +++ b/website/tests/create-group-ride.test.ts @@ -81,4 +81,36 @@ describe('commitGroupRide (WEB Phase 1 — persisted ride creation)', () => { const res = await commitGroupRide(baseRide, deps); expect(res).toEqual({ ok: false, reason: 'commit-failed', error: boom }); }); + + it('guarantees unique ride IDs across concurrent submissions at the same millisecond', async () => { + const sharedNow = new Date('2026-10-14T12:00:00.000Z'); + const { deps: deps1 } = makeDeps({ now: sharedNow }); + const { deps: deps2 } = makeDeps({ now: sharedNow }); + + const [res1, res2] = await Promise.all([ + commitGroupRide(baseRide, deps1), + commitGroupRide(baseRide, deps2), + ]); + + expect(res1.ok).toBe(true); + expect(res2.ok).toBe(true); + if (res1.ok && res2.ok) { + expect(res1.rideId).not.toBe(res2.rideId); + expect(res1.rideId).toMatch(/^ride_\d+_rider-/); + expect(res2.rideId).toMatch(/^ride_\d+_rider-/); + } + }); + + it('handles permission-denied or rejected commit cleanly without throwing', async () => { + const permDenied = new Error('permission-denied: caller lacks organizer permission'); + const { deps } = makeDeps({ + writeBatch: () => ({ set: vi.fn(), commit: vi.fn().mockRejectedValue(permDenied) }), + }); + const res = await commitGroupRide(baseRide, deps); + expect(res.ok).toBe(false); + if (!res.ok) { + expect(res.reason).toBe('commit-failed'); + expect((res as any).error).toBe(permDenied); + } + }); }); diff --git a/website/tests/group-rides-page.test.tsx b/website/tests/group-rides-page.test.tsx index 6ba311d..d391d7d 100644 --- a/website/tests/group-rides-page.test.tsx +++ b/website/tests/group-rides-page.test.tsx @@ -489,6 +489,43 @@ describe('Group Rides Interactive Page & Components', () => { await screen.findByText(/Could not create the ride/i); expect(screen.queryByRole('heading', { name: /Doomed Ride/i })).toBeNull(); }); + + it('enforces commit-before-UI: never inserts into UI state while commit is in-flight', async () => { + let resolveCommit!: () => void; + const pendingCommit = new Promise((res) => { + resolveCommit = res; + }); + fb.commitImpl.mockReset(); + fb.commitImpl.mockImplementation(() => pendingCommit); + + render(); + + fireEvent.click(screen.getByRole('button', { name: /create group ride/i })); + fireEvent.change(screen.getByLabelText(/ride title/i), { target: { value: 'In Flight Ride' } }); + fireEvent.change(screen.getByLabelText(/region \/ city/i), { target: { value: 'Highway 1' } }); + + const publishBtn = screen.getByRole('button', { name: /publish group ride/i }); + fireEvent.click(publishBtn); + + // 1. While in-flight, button disables and shows publishing indicator + await waitFor(() => { + expect(screen.getByText(/Publishing…/i)).toBeDefined(); + }); + // 2. The ride MUST NOT exist in UI state before commit confirms + expect(screen.queryByRole('heading', { name: /In Flight Ride/i })).toBeNull(); + + // 3. Duplicate clicks while in flight are ignored + const inFlightBtn = screen.getByText(/Publishing…/i); + fireEvent.click(inFlightBtn); + expect(fb.commitImpl).toHaveBeenCalledTimes(1); + + // 4. Resolve the commit + resolveCommit(); + + // 5. Ride now appears in UI state strictly after commit confirmation + await screen.findByRole('heading', { name: /In Flight Ride/i }); + expect(fb.commitImpl).toHaveBeenCalledTimes(1); + }); }); describe('Full GroupRidesPage Smart Routing', () => { diff --git a/website/tests/telemetry-playback-sync.test.ts b/website/tests/telemetry-playback-sync.test.ts index dc19adf..effeef2 100644 --- a/website/tests/telemetry-playback-sync.test.ts +++ b/website/tests/telemetry-playback-sync.test.ts @@ -67,3 +67,258 @@ describe('advancePlaybackIndex (replay playback synchronization)', () => { expect(advancePlaybackIndex([], 0, 1000)).toEqual({ index: 0, remainingMs: 0 }); }); }); + +describe('replay regression tests: irregular timestamps', () => { + it('correctly tracks jittered / non-uniform sample intervals', () => { + // Non-uniform offsets: 0, 30, 80, 110, 160 ms + const jittered = [mk(0), mk(30), mk(80), mk(110), mk(160)]; + // 25 ms: not enough for step 0->1 (needs 30) + let r = advancePlaybackIndex(jittered, 0, 25); + expect(r.index).toBe(0); + expect(r.remainingMs).toBe(25); + + // Add another 15 ms (total 40 ms): crosses step 0->1 (30 ms), leaves 10 ms (step 1->2 needs 50 ms) + r = advancePlaybackIndex(jittered, r.index, 15 + r.remainingMs); + expect(r.index).toBe(1); + expect(r.remainingMs).toBe(10); + + // Add 60 ms (total 70 ms): crosses step 1->2 (50 ms, leaves 20 ms). Step 2->3 needs 30 ms (20 < 30). + r = advancePlaybackIndex(jittered, r.index, 60 + r.remainingMs); + expect(r.index).toBe(2); + expect(r.remainingMs).toBe(20); + }); + + it('steps through identical timestamps (dt === 0) without consuming extraneous budget', () => { + // Samples 1 and 2 have the exact same timestamp + const dups = [mk(0), mk(50), mk(50), mk(100)]; + // Budget 50 ms from index 0: covers 0->1 (50 ms) and 1->2 (0 ms) + const r = advancePlaybackIndex(dups, 0, 50); + expect(r.index).toBe(2); + expect(r.remainingMs).toBe(0); + }); + + it('treats inverted / backwards timestamps as disconnected (50 ms step) instead of looping or stalling', () => { + // Sample 2 is out of order (timestamp goes backwards) + const backwards = [mk(0), mk(50), mk(30), mk(100)]; + // From index 1 to 2, delta is -20 ms (disconnected). Should step across with 50 ms step. + const r = advancePlaybackIndex(backwards, 1, 50); + expect(r.index).toBe(2); + expect(r.remainingMs).toBe(0); + }); + + it('handles unparseable or invalid timestamps gracefully without throwing', () => { + const invalidPts: TelemetryPoint[] = [ + mk(0), + { ...mk(50), timestamp: 'invalid-timestamp-string' }, + mk(100), + ]; + const r = advancePlaybackIndex(invalidPts, 0, 100); + expect(r.index).toBe(2); + }); + + it('safely handles non-finite or NaN budget without freezing', () => { + const uniform = [mk(0), mk(50), mk(100)]; + const rNaN = advancePlaybackIndex(uniform, 0, NaN); + expect(rNaN.index).toBe(0); + expect(rNaN.remainingMs).toBe(0); + + const rNeg = advancePlaybackIndex(uniform, 0, -500); + expect(rNeg.index).toBe(0); + expect(rNeg.remainingMs).toBe(0); + }); +}); + +describe('replay regression tests: large gaps & segment breaks', () => { + it('compresses a multi-minute gap (>5s) to a single 50 ms step', () => { + // 15-minute gap + const gapped = [mk(0), mk(50), mk(15 * 60 * 1000)]; + const r = advancePlaybackIndex(gapped, 1, 50); + expect(r.index).toBe(2); + expect(r.remainingMs).toBe(0); + }); + + it('compresses explicit segment boundary transitions to 50 ms', () => { + const segments = [mk(0, 0), mk(50, 0), mk(60, 1), mk(110, 1)]; + // From index 1 to 2, segmentId changes from 0 to 1 + const r = advancePlaybackIndex(segments, 1, 50); + expect(r.index).toBe(2); + expect(r.remainingMs).toBe(0); + }); + + it('handles multiple consecutive disconnected gaps smoothly', () => { + // 3 successive gaps of 10 seconds each + const multiGapped = [mk(0), mk(10_000), mk(20_000), mk(30_000)]; + // Each gap is compressed to 50 ms. Total 150 ms crosses all 3 gaps. + const r = advancePlaybackIndex(multiGapped, 0, 150); + expect(r.index).toBe(3); + expect(r.remainingMs).toBe(0); + }); + + it('compresses a 24-hour pause in ride without stalling', () => { + const dayGap = [mk(0), mk(50), mk(24 * 60 * 60 * 1000)]; + const r = advancePlaybackIndex(dayGap, 1, 55); + expect(r.index).toBe(2); + expect(r.remainingMs).toBe(0); + }); +}); + +describe('replay regression tests: pause and resume playback simulation', () => { + const points = Array.from({ length: 20 }, (_, i) => mk(i * 50)); + + it('simulates play -> pause -> resume without a timing jump', () => { + let currentIndex = 0; + let carry = 0; + let lastTime: number | null = null; + + // Helper simulating an animation frame at 60 fps (16.67 ms frame delta) + const tick = (now: number, isPlaying: boolean, speed = 1.0) => { + if (!isPlaying) { + lastTime = null; + carry = 0; + return; + } + if (lastTime == null) lastTime = now; + const rawDelta = now - lastTime; + const frameDelta = Math.max(0, Math.min(rawDelta, 200)); + lastTime = now; + const budget = frameDelta * speed + carry; + const res = advancePlaybackIndex(points, currentIndex, budget); + currentIndex = res.index; + carry = currentIndex >= points.length - 1 ? 0 : res.remainingMs; + }; + + // 1. Play for 3 frames (16.67 ms each, ~50 ms total) + tick(1000, true); // frame 1: delta 0, index 0, carry 0 + expect(currentIndex).toBe(0); + tick(1016.67, true); // frame 2: delta 16.67, index 0, carry 16.67 + expect(currentIndex).toBe(0); + expect(carry).toBeCloseTo(16.67, 1); + tick(1050, true); // frame 3: delta 33.33 + carry 16.67 = 50 ms -> index 1! + expect(currentIndex).toBe(1); + expect(carry).toBe(0); + + // 2. Pause for 5 seconds (5000 ms wall clock) + tick(1060, false); // pause triggered + expect(lastTime).toBeNull(); + expect(carry).toBe(0); + + // 3. Resume 5 seconds later at t=6050 + tick(6050, true); // first resume frame: lastTime initialized to 6050, delta = 0! + expect(currentIndex).toBe(1); // DID NOT JUMP + expect(carry).toBe(0); + + // 4. Continue playing after resume + tick(6100, true); // 50 ms delta -> index 2! + expect(currentIndex).toBe(2); + expect(carry).toBe(0); + }); +}); + +describe('replay regression tests: manual seek', () => { + const points = Array.from({ length: 50 }, (_, i) => mk(i * 50)); + + it('seeks forward and clears carryover', () => { + let index = 5; + let carry = 25; + // Manual seek to 20 + index = 20; + carry = 0; + const r = advancePlaybackIndex(points, index, 50 + carry); + expect(r.index).toBe(21); + expect(r.remainingMs).toBe(0); + }); + + it('seeks backward and clears carryover', () => { + let index = 30; + let carry = 40; + // Manual seek back to 10 + index = 10; + carry = 0; + const r = advancePlaybackIndex(points, index, 100 + carry); + expect(r.index).toBe(12); + }); + + it('clamps out-of-bounds seek indices safely', () => { + const rNegative = advancePlaybackIndex(points, -100, 100); + expect(rNegative.index).toBe(2); // starts from 0, advances 2 steps + + const rBeyond = advancePlaybackIndex(points, 9999, 100); + expect(rBeyond.index).toBe(49); // clamped to length - 1 + expect(rBeyond.remainingMs).toBe(0); + }); +}); + +describe('replay regression tests: speed changes', () => { + const points = Array.from({ length: 100 }, (_, i) => mk(i * 50)); + + it('scales budget consumption proportionally to speed', () => { + // At 0.5x speed: 100 ms real time = 50 ms virtual budget = 1 step + const rHalf = advancePlaybackIndex(points, 0, 100 * 0.5); + expect(rHalf.index).toBe(1); + + // At 2x speed: 50 ms real time = 100 ms virtual budget = 2 steps + const rDouble = advancePlaybackIndex(points, 0, 50 * 2.0); + expect(rDouble.index).toBe(2); + + // At 5x speed: 50 ms real time = 250 ms virtual budget = 5 steps + const rFive = advancePlaybackIndex(points, 0, 50 * 5.0); + expect(rFive.index).toBe(5); + + // At 10x speed: 50 ms real time = 500 ms virtual budget = 10 steps + const rTen = advancePlaybackIndex(points, 0, 50 * 10.0); + expect(rTen.index).toBe(10); + }); + + it('handles speed changes mid-playback without time discontinuity', () => { + let index = 10; + let carry = 0; + let speed = 1.0; + + // 1 frame at 1x (50 ms): advances 1 step + let r = advancePlaybackIndex(points, index, 50 * speed + carry); + index = r.index; + carry = r.remainingMs; + expect(index).toBe(11); + + // User switches to 4x speed: reset lastTime (delta = 0 on transition), next frame is 50 ms * 4 = 200 ms + speed = 4.0; + r = advancePlaybackIndex(points, index, 50 * speed + carry); + index = r.index; + carry = r.remainingMs; + // 200 ms budget over 50 ms steps = 4 steps: 11 + 4 = 15 + expect(index).toBe(15); + expect(carry).toBe(0); + }); +}); + +describe('replay regression tests: large datasets', () => { + it('advances efficiently across 10,000 points', () => { + const large = Array.from({ length: 10_000 }, (_, i) => mk(i * 50)); + const start = performance.now(); + // Advance 500 steps (25,000 ms budget) + const r = advancePlaybackIndex(large, 1000, 25_000); + const duration = performance.now() - start; + + expect(r.index).toBe(1500); + expect(r.remainingMs).toBe(0); + expect(duration).toBeLessThan(100); + }); + + it('handles 144,000 points (full 2-hour 20 Hz recording) without stalling', () => { + const huge = Array.from({ length: 144_000 }, (_, i) => mk(i * 50)); + const start = performance.now(); + + // Advance 2,000 steps at high speed + const r = advancePlaybackIndex(huge, 100_000, 100_000); + const duration = performance.now() - start; + + expect(r.index).toBe(102_000); + expect(r.remainingMs).toBe(0); + expect(duration).toBeLessThan(100); + + // End of recording clamp + const rEnd = advancePlaybackIndex(huge, 143_990, 1_000_000); + expect(rEnd.index).toBe(143_999); + expect(rEnd.remainingMs).toBe(0); + }); +});