-
Notifications
You must be signed in to change notification settings - Fork 1.4k
feat: enhance model validation with detailed error messages and diagnostics #1155
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?
feat: enhance model validation with detailed error messages and diagnostics #1155
Conversation
…ostics Fixes eigent-ai#231 This PR significantly enhances the model validation system to provide detailed error messages and diagnostic information, helping users understand the root cause of validation failures. - Added enum to track validation stages - Added enum for error categorization - Added class with comprehensive diagnostic information - Added function to classify errors by type - Added function with step-by-step validation: 1. Initialization validation 2. Model creation validation 3. Agent creation validation 4. Model call execution validation 5. Tool call execution validation - Each stage provides detailed error messages and diagnostic information - Error categorization covers: authentication, network, model not found, rate limits, quota, timeouts, tool call support, and configuration errors - Updated to use new detailed validation function - Added parameter for optional detailed diagnostics - Enhanced response model with: - : Specific error category - : Stage where validation failed - : List of stages that succeeded - : Detailed diagnostic information - Improved error messages based on error type and stage - Better logging with diagnostic context - **Better Error Messages**: Users get specific, actionable error messages instead of generic failures - **Root Cause Identification**: Clear indication of which stage failed (initialization, model creation, agent creation, model call, tool call) - **Diagnostic Information**: Optional detailed diagnostics help debug configuration issues - **Error Categorization**: Errors are categorized (authentication, network, quota, etc.) for better handling - **Backward Compatible**: Maintains existing API while adding optional diagnostic features - Authentication: "Authentication failed. Please check your API key..." - Model Not Found: "Model 'X' not found on platform 'Y'..." - Network Error: "Network error occurred. Please check your connection..." - Tool Call Not Supported: "Model call succeeded, but this model does not support tool calling functionality..." - Tool Call Execution Failed: "Tool call was made but execution failed..." All error messages provide actionable guidance to help users resolve issues.
37e6ab0 to
4ae05b6
Compare
|
Thanks @Angel98518 for the contribution! @bytecii could you help with the review? |
Hi @bytecii Thanks for the feedback. After reviewing both PRs: |
| if any( | ||
| keyword in error_str | ||
| for keyword in [ | ||
| "invalid", | ||
| "configuration", | ||
| "config", | ||
| "parameter", | ||
| "param", | ||
| ] | ||
| ): | ||
| return ValidationErrorType.INVALID_CONFIGURATION |
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.
Hi @Angel98518 The "invalid" keyword is too broad and can misclassify errors like "Invalid request" as INVALID_CONFIGURATION. Consider narrowing it to more specific phrases
| @@ -28,6 +103,111 @@ def get_website_content(url: str) -> str: | |||
| return "Tool execution completed successfully for https://www.camel-ai.org, Website Content: Welcome to CAMEL AI!" | |||
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.
Hi @Angel98518 Consider extracting it to a constant, e.g. EXPECTED_TOOL_RESULT, to avoid duplication and simplify future changes.
| if any( | ||
| keyword in error_str | ||
| for keyword in [ | ||
| "invalid_api_key", |
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.
I am not sure whether these keywords checking are robust enough. Because for now eigent supports a lot of different model platform / providers, such as litellm, qwen etc.
- Narrow 'invalid' keyword to specific configuration error phrases to avoid false positives - Extract EXPECTED_TOOL_RESULT constant to avoid duplication - Enhance error categorization for different model platforms/providers (OpenAI, Anthropic, LiteLLM, Qwen, etc.) - Improve error detection by checking exception types first, then error messages - Add more comprehensive keyword lists for better error classification across providers
|
Hi, @4pmtong I have fixed all feedbacks |
|
Thanks for working on this! The staged validation approach looks solid. Just wanted to share some thoughts on the error classification part. The string matching for error categorization (checking for keywords like "401", "authentication", etc.) might be fragile since different AI providers format their errors differently, and they could change the wording anytime. Here's the thing though: error messages from OpenAI, Anthropic and other major providers are already pretty clear, like:
Users can understand what's wrong just by reading this. Adding our own "translation" layer might actually lose useful context. A simpler approach could be: def format_validation_error(exception: Exception) -> str:
raw = str(exception)
if len(raw) > 300:
raw = raw[:300] + "..."
return rawLess code, fewer edge cases. When validation fails, showing the raw error is often good enough, WDYT? |
- Always preserve raw error messages from providers (they are usually clear) - Make error categorization more conservative - only categorize when confident (exception types, HTTP status codes, ModelProcessingError) - Remove fragile keyword-based string matching that could misclassify errors - Use raw error messages as primary messages instead of translating them - Add format_raw_error() helper function to truncate long errors - Add raw_error_message field to ValidationResult for preserving original errors This approach: - Preserves useful context from provider error messages - Reduces risk of misclassification - Still provides error categorization when we have high confidence - Maintains backward compatibility with error_type field
14ad3e0 to
7add156
Compare
|
Hi, @Wendong-Fan , could you review my changes again please? |
- Add missing fields (model_response_info, tool_call_info, validation_stages) to ValidateModelResponse - Reduce step_timeout from 1800s to 60s for validation (more reasonable timeout) - Remove meaningless tools_count from agent_creation diagnostic info
- Remove traceback from error_details (security: don't expose internal info to clients) - Move is_valid=True to after stage 5 (TOOL_CALL_EXECUTION) instead of stage 4 - Add missing fields to ValidateModelResponse (model_response_info, tool_call_info, validation_stages) - Reduce step_timeout from 1800s to 60s for validation - Remove meaningless tools_count from agent_creation diagnostic info
|
Hi, @Zephyroam , I have fixed feedbacks, please check. Thank you for your review. |
bytecii
left a comment
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.
Also let's run the pre-commit for the changed files.
and add the files you changed / added to the
eigent/.github/workflows/pre-commit.yml
Line 34 in 0e6847d
| uv run pre-commit run --files \ |
|
|
||
| result = ValidateModelResponse(**response_data) | ||
|
|
||
| logger.info( |
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.
Can we change to error or warning if there is anything wrong?
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.
Let's add tests to for example tests/app/component/test_model_validation.py and tests/app/controller/test_model_controller.py
Fixes #231
This PR significantly enhances the model validation system to provide detailed error messages and diagnostic information, helping users understand the root cause of validation failures.
Changes
Enhanced Model Validation (
backend/app/component/model_validation.py)ValidationStageenum to track validation stages (initialization, model creation, agent creation, model call, tool call execution, response parsing)ValidationErrorTypeenum for error categorization (authentication, network, model not found, rate limits, quota, timeouts, tool call support, configuration errors)ValidationResultclass with comprehensive diagnostic informationcategorize_error()function to classify errors by typevalidate_model_with_details()function with step-by-step validation:Enhanced Controller (
backend/app/controller/model_controller.py)include_diagnosticsparameter for optional detailed diagnosticserror_type: Specific error categoryfailed_stage: Stage where validation failedsuccessful_stages: List of stages that succeededdiagnostic_info: Detailed diagnostic informationBenefits
Example Error Messages
All error messages provide actionable guidance to help users resolve issues.
Code Statistics