Skip to content

Commit 254951e

Browse files
Copilotstephentoub
andauthored
Fix ChatMessage.CreatedAt being always overwritten by the latest timestamp. (#6885)
* Initial plan * Add failing test for Unix epoch timestamp issue Co-authored-by: stephentoub <[email protected]> * Fix CreatedAt timestamp overwrite by only updating if later Co-authored-by: stephentoub <[email protected]> * Enhance timestamp test with comprehensive scenarios Co-authored-by: stephentoub <[email protected]> * Replace DateTimeOffset.UnixEpoch with manual construction for compatibility Co-authored-by: stephentoub <[email protected]> * Apply suggestions from code review * Update test/Libraries/Microsoft.Extensions.AI.Abstractions.Tests/ChatCompletion/ChatResponseUpdateExtensionsTests.cs * Add theory test for timestamp folding with pairs of timestamps Co-authored-by: stephentoub <[email protected]> * Refactor ToChatResponse_TimestampFolding to use MemberData instead of duplicated InlineData Co-authored-by: stephentoub <[email protected]> --------- Co-authored-by: copilot-swe-agent[bot] <[email protected]> Co-authored-by: stephentoub <[email protected]> Co-authored-by: Stephen Toub <[email protected]>
1 parent 839ef9e commit 254951e

File tree

2 files changed

+94
-2
lines changed

2 files changed

+94
-2
lines changed

src/Libraries/Microsoft.Extensions.AI.Abstractions/ChatCompletion/ChatResponseExtensions.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -346,7 +346,7 @@ private static void ProcessUpdate(ChatResponseUpdate update, ChatResponse respon
346346
message.AuthorName = update.AuthorName;
347347
}
348348

349-
if (update.CreatedAt is not null)
349+
if (message.CreatedAt is null || (update.CreatedAt is not null && update.CreatedAt > message.CreatedAt))
350350
{
351351
message.CreatedAt = update.CreatedAt;
352352
}
@@ -391,7 +391,7 @@ private static void ProcessUpdate(ChatResponseUpdate update, ChatResponse respon
391391
response.ConversationId = update.ConversationId;
392392
}
393393

394-
if (update.CreatedAt is not null)
394+
if (response.CreatedAt is null || (update.CreatedAt is not null && update.CreatedAt > response.CreatedAt))
395395
{
396396
response.CreatedAt = update.CreatedAt;
397397
}

test/Libraries/Microsoft.Extensions.AI.Abstractions.Tests/ChatCompletion/ChatResponseUpdateExtensionsTests.cs

Lines changed: 92 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -359,6 +359,98 @@ public async Task ToChatResponse_UsageContentExtractedFromContents()
359359
Assert.Equal("Hello, world!", Assert.IsType<TextContent>(Assert.Single(Assert.Single(response.Messages).Contents)).Text);
360360
}
361361

362+
[Theory]
363+
[InlineData(false)]
364+
[InlineData(true)]
365+
public async Task ToChatResponse_AlternativeTimestamps(bool useAsync)
366+
{
367+
DateTimeOffset early = new(2024, 1, 1, 10, 0, 0, TimeSpan.Zero);
368+
DateTimeOffset middle = new(2024, 1, 1, 11, 0, 0, TimeSpan.Zero);
369+
DateTimeOffset late = new(2024, 1, 1, 12, 0, 0, TimeSpan.Zero);
370+
DateTimeOffset unixEpoch = new(1970, 1, 1, 0, 0, 0, TimeSpan.Zero);
371+
372+
ChatResponseUpdate[] updates =
373+
[
374+
375+
// Start with an early timestamp
376+
new(ChatRole.Tool, "a") { MessageId = "4", CreatedAt = early },
377+
378+
// Unix epoch (as "null") should not overwrite
379+
new(null, "b") { CreatedAt = unixEpoch },
380+
381+
// Newer timestamp should overwrite
382+
new(null, "c") { CreatedAt = middle },
383+
384+
// Older timestamp should not overwrite
385+
new(null, "d") { CreatedAt = early },
386+
387+
// Even newer timestamp should overwrite
388+
new(null, "e") { CreatedAt = late },
389+
390+
// Unix epoch should not overwrite again
391+
new(null, "f") { CreatedAt = unixEpoch },
392+
393+
// null should not overwrite
394+
new(null, "g") { CreatedAt = null },
395+
];
396+
397+
ChatResponse response = useAsync ?
398+
updates.ToChatResponse() :
399+
await YieldAsync(updates).ToChatResponseAsync();
400+
Assert.Single(response.Messages);
401+
402+
Assert.Equal("abcdefg", response.Messages[0].Text);
403+
Assert.Equal(ChatRole.Tool, response.Messages[0].Role);
404+
Assert.Equal(late, response.Messages[0].CreatedAt);
405+
Assert.Equal(late, response.CreatedAt);
406+
}
407+
408+
public static IEnumerable<object?[]> ToChatResponse_TimestampFolding_MemberData()
409+
{
410+
// Base test cases
411+
var testCases = new (string? timestamp1, string? timestamp2, string? expectedTimestamp)[]
412+
{
413+
(null, null, null),
414+
("2024-01-01T10:00:00Z", null, "2024-01-01T10:00:00Z"),
415+
(null, "2024-01-01T10:00:00Z", "2024-01-01T10:00:00Z"),
416+
("2024-01-01T10:00:00Z", "2024-01-01T11:00:00Z", "2024-01-01T11:00:00Z"),
417+
("2024-01-01T11:00:00Z", "2024-01-01T10:00:00Z", "2024-01-01T11:00:00Z"),
418+
("2024-01-01T10:00:00Z", "1970-01-01T00:00:00Z", "2024-01-01T10:00:00Z"),
419+
("1970-01-01T00:00:00Z", "2024-01-01T10:00:00Z", "2024-01-01T10:00:00Z"),
420+
};
421+
422+
// Yield each test case twice, once for useAsync = false and once for useAsync = true
423+
foreach (var (timestamp1, timestamp2, expectedTimestamp) in testCases)
424+
{
425+
yield return new object?[] { false, timestamp1, timestamp2, expectedTimestamp };
426+
yield return new object?[] { true, timestamp1, timestamp2, expectedTimestamp };
427+
}
428+
}
429+
430+
[Theory]
431+
[MemberData(nameof(ToChatResponse_TimestampFolding_MemberData))]
432+
public async Task ToChatResponse_TimestampFolding(bool useAsync, string? timestamp1, string? timestamp2, string? expectedTimestamp)
433+
{
434+
DateTimeOffset? first = timestamp1 is not null ? DateTimeOffset.Parse(timestamp1) : null;
435+
DateTimeOffset? second = timestamp2 is not null ? DateTimeOffset.Parse(timestamp2) : null;
436+
DateTimeOffset? expected = expectedTimestamp is not null ? DateTimeOffset.Parse(expectedTimestamp) : null;
437+
438+
ChatResponseUpdate[] updates =
439+
[
440+
new(ChatRole.Assistant, "a") { CreatedAt = first },
441+
new(null, "b") { CreatedAt = second },
442+
];
443+
444+
ChatResponse response = useAsync ?
445+
updates.ToChatResponse() :
446+
await YieldAsync(updates).ToChatResponseAsync();
447+
448+
Assert.Single(response.Messages);
449+
Assert.Equal("ab", response.Messages[0].Text);
450+
Assert.Equal(expected, response.Messages[0].CreatedAt);
451+
Assert.Equal(expected, response.CreatedAt);
452+
}
453+
362454
private static async IAsyncEnumerable<ChatResponseUpdate> YieldAsync(IEnumerable<ChatResponseUpdate> updates)
363455
{
364456
foreach (ChatResponseUpdate update in updates)

0 commit comments

Comments
 (0)