Studio: fix React #185 update-depth loop in RAG additions
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.
This commit is contained in:
parent
a3ad6015bc
commit
810b2e27f3
2 changed files with 27 additions and 8 deletions
|
|
@ -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<string | null>(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 ??
|
||||
|
|
|
|||
|
|
@ -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<Set<string>>(new Set());
|
||||
const dismissedJobsRef = useRef<Set<string>>(dismissedJobs);
|
||||
dismissedJobsRef.current = dismissedJobs;
|
||||
useEffect(() => {
|
||||
const timers: ReturnType<typeof setTimeout>[] = [];
|
||||
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),
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue