feat(sdk): gate switch llm default tool#3190
Conversation
all-hands-bot
left a comment
all-hands-bot
left a comment
There was a problem hiding this comment.
Taste Rating: 🟢 Good taste - Clean conditional gating that solves a real UX problem.
[RISK ASSESSMENT]
- [Overall PR]
⚠️ Risk Assessment: 🟢 LOW
Simple setting that conditionally includes SwitchLLMTool based on (1) the new enable_switch_llm_tool flag and (2) presence of saved profiles. Defaults to true for backward compatibility. Well-tested with comprehensive coverage of all scenarios. No agent behavior or eval impact.
VERDICT:
✅ Worth merging: Pragmatic UX improvement with clean implementation.
KEY INSIGHT:
Eliminating the switch tool when no profiles exist removes a special case from the user's perspective - the tool only appears when it's actually usable.
Coverage Report •
|
||||||||||||||||||||||||||||||
all-hands-bot
left a comment
all-hands-bot
left a comment
There was a problem hiding this comment.
✅ QA Report: PASS
Verified that the SwitchLLMTool gating feature works correctly across all scenarios.
Does this PR achieve its stated goal?
Yes. The PR successfully gates the SwitchLLMTool behind the new enable_switch_llm_tool setting (defaulting to true) and automatically excludes it when no saved LLM profiles exist. All four behavioral scenarios were verified through actual execution: (1) the setting exists with the correct default, (2) the tool is included when enabled with profiles, (3) the tool is excluded when disabled, and (4) the tool is excluded when no profiles exist even if the setting is enabled. The schema export also correctly includes the new field with appropriate metadata.
| Phase | Result |
|---|---|
| Environment Setup | ✅ Clean build with make build |
| CI Status | ✅ All checks passing (13/13 pass, qa-changes pending) |
| Functional Verification | ✅ All 5 test scenarios passed |
Functional Verification
Test 1: Setting exists with correct default
Step 1 — Verify the new setting:
Created an OpenHandsAgentSettings instance and checked for the enable_switch_llm_tool field:
settings = OpenHandsAgentSettings(llm=_make_llm("test-model", "test"))
assert hasattr(settings, 'enable_switch_llm_tool')
assert settings.enable_switch_llm_tool is TrueResult:
✅ PASS: Setting exists and defaults to True
This confirms the new setting is properly defined and has the expected default value.
Test 2: SwitchLLMTool included when enabled with profiles
Step 1 — Create a temporary profile directory with profiles:
store = LLMProfileStore(base_dir=profile_dir)
store.save("fast", _make_llm("fast-model", "fast"))
store.save("slow", _make_llm("slow-model", "slow"))Step 2 — Create settings with enable_switch_llm_tool=True:
settings = OpenHandsAgentSettings(
llm=_make_llm("default-model", "default"),
enable_switch_llm_tool=True
)
agent = settings.create_agent()Step 3 — Verify SwitchLLMTool is included:
assert "SwitchLLMTool" in agent.include_default_tools
conversation = LocalConversation(agent=agent, workspace=Path.cwd())
conversation._ensure_agent_ready()
assert "switch_llm" in agent.tools_mapResult:
✅ PASS: SwitchLLMTool is included when enabled with profiles
This confirms the tool is correctly added to both include_default_tools and the runtime tools_map when conditions are met.
Test 3: SwitchLLMTool excluded when disabled
Step 1 — Create profiles (same as Test 2):
Saved LLM profiles to ensure profiles exist.
Step 2 — Create settings with enable_switch_llm_tool=False:
settings = OpenHandsAgentSettings(
llm=_make_llm("default-model", "default"),
enable_switch_llm_tool=False
)
agent = settings.create_agent()Step 3 — Verify SwitchLLMTool is NOT included:
assert "SwitchLLMTool" not in agent.include_default_tools
conversation = LocalConversation(agent=agent, workspace=Path.cwd())
conversation._ensure_agent_ready()
assert "switch_llm" not in agent.tools_mapResult:
✅ PASS: SwitchLLMTool is excluded when disabled
This confirms the setting correctly gates the tool even when profiles are available.
Test 4: SwitchLLMTool excluded without profiles
Step 1 — Create an empty profile directory:
profile_dir.mkdir() # Empty directory, no profilesStep 2 — Create settings with enable_switch_llm_tool=True:
settings = OpenHandsAgentSettings(
llm=_make_llm("default-model", "default"),
enable_switch_llm_tool=True # Enabled but no profiles
)
agent = settings.create_agent()Step 3 — Verify SwitchLLMTool is NOT included:
assert "SwitchLLMTool" not in agent.include_default_tools
conversation = LocalConversation(agent=agent, workspace=Path.cwd())
conversation._ensure_agent_ready()
assert "switch_llm" not in agent.tools_mapResult:
✅ PASS: SwitchLLMTool is excluded when no profiles exist
This confirms the tool is correctly omitted when no profiles are available, even when the setting is enabled.
Test 5: Schema export includes the new field
Step 1 — Export the schema:
schema = OpenHandsAgentSettings.export_schema()Step 2 — Verify the field is present with correct properties:
general_section = [s for s in schema.sections if s.key == 'general'][0]
switch_llm_field = [f for f in general_section.fields if f.key == 'enable_switch_llm_tool'][0]Result:
General section fields: {'enable_sub_agents', 'tools', 'enable_switch_llm_tool', 'agent', 'mcp_config'}
✅ Field found with correct properties:
- key: enable_switch_llm_tool
- value_type: boolean
- default: True
- label: Enable LLM switching tool
- prominence: SettingProminence.MINOR
This confirms the schema export correctly includes the new field with appropriate metadata for client applications.
Issues Found
None.
Verdict: This PR fully achieves its stated goal and is ready for merge. The gating logic works correctly in all scenarios, the setting is properly exported in the schema, and all CI checks pass.
neubig
force-pushed
the
switch-llm-tool-settings
branch
from
ac5b217 to
288639f
Compare
Co-authored-by: openhands <openhands@all-hands.dev>
neubig
force-pushed
the
switch-llm-tool-settings
branch
from
288639f to
6dba02e
Compare
Python API breakage checks — ✅ PASSEDResult: ✅ PASSED |
REST API breakage checks (OpenAPI) — ✅ PASSEDResult: ✅ PASSED |
4 participants
Summary
OpenHandsAgentSettings.enable_switch_llm_tool, defaulting totrue.SwitchLLMToolin settings-created agents only when the setting is enabled and at least one saved LLM profile is available.Stacked on
Validation
env -u LMNR_PROJECT_API_KEY -u LMNR_BASE_URL -u LMNR_FORCE_HTTP uv run pytest tests/sdk/tool/test_switch_llm.py tests/sdk/test_settings.py -qenv -u LMNR_PROJECT_API_KEY -u LMNR_BASE_URL -u LMNR_FORCE_HTTP uv run pre-commit run --files openhands-sdk/openhands/sdk/settings/model.py openhands-sdk/openhands/sdk/tool/builtins/switch_llm.py tests/sdk/tool/test_switch_llm.py tests/sdk/test_settings.pyThis pull request was created by an AI agent (OpenHands) on behalf of the user.
Agent Server images for this PR
• GHCR package: https://github.com/OpenHands/agent-sdk/pkgs/container/agent-server
Variants & Base Images
eclipse-temurin:17-jdknikolaik/python-nodejs:python3.13-nodejs22-slimgolang:1.21-bookwormPull (multi-arch manifest)
# Each variant is a multi-arch manifest supporting both amd64 and arm64 docker pull ghcr.io/openhands/agent-server:6dba02e-pythonRun
All tags pushed for this build
About Multi-Architecture Support
6dba02e-python) is a multi-arch manifest supporting both amd64 and arm646dba02e-python-amd64) are also available if needed