-
Notifications
You must be signed in to change notification settings - Fork 120
fix: cleanup incomplete tool calls #78
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
base: main
Are you sure you want to change the base?
Conversation
|
@huangyajie Let me try to understand what you mean. When the model intends to call Method A and Method B, suppose it has just finished calling Method A, and then the user interrupts the request. This results in the context containing Request A, Request B, and Result A. If you then send a follow-up query to the model, it may lead to certain errors. 我尝试理解下你的意思,当模型想调用方法a和方法b时,此时刚调用完a,然后用户中断了请求,导致上下文中有请求a、请求b,结果a。再追问发给模型可能导致一些错误。 你是想修复这个问题吗 |
|
@huangyajie How is the performance? It seems that every read or write operation requires an analysis of the entire message content?性能情况怎么样,看起来每次读写都要对整个message内容进行一次分析? |
|
1,我尝试理解下你的意思,当模型想调用方法a和方法b时,此时刚调用完a,然后用户中断了请求,导致上下文中有请求a、请求b,结果a。再追问发给模型可能导致一些错误。 你是想修复这个问题吗 |
| break | ||
|
|
||
| matched_ids: set[str] = set() | ||
| for tool_msg in tool_msgs: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
如果tool和tool_call的数量一样,是不是就没必要一个个去找了?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
数字相等本身不够,仍需确认这些 tool 消息就是紧跟该 assistant 的、且 tool_call_id 对得上。否则可能误把前一次/后一次的 tool 消息算进来,依然会触发 400
整体上看是一个很好的修改,有新的消息及时回复下我哈 |
好的,这2个问题,我再确认下 |
… assistant tool calls along with their triggering user and tool messages to keep history valid
|
@huangyajie 新的代码提交进去了吗,我这边没问题了,准备merge吧 |
|
|
@yangbaofu007 辛苦看看,再点一个approve |
| if tool_call_ids: | ||
| tool_msgs: list[ChatCompletionMessageParam] = [] | ||
| j = i + 1 | ||
| while j < len(self._messages): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
代码嵌套比较深,这一层的while循环应该是可以去掉的,直接用66行while循环来统一迭代
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
内层 while j < len(self._messages) 不是多余的,它和外层 while i < len(self._messages) 作用不同:
a.外层遍历尾段每一条消息,遇到 assistant 才进入检查。
b.内层只在发现带 tool_calls 的 assistant 时,向后连续收集紧跟的 tool 消息,并把索引跳到第一条非 tool 消息(i = j)。这样才能一次性判断这一组调用是否完整。
如果去掉内层、依靠外层循环逐条前进,就无法在一次判断里拿到整组连续的 tool 消息,也无法把 i 一步跳过这些消息,逻辑会变复杂且容易漏判/误删。
|
@huangyajie 这是很好的一个修改,代码实现上可以再优化一下,不用那么深的嵌套。 |
|
@yangbaofu007 你好,虽然存在嵌套,但外层循环的 i 会在内层循环结束后赋值为 j,与共用一个循环是性能一样的。 |



Added automatic cleanup of incomplete tool-call messages. In miloco_server/schema/chat_history_schema.py, ChatHistoryMessages now detects assistant messages with tool_calls that are missing corresponding tool replies (typical when the user hits Stop mid-flow) and drops them before reusing or serializing history. This prevents the LLM from rejecting the conversation with “tool_calls must be followed by tool messages.”