-
Notifications
You must be signed in to change notification settings - Fork 2.8k
feat(ai-proxy-multi): support client-driven model selection via models request body field #13084
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -15,6 +15,7 @@ | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| -- limitations under the License. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| -- | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| local ngx = ngx | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| local core = require("apisix.core") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| local schema = require("apisix.plugins.ai-proxy.schema") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| local base = require("apisix.plugins.ai-proxy.base") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -299,6 +300,76 @@ local function get_instance_conf(instances, name) | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| end | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| local function match_client_models(instances, models) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| local ordered_names = {} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| local matched = {} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| for _, model_pref in ipairs(models) do | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| local target_model, target_provider | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if type(model_pref) == "string" then | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| target_model = model_pref | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| elseif type(model_pref) == "table" then | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| target_model = model_pref.model | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| target_provider = model_pref.provider | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| end | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| for _, instance in ipairs(instances) do | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if not matched[instance.name] then | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| local inst_model = instance.options and instance.options.model | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| local matches = false | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if target_provider then | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| matches = (instance.provider == target_provider | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| and inst_model == target_model) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| else | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| matches = (inst_model == target_model) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| end | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if matches then | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| matched[instance.name] = true | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| core.table.insert(ordered_names, instance.name) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| break | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| end | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| end | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| end | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| end | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| for _, instance in ipairs(instances) do | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if not matched[instance.name] then | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| core.table.insert(ordered_names, instance.name) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| end | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| end | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+335
to
+340
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| for _, instance in ipairs(instances) do | |
| if not matched[instance.name] then | |
| core.table.insert(ordered_names, instance.name) | |
| end | |
| end | |
| local unmatched = {} | |
| for idx, instance in ipairs(instances) do | |
| if not matched[instance.name] then | |
| core.table.insert(unmatched, { | |
| name = instance.name, | |
| priority = instance.priority or 0, | |
| index = idx, | |
| }) | |
| end | |
| end | |
| table.sort(unmatched, function(a, b) | |
| if a.priority == b.priority then | |
| return a.index < b.index | |
| end | |
| return a.priority > b.priority | |
| end) | |
| for _, item in ipairs(unmatched) do | |
| core.table.insert(ordered_names, item.name) | |
| end |
Copilot
AI
Mar 11, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The models field is only stripped when allow_client_model_preference is enabled. This contradicts the PR description/docs (and the new tests) which state that models is always stripped before forwarding upstream. As-is, requests sent with models while the feature is disabled will forward an extra models field to upstream providers.
Suggestion: always remove models from the JSON request body when present, but only apply client-driven reordering when allow_client_model_preference is true (i.e., strip unconditionally; reorder conditionally).
Copilot
AI
Mar 11, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When client preference is used, instance selection bypasses pick_target() and therefore bypasses the existing active health-check filtering done via fetch_health_instances()/healthcheck_manager. This means a client can force the plugin to try an instance that health checks currently mark as down, changing behavior compared to the server-driven picker.
Suggestion: apply the same health-check availability filtering to the preferred list (e.g. build an allowlist of healthy instance names before iterating, or reuse the existing picker/checker status logic) so client-driven ordering doesn't ignore configured health checks.
| local name, ai_instance, err = pick_preferred_instance(ctx, conf) | |
| if err then | |
| return 503, err | |
| end | |
| ctx.picked_ai_instance_name = name | |
| ctx.picked_ai_instance = ai_instance | |
| ctx.balancer_ip = name | |
| ctx.bypass_nginx_upstream = true | |
| return | |
| -- apply health-check availability filtering to the preferred list | |
| local healthy_instances, health_err = fetch_health_instances(conf, ctx) | |
| local use_client_preference = true | |
| if not healthy_instances then | |
| core.log.warn("failed to fetch healthy instances for client model preference: ", | |
| health_err, ", falling back to balancer selection") | |
| use_client_preference = false | |
| else | |
| local healthy_set = {} | |
| for _, inst in ipairs(healthy_instances) do | |
| if inst.name then | |
| healthy_set[inst.name] = true | |
| end | |
| end | |
| local filtered_preference = {} | |
| for _, pref in ipairs(ctx.client_model_preference) do | |
| if pref.instance_name and healthy_set[pref.instance_name] then | |
| filtered_preference[#filtered_preference + 1] = pref | |
| end | |
| end | |
| if #filtered_preference == 0 then | |
| core.log.warn("no healthy instances match client model preference; ", | |
| "falling back to balancer selection") | |
| use_client_preference = false | |
| else | |
| ctx.client_model_preference = filtered_preference | |
| end | |
| end | |
| if use_client_preference then | |
| local name, ai_instance, err = pick_preferred_instance(ctx, conf) | |
| if err then | |
| return 503, err | |
| end | |
| ctx.picked_ai_instance_name = name | |
| ctx.picked_ai_instance = ai_instance | |
| ctx.balancer_ip = name | |
| ctx.bypass_nginx_upstream = true | |
| return | |
| end |
Copilot
AI
Mar 11, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The new client-preference retry path in retry_on_error() (falling back through ctx.client_model_preference on 429/5xx) isn’t covered by the added tests. Current test cases validate selection/reordering and stripping, but not that a 429/5xx from the preferred instance causes the plugin to retry the next preferred instance (and that it stops retrying on non-matching status codes).
Suggestion: add a test that makes the first preferred instance return 429/5xx and asserts the response comes from the next preferred instance (similar to existing fallback tests in ai-proxy-multi.balancer.t).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
match_client_models()assumesmodelsis an array and iterates it withipairs(models). If a client sends a non-array JSON value (e.g."models":"gpt-4"),ipairswill raise an error and the request will 500.Suggestion: validate
request_body.modelsis a table/array before callingmatch_client_models(and/or hardenmatch_client_modelsto return the default ordering whentype(models) ~= "table"). Also consider skipping elements that are not a string or object.