Merged
Conversation
📝 WalkthroughWalkthroughAdds a HomePage and route, a ContentNavigation breadcrumb component, integrates a resizable agent chat sidebar into Layout (pointer + keyboard resizing), exposes an optional ChangesHome, Breadcrumbs, and Layout Integration
Sequence Diagram(s)sequenceDiagram
participant User as User (browser)
participant Menu as Menu
participant Router as Router/App
participant Layout as Layout
participant Breadcrumb as ContentNavigation
participant Agent as AgentChatPanel
User->>Menu: click "Open Agent"
Menu->>Layout: call onAgentModeToggle / toggle agent mode
Layout->>Agent: render AgentChatPanel (passes onClose)
Agent->>Layout: invoke onClose (optional)
User->>Layout: drag divider / press Arrow keys to resize
Layout->>Layout: update & persist agentSidebarWidth (clamp)
Router->>Breadcrumb: route change (/home)
Breadcrumb->>Layout: provide breadcrumb items
Layout->>User: render HomePage and breadcrumbs
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@ui/src/layouts/ContentNavigation.tsx`:
- Around line 121-128: When handling agent routes, don't always push a second
literal 'Agent' for nested paths; instead, keep the Administration and top-level
Agent items then, if normalized !== '/agent', push a label derived from
STATIC_ROUTE_LABELS[normalized] or a fallback that preserves the actual subpage
(e.g., humanize segments[1] or join relevant segments) so routes like
/agent-souls/new and /agent-souls/:soulId render the specific page label; update
the block that uses segments, normalized, STATIC_ROUTE_LABELS and items.push
accordingly.
In `@ui/src/layouts/Layout.tsx`:
- Around line 85-104: The saved agentSidebarWidth must be re-clamped whenever
the viewport changes: call clampAgentSidebarWidth(getSavedWidth()) and update
the agentSidebarWidth state (and persisted desktopSidebarWidth) inside the
resize handler and any place where saved width is loaded (initialization and
effects that read desktopSidebarWidth) so the value never exceeds
getAgentSidebarMaxWidth(); specifically, ensure getAgentSidebarMaxWidth() and
clampAgentSidebarWidth(width) are used to clamp values in the resize event
listener and the code paths that set agentSidebarWidth/desktopSidebarWidth
(initial load and any effect handlers around the divider drag/save logic) so the
width is always reset to Math.min(getAgentSidebarMaxWidth(),
Math.max(AGENT_SIDEBAR_MIN_WIDTH, Math.round(width))).
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 99abe7c5-ee02-46ae-a052-55a332751aeb
📒 Files selected for processing (8)
ui/src/App.tsxui/src/features/agent/components/AgentChatPanel.tsxui/src/layouts/ContentNavigation.tsxui/src/layouts/Layout.tsxui/src/layouts/__tests__/Layout.test.tsxui/src/menu.tsxui/src/pages/home/__tests__/index.test.tsxui/src/pages/home/index.tsx
Collaborator
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
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)
ui/src/layouts/Layout.tsx (1)
1-10:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAdd the repository license header to this file.
ui/src/layouts/Layout.tsxis part of the changed surface, but it still lacks the required GPL header used elsewhere in the UI sources.📄 Minimal fix
+// Copyright (C) 2026 Yota Hamada +// SPDX-License-Identifier: GPL-3.0-or-later + import { LicenseBanner } from '@/components/LicenseBanner'; import { UpdateBanner } from '@/components/UpdateBanner';As per coding guidelines,
**/*.{go,ts,tsx,js}: Apply GPL v3 license headers on source files, managed viamake addlicense.🤖 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 `@ui/src/layouts/Layout.tsx` around lines 1 - 10, This file is missing the project's GPL v3 license header; add the same GPL v3 header block used across other UI sources to the top of the Layout component file (the file that defines the Layout component and imports LicenseBanner, UpdateBanner, useConfig, etc.). Ensure the header exactly matches the format used by the repo (so automated tooling recognizes it) or run the repository helper (make addlicense) to apply it consistently.
🤖 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 `@ui/src/layouts/Layout.tsx`:
- Around line 396-400: The main area currently stacks
ContentNavigation/UpdateBanner/LicenseBanner before a body div with h-full which
causes the scroller to include the header; change the <main> (Layout component)
to be a column flex container (e.g., add flex flex-col min-h-0 overflow-hidden)
and move scrolling responsibility to the body wrapper by making the body div the
flex-1 overflow-auto container (remove h-full on the body). Specifically update
the element that renders ContentNavigation, UpdateBanner, LicenseBanner and the
children so ContentNavigation/UpdateBanner/LicenseBanner are static siblings
above the scrolling region and the children container (the div wrapping
{children}) is the only element with flex-1 and overflow-auto and min-h-0.
---
Outside diff comments:
In `@ui/src/layouts/Layout.tsx`:
- Around line 1-10: This file is missing the project's GPL v3 license header;
add the same GPL v3 header block used across other UI sources to the top of the
Layout component file (the file that defines the Layout component and imports
LicenseBanner, UpdateBanner, useConfig, etc.). Ensure the header exactly matches
the format used by the repo (so automated tooling recognizes it) or run the
repository helper (make addlicense) to apply it consistently.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d4543277-2538-42ad-9356-9c421f2d64be
📒 Files selected for processing (4)
ui/e2e/distributed-stack.spec.tsui/src/layouts/ContentNavigation.tsxui/src/layouts/Layout.tsxui/src/layouts/__tests__/Layout.test.tsx
Collaborator
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
🧹 Nitpick comments (1)
ui/src/layouts/Layout.tsx (1)
185-194: 💤 Low valueConsider persisting
agentSidebarWidthonly at drag end.This effect runs synchronously on every state update, including each
pointermovetick during a drag (see the handler at Line 207-209). Each tick triggers a synchronouslocalStorage.setItem, which can be tens of writes per second while resizing. Persisting onpointerup(or via a small debounce onisResizingAgentSidebartransitioning tofalse) achieves the same persistence without serializing to disk on every frame.♻️ Possible refactor
React.useEffect(() => { + if (isResizingAgentSidebar) { + return; + } try { localStorage.setItem( AGENT_SIDEBAR_WIDTH_STORAGE_KEY, agentSidebarWidth.toString() ); } catch { // Ignore unavailable storage. } - }, [agentSidebarWidth]); + }, [agentSidebarWidth, isResizingAgentSidebar]);🤖 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 `@ui/src/layouts/Layout.tsx` around lines 185 - 194, The current React.useEffect writes AGENT_SIDEBAR_WIDTH_STORAGE_KEY to localStorage on every agentSidebarWidth change (including each pointermove during a drag); change this to persist only when the drag finishes: move the localStorage.setItem call out of the synchronous effect and run it in the resize end handler (e.g., the pointerup / onPointerUp that toggles isResizingAgentSidebar) or trigger it when isResizingAgentSidebar transitions from true to false (or use a short debounce on that transition) so you stop doing tens of setItem writes per second; reference agentSidebarWidth, AGENT_SIDEBAR_WIDTH_STORAGE_KEY and the isResizingAgentSidebar/pointerup handler to locate where to place the write.
🤖 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.
Nitpick comments:
In `@ui/src/layouts/Layout.tsx`:
- Around line 185-194: The current React.useEffect writes
AGENT_SIDEBAR_WIDTH_STORAGE_KEY to localStorage on every agentSidebarWidth
change (including each pointermove during a drag); change this to persist only
when the drag finishes: move the localStorage.setItem call out of the
synchronous effect and run it in the resize end handler (e.g., the pointerup /
onPointerUp that toggles isResizingAgentSidebar) or trigger it when
isResizingAgentSidebar transitions from true to false (or use a short debounce
on that transition) so you stop doing tens of setItem writes per second;
reference agentSidebarWidth, AGENT_SIDEBAR_WIDTH_STORAGE_KEY and the
isResizingAgentSidebar/pointerup handler to locate where to place the write.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 38a8d691-6837-4ced-8d07-de44991a110f
📒 Files selected for processing (1)
ui/src/layouts/Layout.tsx
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
/homenavigation hub modeled after the Administration page card layout.Validation
pnpm --dir ui test src/layouts/__tests__/Layout.test.tsx src/pages/home/__tests__/index.test.tsx src/__tests__/menu.test.tsxpnpm --dir ui typecheckpnpm --dir ui exec eslint src/App.tsx src/layouts/Layout.tsx src/layouts/ContentNavigation.tsx src/layouts/__tests__/Layout.test.tsx src/pages/home/index.tsx src/pages/home/__tests__/index.test.tsx src/menu.tsx src/features/agent/components/AgentChatPanel.tsxSummary by CodeRabbit
New Features
Tests