diff --git a/openspec/changes/code-review-fix/tasks.md b/openspec/changes/code-review-fix/tasks.md index 572b3fa..6421db8 100644 --- a/openspec/changes/code-review-fix/tasks.md +++ b/openspec/changes/code-review-fix/tasks.md @@ -64,7 +64,7 @@ ## 7. Frontend — Stale Closures & Race Conditions - [x] 7.1 `[opus]` Fix stale closure in `fetchPredictions`: extract `modelInfo` into a `useRef` that stays in sync with state, use ref in `generateCacheKey` (`src/app/page.tsx:489-552`) -- [ ] 7.2 `[opus]` Add AbortController to `fetchPredictions` and `handleFetchBatchPredictions`: store controller in ref, abort previous on new request, discard stale responses (`src/app/page.tsx:494-663`) +- [x] 7.2 `[opus]` Add AbortController to `fetchPredictions` and `handleFetchBatchPredictions`: store controller in ref, abort previous on new request, discard stale responses (`src/app/page.tsx:494-663`) - [ ] 7.3 `[opus]` Fix stale closure in CandleChart click handler: convert `drawingState`, `selectedLineId`, `dragState`, `annotations` to refs, update refs alongside setState, read refs in handler (`src/components/CandleChart.tsx:572-971`) - [ ] 7.4 `[opus]` Fix stale closure in SpanAnnotationManager keyboard handler: wrap `handleDeleteSpan` in `useCallback`, use ref for `selectedSpanId` (`src/components/SpanAnnotationManager.tsx:461-535`) diff --git a/src/app/page.tsx b/src/app/page.tsx index 87e8f2c..4f741c8 100644 --- a/src/app/page.tsx +++ b/src/app/page.tsx @@ -198,6 +198,14 @@ export default function Home() { modelVersion: string; }>>(new Map()); + // Ref to avoid stale closure over modelInfo in fetchPredictions/generateCacheKey + const modelInfoRef = useRef(predictionState.modelInfo); + const fetchPredictionsAbortRef = useRef(null); + const fetchBatchAbortRef = useRef(null); + useEffect(() => { + modelInfoRef.current = predictionState.modelInfo; + }, [predictionState.modelInfo]); + // Model health state const [isModelOnline, setIsModelOnline] = useState(true); @@ -490,20 +498,21 @@ export default function Home() { // Generate cache key from chart, timerange, and model version const generateCacheKey = useCallback((chartId: number | null, modelVersion?: string | null) => { if (!chartId) return null; - const version = modelVersion || predictionState.modelInfo?.model_version || 'unknown'; + const version = modelVersion || modelInfoRef.current?.model_version || 'unknown'; return `${chartId}_${version}`; - }, [predictionState.modelInfo]); + }, []); // Fetch predictions for visible candles const fetchPredictions = useCallback(async (candles: any[]) => { if (!activeChartId || candles.length === 0) return; - const cacheKey = generateCacheKey(activeChartId, predictionState.modelInfo?.model_version); - + const currentModelInfo = modelInfoRef.current; + const cacheKey = generateCacheKey(activeChartId, currentModelInfo?.model_version); + // Check cache first if (cacheKey && predictionCacheRef.current.has(cacheKey)) { const cached = predictionCacheRef.current.get(cacheKey)!; - if (cached.modelVersion === predictionState.modelInfo?.model_version) { + if (cached.modelVersion === currentModelInfo?.model_version) { setPredictionState((prev) => ({ ...prev, spans: cached.spans, @@ -514,6 +523,11 @@ export default function Home() { } } + // Abort any in-flight prediction request + fetchPredictionsAbortRef.current?.abort(); + const controller = new AbortController(); + fetchPredictionsAbortRef.current = controller; + setPredictionState((prev) => ({ ...prev, isLoading: true, error: null })); try { @@ -521,6 +535,7 @@ export default function Home() { method: 'POST', headers: { 'Content-Type': 'application/json' }, body: JSON.stringify({ candles }), + signal: controller.signal, }); if (!response.ok) { @@ -528,7 +543,7 @@ export default function Home() { } const data = await response.json(); - + // Cache the results if (cacheKey) { predictionCacheRef.current.set(cacheKey, { @@ -546,6 +561,7 @@ export default function Home() { cacheKey, })); } catch (error) { + if (error instanceof Error && error.name === 'AbortError') return; console.error('Failed to fetch predictions:', error); setPredictionState((prev) => ({ ...prev, @@ -553,7 +569,7 @@ export default function Home() { error: error instanceof Error ? error.message : 'Failed to fetch predictions', })); } - }, [activeChartId, predictionState.modelInfo, generateCacheKey]); + }, [activeChartId, generateCacheKey]); // Toggle prediction visibility const togglePredictionVisibility = useCallback(() => { @@ -604,23 +620,32 @@ export default function Home() { const handleFetchBatchPredictions = useCallback(async () => { if (!activeChartId) return; + // Abort any in-flight batch prediction request + fetchBatchAbortRef.current?.abort(); + const controller = new AbortController(); + fetchBatchAbortRef.current = controller; + setPredictionState((prev) => ({ ...prev, isLoading: true, error: null })); try { // Fetch chart data to get pair/timeframe info - const chartResponse = await fetch(`/api/charts/${activeChartId}`); + const chartResponse = await fetch(`/api/charts/${activeChartId}`, { + signal: controller.signal, + }); if (!chartResponse.ok) { throw new Error('Failed to fetch chart info'); } const chartData = await chartResponse.json(); // Fetch candles for the chart - const candlesResponse = await fetch(`/api/candles?chartId=${activeChartId}`); + const candlesResponse = await fetch(`/api/candles?chartId=${activeChartId}`, { + signal: controller.signal, + }); if (!candlesResponse.ok) { throw new Error('Failed to fetch candles'); } const candlesData = await candlesResponse.json(); - + if (candlesData.length === 0) { throw new Error('No candles found for this chart'); } @@ -632,6 +657,7 @@ export default function Home() { body: JSON.stringify({ candles: candlesData, }), + signal: controller.signal, }); if (!response.ok) { @@ -639,7 +665,7 @@ export default function Home() { } const data = await response.json(); - + const cacheKey = generateCacheKey(activeChartId, data.model_info.model_version); if (cacheKey) { predictionCacheRef.current.set(cacheKey, { @@ -657,6 +683,7 @@ export default function Home() { cacheKey, })); } catch (error) { + if (error instanceof Error && error.name === 'AbortError') return; console.error('Failed to fetch batch predictions:', error); setPredictionState((prev) => ({ ...prev,