Skip to content

fix: ai-request-rewrite plugin wrong test #12083

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.

Sign up for GitHub

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

Draft
wants to merge 4 commits into
base: master
Choose a base branch
from
Draft

fix: ai-request-rewrite plugin wrong test #12083

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
b2d002c
fix: ai-request-rewrite test
LiteSun Mar 25, 2025
43f7b85
u
LiteSun Mar 25, 2025
b14b4e2
u
LiteSun Mar 27, 2025
3e23006
f
LiteSun Mar 27, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions apisix/plugins/ai-request-rewrite.lua
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -124,13 +124,12 @@ local function request_to_llm(conf, request_table, ctx)
model_options = conf.options
}

local res, err, httpc = ai_driver:request(conf, request_table, extra_opts)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No need to manully close according to the docs
https://github.com/openresty/lua-nginx-module#tcpsockclose
image

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://github.com/apache/apisix/blob/master/apisix/plugins/ai-drivers/openai-base.lua#L131

It seems that only two parameters were returned, why didn't painc occur before?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think at least one pass case is needed, and there should be no error level logs.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://github.com/apache/apisix/blob/master/apisix/plugins/ai-drivers/openai-base.lua#L131

It seems that only two parameters were returned, why didn't painc occur before?

This issue occurred because when I started development and created my branch, the function was returning three parameters. After this PR was merged: https://github.com/apache/apisix/pull/12030/files#diff-b9bb0990d3ea842c42b094837737acfb597e2c8bf3a976dda8be56c84ef5b3c0L109, the returned parameters were adjusted. The code in my branch hasn't been synchronized with upstream, which caused this inconsistency.

I think at least one pass case is needed, and there should be no error level logs.

The test cases are already covered, and the tests cannot pass without making this change.

local res, err = ai_driver:request(conf, request_table, extra_opts)
if err then
return nil, nil, err
end

local resp_body, err = res:read_body()
httpc:close()
if err then
return nil, nil, err
end
Expand Down
66 changes: 43 additions & 23 deletions t/plugin/ai-request-rewrite2.t
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@ add_block_preprocessor(sub {

default_type 'application/json';


location /check_extra_options {
content_by_lua_block {
local json = require("cjson.safe")
Expand All @@ -58,7 +57,7 @@ add_block_preprocessor(sub {
}
}
}
}
}
local json = require("cjson.safe")
local json_response = json.encode(response)
ngx.say(json_response)
Expand All @@ -67,20 +66,33 @@ add_block_preprocessor(sub {

location /test/params/in/overridden/endpoint {
content_by_lua_block {
local json = require("cjson.safe")
local core = require("apisix.core")
ngx.req.read_body()

local query_auth = ngx.req.get_uri_args()["api_key"]
ngx.log(ngx.INFO, "found query params: ", core.json.stably_encode(ngx.req.get_uri_args()))
local json = require("cjson.safe")
local body = ngx.req.get_body_data()
local request_data = json.decode(body)
local args = ngx.req.get_uri_args()
local response = {
choices = {
{
message = {
content = request_data.messages[1].content .. ' ' .. request_data.messages[2].content
}
}
}
}

if query_auth ~= "apikey" then
ngx.status = 401
ngx.say("Unauthorized")
if not args.some_query or args.some_query ~= "yes" then
ngx.status = 200
response.choices[1].message.content = "missing required query parameter: some_query=yes"
local json_response = json.encode(response)
ngx.say(json_response)
return
end

ngx.status = 200
ngx.say("passed")
local json_response = json.encode(response)
ngx.say(json_response)
}
}
}
Expand Down Expand Up @@ -184,23 +196,20 @@ override.endpoint is required for openai-compatible provider
content_by_lua_block {
local t = require("lib.test_admin").test
local code, body = t('/apisix/admin/routes/1',
ngx.HTTP_PUT,
[[{
ngx.HTTP_PUT,
[[{
"uri": "/anything",
"plugins": {
"ai-proxy": {
"ai-request-rewrite": {
"prompt": "some prompt to test",
"auth": {
"query": {
"api_key": "apikey"
}
},
"model": {
"provider": "openai",
"name": "gpt-35-turbo-instruct",
"options": {
"max_tokens": 512,
"temperature": 1.0
}
"provider": "openai",
"options": {
"model": "gpt-35-turbo-instruct"
},
"override": {
"endpoint": "http://localhost:6724/test/params/in/overridden/endpoint?some_query=yes"
Expand All @@ -217,10 +226,21 @@ override.endpoint is required for openai-compatible provider
}]]
)

if code >= 300 then
ngx.status = code
local code, body, actual_body = t("/anything",
ngx.HTTP_POST,
"some random content",
nil,
{
["Content-Type"] = "text/plain",
}
)
local json = require("cjson.safe")
local response_data = json.decode(actual_body)
if response_data.data == 'some prompt to test' .. ' some random content' then
ngx.say("passed")
else
ngx.say(response_data.data)
end
ngx.say(body)
}
}
--- response_body
Expand Down
Loading