Skip to content

Add ability to proxy the reading and writing of models #50024

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

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open

Conversation

m-nash
Copy link
Member

@m-nash m-nash commented May 13, 2025

Reviving #45715

API Proposal: dotnet/runtime#115664

Add the ability to inject a proxy implementation for reading and writing a model type. This will replace the default implementation that lives on the model itself allowing a caller external to the library to adjust how things are written in some advanced scenarios.

Specific service need is AOAI wants the ability to control which properties of the OAI models it shares are serialized when sending to azure service. To do this they need to replace that implementation with their own custom implementation:

As part of a streaming chat completions request, users can add the stream_options property to the request body and set its include_usage property to true. This results in the service streaming back the number of tokens that were used as part of this request as an additional chunk before finishing the SSE response. OpenAI added the stream_options property for backwards compatibility reasons (in other words, instead of turning it on by default, users have to opt in). In practice, however, there is no reason for a user ever wanting to turn this off. Thus, in the OpenAI library (which is still in preview), we don't expose this property at all and always set it on behalf of the user. The issue then arises because Azure OpenAI does not support this feature yet and would fail if we were to send it in a request. We need a way for the Azure OpenAI library to prevent this property from being serialized.

@Copilot Copilot AI review requested due to automatic review settings May 13, 2025 20:54
@m-nash m-nash requested a review from a team as a code owner May 13, 2025 20:54
Copy link
Contributor

@Copilot 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 the ability to inject custom proxy implementations for model serialization and deserialization, allowing callers to override default behaviors. Key modifications include:

  • Integrating proxy resolution in deserialization and serialization methods across multiple model types.
  • Updating tests and samples to validate proxy behavior for both simple and nested models.
  • Enhancing the ModelReaderWriterOptions API to support proxy registration and lookup.

Reviewed Changes

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

Show a summary per file
File Description
ResourceProviderData.Serialization.cs Uses proxy resolution during deserialization of ProviderResourceType elements.
AvailabilitySetData.cs Adds a new field and constructor parameter for additional raw data.
AvailabilitySetData.Serialization.cs Updates constructor call to include the new raw data dictionary.
ResourceProviderDataProxy.cs Implements a custom proxy for ResourceProviderData with modified serialization (omitting the "id" property).
ProviderResourceTypeProxy.cs Renames a property ("resourceType" to "resourceTypex") for proxy serialization.
AvailabilitySetDataProxy.cs Custom proxy for AvailabilitySetData that adds an extra property during serialization.
ModelReaderWriterExtensions.cs Modifies WriteObjectValue to resolve proxies via the provided options.
ModelReaderWriterOptions.cs Introduces proxy registration/resolution API to support custom model serialization behaviors.
ModelWriterOfT.cs Replaces direct model.Write with proxy resolution before writing.
ModelReaderWriter.cs Adjusts serialization and deserialization flows to resolve proxies appropriately.
JsonModelConverter.cs, JsonCollectionWriter.cs, JsonCollectionReader.cs Updated to call proxy resolution when reading/writing JSON models.

Copy link

github-actions bot commented May 13, 2025

API Change Check

APIView identified API level changes in this PR and created the following API reviews

System.ClientModel

Copy link
Member

@christothes christothes left a comment

Choose a reason for hiding this comment

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

LGTM

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

Successfully merging this pull request may close these issues.

6 participants