Skip to content

Commit 6145781

Browse files
committed
fix(e2e): derive mock stage from request body, not a call counter
Review feedback: the global _STATE["calls"] counter was incremented unlocked and before the response was emitted. Two failure modes follow — ThreadingHTTPServer can serve requests concurrently, and an aborted or retried call consumed the tool stage, leaving the retry to receive the final message instead. Reproduced the latter: aborting call 1 mid-stream made the retry return output_text rather than re-emitting function_call, which would surface as a bare 'no tool spans' failure. The conversation already carries the stage, so read it instead of counting: turn 1 has only the user message, turn 2 carries the tool result (function_call_output on the Responses wire, role: "tool" on Chat Completions). That is idempotent — a retry resends the same body and gets the same stage — so the mutable global goes away entirely rather than being guarded by a lock. Verified: the retry now re-emits function_call; both wires stage correctly on turns 1 and 2; mock E2E passes on latest, v2026.7.30 and v2026.7.20; wheel and real-Opik E2E pass; 113 unit tests pass.
1 parent 6e1729e commit 6145781

1 file changed

Lines changed: 36 additions & 10 deletions

File tree

e2e/mock_llm_server.py

Lines changed: 36 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -17,9 +17,11 @@
1717
1818
It also probes GET /v1/models and POST /api/show (Ollama-style).
1919
20-
Deterministic two-step interaction, identical on either protocol:
21-
call 1 -> a tool call (Hermes runs a tool -> tool span)
22-
call 2+ -> assistant text (turn completes)
20+
Deterministic two-step interaction, identical on either protocol. The stage is
21+
derived from the request body (has the tool result come back yet?), not from a
22+
call counter, so it is both concurrency-safe and stable across retries:
23+
no tool result yet -> a tool call (Hermes runs a tool -> tool span)
24+
tool result present -> assistant text (turn completes)
2325
2426
That yields one LLM -> tool -> LLM cycle: a root trace with two LLM spans and
2527
one tool span. No real model, no key, no external network.
@@ -36,10 +38,36 @@
3638
from http.server import BaseHTTPRequestHandler, ThreadingHTTPServer
3739

3840
PORT = int(os.environ.get("MOCK_LLM_PORT", "18790"))
39-
_STATE = {"calls": 0}
4041
_USAGE = {"prompt_tokens": 20, "completion_tokens": 8, "total_tokens": 28}
4142

4243

44+
def _already_ran_tool(payload: bytes) -> bool:
45+
"""Has the tool already run in this conversation?
46+
47+
The stage is derived from the request body rather than a call counter.
48+
A counter is wrong on two axes: `ThreadingHTTPServer` can serve requests
49+
concurrently, and a retried/aborted call would consume the tool stage and
50+
leave the retry to get the final message — silently producing a run with
51+
no tool span. Reading the conversation is idempotent, so a retry of the
52+
same request always gets the same stage back.
53+
54+
Turn 1 carries only the user message; turn 2 carries the tool result
55+
(Responses: a `function_call_output` input item; Chat Completions: a
56+
`role: "tool"` message).
57+
"""
58+
try:
59+
data = json.loads(payload or b"{}")
60+
except ValueError:
61+
return False
62+
63+
for item in data.get("input") or data.get("messages") or []:
64+
if not isinstance(item, dict):
65+
continue
66+
if item.get("type") == "function_call_output" or item.get("role") == "tool":
67+
return True
68+
return False
69+
70+
4371
def _tool_call_chunks():
4472
"""Stream a single tool_call, then finish_reason=tool_calls."""
4573
base = {"id": "chatcmpl-mock1", "object": "chat.completion.chunk", "model": "gpt-5"}
@@ -186,11 +214,10 @@ def do_POST(self):
186214

187215
# Responses API (codex_responses api_mode).
188216
if self.path.rstrip("/").endswith("/responses"):
189-
_STATE["calls"] += 1
190217
events = (
191-
_responses_tool_events()
192-
if _STATE["calls"] == 1
193-
else _responses_final_events()
218+
_responses_final_events()
219+
if _already_ran_tool(body)
220+
else _responses_tool_events()
194221
)
195222
wants_stream = b'"stream": true' in body or b'"stream":true' in body
196223
if wants_stream:
@@ -204,8 +231,7 @@ def do_POST(self):
204231

205232
# Chat Completions (the other wire Hermes may pick).
206233
if "chat/completions" in self.path:
207-
_STATE["calls"] += 1
208-
chunks = _tool_call_chunks() if _STATE["calls"] == 1 else _final_chunks()
234+
chunks = _final_chunks() if _already_ran_tool(body) else _tool_call_chunks()
209235
wants_stream = b'"stream": true' in body or b'"stream":true' in body
210236
if wants_stream:
211237
self._sse(chunks)

0 commit comments

Comments
 (0)