Skip to content

fix(context): inject memory attachments into the newest user message - #689

Open
wcy12378 wants to merge 1 commit into
OpenBMB:mainfrom
wcy12378:codex/fix-memory-injection-tail
Open

wcy12378 wants to merge 1 commit into
OpenBMB:mainfrom
wcy12378:codex/fix-memory-injection-tail

Conversation

@wcy12378

Copy link
Copy Markdown

Summary

Per-turn memory retrieval results were pushed into the system prompt (DefaultContextRuntime.prepareForModel), so the head of the context changed on every turn and OpenAI-protocol providers could never reuse their implicit prompt/KV cache — every "continuation" turn recomputed the whole prefix.

This moves the retrieval output out of the system prompt and attaches it to the newest non-synthetic user message (prepended as its own text block). Dynamic content now rides at the tail of the context; the system prompt and the entire message history stay byte-stable across turns.

Closes #630.

Why this shape

  • It matches the delivery position the codebase already uses after compaction: buildPostCompactMessages pushes result.attachments (the same MemoryAttachmentBuilder output, wrapped in <memory-context>) into the message stream, not the system prompt.
  • It follows the same tail-stability rule as the <date-update> notices, which are appended at the tail specifically to preserve the cached prefix.
  • Synthetic user messages (e.g. date_update) are skipped, so a date rollover cannot capture the memory block; injection then lands on the last real user message.
  • When the conversation has no real user message to attach to, the old behavior (system prompt) is kept as a fallback so memory is never silently dropped.
  • The attachment text already carries the <memory-context> wrapper from MemoryAttachmentBuilder, so no second wrapper is added.
  • The target message is cloned before mutation; caller-owned message objects (which the agent loop persists) are never touched.

Tests

New regression coverage in tests/context/cache-runtime.spec.ts:

  1. memory attachments ride in the newest user message instead of the system prompt — system prompt (and systemPromptParts) contain no memory text; the newest real user message starts with the <memory-context> block; earlier messages are untouched.
  2. memory injection keeps the system prompt byte-stable across turns — with retrieval output changing between turns, systemPrompt is byte-identical, the shared history prefix is unchanged, and the new turn carries the new memory; also asserts the caller's message object is not mutated in place.
  3. memory injection skips synthetic user messages and falls back to the system prompt — synthetic-only conversations keep the old system-prompt delivery; a synthetic tail message does not capture the block.

Validation (Windows, Node v24.15.0, run with node --import tsx --test):

  • Baseline (main 1732d07, new tests only): tests/context/*.spec.ts → 85 tests, 79 pass / 6 fail — the 3 new tests fail against the old code, reproducing the issue.
  • With this fix: 85 tests, 82 pass / 3 fail. The 3 remaining failures (tool text over token budget is persisted with expanded grep-first preview, large tool error references preserve error semantics for model replay, multibyte truncated tool result references advertise read_file access) fail identically on unmodified main on Windows — they compare /refs/... style paths against backslash paths and are pre-existing environment issues unrelated to this change.
  • tests/agent/loop/model-override-defaults.spec.ts + tests/web/fork-compact-snapshot.spec.ts: 7/7 pass.
  • pnpm exec tsc -p tsconfig.json --noEmit: clean.

Per-turn memory retrieval results were pushed into the system prompt,
which forks the context prefix at its first byte and defeats implicit
prompt caching on OpenAI-protocol providers. Attach the retrieval
output to the newest non-synthetic user message instead - the same
message-stream position buildPostCompactMessages already uses for
post-compact memory re-injection - so the system prompt and message
history stay byte-stable across turns. Fall back to the system prompt
when the conversation has no real user message to attach to.

Closes OpenBMB#630.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] Memory retrieval injected into the system prompt head breaks prefix caching on OpenAI-protocol providers

1 participant