Add extensible chat client routing - #7662
Conversation
Introduce one-shot and failover routing primitives, ordered and semantic routing implementations, lifecycle reporting, streaming safeguards, and focused routing tests. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 74d04840-2379-4615-93f7-84f2299ada74
This comment was marked as resolved.
This comment was marked as resolved.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Treat provider-thrown cancellation as terminal, materialize semantic routing messages before inspection, and enforce non-null selection consistently for streaming. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 74d04840-2379-4615-93f7-84f2299ada74
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 74d04840-2379-4615-93f7-84f2299ada74
Capture Current access failures before committing output so pre-output failover and attempt reporting remain accurate. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 74d04840-2379-4615-93f7-84f2299ada74
Apply ConfigureAwait(false) consistently across selection, invocation, streaming, disposal, and routing update awaits. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 74d04840-2379-4615-93f7-84f2299ada74
| IChatClient client = await SelectClientAsync(context, cancellationToken).ConfigureAwait(false) ?? | ||
| throw new InvalidOperationException($"{nameof(SelectClientAsync)} returned null."); | ||
| await foreach (ChatResponseUpdate update in | ||
| client.GetStreamingResponseAsync(context.Messages, context.ChatOptions, cancellationToken) |
There was a problem hiding this comment.
It seems a little unintuitive that context.ChatOptions is the input to SelectClientAsync but it's also the output since users can mutate it. If we expect people to return modified options, it should be part of the return type for SelectClientAsync. I also think the properties on RequestContext should be readonly for the same reason.
There was a problem hiding this comment.
I see what you mean. The official "API-standard" way ChatOptions should be configured is through a unique ConfiguredChatOptions client (this is better for failover, cooldowns, etc.). So, the user should not mutate ChatOptions inside SelectClientAsync. I'll go ahead and make RoutingContext get-only.
Making ChatOptions get-only does still allow the user to change ChatOptions, since it's a reference, but I'll put some documentation about how they should use ConfiguredChatOptions. I don't know if there is a viable way to absolutely block the user other than cloning.
If the user wants to, in the end, they can still mutate it, but they should know that'll be mutating the original caller-options.
There was a problem hiding this comment.
There is, however, a concern where certain RoutingChatClients, such as SemanticRoutingChatClient, rely on stable messages. Since IEnumerable<ChatMessage> may be lazy or one-shot, the selector and selected client could otherwise observe different results.
Originally, a derived RCC could stabilize messages by replacing them with an IReadOnlyList. Since messages is now get-only, I opted for an optional BufferMessages() method on RoutingContext. Routers only call it when needed; it reuses an existing IReadOnlyList, or enumerates and caches the sequence once for that request.
We could also leave this API out and leave repeatable enumeration up to the caller. This is the design addition I’m less certain about. I'll leave it in for now.
There was a problem hiding this comment.
I don't know if there is a viable way to absolutely block the user other than cloning.
Cloning is currently the only way.
There was a problem hiding this comment.
I'll go ahead and clone options so it's read-only / doesn't affect the caller options. Still leaving the conversation open however for the question about BufferMessages()
There was a problem hiding this comment.
After thinking through the ChatOptions more, I think supporting both configured clients and returned options is useful.
Not every option variation needs a distinct route identity. For example, gpt-5.5 low and high reasoning may belong to the same failure: if high fails, policy state keyed by that client should also apply to low. In that case, the IChatClient reference should remain the route identity while returned options shape only the current attempt.
When configurations do need independent policy state -- such as gpt-5.5 and gpt-5.6 being cooled down separately -- the user can select distinct ConfigureOptionsChatClient wrappers.
I am leaning toward a small per-attempt selection result rather than returning RoutingContext:
RoutingSelection(IChatClient client, ChatOptions? options)RoutingContext would remain one stable object for the request, while each selection explicitly returns the client and options for that attempt. The contract would state that:
IChatClientreference identity is the sole route identity.- Returned options do not create a distinct route.
OrderedFailoverChatClientcontinues to operate on unique client references.RoutingContext.ChatOptionsis the request-level baseline shared across selections.RoutingSelection.ChatOptionsis scoped to the current attempt.FailoverChatClientAttempt.ChatOptionsrecords the options routing passed for that attempt.
There was a problem hiding this comment.
You don't need RoutingSelection to enable something like cooldown-all-clients-with-the-same-backend:
class SharedBackendFailoverChatClient(IReadOnlyList<(IChatClient Client, string BackendKey)> routes)
: FailoverChatClient;I'm not sure about ergonomic value; I personally like the simplicity of just returning an IChatClient and I think you made a good point in mentioning that ConfigureOptions can be used to describe different clients/routes.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
There was a problem hiding this comment.
You don't need RoutingSelection to enable something like cooldown-all-clients-with-the-same-backend
I'm not sure about ergonomic value; I personally like the simplicity of just returning an IChatClient and I think you >made a good point in mentioning that ConfigureOptions can be used to describe different clients/routes.
Yeah, I agree. I stress-tested the RoutingSelection / ChatRoute directions, and they ended up adding more option semantics than they were worth. ChatRoute requires us to merge route options with caller options, while RoutingSelection introduces a second options path that mostly overlaps with ConfigureOptionsChatClient.
I kept SelectClientAsync returning IChatClient. RoutingContext.ChatOptions is now only a snapshot for selection, and each downstream failover attempt gets a fresh clone of the caller options so mutations from one client do not leak into the next attempt. If a policy needs a separate backend or failure-domain identity, it can own that alongside the client exactly like your (Client, BackendKey) example. Configurations that should have distinct client identity can still use separate ConfigureOptionsChatClient wrappers.
Addressed in 87d382d63.
There was a problem hiding this comment.
Addressed in commit 0abbfd1, along with some cancellation races
a19b74b to
0abbfd1
Compare
Clarify RoutingContext input semantics, add optional message buffering, centralize validation, simplify timing and disposal, and separate caller cancellation from provider failures. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 74d04840-2379-4615-93f7-84f2299ada74
| IChatClient client = await SelectClientAsync(context, cancellationToken).ConfigureAwait(false) ?? | ||
| throw new InvalidOperationException($"{nameof(SelectClientAsync)} returned null."); | ||
| await foreach (ChatResponseUpdate update in | ||
| client.GetStreamingResponseAsync(context.Messages, context.ChatOptions, cancellationToken) |
There was a problem hiding this comment.
I don't know if there is a viable way to absolutely block the user other than cloning.
Cloning is currently the only way.
Clone request options for selector shaping and restrict routing updates to concrete client attempts, leaving selection-failure cleanup to selectors. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 74d04840-2379-4615-93f7-84f2299ada74
Follow MEAI's repeatable-enumerable convention and propagate selector failures without cancellation rewriting. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 74d04840-2379-4615-93f7-84f2299ada74
Use ConcurrentDictionary atomic operations instead of explicit lock blocks while preserving the narrow request-state lifetime. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 74d04840-2379-4615-93f7-84f2299ada74
896e51d to
a576773
Compare
Keep client selection simple while preventing selector and failed-client option mutations from affecting downstream attempts. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 74d04840-2379-4615-93f7-84f2299ada74
Document client-reference identity and keep semantic scoring parameters together in the constructor signature. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 74d04840-2379-4615-93f7-84f2299ada74
Align routing code with MEAI formatting and documentation conventions, simplify disposal, and split routing tests by component and assembly. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 74d04840-2379-4615-93f7-84f2299ada74
|
Thank you David for the feedback, addressed in 8b79ae4 |
Summary
Adds experimental chat-routing primitives for selecting configured
IChatClientinstances without imposing a particular routing policy.RoutingChatClientsupports one-shot client selection.FailoverChatClientretries uncanceled failures that occur before output is exposed.OrderedFailoverChatClientprovides ordered fallback.SemanticRoutingChatClientselects clients using cached profile embeddings.RoutingContextexposes mutable request messages and options.FailoverChatClientAttemptreports invocation outcome, duration, time to first update, completion, and output commitment.Related to #7647.
API shape
Routing policies have two seams:
SelectClientAsyncruns before every invocation.OnRoutingUpdateAsyncreports each attempt so policy state can influence the next selection.isTerminalmeans the base will not select another client after the callback completes. A null attempt represents selection terminating before a client was invoked.Failover behavior
Semantic routing
SemanticRoutingChatClient:topK: 1.leaveOpen.Tests
Coverage includes selection failures, retries, cancellation, attempt limits, streaming commitment, early disposal, enumerator failures, concurrent requests, state cleanup, semantic thresholds, top-K aggregation, caching, and ownership.
net10.0net472test project builds successfullyMicrosoft Reviewers: Open in CodeFlow