sangkyoonnam commented on code in PR #1363:
URL: https://github.com/apache/flink-agents/pull/1363#discussion_r4557975629
##########
plan/src/main/java/org/apache/flink/agents/plan/actions/ChatModelAction.java:
##########
@@ -378,35 +405,112 @@ private static void chat(
result.model,
result.retryCount,
result.totalRetryWaitSec);
- if (selection.isRouter()) {
- if (!result.model.equals(selection.getSelectedModel())) {
- // The strategy's pick failed and another candidate
answered; emit a
- // routing Event to report the fallback outcome.
+
+ // Quality gate: on a tier below the last one, an answer that
fails a structural
+ // check or a declared signal is discarded before any of its
tool calls runs, and
+ // the conversation so far goes to the next tier, where the
request then stays.
+ Optional<Escalation> policy = selection.getEscalation();
+ if (policy.isPresent()) {
+ Optional<EscalationGate.Outcome> escalate;
+ try {
+ escalate =
+ EscalationGate.check(
+ policy.get(),
+ initialRequestId,
+ selection.getRequestedModel(),
+ candidate,
+ escalations,
+ messages,
+ promptArgs,
+ result,
+ ctx);
+ } catch (ModelRoutingResolver.RoutingFailure e) {
+ // A signal that cannot judge the round (verifier out
of retries, custom
+ // signal threw) fails the logical request, like a
failed routing decision.
+ takeRoutingMetadata(ctx.getSensoryMemory(),
initialRequestId);
+ sendFailure(initialRequestId, e, ctx);
+ return;
+ }
+ if (escalate.isPresent()) {
Review Comment:
When fallback reaches a lower tier first, two rules in the model routing
docs pull in opposite directions.
Repro: tiers `small, big`, `defaultModel("big")`, fallback on. `big` throws,
fallback gets an answer from `small`, the verifier rejects it, and the gate
sends the request back to `big`. The models called are `[big, small, big]`, and
the request fails with `big`'s second error, after spending the retry budget on
`big` twice. The docs say a request is never failed for quality alone, and also
that a failed escalation target with no candidate left fails the request. This
case matches both, and today it gets the failure.
Having the gate keep the answer when the next tier is already in
`triedModels` isn't enough on its own. With `small, medium, big` and `medium`
as the default that's down, that check would return the rejected `small` answer
without ever asking `big`. On the current head the request does reach `big`,
after a wasted second call to `medium`. So the options I see are (a) treat a
tier that failed this round as unreachable, move on to the next untried higher
tier, and keep the lower answer only when none is left, or (b) keep failing as
documented. I'd lean toward (a), with the tier walk in `EscalationGate.check`
so `ChatModelAction` doesn't grow, which also fits wenjin272's point about
moving routing out of this class. I have a local test for the two-tier case and
can share it once the direction is settled.
Which do you prefer?
--
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]
