fix(ai-content-moderation): stop re-counting converted chunks in realtime batches - #13765
Open
shreemaan-abhishek wants to merge 1 commit into
Open
Conversation
…time batches The realtime batch accumulator read ctx.llm_response_contents_in_chunk, which base.lua refills once per upstream chunk. With a protocol converter active one upstream chunk is dispatched as several converted chunks, so lua_body_filter ran once per converted chunk and appended the same texts every time: the moderation batch held the text N times over, stream_check_cache_size tripped N times sooner, and request volume scaled with the converter's fan-out. base.lua now bumps a per-upstream-chunk counter where it resets the texts, and each plugin takes them only on its first run for that counter.
dosubot
Bot
added
size:L
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
In
stream_check_mode = realtime,ai-aws-content-moderationandai-aliyun-content-moderationbuild their moderation batch fromctx.llm_response_contents_in_chunk, whichai-providers/base.luarefills onceper upstream chunk. When a protocol converter is active (e.g. an Anthropic
client over an OpenAI upstream), that upstream chunk is dispatched to the client
as several converted chunks and
lua_body_filterruns once per convertedchunk, appending the same texts every time. Consequences:
stream_check_cache_sizetripped N times sooner than configured,and third-party cost on cross-protocol routes.
Measured with the fixture used by the new test: one upstream chunk fans out into
7 Anthropic events and produced 7 identical Comprehend calls instead of 1.
Without a converter (1 upstream chunk -> 1 downstream chunk) nothing was
duplicated, which is why this went unnoticed. Moderation coverage was never
affected - the duplicated batch still contained everything.
Fix:
base.luabumps a per-upstream-chunk counter(
ctx.llm_response_chunk_seq) where it already resetsctx.llm_response_contents_in_chunk; each plugin records the sequence it lastconsumed and takes the chunk's text only on its first run. With the counter
absent (callers other than the streaming loop) behaviour is unchanged.
Accumulating the
bodyargument instead was considered and rejected:bodyisthe raw SSE frame of the converted chunk, so the batch would carry the JSON
envelope rather than the response text, and
stream_check_cache_sizewould endup measuring envelope bytes.
Tests added:
t/plugin/ai-aws-content-moderation.tTEST 33 (repeated filter runs for oneupstream chunk take its text once) and TEST 34/35 (end-to-end Anthropic
client over OpenAI upstream: Comprehend is called exactly once; fails on
master with 7 identical calls).
t/plugin/ai-aliyun-content-moderation.tTEST 74 and TEST 75/76, same shape.Behaviour and configuration are unchanged, so no documentation update is needed.
Follow-up to #13735, raised by @AlinsRan in review
(
discussion_r3672582793).Which issue(s) this PR fixes:
No separate issue - reported in review of #13735.
Checklist