From 44f8d3ed37a9d4934ffda176f6dddfefa05249dd Mon Sep 17 00:00:00 2001 From: nesquena-hermes Date: Tue, 22 Sep 2026 17:23:39 +0000 Subject: [PATCH] fix(models): thread incoming_provenance + protect multi-tool blocks Rebase threading fix and a backward scan so a chronological insert cannot split a multi-result tool block. --- CHANGELOG.md | 3 +- api/models.py | 28 ++++-- tests/test_multi_tool_block_insertion.py | 118 +++++++++++++++++++++++ 3 files changed, 138 insertions(+), 11 deletions(-) create mode 100644 tests/test_multi_tool_block_insertion.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 60f5ee11..c64357e3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,7 +5,8 @@ ### Fixed -- **A rejected request no longer poisons the next one on the same connection.** `server.py` +- **A late-arriving prompt no longer renders below the reply it asked for.** When a message + reached the transcript from `state.db` after the sidecar had already been merged⟪HERMES-CONTEXT-COMPRESSION: 809 of 1,009 chars omitted here by Hermes's context compressor. This is NOT part of the original tool call and must never be reproduced in new output — always write full, untruncated content.⟫- **A rejected request no longer poisons the next one on the same connection.** `server.py` is a raw HTTP/1.1 handler where `rfile` is the socket itself, so answering a request before reading its body left those bytes queued. The next request on a keep-alive connection was then parsed starting mid-body, and the client got diff --git a/api/models.py b/api/models.py index feed1aea..d3bcca28 100644 --- a/api/models.py +++ b/api/models.py @@ -10382,16 +10382,22 @@ def _insert_state_message_chronologically(messages: list, msg: dict) -> bool: # preceding assistant's tool_calls, not just the first — a multi-tool # turn has several adjacent tool results, and inserting between any of # them splits the block (assistant, tool, , tool). - if ( - idx < len(messages) - and messages[idx].get("role") == "tool" - and idx > 0 - and messages[idx - 1].get("role") == "assistant" - and messages[idx - 1].get("tool_calls") - ): - while idx < len(messages) and messages[idx].get("role") == "tool": - idx += 1 - advanced = True + if idx < len(messages) and messages[idx].get("role") == "tool": + # Walk back over any contiguous tool rows already emitted for + # this block, so the owning assistant is found even when idx + # lands on the SECOND result of a multi-tool turn (where + # messages[idx - 1] is another tool row, not the assistant). + owner = idx - 1 + while owner >= 0 and messages[owner].get("role") == "tool": + owner -= 1 + if ( + owner >= 0 + and messages[owner].get("role") == "assistant" + and messages[owner].get("tool_calls") + ): + while idx < len(messages) and messages[idx].get("role") == "tool": + idx += 1 + advanced = True # (b) Skip past an equal-timestamp run whose left neighbour shares # this message's role — inserting there would re-order an # already-matched same-role turn (user, , assistant). @@ -10434,6 +10440,7 @@ def merge_session_messages_append_only( state_messages, truncation_watermark=truncation_watermark, truncation_boundary=truncation_boundary, + incoming_provenance=incoming_provenance, ) finally: _STRUCTURED_IDENTITY_MEMO.reset(token) @@ -10445,6 +10452,7 @@ def _merge_session_messages_append_only_impl( *, truncation_watermark=None, truncation_boundary=None, + incoming_provenance=None, ) -> list: """Merge sidecar/context and state.db messages without deleting local rows. diff --git a/tests/test_multi_tool_block_insertion.py b/tests/test_multi_tool_block_insertion.py new file mode 100644 index 00000000..b669be3c --- /dev/null +++ b/tests/test_multi_tool_block_insertion.py @@ -0,0 +1,118 @@ +"""A chronological insert must never land inside a multi-result tool block. + +``_insert_state_message_chronologically`` skips past an +``assistant(tool_calls)`` -> tool-result block so a late state.db row cannot be +placed between a tool call and its result — splitting that pair makes the next +send fail provider validation. + +The original guard only recognised the block when ``messages[idx - 1]`` was the +owning assistant. That holds for a single-result turn, but a multi-tool turn +emits several adjacent tool rows, so when the insertion point landed on the +SECOND result its left neighbour was another ``tool`` row and the guard did not +fire. The row was inserted mid-block: + + assistant(tool_calls), tool(t1), , tool(t2) + +These tests pin the backward scan that finds the owning assistant across any +number of preceding contiguous tool rows. +""" + +import pytest + +from api.models import _insert_state_message_chronologically + + +def _msg(role, ts, content="x", **extra): + row = {"role": role, "timestamp": ts, "content": content} + row.update(extra) + return row + + +def _tool_result(call_id, ts): + return { + "role": "tool", + "timestamp": ts, + "tool_call_id": call_id, + "content": f"result-{call_id}", + } + + +def _block(n_results, first_result_ts=210, step=10): + """An assistant turn with ``n_results`` tool calls and their results.""" + assistant = _msg( + "assistant", + 200, + "", + tool_calls=[ + {"id": f"t{i + 1}", "function": {"name": f"fn{i + 1}"}} + for i in range(n_results) + ], + ) + results = [ + _tool_result(f"t{i + 1}", first_result_ts + i * step) for i in range(n_results) + ] + return assistant, results + + +def _assert_block_intact(messages, assistant, results): + start = messages.index(assistant) + positions = [messages.index(r) for r in results] + expected = [start + 1 + i for i in range(len(results))] + assert positions == expected, ( + "tool block was split: " + f"{[m['role'] for m in messages]} (assistant at {start}, results at {positions})" + ) + + +@pytest.mark.parametrize("late_ts", [205, 210, 215, 220, 225, 235]) +@pytest.mark.parametrize("n_results", [2, 3, 4]) +def test_late_row_never_splits_a_multi_result_tool_block(n_results, late_ts): + """No insertion point may land between an assistant's tool results.""" + assistant, results = _block(n_results) + messages = [_msg("user", 100), assistant, *results, _msg("assistant", 500)] + late = _msg("user", late_ts, "late-prompt") + + _insert_state_message_chronologically(messages, late) + + _assert_block_intact(messages, assistant, results) + assert late in messages, "the late row should still be placed somewhere" + + +def test_single_result_block_still_protected(): + """The original single-result case must keep working.""" + assistant, results = _block(1) + messages = [_msg("user", 100), assistant, *results, _msg("assistant", 500)] + late = _msg("user", 215, "late-prompt") + + _insert_state_message_chronologically(messages, late) + + _assert_block_intact(messages, assistant, results) + + +def test_orphan_tool_rows_do_not_trap_the_backward_scan(): + """Tool rows with no owning assistant must not make the scan run away. + + If the backward walk finds no ``assistant(tool_calls)`` owner it must leave + the insertion point alone rather than skipping the whole run. + """ + orphans = [_tool_result("t1", 210), _tool_result("t2", 220)] + messages = [_msg("user", 100), *orphans, _msg("assistant", 500)] + late = _msg("user", 215, "late-prompt") + + _insert_state_message_chronologically(messages, late) + + assert late in messages + # The orphan rows are still present and still in their original order. + assert [m for m in messages if m.get("role") == "tool"] == orphans + + +def test_late_row_after_the_block_is_unaffected(): + """A row that belongs after the whole block lands after it, not inside.""" + assistant, results = _block(3) + messages = [_msg("user", 100), assistant, *results, _msg("assistant", 500)] + late = _msg("user", 260, "late-prompt") + + _insert_state_message_chronologically(messages, late) + + _assert_block_intact(messages, assistant, results) + assert messages.index(late) > messages.index(results[-1])