Skip to content

fix(middleware): fill context placeholders once per model call - #1753

Merged
Amir-R25 merged 5 commits into
mainfrom
fix-context-injection-refill
Oct 29, 2026
Merged

Amir-R25 merged 5 commits into
mainfrom
fix-context-injection-refill

Conversation

@Amir-R25

Copy link
Copy Markdown
Collaborator

Summary

ContextInjection filled placeholders in the agent's own message history, in place, before every model call. Once a tool call sent the conversation back to the model, the already-filled messages were filled a second time. So text passed through rt.escape_braces was only delivered as written on the first call, and a context value containing {other_key} was expanded into that key's value on the next call.

Each message used to be marked as filled after the first pass. #1441 removed that marker along with the inject_prompt switch, so this regressed in 1.5.0.

  • ContextInjection now fills a copy of the history and passes that copy on, so the caller's messages are never changed.
  • Added a regression test: an agent with one tool, two model calls, and both cases above.

Type of change

  • Bug fix
  • Feature
  • Breaking change
  • Docs
  • Refactor / chore / build / tests

Checklist

  • Lint & format pass (ruff check . && ruff format .)
  • Tests added/updated and pass locally (pytest tests)
  • Docs updated if user-facing behavior changed
  • Breaking changes include migration notes

Notes

The conversation an agent returns now keeps the placeholders as written (for example {user_name}) rather than the filled-in values. The model and the recorded LLM events still see the filled prompt.

Follow-up: _wire_history in llm_helpers.py still copies the system message and gives in-place filling as the reason; that reason no longer holds.

@Amir-R25
Amir-R25 requested a review from CoronRing October 29, 2026 18:17
@CoronRing

Copy link
Copy Markdown
Collaborator

@Amir-R25

One suggestion before merging:

1. Copy the messages, not the whole history

deepcopy(message_history) runs on every model call and copies everything in the history, including attachments (base64 images, PDFs) and tool results, even though only UserMessage and SystemMessage string content is ever filled. fill_prompt reassigns _content rather than mutating it, so a shallow copy of each message is enough:

filled = inject_context(MessageHistory([copy(m) for m in message_history]))

I swapped this in, and your new test plus the prompt, middleware and built_nodes suites all pass. It also lines up with #1695, which is moving the hot paths off deepcopy because it is slow on large values and crashes on non-copyable ones. _llm_observe already deep-copies the history once per call, so this isn't a new failure mode, just a second full copy per round trip that we don't need.


Agreed on _wire_history: once this land

@Amir-R25
Amir-R25 merged commit 8c965d8 into main Oct 29, 2026
11 checks passed
@Amir-R25
Amir-R25 deleted the fix-context-injection-refill branch October 29, 2026 21:20
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.

2 participants