Skip to content

Commit a0b3737

Browse files
authored
fix(conversation): upsert streaming tool calls (AIO-30) (#484)
Multica issue: [AIO-30](https://multica.ai/issues/AIO-30) ## Root Cause Streaming tool-call persistence used separate read/update/insert branches. Concurrent or out-of-order events for the same message ID could either hit a duplicate primary key error or lose fields when an update arrived before the initial insert. ## Changes - Added a repository-level message upsert for `messages.id` conflicts. - Implemented SQLite `INSERT ... ON CONFLICT(id) DO UPDATE` with JSON merge semantics. - Preserved terminal `finish` / `error` status when late `work` events arrive. - Routed ACP and aionrs tool-call persistence through the upsert path. - Added regression coverage for out-of-order ACP and aionrs tool-call events. ## Verification - `cargo test -p aionui-db` - `cargo test -p aionui-conversation` - `cargo clippy -p aionui-db -- -D warnings` - `cargo clippy -p aionui-conversation -- -D warnings`
1 parent e831f9f commit a0b3737

6 files changed

Lines changed: 340 additions & 153 deletions

File tree

crates/aionui-conversation/src/stream_persistence.rs

Lines changed: 38 additions & 153 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ use std::sync::Arc;
22

33
use aionui_ai_agent::protocol::events::{
44
ErrorEventData, TipType, TipsEventData,
5-
tool_call::{AcpToolCallSessionUpdateKind, AcpToolCallStatus, ToolCallStatus},
5+
tool_call::{AcpToolCallStatus, ToolCallStatus},
66
};
77
use aionui_api_types::{ConversationRuntimeSummary, WebSocketMessage};
88
use aionui_common::{ErrorChain, normalize_keys_to_snake_case, now_ms};
@@ -400,63 +400,32 @@ impl StreamPersistenceAdapter {
400400
};
401401
let content = serde_json::to_string(data).unwrap_or_default();
402402

403-
let existing = self
404-
.repo
405-
.get_message_by_msg_id(&self.conversation_id, &data.call_id, "tool_call")
406-
.await
407-
.unwrap_or(None);
408-
409-
if let Some(existing_row) = existing {
410-
let merged_content = Self::merge_json_content(&existing_row.content, &content);
411-
let update = MessageRowUpdate {
412-
content: Some(merged_content),
413-
status: Some(Some(status.to_owned())),
414-
hidden: None,
415-
};
416-
if let Err(e) = self.repo.update_message(&data.call_id, &update).await {
417-
error!(
418-
call_id = %data.call_id,
419-
tool = %data.name,
420-
status,
421-
error = %ErrorChain(&e),
422-
"Failed to update tool_call message"
423-
);
424-
} else {
425-
debug!(
426-
call_id = %data.call_id,
427-
tool = %data.name,
428-
status,
429-
"Updated tool_call message"
430-
);
431-
}
403+
let row = MessageRow {
404+
id: data.call_id.clone(),
405+
conversation_id: self.conversation_id.clone(),
406+
msg_id: Some(data.call_id.clone()),
407+
r#type: "tool_call".into(),
408+
content,
409+
position: Some("left".into()),
410+
status: Some(status.to_owned()),
411+
hidden: false,
412+
created_at: now_ms(),
413+
};
414+
if let Err(e) = self.repo.upsert_message(&row).await {
415+
error!(
416+
call_id = %data.call_id,
417+
tool = %data.name,
418+
status,
419+
error = %ErrorChain(&e),
420+
"Failed to upsert tool_call message"
421+
);
432422
} else {
433-
let row = MessageRow {
434-
id: data.call_id.clone(),
435-
conversation_id: self.conversation_id.clone(),
436-
msg_id: Some(data.call_id.clone()),
437-
r#type: "tool_call".into(),
438-
content,
439-
position: Some("left".into()),
440-
status: Some(status.to_owned()),
441-
hidden: false,
442-
created_at: now_ms(),
443-
};
444-
if let Err(e) = self.repo.insert_message(&row).await {
445-
error!(
446-
call_id = %data.call_id,
447-
tool = %data.name,
448-
status,
449-
error = %ErrorChain(&e),
450-
"Failed to persist tool_call message"
451-
);
452-
} else {
453-
debug!(
454-
call_id = %data.call_id,
455-
tool = %data.name,
456-
status,
457-
"Persisted tool_call message"
458-
);
459-
}
423+
debug!(
424+
call_id = %data.call_id,
425+
tool = %data.name,
426+
status,
427+
"Upserted tool_call message"
428+
);
460429
}
461430
}
462431

@@ -481,104 +450,20 @@ impl StreamPersistenceAdapter {
481450
normalize_keys_to_snake_case(&mut value);
482451
let content = value.to_string();
483452

484-
match data.update.session_update {
485-
AcpToolCallSessionUpdateKind::ToolCall => {
486-
let row = MessageRow {
487-
id: tool_call_id.clone(),
488-
conversation_id: self.conversation_id.clone(),
489-
msg_id: Some(tool_call_id.clone()),
490-
r#type: "acp_tool_call".into(),
491-
content,
492-
position: Some("left".into()),
493-
status: Some(status.to_owned()),
494-
hidden: false,
495-
created_at: now_ms(),
496-
};
497-
if let Err(e) = self.repo.insert_message(&row).await {
498-
error!(error = %ErrorChain(&e), "Failed to persist acp_tool_call message");
499-
}
500-
}
501-
AcpToolCallSessionUpdateKind::ToolCallUpdate => {
502-
let merged_content = self.merge_acp_tool_call_content(tool_call_id, &value).await;
503-
let update = MessageRowUpdate {
504-
content: Some(merged_content.clone()),
505-
status: Some(Some(status.to_owned())),
506-
hidden: None,
507-
};
508-
if let Err(e) = self.repo.update_message(tool_call_id, &update).await {
509-
match e {
510-
DbError::NotFound(_) => {
511-
warn!(
512-
conversation_id = %self.conversation_id,
513-
tool_call_id = %tool_call_id,
514-
"ACP tool call update arrived before initial tool call; inserting placeholder"
515-
);
516-
let row = MessageRow {
517-
id: tool_call_id.clone(),
518-
conversation_id: self.conversation_id.clone(),
519-
msg_id: Some(tool_call_id.clone()),
520-
r#type: "acp_tool_call".into(),
521-
content: merged_content,
522-
position: Some("left".into()),
523-
status: Some(status.to_owned()),
524-
hidden: false,
525-
created_at: now_ms(),
526-
};
527-
if let Err(insert_err) = self.repo.insert_message(&row).await {
528-
error!(
529-
error = %ErrorChain(&insert_err),
530-
"Failed to insert late acp_tool_call placeholder"
531-
);
532-
}
533-
}
534-
other => {
535-
error!(error = %ErrorChain(&other), "Failed to update acp_tool_call message");
536-
}
537-
}
538-
}
539-
}
540-
}
541-
}
542-
543-
/// Merge two JSON content strings: overlays non-null fields from `new_json`
544-
/// onto `existing_json`, preserving fields only present in the original.
545-
fn merge_json_content(existing_json: &str, new_json: &str) -> String {
546-
let mut base: serde_json::Value = serde_json::from_str(existing_json).unwrap_or_default();
547-
let new_value: serde_json::Value = serde_json::from_str(new_json).unwrap_or_default();
548-
if let (Some(base_obj), Some(new_obj)) = (base.as_object_mut(), new_value.as_object()) {
549-
for (key, val) in new_obj {
550-
if !val.is_null() {
551-
base_obj.insert(key.clone(), val.clone());
552-
}
553-
}
554-
}
555-
base.to_string()
556-
}
557-
558-
async fn merge_acp_tool_call_content(&self, tool_call_id: &str, update_value: &serde_json::Value) -> String {
559-
let existing = self
560-
.repo
561-
.get_message_by_msg_id(&self.conversation_id, tool_call_id, "acp_tool_call")
562-
.await
563-
.ok()
564-
.flatten();
565-
566-
let Some(existing_row) = existing else {
567-
return update_value.to_string();
453+
let row = MessageRow {
454+
id: tool_call_id.clone(),
455+
conversation_id: self.conversation_id.clone(),
456+
msg_id: Some(tool_call_id.clone()),
457+
r#type: "acp_tool_call".into(),
458+
content,
459+
position: Some("left".into()),
460+
status: Some(status.to_owned()),
461+
hidden: false,
462+
created_at: now_ms(),
568463
};
569-
570-
let mut base: serde_json::Value = serde_json::from_str(&existing_row.content).unwrap_or_default();
571-
if let (Some(base_update), Some(new_update)) = (
572-
base.get_mut("update").and_then(|v| v.as_object_mut()),
573-
update_value.get("update").and_then(|v| v.as_object()),
574-
) {
575-
for (key, val) in new_update {
576-
if !val.is_null() {
577-
base_update.insert(key.clone(), val.clone());
578-
}
579-
}
464+
if let Err(e) = self.repo.upsert_message(&row).await {
465+
error!(error = %ErrorChain(&e), "Failed to upsert acp_tool_call message");
580466
}
581-
base.to_string()
582467
}
583468

584469
/// Persist a tool_group event (array of tool summaries).

crates/aionui-conversation/src/stream_relay.rs

Lines changed: 93 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1805,6 +1805,75 @@ mod tests {
18051805
fn take_updates(&self) -> Vec<(String, aionui_db::MessageRowUpdate)> {
18061806
std::mem::take(&mut self.updates.lock().unwrap())
18071807
}
1808+
1809+
fn merged_row(existing: &MessageRow, incoming: &MessageRow) -> MessageRow {
1810+
let preserve_terminal_status = matches!(existing.status.as_deref(), Some("finish" | "error"))
1811+
&& incoming.status.as_deref() == Some("work");
1812+
let mut content = Self::merge_json_content(&existing.content, &incoming.content);
1813+
if preserve_terminal_status {
1814+
content = Self::preserve_json_status(&content, &existing.content, &existing.r#type);
1815+
}
1816+
1817+
let mut merged = existing.clone();
1818+
merged.content = content;
1819+
merged.status = if preserve_terminal_status {
1820+
existing.status.clone()
1821+
} else {
1822+
incoming.status.clone()
1823+
};
1824+
merged.hidden = incoming.hidden;
1825+
merged
1826+
}
1827+
1828+
fn merge_json_content(existing_json: &str, incoming_json: &str) -> String {
1829+
let mut existing: serde_json::Value = serde_json::from_str(existing_json).unwrap_or_default();
1830+
let incoming: serde_json::Value = serde_json::from_str(incoming_json).unwrap_or_default();
1831+
Self::merge_json_value(&mut existing, incoming);
1832+
existing.to_string()
1833+
}
1834+
1835+
fn merge_json_value(existing: &mut serde_json::Value, incoming: serde_json::Value) {
1836+
match (existing, incoming) {
1837+
(serde_json::Value::Object(existing_obj), serde_json::Value::Object(incoming_obj)) => {
1838+
for (key, value) in incoming_obj {
1839+
if !value.is_null() {
1840+
if let Some(existing_value) = existing_obj.get_mut(&key) {
1841+
Self::merge_json_value(existing_value, value);
1842+
} else {
1843+
existing_obj.insert(key, value);
1844+
}
1845+
}
1846+
}
1847+
}
1848+
(existing_value, incoming_value) => {
1849+
if !incoming_value.is_null() {
1850+
*existing_value = incoming_value;
1851+
}
1852+
}
1853+
}
1854+
}
1855+
1856+
fn preserve_json_status(merged_json: &str, existing_json: &str, msg_type: &str) -> String {
1857+
let mut merged: serde_json::Value = serde_json::from_str(merged_json).unwrap_or_default();
1858+
let existing: serde_json::Value = serde_json::from_str(existing_json).unwrap_or_default();
1859+
let status = if msg_type == "acp_tool_call" {
1860+
existing.pointer("/update/status").cloned()
1861+
} else {
1862+
existing.get("status").cloned()
1863+
};
1864+
1865+
if let Some(status) = status {
1866+
if msg_type == "acp_tool_call" {
1867+
if let Some(update) = merged.get_mut("update").and_then(|value| value.as_object_mut()) {
1868+
update.insert("status".into(), status);
1869+
}
1870+
} else if let Some(object) = merged.as_object_mut() {
1871+
object.insert("status".into(), status);
1872+
}
1873+
}
1874+
1875+
merged.to_string()
1876+
}
18081877
}
18091878

18101879
#[async_trait::async_trait]
@@ -1881,6 +1950,30 @@ mod tests {
18811950
self.inserts.lock().unwrap().push(row.clone());
18821951
Ok(())
18831952
}
1953+
async fn upsert_message(&self, row: &MessageRow) -> Result<(), DbError> {
1954+
if self.not_found.load(Ordering::Acquire) {
1955+
return Err(DbError::NotFound(format!("Message '{}'", row.id)));
1956+
}
1957+
if self.foreign_key_failure.load(Ordering::Acquire) {
1958+
return Err(DbError::Init("FOREIGN KEY constraint failed".into()));
1959+
}
1960+
1961+
let mut inserts = self.inserts.lock().unwrap();
1962+
if let Some(existing) = inserts.iter().find(|message| message.id == row.id) {
1963+
let merged = Self::merged_row(existing, row);
1964+
self.updates.lock().unwrap().push((
1965+
row.id.clone(),
1966+
aionui_db::MessageRowUpdate {
1967+
content: Some(merged.content),
1968+
status: Some(merged.status),
1969+
hidden: Some(merged.hidden),
1970+
},
1971+
));
1972+
} else {
1973+
inserts.push(row.clone());
1974+
}
1975+
Ok(())
1976+
}
18841977
async fn update_message(&self, id: &str, updates: &aionui_db::MessageRowUpdate) -> Result<(), DbError> {
18851978
if self.not_found.load(Ordering::Acquire) {
18861979
return Err(DbError::NotFound(format!("Message '{id}' not found")));

0 commit comments

Comments
 (0)