feat(gui): Dashboard workspace view + drop Providers classic toggle#428
Conversation
Adds a workspace layout for the Dashboard tab following the Providers workspace DNA. The left rail lists the page sections (Overview, Active providers, Available models); the main pane renders the selected section. Section bodies and the update dialog are extracted into shared blocks so classic and workspace views behave identically. Classic view stays available behind a toggle persisted in localStorage (ocx-dashboard-view). New i18n keys for the Overview section and rail header in all six locales.
…ex/gui-dashboard-workspace
Adds a Workspace/Classic toggle to the sidebar footer that switches the whole app's view preference at once. It writes every tab's ocx-*-view localStorage key and bumps a render key so the current page remounts and re-reads its preference immediately. Per-page toggles keep working, and the sidebar label re-reads the preference on navigation so it stays accurate. Future tabs only need to register their key in WORKSPACE_VIEW_KEYS. Also fills in the 10 missing sub.workspace.* keys in ja.ts that the rebase onto dev surfaced. New i18n key app.viewMode in all six locales; reuses existing pws.workspaceToggle/classicToggle labels.
Bring latest upstream GUI features (ErrorBoundary, Startup page, Select portal, Codex auto-switch) into the solo dashboard branch while preserving the workspace dashboard layout and global workspace toggle.
Remove the per-tab Classic/Workspace buttons from Providers (sidebar owns view mode). Default Select menus to viewport-aware portals, keep language and dialog selects in-tree, restore Codex auto-switch CSS, treat stale startup health as error, and bump the shadow-call mutation epoch only after a successful write.
This comment was marked as resolved.
This comment was marked as resolved.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Use a canonical ocx-view preference, sync Providers hash on global toggle, remount only Dashboard/Providers, snapshot poll epochs before fetch, keep startup health on its dedicated writer, drop the duplicate project-config poll from the 5s loop, and restore guidance-disabled controls plus docs.
…y, poll ownership Pass shared Classic/Workspace mode as props instead of remounting pages, use ocx-global-view with deterministic migration, keep Providers hash aligned, seed startup health only while unknown, own project-config diagnostics on a 30s poll, and add keyboard listbox behaviour for Select plus focused regression tests.
…idge-jun#428) Use history.replaceState for preference/hash reconciliation so Back is not trapped, keep navigateHash for deliberate sidebar/toggle navigation, and expose a select-only combobox ARIA contract with Tab committing the active option.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Keep optimistic shadow-call UI when a poll JSON parse races an in-flight save, dedupe legacy view-mode key literals, and cover option-click selection.
Replace Providers view-mode hashes in place so Back returns to the prior page, keep page+viewMode updates atomic, and test production useAppRouteState with real history.back/forward.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
…extract Assert the exact providers/workspace hash ownership against app-routing and useAppRouteState instead of scraped App.tsx literals.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
gui/src/pages/Dashboard.tsx (1)
325-354: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winDuplicated epoch-gate check between the success and catch branches — extract to avoid future divergence.
The same
settingsPollMayCommit({...}, {...})object literal is repeated verbatim in thetry(Line 330-337) andcatch(Line 344-351) branches. This gate exists precisely to protect a race (parse failure vs. in-flight save/stale poll identity); if it's edited in only one branch during future maintenance, the two paths silently diverge and the original optimistic-UI-clobbering bug can reappear in whichever branch was missed.♻️ Suggested fix: single shared gate closure
+ const shadowCallMayCommit = () => settingsPollMayCommit( + { request: shadowRequestEpoch, mutation: shadowMutationEpoch }, + { + request: shadowCallRequestEpochRef.current, + mutation: shadowCallMutationEpochRef.current, + mutationInFlight: shadowCallMutationInFlightRef.current, + }, + ); try { if (shRes.ok) { const nextShadow = await shRes.json() as ShadowCallData; - if (settingsPollMayCommit( - { request: shadowRequestEpoch, mutation: shadowMutationEpoch }, - { - request: shadowCallRequestEpochRef.current, - mutation: shadowCallMutationEpochRef.current, - mutationInFlight: shadowCallMutationInFlightRef.current, - }, - )) { - setShadowCall(nextShadow); - } + if (shadowCallMayCommit()) setShadowCall(nextShadow); } } catch { - if (settingsPollMayCommit( - { request: shadowRequestEpoch, mutation: shadowMutationEpoch }, - { - request: shadowCallRequestEpochRef.current, - mutation: shadowCallMutationEpochRef.current, - mutationInFlight: shadowCallMutationInFlightRef.current, - }, - )) { - setShadowCall(null); - } + if (shadowCallMayCommit()) setShadowCall(null); }🤖 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/Dashboard.tsx` around lines 325 - 354, Extract the duplicated settingsPollMayCommit call in the surrounding polling logic into one shared gate value or closure before the try/catch, using the existing shadow request and mutation epoch refs. Reuse that single gate for both the successful JSON path and the parse-failure path so setShadowCall updates remain protected by identical race checks.
🤖 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/use-app-route-state.ts`:
- Around line 91-98: Remove setPageState from the public return object of
useAppRouteState, while retaining page, viewMode, toggleGlobalWorkspace, and
navigateToPage. Update any related hook typings or consumers if needed so the
exposed API no longer permits direct route-state mutation.
---
Outside diff comments:
In `@gui/src/pages/Dashboard.tsx`:
- Around line 325-354: Extract the duplicated settingsPollMayCommit call in the
surrounding polling logic into one shared gate value or closure before the
try/catch, using the existing shadow request and mutation epoch refs. Reuse that
single gate for both the successful JSON path and the parse-failure path so
setShadowCall updates remain protected by identical race checks.
🪄 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: b2f37eeb-675e-4ad6-afb8-f8566b22ba80
📒 Files selected for processing (9)
gui/src/App.tsxgui/src/app-routing.tsgui/src/pages/Dashboard.tsxgui/src/use-app-route-state.tsgui/src/view-mode.tsgui/tests/dashboard-contracts.test.tsgui/tests/providers-hash-history.test.tsxgui/tests/select-keyboard.test.tsxgui/tests/view-mode.test.ts
| return { | ||
| page, | ||
| viewMode, | ||
| setPageState, | ||
| toggleGlobalWorkspace, | ||
| navigateToPage, | ||
| }; | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Confirm no other file destructures/uses setPageState from useAppRouteState.
rg -nP '\bsetPageState\b' gui/src gui/testsRepository: lidge-jun/opencodex
Length of output: 429
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== use-app-route-state.ts =="
cat -n gui/src/use-app-route-state.ts | sed -n '1,140p'
echo
echo "== direct consumers of useAppRouteState =="
rg -n "useAppRouteState|setPageState" gui/src gui/testsRepository: lidge-jun/opencodex
Length of output: 4954
Remove setPageState from useAppRouteState. gui/src/App.tsx and gui/tests/providers-hash-history.test.tsx only use page, viewMode, toggleGlobalWorkspace, and navigateToPage; keeping the raw setter in the hook’s public return value leaves a footgun that can desync page from the hash/view mode. Drop it from gui/src/use-app-route-state.ts:91-98.
🤖 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/use-app-route-state.ts` around lines 91 - 98, Remove setPageState
from the public return object of useAppRouteState, while retaining page,
viewMode, toggleGlobalWorkspace, and navigateToPage. Update any related hook
typings or consumers if needed so the exposed API no longer permits direct
route-state mutation.
…t storage Style select-option-active so keyboard navigation is visible without hover, and drive Providers hashes from in-memory viewMode so a localStorage failure cannot revert a live Classic/Workspace toggle.
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
gui/src/use-app-route-state.ts (1)
102-109: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win
setPageStateis still exposed on the public hook API.Calling it directly bypasses
navigateHash/providersHashForViewMode, lettingpagedesync from the URL hash andviewMode. Onlypage,viewMode,toggleGlobalWorkspace, andnavigateToPageare consumed byApp.tsxand the test harness.♻️ Proposed fix
return { page, viewMode, - setPageState, toggleGlobalWorkspace, navigateToPage, };🤖 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/use-app-route-state.ts` around lines 102 - 109, Remove setPageState from the object returned by the useAppRouteState hook, while retaining its internal use where needed. Keep page, viewMode, toggleGlobalWorkspace, and navigateToPage exposed so callers use navigateToPage and preserve URL hash/viewMode synchronization.
🤖 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/tests/dashboard-contracts.test.ts`:
- Around line 19-32: Replace the source-text assertions in the workspace
landmark and multi-agent guidance tests with rendered Dashboard.tsx behavior
tests, mocking network dependencies as needed. Use DOM queries to verify the
workspace section’s accessible label and landmark structure, and render both
enabled and disabled guidance states to assert the actual controls’ disabled
visibility/gating behavior. Remove the brittle string and regex checks for
dashboard-workspace-main, dash.workspace.sections, multiAgentGuidanceEnabled,
and models.v2Mode_.
---
Duplicate comments:
In `@gui/src/use-app-route-state.ts`:
- Around line 102-109: Remove setPageState from the object returned by the
useAppRouteState hook, while retaining its internal use where needed. Keep page,
viewMode, toggleGlobalWorkspace, and navigateToPage exposed so callers use
navigateToPage and preserve URL hash/viewMode synchronization.
🪄 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: 9f3a5a05-6091-4afc-a154-772ec73fa0e9
📒 Files selected for processing (6)
gui/src/styles.cssgui/src/use-app-route-state.tsgui/tests/dashboard-contracts.test.tsgui/tests/providers-hash-history.test.tsxgui/tests/select-keyboard.test.tsxtests/provider-workspace-rail.test.ts
| test("Dashboard workspace pane is a labelled section, not a nested main landmark", async () => { | ||
| const src = await Bun.file(new URL("../src/pages/Dashboard.tsx", import.meta.url)).text(); | ||
| expect(src).toContain("dashboard-workspace-main"); | ||
| expect(src).toContain("dash.workspace.sections"); | ||
| expect(src).not.toMatch(/<main\b[^>]*dashboard-workspace-main/); | ||
| expect(src).toMatch(/<(section)\b[^>]*dashboard-workspace-main/); | ||
| }); | ||
|
|
||
| test("multi-agent guidance gates injection controls and Active badge on the enabled flag", async () => { | ||
| const src = await Bun.file(new URL("../src/pages/Dashboard.tsx", import.meta.url)).text(); | ||
| expect(src).toContain("!multiAgentGuidanceEnabled"); | ||
| expect(src).toContain("multiAgentGuidanceEnabled &&"); | ||
| expect(src).toContain("models.v2Mode_"); | ||
| }); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
Two more contract tests added via raw source-text scraping instead of runtime behavior.
Past reviews on this file already flagged this pattern for the diagnostics/project-config test: asserting on literal substrings/regex of Dashboard.tsx's source text is brittle (a cosmetic rename/reformat fails it) and can miss real regressions (a bug that happens to retain the matched substring passes). Lines 19-25 and 27-32 add two more instances of the same anti-pattern for the workspace landmark and multi-agent gating contracts.
Prefer rendering Dashboard.tsx (mocking network) and asserting on the actual DOM: query for the <section> landmark's aria-label/role instead of regexing the JSX source, and render the guidance controls to assert the real disabled/gating wiring.
🤖 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/tests/dashboard-contracts.test.ts` around lines 19 - 32, Replace the
source-text assertions in the workspace landmark and multi-agent guidance tests
with rendered Dashboard.tsx behavior tests, mocking network dependencies as
needed. Use DOM queries to verify the workspace section’s accessible label and
landmark structure, and render both enabled and disabled guidance states to
assert the actual controls’ disabled visibility/gating behavior. Remove the
brittle string and regex checks for dashboard-workspace-main,
dash.workspace.sections, multiAgentGuidanceEnabled, and models.v2Mode_.
1 participant
Summary
Test plan
bun test ./gui/tests/select-portal.test.tsx ./gui/tests/select-position.test.tsbun run typecheck/ GUI smoke against a local proxySummary by CodeRabbit