fix: split image moderation requests#1908
Conversation
WalkthroughSplits content-filter input generation into separate text and per-image builders, runs one moderation request per built input concurrently, aggregates successful upstream responses into a unified check result, updates logging and API signatures, and adds tests and a two-image edit helper script. Changes
Sequence DiagramsequenceDiagram
participant Client
participant TextBuilder as buildOpenAIContentFilterTextInput
participant ImageBuilder as buildOpenAIContentFilterImageInputs
participant Runner as runOpenAIContentFilterRequest
participant OpenAI as OpenAI_Moderation_API
participant Aggregator
Client->>TextBuilder: messages
TextBuilder-->>Client: textInput
Client->>ImageBuilder: messages
ImageBuilder-->>Client: imageInputs[]
par concurrent moderation requests
alt text request exists
Client->>Runner: textRequest
Runner->>OpenAI: POST moderation (text)
OpenAI-->>Runner: moderation response
Runner-->>Client: {success,response}
end
loop per imageInput
Client->>Runner: imageRequest(i)
Runner->>OpenAI: POST moderation (image_url)
OpenAI-->>Runner: moderation response
Runner-->>Client: {success,response}
end
end
Client->>Aggregator: successful responses[]
Aggregator-->>Client: aggregatedResult (flagged, model, upstreamRequestId, results[])
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
This PR updates the gateway’s OpenAI moderation integration to split moderation into a single text request plus one request per input image, execute image moderation in parallel, and aggregate the results into one final content-filter decision while preserving fail-open behavior on moderation failures.
Changes:
- Split moderation input building into separate text and per-image inputs.
- Add a per-request runner and aggregate multiple moderation responses into a single decision.
- Update/expand unit tests to validate the new request splitting behavior.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| apps/gateway/src/chat/tools/openai-content-filter.ts | Refactors OpenAI moderation to run one text request plus N image requests in parallel and aggregates results/logging. |
| apps/gateway/src/chat/tools/openai-content-filter.spec.ts | Updates tests for the split text/image input builders and adds a test asserting one request per image. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| upstreamText = await upstreamResponse.text(); | ||
| } catch (error) { | ||
| if (requestSignal?.aborted || isCancellationError(error)) { | ||
| if (signal.aborted || isCancellationError(error)) { |
There was a problem hiding this comment.
runOpenAIContentFilterRequest rethrows any fetch error when signal.aborted is true. Because signal includes AbortSignal.timeout(...), this makes timeout errors behave like cancellations and can cause Promise.all(...) to reject, dropping any successful (and potentially flagged) moderation results and failing open for the whole check. To keep the intended fail-open behavior, only rethrow user aborts (AbortError / requestSignal abort), and treat TimeoutError as a normal moderation failure (log + return success:false).
| if (signal.aborted || isCancellationError(error)) { | |
| if (isCancellationError(error)) { |
| const moderationResults = await Promise.all( | ||
| moderationRequests.map((request) => | ||
| runOpenAIContentFilterRequest( | ||
| request, | ||
| context, | ||
| providerEnv.token, | ||
| signal, | ||
| ), | ||
| ), | ||
| ); | ||
|
|
There was a problem hiding this comment.
Promise.all(moderationRequests.map(...)) fires one moderation request per image with unbounded parallelism. A prompt with many images could create a large burst of outbound requests, increasing rate-limit risk and tying up gateway resources. Consider adding a concurrency limit (e.g., a small pool) and/or enforcing a maximum number of images to moderate per request (with clear logging when capped).
| const moderationResults = await Promise.all( | |
| moderationRequests.map((request) => | |
| runOpenAIContentFilterRequest( | |
| request, | |
| context, | |
| providerEnv.token, | |
| signal, | |
| ), | |
| ), | |
| ); | |
| const OPENAI_MODERATION_CONCURRENCY_LIMIT = 5; | |
| const moderationResults = new Array< | |
| Awaited<ReturnType<typeof runOpenAIContentFilterRequest>> | |
| >(moderationRequests.length); | |
| let nextIndex = 0; | |
| const workerCount = Math.min( | |
| OPENAI_MODERATION_CONCURRENCY_LIMIT, | |
| moderationRequests.length, | |
| ); | |
| const workers = Array.from({ length: workerCount }, async () => { | |
| for (;;) { | |
| const currentIndex = nextIndex++; | |
| if (currentIndex >= moderationRequests.length) { | |
| break; | |
| } | |
| const request = moderationRequests[currentIndex]; | |
| moderationResults[currentIndex] = | |
| await runOpenAIContentFilterRequest( | |
| request, | |
| context, | |
| providerEnv.token, | |
| signal, | |
| ); | |
| } | |
| }); | |
| await Promise.all(workers); |
| ): Promise<OpenAIContentFilterCheckResult> { | ||
| const startTime = Date.now(); | ||
| const moderationRequests = buildOpenAIContentFilterRequests(messages); | ||
| const imageInputs = buildOpenAIContentFilterImageInputs(messages); |
There was a problem hiding this comment.
buildOpenAIContentFilterImageInputs(messages) is called twice (once indirectly via buildOpenAIContentFilterRequests and again for imageInputs). This duplicates work and can lead to subtle inconsistencies if the extraction logic changes. Consider computing textInput/imageInputs once and reusing them for both request construction and logging.
| const imageInputs = buildOpenAIContentFilterImageInputs(messages); |
| it("submits one moderation request for text and one per image", async () => { | ||
| process.env.LLM_OPENAI_API_KEY = "sk-openai-test"; | ||
| const requestBodies: Array<{ model: string; input: unknown }> = []; | ||
|
|
||
| vi.spyOn(globalThis, "fetch").mockImplementation(async (_input, init) => { | ||
| const body = JSON.parse(String(init?.body ?? "{}")) as { | ||
| model: string; | ||
| input: string | Array<{ image_url?: { url: string } }>; | ||
| }; | ||
| requestBodies.push(body); | ||
|
|
||
| if (typeof body.input === "string") { | ||
| return new Response( | ||
| JSON.stringify({ | ||
| id: "modr-text", | ||
| model: "omni-moderation-latest", | ||
| results: [{ flagged: false, categories: {} }], | ||
| }), | ||
| { | ||
| status: 200, | ||
| headers: { | ||
| "Content-Type": "application/json", | ||
| "x-request-id": "req-text", | ||
| }, | ||
| }, | ||
| ); | ||
| } | ||
|
|
||
| const imageUrl = body.input[0]?.image_url?.url; | ||
| return new Response( | ||
| JSON.stringify({ | ||
| id: `modr-${imageUrl}`, | ||
| model: "omni-moderation-latest", | ||
| results: [ | ||
| { | ||
| flagged: imageUrl === "https://example.com/dog.png", | ||
| categories: { | ||
| violence: imageUrl === "https://example.com/dog.png", | ||
| }, | ||
| }, | ||
| ], | ||
| }), | ||
| { | ||
| status: 200, | ||
| headers: { | ||
| "Content-Type": "application/json", | ||
| "x-request-id": | ||
| imageUrl === "https://example.com/dog.png" | ||
| ? "req-dog" | ||
| : "req-cat", | ||
| }, | ||
| }, | ||
| ); | ||
| }); | ||
|
|
||
| const result = await checkOpenAIContentFilter( | ||
| [ | ||
| { | ||
| role: "user", | ||
| content: [ | ||
| { | ||
| type: "text", | ||
| text: "Please inspect these images.", | ||
| }, | ||
| { | ||
| type: "image_url", | ||
| image_url: { | ||
| url: "https://example.com/cat.png", | ||
| }, | ||
| }, | ||
| { | ||
| type: "image_url", | ||
| image_url: { | ||
| url: "https://example.com/dog.png", | ||
| }, | ||
| }, | ||
| ], | ||
| }, | ||
| ], | ||
| { | ||
| requestId: "request-id", | ||
| organizationId: "org-id", | ||
| projectId: "project-id", | ||
| apiKeyId: "api-key-id", | ||
| }, | ||
| ); | ||
|
|
||
| expect(requestBodies).toEqual([ | ||
| { | ||
| model: "omni-moderation-latest", | ||
| input: "user: Please inspect these images.", | ||
| }, | ||
| { | ||
| model: "omni-moderation-latest", | ||
| input: [ | ||
| { | ||
| type: "image_url", | ||
| image_url: { | ||
| url: "https://example.com/cat.png", | ||
| }, | ||
| }, | ||
| ], | ||
| }, | ||
| { | ||
| model: "omni-moderation-latest", | ||
| input: [ | ||
| { | ||
| type: "image_url", | ||
| image_url: { | ||
| url: "https://example.com/dog.png", | ||
| }, | ||
| }, | ||
| ], | ||
| }, | ||
| ]); | ||
| expect(result.flagged).toBe(true); | ||
| expect(result.model).toBe("omni-moderation-latest"); | ||
| expect(result.upstreamRequestId).toBe("req-dog"); | ||
| expect(result.results).toHaveLength(3); | ||
| expect(result.results.some((entry) => entry.flagged)).toBe(true); | ||
| }); |
There was a problem hiding this comment.
Test coverage doesn’t currently exercise the key guarantee described in the PR summary: if some moderation calls fail (e.g., 500/invalid JSON/timeout) but at least one successful result is flagged, the aggregated decision should still block. Add a spec case where one image moderation request fails while another returns flagged: true, and assert checkOpenAIContentFilter returns flagged: true and preserves the successful flagged categories.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 65b4e56bed
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (signal.aborted || isCancellationError(error)) { | ||
| throw error; |
There was a problem hiding this comment.
When any per-image moderation fetch hits the 60s timeout, signal.aborted becomes true and this branch rethrows, which causes Promise.all(...) in checkOpenAIContentFilter to reject and fall into the top-level fail-open return. In a mixed outcome (e.g., one image already flagged true, another times out), the flagged success is dropped and the gateway returns flagged: false, allowing content that should be blocked. This regression is introduced by splitting moderation into multiple requests and treating timeout aborts as hard failures.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
apps/gateway/src/chat/tools/openai-content-filter.ts (1)
159-180: Reuse the extracted image inputs.
buildOpenAIContentFilterImageInputs()is called once insidebuildOpenAIContentFilterRequests()and again immediately after for logging. For base64 images that means rebuilding the fulldata:URLs twice on the hot path. Consider returning the image count/inputs from the request builder so this work only happens once.Also applies to: 363-365
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/gateway/src/chat/tools/openai-content-filter.ts` around lines 159 - 180, buildOpenAIContentFilterRequests currently calls buildOpenAIContentFilterImageInputs() and the caller re-calls it again for logging, causing expensive re-construction of base64 data URLs; change the flow so buildOpenAIContentFilterRequests returns (or exposes) the imageInputs/count alongside the requests (e.g., return an object { requests, imageInputs } or include imageCount) and reuse that returned imageInputs/count for logging instead of calling buildOpenAIContentFilterImageInputs() a second time; update callers that currently call buildOpenAIContentFilterImageInputs() after buildOpenAIContentFilterRequests() (and the usage at the later location mentioned) to use the returned imageInputs/count to eliminate duplicate work.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/gateway/src/chat/tools/openai-content-filter.ts`:
- Around line 284-298: The current error path in runOpenAIContentFilterRequest
(where signal.aborted/isCancellationError leads to throwing and later logging
via logModerationError and returning createFailedOpenAIContentFilterResult)
abandons the whole batch on a timeout/cancel and can drop earlier flagged
responses; change this flow to collect/merge settled sibling results before
returning: catch timeout/cancel errors but do not immediately fail-open—inspect
any previously fulfilled moderation results from the aggregated promises, return
a flagged result if any successful response is flagged, otherwise log the
timeout via logModerationError and return the failed result from
createFailedOpenAIContentFilterResult; apply the same aggregation fix to the
analogous branches around the other occurrences (lines referenced in review: the
blocks around 393-402 and 436-449) and add a regression test for the "flagged
success + timed-out sibling" scenario.
---
Nitpick comments:
In `@apps/gateway/src/chat/tools/openai-content-filter.ts`:
- Around line 159-180: buildOpenAIContentFilterRequests currently calls
buildOpenAIContentFilterImageInputs() and the caller re-calls it again for
logging, causing expensive re-construction of base64 data URLs; change the flow
so buildOpenAIContentFilterRequests returns (or exposes) the imageInputs/count
alongside the requests (e.g., return an object { requests, imageInputs } or
include imageCount) and reuse that returned imageInputs/count for logging
instead of calling buildOpenAIContentFilterImageInputs() a second time; update
callers that currently call buildOpenAIContentFilterImageInputs() after
buildOpenAIContentFilterRequests() (and the usage at the later location
mentioned) to use the returned imageInputs/count to eliminate duplicate work.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: f4c8a815-009d-428c-a0b3-3d460f16a665
📒 Files selected for processing (2)
apps/gateway/src/chat/tools/openai-content-filter.spec.tsapps/gateway/src/chat/tools/openai-content-filter.ts
steebchen
force-pushed
the
split-image-moderation
branch
from
65b4e56 to
a22bf99
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a22bf99a90
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (signal.aborted || isCancellationError(error)) { | ||
| throw error; |
There was a problem hiding this comment.
This branch now throws whenever the shared moderation signal is aborted, which includes the 60s timeout used for all parallel image/text requests. In a mixed outcome (e.g., one request already returned flagged: true and another times out), Promise.all(...) rejects and checkOpenAIContentFilter falls back to fail-open, dropping the successful flagged result and returning flagged: false. That allows content that should be blocked whenever any single moderation call times out.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
♻️ Duplicate comments (1)
apps/gateway/src/chat/tools/openai-content-filter.ts (1)
393-402:⚠️ Potential issue | 🔴 CriticalPartial timeout can drop already-fulfilled flagged results.
At Line 393,
Promise.all(...)rejects on the first thrown sibling request, so fulfilled results from other requests are not aggregated. Combined with Line 284 throwing on aborted/cancelled requests, this can fail-open even when another completed request already returnedflagged: true.💡 Suggested fix (aggregate settled siblings before deciding)
- const moderationResults = await Promise.all( + const moderationSettled = await Promise.allSettled( moderationRequests.map((request) => runOpenAIContentFilterRequest( request, context, providerEnv.token, signal, ), ), ); - const successfulResults = moderationResults - .filter((result) => result.success) - .map((result) => result.response); + if (requestSignal?.aborted) { + throw new Error("Request aborted"); + } + + const successfulResults = moderationSettled + .filter( + ( + result, + ): result is PromiseFulfilledResult<OpenAIContentFilterRequestResult> => + result.status === "fulfilled" && result.value.success, + ) + .map((result) => result.value.response); + + const hasRejected = moderationSettled.some( + (result) => result.status === "rejected", + ); + if (hasRejected && successfulResults.length === 0) { + logModerationError(context, { + durationMs: Date.now() - startTime, + error: "One or more moderation requests failed", + timeout: signal.aborted, + }); + return createFailedOpenAIContentFilterResult(); + }#!/bin/bash # Verify aggregation semantics and regression coverage for timeout + flagged sibling. rg -nP 'Promise\.all(Settled)?\(' apps/gateway/src/chat/tools/openai-content-filter.ts rg -nP 'signal\.aborted|isCancellationError|isTimeoutError' apps/gateway/src/chat/tools/openai-content-filter.ts rg -nP 'flagged.*timeout|timeout.*flagged|timed-out sibling|allSettled' apps/gateway/src/chat/tools/openai-content-filter.spec.tsAlso applies to: 436-449
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/gateway/src/chat/tools/openai-content-filter.ts` around lines 393 - 402, The current use of Promise.all when awaiting runOpenAIContentFilterRequest calls (assigned to moderationResults) causes the whole aggregation to reject on the first sibling error and can drop already-fulfilled flagged results; change the aggregation to use Promise.allSettled for the moderationRequests (and the similar block around the other Promise.all at the 436-449 area), then iterate the settled results: treat fulfilled entries by inspecting their value for flagged:true, treat rejected entries that are cancellation/timeout (e.g., checks used elsewhere like isCancellationError/isTimeoutError or signal.aborted) as non-fatal but ignored, and only throw or surface errors for non-cancellation failures—finally compute the overall flagged result as true if any fulfilled result reported flagged:true.
🧹 Nitpick comments (1)
apps/gateway/src/chat/tools/openai-content-filter.ts (1)
363-365: Avoid double image traversal incheckOpenAIContentFilter.
buildOpenAIContentFilterRequests(messages)already includes image requests, but Lines 364 and 423-425 recompute image inputs. DeriveimageRequestCountfrommoderationRequeststo keep one source of truth.♻️ Suggested DRY refactor
- const imageInputs = buildOpenAIContentFilterImageInputs(messages); + const imageRequestCount = moderationRequests.filter( + (request) => request.kind === "image", + ).length; ... - hasImages: imageInputs.length > 0, + hasImages: imageRequestCount > 0, requestCount: moderationRequests.length, - imageRequestCount: imageInputs.length, + imageRequestCount,As per coding guidelines, "Apply DRY principles for code reuse".
Also applies to: 423-425
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/gateway/src/chat/tools/openai-content-filter.ts` around lines 363 - 365, checkOpenAIContentFilter is double-traversing images: buildOpenAIContentFilterRequests(messages) already includes image requests but the code also calls buildOpenAIContentFilterImageInputs(messages) and recomputes image counts later; update checkOpenAIContentFilter to derive imageRequestCount (and any image-related inputs) from the returned moderationRequests array instead of calling buildOpenAIContentFilterImageInputs again, remove the redundant buildOpenAIContentFilterImageInputs call and any separate image traversal, and ensure all logic that relied on imageRequestCount now reads it from moderationRequests (use the structure produced by buildOpenAIContentFilterRequests) to maintain a single source of truth.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@apps/gateway/src/chat/tools/openai-content-filter.ts`:
- Around line 393-402: The current use of Promise.all when awaiting
runOpenAIContentFilterRequest calls (assigned to moderationResults) causes the
whole aggregation to reject on the first sibling error and can drop
already-fulfilled flagged results; change the aggregation to use
Promise.allSettled for the moderationRequests (and the similar block around the
other Promise.all at the 436-449 area), then iterate the settled results: treat
fulfilled entries by inspecting their value for flagged:true, treat rejected
entries that are cancellation/timeout (e.g., checks used elsewhere like
isCancellationError/isTimeoutError or signal.aborted) as non-fatal but ignored,
and only throw or surface errors for non-cancellation failures—finally compute
the overall flagged result as true if any fulfilled result reported
flagged:true.
---
Nitpick comments:
In `@apps/gateway/src/chat/tools/openai-content-filter.ts`:
- Around line 363-365: checkOpenAIContentFilter is double-traversing images:
buildOpenAIContentFilterRequests(messages) already includes image requests but
the code also calls buildOpenAIContentFilterImageInputs(messages) and recomputes
image counts later; update checkOpenAIContentFilter to derive imageRequestCount
(and any image-related inputs) from the returned moderationRequests array
instead of calling buildOpenAIContentFilterImageInputs again, remove the
redundant buildOpenAIContentFilterImageInputs call and any separate image
traversal, and ensure all logic that relied on imageRequestCount now reads it
from moderationRequests (use the structure produced by
buildOpenAIContentFilterRequests) to maintain a single source of truth.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 17beb761-1e28-4e3f-bbd9-a833b0ba4852
📒 Files selected for processing (2)
apps/gateway/src/chat/tools/openai-content-filter.spec.tsapps/gateway/src/chat/tools/openai-content-filter.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/gateway/src/chat/tools/openai-content-filter.spec.ts
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
scripts/two-image-edit.sh (1)
22-33: Add explicit input-file readability checks before encoding.Currently, missing/unreadable inputs fail indirectly during
file/base64. A direct check gives clearer failures earlier.Suggested fix
IMAGE1=$1 IMAGE2=$2 + +for img in "$IMAGE1" "$IMAGE2"; do + if [[ ! -r "$img" ]]; then + echo "Input image is not readable: $img" >&2 + exit 1 + fi +doneAlso applies to: 35-36
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/two-image-edit.sh` around lines 22 - 33, Add an explicit readability check for input files in mime_type and data_url so missing/unreadable files fail with a clear message before calling file/base64: in mime_type (function mime_type) validate the argument exists and is readable (e.g., test -r "$1") and print a descriptive error and non-zero exit if not; in data_url (function data_url) check the local path variable is non-empty and readable (test -r "$path") and similarly error out before calling mime_type or base64, ensuring callers get immediate, clear failures rather than cryptic file/base64 errors.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@scripts/two-image-edit.sh`:
- Around line 41-61: Update the curl invocation in scripts/two-image-edit.sh to
add HTTP-failure and timeout handling: include curl flags like --fail (or
--fail-with-body if available), --max-time (e.g. 30), and --connect-timeout
(e.g. 10) alongside -sS, then immediately check curl's exit status ($?) after
the request and on non-zero exit log an error to stderr and exit with a non-zero
code (remove or avoid treating the response file as valid on failure). Target
the curl call that posts to "$GATEWAY_URL/v1/images/edits" and the surrounding
logic that writes to "$RESPONSE_FILE" so failures/timeouts are detected and
handled rather than silently parsed.
- Around line 46-58: The JSON payload is unsafe because variables MODEL,
IMAGE1_URL, IMAGE2_URL, ASPECT_RATIO, SIZE, and QUALITY are injected raw and can
break JSON if they contain quotes/backslashes/newlines; fix by JSON-escaping
these shell variables before interpolation (use the same technique used for
PROMPT: pipe each variable through a JSON encoder such as python3 -c 'import
json,sys; print(json.dumps(sys.stdin.read()))' or jq -R -s `@json`) and replace
the raw uses of $MODEL, $IMAGE1_URL, $IMAGE2_URL, $ASPECT_RATIO, $SIZE, and
$QUALITY in the payload with their escaped counterparts so the resulting JSON
remains valid.
- Around line 59-60: The request body in scripts/two-image-edit.sh fails to set
response_format, causing the API to return URLs while later code (the parser
around lines 79-81) expects data[0].b64_json; update the request JSON to include
"response_format": "b64_json" so the response contains data[0].b64_json, and
verify the parser still reads data[0].b64_json; specifically modify the payload
construction used for image edits (the block that currently ends with "n": 1) to
add the response_format field set to "b64_json".
---
Nitpick comments:
In `@scripts/two-image-edit.sh`:
- Around line 22-33: Add an explicit readability check for input files in
mime_type and data_url so missing/unreadable files fail with a clear message
before calling file/base64: in mime_type (function mime_type) validate the
argument exists and is readable (e.g., test -r "$1") and print a descriptive
error and non-zero exit if not; in data_url (function data_url) check the local
path variable is non-empty and readable (test -r "$path") and similarly error
out before calling mime_type or base64, ensuring callers get immediate, clear
failures rather than cryptic file/base64 errors.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: c56b9fa3-88a1-4563-ab9d-898a0bd80ddb
📒 Files selected for processing (1)
scripts/two-image-edit.sh
| curl -sS -X POST "$GATEWAY_URL/v1/images/edits" \ | ||
| -H "Authorization: Bearer $TOKEN" \ | ||
| -H "Content-Type: application/json" \ | ||
| -d @- <<EOF > "$RESPONSE_FILE" | ||
| { | ||
| "model": "$MODEL", | ||
| "prompt": $(printf '%s' "$PROMPT" | python3 -c 'import json,sys; print(json.dumps(sys.stdin.read()))'), | ||
| "images": [ | ||
| { | ||
| "image_url": "$IMAGE1_URL" | ||
| }, | ||
| { | ||
| "image_url": "$IMAGE2_URL" | ||
| } | ||
| ], | ||
| "aspect_ratio": "$ASPECT_RATIO", | ||
| "size": "$SIZE", | ||
| "quality": "$QUALITY", | ||
| "n": 1 | ||
| } | ||
| EOF |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify reliability flags are present on curl invocation.
rg -n --fixed-strings 'curl -sS' scripts/two-image-edit.sh
rg -n -- '--fail-with-body|--connect-timeout|--max-time' scripts/two-image-edit.sh
# Expected: second command returns matches for all three flags.Repository: theopenco/llmgateway
Length of output: 116
Add error handling and timeout bounds to curl invocation.
The curl -sS command at line 41 will exit with status 0 on HTTP 4xx/5xx errors and has no timeout limits, causing error responses to be silently parsed and network stalls to hang indefinitely.
Suggested fix
-curl -sS -X POST "$GATEWAY_URL/v1/images/edits" \
+curl -sS --fail-with-body \
+ --connect-timeout "${CONNECT_TIMEOUT:-10}" \
+ --max-time "${MAX_TIME:-120}" \
+ -X POST "$GATEWAY_URL/v1/images/edits" \
-H "Authorization: Bearer $TOKEN" \
-H "Content-Type: application/json" \
-d `@-` <<EOF > "$RESPONSE_FILE"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| curl -sS -X POST "$GATEWAY_URL/v1/images/edits" \ | |
| -H "Authorization: Bearer $TOKEN" \ | |
| -H "Content-Type: application/json" \ | |
| -d @- <<EOF > "$RESPONSE_FILE" | |
| { | |
| "model": "$MODEL", | |
| "prompt": $(printf '%s' "$PROMPT" | python3 -c 'import json,sys; print(json.dumps(sys.stdin.read()))'), | |
| "images": [ | |
| { | |
| "image_url": "$IMAGE1_URL" | |
| }, | |
| { | |
| "image_url": "$IMAGE2_URL" | |
| } | |
| ], | |
| "aspect_ratio": "$ASPECT_RATIO", | |
| "size": "$SIZE", | |
| "quality": "$QUALITY", | |
| "n": 1 | |
| } | |
| EOF | |
| curl -sS --fail-with-body \ | |
| --connect-timeout "${CONNECT_TIMEOUT:-10}" \ | |
| --max-time "${MAX_TIME:-120}" \ | |
| -X POST "$GATEWAY_URL/v1/images/edits" \ | |
| -H "Authorization: Bearer $TOKEN" \ | |
| -H "Content-Type: application/json" \ | |
| -d `@-` <<EOF > "$RESPONSE_FILE" | |
| { | |
| "model": "$MODEL", | |
| "prompt": $(printf '%s' "$PROMPT" | python3 -c 'import json,sys; print(json.dumps(sys.stdin.read()))'), | |
| "images": [ | |
| { | |
| "image_url": "$IMAGE1_URL" | |
| }, | |
| { | |
| "image_url": "$IMAGE2_URL" | |
| } | |
| ], | |
| "aspect_ratio": "$ASPECT_RATIO", | |
| "size": "$SIZE", | |
| "quality": "$QUALITY", | |
| "n": 1 | |
| } | |
| EOF |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@scripts/two-image-edit.sh` around lines 41 - 61, Update the curl invocation
in scripts/two-image-edit.sh to add HTTP-failure and timeout handling: include
curl flags like --fail (or --fail-with-body if available), --max-time (e.g. 30),
and --connect-timeout (e.g. 10) alongside -sS, then immediately check curl's
exit status ($?) after the request and on non-zero exit log an error to stderr
and exit with a non-zero code (remove or avoid treating the response file as
valid on failure). Target the curl call that posts to
"$GATEWAY_URL/v1/images/edits" and the surrounding logic that writes to
"$RESPONSE_FILE" so failures/timeouts are detected and handled rather than
silently parsed.
| "model": "$MODEL", | ||
| "prompt": $(printf '%s' "$PROMPT" | python3 -c 'import json,sys; print(json.dumps(sys.stdin.read()))'), | ||
| "images": [ | ||
| { | ||
| "image_url": "$IMAGE1_URL" | ||
| }, | ||
| { | ||
| "image_url": "$IMAGE2_URL" | ||
| } | ||
| ], | ||
| "aspect_ratio": "$ASPECT_RATIO", | ||
| "size": "$SIZE", | ||
| "quality": "$QUALITY", |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify raw interpolated JSON string fields are eliminated.
rg -nP '"(model|aspect_ratio|size|quality|image_url)":\s*"\$[A-Z0-9_]+"' scripts/two-image-edit.sh
# Expected after fix: no matches.Repository: theopenco/llmgateway
Length of output: 237
🏁 Script executed:
cat -n scripts/two-image-edit.sh | sed -n '40,65p'Repository: theopenco/llmgateway
Length of output: 791
JSON payload interpolation is unsafe for non-trivial env values.
At lines 46, 50, 53, 56–58, values like MODEL, IMAGE1_URL, IMAGE2_URL, ASPECT_RATIO, SIZE, and QUALITY are injected as raw strings. If any contains quotes, newlines, or backslashes, the JSON becomes invalid and the API request fails. Note that PROMPT (line 47) is already properly escaped using Python's json.dumps(), but other fields are not.
Suggested fix
+json_escape() {
+ python3 -c 'import json,sys; print(json.dumps(sys.stdin.read()))'
+}
+
+MODEL_JSON=$(printf '%s' "$MODEL" | json_escape)
+ASPECT_RATIO_JSON=$(printf '%s' "$ASPECT_RATIO" | json_escape)
+SIZE_JSON=$(printf '%s' "$SIZE" | json_escape)
+QUALITY_JSON=$(printf '%s' "$QUALITY" | json_escape)
+IMAGE1_URL_JSON=$(printf '%s' "$IMAGE1_URL" | json_escape)
+IMAGE2_URL_JSON=$(printf '%s' "$IMAGE2_URL" | json_escape)
+
{
- "model": "$MODEL",
+ "model": $MODEL_JSON,
"prompt": $(printf '%s' "$PROMPT" | python3 -c 'import json,sys; print(json.dumps(sys.stdin.read()))'),
"images": [
{
- "image_url": "$IMAGE1_URL"
+ "image_url": $IMAGE1_URL_JSON
},
{
- "image_url": "$IMAGE2_URL"
+ "image_url": $IMAGE2_URL_JSON
}
],
- "aspect_ratio": "$ASPECT_RATIO",
- "size": "$SIZE",
- "quality": "$QUALITY",
+ "aspect_ratio": $ASPECT_RATIO_JSON,
+ "size": $SIZE_JSON,
+ "quality": $QUALITY_JSON,
"n": 1
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "model": "$MODEL", | |
| "prompt": $(printf '%s' "$PROMPT" | python3 -c 'import json,sys; print(json.dumps(sys.stdin.read()))'), | |
| "images": [ | |
| { | |
| "image_url": "$IMAGE1_URL" | |
| }, | |
| { | |
| "image_url": "$IMAGE2_URL" | |
| } | |
| ], | |
| "aspect_ratio": "$ASPECT_RATIO", | |
| "size": "$SIZE", | |
| "quality": "$QUALITY", | |
| json_escape() { | |
| python3 -c 'import json,sys; print(json.dumps(sys.stdin.read()))' | |
| } | |
| MODEL_JSON=$(printf '%s' "$MODEL" | json_escape) | |
| ASPECT_RATIO_JSON=$(printf '%s' "$ASPECT_RATIO" | json_escape) | |
| SIZE_JSON=$(printf '%s' "$SIZE" | json_escape) | |
| QUALITY_JSON=$(printf '%s' "$QUALITY" | json_escape) | |
| IMAGE1_URL_JSON=$(printf '%s' "$IMAGE1_URL" | json_escape) | |
| IMAGE2_URL_JSON=$(printf '%s' "$IMAGE2_URL" | json_escape) | |
| { | |
| "model": $MODEL_JSON, | |
| "prompt": $(printf '%s' "$PROMPT" | python3 -c 'import json,sys; print(json.dumps(sys.stdin.read()))'), | |
| "images": [ | |
| { | |
| "image_url": $IMAGE1_URL_JSON | |
| }, | |
| { | |
| "image_url": $IMAGE2_URL_JSON | |
| } | |
| ], | |
| "aspect_ratio": $ASPECT_RATIO_JSON, | |
| "size": $SIZE_JSON, | |
| "quality": $QUALITY_JSON, | |
| "n": 1 | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@scripts/two-image-edit.sh` around lines 46 - 58, The JSON payload is unsafe
because variables MODEL, IMAGE1_URL, IMAGE2_URL, ASPECT_RATIO, SIZE, and QUALITY
are injected raw and can break JSON if they contain quotes/backslashes/newlines;
fix by JSON-escaping these shell variables before interpolation (use the same
technique used for PROMPT: pipe each variable through a JSON encoder such as
python3 -c 'import json,sys; print(json.dumps(sys.stdin.read()))' or jq -R -s
`@json`) and replace the raw uses of $MODEL, $IMAGE1_URL, $IMAGE2_URL,
$ASPECT_RATIO, $SIZE, and $QUALITY in the payload with their escaped
counterparts so the resulting JSON remains valid.
| "n": 1 | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
For OpenAI-compatible POST /v1/images/edits, what are the supported response_format values and what is the default if omitted?
💡 Result:
The supported response_format values for OpenAI-compatible POST /v1/images/edits are "url" and "b64_json". The default if omitted is "url". This matches the legacy DALL-E 2 behavior, which many OpenAI-compatible servers (e.g., LocalAI, vLLM-Omni, OpenVINO) emulate for compatibility. Official OpenAI docs note this parameter for DALL-E 2 (default "url"), while GPT Image models ignore it and always return b64_json.
Citations:
- 1: https://platform.openai.com/docs/api-reference/images/createEdit?lang=python
- 2: 'response_format' field in OpenAI image creation request does not match OpenAI API spec mudler/LocalAI#1910
- 3: https://community.openai.com/t/edit-endpoint-images-edits-refusing-gpt-image-models/1375581
- 4: https://developers.openai.com/cookbook/examples/dalle/image_generations_edits_and_variations_with_dall-e/
- 5: https://community.openai.com/t/error-when-attempting-to-access-gpt-image-1-5-in-images-edits/1375534
- 6: https://developers.openai.com/api/reference/go/resources/images/methods/edit/
- 7: https://docs.vllm.ai/projects/vllm-omni/en/stable/serving/image_edit_api/
🏁 Script executed:
# First, locate and examine the script
fd "two-image-edit.sh" -type fRepository: theopenco/llmgateway
Length of output: 234
🏁 Script executed:
# Correct syntax for fd to find the script
fd "two-image-edit.sh"Repository: theopenco/llmgateway
Length of output: 89
🏁 Script executed:
# Check file size and read the relevant sections
wc -l scripts/two-image-edit.sh
echo "---"
# Read around lines 59-60 (request body with response_format concern)
sed -n '50,70p' scripts/two-image-edit.sh
echo "---"
# Read around lines 79-81 (parser implementation)
sed -n '70,90p' scripts/two-image-edit.shRepository: theopenco/llmgateway
Length of output: 917
Request/response format contract mismatch will cause script failure.
The request body at lines 59-60 does not explicitly set response_format, causing the API to default to "url". However, the parser at lines 79-81 expects data[0].b64_json, which is only returned when response_format is explicitly set to "b64_json". The script will fail with "Response missing data[0].b64_json" when using any OpenAI-compatible endpoint that follows the standard default behavior.
Required fix
"quality": "$QUALITY",
+ "response_format": "b64_json",
"n": 1
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "n": 1 | |
| } | |
| "response_format": "b64_json", | |
| "n": 1 | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@scripts/two-image-edit.sh` around lines 59 - 60, The request body in
scripts/two-image-edit.sh fails to set response_format, causing the API to
return URLs while later code (the parser around lines 79-81) expects
data[0].b64_json; update the request JSON to include "response_format":
"b64_json" so the response contains data[0].b64_json, and verify the parser
still reads data[0].b64_json; specifically modify the payload construction used
for image edits (the block that currently ends with "n": 1) to add the
response_format field set to "b64_json".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5fffaf90f7
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const moderationResults = await Promise.all( | ||
| moderationRequests.map((request) => |
There was a problem hiding this comment.
This Promise.all fans out every moderation request at once, and each input image is now a separate upstream call; under image-heavy prompts this can easily cause 429/timeout responses from the moderation API. Because aggregation later drops failed calls and fail-opens when none succeed, high fan-out can return flagged: false even when some images were never effectively moderated. Please add a per-request concurrency limit (and/or a hard image cap) so overload cannot degrade moderation coverage.
Useful? React with 👍 / 👎.
Summary
Testing
Summary by CodeRabbit
Tests
Refactor
New Features