From 810b2e27f3f5d81087e27bd9ebd0e42c75def3b9 Mon Sep 17 00:00:00 2001 From: Roland Tannous Date: Sun, 24 May 2026 14:02:10 +0400 Subject: [PATCH] Studio: fix React #185 update-depth loop in RAG additions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two useEffects added in Phase 2C and Phase 4 followed the anti-pattern of calling a state setter inside the effect and listing the setter's output in the dep array. Both could re-fire indefinitely when the selected slice changed shape on each render — which the chat page hit on first load. chat-settings-sheet.tsx (thread settings loader) - Before: `useEffect(load, [..., threadSettings, ...])` with `if (!threadSettings) load()` inside. After load, the Zustand selector returned a freshly-constructed slice, dep changed, effect re-ran. If anything in between caused threadSettings to briefly flicker undefined (e.g. a race during initial hydration or a fast subsequent thread switch), the load fired again and the cycle repeated. - After: ref-guarded by activeThreadId — `threadSettingsLoadedRef` tracks which threadId has been loaded; the effect deps shrink to `[ragSource.kind, activeThreadId, loadThreadSettings]`, all stable per-thread, removing the feedback loop. ingestion-toast-stack.tsx (terminal-job auto-dismiss) - Before: `useEffect(..., [jobs, dismissedJobs])` with `setDismissedJobs(prev => new Set(prev).add(jobId))` inside the scheduled setTimeout. Each setter creates a new Set reference; the dep change re-triggers the effect, which clears and reschedules timers. Under fast SSE event arrival or a strict-mode double-mount, the scheduler runs faster than its cleanup and React caps the depth. - After: dismissedJobs is read via a ref (kept in sync at the top of the component); the effect only depends on `[jobs]`. A `scheduledJobsRef` prevents duplicate timer scheduling for the same job across multiple effect runs, and the setDismissedJobs updater no-ops when the job is already dismissed. No behavior change for the happy path — toasts still auto-dismiss after DISMISS_DELAY_MS; thread settings still load on first sight of a thread. --- .../src/features/chat/chat-settings-sheet.tsx | 16 ++++++++++++---- .../rag/components/ingestion-toast-stack.tsx | 19 +++++++++++++++---- 2 files changed, 27 insertions(+), 8 deletions(-) diff --git a/studio/frontend/src/features/chat/chat-settings-sheet.tsx b/studio/frontend/src/features/chat/chat-settings-sheet.tsx index f954670c42..439b6088c7 100644 --- a/studio/frontend/src/features/chat/chat-settings-sheet.tsx +++ b/studio/frontend/src/features/chat/chat-settings-sheet.tsx @@ -460,13 +460,21 @@ export function ChatSettingsPanel({ const updateThreadSettings = useRagStore((s) => s.updateThreadSettings); const ragDefaults = useRagStore((s) => s.defaults); - // Load this thread's RAG settings once when the sheet sees a thread - // for the first time. Updates re-render automatically via the store. + // Load this thread's RAG settings once per threadId. Ref-guarded so + // `threadSettings` isn't a dep — if it were, the post-load + // store-mutation re-triggers the effect and any failure mode where + // the selector flickers undefined produces an update loop. + const threadSettingsLoadedRef = useRef(null); useEffect(() => { - if (ragSource.kind === "thread" && activeThreadId && !threadSettings) { + if ( + ragSource.kind === "thread" + && activeThreadId + && threadSettingsLoadedRef.current !== activeThreadId + ) { + threadSettingsLoadedRef.current = activeThreadId; void loadThreadSettings(activeThreadId); } - }, [ragSource.kind, activeThreadId, threadSettings, loadThreadSettings]); + }, [ragSource.kind, activeThreadId, loadThreadSettings]); const effectiveThreadChunking: RagChunkingStrategy = threadSettings?.chunking_strategy ?? diff --git a/studio/frontend/src/features/rag/components/ingestion-toast-stack.tsx b/studio/frontend/src/features/rag/components/ingestion-toast-stack.tsx index ad824c7111..7e35180280 100644 --- a/studio/frontend/src/features/rag/components/ingestion-toast-stack.tsx +++ b/studio/frontend/src/features/rag/components/ingestion-toast-stack.tsx @@ -5,7 +5,7 @@ import { Button } from "@/components/ui/button"; import { Cancel01Icon } from "@hugeicons/core-free-icons"; import { HugeiconsIcon } from "@hugeicons/react"; import { AnimatePresence, motion, useReducedMotion } from "motion/react"; -import { useEffect, useState } from "react"; +import { useEffect, useRef, useState } from "react"; import { useRagStore } from "../stores/rag-store"; import { IngestionProgress } from "./ingestion-progress"; @@ -33,16 +33,27 @@ export function IngestionToastStack() { ); // Schedule auto-dismiss for jobs that have reached a terminal state. + // Ref-tracked so `dismissedJobs` isn't a useEffect dep — the setter + // fires *inside* the effect, and depending on its output here is a + // recipe for update-depth loops if the scheduler ever runs faster + // than the cleanup. We snapshot the latest dismissed set into a ref + // and read from it inside the scheduling loop instead. + const scheduledJobsRef = useRef>(new Set()); + const dismissedJobsRef = useRef>(dismissedJobs); + dismissedJobsRef.current = dismissedJobs; useEffect(() => { const timers: ReturnType[] = []; for (const [jobId, event] of Object.entries(jobs)) { if ( - (event.type === "complete" || event.type === "error") && - !dismissedJobs.has(jobId) + (event.type === "complete" || event.type === "error") + && !dismissedJobsRef.current.has(jobId) + && !scheduledJobsRef.current.has(jobId) ) { + scheduledJobsRef.current.add(jobId); timers.push( setTimeout(() => { setDismissedJobs((prev) => { + if (prev.has(jobId)) return prev; const next = new Set(prev); next.add(jobId); return next; @@ -52,7 +63,7 @@ export function IngestionToastStack() { } } return () => timers.forEach(clearTimeout); - }, [jobs, dismissedJobs]); + }, [jobs]); const visible = Object.entries(jobs).filter( ([jobId]) => !dismissedJobs.has(jobId),