-
Notifications
You must be signed in to change notification settings - Fork 3.1k
fix: improve user feedback during long operations and on turn completion #2166
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -71,10 +71,29 @@ pub(crate) fn render_footer(f: &mut Frame, area: Rect, app: &mut App) { | |||||||||||||||||||||||||
| let dot_frame = footer_working_label_frame(now_ms, app.fancy_animations); | ||||||||||||||||||||||||||
| // Surface one compact live status row in the footer whenever a turn | ||||||||||||||||||||||||||
| // is live. Tool turns get the current action plus active/done counts; | ||||||||||||||||||||||||||
| // non-tool work falls back to the existing dot-pulse label. | ||||||||||||||||||||||||||
| // non-tool work falls back to a descriptive label with elapsed time. | ||||||||||||||||||||||||||
| let elapsed_secs = app | ||||||||||||||||||||||||||
| .turn_started_at | ||||||||||||||||||||||||||
| .map(|t| t.elapsed().as_secs()) | ||||||||||||||||||||||||||
| .unwrap_or(0); | ||||||||||||||||||||||||||
| let mut label = active_subagent_status_label(app) | ||||||||||||||||||||||||||
| .or_else(|| active_tool_status_label(app)) | ||||||||||||||||||||||||||
| .unwrap_or_else(|| crate::tui::widgets::footer_working_label(dot_frame, app.ui_locale)); | ||||||||||||||||||||||||||
| .unwrap_or_else(|| { | ||||||||||||||||||||||||||
| // Show a more specific label when the model is still loading | ||||||||||||||||||||||||||
| // or compacting, not just a generic "working…". | ||||||||||||||||||||||||||
| let base = if app.is_loading { | ||||||||||||||||||||||||||
| crate::tui::widgets::footer_working_label(dot_frame, app.ui_locale) | ||||||||||||||||||||||||||
| } else if app.is_compacting { | ||||||||||||||||||||||||||
| "compacting" | ||||||||||||||||||||||||||
| } else { | ||||||||||||||||||||||||||
| crate::tui::widgets::footer_working_label(dot_frame, app.ui_locale) | ||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||
|
Comment on lines
+84
to
+90
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||||||||||||||||
| if elapsed_secs > 0 { | ||||||||||||||||||||||||||
| format!("{base} ({elapsed_secs}s)") | ||||||||||||||||||||||||||
| } else { | ||||||||||||||||||||||||||
| base.to_string() | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||
| // Append stall reason when the turn has been running > 30 s. | ||||||||||||||||||||||||||
| if let Some(reason) = stall_reason(app) { | ||||||||||||||||||||||||||
| label = format!("{label} ({reason})"); | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1490,6 +1490,10 @@ async fn run_event_loop( | |
| } | ||
|
|
||
| // Generate post-turn receipt for completed turns. | ||
| // Also push a persistent status toast so users always | ||
| // see the outcome in the footer (not just the 8-second | ||
| // composer receipt), regardless of notification method | ||
| // or platform. | ||
| if status == crate::core::events::TurnOutcomeStatus::Completed { | ||
| let tool_count = app.tool_evidence.len(); | ||
| let mut receipt = "✓ turn completed".to_string(); | ||
|
|
@@ -1505,6 +1509,15 @@ async fn run_event_loop( | |
| } | ||
| } | ||
| app.set_receipt_text(receipt); | ||
| // Mirror as a persistent status toast (10s TTL). | ||
| // The footer bar visibly shows status toasts, | ||
| // which is more glanceable than the composer | ||
| // border receipt alone. | ||
| app.push_status_toast( | ||
| receipt, | ||
| crate::tui::app::StatusToastLevel::Info, | ||
| Some(10_000), | ||
| ); | ||
|
Comment on lines
+1512
to
+1520
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This will cause a To resolve this, you must clone app.set_receipt_text(receipt.clone());
Comment on lines
1511
to
+1520
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
The fix is to clone |
||
| } | ||
|
|
||
| // Auto-save completed turn and clear crash checkpoint. | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This block will fail to compile due to a type mismatch. The
ifbranch returns aString(fromfooter_working_label), but theelse ifbranch returns a&'static str("compacting"). In Rust, all branches of anifexpression must evaluate to the same type.Additionally,
"compacting"is a hardcoded user-facing string, which violates internationalization (i18n) best practices. Consider localizing this string in the future.