Skip to content

Commit be18e99

Browse files
CopilotjozkeeCopilot
authored
Fix ImageGeneratingChatClient duplicating preceding content and dropping following content (#7622)
* Initial plan * Fix ReplaceImageGenerationFunctionResults to use forward iteration; add regression tests * Fix comment Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: David Cantú <dacantu@microsoft.com> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
1 parent e8801a4 commit be18e99

2 files changed

Lines changed: 152 additions & 3 deletions

File tree

src/Libraries/Microsoft.Extensions.AI/ChatCompletion/ImageGeneratingChatClient.cs

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -319,16 +319,16 @@ public IList<AIContent> ReplaceImageGenerationFunctionResults(IList<AIContent> c
319319
{
320320
List<AIContent>? newContents = null;
321321

322-
// Replace FunctionResultContent instances with generated image content
323-
for (int i = contents.Count - 1; i >= 0; i--)
322+
// Replace image-generation FunctionCallContent with ImageGenerationToolCallContent, and FunctionResultContent with generated image content
323+
for (int i = 0; i < contents.Count; i++)
324324
{
325325
var content = contents[i];
326326

327327
// We must lookup by name because in the streaming case we have not yet been called to record the CallId.
328328
if (content is FunctionCallContent functionCall &&
329329
_toolNames.Contains(functionCall.Name))
330330
{
331-
// create a new list and omit the FunctionCallContent
331+
// create a new list copying all items before this one, and omit the FunctionCallContent
332332
newContents ??= CopyList(contents, i);
333333

334334
if (functionCall.Name != nameof(GetImagesForEdit))
@@ -339,6 +339,7 @@ public IList<AIContent> ReplaceImageGenerationFunctionResults(IList<AIContent> c
339339
else if (content is FunctionResultContent functionResult &&
340340
_imageContentByCallId.TryGetValue(functionResult.CallId, out var imageContents))
341341
{
342+
// create a new list copying all items before this one
342343
newContents ??= CopyList(contents, i);
343344

344345
if (imageContents.Any())

test/Libraries/Microsoft.Extensions.AI.Tests/ChatCompletion/ImageGeneratingChatClientTests.cs

Lines changed: 148 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -383,4 +383,152 @@ async IAsyncEnumerable<ChatResponseUpdate> GetUpdatesAsync()
383383
var imageToolCallContent = Assert.IsType<ImageGenerationToolCallContent>(update.Contents[0]);
384384
Assert.Equal(callId, imageToolCallContent.CallId);
385385
}
386+
387+
[Theory]
388+
[InlineData("at-start")]
389+
[InlineData("in-middle")]
390+
[InlineData("at-end")]
391+
public async Task GetResponseAsync_FunctionCallContent_SurroundingContentPreservedInOrder(string position)
392+
{
393+
// Regression test for: image generation content duplicating preceding content and dropping following content.
394+
// The image-generation FunctionCallContent should be replaced with ImageGenerationToolCallContent,
395+
// while all other content items retain their original order and cardinality.
396+
var imageCallId = "image-call-id";
397+
var otherCallId = "other-call-id";
398+
399+
using var innerClient = new TestChatClient
400+
{
401+
GetResponseAsyncCallback = (messages, options, cancellationToken) =>
402+
{
403+
IList<AIContent> contents = position switch
404+
{
405+
"at-start" => [new FunctionCallContent(imageCallId, "GenerateImage", new Dictionary<string, object?> { ["prompt"] = "a cat" }),
406+
new FunctionResultContent(otherCallId, "other-result")],
407+
"at-end" => [new FunctionResultContent(otherCallId, "other-result"),
408+
new FunctionCallContent(imageCallId, "GenerateImage", new Dictionary<string, object?> { ["prompt"] = "a cat" })],
409+
_ => [new FunctionResultContent(otherCallId, "before-result"),
410+
new FunctionCallContent(imageCallId, "GenerateImage", new Dictionary<string, object?> { ["prompt"] = "a cat" }),
411+
new UsageContent(new UsageDetails { InputTokenCount = 5 })],
412+
};
413+
return Task.FromResult(new ChatResponse(new ChatMessage(ChatRole.Assistant, contents)));
414+
},
415+
};
416+
417+
using var imageGenerator = new TestImageGenerator();
418+
using var client = new ImageGeneratingChatClient(innerClient, imageGenerator);
419+
420+
var chatOptions = new ChatOptions
421+
{
422+
Tools = [new HostedImageGenerationTool()]
423+
};
424+
425+
// Act
426+
var response = await client.GetResponseAsync([new(ChatRole.User, "test")], chatOptions);
427+
428+
// Assert
429+
Assert.NotNull(response);
430+
Assert.Single(response.Messages);
431+
var contents = response.Messages[0].Contents;
432+
433+
if (position == "at-start")
434+
{
435+
Assert.Equal(2, contents.Count);
436+
Assert.IsType<ImageGenerationToolCallContent>(contents[0]);
437+
var otherResult = Assert.IsType<FunctionResultContent>(contents[1]);
438+
Assert.Equal(otherCallId, otherResult.CallId);
439+
}
440+
else if (position == "at-end")
441+
{
442+
Assert.Equal(2, contents.Count);
443+
var otherResult = Assert.IsType<FunctionResultContent>(contents[0]);
444+
Assert.Equal(otherCallId, otherResult.CallId);
445+
Assert.IsType<ImageGenerationToolCallContent>(contents[1]);
446+
}
447+
else
448+
{
449+
Assert.Equal(3, contents.Count);
450+
var beforeResult = Assert.IsType<FunctionResultContent>(contents[0]);
451+
Assert.Equal(otherCallId, beforeResult.CallId);
452+
Assert.IsType<ImageGenerationToolCallContent>(contents[1]);
453+
var usageContent = Assert.IsType<UsageContent>(contents[2]);
454+
Assert.Equal(5, usageContent.Details.InputTokenCount);
455+
}
456+
}
457+
458+
[Theory]
459+
[InlineData("at-start")]
460+
[InlineData("in-middle")]
461+
[InlineData("at-end")]
462+
public async Task GetStreamingResponseAsync_FunctionCallContent_SurroundingContentPreservedInOrder(string position)
463+
{
464+
// Regression test for: image generation content duplicating preceding content and dropping following content.
465+
// The image-generation FunctionCallContent should be replaced with ImageGenerationToolCallContent,
466+
// while all other content items retain their original order and cardinality.
467+
var imageCallId = "image-call-id";
468+
var otherCallId = "other-call-id";
469+
470+
using var innerClient = new TestChatClient
471+
{
472+
GetStreamingResponseAsyncCallback = (messages, options, cancellationToken) => GetUpdatesAsync()
473+
};
474+
475+
async IAsyncEnumerable<ChatResponseUpdate> GetUpdatesAsync()
476+
{
477+
await Task.Yield();
478+
IList<AIContent> contents = position switch
479+
{
480+
"at-start" => [new FunctionCallContent(imageCallId, "GenerateImage", new Dictionary<string, object?> { ["prompt"] = "a cat" }),
481+
new FunctionResultContent(otherCallId, "other-result")],
482+
"at-end" => [new FunctionResultContent(otherCallId, "other-result"),
483+
new FunctionCallContent(imageCallId, "GenerateImage", new Dictionary<string, object?> { ["prompt"] = "a cat" })],
484+
_ => [new FunctionResultContent(otherCallId, "before-result"),
485+
new FunctionCallContent(imageCallId, "GenerateImage", new Dictionary<string, object?> { ["prompt"] = "a cat" }),
486+
new UsageContent(new UsageDetails { InputTokenCount = 5 })],
487+
};
488+
yield return new ChatResponseUpdate(ChatRole.Assistant, contents);
489+
}
490+
491+
using var imageGenerator = new TestImageGenerator();
492+
using var client = new ImageGeneratingChatClient(innerClient, imageGenerator);
493+
494+
var chatOptions = new ChatOptions
495+
{
496+
Tools = [new HostedImageGenerationTool()]
497+
};
498+
499+
// Act
500+
var updates = new List<ChatResponseUpdate>();
501+
await foreach (var update in client.GetStreamingResponseAsync([new(ChatRole.User, "test")], chatOptions))
502+
{
503+
updates.Add(update);
504+
}
505+
506+
// Assert
507+
Assert.Single(updates);
508+
var contents = updates[0].Contents;
509+
510+
if (position == "at-start")
511+
{
512+
Assert.Equal(2, contents.Count);
513+
Assert.IsType<ImageGenerationToolCallContent>(contents[0]);
514+
var otherResult = Assert.IsType<FunctionResultContent>(contents[1]);
515+
Assert.Equal(otherCallId, otherResult.CallId);
516+
}
517+
else if (position == "at-end")
518+
{
519+
Assert.Equal(2, contents.Count);
520+
var otherResult = Assert.IsType<FunctionResultContent>(contents[0]);
521+
Assert.Equal(otherCallId, otherResult.CallId);
522+
Assert.IsType<ImageGenerationToolCallContent>(contents[1]);
523+
}
524+
else
525+
{
526+
Assert.Equal(3, contents.Count);
527+
var beforeResult = Assert.IsType<FunctionResultContent>(contents[0]);
528+
Assert.Equal(otherCallId, beforeResult.CallId);
529+
Assert.IsType<ImageGenerationToolCallContent>(contents[1]);
530+
var usageContent = Assert.IsType<UsageContent>(contents[2]);
531+
Assert.Equal(5, usageContent.Details.InputTokenCount);
532+
}
533+
}
386534
}

0 commit comments

Comments
 (0)