mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-25 17:18:11 +00:00
fix(agent): make dropped tool-call nudge pair ephemeral scaffolding
Review follow-up for the dropped tool-call recovery (#69630): the re-prompt pair was tagged _dropped_toolcall_nudge, but that marker was not part of the ephemeral-scaffolding contract. _persist_session / _flush_messages_to_session_db would therefore write the synthetic 'issue the actual tool call now' user message (and the narration-only interim assistant turn) as real transcript rows — a resumed session could replay the internal retry instruction as user-authored context and prompt unsolicited tool use. - Add _dropped_toolcall_nudge to _EPHEMERAL_SCAFFOLDING_FLAGS (run_agent.py) so both SQLite and JSON persistence skip the pair. - Add it to _SYNTHETIC_USER_FLAGS (conversation_compression.py) so the compressor never treats the nudge as human intent. - Flag the interim assistant half of the pair too, and include the marker in the finalization scaffolding pop so a genuine turn end strips the pair from the live transcript (mirrors the _empty_recovery_synthetic pattern). - Regression tests: flagged messages classify as ephemeral, the returned transcript contains no scaffolding, and the turn tail stays on the real assistant answer.
This commit is contained in:
parent
923704c7c2
commit
5a0f51325c
4 changed files with 84 additions and 0 deletions
|
|
@ -892,6 +892,7 @@ _SYNTHETIC_USER_FLAGS = (
|
|||
"_empty_recovery_synthetic",
|
||||
"_verification_stop_synthetic",
|
||||
"_pre_verify_synthetic",
|
||||
"_dropped_toolcall_nudge",
|
||||
)
|
||||
|
||||
|
||||
|
|
|
|||
|
|
@ -6248,6 +6248,17 @@ def run_conversation(
|
|||
"↻ Model signaled a tool call but sent none — "
|
||||
f"re-prompting ({agent._dropped_toolcall_retries}/3)"
|
||||
)
|
||||
# Both halves of the re-prompt pair are ephemeral recovery
|
||||
# scaffolding (mirrors the empty-response nudge pattern):
|
||||
# the interim narration-only assistant turn exists solely to
|
||||
# keep role alternation valid for the nudge, and the nudge
|
||||
# exists solely to drive the retry. Flag both so the
|
||||
# persistence layer never writes them to the durable
|
||||
# transcript and the finalization pop below can strip an
|
||||
# unanswered tail pair. A recovered (answered) pair stays
|
||||
# buried mid-list in live memory but is skipped by the
|
||||
# flush regardless of position.
|
||||
final_msg["_dropped_toolcall_nudge"] = True
|
||||
messages.append(final_msg)
|
||||
messages.append({
|
||||
"role": "user",
|
||||
|
|
@ -6279,6 +6290,7 @@ def run_conversation(
|
|||
messages[-1].get("_thinking_prefill")
|
||||
or messages[-1].get("_empty_recovery_synthetic")
|
||||
or messages[-1].get("_empty_terminal_sentinel")
|
||||
or messages[-1].get("_dropped_toolcall_nudge")
|
||||
)
|
||||
):
|
||||
messages.pop()
|
||||
|
|
|
|||
|
|
@ -239,6 +239,12 @@ _EPHEMERAL_SCAFFOLDING_FLAGS = (
|
|||
"_pre_verify_synthetic",
|
||||
# kanban worker stop-guard: narrated exit without kanban_complete/block
|
||||
"_kanban_stop_synthetic",
|
||||
# dropped tool-call re-prompt pair (finish_reason=tool_calls with an
|
||||
# empty tool_calls array): the interim narration-only assistant turn
|
||||
# and the "issue the actual tool call now" user nudge exist only to
|
||||
# drive the bounded retry. Persisting them would replay the internal
|
||||
# retry instruction as user-authored context on resume.
|
||||
"_dropped_toolcall_nudge",
|
||||
)
|
||||
|
||||
|
||||
|
|
|
|||
|
|
@ -172,3 +172,68 @@ class TestDroppedToolCallRecovery:
|
|||
"Consecutive dropped tool calls must be bounded (no infinite loop)."
|
||||
)
|
||||
assert result is not None
|
||||
|
||||
def test_nudge_pair_is_ephemeral_scaffolding(self, loop_agent):
|
||||
"""The re-prompt pair (interim assistant turn + synthetic user nudge)
|
||||
must be flagged as ephemeral scaffolding so persistence never writes
|
||||
it to the durable transcript — a resumed session must not replay the
|
||||
internal "issue the actual tool call now" instruction as user-authored
|
||||
context (#69630 review follow-up)."""
|
||||
from run_agent import _is_ephemeral_scaffolding
|
||||
from tests.run_agent.test_run_agent import _mock_response
|
||||
|
||||
loop_agent.client.chat.completions.create.side_effect = [
|
||||
_dropped_tool_call_response("Let me verify the PR."),
|
||||
_mock_response(content="All checks pass. Approved.", finish_reason="stop"),
|
||||
]
|
||||
|
||||
with (
|
||||
patch.object(loop_agent, "_persist_session"),
|
||||
patch.object(loop_agent, "_save_trajectory"),
|
||||
patch.object(loop_agent, "_cleanup_task_resources"),
|
||||
):
|
||||
result = loop_agent.run_conversation("review the PR")
|
||||
|
||||
assert result["completed"] is True
|
||||
# The finalization pop strips the answered pair from the live list —
|
||||
# no flagged scaffolding may survive into the returned transcript.
|
||||
leftover = [
|
||||
m for m in result["messages"]
|
||||
if isinstance(m, dict) and m.get("_dropped_toolcall_nudge")
|
||||
]
|
||||
assert not leftover, (
|
||||
"The re-prompt pair must be stripped at finalization, not kept "
|
||||
"in the returned transcript."
|
||||
)
|
||||
# And the persistence filter must classify the flag as ephemeral so a
|
||||
# mid-turn flush can never write the pair to the durable store either.
|
||||
assert _is_ephemeral_scaffolding(
|
||||
{"role": "user", "content": "nudge", "_dropped_toolcall_nudge": True}
|
||||
), (
|
||||
"_dropped_toolcall_nudge messages must be classified as "
|
||||
"ephemeral scaffolding so they are never persisted."
|
||||
)
|
||||
|
||||
def test_unanswered_nudge_tail_is_stripped_at_finalization(self, loop_agent):
|
||||
"""If the model answers the nudge with a genuine final text turn, the
|
||||
trailing scaffolding must not leave the transcript tail on a synthetic
|
||||
user message (strict role alternation on the next turn)."""
|
||||
from tests.run_agent.test_run_agent import _mock_response
|
||||
|
||||
loop_agent.client.chat.completions.create.side_effect = [
|
||||
_dropped_tool_call_response("Let me check."),
|
||||
_mock_response(content="Final answer.", finish_reason="stop"),
|
||||
]
|
||||
|
||||
with (
|
||||
patch.object(loop_agent, "_persist_session"),
|
||||
patch.object(loop_agent, "_save_trajectory"),
|
||||
patch.object(loop_agent, "_cleanup_task_resources"),
|
||||
):
|
||||
result = loop_agent.run_conversation("review the PR")
|
||||
|
||||
tail = result["messages"][-1]
|
||||
assert tail.get("role") == "assistant", (
|
||||
"The turn must end on the real assistant answer, not scaffolding."
|
||||
)
|
||||
assert not tail.get("_dropped_toolcall_nudge")
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue