-
Notifications
You must be signed in to change notification settings - Fork 2.9k
Modernize Gemini models and add comprehensive error handling with testing #158
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Summary of Changes
Hello @ipbabble, 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 significantly upgrades the application's AI model integration by updating to the latest stable Gemini models and implementing robust error handling for model unavailability. It also enhances the user experience through improved model selection UIs and introduces a comprehensive testing framework to prevent future configuration drift. The changes aim to improve stability, maintainability, and user experience by ensuring reliable model interactions and a streamlined development process.
Highlights
- Model Modernization: Deprecated Gemini 1.5 and preview models have been removed, replaced with stable model names aligned with Google's 2025 roadmap. A cost-effective 'gemini-2.5-flash-lite' option has also been added.
- Comprehensive Error Handling: A new
ModelErrorDialog
is introduced to provide graceful error recovery when models are unavailable. This includes fallback model suggestions, preservation of user input during failures, and the ability to save user model preferences. Crucially, 404 error handling has been integrated into all relevant LangGraph nodes. - Enhanced User Interface: The frontend now features cleaner model display names in dropdowns, dual selection for 'Query' and 'Reasoning' models with visual categorization, and professional model selection with icons and cost indicators.
- Robust Testing Suite: A new regression test suite has been added to validate model configurations, ensuring consistency between frontend and backend, and preventing issues like model naming discrepancies. This includes backend, frontend, and an integration script for automated validation.
- Modernized Development Workflow: Pre-commit hooks have been configured for automated code quality checks, including model validation, Python linting (Ruff), and frontend formatting/linting (Prettier, ESLint, TypeScript check). The project structure and setup instructions have also been updated.
Using Gemini Code Assist
The 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 in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command>
or @gemini-code-assist <command>
. Below is a summary of the supported commands.
Feature | Command | Description |
---|---|---|
Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
Help | /gemini help |
Displays a list of available commands. |
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 .gemini/
folder in the base of the repository. Detailed instructions can be found here.
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
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This is a substantial and well-executed pull request that significantly modernizes the application. The move to stable Gemini models, the addition of comprehensive error handling with a user-friendly dialog, and the introduction of a robust testing suite with pre-commit hooks are all excellent improvements. The code is generally clean and well-structured. I've identified a few areas for improvement, mainly around reducing code duplication in the backend error handling and making the frontend error detection more robust to prevent potential issues. Overall, great work on enhancing the reliability and maintainability of the project.
frontend/src/App.tsx
Outdated
if (error.message && error.message.includes("404") && error.message.includes("models/")) { | ||
const modelMatch = error.message.match(/models\/([\w-]+)/); | ||
if (modelMatch && lastSubmission) { | ||
const failedModel = modelMatch[1]; | ||
setModelError({ | ||
failedModel, | ||
errorMessage: error.message, | ||
lastSubmission: lastSubmission, | ||
}); | ||
return; | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The current error detection logic relies on string matching for '404'
and 'models/'
in the error message. This is fragile and likely won't work as intended because the backend catches the original API error and raises a new ValueError
with a custom message: Model '{model_name}' not found. Please try a different model.
.
The frontend error handler should be updated to match this specific error message from the backend. This will make the error detection more robust and ensure the model error dialog is triggered correctly.
A more reliable approach would be:
if (error.message?.includes("not found. Please try a different model.")) {
const modelMatch = error.message.match(/Model '([\\w.-]+)' not found/);
if (modelMatch && lastSubmission) {
// ... rest of the logic
}
}
if (error.message && error.message.includes("404") && error.message.includes("models/")) { | |
const modelMatch = error.message.match(/models\/([\w-]+)/); | |
if (modelMatch && lastSubmission) { | |
const failedModel = modelMatch[1]; | |
setModelError({ | |
failedModel, | |
errorMessage: error.message, | |
lastSubmission: lastSubmission, | |
}); | |
return; | |
} | |
} | |
if (error.message?.includes("not found. Please try a different model.")) { | |
const modelMatch = error.message.match(/Model '([\w.-]+)' not found/); | |
if (modelMatch && lastSubmission) { | |
const failedModel = modelMatch[1]; | |
setModelError({ | |
failedModel, | |
errorMessage: error.message, | |
lastSubmission: lastSubmission, | |
}); | |
return; | |
} | |
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
|
||
if (!isOpen) return null; | ||
|
||
const recommendedFallback = "gemini-2.0-flash"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The recommendedFallback
model is hardcoded to gemini-2.0-flash
. If this is the model that initially failed, the user will be prompted to retry with the same failing model, which could lead to an infinite loop of failure dialogs. The component should ensure that the suggested fallback is different from the failedModel
.
const recommendedFallback = "gemini-2.0-flash"; | |
const recommendedFallback = | |
failedModel === "gemini-2.0-flash" ? "gemini-2.5-flash" : "gemini-2.0-flash"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
backend/src/agent/graph.py
Outdated
except Exception as e: | ||
# Check if this is a model not found error | ||
if "404" in str(e) and "models/" in str(e): | ||
# Return an error that the frontend can catch | ||
error_msg = ( | ||
f"Model '{reasoning_model}' not found. Please try a different model." | ||
) | ||
unique_sources.append(source) | ||
|
||
return { | ||
"messages": [AIMessage(content=result.content)], | ||
"sources_gathered": unique_sources, | ||
} | ||
raise ValueError(error_msg) from e | ||
# Re-raise other errors | ||
raise |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The try...except
block for handling model 404
errors is duplicated across four functions: generate_query
, web_research
, reflection
, and finalize_answer
. This repetition makes the code harder to maintain, as any change to the error handling logic must be updated in all four places.
To improve maintainability, I suggest refactoring this logic into a centralized helper function. Additionally, catching a generic Exception
and checking the string for '404'
is fragile. It would be more robust to catch the specific google.api_core.exceptions.NotFound
exception.
Here's a suggested implementation:
from google.api_core import exceptions as google_exceptions
def _handle_model_not_found(e: Exception, model_name: str):
"""Checks for a model not found error and raises a specific ValueError."""
if isinstance(e, google_exceptions.NotFound) or (
"404" in str(e) and "models/" in str(e)
):
error_msg = f"Model '{model_name}' not found. Please try a different model."
raise ValueError(error_msg) from e
raise e
# Then, in each node function:
try:
# ... existing logic ...
except Exception as e:
_handle_model_not_found(e, reasoning_model)
This would centralize the error handling logic, making it more robust and easier to manage.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done. Good catch.
assert re.match(r"^gemini-\d+\.\d+-(flash|pro)(-lite)?", model), ( | ||
f"Model {model} doesn't follow expected naming pattern" | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The regex used to validate the model naming pattern is missing a $
at the end to anchor the match to the end of the string. Without it, a model name like gemini-2.5-pro-preview
would incorrectly pass this check because it starts with a valid pattern. Adding a $
will ensure the entire model name conforms to the expected stable pattern.
assert re.match(r"^gemini-\d+\.\d+-(flash|pro)(-lite)?", model), ( | |
f"Model {model} doesn't follow expected naming pattern" | |
) | |
assert re.match(r"^gemini-\d+\.\d+-(flash|pro)(-lite)?$", model), ( | |
f"Model {model} doesn't follow expected naming pattern" | |
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
64d0ac6
to
cf6417c
Compare
I'm going to try and clean up this PR. |
…ting Fix deprecated model configurations causing 404 errors: - Remove deprecated 1.5 and preview models (gemini-2.5-*-preview-*) - Add cost-effective gemini-2.5-flash-lite option - Update to stable model names aligned with Google's 2025 roadmap Implement graceful error recovery: - Add ModelErrorDialog with fallback model suggestions - Preserve user input during model failures - Save user model preferences with "remember choice" - Add 404 error handling to all LangGraph nodes Enhance user interface: - Clean model display names in dropdowns (hide indicators when closed) - Dual Query/Reasoning model selection with visual categorization - Professional model selection with icons and cost indicators Add regression test suite: - Backend: Model naming validation, frontend/backend consistency - Frontend: Source code validation, stable model patterns - Integration script: Automated validation prevents config drift - Support for new model variants (e.g., -lite suffix) Modernize development workflow: - Pre-commit hooks with model validation, Ruff, Prettier, ESLint - Organized tests/ directory with comprehensive documentation - Updated project structure and setup instructions
Apply code quality improvements suggested by Gemini code assist: - Enhanced error pattern matching with more specific message detection - Smart dynamic fallback logic based on which model actually failed - Centralized error handling to eliminate code duplication across 4 functions - More robust exception handling with proper Google API types - Fixed regex pattern security issue with proper end anchoring Fix pre-commit validation issues: - Remove unused variables and parameters in ModelErrorDialog - Fix invalid JSON syntax in tsconfig files (remove comments) - Maintain all functionality while improving code quality All model validation tests continue to pass.
cf6417c
to
5b9fa2f
Compare
Forced some re commits due to cla/google problem. I've added myself to the CLA. |
Fix deprecated model configurations causing 404 errors:
Implement graceful error recovery:
Enhance user interface:
Add regression test suite:
Modernize development workflow: