Skip to content

Conversation

@zouyonghe
Copy link
Collaborator

@zouyonghe zouyonghe commented Nov 20, 2025

增加 provider 命令的提供商可用性显示

Modifications / 改动点

provider指令不仅仅显示提供商,同时显示提供商可用性

provider.py

  • This is NOT a breaking change. / 这不是一个破坏性变更。

Screenshots or Test Results / 运行截图或测试结果

image

Checklist / 检查清单

  • 😊 如果 PR 中有新加入的功能,已经通过 Issue / 邮件等方式和作者讨论过。/ If there are new features added in the PR, I have discussed it with the authors through issues/emails, etc.
  • 👀 我的更改经过了良好的测试,并已在上方提供了“验证步骤”和“运行截图”。/ My changes have been well-tested, and "Verification Steps" and "Screenshots" have been provided above.
  • 🤓 我确保没有引入新依赖库,或者引入了新依赖库的同时将其添加到了 requirements.txtpyproject.toml 文件相应位置。/ I have ensured that no new dependencies are introduced, OR if new dependencies are introduced, they have been added to the appropriate locations in requirements.txt and pyproject.toml.
  • 😮 我的更改没有引入恶意代码。/ My changes do not introduce malicious code.

Sourcery 总结

增强 provider 命令,使其在列出每个 provider 的同时,检查并显示其可达性状态

增强功能:

  • 并发测试每个 provider 的连接性,通过获取模型列表并设置 4 秒超时
  • 在每个 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:

  • Concurrently test each provider's connectivity by fetching model lists with a 4s timeout
  • Display a ✅ or ❌ indicator next to each provider to show its availability

Copy link
Contributor

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

你好 - 我已经审阅了你的更改,它们看起来很棒!


Sourcery 对开源项目免费 - 如果你喜欢我们的评论,请考虑分享它们 ✨
帮助我更有用!请在每条评论上点击 👍 或 👎,我将利用这些反馈来改进你的评论。
Original comment in English

Hey there - I've reviewed your changes and they look great!


Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

@AstrBotDevs AstrBotDevs locked and limited conversation to collaborators Nov 20, 2025
@AstrBotDevs AstrBotDevs unlocked this conversation Nov 20, 2025
@zouyonghe zouyonghe closed this Nov 20, 2025
@zouyonghe
Copy link
Collaborator Author

使用dashboard实现替换提供商检测实现

@zouyonghe zouyonghe reopened this Nov 20, 2025
@zouyonghe
Copy link
Collaborator Author

截图如图
image

image

@zouyonghe zouyonghe requested review from RC-CHN and Copilot November 22, 2025 15:20
Copilot finished reviewing on behalf of zouyonghe November 22, 2025 15:22
Copy link
Contributor

Copilot AI left a 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_check configuration 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 提供商。"
Copy link

Copilot AI Nov 22, 2025

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.

Suggested change
ret += "\n使用 /provider stt <切换> STT 提供商。"
ret += "\n使用 /provider stt <序号> 切换 STT 提供商。"

Copilot uses AI. Check for mistakes.
# 发送 "Ping" 测试对话
response = await asyncio.wait_for(
provider.text_chat(prompt="REPLY `PONG` ONLY"),
timeout=10.0, # CLI 场景下稍微缩短一点超时时间,避免等待太久
Copy link

Copilot AI Nov 22, 2025

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.

Suggested change
timeout=10.0, # CLI 场景下稍微缩短一点超时时间,避免等待太久
timeout=4.0, # 与 get_models 保持一致,避免等待太久

Copilot uses AI. Check for mistakes.
Comment on lines +56 to +57
# 如果样本文件不存在,降级为检查是否实现了方法
return hasattr(provider, "get_text")
Copy link

Copilot AI Nov 22, 2025

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:

  1. Returning None instead of True to indicate "untested" status (no checkmark)
  2. Documenting that this is a known limitation when the sample file is missing
Suggested change
# 如果样本文件不存在,降级为检查是否实现了方法
return hasattr(provider, "get_text")
# 如果样本文件不存在,返回 None 表示未测试
return None

Copilot uses AI. Check for mistakes.
Comment on lines +34 to +59
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)
Copy link

Copilot AI Nov 22, 2025

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)

Copilot uses AI. Check for mistakes.
Comment on lines +24 to +27
response = await asyncio.wait_for(
provider.text_chat(prompt="REPLY `PONG` ONLY"),
timeout=10.0, # CLI 场景下稍微缩短一点超时时间,避免等待太久
)
Copy link

Copilot AI Nov 22, 2025

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:

  1. Using a lighter-weight check method if available (e.g., validating credentials, checking model list)
  2. Adding a warning in the configuration hint that this feature makes actual API calls
  3. Caching results for a short period to avoid repeated costs

Similarly, TTS check on line 43 generates actual audio which may also incur costs.

Copilot uses AI. Check for mistakes.
elif provider_capability_type == ProviderType.TEXT_TO_SPEECH:
# 测试 TTS
audio_result = await provider.get_audio("你好")
if isinstance(audio_result, str) and audio_result:
Copy link

Copilot AI Nov 22, 2025

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
Suggested change
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

Copilot uses AI. Check for mistakes.
Comment on lines +109 to +110
*[self._test_provider_capability(p) for p, _ in all_providers]
)
Copy link

Copilot AI Nov 22, 2025

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]
Suggested change
*[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]

Copilot uses AI. Check for mistakes.
Comment on lines +35 to +36
if isinstance(embedding_result, list) and (
not embedding_result or isinstance(embedding_result[0], float)
Copy link

Copilot AI Nov 22, 2025

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.

Suggested change
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)

Copilot uses AI. Check for mistakes.
elif provider_capability_type == ProviderType.RERANK:
# 测试 Rerank
if isinstance(provider, RerankProvider):
await provider.rerank("Apple", documents=["apple", "banana"])
Copy link

Copilot AI Nov 22, 2025

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
)
Suggested change
await provider.rerank("Apple", documents=["apple", "banana"])
await asyncio.wait_for(
provider.rerank("Apple", documents=["apple", "banana"]),
timeout=4.0
)

Copilot uses AI. Check for mistakes.
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.

1 participant