Skip to content

fix(sidebar): refresh model after session switches #613

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.

Sign up for GitHub

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

Open
Usernames686 wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix(sidebar): refresh model after session switches #613

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 10 additions & 13 deletions src-tauri/src/commands/conversations.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down Expand Up @@ -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}"
),
}
}
Expand Down
85 changes: 43 additions & 42 deletions src-tauri/src/db/service/conversation_service.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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?;
Expand Down Expand Up @@ -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?;
Expand Down Expand Up @@ -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(
Expand All @@ -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 \
Expand All @@ -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(
Expand All @@ -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 \
Expand Down Expand Up @@ -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;
Expand All @@ -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;
Expand Down