Skip to content
Closed
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
54 changes: 34 additions & 20 deletions astrbot/core/provider/sources/openai_source.py
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,38 @@ def _safe_json_dump(value: Any) -> str | None:
except Exception:
return None

@staticmethod
def _is_empty_assistant_content(content: Any) -> bool:
if isinstance(content, str):
return content.strip() == ""
return content is None or content == []

@classmethod
def _clean_assistant_messages_for_provider(cls, payloads: dict) -> None:
messages = payloads.get("messages")
if not isinstance(messages, list):
return

cleaned_messages = []
for idx, msg in enumerate(messages):
if msg.get("role") == "assistant":
content = msg.get("content")
tool_calls = msg.get("tool_calls")

if not tool_calls and cls._is_empty_assistant_content(content):
logger.debug(
f"Filtered empty assistant message at index {idx} "
"(without tool calls)"
)
continue

if tool_calls and cls._is_empty_assistant_content(content):
msg["content"] = None

cleaned_messages.append(msg)
Comment on lines +85 to +100
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.

medium

The logic for cleaning assistant messages can be optimized by checking if the content is empty first. This avoids redundant calls to _is_empty_assistant_content and only fetches tool_calls when necessary, improving both readability and performance.

Suggested change
for idx, msg in enumerate(messages):
if msg.get("role") == "assistant":
content = msg.get("content")
tool_calls = msg.get("tool_calls")
if not tool_calls and cls._is_empty_assistant_content(content):
logger.warning(
f"Filtered empty assistant message at index {idx} "
"(without tool calls)"
)
continue
if tool_calls and cls._is_empty_assistant_content(content):
msg["content"] = None
cleaned_messages.append(msg)
for idx, msg in enumerate(messages):
if msg.get("role") == "assistant":
content = msg.get("content")
if cls._is_empty_assistant_content(content):
tool_calls = msg.get("tool_calls")
if not tool_calls:
logger.warning(
f"Filtered empty assistant message at index {idx} "
"(without tool calls)"
)
continue
msg["content"] = None
cleaned_messages.append(msg)


payloads["messages"] = cleaned_messages

def _get_image_moderation_error_patterns(self) -> list[str]:
"""Return configured moderation patterns (case-insensitive substring match, not regex)."""
configured = self.provider_config.get("image_moderation_error_patterns", [])
Expand Down Expand Up @@ -548,26 +580,7 @@ async def _query(self, payloads: dict, tools: ToolSet | None) -> LLMResponse:

model = payloads.get("model", "").lower()

if "messages" in payloads and isinstance(payloads["messages"], list):
cleaned_messages = []
for idx, msg in enumerate(payloads["messages"]):
# 过滤空的 assistant 消息,防止严格 API(如 Moonshot)返回 400 错误
if msg.get("role") == "assistant":
content = msg.get("content")
tool_calls = msg.get("tool_calls")

# 情况1: 空/null content 且无 tool_calls -> 过滤掉
if not tool_calls and (content == "" or content is None):
logger.warning(f"过滤第 {idx} 条空 assistant 消息 (无工具调用)")
continue

# 情况2: 空 content 但有 tool_calls -> 设为 None (符合 OpenAI 规范)
if content == "" and tool_calls:
msg["content"] = None

cleaned_messages.append(msg)

payloads["messages"] = cleaned_messages
self._clean_assistant_messages_for_provider(payloads)

completion = await self.client.chat.completions.create(
**payloads,
Expand Down Expand Up @@ -618,6 +631,7 @@ async def _query_stream(
for key in to_del:
del payloads[key]
self._apply_provider_specific_extra_body_overrides(extra_body)
self._clean_assistant_messages_for_provider(payloads)

stream = await self.client.chat.completions.create(
**payloads,
Expand Down
149 changes: 149 additions & 0 deletions tests/test_openai_source.py
Original file line number Diff line number Diff line change
Expand Up @@ -1263,6 +1263,98 @@ async def fake_create(**kwargs):
await provider.terminate()


@pytest.mark.asyncio
async def test_query_stream_filters_empty_assistant_messages(monkeypatch):
provider = _make_provider()
try:
captured_kwargs = {}
chunks = [
ChatCompletionChunk.model_validate(
{
"id": "chatcmpl-stream",
"object": "chat.completion.chunk",
"created": 0,
"model": "gpt-4o-mini",
"choices": [
{
"index": 0,
"delta": {
"role": "assistant",
"content": "ok",
},
"finish_reason": None,
}
],
}
),
ChatCompletionChunk.model_validate(
{
"id": "chatcmpl-stream",
"object": "chat.completion.chunk",
"created": 0,
"model": "gpt-4o-mini",
"choices": [
{
"index": 0,
"delta": {},
"finish_reason": "stop",
}
],
}
),
]

async def fake_stream():
for chunk in chunks:
yield chunk

async def fake_create(**kwargs):
captured_kwargs.update(kwargs)
return fake_stream()

monkeypatch.setattr(provider.client.chat.completions, "create", fake_create)

tool_calls = [
{
"id": "call-1",
"type": "function",
"function": {
"name": "get_weather",
"arguments": "{}",
},
}
]
payloads = {
"model": "deepseek-reasoner",
"messages": [
{"role": "user", "content": "hello"},
{"role": "assistant", "content": ""},
{"role": "assistant", "content": None},
{"role": "assistant", "content": []},
{"role": "assistant", "content": " "},
{"role": "assistant", "content": "", "tool_calls": tool_calls},
{"role": "assistant", "content": " ", "tool_calls": tool_calls},
{"role": "user", "content": "world"},
],
}

responses = [
response
async for response in provider._query_stream(payloads=payloads, tools=None)
]

messages = captured_kwargs["messages"]
assert responses[-1].completion_text == "ok"
assert messages == [
{"role": "user", "content": "hello"},
{"role": "assistant", "content": None, "tool_calls": tool_calls},
{"role": "assistant", "content": None, "tool_calls": tool_calls},
{"role": "user", "content": "world"},
]
finally:
await provider.terminate()


@pytest.mark.asyncio
async def test_query_filters_empty_assistant_message_without_tool_calls(monkeypatch):
"""Test that empty assistant messages without tool_calls are filtered out."""
Expand Down Expand Up @@ -1303,6 +1395,7 @@ async def fake_create(**kwargs):
"messages": [
{"role": "user", "content": "hello"},
{"role": "assistant", "content": ""}, # Should be filtered
{"role": "assistant", "content": " "}, # Should be filtered
{"role": "user", "content": "world"},
],
}
Expand Down Expand Up @@ -1375,6 +1468,62 @@ async def fake_create(**kwargs):
await provider.terminate()


@pytest.mark.asyncio
async def test_query_filters_empty_list_assistant_message_without_tool_calls(
monkeypatch,
):
"""Test that assistant messages with empty list content are filtered."""
provider = _make_provider()
try:
captured_kwargs = {}

async def fake_create(**kwargs):
captured_kwargs.update(kwargs)
return ChatCompletion.model_validate(
{
"id": "chatcmpl-test",
"object": "chat.completion",
"created": 0,
"model": "gpt-4o-mini",
"choices": [
{
"index": 0,
"message": {
"role": "assistant",
"content": "ok",
},
"finish_reason": "stop",
}
],
"usage": {
"prompt_tokens": 1,
"completion_tokens": 1,
"total_tokens": 2,
},
}
)

monkeypatch.setattr(provider.client.chat.completions, "create", fake_create)

payloads = {
"model": "gpt-4o-mini",
"messages": [
{"role": "user", "content": "hello"},
{"role": "assistant", "content": []}, # Should be filtered
{"role": "user", "content": "world"},
],
}

await provider._query(payloads=payloads, tools=None)

messages = captured_kwargs["messages"]
assert len(messages) == 2
assert messages[0] == {"role": "user", "content": "hello"}
assert messages[1] == {"role": "user", "content": "world"}
finally:
await provider.terminate()


@pytest.mark.asyncio
async def test_query_converts_empty_content_to_none_with_tool_calls(monkeypatch):
"""Test that empty content with tool_calls is converted to None (OpenAI spec)."""
Expand Down