fix(state): order conversation replay by id, not timestamp

get_messages_as_conversation ordered rows by (timestamp, id). append_message
stamps each row with time.time(), which is not monotonic — on WSL2, after an
NTP step, or when a VM/laptop resumes from sleep the clock can jump backwards
mid-conversation. A later row then carries an earlier timestamp than its
predecessor, so ORDER BY timestamp sorts an assistant tool_calls row after its
tool response, orphaning the tool call and triggering an HTTP 400 on the next
completion. Order by the AUTOINCREMENT id (true insertion order) instead.

This is the sibling path to c03acca50, which already fixed get_messages but
missed get_messages_as_conversation.

Salvaged from #50356.

Co-authored-by: pprism13 <290877921+pprism13@users.noreply.github.com>
This commit is contained in:
pprism13 2026-07-01 15:46:07 +05:30 committed by kshitij
parent cde3ca4ebf
commit f049227f31
2 changed files with 51 additions and 1 deletions

View file

@ -3651,7 +3651,15 @@ class SessionDB:
"finish_reason, reasoning, reasoning_content, reasoning_details, "
"codex_reasoning_items, codex_message_items, platform_message_id, observed, timestamp "
f"FROM messages WHERE session_id IN ({placeholders})"
f"{active_clause} ORDER BY timestamp, id",
# Order by AUTOINCREMENT id (true insertion order), NOT timestamp:
# append_message stamps rows with time.time(), which is not
# monotonic (WSL2, NTP steps, VM/laptop sleep resume). A later
# row can carry an earlier timestamp than its predecessor, and
# ORDER BY timestamp would then sort an assistant tool_calls row
# after its tool response, breaking tool-call/response adjacency
# and triggering an HTTP 400 on replay. This matches get_messages
# — see c03acca50 for the original fix.
f"{active_clause} ORDER BY id",
tuple(session_ids),
).fetchall()

View file

@ -861,6 +861,48 @@ class TestMessageStorage:
assert conv[1]["content"] == "Hi!"
assert isinstance(conv[1]["timestamp"], float)
def test_get_messages_as_conversation_orders_by_id_not_timestamp(self, db):
"""Replay must follow AUTOINCREMENT id (insertion order), never the
wall-clock timestamp.
``append_message`` stamps each row with ``time.time()``, which is not
monotonic on WSL2, after an NTP step, or when a VM/laptop resumes
from sleep the clock can jump backwards mid-conversation. A later
row then carries an *earlier* timestamp than the row before it. If
``get_messages_as_conversation`` ordered by ``timestamp`` it would
sort an assistant ``tool_calls`` row after its ``tool`` response,
orphaning the tool call and triggering an HTTP 400 on the next
completion. Ordering by ``id`` keeps the real insertion order
regardless of clock skew. See c03acca50.
"""
db.create_session(session_id="s1", source="cli")
# Simulate a clock regression across a single tool round-trip: the
# assistant tool_calls row is inserted first but stamped LATER than
# the tool response that follows it.
tool_calls = [
{"id": "call_1", "function": {"name": "web_search", "arguments": "{}"}},
]
db.append_message(
"s1", role="assistant", content="", tool_calls=tool_calls,
timestamp=1000.0,
)
db.append_message(
"s1", role="tool", content="result", tool_name="web_search",
tool_call_id="call_1", timestamp=999.0,
)
db.append_message("s1", role="user", content="thanks", timestamp=998.0)
conv = db.get_messages_as_conversation("s1")
# Insertion order is preserved even though timestamps decrease.
assert [m["role"] for m in conv] == ["assistant", "tool", "user"]
# The tool response stays immediately after the assistant tool_calls
# row — the adjacency invariant the model API enforces.
assert conv[0]["tool_calls"][0]["id"] == "call_1"
assert conv[1]["role"] == "tool"
assert conv[1]["tool_call_id"] == "call_1"
def test_platform_message_id_round_trips(self, db):
"""Platform-side message ids (yuanbao msg_id, telegram update_id, …)
survive append get_messages_as_conversation under the