AlinsRan commented on code in PR #13735:
URL: https://github.com/apache/apisix/pull/13735#discussion_r3672582798
##########
apisix/plugins/ai-aws-content-moderation.lua:
##########
@@ -257,4 +357,62 @@ function _M.access(conf, ctx)
end
end
+
+function _M.lua_body_filter(conf, ctx, headers, body)
+ if not conf.check_response then
+ core.log.info("skip response check for this request")
+ return
+ end
+
+ if ngx.status >= 400 then
+ core.log.info("skip response check because upstream returned error
status: ", ngx.status)
+ return
+ end
+
+ local request_type = ctx.var.request_type
+
+ -- ai-proxy hands us the fully assembled completion, so one check covers
it.
+ if request_type == "ai_chat" then
+ return moderate_response(ctx, conf, ctx.var.llm_response_text)
Review Comment:
On this path `moderate_response` swallows a Comprehend failure and lets the
completion through. For streaming that is forced — bytes are already on the
wire — but here the body has not been sent yet, so fail-closed is still
reachable.
As written, `fail_mode: deny` applies to the request direction only, while
the schema presents it as a plugin-wide setting. Either honour `fail_mode` for
buffered responses, or document that it is request-side only; right now the
option reads stricter than it behaves.
##########
apisix/plugins/ai-aws-content-moderation.lua:
##########
@@ -257,4 +357,62 @@ function _M.access(conf, ctx)
end
end
+
+function _M.lua_body_filter(conf, ctx, headers, body)
+ if not conf.check_response then
+ core.log.info("skip response check for this request")
+ return
+ end
+
+ if ngx.status >= 400 then
+ core.log.info("skip response check because upstream returned error
status: ", ngx.status)
+ return
+ end
+
+ local request_type = ctx.var.request_type
+
+ -- ai-proxy hands us the fully assembled completion, so one check covers
it.
+ if request_type == "ai_chat" then
+ return moderate_response(ctx, conf, ctx.var.llm_response_text)
+ end
+
+ if request_type ~= "ai_stream" then
+ return
+ end
+
+ if conf.stream_check_mode == "final_packet" then
+ -- llm_response_text only appears once the stream is assembled, so
+ -- earlier chunks pass through untouched.
+ if not ctx.var.llm_response_text then
+ return
+ end
+ if not ctx.aws_cm_response_moderated then
+ ctx.aws_cm_response_moderated = true
+ moderate_response(ctx, conf, ctx.var.llm_response_text)
+ end
+ return nil, annotate_stream(ctx, body)
+ end
+
+ -- realtime: moderate batches as they arrive so a hit can cut the stream
off
+ ctx.aws_cm_cache = ctx.aws_cm_cache or ""
+ ctx.aws_cm_cache = ctx.aws_cm_cache
+ .. table.concat(ctx.llm_response_contents_in_chunk or
{}, "")
Review Comment:
`ctx.llm_response_contents_in_chunk` is reset once per *upstream* chunk in
`ai-providers/base.lua`, but when a converter is active that same loop calls
`lua_response_filter` once per *converted* chunk. For an upstream chunk that
yields N converted chunks, this line appends the identical text to
`aws_cm_cache` N times: the batch sent to Comprehend contains duplicated
content, and `stream_check_cache_size` trips N times sooner.
Nothing is missed, but the request volume scales with the converter's
fan-out, which shows up as latency and billing on any cross-protocol route
(e.g. Anthropic client over an OpenAI upstream).
`ai-aliyun-content-moderation` has the same shape, so this can be a separate
fix — but it should be a conscious one rather than inherited.
--
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]
