ADE-115: real machine-level Chats tab + projectless chats redesign#781
Conversation
Navigation: split Chats tab existence (session-only personalChatsTabOpen) from active-ness (route-derived). "+" on /chats now opens and activates Home/New Tab while Chats stays as an inactive, clickable tab; closing New Tab returns to Chats when it is the remaining machine-level tab; closing an inactive Chats tab never navigates; the flag survives project open/switch/close so Chats coexists with project tabs. Redesign: hero composer empty state (greeting + composer + verb-first accent chips as one block, docking once a session is selected), provider brand color on composer focus ring/send via --chat-accent, stateful sidebar (streaming dots, accent-edge selection, sticky search, machine footer), legible projectless warning, first-class loading/unavailable/ reconnecting states, and DidYouKnow suppressed on standalone /chats so it can't cover the composer. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Track A fixes: send-glyph contrast now derives from the tint-resolved accent (neutral chrome forces --chat-accent to gray while the glyph was computed from the provider color, leaving a near-invisible icon for light providers); Did-you-know toast suppressed on every /chats visit, not just the projectless one; hero paints immediately during the initial fetch and provider-unavailable waits for the catalog to load. Track B judo: the near-duplicate Chats/New Tab tiles share a new ShellNavTab presentational wrapper (wrapper div, keyboard activation, close-button chrome); TopBar machine-tab tests share a renderChatsTopBar setup helper. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Extend PersonalChatsPage tests with three regression pins: hero + composer stay visible during the initial fetch (no provider-notice flash), selecting a session with unloaded events docks immediately with exactly one composer instance, and the send glyph contrast follows the tint-resolved accent (light provider fixture makes colored vs neutral discriminating). Docs: personal-chats README (nav contract + per-file source map), ARCHITECTURE.md multi-window/global-routing sections, and the onboarding README now describe the machine-level Chats tab backed by personalChatsTabOpen and the /chats DidYouKnow suppression. Mobile, CLI, and TUI verified as no-change (renderer-only branch). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
📝 WalkthroughWalkthroughThe change adds persistent Personal Chats tab state, route-aware Chats/New Tab navigation, reusable shell tab controls, and a redesigned projectless Chats interface with sidebar, hero, composer, session helpers, contrast handling, and expanded tests. ChangesProjectless Chats experience
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
arul28
changed the title
|
@copilot review but do not make fixes |
|
@coderabbitai review |
|
@codex review |
✅ Action performedReview finished.
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e0e0e0060e
ℹ️ 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".
| }, [isLanesRoute]); | ||
|
|
||
| useEffect(() => { | ||
| if (showWelcome && isPersonalChatsRoute) setPersonalChatsTabOpen(true); |
There was a problem hiding this comment.
When a project is already open, the sidebar Chats link navigates to /chats without setting showWelcome; this effect is the only production path that marks personalChatsTabOpen true, so the Chats route renders but TopBar never creates the Chats tab and the project tab remains selected. This breaks the intended project+Chats tab coexistence whenever users open Chats from an active project; open the tab for any /chats route, not only welcome.
Useful? React with 👍 / 👎.
Any /chats visit now sets personalChatsTabOpen (the AppShell effect was gated on showWelcome, so a project's sidebar Chats link rendered the surface with no tab), and project tabs (local + remote) drop their active styling while the Chats machine tab is the foreground surface so two tabs never read active at once. Docs updated; new TopBar case pins the project-tab/Chats-tab styling split. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
@codex review |
![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 (2)
apps/desktop/src/renderer/components/personalChats/sessionHelpers.ts (1)
28-34: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider typing
providerasAgentChatProviderfor compile-time exhaustiveness.Using
stringmeans any new provider silently falls through to"opencode-chat"without a type error. A typed parameter (or an exhaustive switch) would catch missing mappings at compile time.♻️ Optional type-safety improvement
-export function providerToolType(provider: string): Parameters<typeof ToolLogo>[0]["toolType"] { - if (provider === "claude") return "claude-chat"; - if (provider === "codex") return "codex-chat"; - if (provider === "cursor") return "cursor"; - if (provider === "droid") return "droid-chat"; - return "opencode-chat"; +import type { AgentChatProvider } from "../../../shared/types"; + +export function providerToolType(provider: AgentChatProvider): Parameters<typeof ToolLogo>[0]["toolType"] { + switch (provider) { + case "claude": return "claude-chat"; + case "codex": return "codex-chat"; + case "cursor": return "cursor"; + case "droid": return "droid-chat"; + default: return "opencode-chat"; + } }🤖 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 `@apps/desktop/src/renderer/components/personalChats/sessionHelpers.ts` around lines 28 - 34, Update providerToolType to accept the AgentChatProvider type instead of string, and use an exhaustive mapping or switch so every provider has an explicit toolType mapping; ensure unsupported or newly added providers produce a compile-time error rather than defaulting to "opencode-chat".apps/desktop/src/renderer/components/onboarding/OnboardingBootstrap.tsx (1)
2-18: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRoute-matching for
/chatsdiverges from AppShell.tsx.
startsWith("/chats")here is looser than AppShell'spathname === "/chats" || pathname.startsWith("/chats/")— it would also match an unrelated future route like/chatsomething. Harmless today, but consider extracting a single shared helper (e.g.isPersonalChatsRoute(pathname)) used by both AppShell.tsx and here to avoid this kind of drift.♻️ Suggested consolidation
- const suppressTips = location.pathname.startsWith("/chats"); + const suppressTips = isPersonalChatsRoute(location.pathname);(with
isPersonalChatsRouteexported from a small shared module and reused inAppShell.tsx)🤖 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 `@apps/desktop/src/renderer/components/onboarding/OnboardingBootstrap.tsx` around lines 2 - 18, Replace the loose `location.pathname.startsWith("/chats")` check in `OnboardingBootstrap` with a shared `isPersonalChatsRoute(pathname)` helper that matches exactly `/chats` or paths beginning `/chats/`; export and reuse the same helper from `AppShell.tsx` to keep route matching consistent.
🤖 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 `@apps/desktop/src/renderer/components/app/ShellNavTab.tsx`:
- Around line 40-48: Prevent the nested close button’s keyboard events from
reaching the parent tab handler. In the close button’s onKeyDown handler in
ShellNavTab, call event.stopPropagation() before handling Enter or Space, while
preserving onClose() activation and default prevention.
In `@apps/desktop/src/renderer/components/personalChats/PersonalChatsPage.tsx`:
- Around line 502-503: Update the onNewChat callback in PersonalChatsPage to
call setMenuId(null) alongside clearing the selected chat, draft, and mobile
list state. Verify that ProjectlessSidebar’s aria-label “Back to home” matches
the /work destination; if not, update either the label or route for consistency.
In `@apps/desktop/src/renderer/components/personalChats/ProjectlessComposer.tsx`:
- Around line 91-96: Update the Enter handling in ProjectlessComposer’s
onKeyDown callback to skip submission while IME composition is active: require
event.isComposing to be false before calling preventDefault() and onSubmit().
In `@apps/desktop/src/renderer/components/personalChats/ProjectlessSidebar.tsx`:
- Around line 95-101: Add accessible menu behavior around the session actions in
ProjectlessSidebar: update the DotsThree trigger with aria-haspopup="menu" and
aria-expanded, assign role="menu" to the dropdown and role="menuitem" to
Archive/Delete buttons, and support keyboard navigation with Escape closing and
focus management. Add a click-outside listener or backdrop tied to
menuId/onToggleMenu so clicking elsewhere closes the menu, while preserving
propagation handling and cleanup.
---
Nitpick comments:
In `@apps/desktop/src/renderer/components/onboarding/OnboardingBootstrap.tsx`:
- Around line 2-18: Replace the loose `location.pathname.startsWith("/chats")`
check in `OnboardingBootstrap` with a shared `isPersonalChatsRoute(pathname)`
helper that matches exactly `/chats` or paths beginning `/chats/`; export and
reuse the same helper from `AppShell.tsx` to keep route matching consistent.
In `@apps/desktop/src/renderer/components/personalChats/sessionHelpers.ts`:
- Around line 28-34: Update providerToolType to accept the AgentChatProvider
type instead of string, and use an exhaustive mapping or switch so every
provider has an explicit toolType mapping; ensure unsupported or newly added
providers produce a compile-time error rather than defaulting to
"opencode-chat".
🪄 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: CHILL
Plan: Pro
Run ID: c97bda3a-94ca-4688-941c-0b19e0187d2f
⛔ Files ignored due to path filters (3)
docs/ARCHITECTURE.mdis excluded by!docs/**docs/features/onboarding-and-settings/README.mdis excluded by!docs/**docs/features/personal-chats/README.mdis excluded by!docs/**
📒 Files selected for processing (16)
apps/desktop/src/renderer/components/app/App.workKeepAlive.test.tsxapps/desktop/src/renderer/components/app/AppShell.tsxapps/desktop/src/renderer/components/app/ShellNavTab.tsxapps/desktop/src/renderer/components/app/TopBar.test.tsxapps/desktop/src/renderer/components/app/TopBar.tsxapps/desktop/src/renderer/components/chat/chatSurfaceTheme.tsapps/desktop/src/renderer/components/onboarding/OnboardingBootstrap.tsxapps/desktop/src/renderer/components/personalChats/PersonalChatsPage.test.tsxapps/desktop/src/renderer/components/personalChats/PersonalChatsPage.tsxapps/desktop/src/renderer/components/personalChats/ProjectlessComposer.tsxapps/desktop/src/renderer/components/personalChats/ProjectlessHero.tsxapps/desktop/src/renderer/components/personalChats/ProjectlessSidebar.tsxapps/desktop/src/renderer/components/personalChats/sessionHelpers.tsapps/desktop/src/renderer/index.cssapps/desktop/src/renderer/state/appStore.test.tsapps/desktop/src/renderer/state/appStore.ts
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6e43f48b7f
ℹ️ 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".
| else if (isCurrent) projectTabState = "active"; | ||
| // While the Chats machine tab is the foreground surface, the | ||
| // bound project tab stays rendered but must not also read active. | ||
| else if (isCurrent && !personalChatsRouteActive) projectTabState = "active"; |
There was a problem hiding this comment.
When /chats is foregrounded, this line makes the current project tab look inactive, but the tab click path still only calls handleSwitchProject: for the already-current project that handler just calls cancelNewTab() and returns, and for another project the route-preservation effect skips navigation while isPersonalChatsRoute is true. As a result, clicking an inactive project tab from the Chats tab does not foreground the project; users have to close Chats or use another nav path instead.
Useful? React with 👍 / 👎.
Project tabs clicked while Chats is foreground now navigate off /chats (current project included) so the surface actually foregrounds; the ShellNavTab close button stops keyboard events from bubbling into the wrapper's activate handler; Enter no longer submits mid-IME composition; the sidebar session menu gains menu/menuitem roles, aria-haspopup/expanded, Escape-to-close, and click-outside dismissal; New chat clears any open session menu. Pinned by three new tests. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
@codex review |
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 012591bb67
ℹ️ 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".
| const sendButton = screen.getByLabelText("Send message") as HTMLButtonElement; | ||
| expect(sendButton.className).toContain("bg-[color:var(--chat-accent)]"); | ||
| // Light provider accent on colored tint → dark glyph for contrast. | ||
| expect(sendButton.style.color).toBe("rgb(28, 25, 23)"); |
There was a problem hiding this comment.
This assertion can run as soon as the hero text is present, before the async catalog load and fallback modelId effect have completed. In that state the send button still uses the default purple/white contrast, and npx vitest run src/renderer/components/personalChats/PersonalChatsPage.test.tsx fails here with rgb(255, 255, 255) instead of the expected dark glyph. Wait for the resolved button color/model state before asserting so the new test is deterministic.
Useful? React with 👍 / 👎.
|
|
||
| const bridge = (window as unknown as { ade: { personalChats: { call: ReturnType<typeof vi.fn> } } }); | ||
| const sendish = bridge.ade.personalChats.call.mock.calls.filter( | ||
| ([{ action }]: [CallArgs]) => action === "create" || action === "send", |
There was a problem hiding this comment.
The new IME test narrows each mock.calls entry to [CallArgs], but Vitest exposes those entries as generic argument arrays, so TypeScript cannot prove there is exactly one element. npm --prefix apps/desktop run typecheck fails on this callback, leaving desktop validation red; extract/cast the first argument inside the filter instead of typing the whole parameter as a fixed tuple.
Useful? React with 👍 / 👎.
…erts (codex P1) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 416f560f1c
ℹ️ 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".
| // Clicking a project tab while the Chats machine tab is foreground must | ||
| // leave /chats, or ProjectTabHost's route replay (which skips personal | ||
| // chats routes) never surfaces the project. | ||
| if (personalChatsRouteActive) onNavigate?.("/work", { replace: true }); |
There was a problem hiding this comment.
When Chats is foreground after the user came from a project route like /prs or /files, clicking the project tab first replaces the app location with /work. I checked ProjectTabHost's route-cache effects: once the location changes away from /chats, the active project's stored route is updated from the current location, so this hard-coded navigation overwrites the project's last route with /work before the tab is reactivated. That makes project tabs lose their per-project position whenever users return from Chats; route back through the stored project route instead of always writing /work.
Useful? React with 👍 / 👎.
| <div className="flex h-full items-center justify-center"> | ||
| <SpinnerGap size={18} className="animate-spin text-muted-fg/25" /> |
There was a problem hiding this comment.
If the first send fails after createSession() has already created and selected a session, the selected session has no events; once history returns an empty array and loading is false, heroMode is also false, so this branch shows only a spinner forever in the main pane. This can happen with provider/network errors on the initial message and leaves the chat looking like it is still loading even though there is no pending history fetch; render a stable empty selected-session state after history completes instead of the indefinite spinner.
Useful? React with 👍 / 👎.
…chats (codex P2s) Project-tab clicks from the Chats machine tab now navigate to the current binding's stored route (extracted read/write helpers into app/projectRouteStorage.ts, shared by ProjectTabHost and TopBar) so returning from Chats no longer stamps /work over a project's remembered /prs//files position. A selected chat whose history has settled empty (e.g. failed first send) renders a stable empty state instead of an indefinite spinner. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
@codex review |
|
Codex Review: Didn't find any major issues. What shall we delve into next? Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
…urn (greptile P1) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
@codex review |
|
Codex Review: Didn't find any major issues. Chef's kiss. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
arul28
deleted the
ade-115-fix-navigation-and-redesign-the-projectless-chats-experience
branch
1 participant
Fixes the projectless-Chats navigation dead-end and redesigns the surface (Linear ADE-115, closes #769).
Navigation
The "Chats" top tab was a route-derived pseudo-tab (
standaloneChatsActive = showWelcome && path === "/chats") hardcoded active, andopenNewTab()never navigated — so "+" on /chats was a visual no-op. This PR splits tab existence (new session-onlypersonalChatsTabOpenappStore flag, deliberately omitted from every project-transition reset) from active-ness (route-derived, passed to TopBar as a prop — TopBar stays router-hook-free):ShellNavTabwrapper (chrome, keyboard activation, close button).Redesign
--chat-accentscope; glyph contrast computed from the tint-resolved accent (effectiveChatAccent+chatAccentContrast).Contracts preserved
Project-backed chat renders exactly as before (shared components got prop values only); keep-alive invariants intact; entering/leaving /chats never clears the window's project/remote binding; web client unaffected.
Validation
Typecheck clean; 158 targeted tests green (TopBar 51 incl. 4 new tab-interplay cases, keep-alive 19 incl. round trip, personalChats 10 new, appStore transition invariants, TabNav); live renderer round trip verified (+ → Home with inactive Chats tab → click back → close semantics); docs updated (personal-chats README, ARCHITECTURE.md, onboarding README).
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
Greptile Summary
This PR turns personal Chats into a real machine-level tab and refreshes the projectless chat surface. The main changes are:
personalChatsTabOpenstate that survives project open, switch, and close transitions.TopBarwithout adding router hooks there.ShellNavTabfor keyboard activation and close behavior./chats.Confidence Score: 5/5
Safe to merge with minimal risk.
The changed paths are well-scoped to navigation state and the projectless Chats UI. Targeted tests cover the Chats/New Tab interplay, active-turn submit behavior, loading states, provider availability, IME Enter handling, and accent contrast. No accepted correctness or security issues remain from this review.
No files require special attention.
What T-Rex did
Important Files Changed
/chats, and project tab deactivation while Chats is foreground./chatsvisits as opening the machine-level Chats tab and passes route activity/navigation intoTopBar.personalChatsTabOpenstate and keeps it out of project-transition resets.Sequence Diagram
%%{init: {'theme': 'neutral'}}%% sequenceDiagram participant User participant AppShell participant TopBar participant AppStore participant ProjectTabHost participant PersonalChatsPage User->>AppShell: Navigate to /chats AppShell->>AppStore: setPersonalChatsTabOpen(true) AppShell->>TopBar: "personalChatsRouteActive=true" ProjectTabHost->>PersonalChatsPage: Render Chats foreground User->>TopBar: Click + / Open another project TopBar->>AppStore: openNewTab() TopBar->>AppShell: onNavigate('/work') AppShell->>ProjectTabHost: Show New Tab or project surface User->>TopBar: Click Chats tab TopBar->>AppShell: onNavigate('/chats') ProjectTabHost->>PersonalChatsPage: Restore Chats foreground%%{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"}}}%% sequenceDiagram participant User participant AppShell participant TopBar participant AppStore participant ProjectTabHost participant PersonalChatsPage User->>AppShell: Navigate to /chats AppShell->>AppStore: setPersonalChatsTabOpen(true) AppShell->>TopBar: "personalChatsRouteActive=true" ProjectTabHost->>PersonalChatsPage: Render Chats foreground User->>TopBar: Click + / Open another project TopBar->>AppStore: openNewTab() TopBar->>AppShell: onNavigate('/work') AppShell->>ProjectTabHost: Show New Tab or project surface User->>TopBar: Click Chats tab TopBar->>AppShell: onNavigate('/chats') ProjectTabHost->>PersonalChatsPage: Restore Chats foregroundReviews (6): Last reviewed commit: "ship: iteration 5 — Enter falls through ..." | Re-trigger Greptile