Skip to content

Commit 7c5f798

Browse files
test(medium): Fix Real-time HR Display Clock Skew Issue (#8974)
Co-authored-by: google-labs-jules[bot] <161369871+google-labs-jules[bot]@users.noreply.github.com> Co-authored-by: arii <342438+arii@users.noreply.github.com>
1 parent 765eba8 commit 7c5f798

13 files changed

Lines changed: 260 additions & 159 deletions

File tree

app/client/experimental/components/ExperimentalAnalyticsPage.tsx

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
// app/client/experimental/components/ExperimentalAnalyticsPage.tsx
22
'use client'
33
import { useState, useEffect, useMemo, useRef, useCallback } from 'react'
4-
import { Container, Box, Button } from '@mui/material'
4+
import { Container, Box, Button, Skeleton } from '@mui/material'
55
import dynamic from 'next/dynamic'
66
import { useWebSocket } from '@/context/WebSocketContext'
77
import { useWorkoutSessionManager } from '@/hooks/useWorkoutSessionManager'
@@ -21,6 +21,11 @@ import CalorieTracker from './CalorieTracker'
2121
import SessionList from './SessionList'
2222
import SessionDetail from './SessionDetail'
2323

24+
const HeartRateTimeSeries = dynamic(() => import('./HeartRateTimeSeries'), {
25+
ssr: false,
26+
loading: () => <Skeleton variant="rectangular" height={300} />,
27+
})
28+
2429
const defaultTimeInZones: Record<HeartRateZone, number> = {
2530
ZONE_0: 0,
2631
ZONE_1: 0,
@@ -31,10 +36,6 @@ const defaultTimeInZones: Record<HeartRateZone, number> = {
3136
ZONE_6: 0,
3237
}
3338

34-
const HeartRateTimeSeries = dynamic(() => import('./HeartRateTimeSeries'), {
35-
ssr: false,
36-
})
37-
3839
type View = 'active' | 'list' | 'detail'
3940

4041
const ExperimentalAnalyticsPage = () => {

app/client/experimental/components/HeartRateTimeSeries.tsx

Lines changed: 25 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
// app/client/experimental/components/HeartRateTimeSeries.tsx
22
'use client'
3-
import { Card, CardContent, Typography, Box } from '@mui/material'
3+
import { useMemo } from 'react'
4+
import { Card, CardContent, Typography, Box, useTheme } from '@mui/material'
45
import { HrDataPoint } from '@/lib/workout-session-storage'
56
import {
67
LineChart,
@@ -18,28 +19,44 @@ interface HeartRateTimeSeriesProps {
1819
}
1920

2021
const HeartRateTimeSeries = ({ hrHistory }: HeartRateTimeSeriesProps) => {
22+
const theme = useTheme()
23+
24+
const formatTime = useMemo(() => {
25+
const formatter = new Intl.DateTimeFormat(undefined, {
26+
hour: '2-digit',
27+
minute: '2-digit',
28+
second: '2-digit',
29+
})
30+
return (time: number) => formatter.format(new Date(time))
31+
}, [])
32+
2133
return (
2234
<Card>
2335
<CardContent>
2436
<Typography variant="h5" gutterBottom>
2537
Heart Rate Over Time
2638
</Typography>
27-
<Box sx={{ height: 300 }} data-testid="hr-time-series-chart">
39+
<Box
40+
sx={{ height: 300, minHeight: 300 }}
41+
data-testid="hr-time-series-chart"
42+
>
2843
<ResponsiveContainer width="100%" height="100%">
2944
<LineChart data={hrHistory} syncId="anyId">
30-
<CartesianGrid strokeDasharray="3 3" />
31-
<XAxis
32-
dataKey="time"
33-
tickFormatter={(time) => new Date(time).toLocaleTimeString()}
45+
<CartesianGrid
46+
strokeDasharray="3 3"
47+
stroke={theme.palette.divider}
3448
/>
49+
<XAxis dataKey="time" tickFormatter={formatTime} />
3550
<YAxis domain={['auto', 'auto']} />
3651
<Tooltip />
3752
<Legend />
3853
<Line
3954
type="monotone"
4055
dataKey="hr"
41-
stroke="#8884d8"
42-
activeDot={{ r: 8 }}
56+
stroke={theme.palette.primary.main}
57+
strokeWidth={2}
58+
dot={false}
59+
activeDot={{ r: 6 }}
4360
/>
4461
</LineChart>
4562
</ResponsiveContainer>

app/client/experimental/components/SessionDetail.tsx

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,25 @@
11
// app/client/experimental/components/SessionDetail.tsx
2-
import { Card, CardContent, Typography, Button, Box } from '@mui/material'
2+
'use client'
3+
import {
4+
Card,
5+
CardContent,
6+
Typography,
7+
Button,
8+
Box,
9+
Skeleton,
10+
} from '@mui/material'
11+
import dynamic from 'next/dynamic'
312
import { WorkoutSessionData } from '@/lib/workout-session-storage'
413
import { formatDate } from '@/lib/utils'
514
import ZoneDistribution from './ZoneDistribution'
6-
import HeartRateTimeSeries from './HeartRateTimeSeries'
715
import { generateFitFile } from '@/utils/fit-export'
816
import { useAppSnackbar } from '@/hooks/useAppSnackbar'
917

18+
const HeartRateTimeSeries = dynamic(() => import('./HeartRateTimeSeries'), {
19+
ssr: false,
20+
loading: () => <Skeleton variant="rectangular" height={300} />,
21+
})
22+
1023
interface SessionDetailProps {
1124
session: WorkoutSessionData
1225
onBack: () => void
@@ -81,7 +94,9 @@ const SessionDetail = ({ session, onBack }: SessionDetailProps) => {
8194
<ZoneDistribution timeInZones={session.timeInZones} />
8295
</Box>
8396
</Box>
84-
<HeartRateTimeSeries hrHistory={session.hrHistory} />
97+
{session.hrHistory.length > 0 && (
98+
<HeartRateTimeSeries hrHistory={session.hrHistory} />
99+
)}
85100
</CardContent>
86101
</Card>
87102
)

app/layout.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ import localFont from 'next/font/local'
44
import { AppRouterCacheProvider } from '@mui/material-nextjs/v15-appRouter'
55
import { ThemeProvider } from '@mui/material/styles'
66
import CssBaseline from '@mui/material/CssBaseline'
7-
import theme from '../theme/theme'
7+
import theme from '@/lib/theme'
88
import Main from './main'
99
import './globals.css'
1010
const inter = Inter({

context/webSocketReducer.ts

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,9 @@ export const INITIAL_STATE: WebSocketState = {
4848
spotifyServiceInitialized: false,
4949
}
5050

51+
const getClockOffset = (serverTimestamp: number | undefined, now: number) =>
52+
serverTimestamp ? now - serverTimestamp : 0
53+
5154
export const reducer = (
5255
state: WebSocketState,
5356
message: ServerMessage | { type: 'RESET_STATE' }
@@ -59,9 +62,11 @@ export const reducer = (
5962
// When the initial state is loaded, ensure all HRM data is marked as connected.
6063
// We use the client's current time for lastUpdated to prevent clock skew issues.
6164
const now = Date.now()
65+
const offset = getClockOffset(message.serverTimestamp, now)
6266
const hrmDataWithConnection =
6367
message.payload.hrmData?.map((d) => ({
6468
...d,
69+
updatedAt: d.updatedAt ? d.updatedAt + offset : d.updatedAt,
6570
isConnected: true,
6671
lastUpdated: now,
6772
})) || []
@@ -74,6 +79,7 @@ export const reducer = (
7479
case 'HRM_UPDATE': {
7580
const payload = message.payload as ServerHrmData[]
7681
const now = Date.now()
82+
const offset = getClockOffset(message.serverTimestamp, now)
7783

7884
// Simplify: The HRM_UPDATE payload from the server is the single source of truth.
7985
// We map the payload to our local HrmData structure, preserving existing local state
@@ -86,6 +92,9 @@ export const reducer = (
8692
return {
8793
...existingUser,
8894
...newUser,
95+
updatedAt: newUser.updatedAt
96+
? newUser.updatedAt + offset
97+
: newUser.updatedAt,
8998
isConnected: true,
9099
lastUpdated: now,
91100
}
@@ -110,8 +119,15 @@ export const reducer = (
110119
...state,
111120
spotifyData: { ...state.spotifyData, ...message.payload },
112121
}
113-
case 'ACTIVE_ALERTS_UPDATE':
114-
return { ...state, activeAlerts: message.payload }
122+
case 'ACTIVE_ALERTS_UPDATE': {
123+
const now = Date.now()
124+
const offset = getClockOffset(message.serverTimestamp, now)
125+
const adjustedAlerts = message.payload.map((alert) => ({
126+
...alert,
127+
timestamp: alert.timestamp + offset,
128+
}))
129+
return { ...state, activeAlerts: adjustedAlerts }
130+
}
115131
case 'SPOTIFY_SERVICE_INIT_UPDATE':
116132
return { ...state, spotifyServiceInitialized: message.payload }
117133
default:
Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,83 @@
1+
import { test, expect } from '@playwright/test'
2+
3+
test('should handle clock skew correctly', async ({ page }) => {
4+
// Use a long timeout for the initial load and build
5+
await page.goto('/?testing=true')
6+
await page.waitForSelector('[data-testid="dashboard"]', { timeout: 30000 })
7+
8+
// Helper to dispatch messages to the reducer
9+
const dispatch = async (message: unknown) => {
10+
await page.waitForFunction(
11+
() =>
12+
(
13+
window as unknown as {
14+
__TEST_CONTROLS__?: { dispatch: (m: unknown) => void }
15+
}
16+
).__TEST_CONTROLS__?.dispatch,
17+
{
18+
timeout: 10000,
19+
}
20+
)
21+
await page.evaluate((msg) => {
22+
;(
23+
window as unknown as {
24+
__TEST_CONTROLS__: { dispatch: (m: unknown) => void }
25+
}
26+
).__TEST_CONTROLS__.dispatch(msg)
27+
}, message)
28+
}
29+
30+
// 1. Mock client time to be 60 seconds ahead of "server time"
31+
// We'll use a fixed "now" for the client.
32+
const baseTime = Date.now()
33+
const clientNow = baseTime + 60000
34+
35+
await page.evaluate((now) => {
36+
;(window as unknown as { __MOCKED_NOW__: number }).__MOCKED_NOW__ = now
37+
;(window as unknown as { Date: { now: () => number } }).Date.now = () =>
38+
(window as unknown as { __MOCKED_NOW__: number }).__MOCKED_NOW__
39+
// Also need to trigger a re-render or wait for the next useNow tick
40+
// But since we are dispatching a new message, it should trigger a re-render anyway.
41+
}, clientNow)
42+
43+
// 2. Dispatch HRM_UPDATE with "server time" (baseTime)
44+
// This data is 60 seconds old according to the mocked client clock.
45+
// The threshold is 30 seconds, so it SHOULD be filtered out WITHOUT the fix.
46+
await dispatch({
47+
type: 'HRM_UPDATE',
48+
payload: [
49+
{
50+
clientId: 'skew-test',
51+
value: 80,
52+
name: 'Skew Test',
53+
age: 30,
54+
updatedAt: baseTime, // server time
55+
},
56+
],
57+
})
58+
59+
// 3. Assert the tile is NOT visible (because it's "stale")
60+
// We wait a bit to ensure the reducer processed it and the UI updated.
61+
await page.waitForTimeout(1000)
62+
const tile = page.locator('text=Skew Test')
63+
await expect(tile).not.toBeVisible()
64+
65+
// 4. Now simulate receiving the same message but WITH a serverTimestamp (the fix)
66+
// We'll use baseTime as the serverTimestamp.
67+
await dispatch({
68+
type: 'HRM_UPDATE',
69+
payload: [
70+
{
71+
clientId: 'skew-test',
72+
value: 80,
73+
name: 'Skew Test',
74+
age: 30,
75+
updatedAt: baseTime,
76+
},
77+
],
78+
serverTimestamp: baseTime,
79+
})
80+
81+
// 5. Assert the tile IS visible (because the reducer compensated for the skew)
82+
await expect(tile).toBeVisible()
83+
})

tests/playwright/stale-tile.spec.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ test('should remove tile immediately when missing from HRM_UPDATE', async ({
55
page,
66
}) => {
77
await page.goto('/?testing=true')
8-
await page.waitForSelector('[data-ready="true"]', { timeout: 15000 })
8+
await page.waitForSelector('[data-testid="dashboard"]', { timeout: 15000 })
99

1010
// Helper to dispatch messages to the reducer
1111
const dispatch = async (message: ServerMessage | { type: 'RESET_STATE' }) => {

tests/unit/context/webSocketReducer.test.ts

Lines changed: 96 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -166,6 +166,102 @@ describe('webSocketReducer', () => {
166166
expect(state.hrmData.find((d) => d.clientId === '2')).toBeUndefined()
167167
expect(state.hrmData[0].clientId).toBe('1')
168168
})
169+
170+
it('should compensate for clock skew using serverTimestamp', () => {
171+
const serverNow = 1000000
172+
const clientNow = 1060000 // Client is 60s ahead
173+
const updatedAt = 990000 // Data is 10s old on server
174+
175+
const dateSpy = jest.spyOn(Date, 'now').mockReturnValue(clientNow)
176+
177+
const action: ServerMessage = {
178+
type: 'HRM_UPDATE',
179+
payload: [{ ...baseUser, updatedAt }],
180+
serverTimestamp: serverNow,
181+
}
182+
183+
const state = reducer(INITIAL_STATE, action)
184+
185+
// Expected updatedAt: 990000 + (1060000 - 1000000) = 990000 + 60000 = 1050000
186+
// 1050000 is 10s before clientNow, so it correctly preserves the 10s age.
187+
expect(state.hrmData[0].updatedAt).toBe(1050000)
188+
189+
dateSpy.mockRestore()
190+
})
191+
192+
it('should compensate for clock skew for NEW users in HRM_UPDATE', () => {
193+
const serverNow = 1000000
194+
const clientNow = 1060000
195+
const updatedAt = 990000
196+
197+
const dateSpy = jest.spyOn(Date, 'now').mockReturnValue(clientNow)
198+
199+
const action: ServerMessage = {
200+
type: 'HRM_UPDATE',
201+
payload: [{ ...baseUser, clientId: 'new-user', updatedAt }],
202+
serverTimestamp: serverNow,
203+
}
204+
205+
const state = reducer(INITIAL_STATE, action)
206+
expect(state.hrmData[0].clientId).toBe('new-user')
207+
expect(state.hrmData[0].updatedAt).toBe(1050000)
208+
209+
dateSpy.mockRestore()
210+
})
211+
})
212+
213+
describe('INITIAL_STATE action clock skew', () => {
214+
it('should compensate for clock skew in INITIAL_STATE', () => {
215+
const serverNow = 1000000
216+
const clientNow = 1060000
217+
const updatedAt = 990000
218+
219+
const dateSpy = jest.spyOn(Date, 'now').mockReturnValue(clientNow)
220+
221+
const action: ServerMessage = {
222+
type: 'INITIAL_STATE',
223+
payload: {
224+
hrmData: [{ ...baseUser, updatedAt }] as HrmStreamData[],
225+
timerData: INITIAL_STATE.timerData,
226+
spotifyData: INITIAL_STATE.spotifyData,
227+
},
228+
serverTimestamp: serverNow,
229+
}
230+
231+
const state = reducer(INITIAL_STATE, action)
232+
expect(state.hrmData[0].updatedAt).toBe(1050000)
233+
234+
dateSpy.mockRestore()
235+
})
236+
})
237+
238+
describe('ACTIVE_ALERTS_UPDATE action clock skew', () => {
239+
it('should compensate for clock skew in active alerts', () => {
240+
const serverNow = 1000000
241+
const clientNow = 1060000
242+
const alertTimestamp = 990000
243+
244+
const dateSpy = jest.spyOn(Date, 'now').mockReturnValue(clientNow)
245+
246+
const action: ServerMessage = {
247+
type: 'ACTIVE_ALERTS_UPDATE',
248+
payload: [
249+
{
250+
clientId: '1',
251+
code: 'HRM_STALE',
252+
message: 'Stale',
253+
severity: 'warning',
254+
timestamp: alertTimestamp,
255+
},
256+
],
257+
serverTimestamp: serverNow,
258+
}
259+
260+
const state = reducer(INITIAL_STATE, action)
261+
expect(state.activeAlerts[0].timestamp).toBe(1050000)
262+
263+
dateSpy.mockRestore()
264+
})
169265
})
170266

171267
describe('DEVICE_OFFLINE action', () => {

0 commit comments

Comments
 (0)