refactor(web): decompose AssistantShell into RootLayout + ChatLayout

[ashleeradka]

Prompt / plan

Decompose the monolithic AssistantShell component into proper React Router v7 layout routes, following the *Layout naming convention used in the React Router docs (e.g. AuthLayout).

Problem: AssistantShell mixed app-level concerns (safe-area insets, iOS visual-viewport keyboard tracking) with chat-specific concerns (sidebar rail, mobile drawer, keyboard shortcuts). It was placed in App.tsx wrapping ALL routes — making it impossible to add SettingsLayout as a sibling layout route.

In the platform repo, the architecture is actually three separate shells:

• Layout — app-level (safe areas, optional header)
• AssistantShell — chat-specific (sidebar rail, drawer, shortcuts)
• SettingsShell — settings-specific (overlay panel, settings sidebar)

AssistantShell and SettingsShell are siblings under Layout. The new repo incorrectly flattened this into a single AssistantShell wrapping everything.

Fix: Split into proper layout routes:

RootLayout (pathless — safe areas, viewport tracking)
├── ChatLayout (path="/") — sidebar rail, drawer, shortcuts
│   ├── ChatPage (index)
│   ├── HomePage
│   ├── LibraryPage / LibraryDetailPage
│   └── SettingsTabPage (temporary placeholder)
└── (future: SettingsLayout as sibling — PR 2 of settings migration)

What goes where:

• RootLayout → components/layout/root-layout.tsx — cross-domain, shared by all routes. Safe-area insets (env()), iOS Visual Viewport API keyboard tracking.
• ChatLayout → domains/chat/chat-layout.tsx — chat-domain-specific. Sidebar rail, mobile drawer with focus trap, keyboard shortcuts (Ctrl+\, Ctrl+K, Ctrl+[/]), header with back/forward. Absorbs the history tracking logic previously in App.tsx.
• SideMenu → domains/chat/side-menu.tsx — chat-domain navigation (Home, Chat, Library, Settings links). Not cross-domain shared UI.
• ChatLayoutHeader → domains/chat/chat-layout-header.tsx — renamed from AssistantShellHeader.

What was NOT done (and why):

• Did not rename the CSS class app-shell — it's used in global styles and renaming it has no functional benefit.
• Did not convert to React 19 ref prop pattern in this PR — ChatLayout doesn't use forwardRef. That conversion applies to PanelItem migration (PR 1).
• Did not add SettingsLayout yet — that comes in PR 2 of the settings migration, where it'll be wired as a sibling to ChatLayout.

References:

• React Router — Routing (layout routes)
• React Router — Nested routes
• Visual Viewport API
• env() safe-area-inset

Test plan

• bun run typecheck — 0 errors
• bun run lint — 0 errors
• CI checks

Link to Devin session: https://app.devin.ai/sessions/536ceadb023a4059908b0609b9833bc1
Requested by: @ashleeradka

---

[devin-ai-integration]

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

• Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
• Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

• Disable automatic comment and CI monitoring

[devin-ai-integration]

Human attention: This is an architectural change that affects how all routes are composed.

Risk level: Medium — changes the route hierarchy and component tree for every page.

Where to focus review:

1. routes.tsx — the new layout route nesting (RootLayout → ChatLayout → child routes). Verify the pathless parent + path="/" child pattern is correct for React Router v7 data mode.
2. root-layout.tsx — extracted safe-area + viewport logic. Verify nothing was lost from the original AssistantShell that should be app-level.
3. chat-layout.tsx — absorbs history tracking from App.tsx + all sidebar/drawer/shortcut logic from AssistantShell. Verify the Outlet placement is correct for both mobile and desktop layouts.
4. side-menu.tsx — moved to domains/chat/. Verify the import paths are correct and nothing else depended on the old components/shell/ path.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 8e50886d14

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

[chatgpt-codex-connector]

Do not swallow Ctrl+K without a handler

When the command palette has not been wired, this new global listener still calls preventDefault() for Ctrl/Cmd+K and then does nothing. In the current route setup ChatLayout never passes an onSearchClick/palette handler to the header either, so users on chat routes lose the browser's normal Ctrl/Cmd+K behavior (focus address/search bar) without getting an in-app search. The previous shell only installed this listener when onToggleCommandPalette existed, so this is a regression until the palette is actually connected.

Useful? React with 👍 / 👎.

[devin-ai-integration]

Good catch — this is a real regression. The old AssistantShell only installed the Ctrl+K listener when onToggleCommandPalette was provided. The new code unconditionally called preventDefault() with no action, swallowing the browser's native Ctrl+K.

Fixed in bc72c0f — removed the listener entirely until the command palette is ported. The comment now documents the intent.

[devin-ai-integration]

Devin Review found 2 potential issues.

View 2 additional findings in Devin Review.

[devin-ai-integration]

🚩 Route for /conversations/:conversationId is missing (pre-existing)

The HomePageRoute in routes.tsx:19 navigates to /conversations/${conversationId}, but no route definition matches this path — neither in the old code nor the new. The * catch-all at routes.tsx:57 would match this and render <NotFound />. This is a pre-existing issue unrelated to this PR, but worth noting since the route structure is being reorganized here and this is a good time to add it.

(Refers to line 19)

---

Was this helpful? React with 👍 or 👎 to provide feedback.

[devin-ai-integration]

Confirmed pre-existing — the old routes.tsx also had no /conversations/:id route. ChatPage is currently a placeholder ("Chat UI lands with the platform code port"), so there's no conversation detail component to wire up yet. This will be resolved as part of the Chat UI port (the session that's already handling chat migration). Adding a route pointing to a nonexistent page wouldn't help here — it needs the actual conversation component first.

[devin-ai-integration]

🟡 Ctrl+K listener always calls preventDefault, blocking browser-native shortcut with no replacement

In the old code (assistant-shell.tsx:186-202), the Ctrl+K useEffect had an early return if (!onToggleCommandPalette) return;, and App.tsx never passed onToggleCommandPalette, so no keydown listener was registered and the browser's native Ctrl+K behavior (e.g. Chrome's "focus address bar") was unaffected.

In the new ChatLayout, the effect unconditionally registers a keydown listener that calls event.preventDefault() on every Ctrl/Cmd+K keystroke but performs no action (just a // TODO comment). This silently swallows the shortcut for every user on every page load.

Suggested change

	useEffect(() => {
	const onKeyDown = (event: KeyboardEvent) => {
	if (!shouldHandleShortcut(event, document.activeElement, "k")) {
	return;
	}
	event.preventDefault();
	// TODO: wire command palette when ported
	};
	window.addEventListener("keydown", onKeyDown);
	return () => {
	window.removeEventListener("keydown", onKeyDown);
	};
	}, []);
	// Ctrl/Cmd+K shortcut for command palette
	// TODO: wire command palette when ported — listener is not registered
	// until the handler exists so we don't block the browser's native Ctrl+K.

---

Was this helpful? React with 👍 or 👎 to provide feedback.

[devin-ai-integration]

Already fixed in bc72c0f — removed the listener entirely until the command palette is ported. The effect was swallowing Ctrl+K without any handler, which is a regression from the old AssistantShell that gated on onToggleCommandPalette existing.

[vex-assistant-bot]

✦ APPROVE

Value: Unblocks SettingsLayout as a sibling route by cleanly splitting a 389-line monolith into proper React Router layout routes — net-neutral change (~471+/472-) that removes the architectural constraint preventing chat and settings from being independent layout siblings.

What this does: Splits AssistantShell into RootLayout (cross-domain, pathless — safe-area insets, iOS Visual Viewport keyboard tracking) and ChatLayout (chat-domain — sidebar rail, mobile drawer, focus trap, keyboard shortcuts, history tracking). SideMenu + ChatLayoutHeader move to domains/chat/ where they belong. App.tsx deleted entirely; route hierarchy lives in routes.tsx.

---

root-layout.tsx — Clean extraction. iOS keyboard tracking (KEYBOARD_OPEN_THRESHOLD_PX = 100, translate3d on the inner wrapper) correctly scoped here. Safe-area insets look right. One thing to keep in mind: the old AssistantShell had a viewportOverlays render slot for content that needed to stay anchored to the visual viewport — outside the translate3d wrapper. That slot is gone. Any future overlay needing to escape the transform (e.g. command palette, floating tooltip anchored to the keyboard edge) will need a React portal. This is fine — portals are the right approach — just worth documenting when the command palette is ported.

chat-layout.tsx — All effects have correct cleanups ✓. document.body.style.overflow is restored to its previous value (not just "") ✓. SIDEBAR_COLLAPSED_STORAGE_KEY value is unchanged ("assistantSidebarCollapsed") — existing users' sidebar pref is preserved ✓.

Ctrl+K listener that was swallowing the browser default without a handler is removed in the follow-up commit — right call ✓.

One minor: renderSideMenu is wrapped in useCallback(fn, []) but the callback just renders <SideMenu {...args} /> — memoizing a JSX factory doesn't prevent SideMenu from receiving new element instances anyway. Harmless, could be inlined or defined outside the component.

routes.tsx — Hierarchy is clean and matches the PR description exactly. basename: "/assistant" preserved ✓. Heads-up: NotFound is nested under ChatLayout, so 404s render with the sidebar + header. Fine for now, but when SettingsLayout lands as a sibling, you may want NotFound at the RootLayout level to avoid being layout-specific.

side-menu.tsx + chat-layout-header.tsx — Good domain moves. aria-controls updated to chat-side-menu throughout ✓. data-slot attributes added ✓. Nav items use bg-[var(--surface-lift)] for active/hover — KB calls for --surface-hover on nav rows (anti-patterns-web.md). Pre-existing from old code, not a regression here, but worth aligning in a follow-up.

Anti-patterns: No hex colors, no Tailwind color classes, no --ghost-hover outside button contexts, no barrel files added ✓.

CI: Socket ×2 ✓, Lint/Type Check & Build ✓. All 3 passing.

Reviewed at HEAD bc72c0f7.

Vellum Constitution — Inviting: explicit layout seams and a clean route hierarchy make the codebase feel intentional and lower the cost of adding SettingsLayout as a first-class sibling.