refactor(web): dissolve reconcileFromDaemonConfig bridge utility (LUM-2239)#33464
Conversation
…-2239) The function extracted service mode/provider fields from the daemon config into a flat DaemonConfigReconciliation interface. Now that DaemonConfig is hand-typed and consumers have direct access to the config via useDaemonConfigQuery(), the indirection adds no value. Inline direct field access in both consumers (web-search-card, image-generation-card), delete the function and the DaemonConfigReconciliation type. Closes LUM-2239 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:
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2ed20e7ffe
ℹ️ 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".
| return { | ||
| serverWebSearchMode: (reconciled.webSearchMode ?? getLocalSetting(LS_WEB_SEARCH_MODE, "your-own")) as ServiceMode, | ||
| serverWebSearchProvider: reconciled.webSearchProvider ?? getLocalSetting(LS_WEB_SEARCH_PROVIDER, "inference-provider-native"), | ||
| serverWebSearchMode: (wsService?.mode ?? getLocalSetting(LS_WEB_SEARCH_MODE, "your-own")) as ServiceMode, |
There was a problem hiding this comment.
When the config endpoint returns an unexpected or future services["web-search"].mode, this now seeds webSearchMode with that raw string instead of falling back as reconcileFromDaemonConfig() did. In that version-skew/corrupt-config scenario the segment control has no active option, the card renders the Your Own branch, and a subsequent save can write the invalid mode back; the image-generation path added the same direct cast. Please preserve the previous mode === "managed" || mode === "your-own" guard before treating daemon data as ServiceMode.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Good catch — the old function had if (wsMode === "managed" || wsMode === "your-own") guards that only set the result for known ServiceMode values. My initial ?? replacement would pass through unexpected strings. Fixed in b5d1110 — both cards now validate before using the daemon value.
The dissolved reconcileFromDaemonConfig() only assigned mode fields when
they matched a known ServiceMode value ("managed" | "your-own"). The
initial inline replacement used ?? which falls through on unexpected
strings. Restore the explicit guard so unknown/future mode values fall
back to localStorage defaults.
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| 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); | ||
| const mode = daemonConfig.services?.["image-generation"]?.mode; |
There was a problem hiding this comment.
🟡 Image generation mode now accepts invalid raw config values without validation
serverImageGenMode now directly casts daemonConfig.services["image-generation"].mode to ServiceMode, so any non-enum string in raw config is treated as a valid UI mode. The daemon’s GET /v1/config response is based on raw config (loadRawConfig) rather than schema-validated config, and PATCH /v1/config accepts arbitrary objects via deep-merge, so malformed/stale mode values can exist and be returned (assistant/src/runtime/routes/conversation-query-routes.ts:447-452, assistant/src/config/loader.ts:875-916, assistant/src/runtime/routes/conversation-query-routes.ts:586-603). This regresses the previous guard logic and can leave the segment control in an invalid state and allow re-saving the invalid mode value.
| const mode = daemonConfig.services?.["image-generation"]?.mode; | |
| return ((daemonConfig.services?.["image-generation"]?.mode === "managed" || daemonConfig.services?.["image-generation"]?.mode === "your-own") ? daemonConfig.services["image-generation"]?.mode : getLocalSetting(LS_IMAGE_GEN_MODE, "your-own")) as ServiceMode; |
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Already fixed in b5d1110 — the mode validation guard was restored in the previous commit.
| const reconciled = reconcileFromDaemonConfig(daemonConfig); | ||
| const wsService = daemonConfig.services?.["web-search"]; | ||
| const mode = wsService?.mode; | ||
| return { |
There was a problem hiding this comment.
🟡 Web search mode now uses unchecked raw config values
serverWebSearchMode now uses wsService?.mode with a direct cast to ServiceMode instead of validating that the value is actually "managed" | "your-own". Because /v1/config returns raw file contents and config patching does not enforce schema-level validation on incoming partials, invalid mode strings can reach this component (assistant/src/runtime/routes/conversation-query-routes.ts:447-452, assistant/src/config/loader.ts:875-916, assistant/src/runtime/routes/conversation-query-routes.ts:586-603). That can produce a mode value that is not one of the segment options and can be re-submitted back to config, propagating bad state.
| return { | |
| serverWebSearchMode: ((wsService?.mode === "managed" || wsService?.mode === "your-own") ? wsService.mode : getLocalSetting(LS_WEB_SEARCH_MODE, "your-own")) as ServiceMode, |
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Already fixed in b5d1110 — both cards now validate mode === "managed" || mode === "your-own" before using the daemon value, matching the original guard logic.
| return { | ||
| serverWebSearchMode: (reconciled.webSearchMode ?? getLocalSetting(LS_WEB_SEARCH_MODE, "your-own")) as ServiceMode, | ||
| serverWebSearchProvider: reconciled.webSearchProvider ?? getLocalSetting(LS_WEB_SEARCH_PROVIDER, "inference-provider-native"), | ||
| serverWebSearchMode: (mode === "managed" || mode === "your-own" ? mode : getLocalSetting(LS_WEB_SEARCH_MODE, "your-own")) as ServiceMode, |
There was a problem hiding this comment.
🚩 Provider fallback semantic change depends on whether empty-string providers can still exist
serverWebSearchProvider now uses nullish fallback (wsService?.provider ?? ...) (apps/web/src/domains/settings/ai/web-search-card.tsx:52), whereas the removed helper only propagated truthy providers. I did not mark this as a confirmed bug because validated configs should keep provider as a non-empty enum, but /v1/config is sourced from raw config (assistant/src/runtime/routes/conversation-query-routes.ts:447-452) and loadRawConfig only enforces object shape, not field-level schema (assistant/src/config/loader.ts:875-916). If legacy or manual writes can still persist "", this may surface as an invalid dropdown value and should be verified.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Valid finding — the old code used if (wsProvider) (truthy check) which treats empty string as falsy. Fixed in 85a4ec5 by switching from ?? to || to preserve the original semantics.
The old reconcileFromDaemonConfig used `if (wsProvider)` which treats empty string as falsy. Use || instead of ?? to preserve this behavior. Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
![vex-assistant-bot[bot]](https://avatars.githubusercontent.com/in/3399303?s=60&v=4){kind=small}
There was a problem hiding this comment.
✦ APPROVE
Value: Removes a dead transitional layer between DaemonConfig and the two settings cards that still used it. Net -31 lines, no behavior change at HEAD 85a4ec55.
What this does: Deletes reconcileFromDaemonConfig() + DaemonConfigReconciliation interface; inlines direct daemonConfig.services?.[...] reads in web-search-card.tsx and image-generation-card.tsx. The bridge was a vestige of the pre-hand-typed-DaemonConfig era — now redundant since useDaemonConfigQuery() already exposes the same shape.
Refactor-regression sniff (verified at HEAD):
modevalidation guard preserved:(mode === "managed" || mode === "your-own" ? mode : getLocalSetting(...)). Matches the oldif (wsMode === "managed" || wsMode === "your-own")semantics — invalid raw config values fall back to LS, not the segment-control wedge that Codex P2 flagged on2ed20e7.- Provider truthy-fallback preserved: switched from
??to||so empty-string provider still falls through toLS_WEB_SEARCH_PROVIDER. Matches the oldif (wsProvider)truthy check. - The remaining
as ServiceModecast wraps the validated conditional, not the raw daemon value — narrowing-after-guard, exception-allowed per runtime-boundary rule.
Bot loop:
- Codex P2 (validate before cast) on
2ed20e7→ addressed inb5d1110. - Devin re-review at
85a4ec5re-flagged 2 of the same findings + 1 provider-semantic flag → Devin self-noted both first two were already inb5d1110, then shipped85a4ec5to fix the provider??→||. Tight fix loop, ~6 min start to settle. - Codex hasn't re-reviewed since the fixes landed — re-triggering on this approval.
Clean follow-up to the Settings/AI decomposition arc (#33246/#33256/#33286/#33321/#33352). Tech-debt cleanup that fell out of #33352 review, as planned.
Vellum Constitution — Trust-seeking: validation guards stay explicit at the daemon boundary; raw config strings never silently propagate into the segment control state.
|
@codex review |
|
Codex Review: Didn't find any major issues. Another round soon, 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". |
vex-assistant-bot
Bot
deleted the
devin/1780594491-lum-2239-dissolve-reconcile-from-daemon-config
branch
1 participant
Prompt / plan
Dissolve the
reconcileFromDaemonConfigtransitional bridge utility. The function extracted service mode/provider fields from the daemon config'sserviceskey into a flatDaemonConfigReconciliationinterface. Now thatDaemonConfigis hand-typed and both consumers have direct access to the config object viauseDaemonConfigQuery(), the indirection adds no value.Changes:
web-search-card.tsx(daemonConfig.services?.["web-search"]?.mode/?.provider)image-generation-card.tsx(daemonConfig.services?.["image-generation"]?.mode)reconcileFromDaemonConfig()function fromai-utils.tsDaemonConfigReconciliationinterface fromai-types.tsServiceModeimport fromai-utils.tsWhy safe: Pure behavioral no-op. The function only read fields from the config and returned them — consumers now read the same fields directly. No new logic, no changed data flow.
Closes LUM-2239
Test plan
bunx tsc --noEmitpasses cleanlyLink to Devin session: https://app.devin.ai/sessions/4b3b130bbf274e5487da3b4992883093
Requested by: @ashleeradka