When thread title generation fails, display error and (#54130)
Release Notes: - Fixed an issue where thread titles remained in the generating state if the generation failed.
This commit is contained in:
1 parent
d877bd8a2f
commit
c641f135d2
4 files changed
+124
-25
No files matched your search
@@ -3160,6 +3160,66 @@ async fn test_title_generation(cx: &mut TestAppContext) {
|
||||
});
|
||||
}
|
||||
|
||||
#[gpui::test]
|
||||
async fn test_title_generation_failure_allows_retry(cx: &mut TestAppContext) {
|
||||
let ThreadTest { model, thread, .. } = setup(cx, TestModel::Fake).await;
|
||||
let fake_model = model.as_fake();
|
||||
|
||||
let summary_model = Arc::new(FakeLanguageModel::default());
|
||||
let fake_summary_model = summary_model.as_fake();
|
||||
thread.update(cx, |thread, cx| {
|
||||
thread.set_summarization_model(Some(summary_model.clone()), cx)
|
||||
});
|
||||
|
||||
let send = thread
|
||||
.update(cx, |thread, cx| {
|
||||
thread.send(UserMessageId::new(), ["Hello"], cx)
|
||||
})
|
||||
.unwrap();
|
||||
cx.run_until_parked();
|
||||
|
||||
fake_model.send_last_completion_stream_text_chunk("Hey!");
|
||||
fake_model.end_last_completion_stream();
|
||||
cx.run_until_parked();
|
||||
|
||||
fake_summary_model.send_last_completion_stream_error(
|
||||
LanguageModelCompletionError::UpstreamProviderError {
|
||||
message: "Internal server error".to_string(),
|
||||
status: gpui::http_client::StatusCode::INTERNAL_SERVER_ERROR,
|
||||
retry_after: None,
|
||||
},
|
||||
);
|
||||
fake_summary_model.end_last_completion_stream();
|
||||
send.collect::<Vec<_>>().await;
|
||||
cx.run_until_parked();
|
||||
|
||||
thread.read_with(cx, |thread, _| {
|
||||
assert_eq!(thread.title(), None);
|
||||
assert!(thread.has_failed_title_generation());
|
||||
assert!(!thread.is_generating_title());
|
||||
});
|
||||
|
||||
thread.update(cx, |thread, cx| {
|
||||
thread.generate_title(cx);
|
||||
});
|
||||
cx.run_until_parked();
|
||||
|
||||
thread.read_with(cx, |thread, _| {
|
||||
assert!(!thread.has_failed_title_generation());
|
||||
assert!(thread.is_generating_title());
|
||||
});
|
||||
|
||||
fake_summary_model.send_last_completion_stream_text_chunk("Retried title");
|
||||
fake_summary_model.end_last_completion_stream();
|
||||
cx.run_until_parked();
|
||||
|
||||
thread.read_with(cx, |thread, _| {
|
||||
assert_eq!(thread.title(), Some("Retried title".into()));
|
||||
assert!(!thread.has_failed_title_generation());
|
||||
assert!(!thread.is_generating_title());
|
||||
});
|
||||
}
|
||||
|
||||
#[gpui::test]
|
||||
async fn test_building_request_with_pending_tools(cx: &mut TestAppContext) {
|
||||
let ThreadTest { model, thread, .. } = setup(cx, TestModel::Fake).await;
|
||||
|
||||
+19
-12
@@ -939,6 +939,7 @@ pub struct Thread {
|
||||
updated_at: DateTime<Utc>,
|
||||
title: Option<SharedString>,
|
||||
pending_title_generation: Option<Task<()>>,
|
||||
title_generation_failed: bool,
|
||||
pending_summary_generation: Option<Shared<Task<Option<SharedString>>>>,
|
||||
summary: Option<SharedString>,
|
||||
messages: Vec<Message>,
|
||||
@@ -1065,6 +1066,7 @@ impl Thread {
|
||||
updated_at: Utc::now(),
|
||||
title: None,
|
||||
pending_title_generation: None,
|
||||
title_generation_failed: false,
|
||||
pending_summary_generation: None,
|
||||
summary: None,
|
||||
messages: Vec::new(),
|
||||
@@ -1298,6 +1300,7 @@ impl Thread {
|
||||
Some(db_thread.title.clone())
|
||||
},
|
||||
pending_title_generation: None,
|
||||
title_generation_failed: false,
|
||||
pending_summary_generation: None,
|
||||
summary: db_thread.detailed_summary,
|
||||
messages: db_thread.messages,
|
||||
@@ -2556,6 +2559,10 @@ impl Thread {
|
||||
self.pending_title_generation.is_some()
|
||||
}
|
||||
|
||||
pub fn has_failed_title_generation(&self) -> bool {
|
||||
self.title_generation_failed
|
||||
}
|
||||
|
||||
pub fn summary(&mut self, cx: &mut Context<Self>) -> Shared<Task<Option<SharedString>>> {
|
||||
if let Some(summary) = self.summary.as_ref() {
|
||||
return Task::ready(Some(summary.clone())).shared();
|
||||
@@ -2617,6 +2624,7 @@ impl Thread {
|
||||
}
|
||||
|
||||
pub fn generate_title(&mut self, cx: &mut Context<Self>) {
|
||||
self.title_generation_failed = false;
|
||||
let Some(model) = self.summarization_model.clone() else {
|
||||
return;
|
||||
};
|
||||
@@ -2664,28 +2672,27 @@ impl Thread {
|
||||
anyhow::Ok(())
|
||||
};
|
||||
|
||||
if generate
|
||||
let succeeded = generate
|
||||
.await
|
||||
.context("failed to generate thread title")
|
||||
.log_err()
|
||||
.is_some()
|
||||
{
|
||||
_ = this.update(cx, |this, cx| this.set_title(title.into(), cx));
|
||||
} else {
|
||||
// Emit TitleUpdated even on failure so that the propagation
|
||||
// chain (agent::Thread → NativeAgent → AcpThread) fires and
|
||||
// clears any provisional title that was set before the turn.
|
||||
_ = this.update(cx, |_, cx| {
|
||||
.is_some();
|
||||
_ = this.update(cx, |this, cx| {
|
||||
this.pending_title_generation = None;
|
||||
if succeeded {
|
||||
this.set_title(title.into(), cx);
|
||||
} else {
|
||||
this.title_generation_failed = true;
|
||||
cx.emit(TitleUpdated);
|
||||
cx.notify();
|
||||
});
|
||||
}
|
||||
_ = this.update(cx, |this, _| this.pending_title_generation = None);
|
||||
}
|
||||
});
|
||||
}));
|
||||
}
|
||||
|
||||
pub fn set_title(&mut self, title: SharedString, cx: &mut Context<Self>) {
|
||||
self.pending_title_generation = None;
|
||||
self.title_generation_failed = false;
|
||||
if Some(&title) != self.title.as_ref() {
|
||||
self.title = Some(title);
|
||||
cx.emit(TitleUpdated);
|
||||
|
||||
@@ -80,8 +80,8 @@ use terminal::terminal_settings::TerminalSettings;
|
||||
use terminal_view::{TerminalView, terminal_panel::TerminalPanel};
|
||||
use theme_settings::ThemeSettings;
|
||||
use ui::{
|
||||
Button, Callout, ContextMenu, ContextMenuEntry, PopoverMenu, PopoverMenuHandle, Tab, Tooltip,
|
||||
prelude::*, utils::WithRemSize,
|
||||
Button, Callout, ContextMenu, ContextMenuEntry, IconButton, PopoverMenu, PopoverMenuHandle,
|
||||
Tab, Tooltip, prelude::*, utils::WithRemSize,
|
||||
};
|
||||
use util::{ResultExt as _, debug_panic};
|
||||
use workspace::{
|
||||
@@ -3811,10 +3811,13 @@ impl AgentPanel {
|
||||
let content = match self.visible_surface() {
|
||||
VisibleSurface::AgentThread(conversation_view) => {
|
||||
let server_view_ref = conversation_view.read(cx);
|
||||
let is_generating_title = server_view_ref.as_native_thread(cx).is_some()
|
||||
&& server_view_ref.root_thread_view().map_or(false, |tv| {
|
||||
tv.read(cx).thread.read(cx).has_provisional_title()
|
||||
});
|
||||
let native_thread = server_view_ref.as_native_thread(cx);
|
||||
let is_generating_title = native_thread
|
||||
.as_ref()
|
||||
.is_some_and(|thread| thread.read(cx).is_generating_title());
|
||||
let title_generation_failed = native_thread
|
||||
.as_ref()
|
||||
.is_some_and(|thread| thread.read(cx).has_failed_title_generation());
|
||||
|
||||
if let Some(title_editor) = server_view_ref
|
||||
.root_thread_view()
|
||||
@@ -3833,8 +3836,8 @@ impl AgentPanel {
|
||||
)
|
||||
.into_any_element()
|
||||
} else {
|
||||
div()
|
||||
.w_full()
|
||||
let editable_title = div()
|
||||
.flex_1()
|
||||
.on_action({
|
||||
let conversation_view = conversation_view.downgrade();
|
||||
move |_: &menu::Confirm, window, cx| {
|
||||
@@ -3851,8 +3854,33 @@ impl AgentPanel {
|
||||
}
|
||||
}
|
||||
})
|
||||
.child(title_editor)
|
||||
.into_any_element()
|
||||
.child(title_editor);
|
||||
|
||||
if title_generation_failed {
|
||||
h_flex()
|
||||
.w_full()
|
||||
.gap_1()
|
||||
.items_center()
|
||||
.child(editable_title)
|
||||
.child(
|
||||
IconButton::new("retry-thread-title", IconName::XCircle)
|
||||
.icon_color(Color::Error)
|
||||
.icon_size(IconSize::Small)
|
||||
.tooltip(Tooltip::text("Title generation failed. Retry"))
|
||||
.on_click({
|
||||
let conversation_view = conversation_view.clone();
|
||||
move |_event, _window, cx| {
|
||||
Self::handle_regenerate_thread_title(
|
||||
conversation_view.clone(),
|
||||
cx,
|
||||
);
|
||||
}
|
||||
}),
|
||||
)
|
||||
.into_any_element()
|
||||
} else {
|
||||
editable_title.w_full().into_any_element()
|
||||
}
|
||||
}
|
||||
} else {
|
||||
Label::new(conversation_view.read(cx).title(cx))
|
||||
@@ -3883,7 +3911,10 @@ impl AgentPanel {
|
||||
conversation_view.update(cx, |conversation_view, cx| {
|
||||
if let Some(thread) = conversation_view.as_native_thread(cx) {
|
||||
thread.update(cx, |thread, cx| {
|
||||
thread.generate_title(cx);
|
||||
if !thread.is_generating_title() {
|
||||
thread.generate_title(cx);
|
||||
cx.notify();
|
||||
}
|
||||
});
|
||||
}
|
||||
});
|
||||
|
||||
@@ -4747,8 +4747,9 @@ fn all_thread_infos_for_workspace(
|
||||
let title = thread
|
||||
.title()
|
||||
.unwrap_or_else(|| DEFAULT_THREAD_TITLE.into());
|
||||
let is_native = thread_view_ref.as_native_thread(cx).is_some();
|
||||
let is_title_generating = is_native && thread.has_provisional_title();
|
||||
let is_title_generating = thread_view_ref
|
||||
.as_native_thread(cx)
|
||||
.is_some_and(|native_thread| native_thread.read(cx).is_generating_title());
|
||||
let session_id = thread.session_id().clone();
|
||||
let is_background = agent_panel.is_retained_thread(&conversation_thread_id);
|
||||
|
||||
|
||||
Reference in new issue
Block a user