fix(ai-aws-content-moderation): respect Comprehend's segment limits and reuse the client - #13767
Open
shreemaan-abhishek wants to merge 2 commits into
Open
Conversation
…nd reuse the client DetectToxicContent takes at most 10 text segments per call, each at most 1 KB. The plugin sent the whole prompt or the whole completion as a single segment, so any content over 1 KB came back as a 400: the request side failed closed with a 500, and the response side logged the error and let the content through, which silently disabled check_response on normal-length responses. On that path the verdict was never computed either, yet final_packet still annotated the stream with risk_level, reporting the request-side (or stale) verdict as if the response had been scored. Content is now split on UTF-8 character boundaries into segments bounded by the new request_check_length_limit / response_check_length_limit (default 1000 bytes, capped at Comprehend's 1 KB), and those segments are batched up to the per-call limits so a long body costs as few round trips as possible. When Comprehend fails, the stream is left without a risk_level rather than carrying a verdict that was never computed. The Comprehend client is also built once per request instead of once per batch, and timeout / keepalive / keepalive_timeout are exposed so the connection is reused across calls instead of a fresh TLS handshake each time.
dosubot
Bot
added
the
size:XL
- pass ssl_verify and the endpoint through the Comprehend service config instead of mutating the shared aws instance - require at least 4 bytes per segment, the width of the widest UTF-8 character, so a segment can always hold one whole character - have the Comprehend mock reject segments that are not valid UTF-8, and cover a multibyte body that spans several segments
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
AWS Comprehend's
DetectToxicContenttakes at most 10 text segments per call, each at most 1 KB, with the list capped at 10 KB. Over that it answersTextSizeLimitExceededException(400).ai-aws-content-moderationsent the whole prompt, or the whole completion, as a single segment.Consequences on content longer than ~1 KB, which is most real traffic:
500, so long prompts are unusable.check_responsesilently does nothing.final_packetstill annotates the SSE stream withrisk_level, reporting the request-side (or stale) verdict as if the response had been scored.realtimemostly stays under the cap with the default batch size, but an interval-triggered batch, or a single large upstream SSE event, pushes one batch over 1 KB and that batch leaks.Changes:
request_check_length_limit/response_check_length_limit(default1000bytes, capped at Comprehend's 1 KB), and the segments are batched up to the per-call limits, so a long body costs as few round trips as possible.risk_levelinstead of carrying a verdict that was never computed.realtimethat is a fresh client per batch. They are now built once per request, andtimeout/keepalive/keepalive_timeoutare exposed so the connection is reused instead of a new TLS handshake per batch.The Comprehend mock in the tests now enforces the same input limits as the real service and answers with one result per segment, so content that is sent unsplit fails the tests.
Which issue(s) this PR fixes:
N/A
Checklist