feat(protocols): ImageGenerationCall output metadata — action/background/output_format/quality/size (R6.9) - #1377
Conversation
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughRefactors MCP image-generation extraction to return an ImageGenerationFields struct and forwards five optional OpenAI image metadata fields ( Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
|
Hi @slin1237, this PR has merge conflicts that must be resolved before it can be merged. Please rebase your branch: git fetch origin main
git rebase origin/main
# resolve any conflicts, then:
git push --force-with-lease |
Real OpenAI production `image_generation_call` output items carry five
metadata fields that the OpenAI Rust SDK v2.8.1 declares incompletely:
{
"id": "ig_...",
"type": "image_generation_call",
"status": "completed",
"action": "generate", // NEW
"background": "opaque", // NEW
"output_format": "png", // NEW
"quality": "high", // NEW
"result": "<base64>",
"revised_prompt": "...",
"size": "1024x1024" // NEW
}
Without these fields on our types the gateway silently dropped them on
cloud passthrough, persistence round-trips, and R6.5 integration
assertions couldn't inspect them.
Changes:
* crates/protocols/src/responses.rs: add `action`, `background`,
`output_format`, `quality`, `size` (all `Option<String>`, all
`skip_serializing_if = "Option::is_none"`) to both
`ResponseOutputItem::ImageGenerationCall` and
`ResponseInputOutputItem::ImageGenerationCall`. Placed before
`status` so the last-field convention holds; typed as
`Option<String>` (not narrow enums) so evolving spec values pass
through unchanged — mirrors `ImageGenerationTool` on the input-tool
side.
* crates/mcp/src/transform/transformer.rs: extend
`to_image_generation_call` to extract the five new fields from the
MCP tool result payload (direct object + embedded text-block
shapes). Introduces a private `ImageGenerationFields` helper struct
so adding future fields no longer ripples through call sites.
* model_gateway/src/routers/grpc/regular/responses/conversions.rs:
one test-only struct literal updated with the new fields.
* Tests: four new roundtrip tests in crates/protocols/tests/
responses.rs (full + minimal for both output and input variants)
plus three new MCP transformer tests covering metadata forwarding
from direct objects and text-block payloads, and the no-metadata
case to pin absent-not-null serialization.
Unblocks R6.5 cloud-matrix assertions on these fields.
Refs: R6.9
Signed-off-by: Simo Lin <linsimo.mark@gmail.com>
6ecb30b to
0cba1e7
Compare
Summary
Problem
The OpenAI Rust SDK v2.8.1 we pin (reflected in
openai-protocol'sImageGenerationCall) declares theimage_generation_calloutputitem incompletely — it only carries
id,result,revised_prompt,status. Real OpenAI production responses for this item includefive additional metadata fields:
{ \"id\": \"ig_...\", \"type\": \"image_generation_call\", \"status\": \"completed\", \"action\": \"generate\", \"background\": \"opaque\", \"output_format\": \"png\", \"quality\": \"high\", \"result\": \"<base64>\", \"revised_prompt\": \"...\", \"size\": \"1024x1024\" }Because these fields are not declared on our types:
lose provider-side metadata).
they never land in the emitted output item.
Solution
Additive-only changes to both variants of
ImageGenerationCallandthe MCP-side transformer that builds the output item from tool-call
results:
Protocol types —
crates/protocols/src/responses.rs:add
action,background,output_format,quality,sizetoboth
ResponseOutputItem::ImageGenerationCallandResponseInputOutputItem::ImageGenerationCall. All five areOption<String>(not narrow enums, so evolving spec values passthrough unchanged — mirrors
ImageGenerationToolon theinput-tool side) and all use
#[serde(default, skip_serializing_if = \"Option::is_none\")]soa minimal item still serializes spec-compatibly. Placed before
statusto keep the last-field convention.MCP transformer —
crates/mcp/src/transform/transformer.rs:extend
to_image_generation_callto extract the same five keysfrom the MCP tool-call payload when present (direct object and
text-block shapes). Refactored
extract_image_generation_fieldsto return a named
ImageGenerationFieldsstruct so addingfields no longer ripples through the call site.
Test-only struct literal —
model_gateway/src/routers/grpc/regular/responses/conversions.rs:updated the one struct literal there with the new fields. No
router behavior changes.
Out of scope
ImageGenerationToolinput-side type (already carried these via T4)Test plan
cargo test -p openai-protocol --tests— 10 image-generationroundtrip tests all pass, including 4 new ones:
image_generation_call_output_item_round_trips_with_full_metadataimage_generation_call_output_item_round_trips_minimal_without_metadataimage_generation_call_input_item_round_trips_with_full_metadataimage_generation_call_input_item_round_trips_minimal_without_metadatacargo test -p smg-mcp --tests— 201 tests pass, including 3 newtransformer tests that pin metadata forwarding from direct-object
and text-block MCP payloads plus absent-not-null serialization when
an MCP server surfaces no metadata.
cargo check --workspace --all-targets— clean.cargo clippy --workspace --all-targets -- -D warnings— clean.cargo fmt --check— clean.Compatibility
(
skip_serializing_ifkeepsnulls off the wire).stateless multi-turn replay preserves metadata.
Refs: R6.9
Checklist
Summary by CodeRabbit
New Features
Tests