diff --git a/crates/acp_thread/src/acp_thread.rs b/crates/acp_thread/src/acp_thread.rs index 71b1237fff..15ef8aaf58 100644 --- a/crates/acp_thread/src/acp_thread.rs +++ b/crates/acp_thread/src/acp_thread.rs @@ -296,7 +296,7 @@ pub struct UserMessage { pub protocol_id: Option, pub client_id: Option, pub is_optimistic: bool, - pub content: ContentBlock, + pub content: MessageContent, pub chunks: Vec, pub checkpoint: Option, pub indented: bool, @@ -350,11 +350,11 @@ impl AssistantMessage { pub enum AssistantMessageChunk { Message { id: Option, - block: ContentBlock, + block: MessageContent, }, Thought { id: Option, - block: ContentBlock, + block: MessageContent, }, } @@ -367,13 +367,13 @@ impl AssistantMessageChunk { ) -> Self { Self::Message { id: None, - block: ContentBlock::new(chunk.into(), language_registry, path_style, cx), + block: MessageContent::new(chunk.into(), language_registry, path_style, cx), } } fn to_markdown(&self, cx: &App) -> String { match self { - Self::Message { block, .. } => block.to_markdown(cx).to_string(), + Self::Message { block, .. } => block.to_markdown(cx), Self::Thought { block, .. } => { format!("\n{}\n", block.to_markdown(cx)) } @@ -796,7 +796,6 @@ impl ContextCompaction { &mut self, update: acp::CompactionUpdate, language_registry: &Arc, - path_style: PathStyle, cx: &mut App, ) { self.status = update.status.into(); @@ -806,7 +805,7 @@ impl ContextCompaction { MaybeUndefined::Value(blocks) => { self.summary.clear(); for block in blocks { - self.append_summary(block, language_registry, path_style, cx); + self.append_summary(block, language_registry, cx); } } } @@ -827,7 +826,6 @@ impl ContextCompaction { &mut self, content: acp::ContentBlock, language_registry: &Arc, - path_style: PathStyle, cx: &mut App, ) { if let acp::ContentBlock::Text(text) = &content @@ -835,12 +833,8 @@ impl ContextCompaction { { markdown.update(cx, |markdown, cx| markdown.append(&text.text, cx)); } else { - self.summary.push(ContentBlock::new_output( - content, - language_registry, - path_style, - cx, - )); + self.summary + .push(ContentBlock::new_output(content, language_registry, cx)); } } } @@ -971,7 +965,6 @@ impl ToolCall { tool_call: acp::ToolCall, status: ToolCallStatus, language_registry: Arc, - path_style: PathStyle, terminals: &HashMap>, cx: &mut App, ) -> Result { @@ -979,13 +972,9 @@ impl ToolCall { Some(SharedString::from(tool_call.title)).filter(|title| !title.trim().is_empty()); let mut content = Vec::with_capacity(tool_call.content.len()); for item in tool_call.content { - if let Some(item) = ToolCallContent::from_acp( - item, - language_registry.clone(), - path_style, - terminals, - cx, - )? { + if let Some(item) = + ToolCallContent::from_acp(item, language_registry.clone(), terminals, cx)? + { content.push(item); } } @@ -1081,7 +1070,6 @@ impl ToolCall { fields: acp::ToolCallUpdateFields, meta: Option, language_registry: Arc, - path_style: PathStyle, terminals: &HashMap>, cx: &mut App, ) -> Result<()> { @@ -1173,19 +1161,15 @@ impl ToolCall { // Reuse existing content if we can for (old, new) in self.content.iter_mut().zip(content.by_ref()) { let valid_content = - old.update_from_acp(new, language_registry.clone(), path_style, terminals, cx)?; + old.update_from_acp(new, language_registry.clone(), terminals, cx)?; if !valid_content { new_content_len -= 1; } } for new in content { - if let Some(new) = ToolCallContent::from_acp( - new, - language_registry.clone(), - path_style, - terminals, - cx, - )? { + if let Some(new) = + ToolCallContent::from_acp(new, language_registry.clone(), terminals, cx)? + { self.content.push(new); } else { new_content_len -= 1; @@ -1496,9 +1480,108 @@ fn elicitation_status_for_response(response: &acp::CreateElicitationResponse) -> } } +#[derive(Debug, Default, PartialEq)] +pub struct MessageContent { + blocks: Vec, +} + +impl MessageContent { + pub fn new( + block: acp::ContentBlock, + language_registry: &Arc, + path_style: PathStyle, + cx: &mut App, + ) -> Self { + let mut content = Self::default(); + content.append(block, language_registry, path_style, cx); + content + } + + pub fn blocks(&self) -> &[ContentBlock] { + &self.blocks + } + + pub fn markdowns(&self) -> impl Iterator> { + self.blocks.iter().filter_map(ContentBlock::markdown) + } + + pub fn visible_content(&self, cx: &App) -> bool { + self.blocks.iter().any(|block| block.visible_content(cx)) + } + + pub fn to_markdown(&self, cx: &App) -> String { + self.blocks + .iter() + .map(|block| block.to_markdown(cx)) + .filter(|text| !text.is_empty()) + .join("\n\n") + } + + fn trailing_text(&self) -> Option<&Entity> { + match self.blocks.last()? { + ContentBlock::Markdown { markdown } => Some(markdown), + _ => None, + } + } + + pub fn append( + &mut self, + block: acp::ContentBlock, + language_registry: &Arc, + path_style: PathStyle, + cx: &mut App, + ) { + let block = match block { + acp::ContentBlock::ResourceLink(resource) => { + let mut text = ContentBlock::resource_link_md(&resource.uri, path_style); + if self.blocks.is_empty() { + // Legacy leading links separated the next chunk. Include that separator + // now because streamed text can append directly to the Markdown entity. + text.push('\n'); + } + text.into() + } + block => block, + }; + if let acp::ContentBlock::Text(text) = &block { + if text.text.is_empty() { + return; + } + if let Some(markdown) = self.trailing_text() { + markdown.update(cx, |markdown, cx| markdown.append(&text.text, cx)); + return; + } + } + self.blocks + .push(ContentBlock::new_output(block, language_registry, cx)); + } + + fn append_prompt( + &mut self, + block: acp::ContentBlock, + language_registry: &Arc, + path_style: PathStyle, + cx: &mut App, + ) { + // Prompt text resources are mentions, not output previews. Keep their text + // inline while retaining the original payloads in UserMessage::chunks. + let block = match block { + acp::ContentBlock::Resource(resource) + if matches!( + resource.resource, + acp::EmbeddedResourceResource::TextResourceContents(_) + ) => + { + ContentBlock::embedded_resource_string_contents(&resource, path_style).into() + } + block => block, + }; + self.append(block, language_registry, path_style, cx); + } +} + #[derive(Debug, PartialEq, Clone)] pub enum ContentBlock { - Empty, Markdown { markdown: Entity, }, @@ -1520,37 +1603,16 @@ pub enum ContentBlock { } impl ContentBlock { - pub fn new( - block: acp::ContentBlock, - language_registry: &Arc, - path_style: PathStyle, - cx: &mut App, - ) -> Self { - let mut this = Self::Empty; - this.append(block, language_registry, path_style, cx); - this - } - - pub fn new_combined( - blocks: impl IntoIterator, - language_registry: Arc, - path_style: PathStyle, - cx: &mut App, - ) -> Self { - let mut this = Self::Empty; - for block in blocks { - this.append(block, &language_registry, path_style, cx); - } - this - } - pub fn new_output( block: acp::ContentBlock, language_registry: &Arc, - path_style: PathStyle, cx: &mut App, ) -> Self { match block { + acp::ContentBlock::Text(text) => { + Self::create_markdown_block(text.text, language_registry, cx) + } + acp::ContentBlock::ResourceLink(resource_link) => Self::ResourceLink { resource_link }, acp::ContentBlock::Resource(resource) if matches!( &resource.resource, @@ -1580,9 +1642,6 @@ impl ContentBlock { } } } - block @ (acp::ContentBlock::Text(_) | acp::ContentBlock::ResourceLink(_)) => { - Self::new(block, language_registry, path_style, cx) - } content => { let description = if matches!(&content, acp::ContentBlock::Audio(_)) { "Audio content is not supported." @@ -1597,61 +1656,6 @@ impl ContentBlock { } } - pub fn append( - &mut self, - block: acp::ContentBlock, - language_registry: &Arc, - path_style: PathStyle, - cx: &mut App, - ) { - match (&mut *self, &block) { - (ContentBlock::Empty, acp::ContentBlock::ResourceLink(resource_link)) => { - *self = ContentBlock::ResourceLink { - resource_link: resource_link.clone(), - }; - } - (ContentBlock::Empty, acp::ContentBlock::Image(image_content)) => { - if let Some((image, dimensions)) = Self::decode_image(image_content) { - *self = ContentBlock::Image { image, dimensions }; - } else { - let new_content = Self::image_md(image_content); - *self = Self::create_markdown_block(new_content, language_registry, cx); - } - } - (ContentBlock::Empty, _) => { - let new_content = Self::block_string_contents(&block, path_style); - *self = Self::create_markdown_block(new_content, language_registry, cx); - } - (ContentBlock::Markdown { markdown }, _) => { - let new_content = Self::block_string_contents(&block, path_style); - markdown.update(cx, |markdown, cx| markdown.append(&new_content, cx)); - } - (ContentBlock::ResourceLink { resource_link }, _) => { - let existing_content = Self::resource_link_md(&resource_link.uri, path_style); - let new_content = Self::block_string_contents(&block, path_style); - let combined = format!("{}\n{}", existing_content, new_content); - *self = Self::create_markdown_block(combined, language_registry, cx); - } - (ContentBlock::EmbeddedResource { resource, .. }, _) => { - let existing_content = - Self::embedded_resource_string_contents(resource, path_style); - let new_content = Self::block_string_contents(&block, path_style); - let combined = format!("{}\n{}", existing_content, new_content); - *self = Self::create_markdown_block(combined, language_registry, cx); - } - (ContentBlock::Image { .. }, _) => { - let new_content = Self::block_string_contents(&block, path_style); - let combined = format!("`Image`\n{}", new_content); - *self = Self::create_markdown_block(combined, language_registry, cx); - } - (ContentBlock::Unsupported { markdown, .. }, _) => { - let new_content = Self::block_string_contents(&block, path_style); - let combined = format!("{}\n{}", markdown.read(cx).source(), new_content); - *self = Self::create_markdown_block(combined, language_registry, cx); - } - } - } - /// Updates a Markdown block in place from a streaming text `block`, reusing /// the existing `Markdown` entity rather than recreating it. Appends only the /// new suffix when the update is a continuation (the common streaming case), @@ -1791,9 +1795,7 @@ impl ContentBlock { acp::EmbeddedResourceResource::BlobResourceContents(_) => None, _ => None, }, - ContentBlock::Empty - | ContentBlock::ResourceLink { .. } - | ContentBlock::Image { .. } => None, + ContentBlock::ResourceLink { .. } | ContentBlock::Image { .. } => None, } } @@ -1864,7 +1866,6 @@ impl ContentBlock { pub fn visible_content(&self, cx: &App) -> bool { match self { - ContentBlock::Empty => false, ContentBlock::Markdown { markdown } | ContentBlock::Unsupported { markdown, .. } => { !markdown.read(cx).source().trim().is_empty() } @@ -1876,25 +1877,6 @@ impl ContentBlock { } } - fn block_string_contents(block: &acp::ContentBlock, path_style: PathStyle) -> String { - match block { - acp::ContentBlock::Text(text_content) => text_content.text.clone(), - acp::ContentBlock::ResourceLink(resource_link) => { - Self::resource_link_md(&resource_link.uri, path_style) - } - acp::ContentBlock::Resource(acp::EmbeddedResource { - resource: - acp::EmbeddedResourceResource::TextResourceContents(acp::TextResourceContents { - uri, - .. - }), - .. - }) => Self::resource_link_md(uri, path_style), - acp::ContentBlock::Image(image) => Self::image_md(image), - _ => String::new(), - } - } - fn resource_link_md(uri: &str, path_style: PathStyle) -> String { if let Some(uri) = MentionUri::parse(uri, path_style).log_err() { uri.as_link().to_string() @@ -1903,13 +1885,8 @@ impl ContentBlock { } } - fn image_md(_image: &acp::ImageContent) -> String { - "`Image`".into() - } - pub fn to_markdown<'a>(&'a self, cx: &'a App) -> &'a str { match self { - ContentBlock::Empty => "", ContentBlock::Markdown { markdown } | ContentBlock::Unsupported { markdown, .. } => { markdown.read(cx).source() } @@ -1927,7 +1904,6 @@ impl ContentBlock { pub fn markdown(&self) -> Option<&Entity> { match self { - ContentBlock::Empty => None, ContentBlock::Markdown { markdown } | ContentBlock::Unsupported { markdown, .. } => { Some(markdown) } @@ -2018,19 +1994,13 @@ impl ToolCallContent { pub fn from_acp( content: acp::ToolCallContent, language_registry: Arc, - path_style: PathStyle, terminals: &HashMap>, cx: &mut App, ) -> Result> { match content { - acp::ToolCallContent::Content(acp::Content { content, .. }) => { - Ok(Some(Self::ContentBlock(ContentBlock::new_output( - content, - &language_registry, - path_style, - cx, - )))) - } + acp::ToolCallContent::Content(acp::Content { content, .. }) => Ok(Some( + Self::ContentBlock(ContentBlock::new_output(content, &language_registry, cx)), + )), acp::ToolCallContent::Diff(diff) => Ok(Some(Self::Diff(cx.new(|cx| { Diff::finalized( diff.path.to_string_lossy().into_owned(), @@ -2053,7 +2023,6 @@ impl ToolCallContent { &mut self, new: acp::ToolCallContent, language_registry: Arc, - path_style: PathStyle, terminals: &HashMap>, cx: &mut App, ) -> Result { @@ -2079,7 +2048,7 @@ impl ToolCallContent { _ => true, }; - if let Some(update) = Self::from_acp(new, language_registry, path_style, terminals, cx)? { + if let Some(update) = Self::from_acp(new, language_registry, terminals, cx)? { if needs_update { *self = update; } @@ -2659,7 +2628,7 @@ impl AcpThread { AssistantMessageChunk::Message { block, .. } => block, AssistantMessageChunk::Thought { block, .. } => block, }; - if let Some(markdown) = block.markdown() { + for markdown in block.markdowns() { markdown.update(cx, |markdown, cx| { markdown.invalidate_mermaid_cache(cx); }); @@ -2978,12 +2947,13 @@ impl AcpThread { if existing_protocol_id.is_none() { *existing_protocol_id = protocol_id; } - content.append(chunk.clone(), &language_registry, path_style, cx); + content.append_prompt(chunk.clone(), &language_registry, path_style, cx); chunks.push(chunk); let idx = entries_len - 1; cx.emit(AcpThreadEvent::EntryUpdated(idx)); } else { - let content = ContentBlock::new(chunk.clone(), &language_registry, path_style, cx); + let mut content = MessageContent::default(); + content.append_prompt(chunk.clone(), &language_registry, path_style, cx); self.push_entry( AgentThreadEntry::UserMessage(UserMessage { protocol_id, @@ -3075,7 +3045,7 @@ impl AcpThread { block.append(chunk, &language_registry, path_style, cx) } _ => { - let block = ContentBlock::new(chunk, &language_registry, path_style, cx); + let block = MessageContent::new(chunk, &language_registry, path_style, cx); if is_thought { chunks.push(AssistantMessageChunk::Thought { id: message_id, @@ -3090,7 +3060,7 @@ impl AcpThread { } } } else { - let block = ContentBlock::new(chunk, &language_registry, path_style, cx); + let block = MessageContent::new(chunk, &language_registry, path_style, cx); let chunk = if is_thought { AssistantMessageChunk::Thought { id: message_id, @@ -3133,17 +3103,18 @@ impl AcpThread { ( AssistantMessageChunk::Message { id: existing_id, - block: ContentBlock::Markdown { markdown }, + block, }, false, ) | ( AssistantMessageChunk::Thought { id: existing_id, - block: ContentBlock::Markdown { markdown }, + block, }, true, ) if can_merge_message_chunks(existing_id.as_ref(), message_id) => { + let markdown = block.trailing_text()?; if existing_id.is_none() { *existing_id = message_id.cloned(); } @@ -3283,7 +3254,6 @@ impl AcpThread { ) { let id = ContextCompactionId(update.compaction_id.0.clone()); let language_registry = self.project.read(cx).languages().clone(); - let path_style = self.project.read(cx).path_style(cx); if let Some((entry_index, compaction)) = self.entries @@ -3297,7 +3267,7 @@ impl AcpThread { _ => None, }) { - compaction.apply_update(update, &language_registry, path_style, cx); + compaction.apply_update(update, &language_registry, cx); cx.emit(AcpThreadEvent::EntryUpdated(entry_index)); return; } @@ -3308,7 +3278,7 @@ impl AcpThread { error: None, summary: Vec::new(), }; - compaction.apply_update(update, &language_registry, path_style, cx); + compaction.apply_update(update, &language_registry, cx); self.push_entry(AgentThreadEntry::ContextCompaction(compaction), cx); } @@ -3318,7 +3288,6 @@ impl AcpThread { cx: &mut Context, ) { let language_registry = self.project.read(cx).languages().clone(); - let path_style = self.project.read(cx).path_style(cx); if let Some((entry_index, compaction)) = self.entries .iter_mut() @@ -3334,7 +3303,7 @@ impl AcpThread { _ => None, }) { - compaction.append_summary(chunk.content, &language_registry, path_style, cx); + compaction.append_summary(chunk.content, &language_registry, cx); cx.emit(AcpThreadEvent::EntryUpdated(entry_index)); } } @@ -3345,7 +3314,6 @@ impl AcpThread { cx: &mut Context, ) { let language_registry = self.project.read(cx).languages().clone(); - let path_style = self.project.read(cx).path_style(cx); let Some((ix, compaction)) = self.entries .iter_mut() @@ -3363,7 +3331,6 @@ impl AcpThread { compaction.append_summary( acp::ContentBlock::Text(acp::TextContent::new(update.summary_delta)), &language_registry, - path_style, cx, ); } @@ -3426,7 +3393,6 @@ impl AcpThread { ) -> Result<()> { let update = update.into(); let languages = self.project.read(cx).languages().clone(); - let path_style = self.project.read(cx).path_style(cx); let ix = match self.index_for_tool_call(update.id()) { Some(ix) => ix, @@ -3437,10 +3403,9 @@ impl AcpThread { label: cx.new(|cx| Markdown::new("Tool call not found".into(), None, None, cx)), title: Some("Tool call not found".into()), kind: acp::ToolKind::Fetch, - content: vec![ToolCallContent::ContentBlock(ContentBlock::new( + content: vec![ToolCallContent::ContentBlock(ContentBlock::new_output( "Tool call not found".into(), &languages, - path_style, cx, ))], status: ToolCallStatus::Failed, @@ -3466,14 +3431,9 @@ impl AcpThread { match update { ToolCallUpdate::UpdateFields(update) => { let location_updated = update.fields.locations.is_some(); - if let Err(error) = call.update_fields( - update.fields, - update.meta, - languages, - path_style, - &self.terminals, - cx, - ) { + if let Err(error) = + call.update_fields(update.fields, update.meta, languages, &self.terminals, cx) + { cx.emit(AcpThreadEvent::EntryUpdated(ix)); return Err(error); } @@ -3515,7 +3475,6 @@ impl AcpThread { cx: &mut Context, ) -> Result<(), acp::Error> { let language_registry = self.project.read(cx).languages().clone(); - let path_style = self.project.read(cx).path_style(cx); let id = update.tool_call_id.clone(); let agent_telemetry_id = self.connection().telemetry_id(); @@ -3545,7 +3504,6 @@ impl AcpThread { update.fields, update.meta, language_registry, - path_style, &self.terminals, cx, ) { @@ -3560,7 +3518,6 @@ impl AcpThread { update.try_into()?, status, language_registry, - self.project.read(cx).path_style(cx), &self.terminals, cx, )?; @@ -3967,12 +3924,12 @@ impl AcpThread { push_user_message: bool, cx: &mut Context, ) -> BoxFuture<'static, Result>> { - let block = ContentBlock::new_combined( - message.clone(), - self.project.read(cx).languages().clone(), - self.project.read(cx).path_style(cx), - cx, - ); + let language_registry = self.project.read(cx).languages().clone(); + let path_style = self.project.read(cx).path_style(cx); + let mut block = MessageContent::default(); + for chunk in &message { + block.append_prompt(chunk.clone(), &language_registry, path_style, cx); + } let request = acp::PromptRequest::new(self.session_id.clone(), message.clone()); let git_store = self.project.read(cx).git_store().clone(); @@ -5214,6 +5171,378 @@ mod tests { }); } + fn message_test_image() -> acp::ContentBlock { + acp::ContentBlock::Image(acp::ImageContent::new( + "iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAADUlEQVR42mNkYPhfDwAChwGA60e6kgAAAABJRU5ErkJggg==", + "image/png", + )) + } + + #[gpui::test] + fn test_message_content_preserves_mixed_order(cx: &mut TestAppContext) { + init_test(cx); + cx.update(|cx| { + let language_registry = + Arc::new(LanguageRegistry::test(cx.background_executor().clone())); + for (chunks, expected_kinds, expected_export) in [ + ( + vec!["before".into(), message_test_image()], + vec!["text", "image"], + "before\n\n`Image`", + ), + ( + vec![message_test_image(), "after".into()], + vec!["image", "text"], + "`Image`\n\nafter", + ), + ( + vec![message_test_image(), message_test_image()], + vec!["image", "image"], + "`Image`\n\n`Image`", + ), + ( + vec!["".into(), message_test_image()], + vec!["image"], + "`Image`", + ), + ] { + let mut content = MessageContent::default(); + for chunk in chunks { + content.append(chunk, &language_registry, PathStyle::local(), cx); + } + assert_eq!( + content + .blocks() + .iter() + .map(|block| match block { + ContentBlock::Markdown { .. } => "text", + ContentBlock::Image { .. } => "image", + other => panic!("unexpected block {other:?}"), + }) + .collect::>(), + expected_kinds, + ); + assert_eq!(content.to_markdown(cx), expected_export); + assert!(content.visible_content(cx)); + } + }); + } + + #[gpui::test] + fn test_message_content_resources_and_fallbacks(cx: &mut TestAppContext) { + init_test(cx); + cx.update(|cx| { + let language_registry = + Arc::new(LanguageRegistry::test(cx.background_executor().clone())); + let mut content = MessageContent::default(); + for chunk in [ + "before".into(), + acp::ContentBlock::Resource(acp::EmbeddedResource::new( + acp::EmbeddedResourceResource::TextResourceContents( + acp::TextResourceContents::new("resource text", "tool://preview"), + ), + )), + acp::ContentBlock::ResourceLink(acp::ResourceLink::new( + "link", + "https://example.com/resource", + )), + acp::ContentBlock::Resource(acp::EmbeddedResource::new( + acp::EmbeddedResourceResource::BlobResourceContents( + acp::BlobResourceContents::new("private-blob-data", "tool://archive.bin"), + ), + )), + acp::ContentBlock::Image(acp::ImageContent::new("not-base64", "image/png")), + acp::ContentBlock::Audio(acp::AudioContent::new("private-audio-data", "audio/wav")), + "after".into(), + ] { + content.append(chunk, &language_registry, PathStyle::local(), cx); + } + assert!(matches!( + content.blocks(), + [ + ContentBlock::Markdown { .. }, + ContentBlock::EmbeddedResource { .. }, + ContentBlock::Markdown { .. }, + ContentBlock::EmbeddedResource { markdown: None, .. }, + ContentBlock::Unsupported { .. }, + ContentBlock::Unsupported { .. }, + ContentBlock::Markdown { .. }, + ] + )); + assert_eq!( + content.to_markdown(cx), + concat!( + "before\n\n```\nresource text\n```\n\n", + "[@https://example.com/resource](https://example.com/resource)\n\n", + "tool://archive.bin\n\n", + "Image content could not be displayed.\n\n", + "Audio content is not supported.\n\nafter", + ) + ); + assert_eq!( + content + .markdowns() + .map(|markdown| markdown.read(cx).source()) + .collect::>(), + [ + "before", + "```\nresource text\n```", + "[@https://example.com/resource](https://example.com/resource)", + "Image content could not be displayed.", + "Audio content is not supported.", + "after", + ] + ); + assert!(!content.to_markdown(cx).contains("private-audio-data")); + assert!(!content.to_markdown(cx).contains("private-blob-data")); + }); + } + + #[gpui::test] + async fn test_message_content_preserves_streaming_entities(cx: &mut TestAppContext) { + init_test(cx); + let project = Project::test(FakeFs::new(cx.executor()), [], cx).await; + let connection = Rc::new(FakeAgentConnection::new()); + let thread = cx + .update(|cx| { + connection.new_session(project, PathList::new(&[Path::new(path!("/test"))]), cx) + }) + .await + .expect("session should be created"); + + let (before, after, image) = thread.update(cx, |thread, cx| { + thread.push_assistant_content_block("before".into(), false, cx); + let before = thread + .streaming_markdown_target(None, false, false) + .expect("text should have a streaming target"); + thread.push_assistant_content_block(" image".into(), false, cx); + assert!(thread.streaming_text_buffer.is_some()); + thread.push_assistant_content_block(message_test_image(), false, cx); + assert!(thread.streaming_text_buffer.is_none()); + assert_eq!(before.read(cx).source(), "before image"); + assert!( + thread + .streaming_markdown_target(None, false, false) + .is_none() + ); + + thread.push_assistant_content_block("after".into(), false, cx); + let after = thread + .streaming_markdown_target(None, false, false) + .expect("text after an image should have its own streaming target"); + assert_ne!(before.entity_id(), after.entity_id()); + thread.push_assistant_content_block(" image".into(), false, cx); + assert_eq!( + thread + .streaming_text_buffer + .as_ref() + .expect("streaming text") + .target, + after, + ); + + let [AgentThreadEntry::AssistantMessage(message)] = thread.entries() else { + panic!("expected one assistant message"); + }; + let [AssistantMessageChunk::Message { block, .. }] = message.chunks.as_slice() else { + panic!("expected one message chunk"); + }; + let [_, ContentBlock::Image { image, .. }, _] = block.blocks() else { + panic!("expected text, image, text"); + }; + (before, after, image.clone()) + }); + + cx.run_until_parked(); + cx.executor().advance_clock(Duration::from_secs(1)); + cx.run_until_parked(); + + thread.read_with(cx, |thread, cx| { + assert_eq!(after.read(cx).source(), "after image"); + let [AgentThreadEntry::AssistantMessage(message)] = thread.entries() else { + panic!("expected one assistant message"); + }; + let [AssistantMessageChunk::Message { block, .. }] = message.chunks.as_slice() else { + panic!("expected one message chunk"); + }; + let [ + ContentBlock::Markdown { markdown: first }, + ContentBlock::Image { + image: current_image, + .. + }, + ContentBlock::Markdown { markdown: last }, + ] = block.blocks() + else { + panic!("expected text, image, text"); + }; + assert_eq!(first, &before); + assert_eq!(last, &after); + assert!(Arc::ptr_eq(&image, current_image)); + }); + + let whole = cx.update(|cx| { + let language_registry = + Arc::new(LanguageRegistry::test(cx.background_executor().clone())); + let mut content = MessageContent::default(); + for chunk in [ + "before image".into(), + message_test_image(), + "after image".into(), + ] { + content.append(chunk, &language_registry, PathStyle::local(), cx); + } + content.to_markdown(cx) + }); + thread.read_with(cx, |thread, cx| { + assert_eq!( + thread.to_markdown(cx), + format!("## Assistant\n\n{whole}\n\n") + ); + }); + + for (content, preview) in [ + ( + acp::ContentBlock::Resource(acp::EmbeddedResource::new( + acp::EmbeddedResourceResource::TextResourceContents( + acp::TextResourceContents::new("Resource preview", "tool://preview") + .mime_type("text/markdown".to_string()), + ), + )), + "Resource preview", + ), + ( + acp::ContentBlock::Audio(acp::AudioContent::new("private-audio-data", "audio/wav")), + "Audio content is not supported.", + ), + ] { + thread.update(cx, |thread, cx| { + thread.push_assistant_content_block(content, false, cx); + assert!( + thread + .streaming_markdown_target(None, false, false) + .is_none() + ); + let Some(AgentThreadEntry::AssistantMessage(message)) = thread.entries().last() + else { + panic!("expected assistant output"); + }; + let Some(AssistantMessageChunk::Message { block, .. }) = message.chunks.last() + else { + panic!("expected message content"); + }; + let preview_markdown = block + .blocks() + .last() + .and_then(ContentBlock::markdown) + .expect("Markdown-backed non-text content") + .clone(); + thread.push_assistant_content_block("Following".into(), false, cx); + let target = thread + .streaming_markdown_target(None, false, false) + .expect("new text target"); + assert_ne!(target, preview_markdown); + thread.push_assistant_content_block(" text".into(), false, cx); + AcpThread::flush_streaming_text(&mut thread.streaming_text_buffer, cx); + assert_eq!(target.read(cx).source(), "Following text"); + assert_eq!(preview_markdown.read(cx).source(), preview); + }); + } + + let (streamed, expected) = thread.update(cx, |thread, cx| { + let path_style = thread.project.read(cx).path_style(cx); + let languages = thread.project.read(cx).languages().clone(); + let uri = "file:///project/report.md"; + let link = acp::ContentBlock::ResourceLink(acp::ResourceLink::new("report", uri)); + let expected = format!( + "{}\n## Details\n", + ContentBlock::resource_link_md(uri, path_style), + ); + let mut direct = MessageContent::new(link.clone(), &languages, path_style, cx); + direct.append("## Details\n".into(), &languages, path_style, cx); + assert_eq!(direct.to_markdown(cx), expected); + + thread.push_user_content_block(None, "Next response".into(), cx); + thread.push_assistant_content_block(link, false, cx); + let target = thread + .streaming_markdown_target(None, false, false) + .expect("leading link should retain its Markdown entity"); + thread.push_assistant_content_block("## Det".into(), false, cx); + thread.push_assistant_content_block("ails\n".into(), false, cx); + assert_eq!( + thread + .streaming_text_buffer + .as_ref() + .expect("buffered text") + .target, + target, + ); + (target, expected) + }); + cx.run_until_parked(); + cx.executor().advance_clock(Duration::from_secs(1)); + cx.run_until_parked(); + streamed.read_with(cx, |markdown, _| assert_eq!(markdown.source(), &expected)); + } + + #[gpui::test] + async fn test_message_content_preserves_prompt_mentions_and_raw_chunks( + cx: &mut TestAppContext, + ) { + init_test(cx); + let project = Project::test(FakeFs::new(cx.executor()), [], cx).await; + let connection = Rc::new(FakeAgentConnection::new()); + let thread = cx + .update(|cx| { + connection.new_session(project, PathList::new(&[Path::new(path!("/test"))]), cx) + }) + .await + .expect("session should be created"); + let chunks = vec![ + "read ".into(), + acp::ContentBlock::Resource(acp::EmbeddedResource::new( + acp::EmbeddedResourceResource::TextResourceContents( + acp::TextResourceContents::new( + "attached file contents", + "https://example.com/file", + ), + ), + )), + " and ".into(), + acp::ContentBlock::ResourceLink(acp::ResourceLink::new( + "link", + "https://example.com/link", + )), + message_test_image(), + ]; + + thread.update(cx, |thread, cx| { + for chunk in &chunks { + thread.push_user_content_block(None, chunk.clone(), cx); + } + let [AgentThreadEntry::UserMessage(message)] = thread.entries() else { + panic!("expected one user message"); + }; + assert_eq!(message.chunks, chunks); + assert!(matches!( + message.content.blocks(), + [ContentBlock::Markdown { .. }, ContentBlock::Image { .. }] + )); + let expected = format!( + "read {} and {}\n\n`Image`", + ContentBlock::resource_link_md("https://example.com/file", PathStyle::local()), + ContentBlock::resource_link_md("https://example.com/link", PathStyle::local()), + ); + assert_eq!(message.content.to_markdown(cx), expected); + assert!( + !message + .content + .to_markdown(cx) + .contains("attached file contents") + ); + }); + } + #[test] fn text_resource_markdown_uses_mime_type_for_code_blocks() { let shell = acp::TextResourceContents::new("echo 'hello from exec test'", "tool://preview") @@ -5267,8 +5596,7 @@ mod tests { ), )); - let block = - ContentBlock::new_output(content, &language_registry, PathStyle::local(), cx); + let block = ContentBlock::new_output(content, &language_registry, cx); let ContentBlock::EmbeddedResource { resource, markdown } = &block else { panic!("expected embedded resource block, got {block:?}"); @@ -5302,7 +5630,6 @@ mod tests { ), )), &language_registry, - PathStyle::local(), cx, ); assert_eq!(untyped.to_markdown(cx), "```\n# plain preview\n```"); @@ -5332,7 +5659,6 @@ mod tests { let block = ContentBlock::new_output( image_blob, &language_registry, - PathStyle::local(), cx, ); @@ -5365,8 +5691,7 @@ mod tests { ), )); - let block = - ContentBlock::new_output(archive_blob, &language_registry, PathStyle::local(), cx); + let block = ContentBlock::new_output(archive_blob, &language_registry, cx); let ContentBlock::EmbeddedResource { resource, markdown } = &block else { panic!("expected embedded resource block, got {block:?}"); @@ -5388,12 +5713,7 @@ mod tests { .mime_type("image/png".to_string()), ), )); - let invalid = ContentBlock::new_output( - invalid_image_blob, - &language_registry, - PathStyle::local(), - cx, - ); + let invalid = ContentBlock::new_output(invalid_image_blob, &language_registry, cx); let ContentBlock::EmbeddedResource { resource, markdown } = &invalid else { panic!("expected embedded resource block, got {invalid:?}"); }; @@ -7131,7 +7451,6 @@ mod tests { call, ToolCallStatus::Pending, languages.clone(), - PathStyle::local(), &HashMap::default(), cx, ) @@ -7172,7 +7491,6 @@ mod tests { acp::ToolCall::new("tool", ""), ToolCallStatus::Pending, languages.clone(), - PathStyle::local(), &HashMap::default(), cx, ) @@ -7228,15 +7546,8 @@ mod tests { ), ] { cx.update(|cx| { - call.update_fields( - update, - None, - languages.clone(), - PathStyle::local(), - &HashMap::default(), - cx, - ) - .expect("tool label update should apply"); + call.update_fields(update, None, languages.clone(), &HashMap::default(), cx) + .expect("tool label update should apply"); }); cx.run_until_parked(); cx.read(|cx| { @@ -10073,7 +10384,6 @@ mod tests { let markdown_text = markdown.read(cx).source(); assert!(markdown_text.contains("Tool call not found")); } - ContentBlock::Empty => panic!("Expected markdown content, got empty"), ContentBlock::ResourceLink { .. } => { panic!("Expected markdown content, got resource link") } @@ -10445,7 +10755,7 @@ mod tests { protocol_id: None, client_id: Some(ClientUserMessageId::new()), is_optimistic: true, - content: ContentBlock::Empty, + content: MessageContent::default(), chunks: vec!["Injected message (no checkpoint)".into()], checkpoint: None, indented: false, diff --git a/crates/agent_ui/src/conversation_view.rs b/crates/agent_ui/src/conversation_view.rs index 242e384f54..9c15fe2eee 100644 --- a/crates/agent_ui/src/conversation_view.rs +++ b/crates/agent_ui/src/conversation_view.rs @@ -5680,6 +5680,31 @@ pub(crate) mod tests { setup_conversation_view_with_initial_content_opt(agent, None, cx).await } + fn png_image() -> acp::ContentBlock { + acp::ContentBlock::Image(acp::ImageContent::new( + "iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAADUlEQVR42mNkYPhfDwAChwGA60e6kgAAAABJRU5ErkJggg==", + "image/png", + )) + } + + async fn setup_full_size_conversation_and_send<'a>( + connection: StubAgentConnection, + prompt: &str, + cx: &'a mut TestAppContext, + ) -> (Entity, &'a mut VisualTestContext) { + let (conversation_view, cx) = + setup_conversation_view(StubAgentServer::new(connection), cx).await; + add_to_workspace_with_size(conversation_view.clone(), true, cx); + let thread = + active_thread(&conversation_view, cx).read_with(cx, |view, _| view.thread.clone()); + thread + .update(cx, |thread, cx| thread.send_raw(prompt, cx)) + .await + .expect("prompt should succeed"); + cx.run_until_parked(); + (conversation_view, cx) + } + #[gpui::test] async fn test_completed_plan_snapshot_keeps_list_state_in_sync(cx: &mut TestAppContext) { init_test(cx); @@ -5791,13 +5816,24 @@ pub(crate) mod tests { } fn add_to_workspace(conversation_view: Entity, cx: &mut VisualTestContext) { + add_to_workspace_with_size(conversation_view, false, cx); + } + + fn add_to_workspace_with_size( + conversation_view: Entity, + fill_space: bool, + cx: &mut VisualTestContext, + ) { let workspace = conversation_view.read_with(cx, |thread_view, _cx| thread_view.workspace.clone()); workspace .update_in(cx, |workspace, window, cx| { workspace.add_item_to_active_pane( - Box::new(cx.new(|_| ThreadViewItem(conversation_view.clone()))), + Box::new(cx.new(|_| ThreadViewItem { + conversation_view: conversation_view.clone(), + fill_space, + })), None, true, window, @@ -5807,7 +5843,10 @@ pub(crate) mod tests { .unwrap(); } - struct ThreadViewItem(Entity); + struct ThreadViewItem { + conversation_view: Entity, + fill_space: bool, + } impl Item for ThreadViewItem { type Event = (); @@ -5825,7 +5864,7 @@ pub(crate) mod tests { impl Focusable for ThreadViewItem { fn focus_handle(&self, cx: &App) -> FocusHandle { - self.0.read(cx).focus_handle(cx) + self.conversation_view.read(cx).focus_handle(cx) } } @@ -5834,12 +5873,15 @@ pub(crate) mod tests { // Render the title editor in the element tree too. In the real app // it is part of the agent panel let title_editor = self - .0 + .conversation_view .read(cx) .active_thread() .map(|t| t.read(cx).title_editor.clone()); - v_flex().children(title_editor).child(self.0.clone()) + v_flex() + .when(self.fill_space, |this| this.size_full()) + .children(title_editor) + .child(self.conversation_view.clone()) } } @@ -7265,6 +7307,17 @@ pub(crate) mod tests { acp::SessionUpdate::AgentThoughtChunk(acp::ContentChunk::new( "Hidden papaya reasoning.".into(), )), + acp::SessionUpdate::AgentThoughtChunk(acp::ContentChunk::new( + acp::ContentBlock::Resource(acp::EmbeddedResource::new( + acp::EmbeddedResourceResource::TextResourceContents( + acp::TextResourceContents::new( + "A second hidden papaya.", + "thought://details", + ) + .mime_type("text/markdown".to_string()), + ), + )), + )), acp::SessionUpdate::AgentMessageChunk(acp::ContentChunk::new( "Final answer without that fruit.".into(), )), @@ -7331,11 +7384,297 @@ pub(crate) mod tests { assert_eq!( bar.read_with(cx, |bar, _| bar.match_count()), - 1, - "expanded thinking content should be searchable", + 2, + "every Markdown leaf in expanded thinking content should be searchable", ); } + #[gpui::test] + async fn test_empty_assistant_text_followed_by_image_is_rendered(cx: &mut TestAppContext) { + init_test(cx); + let connection = StubAgentConnection::new(); + connection.set_next_prompt_updates(vec![ + acp::SessionUpdate::AgentMessageChunk(acp::ContentChunk::new("".into())), + acp::SessionUpdate::AgentMessageChunk(acp::ContentChunk::new(png_image())), + ]); + let (_conversation_view, cx) = + setup_full_size_conversation_and_send(connection, "Show an image", cx).await; + + let image_bounds = cx + .debug_bounds("agent-output-image") + .expect("image-only output must not be hidden as a blank assistant message"); + assert!( + image_bounds.size.width > px(0.) && image_bounds.size.height > px(0.), + "decoded image must occupy nonzero rendered bounds, got {image_bounds:?}", + ); + } + + #[gpui::test] + async fn test_mixed_assistant_content_search_and_copy_preserve_order(cx: &mut TestAppContext) { + init_test(cx); + + let image = png_image(); + let resource = acp::ContentBlock::Resource(acp::EmbeddedResource::new( + acp::EmbeddedResourceResource::TextResourceContents( + acp::TextResourceContents::new( + "Resource needle follows the image.", + "memory://ordered", + ) + .mime_type("text/markdown".to_string()), + ), + )); + let connection = StubAgentConnection::new(); + connection.set_next_prompt_updates(vec![ + acp::SessionUpdate::AgentThoughtChunk(acp::ContentChunk::new( + "Private needle must not be copied.".into(), + )), + acp::SessionUpdate::AgentMessageChunk(acp::ContentChunk::new("Before needle.".into())), + acp::SessionUpdate::AgentMessageChunk(acp::ContentChunk::new(image.clone())), + acp::SessionUpdate::AgentMessageChunk(acp::ContentChunk::new("After needle.".into())), + acp::SessionUpdate::AgentMessageChunk(acp::ContentChunk::new(image)), + acp::SessionUpdate::AgentMessageChunk(acp::ContentChunk::new(resource)), + ]); + + let (conversation_view, cx) = + setup_full_size_conversation_and_send(connection, "Show mixed content", cx).await; + let thread_view = active_thread(&conversation_view, cx); + let thread = thread_view.read_with(cx, |view, _| view.thread.clone()); + let first_image_bounds = cx + .debug_bounds("message-content-1-1-1") + .expect("first mixed-content image should be rendered"); + let second_image_bounds = cx + .debug_bounds("message-content-1-1-3") + .expect("second mixed-content image should be rendered"); + for image_bounds in [first_image_bounds, second_image_bounds] { + assert!( + image_bounds.size.width > px(0.) && image_bounds.size.height > px(0.), + "each decoded image must occupy nonzero rendered bounds, got {image_bounds:?}", + ); + } + assert!( + first_image_bounds.origin.y < second_image_bounds.origin.y, + "images must remain visible in content order: first {first_image_bounds:?}, second {second_image_bounds:?}", + ); + + let markdowns = thread.read_with(cx, |thread, _| { + let message = thread + .entries() + .iter() + .find_map(|entry| match entry { + AgentThreadEntry::AssistantMessage(message) => Some(message), + _ => None, + }) + .expect("assistant message should exist"); + message + .chunks + .iter() + .flat_map(|chunk| match chunk { + AssistantMessageChunk::Message { block, .. } + | AssistantMessageChunk::Thought { block, .. } => block.markdowns(), + }) + .cloned() + .collect::>() + }); + + let first_text_bounds = cx + .debug_bounds("message-content-1-1-0") + .expect("first text content"); + cx.simulate_mouse_down( + first_text_bounds.center(), + gpui::MouseButton::Right, + gpui::Modifiers::default(), + ); + cx.simulate_mouse_up( + first_text_bounds.center(), + gpui::MouseButton::Right, + gpui::Modifiers::default(), + ); + let copy_response = cx + .debug_bounds("MENU_ITEM-Copy This Agent Response") + .expect("Copy This Agent Response"); + cx.simulate_click(copy_response.center(), gpui::Modifiers::default()); + let copied = cx + .update(|_, cx| cx.read_from_clipboard().and_then(|item| item.text())) + .expect("assistant response should be copied"); + assert_eq!( + copied, + "Before needle.\n\n`Image`\n\nAfter needle.\n\n`Image`\n\nResource needle follows the image." + ); + assert!(!copied.contains("Private needle")); + + thread_view.update_in(cx, |view, window, cx| { + view.toggle_search(&crate::ToggleSearch, window, cx); + }); + let search_bar = thread_view + .read_with(cx, |view, _| view.thread_search_bar.clone()) + .expect("thread search should be open"); + search_bar.update_in(cx, |search_bar, window, cx| { + search_bar.query_editor.update(cx, |editor, cx| { + editor.set_text("needle", window, cx); + }); + search_bar.update_matches(window, cx); + }); + cx.run_until_parked(); + + assert_eq!(search_bar.read_with(cx, |bar, _| bar.match_count()), 3); + for (source, should_be_highlighted) in [ + ("Private needle must not be copied.", false), + ("Before needle.", true), + ("After needle.", true), + ("Resource needle follows the image.", true), + ] { + let markdown = markdowns + .iter() + .find(|markdown| markdown.read_with(cx, |markdown, _| markdown.source() == source)) + .unwrap_or_else(|| panic!("Markdown entity for {source:?} should exist")); + assert_eq!( + markdown.read_with(cx, |markdown, _| !markdown.search_highlights().is_empty()), + should_be_highlighted, + "unexpected search highlights for {source:?}", + ); + } + } + + #[gpui::test] + async fn test_message_context_menu_uses_clicked_content(cx: &mut TestAppContext) { + init_test(cx); + let acp::ContentBlock::Image(image) = png_image() else { + panic!("expected image fixture"); + }; + let linked_image = format!( + "[![second](data:{};base64,{})](https://two.example/)", + image.mime_type, image.data, + ); + let connection = StubAgentConnection::new(); + connection.set_next_prompt_updates(vec![ + acp::SessionUpdate::AgentMessageChunk(acp::ContentChunk::new( + "[first](https://one.example/)".into(), + )), + acp::SessionUpdate::AgentMessageChunk(acp::ContentChunk::new(png_image())), + acp::SessionUpdate::AgentMessageChunk(acp::ContentChunk::new(linked_image.into())), + ]); + let (conversation_view, cx) = + setup_conversation_view(StubAgentServer::new(connection), cx).await; + add_to_workspace_with_size(conversation_view.clone(), true, cx); + let thread_view = active_thread(&conversation_view, cx); + let thread = thread_view.read_with(cx, |view, _| view.thread.clone()); + thread + .update(cx, |thread, cx| thread.send_raw("Show links", cx)) + .await + .expect("link prompt"); + cx.run_until_parked(); + + for (selector, offset, expected) in [ + ( + "message-content-1-0-0", + point(px(12.), px(8.)), + "https://one.example/", + ), + ( + "message-content-1-0-2", + point(px(0.5), px(0.5)), + "https://two.example/", + ), + ] { + let bounds = cx.debug_bounds(selector).expect("content leaf"); + let position = bounds.origin + offset; + cx.simulate_mouse_down( + position, + gpui::MouseButton::Right, + gpui::Modifiers::default(), + ); + cx.simulate_mouse_up( + position, + gpui::MouseButton::Right, + gpui::Modifiers::default(), + ); + let copy = cx.debug_bounds("MENU_ITEM-Copy Link").expect("Copy Link"); + cx.simulate_click(copy.center(), gpui::Modifiers::default()); + let copied = cx.update(|_, cx| cx.read_from_clipboard().and_then(|item| item.text())); + assert_eq!(copied.as_deref(), Some(expected)); + } + + let image = cx + .debug_bounds("message-content-1-0-1") + .expect("image leaf"); + cx.simulate_mouse_down( + image.center(), + gpui::MouseButton::Right, + gpui::Modifiers::default(), + ); + cx.simulate_mouse_up( + image.center(), + gpui::MouseButton::Right, + gpui::Modifiers::default(), + ); + assert!(cx.debug_bounds("MENU_ITEM-Copy Link").is_none()); + assert!( + cx.debug_bounds("MENU_ITEM-Copy This Agent Response") + .is_some() + ); + } + + #[gpui::test] + async fn test_message_context_menu_copies_selected_content(cx: &mut TestAppContext) { + init_test(cx); + let connection = StubAgentConnection::new(); + connection.set_next_prompt_updates(vec![ + acp::SessionUpdate::AgentMessageChunk(acp::ContentChunk::new("**first**".into())), + acp::SessionUpdate::AgentMessageChunk(acp::ContentChunk::new(png_image())), + acp::SessionUpdate::AgentMessageChunk(acp::ContentChunk::new("*second*".into())), + ]); + let (conversation_view, cx) = + setup_conversation_view(StubAgentServer::new(connection), cx).await; + add_to_workspace_with_size(conversation_view.clone(), true, cx); + let thread_view = active_thread(&conversation_view, cx); + let thread = thread_view.read_with(cx, |view, _| view.thread.clone()); + thread + .update(cx, |thread, cx| thread.send_raw("Show text", cx)) + .await + .expect("text prompt"); + cx.run_until_parked(); + + for (selector, plain, markdown) in [ + ("message-content-1-0-0", "first", "**first**"), + ("message-content-1-0-2", "second", "*second*"), + ] { + for (menu_item, expected) in [ + ("MENU_ITEM-Copy", plain), + ("MENU_ITEM-Copy as Markdown", markdown), + ] { + let bounds = cx.debug_bounds(selector).expect("content leaf"); + let position = bounds.origin + point(px(12.), bounds.size.height / 2.); + cx.simulate_event(gpui::MouseDownEvent { + position, + button: gpui::MouseButton::Left, + modifiers: gpui::Modifiers::default(), + click_count: 2, + first_mouse: false, + }); + cx.simulate_mouse_up( + position, + gpui::MouseButton::Left, + gpui::Modifiers::default(), + ); + cx.simulate_mouse_down( + position, + gpui::MouseButton::Right, + gpui::Modifiers::default(), + ); + cx.simulate_mouse_up( + position, + gpui::MouseButton::Right, + gpui::Modifiers::default(), + ); + let copy = cx.debug_bounds(menu_item).expect("selection copy command"); + cx.simulate_click(copy.center(), gpui::Modifiers::default()); + let copied = + cx.update(|_, cx| cx.read_from_clipboard().and_then(|item| item.text())); + assert_eq!(copied.as_deref(), Some(expected)); + } + } + } + #[gpui::test] async fn test_thread_search_includes_expanded_tool_call_content(cx: &mut TestAppContext) { init_test(cx); @@ -7397,6 +7736,57 @@ pub(crate) mod tests { 1, "expanded tool-call content should be searchable", ); + + thread + .update(cx, |thread, cx| { + thread.handle_session_update( + acp::SessionUpdate::ToolCallUpdate(acp::ToolCallUpdate::new( + "search-tool-content", + acp::ToolCallUpdateFields::new().content(vec![ + acp::ToolCallContent::Content(acp::Content::new( + acp::ContentBlock::Text(acp::TextContent::new( + "Replacement contains mango.", + )), + )), + acp::ToolCallContent::Content(acp::Content::new( + acp::ContentBlock::Resource(acp::EmbeddedResource::new( + acp::EmbeddedResourceResource::TextResourceContents( + acp::TextResourceContents::new( + "Resource mango output.", + "tool://replacement", + ) + .mime_type("text/markdown".to_string()), + ), + )), + )), + ]), + )), + cx, + ) + }) + .expect("tool output replacement should apply"); + cx.run_until_parked(); + cx.executor() + .advance_clock(super::thread_search_bar::SEARCH_UPDATE_DEBOUNCE * 2); + cx.run_until_parked(); + assert_eq!( + bar.read_with(cx, |bar, _| bar.match_count()), + 0, + "replaced tool output should no longer match the old query", + ); + + bar.update_in(cx, |bar, window, cx| { + bar.query_editor.update(cx, |editor, cx| { + editor.set_text("mango", window, cx); + }); + bar.update_matches(window, cx); + }); + cx.run_until_parked(); + assert_eq!( + bar.read_with(cx, |bar, _| bar.match_count()), + 2, + "all Markdown leaves in replacement tool output should be searchable", + ); } #[gpui::test] @@ -7632,7 +8022,7 @@ pub(crate) mod tests { AgentThreadEntry::AssistantMessage(message) => { message.chunks.iter().find_map(|chunk| match chunk { AssistantMessageChunk::Message { block, .. } => { - block.markdown().cloned() + block.markdowns().next().cloned() } AssistantMessageChunk::Thought { .. } => None, }) diff --git a/crates/agent_ui/src/conversation_view/thread_search_bar.rs b/crates/agent_ui/src/conversation_view/thread_search_bar.rs index f8ee82e9e5..d0ac1e6bf4 100644 --- a/crates/agent_ui/src/conversation_view/thread_search_bar.rs +++ b/crates/agent_ui/src/conversation_view/thread_search_bar.rs @@ -891,18 +891,14 @@ fn collect_markdowns( for (chunk_ix, chunk) in message.chunks.iter().enumerate() { match chunk { AssistantMessageChunk::Message { block, .. } => { - if let Some(md) = block.markdown() { - out.push(md.clone()); - } + out.extend(block.markdowns().cloned()); } AssistantMessageChunk::Thought { block, .. } if entry_view_state .thinking_block_state((entry_ix, chunk_ix), cx) .0 => { - if let Some(md) = block.markdown() { - out.push(md.clone()); - } + out.extend(block.markdowns().cloned()); } AssistantMessageChunk::Thought { .. } => {} } @@ -974,7 +970,6 @@ mod tests { )), markdown: unsupported.clone(), }, - ContentBlock::Empty, ], error: Some(error.clone()), }; diff --git a/crates/agent_ui/src/conversation_view/thread_view.rs b/crates/agent_ui/src/conversation_view/thread_view.rs index e5a862e752..eb55ab757f 100644 --- a/crates/agent_ui/src/conversation_view/thread_view.rs +++ b/crates/agent_ui/src/conversation_view/thread_view.rs @@ -6353,43 +6353,31 @@ impl ThreadView { let mut is_blank = true; let is_last = entry_ix + 1 == total_entries; - let style = MarkdownStyle::themed(MarkdownFont::Agent, window, cx); let message_body = v_flex() .w_full() .gap_3() .children(chunks.iter().enumerate().filter_map( |(chunk_ix, chunk)| match chunk { AssistantMessageChunk::Message { block, .. } => { - block.markdown().and_then(|md| { - let this_is_blank = md.read(cx).source().trim().is_empty(); - is_blank = is_blank && this_is_blank; - if this_is_blank { - return None; - } - - Some( - self.render_markdown(md.clone(), style.clone(), cx) - .into_any_element(), - ) + let this_is_blank = !block.visible_content(cx); + is_blank = is_blank && this_is_blank; + (!this_is_blank).then(|| { + div() + .id(("assistant-message-chunk", chunk_ix)) + .child(self.render_message_content( + entry_ix, chunk_ix, block, window, cx, + )) + .into_any_element() }) } AssistantMessageChunk::Thought { block, .. } => { - block.markdown().and_then(|md| { - let this_is_blank = md.read(cx).source().trim().is_empty(); - is_blank = is_blank && this_is_blank; - if this_is_blank { - return None; - } - Some( - self.render_thinking_block( - entry_ix, - chunk_ix, - md.clone(), - window, - cx, - ) - .into_any_element(), + let this_is_blank = !block.visible_content(cx); + is_blank = is_blank && this_is_blank; + (!this_is_blank).then(|| { + self.render_thinking_block( + entry_ix, chunk_ix, block, window, cx, ) + .into_any_element() }) } }, @@ -6407,7 +6395,7 @@ impl ThreadView { .when(is_last, |this| this.pb_4()) .w_full() .text_ui(cx) - .child(self.render_message_context_menu(entry_ix, message_body, cx)) + .child(message_body) .when_some( self.entry_view_state .read(cx) @@ -7458,11 +7446,44 @@ impl ThreadView { cx.notify(); } + fn render_message_content( + &self, + entry_ix: usize, + chunk_ix: usize, + content: &acp_thread::MessageContent, + window: &Window, + cx: &Context, + ) -> Div { + v_flex().w_full().gap_3().children( + content + .blocks() + .iter() + .enumerate() + .filter(|(_, block)| block.visible_content(cx)) + .map(|(block_ix, block)| { + let content = self.render_output_content_block( + entry_ix, block_ix, block, None, false, window, cx, + ); + div() + .id(("message-content-block", block_ix)) + .debug_selector(move || { + format!("message-content-{entry_ix}-{chunk_ix}-{block_ix}") + }) + .child(self.render_message_context_menu( + entry_ix, + block.markdown().cloned(), + content, + cx, + )) + }), + ) + } + fn render_thinking_block( &self, entry_ix: usize, chunk_ix: usize, - chunk: Entity, + chunk: &acp_thread::MessageContent, window: &Window, cx: &Context, ) -> AnyElement { @@ -7487,6 +7508,7 @@ impl ThreadView { let panel_bg = cx.theme().colors().panel_background; v_flex() + .id(("thinking-block", chunk_ix)) .gap_1() .child( h_flex() @@ -7524,7 +7546,10 @@ impl ThreadView { ) .on_click(cx.listener(move |this, _event: &ClickEvent, window, cx| { this.toggle_thinking_block_expansion(key, window, cx); - })), + })) + .map(|header| { + self.render_message_context_menu(entry_ix, None, header.into_any(), cx) + }), ) .when(is_open, |this| { this.child( @@ -7542,11 +7567,11 @@ impl ThreadView { this.track_scroll(&scroll_handle) }) .overflow_hidden() - .child(self.render_markdown( - chunk, - MarkdownStyle::themed(MarkdownFont::Agent, window, cx), - cx, - )), + .child( + self.render_message_content( + entry_ix, chunk_ix, chunk, window, cx, + ), + ), ) .when(is_constrained, |this| { this.child( @@ -7570,6 +7595,7 @@ impl ThreadView { fn render_message_context_menu( &self, entry_ix: usize, + markdown: Option>, message_body: AnyElement, cx: &Context, ) -> AnyElement { @@ -7582,51 +7608,18 @@ impl ThreadView { let focus = window.focused(cx); let entity = entity.clone(); let workspace = workspace.clone(); + let markdown = markdown.clone(); ContextMenu::build(window, cx, move |menu, _, cx| { let this = entity.read(cx); let is_at_top = this.list_state.logical_scroll_top().item_ix == 0; - - let chunks = - this.thread.read(cx).entries().get(entry_ix).and_then( - |entry| match &entry { - AgentThreadEntry::AssistantMessage(msg) => Some(&msg.chunks), - _ => None, - }, - ); - - let context_menu_link = chunks.and_then(|chunks| { - chunks.iter().find_map(|chunk| { - let markdown = match chunk { - AssistantMessageChunk::Message { block, .. } => block.markdown(), - AssistantMessageChunk::Thought { block, .. } => block.markdown(), - }; - markdown - .and_then(|markdown| markdown.read(cx).context_menu_link().cloned()) - }) - }); - let selected_text = chunks.and_then(|chunks| { - chunks.iter().find_map(|chunk| { - let markdown = match chunk { - AssistantMessageChunk::Message { block, .. } => block.markdown(), - AssistantMessageChunk::Thought { block, .. } => block.markdown(), - }; - markdown.and_then(|markdown| { - markdown.read(cx).context_menu_selected_text().cloned() - }) - }) - }); - let selected_markdown = chunks.and_then(|chunks| { - chunks.iter().find_map(|chunk| { - let markdown = match chunk { - AssistantMessageChunk::Message { block, .. } => block.markdown(), - AssistantMessageChunk::Thought { block, .. } => block.markdown(), - }; - markdown.and_then(|markdown| { - markdown.read(cx).context_menu_selected_markdown().cloned() - }) - }) - }); + let markdown = markdown.as_ref().map(|markdown| markdown.read(cx)); + let context_menu_link = + markdown.and_then(|markdown| markdown.context_menu_link().cloned()); + let selected_text = markdown + .and_then(|markdown| markdown.context_menu_selected_text().cloned()); + let selected_markdown = markdown + .and_then(|markdown| markdown.context_menu_selected_markdown().cloned()); let copy_this_agent_response = ContextMenuEntry::new("Copy This Agent Response").handler({ @@ -7746,7 +7739,7 @@ impl ThreadView { if markdown.trim().is_empty() { None } else { - Some(markdown.to_string()) + Some(markdown) } } AssistantMessageChunk::Thought { .. } => None, @@ -10571,6 +10564,7 @@ impl ThreadView { cx: &Context, ) -> AnyElement { v_flex() + .debug_selector(|| "agent-output-image".into()) .gap_2() .map(|this| { if card_layout { diff --git a/crates/eval_cli/src/main.rs b/crates/eval_cli/src/main.rs index fc8074914c..e25730dc5e 100644 --- a/crates/eval_cli/src/main.rs +++ b/crates/eval_cli/src/main.rs @@ -971,7 +971,7 @@ fn log_acp_thread_event( if let Some(acp_thread::AgentThreadEntry::AssistantMessage(message)) = entries.last() { for chunk in &message.chunks { if let acp_thread::AssistantMessageChunk::Message { id: _, block } = chunk { - if let acp_thread::ContentBlock::Markdown { markdown } = block { + for markdown in block.markdowns() { let text = markdown.read(cx).source().to_string(); if !text.is_empty() { eprint!("{text}");