Skip to content

Use model instead of config.model inside an LLM instance #1849

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

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

GasolSun36
Copy link

@GasolSun36 GasolSun36 commented Jun 16, 2025

Features

Fixed bug where model configurations interfere with each other between LLM instances: introduced self.model as an independent copy of config.model
Established new code standards: LLM internally uses self.model instead of self.config.model to avoid accidental modification of global configuration
Support for manually modifying specific LLM instance model names (e.g., role_zero_memory fixed to use 4o-mini for compression) without affecting other instances

Feature Docs

New usage standards:
self.model: Private model configuration for LLM instances, can be safely modified, only affects current instance
self.config.model: Global configuration, should not be directly modified to avoid affecting other LLM instances

Influence

Bug fix: Resolved the issue where modifying self.config.model caused unexpected changes to other LLM instance configurations
Code safety: Improved isolation between LLM instances, avoiding configuration pollution
Backward compatibility: Existing code logic remains largely unchanged, only internal implementation is more secure
Extensibility: Provides a safe implementation approach for model customization in specific scenarios

Result

All related unit tests passed:
✅ tests/metagpt/provider/test_base_llm.py
✅ tests/metagpt/provider/mock_llm_config.py
✅ tests/metagpt/provider/test_bedrock_api.py
✅ tests/metagpt/provider/test_human_provider.py
✅ tests/metagpt/provider/test_ollama_api.py
✅ tests/metagpt/provider/test_openai.py (tts related failures are related to oneapi, not caused by this change)
✅ tests/metagpt/provider/test_qianfan_api.py (TypeError related failures not caused by this change)

image

Other

This change is a preventive bug fix that primarily affects the internal implementation of the LLM provider layer, with no impact on external API calling methods. It is recommended to focus on code paths related to LLM instance creation and model configuration.

@better629
Copy link
Collaborator

Is it possible to initialize directly in basellm's init without having to go to other providers or some initialization functions such as __init_qianfan? Is this more concise?

@GasolSun36
Copy link
Author

Is it possible to initialize directly in basellm's init without having to go to other providers or some initialization functions such as __init_qianfan? Is this more concise?

That feature (direct initialization in BaseLLM's init) could be implemented in a separate PR, as it can be considered a small, independent code optimization. This PR focuses on fixing configuration isolation bugs and enabling per-instance model customization.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants