-
-
Notifications
You must be signed in to change notification settings - Fork 2.7k
fix: add fallback providers to handoff subagents #9332
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?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -365,6 +365,76 @@ async def _fake_tool_loop_agent(**kwargs): | |||||
| assert captured["tool_call_timeout"] == 120 | ||||||
|
|
||||||
|
|
||||||
| @pytest.mark.asyncio | ||||||
| async def test_execute_handoff_passes_valid_fallbacks_and_retries_to_tool_loop_agent( | ||||||
| monkeypatch: pytest.MonkeyPatch, | ||||||
| ): | ||||||
| captured: dict = {} | ||||||
|
|
||||||
| class _ChatProvider: | ||||||
| pass | ||||||
|
|
||||||
| primary = _ChatProvider() | ||||||
| fallback = _ChatProvider() | ||||||
| non_chat_provider = object() | ||||||
| providers = { | ||||||
| "primary": primary, | ||||||
| "fallback": fallback, | ||||||
| "non-chat": non_chat_provider, | ||||||
| } | ||||||
|
|
||||||
| async def _fake_get_current_chat_provider_id(_umo): | ||||||
| return "primary" | ||||||
|
|
||||||
| async def _fake_tool_loop_agent(**kwargs): | ||||||
| captured.update(kwargs) | ||||||
| return SimpleNamespace(completion_text="ok") | ||||||
|
|
||||||
| context = SimpleNamespace( | ||||||
| get_current_chat_provider_id=_fake_get_current_chat_provider_id, | ||||||
| get_provider_by_id=lambda provider_id: providers.get(provider_id), | ||||||
| tool_loop_agent=_fake_tool_loop_agent, | ||||||
| get_config=lambda **_kwargs: { | ||||||
| "provider_settings": { | ||||||
| "fallback_chat_models": [ | ||||||
| "primary", | ||||||
| "fallback", | ||||||
| "fallback", | ||||||
| "missing", | ||||||
| "non-chat", | ||||||
| ], | ||||||
| "request_max_retries": 7, | ||||||
| } | ||||||
| }, | ||||||
| ) | ||||||
| event = _DummyEvent([]) | ||||||
| run_context = ContextWrapper(context=SimpleNamespace(event=event, context=context)) | ||||||
| tool = SimpleNamespace( | ||||||
| name="transfer_to_subagent", | ||||||
| provider_id="primary", | ||||||
| agent=SimpleNamespace( | ||||||
| name="subagent", | ||||||
| tools=[], | ||||||
| instructions="subagent-instructions", | ||||||
| begin_dialogs=[], | ||||||
| run_hooks=None, | ||||||
| ), | ||||||
| ) | ||||||
| monkeypatch.setattr("astrbot.core.astr_agent_tool_exec.Provider", _ChatProvider) | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Since we are refactoring the fallback provider resolution to use
Suggested change
|
||||||
|
|
||||||
| async for _result in FunctionToolExecutor._execute_handoff( | ||||||
| tool, | ||||||
| run_context, | ||||||
| image_urls_prepared=True, | ||||||
| input="hello", | ||||||
| image_urls=[], | ||||||
| ): | ||||||
| pass | ||||||
|
|
||||||
| assert captured["fallback_providers"] == [fallback] | ||||||
| assert captured["request_max_retries"] == 7 | ||||||
|
|
||||||
|
|
||||||
| @pytest.mark.asyncio | ||||||
| async def test_background_wakeup_passes_provider_settings_to_main_agent( | ||||||
| monkeypatch: pytest.MonkeyPatch, | ||||||
|
|
||||||
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.
This block of code is almost identical to the
_get_fallback_chat_providershelper function defined inastrbot/core/astr_main_agent.py. To avoid code duplication and adhere to the general rules, we should reuse that helper function.We can import
_get_fallback_chat_providersinside_execute_handoffto avoid circular dependencies, and pass the resolvedProviderobject directly.References
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.
Thanks for the suggestion. I am keeping the resolution inline: importing a private helper from
astr_main_agentwould couple the tool executor to the main-agent module, which already importsFunctionToolExecutor, creating a circular import risk. This is currently the second use site, so extracting a shared provider utility would add broader scope without improving the handoff fix.