[api][routing] Pluggable in-chat LLM routing (ChatModelRouter + RoutingStrategy)#852
[api][routing] Pluggable in-chat LLM routing (ChatModelRouter + RoutingStrategy)#852purushah wants to merge 2 commits into
Conversation
…ngStrategy) Add a drop-in chat model that selects which underlying model serves each request, then delegates to it. The router is a CHAT_MODEL resource, so an agent points at it by name with no runtime, event, or agent-definition change. Selection is a pluggable SPI (`RoutingStrategy`), decomposed into orthogonal concerns: - RoutingStrategy — pure selection (request -> candidate name). Returning null means "abstain / no opinion". - FallbackPolicy — optional: try remaining candidates on error. - CachingStrategy — optional bounded-LRU memoization of the decision per conversation, so an expensive strategy (e.g. an LLM judge) runs once per conversation rather than once per tool-call round. Built-in strategies: - RuleBasedRoutingStrategy — deterministic keyword/regex rules + default. - LlmRoutingStrategy — a small "judge" model picks the candidate from each candidate's name/description (RouteLLM-style). Bring-your-own strategies are first-class: implement RoutingStrategy and reference it by fully-qualified class name; loaded via the thread context classloader (cluster-safe). ML/learned routing is supported the same way. Routing-miss semantics: a strategy that abstains (null) or names a non-candidate degrades to the configured `default` candidate (validated at construction; defaults to the first candidate) rather than failing the request. The LLM judge distinguishes a transient failure (abstain -> not cached, retried next round) from an unparseable reply (deterministic default). Security: an LLM/ML routing decision is a hint, not an authority — the user's message is sent to the judge, so cost/privilege/safety must not be gated solely on it (prompt-injection risk). This is documented on the strategy. Includes an example (LlmRoutingAgentExample) and unit tests covering rule selection, judge parsing (whole-token match, no substring mis-routing), stickiness, fallback, caching (incl. abstain-not-cached), and bring-your-own. Also mirror the RULE_BASED/LLM ResourceName constants on the Python side (ResourceName.RoutingStrategy.Java) and register RoutingStrategy in the cross-language ResourceName parity check.
…at test Review follow-ups from @weiqingy on the routing PR: - ChatModelRouter.open(): document the load-bearing invariant the no-op relies on — a routed candidate is lazily open()-ed by ResourceCache.getResource() on first resolution, so its connection is non-null before chat() runs. - CachingStrategy / LlmRoutingStrategy: soften "runs once per conversation" to "typically once" and document that memoization is best-effort (a concurrent first-touch on the same key can double-compute; synchronized map, last-writer- wins, benign — so no locking). - RoutingCandidate: reject an empty name (not just null) — an empty name has no resolvable resource and would make LlmRoutingStrategy.parseChoice's whole-token match over-match arbitrary boundaries (mis-route). - Tests: add ChatModelRouterTest cases pinning the open-before-chat invariant (candidate resolved through an opening ResourceContext, mirroring ResourceCache; plus the negative case proving it is load-bearing), and RoutingCandidateTest for the null/empty name guards. 39 routing tests pass; spotless:check clean under JDK 17.
|
Thanks for the follow-ups, @purushah. My comments are resolved. I'll leave the final call to the maintainers. |
|
Thanks @weiqingy — really appreciate you taking the time to go through the design and call out the subtle cases. Your comments were very helpful, especially around the open() invariant and the caching wording. Glad the follow-ups addressed them. |
|
Hi @purushah , Sorry for the late response. I was busy preparing the 0.3 release, Flink Forward Asia, and catching up on a few other works, and only had the chance to carefully look into this PR today. First of all, thank you again for the contribution! I think this is a very useful and interesting feature, and the implementation quality is also very high. It’s clear that you’ve put a lot of thought and effort into both the design and the implementation. That said, for a feature of this importance and complexity, I would generally prefer to discuss the overall design first (e.g., through a GitHub Discussion) before moving into a full implementation. After going through the PR, I realized that most of my questions are actually about the design rather than the implementation itself. If we can align on these design questions first, we can hopefully avoid unnecessary rework later. Here are my current thoughts. 1. Should ChatModelRouter be modeled as a special kind of ChatModelSetup?I’m not entirely convinced yet. The biggest advantage of the current design is exactly what you pointed out: it requires almost no framework changes, and the router can be used just like any other ChatModelSetup, which makes the implementation straightforward. However, I also see some potential drawbacks.
One alternative direction (just as an example—I haven’t fully thought through the design yet) would be to treat model routing as a framework capability rather than a specialized ChatModelSetup, for example by introducing a dedicated ChatModelRouter resource type. My intuition is that this might naturally address some of the observability and metrics concerns mentioned above. That said, this is only an initial thought, not a concrete proposal. 2. The boundary between Public API and Internal Implementation doesn’t seem very clear yetThis was another impression I had while reading through the PR. At the moment, most of the newly introduced concepts and implementations live inside the API module, so it’s difficult to immediately distinguish:
Ideally, I think the API module should contain only the minimal set of interfaces that users actually need to interact with. This would help us maintain API compatibility more easily across future releases while still giving us enough flexibility to evolve the internal implementation. 3. What is the intended user experience of this feature?I also think there is still room to discuss the user-facing API design. In other words, it may be helpful to first step back and think about how users are expected to use this feature end-to-end, and then evaluate whether the API naturally supports that workflow. For example:
I'm not saying that I’m against the current design. In fact, I don’t have firm answers to the above questions either. What I'm trying to say is that, in order to answer these questions, we probably should have a more fundamental design discussion first:
Overall, I think this is a valuable feature, and I really appreciate the amount of work you’ve put into this PR. My suggestion would be to treat this PR as a prototype for now rather than moving directly into detailed code review. As a next step, I think it would be helpful to first flesh out the overall design—either by opening a GitHub Discussion or by adding a design proposal to this PR—especially around the following questions:
Once we have aligned on those aspects, I believe the implementation details will become much easier to converge on, and hopefully we don't need to rework this PR a lot. WDYT? |
|
Thanks for the detailed review, @xintongsong — and no worries on the timing! I think this is a great idea. I agree it's better to align on the overall design first before going deeper on the implementation. Let's keep this PR open and use it as a prototype/reference for the discussion. I'll follow up here shortly with a short design proposal covering the points you raised. Thanks again for the thoughtful feedback! |
What is the purpose of the change
Adds a drop-in chat model that routes each request to the best underlying model, then delegates to it. The router is a
CHAT_MODELresource, so an agent points at it by name with no change to the runtime, events, or agent definition.This is the in-chat selector (which LLM serves a single
chat()call). A DataStream-level content-based agent router (branching records across agent operators) is a separate, follow-up concern.Brief change log
RoutingStrategy— pluggable selection SPI (request -> candidate name). Selection is a pure concern; returningnullmeans "abstain / no opinion".ChatModelRouter— orchestrates select → (optional cache) → validate → delegate. A strategy that abstains (null) or names a non-candidate is a routing miss and degrades to the configureddefaultcandidate (validated at construction; defaults to the first candidate) rather than failing the request.FallbackPolicy— optional: try remaining candidates on error.CachingStrategy— optional bounded-LRU memoization of the decision per conversation, so an expensive strategy (e.g. an LLM judge) runs once per conversation, not once per tool-call round. Abstentions (null) are never cached.RuleBasedRoutingStrategy— deterministic keyword/regex rules + default.LlmRoutingStrategy— a small "judge" model picks the candidate from each candidate's name/description (RouteLLM-style). Distinguishes a transient judge failure (abstain → retried next round, uncached) from an unparseable reply (deterministic default). Parses by whole-token match (no substring mis-routing, e.g.gpt-4o-miniwon't match agpt-4candidate).RoutingStrategyand reference it by fully-qualified class name; loaded via the thread context classloader (cluster-safe). ML/learned routing is supported the same way.LlmRoutingAgentExampleand unit tests.Verifying this change
This change adds tests and can be verified as follows:
api/.../chat/model/routing/covering rule selection, judge parsing (whole-token match), stickiness across tool-call rounds, fallback, caching (incl. abstain-not-cached), routing-miss degrade-to-default, and bring-your-own loading. All pass;spotless:checkclean (JDK 17).Does this pull request potentially affect one of the following parts:
org.apache.flink.agents.api.chat.model.routingpackage (additive; no existing API changed).CHAT_MODELresource resolved by name)Security note
An LLM/ML routing decision is a hint, not an authority — the user's message is sent to the judge model, so a routing decision is susceptible to prompt injection. Cost/privilege/safety controls must not be gated solely on it. This is documented on
LlmRoutingStrategy.Documentation
Documentation
doc-neededdoc-not-neededdoc-included