refactor(web): replace prop-drilled daemon config with TanStack Query cache architecture; fix concurrent optimistic rollback#33094
Conversation
…eplace raw client.patch calls with generated configPatch
The PATCH /v1/config endpoint lacked requestBody and responseBody
definitions in the route metadata, causing the generated SDK to emit
body?: never for configPatch. This forced 11 call sites to use the raw
HeyAPI client.
Changes:
- Add requestBody (z.record) and responseBody ({ok: boolean}) to the
config_patch route in conversation-query-routes.ts
- Regenerate openapi.yaml via generate:openapi
- Replace all 11 raw client.patch calls across use-daemon-config.ts,
manage-profiles-modal.tsx, and call-site-overrides-modal.tsx with the
generated configPatch function
- Remove unused client import from manage-profiles-modal.tsx
Part of LUM-2072
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
…config-patch-spec
… cache architecture Redesign the daemon config mutation pattern in Settings/AI to follow STATE_MANAGEMENT.md. The TanStack Query cache is now the single source of truth for profiles, profileOrder, activeProfile, and callSites. Changes: - useDaemonConfig() exposes typed memoized slices derived from the query cache instead of components copying config into useState - useDaemonConfigMutation() centralizes all config patches in a shared useMutation with onSettled cache invalidation - LanguageModelCard reads directly from cache; removes 4 useState copies, initialized ref guard, and reconcileFromDaemonConfig usage for profiles - ManageProfilesModal calls useDaemonConfig() directly; removes onProfilesChanged callback, implements optimistic toggle/reorder via queryClient.setQueryData with rollback on error - CallSiteOverridesModal calls useDaemonConfig() directly; removes onSaved callback and persistedOverrides/orderedProfiles prop drilling - DaemonConfigReconciliation no longer carries profile fields (only used by web-search and image-gen cards for their service modes) - Dead code removed: unused captureError import, stale assistantId deps Ephemeral UI state (modal open/close, draft dropdown selections, draft call-site overrides) correctly remains in useState per STATE_MANAGEMENT.md. Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8de1dfc790
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
The Zustand store hook was called inside a useMemo callback, violating the Rules of Hooks. Hoist the hook call to the component top level and add it to the dependency array so the profile list recomputes when the flag hydrates. Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b5206968df
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
Restoring the full previousConfig snapshot on toggle failure could overwrite concurrent cache updates from other mutations. Instead, capture only the previous status value and use a setQueryData updater to patch just that field on top of the latest cache state. Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Per TanStack Query docs and TkDodo's concurrent optimistic updates guide, call cancelQueries before setQueryData to prevent a stale refetch (triggered by another mutation's onSettled invalidation) from overwriting the optimistic update. Applied to both toggle and reorder handlers. Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
…ttern The previous example restored the full query snapshot in onError, which silently overwrites concurrent mutations' optimistic updates. Updated to: (1) capture only the changed field, (2) use an updater function in onError to patch just that field back, (3) document why cancelQueries is required before every optimistic setQueryData. References: - https://tanstack.com/query/v5/docs/framework/react/guides/optimistic-updates - https://tkdodo.eu/blog/concurrent-optimistic-updates-in-react-query Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
|
@codex review |
There was a problem hiding this comment.
✦ APPROVE
Value: Closes the "useState-mirrors-server-state" pattern in the AI/Settings domain end-to-end. Query cache becomes the single source of truth for profiles / profileOrder / activeProfile / callSites; modals read from the same cache, mutations invalidate on settle, no prop drilling, no imperative onProfilesChanged callbacks, no parallel sources of truth that can drift. Also lands a real concurrent-safety fix to the optimistic rollback path — full-snapshot rollback would overwrite unrelated concurrent mutations; the new rollback patches only the failed field on top of the latest cache.
Why this matters now: #33056 decomposed ai-page.tsx into seven cards over a shared useDaemonConfig() hook with key-based dedup. This PR finishes the architecture below the card boundary — language-model-card.tsx drops 4 useState copies + initialized ref + reconcileFromDaemonConfig, modals read the same cache, and the useDaemonConfigMutation() shared hook gives every consumer the same onSettled-invalidating mutation surface. The two PRs together complete the TanStack-as-source-of-truth pattern for daemon config.
What this does
use-daemon-config.ts— exposesprofiles/profileOrder/activeProfile/callSitesas memoized selectors over the same query data; addsuseDaemonConfigMutation()wrappingconfigPatchinuseMutationwithonSettledinvalidation. All consumers share ONE cache entry, one mutation surface.language-model-card.tsx-82/+40 — 4useStatecopies of server state deleted,initializedref guard gone,reconcileFromDaemonConfigfor profiles removed. Cache reads directly.draftActiveProfilecorrectly kept asuseState— that's unsaved dropdown selection, an ephemeral UI state, not server state. The diff comment "ephemeral UI state, correct as useState" makes that boundary explicit.manage-profiles-modal.tsx-303/+161 — biggest single delta. Modal callsuseDaemonConfig()directly;onProfilesChangedcallback deleted; optimistic toggle/reorder viacancelQueries+setQueryDataupdater + targeted field rollback on error. 8x directconfigPatch()calls collapsed touseDaemonConfigMutation().call-site-overrides-modal.tsx-78/+23 — same treatment: cache read direct,onSaved/persistedOverrides/orderedProfilesprops deleted, mutations via shared hook.ai-types.ts/ai-utils.ts—DaemonConfigReconciliationprofile fields removed (no longer needed; web-search / image-gen service modes remain on the reconciliation path).- Spec fix (commit 1,
b83f45d9):PATCH /v1/confighad norequestBody/responseBodydefinitions — generated SDK was emittingbody?: never. NewrequestBody: z.record(z.unknown())+responseBody: { ok: boolean }. This is the prerequisite that lets the rest of the refactor use the generated client. STATE_MANAGEMENT.md— "Via the Cache" example updated: full-snapshot rollback → targeted field rollback with updater functions; documents whycancelQueriesis required before every optimisticsetQueryData. Future-proofs the next reviewer who hits this pattern.
Anti-pattern check
- No render-phase ref mutations in any modified file.
- No runtime-boundary
ascasts added. Grepped every ADDED line in the 9-file diff: zero non-commentashits. - Rules of Hooks — caught and fixed mid-PR (Codex P2 #1, see below).
useAssistantFeatureFlagStore.use.queryComplexityRouting()correctly hoisted to component top level and added to theuseMemodeps. Verified at HEAD line 277, deps line 292. - Concurrent-safe optimistic update lifecycle — every optimistic write is preceded by
cancelQueries(toggle line 315, reorder line 462). Rollback patches only the failed field (previousStatusline 317 → updater at line 347 sets just.status). This is structurally the same pattern #33046 settled on for contacts — refetch-failure resilience without overwriting concurrent successful writes. - TanStack canonical mutation shape —
useMutationwithonSettledinvalidation as the baseline; per-callsetQueryDataupdaters only where optimistic UX is worth the complexity. Matches STATE_MANAGEMENT.md as updated in this PR.
Per-bot findings — verification at HEAD 66eead5a
- Codex P2 #1 (manage-profiles-modal.tsx:291) — "Subscribe to the routing flag before memoizing profiles" — FIXED in
b5206968df. Devin reviewer's reply correctly identified this as actually worse than a stale-dep — calling a Zustand store hook insideuseMemoviolates Rules of Hooks. Hook now hoisted to top of component, included in deps. Verified at HEAD. - Codex P2 #2 (manage-profiles-modal.tsx:337) — "Roll back only the toggled profile status" — FIXED in
3d6fb67c91. Toggle rollback now capturespreviousStatusonly and uses asetQueryDataupdater that patches just.statuson top of the latest cache, matching the reorder rollback'sprofileOrdernarrowing. Verified at HEAD —previousStatuscapture at line 317, narrow rollback at line 347. - Devin Review at sha 1
b83f45d9"No Issues Found" with 3 dashboard findings. The 3 dashboard findings drove the audit commits (b5206968,3d6fb67c). All landed. - Final docs commit
66eead5aupdatesSTATE_MANAGEMENT.mdto document the concurrent-safe rollback pattern — Codex hasn't returned verdict at this SHA yet, but the doc-only delta over3d6fb67cis mechanical (the code Codex green-lit in the prior round is untouched).
Merge gate
| Gate | Status |
|---|---|
| Vex APPROVE | ✅ (this review) |
| Second approval | ⏳ Codex still processing at HEAD (👀 reaction; Boss triggered at 21:04 vs HEAD at 20:59). Will merge when Codex returns 👍 — won't re-trigger. |
| CI checks | ✅ all green (Lint / Type Check / Test / Build / FlexFrame / OpenAPI / 2× Socket) |
| Outstanding REQUEST_CHANGES | ✅ none |
| Open inline comments | ✅ both Codex P2s marked resolved by Devin commits |
Vellum Constitution — Distinct: the value here is treating "concurrent optimistic rollback" as a first-class invariant, not an edge case. Most TanStack guides stop at "snapshot → optimistic → restore-on-error" — that's safe in isolation but unsafe under real concurrent mutation. Patching only the failed field on top of the latest cache is the right shape, and codifying it in STATE_MANAGEMENT.md means the next reviewer doesn't have to re-derive it.
|
Codex Review: Didn't find any major issues. More of your lovely PRs please. ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback". |
Prompt / plan
Redesign the daemon config mutation pattern in Settings/AI to follow STATE_MANAGEMENT.md principles. The TanStack Query cache becomes the single source of truth for profiles, profileOrder, activeProfile, and callSites — eliminating prop-drilling, imperative callbacks, and scattered direct API calls.
Also includes the prerequisite spec fix:
PATCH /v1/configlackedrequestBody/responseBodydefinitions, causing the generated SDK to emitbody?: never.Why needed
The previous architecture copied server state into
useState, thenprop-drilled it into modals with imperative
onProfilesChangedcallbacksto sync mutations back to the parent. This created a parallel source of
truth that could drift from the query cache, and made concurrent mutations
unsafe (a failed toggle rollback would restore a full snapshot, silently
reverting unrelated edits).
What was wrong
What the correct architecture is
All consumers read from the same query cache. Mutations invalidate on settle → all consumers re-render. No callbacks, no prop drilling of server state.
Changes
use-daemon-config.ts— exposeprofiles,profileOrder,activeProfile,callSitesas memoized selectors; adduseDaemonConfigMutation()wrappingconfigPatchinuseMutationwithonSettledinvalidationlanguage-model-card.tsx— remove 4xuseState,initializedref,reconcileFromDaemonConfigfor profiles; read from cache; keepdraftActiveProfileas ephemeral UI state (unsaved dropdown ≠ server state)manage-profiles-modal.tsx— calluseDaemonConfig()directly; removeonProfilesChangedcallback; implement optimistic toggle/reorder viacancelQueries+setQueryData+ targeted field rollback on errorcall-site-overrides-modal.tsx— calluseDaemonConfig()directly; removeonSaved/persistedOverrides/orderedProfilesprops; save/reset viauseDaemonConfigMutation()ai-types.ts/ai-utils.ts— remove profile fields fromDaemonConfigReconciliation(no longer needed; web-search/image-gen service modes remain)conversation-query-routes.ts— addrequestBody(z.record) andresponseBody({ok: boolean}) toconfig_patchrouteSTATE_MANAGEMENT.md— update "Via the Cache" example: full-snapshot rollback → targeted field rollback with updater functions, document whycancelQueriesis required before every optimisticsetQueryDataConcurrent-safe optimistic updates
Toggle and reorder use
cancelQueriesbefore optimistic writes to preventstale refetches from overwriting the optimistic cache. Rollback patches
only the specific field that failed (e.g. a profile's
status), not thefull config snapshot — this preserves concurrent mutations' successful
updates. This follows TkDodo's concurrent optimistic updates guidance.
Alternatives NOT taken
Separate
useMutationper operation (toggle, reorder, delete) — wouldgive cleaner
onMutate/onErrorlifecycle, butonMutatecan't bepassed per-call to
mutateAsync(TanStack Query v5), and the per-itemtracking (
togglingNames,deleting) doesn't map to a single mutation'sisPending. The shareduseDaemonConfigMutation+ manual try/catch isthe pragmatic choice given the shared mutation serves 4+ distinct
operation types.
"Via the UI" optimistic pattern — considered for toggle since
mutation/query live in the same component, but rejected because
LanguageModelCard(a sibling component, mounted behind the modal) alsoreads profiles from the cache and needs to see status changes immediately.
What stays as-is (by design)
useStateis correctuseStateis correctreconcileFromDaemonConfigfor web-search/image-gen modes → still used by those cardsRoot cause analysis
How did the code get into this state? The original
ai-page.tsxgodcomponent managed all daemon config in
useStateat the top level.When modals were added, the natural path was prop-drilling the state
down and using callbacks to propagate changes back up. This worked
for a single component tree but created a parallel source of truth
alongside the TanStack Query cache.
What led to the concurrent mutation bug? The optimistic toggle
stored a full config snapshot in
onMutatecontext and restored itin
onError. This pattern comes from the TanStack Query docs' basicexample, which doesn't account for concurrent mutations. Our
STATE_MANAGEMENT.md example had the same gap (now fixed in this PR).
Prevention: Updated STATE_MANAGEMENT.md "Via the Cache" example to
document targeted field rollback +
cancelQueriesas the standardpattern, with references to TkDodo's concurrent optimistic updates guide.
References
Test plan
bunx tsc --noEmit— passes (0 errors)bun run lint— passes (0 errors in touched files)bun test src/domains/settings/ai/— 8/8 passLink to Devin session: https://app.devin.ai/sessions/b87fe17fe84348b89321863e56a947e4
Requested by: @ashleeradka