Repository navigation
fix(images): forward image_config on OpenRouter image edits - #30881
Ewertonslv wants to merge 10 commits into
Conversation
image_config was honored on OpenRouter image generation but silently dropped on image edits. The default edit branch in image_edit() never merged non_default_params (which still carries image_config after the optional-param whitelist filters it out) before calling the handler, unlike the bedrock, stability, and black_forest_labs branches. Merge non_default_params for the openrouter path so image_config survives; OpenRouter's transform already forwards extra top-level params into the chat-completions body, so it then reaches the provider Fixes BerriAI#30753
Greptile SummaryThis PR fixes a bug where non-default params (notably
Confidence Score: 5/5Safe to merge — the change is a minimal, targeted extension of an existing pattern already applied on three other provider branches, and the new hardening layer in the OpenRouter transform is well-covered by the added mock tests. The unconditional No files require special attention.
|
| Filename | Overview |
|---|---|
| litellm/images/main.py | Adds unconditional image_edit_request_params.update(non_default_params) before the default handler call, mirroring the pattern already present on the bedrock/stability/black_forest_labs branches and fixing the silent param drop for all fallthrough providers. |
| litellm/llms/openrouter/image_edit/transformation.py | Adds OPENROUTER_ROUTING_CONTROL_PARAMS frozenset and filters those keys out of the forwarded params in transform_image_edit_request, preventing routing-control fields from being injected into the upstream chat body while allowing legitimate params like image_config through. |
| tests/test_litellm/images/test_image_edit_utils.py | Adds TestImageEditDefaultPathForwardsNonDefaultParams with two mock-only regression tests covering both openrouter and openai (generic fallthrough) paths; no real network calls. |
| tests/test_litellm/llms/openrouter/image_edit/test_openrouter_image_edit_transformation.py | Adds test_transform_image_edit_request_drops_openrouter_routing_controls verifying that routing-control keys are blocked while image_config still passes through; mock-only, no real network calls. |
Reviews (3): Last reviewed commit: "fix(openrouter): drop routing-control fi..." | Re-trigger Greptile
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
Muhtasim-Munif-Fahim
left a comment
There was a problem hiding this comment.
Small focused fix. \image_config\ was being dropped for OpenRouter image edits because the default branch never merged
on_default_params. The 2-line fix is correct and the regression test validates it. LGTM.
|
Thanks for the contribution! A couple of things to get this ready:
Once that's in, we'll take another look — appreciate the work on this! 🙏 |
…all providers Greptile review thread: the merge was gated on custom_llm_provider == openrouter, but every fallthrough provider (openai, azure, vertex_ai, ...) hits the same base_llm_http_handler.image_edit_handler call and silently dropped extra params like image_config. Make the merge unconditional before the default handler call, mirroring the bedrock/stability/black_forest_labs branches. Generalize the regression test to also cover a non-openrouter provider.
|
@Sameerlite thanks for the review! Both asks are addressed: 1. Greptile thread — resolved (and adopted). The reviewer was right that gating the merge on 2. Proof of working. Before/after on the regression tests (the new The tests patch |
PR overviewAll previously flagged issues have been addressed. No open security concerns remain on this pull request. Security reviewNo open security issues remain on this pull request. Fixed/addressed: 1 · PR risk: 0/10 |
|
Great work addressing the Greptile feedback and broadening the fix to all default-path providers, @Ewertonslv — the before/after test output is exactly what we need. Triggering a fresh Greptile review against the updated head.\n\n@greptileai |
The image-edit transform copied every forwarded optional param into the upstream /chat/completions body. Since the default edit path now forwards non-default params (so image_config survives), a caller could smuggle OpenRouter routing controls (models/route/provider/transforms) into an allowed image-edit request and redirect it to other models/providers, bypassing LiteLLM's model authorization and budget checks. Skip those routing-control keys when building the request body; image_config and other intended params still pass through. Addresses the routing-control-bypass review finding on BerriAI#30881.
|
Friendly ping, @Sameerlite — this one's been green since your last message (73/73 checks, all Greptile threads resolved, veria-ai reported no security concerns) and it's carrying an approval. Is there anything else you'd like from me before it lands, or is it just waiting on a merge slot? Happy to rebase onto |
|
@Sameerlite following up — I think this stalled on a mechanical issue rather than the review itself. Your
Re-triggering Greptile against the current head: For reference, the bug is still live on the default branch: the final |
…e_edit_image_config
After merging main, the unconditional non_default_params merge on the
default image_edit path collided with two upstream changes:
- multipart OpenAI-compatible routes now merge caller params themselves
(flattened, extra_body taking precedence); re-merging the raw params
let seed=42 override extra_body={"seed": 7} and undid the flattening.
- provider configs such as Azure AI FLUX.2 map supported params
(size -> width/height, guidance "4.5" -> 4.5); the raw merge re-added
size and overwrote the coerced values.
Skip the fallback merge when the multipart branch already merged, and
otherwise forward only params the provider config does not map and that
are not already in the request (e.g. OpenRouter's image_config).
|
Heads-up on a follow-up change since the approval. After the base moved to
The remaining |
…e_edit_image_config
|
Merged current The one red check is |
Relevant issues
Fixes #30753
Type
🐛 Bug Fix
Changes
image_config was honored on OpenRouter image generation but silently dropped on image edits. The default edit branch in image_edit() never merged non_default_params (which still carries image_config after the optional-param whitelist filters it out) before calling the handler, unlike the bedrock, stability, and black_forest_labs branches. This merges non_default_params for the openrouter path so image_config survives; OpenRouter's transform already forwards extra top-level params into the chat-completions body, so it then reaches the provider.
Added a regression test asserting image_config is forwarded to the handler for OpenRouter image edits
Proof of fix
The regression test fails on current code (image_config is absent from the forwarded params) and passes with the fix. Live proxy verification against an OpenRouter image-edit model to follow
Pre-Submission checklist