Skip to content

Restructure agentex-ui components#15

Merged
MichaelSun48 merged 4 commits into
mainfrom
msun/restructureComponents
Oct 29, 2025
Merged

Restructure agentex-ui components#15
MichaelSun48 merged 4 commits into
mainfrom
msun/restructureComponents

Conversation

@MichaelSun48
Copy link
Copy Markdown
Contributor

**Dependent on #14 **

This PR makes a bunch of DevEX improvements to the agentex-ui components, mostly around improving component naming and organization. Rather than enumerating them here, I'll annotate the diff which is probably easier to follow.

No functionality should be touched — if you notice anything, please let me know.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR refactors the Agentex UI architecture to improve component organization and migrate from finite to infinite pagination for tasks. The main changes centralize state management in URL search parameters, remove prop drilling, and split monolithic components into focused, reusable pieces.

  • Migrates task list from finite to infinite pagination using useInfiniteTasks
  • Centralizes state management in URL search parameters via useSafeSearchParams hook
  • Refactors component structure by extracting PrimaryContent, HomeView, and ChatView components

Reviewed Changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
agentex-ui/hooks/use-tasks.ts Removes deprecated useTasks hook in favor of infinite pagination
agentex-ui/hooks/use-create-task.ts Updates cache mutation logic to work with infinite query structure
agentex-ui/components/agentex/traces-sidebar.tsx Reads taskID from search params instead of props; adds animation wrapper
agentex-ui/components/agentex/task-sidebar.tsx Removes prop drilling by reading state from search params
agentex-ui/app/primary-content.tsx New component extracted from main-content.tsx containing primary UI logic
agentex-ui/app/page.tsx Updates import to use renamed AgentexUIRoot component
agentex-ui/app/main-content.tsx Refactored to focus on provider setup and layout; renames MainContent to AgentexUIRoot
agentex-ui/app/home-view.tsx New component extracted to display the home/agent selection view
agentex-ui/app/chat-view.tsx New component extracted to display the chat interface

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread agentex-ui/app/agentex-ui-root.tsx
>
{createdAtString}
{task.agents && task.agents.length > 0 && task.created_at && ' • '}
{' • '}
Copy link

Copilot AI Oct 29, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The conditional rendering logic for the separator was removed. Previously, the separator ' • ' only appeared when both task.created_at and task.agents existed. Now it always appears, which could result in '• agentsString' being displayed even when there's no created date, creating awkward spacing.

Suggested change
{' • '}
{task.created_at && task.agents && task.agents.length > 0 ? ' • ' : ''}

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would do the suggestion

Comment thread agentex-ui/app/primary-content.tsx
Comment thread agentex-ui/components/agentex/traces-sidebar.tsx
@MichaelSun48 MichaelSun48 force-pushed the msun/restructureComponents branch from 70d37aa to 5ac805e Compare October 29, 2025 02:15
@MichaelSun48 MichaelSun48 changed the title Fix use infinite tasks hook Restructure agentex-ui components Oct 29, 2025
@MichaelSun48 MichaelSun48 marked this pull request as draft October 29, 2025 02:16
Comment on lines +75 to +82
<TaskSidebar />
<PrimaryContent
isTracesSidebarOpen={isTracesSidebarOpen}
toggleTracesSidebar={() =>
setIsTracesSidebarOpen(!isTracesSidebarOpen)
}
/>
<TracesSidebar isOpen={isTracesSidebarOpen} />
Copy link
Copy Markdown
Contributor Author

@MichaelSun48 MichaelSun48 Oct 29, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note here — hopefully this is much more intuitive and conveys the three part page structure we have

toggleTracesSidebar: () => void;
}

export function PrimaryContent({
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This component is the middle third that contains the chat and home screen. Honestly, this one is still a little too big and could be pared down even more, but for teh sake of keeping this PR reasonable, I'll make those in a follow up.

</motion.div>
)}

{taskID ? <ChatView taskID={taskID} /> : <HomeView />}
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note here — these two components definitely should be in their own files

Comment on lines +125 to +137
<motion.div
className="pointer-events-none absolute bottom-full left-1/2 z-10 mb-4 -translate-x-1/2"
initial={{ y: 30, opacity: 0 }}
animate={{ y: 0, opacity: 1 }}
exit={{ y: 30, opacity: 0 }}
transition={{
duration: 0.2,
type: 'spring',
stiffness: 300,
damping: 35,
mass: 0.8,
}}
>
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is definitely my fault, but in a future PR I'm going to move these motion.div into the components, rather than having them in the parent

};

function TaskButton({ task, selectedTaskID, selectTask }: TaskButtonProps) {
function TaskButton({ task }: TaskButtonProps) {
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Common theme in this PR is that I'm trying to get rid of anywhere we pass taskId or agentName through props and instead let any component that needs it get it via useSafeSearchParams hook.

>
{createdAtString}
{task.agents && task.agents.length > 0 && task.created_at && ' • '}
{task.created_at && ' • '}
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if tasks.agent is nothing, the agents string still exists and says "No Agents". I was seeing this:

Image

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is interesting. Initially No date was put there so we could set the block to invisible and maintain the height of the element. Probably better to just set the createdAtString to '' and the agentsString to '' instead of the dummy strings that exist currently. Then we could just manually set the height along with the invisible tailwind class above.

Comment on lines 165 to +169
const handleNewChat = useCallback(() => {
onSelectTask(null);
}, [onSelectTask]);
updateParams({
[SearchParamKey.TASK_ID]: null,
});
}, [updateParams]);
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note here — another example of letting a component consume the useSearchParams hook rather than receiving it as props.

Copy link
Copy Markdown
Collaborator

@declan-scale declan-scale left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, so much cleaner already, thanks! I think let's fix the dot conditions but I can do that in a separate PR if you'd like.

};

function TaskButton({ task, selectedTaskID, selectTask }: TaskButtonProps) {
function TaskButton({ task }: TaskButtonProps) {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does it make sense to pop this out into its own TaskSidebarItem.tsx file?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah definitely — I'll include that in a follow up PR

@MichaelSun48 MichaelSun48 force-pushed the msun/restructureComponents branch from d9840c1 to 0e54f76 Compare October 29, 2025 16:20
@MichaelSun48 MichaelSun48 force-pushed the msun/restructureComponents branch from 0e54f76 to 11dc487 Compare October 29, 2025 16:22
@MichaelSun48 MichaelSun48 marked this pull request as ready for review October 29, 2025 16:23
@MichaelSun48 MichaelSun48 merged commit 3e8668c into main Oct 29, 2025
3 checks passed
@MichaelSun48 MichaelSun48 deleted the msun/restructureComponents branch October 29, 2025 16:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants