Skip to content

Commit

Permalink
.Net: Align the behavior of the ChatMessageContent.Content property s…
Browse files Browse the repository at this point in the history
…etter with streaming counterpart (#6753)

### Motivation and Context
This is a follow-up fix for the `ChatMessageContent.Content` property
setter, similar to the one that was done to the
`StreamingChatMessageContent.Content` property when addressing the PR
comment:
#6449 (comment).

### Description
This change updates the text of the first text item regardless of the
value being set. This helps avoid inconsistency that occurs when text
content already exists in the items collection, null is set via the
Content property, and the setter does nothing, while the getter returns
the text message content:
```csharp
var items = new ChatMessageContentItemCollection
{
    new TextContent("Hi AI."),
};

var chatMessage = new ChatMessageContent(AuthorRole.User, items: items);
chatMessage.Content = null; // The property setter does nothing if the value is null.

// Fails
Assert.Null(chatMessage.Content); // The property getter returns the "Hi AI." text.
```
  • Loading branch information
SergeyMenshykh committed Jun 19, 2024
1 parent 656e0eb commit 95e4c42
Show file tree
Hide file tree
Showing 2 changed files with 20 additions and 11 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -41,18 +41,12 @@ public class ChatMessageContent : KernelContent
}
set
{
if (value is null)
{
return;
}

var textContent = this.Items.OfType<TextContent>().FirstOrDefault();
if (textContent is not null)
{
textContent.Text = value;
textContent.Encoding = this.Encoding;
}
else
else if (value is not null)
{
this.Items.Add(new TextContent(
text: value,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,8 +55,10 @@ public void ContentPropertySetterShouldAddTextContentToItemsCollection()
Assert.Contains(sut.Items, item => item is TextContent textContent && textContent.Text == "fake-content");
}

[Fact]
public void ContentPropertySetterShouldUpdateContentOfFirstTextContentItem()
[Theory]
[InlineData(null)]
[InlineData("fake-content-1-update")]
public void ContentPropertySetterShouldUpdateContentOfFirstTextContentItem(string? content)
{
// Arrange
var items = new ChatMessageContentItemCollection
Expand All @@ -68,10 +70,23 @@ public void ContentPropertySetterShouldUpdateContentOfFirstTextContentItem()

var sut = new ChatMessageContent(AuthorRole.User, items: items)
{
Content = "fake-content-1-update"
Content = content
};

Assert.Equal(content, ((TextContent)sut.Items[1]).Text);
}

[Fact]
public void ContentPropertySetterShouldNotAddTextContentToItemsCollection()
{
// Arrange
var sut = new ChatMessageContent(AuthorRole.User, content: null)
{
Content = null
};

Assert.Equal("fake-content-1-update", ((TextContent)sut.Items[1]).Text);
// Assert
Assert.Empty(sut.Items);
}

[Fact]
Expand Down

0 comments on commit 95e4c42

Please sign in to comment.