-
-
Notifications
You must be signed in to change notification settings - Fork 1k
增加 provider 命令的提供商可用性显示 #3708
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
base: master
Are you sure you want to change the base?
增加 provider 命令的提供商可用性显示 #3708
Conversation
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.
你好 - 我已经审阅了你的更改,它们看起来很棒!
帮助我更有用!请在每条评论上点击 👍 或 👎,我将利用这些反馈来改进你的评论。
Original comment in English
Hey there - I've reviewed your changes and they look great!
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
使用dashboard实现替换提供商检测实现 |
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.
Pull request overview
This PR enhances the /provider command to display real-time provider reachability status alongside the provider list. When users run /provider, the command now concurrently tests each provider's connectivity and displays ✅ or ❌ indicators to show availability. A new configuration option provider_settings.reachability_check allows users to disable this feature if needed.
Key Changes:
- Added
_test_provider_capability()method that tests different provider types (chat, embedding, TTS, STT, rerank) with appropriate health checks - Modified the provider listing logic to gather all providers (LLM, TTS, STT), test them concurrently, and display reachability indicators
- Added
reachability_checkconfiguration option (default:True) in provider settings
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 9 comments.
| File | Description |
|---|---|
| packages/astrbot/commands/provider.py | Implemented provider capability testing with concurrent health checks and visual status indicators |
| astrbot/core/config/default.py | Added reachability_check configuration option to control provider connectivity testing |
| ret += "\n使用 /provider tts <序号> 切换 TTS 提供商。" | ||
| if stt_providers: | ||
| if stts: | ||
| ret += "\n使用 /provider stt <切换> STT 提供商。" |
Copilot
AI
Nov 22, 2025
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.
Typo in the help text: "切换" should be "序号" to match the pattern used for TTS. The current text reads "/provider stt <切换> STT 提供商" but should be "/provider stt <序号> 切换 STT 提供商" to be consistent with the TTS command format above it.
| ret += "\n使用 /provider stt <切换> STT 提供商。" | |
| ret += "\n使用 /provider stt <序号> 切换 STT 提供商。" |
| # 发送 "Ping" 测试对话 | ||
| response = await asyncio.wait_for( | ||
| provider.text_chat(prompt="REPLY `PONG` ONLY"), | ||
| timeout=10.0, # CLI 场景下稍微缩短一点超时时间,避免等待太久 |
Copilot
AI
Nov 22, 2025
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.
The timeout for CHAT_COMPLETION provider check (10 seconds) is significantly longer than the timeout for the fallback get_models() check (4 seconds) on line 74. This inconsistency could lead to long wait times when checking chat providers. Consider using a consistent timeout across all provider types, or documenting why chat providers need 2.5x longer timeout. For a command-line experience, 4-6 seconds might be more appropriate for all checks.
| timeout=10.0, # CLI 场景下稍微缩短一点超时时间,避免等待太久 | |
| timeout=4.0, # 与 get_models 保持一致,避免等待太久 |
| # 如果样本文件不存在,降级为检查是否实现了方法 | ||
| return hasattr(provider, "get_text") |
Copilot
AI
Nov 22, 2025
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.
For the STT fallback check, using hasattr(provider, "get_text") only verifies the method exists but doesn't actually test if the provider is reachable or functional. This could result in showing ✅ for providers that have the method but cannot connect to their backend service. Consider either:
- Returning
Noneinstead ofTrueto indicate "untested" status (no checkmark) - Documenting that this is a known limitation when the sample file is missing
| # 如果样本文件不存在,降级为检查是否实现了方法 | |
| return hasattr(provider, "get_text") | |
| # 如果样本文件不存在,返回 None 表示未测试 | |
| return None |
| embedding_result = await provider.get_embedding("health_check") | ||
| if isinstance(embedding_result, list) and ( | ||
| not embedding_result or isinstance(embedding_result[0], float) | ||
| ): | ||
| return True | ||
| return False | ||
|
|
||
| elif provider_capability_type == ProviderType.TEXT_TO_SPEECH: | ||
| # 测试 TTS | ||
| audio_result = await provider.get_audio("你好") | ||
| if isinstance(audio_result, str) and audio_result: | ||
| return True | ||
| return False | ||
|
|
||
| elif provider_capability_type == ProviderType.SPEECH_TO_TEXT: | ||
| # 测试 STT | ||
| sample_audio_path = os.path.join( | ||
| get_astrbot_path(), | ||
| "samples", | ||
| "stt_health_check.wav", | ||
| ) | ||
| if not os.path.exists(sample_audio_path): | ||
| # 如果样本文件不存在,降级为检查是否实现了方法 | ||
| return hasattr(provider, "get_text") | ||
|
|
||
| text_result = await provider.get_text(sample_audio_path) |
Copilot
AI
Nov 22, 2025
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.
Missing timeout wrapper for embedding, TTS, and STT provider checks. Unlike CHAT_COMPLETION (line 24-26) and the fallback get_models (line 74), these checks don't have timeout protection. If any of these provider methods hang, they could significantly delay the command response.
Add timeouts for:
- Line 34:
await asyncio.wait_for(provider.get_embedding("health_check"), timeout=4.0) - Line 43:
await asyncio.wait_for(provider.get_audio("你好"), timeout=4.0) - Line 59:
await asyncio.wait_for(provider.get_text(sample_audio_path), timeout=4.0)
| response = await asyncio.wait_for( | ||
| provider.text_chat(prompt="REPLY `PONG` ONLY"), | ||
| timeout=10.0, # CLI 场景下稍微缩短一点超时时间,避免等待太久 | ||
| ) |
Copilot
AI
Nov 22, 2025
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.
The CHAT_COMPLETION health check sends an actual prompt ("REPLY PONG ONLY") to the LLM provider, which may incur API costs for paid services. For users with multiple providers, running /provider could trigger several billable API calls. Consider:
- Using a lighter-weight check method if available (e.g., validating credentials, checking model list)
- Adding a warning in the configuration hint that this feature makes actual API calls
- Caching results for a short period to avoid repeated costs
Similarly, TTS check on line 43 generates actual audio which may also incur costs.
| elif provider_capability_type == ProviderType.TEXT_TO_SPEECH: | ||
| # 测试 TTS | ||
| audio_result = await provider.get_audio("你好") | ||
| if isinstance(audio_result, str) and audio_result: |
Copilot
AI
Nov 22, 2025
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.
The TTS health check generates an audio file (returned as a file path string) but doesn't clean up the generated file. If users frequently run /provider, this could accumulate temporary audio files. Consider adding cleanup logic after the check:
audio_result = await provider.get_audio("你好")
if isinstance(audio_result, str) and audio_result:
# Clean up the temporary file
if os.path.exists(audio_result):
try:
os.remove(audio_result)
except Exception:
pass # Ignore cleanup errors
return True| if isinstance(audio_result, str) and audio_result: | |
| if isinstance(audio_result, str) and audio_result: | |
| # Clean up the temporary audio file | |
| if os.path.exists(audio_result): | |
| try: | |
| os.remove(audio_result) | |
| except Exception: | |
| pass # Ignore cleanup errors |
| *[self._test_provider_capability(p) for p, _ in all_providers] | ||
| ) |
Copilot
AI
Nov 22, 2025
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.
Using asyncio.gather without return_exceptions=True could cause the entire provider list operation to fail if any single provider check raises an unhandled exception. While individual checks have try-except blocks, it's safer to add return_exceptions=True to handle any unexpected edge cases gracefully and ensure all providers are still displayed even if some checks fail unexpectedly.
Recommended fix:
check_results = await asyncio.gather(
*[self._test_provider_capability(p) for p, _ in all_providers],
return_exceptions=True
)Then handle exceptions in the results:
check_results = [False if isinstance(r, Exception) else r for r in check_results]| *[self._test_provider_capability(p) for p, _ in all_providers] | |
| ) | |
| *[self._test_provider_capability(p) for p, _ in all_providers], | |
| return_exceptions=True | |
| ) | |
| # 将异常转换为 False,表示不可达 | |
| check_results = [False if isinstance(r, Exception) else r for r in check_results] |
| if isinstance(embedding_result, list) and ( | ||
| not embedding_result or isinstance(embedding_result[0], float) |
Copilot
AI
Nov 22, 2025
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.
The embedding validation logic has a potential bug when checking empty lists. The condition not embedding_result or isinstance(embedding_result[0], float) will return True for an empty list (due to not embedding_result), but then the comment suggests it expects non-empty float lists. If an empty embedding is a valid result, this is fine, but if not, this check incorrectly validates empty lists. Consider clarifying the intended behavior or using embedding_result and all(isinstance(x, float) for x in embedding_result) if non-empty is required.
| if isinstance(embedding_result, list) and ( | |
| not embedding_result or isinstance(embedding_result[0], float) | |
| if ( | |
| isinstance(embedding_result, list) | |
| and embedding_result | |
| and all(isinstance(x, float) for x in embedding_result) |
| elif provider_capability_type == ProviderType.RERANK: | ||
| # 测试 Rerank | ||
| if isinstance(provider, RerankProvider): | ||
| await provider.rerank("Apple", documents=["apple", "banana"]) |
Copilot
AI
Nov 22, 2025
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.
The RERANK provider test doesn't have a timeout wrapper like other provider types. If the rerank() call hangs or takes too long, it could significantly delay the entire provider listing command. Consider wrapping it with asyncio.wait_for() with a reasonable timeout (e.g., 4-5 seconds) consistent with other checks.
Suggested fix:
await asyncio.wait_for(
provider.rerank("Apple", documents=["apple", "banana"]),
timeout=4.0
)| await provider.rerank("Apple", documents=["apple", "banana"]) | |
| await asyncio.wait_for( | |
| provider.rerank("Apple", documents=["apple", "banana"]), | |
| timeout=4.0 | |
| ) |


增加 provider 命令的提供商可用性显示
Modifications / 改动点
provider指令不仅仅显示提供商,同时显示提供商可用性
provider.py
Screenshots or Test Results / 运行截图或测试结果
Checklist / 检查清单
requirements.txt和pyproject.toml文件相应位置。/ I have ensured that no new dependencies are introduced, OR if new dependencies are introduced, they have been added to the appropriate locations inrequirements.txtandpyproject.toml.Sourcery 总结
增强 provider 命令,使其在列出每个 provider 的同时,检查并显示其可达性状态
增强功能:
Original summary in English
Summary by Sourcery
Enhance the provider command to check and display each provider's reachability status alongside its listing
Enhancements: