Conversation
…es pattern - Move chatStore to top-level stores/ directory - Add pages/ directory with ChatPage and DashboardPage - Create root/App.tsx as main entry point - Extract workflow utilities to components/workflow/utils.ts - Add WorkflowRequestResponder component - Add new hooks: use-mobile, use-prompt-input, use-sidebar - Split CSS into modular files (animations, sidebar, theme) - Update tests for new import paths - Add comprehensive test utilities (factories, render helpers) - Add new component tests for chat-messages, prompt-input, chain-of-thought - Clean up deprecated features/chat and features/dashboard directories - Update optimization API tests for new endpoints
Dependency ReviewThe following issues were found:
|
|
Caution Review failedFailed to post review comments Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 📝 WalkthroughSummary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings. WalkthroughReorganizes the frontend into component-centric modules, consolidates self-improvement into an /optimization/jobs flow, introduces a provider-based Sidebar and new Chat/Dashboard pages, replaces markdown rendering with Streamdown and adds streaming UI/hooks, expands tests and test utilities, and adds design-system CSS and workflow/docs. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant ChatPage
participant SidebarProvider
participant ChatContent
participant ChatStore
participant API
participant RightPanel
User->>ChatPage: Open page (mount)
ChatPage->>SidebarProvider: Wrap UI
ChatPage->>ChatStore: loadConversations(reset=true)
ChatStore->>API: GET /conversations?limit=25&offset=0
API-->>ChatStore: conversations[]
ChatStore-->>ChatPage: state updated
User->>ChatContent: Submit message
ChatContent->>ChatStore: sendMessage()
ChatStore->>API: POST /messages (streaming/SSE)
API-->>ChatStore: SSE/stream events (reasoning/log/delta)
loop stream events
ChatStore->>ChatStore: merge delta, attach workflowPhase/steps
ChatStore-->>ChatContent: new assistant content
ChatContent->>RightPanel: provide latest message for trace view
end
User->>RightPanel: Toggle open/close
RightPanel-->>ChatPage: reflect open state
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Areas needing focused review:
Possibly related PRs
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
Summary of ChangesHello @Zochory, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a significant overhaul of the frontend architecture, transitioning to a more scalable and maintainable 'pages/stores' pattern. The changes involve a comprehensive reorganization of directories, refactoring of UI components and hooks, and updates to API interactions and state management. The goal is to streamline development, improve code clarity, and enhance the overall user experience through a more robust and modular design. This foundational work sets the stage for future feature development and easier maintenance. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Ignored Files
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Pull request overview
This PR restructures the frontend architecture from a feature-based structure to a pages/stores pattern with improved separation of concerns.
Key Changes
- Reorganizes code from
features/to domain-organizedcomponents/,pages/, and top-levelstores/ - Splits CSS into modular files (animations.css, sidebar.css, theme.css)
- Updates optimization API endpoints from
/optimizeto/optimization/jobs - Adds pagination support for conversations with infinite scroll
- Adds comprehensive test utilities and new component tests
- Implements concurrent execution error handling
Reviewed changes
Copilot reviewed 70 out of 83 changed files in this pull request and generated 12 comments.
Show a summary per file
| File | Description |
|---|---|
src/frontend/tsconfig.app.json |
Removes baseUrl configuration (potential path resolution issue) |
src/frontend/src/tests/utils/render.tsx |
Adds test rendering utilities with provider wrappers |
src/frontend/src/tests/utils/factories.ts |
Adds mock data factories for testing |
src/frontend/src/tests/pages/*.test.tsx |
New page-level tests for ChatPage and DashboardPage |
src/frontend/src/tests/components/**/*.test.tsx |
New component tests with updated import paths |
src/frontend/src/root/App.tsx |
New app entry point with view routing |
src/frontend/src/pages/*.tsx |
New page components (ChatPage, DashboardPage) |
src/frontend/src/stores/chatStore.ts |
Adds pagination and concurrent error handling |
src/frontend/src/hooks/*.ts |
New custom hooks (use-mobile, use-prompt-input, use-sidebar) |
src/frontend/src/components/**/*.tsx |
Reorganized components by domain |
src/frontend/src/styles/*.css |
Split into modular CSS files |
src/frontend/src/api/client.ts |
Updated optimization API endpoints |
src/frontend/src/api/types.ts |
Updated OptimizationRequest and OptimizationResult types |
src/frontend/package.json |
Dependency updates (@radix-ui/react-separator, shadcn) |
Files not reviewed (1)
- src/frontend/package-lock.json: Language not supported
| it("does not submit when loading", async () => { | ||
| mockChatStore.isLoading = true; | ||
| const user = userEvent.setup(); | ||
| render(<ChatPage />); | ||
|
|
||
| const textarea = screen.getByPlaceholderText("Ask anything..."); | ||
| await user.type(textarea, "Test message"); | ||
|
|
||
| // When loading, the submit button should be disabled or replaced with stop button | ||
| // Check that sendMessage is not called | ||
| const buttons = screen.getAllByRole("button"); | ||
| const submitButton = buttons.find((b) => | ||
| b.textContent?.match(/send|submit/i), | ||
| ); | ||
| if (submitButton) { | ||
| await user.click(submitButton); | ||
| } | ||
|
|
||
| expect(mockChatStore.sendMessage).not.toHaveBeenCalled(); | ||
| }); |
There was a problem hiding this comment.
The test title says "does not submit when loading" but the test doesn't actually verify that sendMessage is not called when isLoading is true. The test only checks for the absence of a submit button or that it doesn't call sendMessage, but the logic is unclear. Consider making the assertion more explicit by checking that the submit button is either disabled or missing when isLoading=true.
| "skipLibCheck": true, | ||
| "noEmit": true, | ||
| "baseUrl": ".", | ||
| "paths": { |
There was a problem hiding this comment.
Removing baseUrl while keeping paths configuration may cause path resolution issues. When using path aliases like @/*, TypeScript typically requires either baseUrl set to "." or the paths to be absolute. Consider keeping baseUrl: "." or verify that path resolution works correctly without it in your TypeScript version.
There was a problem hiding this comment.
Code Review
This is an excellent and comprehensive refactoring of the frontend architecture. The move to a pages/stores pattern, organizing components by domain, and modularizing CSS and hooks significantly improves the codebase's structure, maintainability, and scalability. The updated API client, hooks, and types are consistent with the backend changes. The addition of extensive testing utilities, new component tests, and detailed documentation in TESTING.md and CLAUDE.md is a huge win for the project's quality and developer experience. I have one suggestion to improve type safety.
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> Signed-off-by: Zachary BENSALEM <zachary@qredence.ai>
|
/q |
|
@copilot open a new pull request to apply changes based on the comments in this thread |
|
@copilot open a new pull request to apply changes based on the comments in this thread |
* Initial plan * fix: address PR review comments - fix tests, typos, and config Co-authored-by: Zochory <60674042+Zochory@users.noreply.github.com> * chore: fix code formatting with Prettier Co-authored-by: Zochory <60674042+Zochory@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: Zochory <60674042+Zochory@users.noreply.github.com>
- Added a step in the GitHub workflow to ensure the log directory and file are created, along with a listing of the directory contents. - Modified several test cases in chat-page.test.tsx to remove async/await as they were not necessary, improving readability and performance.
… TypeScript configurations
restructure frontend architecture with pages/stores pattern