fix(agent): finish the #80622 bug class — sibling predicates, refund ordering, prompt carve-out, honest skip response
Follow-ups on top of the salvaged #80696 fix (review findings): - Sibling sites: rollback.restore, gateway /retry, CLI /retry and /undo N, and both CLI resume turn counters now use is_user_originated_turn so legacy-persisted standalone handoffs (durable role=user, no display_kind) can never be truncation targets or counted as user turns (#80622 suggested regression 4, dispatcher-wide). - Site-1 guard: hoist the api_call_count decrement + iteration-budget refund above the break so a skipped turn no longer leaks a budget unit and finalize_turn logs the true call count (matches the ollama early-exit and the site-2 sibling). - Site-2 guard: run the handoff guard BEFORE reanchoring so a restored user ask is what the anchor lands on, not a stale pre-restore index. - SUMMARY_PREFIX: add the mid-tool-loop carve-out the code-side guard already implements, so a literal-minded model doesn't halt an in-flight exchange after in-place compaction. - Skip path returns a short compaction status instead of replaying the previous turn's answer (finalize_turn would append it as a fresh assistant row — duplicate prose in transcript and delivery).
This commit is contained in:
parent
b9636b1047
commit
4eabb595f0
|
|
@ -110,7 +110,9 @@ SUMMARY_PREFIX = (
|
|||
"resume, wrap up, or continue work from "
|
||||
f"'{HISTORICAL_TASK_HEADING}' or any other section, do not call tools, "
|
||||
"and wait for a new user message. This handoff must never become the "
|
||||
"active turn by itself. "
|
||||
"active turn by itself. (Exception: if tool results or your own "
|
||||
"tool calls appear after this summary, you are mid-way through an "
|
||||
"in-flight exchange — continue that exchange normally.) "
|
||||
"Topic overlap with the summary does NOT mean you should resume its "
|
||||
"task: even on similar topics, the latest user message WINS. Treat ONLY "
|
||||
"the latest message as the active task and discard stale items from "
|
||||
|
|
|
|||
|
|
@ -142,20 +142,18 @@ def _should_skip_model_call_for_reference_handoff(
|
|||
|
||||
|
||||
def _final_response_from_messages(messages: List[Dict[str, Any]]) -> str:
|
||||
"""Best-effort recovery of the last real assistant text after a skipped call."""
|
||||
from agent.context_compressor import is_compaction_summary_message
|
||||
"""Fallback text for a turn ended by the sole-handoff skip (#80622).
|
||||
|
||||
for message in reversed(messages or []):
|
||||
if not isinstance(message, dict) or message.get("role") != "assistant":
|
||||
continue
|
||||
if message.get("tool_calls"):
|
||||
continue
|
||||
if is_compaction_summary_message(message):
|
||||
continue
|
||||
content = message.get("content")
|
||||
if isinstance(content, str) and content.strip():
|
||||
return content
|
||||
return ""
|
||||
Deliberately NOT a replay of the last assistant text: finalize_turn's
|
||||
non-assistant-tail chokepoint (#43849) appends ``final_response`` as a
|
||||
fresh assistant row, so recovering the previous turn's prose here would
|
||||
duplicate it in the durable transcript AND re-deliver it to the user as
|
||||
if it were this turn's answer. A short status is honest and idempotent.
|
||||
"""
|
||||
return (
|
||||
"Context was compacted. The previous response is complete — "
|
||||
"awaiting your next message."
|
||||
)
|
||||
|
||||
|
||||
# Stable prefix of the local interrupt status string emitted when a turn is
|
||||
|
|
@ -2215,6 +2213,17 @@ def run_conversation(
|
|||
conversation_history = conversation_history_after_compression(
|
||||
agent, messages, conversation_history
|
||||
)
|
||||
# This preflight iteration never reaches the provider whether
|
||||
# we skip the turn (handoff guard below) or re-run the loop —
|
||||
# refund the consumed call/budget in BOTH cases, mirroring the
|
||||
# ollama_runtime_context_too_small early-exit above. Without
|
||||
# the refund on the break path, every skipped turn leaked one
|
||||
# iteration-budget unit for the agent's lifetime and
|
||||
# finalize_turn logged an api_call_count including a call that
|
||||
# was never made.
|
||||
api_call_count -= 1
|
||||
agent._api_call_count = api_call_count
|
||||
agent.iteration_budget.refund()
|
||||
if _should_skip_model_call_for_reference_handoff(
|
||||
messages, user_message
|
||||
):
|
||||
|
|
@ -2228,9 +2237,6 @@ def run_conversation(
|
|||
final_response = _final_response_from_messages(messages)
|
||||
_turn_exit_reason = "compaction_handoff_not_actionable"
|
||||
break
|
||||
api_call_count -= 1
|
||||
agent._api_call_count = api_call_count
|
||||
agent.iteration_budget.refund()
|
||||
continue
|
||||
elif (
|
||||
agent.compression_enabled
|
||||
|
|
@ -5801,16 +5807,6 @@ def run_conversation(
|
|||
# to fit the context window.
|
||||
retry_count += 1
|
||||
_retry.restart_with_compressed_messages = False
|
||||
# In-loop compression rebuilt `messages` with fresh compaction
|
||||
# copies, so the pre-compression current-turn index is stale.
|
||||
# Re-anchor exactly like the prologue does: a stale index that
|
||||
# lands on a historical user message would make the live-compose
|
||||
# fallback inject this turn's prefetch into that message on the
|
||||
# wire only, diverging the next turn's replayed prefix there.
|
||||
current_turn_user_idx = reanchor_current_turn_user_idx(
|
||||
messages, user_message
|
||||
)
|
||||
agent._persist_user_message_idx = current_turn_user_idx
|
||||
if _should_skip_model_call_for_reference_handoff(
|
||||
messages, user_message
|
||||
):
|
||||
|
|
@ -5822,6 +5818,19 @@ def run_conversation(
|
|||
final_response = _final_response_from_messages(messages)
|
||||
_turn_exit_reason = "compaction_handoff_not_actionable"
|
||||
break
|
||||
# In-loop compression rebuilt `messages` with fresh compaction
|
||||
# copies, so the pre-compression current-turn index is stale.
|
||||
# Re-anchor exactly like the prologue does: a stale index that
|
||||
# lands on a historical user message would make the live-compose
|
||||
# fallback inject this turn's prefetch into that message on the
|
||||
# wire only, diverging the next turn's replayed prefix there.
|
||||
# Ordered AFTER the handoff guard: the guard may have re-appended
|
||||
# this turn's real user ask (restore path), and the anchor must
|
||||
# land on that restored row, not on -1 / a pre-restore index.
|
||||
current_turn_user_idx = reanchor_current_turn_user_idx(
|
||||
messages, user_message
|
||||
)
|
||||
agent._persist_user_message_idx = current_turn_user_idx
|
||||
continue
|
||||
|
||||
if _retry.restart_with_rebuilt_messages:
|
||||
|
|
|
|||
17
cli.py
17
cli.py
|
|
@ -8410,11 +8410,15 @@ class HermesCLI(CLIAgentSetupMixin, CLICommandsMixin, CLIBillingMixin):
|
|||
|
||||
# Walk backwards to the last *real* user message. Timeline bookkeeping
|
||||
# rows (display_kind set) are role=user but are not user turns — match
|
||||
# CLI resume counting and list_recent_user_messages.
|
||||
# CLI resume counting and list_recent_user_messages. Compaction
|
||||
# handoffs are excluded too (durable role=user, sometimes without
|
||||
# display_kind on legacy sessions; #80622).
|
||||
from agent.context_compressor import is_user_originated_turn
|
||||
|
||||
last_user_idx = None
|
||||
for i in range(len(self.conversation_history) - 1, -1, -1):
|
||||
msg = self.conversation_history[i]
|
||||
if msg.get("role") == "user" and not msg.get("display_kind"):
|
||||
if is_user_originated_turn(msg):
|
||||
last_user_idx = i
|
||||
break
|
||||
|
||||
|
|
@ -8460,12 +8464,15 @@ class HermesCLI(CLIAgentSetupMixin, CLICommandsMixin, CLIBillingMixin):
|
|||
n = 1
|
||||
|
||||
# Walk backwards collecting the indices of the last N *real* user
|
||||
# messages (exclude display_kind timeline rows — same predicate as
|
||||
# list_recent_user_messages and resume turn counting).
|
||||
# messages (exclude display_kind timeline rows and compaction
|
||||
# handoffs — same predicate as list_recent_user_messages, resume
|
||||
# turn counting, and /retry; #80622).
|
||||
from agent.context_compressor import is_user_originated_turn
|
||||
|
||||
user_indices = []
|
||||
for i in range(len(self.conversation_history) - 1, -1, -1):
|
||||
msg = self.conversation_history[i]
|
||||
if msg.get("role") == "user" and not msg.get("display_kind"):
|
||||
if is_user_originated_turn(msg):
|
||||
user_indices.append(i)
|
||||
if len(user_indices) >= n:
|
||||
break
|
||||
|
|
|
|||
|
|
@ -2572,9 +2572,15 @@ class GatewaySlashCommandsMixin:
|
|||
# and re-sent opaque bookkeeping text (same class as the TUI ordinal).
|
||||
last_user_msg = None
|
||||
last_user_idx = None
|
||||
# is_user_originated_turn: excludes display_kind bookkeeping AND
|
||||
# compaction handoffs (durable role=user, sometimes without
|
||||
# display_kind on legacy sessions; #80622) — /retry must never
|
||||
# re-send a reference-only summary as if the user asked it.
|
||||
from agent.context_compressor import is_user_originated_turn
|
||||
|
||||
for i in range(len(history) - 1, -1, -1):
|
||||
msg = history[i]
|
||||
if msg.get("role") == "user" and not msg.get("display_kind"):
|
||||
if is_user_originated_turn(msg):
|
||||
last_user_msg = msg.get("content", "")
|
||||
last_user_idx = i
|
||||
break
|
||||
|
|
|
|||
|
|
@ -623,11 +623,15 @@ class CLIAgentSetupMixin:
|
|||
self._resume_display_history = [
|
||||
m for m in display_history if m.get("role") != "session_meta"
|
||||
]
|
||||
from agent.context_compressor import is_user_originated_turn
|
||||
|
||||
# Count only user-originated turns (#80622): legacy compaction
|
||||
# handoffs are durable role=user rows without display_kind.
|
||||
msg_count = len(
|
||||
[
|
||||
m
|
||||
for m in self._resume_display_history
|
||||
if m.get("role") == "user" and not m.get("display_kind")
|
||||
if is_user_originated_turn(m)
|
||||
]
|
||||
)
|
||||
title_part = ""
|
||||
|
|
|
|||
|
|
@ -1110,7 +1110,11 @@ class CLICommandsMixin:
|
|||
pass
|
||||
|
||||
title_part = f" \"{session_meta['title']}\"" if session_meta.get("title") else ""
|
||||
msg_count = len([m for m in self._resume_display_history if m.get("role") == "user" and not m.get("display_kind")])
|
||||
from agent.context_compressor import is_user_originated_turn
|
||||
|
||||
# Count only user-originated turns (#80622): legacy compaction
|
||||
# handoffs are durable role=user rows without display_kind.
|
||||
msg_count = len([m for m in self._resume_display_history if is_user_originated_turn(m)])
|
||||
if self.conversation_history:
|
||||
_cprint(
|
||||
f" ↻ Resumed session {target_id}{title_part}"
|
||||
|
|
|
|||
|
|
@ -1298,12 +1298,17 @@ def _(rid, params: dict) -> dict:
|
|||
removed = 0
|
||||
with session["history_lock"]:
|
||||
history = session.get("history", [])
|
||||
# Truncate from the last *real* user turn (no display_kind).
|
||||
# Same predicate as list_recent_user_messages / /undo / /retry.
|
||||
# Truncate from the last *real* user turn. Same predicate
|
||||
# as list_recent_user_messages / /undo / /retry —
|
||||
# is_user_originated_turn also excludes compaction
|
||||
# handoffs (durable role=user, sometimes without
|
||||
# display_kind on legacy sessions; #80622).
|
||||
from agent.context_compressor import is_user_originated_turn
|
||||
|
||||
last_user_idx = None
|
||||
for i in range(len(history) - 1, -1, -1):
|
||||
msg = history[i]
|
||||
if msg.get("role") == "user" and not msg.get("display_kind"):
|
||||
if is_user_originated_turn(msg):
|
||||
last_user_idx = i
|
||||
break
|
||||
if last_user_idx is not None:
|
||||
|
|
|
|||
Loading…
Reference in New Issue