refactor(turn_finalizer): extract _is_pure_tool_call_tail, fix SQLite durability
Extract the inline pure-tool-call tail check to a named helper using flatten_message_text (canonical content extraction). Fix a SQLite durability regression: the incremental tool-call persist (conversation_loop.py:4990) stamps _DB_PERSISTED_MARKER on the assistant row, so the next _persist_session flush skips it — the filled content reaches the in-memory transcript but NOT the durable store, and /resume reloads content="". Pop the marker so the next flush re-writes the row. Tests pass, ruff clean.
This commit is contained in:
parent
56ac96976b
commit
71157cbf66
|
|
@ -25,6 +25,21 @@ from __future__ import annotations
|
|||
import os
|
||||
|
||||
from agent.codex_responses_adapter import _summarize_user_message_for_log
|
||||
from agent.message_content import flatten_message_text
|
||||
|
||||
|
||||
def _is_pure_tool_call_tail(msg: dict) -> bool:
|
||||
"""An assistant row with ``tool_calls`` but no visible text content of its own.
|
||||
|
||||
Such a row satisfies the role check (``tail role == "assistant"``) while
|
||||
carrying none of the delivered answer — see the #43849/#44100 invariant
|
||||
block in :func:`finalize_turn`. Uses :func:`flatten_message_text` so that
|
||||
multimodal (list-type) content is evaluated by its text parts, not just
|
||||
its type.
|
||||
"""
|
||||
if not msg.get("tool_calls"):
|
||||
return False
|
||||
return not flatten_message_text(msg.get("content")).strip()
|
||||
|
||||
|
||||
def finalize_turn(
|
||||
|
|
@ -228,19 +243,25 @@ def finalize_turn(
|
|||
_tail_role = _tail.get("role") if isinstance(_tail, dict) else None
|
||||
if _tail_role != "assistant":
|
||||
messages.append({"role": "assistant", "content": final_response})
|
||||
elif _tail.get("tool_calls") and not (
|
||||
_tail.get("content") if isinstance(_tail.get("content"), str) else ""
|
||||
).strip():
|
||||
elif isinstance(_tail, dict) and _is_pure_tool_call_tail(_tail):
|
||||
# The tail IS an assistant row, but a *pure tool-call turn*:
|
||||
# tool_calls with no text of its own. It carries none of the
|
||||
# delivered answer, so the role check alone leaves the invariant
|
||||
# unmet — the user saw a response that never reached the
|
||||
# transcript, and the next turn replays the user backlog and
|
||||
# re-answers it (the very symptom this block was added for).
|
||||
# Fill that row's empty content instead of appending, so the
|
||||
# durable turn ends with the answer without disturbing the
|
||||
# tool-call structure or creating an assistant→assistant pair.
|
||||
# tool_calls with no text of its own. The role check alone
|
||||
# leaves the #43849/#44100 invariant unmet — the user saw a
|
||||
# response that never reached the transcript, and the next turn
|
||||
# replays the user backlog and re-answers it (the very symptom
|
||||
# this block was added for). Fill that row's empty content
|
||||
# instead of appending, so the durable turn ends with the answer
|
||||
# without disturbing the tool-call structure or creating an
|
||||
# assistant→assistant pair.
|
||||
_tail["content"] = final_response
|
||||
# The row may have already been flushed to SQLite by the
|
||||
# incremental tool-call persist (conversation_loop.py:4990),
|
||||
# which stamps ``_DB_PERSISTED_MARKER`` so subsequent flushes
|
||||
# skip it. Pop the marker so the next ``_persist_session``
|
||||
# re-writes the filled content to the durable store —
|
||||
# otherwise ``/resume`` reloads ``content=""`` and the bug
|
||||
# resurfaces cross-session.
|
||||
_tail.pop("_db_persisted", None)
|
||||
|
||||
# The model has completed its request, so replace API-local
|
||||
# voice/model/skill guidance with the clean user input before writing the
|
||||
|
|
|
|||
|
|
@ -280,3 +280,51 @@ def test_final_response_does_not_clobber_tool_call_tail_with_text(monkeypatch):
|
|||
)
|
||||
|
||||
assert agent.persisted_messages[-1]["content"] == "partial text"
|
||||
|
||||
|
||||
def test_fill_pops_db_persisted_marker_for_durable_rewrite(monkeypatch):
|
||||
"""The incremental tool-call persist stamps ``_db_persisted`` on the row.
|
||||
|
||||
If finalize_turn fills the tail's content but leaves the marker, the next
|
||||
``_flush_messages_to_session_db`` skips the row and the durable SQLite
|
||||
store keeps ``content=""`` — so ``/resume`` reloads the empty content and
|
||||
the bug resurfaces cross-session. The fix pops the marker so the filled
|
||||
content is re-written.
|
||||
"""
|
||||
agent = FakeAgent()
|
||||
messages = [
|
||||
{"role": "user", "content": "q"},
|
||||
{
|
||||
"role": "assistant",
|
||||
"content": "",
|
||||
"tool_calls": [
|
||||
{"id": "t1", "type": "function",
|
||||
"function": {"name": "f", "arguments": "{}"}}
|
||||
],
|
||||
"_db_persisted": True, # stamped by conversation_loop.py:4990
|
||||
},
|
||||
]
|
||||
|
||||
finalize_turn(
|
||||
agent,
|
||||
final_response="Here is your answer.",
|
||||
api_call_count=3,
|
||||
interrupted=False,
|
||||
failed=False,
|
||||
messages=messages,
|
||||
conversation_history=[],
|
||||
effective_task_id="t",
|
||||
turn_id="tid",
|
||||
user_message="q",
|
||||
original_user_message="q",
|
||||
_should_review_memory=False,
|
||||
_turn_exit_reason="text_response(final)",
|
||||
)
|
||||
|
||||
persisted = agent.persisted_messages
|
||||
assert persisted is not None
|
||||
assert persisted[-1]["content"] == "Here is your answer."
|
||||
assert persisted[-1]["tool_calls"]
|
||||
assert "_db_persisted" not in persisted[-1], (
|
||||
"marker must be popped so the next flush re-writes the filled content"
|
||||
)
|
||||
|
|
|
|||
Loading…
Reference in New Issue