fix: Handle Gemini chunk.text ValueError when finish_reason=1#1809
fix: Handle Gemini chunk.text ValueError when finish_reason=1#1809
Conversation
Fixes issue #1808 where Gemini models hitting token limits cause ValueError when accessing chunk.text due to missing Part objects. Changes: - Add graceful error handling in extract_json() for GEMINI_JSON and GENAI_STRUCTURED_OUTPUTS modes - Skip chunks with invalid Parts instead of crashing - Preserve existing behavior for other ValueError types - Add comprehensive tests for both sync and async scenarios The fix allows streaming responses to continue processing valid chunks even when some chunks are incomplete due to token limits. Amp-Thread-ID: https://ampcode.com/threads/T-1bfe4270-cd9b-4c65-be2e-23a0f589dc10 Co-authored-by: Amp <[email protected]>
Deploying with
|
| Status | Name | Latest Commit | Updated (UTC) |
|---|---|---|---|
| ✅ Deployment successful! View logs |
instructor | 11558e3 | Oct 01 2025, 01:44 PM |
|
Claude finished @jxnl's task —— View job Code Review for PR #1809: Fix Gemini chunk.text ValueErrorTodo List:
Review SummaryThis PR effectively addresses issue #1808 with a well-implemented fix for handling Gemini chunk.text ValueError when finish_reason=1. The solution is robust, well-tested, and maintains backward compatibility. ✅ Code Quality & Best PracticesStrengths:
Minor suggestions:
✅ Test CoverageExcellent test coverage with:
The test file ✅ Bug AnalysisNo bugs identified. The implementation is solid:
✅ Performance Considerations
✅ Security Considerations
🔧 Technical Implementation DetailsChanges in
The error handling pattern is consistent: try:
yield chunk.text
except ValueError as e:
if "valid `Part`" in str(e):
continue # Skip invalid chunk
raise # Re-raise other ValueErrors✅ RecommendationAPPROVED - This is a well-crafted fix that:
The implementation correctly addresses the root cause (Gemini token limit causing invalid Part objects) while preserving existing functionality for all other scenarios. |
There was a problem hiding this comment.
Caution
Changes requested ❌
Reviewed everything up to 11ed2ea in 1 minute and 52 seconds. Click for details.
- Reviewed
189lines of code in2files - Skipped
0files when reviewing. - Skipped posting
2draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. instructor/dsl/partial.py:306
- Draft comment:
Avoid hardcoding the error substring 'validPart'. Define a module-level constant for this error message to ease future changes. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 20% vs. threshold = 85% The error message appears to be coming from an external library (likely the Google Generative AI library) based on the context. It's not our own error message that we control. Making it a constant wouldn't help much since we're just pattern matching against an external error. If the external library changes their error message, we'd need to update our code regardless of whether it's a constant or not. The comment has a valid point about avoiding magic strings in code. Constants can make code more maintainable and centralize changes. However, since this is an external error message we're pattern matching against, making it a constant doesn't provide significant value. The string isn't used for our own error handling but for identifying specific external errors. The comment should be deleted. While using constants for strings is generally good practice, in this case we're pattern matching an external error message and a constant wouldn't meaningfully improve the code.
2. tests/test_gemini_chunk_error.py:1
- Draft comment:
Excellent test coverage; both sync and async scenarios are thoroughly validated for the error handling logic. - Reason this comment was not posted:
Confidence changes required:0%<= threshold85%None
Workflow ID: wflow_5u2VOq7cKo4kuLHX
You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.
|
|
||
| if mode == Mode.GENAI_STRUCTURED_OUTPUTS: | ||
| yield chunk.text | ||
| try: |
There was a problem hiding this comment.
The try/except block wrapping 'yield chunk.text' is repeated for both GENAI_STRUCTURED_OUTPUTS and GEMINI_JSON (in sync and async functions). Consider extracting this logic into a helper to enforce DRY and simplify maintenance.
|
Claude finished @jxnl's task —— View job Code Review for PR #1809: Fix Gemini chunk.text ValueErrorTodo List:
Review SummaryThis PR effectively addresses issue #1808 with a well-implemented fix for handling Gemini chunk.text ValueError when finish_reason=1. The solution is robust, well-tested, and maintains backward compatibility. ✅ Code Quality & Best PracticesStrengths:
Minor suggestions:
✅ Test CoverageExcellent test coverage with:
The test file ✅ Bug AnalysisNo bugs identified. The implementation is solid:
✅ Performance Considerations
✅ Security Considerations
🔧 Technical Implementation DetailsChanges in
The error handling pattern is consistent: try:
yield chunk.text
except ValueError as e:
if "valid `Part`" in str(e):
continue # Skip invalid chunk
raise # Re-raise other ValueErrors✅ RecommendationAPPROVED - This is a well-crafted fix that:
The implementation correctly addresses the root cause (Gemini token limit causing invalid Part objects) while preserving existing functionality for all other scenarios. |
|
Claude finished @jxnl's task —— View job Code Review for PR #1809: Fix Gemini chunk.text ValueErrorTodo List:
Review SummaryThis PR effectively addresses issue #1808 with a well-implemented fix for handling Gemini chunk.text ValueError when finish_reason=1. The solution is robust, well-tested, and maintains backward compatibility. ✅ Code Quality & Best PracticesStrengths:
Minor suggestions:
✅ Test CoverageExcellent test coverage with:
The test file ✅ Bug AnalysisNo bugs identified. The implementation is solid:
✅ Performance Considerations
✅ Security Considerations
🔧 Technical Implementation DetailsChanges in
The error handling pattern is consistent: try:
yield chunk.text
except ValueError as e:
if "valid `Part`" in str(e):
continue # Skip invalid chunk
raise # Re-raise other ValueErrors✅ RecommendationAPPROVED - This is a well-crafted fix that:
The implementation correctly addresses the root cause (Gemini token limit causing invalid Part objects) while preserving existing functionality for all other scenarios. |
|
Claude finished @jxnl's task —— View job Code Review for PR #1809: Fix Gemini chunk.text ValueErrorTodo List:
Review SummaryThis PR effectively addresses issue #1808 with a well-implemented fix for handling Gemini chunk.text ValueError when finish_reason=1. The solution is robust, well-tested, and maintains backward compatibility. ✅ Code Quality & Best PracticesStrengths:
Minor suggestions:
✅ Test CoverageExcellent test coverage with:
Note: The test file ✅ Bug AnalysisNo bugs identified. The implementation is solid:
✅ Performance Considerations
✅ Security Considerations
🔧 Technical Implementation DetailsChanges in
The error handling pattern is consistent: try:
yield chunk.text
except ValueError as e:
if "valid `Part`" in str(e):
continue # Skip invalid chunk
raise # Re-raise other ValueErrors✅ RecommendationAPPROVED - This is a well-crafted fix that:
The implementation correctly addresses the root cause (Gemini token limit causing invalid Part objects) while preserving existing functionality for all other scenarios. |
Fixes issue #1808 where Gemini models hitting token limits cause ValueError when accessing chunk.text due to missing Part objects.
Changes:
The fix allows streaming responses to continue processing valid chunks even when some chunks are incomplete due to token limits.
Amp-Thread-ID: https://ampcode.com/threads/T-1bfe4270-cd9b-4c65-be2e-23a0f589dc10
Important
Fixes
ValueErrorhandling inextract_jsonfunctions forGEMINI_JSONandGENAI_STRUCTURED_OUTPUTSmodes by skipping invalid chunks.partial.py,extract_json()andextract_json_async()now handleValueErrorforGEMINI_JSONandGENAI_STRUCTURED_OUTPUTSmodes by skipping chunks with invalidPartobjects.ValueErrortypes by re-raising them.test_gemini_chunk_error.pyfor both sync and async scenarios to verify handling of invalidPartobjects inGEMINI_JSONandGENAI_STRUCTURED_OUTPUTSmodes.ValueErrortypes are re-raised.This description was created by
for 11ed2ea. You can customize this summary. It will automatically update as commits are pushed.