Investigate generator support for client-typed ProcessMessage<TClient> calls

[christothes]

Follow-up from PR #1105 review thread.

In custom protocol code (e.g. src/Custom/Evals/EvaluationClient.Protocol.cs), ProcessMessage/ProcessMessageAsync is invoked with an explicit client type parameter:

return ClientResult.FromResponse(Pipeline.ProcessMessage<EvaluationClient>(message, options));

This routes error deserialization to the more specific DeserializeInternal<Foo>ErrorResponse for that client, yielding better-typed error responses. Generated methods, however, call ProcessMessage without the client type argument, so they don't benefit from the specialized error model.

Investigate

• Whether the TypeSpec/ClientModel generator should emit ProcessMessage<TClient>/ProcessMessageAsync<TClient> calls by default.
• Trade-offs: consistency of error model deserialization across all operations vs. any perf/size costs or generator complexity.
• Whether current manually-authored usages (Evals and any others) should be unified with generator output, or if the custom pattern should continue to be used selectively.

Context

• Comment: Refactor common tsp types #1105 (comment)
• Resolution note: Refactor common tsp types #1105 (comment)

[joseharriaga]

A few thoughts on this:

1. I think is important that we do not have divergent error handling logic between custom and generated operations.
Operations evolve over time, and it is not uncommon for them go from custom --> generated or from generated --> custom. If this transition had a side-effect on error handling, I think that would feel a little arbitrary and perhaps even be incorrect in certain cases.

2. We should strive to keep custom code and generated code as close to each other as possible.
For example: When customizing operations, good practice is to only change the bare minimum to achieve the desired behavior. The reason for this is that generated code evolves over time with generator changes, but custom code remains static and needs to be updated manually. The more that the custom code diverges from the generated code, the harder it is to do these manual fixups.

With this in mind, the question now is: Do we change the custom code to make it like the generated code, or do we use a visitor to make it like the custom code? We were discussing the latter in the PR, but looking more closely at the problem, I'm now thinking that the right solution might actually be the former. In other words, rather than using a visitor to emit ProcessMessage<TClient>/ProcessMessageAsync<TClient>, I'm thinking we might want to return to the old behavior of simply using ProcessMessage/ProcessMessageAsync. This is because the fact that we have different types of errors is artificial. We are separating the common error definition into different types per feature area because this makes it easier for us to maintain, but in practice, the error definition across all feature areas is the same. In that sense, there is no reason to branch into different paths if the end-result is the same by definition.

NOTE: Today, Responses and Evals have a slightly different error definitions in OpenAI's spec (they are missing "type" and "param") but this is incorrect. The error definition is the same across the board (I have confirmed it).

Here's what we should do:

1. Fix the error definitions of Responses and Evals in the TypeSpec.

2. Remove the new methods:

• ProcessMessage<TClient>
• ProcessMessageAsync<TClient>
• TryBufferResponseAndCreateError<TClient>
• `TryBufferResponseAndCreateErrorAsync
• TryCreateErrorMessageFromResponse<TClient>

3. Replace the uses of ProcessMessage<TClient> for ProcessMessage and the uses ProcessMessageAsync<TClient> for ProcessMessageAsync. This will fix a bug introduced in the PR where errors are not deserialized correctly in the Evals API.

4. The PR introduced a bug where if the error doesn't have a "message" value, nothing gets printed.
   

   openai-dotnet/src/Custom/Internal/ClientPipelineExtensions.cs

   Line 274 in ea9f1e4

   if (message != null)

   
   According to the spec, technically this is fine because the "message" is supposedly required. But I recall having seen cases where there is nothing but a "type" value, which this code is now hiding, so I recommend reverting to the old behavior without that check.