fix(config,providers): resolve api_key, model lookup, and fallback bugs (#2371, #2286, #2334)#2372
Open
SaiBalusu-usf wants to merge 6 commits intosipeed:mainfrom
Open
Conversation
- Extract ContextCompressor (compression, summarization, token estimation) from AgentLoop - Extract ToolExecutor (tool dispatch, result routing, context updates) from AgentLoop - Create named constants replacing magic numbers (retry limits, thresholds, timeouts) - Replace fragile string-matching error detection with structured IsContextWindowError() - Add contextWindowPatterns and FailoverContextWindow to error classifier - Refactor provider factory to table-driven registry approach - Add graceful shutdown with context cancellation and WaitGroup - Net reduction of ~143 lines from loop.go via component extraction
Resolve conflicts between the decomposed agent loop architecture and main's new features (media store, voice transcription, command registry, MCP servers, parallel tool execution, model routing, and new providers). Conflict resolution strategy: - Keep decomposed components (ContextCompressor, ToolExecutor) alongside new features (mediaStore, transcriber, cmdRegistry) - Take main's parallel tool execution (more advanced than sequential ToolExecutor) - Keep ToolExecutor.UpdateToolContexts for context injection - Take main's timeout error detection + use extracted IsContextWindowError helper - Add selectCandidates for model routing support - Add new providers (litellm, vivgrid, avian, minimax) to standardProviderRegistry and modelInferenceRegistry, maintaining the data-driven refactored pattern Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…ror classifier - Fix token estimation regression in context_compressor (missing context, shutdown wiring, correct token counting) - Add EstimateTokens tests in context_compressor_test.go - Wire graceful shutdown in loop.go, remove dead summarizing field - Add iteration number to tool_executor logs, fix missing ctx - Tighten invalidparameter regex in error_classifier to avoid false positives - Add IsContextWindowError tests in error_classifier_test.go - Reduce duplication in factory.go with simpleInference helper - Update README with new providers (mistral, minimax, avian) and features Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Resolve conflicts by accepting main's versions for loop.go, factory.go, types.go, and README.md. Remove obsolete branch files (constants.go, context_compressor.go, context_compressor_test.go, tool_executor.go) that were superseded by main's ContextManager system. Retain contextWindowPatterns and IsContextWindowError additions to error_classifier.go. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…gs (sipeed#2371, sipeed#2286, sipeed#2334) - Accept legacy "api_key" (singular string) in ModelConfig JSON via custom UnmarshalJSON, fixing silent key drop that caused 401 auth errors - Extend findMatches to fall back to Model field and bare model ID when ModelName doesn't match, fixing thinking_level lookup failures - Classify HTTP 404 and "no endpoints found" as FailoverModelNotFound so the fallback chain correctly tries the next candidate instead of aborting Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Summary
"api_key"(singular string) inModelConfigJSON via customUnmarshalJSON, fixing silent key drop that caused 401 auth errorsthinking_levelfrom config when model and model_name are different #2286: ExtendfindMatchesto fall back toModelfield and bare model ID whenModelNamedoesn't match, fixingthinking_levellookup failuresFailoverModelNotFoundso the fallback chain correctly tries the next candidate instead of abortingTest plan
TestModelConfig_APIKeySingular— singularapi_keypopulatesAPIKeysTestModelConfig_APIKeysPlural— pluralapi_keysstill worksTestModelConfig_APIKeyBothForms— both forms merge correctlyTestModelConfig_APIKeyDuplicate— deduplication worksTestGetModelConfig_ByModelID— lookup by full model ID and bare IDTestClassifyError_ModelNotFound— 404 and "no endpoints" classified correctlyTestFailoverError_IsRetriable—model_not_foundis retriableCloses #2371, closes #2286, closes #2334
🤖 Generated with Claude Code