nic-6443 commented on code in PR #14139:
URL: https://github.com/apache/apisix/pull/14139#discussion_r4464296151
##########
apisix/plugins/ai-protocols/anthropic-messages.lua:
##########
@@ -345,31 +345,78 @@ function _M.get_messages(body)
end
+local function system_blocks(system)
+ local blocks = {}
+ if type(system) == "string" then
+ core.table.insert(blocks, {type = "text", text = system})
+ elseif type(system) == "table" then
+ for _, block in ipairs(system) do
+ core.table.insert(blocks, block)
+ end
+ end
+ return blocks
+end
+
+
+local function configured_messages(msgs)
+ local systems = {}
+ local messages = {}
+ for _, msg in ipairs(msgs or {}) do
+ if msg.role == "system" then
+ core.table.insert(systems, {type = "text", text = msg.content})
+ else
+ core.table.insert(messages, {role = msg.role, content =
msg.content})
+ end
+ end
+ return systems, messages
+end
+
+
--- Prepend messages to the request body.
function _M.prepend_messages(body, msgs)
if not msgs or #msgs == 0 then return end
- if not body.messages then
- body.messages = {}
+
+ local systems, messages = configured_messages(msgs)
+ if #systems > 0 then
+ local merged = systems
+ for _, block in ipairs(system_blocks(body.system)) do
+ core.table.insert(merged, block)
+ end
+ body.system = merged
Review Comment:
This turns `body.system` into a block array, but `get_messages` (line 329)
only reads `system` when it's a string, so the system prompt disappears from
the normalized messages as soon as the decorator prepends one.
The decorator runs in rewrite, and ai-prompt-guard (`match_all_roles`) and
ai-lakera-guard read `get_messages` in access. So a client sending a string
`system` with denied content now gets through, and the decorated content isn't
checked either. Before this PR both were checked. A client sending an array
`system` could already skip this, so having `get_messages` reuse
`extract_system_content` would fix both.
##########
apisix/plugins/ai-prompt-template.lua:
##########
@@ -88,43 +72,63 @@ local templates_lrucache = core.lrucache.new({
ttl = 300, count = 256
})
-local templates_json_lrucache = core.lrucache.new({
- ttl = 300, count = 256
-})
-
function _M.check_schema(conf)
return core.schema.check(schema, conf)
end
-local function get_request_body_table(max_size)
- local body, err = core.request.get_body(max_size)
- if not body then
- return nil, { message = "could not get body: " .. (err or "request
body is empty") }
+local function find_template(conf, template_name)
+ for _, template in ipairs(conf.templates) do
+ if template.name == template_name then
+ return template.template
+ end
end
+ return nil
+end
- local body_tab, err = core.json.decode(body)
- if not body_tab then
- return nil, { message = "could not parse JSON request body: " .. (err
or "invalid JSON") }
+
+local function render_string(value, input)
+ local raw_template = string_gsub(value, "{{(.-)}}", "{*%1*}")
+ local ok, render = pcall(template.compile, raw_template)
+ if not ok then
+ return nil, "template compile: " .. render
end
- return body_tab
+ local output
+ ok, output = pcall(render, input)
+ if not ok then
+ return nil, "template rendering: " .. output
+ end
+ return output
end
-local function find_template(conf, template_name)
- for _, template in ipairs(conf.templates) do
- if template.name == template_name then
- return template.template
+local function render_template(configured, input)
Review Comment:
Same regression membphis raised above, just adding to the list: `system`,
`input` and `messages[].name` are affected too, not only `user` and `metadata`.
There's no error, the literal `{{ x }}` just goes to the LLM, so nobody will
notice after an upgrade until they look at the model output.
##########
apisix/plugins/ai-prompt-template.lua:
##########
@@ -88,43 +72,63 @@ local templates_lrucache = core.lrucache.new({
ttl = 300, count = 256
})
-local templates_json_lrucache = core.lrucache.new({
- ttl = 300, count = 256
-})
-
function _M.check_schema(conf)
return core.schema.check(schema, conf)
end
-local function get_request_body_table(max_size)
- local body, err = core.request.get_body(max_size)
- if not body then
- return nil, { message = "could not get body: " .. (err or "request
body is empty") }
+local function find_template(conf, template_name)
+ for _, template in ipairs(conf.templates) do
+ if template.name == template_name then
+ return template.template
+ end
end
+ return nil
+end
- local body_tab, err = core.json.decode(body)
- if not body_tab then
- return nil, { message = "could not parse JSON request body: " .. (err
or "invalid JSON") }
+
+local function render_string(value, input)
+ local raw_template = string_gsub(value, "{{(.-)}}", "{*%1*}")
+ local ok, render = pcall(template.compile, raw_template)
+ if not ok then
+ return nil, "template compile: " .. render
end
- return body_tab
+ local output
+ ok, output = pcall(render, input)
Review Comment:
`render(input)` uses the request body as the context directly, which drops
the `_ctx`, `_body`, `_escape_json`, `_escape_xml` and `_multipart` helpers
that body-transformer injected through the metatable.
An existing template like `ip {{ _ctx.var.remote_addr }}` or `{*
_escape_json(x) *}` now returns 503 on every request with `attempt to index
global '_ctx' (a nil value)`. I reproduced that one with a small `resty`
script. Neither the docs nor the PR description mention this as a breaking
change.
##########
apisix/plugins/ai-prompt-decorator.lua:
##########
@@ -71,36 +55,30 @@ function _M.check_schema(conf)
end
-local function get_request_body_table(max_size)
- local body, err = core.request.get_body(max_size)
- if not body then
- return nil, { message = "could not get body: " .. (err or "request
body is empty") }
- end
-
- local body_tab, err = core.json.decode(body)
- if not body_tab then
- return nil, { message = "could not parse JSON request body: " .. (err
or "invalid JSON") }
- end
-
- return body_tab
-end
-
-
function _M.rewrite(conf, ctx)
- local body_tab, err = get_request_body_table(conf.max_req_body_size)
+ local body_tab, status, err = body.read(conf.max_req_body_size)
Review Comment:
`fail_mode` doesn't cover this path. `body.read` returns before protocol
detection, so `skip` and `warn` never apply to read or parse failures.
With the default `fail_mode=skip`, a Consumer-bound decorator returns 400
for an empty-body GET or a non-JSON form post, while `binding.lua` and the docs
say that kind of non-AI traffic should pass through. ai-prompt-guard already
sends non-JSON bodies to `binding.on_unsupported` (line 119), so the two
plugins disagree here too.
##########
apisix/plugins/ai-prompt/schema.lua:
##########
@@ -0,0 +1,44 @@
+--
+-- Licensed to the Apache Software Foundation (ASF) under one or more
+-- contributor license agreements. See the NOTICE file distributed with
+-- this work for additional information regarding copyright ownership.
+-- The ASF licenses this file to You under the Apache License, Version 2.0
+-- (the "License"); you may not use this file except in compliance with
+-- the License. You may obtain a copy of the License at
+--
+-- http://www.apache.org/licenses/LICENSE-2.0
+--
+-- Unless required by applicable law or agreed to in writing, software
+-- distributed under the License is distributed on an "AS IS" BASIS,
+-- WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+-- See the License for the specific language governing permissions and
+-- limitations under the License.
+--
+
+local _M = {}
+
+
+_M.message = {
+ type = "object",
+ properties = {
+ role = {
+ type = "string",
+ enum = {"system", "user", "assistant"},
+ },
+ content = {
+ type = "string",
+ minLength = 1,
+ },
+ },
+ required = {"role", "content"},
+}
+
+
+_M.messages = {
+ type = "array",
+ minItems = 1,
Review Comment:
Sharing `minItems = 1` with the decorator's `prepend` and `append` tightens
the schema for configs already in etcd. `{"prepend": [], "append": [{...}]}` is
easy to get from generated config, and the `append` works fine today.
After an upgrade that route fails `check_route`, config_etcd logs `failed to
check item data` and skips it, so its traffic gets a 404. That's a lot worse
than the "ineffective config" the PR description calls it. I'd either keep
tolerating empty arrays or give this a migration path.
##########
apisix/plugins/ai-protocols/anthropic-messages.lua:
##########
@@ -345,31 +345,78 @@ function _M.get_messages(body)
end
+local function system_blocks(system)
+ local blocks = {}
+ if type(system) == "string" then
Review Comment:
An empty-string `system` also becomes `{type = "text", text = ""}` here.
`"system": ""` is fine as a string, but after a `prepend` with role system the
request becomes `[{text: "x"}, {type: "text", text: ""}]`.
From memory Anthropic rejects that with a 400 (`text content blocks must be
non-empty`), I haven't confirmed it against the live API. Skipping empty
strings here avoids the question either way.
##########
apisix/plugins/ai-prompt-template.lua:
##########
@@ -88,43 +72,63 @@ local templates_lrucache = core.lrucache.new({
ttl = 300, count = 256
})
-local templates_json_lrucache = core.lrucache.new({
- ttl = 300, count = 256
-})
-
function _M.check_schema(conf)
return core.schema.check(schema, conf)
end
-local function get_request_body_table(max_size)
- local body, err = core.request.get_body(max_size)
- if not body then
- return nil, { message = "could not get body: " .. (err or "request
body is empty") }
+local function find_template(conf, template_name)
+ for _, template in ipairs(conf.templates) do
+ if template.name == template_name then
+ return template.template
+ end
end
+ return nil
+end
- local body_tab, err = core.json.decode(body)
- if not body_tab then
- return nil, { message = "could not parse JSON request body: " .. (err
or "invalid JSON") }
+
+local function render_string(value, input)
+ local raw_template = string_gsub(value, "{{(.-)}}", "{*%1*}")
+ local ok, render = pcall(template.compile, raw_template)
+ if not ok then
+ return nil, "template compile: " .. render
end
- return body_tab
+ local output
+ ok, output = pcall(render, input)
Review Comment:
Not new in this PR, but worth fixing while this is being rewritten:
resty.template looks names up in the context first, so request fields can
shadow the globals the compiled template relies on, and falsy values fall
through to `template` and `_G`.
`{"template_name": "sql", "table": "users"}` with `Describe {{ table }}`
shadows `table.concat` and returns 503 with `attempt to call field 'concat' (a
nil value)`. `{"flag": false}` with `{{ flag }}` renders as an empty string.
`{{ type }}` with no `type` in the body resolves to `_G.type` and errors. All
reproduced with a `resty` script. Since this no longer reuses body-transformer,
plain variable path substitution may fit better than compiling and running Lua.
##########
apisix/plugins/ai-prompt/body.lua:
##########
@@ -0,0 +1,63 @@
+--
+-- Licensed to the Apache Software Foundation (ASF) under one or more
+-- contributor license agreements. See the NOTICE file distributed with
+-- this work for additional information regarding copyright ownership.
+-- The ASF licenses this file to You under the Apache License, Version 2.0
+-- (the "License"); you may not use this file except in compliance with
+-- the License. You may obtain a copy of the License at
+--
+-- http://www.apache.org/licenses/LICENSE-2.0
+--
+-- Unless required by applicable law or agreed to in writing, software
+-- distributed under the License is distributed on an "AS IS" BASIS,
+-- WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+-- See the License for the specific language governing permissions and
+-- limitations under the License.
+--
+
+local core = require("apisix.core")
+local ngx = ngx
+local pairs = pairs
+local type = type
+
+local _M = {}
+
+
+function _M.read(max_size)
+ local body, err = core.request.get_json_request_body_table(max_size)
Review Comment:
membphis already covered the size limit bypass on the cache-hit path.
There's a second problem from the same cache: the type check.
Say two routes share a URI, one with `vars` on `post_arg.model` and a
fallback carrying the prompt plugin. A JSON body sent as
`application/x-www-form-urlencoded` (what `curl -d` does by default) gets
parsed and cached as form while the first route's vars are evaluated. It then
matches the fallback, and `body.read` returns 400 `request body type mismatch`,
where the old `get_body` plus decode parsed the JSON fine.
##########
apisix/plugins/ai-prompt-template.lua:
##########
@@ -88,43 +72,63 @@ local templates_lrucache = core.lrucache.new({
ttl = 300, count = 256
})
-local templates_json_lrucache = core.lrucache.new({
- ttl = 300, count = 256
-})
-
function _M.check_schema(conf)
return core.schema.check(schema, conf)
end
-local function get_request_body_table(max_size)
- local body, err = core.request.get_body(max_size)
- if not body then
- return nil, { message = "could not get body: " .. (err or "request
body is empty") }
+local function find_template(conf, template_name)
+ for _, template in ipairs(conf.templates) do
+ if template.name == template_name then
+ return template.template
+ end
end
+ return nil
+end
- local body_tab, err = core.json.decode(body)
- if not body_tab then
- return nil, { message = "could not parse JSON request body: " .. (err
or "invalid JSON") }
+
+local function render_string(value, input)
+ local raw_template = string_gsub(value, "{{(.-)}}", "{*%1*}")
+ local ok, render = pcall(template.compile, raw_template)
Review Comment:
`template.compile` without `plain = true` makes resty.template try each
string as a file path first, and if it can read one it uses the file's contents
as the template.
That couldn't happen in the old flow since it compiled a whole JSON string
starting with `{`, but `model` and `content` are compiled individually now. A
literal `../../../../../../../../../etc/hostname` rendered that file's contents
when I tried it, so a configured string that happens to match an existing path
sends the file to the LLM. It's also an extra blocking `io.open` per new string
per worker. `pcall(template.compile, raw_template, nil, true)` fixes both.
##########
apisix/plugins/ai-prompt-template.lua:
##########
@@ -88,43 +72,63 @@ local templates_lrucache = core.lrucache.new({
ttl = 300, count = 256
})
-local templates_json_lrucache = core.lrucache.new({
- ttl = 300, count = 256
-})
-
function _M.check_schema(conf)
return core.schema.check(schema, conf)
end
-local function get_request_body_table(max_size)
- local body, err = core.request.get_body(max_size)
- if not body then
- return nil, { message = "could not get body: " .. (err or "request
body is empty") }
+local function find_template(conf, template_name)
+ for _, template in ipairs(conf.templates) do
+ if template.name == template_name then
+ return template.template
+ end
end
+ return nil
+end
- local body_tab, err = core.json.decode(body)
- if not body_tab then
- return nil, { message = "could not parse JSON request body: " .. (err
or "invalid JSON") }
+
+local function render_string(value, input)
+ local raw_template = string_gsub(value, "{{(.-)}}", "{*%1*}")
+ local ok, render = pcall(template.compile, raw_template)
+ if not ok then
+ return nil, "template compile: " .. render
end
- return body_tab
+ local output
+ ok, output = pcall(render, input)
+ if not ok then
+ return nil, "template rendering: " .. output
+ end
+ return output
end
-local function find_template(conf, template_name)
- for _, template in ipairs(conf.templates) do
- if template.name == template_name then
- return template.template
+local function render_template(configured, input)
+ local rendered = core.table.deepcopy(configured)
+ local err
+
+ if rendered.model then
+ rendered.model, err = render_string(rendered.model, input)
+ if not rendered.model then
+ return nil, err
end
end
- return nil
+
+ for _, message in ipairs(rendered.messages or {}) do
+ message.content, err = render_string(message.content, input)
+ if not message.content then
+ return nil, err
+ end
+ end
+
+ return rendered
end
+
function _M.rewrite(conf, ctx)
- local body_tab, err = get_request_body_table(conf.max_req_body_size)
+ local body_tab, status, err = body.read(conf.max_req_body_size)
if not body_tab then
- return 400, err
+ return status, err
end
local template_name = body_tab.template_name
Review Comment:
Also pre-existing, but the PR adds a strict body helper and this still gets
through: `body.read` doesn't check that the top level is an object and
`template_name` isn't type checked. A POST body of `123` or `true` throws here
and comes back as a 500 with an error log, and `{"template_name": {}}` throws
at the string concat on line 140.
The decorator has the same problem on `/v1/messages`. `{"messages": "x"}`
throws at the `ipairs` on line 392 of `anthropic-messages.lua` because
`matches` only checks the body is a table, while the Bedrock path guards with
`type(body.messages) == "table"`. These should all be 4xx.
##########
apisix/plugins/ai-prompt-template.lua:
##########
@@ -88,43 +72,63 @@ local templates_lrucache = core.lrucache.new({
ttl = 300, count = 256
})
-local templates_json_lrucache = core.lrucache.new({
- ttl = 300, count = 256
-})
-
function _M.check_schema(conf)
return core.schema.check(schema, conf)
end
-local function get_request_body_table(max_size)
- local body, err = core.request.get_body(max_size)
- if not body then
- return nil, { message = "could not get body: " .. (err or "request
body is empty") }
+local function find_template(conf, template_name)
+ for _, template in ipairs(conf.templates) do
+ if template.name == template_name then
+ return template.template
+ end
end
+ return nil
+end
- local body_tab, err = core.json.decode(body)
- if not body_tab then
- return nil, { message = "could not parse JSON request body: " .. (err
or "invalid JSON") }
+
+local function render_string(value, input)
+ local raw_template = string_gsub(value, "{{(.-)}}", "{*%1*}")
Review Comment:
Rewriting `{{ }}` to `{* *}` with `gsub` breaks resty.template's escape and
raw semantics. `literal \{{ x }} end` now outputs `literal {* x *} end` and
`{-raw-}{{ x }}{-raw-}` outputs `{* x *}`, where both used to give a literal
`{{ x }}`.
A Lua table constructor like `{{a=1}}` inside `{% %}` gets rewritten too and
fails to compile. And because each string is compiled on its own now, `\r\n` in
config gets normalized to `\n` by the Lua long string, which the old flow kept
through JSON escaping. All of these reproduced with a `resty` script.
##########
apisix/plugins/ai-protocols/anthropic-messages.lua:
##########
@@ -345,31 +345,78 @@ function _M.get_messages(body)
end
+local function system_blocks(system)
+ local blocks = {}
+ if type(system) == "string" then
+ core.table.insert(blocks, {type = "text", text = system})
+ elseif type(system) == "table" then
+ for _, block in ipairs(system) do
+ core.table.insert(blocks, block)
+ end
+ end
+ return blocks
+end
+
+
+local function configured_messages(msgs)
+ local systems = {}
+ local messages = {}
+ for _, msg in ipairs(msgs or {}) do
+ if msg.role == "system" then
+ core.table.insert(systems, {type = "text", text = msg.content})
Review Comment:
Decorated content is now its own system block, and `convert_system` in
`converters/anthropic-messages-to-openai-chat.lua:362` joins blocks with
`table.concat(parts, "")`. That file isn't in this diff, but this change is
what makes it matter.
With prepend `Respond in French`, append `Be brief` and a client system `You
are a tutor` going to an OpenAI upstream, the system message becomes `Respond
in FrenchYou are a tutorBe brief`. They used to be separate messages. Joining
with a newline there would fix it.
##########
t/plugin/ai-prompt-decorator.t:
##########
@@ -78,7 +78,265 @@ passed
-=== TEST 2: test prepend
+=== TEST 2: reject empty and non-object prompt arrays
+--- config
+ location /t {
+ content_by_lua_block {
+ local t = require("lib.test_admin").test
+ local empty_code = t('/apisix/admin/routes/18',
+ ngx.HTTP_PUT,
+ [[{
+ "uri": "/invalid-empty",
+ "plugins": {"ai-prompt-decorator": {"prepend": []}}
+ }]]
+ )
+ local item_code = t('/apisix/admin/routes/19',
+ ngx.HTTP_PUT,
+ [[{
+ "uri": "/invalid-item",
+ "plugins": {"ai-prompt-decorator": {"append": ["bad"]}}
+ }]]
+ )
+ ngx.say("empty=", empty_code, " item=", item_code)
+ }
+ }
+--- response_body
+empty=400 item=400
+
+
+
+=== TEST 3: configure protocol-aware decoration
+--- config
+ location /t {
+ content_by_lua_block {
+ local t = require("lib.test_admin").test
+ local code, body = t('/apisix/admin/routes/20',
+ ngx.HTTP_PUT,
+ [[{
+ "uris": ["/v1/messages", "/model/test/converse",
"/v1/responses"],
+ "upstream": {
+ "type": "roundrobin",
+ "nodes": {"127.0.0.1:1980": 1}
+ },
+ "plugins": {
+ "ai-prompt-decorator": {
+ "prepend": [
+ {"role": "system", "content": "first system"},
+ {"role": "assistant", "content": "first
assistant"}
+ ],
+ "append": [
+ {"role": "assistant", "content": "last
assistant"},
+ {"role": "system", "content": "last system"}
+ ]
+ },
+ "proxy-rewrite": {
+ "uri": "/echo"
+ }
+ }
+ }]]
+ )
+ if code >= 300 then
+ ngx.status = code
+ end
+ ngx.say(body)
+ }
+ }
+--- response_body
+passed
+
+
+
+=== TEST 4: decorate Anthropic system and conversation fields
+--- config
+ location /t {
+ content_by_lua_block {
+ local t = require("lib.test_admin").test
+ ngx.sleep(1)
+ local code, body = t('/v1/messages',
+ ngx.HTTP_POST,
+ [[{
+ "model": "claude-test",
+ "system": "existing system",
+ "messages": [{"role": "user", "content": "question"}]
+ }]],
+ [[{
+ "model": "claude-test",
+ "system": [
+ {"type": "text", "text": "first system"},
+ {"type": "text", "text": "existing system"},
+ {"type": "text", "text": "last system"}
+ ],
+ "messages": [
+ {"role": "assistant", "content": "first assistant"},
+ {"role": "user", "content": "question"},
+ {"role": "assistant", "content": "last assistant"}
+ ]
+ }]]
+ )
+ if code >= 300 then
+ ngx.status = code
+ ngx.say(body)
+ return
+ end
+ ngx.say("passed")
+ }
+ }
+--- response_body
+passed
+
+
+
+=== TEST 5: decorate Bedrock system and conversation fields
+--- config
+ location /t {
+ content_by_lua_block {
+ local t = require("lib.test_admin").test
+ local code, body = t('/model/test/converse',
+ ngx.HTTP_POST,
+ [[{
+ "system": [{"text": "existing system"}],
+ "messages": [{"role": "user", "content": [{"text":
"question"}]}]
+ }]],
+ [[{
+ "system": [
+ {"text": "first system"},
+ {"text": "existing system"},
+ {"text": "last system"}
+ ],
+ "messages": [
+ {"role": "assistant", "content": [{"text": "first
assistant"}]},
+ {"role": "user", "content": [{"text": "question"}]},
+ {"role": "assistant", "content": [{"text": "last
assistant"}]}
+ ]
+ }]]
+ )
+ if code >= 300 then
+ ngx.status = code
+ ngx.say(body)
+ return
+ end
+ ngx.say("passed")
+ }
+ }
+--- response_body
+passed
+
+
+
+=== TEST 6: preserve roles when decorating Responses input
+--- config
+ location /t {
+ content_by_lua_block {
+ local t = require("lib.test_admin").test
+ local code, body = t('/v1/responses',
+ ngx.HTTP_POST,
+ [[{
+ "model": "gpt-test",
+ "instructions": "existing system",
+ "input": "question"
+ }]],
+ [[{
+ "model": "gpt-test",
+ "instructions": "first system\nexisting system\nlast
system",
+ "input": [
+ {"type": "message", "role": "assistant", "content":
"first assistant"},
+ {"type": "message", "role": "user", "content":
"question"},
+ {"type": "message", "role": "assistant", "content":
"last assistant"}
+ ]
+ }]]
+ )
+ if code >= 300 then
+ ngx.status = code
+ ngx.say(body)
+ return
+ end
+ ngx.say("passed")
+ }
+ }
+--- response_body
+passed
+
+
+
+=== TEST 7: fail_mode controls unsupported request handling
+--- config
+ location /t {
+ content_by_lua_block {
+ local t = require("lib.test_admin").test
+ local code, body = t('/apisix/admin/routes/23',
Review Comment:
A few test issues. TEST 7 and TEST 8 here and TEST 5 in
`ai-prompt-template.t` send a request right after PUTting a new route with no
wait, and `t()` doesn't sleep, so slow config sync gives a 404 instead of the
expected 400 or 413. TEST 4 here uses `ngx.sleep(1)` and TEST 4 in the template
test uses `ngx.sleep(0.2)`.
Route 20 (line 113) is never cleaned up and matches `/v1/responses` with the
same priority as route 1 in TEST 16 and TEST 22. From reading the code TEST 17
only passes because route 1 loads first in etcd key order, I haven't run it.
Also the default `fail_mode=skip` and `warn` have no tests at all, only
`error` does.
##########
apisix/plugins/ai-prompt-template.lua:
##########
@@ -136,17 +140,13 @@ function _M.rewrite(conf, ctx)
return 400, { message = "template: " .. template_name .. " not
configured." }
end
- local template_json = templates_json_lrucache(template, template,
core.json.encode, template)
- core.log.info("sending template to body_transformer, name: ",
template_name)
- return body_transformer.rewrite(
- {
- request = {
- template = template_json,
- input_format = "json"
- }
- },
- ctx
- )
+ local rendered, render_err = render_template(template, body_tab)
+ if not rendered then
+ return 503, {message = render_err}
Review Comment:
The old flow logged render failures with `core.log.error` in
body-transformer (`body-transformer.lua:218`), and that's gone here, so the
error only goes back to the client.
A template with `{{ nested.name }}` and a request without `nested` returns
503 with no gateway log, so ops see a 5xx with nothing to trace, when it's
really a client error that should be a 4xx. I get not logging the rendered
body, but the error text doesn't contain it, so this log line can stay. The
response also exposes a chunk fragment like `[string "context=... or {}..."]:5`.
##########
apisix/plugins/ai-prompt-template.lua:
##########
@@ -136,17 +140,13 @@ function _M.rewrite(conf, ctx)
return 400, { message = "template: " .. template_name .. " not
configured." }
end
- local template_json = templates_json_lrucache(template, template,
core.json.encode, template)
- core.log.info("sending template to body_transformer, name: ",
template_name)
- return body_transformer.rewrite(
- {
- request = {
- template = template_json,
- input_format = "json"
- }
- },
- ctx
- )
+ local rendered, render_err = render_template(template, body_tab)
+ if not rendered then
+ return 503, {message = render_err}
+ end
+
+ body.replace(body_tab, rendered)
Review Comment:
The PR is about aligning protocols, but ai-prompt-template still replaces
the whole body with a Chat-shaped template whatever the protocol is.
With a `{role: system}` message on `/v1/messages`, the system message stays
in `messages` and Anthropic rejects it, which is the same bug this PR fixes in
the decorator. On `/model/x/converse`, `content` is a string rather than
`[{text}]`, which Bedrock rejects. On `/v1/responses` the body has no `input`,
so detection falls back to openai-chat. I think this should write through the
module `protocols.detect` returns.
--
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]
