Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 18 additions & 3 deletions litellm/llms/anthropic/chat/handler.py
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,12 @@ async def make_call(

try:
response = await client.post(
api_base, headers=headers, data=data, stream=True, timeout=timeout
api_base,
headers=headers,
data=data,
stream=True,
timeout=timeout,
logging_obj=logging_obj,
)
except httpx.HTTPStatusError as e:
error_headers = getattr(e, "headers", None)
Expand Down Expand Up @@ -138,7 +143,12 @@ def make_sync_call(

try:
response = client.post(
api_base, headers=headers, data=data, stream=True, timeout=timeout
api_base,
headers=headers,
data=data,
stream=True,
timeout=timeout,
logging_obj=logging_obj,
)
except httpx.HTTPStatusError as e:
error_headers = getattr(e, "headers", None)
Expand Down Expand Up @@ -262,7 +272,11 @@ async def acompletion_function(

try:
response = await async_handler.post(
api_base, headers=headers, json=data, timeout=timeout
api_base,
headers=headers,
json=data,
timeout=timeout,
logging_obj=logging_obj,
)
except Exception as e:
## LOGGING
Expand Down Expand Up @@ -465,6 +479,7 @@ def completion(
headers=headers,
data=json.dumps(data),
timeout=timeout,
logging_obj=logging_obj,
)
except Exception as e:
status_code = getattr(e, "status_code", 500)
Expand Down
Original file line number Diff line number Diff line change
@@ -1,13 +1,42 @@
from unittest.mock import MagicMock
from unittest.mock import AsyncMock, MagicMock

import pytest

from litellm.constants import RESPONSE_FORMAT_TOOL_NAME
from litellm.llms.anthropic.chat.handler import ModelResponseIterator
from litellm.llms.anthropic.chat.handler import ModelResponseIterator, make_call
from litellm.types.llms.openai import (
ChatCompletionToolCallChunk,
ChatCompletionToolCallFunctionChunk,
)


@pytest.mark.asyncio
async def test_make_call_passes_logging_obj_to_client_post():
"""make_call must pass logging_obj to client.post so track_llm_api_timing can set llm_api_duration_ms for litellm_overhead_time_ms."""
mock_client = AsyncMock()
mock_response = MagicMock()
mock_response.aiter_lines = MagicMock(return_value=iter([b'data: {"type":"message_start"}\n', b'data: {"type":"message_delta"}\n']))
mock_client.post.return_value = mock_response

logging_obj = MagicMock()

await make_call(
client=mock_client,
api_base="https://api.anthropic.com/v1/messages",
headers={},
data="{}",
model="claude-3-5-haiku",
messages=[{"role": "user", "content": "Hi"}],
logging_obj=logging_obj,
timeout=60.0,
json_mode=False,
)

mock_client.post.assert_called_once()
call_kwargs = mock_client.post.call_args[1]
assert call_kwargs.get("logging_obj") is logging_obj
Comment on lines +35 to +37
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Use .kwargs instead of index-based call_args[1]

call_args[1] is the legacy tuple-indexing API for accessing keyword arguments on a call object. The modern, more readable approach is .kwargs. Both work, but the newer form is clearer and less likely to break if positional args are added.

Suggested change
mock_client.post.assert_called_once()
call_kwargs = mock_client.post.call_args[1]
assert call_kwargs.get("logging_obj") is logging_obj
call_kwargs = mock_client.post.call_args.kwargs
assert call_kwargs.get("logging_obj") is logging_obj

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Comment on lines +13 to +37
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Test only covers one of four fixed call sites

The PR fixes logging_obj being passed at four separate call sites in handler.py:

  1. make_call (async streaming) — ✅ covered by this test
  2. make_sync_call (sync streaming) — ❌ not tested
  3. acompletion_function async non-streaming — ❌ not tested
  4. completion sync non-streaming — ❌ not tested

Per the project's testing guidelines (at least 1 test is a hard requirement), this technically satisfies the bar, but adding tests for the sync path and non-streaming paths would prevent regressions where a future refactor accidentally drops logging_obj from one of those call sites.

Consider adding parallel tests for make_sync_call and the non-streaming paths.



def test_redacted_thinking_content_block_delta():
chunk = {
"type": "content_block_start",
Expand Down
Loading