mirror of
https://github.com/microsoft/agent-framework.git
synced 2026-06-16 21:04:09 +08:00
.NET: Fix bug in store-false helper to ensure addition rather than replacement (#5895)
* Fix bug in store-false helper to ensure addition rather than replacement * Address PR comments
This commit is contained in:
committed by
GitHub
Unverified
parent
0d09d40f0f
commit
c885ca3d7a
@@ -120,9 +120,24 @@ public static class OpenAIResponseClientExtensions
|
||||
return Throw.IfNull(responseClient)
|
||||
.AsIChatClient(model)
|
||||
.AsBuilder()
|
||||
.ConfigureOptions(x => x.RawRepresentationFactory = _ => includeReasoningEncryptedContent
|
||||
? new CreateResponseOptions() { StoredOutputEnabled = false, IncludedProperties = { IncludedResponseProperty.ReasoningEncryptedContent } }
|
||||
: new CreateResponseOptions() { StoredOutputEnabled = false })
|
||||
.ConfigureOptions(x =>
|
||||
{
|
||||
var previousFactory = x.RawRepresentationFactory;
|
||||
x.RawRepresentationFactory = state =>
|
||||
{
|
||||
var responseOptions = previousFactory?.Invoke(state) as CreateResponseOptions ?? new CreateResponseOptions();
|
||||
|
||||
responseOptions.StoredOutputEnabled = false;
|
||||
|
||||
if (includeReasoningEncryptedContent &&
|
||||
!responseOptions.IncludedProperties.Contains(IncludedResponseProperty.ReasoningEncryptedContent))
|
||||
{
|
||||
responseOptions.IncludedProperties.Add(IncludedResponseProperty.ReasoningEncryptedContent);
|
||||
}
|
||||
|
||||
return responseOptions;
|
||||
};
|
||||
})
|
||||
.Build();
|
||||
}
|
||||
}
|
||||
|
||||
+78
-1
@@ -129,6 +129,75 @@ public sealed class ProjectResponsesClientExtensionsTests
|
||||
Assert.DoesNotContain(IncludedResponseProperty.ReasoningEncryptedContent, createResponseOptions.IncludedProperties);
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Verify that AsIChatClientWithStoredOutputDisabled preserves an existing RawRepresentationFactory
|
||||
/// set on ChatOptions, augmenting it with StoredOutputEnabled and ReasoningEncryptedContent
|
||||
/// rather than replacing it.
|
||||
/// </summary>
|
||||
[Fact]
|
||||
public void AsIChatClientWithStoredOutputDisabled_PreservesExistingRawRepresentationFactory()
|
||||
{
|
||||
// Arrange
|
||||
var responseClient = CreateTestClient();
|
||||
var chatClient = responseClient.AsIChatClientWithStoredOutputDisabled();
|
||||
|
||||
// Simulate a caller setting their own RawRepresentationFactory on ChatOptions
|
||||
// (e.g., to add WebSearchCallActionSources).
|
||||
var options = new ChatOptions
|
||||
{
|
||||
RawRepresentationFactory = _ => new CreateResponseOptions
|
||||
{
|
||||
IncludedProperties = { IncludedResponseProperty.WebSearchCallActionSources },
|
||||
},
|
||||
};
|
||||
|
||||
// Act
|
||||
var createResponseOptions = GetCreateResponseOptionsFromPipeline(chatClient, options);
|
||||
|
||||
// Assert
|
||||
Assert.NotNull(createResponseOptions);
|
||||
Assert.False(createResponseOptions.StoredOutputEnabled);
|
||||
Assert.Contains(IncludedResponseProperty.ReasoningEncryptedContent, createResponseOptions.IncludedProperties);
|
||||
Assert.Contains(IncludedResponseProperty.WebSearchCallActionSources, createResponseOptions.IncludedProperties);
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Verify that AsIChatClientWithStoredOutputDisabled does not duplicate ReasoningEncryptedContent
|
||||
/// when the existing factory already includes it.
|
||||
/// </summary>
|
||||
[Fact]
|
||||
public void AsIChatClientWithStoredOutputDisabled_DoesNotDuplicateReasoningEncryptedContent()
|
||||
{
|
||||
// Arrange
|
||||
var responseClient = CreateTestClient();
|
||||
var chatClient = responseClient.AsIChatClientWithStoredOutputDisabled();
|
||||
|
||||
// Simulate a caller that already includes ReasoningEncryptedContent
|
||||
var options = new ChatOptions
|
||||
{
|
||||
RawRepresentationFactory = _ => new CreateResponseOptions
|
||||
{
|
||||
IncludedProperties = { IncludedResponseProperty.ReasoningEncryptedContent },
|
||||
},
|
||||
};
|
||||
|
||||
// Act
|
||||
var createResponseOptions = GetCreateResponseOptionsFromPipeline(chatClient, options);
|
||||
|
||||
// Assert - ReasoningEncryptedContent should appear exactly once
|
||||
Assert.NotNull(createResponseOptions);
|
||||
int count = 0;
|
||||
foreach (var prop in createResponseOptions.IncludedProperties)
|
||||
{
|
||||
if (prop == IncludedResponseProperty.ReasoningEncryptedContent)
|
||||
{
|
||||
count++;
|
||||
}
|
||||
}
|
||||
|
||||
Assert.Equal(1, count);
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Verify that AsIChatClientWithStoredOutputDisabled works with an optional deployment name.
|
||||
/// </summary>
|
||||
@@ -153,6 +222,15 @@ public sealed class ProjectResponsesClientExtensionsTests
|
||||
/// by using reflection to access the configure action and invoking it on a test <see cref="ChatOptions"/>.
|
||||
/// </summary>
|
||||
private static CreateResponseOptions? GetCreateResponseOptionsFromPipeline(IChatClient chatClient)
|
||||
{
|
||||
return GetCreateResponseOptionsFromPipeline(chatClient, new ChatOptions());
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Overload that runs the configure action on caller-supplied <see cref="ChatOptions"/>,
|
||||
/// useful for testing that existing factories are preserved.
|
||||
/// </summary>
|
||||
private static CreateResponseOptions? GetCreateResponseOptionsFromPipeline(IChatClient chatClient, ChatOptions options)
|
||||
{
|
||||
var configureField = chatClient.GetType().GetField("_configureOptions", BindingFlags.NonPublic | BindingFlags.Instance);
|
||||
Assert.NotNull(configureField);
|
||||
@@ -160,7 +238,6 @@ public sealed class ProjectResponsesClientExtensionsTests
|
||||
var configureAction = configureField.GetValue(chatClient) as Action<ChatOptions>;
|
||||
Assert.NotNull(configureAction);
|
||||
|
||||
var options = new ChatOptions();
|
||||
configureAction(options);
|
||||
|
||||
Assert.NotNull(options.RawRepresentationFactory);
|
||||
|
||||
+78
-1
@@ -370,6 +370,75 @@ public sealed class OpenAIResponseClientExtensionsTests
|
||||
Assert.DoesNotContain(IncludedResponseProperty.ReasoningEncryptedContent, createResponseOptions.IncludedProperties);
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Verify that AsIChatClientWithStoredOutputDisabled preserves an existing RawRepresentationFactory
|
||||
/// set on ChatOptions, augmenting it with StoredOutputEnabled and ReasoningEncryptedContent
|
||||
/// rather than replacing it.
|
||||
/// </summary>
|
||||
[Fact]
|
||||
public void AsIChatClientWithStoredOutputDisabled_PreservesExistingRawRepresentationFactory()
|
||||
{
|
||||
// Arrange
|
||||
var responseClient = new TestOpenAIResponseClient();
|
||||
var chatClient = responseClient.AsIChatClientWithStoredOutputDisabled();
|
||||
|
||||
// Simulate a caller setting their own RawRepresentationFactory on ChatOptions
|
||||
// (e.g., to add WebSearchCallActionSources).
|
||||
var options = new ChatOptions
|
||||
{
|
||||
RawRepresentationFactory = _ => new CreateResponseOptions
|
||||
{
|
||||
IncludedProperties = { IncludedResponseProperty.WebSearchCallActionSources },
|
||||
},
|
||||
};
|
||||
|
||||
// Act
|
||||
var createResponseOptions = GetCreateResponseOptionsFromPipeline(chatClient, options);
|
||||
|
||||
// Assert
|
||||
Assert.NotNull(createResponseOptions);
|
||||
Assert.False(createResponseOptions.StoredOutputEnabled);
|
||||
Assert.Contains(IncludedResponseProperty.ReasoningEncryptedContent, createResponseOptions.IncludedProperties);
|
||||
Assert.Contains(IncludedResponseProperty.WebSearchCallActionSources, createResponseOptions.IncludedProperties);
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Verify that AsIChatClientWithStoredOutputDisabled does not duplicate ReasoningEncryptedContent
|
||||
/// when the existing factory already includes it.
|
||||
/// </summary>
|
||||
[Fact]
|
||||
public void AsIChatClientWithStoredOutputDisabled_DoesNotDuplicateReasoningEncryptedContent()
|
||||
{
|
||||
// Arrange
|
||||
var responseClient = new TestOpenAIResponseClient();
|
||||
var chatClient = responseClient.AsIChatClientWithStoredOutputDisabled();
|
||||
|
||||
// Simulate a caller that already includes ReasoningEncryptedContent
|
||||
var options = new ChatOptions
|
||||
{
|
||||
RawRepresentationFactory = _ => new CreateResponseOptions
|
||||
{
|
||||
IncludedProperties = { IncludedResponseProperty.ReasoningEncryptedContent },
|
||||
},
|
||||
};
|
||||
|
||||
// Act
|
||||
var createResponseOptions = GetCreateResponseOptionsFromPipeline(chatClient, options);
|
||||
|
||||
// Assert - ReasoningEncryptedContent should appear exactly once
|
||||
Assert.NotNull(createResponseOptions);
|
||||
int count = 0;
|
||||
foreach (var prop in createResponseOptions.IncludedProperties)
|
||||
{
|
||||
if (prop == IncludedResponseProperty.ReasoningEncryptedContent)
|
||||
{
|
||||
count++;
|
||||
}
|
||||
}
|
||||
|
||||
Assert.Equal(1, count);
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// A simple test IServiceProvider implementation for testing.
|
||||
/// </summary>
|
||||
@@ -394,6 +463,15 @@ public sealed class OpenAIResponseClientExtensionsTests
|
||||
/// by using reflection to access the configure action and invoking it on a test <see cref="ChatOptions"/>.
|
||||
/// </summary>
|
||||
private static CreateResponseOptions? GetCreateResponseOptionsFromPipeline(IChatClient chatClient)
|
||||
{
|
||||
return GetCreateResponseOptionsFromPipeline(chatClient, new ChatOptions());
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Overload that runs the configure action on caller-supplied <see cref="ChatOptions"/>,
|
||||
/// useful for testing that existing factories are preserved.
|
||||
/// </summary>
|
||||
private static CreateResponseOptions? GetCreateResponseOptionsFromPipeline(IChatClient chatClient, ChatOptions options)
|
||||
{
|
||||
// The ConfigureOptionsChatClient stores the configure action in a private field.
|
||||
var configureField = chatClient.GetType().GetField("_configureOptions", BindingFlags.NonPublic | BindingFlags.Instance);
|
||||
@@ -402,7 +480,6 @@ public sealed class OpenAIResponseClientExtensionsTests
|
||||
var configureAction = configureField.GetValue(chatClient) as Action<ChatOptions>;
|
||||
Assert.NotNull(configureAction);
|
||||
|
||||
var options = new ChatOptions();
|
||||
configureAction(options);
|
||||
|
||||
Assert.NotNull(options.RawRepresentationFactory);
|
||||
|
||||
Reference in New Issue
Block a user