Skip to content

Commit 21a43c3

Browse files
committed
test(client): pin streaming terminator finish_reason parity
New tests/test_handle_chat_stream_finish.py drives _handle_chat directly with a mock ws for both async-gen and sync-gen handlers that yield a finish_reason="length" chunk, asserting the terminator carries "length" not "stop"; a third case pins the pre-existing "stop" default when the handler stays silent so the fallback doesn't regress. Mutation-verified against the pre-fix client.py (2 fail with "stop" on the wire). The existing tests/test_client_sync_generator.py had a line pinning the terminator to "stop" while the handler yielded finish_reason="length" — that assertion was pinning the very bug being fixed. Updated to expect "length" on the terminator, matching the JS port's semantics.
1 parent bfaf222 commit 21a43c3

2 files changed

Lines changed: 105 additions & 2 deletions

File tree

tests/test_client_sync_generator.py

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -51,8 +51,10 @@ def handler(messages, options):
5151
# The chunk's own done/finish_reason survive serialization.
5252
assert payloads[1]["done"] is True
5353
assert payloads[1]["finish_reason"] == "length"
54-
# Trailing synthetic terminator closes the stream.
55-
assert payloads[-1] == {"delta": "", "done": True, "finish_reason": "stop"}
54+
# Trailing synthetic terminator closes the stream and echoes the
55+
# handler-supplied finish_reason (here 'length' — max-tokens truncation),
56+
# not a hardcoded 'stop' that would misrepresent the outcome.
57+
assert payloads[-1] == {"delta": "", "done": True, "finish_reason": "length"}
5658

5759

5860
@pytest.mark.asyncio
Lines changed: 101 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,101 @@
1+
"""Regression: _handle_chat's streaming paths (async-gen + sync-gen) hardcoded
2+
`finish_reason="stop"` on the terminator chat-chunk regardless of what the
3+
handler yielded. A handler that flagged a max-tokens truncation with
4+
`finish_reason="length"` had it silently rewritten to `"stop"` on the wire,
5+
so the client couldn't distinguish clean stop from truncation.
6+
7+
The non-streaming accumulate paths already carry the handler's finish_reason
8+
through `_finalize_accumulated`. The JS port was fixed for the streaming
9+
paths in 7427ed4 — this brings Python back to parity.
10+
"""
11+
12+
import json
13+
14+
import pytest
15+
16+
from zhub.client import _handle_chat
17+
from zhub.protocol import Envelope
18+
19+
20+
class _FakeWS:
21+
def __init__(self) -> None:
22+
self.sent: list[dict] = []
23+
24+
async def send(self, text: str) -> None:
25+
self.sent.append(json.loads(text))
26+
27+
28+
class _Pub:
29+
def __init__(self, handler) -> None:
30+
self.chat_handler = handler
31+
32+
33+
def _last_chunk(ws: _FakeWS) -> dict:
34+
"""The terminator envelope — the last chat-chunk on the wire."""
35+
chunks = [e for e in ws.sent if e["type"] == "chat-chunk"]
36+
assert chunks, f"no chat-chunk envelopes emitted; got {[e['type'] for e in ws.sent]}"
37+
return chunks[-1]
38+
39+
40+
@pytest.mark.asyncio
41+
async def test_asyncgen_streaming_terminator_echoes_handler_finish_reason():
42+
"""Handler yields a final chunk with finish_reason='length'. The terminator
43+
chat-chunk (done=True) must carry finish_reason='length', not 'stop'."""
44+
45+
async def handler(messages, options):
46+
yield "hello "
47+
yield "world"
48+
yield {"delta": "", "finish_reason": "length"}
49+
50+
ws = _FakeWS()
51+
env = Envelope(
52+
type="chat-request",
53+
payload={"messages": [], "stream": True},
54+
)
55+
await _handle_chat(_Pub(handler), ws, env)
56+
57+
terminator = _last_chunk(ws)
58+
assert terminator["payload"]["done"] is True
59+
assert terminator["payload"]["finish_reason"] == "length"
60+
61+
62+
@pytest.mark.asyncio
63+
async def test_syncgen_streaming_terminator_echoes_handler_finish_reason():
64+
"""Same guarantee for a sync generator handler."""
65+
66+
def handler(messages, options):
67+
yield "partial "
68+
yield "answer"
69+
yield {"delta": "", "finish_reason": "length"}
70+
71+
ws = _FakeWS()
72+
env = Envelope(
73+
type="chat-request",
74+
payload={"messages": [], "stream": True},
75+
)
76+
await _handle_chat(_Pub(handler), ws, env)
77+
78+
terminator = _last_chunk(ws)
79+
assert terminator["payload"]["done"] is True
80+
assert terminator["payload"]["finish_reason"] == "length"
81+
82+
83+
@pytest.mark.asyncio
84+
async def test_asyncgen_streaming_defaults_to_stop_when_handler_silent():
85+
"""Handler never sets finish_reason → terminator carries the default 'stop'.
86+
Regression guard so the fix doesn't drop the pre-existing default."""
87+
88+
async def handler(messages, options):
89+
yield "one"
90+
yield "two"
91+
92+
ws = _FakeWS()
93+
env = Envelope(
94+
type="chat-request",
95+
payload={"messages": [], "stream": True},
96+
)
97+
await _handle_chat(_Pub(handler), ws, env)
98+
99+
terminator = _last_chunk(ws)
100+
assert terminator["payload"]["done"] is True
101+
assert terminator["payload"]["finish_reason"] == "stop"

0 commit comments

Comments
 (0)