Skip to content

Conversation

@AlexDaines
Copy link
Contributor

@AlexDaines AlexDaines commented Nov 7, 2025

Add ChatOptions.ResponseFormat support for Bedrock MEAI

Description

Implements support for ChatOptions.ResponseFormat in the AWSSDK.Extensions.Bedrock.MEAI implementation of IChatClient. When ResponseFormat is set to Json or ForJsonSchema, the client now uses Bedrock's tool mechanism to enforce structured JSON responses from models.

Implementation approach:

  • Creates a synthetic tool called "generate_response" with the provided JSON schema
  • Forces model to use this tool via toolChoice
  • Extracts JSON response from the tool use output
  • Converts Bedrock Document objects to standard JSON

Why synthetic tool?:

Bedrock lacks a native responseFormat API; all AWS SDKs (boto3, Java, now .NET) use tool calling as the official mechanism for structured output—we inject a synthetic tool with the JSON schema to implement ChatOptions.ResponseFormat transparently.

Key behavior:

  • ResponseFormat.Json: Requests JSON with generic object schema
  • ResponseFormat.ForJsonSchema: Requests JSON conforming to custom schema
  • ResponseFormat.Text: No changes to request (default behavior)
  • Throws ArgumentException if ResponseFormat is used with user-provided tools (mutual exclusivity)
  • Throws NotSupportedException for streaming requests (Bedrock limitation)

Motivation and Context

Closes #3911

Users need consistent behavior when using IChatClient across different AI providers. Currently, the Bedrock implementation ignores ChatOptions.ResponseFormat, making it impossible to request structured responses through the standardized Microsoft.Extensions.AI interface. This prevents Bedrock from being a drop-in replacement for other providers in structured data workflows.

Testing

Dryrun:

.NET v4 Build: DRY_RUN-9edf05db-56d5-4398-902c-826d8573804d

  • Added 2 core unit tests covering request creation with schemas and response extraction
  • Created local sample application demonstrating the feature with real Bedrock API calls
  • Verified JSON responses are correctly structured and parsed
  • Confirmed error handling for invalid configurations (tools + ResponseFormat, streaming)

Test coverage:

  • ResponseFormat_Json_WithSchema_CreatesSyntheticToolWithCorrectSchema: Validates synthetic tool creation with custom schema
  • ResponseFormat_Json_ModelReturnsToolUse_ExtractsJsonCorrectly: Validates JSON extraction from tool use responses

Types of changes

  • New feature (non-breaking change which adds functionality)

Checklist

  • My code follows the code style of this project
  • My change requires a change to the documentation
  • I have updated the documentation accordingly
  • I have read the README document
  • I have added tests to cover my changes
  • All new and existing tests passed

License

  • I confirm that this pull request can be released under the Apache 2 license

@GarrettBeatty GarrettBeatty requested a review from Copilot November 7, 2025 18:01
Copy link
Contributor

Copilot AI left a 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 adds ResponseFormat support to the AWS Bedrock ChatClient for Microsoft.Extensions.AI, enabling structured JSON output from Bedrock models. The implementation uses Bedrock's tool mechanism with a synthetic tool to enforce structured responses, requiring models with ToolChoice support (Claude 3+ and Mistral Large).

Key changes:

  • Implemented ResponseFormat handling via synthetic tool creation that forces models to return structured JSON
  • Added error handling for unsupported models and missing structured responses
  • Added Document-to-JSON conversion utilities for extracting structured content from tool use responses

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File Description
extensions/src/AWSSDK.Extensions.Bedrock.MEAI/BedrockChatClient.cs Core implementation of ResponseFormat support including synthetic tool creation, error handling for unsupported models, Document-to-JSON conversion, and validation that ResponseFormat conflicts with user-provided tools
extensions/test/BedrockMEAITests/BedrockChatClientTests.cs Added MockBedrockRuntime test infrastructure and two tests validating schema conversion and JSON extraction from tool use responses

Copy link
Contributor

@GarrettBeatty GarrettBeatty left a comment

Choose a reason for hiding this comment

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

can you explain more a little more in the PR why we need make this synthetic tool and what not

}

// Check if ResponseFormat is set - not supported for streaming yet
if (options?.ResponseFormat is ChatResponseFormatJson)
Copy link
Contributor

Choose a reason for hiding this comment

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

just wondering why this is

Copy link
Contributor

Choose a reason for hiding this comment

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

as discussed its because by supplying only one tool it forces bedrock to use our json one. please add comment explaining this and also link to other sdk with example

}

/// <summary>Converts a <see cref="Document"/> to a JSON string.</summary>
private static string DocumentToJsonString(Document document)
Copy link
Contributor

Choose a reason for hiding this comment

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

all of this stuff i would be surprised if it doesnt exist already in a jsonutils or utils file. either way it shouldnt be in this class

Copy link
Contributor

Choose a reason for hiding this comment

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

Agree with garret, it should live in a utils class at the least

Copy link
Contributor

Choose a reason for hiding this comment

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

I haven't looked at the entire PR, but we've had requests in the past to make the Document class interop better with JSON.

It's something we should do, but we have to be aware the document type is meant to be agnostic (the service could start returning CBOR tomorrow for example). See this comment from Norm: #3915 (comment)

It'd probably make more sense to include this functionality in Core, but now I'm even wondering if it's better to do that first (and separately) from this PR.

Copy link
Contributor

@peterrsongg peterrsongg left a comment

Choose a reason for hiding this comment

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

My long comment on testing different json responses returned by the service might not make sense depending on what this feature is supposed to do. So if this option is set, that tells Bedrock to return the response in a certain way?

I still think we shouldn't have a MockBedrockRuntime that implements IAmazonBedrockRuntime. We will have to update this class every time a new operation is released.

}

/// <summary>Converts a <see cref="Document"/> to a JSON string.</summary>
private static string DocumentToJsonString(Document document)
Copy link
Contributor

Choose a reason for hiding this comment

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

Agree with garret, it should live in a utils class at the least

// Check if this is a ToolChoice validation error (model doesn't support it)
bool isToolChoiceNotSupported =
ex.ErrorCode == "ValidationException" &&
(ex.Message.IndexOf("toolChoice", StringComparison.OrdinalIgnoreCase) >= 0 ||
Copy link
Contributor

Choose a reason for hiding this comment

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

is checking the error message the only way to achieve this? error messages aren't gauranteed to stay the same.

}

// Assert
var tool = mock.CapturedRequest.ToolConfig.Tools[0];
Copy link
Contributor

Choose a reason for hiding this comment

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

i'm a bit confused, and maybe it is because i don't understand who is supposed to return the response in the provided schema (bedrock or us), but this test case just seems to be asserting that the tool has the correct schema set on it. Is there no way to test the actual functionality?

AlexDaines added a commit that referenced this pull request Nov 19, 2025
@AlexDaines AlexDaines force-pushed the adaines/support-chatoptions-responseformat branch from a459b62 to b7bd419 Compare November 19, 2025 23:02
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.

Comments suppressed due to low confidence (1)

extensions/test/BedrockMEAITests/BedrockMEAITests.NetFramework.csproj:37

  • Missing corresponding NetStandard test project. The repository follows a pattern where extension tests have both NetFramework and NetStandard project files to ensure platform compatibility (see CloudFront.SignersTests and EC2.DecryptPasswordTests as examples). A NetStandard test project targeting netcoreapp3.1;net8.0 is needed to ensure tests run on .NET Core 3.1 and .NET 8.0, as required by the contributing guidelines.
<Project Sdk="Microsoft.NET.Sdk">
  <PropertyGroup>
    <TargetFrameworks>net472</TargetFrameworks>
    <DefineConstants>$(DefineConstants);BCL</DefineConstants>
    <AssemblyName>BedrockMEAITests</AssemblyName>
    <PackageId>BedrockMEAITests</PackageId>

    <GenerateAssemblyTitleAttribute>false</GenerateAssemblyTitleAttribute>
    <GenerateAssemblyConfigurationAttribute>false</GenerateAssemblyConfigurationAttribute>
    <GenerateAssemblyCompanyAttribute>false</GenerateAssemblyCompanyAttribute>
    <GenerateAssemblyProductAttribute>false</GenerateAssemblyProductAttribute>
    <GenerateAssemblyDescriptionAttribute>false</GenerateAssemblyDescriptionAttribute>
    <GenerateAssemblyCopyrightAttribute>false</GenerateAssemblyCopyrightAttribute>
    <GenerateAssemblyVersionAttribute>false</GenerateAssemblyVersionAttribute>
    <GenerateAssemblyFileVersionAttribute>false</GenerateAssemblyFileVersionAttribute>

    <TreatWarningsAsErrors>true</TreatWarningsAsErrors>
    <LangVersion>Latest</LangVersion>
  </PropertyGroup>

  <ItemGroup>
    <PackageReference Include="Microsoft.Extensions.AI.Abstractions" Version="9.9.1" />
    <PackageReference Include="Moq" Version="4.8.3" />
    <PackageReference Include="xunit" Version="2.9.2" />
    <PackageReference Include="xunit.runner.visualstudio" Version="2.8.2" />
  </ItemGroup>

  <ItemGroup>
    <ProjectReference Include="../../../sdk/src/Core/AWSSDK.Core.NetFramework.csproj" />
    <ProjectReference Include="../../src/AWSSDK.Extensions.Bedrock.MEAI/AWSSDK.Extensions.Bedrock.MEAI.NetFramework.csproj" />
    <ProjectReference Include="../../../sdk/test/UnitTests/Custom/AWSSDK.UnitTestUtilities.NetFramework.csproj" />
  </ItemGroup>

  <ItemGroup>
    <PackageReference Include="Microsoft.NET.Test.Sdk" Version="17.11.1" />
  </ItemGroup>
</Project>

AlexDaines and others added 3 commits November 20, 2025 12:44
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Copy link
Contributor

@peterrsongg peterrsongg left a comment

Choose a reason for hiding this comment

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

just a few comments, but this looks much better with all the additional test cases i feel much more confident about this change. thanks for applying my earlier feedback

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

Role = ChatRole.Assistant,
MessageId = Guid.NewGuid().ToString("N"),
};

Copy link
Contributor

Choose a reason for hiding this comment

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

public async Task<ChatResponse> GetResponseAsync(
    IEnumerable<ChatMessage> messages, ChatOptions? options = null, CancellationToken cancellationToken = default)
{
    if (messages is null)
    {
        throw new ArgumentNullException(nameof(messages));
    }

    // Validate ResponseFormat usage early
    ValidateResponseFormatUsage(options, isStreaming: false);

    ConverseRequest request = options?.RawRepresentationFactory?.Invoke(this) as ConverseRequest ?? new();
    // ... rest of request setup ...

    ConverseResponse response = await _runtime.ConverseAsync(request, cancellationToken).ConfigureAwait(false);

    // Extract structured response if using ResponseFormat
    if (options?.ResponseFormat is ChatResponseFormatJson)
    {
        return TryExtractStructuredResponse(response, request.ModelId, options);
    }

    // Normal content processing...
    ChatMessage result = CreateChatMessageFromResponse(response, options);
    return CreateChatResponse(result, response);
}

i dont want to add more work to this PR but im thinking eventually we need to refactor this file. i was thinking something like this. @peterrsongg. this function is getting really big and we are adding onto it now

Copy link
Contributor

@GarrettBeatty GarrettBeatty left a comment

Choose a reason for hiding this comment

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

approving since i read through https://github.com/AlexDaines/pr-4113-responseformat-demo/blob/main/Program.cs and the test program seems correct.

Can we create backlogs items for the following though.

  1. Setup integration tests for this project.
  2. Clean up this chat client class/restructure it. e.g. the get GetResponseAsync function is still super long) and i think the class should have some restructuring

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants