fix: address completion verdict review findings
- fidelity cacheability also requires a non-empty projected body, so think-tag-only answers are no longer cached - a no-argument tool sent with arguments null only is a deterministic dialect: ResultInvalid tool_arguments_null instead of retried - text mode treats choice index null as 0 and no longer lets False pose as 0 - null-only tool_calls elements no longer produce an empty tool_calls list; non-stream fidelity checks role before the verdict; warning paths are length-bounded - docs: ARCHITECTURE 7.8 field counts, README cache/watchdog wording, CHANGELOG notes, review record in the design, hook note in the plan Red: 8 new tests failed before this change (e.g. test_openai_cacheable_requires_projected_body, test_null_arguments_tool_is_result_invalid_stream, test_text_mode_choice_index_null_is_zero[None-True]).
This commit is contained in:
@@ -1116,3 +1116,14 @@ async def test_new_fields_roundtrip_through_cache():
|
||||
assert stored["finish_reason"] == "stop" and stored["stream_end"] == "eof_after_finish"
|
||||
hit = await mw(ChatRequest(messages=_MSGS), terminal)
|
||||
assert hit.cache_hit is True and hit.finish_reason == "stop" and hit.stream_end is None
|
||||
|
||||
|
||||
async def test_fidelity_think_only_projection_not_cached():
|
||||
"""审查 I1: qwen think 标签剥离后投影为空的保真结果不写缓存。"""
|
||||
completion = OpenAIChatCompletion(
|
||||
{"role": "assistant", "content": "<think>x</think>"}, "stop", "up", None
|
||||
)
|
||||
response = _resp(content="", thinking="x", finish_reason="stop", completion=completion)
|
||||
backend = InMemoryCache()
|
||||
await _mw(backend)(ChatRequest(messages=_MSGS, response_mode="openai"), _Terminal(response))
|
||||
assert not backend._store
|
||||
|
||||
@@ -85,7 +85,7 @@ async def test_eof_after_finish_usage_only_frame_and_tools():
|
||||
result = await _run(frames, done=False, mode="openai")
|
||||
tool = result.completion.message["tool_calls"][0]
|
||||
assert tool["function"] == {"name": "lookup", "arguments": '{"q": 1}'}
|
||||
assert result.stream_end == "eof_after_finish"
|
||||
assert result.stream_end == "eof_after_finish" and result.usage_source == "measured"
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
@@ -416,3 +416,38 @@ async def test_openai_reasoning_length_single_attempt_via_client():
|
||||
await client._transport.aclose()
|
||||
assert len(calls) == 1 and response.call_stats.attempts == 1
|
||||
assert response.stream_end == "eof_after_finish" and response.finish_reason == "length"
|
||||
|
||||
|
||||
async def test_eof_after_finish_stop_single_attempt_via_client():
|
||||
"""#31 验收 1 经治理链: stop + usage 无 DONE,attempts=1、不重试到耗尽。"""
|
||||
from tests.unit.test_client import _client
|
||||
|
||||
calls = []
|
||||
|
||||
def handler(_):
|
||||
calls.append(1)
|
||||
return _sse_stream(_assistant(content="ok"), _f({}, "stop", _USAGE), done=False)
|
||||
|
||||
client = _client(handler=handler)
|
||||
try:
|
||||
response = await client.chat([{"role": "user", "content": "hi"}])
|
||||
finally:
|
||||
await client._transport.aclose()
|
||||
assert len(calls) == 1 and response.call_stats.attempts == 1
|
||||
assert response.stream_end == "eof_after_finish" and response.usage_source == "measured"
|
||||
|
||||
|
||||
async def test_text_salvage_with_length_partial_content_delivered():
|
||||
"""文本 salvage + length + 部分正文(finish 后无 usage): 交付、salvaged。"""
|
||||
frames = [_assistant(content="part"), _f({}, finish="length")]
|
||||
result = await _run(frames, done=False, missing_done="salvage")
|
||||
assert result.content == "part" and result.stream_end == "salvaged"
|
||||
assert result.finish_reason == "length"
|
||||
|
||||
|
||||
async def test_null_arguments_tool_is_result_invalid_stream():
|
||||
"""审查 I2: 无参函数 arguments 全程 null → ResultInvalid,不重试。"""
|
||||
tool = {"index": 0, "id": "c", "type": "function", "function": {"name": "f", "arguments": None}}
|
||||
frames = [_assistant(tool_calls=[tool]), _f({}, "tool_calls", _USAGE)]
|
||||
with pytest.raises(ResultInvalidError, match="tool_arguments_null"):
|
||||
await _run(frames, done=True, mode="openai")
|
||||
|
||||
@@ -244,3 +244,22 @@ def test_stream_end_exported():
|
||||
import polygateway
|
||||
|
||||
assert "StreamEnd" in polygateway.__all__
|
||||
|
||||
|
||||
def test_openai_cacheable_requires_projected_body():
|
||||
"""审查 I1: think 标签剥离后投影为空,即使原文非空也不可缓存。"""
|
||||
think_only = _completion(content="<think>x</think>")
|
||||
assert not _is_cacheable(
|
||||
mode="openai", content="", finish_reason="stop", stream_end="done", completion=think_only
|
||||
)
|
||||
|
||||
|
||||
def test_null_arguments_only_tool_is_result_invalid():
|
||||
"""审查 I2: 无参函数 arguments 全程 null(其余齐全)→ 确定性方言,ResultInvalid。"""
|
||||
tool = {"id": "t1", "type": "function", "function": {"name": "f", "arguments": None}}
|
||||
summary = _summarize_message({"role": "assistant", "tool_calls": [tool]}, "")
|
||||
assert summary.null_arguments and not summary.tools_ok
|
||||
verdict = _judge_outcome(mode="openai", summary=summary, finish="natural")
|
||||
assert (verdict.error_kind, verdict.code) == ("result_invalid", "tool_arguments_null")
|
||||
broken = {"id": "t1", "type": "function", "function": {"arguments": None}}
|
||||
assert not _summarize_message({"role": "assistant", "tool_calls": [broken]}, "").null_arguments
|
||||
|
||||
@@ -421,3 +421,48 @@ def test_nonstream_null_tool_elements_stripped():
|
||||
assert parse_chat_completion(body).message["tool_calls"] == [tool]
|
||||
body["choices"][0]["message"]["tool_calls"] = None
|
||||
assert parse_chat_completion(body).message["tool_calls"] is None
|
||||
|
||||
|
||||
@pytest.mark.parametrize(("index", "taken"), [(None, True), (0, True), (False, False), (1, False)])
|
||||
def test_text_mode_choice_index_null_is_zero(index, taken):
|
||||
"""审查 I3: 文本模式 index:null 视缺省(=0);bool 不冒充 0。"""
|
||||
state = ChatCompletionAccumulator(response_mode="text")
|
||||
delta = state.add_chunk(
|
||||
{"choices": [{"index": index, "delta": {"content": "hi"}, "finish_reason": "stop"}]}
|
||||
)
|
||||
assert (delta.content == "hi") is taken and (state.finish_reason == "stop") is taken
|
||||
|
||||
|
||||
def test_null_only_tool_elements_do_not_create_empty_list():
|
||||
"""审查 Minor1: tool_calls null 后 [null] 不得产出会被上游拒收的空数组。"""
|
||||
state = ChatCompletionAccumulator(response_mode="openai")
|
||||
_feed(
|
||||
state,
|
||||
_frame({"role": "assistant", "content": "a", "tool_calls": None}),
|
||||
_frame({"tool_calls": [None]}, finish="stop"),
|
||||
)
|
||||
assert state.finish().message["tool_calls"] is None
|
||||
body = {"choices": [{"message": {"role": "assistant", "content": "a", "tool_calls": [None]}}]}
|
||||
assert parse_chat_completion(body).message["tool_calls"] is None
|
||||
|
||||
|
||||
def test_tool_calls_null_then_list_merges():
|
||||
"""tool_calls 先 null 后列表: 正常聚合工具。"""
|
||||
state = ChatCompletionAccumulator(response_mode="openai")
|
||||
tool = {"index": 0, "id": "a", "type": "function", "function": {"name": "f", "arguments": "{}"}}
|
||||
_feed(
|
||||
state,
|
||||
_frame({"role": "assistant", "tool_calls": None}),
|
||||
_frame({"tool_calls": [tool]}, finish="tool_calls"),
|
||||
)
|
||||
assert state.finish().message["tool_calls"][0]["id"] == "a"
|
||||
|
||||
|
||||
def test_nonstream_role_checked_before_verdict():
|
||||
"""审查 Minor3: 非流式缺 role 是形态失败,先于结局判定。"""
|
||||
from polygateway.transports._openai_completion import check_completion_body
|
||||
|
||||
with pytest.raises(TransientError, match="role"):
|
||||
check_completion_body(
|
||||
{"choices": [{"message": {"content": "a"}, "finish_reason": "length"}]}
|
||||
)
|
||||
|
||||
Reference in New Issue
Block a user