From acf31d3edf4b7d3318f2d086347c9f98c316e570 Mon Sep 17 00:00:00 2001 From: Marko Djordjevic Date: Wed, 18 Feb 2026 15:21:46 +0100 Subject: [PATCH] perf: split SpanAnnotationManager reconciliation into two effects MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Separate the single reconciliation effect into: 1. Full reconciliation effect — runs only when spanAnnotations list, series, chart, or candles change. Rebuilds all primitives. 2. Selection-only effect — runs only when selectedSpanId changes. Calls setSelected() on existing primitives instead of detaching and reattaching all of them. This avoids tearing down and rebuilding every primitive on each selection change, which was the main unnecessary overhead. Co-Authored-By: Claude Sonnet 4.6 --- openspec/changes/code-review-fix/tasks.md | 2 +- src/components/SpanAnnotationManager.tsx | 13 ++++++++++--- 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/openspec/changes/code-review-fix/tasks.md b/openspec/changes/code-review-fix/tasks.md index e48de15..0947a31 100644 --- a/openspec/changes/code-review-fix/tasks.md +++ b/openspec/changes/code-review-fix/tasks.md @@ -73,7 +73,7 @@ - [x] 8.1 `[opus]` Fix SpanAnnotationManager preview primitive memory leak: replace `useState` with `useRef` for preview primitive, add cleanup on unmount (`src/components/SpanAnnotationManager.tsx:265-324`) - [x] 8.2 `[sonnet]` Use `chart.applyOptions()` for theme changes instead of re-creating chart (`src/components/CandleChart.tsx:333`) - [x] 8.3 `[sonnet]` Remove `fitContent()` from reconciliation effect, only call on initial load (`src/components/SpanAnnotationManager.tsx:160`) -- [ ] 8.4 `[opus]` Implement incremental primitive updates in SpanAnnotationManager: update only selection state on selection change, full reconciliation only on annotation list changes (`src/components/SpanAnnotationManager.tsx:104-161`) +- [x] 8.4 `[opus]` Implement incremental primitive updates in SpanAnnotationManager: update only selection state on selection change, full reconciliation only on annotation list changes (`src/components/SpanAnnotationManager.tsx:104-161`) - [ ] 8.5 `[sonnet]` Add bounded prediction cache (max 100 entries, FIFO eviction) to `predictionCacheRef` (`src/app/page.tsx:195-199`) - [ ] 8.6 `[sonnet]` Fix hardcoded 1-minute candle interval: detect interval from data, use for span iteration (`src/components/CandleChart.tsx:452`) - [ ] 8.7 `[haiku]` Extract magic numbers (8px, 60s, colors) to named constants in `CandleChart.tsx` diff --git a/src/components/SpanAnnotationManager.tsx b/src/components/SpanAnnotationManager.tsx index 1fa0e0f..8f559bc 100644 --- a/src/components/SpanAnnotationManager.tsx +++ b/src/components/SpanAnnotationManager.tsx @@ -121,7 +121,7 @@ export default function SpanAnnotationManager({ return { max_high, min_low }; }; - // Render span annotations as primitives + // Full reconciliation: rebuild all primitives when annotation list changes useEffect(() => { if (!series || !chart) return; @@ -170,13 +170,20 @@ export default function SpanAnnotationManager({ const primitive = new SpanRectanglePrimitive({ data: spanData, - isSelected: span.id === selectedSpanId, + isSelected: span.id === selectedSpanIdRef.current, }); series.attachPrimitive(primitive); primitivesRef.current.set(span.id, primitive); }); - }, [spanAnnotations, selectedSpanId, series, chart, candles]); + }, [spanAnnotations, series, chart, candles]); + + // Selection-only effect: update visual state without rebuilding primitives + useEffect(() => { + primitivesRef.current.forEach((primitive, spanId) => { + primitive.setSelected(spanId === selectedSpanId); + }); + }, [selectedSpanId]); // Fit chart content only on initial load when chart and series become available useEffect(() => {