-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
添加MistralAI的Adapter #1329
base: browser-version
Are you sure you want to change the base?
添加MistralAI的Adapter #1329
Conversation
添加config.example.cfg中关于MistralAI的示例 更新Quart包到0.19.4(修复0.17.0中自带的flask版本和url_decode在python3.11中不兼容的错误)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PR Type: Enhancement
PR Summary: This pull request introduces an adapter for MistralAI, including the necessary configurations and updates to integrate MistralAI's services into the existing system. It also includes updates to the Quart package to address compatibility issues with Python 3.11. The changes span across multiple files, adding new classes for handling MistralAI's API interactions, updating configuration files to include MistralAI settings, and extending the bot manager to support MistralAI accounts.
Decision: Comment
📝 Type: 'Enhancement' - not supported yet.
- Sourcery currently only approves 'Typo fix' PRs.
✅ Issue addressed: this change correctly addresses the issue or implements the desired feature.
No details provided.
📝 Complexity: the changes are too large or complex for Sourcery to approve.
- Unsupported files: the diff contains files that Sourcery does not currently support during reviews.
General suggestions:
- Consider simplifying the configuration classes to reduce complexity and improve maintainability. Consolidating related configurations or utilizing inheritance could make the codebase more manageable.
- To streamline the addition of new adapters and reduce repetitive code, explore using a mapping strategy for bot types to their corresponding adapter classes. This could make the system more scalable and easier to extend with new services.
- Given the significant new functionality introduced with the MistralAI adapter, ensure comprehensive testing coverage. Including unit, integration, and end-to-end tests will be crucial for maintaining the reliability of the system.
- Documenting the use cases and scenarios where the new
preset_ask
method could be beneficial would help developers and testers understand its purpose and potential applications.
Thanks for using Sourcery. We offer it for free for open source projects and would be very grateful if you could help us grow. If you like it, would you consider sharing Sourcery on your favourite social media? ✨
@@ -279,6 +279,37 @@ class G4fAuths(BaseModel): | |||
"""支持的模型""" | |||
|
|||
|
|||
class MistralAIParams(BaseModel): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
issue (llm): The introduction of new classes (MistralAIParams
, MistralAIAPIKey
, MistralAuths
) and the associated configuration options significantly increases the complexity of the codebase. While these changes add structure and potentially enhance flexibility, they also introduce a higher cognitive load for understanding and managing configurations. Here are a few suggestions to consider for reducing complexity:
-
Consolidate Configuration Classes: If
MistralAIParams
andMistralAIAPIKey
share common parameters or can logically be grouped, consider merging them or creating a hierarchical structure. This could reduce the need to navigate between multiple classes to understand related configurations. -
Utilize Inheritance: If the new classes share attributes or methods with existing ones, leveraging inheritance or mixins could help in sharing code and reducing redundancy, making the overall structure cleaner.
-
Simplify Default Values Management: Consider using a configuration file (JSON, YAML, etc.) for managing default values. This approach can make the code cleaner and the configuration process more flexible, as it externalizes configuration from the code.
-
Review the Necessity of Nested Classes: The use of nested classes, such as in
MistralAuths
, adds another layer of complexity. Evaluate if this nesting is essential or if there's a simpler way to achieve the same functionality.
By addressing these points, we can aim for a balance between flexibility and maintainability, ensuring the code remains accessible and manageable as it evolves.
@@ -112,6 +113,12 @@ def __init__(self, _type: str, session_id: str): | |||
self.adapter = XinghuoAdapter(self.session_id) | |||
elif g4f_parse(_type): | |||
self.adapter = Gpt4FreeAdapter(self.session_id, g4f_parse(_type)) | |||
elif _type == LlmName.MistralLarge.value: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
issue (llm): The addition of separate if-elif
statements for MistralLarge
, MistralMedium
, and MistralSmall
introduces repetitive code that could be streamlined. Consider using a dictionary to map LlmName
enum values to their corresponding adapter classes. This approach would not only reduce the complexity by eliminating repetitive code but also make the codebase more maintainable and scalable. Here's a suggested implementation:
adapter_mapping = {
LlmName.MistralLarge.value: MistralAIAPIAdapter,
LlmName.MistralMedium.value: MistralAIAPIAdapter,
LlmName.MistralSmall.value: MistralAIAPIAdapter,
# Add other mappings here
}
adapter_class = adapter_mapping.get(_type)
if adapter_class:
self.adapter = adapter_class(self.session_id)
else:
raise BotTypeNotFoundException(_type)
This way, you clearly separate the logic for determining the adapter type from the instantiation of the adapter objects, making the code easier to read and maintain.
@@ -398,6 +406,17 @@ async def login_openai(self): # sourcery skip: raise-specific-error | |||
logger.error("所有 OpenAI 账号均登录失败!") | |||
logger.success(f"成功登录 {counter}/{len(self.openai)} 个 OpenAI 账号!") | |||
|
|||
def login_mistral(self): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
issue (llm): The addition of the "mistral"
bot type introduces additional complexity to the codebase, primarily due to the manual handling of each bot type, which increases the code size and introduces repetitive patterns. This approach might not scale well as more bots are added in the future, leading to a codebase that's harder to maintain and extend.
To mitigate these issues, consider abstracting the common behavior of bot handling into more generic methods. This could involve using a dictionary to map bot types to their respective login functions and account information. By doing so, adding a new bot type would require minimal changes, significantly reducing repetition and making the codebase more scalable and easier to maintain.
Here's a simplified approach that abstracts the bot handling logic, making it easier to add new bots with minimal code changes:
class BotManager:
def __init__(self, config: Config) -> None:
self.config = config
self.bot_accounts = {
"openai": config.openai.accounts,
"bing": config.bing.accounts,
"bard": config.bard.accounts,
"mistral": config.mistral.accounts, # New bot added with minimal code change
}
self.bot_login_functions = {
"openai": self.handle_openai,
"bing": self.login_bing,
"mistral": self.login_mistral, # New bot login function
}
self.bots = {bot_type: [] for bot_type in self.bot_accounts.keys()}
self.__login_all_bots()
async def __login_all_bots(self):
for bot_type, login_func in self.bot_login_functions.items():
accounts = self.bot_accounts[bot_type]
if accounts:
await self.__login_bot(accounts, login_func)
async def __login_bot(self, accounts, login_func):
# Generic login logic here
pass
# Define login functions for each bot type
async def login_mistral(self, accounts):
# Specific login logic for Mistral
pass
# Other bot-specific login functions...
This approach reduces the complexity introduced by the new bot type and makes the codebase more maintainable and scalable.
@@ -0,0 +1,326 @@ | |||
import json |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
issue (llm): I noticed that the new adapter for MistralAI has been added, but there are no unit tests accompanying this addition. It's crucial to have unit tests to verify the behavior of the new adapter, especially for methods like rollback
, add_to_conversation
, count_tokens
, request
, and ask
. Could you please add unit tests to ensure these methods work as expected under various conditions?
yield f"发生错误: \n{e}" | ||
raise | ||
|
||
async def preset_ask(self, role: str, text: str): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
suggestion (llm): It's great to see the implementation of preset_ask
for handling predefined interactions. However, to ensure its reliability and correctness, could you add integration tests that simulate real-world scenarios where preset_ask
is used? This would help in catching any potential issues with the interaction between this method and the MistralAI API.
@@ -0,0 +1,326 @@ | |||
import json |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
suggestion (llm): The addition of the MistralAI adapter introduces significant new functionality. To ensure the adapter's resilience and maintainability, consider adding end-to-end tests that cover the full lifecycle of a conversation through the adapter, including error handling and retries for API failures. This will help ensure the adapter behaves correctly in the context of the larger application.
yield f"发生错误: \n{e}" | ||
raise | ||
|
||
async def preset_ask(self, role: str, text: str): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
suggestion (llm): Given the complexity of managing conversations with different roles (assistant
, user
, system
), it would be beneficial to have tests that specifically verify the conversation state is managed correctly over multiple interactions. This includes testing the conversation history is accurately maintained and that role-based behaviors are correctly implemented.
yield f"发生错误: \n{e}" | ||
raise | ||
|
||
async def preset_ask(self, role: str, text: str): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
praise (llm): Praise for implementing the preset_ask
method to handle predefined interactions. This is a valuable feature for simulating specific scenarios or testing. However, to fully leverage its potential, consider documenting example use cases or scenarios where preset_ask
could be particularly useful. This documentation could be included in the method comments or a separate documentation file.
添加MistralAI的Adapter
添加config.example.cfg中关于MistralAI的示例
更新Quart包到0.19.4(修复0.17.0中自带的flask版本和url_decode在python3.11中不兼容的错误)