Skip to content

Fix out of order messages #7472

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

Merged
DOsinga merged 2 commits into from
Feb 24, 2026
Merged

Fix out of order messages #7472

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
581065e
Fix out of order messages
Feb 24, 2026
845df2d
Sort by id
Feb 24, 2026
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
1 change: 1 addition & 0 deletions crates/goose/src/agents/agent.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1638,6 +1638,7 @@ impl Agent {
session_manager.add_message(&session_config.id, msg).await?;
}
conversation.extend(messages_to_add);
conversation.sort_by_created();

Copilot AI Feb 24, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorting the entire conversation on every loop iteration is O(n log n) and will get more expensive as history grows; consider only calling sort_by_created() when you actually inserted an out-of-order message (e.g., after tool-pair summarization) or when the newest message’s created is older than the current last message.

Suggested change
conversation.sort_by_created();
if !conversation.messages().is_empty() {
let messages_ref = conversation.messages();
if messages_ref.len() >= 2 {
let last_idx = messages_ref.len() - 1;
let last_created = messages_ref[last_idx].created;
let prev_created = messages_ref[last_idx - 1].created;
if last_created < prev_created {
conversation.sort_by_created();
}
}
}

Copilot uses AI. Check for mistakes.
if exit_chat {
break;
}
Expand Down
4 changes: 4 additions & 0 deletions crates/goose/src/conversation/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,10 @@ impl Conversation {
self.0.clear();
}

pub fn sort_by_created(&mut self) {
self.0.sort_by_key(|m| m.created);
}

@jamadeo jamadeo Feb 24, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what about making Conversation always keep them sorted? this method could be private and called on every mutable method

pub fn filtered_messages<F>(&self, filter: F) -> Vec<Message>
where
F: Fn(&MessageMetadata) -> bool,
Expand Down
2 changes: 1 addition & 1 deletion crates/goose/src/session/session_manager.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1082,7 +1082,7 @@ impl SessionStorage {
async fn get_conversation(&self, session_id: &str) -> Result<Conversation> {
let pool = self.pool().await?;
let rows = sqlx::query_as::<_, (String, String, i64, Option<String>, Option<String>)>(
"SELECT role, content_json, created_timestamp, metadata_json, message_id FROM messages WHERE session_id = ? ORDER BY timestamp",
"SELECT role, content_json, created_timestamp, metadata_json, message_id FROM messages WHERE session_id = ? ORDER BY created_timestamp",

Copilot AI Feb 24, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ORDER BY created_timestamp alone can still return messages out of order when multiple rows share the same created_timestamp (timestamps are second-granularity), so consider adding a deterministic tie-breaker like ORDER BY created_timestamp, id (the AUTOINCREMENT PK) to preserve insertion order for ties.

Suggested change
"SELECT role, content_json, created_timestamp, metadata_json, message_id FROM messages WHERE session_id = ? ORDER BY created_timestamp",
"SELECT role, content_json, created_timestamp, metadata_json, message_id FROM messages WHERE session_id = ? ORDER BY created_timestamp, rowid",

Copilot uses AI. Check for mistakes.

Copilot AI Feb 24, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This query now orders by created_timestamp but the schema only indexes timestamp, so fetching a long conversation may require a full sort; consider adding an index (ideally composite on (session_id, created_timestamp)) to keep this query efficient.

Copilot uses AI. Check for mistakes.

Copilot AI Feb 24, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are existing tests in this file, but this ordering change isn’t covered; adding a regression test that inserts messages with non-monotonic created_timestamp values and asserts get_conversation() returns them sorted would help prevent this from regressing.

Copilot uses AI. Check for mistakes.
)
.bind(session_id)
.fetch_all(pool)
Expand Down
Loading