🤖 fix: correctly classify (and not retry) model not found errors #535
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Moved model_not_found detection logic from error handling into the categorizeError method to eliminate code duplication. This ensures consistent error classification across all error handling paths. Previously, the logic existed in two places: 1. In categorizeError (but returned 'api' for 404s) 2. In error handling (as a workaround to override 'api' to 'model_not_found') Now categorizeError directly returns 'model_not_found' for both: - OpenAI: 400 with error.code === 'model_not_found' - Anthropic: 404 with error.type === 'not_found_error' This prevents retry spam because model_not_found is in the NON_RETRYABLE_STREAM_ERRORS list in retryEligibility.ts. ## Test Coverage **Backend (IPC layer integration tests)**: - ✅ Anthropic 404 errors are classified as model_not_found - ✅ OpenAI 400 errors are classified as model_not_found **Frontend (unit tests in retryEligibility.test.ts)**: - ✅ model_not_found is in NON_RETRYABLE_STREAM_ERRORS list - ✅ isEligibleForAutoRetry returns false for model_not_found errors Generated with `cmux`
ammar-agent
force-pushed
the
fix-retry-spam
branch
from
5586b26 to
118452d
Compare
Removed manual loadTokenizerModules() calls from 8 integration test files. The tokenizer is already preloaded globally in tests/setup.ts via the preloadTestModules() function, which runs once per Jest worker. This eliminates duplication and ensures consistent test initialization across all integration tests. Files cleaned up: - anthropic1MContext.test.ts - forkWorkspace.test.ts - modelNotFound.test.ts - openai-web-search.test.ts - resumeStream.test.ts - sendMessage.test.ts - streamErrorRecovery.test.ts - truncate.test.ts Generated with `cmux`
ammario
changed the title
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Moved model_not_found detection logic from error handling into the categorizeError method to eliminate code duplication.
Problem
There were two bugs related to model_not_found error handling:
Code duplication: The model_not_found detection logic existed in two places:
categorizeError(but it returned'api'for 404s)'api'to'model_not_found')Potential retry spam: If the workaround code was bypassed, 404 errors would be classified as
'api', which is not in theNON_RETRYABLE_STREAM_ERRORSlist, leading to retry spam.Solution
Refactored
categorizeErrorto directly return'model_not_found'for both:error.code === 'model_not_found'error.type === 'not_found_error'Removed the duplicate override logic from error handling, leaving only the error message enhancement.
Benefits
model_not_foundis inNON_RETRYABLE_STREAM_ERRORSTest Coverage
Backend (IPC layer integration tests):
model_not_foundmodel_not_foundFrontend (unit tests in retryEligibility.test.ts):
model_not_foundis inNON_RETRYABLE_STREAM_ERRORSlistisEligibleForAutoRetryreturns false formodel_not_founderrorsGenerated with
cmux