From e5b0cc25408f05c86d7cc82aa41349d1ad9fc34d Mon Sep 17 00:00:00 2001
From: Marko Djordjevic <marko.homoludens@gmail.com>
Date: Wed, 18 Feb 2026 15:16:55 +0100
Subject: [PATCH] fix: resolve stale closure in SpanAnnotationManager keyboard
 handler

Use selectedSpanIdRef to avoid capturing stale selectedSpanId in the
keyboard event handler closure. Wrap handleDeleteSpan in useCallback
with stable dependencies (reads selectedSpanIdRef.current instead of
the prop directly). Remove selectedSpanId from the useEffect dependency
array since the ref is used instead.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
---
 openspec/changes/code-review-fix/tasks.md |  2 +-
 src/components/SpanAnnotationManager.tsx  | 25 +++++++++++++++--------
 2 files changed, 17 insertions(+), 10 deletions(-)

diff --git a/openspec/changes/code-review-fix/tasks.md b/openspec/changes/code-review-fix/tasks.md
index cd35588..f83bfc1 100644
--- a/openspec/changes/code-review-fix/tasks.md
+++ b/openspec/changes/code-review-fix/tasks.md
@@ -66,7 +66,7 @@
 - [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`)
 - [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`)
 - [x] 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`)
+- [x] 7.4 `[opus]` Fix stale closure in SpanAnnotationManager keyboard handler: wrap `handleDeleteSpan` in `useCallback`, use ref for `selectedSpanId` (`src/components/SpanAnnotationManager.tsx:461-535`)
 
 ## 8. Frontend — Memory Leaks & Performance
 
diff --git a/src/components/SpanAnnotationManager.tsx b/src/components/SpanAnnotationManager.tsx
index 79c85bb..8382da7 100644
--- a/src/components/SpanAnnotationManager.tsx
+++ b/src/components/SpanAnnotationManager.tsx
@@ -1,6 +1,6 @@
 'use client';
 
-import { useEffect, useState, useRef } from 'react';
+import { useEffect, useState, useRef, useCallback } from 'react';
 import { IChartApi, ISeriesApi, Time } from 'lightweight-charts';
 import { SpanRectanglePrimitive, SpanData } from './SpanRectanglePrimitive';
 import SpanPopover from './SpanPopover';
@@ -73,6 +73,12 @@ export default function SpanAnnotationManager({
   const [popoverOpen, setPopoverOpen] = useState(false);
   const [editingSpan, setEditingSpan] = useState<SpanAnnotation | null>(null);
   const primitivesRef = useRef<Map<number, SpanRectanglePrimitive>>(new Map());
+  const selectedSpanIdRef = useRef<number | null>(selectedSpanId);
+
+  // Keep selectedSpanIdRef in sync with prop
+  useEffect(() => {
+    selectedSpanIdRef.current = selectedSpanId;
+  }, [selectedSpanId]);
 
   // Find nearest candle to a timestamp
   const findNearestCandle = (timestamp: number): Candle | null => {
@@ -347,7 +353,7 @@ export default function SpanAnnotationManager({
   }, [activeTool, interactionState, previewPrimitive, series]);
 
   // Handle span deletion
-  const handleDeleteSpan = async (spanId: number) => {
+  const handleDeleteSpan = useCallback(async (spanId: number) => {
     try {
       const response = await fetch(`/api/span-annotations/${spanId}`, {
         method: 'DELETE',
@@ -355,7 +361,7 @@ export default function SpanAnnotationManager({
 
       if (response.ok) {
         onSpanAnnotationsChange();
-        if (selectedSpanId === spanId) {
+        if (selectedSpanIdRef.current === spanId) {
           onSelectedSpanChange(null);
         }
       } else {
@@ -364,7 +370,7 @@ export default function SpanAnnotationManager({
     } catch (error) {
       console.error('Error deleting span annotation:', error);
     }
-  };
+  }, [onSpanAnnotationsChange, onSelectedSpanChange]);
 
   // Handle popover save
   const handlePopoverSave = async (data: {
@@ -461,15 +467,16 @@ export default function SpanAnnotationManager({
   useEffect(() => {
     const handleKeyDown = async (e: KeyboardEvent) => {
       // Delete/Backspace: delete selected span
-      if ((e.key === 'Delete' || e.key === 'Backspace') && selectedSpanId !== null) {
+      const currentSelectedSpanId = selectedSpanIdRef.current;
+      if ((e.key === 'Delete' || e.key === 'Backspace') && currentSelectedSpanId !== null) {
         e.preventDefault();
-        handleDeleteSpan(selectedSpanId);
+        handleDeleteSpan(currentSelectedSpanId);
         return;
       }
 
       // Enter: open edit popover for selected span
-      if (e.key === 'Enter' && selectedSpanId !== null) {
-        const span = spanAnnotations.find((s) => s.id === selectedSpanId);
+      if (e.key === 'Enter' && currentSelectedSpanId !== null) {
+        const span = spanAnnotations.find((s) => s.id === currentSelectedSpanId);
         if (span) {
           setEditingSpan(span);
           setPopoverOpen(true);
@@ -532,7 +539,7 @@ export default function SpanAnnotationManager({
 
     window.addEventListener('keydown', handleKeyDown);
     return () => window.removeEventListener('keydown', handleKeyDown);
-  }, [selectedSpanId, spanAnnotations, activeTool, interactionState, startCandle, cursorCandle, spanLabelTypes, activeChartId, previewPrimitive, series, onSpanAnnotationsChange]);
+  }, [handleDeleteSpan, spanAnnotations, activeTool, interactionState, startCandle, cursorCandle, spanLabelTypes, activeChartId, previewPrimitive, series, onSpanAnnotationsChange]);
 
   // Handle double-click to edit span
   useEffect(() => {