Skip to content

fix: include tool_choice in ChatCompletionCache cache key - #7948

Open
Humphrey (HumphreySun98) wants to merge 1 commit into
microsoft:mainfrom
HumphreySun98:fix/cache-key-tool-choice
Open

Humphrey (HumphreySun98) wants to merge 1 commit into
microsoft:mainfrom
HumphreySun98:fix/cache-key-tool-choice

Conversation

@HumphreySun98

@HumphreySun98 Humphrey (HumphreySun98) commented Jul 11, 2026

Copy link
Copy Markdown

Why are these changes needed?

ChatCompletionCache forwards tool_choice to the underlying client, where it
materially changes the model's output ("auto" vs "required" vs "none" vs a
specific forced Tool). However _check_cache did not include tool_choice
when computing the cache key, so two otherwise-identical calls that differ only
in tool_choice collide on the same key — the second call silently returns the
first call's cached result:

r1 = await cached_client.create(messages, tools=[tool], tool_choice="none")
r2 = await cached_client.create(messages, tools=[tool], tool_choice="required")
# r2 is r1's cached result: no tool call, despite tool_choice="required"

This PR includes tool_choice in the cache-key data (representing a forced
Tool by its schema name) and passes it through from both create and
create_stream. A regression test asserts the four tool_choice forms produce
distinct cache keys and that the default matches explicit "auto".

Note: existing caches are effectively invalidated by the key change (same as any
cache-key fix); entries were ambiguous between tool_choice values anyway.

Related issue number

N/A — found during review of the cache key computation.

Related PRs

The same bug has since been reported and fixed independently three more times, which suggests it bites in practice: #8096 (2026-08-25), #8185 (2026-09-04, bundled with unrelated doc changes), #8213 (2026-09-09). This PR is the earliest and is limited to the cache module plus a regression test.

Checks

  • I've included any doc changes needed for https://microsoft.github.io/autogen/ (none required for this change).
  • I've added tests (if relevant) corresponding to the changes introduced in this PR.
  • I've made sure all auto checks have passed (ran ruff, mypy, and the relevant pytest locally).

`ChatCompletionCache` forwards `tool_choice` to the underlying client, where it
materially changes the model's output (`"auto"` vs `"required"` vs `"none"` vs a
specific forced tool). However `_check_cache` did not include `tool_choice` when
computing the cache key, so two otherwise-identical calls that differed only in
`tool_choice` collided and the second call silently returned the first call's
cached result.

Include `tool_choice` in the cache-key data (representing a forced `Tool` by its
name) and pass it through from both `create` and `create_stream`.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@HumphreySun98

Copy link
Copy Markdown
Author

Closing and immediately reopening to re-create the expired CI workflow runs (explanation follows).

@HumphreySun98

Copy link
Copy Markdown
Author

Explanation for the reopen above: the workflow runs created when this PR opened in July had expired (GitHub expires unapproved fork runs after 30 days) and been purged, so the PR carried zero runs. There was therefore nothing for a maintainer to approve, and the 18 required status checks could never have reported.

Reopening at the same commit re-created them, with no commits changed and nothing force-pushed. This PR now has 7 runs in action_required, so "Approve and run workflows" is actionable. Full write-up in #7931.

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