Refactor recommendation: no internal ghost variables
There are many ghost params which are not configurable. For example in the gemini provider, you switch between models, large and small:
DEFAULT_MODEL = 'gemini-2.5-flash'
DEFAULT_SMALL_MODEL = 'gemini-2.5-flash-lite-preview-06-17'
This is problematic because the flash lite model has location constraints (global only) in Vertex AI, which differ from the flash model.
I recommend you allow a configuration of all clients + models, similar to how I did on PR #679, and not swap them out in code.
The (model + client + config) choice is really a provider, so you could simply collect all the provider variations you need up front, and swap between them as needed.
It will be functionally about the same, but simpler to customize and not be surprised by internal variables not changeable without editing the package.
Thanks for the suggestion. @claude created a PR with a proposed solution based on this suggestion. Think hard about a generalized solution while limiting breaking changes.
Claude finished @danielchalef's task —— View job • claude/issue-681-20250709_144821 • Create PR ➔
Ghost Variables Issue Resolution ✅
Successfully implemented a generalized solution for eliminating internal ghost variables while maintaining backward compatibility.
Todo List:
- [x] Set up development environment and analyze codebase
- [x] Examine current LLM client implementations for ghost variables
- [x] Review Gemini provider specific issue with model switching
- [x] Check PR #679 for the desired configuration pattern
- [x] Design generalized configurable provider system
- [x] Implement the solution
- [x] Run tests and linting
- [x] Create PR with documentation
Solution Summary:
✅ Complete Provider Configuration System Implemented
Key Features:
- Centralized Configuration: All provider defaults moved to
provider_defaults.py - Environment Variable Support: Easy customization without code changes
- Backward Compatibility: Existing configurations continue to work unchanged
- Provider-Specific Defaults: Each provider has optimized model selections
Ghost Variables Eliminated:
- ✅
gemini_client.py: Replaced hardcoded model constants - ✅
openai_base_client.py: Replaced hardcoded model constants - ✅
openai_generic_client.py: Updated to use provider defaults - ✅
anthropic_client.py: Updated to use provider defaults - ✅
groq_client.py: Updated to use provider defaults - ✅
edge_operations.py: Madeextract_edges_max_tokensconfigurable - ✅
gemini_reranker_client.py: Updated reranker to use provider defaults - ✅
openai_reranker_client.py: Updated reranker to use provider defaults
Environment Variables Added:
# Example: Solve Gemini Vertex AI location constraints
export GEMINI_DEFAULT_MODEL="gemini-2.5-flash"
export GEMINI_DEFAULT_SMALL_MODEL="gemini-2.5-flash" # Use flash instead of flash-lite
# Other providers
export OPENAI_DEFAULT_MODEL="gpt-4o"
export ANTHROPIC_DEFAULT_MODEL="claude-3-5-sonnet-latest"
export GROQ_DEFAULT_MODEL="llama-3.1-70b-versatile"
Files Modified:
- Created
provider_defaults.py- Core configuration system - Updated 5 LLM clients to use configurable defaults
- Updated 2 cross-encoder reranker clients
- Updated edge operations for configurable token limits
- Added comprehensive documentation in
PROVIDER_CONFIGURATION.md
Benefits Achieved:
- ✅ No more hardcoded "ghost variables"
- ✅ Easy model updates via environment variables
- ✅ Addresses Gemini Vertex AI location constraint issues
- ✅ Maintains full backward compatibility
- ✅ Supports all existing and future providers
- ✅ Reduces maintenance burden for model updates
@zeroasterisk Is this still relevant? Please confirm within 14 days or this issue will be closed.
Yes, I think it's still a good design principle to let users control the config.
I think it looks better. But this is your project and not mine.
@zeroasterisk Is this still an issue? Please confirm within 14 days or this issue will be closed.
@zeroasterisk Is this still an issue? Please confirm within 14 days or this issue will be closed.
@zeroasterisk Is this still relevant? Please confirm within 14 days or this issue will be closed.