GitHub user wenjin272 added a comment to the discussion: [Discussion] Pluggable model routing — API & design (PR #852 follow-up)
Thanks for the clear and comprehensive proposal @purushah. I have a few implementation details that I’d like to confirm: 1. My understanding is that users only need to send a `ChatRequestEvent` whose model name refers to a registered `ModelRouter`. `ChatModelAction` would then detect the router, invoke `ModelRouter.route()` to resolve the concrete chat model, emit a `ModelRoutingEvent`, and continue through the existing chat/tool-call loop using the selected model. In this design, is `ModelRoutingEvent` intended purely as an observability event for logging, tracing, and evaluation, with no built-in consumer and no second `ChatRequestEvent` being dispatched? Or do you expect it to drive a separate step that re-dispatches the request to the selected model? 2. I believe a routing decision should apply to one complete ReAct loop, from the initial `ChatRequestEvent` through all intermediate tool-call rounds until the final `ChatResponseEvent`. The selected model should therefore be associated with the loop’s `initialRequestId` and reused for subsequent `ToolResponseEvents` rather than invoking the router again. Conversation-level caching across separate ReAct loops seems like a distinct, optional behavior. Is this also the intended routing scope? 3. In #852, `CachingStrategy` keys the routing decision by the first user message because the router is itself a `ChatModelSetup` and is invoked again on every tool-call round. With `ModelRouter` separated from the chat model, would it be cleaner for `ChatModelAction` to associate the `RoutingDecision` with the `initialRequestId` and store the selected concrete model in the existing tool-call context? The router would then run once per ReAct loop, and all subsequent `ToolResponseEvent`s would reuse that model until the final `ChatResponseEvent`. In that case, the routing decision is part of the loop execution state, and the cache introduced in #852 may no longer be necessary. Is there another use case for caching routing decisions in the new design, or could `cache` and `cacheSize` be removed? Regarding the three open questions in the proposal: 1. I prefer keeping `RoutingStrategy` as the single extension point, with built-in strategies provided through factories such as `Strategies.rules(...)` and `Strategies.llm(...)`. This keeps built-in and custom strategies under the same abstraction and preserves extensibility without expanding the `ModelRouter` API for every new strategy. 2. I think `fallback` and similar options can remain simple configuration flags for now. We can introduce richer policy objects later when there are concrete requirements that cannot be expressed through configuration. For `cache` specifically, however, I think we should first clarify whether it is still necessary in the new design, as discussed above. 3. The proposed information exposed by `RoutingContext` seems sufficient for the initial scope. Request messages, prompt arguments, candidate descriptions and metadata, and access to resources should cover rule-based, LLM-based, and custom semantic or ML strategies. We can extend it later when a concrete strategy requires additional context. GitHub link: https://github.com/apache/flink-agents/discussions/897#discussioncomment-17681865 ---- This is an automatically sent email for [email protected]. To unsubscribe, please send an email to: [email protected]
