Conversation
Summary of ChangesHello @vinci-grape, 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 is a major feature addition, integrating a powerful new multi-agent code generation framework called 'Code Genesis' and overhauling the web user interface to provide a more interactive and informative experience. The changes span both frontend and backend, focusing on robust communication between the UI and the agent, flexible configuration, and a dedicated chat mode for general AI assistance. The goal is to make the code generation process more transparent, controllable, and user-friendly. 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. 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
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces significant new features, including a web UI chat mode and a more robust agent output parser. The changes are extensive and well-implemented for the most part. I've identified a high-severity issue in the WebSocket handling that could lead to a frontend crash, which should be addressed. I also noted a couple of potential feature regressions in the frontend regarding file previews and message retries that might need a second look. Additionally, there are some minor style issues with misplaced imports in the Python backend code.
| socket.onmessage = (event) => { | ||
| try { | ||
| const data = JSON.parse(event.data); | ||
| handleWebSocketMessage(data); | ||
| } catch (e) { | ||
| console.error('[WS] Non-JSON message:', event.data, e); | ||
|
|
||
| // Fallback: stop the loading state to avoid being stuck in "Processing". | ||
| endRunningState('error', typeof event.data === 'string' | ||
| ? event.data | ||
| : 'Agent failed with non-JSON output'); | ||
|
|
||
| // Don't throw again to avoid a blank screen. | ||
| } | ||
| const data = JSON.parse(event.data); | ||
| handleWebSocketMessage(data); | ||
| }; |
There was a problem hiding this comment.
The try...catch block around JSON.parse(event.data) has been removed. If the WebSocket receives a non-JSON message, this will throw an unhandled exception and crash the application. It's crucial to restore the error handling to ensure the UI remains stable even if unexpected data is received from the backend.
| socket.onmessage = (event) => { | |
| try { | |
| const data = JSON.parse(event.data); | |
| handleWebSocketMessage(data); | |
| } catch (e) { | |
| console.error('[WS] Non-JSON message:', event.data, e); | |
| // Fallback: stop the loading state to avoid being stuck in "Processing". | |
| endRunningState('error', typeof event.data === 'string' | |
| ? event.data | |
| : 'Agent failed with non-JSON output'); | |
| // Don't throw again to avoid a blank screen. | |
| } | |
| const data = JSON.parse(event.data); | |
| handleWebSocketMessage(data); | |
| }; | |
| socket.onmessage = (event) => { | |
| try { | |
| const data = JSON.parse(event.data); | |
| handleWebSocketMessage(data); | |
| } catch (e) { | |
| console.error('Failed to parse WebSocket message:', event.data, e); | |
| } | |
| }; |
| # Pattern: [INFO:ms_agent] without [agent_name] afterwards | ||
| if '[INFO:ms_agent]' in line: | ||
| # Check if there's an agent name tag [xxx] after [INFO:ms_agent] | ||
| import re |
There was a problem hiding this comment.
Imports should generally be at the top of the file. import re is used here and in other places within methods. The same applies to import json and import traceback in this file. Moving all imports to the top of the file is a standard Python convention (PEP 8) and improves readability and avoids repeated import overhead.
| import ms_agent | ||
| from pathlib import Path |
There was a problem hiding this comment.
Imports should be at the top of the file. import ms_agent and from pathlib import Path are imported inside a function. Please move them to the top of the file to follow standard Python conventions (PEP 8) and improve code organization.
| const handleViewFile = async (path: string) => { | ||
| setSelectedFile(path); | ||
| setFileLoading(true); | ||
| try { | ||
| // Build path variants - add project-specific paths | ||
| const pathVariants = [ | ||
| path, // Original path from file tree | ||
| ]; | ||
|
|
||
| // If we have a project, try project-specific paths | ||
| if (currentSession?.project_id) { | ||
| pathVariants.push(`projects/${currentSession.project_id}/output/${path}`); | ||
| } | ||
|
|
||
| // Also try without prefix | ||
| pathVariants.push(path.replace(/^output\//, '')); | ||
| pathVariants.push(path.split('/').pop() || path); | ||
|
|
||
| setSelectedFile(path); | ||
| setFileLoading(true); | ||
| const kind = getFileKind(path); | ||
| setFileKind(kind); | ||
| let lastError: Error | null = null; | ||
|
|
||
| try { | ||
| if (kind === 'text') { | ||
| for (const pathVariant of pathVariants) { | ||
| try { | ||
| const response = await fetch('/api/files/read', { | ||
| method: 'POST', | ||
| headers: { 'Content-Type': 'application/json' }, | ||
| body: JSON.stringify({ path: path, session_id: currentSession?.id }), | ||
| body: JSON.stringify({ path: pathVariant }), | ||
| }); | ||
|
|
||
| if (!response.ok) { | ||
| const error = await response.json(); | ||
| throw new Error(error.detail || 'Failed to load file'); | ||
| if (response.ok) { | ||
| const data = await response.json(); | ||
| setFileContent(data.content); | ||
| return; // Success, exit early | ||
| } else { | ||
| const errorData = await response.json().catch(() => ({ detail: `HTTP ${response.status}` })); | ||
| lastError = new Error(errorData.detail || `Failed to load file: ${pathVariant}`); | ||
| } | ||
|
|
||
| const data = await response.json(); | ||
| setFileContent(data.content); | ||
| setFileUrl(null); | ||
| return; | ||
| } catch (err) { | ||
| lastError = err instanceof Error ? err : new Error('Unknown error'); | ||
| } | ||
| } | ||
|
|
||
| const sid = currentSession?.id; | ||
| const streamUrl = | ||
| `/api/files/stream?path=${encodeURIComponent(path)}&session_id=${encodeURIComponent(sid || '')}`; | ||
| setFileUrl(streamUrl); | ||
| setFileContent(null); | ||
| } catch (err) { | ||
| setFileError(err instanceof Error ? err.message : 'Failed to load file'); | ||
| } finally { | ||
| setFileLoading(false); | ||
| // If all attempts failed, show the last error | ||
| if (lastError) { | ||
| console.error('Failed to load file with all path variants:', lastError); | ||
| setFileContent(`Error: ${lastError.message}\n\nTried paths:\n${pathVariants.join('\n')}`); | ||
| } | ||
| }; | ||
| } catch (err) { | ||
| console.error('Failed to load file:', err); | ||
| setFileContent(`Error: ${err instanceof Error ? err.message : 'Unknown error'}`); | ||
| } finally { | ||
| setFileLoading(false); | ||
| } | ||
| }; |
There was a problem hiding this comment.
This refactoring seems to have removed the functionality to preview binary files like images and videos. The previous implementation handled different file kinds and used the /api/files/stream endpoint for non-text files. The new implementation only fetches and displays text content. Was this removal of functionality intentional? If not, it should be restored as it's a regression.
| const sendMessage = useCallback((content: string) => { | ||
| if (!currentSession || !ws || ws.readyState !== WebSocket.OPEN) return; | ||
|
|
||
| // Add user message locally | ||
| setMessages(prev => [...prev, { | ||
| id: Date.now().toString(), | ||
| role: 'user', | ||
| content, | ||
| type: 'text', | ||
| timestamp: new Date().toISOString(), | ||
| status: 'running', | ||
| client_request_id: clientRequestId, | ||
| }]); | ||
| } | ||
|
|
||
| ws.send(JSON.stringify({ | ||
| action: 'start', | ||
| query: content, | ||
| client_request_id: clientRequestId, // If the backend can pass it back unchanged, that would be better. | ||
| })); | ||
| // For chat mode, send as input to existing process | ||
| // For project mode, start a new agent | ||
| if (currentSession.session_type === 'chat') { | ||
| ws.send(JSON.stringify({ | ||
| action: 'send_input', | ||
| input: content, | ||
| })); | ||
| } else { | ||
| ws.send(JSON.stringify({ | ||
| action: 'start', | ||
| query: content, | ||
| })); | ||
| } | ||
|
|
||
| setIsStreaming(false); | ||
| setStreamingContent(''); | ||
| setIsLoading(true); | ||
| }, [currentSession, ws]); | ||
| setIsLoading(true); | ||
| }, [currentSession, ws]); |
There was a problem hiding this comment.
The sendMessage function has been refactored, and in the process, the ability to retry sending a message (using reuseMessageId) has been removed. This was a useful feature for recovering from transient errors without re-typing the query. Was this removal intentional? If not, consider re-introducing this capability.
2 participants
Change Summary
Fix post-build bugs
Related issue number
Checklist
pre-commit installandpre-commit run --all-filesbefore git commit, and passed lint check.