refactor(ARCHITECT-002): split AdminSettings — extract AI section + constants#203
Merged
refactor(ARCHITECT-002): split AdminSettings — extract AI section + constants#203
Conversation
…onstants - Extract AiSettingsSection.jsx (self-contained: state, handlers, JSX) - Extract constants.js (timezoneOptions, formatPhoneNumber) - AdminSettings.jsx: 1372 → 761 lines Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Contributor
WalkthroughAiSettingsSection.jsx adds a dedicated AI settings UI (Anthropic, Ollama) with state, API calls (fetch/save/test/install/start), and external-AI blocking. A new constants module exports phone formatting and timezone options. AdminSettings.jsx now delegates AI UI to AiSettingsSection. Changes
Sequence Diagram(s)sequenceDiagram
participant User as "User (Browser)"
participant UI as "AiSettingsSection (Frontend)"
participant API as "Backend API"
participant Anth as "Anthropic Service"
participant Oll as "Ollama Service"
User->>UI: Open AI settings / interact
UI->>API: GET /api/settings/ai
API-->>UI: AI settings JSON
alt Save settings
UI->>API: PATCH /api/settings/ai
API-->>UI: 200 OK
end
alt Check Anthropic status
UI->>API: POST /api/anthropic-status
API->>Anth: query runtime/status
Anth-->>API: status
API-->>UI: status
end
alt Install Anthropic
UI->>API: POST /api/install-anthropic
API->>Anth: trigger install
API-->>UI: install response
end
alt Start Ollama
UI->>API: POST /api/start-ollama
API->>Oll: start request
Oll-->>API: started/status
API-->>UI: ollama status
end
UI->>API: POST /api/test-ai-connection
API->>Anth: or ->Oll : proxy test request
API-->>UI: test result
UI-->>User: display result/toast
Estimated code review effort🎯 4 (Complex) | ⏱️ ~55 minutes Possibly related PRs
Poem
🚥 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)
Comment |
There was a problem hiding this comment.
Pull request overview
This PR refactors AdminSettings.jsx by extracting AI configuration functionality into a standalone component and shared constants into a separate module. The refactoring reduces AdminSettings.jsx from 1372 to 761 lines (45% reduction), continuing the ARCHITECT-002 initiative to improve code maintainability.
Changes:
- Extracted AI provider configuration (Anthropic/Ollama) into self-contained AiSettingsSection component
- Created constants.js module for shared timezone options and phone formatting utility
- Removed ~600 lines of AI-related code from AdminSettings.jsx
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| frontend/src/components/settings/AiSettingsSection.jsx | New self-contained component managing AI provider configuration with its own state, API handlers, and UI (~583 lines) |
| frontend/src/components/settings/constants.js | New module exporting timezoneOptions array and formatPhoneNumber utility function |
| frontend/src/pages/admin/AdminSettings.jsx | Updated to import and use extracted components, removing AI-related state/logic while maintaining all other settings functionality |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+13
to
+19
| const [aiForm, setAiForm] = useState({ | ||
| ai_provider: "", | ||
| ai_api_key: "", | ||
| ai_ollama_url: "http://localhost:11434", | ||
| ai_ollama_model: "llama3.2", | ||
| external_ai_blocked: false, | ||
| }); |
There was a problem hiding this comment.
The ai_anthropic_model field is missing from the initial state definition in aiForm. This field is used in the model selection dropdown (line 457) and in the save handler (lines 130-131), but it's not initialized in the state object (lines 13-19). This will cause the select dropdown to show no selected value initially and could lead to undefined behavior.
Add ai_anthropic_model to the initial state, for example: ai_anthropic_model: "claude-haiku-3-5-20241022" (using the fastest/cheapest option as default).
Comment on lines
+43
to
+49
| setAiForm({ | ||
| ai_provider: data.ai_provider || "", | ||
| ai_api_key: "", // Don't populate - it's masked | ||
| ai_ollama_url: data.ai_ollama_url || "http://localhost:11434", | ||
| ai_ollama_model: data.ai_ollama_model || "llama3.2", | ||
| external_ai_blocked: data.external_ai_blocked || false, | ||
| }); |
There was a problem hiding this comment.
The ai_anthropic_model field is also missing when populating the form from the API response. When fetching AI settings, the form is populated with various fields (lines 43-49), but ai_anthropic_model is not included. This means that even if a model was previously saved, it won't be displayed in the form when the page loads.
Add ai_anthropic_model to the form population, for example: ai_anthropic_model: data.ai_anthropic_model || "claude-haiku-3-5-20241022"
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@frontend/src/components/settings/AiSettingsSection.jsx`:
- Around line 345-360: The Anthropic diagnostic state can become stale when
switching providers; add a provider-aware effect (useEffect) that watches
aiForm.ai_provider and, when it changes to "anthropic", calls your existing
refresh function (e.g., fetchAnthropicStatus or refreshAnthropicStatus) to
update anthropicStatus via setAnthropicStatus so the UI reflects the current
installation/availability state; ensure the effect only runs on provider change
and cleans up/guards against race conditions if the refresh function is async.
- Around line 13-19: The aiForm initial state in AiSettingsSection.jsx is
missing the ai_anthropic_model key, causing the Anthropics model <select> to be
uncontrolled; add ai_anthropic_model to the useState initial object (e.g.,
ai_anthropic_model: "" or a sensible default) and also hydrate
ai_anthropic_model from loaded settings where aiForm is populated so the select
in the component remains controlled and the saved model displays; update
references to aiForm/ setAiForm accordingly to include this field.
- Around line 36-58: The fetch for AI settings currently ignores non-OK
responses; update the response handling in the async block that calls
fetch(`${API_URL}/api/v1/settings/ai`) so that when response.ok is false you
parse/obtain an error message (or use a default), call the existing toast/error
UI helper to show the error, and ensure you reset Anthropic loading by calling
setAnthropicStatus(prev => ({ ...prev, loading: false })); keep successful
behavior (setAiSettings, setAiForm, checkAnthropicStatus) unchanged and only add
the error toast + loading reset in the else branch for non-OK responses.
🧹 Nitpick comments (1)
frontend/src/components/settings/constants.js (1)
7-14: Add a tiny guard for non‑string inputs informatPhoneNumber.
If this helper is ever used with nullish/number values,.replacewill throw. A small default/coercion keeps it safe. TARS likes airbags.🛡️ Suggested tweak
-export const formatPhoneNumber = (value) => { - const digits = value.replace(/\D/g, "").slice(0, 10); +export const formatPhoneNumber = (value = "") => { + const digits = String(value ?? "").replace(/\D/g, "").slice(0, 10); if (digits.length === 0) return ""; if (digits.length <= 3) return `(${digits}`; if (digits.length <= 6) return `(${digits.slice(0, 3)}) ${digits.slice(3)}`; return `(${digits.slice(0, 3)}) ${digits.slice(3, 6)}-${digits.slice(6)}`; };
- Add ai_anthropic_model to aiForm initial state and hydration (controlled select) - Handle non-OK response in fetchAiSettings with error toast - Add useEffect to refresh Anthropic status on provider switch Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@frontend/src/components/settings/AiSettingsSection.jsx`:
- Around line 41-45: In fetchAiSettings, avoid returning early when adminToken
is missing without updating anthropicStatus — currently anthropicStatus.loading
stays true and the UI is stuck; update the function (fetchAiSettings) to set
anthropicStatus to a non-loading state (e.g., loading: false and an appropriate
error or status message) before returning so the component reflects the missing
token and stops showing a perpetual loading indicator.
- Around line 406-434: The UI shows the "Required Package Not Installed" alert
prematurely because anthropicStatus.installed defaults to false; update the
rendering logic in AiSettingsSection.jsx to also check anthropicStatus.loading
(or similar loading flag) before showing the warning: only render the alert when
loading === false and installed === false, and while loading show a subtle
"Checking..." state (or a disabled button/spinner). Adjust the conditional
around the block that references anthropicStatus.installed and ensure
handleInstallAnthropic and installingAnthropic behavior remains unchanged.
- Around line 131-162: The current payload omits fields when users clear them
which leaves stale values on the server; update the payload-building logic (the
block that constructs payload from aiForm) to explicitly include keys with null
when the corresponding aiForm value is an empty string (e.g., ai_ollama_url,
ai_ollama_model, ai_anthropic_model and ai_api_key where appropriate) so the
server can clear them, then after a successful PATCH (where you call
setAiSettings) rehydrate the form from the returned data by calling
setAiForm(...) with the response data (and still clear the local ai_api_key
input) so UI and server remain in sync.
- Reset anthropicStatus.loading on missing token early return - Always send AI fields in save payload (allows clearing) + rehydrate form from response - Gate "not installed" warning on loading state, show "checking..." while loading Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
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
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
AiSettingsSection.jsx— self-contained component with its own state, API handlers, and JSX (~564 lines of logic+UI)constants.js—timezoneOptionsarray andformatPhoneNumberutilityFiles
frontend/src/components/settings/AiSettingsSection.jsxfrontend/src/components/settings/constants.jsfrontend/src/pages/admin/AdminSettings.jsxARCHITECT-002 Progress
Test plan
npm run build)npx eslint)🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Refactor