feat(chat): group consecutive tool calls into one summarized chain card (#8995)

Signed-off-by: Taylor Ho <taylorkmho@gmail.com>
Signed-off-by: Matt Toohey <contact@matttoohey.com>
Co-authored-by: Matt Toohey <contact@matttoohey.com>
This commit is contained in:
Taylor Ho
2026-05-05 11:37:13 -07:00
committed by GitHub
parent fb79c7de0c
commit dd95b7bb85
22 changed files with 4229 additions and 333 deletions
File diff suppressed because it is too large Load Diff
+122
View File
@@ -116,12 +116,60 @@ impl ToolRequest {
.and_then(|v| v.as_bool())
.unwrap_or(false)
}
/// Returns the persisted LLM-generated title for this tool call, if any.
/// Set asynchronously by [`crate::acp::server`] after `provider.complete_fast`
/// resolves; survives session reload via SQLite. Falls back to `None` for
/// older sessions that predate persistence — callers should use a deterministic
/// title in that case.
pub fn persisted_title(&self) -> Option<&str> {
self.tool_meta
.as_ref()
.and_then(|v| v.get(TOOL_META_TITLE_KEY))
.and_then(|v| v.as_str())
}
/// Returns the persisted per-chain summary anchored on this tool request,
/// if any. Only the FIRST tool request in a chain (a run of consecutive
/// tool blocks within one assistant message) carries this. See
/// [`crate::acp::server`] for how chains are detected and summarized.
pub fn persisted_chain_summary(&self) -> Option<PersistedChainSummary> {
let obj = self
.tool_meta
.as_ref()
.and_then(|v| v.get(TOOL_META_CHAIN_SUMMARY_KEY))?;
let summary = obj.get("summary").and_then(|v| v.as_str())?.to_string();
let count = obj.get("count").and_then(|v| v.as_u64())?;
if count == 0 {
return None;
}
Some(PersistedChainSummary {
summary,
count: count as usize,
})
}
}
/// A chain summary persisted on the first tool request of a chain.
#[derive(Debug, Clone, PartialEq)]
pub struct PersistedChainSummary {
pub summary: String,
pub count: usize,
}
/// Marker key under `ToolRequest.tool_meta` indicating the tool was already
/// executed externally; the agent loop must skip redispatch.
pub const TOOL_META_EXTERNAL_DISPATCH_KEY: &str = "goose.external_dispatch";
/// Key under `ToolRequest.tool_meta` storing the LLM-generated short title
/// for this tool call. Used to make the title survive session reload.
pub const TOOL_META_TITLE_KEY: &str = "goose.toolSummary.title";
/// Key under `ToolRequest.tool_meta` storing the LLM-generated chain summary
/// for the chain that starts at this tool request. Shape: `{ "summary": String,
/// "count": u64 }`. Only attached to the FIRST tool request in a chain.
pub const TOOL_META_CHAIN_SUMMARY_KEY: &str = "goose.toolChain.summary";
#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)]
#[serde(rename_all = "camelCase")]
#[derive(ToSchema)]
@@ -1635,4 +1683,78 @@ mod tests {
}
}
}
fn make_tool_request(meta: Option<serde_json::Value>) -> super::ToolRequest {
super::ToolRequest {
id: "id-1".to_string(),
tool_call: Ok(CallToolRequestParams::new("test_tool")),
metadata: None,
tool_meta: meta,
}
}
#[test]
fn persisted_title_returns_none_when_meta_missing() {
let req = make_tool_request(None);
assert_eq!(req.persisted_title(), None);
}
#[test]
fn persisted_title_returns_value_when_present() {
let meta = serde_json::json!({
super::TOOL_META_TITLE_KEY: "reading project configuration",
});
let req = make_tool_request(Some(meta));
assert_eq!(req.persisted_title(), Some("reading project configuration"));
}
#[test]
fn persisted_title_returns_none_for_non_string_value() {
let meta = serde_json::json!({ super::TOOL_META_TITLE_KEY: 42 });
let req = make_tool_request(Some(meta));
assert_eq!(req.persisted_title(), None);
}
#[test]
fn persisted_title_does_not_collide_with_external_dispatch() {
let meta = serde_json::json!({
super::TOOL_META_EXTERNAL_DISPATCH_KEY: true,
super::TOOL_META_TITLE_KEY: "running commands",
});
let req = make_tool_request(Some(meta));
assert!(req.is_externally_dispatched());
assert_eq!(req.persisted_title(), Some("running commands"));
}
#[test]
fn persisted_chain_summary_round_trips() {
let meta = serde_json::json!({
super::TOOL_META_CHAIN_SUMMARY_KEY: {
"summary": "applied dark mode polish",
"count": 4,
},
});
let req = make_tool_request(Some(meta));
let summary = req.persisted_chain_summary().expect("summary present");
assert_eq!(summary.summary, "applied dark mode polish");
assert_eq!(summary.count, 4);
}
#[test]
fn persisted_chain_summary_returns_none_for_missing_or_zero_count() {
let req = make_tool_request(None);
assert!(req.persisted_chain_summary().is_none());
let meta_zero = serde_json::json!({
super::TOOL_META_CHAIN_SUMMARY_KEY: { "summary": "x", "count": 0 },
});
let req_zero = make_tool_request(Some(meta_zero));
assert!(req_zero.persisted_chain_summary().is_none());
let meta_no_summary = serde_json::json!({
super::TOOL_META_CHAIN_SUMMARY_KEY: { "count": 3 },
});
let req_no_summary = make_tool_request(Some(meta_no_summary));
assert!(req_no_summary.persisted_chain_summary().is_none());
}
}
+515 -1
View File
@@ -1,5 +1,5 @@
use super::session_manager::{role_to_string, SessionStorage};
use crate::conversation::message::Message;
use crate::conversation::message::{Message, MessageContent};
use anyhow::Result;
use chrono::{DateTime, Utc};
use rmcp::model::Role;
@@ -378,6 +378,73 @@ impl ThreadManager {
self.get_thread(&new_id).await
}
/// Merge a JSON object patch into the `tool_meta` of the `ToolRequest` whose
/// `id == tool_call_id` inside the message identified by `(thread_id,
/// message_id)`. Existing keys in `tool_meta` are preserved.
///
/// No-ops (returns `Ok(())`) if the row containing the tool request can't
/// be found — callers (e.g. async title tasks) treat persistence as
/// best-effort.
///
/// `message_id` is used as a coarse filter, but multiple `thread_messages`
/// rows can share the same `message_id` when the agent splits a single
/// LLM response (e.g. text + tool_request) into separate
/// `AgentEvent::Message` events. We disambiguate by walking the matching
/// rows and picking the one whose content actually contains a
/// `ToolRequest` with `tool_call_id`, then update only that row by its
/// auto-incremented primary key. Without this, the title for the first
/// tool in such a split message never persists, because `fetch_optional`
/// returns the text-only row first and finds no matching tool call.
pub async fn update_tool_request_meta(
&self,
thread_id: &str,
message_id: &str,
tool_call_id: &str,
patch: serde_json::Value,
) -> Result<()> {
let pool = self.storage.pool().await?;
let mut tx = pool.begin_with("BEGIN IMMEDIATE").await?;
let rows = sqlx::query_as::<_, (i64, String)>(
"SELECT id, content_json FROM thread_messages \
WHERE thread_id = ? AND message_id = ? \
ORDER BY id ASC",
)
.bind(thread_id)
.bind(message_id)
.fetch_all(&mut *tx)
.await?;
for (row_id, content_json) in rows {
let mut content: Vec<MessageContent> = serde_json::from_str(&content_json)?;
let mut found = false;
for block in &mut content {
if let MessageContent::ToolRequest(tr) = block {
if tr.id == tool_call_id {
tr.tool_meta = Some(merge_tool_meta(tr.tool_meta.take(), &patch));
found = true;
break;
}
}
}
if !found {
continue;
}
let updated_json = serde_json::to_string(&content)?;
sqlx::query("UPDATE thread_messages SET content_json = ? WHERE id = ?")
.bind(updated_json)
.bind(row_id)
.execute(&mut *tx)
.await?;
tx.commit().await?;
return Ok(());
}
tx.commit().await?;
Ok(())
}
pub async fn list_messages(&self, thread_id: &str) -> Result<Vec<Message>> {
let pool = self.storage.pool().await?;
let rows = sqlx::query_as::<_, (Option<String>, String, Option<String>, String, i64, String)>(
@@ -431,3 +498,450 @@ fn append_text_json(content_json: &str, new_text: &str) -> anyhow::Result<String
}
Ok(serde_json::to_string(&items)?)
}
/// Merge a JSON object `patch` into an existing optional object value,
/// preserving keys not present in the patch. Non-object values are replaced.
fn merge_tool_meta(
existing: Option<serde_json::Value>,
patch: &serde_json::Value,
) -> serde_json::Value {
let mut base = match existing {
Some(serde_json::Value::Object(map)) => map,
_ => serde_json::Map::new(),
};
if let serde_json::Value::Object(patch_map) = patch {
for (k, v) in patch_map {
base.insert(k.clone(), v.clone());
}
}
serde_json::Value::Object(base)
}
#[cfg(test)]
mod tests {
use super::*;
use crate::conversation::message::{
Message, MessageContent, ToolRequest, TOOL_META_CHAIN_SUMMARY_KEY,
TOOL_META_EXTERNAL_DISPATCH_KEY, TOOL_META_TITLE_KEY,
};
use crate::session::SessionManager;
use rmcp::model::CallToolRequestParams;
use tempfile::TempDir;
fn assistant_message_with_tool_request(
tool_id: &str,
tool_meta: Option<serde_json::Value>,
) -> Message {
let tool_request = ToolRequest {
id: tool_id.to_string(),
tool_call: Ok(CallToolRequestParams::new("developer__shell")),
metadata: None,
tool_meta,
};
Message::new(
Role::Assistant,
chrono::Utc::now().timestamp_millis(),
vec![MessageContent::ToolRequest(tool_request)],
)
}
async fn fresh_thread_manager(temp: &TempDir) -> Arc<ThreadManager> {
let session_manager = SessionManager::new(temp.path().to_path_buf());
Arc::new(ThreadManager::new(session_manager.storage().clone()))
}
#[tokio::test]
async fn update_tool_request_meta_sets_title_when_missing() {
let temp = TempDir::new().unwrap();
let mgr = fresh_thread_manager(&temp).await;
let thread = mgr.create_thread(None, None, None).await.unwrap();
let stored = mgr
.append_message(
&thread.id,
None,
&assistant_message_with_tool_request("tc-1", None),
)
.await
.unwrap();
let message_id = stored.id.clone().unwrap();
mgr.update_tool_request_meta(
&thread.id,
&message_id,
"tc-1",
serde_json::json!({ TOOL_META_TITLE_KEY: "reading config" }),
)
.await
.unwrap();
let messages = mgr.list_messages(&thread.id).await.unwrap();
let req = match &messages[0].content[0] {
MessageContent::ToolRequest(r) => r,
_ => panic!("expected tool request"),
};
assert_eq!(req.persisted_title(), Some("reading config"));
}
#[tokio::test]
async fn update_tool_request_meta_preserves_existing_keys() {
let temp = TempDir::new().unwrap();
let mgr = fresh_thread_manager(&temp).await;
let thread = mgr.create_thread(None, None, None).await.unwrap();
let stored = mgr
.append_message(
&thread.id,
None,
&assistant_message_with_tool_request(
"tc-1",
Some(serde_json::json!({ TOOL_META_EXTERNAL_DISPATCH_KEY: true })),
),
)
.await
.unwrap();
let message_id = stored.id.clone().unwrap();
mgr.update_tool_request_meta(
&thread.id,
&message_id,
"tc-1",
serde_json::json!({ TOOL_META_TITLE_KEY: "running commands" }),
)
.await
.unwrap();
let messages = mgr.list_messages(&thread.id).await.unwrap();
let req = match &messages[0].content[0] {
MessageContent::ToolRequest(r) => r,
_ => panic!("expected tool request"),
};
assert!(
req.is_externally_dispatched(),
"external_dispatch key should be preserved across the merge"
);
assert_eq!(req.persisted_title(), Some("running commands"));
}
#[tokio::test]
async fn update_tool_request_meta_overwrites_existing_value() {
let temp = TempDir::new().unwrap();
let mgr = fresh_thread_manager(&temp).await;
let thread = mgr.create_thread(None, None, None).await.unwrap();
let stored = mgr
.append_message(
&thread.id,
None,
&assistant_message_with_tool_request(
"tc-1",
Some(serde_json::json!({ TOOL_META_TITLE_KEY: "old" })),
),
)
.await
.unwrap();
let message_id = stored.id.clone().unwrap();
mgr.update_tool_request_meta(
&thread.id,
&message_id,
"tc-1",
serde_json::json!({ TOOL_META_TITLE_KEY: "new" }),
)
.await
.unwrap();
let messages = mgr.list_messages(&thread.id).await.unwrap();
let req = match &messages[0].content[0] {
MessageContent::ToolRequest(r) => r,
_ => panic!("expected tool request"),
};
assert_eq!(req.persisted_title(), Some("new"));
}
#[tokio::test]
async fn update_tool_request_meta_no_op_for_unknown_message() {
let temp = TempDir::new().unwrap();
let mgr = fresh_thread_manager(&temp).await;
let thread = mgr.create_thread(None, None, None).await.unwrap();
mgr.update_tool_request_meta(
&thread.id,
"missing-message-id",
"tc-1",
serde_json::json!({ TOOL_META_TITLE_KEY: "x" }),
)
.await
.expect("missing message must be a no-op, not an error");
}
#[tokio::test]
async fn update_tool_request_meta_no_op_for_unknown_tool_call() {
let temp = TempDir::new().unwrap();
let mgr = fresh_thread_manager(&temp).await;
let thread = mgr.create_thread(None, None, None).await.unwrap();
let stored = mgr
.append_message(
&thread.id,
None,
&assistant_message_with_tool_request("tc-1", None),
)
.await
.unwrap();
let message_id = stored.id.clone().unwrap();
mgr.update_tool_request_meta(
&thread.id,
&message_id,
"tc-other",
serde_json::json!({ TOOL_META_TITLE_KEY: "x" }),
)
.await
.unwrap();
let messages = mgr.list_messages(&thread.id).await.unwrap();
let req = match &messages[0].content[0] {
MessageContent::ToolRequest(r) => r,
_ => panic!("expected tool request"),
};
assert!(
req.persisted_title().is_none(),
"no-match must leave tool_meta untouched"
);
}
#[tokio::test]
async fn update_tool_request_meta_targets_correct_row_when_message_id_is_shared() {
// Regression for "first tool call in a chain consistently shows the
// deterministic title on reload." Bedrock/Anthropic-style streaming
// produces a single LLM message id (e.g. `msg_bdrk_…`) but the agent
// splits it across multiple `AgentEvent::Message` events — one for
// text, one for the trailing tool_request — and `append_message`
// writes a separate row per event. Both rows end up with the SAME
// `message_id`. `fetch_optional` returned the text-only row first and
// the title never persisted.
use crate::conversation::message::ToolRequest;
use rmcp::model::CallToolRequestParams;
let temp = TempDir::new().unwrap();
let mgr = fresh_thread_manager(&temp).await;
let thread = mgr.create_thread(None, None, None).await.unwrap();
let shared_id = "msg_bdrk_shared".to_string();
let mut text_only = Message::new(
Role::Assistant,
chrono::Utc::now().timestamp_millis(),
vec![MessageContent::text(
"Let me look at the project structure.",
)],
);
text_only.id = Some(shared_id.clone());
let stored_text = mgr
.append_message(&thread.id, None, &text_only)
.await
.unwrap();
assert_eq!(stored_text.id.as_deref(), Some(shared_id.as_str()));
let mut tool_message = Message::new(
Role::Assistant,
chrono::Utc::now().timestamp_millis(),
vec![MessageContent::ToolRequest(ToolRequest {
id: "toolu_tree".to_string(),
tool_call: Ok(CallToolRequestParams::new("tree")),
metadata: None,
tool_meta: None,
})],
);
tool_message.id = Some(shared_id.clone());
let stored_tool = mgr
.append_message(&thread.id, None, &tool_message)
.await
.unwrap();
assert_eq!(stored_tool.id.as_deref(), Some(shared_id.as_str()));
mgr.update_tool_request_meta(
&thread.id,
&shared_id,
"toolu_tree",
serde_json::json!({ TOOL_META_TITLE_KEY: "exploring project structure" }),
)
.await
.unwrap();
let messages = mgr.list_messages(&thread.id).await.unwrap();
assert_eq!(messages.len(), 2, "two distinct rows must be preserved");
let text_msg = &messages[0];
let tool_msg = &messages[1];
assert!(
matches!(&text_msg.content[0], MessageContent::Text(_)),
"first row must remain text-only and untouched",
);
let tr = match &tool_msg.content[0] {
MessageContent::ToolRequest(r) => r,
_ => panic!("expected tool request in second row"),
};
assert_eq!(
tr.persisted_title(),
Some("exploring project structure"),
"title must land on the row that actually contains the tool call",
);
}
#[tokio::test]
async fn update_tool_request_meta_serializes_concurrent_writes_preserving_all_keys() {
// Regression for "occasional bad replay" when multiple persist tasks
// (per-tool title for tc-1, per-tool title for tc-2, chain summary on
// tc-1) race against each other for the same row's tool_meta. They
// must serialize via BEGIN IMMEDIATE and merge rather than clobber.
use crate::conversation::message::ToolRequest;
use rmcp::model::CallToolRequestParams;
let temp = TempDir::new().unwrap();
let mgr = fresh_thread_manager(&temp).await;
let thread = mgr.create_thread(None, None, None).await.unwrap();
let message = Message::new(
Role::Assistant,
chrono::Utc::now().timestamp_millis(),
vec![
MessageContent::ToolRequest(ToolRequest {
id: "tc-1".to_string(),
tool_call: Ok(CallToolRequestParams::new("developer__shell")),
metadata: None,
tool_meta: None,
}),
MessageContent::ToolRequest(ToolRequest {
id: "tc-2".to_string(),
tool_call: Ok(CallToolRequestParams::new("developer__shell")),
metadata: None,
tool_meta: None,
}),
],
);
let stored = mgr
.append_message(&thread.id, None, &message)
.await
.unwrap();
let message_id = stored.id.clone().unwrap();
let m1 = mgr.clone();
let t1 = thread.id.clone();
let mid1 = message_id.clone();
let h1 = tokio::spawn(async move {
m1.update_tool_request_meta(
&t1,
&mid1,
"tc-1",
serde_json::json!({ TOOL_META_TITLE_KEY: "ran shell command" }),
)
.await
.unwrap();
});
let m2 = mgr.clone();
let t2 = thread.id.clone();
let mid2 = message_id.clone();
let h2 = tokio::spawn(async move {
m2.update_tool_request_meta(
&t2,
&mid2,
"tc-2",
serde_json::json!({ TOOL_META_TITLE_KEY: "ran another shell command" }),
)
.await
.unwrap();
});
let m3 = mgr.clone();
let t3 = thread.id.clone();
let mid3 = message_id.clone();
let h3 = tokio::spawn(async move {
m3.update_tool_request_meta(
&t3,
&mid3,
"tc-1",
serde_json::json!({
TOOL_META_CHAIN_SUMMARY_KEY: { "summary": "inspected codebase", "count": 2 },
}),
)
.await
.unwrap();
});
h1.await.unwrap();
h2.await.unwrap();
h3.await.unwrap();
let messages = mgr.list_messages(&thread.id).await.unwrap();
let content = &messages[0].content;
let tc1 = match &content[0] {
MessageContent::ToolRequest(r) => r,
_ => panic!("expected tool request"),
};
let tc2 = match &content[1] {
MessageContent::ToolRequest(r) => r,
_ => panic!("expected tool request"),
};
assert_eq!(
tc1.persisted_title(),
Some("ran shell command"),
"concurrent writes must not drop tc-1's title",
);
let chain_summary = tc1
.persisted_chain_summary()
.expect("tc-1 must keep its chain summary");
assert_eq!(chain_summary.summary, "inspected codebase");
assert_eq!(chain_summary.count, 2);
assert_eq!(
tc2.persisted_title(),
Some("ran another shell command"),
"concurrent writes must not drop tc-2's title",
);
}
#[tokio::test]
async fn update_tool_request_meta_persists_chain_summary_object() {
let temp = TempDir::new().unwrap();
let mgr = fresh_thread_manager(&temp).await;
let thread = mgr.create_thread(None, None, None).await.unwrap();
let stored = mgr
.append_message(
&thread.id,
None,
&assistant_message_with_tool_request(
"tc-1",
Some(serde_json::json!({ TOOL_META_TITLE_KEY: "first step" })),
),
)
.await
.unwrap();
let message_id = stored.id.clone().unwrap();
mgr.update_tool_request_meta(
&thread.id,
&message_id,
"tc-1",
serde_json::json!({
TOOL_META_CHAIN_SUMMARY_KEY: { "summary": "applied dark mode polish", "count": 4 },
}),
)
.await
.unwrap();
let messages = mgr.list_messages(&thread.id).await.unwrap();
let req = match &messages[0].content[0] {
MessageContent::ToolRequest(r) => r,
_ => panic!("expected tool request"),
};
let chain = req
.persisted_chain_summary()
.expect("chain summary should be present");
assert_eq!(chain.summary, "applied dark mode polish");
assert_eq!(chain.count, 4);
assert_eq!(req.persisted_title(), Some("first step"));
}
}