fix(core): route image requests to vision fallback#8089
fix(core): route image requests to vision fallback#8089he-yufeng wants to merge 3 commits intoAstrBotDevs:masterfrom
Conversation
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- In
_select_image_chat_provider, mutatingreq.modelas a side effect of choosing a fallback provider can be surprising for callers; consider either returning a new/updatedProviderRequestor making the mutation explicit at the call site to keep the helper side-effect-free. - The
_provider_supports_modalityhelper treats a missing or non-listmodalitiesentry as supporting all modalities; if a misconfigured provider shouldn’t be assumed to support images, consider tightening this toisinstance(modalities, list) and modality in modalities(and treating other cases as unsupported).
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `_select_image_chat_provider`, mutating `req.model` as a side effect of choosing a fallback provider can be surprising for callers; consider either returning a new/updated `ProviderRequest` or making the mutation explicit at the call site to keep the helper side-effect-free.
- The `_provider_supports_modality` helper treats a missing or non-list `modalities` entry as supporting all modalities; if a misconfigured provider shouldn’t be assumed to support images, consider tightening this to `isinstance(modalities, list) and modality in modalities` (and treating other cases as unsupported).Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
There was a problem hiding this comment.
Code Review
This pull request introduces a mechanism to automatically switch to an image-capable fallback provider when the primary provider does not support image inputs. It adds helper functions to check provider modalities and select appropriate fallbacks, along with corresponding unit tests. The reviewer suggested filtering the newly selected primary provider from the fallback list to avoid redundant retry attempts in the AgentRunner.
|
The Windows Python 3.12 smoke job timed out waiting for the health endpoint, while the rest of the smoke matrix and the unit/format/build checks completed successfully. I cannot rerun that upstream job from this fork (
It passed locally ( |
|
Addressed the remaining Sourcery point in 282f3b0: provider modality support now requires an explicit list containing the target modality, so missing or malformed modalities are not treated as image-capable. The other Sourcery point was already handled in the current branch: _select_image_chat_provider only returns the provider, and the request model reset is explicit at the call site. Validation:
|
Summary
Fixes #8087.
To verify
.venv\Scripts\python.exe -m pytest tests\unit\test_astr_main_agent.py::TestBuildMainAgent::test_build_main_agent_with_images tests\unit\test_astr_main_agent.py::TestBuildMainAgent::test_build_main_agent_uses_image_fallback_provider tests\unit\test_astr_main_agent.py::TestBuildMainAgent::test_build_main_agent_keeps_text_provider_without_image_fallback tests\test_tool_loop_agent_runner.py::test_runner_builds_placeholder_for_unsupported_request_image -q.venv\Scripts\python.exe -m ruff check astrbot\core\astr_main_agent.py tests\unit\test_astr_main_agent.py.venv\Scripts\python.exe -m ruff format astrbot\core\astr_main_agent.py tests\unit\test_astr_main_agent.py --check.venv\Scripts\python.exe -m py_compile astrbot\core\astr_main_agent.py tests\unit\test_astr_main_agent.pygit diff --checkSummary by Sourcery
Ensure image-based chat requests are handled by an image-capable provider when available, while preserving existing behavior when no such fallback exists.
Bug Fixes:
Tests: