Preserve transcript position when opening settings pickers (#46910)

## Why

Opening a settings picker while reading older messages should preserve the
reading position. Settings errors and confirmations still need to be visible.

## What changed

- Keep the transcript anchored when opening `/model`, `/theme`, `/keymap`,
  `/memories`, `/title`, and `/statusline` without arguments.
- Resume transcript following for settings actions that report confirmations
  or errors, including save failures and unavailable commands.
- Keep inline commands and commands with arguments following their output.

## Testing

Add regression tests for picker open/close behavior, settings save failures,
confirmation output, and `/model` before startup completes. Include snapshots
of the preserved reading position and a visible theme save error.

GitOrigin-RevId: 0680e6414d43b8b5d2a3ab9a2df187abfb727d49
This commit is contained in:
Eric Traut
2026-09-21 00:16:33 +00:00
committed by copyberry
parent 88ef37c649
commit ee2149a9b4
6 changed files with 304 additions and 19 deletions
@@ -1,6 +1,7 @@
//! Apply backend-authorized fallback and task-local Reserve settings without saving defaults.
use super::App;
use crate::app_event::AppEvent;
use crate::app_server_session::AppServerSession;
use crate::chatwidget::AutomaticModelSwitchReason;
use crate::model_catalog::LUNA_RESERVE_MODEL;
@@ -25,6 +26,7 @@ impl App {
{
return;
}
self.app_event_tx.send(AppEvent::FollowTranscript);
let mut mode = self.chat_widget.effective_collaboration_mode();
mode.settings.reasoning_effort = effort.clone();
let params = ThreadSettingsUpdateParams {
@@ -851,6 +851,7 @@ impl App {
Ok(response) => response,
Err(err) => {
tracing::error!(error = %err, "failed to persist memory settings");
self.app_event_tx.send(AppEvent::FollowTranscript);
self.chat_widget
.add_error_message(format!("Failed to save memory settings: {err}"));
return false;
@@ -862,6 +863,7 @@ impl App {
message,
"memory settings config write was overridden by effective config"
);
self.app_event_tx.send(AppEvent::FollowTranscript);
self.chat_widget.add_error_message(format!(
"Memory setting changes were saved but not applied: {message}"
));
@@ -912,6 +914,7 @@ impl App {
if let Err(err) = app_server.thread_memory_mode_set(thread_id, mode).await {
tracing::error!(error = %err, %thread_id, "failed to update thread memory mode");
self.app_event_tx.send(AppEvent::FollowTranscript);
self.chat_widget.add_error_message(format!(
"Saved memory settings, but failed to update the current thread: {err}"
));
@@ -922,6 +925,7 @@ impl App {
&mut self,
app_server: &mut AppServerSession,
) {
self.app_event_tx.send(AppEvent::FollowTranscript);
if let Err(err) = app_server.memory_reset().await {
tracing::error!(error = %err, "failed to reset memories");
self.chat_widget
+15
View File
@@ -1962,6 +1962,7 @@ impl App {
self.chat_widget.open_advanced_reasoning_popup(model);
}
AppEvent::ApplyAdvancedReasoning { model, effort } => {
self.app_event_tx.send(AppEvent::FollowTranscript);
if self
.active_thread_model_setting_update_params(model.clone())
.is_some_and(|params| params.permissions.is_some())
@@ -2199,6 +2200,7 @@ impl App {
}
}
AppEvent::PersistModelSelection { model, effort } => {
self.app_event_tx.send(AppEvent::FollowTranscript);
match self.persist_model_defaults(
app_server.request_handle(),
crate::config_update::build_model_selection_edits(
@@ -2234,6 +2236,7 @@ impl App {
}
}
AppEvent::SelectSessionModel { model, effort } => {
self.app_event_tx.send(AppEvent::FollowTranscript);
self.select_session_model(app_server, model, effort).await;
}
AppEvent::CyberModelAutoReviewNotice => {
@@ -2449,6 +2452,7 @@ impl App {
}
}
AppEvent::PersistPlanModeReasoningEffort(effort) => {
self.app_event_tx.send(AppEvent::FollowTranscript);
let key_path = "plan_mode_reasoning_effort";
let edit = if let Some(effort) = effort {
crate::config_update::replace_config_value(
@@ -2899,6 +2903,7 @@ impl App {
Err(err) => {
let error = format_config_error(&err);
tracing::error!(error = %error, "failed to persist status line settings; keeping previous selection");
self.app_event_tx.send(AppEvent::FollowTranscript);
self.chat_widget.add_error_message(format!(
"Failed to save status line settings: {error}"
));
@@ -2938,6 +2943,7 @@ impl App {
}
Err(err) => {
tracing::error!(error = %err, "failed to persist terminal title items; keeping previous selection");
self.app_event_tx.send(AppEvent::FollowTranscript);
self.chat_widget.revert_terminal_title_setup_preview();
self.chat_widget.add_error_message(format!(
"Failed to save terminal title items: {err}"
@@ -2977,6 +2983,7 @@ impl App {
self.restore_runtime_theme_from_config();
self.refresh_status_line();
tracing::error!(error = %err, "failed to persist theme selection");
self.app_event_tx.send(AppEvent::FollowTranscript);
self.chat_widget
.add_error_message(format!("Failed to save theme: {err}"));
}
@@ -3113,6 +3120,7 @@ impl App {
) {
Ok(outcome) => outcome,
Err(err) => {
self.app_event_tx.send(AppEvent::FollowTranscript);
self.chat_widget.add_error_message(err);
return;
}
@@ -3124,6 +3132,7 @@ impl App {
message,
} => (*keymap_config, bindings, message),
crate::keymap_setup::KeymapEditOutcome::Unchanged { message } => {
self.app_event_tx.send(AppEvent::FollowTranscript);
self.chat_widget.add_info_message(message, /*hint*/ None);
return;
}
@@ -3161,10 +3170,12 @@ impl App {
self.sync_side_thread_ui();
self.chat_widget
.return_to_keymap_picker(&context, &action, &runtime_keymap);
self.app_event_tx.send(AppEvent::FollowTranscript);
self.chat_widget.add_info_message(message, /*hint*/ None);
}
Err(err) => {
tracing::error!(error = %err, "failed to persist keymap binding");
self.app_event_tx.send(AppEvent::FollowTranscript);
self.chat_widget
.add_error_message(format!("Failed to save shortcut: {err}"));
}
@@ -3186,6 +3197,7 @@ impl App {
) {
Ok(keymap_config) => keymap_config,
Err(err) => {
self.app_event_tx.send(AppEvent::FollowTranscript);
self.chat_widget.add_error_message(err);
return;
}
@@ -3194,6 +3206,7 @@ impl App {
let runtime_keymap = match RuntimeKeymap::from_config(&keymap_config) {
Ok(runtime_keymap) => runtime_keymap,
Err(err) => {
self.app_event_tx.send(AppEvent::FollowTranscript);
self.chat_widget
.add_error_message(format!("Failed to refresh shortcuts: {err}"));
return;
@@ -3215,6 +3228,7 @@ impl App {
self.sync_side_thread_ui();
self.chat_widget
.return_to_keymap_picker(&context, &action, &runtime_keymap);
self.app_event_tx.send(AppEvent::FollowTranscript);
self.chat_widget.add_info_message(
format!("Removed custom shortcut for `{context}.{action}`."),
/*hint*/ None,
@@ -3222,6 +3236,7 @@ impl App {
}
Err(err) => {
tracing::error!(error = %err, "failed to clear keymap binding");
self.app_event_tx.send(AppEvent::FollowTranscript);
self.chat_widget
.add_error_message(format!("Failed to remove shortcut: {err}"));
}
+14 -8
View File
@@ -40,14 +40,20 @@ impl App {
"{label} setting saved on the server for new threads. This thread is unchanged. Project or task settings may override it."
)))
}
Ok(response) => Box::new(history_cell::new_error_event(format!(
"{label} setting was saved but is overridden: {}",
overridden_write_message(&response)
))),
Err(err) => Box::new(history_cell::new_error_event(format!(
"Failed to save {label} setting: {}",
crate::config_update::format_config_error(&err)
))),
Ok(response) => {
self.transcript_view.jump_to_latest();
Box::new(history_cell::new_error_event(format!(
"{label} setting was saved but is overridden: {}",
overridden_write_message(&response)
)))
}
Err(err) => {
self.transcript_view.jump_to_latest();
Box::new(history_cell::new_error_event(format!(
"Failed to save {label} setting: {}",
crate::config_update::format_config_error(&err)
)))
}
};
self.insert_history_cell(tui, notice);
}
@@ -47,6 +47,244 @@ fn submit_local_command(app: &mut App, command: &str) {
assert_eq!(app.chat_widget.composer_text_with_pending(), "");
}
#[tokio::test]
async fn settings_pickers_preserve_the_reading_anchor_through_open_and_close() -> Result<()> {
let (mut app, mut events, _op_rx) = make_test_app_with_channels().await;
let mut tui = crate::tui::test_support::make_test_tui()?;
let mut server = start_config_write_test_app_server(&app).await?;
tui.set_owned_screen(/*owned*/ true)?;
app.chat_widget.handle_thread_session(test_thread_session(
ThreadId::new(),
app.config.cwd.to_path_buf(),
));
app.sync_tui_theme_selection("base16-ocean-dark".to_string());
hold_older_history(&mut app, &mut tui);
let held = transcript_buffer(&mut app);
for (command, close) in [
("/model", KeyCode::Esc),
("/theme", KeyCode::Enter),
("/keymap", KeyCode::Esc),
("/memories", KeyCode::Esc),
("/title", KeyCode::Esc),
("/statusline", KeyCode::Esc),
] {
while events.try_recv().is_ok() {}
submit_local_command(&mut app, command);
while let Ok(event) = events.try_recv() {
app.handle_event(&mut tui, &mut server, event).await?;
}
assert!(app.chat_widget.has_active_view(), "{command}");
assert!(!app.transcript_view.is_following(), "{command} opened");
assert_eq!(transcript_buffer(&mut app), held, "{command} opened");
app.handle_tui_event(&mut tui, &mut server, TuiEvent::Key(close.into()))
.await?;
let mut selected_theme = None;
while let Ok(event) = events.try_recv() {
if let AppEvent::SyntaxThemeSelected { name } = &event {
selected_theme = Some(name.clone());
}
app.handle_event(&mut tui, &mut server, event).await?;
}
if command == "/theme" {
assert_eq!(selected_theme, Some("base16-ocean-dark".to_string()));
}
assert!(!app.chat_widget.has_active_view(), "{command}");
assert!(!app.transcript_view.is_following(), "{command} closed");
assert_eq!(transcript_buffer(&mut app), held, "{command} closed");
}
let visible = transcript_buffer(&mut app)
.content
.chunks(/*chunk_size*/ 80)
.map(|row| {
row.iter()
.map(ratatui::buffer::Cell::symbol)
.collect::<String>()
.trim_end()
.to_owned()
})
.collect::<Vec<_>>()
.join("\n");
insta::assert_snapshot!(visible, @"
older 0
older 1
older 2
");
tui.set_owned_screen(/*owned*/ false)?;
server.shutdown().await?;
Ok(())
}
#[tokio::test]
async fn settings_picker_save_failures_reveal_their_errors() -> Result<()> {
let (mut app, mut events, _op_rx) = make_test_app_with_channels().await;
let mut tui = crate::tui::test_support::make_test_tui()?;
let mut server = start_config_write_test_app_server(&app).await?;
tui.set_owned_screen(/*owned*/ true)?;
app.sync_tui_theme_selection("base16-ocean-dark".to_string());
// Break writes only after startup, as can happen when the config is edited externally.
std::fs::write(app.local_settings.user_config_path.as_path(), "[broken")?;
for event in [
AppEvent::SyntaxThemeSelected {
name: "base16-ocean-dark".into(),
},
AppEvent::StatusLineSetup {
items: vec![],
use_theme_colors: false,
},
AppEvent::TerminalTitleSetup { items: vec![] },
AppEvent::PersistModelSelection {
model: "gpt-5.5".into(),
effort: Some(ReasoningEffortConfig::High),
},
AppEvent::ApplyAdvancedReasoning {
model: "gpt-5.5".into(),
effort: ReasoningEffortConfig::Ultra,
},
AppEvent::PersistPlanModeReasoningEffort(Some(ReasoningEffortConfig::High)),
AppEvent::UpdateMemorySettings {
use_memories: true,
generate_memories: true,
},
AppEvent::EnableFeatureForNewThreads(Feature::MemoryTool),
AppEvent::KeymapCaptured {
context: "global".into(),
action: "find_transcript".into(),
key: "f12".into(),
intent: crate::app_event::KeymapEditIntent::ReplaceAll,
},
AppEvent::KeymapCleared {
context: "global".into(),
action: "find_transcript".into(),
},
] {
hold_older_history(&mut app, &mut tui);
while events.try_recv().is_ok() {}
let selection = format!("{event:?}");
let is_theme = matches!(event, AppEvent::SyntaxThemeSelected { .. });
if is_theme {
submit_local_command(&mut app, "/theme");
app.handle_tui_event(&mut tui, &mut server, TuiEvent::Key(KeyCode::Enter.into()))
.await?;
} else {
app.handle_event(&mut tui, &mut server, event).await?;
}
while let Ok(event) = events.try_recv() {
app.handle_event(&mut tui, &mut server, event).await?;
}
assert!(app.transcript_view.is_following(), "{selection}");
let error = app.transcript_cells.last().expect("save error");
assert!(
lines_to_single_string(&error.display_lines(/*width*/ 80)).contains("Failed to"),
"{selection}"
);
if is_theme {
let area = Rect::new(
/*x*/ 0, /*y*/ 0, /*width*/ 80, /*height*/ 8,
);
let mut buffer = Buffer::empty(area);
app.transcript_view
.render(area, &mut buffer, &app.transcript_cells);
let visible = buffer
.content
.chunks(/*chunk_size*/ 80)
.map(|row| {
row.iter()
.map(ratatui::buffer::Cell::symbol)
.collect::<String>()
.trim_end()
.to_owned()
})
.collect::<Vec<_>>()
.join("\n");
insta::assert_snapshot!(visible, @"
older 4
older 5
■ Failed to save theme: TOML parse error at line 1, column 8
|
1 | [broken
| ^
unclosed table, expected `]`
");
}
}
tui.set_owned_screen(/*owned*/ false)?;
server.shutdown().await?;
Ok(())
}
#[tokio::test]
async fn settings_confirmations_reveal_their_history_output() -> Result<()> {
let (mut app, mut events, _op_rx) = make_test_app_with_channels().await;
let mut tui = crate::tui::test_support::make_test_tui()?;
let mut server = start_config_write_test_app_server(&app).await?;
tui.set_owned_screen(/*owned*/ true)?;
// A stale thread id makes the active-thread update fail independently of config writes.
let thread_id = ThreadId::new();
app.active_thread_id = Some(thread_id);
app.chat_widget
.handle_thread_session(test_thread_session(thread_id, app.config.cwd.to_path_buf()));
for (event, output) in [
(
AppEvent::SelectSessionModel {
model: "gpt-5.5".into(),
effort: Some(ReasoningEffortConfig::High),
},
"Failed to update thread settings",
),
(
AppEvent::UpdateLunaReserveReasoning {
thread_id,
effort: Some(ReasoningEffortConfig::High),
},
"Failed to update thread settings",
),
(AppEvent::ResetMemories, "Reset local memories."),
(
AppEvent::KeymapCaptured {
context: "global".into(),
action: "find_transcript".into(),
key: "f12".into(),
intent: crate::app_event::KeymapEditIntent::ReplaceAll,
},
"global.find_transcript",
),
(
AppEvent::KeymapCleared {
context: "global".into(),
action: "find_transcript".into(),
},
"Removed custom shortcut",
),
] {
hold_older_history(&mut app, &mut tui);
while events.try_recv().is_ok() {}
if matches!(event, AppEvent::UpdateLunaReserveReasoning { .. }) {
app.chat_widget
.set_model(crate::model_catalog::LUNA_RESERVE_MODEL);
}
let previous_cells = app.transcript_cells.len();
app.handle_event(&mut tui, &mut server, event).await?;
while let Ok(event) = events.try_recv() {
app.handle_event(&mut tui, &mut server, event).await?;
}
assert!(app.transcript_view.is_following());
let messages = app.transcript_cells[previous_cells..]
.iter()
.map(|cell| lines_to_single_string(&cell.display_lines(/*width*/ 80)))
.collect::<Vec<_>>()
.join("\n");
assert!(messages.contains(output), "expected {output:?}: {messages}");
}
tui.set_owned_screen(/*owned*/ false)?;
server.shutdown().await?;
Ok(())
}
#[tokio::test]
async fn local_command_and_inline_error_reveal_their_history_output() -> Result<()> {
let (mut app, mut events, _op_rx) = make_test_app_with_channels().await;
@@ -55,21 +293,22 @@ async fn local_command_and_inline_error_reveal_their_history_output() -> Result<
tui.set_owned_screen(/*owned*/ true)?;
for (command, output) in [
(
"/model",
"Model selection is disabled until startup completes.",
),
("/status", "/status"),
("/keymap invalid", "Usage: /keymap [debug]"),
] {
hold_older_history(&mut app, &mut tui);
while events.try_recv().is_ok() {}
submit_local_command(&mut app, command);
let follow = events.try_recv().expect("command follow event");
assert_matches!(&follow, AppEvent::FollowTranscript);
app.handle_event(&mut tui, &mut server, follow).await?;
let mut inserted = false;
while let Ok(event) = events.try_recv() {
if matches!(&event, AppEvent::InsertHistoryCell(_)) {
app.handle_event(&mut tui, &mut server, event).await?;
if matches!(event, AppEvent::InsertHistoryCell(_)) {
inserted = true;
}
app.handle_event(&mut tui, &mut server, event).await?;
}
assert!(inserted, "{command} should emit local history output");
assert!(app.transcript_view.is_following());
+25 -6
View File
@@ -3,7 +3,8 @@
//! The queue data itself lives in `input_queue`; this module owns the app-level
//! effects around taking composer input, submitting user turns, draining queued
//! follow-ups, and restoring draft state across interrupts or thread switches.
//! Composer submissions resume transcript following before dispatch or startup queueing.
//! Composer submissions resume transcript following before dispatch or startup queueing,
//! except for reversible settings pickers that preserve the reading position.
use super::*;
use crate::bottom_pane::prompt_args::parse_slash_name;
@@ -22,12 +23,26 @@ impl ChatWidget {
input_result: InputResult,
had_modal_or_popup: bool,
) {
if matches!(
&input_result,
let follow_transcript = match &input_result {
// Opening settings is not a request to leave the current reading anchor.
// Inline commands still follow so their output (including usage errors) is visible.
InputResult::Command(
SlashCommand::Model
| SlashCommand::Keymap
| SlashCommand::Memories
| SlashCommand::Title
| SlashCommand::Statusline
| SlashCommand::Theme,
) => false,
InputResult::Command(_)
| InputResult::ServiceTierCommand(_)
| InputResult::CommandWithArgs(..)
) {
| InputResult::ServiceTierCommand(_)
| InputResult::CommandWithArgs(..) => true,
InputResult::Submitted { .. }
| InputResult::Queued { .. }
| InputResult::ParentOwnedInputBlocked
| InputResult::None => false,
};
if follow_transcript {
self.app_event_tx.send(AppEvent::FollowTranscript);
}
match input_result {
@@ -80,6 +95,10 @@ impl ChatWidget {
}
InputResult::Command(cmd) => {
self.handle_slash_command_dispatch(cmd);
// A settings command can instead report why it is unavailable.
if !follow_transcript && self.bottom_pane.no_modal_or_popup_active() {
self.app_event_tx.send(AppEvent::FollowTranscript);
}
}
InputResult::ServiceTierCommand(command) => {
self.handle_service_tier_command_dispatch(command);