fix(chat): de-duplicate the double turn execution at its source
The UI POSTs the SSE stream and, when nothing streams to the browser (iOS can't read a fetch-stream body → the fetch throws in ~1s), falls back to the blocking endpoint. But the server-side stream runs to completion regardless, so BOTH turns executed — double-persisting the message and (once logging became guaranteed) double-logging the hand. Make a turn idempotent instead of chasing why the client bails: the first request for a (session, message) owns it; a concurrent duplicate waits on the owner's Event and reuses its reply rather than running a second full turn. respond and respond_stream both claim/await; a finally always releases waiters. Short window so a genuine later resend still runs fresh. Verified with a threaded race: two simultaneous calls, body runs once, both get the same reply. Also fixes the duplicate user-message persistence (the same double-execution) that was polluting reconstructed history. 6 dedup tests + concurrency check; suite 232. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -10,11 +10,55 @@ deliberate) and hands back a ready message list + the active mode. Then:
|
||||
"""
|
||||
from __future__ import annotations
|
||||
|
||||
import threading
|
||||
import time
|
||||
|
||||
from lyra import config, llm, logbus, memory, mind, modes, poker_prompts, summary
|
||||
from lyra import tools as toolkit
|
||||
from lyra.llm import Backend
|
||||
|
||||
MAX_TOOL_ROUNDS = 5 # cap tool-call iterations per turn
|
||||
|
||||
# --- turn de-duplication --------------------------------------------------
|
||||
# The web UI hits TWO endpoints for one message: it POSTs the SSE stream, and if
|
||||
# nothing streams to the browser (iOS can't read a fetch stream body → the fetch
|
||||
# throws in ~1s) it falls back to the blocking endpoint. But the server-side stream
|
||||
# runs to completion regardless, so BOTH turns execute — double-persisting the
|
||||
# message and (once logging became guaranteed) double-logging the hand. This guard
|
||||
# makes a turn idempotent: the first request for a given (session, message) owns it;
|
||||
# a duplicate that arrives while it's in flight waits for and reuses that result
|
||||
# instead of running a second full turn. Window is short so a genuine re-send later
|
||||
# still runs fresh.
|
||||
_TURN_TTL = 20.0
|
||||
_turn_lock = threading.Lock()
|
||||
_turns: dict[tuple, dict] = {} # (session_id, msg) -> {event, reply, ts}
|
||||
|
||||
|
||||
def _claim_turn(session_id: str, user_msg: str):
|
||||
"""(is_owner, rec). Owner executes the turn then calls _finish_turn; a non-owner
|
||||
(the near-simultaneous duplicate) waits on rec['event'] and reuses rec['reply']."""
|
||||
key = (session_id, (user_msg or "").strip())
|
||||
now = time.monotonic()
|
||||
with _turn_lock:
|
||||
for k in [k for k, r in _turns.items() if now - r["ts"] > _TURN_TTL]:
|
||||
del _turns[k]
|
||||
rec = _turns.get(key)
|
||||
if rec is not None:
|
||||
return False, rec
|
||||
rec = {"event": threading.Event(), "reply": None, "ts": now}
|
||||
_turns[key] = rec
|
||||
return True, rec
|
||||
|
||||
|
||||
def _finish_turn(rec: dict, reply: str) -> None:
|
||||
rec["reply"] = reply
|
||||
rec["ts"] = time.monotonic()
|
||||
rec["event"].set()
|
||||
|
||||
|
||||
def _await_duplicate(rec: dict) -> str:
|
||||
rec["event"].wait(timeout=_TURN_TTL)
|
||||
return rec["reply"] or _TANGLED
|
||||
# Which backends get function-calling tools is config-driven (cfg.tool_backends,
|
||||
# env TOOL_BACKENDS, default "cloud"). The MI50's llama.cpp server only does tools
|
||||
# when launched with --jinja + a tool-capable model, else it 500s on the tools
|
||||
@@ -133,6 +177,15 @@ def respond(session_id: str, user_msg: str, backend: Backend = "cloud",
|
||||
logbus.log("info", "chat request", session=session_id, backend=backend,
|
||||
model=model, embed=cfg.embed_backend)
|
||||
|
||||
# A concurrent duplicate (the UI's stream + blocking fallback for one message)
|
||||
# reuses the owner's result instead of running a second full turn.
|
||||
is_owner, rec = _claim_turn(session_id, user_msg)
|
||||
if not is_owner:
|
||||
logbus.log("info", "duplicate turn deduped", session=session_id, path="respond")
|
||||
return _await_duplicate(rec)
|
||||
|
||||
reply = _TANGLED
|
||||
try:
|
||||
turn = mind.assemble(session_id, user_msg, backend, model)
|
||||
messages = turn.messages
|
||||
tool_specs = toolkit.specs(turn.mode.tools) if backend in cfg.tool_backends else None
|
||||
@@ -153,6 +206,8 @@ def respond(session_id: str, user_msg: str, backend: Backend = "cloud",
|
||||
memory.remember(session_id, "assistant", reply)
|
||||
summary.maybe_summarize_async(session_id) # compact once enough new turns pile up
|
||||
return reply
|
||||
finally:
|
||||
_finish_turn(rec, reply)
|
||||
|
||||
|
||||
def respond_stream(session_id: str, user_msg: str, backend: Backend = "cloud",
|
||||
@@ -164,6 +219,18 @@ def respond_stream(session_id: str, user_msg: str, backend: Backend = "cloud",
|
||||
logbus.log("info", "chat request (stream)", session=session_id, backend=backend,
|
||||
model=model, embed=cfg.embed_backend)
|
||||
|
||||
# A concurrent duplicate (this stream + the UI's blocking fallback for one
|
||||
# message) reuses the owner's result instead of running a second full turn.
|
||||
is_owner, rec = _claim_turn(session_id, user_msg)
|
||||
if not is_owner:
|
||||
logbus.log("info", "duplicate turn deduped", session=session_id, path="stream")
|
||||
reply = _await_duplicate(rec)
|
||||
yield ("delta", reply)
|
||||
yield ("done", reply)
|
||||
return
|
||||
|
||||
reply = _TANGLED
|
||||
try:
|
||||
turn = mind.assemble(session_id, user_msg, backend, model)
|
||||
messages = turn.messages
|
||||
tool_specs = toolkit.specs(turn.mode.tools) if backend in cfg.tool_backends else None
|
||||
@@ -234,3 +301,5 @@ def respond_stream(session_id: str, user_msg: str, backend: Backend = "cloud",
|
||||
memory.remember(session_id, "assistant", reply)
|
||||
summary.maybe_summarize_async(session_id)
|
||||
yield ("done", reply)
|
||||
finally:
|
||||
_finish_turn(rec, reply)
|
||||
|
||||
@@ -70,7 +70,7 @@ def test_parse_prompt_records_straddles():
|
||||
# --- hero stack auto-fill from the last logged stack ----------------------
|
||||
|
||||
def test_hero_stack_filled_from_last_stack_log(poker, monkeypatch):
|
||||
sid = poker.start_session(venue="Meadows", stakes="1/3", buy_in=400)
|
||||
poker.start_session(venue="Meadows", stakes="1/3", buy_in=400)
|
||||
poker.log_stack(275) # his last reported stack
|
||||
monkeypatch.setattr(poker, "parse_hand",
|
||||
lambda *a, **k: {"game": "NLH", "hero_involved": True,
|
||||
@@ -84,7 +84,7 @@ def test_hero_stack_filled_from_last_stack_log(poker, monkeypatch):
|
||||
|
||||
|
||||
def test_stated_stack_is_never_overridden(poker, monkeypatch):
|
||||
sid = poker.start_session(venue="Meadows", buy_in=400)
|
||||
poker.start_session(venue="Meadows", buy_in=400)
|
||||
poker.log_stack(275)
|
||||
monkeypatch.setattr(poker, "parse_hand",
|
||||
lambda *a, **k: {"game": "NLH", "hero_involved": True,
|
||||
|
||||
@@ -0,0 +1,69 @@
|
||||
"""Turn de-duplication: the UI hits two endpoints for one message (SSE stream +
|
||||
blocking fallback). Only the first should execute; the duplicate reuses its result."""
|
||||
from __future__ import annotations
|
||||
|
||||
import pytest
|
||||
|
||||
from lyra import chat
|
||||
|
||||
|
||||
@pytest.fixture(autouse=True)
|
||||
def _clean_turns():
|
||||
chat._turns.clear()
|
||||
yield
|
||||
chat._turns.clear()
|
||||
|
||||
|
||||
def test_claim_is_owner_once_per_key():
|
||||
o1, r1 = chat._claim_turn("s1", "flopped a set")
|
||||
o2, r2 = chat._claim_turn("s1", "flopped a set")
|
||||
assert o1 is True and o2 is False
|
||||
assert r1 is r2 # the duplicate waits on the SAME record
|
||||
|
||||
|
||||
def test_different_messages_each_own():
|
||||
o1, _ = chat._claim_turn("s1", "hand A")
|
||||
o2, _ = chat._claim_turn("s1", "hand B")
|
||||
o3, _ = chat._claim_turn("s2", "hand A") # different session
|
||||
assert o1 and o2 and o3
|
||||
|
||||
|
||||
def test_await_returns_owner_reply():
|
||||
_, rec = chat._claim_turn("s1", "msg")
|
||||
chat._finish_turn(rec, "the answer")
|
||||
assert chat._await_duplicate(rec) == "the answer"
|
||||
|
||||
|
||||
def test_respond_duplicate_reuses_result_without_running_turn(monkeypatch):
|
||||
# owner already ran and cached its reply
|
||||
_, rec = chat._claim_turn("s1", "same hand")
|
||||
chat._finish_turn(rec, "owner reply")
|
||||
|
||||
def boom(*a, **k):
|
||||
raise AssertionError("duplicate must NOT execute the turn")
|
||||
monkeypatch.setattr(chat.mind, "assemble", boom)
|
||||
|
||||
out = chat.respond("s1", "same hand", "cloud")
|
||||
assert out == "owner reply"
|
||||
|
||||
|
||||
def test_respond_stream_duplicate_yields_cached_reply(monkeypatch):
|
||||
_, rec = chat._claim_turn("s1", "same hand")
|
||||
chat._finish_turn(rec, "owner reply")
|
||||
|
||||
def boom(*a, **k):
|
||||
raise AssertionError("duplicate must NOT execute the turn")
|
||||
monkeypatch.setattr(chat.mind, "assemble", boom)
|
||||
|
||||
events = list(chat.respond_stream("s1", "same hand", "cloud"))
|
||||
assert ("delta", "owner reply") in events
|
||||
assert ("done", "owner reply") in events
|
||||
|
||||
|
||||
def test_fresh_message_after_window_runs_again():
|
||||
# a completed turn lingers only briefly; simulate expiry and confirm re-ownership
|
||||
o1, rec = chat._claim_turn("s1", "later resend")
|
||||
chat._finish_turn(rec, "first")
|
||||
rec["ts"] -= chat._TURN_TTL + 1 # age it past the window
|
||||
o2, _ = chat._claim_turn("s1", "later resend")
|
||||
assert o1 and o2 # a genuine later resend runs fresh
|
||||
Reference in New Issue
Block a user