-
Notifications
You must be signed in to change notification settings - Fork 19.4k
fix(core): remove orphaned ToolMessages in trim_message #33265
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix(core): remove orphaned ToolMessages in trim_message #33265
Conversation
CodSpeed Performance ReportMerging #33265 will not alter performanceComparing
|
|
Hi @yashv6655! Thank you for the PR. I haven't reviewed in detail yet, but noticed that the issue is missing a parameter If you look at the how-to docs: https://python.langchain.com/docs/how_to/trim_messages/#trimming-based-on-message-count We recommend adding: Could you confirm whether this resolves the issue for you? I'm basically wondering whetherthis is a bug vs. a devx issue (i.e., the API isn't intuitive) |
|
@eyurtsev Thanks for the feedback! You're right that However, I believe the fix is still needed: 1. The parameter is optionalThe bug report used: trim_messages(messages, strategy="last", token_counter=len, max_tokens=5)The docs recommend 2.
|
|
@eyurtsev could you please take a look and give feedback on the PR? |
Description
Fixes a bug where
trim_messageswithstrategy="last"could create invalid message histories by orphaningToolMessages when their correspondingAIMessagewithtool_callswas trimmed away.Issue
When trimming message history, if a
ToolMessagewas included in the trimmed result but its correspondingAIMessage(containing the tool call that theToolMessageresponds to) was removed, this created an orphanedToolMessagewith atool_call_idthat references a non-existent tool call. This invalid message history would be rejected by most LLM APIs.Fix
Added a
_remove_orphaned_tool_messages()helper function that:tool_call_ids fromAIMessagesToolMessages whosetool_call_iddoesn't match a valid tool callToolMessages removedThis function is called in
_first_max_tokens()before returning, which fixes bothstrategy="first"andstrategy="last"(since "last" internally uses "first" with reversed messages).Example
Before (broken):
After (fixed):
Issue
Resolves #33245
Dependencies
None - this is a pure bug fix with no new dependencies.
Testing