From 0f1e8a6e3dd6f8922d5c9d4d0cd9cb9bd1969cfb Mon Sep 17 00:00:00 2001 From: Ben Brandt Date: Sat, 21 Mar 2026 19:07:13 +0100 Subject: [PATCH] agent: Fix summarization model being cleared by unrelated registry events (#52080) ## Context We were seeing lots of pending title generation, which should only happen if we don't have a summarization model. `handle_models_updated_event` unconditionally overwrote the thread's summarization model on every registry event, even with `None`. We should only setting if explicitly changed by the user or we haven't set it yet, and only if we actually have one. It is hard to reproduce this issue.. but I don't think this code was right in the first place anyway. ## Self-Review Checklist - [x] I've reviewed my own diff for quality, security, and reliability - [x] Unsafe blocks (if any) have justifying comments - [x] The content is consistent with the [UI/UX checklist](https://github.com/zed-industries/zed/blob/main/CONTRIBUTING.md#uiux-checklist) - [x] Tests cover the new/changed behavior - [x] Performance impact has been considered and is acceptable Release Notes: - N/A --- crates/agent/src/agent.rs | 65 +++++++++++++++++++++++++++++++++++++-- 1 file changed, 63 insertions(+), 2 deletions(-) diff --git a/crates/agent/src/agent.rs b/crates/agent/src/agent.rs index 62a26f5b10672e3d1367d0fb7b085602a049df47..37dee2d97f44f7290ad9a084fccb3fc226f6de52 100644 --- a/crates/agent/src/agent.rs +++ b/crates/agent/src/agent.rs @@ -727,7 +727,7 @@ impl NativeAgent { fn handle_models_updated_event( &mut self, _registry: Entity, - _event: &language_model::Event, + event: &language_model::Event, cx: &mut Context, ) { self.models.refresh_list(cx); @@ -744,7 +744,13 @@ impl NativeAgent { thread.set_model(model, cx); cx.notify(); } - thread.set_summarization_model(summarization_model.clone(), cx); + if let Some(model) = summarization_model.clone() { + if thread.summarization_model().is_none() + || matches!(event, language_model::Event::ThreadSummaryModelChanged) + { + thread.set_summarization_model(Some(model), cx); + } + } }); } } @@ -2456,6 +2462,61 @@ mod internal_tests { }); } + #[gpui::test] + async fn test_summarization_model_survives_transient_registry_clearing( + cx: &mut TestAppContext, + ) { + init_test(cx); + let fs = FakeFs::new(cx.executor()); + fs.insert_tree("/", json!({ "a": {} })).await; + let project = Project::test(fs.clone(), [], cx).await; + + let thread_store = cx.new(|cx| ThreadStore::new(cx)); + let agent = + cx.update(|cx| NativeAgent::new(thread_store, Templates::new(), None, fs.clone(), cx)); + let connection = Rc::new(NativeAgentConnection(agent.clone())); + + let acp_thread = cx + .update(|cx| { + connection.clone().new_session( + project.clone(), + PathList::new(&[Path::new("/a")]), + cx, + ) + }) + .await + .unwrap(); + let session_id = acp_thread.read_with(cx, |thread, _| thread.session_id().clone()); + + let thread = agent.read_with(cx, |agent, _| { + agent.sessions.get(&session_id).unwrap().thread.clone() + }); + + thread.read_with(cx, |thread, _| { + assert!( + thread.summarization_model().is_some(), + "session should have a summarization model from the test registry" + ); + }); + + // Simulate what happens during a provider blip: + // update_active_language_model_from_settings calls set_default_model(None) + // when it can't resolve the model, clearing all fallbacks. + cx.update(|cx| { + LanguageModelRegistry::global(cx).update(cx, |registry, cx| { + registry.set_default_model(None, cx); + }); + }); + cx.run_until_parked(); + + thread.read_with(cx, |thread, _| { + assert!( + thread.summarization_model().is_some(), + "summarization model should survive a transient default model clearing" + ); + }); + } + #[gpui::test] async fn test_loaded_thread_preserves_thinking_enabled(cx: &mut TestAppContext) { init_test(cx);