Skip to content

Commit 4aef0fb

Browse files
agu-zrtfeldman
authored and
JosephTLyons
committed
agent: Handle thread title generation errors (#30273)
The title of a (text) thread would get stuck in "Loading Summary..." when the request to generate it failed. We now handle this case by falling back to the default title, and letting the user manually edit the title or retry generating it. https://github.com/user-attachments/assets/898d26ad-d31f-4b62-9b05-519d923b1b22 Release Notes: - agent: Handle thread title generation errors --------- Co-authored-by: Richard Feldman <[email protected]>
1 parent 9d00e26 commit 4aef0fb

File tree

11 files changed

+678
-141
lines changed

11 files changed

+678
-141
lines changed

‎crates/agent/src/active_thread.rs

+6-13
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ use crate::context_strip::{ContextStrip, ContextStripEvent, SuggestContextKind};
66
use crate::message_editor::insert_message_creases;
77
use crate::thread::{
88
LastRestoreCheckpoint, MessageCrease, MessageId, MessageSegment, Thread, ThreadError,
9-
ThreadEvent, ThreadFeedback,
9+
ThreadEvent, ThreadFeedback, ThreadSummary,
1010
};
1111
use crate::thread_store::{RulesLoadingError, TextThreadStore, ThreadStore};
1212
use crate::tool_use::{PendingToolUseStatus, ToolUse};
@@ -823,12 +823,12 @@ impl ActiveThread {
823823
self.messages.is_empty()
824824
}
825825

826-
pub fn summary(&self, cx: &App) -> Option<SharedString> {
826+
pub fn summary<'a>(&'a self, cx: &'a App) -> &'a ThreadSummary {
827827
self.thread.read(cx).summary()
828828
}
829829

830-
pub fn summary_or_default(&self, cx: &App) -> SharedString {
831-
self.thread.read(cx).summary_or_default()
830+
pub fn regenerate_summary(&self, cx: &mut App) {
831+
self.thread.update(cx, |thread, cx| thread.summarize(cx))
832832
}
833833

834834
pub fn cancel_last_completion(&mut self, window: &mut Window, cx: &mut App) -> bool {
@@ -1134,11 +1134,7 @@ impl ActiveThread {
11341134
return;
11351135
}
11361136

1137-
let title = self
1138-
.thread
1139-
.read(cx)
1140-
.summary()
1141-
.unwrap_or("Agent Panel".into());
1137+
let title = self.thread.read(cx).summary().unwrap_or("Agent Panel");
11421138

11431139
match AssistantSettings::get_global(cx).notify_when_agent_waiting {
11441140
NotifyWhenAgentWaiting::PrimaryScreen => {
@@ -3442,10 +3438,7 @@ pub(crate) fn open_active_thread_as_markdown(
34423438
workspace.update_in(cx, |workspace, window, cx| {
34433439
let thread = thread.read(cx);
34443440
let markdown = thread.to_markdown(cx)?;
3445-
let thread_summary = thread
3446-
.summary()
3447-
.map(|summary| summary.to_string())
3448-
.unwrap_or_else(|| "Thread".to_string());
3441+
let thread_summary = thread.summary().or_default().to_string();
34493442

34503443
let project = workspace.project().clone();
34513444

‎crates/agent/src/agent_diff.rs

+2-10
Original file line numberDiff line numberDiff line change
@@ -215,11 +215,7 @@ impl AgentDiffPane {
215215
}
216216

217217
fn update_title(&mut self, cx: &mut Context<Self>) {
218-
let new_title = self
219-
.thread
220-
.read(cx)
221-
.summary()
222-
.unwrap_or("Agent Changes".into());
218+
let new_title = self.thread.read(cx).summary().unwrap_or("Agent Changes");
223219
if new_title != self.title {
224220
self.title = new_title;
225221
cx.emit(EditorEvent::TitleChanged);
@@ -469,11 +465,7 @@ impl Item for AgentDiffPane {
469465
}
470466

471467
fn tab_content(&self, params: TabContentParams, _window: &Window, cx: &App) -> AnyElement {
472-
let summary = self
473-
.thread
474-
.read(cx)
475-
.summary()
476-
.unwrap_or("Agent Changes".into());
468+
let summary = self.thread.read(cx).summary().unwrap_or("Agent Changes");
477469
Label::new(format!("Review: {}", summary))
478470
.color(if params.selected {
479471
Color::Default

‎crates/agent/src/agent_panel.rs

+68-24
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,8 @@ use serde::{Deserialize, Serialize};
1010
use anyhow::{Result, anyhow};
1111
use assistant_context_editor::{
1212
AgentPanelDelegate, AssistantContext, ConfigurationError, ContextEditor, ContextEvent,
13-
SlashCommandCompletionProvider, humanize_token_count, make_lsp_adapter_delegate,
14-
render_remaining_tokens,
13+
ContextSummary, SlashCommandCompletionProvider, humanize_token_count,
14+
make_lsp_adapter_delegate, render_remaining_tokens,
1515
};
1616
use assistant_settings::{AssistantDockPosition, AssistantSettings};
1717
use assistant_slash_command::SlashCommandWorkingSet;
@@ -59,7 +59,7 @@ use crate::agent_configuration::{AgentConfiguration, AssistantConfigurationEvent
5959
use crate::agent_diff::AgentDiff;
6060
use crate::history_store::{HistoryStore, RecentEntry};
6161
use crate::message_editor::{MessageEditor, MessageEditorEvent};
62-
use crate::thread::{Thread, ThreadError, ThreadId, TokenUsageRatio};
62+
use crate::thread::{Thread, ThreadError, ThreadId, ThreadSummary, TokenUsageRatio};
6363
use crate::thread_history::{HistoryEntryElement, ThreadHistory};
6464
use crate::thread_store::ThreadStore;
6565
use crate::ui::AgentOnboardingModal;
@@ -196,7 +196,7 @@ impl ActiveView {
196196
}
197197

198198
pub fn thread(thread: Entity<Thread>, window: &mut Window, cx: &mut App) -> Self {
199-
let summary = thread.read(cx).summary_or_default();
199+
let summary = thread.read(cx).summary().or_default();
200200

201201
let editor = cx.new(|cx| {
202202
let mut editor = Editor::single_line(window, cx);
@@ -218,7 +218,7 @@ impl ActiveView {
218218
}
219219
EditorEvent::Blurred => {
220220
if editor.read(cx).text(cx).is_empty() {
221-
let summary = thread.read(cx).summary_or_default();
221+
let summary = thread.read(cx).summary().or_default();
222222

223223
editor.update(cx, |editor, cx| {
224224
editor.set_text(summary, window, cx);
@@ -233,7 +233,7 @@ impl ActiveView {
233233
let editor = editor.clone();
234234
move |thread, event, window, cx| match event {
235235
ThreadEvent::SummaryGenerated => {
236-
let summary = thread.read(cx).summary_or_default();
236+
let summary = thread.read(cx).summary().or_default();
237237

238238
editor.update(cx, |editor, cx| {
239239
editor.set_text(summary, window, cx);
@@ -296,7 +296,8 @@ impl ActiveView {
296296
.read(cx)
297297
.context()
298298
.read(cx)
299-
.summary_or_default();
299+
.summary()
300+
.or_default();
300301

301302
editor.update(cx, |editor, cx| {
302303
editor.set_text(summary, window, cx);
@@ -311,7 +312,7 @@ impl ActiveView {
311312
let editor = editor.clone();
312313
move |assistant_context, event, window, cx| match event {
313314
ContextEvent::SummaryGenerated => {
314-
let summary = assistant_context.read(cx).summary_or_default();
315+
let summary = assistant_context.read(cx).summary().or_default();
315316

316317
editor.update(cx, |editor, cx| {
317318
editor.set_text(summary, window, cx);
@@ -1452,38 +1453,59 @@ impl AgentPanel {
14521453
..
14531454
} => {
14541455
let active_thread = self.thread.read(cx);
1455-
let is_empty = active_thread.is_empty();
1456-
1457-
let summary = active_thread.summary(cx);
1456+
let state = if active_thread.is_empty() {
1457+
&ThreadSummary::Pending
1458+
} else {
1459+
active_thread.summary(cx)
1460+
};
14581461

1459-
if is_empty {
1460-
Label::new(Thread::DEFAULT_SUMMARY.clone())
1462+
match state {
1463+
ThreadSummary::Pending => Label::new(ThreadSummary::DEFAULT.clone())
14611464
.truncate()
1462-
.into_any_element()
1463-
} else if summary.is_none() {
1464-
Label::new(LOADING_SUMMARY_PLACEHOLDER)
1465+
.into_any_element(),
1466+
ThreadSummary::Generating => Label::new(LOADING_SUMMARY_PLACEHOLDER)
14651467
.truncate()
1466-
.into_any_element()
1467-
} else {
1468-
div()
1468+
.into_any_element(),
1469+
ThreadSummary::Ready(_) => div()
14691470
.w_full()
14701471
.child(change_title_editor.clone())
1471-
.into_any_element()
1472+
.into_any_element(),
1473+
ThreadSummary::Error => h_flex()
1474+
.w_full()
1475+
.child(change_title_editor.clone())
1476+
.child(
1477+
ui::IconButton::new("retry-summary-generation", IconName::RotateCcw)
1478+
.on_click({
1479+
let active_thread = self.thread.clone();
1480+
move |_, _window, cx| {
1481+
active_thread.update(cx, |thread, cx| {
1482+
thread.regenerate_summary(cx);
1483+
});
1484+
}
1485+
})
1486+
.tooltip(move |_window, cx| {
1487+
cx.new(|_| {
1488+
Tooltip::new("Failed to generate title")
1489+
.meta("Click to try again")
1490+
})
1491+
.into()
1492+
}),
1493+
)
1494+
.into_any_element(),
14721495
}
14731496
}
14741497
ActiveView::PromptEditor {
14751498
title_editor,
14761499
context_editor,
14771500
..
14781501
} => {
1479-
let context_editor = context_editor.read(cx);
1480-
let summary = context_editor.context().read(cx).summary();
1502+
let summary = context_editor.read(cx).context().read(cx).summary();
14811503

14821504
match summary {
1483-
None => Label::new(AssistantContext::DEFAULT_SUMMARY.clone())
1505+
ContextSummary::Pending => Label::new(ContextSummary::DEFAULT)
14841506
.truncate()
14851507
.into_any_element(),
1486-
Some(summary) => {
1508+
ContextSummary::Content(summary) => {
14871509
if summary.done {
14881510
div()
14891511
.w_full()
@@ -1495,6 +1517,28 @@ impl AgentPanel {
14951517
.into_any_element()
14961518
}
14971519
}
1520+
ContextSummary::Error => h_flex()
1521+
.w_full()
1522+
.child(title_editor.clone())
1523+
.child(
1524+
ui::IconButton::new("retry-summary-generation", IconName::RotateCcw)
1525+
.on_click({
1526+
let context_editor = context_editor.clone();
1527+
move |_, _window, cx| {
1528+
context_editor.update(cx, |context_editor, cx| {
1529+
context_editor.regenerate_summary(cx);
1530+
});
1531+
}
1532+
})
1533+
.tooltip(move |_window, cx| {
1534+
cx.new(|_| {
1535+
Tooltip::new("Failed to generate title")
1536+
.meta("Click to try again")
1537+
})
1538+
.into()
1539+
}),
1540+
)
1541+
.into_any_element(),
14981542
}
14991543
}
15001544
ActiveView::History => Label::new("History").truncate().into_any_element(),

‎crates/agent/src/context.rs

+3-8
Original file line numberDiff line numberDiff line change
@@ -586,20 +586,15 @@ impl ThreadContextHandle {
586586
}
587587

588588
pub fn title(&self, cx: &App) -> SharedString {
589-
self.thread
590-
.read(cx)
591-
.summary()
592-
.unwrap_or_else(|| "New thread".into())
589+
self.thread.read(cx).summary().or_default()
593590
}
594591

595592
fn load(self, cx: &App) -> Task<Option<(AgentContext, Vec<Entity<Buffer>>)>> {
596593
cx.spawn(async move |cx| {
597594
let text = Thread::wait_for_detailed_summary_or_text(&self.thread, cx).await?;
598595
let title = self
599596
.thread
600-
.read_with(cx, |thread, _cx| {
601-
thread.summary().unwrap_or_else(|| "New thread".into())
602-
})
597+
.read_with(cx, |thread, _cx| thread.summary().or_default())
603598
.ok()?;
604599
let context = AgentContext::Thread(ThreadContext {
605600
title,
@@ -642,7 +637,7 @@ impl TextThreadContextHandle {
642637
}
643638

644639
pub fn title(&self, cx: &App) -> SharedString {
645-
self.context.read(cx).summary_or_default()
640+
self.context.read(cx).summary().or_default()
646641
}
647642

648643
fn load(self, cx: &App) -> Task<Option<(AgentContext, Vec<Entity<Buffer>>)>> {

‎crates/agent/src/context_strip.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -160,7 +160,7 @@ impl ContextStrip {
160160
}
161161

162162
Some(SuggestedContext::Thread {
163-
name: active_thread.summary_or_default(),
163+
name: active_thread.summary().or_default(),
164164
thread: weak_active_thread,
165165
})
166166
} else if let Some(active_context_editor) = panel.active_context_editor() {
@@ -174,7 +174,7 @@ impl ContextStrip {
174174
}
175175

176176
Some(SuggestedContext::TextThread {
177-
name: context.summary_or_default(),
177+
name: context.summary().or_default(),
178178
context: weak_context,
179179
})
180180
} else {

‎crates/agent/src/history_store.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -71,8 +71,8 @@ impl Eq for RecentEntry {}
7171
impl RecentEntry {
7272
pub(crate) fn summary(&self, cx: &App) -> SharedString {
7373
match self {
74-
RecentEntry::Thread(_, thread) => thread.read(cx).summary_or_default(),
75-
RecentEntry::Context(context) => context.read(cx).summary_or_default(),
74+
RecentEntry::Thread(_, thread) => thread.read(cx).summary().or_default(),
75+
RecentEntry::Context(context) => context.read(cx).summary().or_default(),
7676
}
7777
}
7878
}

0 commit comments

Comments
 (0)