-
Notifications
You must be signed in to change notification settings - Fork 1
[feat] 添加了对多语言的支持并更改了Cl #3
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
Conversation
审阅者指南此拉取请求(PR)全面修订了配置加载器,使其在单例ConfigManager中使用Pydantic模型;实现了一个带有消息目录的本地化层(I18N),并将核心模块中硬编码的字符串替换为i18n.gettext调用;并更新了两个README文件,以记录多语言支持和完善的技术规范。 机器人初始化与本地化序列图sequenceDiagram
actor User
participant Main as main.py
participant Config as ConfigManager
participant Bot as MeshBot
participant I18N as I18N
User->>Main: Start bot
Main->>Config: load_config()
Config->>Config: Load and validate config
Config->>I18N: Set language
Main->>Bot: Instantiate MeshBot
Bot->>Config: Get platform, language
Bot->>I18N: Use gettext for logs
Bot->>User: Log messages in selected language
消息处理与i18n序列图sequenceDiagram
participant Bot as MeshBot
participant Processor as MessageProcessor
participant I18N as I18N
participant User
Bot->>Processor: handle_incoming_message()
Processor->>I18N: gettext() for log and reply
Processor->>User: Send reply in selected language
新配置和本地化系统类图classDiagram
class ConfigManager {
+DEFAULT_CONFIG: Dict[str, Any]
+_instance: Optional[ConfigManager]
+_config: Optional[FullConfig]
+_user_config: Optional[Dict[str, Any]]
+_config_path: Optional[Path]
+load(config_path: Optional[str])
+reload(config_path: Optional[str])
+create_example_config(overwrite: bool)
+get_current_config()
+platform: str
+system_prompt: str
+max_response_length: int
+message_queue_timeout: int
+ai_client_config: Dict[str, ClientConfig]
+language: str
+timezone: str
+encoding: str
+get_client_config(client_name: str)
}
class FullConfig {
+system: SystemConfig
+localization: LocalizationConfig
+clients: Dict[str, ClientConfig]
+app: AppConfig
}
class SystemConfig {
+system_prompt: str
+max_response_length: int
+message_queue_timeout: int
}
class LocalizationConfig {
+language: str
+timezone: str
+encoding: str
}
class AppConfig {
+platform: str
+api_keys: Dict[str, str]
+model_settings: Dict[str, str]
+service_urls: Dict[str, str]
+system_prompt: Optional[str]
}
class ClientConfig {
+module: str
+class_name: str
+kwargs: Dict[str, Any]
+__getitem__(item)
}
class I18N {
+language: str
+messages: dict
+gettext(key: str, **kwargs)
}
ConfigManager --> FullConfig
FullConfig --> SystemConfig
FullConfig --> LocalizationConfig
FullConfig --> AppConfig
FullConfig --> "clients" ClientConfig
I18N --> "messages" LocalizationConfig
文件级变更
提示与命令与 Sourcery 交互
自定义你的体验访问你的 仪表盘 以:
获取帮助Original review guide in EnglishReviewer's GuideThis PR overhauls the configuration loader to use Pydantic models within a singleton ConfigManager, implements a localization layer (I18N) with message catalogs and replaces hardcoded strings across core modules with i18n.gettext calls, and updates both README files to document multi-language support and refined technical specifications. Sequence diagram for bot initialization with localizationsequenceDiagram
actor User
participant Main as main.py
participant Config as ConfigManager
participant Bot as MeshBot
participant I18N as I18N
User->>Main: Start bot
Main->>Config: load_config()
Config->>Config: Load and validate config
Config->>I18N: Set language
Main->>Bot: Instantiate MeshBot
Bot->>Config: Get platform, language
Bot->>I18N: Use gettext for logs
Bot->>User: Log messages in selected language
Sequence diagram for message processing with i18nsequenceDiagram
participant Bot as MeshBot
participant Processor as MessageProcessor
participant I18N as I18N
participant User
Bot->>Processor: handle_incoming_message()
Processor->>I18N: gettext() for log and reply
Processor->>User: Send reply in selected language
Class diagram for new configuration and localization systemclassDiagram
class ConfigManager {
+DEFAULT_CONFIG: Dict[str, Any]
+_instance: Optional[ConfigManager]
+_config: Optional[FullConfig]
+_user_config: Optional[Dict[str, Any]]
+_config_path: Optional[Path]
+load(config_path: Optional[str])
+reload(config_path: Optional[str])
+create_example_config(overwrite: bool)
+get_current_config()
+platform: str
+system_prompt: str
+max_response_length: int
+message_queue_timeout: int
+ai_client_config: Dict[str, ClientConfig]
+language: str
+timezone: str
+encoding: str
+get_client_config(client_name: str)
}
class FullConfig {
+system: SystemConfig
+localization: LocalizationConfig
+clients: Dict[str, ClientConfig]
+app: AppConfig
}
class SystemConfig {
+system_prompt: str
+max_response_length: int
+message_queue_timeout: int
}
class LocalizationConfig {
+language: str
+timezone: str
+encoding: str
}
class AppConfig {
+platform: str
+api_keys: Dict[str, str]
+model_settings: Dict[str, str]
+service_urls: Dict[str, str]
+system_prompt: Optional[str]
}
class ClientConfig {
+module: str
+class_name: str
+kwargs: Dict[str, Any]
+__getitem__(item)
}
class I18N {
+language: str
+messages: dict
+gettext(key: str, **kwargs)
}
ConfigManager --> FullConfig
FullConfig --> SystemConfig
FullConfig --> LocalizationConfig
FullConfig --> AppConfig
FullConfig --> "clients" ClientConfig
I18N --> "messages" LocalizationConfig
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
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.
大家好 - 我已经审阅了您的更改 - 以下是一些反馈:
- ConfigManager 仍然手动深度合并默认值并应用覆盖;考虑利用 Pydantic 模型默认值和验证器来简化或消除自定义合并逻辑。
- I18N 类在 gettext 内部延迟确定语言;考虑一次性配置语言(例如,在加载配置后)以避免每次调用时都检查配置管理器。
- 许多日志消息现在依赖于动态翻译键;考虑添加一个加载时检查,以确保 gettext 调用中使用的所有键都存在于本地化映射中,以防止运行时格式错误。
给 AI 代理的提示
请处理此代码审查中的评论:
## 总体评论
- ConfigManager 仍然手动深度合并默认值并应用覆盖;考虑利用 Pydantic 模型默认值和验证器来简化或消除自定义合并逻辑。
- I18N 类在 gettext 内部延迟确定语言;考虑一次性配置语言(例如,在加载配置后)以避免每次调用时都检查配置管理器。
- 许多日志消息现在依赖于动态翻译键;考虑添加一个加载时检查,以确保 gettext 调用中使用的所有键都存在于本地化映射中,以防止运行时格式错误。
## 单独评论
### 评论 1
<location> `meshbot/config/config_loader.py:194-203` </location>
<code_context>
+ def _apply_user_overrides(self) -> None:
</code_context>
<issue_to_address>
**issue (bug_risk):** API 密钥覆盖逻辑可能不会更新密钥,如果默认值不是 'your-...'。
目前,覆盖仅在默认密钥以 'your-' 开头时才适用。为避免默认值更改或用户需要覆盖有效密钥时出现问题,请更新逻辑以始终应用用户提供的密钥。
</issue_to_address>
### 评论 2
<location> `meshbot/utils/localize.py:4-6` </location>
<code_context>
+from meshbot.config.config_loader import _config_manager
+from meshbot.localizations.localization import MESSAGES
+
+class I18N:
+ def __init__(self):
+ self.language = ""
+ self.messages = MESSAGES.get(self.language, MESSAGES['zh_CN'])
+
</code_context>
<issue_to_address>
**issue (bug_risk):** I18N 中的语言初始化可能导致回退问题。
由于语言在初始化期间只设置一次,因此配置管理器语言的更改将不会反映。为避免不一致的本地化,请每次从配置管理器获取语言或允许动态更新。
</issue_to_address>
### 评论 3
<location> `meshbot/utils/localize.py:9-18` </location>
<code_context>
+ self.language = ""
+ self.messages = MESSAGES.get(self.language, MESSAGES['zh_CN'])
+
+ def gettext(self, key: str, **kwargs) -> str:
+ """获取本地化消息,支持格式化参数"""
+ if self.language:
+ pass
+ else:
+ self.language = _config_manager.language
+
+ message_template = self.messages.get(key, key)
+
+ # 如果有参数,进行格式化
+ if kwargs:
+ try:
+ return message_template.format(**kwargs)
+ except KeyError as e:
+ return f"[Format error in '{key}': missing {e}]"
+
+ return message_template
</code_context>
<issue_to_address>
**🚨 suggestion (security):** 在面向用户的字符串中返回格式错误可能会泄露内部细节。
记录格式错误并返回通用消息将防止向用户暴露内部细节并改善用户体验。
建议实现:
```python
import logging
from meshbot.config.config_loader import _config_manager
from meshbot.localizations.localization import MESSAGES
logger = logging.getLogger(__name__)
class I18N:
```
```python
# 如果有参数,进行格式化
if kwargs:
try:
return message_template.format(**kwargs)
except KeyError as e:
logger.error("Format error in '%s': missing %s", key, e)
return "Message formatting error"
return message_template
```
</issue_to_address>
### 评论 4
<location> `meshbot/core/message_processor.py:183-186` </location>
<code_context>
# 判断消息类型并添加相应标识
is_broadcast = self._is_broadcast_message(to_id)
- message_type = "📢 群发" if is_broadcast else "📩 私聊"
+ message_type = i18n.gettext('broadcast_message_received') if is_broadcast else i18n.gettext('private_message_received')
logger.info(
- f"{message_type} 来自 {from_id}{name_info}: {short_text}"
+ message_type.format(from_id=from_id, name_info=name_info, short_text=short_text)
)
</code_context>
<issue_to_address>
**issue (bug_risk):** 对本地化字符串使用格式化可能会在占位符缺失时导致运行时错误。
在格式化之前验证本地化字符串是否包含所有必需的占位符,或者使用能够优雅处理缺失占位符的格式化方法。
</issue_to_address>
### 评论 5
<location> `meshbot/config/config_loader.py:202-205` </location>
<code_context>
if (platform in self._config.clients and
api_key not in ["your-api-key", "your-openai-api-key", ""]):
if self._config.clients[platform].kwargs.get("api_key", "").startswith("your-"):
self._config.clients[platform].kwargs["api_key"] = api_key
</code_context>
<issue_to_address>
**suggestion (code-quality):** 合并嵌套的 if 条件 ([`merge-nested-ifs`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/Python/Default-Rules/merge-nested-ifs))
```suggestion
if (platform in self._config.clients and
api_key not in ["your-api-key", "your-openai-api-key", ""]) and self._config.clients[platform].kwargs.get("api_key", "").startswith("your-"):
self._config.clients[platform].kwargs["api_key"] = api_key
```
<br/><details><summary>Explanation</summary>过多的嵌套会使代码难以理解,在 Python 中尤其如此,因为没有括号来帮助区分不同的嵌套级别。
阅读深度嵌套的代码令人困惑,因为您必须跟踪哪些条件与哪些级别相关。因此,我们努力在可能的情况下减少嵌套,而两个 `if` 条件可以使用 `and` 组合的情况是一个简单的胜利。
</details>
</issue_to_address>
### 评论 6
<location> `meshbot/config/config_loader.py:210-212` </location>
<code_context>
if platform in self._config.clients:
if "default_model" in self._config.clients[platform].kwargs:
self._config.clients[platform].kwargs["default_model"] = model
</code_context>
<issue_to_address>
**suggestion (code-quality):** 合并嵌套的 if 条件 ([`merge-nested-ifs`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/Python/Default-Rules/merge-nested-ifs))
```suggestion
if platform in self._config.clients and "default_model" in self._config.clients[platform].kwargs:
self._config.clients[platform].kwargs["default_model"] = model
```
<br/><details><summary>Explanation</summary>过多的嵌套会使代码难以理解,在 Python 中尤其如此,因为没有括号来帮助区分不同的嵌套级别。
阅读深度嵌套的代码令人困惑,因为您必须跟踪哪些条件与哪些级别相关。因此,我们努力在可能的情况下减少嵌套,而两个 `if` 条件可以使用 `and` 组合的情况是一个简单的胜利。
</details>
</issue_to_address>
### 评论 7
<location> `meshbot/config/config_loader.py:18-20` </location>
<code_context>
def __getitem__(self, item):
if item == "class":
return getattr(self, "class_name")
return getattr(self, item)
</code_context>
<issue_to_address>
**suggestion (code-quality):** 我们发现了这些问题:
- 在控制流跳转后将代码提升到 else ([`reintroduce-else`](https://docs.sourcery.ai/Reference/Default-Rules/refactorings/reintroduce-else/))
- 将 if 语句替换为 if 表达式 ([`assign-if-exp`](https://docs.sourcery.ai/Reference/Default-Rules/refactorings/assign-if-exp/))
```suggestion
return getattr(self, "class_name") if item == "class" else getattr(self, item)
```
</issue_to_address>
### 评论 8
<location> `meshbot/config/config_loader.py:157` </location>
<code_context>
def load(self, config_path: Optional[str] = None) -> None:
"""从 JSON 文件加载配置并与默认配置合并"""
if config_path is None:
config_path = self.get_default_config_path()
self._config_path = Path(config_path)
try:
with open(self._config_path, "r", encoding="utf-8") as f:
self._user_config = json.load(f)
except FileNotFoundError:
raise RuntimeError(f"配置文件未找到: {config_path}")
except json.JSONDecodeError as e:
raise RuntimeError(f"配置文件格式错误: {e}")
except Exception as e:
raise RuntimeError(f"读取配置文件失败: {e}")
# 合并配置
merged_config = self._deep_merge(self.DEFAULT_CONFIG.copy(), self._user_config or {})
# 使用 Pydantic 验证和转换
try:
self._config = FullConfig(**merged_config)
except Exception as e:
raise RuntimeError(f"配置验证失败: {e}")
# 应用用户配置覆盖
self._apply_user_overrides()
logger.info("✅ 配置加载成功")
logger.info(f"🎯 当前平台: {self.platform}")
logger.info(f"🌐 语言设置: {self.language}")
</code_context>
<issue_to_address>
**issue (code-quality):** 明确地从先前的错误中引发异常 [×4] ([`raise-from-previous-error`](https://docs.sourcery.ai/Reference/Default-Rules/suggestions/raise-from-previous-error/))
</issue_to_address>
### 评论 9
<location> `meshbot/core/message_processor.py:356-364` </location>
<code_context>
def update_broadcast_settings(self, enabled: bool = False, keywords: List[str] = [""]):
"""更新群发消息设置"""
if enabled is not None:
self.broadcast_enabled = enabled
status = i18n.gettext('enabled') if enabled else i18n.gettext('disabled')
logger.info(i18n.gettext('broadcast_settings_updated', status=status))
if keywords is not None:
self.broadcast_keywords = keywords
logger.info(i18n.gettext('keywords_updated', keywords=keywords))
</code_context>
<issue_to_address>
**issue (code-quality):** 将可变默认参数替换为 None ([`default-mutable-arg`](https://docs.sourcery.ai/Reference/Default-Rules/suggestions/default-mutable-arg/))
</issue_to_address>
### 评论 10
<location> `meshbot/utils/ai_client_factory.py:49-54` </location>
<code_context>
def create_ai_client(platform: str = ""):
"""
创建指定平台的 AI 客户端,失败时回退到 Ollama。
如果不指定 platform,则使用配置中的默认平台。
"""
# 获取配置
ai_client_config = get_ai_client_config()
default_platform = get_platform()
# 如果没有指定平台,使用默认平台
if platform is None:
platform = default_platform
# 获取配置,优先使用传入的 platform,否则使用默认 PLATFORM
config = ai_client_config.get(platform) or ai_client_config.get(default_platform)
if not config:
logger.error(i18n.gettext('platform_not_found', platform = platform, default_platform = default_platform))
# 回退到内置 Ollama 配置
logger.info(i18n.gettext('back_to_ollama'))
from api.ollama_api import AsyncOllamaChatClient
return AsyncOllamaChatClient(default_model="qwen2.5:7b")
try:
# 动态导入模块和类
module = importlib.import_module(config["module"])
client_class = getattr(module, config["class"])
# 复制 kwargs,避免污染原始配置
kwargs = config["kwargs"].copy()
# 创建实例
logger.info(i18n.gettext('ai_client_created', platform = platform))
return client_class(**kwargs)
except (ImportError, AttributeError, KeyError) as e:
logger.error(
i18n.gettext('ai_client_creation_failed', platform = platform, error_type = type(e).__name__, error_msg = e)
)
try:
from api.ollama_api import AsyncOllamaChatClient
return AsyncOllamaChatClient(default_model="qwen2.5:7b")
except ImportError:
logger.critical(i18n.gettext('fallback_failed'))
raise RuntimeError(i18n.gettext('ai_client_init_failed'))
</code_context>
<issue_to_address>
**issue (code-quality):** 明确地从先前的错误中引发异常 ([`raise-from-previous-error`](https://docs.sourcery.ai/Reference/Default-Rules/suggestions/raise-from-previous-error/))
</issue_to_address>
### 评论 11
<location> `meshbot/utils/localize.py:11-14` </location>
<code_context>
def gettext(self, key: str, **kwargs) -> str:
"""获取本地化消息,支持格式化参数"""
if self.language:
pass
else:
self.language = _config_manager.language
message_template = self.messages.get(key, key)
# 如果有参数,进行格式化
if kwargs:
try:
return message_template.format(**kwargs)
except KeyError as e:
return f"[Format error in '{key}': missing {e}]"
return message_template
</code_context>
<issue_to_address>
**issue (code-quality):** 交换 if/else 以移除空的 if 主体 ([`remove-pass-body`](https://docs.sourcery.ai/Reference/Default-Rules/refactorings/remove-pass-body/))
</issue_to_address>帮助我更有用!请点击每个评论上的 👍 或 👎,我将使用反馈来改进您的评论。
Original comment in English
Hey there - I've reviewed your changes - here's some feedback:
- The ConfigManager still manually deep-merges defaults and applies overrides; consider leveraging Pydantic model defaults and validators to simplify or eliminate the custom merge logic.
- The I18N class determines the language lazily inside gettext; consider configuring the language once (e.g., after loading config) to avoid checking the config manager on each call.
- Many log messages now rely on dynamic translation keys; consider adding a load-time check to ensure all keys used in gettext calls exist in the localization maps to prevent runtime format errors.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The ConfigManager still manually deep-merges defaults and applies overrides; consider leveraging Pydantic model defaults and validators to simplify or eliminate the custom merge logic.
- The I18N class determines the language lazily inside gettext; consider configuring the language once (e.g., after loading config) to avoid checking the config manager on each call.
- Many log messages now rely on dynamic translation keys; consider adding a load-time check to ensure all keys used in gettext calls exist in the localization maps to prevent runtime format errors.
## Individual Comments
### Comment 1
<location> `meshbot/config/config_loader.py:194-203` </location>
<code_context>
+ def _apply_user_overrides(self) -> None:
</code_context>
<issue_to_address>
**issue (bug_risk):** API key override logic may not update keys if the default is not 'your-...'.
Currently, the override only applies when the default key starts with 'your-'. To avoid issues if the default changes or users need to override a valid key, update the logic to always apply the user-provided key.
</issue_to_address>
### Comment 2
<location> `meshbot/utils/localize.py:4-6` </location>
<code_context>
+from meshbot.config.config_loader import _config_manager
+from meshbot.localizations.localization import MESSAGES
+
+class I18N:
+ def __init__(self):
+ self.language = ""
+ self.messages = MESSAGES.get(self.language, MESSAGES['zh_CN'])
+
</code_context>
<issue_to_address>
**issue (bug_risk):** Language initialization in I18N may cause fallback issues.
Since the language is set only once during initialization, changes to the config manager's language won't be reflected. To avoid inconsistent localization, fetch the language from the config manager each time or allow dynamic updates.
</issue_to_address>
### Comment 3
<location> `meshbot/utils/localize.py:9-18` </location>
<code_context>
+ self.language = ""
+ self.messages = MESSAGES.get(self.language, MESSAGES['zh_CN'])
+
+ def gettext(self, key: str, **kwargs) -> str:
+ """获取本地化消息,支持格式化参数"""
+ if self.language:
+ pass
+ else:
+ self.language = _config_manager.language
+
+ message_template = self.messages.get(key, key)
+
+ # 如果有参数,进行格式化
+ if kwargs:
+ try:
+ return message_template.format(**kwargs)
+ except KeyError as e:
+ return f"[Format error in '{key}': missing {e}]"
+
+ return message_template
</code_context>
<issue_to_address>
**🚨 suggestion (security):** Returning format errors in user-facing strings may leak internal details.
Logging the format error and returning a generic message will prevent exposing internal details to users and improve user experience.
Suggested implementation:
```python
import logging
from meshbot.config.config_loader import _config_manager
from meshbot.localizations.localization import MESSAGES
logger = logging.getLogger(__name__)
class I18N:
```
```python
# 如果有参数,进行格式化
if kwargs:
try:
return message_template.format(**kwargs)
except KeyError as e:
logger.error("Format error in '%s': missing %s", key, e)
return "Message formatting error"
return message_template
```
</issue_to_address>
### Comment 4
<location> `meshbot/core/message_processor.py:183-186` </location>
<code_context>
# 判断消息类型并添加相应标识
is_broadcast = self._is_broadcast_message(to_id)
- message_type = "📢 群发" if is_broadcast else "📩 私聊"
+ message_type = i18n.gettext('broadcast_message_received') if is_broadcast else i18n.gettext('private_message_received')
logger.info(
- f"{message_type} 来自 {from_id}{name_info}: {short_text}"
+ message_type.format(from_id=from_id, name_info=name_info, short_text=short_text)
)
</code_context>
<issue_to_address>
**issue (bug_risk):** Using format on a localized string may cause runtime errors if placeholders are missing.
Validate that the localized string includes all required placeholders before formatting, or use a formatting approach that handles missing placeholders gracefully.
</issue_to_address>
### Comment 5
<location> `meshbot/config/config_loader.py:202-205` </location>
<code_context>
if (platform in self._config.clients and
api_key not in ["your-api-key", "your-openai-api-key", ""]):
if self._config.clients[platform].kwargs.get("api_key", "").startswith("your-"):
self._config.clients[platform].kwargs["api_key"] = api_key
</code_context>
<issue_to_address>
**suggestion (code-quality):** Merge nested if conditions ([`merge-nested-ifs`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/Python/Default-Rules/merge-nested-ifs))
```suggestion
if (platform in self._config.clients and
api_key not in ["your-api-key", "your-openai-api-key", ""]) and self._config.clients[platform].kwargs.get("api_key", "").startswith("your-"):
self._config.clients[platform].kwargs["api_key"] = api_key
```
<br/><details><summary>Explanation</summary>Too much nesting can make code difficult to understand, and this is especially
true in Python, where there are no brackets to help out with the delineation of
different nesting levels.
Reading deeply nested code is confusing, since you have to keep track of which
conditions relate to which levels. We therefore strive to reduce nesting where
possible, and the situation where two `if` conditions can be combined using
`and` is an easy win.
</details>
</issue_to_address>
### Comment 6
<location> `meshbot/config/config_loader.py:210-212` </location>
<code_context>
if platform in self._config.clients:
if "default_model" in self._config.clients[platform].kwargs:
self._config.clients[platform].kwargs["default_model"] = model
</code_context>
<issue_to_address>
**suggestion (code-quality):** Merge nested if conditions ([`merge-nested-ifs`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/Python/Default-Rules/merge-nested-ifs))
```suggestion
if platform in self._config.clients and "default_model" in self._config.clients[platform].kwargs:
self._config.clients[platform].kwargs["default_model"] = model
```
<br/><details><summary>Explanation</summary>Too much nesting can make code difficult to understand, and this is especially
true in Python, where there are no brackets to help out with the delineation of
different nesting levels.
Reading deeply nested code is confusing, since you have to keep track of which
conditions relate to which levels. We therefore strive to reduce nesting where
possible, and the situation where two `if` conditions can be combined using
`and` is an easy win.
</details>
</issue_to_address>
### Comment 7
<location> `meshbot/config/config_loader.py:18-20` </location>
<code_context>
def __getitem__(self, item):
if item == "class":
return getattr(self, "class_name")
return getattr(self, item)
</code_context>
<issue_to_address>
**suggestion (code-quality):** We've found these issues:
- Lift code into else after jump in control flow ([`reintroduce-else`](https://docs.sourcery.ai/Reference/Default-Rules/refactorings/reintroduce-else/))
- Replace if statement with if expression ([`assign-if-exp`](https://docs.sourcery.ai/Reference/Default-Rules/refactorings/assign-if-exp/))
```suggestion
return getattr(self, "class_name") if item == "class" else getattr(self, item)
```
</issue_to_address>
### Comment 8
<location> `meshbot/config/config_loader.py:157` </location>
<code_context>
def load(self, config_path: Optional[str] = None) -> None:
"""从 JSON 文件加载配置并与默认配置合并"""
if config_path is None:
config_path = self.get_default_config_path()
self._config_path = Path(config_path)
try:
with open(self._config_path, "r", encoding="utf-8") as f:
self._user_config = json.load(f)
except FileNotFoundError:
raise RuntimeError(f"配置文件未找到: {config_path}")
except json.JSONDecodeError as e:
raise RuntimeError(f"配置文件格式错误: {e}")
except Exception as e:
raise RuntimeError(f"读取配置文件失败: {e}")
# 合并配置
merged_config = self._deep_merge(self.DEFAULT_CONFIG.copy(), self._user_config or {})
# 使用 Pydantic 验证和转换
try:
self._config = FullConfig(**merged_config)
except Exception as e:
raise RuntimeError(f"配置验证失败: {e}")
# 应用用户配置覆盖
self._apply_user_overrides()
logger.info("✅ 配置加载成功")
logger.info(f"🎯 当前平台: {self.platform}")
logger.info(f"🌐 语言设置: {self.language}")
</code_context>
<issue_to_address>
**issue (code-quality):** Explicitly raise from a previous error [×4] ([`raise-from-previous-error`](https://docs.sourcery.ai/Reference/Default-Rules/suggestions/raise-from-previous-error/))
</issue_to_address>
### Comment 9
<location> `meshbot/core/message_processor.py:356-364` </location>
<code_context>
def update_broadcast_settings(self, enabled: bool = False, keywords: List[str] = [""]):
"""更新群发消息设置"""
if enabled is not None:
self.broadcast_enabled = enabled
status = i18n.gettext('enabled') if enabled else i18n.gettext('disabled')
logger.info(i18n.gettext('broadcast_settings_updated', status=status))
if keywords is not None:
self.broadcast_keywords = keywords
logger.info(i18n.gettext('keywords_updated', keywords=keywords))
</code_context>
<issue_to_address>
**issue (code-quality):** Replace mutable default arguments with None ([`default-mutable-arg`](https://docs.sourcery.ai/Reference/Default-Rules/suggestions/default-mutable-arg/))
</issue_to_address>
### Comment 10
<location> `meshbot/utils/ai_client_factory.py:49-54` </location>
<code_context>
def create_ai_client(platform: str = ""):
"""
创建指定平台的 AI 客户端,失败时回退到 Ollama。
如果不指定 platform,则使用配置中的默认平台。
"""
# 获取配置
ai_client_config = get_ai_client_config()
default_platform = get_platform()
# 如果没有指定平台,使用默认平台
if platform is None:
platform = default_platform
# 获取配置,优先使用传入的 platform,否则使用默认 PLATFORM
config = ai_client_config.get(platform) or ai_client_config.get(default_platform)
if not config:
logger.error(i18n.gettext('platform_not_found', platform = platform, default_platform = default_platform))
# 回退到内置 Ollama 配置
logger.info(i18n.gettext('back_to_ollama'))
from api.ollama_api import AsyncOllamaChatClient
return AsyncOllamaChatClient(default_model="qwen2.5:7b")
try:
# 动态导入模块和类
module = importlib.import_module(config["module"])
client_class = getattr(module, config["class"])
# 复制 kwargs,避免污染原始配置
kwargs = config["kwargs"].copy()
# 创建实例
logger.info(i18n.gettext('ai_client_created', platform = platform))
return client_class(**kwargs)
except (ImportError, AttributeError, KeyError) as e:
logger.error(
i18n.gettext('ai_client_creation_failed', platform = platform, error_type = type(e).__name__, error_msg = e)
)
try:
from api.ollama_api import AsyncOllamaChatClient
return AsyncOllamaChatClient(default_model="qwen2.5:7b")
except ImportError:
logger.critical(i18n.gettext('fallback_failed'))
raise RuntimeError(i18n.gettext('ai_client_init_failed'))
</code_context>
<issue_to_address>
**issue (code-quality):** Explicitly raise from a previous error ([`raise-from-previous-error`](https://docs.sourcery.ai/Reference/Default-Rules/suggestions/raise-from-previous-error/))
</issue_to_address>
### Comment 11
<location> `meshbot/utils/localize.py:11-14` </location>
<code_context>
def gettext(self, key: str, **kwargs) -> str:
"""获取本地化消息,支持格式化参数"""
if self.language:
pass
else:
self.language = _config_manager.language
message_template = self.messages.get(key, key)
# 如果有参数,进行格式化
if kwargs:
try:
return message_template.format(**kwargs)
except KeyError as e:
return f"[Format error in '{key}': missing {e}]"
return message_template
</code_context>
<issue_to_address>
**issue (code-quality):** Swap if/else to remove empty if body ([`remove-pass-body`](https://docs.sourcery.ai/Reference/Default-Rules/refactorings/remove-pass-body/))
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| def _apply_user_overrides(self) -> None: | ||
| """应用用户特定的配置覆盖""" | ||
| if not self._user_config or not self._config: | ||
| return | ||
|
|
||
| # 应用 API keys | ||
| if "api_keys" in self._user_config: | ||
| for platform, api_key in self._user_config["api_keys"].items(): | ||
| if (platform in self._config.clients and | ||
| api_key not in ["your-api-key", "your-openai-api-key", ""]): |
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 (bug_risk): API 密钥覆盖逻辑可能不会更新密钥,如果默认值不是 'your-...'。
目前,覆盖仅在默认密钥以 'your-' 开头时才适用。为避免默认值更改或用户需要覆盖有效密钥时出现问题,请更新逻辑以始终应用用户提供的密钥。
Original comment in English
issue (bug_risk): API key override logic may not update keys if the default is not 'your-...'.
Currently, the override only applies when the default key starts with 'your-'. To avoid issues if the default changes or users need to override a valid key, update the logic to always apply the user-provided key.
| class I18N: | ||
| def __init__(self): | ||
| self.language = "" |
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 (bug_risk): I18N 中的语言初始化可能导致回退问题。
由于语言在初始化期间只设置一次,因此配置管理器语言的更改将不会反映。为避免不一致的本地化,请每次从配置管理器获取语言或允许动态更新。
Original comment in English
issue (bug_risk): Language initialization in I18N may cause fallback issues.
Since the language is set only once during initialization, changes to the config manager's language won't be reflected. To avoid inconsistent localization, fetch the language from the config manager each time or allow dynamic updates.
| def gettext(self, key: str, **kwargs) -> str: | ||
| """获取本地化消息,支持格式化参数""" | ||
| if self.language: | ||
| pass | ||
| else: | ||
| self.language = _config_manager.language | ||
|
|
||
| message_template = self.messages.get(key, key) | ||
|
|
||
| # 如果有参数,进行格式化 |
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 (security): 在面向用户的字符串中返回格式错误可能会泄露内部细节。
记录格式错误并返回通用消息将防止向用户暴露内部细节并改善用户体验。
建议实现:
import logging
from meshbot.config.config_loader import _config_manager
from meshbot.localizations.localization import MESSAGES
logger = logging.getLogger(__name__)
class I18N: # 如果有参数,进行格式化
if kwargs:
try:
return message_template.format(**kwargs)
except KeyError as e:
logger.error("Format error in '%s': missing %s", key, e)
return "Message formatting error"
return message_templateOriginal comment in English
🚨 suggestion (security): Returning format errors in user-facing strings may leak internal details.
Logging the format error and returning a generic message will prevent exposing internal details to users and improve user experience.
Suggested implementation:
import logging
from meshbot.config.config_loader import _config_manager
from meshbot.localizations.localization import MESSAGES
logger = logging.getLogger(__name__)
class I18N: # 如果有参数,进行格式化
if kwargs:
try:
return message_template.format(**kwargs)
except KeyError as e:
logger.error("Format error in '%s': missing %s", key, e)
return "Message formatting error"
return message_template| message_type = i18n.gettext('broadcast_message_received') if is_broadcast else i18n.gettext('private_message_received') | ||
|
|
||
| logger.info( | ||
| f"{message_type} 来自 {from_id}{name_info}: {short_text}" | ||
| message_type.format(from_id=from_id, name_info=name_info, short_text=short_text) |
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 (bug_risk): 对本地化字符串使用格式化可能会在占位符缺失时导致运行时错误。
在格式化之前验证本地化字符串是否包含所有必需的占位符,或者使用能够优雅处理缺失占位符的格式化方法。
Original comment in English
issue (bug_risk): Using format on a localized string may cause runtime errors if placeholders are missing.
Validate that the localized string includes all required placeholders before formatting, or use a formatting approach that handles missing placeholders gracefully.
| if (platform in self._config.clients and | ||
| api_key not in ["your-api-key", "your-openai-api-key", ""]): | ||
| if self._config.clients[platform].kwargs.get("api_key", "").startswith("your-"): | ||
| self._config.clients[platform].kwargs["api_key"] = api_key |
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 (code-quality): 合并嵌套的 if 条件 (merge-nested-ifs)
| if (platform in self._config.clients and | |
| api_key not in ["your-api-key", "your-openai-api-key", ""]): | |
| if self._config.clients[platform].kwargs.get("api_key", "").startswith("your-"): | |
| self._config.clients[platform].kwargs["api_key"] = api_key | |
| if (platform in self._config.clients and | |
| api_key not in ["your-api-key", "your-openai-api-key", ""]) and self._config.clients[platform].kwargs.get("api_key", "").startswith("your-"): | |
| self._config.clients[platform].kwargs["api_key"] = api_key | |
Explanation
过多的嵌套会使代码难以理解,在 Python 中尤其如此,因为没有括号来帮助区分不同的嵌套级别。阅读深度嵌套的代码令人困惑,因为您必须跟踪哪些条件与哪些级别相关。因此,我们努力在可能的情况下减少嵌套,而两个 if 条件可以使用 and 组合的情况是一个简单的胜利。
Original comment in English
suggestion (code-quality): Merge nested if conditions (merge-nested-ifs)
| if (platform in self._config.clients and | |
| api_key not in ["your-api-key", "your-openai-api-key", ""]): | |
| if self._config.clients[platform].kwargs.get("api_key", "").startswith("your-"): | |
| self._config.clients[platform].kwargs["api_key"] = api_key | |
| if (platform in self._config.clients and | |
| api_key not in ["your-api-key", "your-openai-api-key", ""]) and self._config.clients[platform].kwargs.get("api_key", "").startswith("your-"): | |
| self._config.clients[platform].kwargs["api_key"] = api_key | |
Explanation
Too much nesting can make code difficult to understand, and this is especiallytrue in Python, where there are no brackets to help out with the delineation of
different nesting levels.
Reading deeply nested code is confusing, since you have to keep track of which
conditions relate to which levels. We therefore strive to reduce nesting where
possible, and the situation where two if conditions can be combined using
and is an easy win.
| if item == "class": | ||
| return getattr(self, "class_name") | ||
| return getattr(self, item) |
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 (code-quality): 我们发现了这些问题:
- 在控制流跳转后将代码提升到 else (
reintroduce-else) - 将 if 语句替换为 if 表达式 (
assign-if-exp)
| if item == "class": | |
| return getattr(self, "class_name") | |
| return getattr(self, item) | |
| return getattr(self, "class_name") if item == "class" else getattr(self, item) |
Original comment in English
suggestion (code-quality): We've found these issues:
- Lift code into else after jump in control flow (
reintroduce-else) - Replace if statement with if expression (
assign-if-exp)
| if item == "class": | |
| return getattr(self, "class_name") | |
| return getattr(self, item) | |
| return getattr(self, "class_name") if item == "class" else getattr(self, item) |
|
|
||
| self._config_path = Path(config_path) | ||
|
|
||
| try: |
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 (code-quality): 明确地从先前的错误中引发异常 [×4] (raise-from-previous-error)
Original comment in English
issue (code-quality): Explicitly raise from a previous error [×4] (raise-from-previous-error)
| """更新群发消息设置""" | ||
| if enabled is not None: | ||
| self.broadcast_enabled = enabled | ||
| logger.info(f"🔄 群发消息处理: {'启用' if enabled else '禁用'}") | ||
| status = i18n.gettext('enabled') if enabled else i18n.gettext('disabled') | ||
| logger.info(i18n.gettext('broadcast_settings_updated', status=status)) | ||
|
|
||
| if keywords is not None: | ||
| self.broadcast_keywords = keywords | ||
| logger.info(f"🔄 更新群发触发关键词: {keywords}") | ||
| logger.info(i18n.gettext('keywords_updated', keywords=keywords)) |
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 (code-quality): 将可变默认参数替换为 None (default-mutable-arg)
Original comment in English
issue (code-quality): Replace mutable default arguments with None (default-mutable-arg)
| try: | ||
| from api.ollama_api import AsyncOllamaChatClient | ||
| return AsyncOllamaChatClient(default_model="qwen2.5:7b") | ||
| except ImportError: | ||
| logger.critical("回退失败:无法导入 AsyncOllamaChatClient") | ||
| raise RuntimeError("AI 客户端初始化失败,且无法回退到 Ollama") | ||
| logger.critical(i18n.gettext('fallback_failed')) | ||
| raise RuntimeError(i18n.gettext('ai_client_init_failed')) |
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 (code-quality): 明确地从先前的错误中引发异常 (raise-from-previous-error)
Original comment in English
issue (code-quality): Explicitly raise from a previous error (raise-from-previous-error)
| if self.language: | ||
| pass | ||
| else: | ||
| self.language = _config_manager.language |
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 (code-quality): 交换 if/else 以移除空的 if 主体 (remove-pass-body)
Original comment in English
issue (code-quality): Swap if/else to remove empty if body (remove-pass-body)
|
@sourcery-ai issue |
|
Thank you for your request! I will create a GitHub issue based on the review guide provided above. The issue will summarize the major changes, diagrams, and file-level details for easier tracking and follow-up. If you have a specific aspect you'd like highlighted or a particular question/concern to be addressed in the issue, please let me know so I can tailor the content accordingly. |
Sourcery 总结
将配置加载重构为由 Pydantic 支持的单例管理器,在代码和文档中引入本地化支持,并用国际化模板替换硬编码的日志消息。
新特性:
改进:
文档:
Original summary in English
Summary by Sourcery
Refactor configuration loading into a Pydantic-backed singleton manager, introduce localization support across both code and docs, and replace hardcoded log messages with internationalized templates.
New Features:
Enhancements:
Documentation: