Copilot commented on code in PR #12895:
URL: https://github.com/apache/apisix/pull/12895#discussion_r2922868140
##########
t/plugin/ai-proxy.openai-compatible.t:
##########
@@ -338,3 +348,80 @@ passed
}
--- response_body_eval
qr/6data: \[DONE\]\n\n/
+
+
+
+=== TEST 6: set route with defaults field
+--- config
+ location /t {
+ content_by_lua_block {
+ local t = require("lib.test_admin").test
+ local code, body = t('/apisix/admin/routes/1',
+ ngx.HTTP_PUT,
+ [[{
+ "uri": "/anything",
+ "plugins": {
+ "ai-proxy": {
+ "provider": "openai-compatible",
+ "auth": {
+ "header": {
+ "Authorization": "Bearer token"
+ }
+ },
+ "options": {
+ "model": "server-model"
+ },
+ "defaults": {
+ "max_tokens": 512,
+ "temperature": 0.7
+ },
+ "override": {
+ "endpoint":
"http://localhost:6724/v1/chat/completions";
+ },
+ "ssl_verify": false
+ }
+ }
+ }]]
+ )
+
+ if code >= 300 then
+ ngx.status = code
+ end
+ ngx.say(body)
+ }
+ }
+--- response_body
+passed
+
+
+
+=== TEST 7: defaults applied when not set in request
+--- request
+POST /anything
+{ "messages": [ { "role": "user", "content": "hello" } ] }
+--- more_headers
+test-type: defaults
+--- response_body
+{"max_tokens":512,"model":"server-model","temperature":0.7}
+
Review Comment:
These assertions compare the raw JSON string exactly, but the response is
produced via `cjson.safe.encode` on a Lua table with string keys, where key
order is not guaranteed. This can make the test flaky across Lua/cjson builds.
Prefer asserting via `response_body_like`/`response_body eval` (regexes) or
decode the JSON and compare fields rather than matching the exact serialized
key order.
##########
t/plugin/ai-proxy-multi.openai-compatible.t:
##########
@@ -296,3 +306,86 @@ passed
}
--- response_body_like eval
qr/6data: \[DONE\]\n\n/
+
+
+
+=== TEST 5: set route with defaults field
+--- config
+ location /t {
+ content_by_lua_block {
+ local t = require("lib.test_admin").test
+ local code, body = t('/apisix/admin/routes/1',
+ ngx.HTTP_PUT,
+ [[{
+ "uri": "/anything",
+ "plugins": {
+ "ai-proxy-multi": {
+ "instances": [
+ {
+ "name": "self-hosted",
+ "provider": "openai-compatible",
+ "weight": 1,
+ "auth": {
+ "header": {
+ "Authorization": "Bearer token"
+ }
+ },
+ "options": {
+ "model": "server-model"
+ },
+ "defaults": {
+ "max_tokens": 512,
+ "temperature": 0.7
+ },
+ "override": {
+ "endpoint":
"http://localhost:6724/v1/chat/completions";
+ }
+ }
+ ],
+ "ssl_verify": false
+ }
+ }
+ }]]
+ )
+
+ if code >= 300 then
+ ngx.status = code
+ end
+ ngx.say(body)
+ }
+ }
+--- response_body
+passed
+
+
+
+=== TEST 6: defaults applied when not set in request
+--- request
+POST /anything
+{ "messages": [ { "role": "user", "content": "hello" } ] }
+--- more_headers
+test-type: defaults
+--- response_body
+{"max_tokens":512,"model":"server-model","temperature":0.7}
+
+
+
+=== TEST 7: user value takes precedence over defaults
+--- request
+POST /anything
+{ "messages": [ { "role": "user", "content": "hello" } ], "max_tokens": 100,
"temperature": 0.5 }
+--- more_headers
+test-type: defaults
+--- response_body
+{"max_tokens":100,"model":"server-model","temperature":0.5}
Review Comment:
These assertions compare the raw JSON string exactly, but the response is
produced via `cjson.safe.encode` on a Lua table with string keys, where key
order is not guaranteed. This can make the test flaky across Lua/cjson builds.
Prefer asserting via `response_body_like`/`response_body eval` (regexes) or
decode the JSON and compare fields rather than matching the exact serialized
key order.
##########
apisix/plugins/ai-proxy/base.lua:
##########
@@ -62,6 +62,7 @@ function _M.before_proxy(conf, ctx, on_error)
name = ai_instance.name,
endpoint = core.table.try_read_attr(ai_instance, "override",
"endpoint"),
model_options = ai_instance.options,
+ model_defaults = ai_instance.defaults,
conf = ai_instance.provider_conf or {},
Review Comment:
Now that `ai_instance.defaults` is passed through as
`extra_opts.model_defaults`, fields like `defaults.model` can affect the
effective model sent upstream, but `before_proxy` currently sets
`ctx.var.llm_model` based only on `options.model` or `request_body.model`. This
means logging/metrics may miss the actual model when it comes solely from
`defaults`. Consider including `ai_instance.defaults.model` in the model
selection logic (with the intended precedence) so ctx vars reflect the request
actually sent.
##########
t/plugin/ai-proxy-multi.openai-compatible.t:
##########
@@ -296,3 +306,86 @@ passed
}
--- response_body_like eval
qr/6data: \[DONE\]\n\n/
+
+
+
+=== TEST 5: set route with defaults field
+--- config
+ location /t {
+ content_by_lua_block {
+ local t = require("lib.test_admin").test
+ local code, body = t('/apisix/admin/routes/1',
+ ngx.HTTP_PUT,
+ [[{
+ "uri": "/anything",
+ "plugins": {
+ "ai-proxy-multi": {
+ "instances": [
+ {
+ "name": "self-hosted",
+ "provider": "openai-compatible",
+ "weight": 1,
+ "auth": {
+ "header": {
+ "Authorization": "Bearer token"
+ }
+ },
+ "options": {
+ "model": "server-model"
+ },
+ "defaults": {
+ "max_tokens": 512,
+ "temperature": 0.7
+ },
+ "override": {
+ "endpoint":
"http://localhost:6724/v1/chat/completions";
+ }
+ }
+ ],
+ "ssl_verify": false
+ }
+ }
+ }]]
+ )
+
+ if code >= 300 then
+ ngx.status = code
+ end
+ ngx.say(body)
+ }
+ }
+--- response_body
+passed
+
+
+
+=== TEST 6: defaults applied when not set in request
+--- request
+POST /anything
+{ "messages": [ { "role": "user", "content": "hello" } ] }
+--- more_headers
+test-type: defaults
+--- response_body
+{"max_tokens":512,"model":"server-model","temperature":0.7}
+
Review Comment:
These assertions compare the raw JSON string exactly, but the response is
produced via `cjson.safe.encode` on a Lua table with string keys, where key
order is not guaranteed. This can make the test flaky across Lua/cjson builds.
Prefer asserting via `response_body_like`/`response_body eval` (regexes) or
decode the JSON and compare fields rather than matching the exact serialized
key order.
##########
t/plugin/ai-proxy.openai-compatible.t:
##########
@@ -338,3 +348,80 @@ passed
}
--- response_body_eval
qr/6data: \[DONE\]\n\n/
+
+
+
+=== TEST 6: set route with defaults field
+--- config
+ location /t {
+ content_by_lua_block {
+ local t = require("lib.test_admin").test
+ local code, body = t('/apisix/admin/routes/1',
+ ngx.HTTP_PUT,
+ [[{
+ "uri": "/anything",
+ "plugins": {
+ "ai-proxy": {
+ "provider": "openai-compatible",
+ "auth": {
+ "header": {
+ "Authorization": "Bearer token"
+ }
+ },
+ "options": {
+ "model": "server-model"
+ },
+ "defaults": {
+ "max_tokens": 512,
+ "temperature": 0.7
+ },
+ "override": {
+ "endpoint":
"http://localhost:6724/v1/chat/completions";
+ },
+ "ssl_verify": false
+ }
+ }
+ }]]
+ )
+
+ if code >= 300 then
+ ngx.status = code
+ end
+ ngx.say(body)
+ }
+ }
+--- response_body
+passed
+
+
+
+=== TEST 7: defaults applied when not set in request
+--- request
+POST /anything
+{ "messages": [ { "role": "user", "content": "hello" } ] }
+--- more_headers
+test-type: defaults
+--- response_body
+{"max_tokens":512,"model":"server-model","temperature":0.7}
+
+
+
+=== TEST 8: user value takes precedence over defaults
+--- request
+POST /anything
+{ "messages": [ { "role": "user", "content": "hello" } ], "max_tokens": 100,
"temperature": 0.5 }
+--- more_headers
+test-type: defaults
+--- response_body
+{"max_tokens":100,"model":"server-model","temperature":0.5}
+
Review Comment:
These assertions compare the raw JSON string exactly, but the response is
produced via `cjson.safe.encode` on a Lua table with string keys, where key
order is not guaranteed. This can make the test flaky across Lua/cjson builds.
Prefer asserting via `response_body_like`/`response_body eval` (regexes) or
decode the JSON and compare fields rather than matching the exact serialized
key order.
##########
t/plugin/ai-proxy.openai-compatible.t:
##########
@@ -338,3 +348,80 @@ passed
}
--- response_body_eval
qr/6data: \[DONE\]\n\n/
+
+
+
+=== TEST 6: set route with defaults field
+--- config
+ location /t {
+ content_by_lua_block {
+ local t = require("lib.test_admin").test
+ local code, body = t('/apisix/admin/routes/1',
+ ngx.HTTP_PUT,
+ [[{
+ "uri": "/anything",
+ "plugins": {
+ "ai-proxy": {
+ "provider": "openai-compatible",
+ "auth": {
+ "header": {
+ "Authorization": "Bearer token"
+ }
+ },
+ "options": {
+ "model": "server-model"
+ },
+ "defaults": {
+ "max_tokens": 512,
+ "temperature": 0.7
+ },
+ "override": {
+ "endpoint":
"http://localhost:6724/v1/chat/completions";
+ },
+ "ssl_verify": false
+ }
+ }
+ }]]
+ )
+
+ if code >= 300 then
+ ngx.status = code
+ end
+ ngx.say(body)
+ }
+ }
+--- response_body
+passed
+
+
+
+=== TEST 7: defaults applied when not set in request
+--- request
+POST /anything
+{ "messages": [ { "role": "user", "content": "hello" } ] }
+--- more_headers
+test-type: defaults
+--- response_body
+{"max_tokens":512,"model":"server-model","temperature":0.7}
+
+
+
+=== TEST 8: user value takes precedence over defaults
+--- request
+POST /anything
+{ "messages": [ { "role": "user", "content": "hello" } ], "max_tokens": 100,
"temperature": 0.5 }
+--- more_headers
+test-type: defaults
+--- response_body
+{"max_tokens":100,"model":"server-model","temperature":0.5}
+
+
+
+=== TEST 9: options always override user value
+--- request
+POST /anything
+{ "messages": [ { "role": "user", "content": "hello" } ], "model":
"user-model", "max_tokens": 100 }
+--- more_headers
+test-type: defaults
+--- response_body
+{"max_tokens":100,"model":"server-model","temperature":0.7}
Review Comment:
These assertions compare the raw JSON string exactly, but the response is
produced via `cjson.safe.encode` on a Lua table with string keys, where key
order is not guaranteed. This can make the test flaky across Lua/cjson builds.
Prefer asserting via `response_body_like`/`response_body eval` (regexes) or
decode the JSON and compare fields rather than matching the exact serialized
key order.
##########
t/plugin/ai-proxy-multi.openai-compatible.t:
##########
@@ -296,3 +306,86 @@ passed
}
--- response_body_like eval
qr/6data: \[DONE\]\n\n/
+
+
+
+=== TEST 5: set route with defaults field
+--- config
+ location /t {
+ content_by_lua_block {
+ local t = require("lib.test_admin").test
+ local code, body = t('/apisix/admin/routes/1',
+ ngx.HTTP_PUT,
+ [[{
+ "uri": "/anything",
+ "plugins": {
+ "ai-proxy-multi": {
+ "instances": [
+ {
+ "name": "self-hosted",
+ "provider": "openai-compatible",
+ "weight": 1,
+ "auth": {
+ "header": {
+ "Authorization": "Bearer token"
+ }
+ },
+ "options": {
+ "model": "server-model"
+ },
+ "defaults": {
+ "max_tokens": 512,
+ "temperature": 0.7
+ },
+ "override": {
+ "endpoint":
"http://localhost:6724/v1/chat/completions";
+ }
+ }
+ ],
+ "ssl_verify": false
+ }
+ }
+ }]]
+ )
+
+ if code >= 300 then
+ ngx.status = code
+ end
+ ngx.say(body)
+ }
+ }
+--- response_body
+passed
+
+
+
+=== TEST 6: defaults applied when not set in request
+--- request
+POST /anything
+{ "messages": [ { "role": "user", "content": "hello" } ] }
+--- more_headers
+test-type: defaults
+--- response_body
+{"max_tokens":512,"model":"server-model","temperature":0.7}
+
+
+
+=== TEST 7: user value takes precedence over defaults
+--- request
+POST /anything
+{ "messages": [ { "role": "user", "content": "hello" } ], "max_tokens": 100,
"temperature": 0.5 }
+--- more_headers
+test-type: defaults
+--- response_body
+{"max_tokens":100,"model":"server-model","temperature":0.5}
+
+
+
+=== TEST 8: options always override user value
+--- request
+POST /anything
+{ "messages": [ { "role": "user", "content": "hello" } ], "model":
"user-model", "max_tokens": 100 }
+--- more_headers
+test-type: defaults
+--- response_body
+{"max_tokens":100,"model":"server-model","temperature":0.7}
Review Comment:
These assertions compare the raw JSON string exactly, but the response is
produced via `cjson.safe.encode` on a Lua table with string keys, where key
order is not guaranteed. This can make the test flaky across Lua/cjson builds.
Prefer asserting via `response_body_like`/`response_body eval` (regexes) or
decode the JSON and compare fields rather than matching the exact serialized
key order.
##########
apisix/plugins/ai-drivers/openai-base.lua:
##########
@@ -315,6 +315,16 @@ function _M.request(self, ctx, conf, request_table,
extra_opts)
ssl_server_name = parsed_url and parsed_url.host or self.host,
}
+ -- defaults: apply only when not set in request
+ if extra_opts.model_defaults then
+ for opt, val in pairs(extra_opts.model_defaults) do
+ if request_table[opt] == nil then
+ request_table[opt] = val
+ end
+ end
+ end
Review Comment:
Applying `model_defaults` here mutates `request_table` after
`apisix/plugins/ai-proxy/base.lua` has already derived `ctx.var.request_type`
and `stream_options` from the original request body. If `defaults` (or
`options`) sets fields that affect control flow (e.g., `stream`), APISIX can
end up sending a streamed request while still treating it as non-streaming for
logging/metrics and without setting `stream_options.include_usage`. Consider
applying defaults/options earlier (before the `request_body.stream` branch in
`before_proxy`) or re-evaluating stream-related ctx vars after merging
defaults/options.
--
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]
