mirror of
https://github.com/tiennm99/litellm.git
synced 2026-07-21 06:19:18 +00:00
fix: address Greptile review feedback
- Log provider token counting failures instead of silently swallowing - Fall back to local tokenizer when provider returns error response - Map assistant tool_calls to Responses API function_call items - Concatenate multiple system messages instead of overwriting - Hide internal error details from proxy API responses - Narrow exception catch in handler to network/JSON errors only - Update test to match new fallback behavior
This commit is contained in:
@@ -4,6 +4,7 @@ OpenAI Responses API token counting handler.
|
||||
Uses httpx for HTTP requests to OpenAI's /v1/responses/input_tokens endpoint.
|
||||
"""
|
||||
|
||||
import json
|
||||
from typing import Any, Dict, List, Optional, Union
|
||||
|
||||
import httpx
|
||||
@@ -96,7 +97,7 @@ class OpenAICountTokensHandler(OpenAICountTokensConfig):
|
||||
status_code=e.response.status_code,
|
||||
message=e.response.text,
|
||||
)
|
||||
except Exception as e:
|
||||
except (httpx.RequestError, json.JSONDecodeError) as e:
|
||||
verbose_logger.error(f"Error in CountTokens handler: {str(e)}")
|
||||
raise OpenAIError(
|
||||
status_code=500,
|
||||
|
||||
@@ -98,7 +98,7 @@ class OpenAICountTokensConfig:
|
||||
from system/developer messages.
|
||||
"""
|
||||
input_items: List[Dict[str, Any]] = []
|
||||
instructions: Optional[str] = None
|
||||
instructions_parts: List[str] = []
|
||||
|
||||
for msg in messages:
|
||||
role = msg.get("role", "")
|
||||
@@ -107,7 +107,7 @@ class OpenAICountTokensConfig:
|
||||
if role in ("system", "developer"):
|
||||
# Extract system/developer messages as instructions
|
||||
if isinstance(content, str):
|
||||
instructions = content
|
||||
instructions_parts.append(content)
|
||||
elif isinstance(content, list):
|
||||
# Handle content blocks - extract text
|
||||
text_parts = []
|
||||
@@ -116,11 +116,23 @@ class OpenAICountTokensConfig:
|
||||
text_parts.append(block.get("text", ""))
|
||||
elif isinstance(block, str):
|
||||
text_parts.append(block)
|
||||
instructions = "\n".join(text_parts)
|
||||
instructions_parts.append("\n".join(text_parts))
|
||||
elif role == "user":
|
||||
input_items.append({"role": "user", "content": content})
|
||||
elif role == "assistant":
|
||||
input_items.append({"role": "assistant", "content": content})
|
||||
# Map tool_calls to Responses API function_call items
|
||||
tool_calls = msg.get("tool_calls")
|
||||
if tool_calls:
|
||||
for tc in tool_calls:
|
||||
func = tc.get("function", {})
|
||||
input_items.append({
|
||||
"type": "function_call",
|
||||
"call_id": tc.get("id", ""),
|
||||
"name": func.get("name", ""),
|
||||
"arguments": func.get("arguments", ""),
|
||||
})
|
||||
else:
|
||||
input_items.append({"role": "assistant", "content": content})
|
||||
elif role == "tool":
|
||||
input_items.append({
|
||||
"type": "function_call_output",
|
||||
@@ -128,4 +140,5 @@ class OpenAICountTokensConfig:
|
||||
"output": content if isinstance(content, str) else str(content),
|
||||
})
|
||||
|
||||
instructions = "\n".join(instructions_parts) if instructions_parts else None
|
||||
return input_items, instructions
|
||||
|
||||
+5
-3
@@ -7609,10 +7609,12 @@ async def acount_tokens(
|
||||
tools=tools,
|
||||
system=system,
|
||||
)
|
||||
if result is not None:
|
||||
if result is not None and not result.error:
|
||||
return result
|
||||
except Exception:
|
||||
pass
|
||||
except Exception as e:
|
||||
verbose_logger.debug(
|
||||
f"Provider token counting failed for model={model}, falling back to local: {e}"
|
||||
)
|
||||
|
||||
# Fallback to local tiktoken-based token counting
|
||||
local_count = litellm.token_counter(
|
||||
|
||||
@@ -517,7 +517,7 @@ async def count_response_input_tokens(
|
||||
)
|
||||
)
|
||||
raise HTTPException(
|
||||
status_code=500, detail={"error": f"Internal server error: {str(e)}"}
|
||||
status_code=500, detail={"error": "Internal server error"}
|
||||
)
|
||||
|
||||
|
||||
|
||||
@@ -133,9 +133,10 @@ def test_acount_tokens_api_error_falls_back():
|
||||
)
|
||||
)
|
||||
|
||||
# Should return error response, not raise
|
||||
assert result.error is True
|
||||
assert result.status_code == 401
|
||||
# Should fall back to local tokenizer when provider API errors
|
||||
assert result.error is False
|
||||
assert result.tokenizer_type == "local_tokenizer"
|
||||
assert result.total_tokens > 0
|
||||
|
||||
|
||||
def test_acount_tokens_no_api_key_falls_back():
|
||||
|
||||
Reference in New Issue
Block a user