From 50e2393c11eda7b229384c3fd702171cd6193d74 Mon Sep 17 00:00:00 2001 From: Usernames686 <222501465+Usernames686@users.noreply.github.com> Date: Mon, 31 Aug 2026 13:11:56 +0800 Subject: [PATCH] fix(sidebar): refresh model after switches --- src-tauri/src/commands/conversations.rs | 23 +++-- .../src/db/service/conversation_service.rs | 85 ++++++++++--------- 2 files changed, 53 insertions(+), 55 deletions(-) diff --git a/src-tauri/src/commands/conversations.rs b/src-tauri/src/commands/conversations.rs index da4685dcce..0f575f11e6 100644 --- a/src-tauri/src/commands/conversations.rs +++ b/src-tauri/src/commands/conversations.rs @@ -1243,12 +1243,9 @@ pub async fn get_folder_conversation_core( // while the rollout file knows the answer. // // The parse WINS over the column rather than merely filling a hole in it. - // `seed_model_if_empty` now persists the first model a session is seen - // using, so "fill only when NULL" would pin this summary — and with it the - // details dialog, which reads `summary.model` ahead of the turns — to that - // first value for the life of the conversation, and a mid-session `/model` - // switch would never show. The stored value stays as the fallback for a - // transcript that names no model at all. + // `refresh_model` below persists this same parsed value for the sidebar, so + // both surfaces converge after a mid-session `/model` switch. The stored + // value stays as the fallback for a transcript that names no model at all. if let Some(parsed) = parsed_model.filter(|m| !m.trim().is_empty()) { summary.model = Some(parsed); } @@ -1527,17 +1524,17 @@ pub async fn get_folder_conversation_with_live_core( } } - // Session-model backfill, the sibling of the auto-title above and for the - // same reason: the row was inserted before any model was named, and the - // sidebar reads the row rather than the transcript this parse just walked. - // `seed_model_if_empty` re-checks emptiness in SQL, so once a session has a - // model this is a no-op that writes nothing. + // Session-model refresh, the sibling of the auto-title above: the row was + // inserted before any model was named, and the sidebar reads the row rather + // than the transcript this parse just walked. `refresh_model` compares in + // SQL, so an unchanged model is a no-op while a `/model` switch updates the + // sidebar projection. if let Some(model) = detail.summary.model.clone() { - match conversation_service::seed_model_if_empty(conn, conversation_id, &model).await { + match conversation_service::refresh_model(conn, conversation_id, &model).await { Ok(true) => upserted = true, Ok(false) => {} Err(e) => tracing::error!( - "[conversations] session-model backfill failed for {conversation_id}: {e}" + "[conversations] session-model refresh failed for {conversation_id}: {e}" ), } } diff --git a/src-tauri/src/db/service/conversation_service.rs b/src-tauri/src/db/service/conversation_service.rs index c8ad4dfc98..584aa23233 100644 --- a/src-tauri/src/db/service/conversation_service.rs +++ b/src-tauri/src/db/service/conversation_service.rs @@ -252,23 +252,17 @@ pub async fn seed_auto_title_if_empty( Ok(res.rows_affected > 0) } -/// Write the session's `model` ONLY when the row still has none. The sibling -/// of [`seed_auto_title_if_empty`], and for the same reason: a conversation -/// row is inserted before the agent has named a model, so the column is NULL -/// for every session started in-app and only the transcript knows the answer. -/// Without this the sidebar — which reads the row, not the transcript — could -/// only show a model for imported sessions. +/// Refresh the session's `model` when the transcript reports a different, +/// non-empty value. A conversation row is inserted before the agent has named +/// a model, and the transcript remains the richer source after a mid-session +/// model switch, while the sidebar reads this database projection. /// -/// First value wins. The detail view re-reads the transcript on every open and -/// stays exact, so the stored value is a cheap projection for the list rather -/// than a second source of truth; re-writing it on every model switch would -/// buy a row write per switch for a chip nobody reads mid-turn. -/// -/// Returns `true` when a row was written so the caller can broadcast a sidebar -/// upsert. Does not bump `updated_at` — the sidebar sorts on it, and merely -/// opening a conversation must not float it to the top of Recent (same -/// reasoning as [`update_pin`]). -pub async fn seed_model_if_empty( +/// The equality check and write happen in one conditional UPDATE so unchanged +/// detail reads stay write-free. Returns `true` when a row was written so the +/// caller can broadcast a sidebar upsert. Does not bump `updated_at` — the +/// sidebar sorts on it, and merely opening a conversation must not float it to +/// the top of Recent (same reasoning as [`update_pin`]). +pub async fn refresh_model( conn: &DatabaseConnection, conversation_id: i32, model: &str, @@ -285,7 +279,7 @@ pub async fn seed_model_if_empty( .filter( sea_orm::Condition::any() .add(conversation::Column::Model.is_null()) - .add(conversation::Column::Model.eq("")), + .add(conversation::Column::Model.ne(model)), ) .exec(conn) .await?; @@ -801,10 +795,9 @@ pub async fn bind_external_id( active.external_id = Set(Some(external_id.clone())); // The model described the session being released, and `carried` // has already taken it for the row that keeps S1's history. Left - // in place it would name S1's model on a row that is now S2 — - // and `seed_model_if_empty` only fills an EMPTY column, so - // nothing would ever correct it. Cleared, S2 seeds itself on its - // next open. + // in place it would name S1's model on a row that is now S2 until + // S2's transcript is parsed. Clear it so the projection never + // claims to know the new session's model prematurely. active.model = Set(None); active.updated_at = Set(now); active.update(txn).await?; @@ -1532,7 +1525,7 @@ mod tests { } #[tokio::test] - async fn seed_model_fills_an_empty_column_once_without_bumping_updated_at() { + async fn model_projection_tracks_latest_value_without_bumping_updated_at() { let db = fresh_in_memory_db().await; let folder = seed_folder(&db, "/tmp/codeg-seed-model").await; let conv = create( @@ -1553,7 +1546,7 @@ mod tests { let updated_at_before = before.updated_at; assert!( - seed_model_if_empty(&db.conn, conv.id, " gpt-5-codex ") + refresh_model(&db.conn, conv.id, " gpt-5-codex ") .await .expect("seed"), "an empty column must be filled, and report that it was so the \ @@ -1567,34 +1560,43 @@ mod tests { merely opening a conversation must not float it to the top" ); - // First value wins. The detail view re-reads the transcript and stays - // exact; the column is a projection for the list, not a second source - // of truth that fights the parse. + // Re-reading an unchanged transcript is a no-op, avoiding a database + // write and sidebar broadcast on every detail fetch. assert!( - !seed_model_if_empty(&db.conn, conv.id, "gpt-5.2") + !refresh_model(&db.conn, conv.id, "gpt-5-codex") .await - .expect("second seed"), - "a populated column must be left alone, and say nothing was written" + .expect("refresh unchanged"), + "an unchanged model must report that nothing was written" ); - assert_eq!( - get_by_id(&db.conn, conv.id) + + // A mid-session model switch must replace the sidebar projection. + assert!( + refresh_model(&db.conn, conv.id, "gpt-5.2") .await - .expect("get after") - .model - .as_deref(), - Some("gpt-5-codex") + .expect("refresh changed model"), + "a changed model must be stored and broadcast" + ); + let refreshed = get_by_id(&db.conn, conv.id).await.expect("get refreshed"); + assert_eq!( + refreshed.model.as_deref(), + Some("gpt-5.2"), + "the projection follows the transcript's latest model" + ); + assert_eq!( + refreshed.updated_at, updated_at_before, + "refreshing the model must not change sidebar recency" ); // A transcript that names no model asks for no write at all. assert!( - !seed_model_if_empty(&db.conn, conv.id, " ") + !refresh_model(&db.conn, conv.id, " ") .await .expect("blank seed") ); } #[tokio::test] - async fn seed_model_skips_a_soft_deleted_row() { + async fn refresh_model_skips_a_soft_deleted_row() { let db = fresh_in_memory_db().await; let folder = seed_folder(&db, "/tmp/codeg-seed-model-deleted").await; let conv = create( @@ -1609,7 +1611,7 @@ mod tests { soft_delete(&db.conn, conv.id).await.expect("delete"); assert!( - !seed_model_if_empty(&db.conn, conv.id, "gpt-5-codex") + !refresh_model(&db.conn, conv.id, "gpt-5-codex") .await .expect("seed"), "a deleted conversation is not something an open can resurrect a \ @@ -1756,7 +1758,7 @@ mod tests { bind_external_id(&db.conn, row.id, "S1", &[]) .await .expect("first bind"); - seed_model_if_empty(&db.conn, row.id, "gpt-5-codex") + refresh_model(&db.conn, row.id, "gpt-5-codex") .await .expect("seed S1's model"); let before = raw_row(&db.conn, row.id).await; @@ -1774,9 +1776,8 @@ mod tests { ); assert!( current.model.is_none(), - "the model described S1; left behind it would name S1's model on a \ - row that is now S2, and `seed_model_if_empty` only fills an EMPTY \ - column, so nothing would ever correct it" + "the model described S1; left behind it would temporarily name S1's \ + model on a row that is now S2" ); let preserved = raw_row(&db.conn, preserved_id).await;