protocol models responses #828

christothes · 2025-11-11T18:36:43Z

No description provided.

KrzysztofCwalina

This is very nice and close. I added 3 comments, but supper happy to see how this does not complicate the namespace too much (or at all)

api/OpenAI.net8.0.cs

joseharriaga · 2025-11-12T06:14:34Z

api/OpenAI.net8.0.cs

        protected override BinaryData PersistableModelWriteCore(ModelReaderWriterOptions options);
    }
    [Experimental("OPENAI001")]
+    public class CreateResponseOptions : IJsonModel<CreateResponseOptions>, IPersistableModel<CreateResponseOptions> {


There are only minor differences between this class and the existing ResponseCreationOptions:

Renames.

The Input property, which is a parameter of the CreateResponse client method.

The Model property, which is missing here but it is also a parameter of the CreateResponse client method.

The Stream property, which is hidden because we set on behalf of the user when they call CreateResponseStreaming.

I think the renames are desirable, and I would be in favor of adding the missing properties to ResponseCreationOptions and have that one be the protocol model.

I implemented the renames.

joseharriaga · 2025-11-12T06:22:24Z

api/OpenAI.net8.0.cs

        protected override ResponseTool PersistableModelCreateCore(BinaryData data, ModelReaderWriterOptions options);
        protected override BinaryData PersistableModelWriteCore(ModelReaderWriterOptions options);
    }
+    public partial struct GetResponseInputItemsOptions {


This is identical to ResponseItemCollectionOptions except for one single difference: This class includes the ResponseId.

Furthermore, this is an interesting case because technically this model does not actually exist. What I mean is that the properties here actually correspond to path and query parameters instead of a body parameter. Personally, I do see merits in having a property bag that holds the optional parameters (which is what we do today with ResponseItemCollectionOptions), but I have a feeling that a "protocol model" is not applicable in this case.

joseharriaga · 2025-11-12T06:27:25Z

api/OpenAI.net8.0.cs

+        public string Order { get; set; }
+        public string ResponseId { get; set; }
+    }
+    public class GetResponseOptions {


This is similar to my comment about GetResposneInputItemsOptions: The properties here are path and query parameters (not a body parameter), so I wonder if having a "protocol model" here is actually applicable.

joseharriaga · 2025-11-12T06:29:39Z

api/OpenAI.net8.0.cs

        public override readonly string ToString();
    }
    [Experimental("OPENAI001")]
+    public enum Includable {


I have a PR out that adds this enum:
🔗 #820

Once it's merged, that's one less thing that you will need to keep in this PR. 🙂

This PR has been merged.

joseharriaga · 2025-11-12T06:41:49Z

api/OpenAI.net8.0.cs

        public override readonly string ToString();
    }
    [Experimental("OPENAI001")]
+    public class ResponseResult : IJsonModel<ResponseResult>, IPersistableModel<ResponseResult> {


There are only minor differences between this class and the existing OpenAIResponse:

Renames.

The Object property, which is hidden in OpenAIResponse because it's not very useful in .NET.

The OutputText property, which is not a real service property, but a convenience the Stainless added for their own libraries. As such, it shouldn't be included in the "protocol model". In OpenAIResponse, we exposed it via the GetOutputText method.

I think the renames are desirable. I would be in favor of adding the Object property to OpenAIResponse but hiding it with EBN. And I think the GetOutputText is desirable over an OutputText property.

I implemented the renames. I also marked OutputText as internal. We already had GetOutputText

joseharriaga · 2025-11-12T06:44:16Z

api/OpenAI.net8.0.cs

+        public virtual Uri Endpoint { get; }
        [Experimental("OPENAI001")]
-        public string Model { get; }
+        public virtual string Model { get; }


I'm curious: Why do Endpoint and Model need to be virtual?

I found that sometimes non-virtual properties are nulled when referncing them in an instrumented client (during tests)

joseharriaga · 2025-11-12T08:06:04Z

api/OpenAI.net8.0.cs

+        public virtual Task<ClientResult<ResponseResult>> GetResponseAsync(GetResponseOptions options, CancellationToken cancellationToken = default);
        public virtual Task<ClientResult> GetResponseAsync(string responseId, bool? stream, int? startingAfter, RequestOptions options);
-        public virtual Task<ClientResult<OpenAIResponse>> GetResponseAsync(string responseId, CancellationToken cancellationToken = default);
+        public virtual ClientResult<ResponseItemList> GetResponseInputItems(GetResponseInputItemsOptions options = default, CancellationToken cancellationToken = default);


public virtual ClientResult<ResponseItemList> GetResponseInputItems(GetResponseInputItemsOptions options = default, CancellationToken cancellationToken = default); public virtual CollectionResult<ResponseItem> GetResponseInputItems(string responseId, ResponseItemCollectionOptions options = null, CancellationToken cancellationToken = default); public virtual CollectionResult GetResponseInputItems(string responseId, int? limit, string order, string after, string before, RequestOptions options); public virtual Task<ClientResult<ResponseItemList>> GetResponseInputItemsAsync(GetResponseInputItemsOptions options = default, CancellationToken cancellationToken = default); public virtual AsyncCollectionResult<ResponseItem> GetResponseInputItemsAsync(string responseId, ResponseItemCollectionOptions options = null, CancellationToken cancellationToken = default); public virtual AsyncCollectionResult GetResponseInputItemsAsync(string responseId, int? limit, string order, string after, string before, RequestOptions options);

Looking at the above, I'm a little conflicted about having six methods for each list operation. Additionally, I think it's a little odd that the protocol method has pagination conveniences, while the convenience method that takes protocol models does not have pagination conveniences (it's like neither variant is fully protocol nor fully convenience). I wonder if we can simplify this...

What would you think of the following: What if we do not add list methods that return a protocol model? 99% of users won't benefit from these, and they would instead favor the methods that can paginate. If necessary, I would be in favor of shipping the protocol model by itself to support MRW scenarios and server scenarios.

I think the protocol model return types give better control over advanced pagination scenarios, but I agree they are probably needed rarely. I do think there is some value in having access to the raw protocol model without the pagination abstractions.

@KrzysztofCwalina What is the story for users growing from paginating protocol methods to convenience methods?

KrzysztofCwalina · 2025-11-12T17:54:26Z

examples/Responses/Example01_SimpleResponse.cs

    public void Example01_SimpleResponse()
    {
-        OpenAIResponseClient client = new(model: "gpt-5", apiKey: Environment.GetEnvironmentVariable("OPENAI_API_KEY"));
+        ResponseClient client = new(model: "gpt-5", apiKey: Environment.GetEnvironmentVariable("OPENAI_API_KEY"));


we should call it ResponsesClient, not ResponseClient

api/OpenAI.net8.0.cs

KrzysztofCwalina · 2025-11-13T16:27:47Z

api/OpenAI.net8.0.cs

-        public virtual AsyncCollectionResult GetResponseInputItemsAsync(string responseId, int? limit, string order, string after, string before, RequestOptions options);
-        public virtual CollectionResult<StreamingResponseUpdate> GetResponseStreaming(GetResponseOptions options, CancellationToken cancellationToken = default);
-        public virtual AsyncCollectionResult<StreamingResponseUpdate> GetResponseStreamingAsync(GetResponseOptions options, CancellationToken cancellationToken = default);
+    public readonly partial struct ModelIdsResponses : IEquatable<ModelIdsResponses> {


@joseharriaga, do we really want this type? It seems like it will be continuously out of date.

We don't. The model should be represented as a simple string.

KrzysztofCwalina · 2025-11-13T16:28:20Z

api/OpenAI.net8.0.cs

+        public string FirstId { get; }
+        public bool HasMore { get; }
+        public string LastId { get; }
+        public string Object { get; }


we should consider renaming this.

About the Object property, I think we should make it internal, although in the spirit of protocol models, I suppose we need to keep it? If so, we should at least EBN it.

KrzysztofCwalina · 2025-11-13T16:30:05Z

api/OpenAI.net8.0.cs

    [Experimental("OPENAI001")]
+    public class ResponsesClient {
+        protected ResponsesClient();
+        protected internal ResponsesClient(ClientPipeline pipeline, string model, OpenAIClientOptions options);


@m-nash, it would be good to change this into your new ctor pattern before we GA, i.e. not necessarily in this PR.

joseharriaga · 2025-11-17T19:49:35Z

api/OpenAI.net8.0.cs

+        public IList<Includable> Include { get; set; }
+        public IList<ResponseItem> Input { get; }
+        public string Instructions { get; set; }
+        public bool? IsBackgroundModeEnabled { get; set; }


The pattern that we have used so far does not use the "Is" prefix, so I'm thinking that maybe we should stick to it for consistency.

joseharriaga · 2025-11-17T19:51:06Z

api/OpenAI.net8.0.cs

+        public bool? IsStreamingEnabled { get; set; }
+        public int? MaxOutputTokenCount { get; set; }
+        public IDictionary<string, string> Metadata { get; }
+        public ModelIdsResponses? Model { get; set; }


Historically, we don't use an enum for model names, and instead we keep it just as a string.

joseharriaga · 2025-11-17T19:51:45Z

api/OpenAI.net8.0.cs

+    public class CreateResponseOptions : IJsonModel<CreateResponseOptions>, IPersistableModel<CreateResponseOptions> {
+        public CreateResponseOptions(List<ResponseItem> input);
+        public string EndUserId { get; set; }
+        public IList<Includable> Include { get; set; }


In the change where I introduced this property (which just got merged), I called it "IncludedProperties".

It's also not supposed to have a setter because it's a collection property.

joseharriaga · 2025-11-17T19:52:35Z

api/OpenAI.net8.0.cs

+        public CreateResponseOptions(List<ResponseItem> input);
+        public string EndUserId { get; set; }
+        public IList<Includable> Include { get; set; }
+        public IList<ResponseItem> Input { get; }


nit: Rename to "InputItems" to make it clearer that this is a collection of items.

joseharriaga · 2025-11-17T19:59:39Z

api/OpenAI.net8.0.cs

    }
    [Experimental("OPENAI001")]
+    public class CreateResponseOptions : IJsonModel<CreateResponseOptions>, IPersistableModel<CreateResponseOptions> {
+        public CreateResponseOptions(List<ResponseItem> input);


For this constructor, we should consider:

Adding a string model parameter

Renaming the input parameter to "inputItems"

Changing the type of the input parameter from List<ResponseItem> to IEnumerable<ResponseItem>.

joseharriaga · 2025-11-17T20:31:45Z

api/OpenAI.net8.0.cs

    }
    [Experimental("OPENAI001")]
+    public class ResponseResult : IJsonModel<ResponseResult>, IPersistableModel<ResponseResult> {
+        public DateTimeOffset CreatedAt { get; }


ResponseResult is an output model, but it is also a protocol model. Do the properties need both: getters and setters?

joseharriaga · 2025-11-17T20:32:47Z

api/OpenAI.net8.0.cs

+        public override readonly string ToString();
    }
    [Experimental("OPENAI001")]
    public static class OpenAIResponsesModelFactory {


Assuming that the properties of the new ResponseResult don't have setters, we need a new method in this model factory to instantiate a mock ResponseResult.

joseharriaga · 2025-11-17T20:48:55Z

src/Custom/OpenAIClient.cs

+    /// <returns> A new <see cref="ResponsesClient"/>. </returns>
    [Experimental("OPENAI001")]
-    public virtual OpenAIResponseClient GetOpenAIResponseClient(string model) => new(Pipeline, model, _options);
+    public virtual ResponsesClient GetResponsesClient(string model) => new(Pipeline, model, _options);


There are a few generator attributes at the top of this file that need to be updated with the new name.

joseharriaga · 2025-11-17T21:10:25Z

api/OpenAI.net8.0.cs

-        public virtual Task<ClientResult> CancelResponseAsync(string responseId, RequestOptions options);
-        public virtual Task<ClientResult<OpenAIResponse>> CancelResponseAsync(string responseId, CancellationToken cancellationToken = default);
-        public virtual ClientResult CreateResponse(BinaryContent content, RequestOptions options = null);
-        public virtual ClientResult<OpenAIResponse> CreateResponse(IEnumerable<ResponseItem> inputItems, ResponseCreationOptions options = null, CancellationToken cancellationToken = default);


In the OpenAI library, we started using this pattern of separating the required properties from the optional properties for a few reasons:

Users found it awkward to have use two different ways to set the properties of the options class: They had to use the constructor for required properties, and initialization syntax for the optional properties. The options classes in the OpenAI API usually have tons of properties, and it is very common to at least want to set one or two. So this issue was very front-and-center. By making the options class all optional properties, this pain point was mitigated.

Users wanted to re-use an instance of the options class across requests. Consider the following scenario: When using stateful responses, if the input items are in the options class, users need to clear the collection and then re-populate it with the new input items every time. By moving the input items out of the options class, users can forget about the options instance entirely.

Would we want to preserve this functionality?

What if we kept the convenience methods as they are right now?

Here's an example of what I'm thinking...

We keep the following method:

public virtual ClientResult<OpenAIResponse> CreateResponse(IEnumerable<ResponseItem> inputItems, ResponseCreationOptions options = null, CancellationToken cancellationToken = default);

And then, we add a parameterless constructor to the options class. If a user specifies inputItems in both, the method parameter and the options property, we throw an exception (similar to what we would do if they set the stream property but call the non-streaming method).

joseharriaga · 2025-11-17T21:14:28Z

api/OpenAI.net8.0.cs

+        public virtual Task<ClientResult<ResponseResult>> GetResponseAsync(GetResponseOptions options, CancellationToken cancellationToken = default);
        public virtual Task<ClientResult> GetResponseAsync(string responseId, bool? stream, int? startingAfter, RequestOptions options);
-        public virtual Task<ClientResult<OpenAIResponse>> GetResponseAsync(string responseId, CancellationToken cancellationToken = default);
+        public virtual ClientResult<ResponseItemList> GetResponseInputItems(GetResponseInputItemsOptions options = default, CancellationToken cancellationToken = default);


@KrzysztofCwalina What is the story for users growing from paginating protocol methods to convenience methods?

christothes added 8 commits October 30, 2025 17:04

CreateResponse

c3fff0f

CreateResponseStreaming

9157407

GetResponse

b91fb1d

GetResponseInputItems

72a14c3

export

994700f

wip

09b98ef

hide convenience APIs

df1aab0

merge

4a23a08

KrzysztofCwalina reviewed Nov 11, 2025

View reviewed changes

api/OpenAI.net8.0.cs Show resolved Hide resolved

api/OpenAI.net8.0.cs Outdated Show resolved Hide resolved

api/OpenAI.net8.0.cs Outdated Show resolved Hide resolved

christothes added 2 commits November 11, 2025 13:08

fb

036535a

fb

c759c61

joseharriaga reviewed Nov 12, 2025

View reviewed changes

KrzysztofCwalina reviewed Nov 12, 2025

View reviewed changes

christothes added 3 commits November 12, 2025 12:53

fb

52df809

export

2b9deaa

merge

695e78e

KrzysztofCwalina reviewed Nov 13, 2025

View reviewed changes

fb

24c52bb

christothes marked this pull request as ready for review November 13, 2025 18:55

christothes requested review from ShivangiReja and trrwilson as code owners November 13, 2025 18:55

joseharriaga reviewed Nov 17, 2025

View reviewed changes

protocol models responses #828

Are you sure you want to change the base?

protocol models responses #828

Conversation

christothes commented Nov 11, 2025

Uh oh!

KrzysztofCwalina left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!