Merged
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThis PR implements a two-step setup flow where users first create an admin account, then optionally configure an AI agent. It adds configuration update capabilities and integrates agent chat auto-opening via state-based navigation when redirected from setup. The setup process validates configuration, calls remote APIs to configure the agent, updates global config state, and navigates to the main page with prefilled agent chat. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant SetupPage as Setup Page
participant RemoteAPI as Remote API
participant ConfigCtx as Config Context
participant Layout
participant AgentChat as Agent Chat
User->>SetupPage: Complete admin setup (Step 1)
SetupPage->>SetupPage: Navigate to Step 2
User->>SetupPage: Enable agent & configure (Step 2)
SetupPage->>RemoteAPI: Fetch model presets
RemoteAPI-->>SetupPage: Presets loaded
User->>SetupPage: Submit agent config
SetupPage->>RemoteAPI: POST create model config
RemoteAPI-->>SetupPage: Config created
SetupPage->>RemoteAPI: PUT set as default model
RemoteAPI-->>SetupPage: Updated
SetupPage->>ConfigCtx: updateConfig(agentEnabled)
ConfigCtx-->>SetupPage: Config updated
SetupPage->>Layout: Navigate with state{openAgent, initialMessage}
Layout->>AgentChat: Detect openAgent in location state
Layout->>AgentChat: setInitialInputValue(message)
Layout->>AgentChat: openChat()
AgentChat->>AgentChat: Pre-fill input & open modal
Layout->>Layout: Clear navigation state
sequenceDiagram
participant User
participant AgentChatModal as AgentChatModal
participant ChatInput
participant AgentChatContext as Context
AgentChatModal->>AgentChatContext: Read initialInputValue
AgentChatContext-->>ChatInput: Provide initialInputValue
ChatInput->>ChatInput: useEffect: Pre-fill textarea
ChatInput-->>User: Display pre-filled input
User->>ChatInput: Type/edit message
User->>AgentChatModal: Click send
AgentChatModal->>AgentChatModal: Send message
AgentChatModal->>AgentChatContext: setInitialInputValue(null)
AgentChatContext-->>ChatInput: Clear initial value
ChatInput->>ChatInput: useEffect: Reset textarea
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (4)
ui/src/layouts/Layout.tsx (1)
86-92: Hardcoded prompt string should be a constant or passed via navigation state.The string
'Create a simple DAG that prints Hello World every minute'on Line 88 is a magic string embedded in layout logic. If the setup page wants to customize this message in the future, it would need to change Layout.tsx. Consider passing it throughlocation.statefromsetup.tsx:♻️ Suggested approach — use location state for the message
In
setup.tsxLine 230:- navigate('/', { replace: true, state: { openAgent: true } }); + navigate('/', { replace: true, state: { openAgent: true, agentPrompt: 'Create a simple DAG that prints Hello World every minute' } });In
Layout.tsx:React.useEffect(() => { - if ((location.state as { openAgent?: boolean })?.openAgent) { - setInitialInputValue('Create a simple DAG that prints Hello World every minute'); + const state = location.state as { openAgent?: boolean; agentPrompt?: string } | null; + if (state?.openAgent) { + if (state.agentPrompt) { + setInitialInputValue(state.agentPrompt); + } openChat(); navigate(location.pathname, { replace: true, state: {} }); } }, [location.state, location.pathname, openChat, setInitialInputValue, navigate]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/src/layouts/Layout.tsx` around lines 86 - 92, Replace the hardcoded prompt string in the Layout useEffect with a configurable value: read a prompt from location.state (e.g. (location.state as { openAgent?: boolean, initialPrompt?: string })?.initialPrompt) and fall back to a shared constant exported from the file (or a constants module) if not provided; update the effect to call setInitialInputValue with that variable before openChat and leave references to setInitialInputValue, openChat and navigate unchanged so behavior is preserved.ui/src/pages/setup.tsx (2)
84-98:fetchPresetsswallows all errors silently.The
catch {}block (Line 93) means a 401/403 error (expired auth) or a network failure would be invisible to the user. Consider at minimum logging to console, or showing a non-blocking warning so the user knows presets couldn't be loaded.♻️ Suggested improvement
} catch { - // Presets are optional, don't block the user + // Presets are optional — log but don't block the user + console.warn('Failed to fetch model presets'); } finally {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/src/pages/setup.tsx` around lines 84 - 98, The fetchPresets function currently swallows all errors in its catch block; update the catch to capture the error (e.g., catch (err)) and at minimum log it (console.warn or process logger) and/or set a non-blocking UI flag/toast so the user knows presets couldn't be loaded without blocking them; make changes around the fetchPresets function, the catch block, and any related state (setPresetsLoading remains in finally) and use client.GET error information when logging or populating the warning.
183-195: ReplaceRecord<string, unknown>with theCreateModelConfigRequestschema type.The code constructs an object matching the API request schema but uses an untyped
Record<string, unknown>. Since the POST to/settings/agent/modelsexpectsCreateModelConfigRequest, useconst modelBody: components['schemas']['CreateModelConfigRequest'] = { ... }to catch shape mismatches at compile time. The necessarycomponentsimport is already present.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/src/pages/setup.tsx` around lines 183 - 195, Change the type of modelBody from the loose Record<string, unknown> to the generated API schema type so TypeScript verifies the shape: replace the declaration of modelBody with const modelBody: components['schemas']['CreateModelConfigRequest'] = { ... } (using the existing components import), keeping the same properties (id from generateSlugId(preset.name), name, provider, model, apiKey.trim(), description/contextWindow/maxOutputTokens/inputCostPer1M/outputCostPer1M as optional/undefined, supportsThinking defaulting to false) and adjust any property names or optionality to match CreateModelConfigRequest if the compiler flags mismatches.ui/src/App.tsx (1)
95-99: Shallow merge won't handle nested config fields correctly.
updateConfiguses a shallow spread ({ ...prev, ...patch }). If a future caller passes a partialpermissionsorpathsobject, it will replace the entire nested object rather than merging into it. Currently onlyagentEnabled(a flat boolean) is patched, so this is safe today, but consider documenting or guarding against nested partial patches.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/src/App.tsx` around lines 95 - 99, The updateConfig function in AppInner currently does a shallow merge via setConfig((prev) => ({ ...prev, ...patch })), which will replace nested objects like permissions or paths instead of merging them; change updateConfig to perform a deep/recursive merge (or merge specific nested keys) so partial updates to nested fields are merged into prev rather than overwritten — e.g., use a utility such as lodash.merge or a simple recursiveMerge helper and call setConfig(prev => merge({}, prev, patch)); keep references to AppInner, updateConfig, config, setConfig, and nested keys like permissions, paths (agentEnabled is currently flat) and add a comment documenting that updateConfig performs deep merges for nested config fields.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@ui/src/features/agent/components/ChatInput.tsx`:
- Around line 93-98: The useEffect in ChatInput sets state with
setMessage(initialValue) but doesn't update the textarea's height; after
pre-filling call the same resize logic used by the textarea onInput so the
height recalculates. Locate the textarea ref and the resize function (e.g.,
resizeTextarea or the onInput handler) in the ChatInput component and invoke it
after setMessage(initialValue) — either directly using the textareaRef (if
available) or by scheduling the onInput/resize call (e.g., with
requestAnimationFrame or setTimeout 0) so the DOM reflects the new value and the
textarea auto-resizes to fit the pre-filled text.
In `@ui/src/pages/setup.tsx`:
- Around line 37-43: generateSlugId can return an empty string for inputs like
"!!!" or whitespace, causing invalid model ids; update the function so after
sanitization it guarantees a non-empty slug (for example fallback to a
deterministic placeholder/fallback such as "model-<timestamp>" or
"model-<randomSuffix>" or throw a validation error) and return that instead of
an empty string so calls that use generateSlugId for model id creation always
receive a valid id; change the logic inside generateSlugId to check the
sanitized result and substitute the chosen fallback before returning.
- Around line 146-153: handleStep2Submit currently short-circuits when
agentEnabled is false and skips updating the server, leaving a previously
enabled agent still enabled; modify handleStep2Submit so that when agentEnabled
is false it issues the same PATCH/update call used when enabling (or a dedicated
PATCH to set enabled: false) before returning/navigating, ensuring the
server-side agent state is set to disabled (refer to the existing PATCH/update
logic used on enable within handleStep2Submit or its helper function and reuse
it to send enabled: false).
- Around line 198-212: Replace the raw fetch call that assigns createResp and
subsequent createResp.json() with the typed client.POST call so the request uses
the codebase auth/error middleware and includes remoteNode as a query param;
call client.POST('/settings/agent/models', { remoteNode }, modelBody) (or the
established client.POST signature used elsewhere in this file), remove
getAuthHeaders/config.apiURL usage, and assign the parsed response to
createdModel; ensure you handle parse errors by catching the client response
parsing or wrap awaiting the POST in try/catch and throw a new
Error(response.message || 'Failed to create model') so malformed JSON is handled
consistently with other client calls.
---
Nitpick comments:
In `@ui/src/App.tsx`:
- Around line 95-99: The updateConfig function in AppInner currently does a
shallow merge via setConfig((prev) => ({ ...prev, ...patch })), which will
replace nested objects like permissions or paths instead of merging them; change
updateConfig to perform a deep/recursive merge (or merge specific nested keys)
so partial updates to nested fields are merged into prev rather than overwritten
— e.g., use a utility such as lodash.merge or a simple recursiveMerge helper and
call setConfig(prev => merge({}, prev, patch)); keep references to AppInner,
updateConfig, config, setConfig, and nested keys like permissions, paths
(agentEnabled is currently flat) and add a comment documenting that updateConfig
performs deep merges for nested config fields.
In `@ui/src/layouts/Layout.tsx`:
- Around line 86-92: Replace the hardcoded prompt string in the Layout useEffect
with a configurable value: read a prompt from location.state (e.g.
(location.state as { openAgent?: boolean, initialPrompt?: string
})?.initialPrompt) and fall back to a shared constant exported from the file (or
a constants module) if not provided; update the effect to call
setInitialInputValue with that variable before openChat and leave references to
setInitialInputValue, openChat and navigate unchanged so behavior is preserved.
In `@ui/src/pages/setup.tsx`:
- Around line 84-98: The fetchPresets function currently swallows all errors in
its catch block; update the catch to capture the error (e.g., catch (err)) and
at minimum log it (console.warn or process logger) and/or set a non-blocking UI
flag/toast so the user knows presets couldn't be loaded without blocking them;
make changes around the fetchPresets function, the catch block, and any related
state (setPresetsLoading remains in finally) and use client.GET error
information when logging or populating the warning.
- Around line 183-195: Change the type of modelBody from the loose
Record<string, unknown> to the generated API schema type so TypeScript verifies
the shape: replace the declaration of modelBody with const modelBody:
components['schemas']['CreateModelConfigRequest'] = { ... } (using the existing
components import), keeping the same properties (id from
generateSlugId(preset.name), name, provider, model, apiKey.trim(),
description/contextWindow/maxOutputTokens/inputCostPer1M/outputCostPer1M as
optional/undefined, supportsThinking defaulting to false) and adjust any
property names or optionality to match CreateModelConfigRequest if the compiler
flags mismatches.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
Release Notes