Skip to content

Conversation

@crickman
Copy link
Contributor

@crickman crickman commented Jun 27, 2024

Motivation and Context

Assist caller in avoiding an improper assignmemt of:

  • ChatMessageContent.AuthorRole
  • StreamingChatMessageContent.AuthorRole

Description

Inadvertent assignment of an empty string (or whitespace) to ChatMessageContant.AuthorName 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

@crickman crickman added PR: ready for review All feedback addressed, ready for reviews .NET Issue or Pull requests regarding .NET code kernel Issues or pull requests impacting the core kernel enhancement labels Jun 27, 2024
@crickman crickman self-assigned this Jun 27, 2024
@crickman crickman requested a review from a team as a code owner June 27, 2024 16:31
@crickman crickman added bug Something isn't working and removed enhancement kernel.core labels Jun 27, 2024
Copy link
Contributor

@glahaye glahaye left a comment

Choose a reason for hiding this comment

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

LGTM

@crickman crickman changed the title .Net - Convert whitespace or empty string AuthorName assignment to null .Net - Convert message AuthorName whitespace or empty string assignment to null Jun 27, 2024
@crickman crickman added this pull request to the merge queue Jun 27, 2024
Merged via the queue into main with commit 07bcd90 Jun 27, 2024
@crickman crickman deleted the fix_messagecontent_empty_authorname branch June 27, 2024 21:42
@crickman crickman removed the PR: ready for review All feedback addressed, ready for reviews label Jun 27, 2024
LudoCorporateShark pushed a commit to LudoCorporateShark/semantic-kernel that referenced this pull request Aug 25, 2024
…ment to `null` (microsoft#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 😄
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working kernel Issues or pull requests impacting the core kernel .NET Issue or Pull requests regarding .NET code

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

.Net AgentGroupChat got 'does not match '^[a-zA-Z0-9_-]{1,64}$' - 'messages.1.name' when using ToolCallBehavior.AutoInvokeKernelFunctions

4 participants