fix(agent): persist the multimodal pre_llm_call text part and stop double injection (#71998)

Builds on #72026 (@PRATHAMESH75): list content carries the turn's memory-prefetch /
pre_llm_call context as a durable text part appended once in the prologue, in every
api mode (MoA and codex_app_server included), so the request, the persisted row,
compaction and a later resume all see the message the model saw.

Persistence gap from the #72026 review: in-place preflight compaction (and a
close/early flush that races the prologue) writes the current user row BEFORE the
part exists and the crash persist identity-skips that dict, so a resumed session
replayed the turn without the context. The list branch now pushes the appended part
into that row via set_user_message_content under the same _row_id-under-lock
protocol as the string sidecar backfill, keeping the writer's shape (compaction: raw
parts; flush: text projection).

Titling moves before the injection step so a list turn's title is derived from the
user's ask, not the injected tail. Tests trimmed to one invariant per layer: hook
edit reaches the wire on a list turn and replays after reload; in-place compaction
+ reload keeps the part (red without the backfill); memory query flattens parts.
This commit is contained in:
teknium1
2026-09-22 00:45:33 -07:00
committed by Teknium
parent d1267d8045
commit bb87e6abce
3 changed files with 136 additions and 170 deletions

View File

@@ -79,72 +79,29 @@ def _agent_stale_thinking_on_wire(agent: Any) -> bool:
return True
def _context_injection_parts(
ext_prefetch_cache: str,
plugin_user_context: str,
) -> list[str]:
"""The ephemeral context pieces (memory prefetch + ``pre_llm_call``).
Shared by the string sidecar (:func:`compose_user_api_content`) and the
multimodal text-part (:func:`compose_multimodal_context_part`) paths so
both inject byte-identical context regardless of the turn's content shape.
"""
injections: list[str] = []
if ext_prefetch_cache:
fenced = build_memory_context_block(ext_prefetch_cache)
if fenced:
injections.append(fenced)
if plugin_user_context:
injections.append(plugin_user_context)
return injections
def compose_multimodal_context_part(
ext_prefetch_cache: str, plugin_user_context: str,
) -> Optional[str]:
"""The ephemeral context of one turn (memory prefetch + ``pre_llm_call``) as one text
block; ``None`` when nothing is injected. The string sidecar appends it to ``content``;
a multimodal (list) turn carries it as a durable text part (#71998)."""
fenced = build_memory_context_block(ext_prefetch_cache) if ext_prefetch_cache else ""
injections = [part for part in (fenced, plugin_user_context) if part]
return "\n\n".join(injections) if injections else None
def compose_user_api_content(
content: Any, ext_prefetch_cache: str, plugin_user_context: str
) -> Optional[str]:
"""Compose the API-bound content of the current turn's user message.
"""Compose the API-bound content of the current turn's string user message.
Sources: memory-manager prefetch + ``pre_llm_call`` plugin context with
target="user_message" (the default). Both are appended to the *API copy*
of the user message only — the stored content stays clean.
This is the single source of that composition. The prologue stamps the
result onto the live message as ``api_content`` (persisted alongside the
clean content) and the ``api_messages`` build in ``conversation_loop``
sends the same helper's output, so the persisted sidecar can never drift
from the bytes on the wire — which is the whole prompt-cache invariant:
what turn N sends must be what turn N+1 replays.
Returns ``None`` when nothing is injected (multimodal/non-string content,
or no ephemeral context), meaning the message is sent as-is. Multimodal
(list) turns take the injection via :func:`compose_multimodal_context_part`
instead, since the string sidecar can't ride on list content.
"""
Single source for the ``api_content`` sidecar and the wire bytes so they never drift
(what turn N sends is what turn N+1 replays). ``None`` when nothing is injected or the
content is not a string (list content takes the text-part path)."""
if not isinstance(content, str):
return None
injections = _context_injection_parts(ext_prefetch_cache, plugin_user_context)
if not injections:
return None
return content + "\n\n" + "\n\n".join(injections)
def compose_multimodal_context_part(
ext_prefetch_cache: str,
plugin_user_context: str,
) -> Optional[str]:
"""The memory-prefetch + ``pre_llm_call`` context as a single text part.
:func:`compose_user_api_content` returns ``None`` for multimodal (list)
turns, so the string ``api_content`` sidecar can't carry this context and
it would silently drop on image/attachment turns (#71998). Callers append
the returned text as a durable content part instead — the same channel the
gateway must-deliver notes use (:func:`append_notes_to_multimodal_content`)
— so an image-only turn still reaches the model with the injected context.
Returns ``None`` when nothing is injected.
"""
injections = _context_injection_parts(ext_prefetch_cache, plugin_user_context)
return "\n\n".join(injections) if injections else None
injection = compose_multimodal_context_part(ext_prefetch_cache, plugin_user_context)
return None if injection is None else content + "\n\n" + injection
def substitute_api_content(api_msg: Dict[str, Any]) -> Optional[str]:
@@ -189,7 +146,7 @@ def consume_surface_switch_note(agent: Any) -> str:
return _pop_turn_note(agent, "_surface_switch_note")
def append_notes_to_multimodal_content(content: Any, notes: str) -> bool:
def append_notes_to_multimodal_content(content: Any, notes: Optional[str]) -> bool:
"""Append must-deliver notes as a durable text part on a multimodal (list) user
message (the sidecar path returns ``None`` for non-string content)."""
if not notes or not isinstance(content, list):
@@ -885,16 +842,9 @@ def _bind_interrupt_scope(agent: Any, ra) -> None:
def _memory_query_text(original_user_message: Any) -> str:
"""The semantic text of a turn's user content for memory queries.
A multimodal (list) turn carries its text in content parts, so keying the
query off ``isinstance(str)`` alone collapses it to ``""`` — ``on_turn_start``
sees an empty turn and ``is_trivial_prompt("")`` skips ``prefetch_all``
entirely, so memory recall never even runs on an image+text turn. That is the
execution-side twin of the delivery gap this PR (#71998) closes: the sidecar
can now carry recall on a multimodal turn, but only if prefetch produced any.
Flatten str/list to text; an image-only turn still yields ``""`` and is
correctly treated as trivial (no semantic text to query on)."""
"""Semantic text of the turn for memory queries: a multimodal (list) turn carries its text
in parts, so keying off ``isinstance(str)`` collapsed it to ``""`` and ``is_trivial_prompt``
skipped prefetch entirely. An image-only turn still flattens to ``""`` (trivial)."""
if isinstance(original_user_message, (str, list)):
return flatten_message_text(original_user_message)
return ""
@@ -936,26 +886,9 @@ def _stamp_api_content_sidecar(
plugin_user_context: str, *, preflight_compressed: bool,
) -> None:
"""api_content sidecar — persist what you send: injected context lives only in the
API copy, so stamp the exact sent bytes on the live dict for replay.
Multimodal (list) turns can't carry the string ``api_content`` sidecar —
:func:`compose_user_api_content` returns ``None`` for them — so the memory/
plugin context would silently drop on an image-only turn (#71998). For those,
deliver the context as a durable text part instead (the same channel the
gateway must-deliver notes use), keeping the wire byte-identical to what is
persisted and replayed."""
API copy, so stamp the exact sent bytes on the live dict for replay."""
_turn_user_msg = messages[current_turn_user_idx]
live_content = _turn_user_msg.get("content")
# Multimodal (list) turns can't carry the string ``api_content`` sidecar —
# :func:`compose_user_api_content` returns ``None`` for them — so the memory/
# plugin context would silently drop on an image-only turn (#71998). Deliver it
# as a durable text part instead (the same channel the gateway must-deliver notes
# use), which is persisted and replayed as ordinary content — no sidecar needed.
if isinstance(live_content, list):
_mm_ctx = compose_multimodal_context_part(ext_prefetch_cache, plugin_user_context)
if _mm_ctx:
append_notes_to_multimodal_content(live_content, _mm_ctx)
return
from agent.session_persistence import _persist_lock, durable_user_row_content
# Match the row the flush wrote (persist override = clean transcript), not the live bytes.
durable_content, _api_content = durable_user_row_content(
@@ -995,6 +928,38 @@ def _stamp_api_content_sidecar(
logger.warning("api_content backfill failed for session=%s", agent.session_id or "none", exc_info=True)
def _append_multimodal_context(
agent: Any, turn_user_msg: Dict[str, Any], ext_prefetch_cache: str, plugin_user_context: str,
*, preflight_compressed: bool,
) -> None:
"""Multimodal (list) content takes no string sidecar: the turn's context becomes a durable
text part on the current turn's live list (the gateway must-deliver-note channel, #71998),
so wire, persisted row, compaction and replay all carry the same parts. Runs once per turn,
before the first request; historical rows are never touched.
A user row another writer materialized BEFORE the prologue (in-place preflight compaction,
a close/early flush that raced it) is updated in place: the crash persist marker-skips that
message, so without this a resumed session replays a view the model never saw. Same
``_row_id``-under-lock protocol as the string sidecar backfill; the row keeps its writer's
shape (compaction inserted the raw parts, a flush the text projection)."""
_mm_ctx = compose_multimodal_context_part(ext_prefetch_cache, plugin_user_context)
if not append_notes_to_multimodal_content(turn_user_msg.get("content"), _mm_ctx):
return
from agent.session_persistence import _durable_content, _persist_lock
with _persist_lock(agent):
_row_id = turn_user_msg.get("_row_id")
_db = getattr(agent, "_session_db", None)
if _db is None or not isinstance(_row_id, int):
return
_in_place_compacted = preflight_compressed and bool(getattr(agent, "_last_compaction_in_place", False))
content = turn_user_msg["content"] if _in_place_compacted else _durable_content(turn_user_msg["content"])
try:
_db.set_user_message_content(agent.session_id, _row_id, content)
except Exception:
logger.warning("multimodal context backfill failed for session=%s", agent.session_id or "none", exc_info=True)
def _persist_turn_start(
agent: Any, messages: List[Any], conversation_history: Optional[List[Any]],
pending_cli_message: Any,
@@ -1158,24 +1123,26 @@ def build_turn_context(
_bind_interrupt_scope(agent, ra)
ext_prefetch_cache = _memory_turn_start_and_prefetch(agent, original_user_message, turn_author)
# Sidecar skipped for codex_app_server/MoA.
if (
not moa_active
and getattr(agent, "api_mode", None) != "codex_app_server"
and 0 <= current_turn_user_idx < len(messages)
and messages[current_turn_user_idx].get("role") == "user"
):
_stamp_api_content_sidecar(
agent, messages, current_turn_user_idx, ext_prefetch_cache,
plugin_user_context, preflight_compressed=compaction.compressed,
)
# Title the session now: titling depends only on the user's ask (before any injected
# context lands on list content), so it runs concurrently with the turn. Daemon thread,
# no-op once titled; it ensures the session row itself.
_maybe_title_session_at_turn_start(agent, messages)
# Sidecar skipped for codex_app_server/MoA; list content carries its context as a part in every mode.
if 0 <= current_turn_user_idx < len(messages) and messages[current_turn_user_idx].get("role") == "user":
if isinstance(messages[current_turn_user_idx].get("content"), list):
_append_multimodal_context(
agent, messages[current_turn_user_idx], ext_prefetch_cache, plugin_user_context,
preflight_compressed=compaction.compressed,
)
elif not moa_active and getattr(agent, "api_mode", None) != "codex_app_server":
_stamp_api_content_sidecar(
agent, messages, current_turn_user_idx, ext_prefetch_cache,
plugin_user_context, preflight_compressed=compaction.compressed,
)
_persist_turn_start(agent, messages, conversation_history, pending_cli_message)
# Title the session now: the row exists and titling depends only on the user's ask,
# so it runs concurrently with the turn. Daemon thread, no-op once titled.
_maybe_title_session_at_turn_start(agent, messages)
return TurnContext(
user_message=user_message, original_user_message=original_user_message, messages=messages,
conversation_history=conversation_history, active_system_prompt=active_system_prompt,

View File

@@ -57,67 +57,27 @@ class TestComposeUserApiContent:
class TestComposeMultimodalContextPart:
"""#71998: the injection composed as a standalone text part for the
multimodal (list) content path, mirroring the string sidecar's sources."""
def test_none_when_nothing_to_inject(self):
def test_is_the_string_sidecar_injection_tail(self):
"""Both content shapes inject byte-identical context (#71998): the text part a list
turn carries is exactly what the string sidecar appends after ``content``."""
assert compose_multimodal_context_part("", "") is None
def test_plugin_context_only(self):
assert compose_multimodal_context_part("", "CTX") == "CTX"
def test_memory_block_and_plugin_context(self):
out = compose_multimodal_context_part("likes tea", "CTX")
fenced = build_memory_context_block("likes tea")
assert out == fenced + "\n\n" + "CTX"
def test_matches_string_sidecar_injection_tail(self):
# The multimodal part is exactly the string sidecar minus the leading
# content + separator, so both paths inject byte-identical context.
sidecar = compose_user_api_content("hello", "likes tea", "CTX")
part = compose_multimodal_context_part("likes tea", "CTX")
assert sidecar == "hello\n\n" + part
class TestMemoryQueryText:
"""#71998 execution side: the memory query must flatten multimodal (list)
content to its text, or prefetch/on_turn_start never run on an image+text
turn and the delivery sidecar has nothing to carry."""
def test_str_passthrough(self):
assert _memory_query_text("what is my address") == "what is my address"
def test_text_plus_image_list_yields_text(self):
content = [
{"type": "text", "text": "what is in this photo of my house"},
{"type": "image_url", "image_url": {"url": "data:image/png;base64,AAAA"}},
]
assert _memory_query_text(content) == "what is in this photo of my house"
def test_image_only_list_yields_empty(self):
content = [
{"type": "image_url", "image_url": {"url": "data:image/png;base64,AAAA"}},
]
assert _memory_query_text(content) == ""
def test_non_text_types_yield_empty(self):
assert _memory_query_text(None) == ""
assert _memory_query_text(12345) == ""
def test_drives_trivial_prompt_gate_correctly(self):
def test_list_turn_queries_its_text_and_image_only_stays_trivial(self):
"""#71998 execution side: a text+image turn must drive prefetch off its text (it used to
collapse to ``""`` and skip recall silently); an image-only turn has no text to query."""
from agent.memory_provider import is_trivial_prompt
text_plus_image = [
{"type": "text", "text": "remind me what my dog's name is"},
{"type": "image_url", "image_url": {"url": "data:image/png;base64,AAAA"}},
]
image_only = [
{"type": "image_url", "image_url": {"url": "data:image/png;base64,AAAA"}},
]
# A text+image turn is NOT trivial once flattened, so prefetch runs...
image = {"type": "image_url", "image_url": {"url": "data:image/png;base64,AAAA"}}
text_plus_image = [{"type": "text", "text": "remind me what my dog's name is"}, image]
assert _memory_query_text(text_plus_image) == "remind me what my dog's name is"
assert is_trivial_prompt(_memory_query_text(text_plus_image)) is False
# ...while an image-only turn stays trivial (no semantic text to query).
assert is_trivial_prompt(_memory_query_text(image_only)) is True
assert _memory_query_text([image]) == ""
assert is_trivial_prompt(_memory_query_text([image])) is True
# ---------------------------------------------------------------------------
@@ -376,20 +336,6 @@ class TestPrologueStamping:
# Multimodal turns carry the context durably, not via the string sidecar.
assert "api_content" not in ctx.messages[ctx.current_turn_user_idx]
def test_no_context_part_for_multimodal_without_injections(self):
"""A multimodal turn with no ephemeral context is left untouched."""
agent = _FakeAgent()
blocks = [{"type": "image_url", "image_url": {"url": "data:img"}}]
with patch("hermes_cli.plugins.invoke_hook", return_value=[]):
ctx = _build(
agent,
user_message=blocks,
summarize_user_message_for_log=lambda _m: "[image]",
)
content = ctx.messages[ctx.current_turn_user_idx]["content"]
assert content == blocks # no stray text part appended
assert "api_content" not in ctx.messages[ctx.current_turn_user_idx]
# ---------------------------------------------------------------------------
# Flush: persist-override rows keep the sent bytes in the sidecar (#48677)
@@ -655,6 +601,32 @@ class TestWireInvariant:
current = _user_messages(_chat_requests(handler)[0])[-1]
assert current["content"] == "second question\n\nPLUGIN-CTX"
def test_multimodal_turn_sends_persists_and_replays_context_part(self, wire_env):
"""#71998: on a list-content (image) turn the ``pre_llm_call`` context reaches the
wire as a text part, the persisted row carries it, and a resumed turn N+1 replays
the same view — same contract as the string sidecar path."""
make_agent, handler, db, sid = wire_env
from run_agent import AIAgent
image = {"type": "image_url", "image_url": {"url": "data:image/png;base64,AAAA"}}
turn = [{"type": "text", "text": "what is this"}, image]
agent1 = make_agent()
with patch.object(AIAgent, "_model_supports_vision", return_value=True): # keep native parts on the wire
agent1.run_conversation(list(turn), conversation_history=[], task_id="t1")
sent = _user_messages(_chat_requests(handler)[0])[0]["content"]
assert sent == [*turn, {"type": "text", "text": "PLUGIN-CTX"}]
history = db.get_messages_as_conversation(sid)
assert "PLUGIN-CTX" in history[0]["content"] # persisted with the turn, not dropped
handler.captured_requests = []
agent2 = make_agent()
with patch.object(AIAgent, "_model_supports_vision", return_value=True):
agent2.run_conversation("second question", conversation_history=history, task_id="t2")
replayed = _user_messages(_chat_requests(handler)[0])[0]["content"]
assert replayed == history[0]["content"]
# ---------------------------------------------------------------------------
# Review fixes: re-anchoring, MoA, in-place compaction backfill, override
@@ -1013,7 +985,7 @@ class TestSessionRowExistsBeforePreflightCompaction:
before the delayed persist. Drives the real ``compress_context`` path
against a real, empty SessionDB."""
def _make_agent(self, db, sid, *, in_place):
def _make_agent(self, db, sid, *, in_place, current_user_content="hello"):
from run_agent import AIAgent
with patch.dict(os.environ, {"OPENROUTER_API_KEY": "test-key"}):
@@ -1044,7 +1016,7 @@ class TestSessionRowExistsBeforePreflightCompaction:
seen = {}
compacted = [
{"role": "assistant", "content": "[CONTEXT COMPACTION] summary"},
{"role": "user", "content": "hello"},
{"role": "user", "content": current_user_content},
]
def _compress(_messages, **_kwargs):
@@ -1101,6 +1073,31 @@ class TestSessionRowExistsBeforePreflightCompaction:
finally:
db.close()
def test_in_place_compaction_multimodal_context_part_survives_reload(self, tmp_path):
"""#71998 persistence: in-place ``archive_and_compact`` writes the current-turn user
row BEFORE the prologue appends the ``pre_llm_call`` text part, and the crash persist
identity-skips compacted dicts — the part must be pushed into that row, or a
resumed session replays a view the model never saw."""
db = SessionDB(db_path=tmp_path / "state.db")
sid = "sess-inplace-mm"
image = {"type": "image_url", "image_url": {"url": "data:image/png;base64,AAAA"}}
turn = [{"type": "text", "text": "what is this"}, image]
try:
agent, _seen = self._make_agent(db, sid, in_place=True, current_user_content=list(turn))
with patch("hermes_cli.plugins.invoke_hook", return_value=[{"context": "PLUGIN-CTX"}]):
ctx = _build(
agent, user_message=list(turn), conversation_history=self._oversized_history(),
summarize_user_message_for_log=lambda _m: "[image]",
)
assert agent._last_compaction_in_place is True
live = ctx.messages[ctx.current_turn_user_idx]["content"]
assert live == [*turn, {"type": "text", "text": "PLUGIN-CTX"}]
# Reload: the durable row carries the same parts the model saw.
reloaded = [m for m in db.get_messages_as_conversation(sid) if m["role"] == "user"]
assert reloaded[-1]["content"] == live
finally:
db.close()
def test_rotation_first_turn_compaction_creates_child(self, tmp_path):
db = SessionDB(db_path=tmp_path / "state.db")
sid = "sess-fresh-rot"

View File

@@ -682,7 +682,7 @@ def my_callback(session_id: str, user_message: str, conversation_history: list,
| Parameter | Type | Description |
|-----------|------|-------------|
| `session_id` | `str` | Unique identifier for the current session |
| `user_message` | `str` | The user's original message for this turn (before any skill injection) |
| `user_message` | `str \| list` | The user's original message for this turn (before any skill injection). A multimodal turn (image or other attachment) is the list of content parts, exactly as sent |
| `conversation_history` | `list` | Copy of the full message list (OpenAI format: `[{"role": "user", "content": "..."}]`) |
| `is_first_turn` | `bool` | `True` if this is the first turn of a new session, `False` on subsequent turns |
| `model` | `str` | The model identifier (e.g. `"anthropic/claude-sonnet-4.6"`) |
@@ -707,6 +707,8 @@ return None
The clean user-message `content` remains unchanged. For replay and prompt-cache stability, Hermes may persist the exact API-bound message, including plugin-injected context, in the row's `api_content` sidecar.
On a **multimodal turn** (the user message is a list of content parts — an image attachment, or text sent as parts) there is no string sidecar: the joined context is appended to that turn's content as one extra `{"type": "text"}` part, before the first request, and the part is persisted with the turn so a resumed session, compaction and replay all see the same message the model saw. Earlier messages and the system prompt are never touched.
When **multiple plugins** return context, their outputs are joined with double newlines in plugin discovery order (alphabetical by directory name).
**Use cases:** Memory recall, RAG context injection, guardrails, per-turn analytics.