feat(tool_calling): make tool_choice configurable via model_settings - #233
Conversation
elronbandel
left a comment
There was a problem hiding this comment.
Thanks for the contribution, @almogtavor — clean, well-scoped change, and the core design is right: excluding tool_choice from the shared model_dump and applying it only on the action completion is exactly correct, since the tool-shortlisting call passes no tools and tool_choice="required" there would error. I checked the branch out and verified all three behaviors — forwarded on the action call when set, never on the shortlisting call, and omitted entirely when unset (so no change for existing runs).
A few things before it can merge:
Blocking
- Add a test. This is a behavioral change with no test. The harness already exists in
tests/agents/test_litellm_params_extra.py(patch_completion_with_retries, capturecall_kwargs) — asserttool_choiceis forwarded on the action call when set, absent on the shortlisting call, and absent whenNone. - Formatting.
ruff formatcollapses the wrapped conditional spread to a single line (it's under the 120-char limit), so the pre-commit CI job fails.pre-commit run -afixes it. - DCO. The DCO check is red — the commit needs a sign-off (
git commit --amend --signoff, then force-push).
Worth doing
- The OpenAI agent has the same concept: its SDK
ModelSettings(openai-agents 0.13.2) acceptstool_choice, and we already build that object viaOpenAIModelSettings(...)insrc/exgentic/agents/openai/instance.py, so honoring it there is a one-line add. Without it, settingtool_choiceon an OpenAI run silently does nothing. smolagents/CLI agents own their tool-call loop and can't take it — alogger.warningwhen it's set on those (instead of a silent no-op) would avoid a confusing footgun. - Trim the comments. The inline comment at the call site mostly restates the code and the feature rationale (which belongs in the PR description). The one on
exclude={...}is the one worth keeping —tool_choicebeing excluded there is genuinely non-obvious.
Nice-to-have: link an issue, or note it's a standalone enhancement, for traceability.
Overall a solid change — it mainly needs the test + green CI, and ideally the OpenAI parity so the setting isn't silently agent-specific.
Add an optional tool_choice field to ModelSettings, plumbed only to the action completion. Defaults to None, so litellm keeps applying "auto" and existing runs are unchanged. When set (e.g. "required") it enables guided decoding for models that cannot freely emit the native tool-call format. Kept out of the generic model_dump so it never reaches the tool-less shortlisting completion. Also honor tool_choice on the OpenAI agent via OpenAIModelSettings, and log a warning on the smolagents/CLI agents (which own their tool-call loop and cannot apply it) instead of silently ignoring it. Standalone enhancement; not tracking a specific issue. Signed-off-by: Almog Tavor <almogtavor@gmail.com>
ac865c3 to
6f7788e
Compare
|
Thanks @elronbandel, all four are in: test added (action forwards / shortlist absent / unset absent), pre-commit clean, DCO signed off, plus OpenAI parity and a warning on the loop-owning agents. |
Problem
The litellm tool_calling agent sends no
tool_choice, so litellm defaults it to"auto". Models whose output distribution cannot reliably produce the native tool-call wire format then return 0 tool_calls and emit free-text instead. Driving such a model requirestool_choice="required"so the serving engine applies guided decoding and constrains generation to a valid tool-call grammar — but there is currently no way to settool_choicewithout editing the agent.Change
Make
tool_choiceamodel_settingsfield, plumbed through to the action completion only.tool_choice: str | None = NonetoModelSettings.model_dump()in_completionso it never leaks into the (tool-less) tool-shortlisting completion.Usage
Compatibility
tool_choicedefaults toNone, which is omitted from the request, so litellm keeps applying"auto". No behavior change for existing runs.