refactor(settings): modularize Settings page (1969→399 lines, all files under 500) - #758
Conversation
…s (1969→602 lines) Settings.jsx had grown to 1969 lines — every edit reloaded the whole file into context and risked unrelated breakage. This finishes the migration the existing components/settings/*Panel.jsx pattern started: the page is now a thin orchestrator and each heavy tab lives in its own file. Extracted (logic byte-for-byte identical; only import paths adjusted + the shared isTauri/askConfirm moved to components/settings/native.js): - GeneralTab, ModelStoreTab, EnginesTab, HotkeyTab, CredentialsTab - native.js — shared isTauri() wrapper + askConfirm() Tauri-dialog helper Also establishes the standard so files can't silently regrow: - CONTRIBUTING.md: frontend file-structure & size limits (soft 300 / hard 500) - eslint.config.js: warn-only max-lines:500 guardrail (CI stays green) - docs/maintenance-pages-modularization.md: the phased refactor plan Verified: vite build passes (all imports resolve); 18/18 settings tests pass; no new lint errors introduced (the pruned imports were the only regressions). Follow-ups (tracked in the plan doc): ModelStoreTab.jsx is 836 lines and Settings.jsx 602 — both still over the 500 cap (warn-only); split next. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…line cap Follow-up to the tab extraction: bring the two remaining over-cap files into compliance with the new standard. Pure-mechanical, no behavior change. Settings.jsx 602 → 399: - Extract AboutTab, PrivacyTab, LogsTab into components/settings/ - Move the shared Row helper to components/settings/Row.jsx - LogsTab keeps its state in Settings() (lower-risk); About/Privacy take props ModelStoreTab.jsx 836 → 439, split into components/settings/models/: - format.js (fmtBytes/orgColor), runtime.js (computeRowRuntime) - columns.jsx exposes makeModelColumns(...) — a factory so the TanStack cell closures keep working; called with the same useMemo dep array as before - ModelsTable.jsx (virtualized table view), RecoBanner.jsx Every settings file is now under 500 lines. Verified: vite build passes; 18/18 settings tests pass; no new lint errors (the 4 remaining in Settings.jsx are pre-existing — refreshInfo no-op, a catch(e), two set-state-in-effect). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Caution Review failedFailed to post review comments. GitHub was unavailable or timed out while CodeRabbit was posting the review. Please request a new review later if the pull request still needs one. This happened while posting 17 inline comments. Use ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (19)
⏰ Context from checks skipped due to timeout. (1)
|
| Layer | Contents | Ships in DMG? |
|---|---|---|
| Tauri v2 shell (Rust) | Native window, process lifecycle, filesystem paths | Yes |
| Frontend bundle | React/Vite build in .app/Contents/Resources/dist/ |
Yes |
| FastAPI sidecar binary | PyInstaller-frozen omnivoice-backend with Python + torch + mlx + soundfile + demucs + yt_dlp + omnivoice TTS |
Yes (~400–500 MB bundle) |
| ffmpeg | arm64 binary in .app/Contents/Resources/bin/ |
Yes (~20 MB) |
| Model weights (OmniVoice TTS, MLX Whisper) | ~/Library/Application Support/OmniVoice/models/ |
No — first-run download |
| Optional engine packs (VoxCPM2 CUDA, pyannote, MOSS-TTS) | Separate .tar.gz via GitHub Releases manifest |
No — first-run download if user opts in |
Target DMG size: ~500 MB.
First-run model download: ~5 GB one-time.
Four key techniques
1. Sidecar port-reuse dance (dev ergonomics + crash recovery)
Tauri's startup flow:
- Check if
127.0.0.1:17493/healthresponds. - If yes, verify JSON shape:
status == "healthy",model_loaded: bool,gpu_available: bool. If valid, attach to the existing process instead of spawning. - If a legacy port (8000) has an orphan, kill via
lsof -ti :8000 | xargs kill -9. - Otherwise spawn the frozen backend sidecar via Tauri's
externalBin. - On app close: send SIGTERM, wait 2 s, SIGKILL if still alive.
Why this matters:...
Files:
docs/maintenance-pages-modularization.md
🧠 Learnings (1)
📚 Learning: 2026-06-02T03:11:49.918Z
Learnt from: debpalash
Repo: debpalash/OmniVoice-Studio PR: 240
File: docs/superpowers/plans/2026-06-02-updates-status-bar.md:363-459
Timestamp: 2026-06-02T03:11:49.918Z
Learning: When reviewing code for OmniVoice Studio’s “fully-local / nothing leaves your machine” guarantee, treat it as targeting telemetry, required cloud services, accounts, and API keys—not every possible network call. Do NOT flag “local-first” violations for read-only, unauthenticated, user-initiated requests that can gracefully degrade if offline. Example: fetching the GitHub Releases list via `api.github.com` when the user opens the Updates panel is acceptable without requiring an extra opt-in gate (and the auto-updater already makes similar calls). Flag only when the network activity involves telemetry, required cloud services/accounts, or API keys/authenticated data flows, or prevents functionality without an opt-in.
Applied to files:
frontend/src/components/settings/Row.jsxfrontend/src/components/settings/models/ModelsTable.jsxfrontend/src/components/settings/native.jsfrontend/src/components/settings/PrivacyTab.jsxfrontend/src/components/settings/models/format.jsfrontend/eslint.config.jsfrontend/src/components/settings/EnginesTab.jsxfrontend/src/components/settings/models/runtime.jsfrontend/src/components/settings/AboutTab.jsxfrontend/src/components/settings/GeneralTab.jsxfrontend/src/components/settings/HotkeyTab.jsxfrontend/src/components/settings/models/RecoBanner.jsxfrontend/src/components/settings/LogsTab.jsxfrontend/src/components/settings/CredentialsTab.jsxfrontend/src/components/settings/models/columns.jsxfrontend/src/components/settings/ModelStoreTab.jsxfrontend/src/pages/Settings.jsx
🪛 ast-grep (0.44.0)
frontend/src/components/settings/models/ModelsTable.jsx
[warning] 33-33: A list component should have a key to prevent re-rendering
Context: ↑
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.
(list-component-needs-key)
[warning] 34-34: A list component should have a key to prevent re-rendering
Context: ↓
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.
(list-component-needs-key)
frontend/src/components/settings/AboutTab.jsx
[warning] 153-161: A list component should have a key to prevent re-rendering
Context:
<Badge tone={c.status === 'ok' ? 'success' : c.status === 'warn' ? 'warn' : 'danger'}>
{c.status === 'ok'
?
: } {t(about.self_check_${c.status})}
{' '}{c.detail}
{c.hint && — {c.hint}}
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.
(list-component-needs-key)
[warning] 154-158: A list component should have a key to prevent re-rendering
Context: <Badge tone={c.status === 'ok' ? 'success' : c.status === 'warn' ? 'warn' : 'danger'}>
{c.status === 'ok'
?
: } {t(about.self_check_${c.status})}
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.
(list-component-needs-key)
[warning] 156-156: A list component should have a key to prevent re-rendering
Context:
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.
(list-component-needs-key)
[warning] 157-157: A list component should have a key to prevent re-rendering
Context:
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.
(list-component-needs-key)
[warning] 160-160: A list component should have a key to prevent re-rendering
Context: — {c.hint}
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.
(list-component-needs-key)
frontend/src/components/settings/GeneralTab.jsx
[warning] 61-61: Avoid using the initial state variable in setState
Context: setLocale(id)
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.
(setstate-same-var)
[warning] 75-75: Avoid using the initial state variable in setState
Context: setEnv('HTTP_PROXY', value)
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.
(setstate-same-var)
[warning] 78-78: Avoid using the initial state variable in setState
Context: setEnv('HTTPS_PROXY', value)
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.
(setstate-same-var)
[warning] 79-79: Avoid using the initial state variable in setState
Context: setEnv('ALL_PROXY', value)
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.
(setstate-same-var)
[warning] 80-80: Avoid using the initial state variable in setState
Context: setEnv('http_proxy', value)
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.
(setstate-same-var)
[warning] 81-81: Avoid using the initial state variable in setState
Context: setEnv('https_proxy', value)
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.
(setstate-same-var)
[warning] 82-82: Avoid using the initial state variable in setState
Context: setEnv('all_proxy', value)
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.
(setstate-same-var)
frontend/src/components/settings/HotkeyTab.jsx
[warning] 70-70: Avoid using the initial state variable in setState
Context: setPending(accel)
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.
(setstate-same-var)
[warning] 84-84: Avoid using the initial state variable in setState
Context: setCurrent(saved)
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.
(setstate-same-var)
[warning] 103-103: Avoid using the initial state variable in setState
Context: setCurrent(saved)
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.
(setstate-same-var)
frontend/src/components/settings/models/RecoBanner.jsx
[warning] 65-65: A list component should have a key to prevent re-rendering
Context: {m.size_gb}
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.
(list-component-needs-key)
[warning] 66-66: A list component should have a key to prevent re-rendering
Context: {t('models.req_tag')}
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.
(list-component-needs-key)
frontend/src/components/settings/CredentialsTab.jsx
[warning] 38-38: Avoid using the initial state variable in setState
Context: setSaving(key)
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.
(setstate-same-var)
[warning] 84-91: A list component should have a key to prevent re-rendering
Context: <>
{t(field.labelKey)}
{field.key === 'HF_TOKEN' && (
<Badge tone={info?.has_hf_token || saved.HF_TOKEN ? 'success' : 'warn'} size="xs">
{info?.has_hf_token || saved.HF_TOKEN ? t('credentials.saved') : t('credentials.not_set')}
)}
</>
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.
(list-component-needs-key)
[warning] 87-89: A list component should have a key to prevent re-rendering
Context: <Badge tone={info?.has_hf_token || saved.HF_TOKEN ? 'success' : 'warn'} size="xs">
{info?.has_hf_token || saved.HF_TOKEN ? t('credentials.saved') : t('credentials.not_set')}
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.
(list-component-needs-key)
[warning] 94-99: A list component should have a key to prevent re-rendering
Context: <>
{t(field.helpKey)}
{field.link && (
<> <a href="#" onClick={e => { e.preventDefault(); openExternal(field.link); }}>{t('credentials.get_token')}</>
)}
</>
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.
(list-component-needs-key)
[warning] 97-97: A list component should have a key to prevent re-rendering
Context: <> <a href="#" onClick={e => { e.preventDefault(); openExternal(field.link); }}>{t('credentials.get_token')}</>
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.
(list-component-needs-key)
[warning] 97-97: A list component should have a key to prevent re-rendering
Context: <a href="#" onClick={e => { e.preventDefault(); openExternal(field.link); }}>{t('credentials.get_token')}
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.
(list-component-needs-key)
[warning] 102-120: A list component should have a key to prevent re-rendering
Context: <>
<SettingsInput
type={field.isPassword ? 'password' : 'text'}
mono
placeholder={field.placeholderKey}
value={values[field.key] || ''}
onChange={e => setValues(prev => ({ ...prev, [field.key]: e.target.value }))}
onKeyDown={e => e.key === 'Enter' && save(field.key)}
/>
<Button
size="sm"
variant="subtle"
loading={saving === field.key}
onClick={() => save(field.key)}
disabled={!(values[field.key] || '').trim()}
>
{t('credentials.save')}
</>
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.
(list-component-needs-key)
[warning] 103-110: A list component should have a key to prevent re-rendering
Context: <SettingsInput
type={field.isPassword ? 'password' : 'text'}
mono
placeholder={field.placeholderKey}
value={values[field.key] || ''}
onChange={e => setValues(prev => ({ ...prev, [field.key]: e.target.value }))}
onKeyDown={e => e.key === 'Enter' && save(field.key)}
/>
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.
(list-component-needs-key)
[warning] 111-119: A list component should have a key to prevent re-rendering
Context: <Button
size="sm"
variant="subtle"
loading={saving === field.key}
onClick={() => save(field.key)}
disabled={!(values[field.key] || '').trim()}
>
{t('credentials.save')}
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.
(list-component-needs-key)
🪛 markdownlint-cli2 (0.22.1)
docs/maintenance-pages-modularization.md
[warning] 56-56: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
[warning] 63-63: Table pipe style
Expected: leading_and_trailing; Actual: no_leading_or_trailing; Missing leading pipe
(MD055, table-pipe-style)
[warning] 63-63: Table pipe style
Expected: leading_and_trailing; Actual: no_leading_or_trailing; Missing trailing pipe
(MD055, table-pipe-style)
[warning] 63-63: Table column count
Expected: 3; Actual: 1; Too few cells, row will be missing data
(MD056, table-column-count)
[warning] 64-64: Table pipe style
Expected: leading_and_trailing; Actual: no_leading_or_trailing; Missing leading pipe
(MD055, table-pipe-style)
[warning] 64-64: Table pipe style
Expected: leading_and_trailing; Actual: no_leading_or_trailing; Missing trailing pipe
(MD055, table-pipe-style)
[warning] 64-64: Table column count
Expected: 3; Actual: 1; Too few cells, row will be missing data
(MD056, table-column-count)
📝 Walkthrough
Walkthrough
Settings.jsx is refactored from ~1600 lines into a thin tab orchestrator that imports eight new dedicated tab components (GeneralTab, ModelStoreTab, EnginesTab, HotkeyTab, CredentialsTab, LogsTab, AboutTab, PrivacyTab). Shared helpers (native.js, Row.jsx, format.js, runtime.js) and model-store subcomponents (ModelsTable, RecoBanner, columns.jsx) are extracted. An ESLint max-lines warn rule and documentation for the multi-phase modularization plan are added.
Changes
Settings Modularization
| Layer / File(s) | Summary |
|---|---|
ESLint guardrail and modularization docs frontend/eslint.config.js, CONTRIBUTING.md, docs/maintenance-pages-modularization.md |
Adds max-lines warn rule at 500 lines, contributing guidelines for frontend file structure, and a maintenance doc covering the multi-phase modularization plan with constraints and verification steps. |
Shared helpers frontend/src/components/settings/native.js, frontend/src/components/settings/Row.jsx, frontend/src/components/settings/models/format.js, frontend/src/components/settings/models/runtime.js |
Introduces isTauri/askConfirm Tauri helpers, a Row wrapper delegating to SettingRow, fmtBytes/orgColor formatters, and computeRowRuntime for SSE-driven download state derivation. |
Model store subcomponents frontend/src/components/settings/models/columns.jsx, frontend/src/components/settings/models/ModelsTable.jsx, frontend/src/components/settings/models/RecoBanner.jsx |
Extracts makeModelColumns (TanStack column factory with name/role/size/status/actions definitions), ModelsTable (virtualized renderer), and RecoBanner (recommendation install banner) from the former inline Settings implementation. |
Simple tab components frontend/src/components/settings/GeneralTab.jsx, frontend/src/components/settings/CredentialsTab.jsx, frontend/src/components/settings/EnginesTab.jsx, frontend/src/components/settings/PrivacyTab.jsx, frontend/src/components/settings/LogsTab.jsx |
Five new tab components encapsulate locale/theme/proxy/FFmpeg settings, credential save flows with per-field saving state, engine selection with selectEngine wiring, privacy info display, and log source selection/viewing. |
Complex tab components frontend/src/components/settings/ModelStoreTab.jsx, frontend/src/components/settings/HotkeyTab.jsx, frontend/src/components/settings/AboutTab.jsx |
ModelStoreTab wires SSE download streaming, TanStack table virtualization, and install/delete/reinstall actions. HotkeyTab captures keydown events and writes Tauri accelerators. AboutTab renders system info rows, Tauri-conditional update controls, self-check results, and diagnostic actions. |
Settings.jsx thin orchestrator frontend/src/pages/Settings.jsx |
Drops ~1600 lines of inline tab UI, narrows imports to the eight new tab components plus isTauri/askConfirm from native.js, and retains tab navigation, deep-link state, and callback prop wiring. |
Estimated code review effort
🎯 4 (Complex) | ⏱️ ~60 minutes
Possibly related PRs
- debpalash/OmniVoice-Studio#686:
Row.jsxhere delegates to theSettingRowprimitive introduced in that design-system refactor. - debpalash/OmniVoice-Studio#690: Both PRs modify
Settings.jsxand depend on the sameSettingRow/SettingsInputshared primitives layer. - debpalash/OmniVoice-Studio#157: Both PRs directly overlap in
Settings.jsxtab rendering — that PR wired i18n text for the same about/privacy/credentials tabs that this PR extracts into components.
🚥 Pre-merge checks | ✅ 6 | ❌ 3
❌ Failed checks (3 warnings)
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Title check | Conventional-commit format is correct, but the title/body lacks the required issue reference. | Add the issue key/reference to the title or PR body while keeping the refactor(scope): summary format. | |
| Docstring Coverage | Docstring coverage is 58.82% which is insufficient. The required threshold is 80.00%. | Write docstrings for the functions missing them to satisfy the coverage threshold. | |
| I18n Completeness (21 Locales) | FAIL: 12 new i18n keys are missing from all 21 locale JSONs, and several settings UI strings still hardcode English (Confirm, theme labels, role labels, placeholders, fallbacks). | Add the missing keys to every frontend/src/i18n/locales/*.json file and replace raw strings/default fallbacks with t(...) or translated constants in native.js, GeneralTab, ModelStoreTab, CredentialsTab, and models/columns.jsx. |
✅ Passed checks (6 passed)
| Check name | Status | Explanation |
|---|---|---|
| Description check | ✅ Passed | The description covers summary, key changes, verification, and rollout context, though it doesn't mirror every template section. |
| Linked Issues check | ✅ Passed | Check skipped because no linked issues were found for this pull request. |
| Out of Scope Changes check | ✅ Passed | Check skipped because no linked issues were found for this pull request. |
| Cross-Platform Default Parity | ✅ Passed | PASS: Platform-sensitive paths are gated (native.js:10-15, HotkeyTab.jsx:41-160); no new default behavior diverges across macOS/Windows/Linux. |
| Local-First Guarantee | ✅ Passed | New network paths are opt-in only—bug reports open prefilled GitHub Issues, model installs use HF downloads, and no telemetry hooks or mandatory cloud auth were added. |
| Backward Compatibility | ✅ Passed | Only frontend Settings files changed; backend model discovery still uses scan_cache_dir/on-disk fallback, and DB reconciliation remains additive-only. |
✨ Finishing Touches
📝 Generate docstrings
- Create stacked PR
- Commit on current branch
🧪 Generate unit tests (beta)
- Create PR with unit tests
- Commit unit tests in branch
refactor/settings-page-modularization
Warning
There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.
🔧 ESLint
If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.
ESLint install failed. For unrecoverable errors, disable the tool in CodeRabbit configuration.
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 help to get the list of available commands.
|
| Filename | Overview |
|---|---|
| frontend/src/pages/Settings.jsx | Slimmed to 399 lines — now a thin orchestrator that wires shared state (logs, update channel, sysinfo) and delegates all tab rendering to imported components. |
| frontend/src/components/settings/ModelStoreTab.jsx | Largest extracted component (439L). Pre-existing naming collision: const t = setTimeout(...) inside the lifecycle-terminator useEffect shadows the t translation function from the enclosing scope. |
| frontend/src/components/settings/CredentialsTab.jsx | Extracted from Settings.jsx. Pre-existing dead code: HF_TOKEN is filtered from the field list but the field.key === 'HF_TOKEN' badge branch inside the map is never reachable. |
| frontend/src/components/settings/EnginesTab.jsx | Extracted from Settings.jsx. Pre-existing stale-closure lint issue: t is used inside the useCallback but omitted from the dependency array []. |
| frontend/src/components/settings/native.js | New shared module wrapping the isTauri boolean from utils/media as a callable and providing a Tauri-dialog-safe askConfirm. All callers use it consistently as isTauri(). |
| frontend/src/components/settings/models/columns.jsx | Column factory (makeModelColumns) extracted from ModelStoreTab. Dependency array for useMemo in the host is unchanged; logic is identical to the original inline definition. |
| frontend/src/components/settings/models/runtime.js | Pure helper (computeRowRuntime) with no React deps — correctly extracted so it can be unit-tested independently. |
| docs/maintenance-pages-modularization.md | New planning doc whose header still reads Status: Plan (not yet executed) even though Phase 1 (Settings.jsx split) is being executed in this very PR. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
S["pages/Settings.jsx\n399 lines (was 1969)"]
S --> G["settings/GeneralTab\n209L"]
S --> M["settings/ModelStoreTab\n439L"]
S --> E["settings/EnginesTab\n52L"]
S --> H["settings/HotkeyTab\n166L"]
S --> C["settings/CredentialsTab\n128L"]
S --> A["settings/AboutTab\n175L"]
S --> P["settings/PrivacyTab\n33L"]
S --> L["settings/LogsTab\n79L"]
S --> N["settings/native.js\nisTauri() / askConfirm()"]
M --> MT["models/ModelsTable\n82L"]
M --> RB["models/RecoBanner\n73L"]
M --> CO["models/columns.jsx\n244L"]
M --> RT["models/runtime.js\n75L"]
M --> FO["models/format.js\n17L"]
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
S["pages/Settings.jsx\n399 lines (was 1969)"]
S --> G["settings/GeneralTab\n209L"]
S --> M["settings/ModelStoreTab\n439L"]
S --> E["settings/EnginesTab\n52L"]
S --> H["settings/HotkeyTab\n166L"]
S --> C["settings/CredentialsTab\n128L"]
S --> A["settings/AboutTab\n175L"]
S --> P["settings/PrivacyTab\n33L"]
S --> L["settings/LogsTab\n79L"]
S --> N["settings/native.js\nisTauri() / askConfirm()"]
M --> MT["models/ModelsTable\n82L"]
M --> RB["models/RecoBanner\n73L"]
M --> CO["models/columns.jsx\n244L"]
M --> RT["models/runtime.js\n75L"]
M --> FO["models/format.js\n17L"]
Comments Outside Diff (1)
-
frontend/src/components/settings/CredentialsTab.jsx, line 411-425 (link)Dead HF_TOKEN badge inside a filtered loop. The list is built with
.filter(f => f.key !== 'HF_TOKEN'), sofield.key === 'HF_TOKEN'is alwaysfalseinside the map — the saved/not-setBadgewill never render. This was already dead in the originalSettings.jsxand is preserved verbatim here. SinceApiKeysPanelnow owns HF-token display, the inner condition can safely be dropped: change thetitleprop to just{t(field.labelKey)}.
Reviews (1): Last reviewed commit: "refactor(settings): split ModelStoreTab ..." | Re-trigger Greptile
…00) (#760) Phase 3 — same standard as #758/#759, applied to the last three over-cap pages. Pure-mechanical, no behavior change. - VoiceGallery.jsx 768 → 205: relocate the already-separate zone components (ArchetypesZone, ArchetypeCard, CommunityZone, ImportsZone) + shared helpers into components/gallery/. - CloneDesignTab.jsx 837 → 395: split the ~540-line JSX return into section components (ScriptPanel, AudioMethodPanel, DesignMethodPanel, ActionBar) + MicButton, under components/clone/. State stays in the page. - VoiceProfile.jsx 515 → 287: split the main return into ProfileHeader / ProfileDetails / ProfileActivity under components/profile/. Safety contract for the JSX splits (no render tests): explicit NAMED props on every section so eslint no-undef verifies completeness on both ends; JSX moved verbatim. Verified: 0 no-undef across all changed files; every original className preserved (diffed main vs new set); every file <500 lines. Verified: vite build passes; FULL frontend suite 638/638 pass. Co-authored-by: mergetest <test@local> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 participant
What & why
Settings.jsxhad grown to 1969 lines — every edit reloaded the whole file into context and risked unrelated breakage. This finishes the migration that the existingcomponents/settings/*Panel.jsxpattern started: the page is now a thin orchestrator, and each heavy tab lives in its own file. Pure-mechanical refactor — no behavior change (logic moved byte-for-byte; only import paths and the sharedisTauri/askConfirmhelper changed).Result
pages/Settings.jsxExtracted into
components/settings/:GeneralTab(209),ModelStoreTab(439),EnginesTab(52),HotkeyTab(166),CredentialsTab(128),AboutTab(175),PrivacyTab(33),LogsTab(79), sharedRow(8) +native.js(16).ModelStoreTabfurther split intocomponents/settings/models/(columns.jsxwith amakeModelColumns()factory,ModelsTable,runtime,RecoBanner,format).Every settings file is now under the 500-line cap.
Standard (Phase 0)
So files can't silently regrow:
CONTRIBUTING.md— frontend file-structure & size limits (soft 300 / hard 500)eslint.config.js— warn-onlymax-lines: 500guardrail (keeps CI green)docs/maintenance-pages-modularization.md— the phased refactor planVerification
vite buildpasses — all imports resolvevitest run src/components/settings— 18/18 passuseMemodep array, so memoization is unchanged🤖 Generated with Claude Code
Refactored the Settings page into smaller tab modules and extracted shared helpers.
pages/Settings.jsxinto thin orchestration plus dedicated tabs:GeneralTabModelStoreTabEnginesTabHotkeyTabCredentialsTabLogsTabAboutTabPrivacyTabmodels/format.jsmodels/runtime.jsmodels/columns.jsxmodels/ModelsTable.jsxmodels/RecoBanner.jsxnative.jsCONTRIBUTING.mdeslint.config.js(max-lines: 500warn-only)docs/maintenance-pages-modularization.mdUI sketch:
Before
After
Verified with build/tests; no behavior changes intended.