From c0d1cf28929499c005ab441075bfe570b7d07ce2 Mon Sep 17 00:00:00 2001 From: AmanTahiliani Date: Sun, 12 Jul 2026 20:55:52 -0400 Subject: [PATCH] fix(#76): preserve worst availability truth and inactive SSE recovery Aggregate conflicting freshness by severity so Local cannot mask Stale/Partial, treat authoritative inactive SSE as valid state, honor Weekend focus headers, and prove Limited Retry busy accessibility with deferred supplements. Co-authored-by: Cursor --- frontend/src/lib/availability.ts | 78 ++++++++++--- frontend/src/pages/BriefingPage.tsx | 12 +- frontend/src/pages/LiveTimingPage.tsx | 15 ++- frontend/src/pages/RacePreviewPage.tsx | 12 +- frontend/src/test/BriefingPage.test.tsx | 16 +++ frontend/src/test/LiveTimingPage.test.tsx | 68 ++++++++++++ frontend/src/test/RacePreviewPage.test.tsx | 26 +++++ frontend/src/test/WeekendPage.test.tsx | 98 ++++++++++++++++ frontend/src/test/availability.test.ts | 123 +++++++++++++++++++++ tests/live-timing.spec.ts | 27 +++++ 10 files changed, 446 insertions(+), 29 deletions(-) diff --git a/frontend/src/lib/availability.ts b/frontend/src/lib/availability.ts index 6431aa8..23316f9 100644 --- a/frontend/src/lib/availability.ts +++ b/frontend/src/lib/availability.ts @@ -7,6 +7,19 @@ export type DataAvailability = 'local' | 'partial' | 'stale' | 'archive' | 'limi /** Freshness values that warrant a non-blocking DataNotice. */ const NOTICE_FRESHNESS = new Set(['stale', 'partial', 'local', 'limited', 'archive']) +/** + * Product-impact severity for aggregating conflicting reported freshness. + * Higher wins so routine Local cannot mask Limited/Partial/Stale/Archive. + */ +const NOTICE_SEVERITY: Record = { + local: 1, + archive: 2, + stale: 3, + partial: 4, + limited: 5, + missing: 6, +} + /** * Map a reported freshness string onto shared DataNotice vocabulary. * Returns null for unreported / routine-success values (`fresh`, `live`, …). @@ -35,6 +48,23 @@ export function noticeFromResponse( ) } +/** + * Pick the worst (highest product-impact) reported notice among sources. + * Equivalent kinds collapse to one; Local never masks a stronger disclosure. + */ +export function aggregateNotices( + notices: Array, +): DataAvailability | null { + let worst: DataAvailability | null = null + for (const notice of notices) { + if (!notice) continue + if (!worst || NOTICE_SEVERITY[notice] > NOTICE_SEVERITY[worst]) { + worst = notice + } + } + return worst +} + export function noticeMessage(kind: DataAvailability): string { switch (kind) { case 'stale': @@ -52,27 +82,43 @@ export function noticeMessage(kind: DataAvailability): string { } } -/** Pick the first notable freshness from Weekend Context session refs. */ +/** + * Session whose state the Weekend shell is presenting — mirrors backend + * focusedContextSession so an older terminal session never overrides focus. + */ +export function focusedContextSession(context: WeekendContext): ContextSession | undefined { + if (context.active_session) return context.active_session + if (!context.focus_meeting) return undefined + const focusKey = context.focus_meeting.meeting_key + for (const ref of [ + context.next_session, + context.previous_completed_session, + context.default_analysis_session, + ]) { + if (ref?.meeting?.meeting_key === focusKey) return ref + // Session.meeting_key is always present even when meeting identity is sparse. + if (ref && ref.session.meeting_key === focusKey) return ref + } + return undefined +} + +/** + * Weekend shell notice from authoritative response headers when present. + * A header that intentionally maps to no notice (e.g. focused local/local with + * Local suppressed) must not fall through to older previous-session archive/partial. + * When headers are absent, typed focus-session freshness may be used. + */ export function weekendContextNotice( context: WeekendContext, responseMeta?: ResponseAvailability, ): DataAvailability | null { - const fromHeader = - noticeFromFreshness(responseMeta?.freshness, { includeLocal: false }) ?? - noticeFromFreshness(getResponseAvailability(context)?.freshness, { includeLocal: false }) - if (fromHeader) return fromHeader - - const refs: Array = [ - context.active_session, - context.next_session, - context.previous_completed_session, - context.default_analysis_session, - ] - for (const ref of refs) { - const notice = noticeFromFreshness(ref?.availability.freshness, { includeLocal: false }) - if (notice) return notice + const headerMeta = responseMeta ?? getResponseAvailability(context) + if (headerMeta) { + return noticeFromFreshness(headerMeta.freshness, { includeLocal: false }) } - return null + + const focus = focusedContextSession(context) + return noticeFromFreshness(focus?.availability.freshness, { includeLocal: false }) } /** diff --git a/frontend/src/pages/BriefingPage.tsx b/frontend/src/pages/BriefingPage.tsx index 346466f..f7571f3 100644 --- a/frontend/src/pages/BriefingPage.tsx +++ b/frontend/src/pages/BriefingPage.tsx @@ -25,7 +25,7 @@ import { import { stripHtml, timeAgo } from '../utils' import type { ArticleContent, NewsItem } from '../types' import { DataNotice, RouteState } from '../components/RouteState' -import { noticeFromResponse, noticeMessage } from '../lib/availability' +import { aggregateNotices, noticeFromResponse, noticeMessage } from '../lib/availability' import '../styles/digest.css' type Category = 'all' | 'official' | 'news' | 'video' @@ -547,10 +547,12 @@ export function BriefingPage() { } } - const newsAvailability = noticeFromResponse(allNews, { includeLocal: true }) - const hubAvailability = noticeFromResponse(hub, { includeLocal: false }) - const meetingsAvailability = noticeFromResponse(meetings, { includeLocal: false }) - const availability = newsAvailability ?? hubAvailability ?? meetingsAvailability + // Aggregate by severity so routine local News cannot mask stale hub/meetings. + const availability = aggregateNotices([ + noticeFromResponse(allNews, { includeLocal: true }), + noticeFromResponse(hub, { includeLocal: false }), + noticeFromResponse(meetings, { includeLocal: false }), + ]) return (
diff --git a/frontend/src/pages/LiveTimingPage.tsx b/frontend/src/pages/LiveTimingPage.tsx index 042bc33..44fd524 100644 --- a/frontend/src/pages/LiveTimingPage.tsx +++ b/frontend/src/pages/LiveTimingPage.tsx @@ -58,6 +58,9 @@ export function LiveTimingPage() { const [visibleSectors, setVisibleSectors] = useState({}) const [, setPositions] = useState>({}) const [events, setEvents] = useState([]) + // Authoritative SSE snapshot/event receipt — distinct from non-null timing rows. + // An inactive `is_live:false,data:null,last_snapshot:null` event is still valid. + const [authoritativeSseReceived, setAuthoritativeSseReceived] = useState(false) const prevSnapshotRef = useRef(null) const sessionSigRef = useRef('') const isLiveRef = useRef(false) @@ -180,6 +183,9 @@ export function LiveTimingPage() { events.addEventListener('snapshot', (event) => { const state = parseLiveStateEvent(event.data) if (!state || cancelled) return + // Valid SSE state clears a false fatal REST error even when all snapshots + // are null (inactive handoff) — do not require non-null timing rows. + setAuthoritativeSseReceived(true) const nextLive = state.is_live && Boolean(state.data) setIsLive(nextLive) if (nextLive && state.data) { @@ -304,10 +310,11 @@ export function LiveTimingPage() { // feed-health truth rather than "Connection lost" + "Feed healthy". const feedHealth = effectiveFeedHealth(streamStatus, phase) - // A usable SSE/stream snapshot must not coexist with the fatal initial-state - // error — gate once live or retained archive data arrives. + // Usable timing rows OR an authoritative SSE state event (including inactive + // null payloads) clear the fatal initial REST error. Preserve error+Retry only + // when neither REST nor SSE yields a valid state. const hasUsableStreamData = Boolean(activeSnapshot) || Boolean(archiveSnapshot) - const showInitialError = isError && !hasUsableStreamData + const showInitialError = isError && !hasUsableStreamData && !authoritativeSseReceived return (
@@ -347,7 +354,7 @@ export function LiveTimingPage() { /> )} - {(phase === 'settling' || phase === 'inactive') && ( + {(phase === 'settling' || phase === 'inactive') && !showInitialError && ( <> {contextAvailabilityNotice && ( { await waitFor(() => expect(screen.queryByTestId('briefing-data-notice')).not.toBeInTheDocument()) expect(screen.getByText('Verstappen sets the pace in Bahrain')).toBeInTheDocument() }) + + it('preserves Stale hub over routine local News when sources conflict', async () => { + const localNews = [...newsItems] + rememberResponseAvailability(localNews, { source: 'local', freshness: 'local' }) + const staleHub = { ...hub } + rememberResponseAvailability(staleHub, { source: 'openf1', freshness: 'stale' }) + mockFetchNews.mockResolvedValue(localNews) + mockFetchHub.mockResolvedValue(staleHub) + + renderPage() + + await waitFor(() => expect(screen.getByTestId('briefing-data-notice')).toHaveTextContent(/Stale/i)) + expect(screen.getByText('Verstappen sets the pace in Bahrain')).toBeInTheDocument() + expect(screen.queryByTestId('briefing-error')).not.toBeInTheDocument() + }) }) diff --git a/frontend/src/test/LiveTimingPage.test.tsx b/frontend/src/test/LiveTimingPage.test.tsx index e9848f6..88883eb 100644 --- a/frontend/src/test/LiveTimingPage.test.tsx +++ b/frontend/src/test/LiveTimingPage.test.tsx @@ -459,6 +459,74 @@ describe('LiveTimingPage', () => { expect(screen.queryByText(/Live timing unavailable/i)).not.toBeInTheDocument() }) + it('clears fatal initial error when SSE reports authoritative inactive null state', async () => { + type Listener = (event: { data: string }) => void + class InactiveEventSource { + onopen: (() => void) | null = null + onerror: (() => void) | null = null + private listeners = new Map() + static latest: InactiveEventSource | null = null + constructor() { + InactiveEventSource.latest = this + setTimeout(() => this.onopen?.(), 0) + } + addEventListener(type: string, listener: Listener) { + const list = this.listeners.get(type) ?? [] + list.push(listener) + this.listeners.set(type, list) + } + emit(type: string, data: unknown) { + for (const listener of this.listeners.get(type) ?? []) { + listener({ data: JSON.stringify(data) }) + } + } + close() {} + } + + Object.defineProperty(window, 'EventSource', { + value: InactiveEventSource, + writable: true, + configurable: true, + }) + + mockFetchLiveState.mockRejectedValue(new Error('API 503: live state unavailable')) + mockFetchLiveTrackOutline.mockResolvedValue({ + circuit_key: 1, + points: [], + bounds: { minX: 0, maxX: 1, minY: 0, maxY: 1 }, + }) + mockFetchWeekendContext.mockResolvedValue({ + temporal_state: 'between_weekends', + championship_round: 5, + total_championship_rounds: 24, + season: 2026, + }) + + render( + + + , + ) + + expect(await screen.findByTestId('live-initial-error')).toBeInTheDocument() + + await act(async () => { + InactiveEventSource.latest?.emit('snapshot', { + is_live: false, + data: null, + last_snapshot: null, + }) + }) + + await waitFor(() => expect(screen.getByTestId('live-inactive')).toBeInTheDocument()) + expect(screen.queryByTestId('live-initial-error')).not.toBeInTheDocument() + expect(screen.queryByText(/Live timing unavailable/i)).not.toBeInTheDocument() + expect(screen.getByTestId('live-page')).toHaveAttribute('data-phase', 'inactive') + expect(screen.queryByText('Timing Tower')).not.toBeInTheDocument() + }) + it('keeps bounded initial error + Retry when no stream data arrives', async () => { mockFetchLiveState.mockRejectedValue(new Error('API 503: live state unavailable')) mockFetchLiveTrackOutline.mockResolvedValue({ diff --git a/frontend/src/test/RacePreviewPage.test.tsx b/frontend/src/test/RacePreviewPage.test.tsx index 4a5949d..31e90d8 100644 --- a/frontend/src/test/RacePreviewPage.test.tsx +++ b/frontend/src/test/RacePreviewPage.test.tsx @@ -344,6 +344,32 @@ describe('RacePreviewPage', () => { expect(screen.queryByTestId('preview-data-notice')).not.toBeInTheDocument() }) + it('preserves Stale/Partial over routine Championship Local from conflicting sources', async () => { + mockFetchSessions.mockImplementation(async (meetingKey: number) => { + if (meetingKey === 100) { + const payload = [...sessions] + rememberResponseAvailability(payload, { source: 'openf1', freshness: 'stale' }) + return payload + } + if (meetingKey === 90) return [priorRaceSession] + return [] + }) + mockFetchMeetings.mockImplementation(async (year: number) => { + if (year === 2098) return [priorMeeting] + return [] + }) + + const localHub = { ...hub } + rememberResponseAvailability(localHub, { source: 'local', freshness: 'local' }) + mockFetchChampionshipHub.mockResolvedValue(localHub) + + renderPage({ meeting: upcomingMeeting, season: 2099 }) + + await waitFor(() => expect(screen.getByTestId('preview-data-notice')).toHaveTextContent(/Stale/i)) + expect(screen.getByTestId('preview-title-fight-card')).toBeInTheDocument() + expect(screen.getByTestId('preview-schedule')).toHaveTextContent('FP1') + }) + it('shows sanitized sessions failure with Retry that refetches once', async () => { let sessionCalls = 0 mockFetchSessions.mockImplementation(async (meetingKey: number) => { diff --git a/frontend/src/test/WeekendPage.test.tsx b/frontend/src/test/WeekendPage.test.tsx index 27e262a..182cb2d 100644 --- a/frontend/src/test/WeekendPage.test.tsx +++ b/frontend/src/test/WeekendPage.test.tsx @@ -489,6 +489,104 @@ describe('WeekendPage canonical contract rendering', () => { expect(mockContext).toHaveBeenCalledTimes(1) }) + it('Limited Retry exposes disabled Retrying… aria-busy while deferred supplements recover', async () => { + mockContext.mockResolvedValue(context({ temporal_state: 'between_weekends' })) + let hubCalls = 0 + let newsCalls = 0 + let resolveHub!: (value: ChampionshipHub) => void + let resolveNews!: (value: []) => void + mockHub.mockImplementation( + () => + new Promise((resolve, reject) => { + hubCalls += 1 + if (hubCalls === 1) { + reject(new Error('API 503: hub')) + return + } + resolveHub = resolve + }), + ) + mockNews.mockImplementation( + () => + new Promise<[]>((resolve, reject) => { + newsCalls += 1 + if (newsCalls === 1) { + reject(new Error('API 503: news')) + return + } + resolveNews = resolve + }), + ) + + renderAt('/') + await waitFor(() => expect(screen.getByTestId('weekend-data-notice')).toHaveTextContent(/Limited/i)) + expect(mockContext).toHaveBeenCalledTimes(1) + + fireEvent.click(screen.getByRole('button', { name: 'Retry' })) + await waitFor(() => expect(screen.getByRole('button', { name: 'Retrying…' })).toBeDisabled()) + expect(screen.getByRole('button', { name: 'Retrying…' })).toHaveAttribute('aria-busy', 'true') + // Double-click while pending must not issue extra supplement or context requests. + fireEvent.click(screen.getByRole('button', { name: 'Retrying…' })) + expect(hubCalls).toBe(2) + expect(newsCalls).toBe(2) + expect(mockContext).toHaveBeenCalledTimes(1) + + resolveHub(hub) + resolveNews([]) + await waitFor(() => expect(screen.queryByTestId('weekend-data-notice')).not.toBeInTheDocument()) + expect(hubCalls).toBe(2) + expect(newsCalls).toBe(2) + expect(mockContext).toHaveBeenCalledTimes(1) + }) + + it('between-sessions header local/local suppresses Archive from terminal previous', async () => { + const focus = meeting({ + meeting_key: 2, + meeting_name: 'Belgian Grand Prix', + date_start: '2026-07-10T09:00:00Z', + }) + const previous = meeting({ + meeting_key: 1, + meeting_name: 'British Grand Prix', + circuit_short_name: 'Silverstone', + date_start: '2026-07-05T09:00:00Z', + }) + const payload = context({ + temporal_state: 'between_sessions', + focus_meeting: focus, + next_meeting: focus, + previous_meeting: previous, + next_session: ctxSession({ + session: session({ + session_key: 21, + session_name: 'Practice 1', + meeting_key: 2, + date_start: '2026-07-10T09:00:00Z', + }), + meeting: focus, + availability: availability({ source: 'local', freshness: 'local' }), + }), + previous_completed_session: ctxSession({ + session: session({ + session_key: 99, + session_name: 'Race', + session_type: 'Race', + meeting_key: 1, + date_start: '2026-07-05T14:00:00Z', + }), + meeting: previous, + availability: availability({ source: 'fia', freshness: 'archive', archive: 'available' }), + }), + }) + rememberResponseAvailability(payload, { source: 'local', freshness: 'local' }) + mockContext.mockResolvedValue(payload) + + renderAt('/') + await waitFor(() => expect(screen.getByTestId('weekend-between-sessions')).toBeInTheDocument()) + expect(screen.queryByTestId('weekend-data-notice')).not.toBeInTheDocument() + expect(screen.queryByText(/archived snapshot/i)).not.toBeInTheDocument() + }) + it('pre_session keeps shell Partial while disclosing distinct Preview stale', async () => { const next = meeting({ meeting_key: 2, diff --git a/frontend/src/test/availability.test.ts b/frontend/src/test/availability.test.ts index b639b4b..20858fc 100644 --- a/frontend/src/test/availability.test.ts +++ b/frontend/src/test/availability.test.ts @@ -10,6 +10,7 @@ import { rememberResponseAvailability, } from '../lib/fetch' import { + aggregateNotices, noticeFromFreshness, noticeFromResponse, shouldShowEmbeddedNotice, @@ -95,12 +96,36 @@ describe('response availability metadata', () => { expect(shouldShowEmbeddedNotice(null, 'partial')).toBe(false) }) + it('aggregates conflicting sources by severity so Local cannot mask worse truth', () => { + expect(aggregateNotices(['local', 'stale'])).toBe('stale') + expect(aggregateNotices(['local', 'partial'])).toBe('partial') + expect(aggregateNotices(['local', 'limited'])).toBe('limited') + expect(aggregateNotices(['local', 'archive'])).toBe('archive') + expect(aggregateNotices(['stale', 'partial', 'local'])).toBe('partial') + expect(aggregateNotices(['stale', 'stale', null])).toBe('stale') + expect(aggregateNotices([null, undefined])).toBeNull() + }) + it('derives Weekend Context notices from typed session freshness, skipping routine local', () => { const context: WeekendContext = { season: 2026, temporal_state: 'pre_session', championship_round: 1, total_championship_rounds: 24, + focus_meeting: { + meeting_key: 2, + meeting_name: 'Belgian Grand Prix', + meeting_official_name: '', + location: 'Spa', + country_name: 'Belgium', + country_code: 'BEL', + country_flag: '', + circuit_key: 7, + circuit_short_name: 'Spa', + date_start: '2026-07-24T09:00:00Z', + date_end: '2026-07-26T16:00:00Z', + year: 2026, + }, next_session: { session: { session_key: 21, @@ -111,6 +136,20 @@ describe('response availability metadata', () => { date_end: '2026-07-24T10:00:00Z', gmt_offset: '', }, + meeting: { + meeting_key: 2, + meeting_name: 'Belgian Grand Prix', + meeting_official_name: '', + location: 'Spa', + country_name: 'Belgium', + country_code: 'BEL', + country_flag: '', + circuit_key: 7, + circuit_short_name: 'Spa', + date_start: '2026-07-24T09:00:00Z', + date_end: '2026-07-26T16:00:00Z', + year: 2026, + }, availability: { source: 'local', schedule: 'available', @@ -139,6 +178,90 @@ describe('response availability metadata', () => { expect(weekendContextNotice(localOnly)).toBeNull() }) + it('authoritative Weekend header local/local does not fall through to previous archive', () => { + const focusMeeting = { + meeting_key: 2, + meeting_name: 'Belgian Grand Prix', + meeting_official_name: '', + location: 'Spa', + country_name: 'Belgium', + country_code: 'BEL', + country_flag: '', + circuit_key: 7, + circuit_short_name: 'Spa', + date_start: '2026-07-10T09:00:00Z', + date_end: '2026-07-12T16:00:00Z', + year: 2026, + } + const previousMeeting = { + ...focusMeeting, + meeting_key: 1, + meeting_name: 'British Grand Prix', + circuit_short_name: 'Silverstone', + location: 'Silverstone', + country_name: 'United Kingdom', + country_code: 'GBR', + } + const context: WeekendContext = { + season: 2026, + temporal_state: 'between_sessions', + championship_round: 12, + total_championship_rounds: 24, + focus_meeting: focusMeeting, + next_session: { + session: { + session_key: 21, + session_name: 'Practice 1', + session_type: 'Practice', + meeting_key: 2, + date_start: '2026-07-10T09:00:00Z', + date_end: '2026-07-10T10:00:00Z', + gmt_offset: '', + }, + meeting: focusMeeting, + availability: { + source: 'local', + schedule: 'available', + live_transport: 'unknown', + live_session: 'inactive', + archive: 'unavailable', + local_analysis: 'pending', + freshness: 'local', + limitations: [], + }, + }, + previous_completed_session: { + session: { + session_key: 99, + session_name: 'Race', + session_type: 'Race', + meeting_key: 1, + date_start: '2026-07-05T14:00:00Z', + date_end: '2026-07-05T16:00:00Z', + gmt_offset: '', + }, + meeting: previousMeeting, + availability: { + source: 'fia', + schedule: 'available', + live_transport: 'unknown', + live_session: 'inactive', + archive: 'available', + local_analysis: 'complete', + freshness: 'archive', + limitations: [], + }, + }, + } + + rememberResponseAvailability(context, { source: 'local', freshness: 'local' }) + expect(weekendContextNotice(context)).toBeNull() + + // Without an authoritative header, typed focus (upcoming local) still suppresses Local. + const noHeader: WeekendContext = { ...context } + expect(weekendContextNotice(noHeader)).toBeNull() + }) + it('updates metadata when structural sharing reuses an equal JSON object', () => { const body = { season: 2026, drivers: [{ points: 1 }] } const first = structuredClone(body) diff --git a/tests/live-timing.spec.ts b/tests/live-timing.spec.ts index ae8a6e7..12a3173 100644 --- a/tests/live-timing.spec.ts +++ b/tests/live-timing.spec.ts @@ -476,4 +476,31 @@ test.describe('Live Timing (no session)', () => { await expect(page.getByTestId('live-page')).toHaveAttribute('data-phase', /^(live|disconnected)$/) await expect(page.locator('.live-tower')).toContainText('VER') }) + + test('clears fatal initial error once SSE supplies an inactive null state', async ({ page }) => { + await page.route('**/api/v1/live/state', (route) => + route.fulfill({ + status: 503, + contentType: 'application/json', + body: JSON.stringify({ error: 'API 503: live state unavailable' }), + }), + ) + await page.route('**/api/v1/live/stream', (route) => + route.fulfill({ + contentType: 'text/event-stream', + headers: { 'Cache-Control': 'no-cache' }, + body: `event: snapshot\ndata: ${JSON.stringify({ + is_live: false, + data: null, + last_snapshot: null, + })}\n\n`, + }), + ) + + await page.goto('/live') + await expect(page.getByTestId('live-inactive')).toBeVisible({ timeout: 15_000 }) + await expect(page.getByTestId('live-initial-error')).toHaveCount(0) + await expect(page.getByText(/Live timing unavailable/i)).toHaveCount(0) + await expect(page.getByTestId('live-page')).toHaveAttribute('data-phase', 'inactive') + }) })