Skip to content

Comments

Add copyWith and concatenate methods to ChatMessage#760

Merged
polina-c merged 17 commits intoflutter:mainfrom
polina-c:concat-message
Feb 24, 2026
Merged

Add copyWith and concatenate methods to ChatMessage#760
polina-c merged 17 commits intoflutter:mainfrom
polina-c:concat-message

Conversation

@polina-c
Copy link
Collaborator

No description provided.

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request introduces copyWith and concatenate methods, along with a + operator, to the ChatMessage class. The changes are well-implemented and include thorough tests, following standard Dart practices. My review identified a minor inaccuracy in the documentation for the + operator concerning how role and finishStatus are handled during concatenation. I've provided a suggestion to make these comments more precise and align them with the code's behavior.

polina-c and others added 3 commits February 24, 2026 10:47
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
@polina-c polina-c changed the title Add copyWith and concatenate methods and + operator to ChatMessage Add copyWith and concatenate methods to ChatMessage Feb 24, 2026
/// - The finish status of the first message is used if it's not null;
/// otherwise, the second message's finish status is used.
/// - If metadata keys are the same, the first message's metadata is used.
ChatMessage concatenate(ChatMessage other, {bool throwOnConflicts = true}) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I wonder if it would be better to just call this concatenateParts and just supply it with List<Part> to avoid the complications of combining the other fields of ChatMessage with this message.

Or maybe just remove throwOnConflicts entirely and make it always throw of these things don't combine well. The non-throwing version just feels like it will cause more problems than it will help.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yes, removed throwOnConflicts, thanks for good suggestion

@@ -189,7 +189,7 @@ final class ChatMessage {
return ChatMessage(
Copy link
Collaborator

Choose a reason for hiding this comment

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

This could use copyWith and only copy parts and status.

@polina-c polina-c merged commit d5663eb into flutter:main Feb 24, 2026
25 checks passed
@polina-c polina-c deleted the concat-message branch February 24, 2026 22:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants