Conversation
jamadeo
requested review from
DOsinga and
katzdave
and removed request for
katzdave
| format!( | ||
| "{}\nThe last user message was:\n\n{}", | ||
| TOOL_LOOP_CONTINUATION_TEXT, | ||
| user_message.as_concat_text() |
There was a problem hiding this comment.
seems like a good way to include the last user message 👍
| &messages[..messages.len() - 1], | ||
| vec![conversation_continuation_message(), message.clone()], | ||
| ), | ||
| Some(message) if is_tool_response_message(message) => { |
There was a problem hiding this comment.
This check on is_tool_response_message feels a little iffy to me.
For manual compact, I guess it won't happen under normal circumstances, but is that guaranteed for the case where the agent loop is running and runs out of context?
Would consider passing a boolean is_manual_compact passed into compact_messages.
| } | ||
| let mut new_messages = vec![summary_msg]; | ||
| new_messages.extend(continuation_messages); | ||
| let (new_messages, _issues) = merge_consecutive_messages(new_messages); |
There was a problem hiding this comment.
Nice use of merge messages. What should we do with issues? Ideally there are none, but would be nice to know.
|
FYI, noticed one more issue: Since I changed manual compaction to actually send a message, we're now picking up that message (as the most recent user message) and moving it after the compaction. Maybe another argument for just passing in a manual_compaction flag and skipping the whole last user search entirely; I think otherwise we always just want to find the most recent user message and might not need to special case the last one. |
|
should we close this one now? or do we need to copy the test? |
Yeah closing. Moved the test to #5618 Still porting a little more functionality there |
3 participants
No description provided.