Skip to content

Commit 07bcd90

Browse files
authored
.Net - Convert message AuthorName whitespace or empty string assignment to null (#6990)
### Motivation and Context <!-- Thank you for your contribution to the semantic-kernel repo! Please help reviewers and future users, providing the following information: 1. Why is this change required? 2. What problem does it solve? 3. What scenario does it contribute to? 4. If it fixes an open issue, please link to the issue here. --> Assist caller in avoiding an improper assignmemt of: - `ChatMessageContent.AuthorRole` - `StreamingChatMessageContent.AuthorRole` ### Description <!-- Describe your changes, the overall approach, the underlying design. These notes will help understanding how your code works. Thanks! --> In advertent assignment of an empty string (or whitespace) results in an `HttpOperationException` when using the message as input to `IChatCompletionService`. While it may be ideal for caller to take care to _not_ assign such a value, intercepting this state at the earliest possible moment reduces debugging confusion. > Stronger validation may not be appropriate as it may be model specific. ### Contribution Checklist <!-- Before submitting this PR, please make sure: --> - [X] The code builds clean without any errors or warnings - [X] The PR follows the [SK Contribution Guidelines](https://github.com/microsoft/semantic-kernel/blob/main/CONTRIBUTING.md) and the [pre-submission formatting script](https://github.com/microsoft/semantic-kernel/blob/main/CONTRIBUTING.md#development-scripts) raises no violations - [X] All unit tests pass, and I have added new tests where possible - [X] I didn't break anyone 😄
1 parent 8381e50 commit 07bcd90

File tree

4 files changed

+50
-2
lines changed

4 files changed

+50
-2
lines changed

dotnet/src/SemanticKernel.Abstractions/Contents/ChatMessageContent.cs

+6-1
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,11 @@ public class ChatMessageContent : KernelContent
2020
/// </summary>
2121
[Experimental("SKEXP0001")]
2222
[JsonIgnore(Condition = JsonIgnoreCondition.WhenWritingNull)]
23-
public string? AuthorName { get; set; }
23+
public string? AuthorName
24+
{
25+
get => this._authorName;
26+
set => this._authorName = string.IsNullOrWhiteSpace(value) ? null : value;
27+
}
2428

2529
/// <summary>
2630
/// Role of the author of the message
@@ -171,4 +175,5 @@ public override string ToString()
171175

172176
private ChatMessageContentItemCollection? _items;
173177
private Encoding _encoding;
178+
private string? _authorName;
174179
}

dotnet/src/SemanticKernel.Abstractions/Contents/StreamingChatMessageContent.cs

+6-1
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,11 @@ public StreamingKernelContentItemCollection Items
6464
/// </summary>
6565
[Experimental("SKEXP0001")]
6666
[JsonIgnore(Condition = JsonIgnoreCondition.WhenWritingNull)]
67-
public string? AuthorName { get; set; }
67+
public string? AuthorName
68+
{
69+
get => this._authorName;
70+
set => this._authorName = string.IsNullOrWhiteSpace(value) ? null : value;
71+
}
6872

6973
/// <summary>
7074
/// Role of the author of the message
@@ -126,4 +130,5 @@ public StreamingChatMessageContent(AuthorRole? role, string? content, object? in
126130

127131
private StreamingKernelContentItemCollection? _items;
128132
private Encoding _encoding;
133+
private string? _authorName;
129134
}

dotnet/src/SemanticKernel.UnitTests/Contents/ChatMessageContentTests.cs

+19
Original file line numberDiff line numberDiff line change
@@ -128,6 +128,25 @@ public void ContentPropertyGetterShouldReturnContentOfTheFirstTextContentItem()
128128
Assert.Equal("fake-content-1", sut.Content);
129129
}
130130

131+
[Theory]
132+
[InlineData(null)]
133+
[InlineData("")]
134+
[InlineData(" ")]
135+
[InlineData("\t")]
136+
[InlineData("\n")]
137+
[InlineData("\r\n")]
138+
public void ContentPropertySetterShouldConvertEmptyOrWhitespaceAuthorNameToNull(string? authorName)
139+
{
140+
// Arrange
141+
var message = new ChatMessageContent(AuthorRole.User, content: null)
142+
{
143+
AuthorName = authorName
144+
};
145+
146+
// Assert
147+
Assert.Null(message.AuthorName);
148+
}
149+
131150
[Fact]
132151
public void ItShouldBePossibleToSetAndGetEncodingEvenIfThereAreNoItems()
133152
{

dotnet/src/SemanticKernel.UnitTests/Contents/StreamingChatMessageContentTests.cs

+19
Original file line numberDiff line numberDiff line change
@@ -118,6 +118,25 @@ public void ContentPropertyGetterShouldReturnContentOfTheFirstTextContentItem()
118118
Assert.Equal("fake-content-1", sut.Content);
119119
}
120120

121+
[Theory]
122+
[InlineData(null)]
123+
[InlineData("")]
124+
[InlineData(" ")]
125+
[InlineData("\t")]
126+
[InlineData("\n")]
127+
[InlineData("\r\n")]
128+
public void ContentPropertySetterShouldConvertEmptyOrWhitespaceAuthorNameToNull(string? authorName)
129+
{
130+
// Arrange
131+
var message = new StreamingChatMessageContent(AuthorRole.User, content: null)
132+
{
133+
AuthorName = authorName
134+
};
135+
136+
// Assert
137+
Assert.Null(message.AuthorName);
138+
}
139+
121140
[Fact]
122141
public void ItShouldBePossibleToSetAndGetEncodingEvenIfThereAreNoItems()
123142
{

0 commit comments

Comments
 (0)