-
Notifications
You must be signed in to change notification settings - Fork 840
Fix ChatMessage.CreatedAt being always overwritten by the latest timestamp. #6885
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.
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
Fix ChatMessage.CreatedAt being always overwritten by the latest timestamp. #6885
Conversation
Co-authored-by: stephentoub <[email protected]>
Co-authored-by: stephentoub <[email protected]>
...crosoft.Extensions.AI.Abstractions.Tests/ChatCompletion/ChatResponseUpdateExtensionsTests.cs
Outdated
Show resolved
Hide resolved
Co-authored-by: stephentoub <[email protected]>
...crosoft.Extensions.AI.Abstractions.Tests/ChatCompletion/ChatResponseUpdateExtensionsTests.cs
Outdated
Show resolved
Hide resolved
…bility Co-authored-by: stephentoub <[email protected]>
...crosoft.Extensions.AI.Abstractions.Tests/ChatCompletion/ChatResponseUpdateExtensionsTests.cs
Show resolved
Hide resolved
...crosoft.Extensions.AI.Abstractions.Tests/ChatCompletion/ChatResponseUpdateExtensionsTests.cs
Show resolved
Hide resolved
...crosoft.Extensions.AI.Abstractions.Tests/ChatCompletion/ChatResponseUpdateExtensionsTests.cs
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR fixes an issue where ChatMessage.CreatedAt and ChatResponse.CreatedAt timestamps were being overwritten by Unix epoch timestamps (1/1/1970) from LLM streaming responses, which some providers use to represent "null" timestamps.
Key changes:
- Updated timestamp comparison logic to only update when the new timestamp is later than the current one
- Added comprehensive test coverage for various timestamp scenarios including Unix epoch handling
- Applied the fix consistently to both message-level and response-level timestamp updates
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
src/Libraries/Microsoft.Extensions.AI.Abstractions/ChatCompletion/ChatResponseExtensions.cs |
Updated ProcessUpdate method to use timestamp comparison logic that prevents Unix epoch overwrites |
test/Libraries/Microsoft.Extensions.AI.Abstractions.Tests/ChatCompletion/ChatResponseUpdateExtensionsTests.cs |
Added comprehensive test case validating timestamp update behavior across multiple scenarios |
Summary
Fixed the issue where
ChatMessage.CreatedAtandChatResponse.CreatedAtwere being overwritten by Unix epoch timestamps from LLM responses.The fix changes the timestamp update logic to only update if:
This prevents "null" timestamps (represented as Unix epoch) from overwriting valid timestamps while still allowing proper chronological updates.
Changes
ChatResponseExtensions.ProcessUpdate()to use comparison logic similar toSpeechToTextResponseUpdateExtensionsToChatResponse_AlternativeTimestampsthat validates multiple timestamp scenariosToChatResponse_TimestampFoldingthat tests pairs of timestamps being folded together, using MemberData to avoid duplicationDateTimeOffset.UnixEpochwith manual construction for .NET Framework compatibilityFixes #6884
Original prompt
✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.
Microsoft Reviewers: Open in CodeFlow