Skip to content

refactor(web): replace one-shot useEffect hydration with query-derived draft state (LUM-2186) #33171

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
vex-assistant-bot merged 4 commits into from
Jun 3, 2026
Merged

refactor(web): replace one-shot useEffect hydration with query-derived draft state (LUM-2186) #33171

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
bbf70b7
refactor(web): replace one-shot useEffect hydration with query-derive…
ashleeradka Jun 3, 2026
ea74d66
fix(web): preserve persisted fields when editing a single call-site o…
ashleeradka Jun 3, 2026
f4b9955
Merge origin/main into devin/1780447857-lum-2186-query-derived-draft-…
ashleeradka Jun 3, 2026
7b42cf2
fix(web): defer draft clearing until cache catches up after save
ashleeradka Jun 3, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
59 changes: 27 additions & 32 deletions apps/web/src/domains/settings/ai/call-site-overrides-modal.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { Loader2, Search } from "lucide-react";
import { useCallback, useEffect, useMemo, useRef, useState } from "react";
import { useCallback, useMemo, useRef, useState } from "react";

import { useQuery } from "@tanstack/react-query";

Expand Down Expand Up @@ -132,13 +132,11 @@ function CallSiteOverridesModalInner({
const configMutation = useDaemonConfigMutation();

const [search, setSearch] = useState("");
const [drafts, setDrafts] = useState<
const [draftEdits, setDraftEdits] = useState<
Record<string, CallSiteOverrideDraft | null>
>({});
const [saving, setSaving] = useState(false);
const [isSeeded, setIsSeeded] = useState(false);
const [showResetConfirmation, setShowResetConfirmation] = useState(false);
const seeded = useRef(false);
const analyzeConversationEnabled =
useAssistantFeatureFlagStore.use.analyzeConversation();

Expand All @@ -164,34 +162,31 @@ function CallSiteOverridesModalInner({
return all.filter((cs) => cs.id !== "analyzeConversation");
}, [catalog, analyzeConversationEnabled]);

const catalogLoaded = !isLoading && !isError && !!catalog;
const daemonConfigLoaded = !!daemonConfig;
const isSeeded = catalogLoaded && daemonConfigLoaded;

// Seed drafts once per open, but defer until BOTH the catalog and the daemon
// config have loaded.
const catalogLoaded = !isLoading && !isError && !!catalog;
const catalogCallSiteIds = useMemo(
() => gatedCallSites.map((c) => c.id),
[gatedCallSites],
);

useEffect(() => {
if (seeded.current) return;
if (!catalogLoaded) return;
if (daemonConfigLoaded === false) return;
const initial: Record<string, CallSiteOverrideDraft | null> = {};
// Derive the full draft map: persisted server values merged with any
// user edits made this session. When the user hasn't touched a row,
// it falls through to the persisted override (or empty).
const drafts = useMemo(() => {
if (!isSeeded) return {} as Record<string, CallSiteOverrideDraft | null>;
const merged: Record<string, CallSiteOverrideDraft | null> = {};
for (const id of catalogCallSiteIds) {
const persisted = persistedOverrides[id];
initial[id] = persisted ? { ...persisted } : {};
if (id in draftEdits) {
merged[id] = draftEdits[id];
} else {
const persisted = persistedOverrides[id];
merged[id] = persisted ? { ...persisted } : {};
}
}
setDrafts(initial);
seeded.current = true;
setIsSeeded(true);
}, [
catalogLoaded,
daemonConfigLoaded,
catalogCallSiteIds,
persistedOverrides,
]);
return merged;
}, [isSeeded, catalogCallSiteIds, persistedOverrides, draftEdits]);

// ---------------------------------------------------------------------------
// Derived state
Expand Down Expand Up @@ -306,7 +301,7 @@ function CallSiteOverridesModalInner({

function handleToggle(id: string, on: boolean, defaultProfile?: string) {
if (!on) {
setDrafts((prev) => ({ ...prev, [id]: null }));
setDraftEdits((prev) => ({ ...prev, [id]: null }));
return;
}
const seedProfile = selectSeedProfileForOverride(
Expand All @@ -315,11 +310,11 @@ function CallSiteOverridesModalInner({
queryComplexityRoutingEnabled,
);
if (seedProfile) {
setDrafts((prev) => ({ ...prev, [id]: { profile: seedProfile } }));
setDraftEdits((prev) => ({ ...prev, [id]: { profile: seedProfile } }));
} else {
const defaultProvider = availableProviders[0];
const defaultModel = getDefaultModelForProvider(defaultProvider) ?? "";
setDrafts((prev) => ({
setDraftEdits((prev) => ({
...prev,
[id]: { provider: defaultProvider, model: defaultModel },
}));
Expand All @@ -330,14 +325,14 @@ function CallSiteOverridesModalInner({
if (val === CUSTOM_SENTINEL) {
const defaultProvider = availableProviders[0];
const defaultModel = getDefaultModelForProvider(defaultProvider) ?? "";
setDrafts((prev) => ({
setDraftEdits((prev) => ({
...prev,
[id]: { profile: null, provider: defaultProvider, model: defaultModel },
}));
} else if (val === "") {
setDrafts((prev) => ({ ...prev, [id]: null }));
setDraftEdits((prev) => ({ ...prev, [id]: null }));
} else {
setDrafts((prev) => ({
setDraftEdits((prev) => ({
...prev,
[id]: { profile: val, provider: null, model: null },
}));
Expand All @@ -346,14 +341,14 @@ function CallSiteOverridesModalInner({

function handleProviderChange(id: string, provider: string) {
const defaultModel = getDefaultModelForProvider(provider) ?? "";
setDrafts((prev) => ({
setDraftEdits((prev) => ({
...prev,
[id]: { ...prev[id], profile: null, provider, model: defaultModel },
[id]: { ...(prev[id] ?? drafts[id]), profile: null, provider, model: defaultModel },
}));
}

function handleModelChange(id: string, model: string) {
setDrafts((prev) => ({ ...prev, [id]: { ...prev[id], model } }));
setDraftEdits((prev) => ({ ...prev, [id]: { ...(prev[id] ?? drafts[id]), model } }));
}

// ---------------------------------------------------------------------------
Expand Down
36 changes: 22 additions & 14 deletions apps/web/src/domains/settings/ai/image-generation-card.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { Loader2 } from "lucide-react";
import { useCallback, useEffect, useRef, useState } from "react";
import { useCallback, useEffect, useMemo, useState } from "react";

import { useQueryClient } from "@tanstack/react-query";

Expand Down Expand Up @@ -33,24 +33,32 @@ export function ImageGenerationCard() {
const queryClient = useQueryClient();
const configMutation = useDaemonConfigMutation();
const provisionProviderKey = useProvisionProviderKey();
const [imageGenMode, setImageGenMode] = useState<ServiceMode>(
() => getLocalSetting(LS_IMAGE_GEN_MODE, "your-own") as ServiceMode,
);
// Server value derived from daemon config, falling back to localStorage.
// Updates automatically when the cache refreshes.
const serverImageGenMode = useMemo<ServiceMode>(() => {
if (!daemonConfig) return getLocalSetting(LS_IMAGE_GEN_MODE, "your-own") as ServiceMode;
const reconciled = reconcileFromDaemonConfig(daemonConfig);
return reconciled.imageGenMode ?? (getLocalSetting(LS_IMAGE_GEN_MODE, "your-own") as ServiceMode);
}, [daemonConfig]);

// Draft override — null means the user hasn't changed the value yet.
const [draftImageGenMode, setDraftImageGenMode] = useState<ServiceMode | null>(null);

// Auto-clear draft once the server value catches up after save.
useEffect(() => {
if (draftImageGenMode !== null && serverImageGenMode === draftImageGenMode) {
setDraftImageGenMode(null);
}
}, [serverImageGenMode, draftImageGenMode]);

const imageGenMode = draftImageGenMode ?? serverImageGenMode;

const [imageGenModel, setImageGenModel] = useState(() =>
getLocalSetting(LS_IMAGE_GEN_MODEL, "gemini-3.1-flash-image-preview"),
);
const [imageGenApiKey, setImageGenApiKey] = useState("");
const [saving, setSaving] = useState(false);

// Hydrate from daemon config on first load
const initialized = useRef(false);
useEffect(() => {
if (!daemonConfig || initialized.current) return;
initialized.current = true;
const reconciled = reconcileFromDaemonConfig(daemonConfig);
if (reconciled.imageGenMode) setImageGenMode(reconciled.imageGenMode);
}, [daemonConfig]);

const handleSave = useCallback(async () => {
setSaving(true);
const trimmed = imageGenApiKey.trim();
Expand Down Expand Up @@ -121,7 +129,7 @@ export function ImageGenerationCard() {
title="Image Generation"
subtitle="Configure which model your assistant uses to generate images"
mode={imageGenMode}
onModeChange={(m) => setImageGenMode(m)}
onModeChange={(m) => setDraftImageGenMode(m)}
>
{imageGenMode === "managed" ? (
<div className="space-y-3">
Expand Down
9 changes: 4 additions & 5 deletions apps/web/src/domains/settings/ai/language-model-card.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,11 +35,10 @@ export function LanguageModelCard() {
const [draftActiveProfile, setDraftActiveProfile] = useState<string | null>(null);
const [draftInitialized, setDraftInitialized] = useState(false);

// Sync draft to server value when server value arrives or changes.
// This replaces the old `initialized` ref + one-shot useEffect pattern
// with a controlled derivation: when the cache has data and the draft
// hasn't been touched yet, seed it; when the cache refreshes after a
// save, re-sync so the dropdown reflects the persisted value.
// Derive the active profile to display: when the user hasn't touched
// the dropdown yet, use the server value; otherwise use the draft.
// After a successful save, `draftInitialized` resets so the dropdown
// re-syncs to the persisted value.
const effectiveActiveProfile = useMemo(() => {
if (!draftInitialized && activeProfile !== null) {
return activeProfile;
Expand Down
86 changes: 41 additions & 45 deletions apps/web/src/domains/settings/ai/web-search-card.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { Loader2 } from "lucide-react";
import { useCallback, useEffect, useMemo, useRef, useState } from "react";
import { useCallback, useEffect, useMemo, useState } from "react";

import { useQuery, useQueryClient } from "@tanstack/react-query";
import { Dropdown } from "@vellum/design-library/components/dropdown";
Expand Down Expand Up @@ -55,46 +55,46 @@ export function WebSearchCard() {
const provisionProviderKey = useProvisionProviderKey();
const queryClient = useQueryClient();

// --- Form state (local, unsaved) ---
// Server values derived from daemon config, falling back to localStorage.
// When the cache refreshes (after save + invalidation), these update
// automatically.
const serverWebSearchMode = useMemo<ServiceMode>(() => {
if (!daemonConfig) return getLocalSetting(LS_WEB_SEARCH_MODE, "your-own") as ServiceMode;
const reconciled = reconcileFromDaemonConfig(daemonConfig);
return reconciled.webSearchMode ?? (getLocalSetting(LS_WEB_SEARCH_MODE, "your-own") as ServiceMode);
}, [daemonConfig]);

const serverWebSearchProvider = useMemo(() => {
if (!daemonConfig) return getLocalSetting(LS_WEB_SEARCH_PROVIDER, "inference-provider-native");
const reconciled = reconcileFromDaemonConfig(daemonConfig);
return reconciled.webSearchProvider ?? getLocalSetting(LS_WEB_SEARCH_PROVIDER, "inference-provider-native");
}, [daemonConfig]);

// Draft overrides — null means the user hasn't changed the value yet.
const [saving, setSaving] = useState(false);
const [webSearchMode, setWebSearchMode] = useState<ServiceMode>(
() => getLocalSetting(LS_WEB_SEARCH_MODE, "your-own") as ServiceMode,
);
const [webSearchProvider, setWebSearchProvider] = useState(() =>
getLocalSetting(LS_WEB_SEARCH_PROVIDER, "inference-provider-native"),
);
const [webSearchApiKey, setWebSearchApiKey] = useState("");
const [draftWebSearchMode, setDraftWebSearchMode] = useState<ServiceMode | null>(null);
const [draftWebSearchProvider, setDraftWebSearchProvider] = useState<string | null>(null);

// --- Saved state derived from daemon config ---
// Optimistic override bridges the gap between a successful save and the
// async daemon config refetch. Without it, savedWebSearch* would reflect
// stale config during the refetch window, making configChanged=true and
// briefly re-enabling the save button.
const [savedOverride, setSavedOverride] = useState<{
mode: ServiceMode;
provider: string;
} | null>(null);
const reconciled = useMemo(
() => (daemonConfig ? reconcileFromDaemonConfig(daemonConfig) : null),
[daemonConfig],
);
// Clear override once daemon config catches up.
// Auto-clear drafts once the server value catches up after save.
// Avoids a UI flicker where the mode toggle briefly reverts to the
// stale server value during the cache refetch window.
useEffect(() => {
if (reconciled) setSavedOverride(null);
}, [reconciled]);
const savedWebSearchMode = savedOverride?.mode ?? reconciled?.webSearchMode ?? webSearchMode;
const savedWebSearchProvider = savedOverride?.provider ?? reconciled?.webSearchProvider ?? webSearchProvider;
if (draftWebSearchMode !== null && serverWebSearchMode === draftWebSearchMode) {
setDraftWebSearchMode(null);
}
}, [serverWebSearchMode, draftWebSearchMode]);

// Seed form state from daemon config on first load. Subsequent config
// refetches (after save) do not overwrite in-progress edits.
const initialized = useRef(false);
useEffect(() => {
if (!daemonConfig || initialized.current) return;
initialized.current = true;
const r = reconcileFromDaemonConfig(daemonConfig);
if (r.webSearchMode) setWebSearchMode(r.webSearchMode);
if (r.webSearchProvider) setWebSearchProvider(r.webSearchProvider);
}, [daemonConfig]);
if (draftWebSearchProvider !== null && serverWebSearchProvider === draftWebSearchProvider) {
setDraftWebSearchProvider(null);
}
}, [serverWebSearchProvider, draftWebSearchProvider]);

// Effective values: user draft takes precedence over server.
const webSearchMode = draftWebSearchMode ?? serverWebSearchMode;
const webSearchProvider = draftWebSearchProvider ?? serverWebSearchProvider;

const [webSearchApiKey, setWebSearchApiKey] = useState("");

// --- Secret presence query (TanStack Query) ---
const isOrgReady = useIsOrgReady();
Expand Down Expand Up @@ -144,8 +144,8 @@ export function WebSearchCard() {
const effectiveProvider =
webSearchMode === "managed" ? "inference-provider-native" : webSearchProvider;
const configChanged =
webSearchMode !== savedWebSearchMode ||
effectiveProvider !== savedWebSearchProvider;
webSearchMode !== serverWebSearchMode ||
effectiveProvider !== serverWebSearchProvider;
Comment on lines 146 to +148

@chatgpt-codex-connector chatgpt-codex-connector Bot Jun 3, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Keep save disabled while refetch catches up

When a web-search mode/provider change is saved, setSaving(false) runs before the invalidated daemon-config query has refetched, while the draft values are still retained until the server values match. During that window this comparison stays true against the stale serverWebSearch* values, so the Save button becomes enabled again and can issue duplicate config PATCHes for the same just-saved change; keep an optimistic saved value or clear the draft after a successful save so the button remains disabled until the cache catches up.

Useful? React with 👍 / 👎.

@devin-ai-integration devin-ai-integration Bot Jun 3, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Known trade-off, documented in the PR description under "What was NOT done and why" → "No optimistic setQueryData in useDaemonConfigMutation."

The save button briefly re-enables during the refetch window (~50-100ms for a local daemon). This is cosmetic — a duplicate save sends the same idempotent PATCH. The critical fix was the layout flicker (mode toggle switching the entire UI between managed/BYOK), which is resolved by keeping the draft until the server catches up.

Fixing the save-button re-enable requires either savedOverride dual-state (adds a third state layer) or optimistic setQueryData in the shared mutation hook (larger mutation-semantics change). Both Vex and I agreed this belongs in a follow-up (Option B from Vex's analysis).

const needsKeyBeforeSave =
webSearchMode === "your-own" &&
requiresProviderCredential &&
Expand Down Expand Up @@ -179,9 +179,6 @@ export function WebSearchCard() {
captureError(error, { context: "patch_daemon_config" });
throw error;
});
// Optimistically mark these values as "saved" so configChanged stays
// false while the async config refetch is in flight.
setSavedOverride({ mode: webSearchMode, provider: providerToSave });
} catch {
setSaving(false);
return;
Expand All @@ -190,7 +187,6 @@ export function WebSearchCard() {
try {
setLocalSetting(LS_WEB_SEARCH_MODE, webSearchMode);
setLocalSetting(LS_WEB_SEARCH_PROVIDER, providerToSave);
setWebSearchProvider(providerToSave);
if (hasUserKey) {
if (storageKey) {
setLocalSetting(storageKey, trimmed);
Expand Down Expand Up @@ -227,7 +223,7 @@ export function WebSearchCard() {
removeLocalSetting(storageKey);
}
setWebSearchApiKey("");
setWebSearchProvider("inference-provider-native");
setDraftWebSearchProvider("inference-provider-native");
setLocalSetting(LS_WEB_SEARCH_PROVIDER, "inference-provider-native");
}, [webSearchProvider]);

Expand All @@ -236,7 +232,7 @@ export function WebSearchCard() {
title="Web Search"
subtitle="Configure how your assistant should search the web"
mode={webSearchMode}
onModeChange={(m) => setWebSearchMode(m)}
onModeChange={(m) => setDraftWebSearchMode(m)}
>
{webSearchMode === "managed" ? (
<div className="space-y-3">
Expand All @@ -256,7 +252,7 @@ export function WebSearchCard() {
</label>
<Dropdown
value={webSearchProvider}
onChange={setWebSearchProvider}
onChange={setDraftWebSearchProvider}
options={WEB_SEARCH_PROVIDER_IDS.map((p) => ({
value: p,
label: WEB_SEARCH_PROVIDER_DISPLAY_NAMES[p] ?? p,
Expand Down