sangkyoonnam opened a new pull request, #1205: URL: https://github.com/apache/flink-agents/pull/1205
Linked issue: Closes #1197 ### Purpose of change A custom strategy's selection outside the router's candidates, or a persisted decision replayed after its candidate was removed, now lands on the default model instead of failing the request, the rule the judge already followed. #### Runtime flow The decision is computed, or replayed from `route:<router>` when an action state store is enabled. `ModelRoutingResolver.normalizeAndFinish` checks it against the current candidates and turns a non-candidate into an abstaining copy, which resolves to the default model, else the first candidate. #### Key decisions Normalizing after persistence gives fresh and replayed decisions one check and leaves the stored record as the strategy wrote it. The rejected name goes into metadata because an abstaining `RoutingDecision` needs a null `selectedModel`. Strict failure was rejected: a judge reply is untrusted output, and strict replay fails every recovered request whose stored selection names a removed candidate. ### Behavioral Semantics #### Interaction decisions | Strategy | Run | Selection | Result | |---|---|---|---| | any | either | candidate | that candidate (unchanged) | | custom | fresh | non-candidate | default, `rejected_*` recorded (was: failed) | | rule | fresh | non-candidate | unreachable: rule keys are validated at declaration | | judge | fresh | non-candidate | judge abstains with its own reason (unchanged) | | any | replayed | removed candidate | current default; custom or rule strategy not rerun, judge model not invoked (was: failed) | | any | fresh | strategy throws, judge retries exhausted | failed `ChatResponseEvent` (unchanged) | #### Behavioral contracts 1. A fresh custom non-candidate selection is served by the default model, with `decision_source` `default`. 2. With no default, the first candidate serves it. 3. The event's `reason` becomes `selected model '<name>' is not a candidate`; metadata keeps its keys and adds `rejected_model`, plus `rejected_reason` if the strategy gave a reason. 4. The event's `score` is null and `decision_ms` keeps the strategy's value. 5. A replayed custom or rule decision naming a removed candidate gets the current default without rerunning the strategy. 6. A replayed judge verdict does too, without invoking the judge model. 7. A fresh judge non-candidate verdict carries no `rejected_model`. 8. Strategy exceptions, executor construction failures, and exhausted judge retries still yield a failed `ChatResponseEvent`. 9. A non-candidate default or rule key still fails at `build()` and at plan construction. #### Failure behavior A runtime non-candidate is absorbed, with no log or counter; it shows in the routing event and the response's `model_routing` block. `rejected_model` overwrites a strategy key of that name, `rejected_reason` only if the strategy gave a reason. Cancellation and persistence failures propagate and declaration errors throw, as before. ### Tests | # | Tests (`ChatModelActionRoutingTest` unless noted) | |---|---| | 1 | `customNonCandidateSelectionAbstainsToDefault` | | 2 | `customNonCandidateSelectionWithoutDefaultUsesFirstCandidate`, `storedNonCandidateDecisionFollowsCurrentDefault` | | 3 | `customNonCandidateSelectionAbstainsToDefault`, `storedDecisionForRemovedCandidateAbstainsToCurrentDefault` | | 4 | as 3; `decision_ms` value checked on replay only | | 5 | custom: the two `stored*` tests in rows 2 and 3; rule: none | | 6 | `storedJudgeVerdictForRemovedCandidateAbstainsToCurrentDefault` | | 7 | `llmJudgeVerdictOutsideCandidatesAbstainsToDefault` | | 8 | `strategyFailurePropagatesUnderDefaultPolicy`, `failingCustomExecutorConstructorIsNotPersistedAsTheDecision`, `llmJudgeExhaustedRetriesFailsRequest` | | 9 | `RoutingTest`: `builderRejectsDefaultModelThatIsNotACandidate`, `builderRejectsRuleKeyThatIsNotACandidate`; `AgentPlanLlmJudgeValidationTest`: `ruleKeyNamingNonCandidateFailsAtPlanConstruction`, `defaultModelNamingNonCandidateFailsAtPlanConstruction` | Coverage by risk: the changed behavior (1 to 6) has five new or rewritten tests, which fail against the old resolver logic; unchanged paths (7 to 9) have existing tests. Not verified: - A replayed rule decision (same path as custom). - Recovery through a real action state store; tests seed records in a fake context. - Response `model_routing` carries `rejected_model`. - `rejected_reason` absent without a strategy reason; overwriting a strategy's `rejected_*` keys. - Fallback after a normalized decision. <details> <summary>Implementation invariants and revert check</summary> - The stored `route:<router>` record is never rewritten; normalization works on a copy. No test asserts the stored record after normalization. - The abstaining copy is built as `new RoutingDecision(null, true, reason, null, metadata, decisionMs)`, satisfying the constructor's abstain invariant. Metadata is copied into a new map before the keys are added. - Revert check: restore only the old rejecting branch in `ModelRoutingResolver` and keep the two `REJECTED_*_KEY` constants, which the tests reference. `ChatModelActionRoutingTest` then runs 39 with 5 failing: `customNonCandidateSelectionAbstainsToDefault`, `customNonCandidateSelectionWithoutDefaultUsesFirstCandidate`, `storedDecisionForRemovedCandidateAbstainsToCurrentDefault`, `storedNonCandidateDecisionFollowsCurrentDefault`, `storedJudgeVerdictForRemovedCandidateAbstainsToCurrentDefault`. The extended `llmJudgeVerdictOutsideCandidatesAbstainsToDefault` passes on both, since the judge path is unchanged. With the change, 39 pass. </details> ### API No signature changes; new public constants `ModelRoutingResolver.REJECTED_MODEL_KEY` and `REJECTED_REASON_KEY` name the keys. A caller that changes nothing gets the default model where it got a failure; to spot a misconfigured strategy, read `rejected_model` from the `ModelRoutingEvent`. Unchanged: the persisted `RoutingDecision` shape, event fields, fresh judge behavior, contract 8's failures, and declaration validation. Python routing is unsupported, so unaffected. ### Documentation - [ ] `doc-needed` - [ ] `doc-not-needed` - [x] `doc-included` Also corrects two stale Advanced paragraphs, Validation (default-model check timing) and Recovery (judge replay), to match existing behavior. ### Was this patch authored or co-authored using generative AI tooling? - [x] Yes - [ ] No Generated-by: Claude Code 2.1.292 (Claude Opus 5.5) -- 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]
