fix(gui): denser Storage, API Access, Subagents, Usage, and Claude layouts - #741
Conversation
📝 WalkthroughWalkthroughThis PR adds API key, storage, subagent, and usage workspace interfaces; introduces session-cached page loading and soft refreshes; refactors cleanup and Claude controls; adds localized strings, responsive styles, and expanded behavioral tests. ChangesWorkspace interfaces and page integration
Cached state and control behavior
Validation
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 2📝 Generate docstrings 💡
🛠️ Fix failing CI checks 💡
🧪 Generate unit tests (beta)
Comment |
|
This pull request currently targets @Wibias Please retarget this PR to Its title has been prefixed with This pull request is being kept as a draft automatically. Once every issue above is resolved, it will be marked ready for review again. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 99d07e1ffe
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| { | ||
| id: "modelMap", | ||
| label: t("claude.modelMap"), | ||
| body: <ClaudeCodeModelMapSection rows={rows} onRowsChange={setRows} onSave={() => { void save(); }} />, | ||
| }, |
There was a problem hiding this comment.
The new section switcher initially shows Settings, but the only control that invokes save() is now inside the Model mappings section. Changes to Enabled, authentication, fast mode, context settings, sidecars, or the Small fast model therefore cannot be persisted from the section where they are made and are silently lost if the user leaves without discovering that an unrelated section contains the global Save button. Keep a persistent workspace-level Save action or provide it in each editable section.
Useful? React with 👍 / 👎.
| tabIndex={tab === id ? 0 : -1} | ||
| className={`usage-segmented-btn${tab === id ? " active" : ""}`} | ||
| onClick={() => setTab(id)} |
There was a problem hiding this comment.
When a keyboard user enters this tablist, only the selected tab is tabbable because the other receives tabIndex={-1}, but the buttons have no ArrowLeft/ArrowRight, Home, or End handler to move focus and select another tab. Consequently, the Quarantine panel is unreachable without a pointer. Implement the standard roving-tab keyboard behavior (including moving focus) or leave both controls in the sequential tab order.
AGENTS.md reference: gui/AGENTS.md:L31-L34
Useful? React with 👍 / 👎.
| </div> | ||
| </aside> | ||
|
|
||
| <main className="storage-workspace-main"> |
There was a problem hiding this comment.
The application already wraps every page in the <main> landmark in gui/src/App.tsx, so rendering another <main> here produces a nested duplicate main landmark whenever Storage has files. This gives assistive technology an invalid/ambiguous page structure; use a labelled <section> as the ApiKeys and Dashboard workspace panes do.
AGENTS.md reference: gui/AGENTS.md:L31-L34
Useful? React with 👍 / 👎.
| hidden={tab !== "desktop"} | ||
| > | ||
| {tab === "desktop" && <ClaudeDesktop apiBase={apiBase} />} | ||
| <ClaudeDesktop apiBase={apiBase} /> |
There was a problem hiding this comment.
When a user stays on the default Claude Code tab and opens Desktop later, this unconditional mount means ClaudeDesktop fetched its profile and model catalog while hidden, potentially much earlier. Its only subsequent polling targets /api/claude-desktop/status, so provider/model/profile changes made in the meantime are never loaded when the tab is revealed. Revalidate the Desktop payload on tab activation (and likewise Code if it can be hidden for a long time), or restore mount-on-selection while retaining the session cache for fast paint.
Useful? React with 👍 / 👎.
| hasCacheRef.current = true; | ||
| writeSessionListCache(cacheKey, { available: avail, chosen: nextChosen }); |
There was a problem hiding this comment.
The cache is written only by the initial GET, while a successful PUT later reconciles chosen from d.applied without updating that cache. After navigating away and back, the page therefore paints the pre-save selection; if the revalidation GET is unavailable, hasCacheRef suppresses the load error and leaves that stale selection indefinitely, where saving again can revert the server configuration. Write the applied selection and current available list to this cache after a successful save.
Useful? React with 👍 / 👎.
| // Run now is a manual action: if auto-cleanup is still off, turn it on so the | ||
| // job is not immediately skipped as "disabled". | ||
| const saveBody = base.enabled ? base : { ...base, enabled: true }; |
There was a problem hiding this comment.
If auto-cleanup is disabled, clicking the newly enabled Run now button silently persists enabled: true before starting the job. That converts a one-time manual action into an opt-in to future startup/daily/weekly runs and can later delete archives automatically, including permanently when that mode is configured. This contradicts docs-site/src/content/docs/reference/configuration.md:54, which specifies that cleanup is default-off and never enabled implicitly; require the user to enable the policy explicitly instead of mutating this flag as a side effect.
AGENTS.md reference: AGENTS.md:L167-L168
Useful? React with 👍 / 👎.
| <code key={a.id} className="claude-aliases-chip" title={a.display_name}> | ||
| {a.id} | ||
| </code> |
There was a problem hiding this comment.
The old alias rows displayed both id and display_name, but the new chips render only the id and move the display name into a title attribute on a non-focusable <code> element. Touch users and keyboard-only users therefore cannot discover the display name at all, and assistive technology does not get a reliable labelled control to expose it. Render the display name visibly or provide a keyboard-accessible disclosure with an explicit accessible label.
AGENTS.md reference: gui/AGENTS.md:L31-L34
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 99d07e1ffe
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| setTargetMode("percent"); | ||
| setPercent(String(Math.min(100, Math.max(1, Math.floor(json.target.removeOldestPercent ?? 25))))); | ||
| } | ||
| applyPolicy(json); |
There was a problem hiding this comment.
When a cached policy exists, the controls are enabled immediately while this GET runs, but its response unconditionally calls applyPolicy. If a user changes a field or toggles cleanup before the background request completes, a response containing the earlier policy overwrites every draft and can leave the UI/cache disagreeing with a successful PUT; a later save can then restore the old policy. Guard revalidation with a dirty/mutation generation or defer applying it while a policy mutation is active.
AGENTS.md reference: gui/AGENTS.md:L9-L10
Useful? React with 👍 / 👎.
| setExcluded(saved); | ||
| setSavedExcluded(saved); | ||
| hasCacheRef.current = true; | ||
| writeSessionListCache(cacheKey, next); |
There was a problem hiding this comment.
The new session cache is updated only by load(), while the successful Save path (applyAfter === false) updates savedExcluded without refreshing or rewriting this cached payload. After navigating away and back, the page therefore seeds the pre-save exclusions; if revalidation fails, the cached state remains silently active and saving it again can undo the previously persisted selection. Rewrite the cached status after every successful PUT or call load() for both save paths.
AGENTS.md reference: gui/AGENTS.md:L9-L10
Useful? React with 👍 / 👎.
| const hasModelsCacheRef = useRef(Boolean(cachedModels)); | ||
| const [keys, setKeys] = useState<ApiKeyEntry[]>(() => cachedKeys?.keys ?? []); | ||
| const [endpoints, setEndpoints] = useState<ApiEndpointInfo>(() => | ||
| cachedKeys?.endpoints ?? deriveApiEndpoints(`${apiBase.replace(/\/$/, "")}/v1/responses`), |
There was a problem hiding this comment.
In the normal bundled deployment apiBase is an empty string, so this initializer derives /v1 and renders relative endpoint and usage-example URLs as if they were ready. Before /api/keys returns—and permanently when that request fails—the copy controls expose URLs without the host-aware authority that the management API normally supplies, making them unusable as external-client Base URLs. Seed from the current origin or keep the endpoint panels loading until the authoritative response arrives.
AGENTS.md reference: gui/AGENTS.md:L9-L10
Useful? React with 👍 / 👎.
| const generation = ++loadGenerationRef.current; | ||
| setLoading(true); | ||
| // Soft refresh when a prior scan is already painted. | ||
| if (!hasReportRef.current) setLoading(true); |
There was a problem hiding this comment.
Once a report has been cached, this condition leaves loading false for every subsequent scan, including scans started by the Refresh button. The button therefore remains enabled while /api/storage is in flight, so clicking it again—or clicking it during the automatic cached revalidation—queues additional uncancelled scans; the generation guard discards their responses but does not stop the server-side synchronous filesystem traversals. Track explicit refresh activity separately or set a busy flag for user-triggered scans so only one scan can run at a time.
Useful? React with 👍 / 👎.
6cbe980 to
8cfa8a0
Compare
99d07e1 to
800ec98
Compare
|
@coderabbitai review |
✅ Action performedReview finished.
|
dd0be07 to
16b0aaf
Compare
800ec98 to
96bf28d
Compare
|
@coderabbitai review |
✅ Action performedReview finished.
|
16b0aaf to
877247d
Compare
d471393 to
af42a55
Compare
There was a problem hiding this comment.
Actionable comments posted: 18
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
gui/src/pages/ApiKeys.tsx (1)
124-152: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winThe soft-refresh guard preserves cached models but the failure flag hides them anyway.
On every failure path (Lines 124, 135, 151) the code deliberately keeps the cached rows —
if (!hasModelsCacheRef.current) setModels([])— and then unconditionally setssetModelsLoadFailed(true). ButApiKeysModelsPaneltreats that flag as terminal:gui/src/pages/api-keys-panels.tsxLine 392 renderst("api.modelsLoadFailed")instead of the table whenevermodelsLoadFailedis true. Net effect on a failed background revalidate with a warm cache: the rows are still in state, the user just can't see them — the same regression the guard was written to prevent.Gate the flag on the same condition as the data reset:
🐛 Proposed fix
const res = await fetch(`${apiBase}/v1/models`); if (!res.ok) { - if (!hasModelsCacheRef.current) setModels([]); - setModelsLoadFailed(true); + if (!hasModelsCacheRef.current) { + setModels([]); + setModelsLoadFailed(true); + } return; } @@ if (!rawRows) { - if (!hasModelsCacheRef.current) setModels([]); - setModelsLoadFailed(true); + if (!hasModelsCacheRef.current) { + setModels([]); + setModelsLoadFailed(true); + } return; } @@ } catch { - if (!hasModelsCacheRef.current) setModels([]); - setModelsLoadFailed(true); + if (!hasModelsCacheRef.current) { + setModels([]); + setModelsLoadFailed(true); + } } finally {If surfacing a stale-data warning is desired, that needs a separate non-terminal indicator rather than the flag that suppresses the table.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@gui/src/pages/ApiKeys.tsx` around lines 124 - 152, Update all failure paths in the model-loading flow around the response validation and catch block to setModelsLoadFailed(true) only when hasModelsCacheRef.current is false, matching the existing conditional setModels([]) behavior. Keep cached models visible during failed background revalidation, while preserving the terminal failure state for users without cached models.gui/src/pages/ClaudeCode.tsx (1)
37-67: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy liftProtect editable cached drafts from background revalidation. These pages intentionally paint cached controls immediately, but their initial GETs unconditionally replace user-editable state.
gui/src/pages/ClaudeCode.tsx#L37-L67: mark state/model-map edits dirty and skip replacing those drafts while dirty.gui/src/pages/ClaudeDesktop.tsx#L160-L178: consult a dirty ref before replacing profile and destination state; also synchronize the cache after successful saves.gui/src/pages/Grok.tsx#L72-L87: preserve locally changed exclusions until saved or explicitly discarded.As per path instructions, GUI state must remain consistent with user actions and management API responses.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@gui/src/pages/ClaudeCode.tsx` around lines 37 - 67, Protect editable drafts from background GET revalidation by tracking dirty state and skipping server replacements while unsaved changes exist. In gui/src/pages/ClaudeCode.tsx:37-67, mark state and model-map edits dirty and have load preserve those drafts; in gui/src/pages/ClaudeDesktop.tsx:160-178, consult the dirty ref before replacing profile or destination state and synchronize the cache after successful saves; in gui/src/pages/Grok.tsx:72-87, preserve locally changed exclusions until save or explicit discard. Ensure state remains consistent with user actions and management API responses.Source: Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@gui/src/components/apikeys-workspace/ApiKeysWorkspace.tsx`:
- Around line 203-221: Update the ApiKeysManagePanel props contract in
api-keys-panels.tsx to make confirmDelete, onConfirmDelete, onCancelDelete, and
onDelete optional while preserving their required use within the showKeyList
branch. Remove these four stub props from the ApiKeysManagePanel invocation in
ApiKeysWorkspace; leave all other props unchanged.
- Around line 157-175: Update the confirmDelete branch in ApiKeysWorkspace so
the confirmation control is rendered as a distinct remounted element from the
initial delete button, using a different key or DOM position and a ref callback
with no return value. Ensure focus and carried-over click/Enter events cannot
invoke handleDeleteClick again immediately after confirmation appears, while
preserving the existing cancel and deletion behavior.
In `@gui/src/components/subagents-workspace/SubagentsWorkspace.tsx`:
- Around line 177-188: Update the detail-pane actions in SubagentsWorkspace to
provide a Save action alongside the featured-model toggle, reusing the existing
onSave handler and pending state used by the roster branch. Ensure saving
persists the current chosen models through the management API and reflects the
API response in local state, while disabling or indicating the save operation
when appropriate.
- Line 31: Remove the duplicated FEATURED_MAX definition from SubagentsWorkspace
and export the shared cap from that module, then import and use it in
Subagents.tsx for the selection-length guard instead of the hardcoded 5. Ensure
the workspace counters, button state, and page update logic all derive from the
same exported limit.
- Line 159: Update the detail heading in SubagentsWorkspace to render the
selected model through modelLabel, preserving the provider icon and consistent
display formatting. Remove the redundant sub.workspace.selector row beneath it
so the selector is not repeated in the detail pane.
In `@gui/src/pages/api-keys-panels.tsx`:
- Around line 111-137: Remove the unsafe anchorRef as never cast from the shared
prop bag in the tooltip trigger logic. Split the props by the rendered element
selected through as, giving the button and div branches correctly typed refs and
handlers, or make the helper generic over that element type while preserving the
existing copy, focus, and keyboard behavior.
In `@gui/src/pages/Claude.tsx`:
- Around line 74-80: Keep ClaudeDesktop mounted when switching tabs by removing
the tab-dependent conditional around its render in the claude-desktop-panel and
relying on the panel’s existing hidden attribute to control visibility. Preserve
the existing apiBase prop and ensure the component remains mounted for both tab
values so its in-flight and draft state survives.
In `@gui/src/pages/ClaudeCode.tsx`:
- Around line 27-35: Rebind all session-cached state whenever apiBase changes:
in gui/src/pages/ClaudeCode.tsx:27-35 reset or hydrate state, rows, and
hasCacheRef; in gui/src/pages/Storage.tsx:633-655 reset cleanup-policy drafts
and cache state, and in gui/src/pages/Storage.tsx:1355-1366 replace the report
and reset hasReportRef; in gui/src/pages/ClaudeDesktop.tsx:126-158 reset
profile, saved profile, destinations, and cache presence together; in
gui/src/pages/Grok.tsx:58-70 reset status and both exclusion sets; and in
gui/src/pages/Logs.tsx:343-360 clear or hydrate logs from the new cache before
refreshing. Ensure each page’s GUI state remains consistent with management API
responses.
In `@gui/src/pages/Combos.tsx`:
- Around line 168-173: Separate user-initiated refreshes from background
revalidation in fetchAll: add an intent parameter, bypass the loading guard when
refresh is explicit, and notify on failure for that path even when
hasCacheRef.current is true. Update the onRefresh handler to pass the
user-initiated intent, while keeping saveCombo and removeCombo calls on the
existing non-user path and preserving their messaging.
In `@gui/src/pages/Debug.tsx`:
- Around line 115-139: Fence asynchronous log reads so stale responses cannot
update active GUI state. In gui/src/pages/Debug.tsx lines 115-139, include
apiBase in the stream identity and add abort or generation checks before
committing rows or updating afterRef; in gui/src/pages/Logs.tsx lines 374-402,
serialize polling or generation-check full-list responses before updating state
or session storage. Use the existing fetchLogs and polling flows, ensuring
displayed state always matches the active management API response.
In `@gui/src/pages/Models.tsx`:
- Around line 57-63: Harden readSessionListCache in
gui/src/session-list-cache.ts with an optional type-guard validator that evicts
invalid entries and returns null. Update gui/src/pages/Models.tsx:57-63 to
validate models, providers, and disabled arrays plus non-null selectedModels;
gui/src/pages/Usage.tsx:750-753 to validate summary.requests and summary
days/models/providers arrays; gui/src/pages/ApiKeys.tsx:59-61 to validate keys
and models arrays plus endpoints; gui/src/pages/Combos.tsx:52-59 to validate all
four arrays; and gui/src/pages/Subagents.tsx:13-15 to validate available and
chosen arrays.
- Around line 205-221: Update the cache write in the Models load callback to
persist the same context cap value used by live state: when the API omits value,
retain the current contextCapValue instead of falling back to 350_000. Add
contextCapValue to the load callback dependencies so the callback reflects cap
changes, while preserving the existing API-provided value behavior.
- Line 682: Add styling for the models-field-stack class used by the
custom-model modal, restoring the spacing and layout previously provided by the
replaced inline styles. Update the imported models workspace stylesheet chain,
or replace the class usage with equivalent inline styles at the models provider
card/modal markup.
- Around line 621-630: Extract the duplicated shadow-call control markup into a
shared `shadowRow` JSX value, preserving its existing props, handlers, and
layout. Declare `shadowRow` before the early return so both render paths can
access it, then replace the copies in the initial controls section and
`controlsBlock` with `{shadowRow}`.
In `@gui/src/pages/Storage.tsx`:
- Around line 1402-1408: Update refreshAll to handle fetchStorage returning null
or a report with an error by setting scan status to a localized failure message,
while preserving the existing success message for successful rescans. Add or
reuse the appropriate locale entry rather than introducing hardcoded visible
text, and keep the cached report behavior unchanged.
In `@gui/src/pages/Subagents.tsx`:
- Around line 81-85: Update the success status in the save flow to derive its
model count from the existing effective list `applied`, matching the value
passed to `writeSessionListCache`. Keep the `d?.applied` state update behavior
unchanged while ensuring responses without `applied` report the submitted
`chosen` count instead of zero.
In `@gui/src/pages/Usage.tsx`:
- Around line 765-767: Move the readHeldUsage call into the lazy initializer for
the data state so sessionStorage and JSON parsing occur only during initial
state creation, and initialize loading from the same cached value without
performing another render-time read. Use the existing range and surface state
values instead of duplicated "30d" and "all" literals so the initial cache
bucket stays aligned when those defaults change.
In `@gui/tests/subagents-classic.test.ts`:
- Around line 8-12: Update the test named “Subagents mounts the denser workspace
as the only layout” to load the SubagentsWorkspace component source when
asserting the “subagents-workspace” class, while keeping the Subagents.tsx
source assertion for “SubagentsWorkspace”.
---
Outside diff comments:
In `@gui/src/pages/ApiKeys.tsx`:
- Around line 124-152: Update all failure paths in the model-loading flow around
the response validation and catch block to setModelsLoadFailed(true) only when
hasModelsCacheRef.current is false, matching the existing conditional
setModels([]) behavior. Keep cached models visible during failed background
revalidation, while preserving the terminal failure state for users without
cached models.
In `@gui/src/pages/ClaudeCode.tsx`:
- Around line 37-67: Protect editable drafts from background GET revalidation by
tracking dirty state and skipping server replacements while unsaved changes
exist. In gui/src/pages/ClaudeCode.tsx:37-67, mark state and model-map edits
dirty and have load preserve those drafts; in
gui/src/pages/ClaudeDesktop.tsx:160-178, consult the dirty ref before replacing
profile or destination state and synchronize the cache after successful saves;
in gui/src/pages/Grok.tsx:72-87, preserve locally changed exclusions until save
or explicit discard. Ensure state remains consistent with user actions and
management API responses.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 395ca666-9467-4eed-b021-dbf61ebb10fb
📒 Files selected for processing (46)
gui/src/components/apikeys-workspace/ApiKeysWorkspace.tsxgui/src/components/storage-workspace/StorageWorkspace.tsxgui/src/components/subagents-workspace/SubagentsWorkspace.tsxgui/src/i18n/de.tsgui/src/i18n/en.tsgui/src/i18n/ja.tsgui/src/i18n/ko.tsgui/src/i18n/ru.tsgui/src/i18n/zh.tsgui/src/model-display.tsgui/src/pages/ApiKeys.tsxgui/src/pages/Claude.tsxgui/src/pages/ClaudeCode.tsxgui/src/pages/ClaudeDesktop.tsxgui/src/pages/Combos.tsxgui/src/pages/Debug.tsxgui/src/pages/Grok.tsxgui/src/pages/Logs.tsxgui/src/pages/Models.tsxgui/src/pages/Storage.tsxgui/src/pages/Subagents.tsxgui/src/pages/Usage.tsxgui/src/pages/api-keys-panels.tsxgui/src/pages/claude-code-sections.tsxgui/src/pages/claude-desktop-lane.tsgui/src/pages/models-shared.tsgui/src/styles-apikeys-workspace.cssgui/src/styles-claudecode-workspace.cssgui/src/styles-models-workspace.cssgui/src/styles-storage-workspace.cssgui/src/styles-subagents-workspace.cssgui/src/styles-usage-workspace.cssgui/src/styles.cssgui/tests/apikeys-layout.test.tsgui/tests/apikeys-refresh-preserve.test.tsxgui/tests/apikeys-workspace.test.tsxgui/tests/claude-code-sidecar-draft.test.tsxgui/tests/claude-desktop-row-disclosure.test.tsxgui/tests/claude-desktop-vertical.test.tsxgui/tests/claudecode-layout.test.tsgui/tests/logs-surface-filter.test.tsgui/tests/storage-loading-race.test.tsxgui/tests/subagents-busy-race.test.tsxgui/tests/subagents-classic.test.tsgui/tests/subagents-classic.test.tsxgui/tests/usage-layout.test.ts
| <ApiKeysManagePanel | ||
| keys={keys} | ||
| keysLoading={keysLoading} | ||
| keysLoadFailed={keysLoadFailed} | ||
| newName={newName} | ||
| creating={creating} | ||
| newKey={newKey} | ||
| copied={copied} | ||
| confirmDelete={null} | ||
| localeTag={localeTag} | ||
| showKeyList={false} | ||
| onNewNameChange={onNewNameChange} | ||
| onCreate={onCreate} | ||
| onDismissNewKey={onDismissNewKey} | ||
| onCopyKey={onCopyKey} | ||
| onConfirmDelete={() => {}} | ||
| onCancelDelete={() => {}} | ||
| onDelete={() => {}} | ||
| /> |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
Four stubbed props are dead plumbing here — make them optional in ApiKeysManagePanel.
With showKeyList={false}, ApiKeysManagePanel renders only the new-key/generate blocks (see gui/src/pages/api-keys-panels.tsx Lines 302-342 — confirmDelete, onConfirmDelete, onCancelDelete, onDelete are read exclusively inside the showKeyList && branch). So confirmDelete={null} plus three () => {} stubs exist only to satisfy a required-prop contract. That's a trap: if someone later reads one of those props outside the list branch, the workspace silently no-ops instead of failing a type check.
Suggested contract tightening in gui/src/pages/api-keys-panels.tsx:
- confirmDelete: string | null;
+ confirmDelete?: string | null;
localeTag?: string;
/** When false, only generate / reveal-new-key UI is shown (workspace rail owns the list). */
showKeyList?: boolean;
onNewNameChange: (value: string) => void;
onCreate: () => void;
onDismissNewKey: () => void;
onCopyKey: () => void;
- onConfirmDelete: (id: string) => void;
- onCancelDelete: () => void;
- onDelete: (id: string) => void;
+ onConfirmDelete?: (id: string) => void;
+ onCancelDelete?: () => void;
+ onDelete?: (id: string) => void;Then drop the four stub props from this call site.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@gui/src/components/apikeys-workspace/ApiKeysWorkspace.tsx` around lines 203 -
221, Update the ApiKeysManagePanel props contract in api-keys-panels.tsx to make
confirmDelete, onConfirmDelete, onCancelDelete, and onDelete optional while
preserving their required use within the showKeyList branch. Remove these four
stub props from the ApiKeysManagePanel invocation in ApiKeysWorkspace; leave all
other props unchanged.
| </button> | ||
| <div className="swi-detail-head"> | ||
| <span className="swi-detail-icon"><IconBot style={{ width: 24, height: 24 }} /></span> | ||
| <h2 className="swi-detail-title">{selected}</h2> |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Detail title renders the raw selector while every sibling view uses modelLabel — and it duplicates the row right below it.
Lines 99, 131 and 208 all render modelLabel(m) (which wraps the slug with the provider icon per gui/src/model-display.ts Lines 30-37). Only this heading prints the bare string, so opening a detail pane drops the icon the user just clicked on. Worse, Line 166 immediately restates the same raw string as the sub.workspace.selector value, so the header carries zero extra information.
🎨 Proposed fix
- <h2 className="swi-detail-title">{selected}</h2>
+ <h2 className="swi-detail-title">{modelLabel(selected)}</h2>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <h2 className="swi-detail-title">{selected}</h2> | |
| <h2 className="swi-detail-title">{modelLabel(selected)}</h2> |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@gui/src/components/subagents-workspace/SubagentsWorkspace.tsx` at line 159,
Update the detail heading in SubagentsWorkspace to render the selected model
through modelLabel, preserving the provider icon and consistent display
formatting. Remove the redundant sub.workspace.selector row beneath it so the
selector is not repeated in the detail pane.
877247d to
a2072d5
Compare
c08f45a to
f0acc36
Compare
|
@coderabbitai review |
✅ Action performedReview finished.
|
Merge-ready summaryReview follow-ups from CodeRabbit/Codex that were necessary or useful are addressed. Remaining bot notes are either already fixed on tip, intentionally deferred (e.g. versioned alias family for pathological Merge order (bottom-up): #738 → #739 → #740 → #741. After each merge, retarget the next PR to Codex cloud review is currently usage-capped on this account; local/CI verification was used instead for later rounds. This PR (#741)
Merge-ready after #738–#740 (then retarget to |
Summary
apiBase; API models column fill; Storage cleanup card chromeStack
stack/gui-dashboard-startup-perf) — fix(gui): denser Storage, API Access, Subagents, Usage, and Claude layouts #741Test plan
bun test gui/tests/apikeys-workspace.test.tsx gui/tests/apikeys-layout.test.ts gui/tests/storage-loading-race.test.tsx gui/tests/usage-layout.test.ts gui/tests/subagents-classic.test.tsxSummary by CodeRabbit