mirror of
https://github.com/BerriAI/litellm.git
synced 2026-10-09 03:18:44 +00:00
fix: restore tool names in async streaming via shared ContextVar dict
acompletion() runs completion() inside copy_context()+run_in_executor.
Any _CTX.set({}) inside the executor only affects the copied context, so
the stream consumer in the outer context saw _CTX=None and could not
restore sanitized tool names.
Fix:
- begin_openai_tool_name_mapping_scope() is now idempotent: it skips the
reset if _CTX already holds a dict (force_reset=True for explicit resets).
- acompletion() calls begin_openai_tool_name_mapping_scope() before
copy_context(), seeding _CTX with a live {} in the outer context.
- copy_context() copies the reference to the same dict; mutations by the
executor are visible in the outer context because Python dict mutation
is in-place.
- Tests updated to use force_reset=True where a clean slate is needed;
new test verifies the shared-reference invariant end-to-end.
Co-authored-by: Cursor <cursoragent@cursor.com>
This commit is contained in:
parent
f0df19af21
commit
4edb5b9481
3 changed files with 64 additions and 9 deletions
|
|
@ -50,10 +50,23 @@ def should_sanitize_openai_tool_names(litellm_provider: str) -> bool:
|
|||
return litellm_provider in _OPENAI_TOOL_NAME_VALIDATION_PROVIDERS
|
||||
|
||||
|
||||
def begin_openai_tool_name_mapping_scope() -> None:
|
||||
"""Reset both mappings for this completion (call once at the start of completion())."""
|
||||
_CTX.set({})
|
||||
_CTX_REV.set({})
|
||||
def begin_openai_tool_name_mapping_scope(*, force_reset: bool = False) -> None:
|
||||
"""Initialise (or reset) both mapping dicts for one completion request.
|
||||
|
||||
Idempotent by default: if the ContextVar already holds a dict this is a
|
||||
no-op. This matters for async streaming where ``acompletion`` calls this
|
||||
function in the *outer* async context before ``contextvars.copy_context()``
|
||||
so that both the executor and the stream consumer share the same dict
|
||||
objects by reference. The inner ``completion()`` call then sees a non-None
|
||||
dict and skips the reset, preserving the shared reference.
|
||||
|
||||
Pass ``force_reset=True`` only when you explicitly want a clean slate
|
||||
(e.g. in tests).
|
||||
"""
|
||||
if force_reset or _CTX.get() is None:
|
||||
_CTX.set({})
|
||||
if force_reset or _CTX_REV.get() is None:
|
||||
_CTX_REV.set({})
|
||||
|
||||
|
||||
def _store(sanitized: str, original: str) -> None:
|
||||
|
|
|
|||
|
|
@ -611,6 +611,9 @@ async def acompletion( # noqa: PLR0915
|
|||
# Use a partial function to pass your keyword arguments
|
||||
func = partial(completion, **completion_kwargs, **kwargs)
|
||||
|
||||
# Initialise the OpenAI tool-name mapping scope in the *outer* async
|
||||
begin_openai_tool_name_mapping_scope()
|
||||
|
||||
# Add the context to the function
|
||||
ctx = contextvars.copy_context()
|
||||
func_with_context = partial(ctx.run, func)
|
||||
|
|
|
|||
|
|
@ -4032,7 +4032,7 @@ def test_openai_tool_name_mapping_per_request_context():
|
|||
)
|
||||
from litellm.utils import validate_and_fix_openai_tools
|
||||
|
||||
begin_openai_tool_name_mapping_scope()
|
||||
begin_openai_tool_name_mapping_scope(force_reset=True)
|
||||
validate_and_fix_openai_tools(
|
||||
tools=[
|
||||
{
|
||||
|
|
@ -4049,7 +4049,8 @@ def test_openai_tool_name_mapping_per_request_context():
|
|||
restore_openai_tool_name_for_user("invalid_name_at_index_71")
|
||||
== "invalid.name.at.index.71"
|
||||
)
|
||||
begin_openai_tool_name_mapping_scope()
|
||||
# Simulate a second request: force_reset=True clears the mapping.
|
||||
begin_openai_tool_name_mapping_scope(force_reset=True)
|
||||
assert (
|
||||
restore_openai_tool_name_for_user("invalid_name_at_index_71")
|
||||
== "invalid_name_at_index_71"
|
||||
|
|
@ -4066,7 +4067,7 @@ def test_convert_to_model_response_restores_openai_tool_call_names_when_mapped()
|
|||
from litellm.types.utils import ModelResponse
|
||||
from litellm.utils import validate_and_fix_openai_tools
|
||||
|
||||
begin_openai_tool_name_mapping_scope()
|
||||
begin_openai_tool_name_mapping_scope(force_reset=True)
|
||||
validate_and_fix_openai_tools(
|
||||
tools=[
|
||||
{
|
||||
|
|
@ -4121,7 +4122,7 @@ def test_tool_choice_function_name_sanitized_with_tools():
|
|||
)
|
||||
from litellm.utils import validate_and_fix_openai_tools
|
||||
|
||||
begin_openai_tool_name_mapping_scope()
|
||||
begin_openai_tool_name_mapping_scope(force_reset=True)
|
||||
validate_and_fix_openai_tools(
|
||||
tools=[
|
||||
{
|
||||
|
|
@ -4152,7 +4153,7 @@ def test_tool_choice_function_name_uses_collision_suffix():
|
|||
)
|
||||
from litellm.utils import validate_and_fix_openai_tools
|
||||
|
||||
begin_openai_tool_name_mapping_scope()
|
||||
begin_openai_tool_name_mapping_scope(force_reset=True)
|
||||
validate_and_fix_openai_tools(
|
||||
tools=[
|
||||
{
|
||||
|
|
@ -4175,3 +4176,41 @@ def test_tool_choice_function_name_uses_collision_suffix():
|
|||
# "a.b" → "a_b"; "a/b" also sanitizes to "a_b" but gets suffix → "a_b_1"
|
||||
assert get_sanitized_tool_name("a.b") == "a_b"
|
||||
assert get_sanitized_tool_name("a/b") == "a_b_1"
|
||||
|
||||
|
||||
def test_begin_scope_is_idempotent_shared_reference():
|
||||
"""begin_openai_tool_name_mapping_scope must not replace the dict when already set.
|
||||
|
||||
This simulates the async-streaming path: the outer acompletion() call
|
||||
initialises the scope (sets _CTX = {}), copy_context() copies the
|
||||
reference, and then the inner completion() call must not clobber it with a
|
||||
new empty dict — otherwise mutations from the executor are invisible to the
|
||||
stream consumer in the outer context.
|
||||
"""
|
||||
import contextvars
|
||||
|
||||
from litellm.litellm_core_utils.openai_tool_name_mapping import (
|
||||
_CTX,
|
||||
begin_openai_tool_name_mapping_scope,
|
||||
restore_openai_tool_name_for_user,
|
||||
store_openai_tool_name_mapping,
|
||||
)
|
||||
|
||||
# Outer context initialises the scope (simulates acompletion pre-copy_context).
|
||||
begin_openai_tool_name_mapping_scope(force_reset=True)
|
||||
outer_dict = _CTX.get()
|
||||
assert outer_dict is not None
|
||||
|
||||
# Inner context (executor) calls begin_openai_tool_name_mapping_scope again
|
||||
# and then writes a mapping — simulates what completion() does.
|
||||
def inner():
|
||||
begin_openai_tool_name_mapping_scope() # must be a no-op (no force_reset)
|
||||
inner_dict = _CTX.get()
|
||||
assert inner_dict is outer_dict, "inner call must not replace the shared dict"
|
||||
store_openai_tool_name_mapping("my_tool", "my.tool")
|
||||
|
||||
ctx = contextvars.copy_context()
|
||||
ctx.run(inner)
|
||||
|
||||
# The outer context must see the mapping written by the inner context.
|
||||
assert restore_openai_tool_name_for_user("my_tool") == "my.tool"
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue