Add a routing context factory for request-scoped policy state - #7684
Open
joshuajyue wants to merge 2 commits into
Open
Add a routing context factory for request-scoped policy state#7684joshuajyue wants to merge 2 commits into
joshuajyue wants to merge 2 commits into
Conversation
Add a protected virtual CreateContext to FailoverChatClient so a derived class can return its own RoutingContext subclass. One context is already created per request and supplied to every selection and routing update, so state stored on it is scoped to the request and released with it. Previously a policy that needed state across attempts had to keep a side table keyed by the context and remove the entry on the terminal update. That state outlives the request whenever routing ends without a terminal update, such as when selection throws after a nonterminal update or when a streaming enumerator is abandoned without being disposed. Move OrderedFailoverChatClient to the new pattern. Its next-client index is now a field on its own context, which removes the ConcurrentDictionary, the lookups on every selection and update, and the explicit cleanup on termination. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 74d04840-2379-4615-93f7-84f2299ada74
joshuajyue
requested review from
PranavSenthilnathan and
jozkee
and removed request for
a team,
PranavSenthilnathan,
Copilot and
jozkee
August 6, 2026 18:29
jozkee
approved these changes
Aug 7, 2026
The cast cannot fail: OrderedFailoverChatClient is sealed, FailoverChatClient seals both invocation methods, and the selection and update methods are protected, so the only context they receive is the one CreateContext produced. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 74d04840-2379-4615-93f7-84f2299ada74
Contributor
There was a problem hiding this comment.
Pull request overview
Adds an extensibility point to chat routing so derived routing clients can create a per-request RoutingContext (including custom subclasses) and have it flow consistently through selection and (for failover) routing updates, enabling request-scoped policy state without side tables.
Changes:
- Add
protected virtual RoutingContext CreateContext(IEnumerable<ChatMessage>, ChatOptions?)toRoutingChatClientand use it in both non-streaming and streaming entry points (with a null-return guard). - Update
FailoverChatClientto useCreateContextin its sealed overrides and document the request-scoped-state pattern. - Refactor
OrderedFailoverChatClientto store its “next client index” on a privateRoutingContextsubclass, removing the per-requestConcurrentDictionaryand related cleanup paths; update tests and API manifests accordingly.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| test/Libraries/Microsoft.Extensions.AI.Tests/ChatRouting/OrderedFailoverChatClientTests.cs | Replaces reflection-based state-leak test with an abandoned-stream scenario to validate state scoping. |
| test/Libraries/Microsoft.Extensions.AI.Tests/ChatRouting/FailoverChatClientTests.cs | Adds coverage ensuring custom contexts flow through selection and updates; adds null-context guard coverage. |
| test/Libraries/Microsoft.Extensions.AI.Abstractions.Tests/ChatRouting/RoutingChatClientTests.cs | Adds coverage ensuring custom contexts flow to selection; adds null-context guard coverage. |
| src/Libraries/Microsoft.Extensions.AI/Microsoft.Extensions.AI.json | Updates API manifest to include OrderedFailoverChatClient.CreateContext override. |
| src/Libraries/Microsoft.Extensions.AI/ChatRouting/OrderedFailoverChatClient.cs | Moves ordered failover request state onto a context subclass; removes side-table state. |
| src/Libraries/Microsoft.Extensions.AI/ChatRouting/FailoverChatClient.cs | Switches to CreateContext for request context creation and documents request-scoped context state. |
| src/Libraries/Microsoft.Extensions.AI.Abstractions/Microsoft.Extensions.AI.Abstractions.json | Updates API manifest to include RoutingChatClient.CreateContext. |
| src/Libraries/Microsoft.Extensions.AI.Abstractions/ChatRouting/RoutingChatClient.cs | Introduces CreateContext factory and uses it in both invocation paths with a null guard. |
Comment on lines
+129
to
+133
| private static OrderedRoutingContext GetState(RoutingContext context) | ||
| { | ||
| Debug.Assert(context is OrderedRoutingContext, "The context was created by CreateContext."); | ||
| return (OrderedRoutingContext)context; | ||
| } |
Member
There was a problem hiding this comment.
if an unexpected RoutingContext instance is ever passed here
that would require src changes, which will trigger this assertion during testing.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up to #7662.
Summary
Adds one
protected virtualmethod toRoutingChatClient:GetResponseAsyncandGetStreamingResponseAsynccall it instead of constructing aRoutingContextdirectly, so a derived class can return its own subclass and have it flow through selection and, forFailoverChatClient, every routing update for that request.OrderedFailoverChatClientmoves onto it, which removes itsConcurrentDictionary<RoutingContext, int>.Why
Request-scoped policy state already exists —
OrderedFailoverChatClientkeeps a next-client index across attempts. The question is only where it lives.Today it lives in a side table keyed by the context, because there is nowhere else to put it. One
RoutingContextis already created per request and handed to everySelectClientAsyncandOnRoutingUpdateAsynccall for that request, so it has exactly the right lifetime, and it is already a non-sealedpublic classwith a public constructor. The only thing missing is that the base class hardcodes its construction, so a subclass instance can never reach the selector.ConcurrentDictionary; every access hashes and may contendThe capability row is the unchanged one: a side table works.
RoutingContextdoes not overrideEquals/GetHashCode, so reference-keying is sound.CreateContextis not about adding functionality.The leak is the difference: state must survive from a nonterminal update to the following selection, so it cannot be removed eagerly, and a request can end without a terminal update — an abandoned streaming enumerator, or selection failing after state was already stored. Today the derived class is responsible for cleaning up on those paths. Context-owned state hands that responsibility to the GC.
RoutingContextstill clones the caller's options in its constructor, and a subclass has to call that constructor, so the caller's instance stays protected either way. A derived context can also be a natural home for per-attempt options a selector wants to shape, without adding anything to the routing API.Changes
RoutingChatClient— addsCreateContext; both invocation methods call it, with anullguard matching the existingSelectClientAsynccheck.FailoverChatClient— no new API. It inheritsCreateContext, and because it declares both invocation methodssealed override, the factory is guaranteed to run for its derived types.OrderedFailoverChatClient— moves its next-client index onto a private context subclass; removes theConcurrentDictionary, the per-call lookups, both cleanup paths, and the_requestStates.Clear()inDispose.CreateContextcoverage at both theRoutingChatClientandFailoverChatClientlevels, including anullreturn;OrderedFailover_AbandonedStreamDoesNotAffectLaterRequestsreplaces the test that used reflection to inspect the removed dictionary.Notes
CreateContextcasts the context back to its own type inSelectClientAsync.OrderedFailoverChatClientthrowsArgumentExceptionnamingCreateContextif the type does not match, rather than surfacing a rawInvalidCastException.RoutingChatClient<TContext>would remove the downcast, at the cost of a type parameter across the whole hierarchy.Validation
Microsoft.Extensions.AI.Tests— 762 passed.Microsoft.Extensions.AI.Abstractions.Tests— 1646 passed.MakeApiBaselines.ps1output.Microsoft Reviewers: Open in CodeFlow