sangkyoonnam opened a new pull request, #1151: URL: https://github.com/apache/flink-agents/pull/1151
Linked issue: #1150 ### Purpose of change A `ModelRouter` declaration with a duplicate or empty candidate, no candidates, or a default model that is not a candidate passed `Builder#build()` and `AgentPlan` construction, and failed only in the router constructor on the TaskManager, inside the durable routing call, per routed request. This change fails those declarations at the registration call site and at plan construction, the way rule keys and the judge model already do since #1042. Runtime flow: unchanged for a valid declaration. For an invalid one the `IllegalArgumentException` now surfaces at `build()` (fluent path) or `new AgentPlan(...)` (descriptor path) instead of `ModelRoutingResolver#resolve` on the TaskManager. Key decisions: - One static check, `RoutingCandidateValidator#validate(subject, candidates, defaultModel)` in `api`, called by the constructor, the builder and `AgentPlan`, so the three paths share the check and the message without exception wrapping. It is a util class like `ToolParameterInjectionValidator` (also called from `AgentPlan`) rather than a method on `ModelRouter`, which keeps `ModelRouter` to candidates and the strategy declaration. - The validator takes `List<?>` and `Object` because a deserialized descriptor may carry non-String elements; those are rejected with the candidate-name message rather than a `ClassCastException`. - `ModelRouter.DEFAULT_MODEL_KEY` replaces the literal `"default_model"` in the constructor, the builder and `AgentPlan`, as `CANDIDATES_KEY` already does. - A missing or empty `candidates` list is no longer deferred to the constructor at plan construction, so the `candidates == null` early return in `validateRuleKeys` became dead and is removed. - `AgentPlanRoutingBackstopTest`'s minimal router fixture gains one candidate, as it gained a strategy when strategy validation was added. ### Behavioral Semantics Interaction decisions: `build()` validates candidates before the existing rule-key check, so a bad declaration is reported by the first failing rule in declaration order (candidates, then default model, then rule keys). `AgentPlan#validateRoutingStrategies` validates after the `candidates` shape guard for every strategy type. Behavioral contracts: the validator throws `IllegalArgumentException` when the list is null or empty, when any element is not a non-empty `String`, when a name repeats, or when the default model is not one of the names. The constructor's messages for the duplicate and default-model cases are unchanged; plan construction prefixes them with `Model router '<name>'` like the other plan-time router diagnostics. Failure behavior: an invalid declaration now fails the registration call or plan construction and never reaches the TaskManager. Previously, under `FAIL` the job died on the first routed record, and under `IGNORE` every routed record was dropped with the job RUNNING. ### Tests | Contract | Test | |---|---| | `build()` rejects duplicate candidate | `RoutingTest#builderRejectsDuplicateCandidate` | | `build()` rejects default model not a candidate | `RoutingTest#builderRejectsDefaultModelThatIsNotACandidate` | | `build()` rejects empty candidate list | `RoutingTest#builderRequiresAtLeastOneCandidate` | | `build()` rejects empty candidate name | `RoutingTest#builderRejectsBlankCandidateName` | | constructor rejects empty candidate name from a raw descriptor | `RoutingTest#routerConstructionRejectsBlankCandidateName` | | plan rejects default model not a candidate, names router | `AgentPlanLlmJudgeValidationTest#defaultModelNamingNonCandidateFailsAtPlanConstruction` | | plan rejects duplicate candidate | `AgentPlanLlmJudgeValidationTest#duplicateCandidateFailsAtPlanConstruction` | | plan rejects empty candidate list | `AgentPlanLlmJudgeValidationTest#emptyCandidateListFailsAtPlanConstruction` | Verified with `mvn -pl api,plan test` (api 459, plan 400 passed), `mvn -pl runtime test` (980 passed), `spotless:check` and `./tools/check-license.sh`. Without the `api` change the five new `RoutingTest` cases fail; without the `AgentPlan` change the three new plan cases fail; nothing else changes. ### API `RoutingCandidateValidator` (api) and `ModelRouter.DEFAULT_MODEL_KEY` are added; no existing signature changes. Empty-string candidate names, previously accepted by `build()`, are now rejected. A plan that previously constructed with an empty or missing `candidates` list, or a non-candidate `default_model`, now fails at plan construction; such a plan could never route a request. Python is not affected: routers cannot be registered from Python yet. ### Documentation <!-- Do not remove this section. Check the proper box only. --> - [ ] `doc-needed` <!-- Your PR changes impact docs --> - [ ] `doc-not-needed` <!-- Your PR changes do not impact docs --> - [x] `doc-included` <!-- Your PR already contains the necessary documentation updates --> ### Was this patch authored or co-authored using generative AI tooling? <!-- Do not remove this section. Check the proper box only. --> - [x] Yes - [ ] No Generated-by: Claude Code 2.1.280 (Claude Fable 5.1) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
