refactor(frontend): ONE agent overview — body, actions menu and rail, on both surfaces - #5884
refactor(frontend): ONE agent overview — body, actions menu and rail, on both surfaces#5884ardaerzin wants to merge 12 commits into
Conversation
|
@coderabbitai review |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository YAML (base), Organization UI (inherited) Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe PR consolidates agent overview layouts into shared UI components, adds reusable agent and session actions, introduces mobile agent composition, and centralizes sidebar navigation controls and version display. ChangesAgent overview consolidation
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant AgentOverview
participant AgentOverviewBody
participant SessionListCard
participant useSessionCardVerbs
AgentOverview->>AgentOverviewBody: render overview props
AgentOverviewBody->>SessionListCard: render scoped session cards
SessionListCard->>useSessionCardVerbs: use row and menu callbacks
useSessionCardVerbs->>AgentOverview: open selected agent session
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
📝 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 |
f4738a0 to
acf18f4
Compare
fe74b64 to
2a8c549
Compare
acf18f4 to
001fad6
Compare
2a8c549 to
4218be4
Compare
|
@coderabbitai review |
✅ Action performedReview finished.
|
![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 (3)
web/packages/agenta-entity-ui/src/agent/AgentActionsMenu.tsx (1)
13-29: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReduce the new implementation comments.
These comments exceed the repository limit. Keep only short comments that describe non-obvious constraints.
web/packages/agenta-entity-ui/src/agent/AgentActionsMenu.tsx#L13-L29: Remove or reduce the prop and component comments to one short line each.web/packages/agenta-entity-ui/src/agent/useAgentActions.tsx#L16-L22: Remove or reduce the hook comment to one short line.web/oss/src/pages/w/[workspace_id]/p/[project_id]/apps/[app_id]/overview/index.tsx#L63-L64: Remove or reduce the host-flow comment to one short line.As per coding guidelines, keep in-code comments to at most one short line.
Source: Coding guidelines
web/packages/agenta-home-ui/src/HomeTaskComposer.tsx (1)
69-96: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRun the formatter on this block.
The
<span>wrapper was added around the existingSelect, but theSelectsubtree keeps its previous indentation, so lines 74-93 are misaligned inside the new element. This indicates the formatter did not run on this change.As per coding guidelines: "Before committing frontend changes, run
pnpm lint-fixfrom thewebdirectory."Source: Coding guidelines
web/packages/agenta-entity-ui/src/agent/AgentFilesCard.tsx (1)
124-133: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueHoist
TooltipProviderout ofrows.map.A shared provider preserves Radix skip-delay behavior across file triggers and avoids one provider context per row. The local provider does not render a DOM wrapper.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: daa09821-d4fd-4182-b0ac-3bb90533ffc5
⛔ Files ignored due to path filters (1)
web/pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (23)
web/mobile/src/features/agents/AgentComposer.tsxweb/mobile/src/features/agents/AgentOverviewScreen.tsxweb/mobile/src/features/agents/AgentOverviewSection.tsxweb/mobile/src/features/nav/mobileNavScope.tsxweb/mobile/src/features/nav/useMobileNavItems.tsxweb/oss/src/components/Sidebar/components/SidebarToggleButton.tsxweb/oss/src/components/pages/overview/agent/AgentConfigurationCard.tsxweb/oss/src/components/pages/overview/agent/AgentOverview.tsxweb/oss/src/components/pages/sessions/components/SessionListCard.tsxweb/oss/src/components/pages/sessions/components/useSessionCardVerbs.tsweb/oss/src/pages/w/[workspace_id]/p/[project_id]/apps/[app_id]/overview/index.tsxweb/packages/agenta-entity-ui/package.jsonweb/packages/agenta-entity-ui/src/agent/AgentActionsMenu.tsxweb/packages/agenta-entity-ui/src/agent/AgentFilesCard.tsxweb/packages/agenta-entity-ui/src/agent/AgentOverviewBody.tsxweb/packages/agenta-entity-ui/src/agent/index.tsweb/packages/agenta-entity-ui/src/agent/useAgentActions.tsxweb/packages/agenta-home-ui/src/HomeTaskComposer.tsxweb/packages/agenta-navigation-ui/src/NavMenu.tsxweb/packages/agenta-navigation-ui/src/SidebarToggleButton.tsxweb/packages/agenta-navigation-ui/src/index.tsweb/packages/agenta-sessions-ui/src/SessionListCard.tsxweb/packages/agenta-sessions-ui/src/index.ts
💤 Files with no reviewable changes (2)
- web/mobile/src/features/agents/AgentOverviewSection.tsx
- web/oss/src/components/pages/overview/agent/AgentConfigurationCard.tsx
| const start = async ({text}: {agentId: string; text: string}) => { | ||
| const staged = attachments.files | ||
| const parts = staged.length > 0 ? stagedFilesToParts(staged, sessionId) : undefined | ||
| stash({sessionId, task: {agentId, text, parts}}) | ||
| attachments.clearAttachments(staged.map((file) => file.uid)) | ||
| await router.push(`${base}/sessions/${sessionId}?agent=${agentId}`) | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Handle a failed router.push in start.
start clears the staged attachments and stashes the pending task before it navigates. If router.push rejects, the task stays in pendingTasksAtom under a session that the user never opens, the attachments are already cleared, and the rejection propagates out of the composer's submit handler as an unhandled rejection. The desktop equivalent guards this: useOpenAgentSession in web/oss/src/components/AgentChatSlice/hooks/useOpenAgentSession.ts clears the pending target when the push fails.
🛡️ Proposed fix
const start = async ({text}: {agentId: string; text: string}) => {
const staged = attachments.files
const parts = staged.length > 0 ? stagedFilesToParts(staged, sessionId) : undefined
stash({sessionId, task: {agentId, text, parts}})
attachments.clearAttachments(staged.map((file) => file.uid))
- await router.push(`${base}/sessions/${sessionId}?agent=${agentId}`)
+ // Drop the stashed task on a failed navigation, or it is adopted by the next session view.
+ await router.push(`${base}/sessions/${sessionId}?agent=${agentId}`).catch(() => {
+ clearPendingTask(sessionId)
+ })
}web/mobile/src/features/home/pendingTask.ts currently exposes only stashPendingTaskAtom; add a matching clear writer there.
| {/* The same verbs the desktop header offers; rename and delete | ||
| fall through to the shared implementations here, since /m has | ||
| no app-management modals of its own. */} | ||
| <AgentActionsMenu agent={{id: agentId, name, slug: agent?.slug}} /> |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
Render AgentActionsMenu only after the agent record resolves.
While agentsQuery is pending, agent is undefined and name falls back to "Agent" (line 40). The menu is interactive at that point, so a rename started early is seeded with the placeholder name, and the destructive verbs act on an agent whose real name and slug are unknown.
🐛 Proposed fix
- <AgentActionsMenu agent={{id: agentId, name, slug: agent?.slug}} />
+ {agent ? (
+ <AgentActionsMenu
+ agent={{id: agentId, name, slug: agent.slug}}
+ />
+ ) : null}📝 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.
| <AgentActionsMenu agent={{id: agentId, name, slug: agent?.slug}} /> | |
| {agent ? ( | |
| <AgentActionsMenu | |
| agent={{id: agentId, name, slug: agent.slug}} | |
| /> | |
| ) : null} |
| <AgentOverviewBody | ||
| agentId={appId} | ||
| sessionsHref={sessionsHref ?? ""} | ||
| onEditConfig={openConfig} | ||
| usage={<UsageSummary variant="strip" />} | ||
| {...verbs} | ||
| composer={ |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Avoid passing an empty sessionsHref.
Line 57 leaves sessionsHref undefined until appURL resolves. Line 77 then passes "". Both list cards render their "View all" link and the waiting badge with href="", which resolves to the current route, so the click applies the session scope but does not navigate. Prefer a project-level fallback, or render the cards only after appURL resolves.
🐛 Proposed fix using the project sessions route as fallback
- const {appURL} = useURL()
- const sessionsHref = appURL ? `${appURL}/sessions` : undefined
+ const {appURL, projectURL} = useURL()
+ const sessionsHref = `${appURL || projectURL}/sessions`- sessionsHref={sessionsHref ?? ""}
+ sessionsHref={sessionsHref}📝 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.
| <AgentOverviewBody | |
| agentId={appId} | |
| sessionsHref={sessionsHref ?? ""} | |
| onEditConfig={openConfig} | |
| usage={<UsageSummary variant="strip" />} | |
| {...verbs} | |
| composer={ | |
| const {appURL, projectURL} = useURL() | |
| const sessionsHref = `${appURL || projectURL}/sessions` | |
| <AgentOverviewBody | |
| agentId={appId} | |
| sessionsHref={sessionsHref} | |
| onEditConfig={openConfig} | |
| usage={<UsageSummary variant="strip" />} | |
| {...verbs} | |
| composer={ |
| // Every link out of this card lands on the set the card was showing, not on a default list. | ||
| const linkScope = useMemo(() => ({agentId: null, origin}), [origin]) | ||
| const handleViewAll = useCallback(() => { | ||
| applyScope(linkScope) | ||
| }, [applyScope, linkScope]) | ||
| const handleWaitingClick = useCallback(() => { | ||
| applyScope({...linkScope, status: "waiting"}) | ||
| }, [applyScope, linkScope]) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Inspect the scope contract and how hosts re-apply agent scope on the sessions route.
rg -n 'applySessionScopeAtom' -C 15 web/packages/agenta-sessions/src | head -80
rg -n 'agentId' -C 3 web/packages/agenta-sessions/src/state/scope*.ts 2>/dev/null | head -60Repository: Agenta-AI/agenta
Length of output: 4119
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- SessionListCard.tsx ---'
cat -n web/packages/agenta-sessions-ui/src/SessionListCard.tsx | sed -n '1,180p'
printf '%s\n' '--- session scope state ---'
cat -n web/packages/agenta-sessions/src/state/filters.ts | sed -n '1,130p'
printf '%s\n' '--- SessionListCard usages and host props ---'
rg -n -C 12 'SessionListCard|viewAllHref|base}/sessions|base.*sessions' web --glob '*.{ts,tsx}' | head -300Repository: Agenta-AI/agenta
Length of output: 38110
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- SessionListPanel scope implementation ---'
cat -n web/packages/agenta-sessions-ui/src/SessionListPanel.tsx | sed -n '35,115p'
printf '%s\n' '--- Agent overview host ---'
cat -n web/packages/agenta-entity-ui/src/agent/AgentOverviewBody.tsx | sed -n '1,180p'
printf '%s\n' '--- sessionsHref definitions and route consumers ---'
rg -n -C 8 'sessionsHref' web/packages --glob '*.{ts,tsx}' | head -240
printf '%s\n' '--- sessions page filter initialization and navigation ---'
rg -n -C 10 'sessionAgentFilterAtom|sessionShowTriggeredAtom|applySessionScopeAtom|usePathname|sessions' web/packages/agenta-sessions-ui web/packages/agenta-sessions --glob '*.{ts,tsx}' | head -320Repository: Agenta-AI/agenta
Length of output: 50372
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- AgentOverviewBody callers ---'
rg -l 'AgentOverviewBody' web --glob '*.{ts,tsx}' | sort
printf '%s\n' '--- sessionsHref assignments and literal sessions routes ---'
rg -n 'sessionsHref\s*=|sessionsHref:|base}/sessions|/sessions' web --glob '*.{ts,tsx}' | grep -E 'sessionsHref|base.*/sessions|href|sessions' | head -220
printf '%s\n' '--- route filter atom consumers ---'
rg -n -C 6 'sessionAgentFilterAtom|sessionShowTriggeredAtom|sessionStatusFilterAtom' web/packages/agenta-sessions web/packages/agenta-sessions-ui --glob '*.{ts,tsx}' | head -220Repository: Agenta-AI/agenta
Length of output: 41407
Preserve agentId in linkScope. The desktop route re-applies the agent scope, but mobile navigates to project-wide ${base}/sessions. applySessionScopeAtom then resets the agent filter, so both “View all” and “waiting” can show sessions from other agents. Use {agentId: agentId ?? null, origin} and include agentId in the memo dependencies.
…st the frame The components on this page were already shared; the ARRANGEMENT was not, so /m assembled its own sections in its own order and was missing Files and Usage entirely. AgentOverviewBody is the whole body — composer slot, both list cards, and the rail in one order (what the agent IS, CARRIES, WILL DO, COST) — rendered by the desktop page and the mobile screen alike. SessionListCard moves to @agenta/sessions-ui with the three app verbs as props (mobile's useSessionRowMenu already returns exactly those), replacing mobile's own AgentOverviewSection. AgentFilesCard moves to @agenta/entity-ui/agent, de-antd'd so mobile can render it. The OSS wrappers those two replaced are gone.
/m already had a start-session composer on Home; the agent page had none. Rather than a second composer, HomeTaskComposer gains fixedAgentId — with it the picker drops, because an agent's own page already answers which agent and a picker there would let you send from it to a different one. AgentComposer is the mobile binding, on HomeComposer's mint-stash-route flow so attachments staged before the session exists keep their scope.
…s its margins ContentRail renders a plain div, so the body's `flex-1`/`h-full` columns had no definite height to resolve against and the left one scrolled inside a stunted box. Making the wrapper a flex column restores the chain. Padding scales with the viewport — tight on a phone, desktop-like margins from lg.
…sktop margins UsageCard already serves /m's home and takes an appId, so the agent rail scopes it to this agent — the same figures the desktop strip shows. Margins go from 24px to 64px at lg, measured against the desktop page rather than guessed.
The kebab lived in the OSS page: an antd Dropdown wired to app-management modals, with /m offering nothing. AgentActionsMenu is the shared verb set — rename, copy id, copy slug, delete — on Radix, and useAgentActions is its default implementation (updateWorkflow / archiveWorkflow, both already in entities), built the way useSessionActions already does shared verbs. Rename and delete take host overrides so the desktop keeps its app-management modals, which validate and refresh the apps cache; /m has none and falls through to the shared ones. Copying is shared outright. The swap left the OSS header's antd Dropdown, Button, MoreOutlined and copyToClipboard imports dead; they're gone.
The title carried flex-1, so it absorbed the row and pushed the menu to the opposite end. Sizing it to its text puts the two together as on the desktop; min-w-0 keeps a long name truncating.
… shared ones Both were desktop-only. SidebarToggleButton moves to @agenta/navigation-ui — it needed nothing OSS-specific once sidebarCollapsedAtom was in @agenta/navigation and EnhancedButton was antd-free — and takes collapsedAtom as a prop so it does not quietly bypass SidebarShell's own contract. The OSS path stays a re-export because settingsScope imports it too. The version rides buildHelpDocsNavItem's suffix, lazy-loaded from mobile's own package.json.
Help & Docs is a group inside a `vertical` bottom section, so NavMenu draws it through the flyout trigger, not RowLabel — where the suffix had been ported. The label was therefore missing on BOTH the desktop rail and /m. Drawn on both paths now; the collapsed icon rail still omits it, as 111 intended.
… antd items useSessionCardVerbs handed actions.menuItems() straight to SessionListCard's menuFor, which is typed (vm) => SessionMenuEntry[]. antd's MenuProps["items"] admits null entries, groups and submenus, so the shapes do not line up. toSessionMenuEntries is the existing converter the sessions page already used.
…inks to The extraction into `@agenta/sessions-ui` hardcoded `linkScope.agentId` to null, so "View all" and the waiting badge cleared the agent filter on the way out. On /m, whose sessions route is project-wide and narrows by that filter alone, both landed the user on every agent's sessions. The original conditional keyed on whether the host passed a `viewAllHref` at all, which no longer discriminates: both hosts pass one, and what differs is where it points. The desktop's is the agent's OWN sessions route, which re-applies the scope from the route — setting the filter there is invisible (the page hides the chip) but survives to the next project-wide list. So the distinction becomes an explicit `viewAllScopesAgent`, threaded through `AgentOverviewBody`: set on the desktop, absent on /m, and by default the agent travels with the link. While the app id is still resolving, the desktop now falls back to the project sessions route (correct, because the flag then carries the agent as a filter) rather than passing `""` — a link back to the current route, i.e. a click that visibly does nothing. `viewAllHref` is optional for the window where even the project is unknown: "View all" is then not offered and the waiting badge renders as plain text, instead of holding the whole page or offering a dead link.
…gent is known The actions menu was interactive while the agents query was still pending, when `name` is the "Agent" placeholder and the slug is unknown. A rename opened seeded with the placeholder, and archive and delete acted on a record the screen could not yet describe. The menu now waits for the record, which is how the rest of the screen already treats an unresolved agent — no skeleton, since the kebab is last in the header row and nothing shifts when it arrives.
…the attachments `AgentComposer` stashed the pending task and dropped the staged attachments before navigating, then awaited the push without handling it. A push that rejects left the task under a session id the user was never taken to — where it would fire at whoever opened that id later — took the attachments with it, and let the rejection escape the composer's submit handler. The push is now resolved both ways the Pages Router can fail it (`false` on a cancelled route change, a rejection on a real one), and neither leaves residue: the stash comes back out through `dropPendingTaskAtom`, and the attachments go back in the tray. They still have to be cleared BEFORE the navigation — the chat screen's composer restores this session's staged rows from the per-session store, so rows left behind would return as a second copy of what the first turn already carries — which is why `useComposerAttachments` grows the `restoreAttachments` counterpart to `clearAttachments`. It re-stages the entries settled rather than through `addFiles`, which would upload them a second time.
001fad6 to
2fa46e8
Compare
4218be4 to
e982ee7
Compare
1 participant
The lane below shared the agent-overview frame. This one shares the body: the composition
itself, the actions menu (antd-free, both surfaces), the composer, the rail's usage card, and the
rail's collapse toggle and version label.
Eight commits, mostly
/mcatching up to the shared components and then fixing what that exposed— column heights, page margins, the kebab sitting beside the title instead of at the far edge, and
the version label rendering on the vertical rail too.
Not run in a browser — static gates only (
pnpm lint-fix24/24,tsc --noEmitcleanfor
@agenta/shared,ui,entities,entity-ui,settings-ui,oss,ee,mobile).Stacked on
pkg/agent-overview-layout; review only this lane's diff.