Conversation
Summary of ChangesHello @tastelikefeet, 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 addresses critical issues related to the 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
|
There was a problem hiding this comment.
Code Review
This pull request aims to fix the web UI build process. The changes include significant updates to setup.py to automate the frontend build and packaging, modifications to MANIFEST.in to control packaged files, and some refactoring in the frontend React components.
My review has identified a critical issue in MANIFEST.in that would likely cause the source distribution (sdist) to be broken and uninstallable. I've also pointed out a couple of medium-severity issues related to code style in setup.py and unused props in a React component that should be addressed for better maintainability. Overall, the changes to automate the web UI build are a great step forward, but the packaging configuration needs to be corrected.
| include README.md | ||
| include LICENSE | ||
| include requirements.txt | ||
| recursive-include requirements *.txt | ||
|
|
||
| # Include projects | ||
| recursive-include projects * | ||
|
|
||
| # Include webui backend | ||
| recursive-include webui/backend *.py | ||
|
|
||
| # Include webui frontend dist (will be built during setup) | ||
| recursive-include webui/frontend/dist * | ||
| recursive-include projects * | ||
|
|
||
| # Exclude development files | ||
| global-exclude *.pyc | ||
| global-exclude __pycache__ | ||
| global-exclude .DS_Store | ||
| global-exclude *.so |
There was a problem hiding this comment.
The current MANIFEST.in configuration will likely create a broken source distribution (sdist). It includes webui/frontend/dist, which is a build artifact, but omits the frontend source files (webui/frontend/src, webui/frontend/package.json, etc.).
When building from a source distribution, the setup.py script needs the frontend source files to run npm install and npm run build. Without them, the build will fail.
To fix this, you should include the frontend source files and exclude build artifacts and dependencies from the sdist.
include README.md
include LICENSE
include requirements.txt
recursive-include requirements *.txt
# Include projects
recursive-include projects *
# Include webui backend and frontend sources
recursive-include webui/backend *.py
recursive-include webui/frontend *
# Exclude development files and build artifacts from sdist
global-exclude *.pyc
global-exclude __pycache__
global-exclude .DS_Store
global-exclude *.so
global-exclude webui/frontend/node_modules *
global-exclude webui/frontend/dist *
|
|
||
| def _build_and_copy_webui(self): | ||
| """Build frontend and copy webui files to build directory""" | ||
| import subprocess |
|
|
||
| const MessageBubble: React.FC<MessageBubbleProps> = ({ | ||
| message, isStreaming, sessionStatus, completedSteps, | ||
| message, isStreaming, |
There was a problem hiding this comment.
While you've removed sessionStatus and completedSteps from being destructured here, they are still part of the MessageBubbleProps interface and are being passed to this component from ConversationView. To fully clean this up and prevent passing unused props, please also remove them from the MessageBubbleProps interface and from the <MessageBubble ... /> invocation in ConversationView.
Change Summary
Related issue number
Checklist
pre-commit installandpre-commit run --all-filesbefore git commit, and passed lint check.