mirror of
https://github.com/openai/codex.git
synced 2026-10-21 09:54:56 +00:00
Avoid recursive TUI event dispatch for model picker selections (#47717)
## Why Confirming an Astra model selection recursively polled the large event dispatcher on the TUI thread's stack. ## What changed Unwrap `AstraSelectedFromModelPicker` before dispatching its model update, then trigger the sparkle afterward when eligible. Keep the eligibility check and model update in the same event so queued backend updates cannot turn a no-op confirmation into a sparkle. ## Testing Add a regression test that confirms `gpt-6-astra` with `XHigh` reasoning on a thread with a 4 MiB stack and checks the resulting model and reasoning effort. GitOrigin-RevId: 89df8b417a9fd0107ebb918edeb2d240b53d42bd
This commit is contained in:
@@ -102,6 +102,21 @@ impl App {
|
||||
return Ok(AppRunControl::Continue);
|
||||
}
|
||||
|
||||
// Keep the picker check and model update in one event without recursively polling this
|
||||
// large dispatcher on the TUI thread's stack.
|
||||
let (event, sparkle_model) = match event {
|
||||
AppEvent::AstraSelectedFromModelPicker {
|
||||
thread_id,
|
||||
model,
|
||||
action,
|
||||
} => {
|
||||
let should_offer = self.chat_widget.current_model() != model
|
||||
&& self.chat_widget.sparkle_thread_for_picker_action(&model) == Some(thread_id);
|
||||
let next_event = action.into_app_event(model.clone());
|
||||
(next_event, should_offer.then_some(model))
|
||||
}
|
||||
event => (event, None),
|
||||
};
|
||||
match event {
|
||||
AppEvent::OpenDaemonMenu => self.open_daemon_menu(),
|
||||
AppEvent::ConfirmDaemonUpdate(source) => self.confirm_daemon_update(source),
|
||||
@@ -1904,22 +1919,7 @@ impl App {
|
||||
.await;
|
||||
}
|
||||
}
|
||||
AppEvent::AstraSelectedFromModelPicker { thread_id, model, action } => {
|
||||
// Check and apply in the same event so a queued backend update cannot turn a
|
||||
// no-op picker confirmation into a sparkle.
|
||||
let should_offer = self.chat_widget.current_model() != model
|
||||
&& self.chat_widget.sparkle_thread_for_picker_action(&model) == Some(thread_id);
|
||||
let control = Box::pin(self.handle_event(
|
||||
tui,
|
||||
app_server,
|
||||
action.into_app_event(model.clone()),
|
||||
))
|
||||
.await?;
|
||||
if should_offer {
|
||||
self.chat_widget.on_sparkle_model_selected_from_picker(&model);
|
||||
}
|
||||
return Ok(control);
|
||||
}
|
||||
AppEvent::AstraSelectedFromModelPicker { .. } => unreachable!("picker event unwrapped"),
|
||||
AppEvent::BackgroundVoiceError { thread_id, message } => {
|
||||
if self.chat_widget.thread_id() == Some(thread_id) {
|
||||
self.chat_widget.add_error_message(message);
|
||||
@@ -3172,6 +3172,10 @@ impl App {
|
||||
}
|
||||
}
|
||||
}
|
||||
if let Some(model) = sparkle_model {
|
||||
self.chat_widget
|
||||
.on_sparkle_model_selected_from_picker(&model);
|
||||
}
|
||||
Ok(AppRunControl::Continue)
|
||||
}
|
||||
|
||||
|
||||
@@ -571,6 +571,78 @@ async fn astra_picker_confirms_the_model_at_application_after_an_automatic_updat
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn astra_picker_xhigh_applies_on_a_bounded_stack() -> Result<()> {
|
||||
const STACK_SIZE: usize = 4 * 1024 * 1024;
|
||||
const SERVER_STACK_SIZE: usize = 8 * 1024 * 1024;
|
||||
const WORKER_THREADS: usize = 1;
|
||||
|
||||
std::thread::Builder::new()
|
||||
.name("astra-picker".into())
|
||||
.stack_size(STACK_SIZE)
|
||||
.spawn(|| {
|
||||
let runtime = tokio::runtime::Builder::new_multi_thread()
|
||||
.worker_threads(WORKER_THREADS)
|
||||
.thread_stack_size(SERVER_STACK_SIZE)
|
||||
.enable_all()
|
||||
.build()?;
|
||||
runtime.block_on(Box::pin(async {
|
||||
let (mut app, mut events, _ops) = Box::pin(make_test_app_with_channels()).await;
|
||||
app.config.model = Some("gpt-5.5".into());
|
||||
let mut server = Box::pin(start_config_write_test_app_server(&app)).await?;
|
||||
let mut tui = crate::tui::test_support::make_test_tui()?;
|
||||
let thread = server.start_thread(&app.config).await?;
|
||||
Box::pin(app.replace_chat_widget_with_app_server_thread(
|
||||
&mut tui,
|
||||
thread,
|
||||
ThreadAttachPresentation::Fresh,
|
||||
/*initial_user_message*/ None,
|
||||
))
|
||||
.await?;
|
||||
|
||||
let mut preset = crate::test_support::TEST_MODEL_PRESETS[0].clone();
|
||||
preset.model = "gpt-6-astra".into();
|
||||
preset.default_reasoning_effort = ReasoningEffortConfig::XHigh;
|
||||
preset.supported_reasoning_efforts = vec![
|
||||
ReasoningEffortPreset {
|
||||
effort: ReasoningEffortConfig::XHigh,
|
||||
description: "Extra High".into(),
|
||||
},
|
||||
ReasoningEffortPreset {
|
||||
effort: ReasoningEffortConfig::Low,
|
||||
description: "Low".into(),
|
||||
},
|
||||
];
|
||||
app.chat_widget.open_reasoning_popup(preset);
|
||||
app.chat_widget.handle_key_event(KeyCode::Enter.into());
|
||||
let mut confirmed = false;
|
||||
while let Ok(event) = events.try_recv() {
|
||||
confirmed |= matches!(event, AppEvent::AstraSelectedFromModelPicker { .. });
|
||||
if matches!(
|
||||
&event,
|
||||
AppEvent::AstraSelectedFromModelPicker { .. }
|
||||
| AppEvent::UpdateReasoningEffort(_)
|
||||
| AppEvent::PersistModelSelection { .. }
|
||||
) {
|
||||
Box::pin(app.handle_event(&mut tui, &mut server, event)).await?;
|
||||
}
|
||||
}
|
||||
assert!(confirmed);
|
||||
assert_eq!(
|
||||
(
|
||||
app.chat_widget.current_model(),
|
||||
app.chat_widget.current_reasoning_effort(),
|
||||
),
|
||||
("gpt-6-astra", Some(ReasoningEffortConfig::XHigh)),
|
||||
);
|
||||
server.shutdown().await?;
|
||||
Ok(())
|
||||
}))
|
||||
})?
|
||||
.join()
|
||||
.expect("Astra picker thread panicked")
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn session_only_astra_picker_shows_stars_only_on_an_untouched_task() -> Result<()> {
|
||||
let (mut app, mut events, _ops) = make_test_app_with_channels().await;
|
||||
|
||||
Reference in New Issue
Block a user