feat(gateway): add image_generation infrastructure for hosted-tool MCP plumbing (R6.1) - #1355
Merged
Merged
Conversation
Adds the shared infrastructure that lets the gateway route
`image_generation` hosted tool calls through the same MCP plumbing
already used for `web_search` / `code_interpreter` / `file_search`.
This is PR 1 of 4 for audit item R6; PRs R6.2, R6.3, and R6.4 wire
the per-router event emission sites on top of this foundation.
Layered additions (all mirroring existing patterns for the other
three hosted tools):
- crates/protocols/src/event_types.rs:
* new `ImageGenerationCallEvent { InProgress, Generating,
PartialImage, Completed }` with
`response.image_generation_call.*` constants, `as_str`, and
`Display` (spec source: OpenAI SDK v2.8.1
`response_image_gen_call_*_event.py` +
`.claude/_audit/openai-responses-api-spec.md`)
* `ItemType::ImageGenerationCall` + `IMAGE_GENERATION_CALL`
const; `is_builtin_tool_call` now includes it
- crates/mcp/src/core/config.rs:
* `BuiltinToolType::ImageGeneration` with `Display` =
`"image_generation"` and `response_format()` →
`ResponseFormatConfig::ImageGenerationCall`
* new `ResponseFormatConfig::ImageGenerationCall` variant
* existing exhaustive-variant tests extended
- crates/mcp/src/transform/{types,transformer}.rs:
* `ResponseFormat::ImageGenerationCall` variant +
`From<ResponseFormatConfig>` arm + serde round-trip test
* `ResponseTransformer::transform` dispatches to new
`to_image_generation_call(result, tool_call_id)` that maps
MCP `CallToolResult` → `ResponseOutputItem::ImageGenerationCall`
using only T4's actual fields: `id`, `result` (base64),
`revised_prompt?`, `status`. Probes direct object fields and
embedded `openai_response` text-block payloads, mirroring the
fallback pattern in `to_web_search_call`
* new `compact_image_generation_output(item)` helper strips the
base64 `result` payload for multi-turn storage (no-op for any
non-image item); re-exported from `transform` and crate root
* unit tests for direct field extraction, `b64_json` alias,
embedded text-block extraction, missing-image fallback, and
the compactor's strip + no-op behavior
- model_gateway/src/routers/grpc/common/responses/streaming.rs:
* `emit_tool_call_in_progress` / `emit_tool_call_searching`
(emits `generating` as the intermediate) /
`emit_tool_call_completed` all handle the new variant
* new `emit_image_generation_partial_image` helper for the
image-specific `partial_image` event that carries
`partial_image_index` + `partial_image_b64`. Gated on
`ResponseFormat::ImageGenerationCall`; per-router wiring in
R6.2/R6.3/R6.4 decides when to call it. Marked `#[expect(
dead_code, reason="...")]` until then
* `type_str_for_format` / `output_item_type_for_format` /
`OutputItemType` variants + `allocate_output_index` id prefix
`"ig"` all extended
- model_gateway/src/routers/common/mcp_utils.rs:
* `collect_builtin_routing` and `extract_builtin_types` now
recognize `ResponseTool::ImageGeneration(_)` →
`BuiltinToolType::ImageGeneration`
* new test `test_collect_builtin_routing_image_generation`
asserts the full chain produces
`ResponseFormat::ImageGenerationCall`
- model_gateway/src/routers/openai/mcp/tool_loop.rs (stubs for
exhaustive matches so R6.2 can fill them in without breaking
compilation):
* `send_tool_call_intermediate_event` emits
`ImageGenerationCallEvent::GENERATING` as the neutral
intermediate
* `send_tool_call_completion_events` maps
`ItemType::IMAGE_GENERATION_CALL` →
`ImageGenerationCallEvent::COMPLETED`
* `stable_streaming_tool_item_id` prefixes item ids with `"ig_"`
(matches the transformer's output id)
* `non_streaming_tool_item_id_source` joins the existing
`strip_prefix("fc_"|"call_")` arm
All stubs use real neutral values (no `todo!`/`unreachable!`/
`panic!`) so runtime behavior stays safe until R6.2/R6.3/R6.4
land real wiring.
Non-goals for R6.1 (explicitly left for the follow-up PRs):
- No router-specific event emission beyond what the shared emitter
provides. Harmony and regular gRPC streaming paths untouched.
- No MCP server-config additions beyond the new `BuiltinToolType`
variant — operators register their image-gen MCP server through
existing config mechanisms.
- No changes to `crates/protocols/src/responses.rs` (T4 owns those
types).
- No gRPC BUILTIN_TOOLS changes — being handled separately in PR
\#1353.
- No `model_gateway/src/routers/common/tool_overrides.rs` yet —
flagged for a follow-up if scope warrants.
Gates:
- cargo check -p openai-protocol -p smg-mcp -p smg --lib --tests
--benches ✓
- cargo test -p openai-protocol -p smg-mcp -p smg --lib ✓ (855
passed total; 8 new image_generation tests green)
- cargo fmt --all ✓
- cargo clippy -p openai-protocol -p smg-mcp -p smg --lib --tests
--benches -- -D warnings ✓
Replaces earlier attempts in PR #1352 and #1091; avoids their
`unreachable!()` stubs by using real neutral values that keep
runtime safe until real wiring lands.
Refs: R6 in .claude/_audit/responses-api-gap-audit.md
Signed-off-by: Simo Lin <linsimo.mark@gmail.com>
slin1237
requested review from
CatherineSue,
key4ng,
zhaowenzi and
zhoug9127
as code owners
April 23, 2026 16:35
|
Caution Review failedPull request was closed or merged during review 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 (1)
📝 WalkthroughWalkthroughAdds image-generation as a built-in tool/response type across MCP, transformer, protocols, and gateway: new enums/events, transformer extraction/compaction logic, streaming event emission (including partial-image), routing updates, and tests covering serialization and transformation behaviors. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant MCP
participant Transformer
participant Model
participant Gateway
Client->>MCP: Request with image-generation tool call
MCP->>Transformer: Deliver MCP result payload
Transformer->>Transformer: Extract image b64 (result.image_base64 / b64_json / embedded openai_response)
Transformer-->>MCP: Emit ResponseOutputItem::ImageGenerationCall (id: ig_...)
MCP->>Gateway: Forward transformed item
Gateway->>Client: Stream events (InProgress -> Generating -> PartialImage* -> Completed)
Gateway->>Transformer: (optional) call compact_image_generation_output to remove large payloads
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested labels
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)
Warning Review ran into problems🔥 ProblemsTimed out fetching pipeline failures after 30000ms Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request introduces support for image generation tools within the MCP framework, adding new configuration options, response formats, and streaming events. Key changes include the implementation of logic to transform MCP image results into OpenAI-compatible formats, a utility to compact image data for storage, and infrastructure for emitting image generation progress and partial image events. Feedback was provided to refactor the image field extraction logic to eliminate code duplication and prevent potential data loss when fields are distributed across multiple response blocks.
Comment on lines
+272
to
+321
| fn extract_image_generation_fields( | ||
| result: &serde_json::Value, | ||
| ) -> (Option<String>, Option<String>) { | ||
| // 1) Direct object fields: { result | image_base64 | b64_json, revised_prompt } | ||
| if let Some(obj) = result.as_object() { | ||
| let image_b64 = obj | ||
| .get("result") | ||
| .and_then(|v| v.as_str()) | ||
| .or_else(|| obj.get("image_base64").and_then(|v| v.as_str())) | ||
| .or_else(|| obj.get("b64_json").and_then(|v| v.as_str())) | ||
| .map(String::from); | ||
|
|
||
| let revised_prompt = obj | ||
| .get("revised_prompt") | ||
| .and_then(|v| v.as_str()) | ||
| .map(String::from); | ||
|
|
||
| if image_b64.is_some() || revised_prompt.is_some() { | ||
| return (image_b64, revised_prompt); | ||
| } | ||
| } | ||
|
|
||
| // 2) Embedded openai_response payloads inside MCP text blocks. | ||
| if result.is_array() { | ||
| for openai_response in extract_embedded_openai_responses(result) { | ||
| let obj = match openai_response.as_object() { | ||
| Some(o) => o, | ||
| None => continue, | ||
| }; | ||
|
|
||
| let image_b64 = obj | ||
| .get("result") | ||
| .and_then(|v| v.as_str()) | ||
| .or_else(|| obj.get("image_base64").and_then(|v| v.as_str())) | ||
| .or_else(|| obj.get("b64_json").and_then(|v| v.as_str())) | ||
| .map(String::from); | ||
|
|
||
| let revised_prompt = obj | ||
| .get("revised_prompt") | ||
| .and_then(|v| v.as_str()) | ||
| .map(String::from); | ||
|
|
||
| if image_b64.is_some() || revised_prompt.is_some() { | ||
| return (image_b64, revised_prompt); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| (None, None) | ||
| } |
Contributor
There was a problem hiding this comment.
The current implementation of extract_image_generation_fields has two areas for improvement:
- Logic Duplication: The field extraction logic (checking for
result,image_base64, andb64_json) is repeated for both the direct object and the embeddedopenai_responseblocks. - Robustness/Data Loss: The early return at line 290 and line 315 means that if an MCP server distributes the fields across different blocks (e.g.,
revised_promptin one andimage_base64in another), only the first one encountered will be captured, and the other will be lost.
Refactoring this to use a helper closure that accumulates fields across all available sources would be more robust and maintainable, adhering to the rule of extracting duplicated logic into a shared helper function.
fn extract_image_generation_fields(
result: &serde_json::Value,
) -> (Option<String>, Option<String>) {
let mut image_b64 = None;
let mut revised_prompt = None;
let mut update_fields = |obj: &serde_json::Map<String, serde_json::Value>| {
if image_b64.is_none() {
image_b64 = obj
.get("result")
.and_then(|v| v.as_str())
.or_else(|| obj.get("image_base64").and_then(|v| v.as_str()))
.or_else(|| obj.get("b64_json").and_then(|v| v.as_str()))
.map(String::from);
}
if revised_prompt.is_none() {
revised_prompt = obj
.get("revised_prompt")
.and_then(|v| v.as_str())
.map(String::from);
}
};
// 1) Direct object fields: { result | image_base64 | b64_json, revised_prompt }
if let Some(obj) = result.as_object() {
update_fields(obj);
}
// 2) Embedded openai_response payloads inside MCP text blocks.
if result.is_array() {
for openai_response in extract_embedded_openai_responses(result) {
if let Some(obj) = openai_response.as_object() {
update_fields(obj);
}
}
}
(image_b64, revised_prompt)
}References
- Extract duplicated logic into a shared helper function to improve maintainability and reduce redundancy.
github-actions
Bot
added
grpc
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| /// For any non-image item this is a no-op. | ||
| pub fn compact_image_generation_output(item: &mut ResponseOutputItem) { | ||
| if let ResponseOutputItem::ImageGenerationCall { result, .. } = item { | ||
| result.clear(); |
There was a problem hiding this comment.
🟡 Nit: result.clear() zeros the length but keeps the heap capacity allocated. For base64-encoded images this can be several megabytes of unused capacity per item held in multi-turn context — partially undermining the stated goal of preventing memory bloat.
Suggested change
| result.clear(); | |
| *result = String::new(); |
String::new() is a zero-allocation replacement that actually frees the backing buffer. Alternatively drop(std::mem::take(result)) would also work.
Addresses Gemini review feedback on PR #1355: - Deduplicates the field-extraction logic in `extract_image_generation_fields` via a shared `update_fields` closure so the same `result | image_base64 | b64_json` probe and `revised_prompt` probe don't appear twice. - Switches from "early-return on first hit" to "accumulate across all sources". If an MCP server distributes fields across multiple text blocks (e.g. `revised_prompt` in block 1 and `image_base64` in block 2), both are now captured instead of the first one silently winning. - Adds `test_image_generation_transform_fields_distributed_across_blocks` to regression-guard the split case. Direct object lookup runs first and wins for each slot it fills; embedded `openai_response` text blocks only fill slots that are still `None`. Behavior when fields come from a single source is unchanged (covered by the existing direct-fields / b64_json / embedded tests). Signed-off-by: Simo Lin <linsimo.mark@gmail.com>
Member
Author
|
Thanks for the review! Addressed both points in commit 05e4ea0:
First occurrence still wins for each slot; direct object lookup runs before embedded blocks so its values win when both are present. |
Addresses Claude PR review feedback on PR #1355: `String::clear()` zeros the length but leaves the heap allocation in place. For base64 image bytes this can be several megabytes of retained capacity per stored item — partially defeating the compaction goal. Switch to `*result = String::new()` to actually release the backing buffer. The existing unit tests (strip + no-op) already cover behavior, so no test changes needed. Signed-off-by: Simo Lin <linsimo.mark@gmail.com>
Member
Author
|
Thanks for the catch! Addressed in commit 508c78d — switched to |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
![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: 8e7e675bea
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| Some(ResponseFormat::WebSearchCall) => "web_search_call", | ||
| Some(ResponseFormat::CodeInterpreterCall) => "code_interpreter_call", | ||
| Some(ResponseFormat::FileSearchCall) => "file_search_call", | ||
| Some(ResponseFormat::ImageGenerationCall) => "image_generation_call", |
There was a problem hiding this comment.
type_str_for_format now maps ResponseFormat::ImageGenerationCall to "image_generation_call", but the gRPC streaming routers still build generic MCP-shaped items (name/arguments/output) in model_gateway/src/routers/grpc/regular/responses/streaming.rs (lines 653-739) instead of the required image-generation shape (result, optional revised_prompt). In this state, image-generation tool calls will emit malformed image_generation_call items and finalize() drops them when deserialization fails (model_gateway/src/routers/grpc/common/responses/streaming.rs lines 718-725), so users can lose tool outputs in regular/harmony streaming flows when this format is enabled.
Useful? React with 👍 / 👎.
Member
Author
|
@codex re: P1 on Concretely, R6.3 (harmony gRPC) and R6.4 (regular gRPC) are both responsible for replacing the generic The failure mode you describe (malformed R6.2 (OpenAI router) is unaffected because |
|
To use Codex here, create a Codex account and connect to github. |
This was referenced Apr 23, 2026
1 of 2 tasks
slin1237
added a commit
that referenced
this pull request
Apr 23, 2026
Respond to the coordinator directive on PR #1365: remove the R6.3/R6.4 ``skip_for_runtime`` decorators, add local-gRPC fixtures for both engines, and harden the assertions on what the gateway emits. R6.1-R6.4 all merged. Real lane failures surfaced by this suite will be filed as R6.6/R6.7 follow-ups rather than patched from R6.5, so the CI signal is genuine rather than papered over. Engine matrix ------------- Three test classes, sharing a ``_ImageGenerationAssertions`` mix-in so the cloud + gRPC lanes stay strictly in lock-step: * ``TestImageGenerationCloud`` (vendor=openai, gpu=0) — OpenAI cloud backend with gpt-5-nano, mock MCP server standing in for the image backend. Exercises the OpenAI-compat router (R6.2). * ``TestImageGenerationGrpcSglang`` (engine=sglang, gpu=1, model=openai/gpt-oss-20b) — local SGLang worker via harmony. Exercises R6.3 gRPC-harmony wiring. * ``TestImageGenerationGrpcVllm`` (engine=vllm, gpu=1, model=meta-llama/Llama-3.1-8B-Instruct) — local vLLM worker via regular. Exercises R6.4 gRPC-regular wiring. Each lane's fixture wires its gateway at the same shared in-process ``MockMcpServer`` so deterministic base64-roundtrip / size-override assertions stay valid across engines. Fixture additions (``e2e_test/responses/conftest.py``) ------------------------------------------------------ * ``gateway_with_mock_mcp_cloud`` replaces the old ``gateway_with_mock_mcp``. Yields ``(gateway, client, mock, model)`` with ``model="gpt-5-nano"``. The old name is kept as a backward-compat alias so any unmerged branch referencing it still works. * ``_start_local_grpc_gateway_with_mcp`` helper: launches one gRPC worker for a given engine + model_id, spins up a gateway with ``--mcp-config-path`` pointing at the shared mock MCP config, wraps client instantiation in try/except so a failed init doesn't leak the gateway or the worker. * ``gateway_with_mock_mcp_grpc_sglang`` / ``gateway_with_mock_mcp_grpc_vllm``: class-scoped fixtures built on the helper. Each skips (not fails) when the gRPC worker can't start — CI lanes without GPUs otherwise poison every engine-parametrized suite. Harder assertions (``test_image_generation.py``) ------------------------------------------------ * ``_assert_image_generation_call_item`` now asserts every documented field (``type``, ``id`` ig-prefix, ``status``, ``result``, ``revised_prompt``) rather than just a subset. * ``_assert_streaming_envelope`` validates the full emitted envelope: * ``response.created`` → ``response.output_item.added`` → ``response.image_generation_call.{in_progress, generating, [partial_image], completed}`` → ``response.output_item.done`` → ``response.completed`` * each event's first occurrence strictly precedes the next required event's first occurrence * exactly one ``response.created`` / ``response.completed`` / ``output_item.added`` / ``output_item.done`` per image_gen call * ``sequence_number`` strictly monotonically increasing with no gaps > 1 (only checked when every event has one) * optional ``partial_image`` sits between ``generating`` and ``completed`` when present * Streaming and non-streaming paths share the same mix-in body, so the gRPC lanes exercise the full assertion surface without duplication. Local gates (ruff check/format, mypy, pytest --collect-only) clean; 12 tests collected (3 classes × 4 tests). Refs: R6.1 #1355, R6.2 #1356, R6.3 #1359, R6.4 #1358 (all merged) Co-authored-by: Tingting Zhou <zhoutt96@gmail.com> Signed-off-by: Simo Lin <linsimo.mark@gmail.com>
3 of 4 tasks
5 of 6 tasks
slin1237
added a commit
that referenced
this pull request
Apr 24, 2026
Introduce e2e_test/infra/mock_mcp_server.py — a thin wrapper around the official MCP SDK's FastMCP base class that exposes streamable-HTTP transport on a local port. The existing MCP e2e test (e2e_test/messages/test_mcp_tool.py) depends on a live Brave MCP server which is flaky, slow, and non-deterministic; routing-layer tests for the gateway's built-in tool plumbing need an in-process, deterministic counterpart. This mock supplies one. Files added: * e2e_test/infra/mock_mcp_server.py — MockMcpServer class. Runs FastMCP via uvicorn on a background thread; auto-allocates a free port when the caller passes port=0. Registers an image_generation tool that returns a hard-coded 1x1 transparent PNG (92-char base64) plus the prompt echoed as revised_prompt, so tests can make byte-for-byte assertions. Records every call into call_log and surfaces the last call via last_call_args for override-verification tests. TODO stubs for web_search, file_search, code_interpreter sit inline with the same shape so future R6.x PRs just uncomment and adjust. * e2e_test/infra/mock_mcp.py — mock_mcp_server session-scoped pytest fixture plus IMAGE_GENERATION_PNG_BASE64 re-export. Files modified: * e2e_test/infra/constants.py — new MOCK_MCP_HOST constant (default 127.0.0.1) paralleling the existing BRAVE_MCP_HOST. * e2e_test/infra/__init__.py — export MockMcpServer, mock_mcp_server, IMAGE_GENERATION_PNG_BASE64, MOCK_MCP_HOST. Why streamable HTTP: matches the protocol the gateway's MCP client already speaks against Brave in production (see crates/mcp/src/core/config.rs::McpTransport::Streamable). The streamable_http_app() FastMCP method yields a /mcp Starlette route mountable directly under uvicorn. How to extend: FastMCP exposes a decorator-driven registration API; add a new @fastmcp.tool in MockMcpServer._register_tools, append to self._call_log inside the body for introspection, and you're done. See the module docstring for the extension recipe. Refs: R6.1 #1355, R6.2 #1356, R6.3 #1359, R6.4 #1358 (all merged) Co-authored-by: Tingting Zhou <zhoutt96@gmail.com> Signed-off-by: Simo Lin <linsimo.mark@gmail.com>
slin1237
added a commit
that referenced
this pull request
Apr 24, 2026
Add the first end-to-end tests for the image_generation built-in tool,
using the in-process MockMcpServer introduced in the previous commit.
R6.1-R6.4 wired image_generation across the openai, gRPC-regular, and
gRPC-harmony routers end-to-end; this PR provides the first automated
coverage that exercises those paths without external-service
dependencies.
Files added:
* e2e_test/responses/conftest.py — fixture module for the Responses
suite. Provides:
- mock_mcp_config_file: writes a session-scoped YAML config to a
tempdir that matches crates/mcp/src/core/config.rs::McpConfig,
registering the mock server with builtin_type: image_generation
and response_format: image_generation_call.
- gateway_with_mock_mcp: launches an OpenAI cloud gateway with
--mcp-config-path pointing at that tempfile; yields
(gateway, client, mock_mcp_server). Skips if OPENAI_API_KEY is
absent.
- image_gen_tool_args: canonical tool payload shared by all tests
so size/quality overrides have a clean starting point.
* e2e_test/responses/test_image_generation.py — TestImageGeneration
class with four tests on the OpenAI cloud backend:
1. test_image_generation_non_streaming: verifies the response
carries an ImageGenerationCall output item with the id ig_
prefix, status=completed, result=<mock base64>, and
revised_prompt echoing the input (R6.2 output-item emission).
2. test_image_generation_streaming: verifies the
response.image_generation_call.{in_progress,generating,
completed} events fire in the documented order; partial_image
is asserted optional-but-ordered-correctly (R6.2/R6.3/R6.4
streaming contract).
3. test_image_generation_tool_overrides_size: pins
size=512x512 and quality=high on the tool payload and asserts
the mock observed those exact values via last_call_args. This
catches compactor regressions where the override pipeline
would silently drop or rewrite user-provided arguments.
4. test_image_generation_compactor_strips_base64: creates a
stored conversation, fetches /v1/conversations/{id}/items, and
asserts the raw base64 bytes do NOT appear in persisted state
— so multi-turn replay never re-ships the image to the model
(R6.1 compactor behavior).
Engine matrix: openai only for now. skip_for_runtime("sglang") and
skip_for_runtime("vllm") guard the gRPC lanes until R6.3 and R6.4
stabilise in CI; a follow-up PR can drop those skip decorators once
the local-worker lanes are green.
Other changes:
* e2e_test/pyproject.toml — add mcp>=1.0 and uvicorn to the e2e test
deps. The mcp SDK is required by the mock server; uvicorn was
already pulled transitively but is called directly from the mock
server module and so we declare it explicitly.
Manual verification:
* ruff check e2e_test/responses/test_image_generation.py \
e2e_test/infra/mock_mcp_server.py e2e_test/infra/mock_mcp.py \
e2e_test/responses/conftest.py — clean.
* mypy on the same files with --ignore-missing-imports — clean.
* pytest e2e_test/responses/test_image_generation.py --collect-only —
4 tests discovered, no import errors.
* In-process smoke test of MockMcpServer via the mcp streamable_http
client confirmed the tool registration, deterministic payload, and
last_call_args introspection work end-to-end.
Full gated run (live gateway + OpenAI key) is deferred to CI.
Refs: R6.1 #1355, R6.2 #1356, R6.3 #1359, R6.4 #1358 (all merged)
Co-authored-by: Tingting Zhou <zhoutt96@gmail.com>
Signed-off-by: Simo Lin <linsimo.mark@gmail.com>
slin1237
added a commit
that referenced
this pull request
Apr 24, 2026
Respond to the coordinator directive on PR #1365: remove the R6.3/R6.4 ``skip_for_runtime`` decorators, add local-gRPC fixtures for both engines, and harden the assertions on what the gateway emits. R6.1-R6.4 all merged. Real lane failures surfaced by this suite will be filed as R6.6/R6.7 follow-ups rather than patched from R6.5, so the CI signal is genuine rather than papered over. Engine matrix ------------- Three test classes, sharing a ``_ImageGenerationAssertions`` mix-in so the cloud + gRPC lanes stay strictly in lock-step: * ``TestImageGenerationCloud`` (vendor=openai, gpu=0) — OpenAI cloud backend with gpt-5-nano, mock MCP server standing in for the image backend. Exercises the OpenAI-compat router (R6.2). * ``TestImageGenerationGrpcSglang`` (engine=sglang, gpu=1, model=openai/gpt-oss-20b) — local SGLang worker via harmony. Exercises R6.3 gRPC-harmony wiring. * ``TestImageGenerationGrpcVllm`` (engine=vllm, gpu=1, model=meta-llama/Llama-3.1-8B-Instruct) — local vLLM worker via regular. Exercises R6.4 gRPC-regular wiring. Each lane's fixture wires its gateway at the same shared in-process ``MockMcpServer`` so deterministic base64-roundtrip / size-override assertions stay valid across engines. Fixture additions (``e2e_test/responses/conftest.py``) ------------------------------------------------------ * ``gateway_with_mock_mcp_cloud`` replaces the old ``gateway_with_mock_mcp``. Yields ``(gateway, client, mock, model)`` with ``model="gpt-5-nano"``. The old name is kept as a backward-compat alias so any unmerged branch referencing it still works. * ``_start_local_grpc_gateway_with_mcp`` helper: launches one gRPC worker for a given engine + model_id, spins up a gateway with ``--mcp-config-path`` pointing at the shared mock MCP config, wraps client instantiation in try/except so a failed init doesn't leak the gateway or the worker. * ``gateway_with_mock_mcp_grpc_sglang`` / ``gateway_with_mock_mcp_grpc_vllm``: class-scoped fixtures built on the helper. Each skips (not fails) when the gRPC worker can't start — CI lanes without GPUs otherwise poison every engine-parametrized suite. Harder assertions (``test_image_generation.py``) ------------------------------------------------ * ``_assert_image_generation_call_item`` now asserts every documented field (``type``, ``id`` ig-prefix, ``status``, ``result``, ``revised_prompt``) rather than just a subset. * ``_assert_streaming_envelope`` validates the full emitted envelope: * ``response.created`` → ``response.output_item.added`` → ``response.image_generation_call.{in_progress, generating, [partial_image], completed}`` → ``response.output_item.done`` → ``response.completed`` * each event's first occurrence strictly precedes the next required event's first occurrence * exactly one ``response.created`` / ``response.completed`` / ``output_item.added`` / ``output_item.done`` per image_gen call * ``sequence_number`` strictly monotonically increasing with no gaps > 1 (only checked when every event has one) * optional ``partial_image`` sits between ``generating`` and ``completed`` when present * Streaming and non-streaming paths share the same mix-in body, so the gRPC lanes exercise the full assertion surface without duplication. Local gates (ruff check/format, mypy, pytest --collect-only) clean; 12 tests collected (3 classes × 4 tests). Refs: R6.1 #1355, R6.2 #1356, R6.3 #1359, R6.4 #1358 (all merged) Co-authored-by: Tingting Zhou <zhoutt96@gmail.com> Signed-off-by: Simo Lin <linsimo.mark@gmail.com>
slin1237
added a commit
that referenced
this pull request
Apr 24, 2026
Introduce e2e_test/infra/mock_mcp_server.py — a thin wrapper around the official MCP SDK's FastMCP base class that exposes streamable-HTTP transport on a local port. The existing MCP e2e test (e2e_test/messages/test_mcp_tool.py) depends on a live Brave MCP server which is flaky, slow, and non-deterministic; routing-layer tests for the gateway's built-in tool plumbing need an in-process, deterministic counterpart. This mock supplies one. Files added: * e2e_test/infra/mock_mcp_server.py — MockMcpServer class. Runs FastMCP via uvicorn on a background thread; auto-allocates a free port when the caller passes port=0. Registers an image_generation tool that returns a hard-coded 1x1 transparent PNG (92-char base64) plus the prompt echoed as revised_prompt, so tests can make byte-for-byte assertions. Records every call into call_log and surfaces the last call via last_call_args for override-verification tests. TODO stubs for web_search, file_search, code_interpreter sit inline with the same shape so future R6.x PRs just uncomment and adjust. * e2e_test/infra/mock_mcp.py — mock_mcp_server session-scoped pytest fixture plus IMAGE_GENERATION_PNG_BASE64 re-export. Files modified: * e2e_test/infra/constants.py — new MOCK_MCP_HOST constant (default 127.0.0.1) paralleling the existing BRAVE_MCP_HOST. * e2e_test/infra/__init__.py — export MockMcpServer, mock_mcp_server, IMAGE_GENERATION_PNG_BASE64, MOCK_MCP_HOST. Why streamable HTTP: matches the protocol the gateway's MCP client already speaks against Brave in production (see crates/mcp/src/core/config.rs::McpTransport::Streamable). The streamable_http_app() FastMCP method yields a /mcp Starlette route mountable directly under uvicorn. How to extend: FastMCP exposes a decorator-driven registration API; add a new @fastmcp.tool in MockMcpServer._register_tools, append to self._call_log inside the body for introspection, and you're done. See the module docstring for the extension recipe. Refs: R6.1 #1355, R6.2 #1356, R6.3 #1359, R6.4 #1358 (all merged) Co-authored-by: Tingting Zhou <zhoutt96@gmail.com> Signed-off-by: Simo Lin <linsimo.mark@gmail.com>
slin1237
added a commit
that referenced
this pull request
Apr 24, 2026
Add the first end-to-end tests for the image_generation built-in tool,
using the in-process MockMcpServer introduced in the previous commit.
R6.1-R6.4 wired image_generation across the openai, gRPC-regular, and
gRPC-harmony routers end-to-end; this PR provides the first automated
coverage that exercises those paths without external-service
dependencies.
Files added:
* e2e_test/responses/conftest.py — fixture module for the Responses
suite. Provides:
- mock_mcp_config_file: writes a session-scoped YAML config to a
tempdir that matches crates/mcp/src/core/config.rs::McpConfig,
registering the mock server with builtin_type: image_generation
and response_format: image_generation_call.
- gateway_with_mock_mcp: launches an OpenAI cloud gateway with
--mcp-config-path pointing at that tempfile; yields
(gateway, client, mock_mcp_server). Skips if OPENAI_API_KEY is
absent.
- image_gen_tool_args: canonical tool payload shared by all tests
so size/quality overrides have a clean starting point.
* e2e_test/responses/test_image_generation.py — TestImageGeneration
class with four tests on the OpenAI cloud backend:
1. test_image_generation_non_streaming: verifies the response
carries an ImageGenerationCall output item with the id ig_
prefix, status=completed, result=<mock base64>, and
revised_prompt echoing the input (R6.2 output-item emission).
2. test_image_generation_streaming: verifies the
response.image_generation_call.{in_progress,generating,
completed} events fire in the documented order; partial_image
is asserted optional-but-ordered-correctly (R6.2/R6.3/R6.4
streaming contract).
3. test_image_generation_tool_overrides_size: pins
size=512x512 and quality=high on the tool payload and asserts
the mock observed those exact values via last_call_args. This
catches compactor regressions where the override pipeline
would silently drop or rewrite user-provided arguments.
4. test_image_generation_compactor_strips_base64: creates a
stored conversation, fetches /v1/conversations/{id}/items, and
asserts the raw base64 bytes do NOT appear in persisted state
— so multi-turn replay never re-ships the image to the model
(R6.1 compactor behavior).
Engine matrix: openai only for now. skip_for_runtime("sglang") and
skip_for_runtime("vllm") guard the gRPC lanes until R6.3 and R6.4
stabilise in CI; a follow-up PR can drop those skip decorators once
the local-worker lanes are green.
Other changes:
* e2e_test/pyproject.toml — add mcp>=1.0 and uvicorn to the e2e test
deps. The mcp SDK is required by the mock server; uvicorn was
already pulled transitively but is called directly from the mock
server module and so we declare it explicitly.
Manual verification:
* ruff check e2e_test/responses/test_image_generation.py \
e2e_test/infra/mock_mcp_server.py e2e_test/infra/mock_mcp.py \
e2e_test/responses/conftest.py — clean.
* mypy on the same files with --ignore-missing-imports — clean.
* pytest e2e_test/responses/test_image_generation.py --collect-only —
4 tests discovered, no import errors.
* In-process smoke test of MockMcpServer via the mcp streamable_http
client confirmed the tool registration, deterministic payload, and
last_call_args introspection work end-to-end.
Full gated run (live gateway + OpenAI key) is deferred to CI.
Refs: R6.1 #1355, R6.2 #1356, R6.3 #1359, R6.4 #1358 (all merged)
Co-authored-by: Tingting Zhou <zhoutt96@gmail.com>
Signed-off-by: Simo Lin <linsimo.mark@gmail.com>
slin1237
added a commit
that referenced
this pull request
Apr 24, 2026
Respond to the coordinator directive on PR #1365: remove the R6.3/R6.4 ``skip_for_runtime`` decorators, add local-gRPC fixtures for both engines, and harden the assertions on what the gateway emits. R6.1-R6.4 all merged. Real lane failures surfaced by this suite will be filed as R6.6/R6.7 follow-ups rather than patched from R6.5, so the CI signal is genuine rather than papered over. Engine matrix ------------- Three test classes, sharing a ``_ImageGenerationAssertions`` mix-in so the cloud + gRPC lanes stay strictly in lock-step: * ``TestImageGenerationCloud`` (vendor=openai, gpu=0) — OpenAI cloud backend with gpt-5-nano, mock MCP server standing in for the image backend. Exercises the OpenAI-compat router (R6.2). * ``TestImageGenerationGrpcSglang`` (engine=sglang, gpu=1, model=openai/gpt-oss-20b) — local SGLang worker via harmony. Exercises R6.3 gRPC-harmony wiring. * ``TestImageGenerationGrpcVllm`` (engine=vllm, gpu=1, model=meta-llama/Llama-3.1-8B-Instruct) — local vLLM worker via regular. Exercises R6.4 gRPC-regular wiring. Each lane's fixture wires its gateway at the same shared in-process ``MockMcpServer`` so deterministic base64-roundtrip / size-override assertions stay valid across engines. Fixture additions (``e2e_test/responses/conftest.py``) ------------------------------------------------------ * ``gateway_with_mock_mcp_cloud`` replaces the old ``gateway_with_mock_mcp``. Yields ``(gateway, client, mock, model)`` with ``model="gpt-5-nano"``. The old name is kept as a backward-compat alias so any unmerged branch referencing it still works. * ``_start_local_grpc_gateway_with_mcp`` helper: launches one gRPC worker for a given engine + model_id, spins up a gateway with ``--mcp-config-path`` pointing at the shared mock MCP config, wraps client instantiation in try/except so a failed init doesn't leak the gateway or the worker. * ``gateway_with_mock_mcp_grpc_sglang`` / ``gateway_with_mock_mcp_grpc_vllm``: class-scoped fixtures built on the helper. Each skips (not fails) when the gRPC worker can't start — CI lanes without GPUs otherwise poison every engine-parametrized suite. Harder assertions (``test_image_generation.py``) ------------------------------------------------ * ``_assert_image_generation_call_item`` now asserts every documented field (``type``, ``id`` ig-prefix, ``status``, ``result``, ``revised_prompt``) rather than just a subset. * ``_assert_streaming_envelope`` validates the full emitted envelope: * ``response.created`` → ``response.output_item.added`` → ``response.image_generation_call.{in_progress, generating, [partial_image], completed}`` → ``response.output_item.done`` → ``response.completed`` * each event's first occurrence strictly precedes the next required event's first occurrence * exactly one ``response.created`` / ``response.completed`` / ``output_item.added`` / ``output_item.done`` per image_gen call * ``sequence_number`` strictly monotonically increasing with no gaps > 1 (only checked when every event has one) * optional ``partial_image`` sits between ``generating`` and ``completed`` when present * Streaming and non-streaming paths share the same mix-in body, so the gRPC lanes exercise the full assertion surface without duplication. Local gates (ruff check/format, mypy, pytest --collect-only) clean; 12 tests collected (3 classes × 4 tests). Refs: R6.1 #1355, R6.2 #1356, R6.3 #1359, R6.4 #1358 (all merged) Co-authored-by: Tingting Zhou <zhoutt96@gmail.com> Signed-off-by: Simo Lin <linsimo.mark@gmail.com>
This was referenced Apr 24, 2026
slin1237
added a commit
that referenced
this pull request
Apr 24, 2026
Introduce e2e_test/infra/mock_mcp_server.py — a thin wrapper around the official MCP SDK's FastMCP base class that exposes streamable-HTTP transport on a local port. The existing MCP e2e test (e2e_test/messages/test_mcp_tool.py) depends on a live Brave MCP server which is flaky, slow, and non-deterministic; routing-layer tests for the gateway's built-in tool plumbing need an in-process, deterministic counterpart. This mock supplies one. Files added: * e2e_test/infra/mock_mcp_server.py — MockMcpServer class. Runs FastMCP via uvicorn on a background thread; auto-allocates a free port when the caller passes port=0. Registers an image_generation tool that returns a hard-coded 1x1 transparent PNG (92-char base64) plus the prompt echoed as revised_prompt, so tests can make byte-for-byte assertions. Records every call into call_log and surfaces the last call via last_call_args for override-verification tests. TODO stubs for web_search, file_search, code_interpreter sit inline with the same shape so future R6.x PRs just uncomment and adjust. * e2e_test/infra/mock_mcp.py — mock_mcp_server session-scoped pytest fixture plus IMAGE_GENERATION_PNG_BASE64 re-export. Files modified: * e2e_test/infra/constants.py — new MOCK_MCP_HOST constant (default 127.0.0.1) paralleling the existing BRAVE_MCP_HOST. * e2e_test/infra/__init__.py — export MockMcpServer, mock_mcp_server, IMAGE_GENERATION_PNG_BASE64, MOCK_MCP_HOST. Why streamable HTTP: matches the protocol the gateway's MCP client already speaks against Brave in production (see crates/mcp/src/core/config.rs::McpTransport::Streamable). The streamable_http_app() FastMCP method yields a /mcp Starlette route mountable directly under uvicorn. How to extend: FastMCP exposes a decorator-driven registration API; add a new @fastmcp.tool in MockMcpServer._register_tools, append to self._call_log inside the body for introspection, and you're done. See the module docstring for the extension recipe. Refs: R6.1 #1355, R6.2 #1356, R6.3 #1359, R6.4 #1358 (all merged) Co-authored-by: Tingting Zhou <zhoutt96@gmail.com> Signed-off-by: Simo Lin <linsimo.mark@gmail.com>
slin1237
added a commit
that referenced
this pull request
Apr 24, 2026
Add the first end-to-end tests for the image_generation built-in tool,
using the in-process MockMcpServer introduced in the previous commit.
R6.1-R6.4 wired image_generation across the openai, gRPC-regular, and
gRPC-harmony routers end-to-end; this PR provides the first automated
coverage that exercises those paths without external-service
dependencies.
Files added:
* e2e_test/responses/conftest.py — fixture module for the Responses
suite. Provides:
- mock_mcp_config_file: writes a session-scoped YAML config to a
tempdir that matches crates/mcp/src/core/config.rs::McpConfig,
registering the mock server with builtin_type: image_generation
and response_format: image_generation_call.
- gateway_with_mock_mcp: launches an OpenAI cloud gateway with
--mcp-config-path pointing at that tempfile; yields
(gateway, client, mock_mcp_server). Skips if OPENAI_API_KEY is
absent.
- image_gen_tool_args: canonical tool payload shared by all tests
so size/quality overrides have a clean starting point.
* e2e_test/responses/test_image_generation.py — TestImageGeneration
class with four tests on the OpenAI cloud backend:
1. test_image_generation_non_streaming: verifies the response
carries an ImageGenerationCall output item with the id ig_
prefix, status=completed, result=<mock base64>, and
revised_prompt echoing the input (R6.2 output-item emission).
2. test_image_generation_streaming: verifies the
response.image_generation_call.{in_progress,generating,
completed} events fire in the documented order; partial_image
is asserted optional-but-ordered-correctly (R6.2/R6.3/R6.4
streaming contract).
3. test_image_generation_tool_overrides_size: pins
size=512x512 and quality=high on the tool payload and asserts
the mock observed those exact values via last_call_args. This
catches compactor regressions where the override pipeline
would silently drop or rewrite user-provided arguments.
4. test_image_generation_compactor_strips_base64: creates a
stored conversation, fetches /v1/conversations/{id}/items, and
asserts the raw base64 bytes do NOT appear in persisted state
— so multi-turn replay never re-ships the image to the model
(R6.1 compactor behavior).
Engine matrix: openai only for now. skip_for_runtime("sglang") and
skip_for_runtime("vllm") guard the gRPC lanes until R6.3 and R6.4
stabilise in CI; a follow-up PR can drop those skip decorators once
the local-worker lanes are green.
Other changes:
* e2e_test/pyproject.toml — add mcp>=1.0 and uvicorn to the e2e test
deps. The mcp SDK is required by the mock server; uvicorn was
already pulled transitively but is called directly from the mock
server module and so we declare it explicitly.
Manual verification:
* ruff check e2e_test/responses/test_image_generation.py \
e2e_test/infra/mock_mcp_server.py e2e_test/infra/mock_mcp.py \
e2e_test/responses/conftest.py — clean.
* mypy on the same files with --ignore-missing-imports — clean.
* pytest e2e_test/responses/test_image_generation.py --collect-only —
4 tests discovered, no import errors.
* In-process smoke test of MockMcpServer via the mcp streamable_http
client confirmed the tool registration, deterministic payload, and
last_call_args introspection work end-to-end.
Full gated run (live gateway + OpenAI key) is deferred to CI.
Refs: R6.1 #1355, R6.2 #1356, R6.3 #1359, R6.4 #1358 (all merged)
Co-authored-by: Tingting Zhou <zhoutt96@gmail.com>
Signed-off-by: Simo Lin <linsimo.mark@gmail.com>
slin1237
added a commit
that referenced
this pull request
Apr 24, 2026
Respond to the coordinator directive on PR #1365: remove the R6.3/R6.4 ``skip_for_runtime`` decorators, add local-gRPC fixtures for both engines, and harden the assertions on what the gateway emits. R6.1-R6.4 all merged. Real lane failures surfaced by this suite will be filed as R6.6/R6.7 follow-ups rather than patched from R6.5, so the CI signal is genuine rather than papered over. Engine matrix ------------- Three test classes, sharing a ``_ImageGenerationAssertions`` mix-in so the cloud + gRPC lanes stay strictly in lock-step: * ``TestImageGenerationCloud`` (vendor=openai, gpu=0) — OpenAI cloud backend with gpt-5-nano, mock MCP server standing in for the image backend. Exercises the OpenAI-compat router (R6.2). * ``TestImageGenerationGrpcSglang`` (engine=sglang, gpu=1, model=openai/gpt-oss-20b) — local SGLang worker via harmony. Exercises R6.3 gRPC-harmony wiring. * ``TestImageGenerationGrpcVllm`` (engine=vllm, gpu=1, model=meta-llama/Llama-3.1-8B-Instruct) — local vLLM worker via regular. Exercises R6.4 gRPC-regular wiring. Each lane's fixture wires its gateway at the same shared in-process ``MockMcpServer`` so deterministic base64-roundtrip / size-override assertions stay valid across engines. Fixture additions (``e2e_test/responses/conftest.py``) ------------------------------------------------------ * ``gateway_with_mock_mcp_cloud`` replaces the old ``gateway_with_mock_mcp``. Yields ``(gateway, client, mock, model)`` with ``model="gpt-5-nano"``. The old name is kept as a backward-compat alias so any unmerged branch referencing it still works. * ``_start_local_grpc_gateway_with_mcp`` helper: launches one gRPC worker for a given engine + model_id, spins up a gateway with ``--mcp-config-path`` pointing at the shared mock MCP config, wraps client instantiation in try/except so a failed init doesn't leak the gateway or the worker. * ``gateway_with_mock_mcp_grpc_sglang`` / ``gateway_with_mock_mcp_grpc_vllm``: class-scoped fixtures built on the helper. Each skips (not fails) when the gRPC worker can't start — CI lanes without GPUs otherwise poison every engine-parametrized suite. Harder assertions (``test_image_generation.py``) ------------------------------------------------ * ``_assert_image_generation_call_item`` now asserts every documented field (``type``, ``id`` ig-prefix, ``status``, ``result``, ``revised_prompt``) rather than just a subset. * ``_assert_streaming_envelope`` validates the full emitted envelope: * ``response.created`` → ``response.output_item.added`` → ``response.image_generation_call.{in_progress, generating, [partial_image], completed}`` → ``response.output_item.done`` → ``response.completed`` * each event's first occurrence strictly precedes the next required event's first occurrence * exactly one ``response.created`` / ``response.completed`` / ``output_item.added`` / ``output_item.done`` per image_gen call * ``sequence_number`` strictly monotonically increasing with no gaps > 1 (only checked when every event has one) * optional ``partial_image`` sits between ``generating`` and ``completed`` when present * Streaming and non-streaming paths share the same mix-in body, so the gRPC lanes exercise the full assertion surface without duplication. Local gates (ruff check/format, mypy, pytest --collect-only) clean; 12 tests collected (3 classes × 4 tests). Refs: R6.1 #1355, R6.2 #1356, R6.3 #1359, R6.4 #1358 (all merged) Co-authored-by: Tingting Zhou <zhoutt96@gmail.com> Signed-off-by: Simo Lin <linsimo.mark@gmail.com>
slin1237
added a commit
that referenced
this pull request
Apr 24, 2026
Introduce e2e_test/infra/mock_mcp_server.py — a thin wrapper around the official MCP SDK's FastMCP base class that exposes streamable-HTTP transport on a local port. The existing MCP e2e test (e2e_test/messages/test_mcp_tool.py) depends on a live Brave MCP server which is flaky, slow, and non-deterministic; routing-layer tests for the gateway's built-in tool plumbing need an in-process, deterministic counterpart. This mock supplies one. Files added: * e2e_test/infra/mock_mcp_server.py — MockMcpServer class. Runs FastMCP via uvicorn on a background thread; auto-allocates a free port when the caller passes port=0. Registers an image_generation tool that returns a hard-coded 1x1 transparent PNG (92-char base64) plus the prompt echoed as revised_prompt, so tests can make byte-for-byte assertions. Records every call into call_log and surfaces the last call via last_call_args for override-verification tests. TODO stubs for web_search, file_search, code_interpreter sit inline with the same shape so future R6.x PRs just uncomment and adjust. * e2e_test/infra/mock_mcp.py — mock_mcp_server session-scoped pytest fixture plus IMAGE_GENERATION_PNG_BASE64 re-export. Files modified: * e2e_test/infra/constants.py — new MOCK_MCP_HOST constant (default 127.0.0.1) paralleling the existing BRAVE_MCP_HOST. * e2e_test/infra/__init__.py — export MockMcpServer, mock_mcp_server, IMAGE_GENERATION_PNG_BASE64, MOCK_MCP_HOST. Why streamable HTTP: matches the protocol the gateway's MCP client already speaks against Brave in production (see crates/mcp/src/core/config.rs::McpTransport::Streamable). The streamable_http_app() FastMCP method yields a /mcp Starlette route mountable directly under uvicorn. How to extend: FastMCP exposes a decorator-driven registration API; add a new @fastmcp.tool in MockMcpServer._register_tools, append to self._call_log inside the body for introspection, and you're done. See the module docstring for the extension recipe. Refs: R6.1 #1355, R6.2 #1356, R6.3 #1359, R6.4 #1358 (all merged) Co-authored-by: Tingting Zhou <zhoutt96@gmail.com> Signed-off-by: Simo Lin <linsimo.mark@gmail.com>
slin1237
added a commit
that referenced
this pull request
Apr 24, 2026
Add the first end-to-end tests for the image_generation built-in tool,
using the in-process MockMcpServer introduced in the previous commit.
R6.1-R6.4 wired image_generation across the openai, gRPC-regular, and
gRPC-harmony routers end-to-end; this PR provides the first automated
coverage that exercises those paths without external-service
dependencies.
Files added:
* e2e_test/responses/conftest.py — fixture module for the Responses
suite. Provides:
- mock_mcp_config_file: writes a session-scoped YAML config to a
tempdir that matches crates/mcp/src/core/config.rs::McpConfig,
registering the mock server with builtin_type: image_generation
and response_format: image_generation_call.
- gateway_with_mock_mcp: launches an OpenAI cloud gateway with
--mcp-config-path pointing at that tempfile; yields
(gateway, client, mock_mcp_server). Skips if OPENAI_API_KEY is
absent.
- image_gen_tool_args: canonical tool payload shared by all tests
so size/quality overrides have a clean starting point.
* e2e_test/responses/test_image_generation.py — TestImageGeneration
class with four tests on the OpenAI cloud backend:
1. test_image_generation_non_streaming: verifies the response
carries an ImageGenerationCall output item with the id ig_
prefix, status=completed, result=<mock base64>, and
revised_prompt echoing the input (R6.2 output-item emission).
2. test_image_generation_streaming: verifies the
response.image_generation_call.{in_progress,generating,
completed} events fire in the documented order; partial_image
is asserted optional-but-ordered-correctly (R6.2/R6.3/R6.4
streaming contract).
3. test_image_generation_tool_overrides_size: pins
size=512x512 and quality=high on the tool payload and asserts
the mock observed those exact values via last_call_args. This
catches compactor regressions where the override pipeline
would silently drop or rewrite user-provided arguments.
4. test_image_generation_compactor_strips_base64: creates a
stored conversation, fetches /v1/conversations/{id}/items, and
asserts the raw base64 bytes do NOT appear in persisted state
— so multi-turn replay never re-ships the image to the model
(R6.1 compactor behavior).
Engine matrix: openai only for now. skip_for_runtime("sglang") and
skip_for_runtime("vllm") guard the gRPC lanes until R6.3 and R6.4
stabilise in CI; a follow-up PR can drop those skip decorators once
the local-worker lanes are green.
Other changes:
* e2e_test/pyproject.toml — add mcp>=1.0 and uvicorn to the e2e test
deps. The mcp SDK is required by the mock server; uvicorn was
already pulled transitively but is called directly from the mock
server module and so we declare it explicitly.
Manual verification:
* ruff check e2e_test/responses/test_image_generation.py \
e2e_test/infra/mock_mcp_server.py e2e_test/infra/mock_mcp.py \
e2e_test/responses/conftest.py — clean.
* mypy on the same files with --ignore-missing-imports — clean.
* pytest e2e_test/responses/test_image_generation.py --collect-only —
4 tests discovered, no import errors.
* In-process smoke test of MockMcpServer via the mcp streamable_http
client confirmed the tool registration, deterministic payload, and
last_call_args introspection work end-to-end.
Full gated run (live gateway + OpenAI key) is deferred to CI.
Refs: R6.1 #1355, R6.2 #1356, R6.3 #1359, R6.4 #1358 (all merged)
Co-authored-by: Tingting Zhou <zhoutt96@gmail.com>
Signed-off-by: Simo Lin <linsimo.mark@gmail.com>
slin1237
added a commit
that referenced
this pull request
Apr 24, 2026
Respond to the coordinator directive on PR #1365: remove the R6.3/R6.4 ``skip_for_runtime`` decorators, add local-gRPC fixtures for both engines, and harden the assertions on what the gateway emits. R6.1-R6.4 all merged. Real lane failures surfaced by this suite will be filed as R6.6/R6.7 follow-ups rather than patched from R6.5, so the CI signal is genuine rather than papered over. Engine matrix ------------- Three test classes, sharing a ``_ImageGenerationAssertions`` mix-in so the cloud + gRPC lanes stay strictly in lock-step: * ``TestImageGenerationCloud`` (vendor=openai, gpu=0) — OpenAI cloud backend with gpt-5-nano, mock MCP server standing in for the image backend. Exercises the OpenAI-compat router (R6.2). * ``TestImageGenerationGrpcSglang`` (engine=sglang, gpu=1, model=openai/gpt-oss-20b) — local SGLang worker via harmony. Exercises R6.3 gRPC-harmony wiring. * ``TestImageGenerationGrpcVllm`` (engine=vllm, gpu=1, model=meta-llama/Llama-3.1-8B-Instruct) — local vLLM worker via regular. Exercises R6.4 gRPC-regular wiring. Each lane's fixture wires its gateway at the same shared in-process ``MockMcpServer`` so deterministic base64-roundtrip / size-override assertions stay valid across engines. Fixture additions (``e2e_test/responses/conftest.py``) ------------------------------------------------------ * ``gateway_with_mock_mcp_cloud`` replaces the old ``gateway_with_mock_mcp``. Yields ``(gateway, client, mock, model)`` with ``model="gpt-5-nano"``. The old name is kept as a backward-compat alias so any unmerged branch referencing it still works. * ``_start_local_grpc_gateway_with_mcp`` helper: launches one gRPC worker for a given engine + model_id, spins up a gateway with ``--mcp-config-path`` pointing at the shared mock MCP config, wraps client instantiation in try/except so a failed init doesn't leak the gateway or the worker. * ``gateway_with_mock_mcp_grpc_sglang`` / ``gateway_with_mock_mcp_grpc_vllm``: class-scoped fixtures built on the helper. Each skips (not fails) when the gRPC worker can't start — CI lanes without GPUs otherwise poison every engine-parametrized suite. Harder assertions (``test_image_generation.py``) ------------------------------------------------ * ``_assert_image_generation_call_item`` now asserts every documented field (``type``, ``id`` ig-prefix, ``status``, ``result``, ``revised_prompt``) rather than just a subset. * ``_assert_streaming_envelope`` validates the full emitted envelope: * ``response.created`` → ``response.output_item.added`` → ``response.image_generation_call.{in_progress, generating, [partial_image], completed}`` → ``response.output_item.done`` → ``response.completed`` * each event's first occurrence strictly precedes the next required event's first occurrence * exactly one ``response.created`` / ``response.completed`` / ``output_item.added`` / ``output_item.done`` per image_gen call * ``sequence_number`` strictly monotonically increasing with no gaps > 1 (only checked when every event has one) * optional ``partial_image`` sits between ``generating`` and ``completed`` when present * Streaming and non-streaming paths share the same mix-in body, so the gRPC lanes exercise the full assertion surface without duplication. Local gates (ruff check/format, mypy, pytest --collect-only) clean; 12 tests collected (3 classes × 4 tests). Refs: R6.1 #1355, R6.2 #1356, R6.3 #1359, R6.4 #1358 (all merged) Co-authored-by: Tingting Zhou <zhoutt96@gmail.com> Signed-off-by: Simo Lin <linsimo.mark@gmail.com>
This was referenced Apr 24, 2026
7 of 8 tasks
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
1 participant
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.
Summary
Audit item R6 of
.claude/_audit/responses-api-gap-audit.mdcalls forimage_generationto be plumbed through the same hosted-tool MCP pathalready wired for
web_search/code_interpreter/file_search.This PR is 1 of 4 — it adds only the shared infrastructure so
PRs R6.2, R6.3, and R6.4 (router-specific event emission:
OpenAI router, harmony gRPC, regular gRPC) can be built in parallel on
top.
Supersedes PR #1352 (closed) and PR #1091; avoids their
unreachable!()stubs by using real neutral values that keep runtime safe until real
wiring lands.
What's in this PR
Layered additions, each mirroring the existing shape of the other
three hosted tools:
Protocol layer (
crates/protocols/src/event_types.rs)ImageGenerationCallEvent { InProgress, Generating, PartialImage, Completed }enum withresponse.image_generation_call.*constants,as_str(), andDisplay. Spec: OpenAI SDK v2.8.1response_image_gen_call_*_event.py+.claude/_audit/openai-responses-api-spec.md.ItemType::ImageGenerationCall+IMAGE_GENERATION_CALLconstant;is_builtin_tool_call()extended.MCP config layer (
crates/mcp/src/core/config.rs)BuiltinToolType::ImageGeneration(Display = "image_generation",response_format()→ResponseFormatConfig::ImageGenerationCall).ResponseFormatConfig::ImageGenerationCallvariant.MCP transform layer (
crates/mcp/src/transform/)ResponseFormat::ImageGenerationCallvariant +FromResponseFormatConfigarm + serde round-trip test.ResponseTransformer::transformdispatches toto_image_generation_call(result, tool_call_id)which maps MCPCallToolResult→ResponseOutputItem::ImageGenerationCallusingonly T4's actual fields:
id,result(base64),revised_prompt?,status. No invented fields.openai_responsetext-block payloads (mirrorsto_web_search_call).compact_image_generation_output(item)helper strips thebase64
resultpayload for stored multi-turn context (no-op fornon-image items). Re-exported from
transformand the crate root.b64_jsonalias, embedded text blocks,missing-image fallback, compactor strip + no-op.
Router shared streaming
(
model_gateway/src/routers/grpc/common/responses/streaming.rs)emit_tool_call_in_progress/emit_tool_call_searching(emitsgeneratingas intermediate) /emit_tool_call_completednowhandle
ImageGenerationCall.emit_image_generation_partial_image(...)helper for theimage-specific
partial_imageevent withpartial_image_index+partial_image_b64payload. Gated onformat; per-router wiring in R6.2/R6.3/R6.4 decides when to call
it. Marked
#[expect(dead_code, reason="...")]until then.type_str_for_format/output_item_type_for_format/OutputItemTypevariants +allocate_output_indexid prefix"ig"all extended.Router common util
(
model_gateway/src/routers/common/mcp_utils.rs)collect_builtin_routingandextract_builtin_typesrecognizeResponseTool::ImageGeneration(_)→BuiltinToolType::ImageGeneration.test_collect_builtin_routing_image_generation.Stub arms in exhaustive ResponseFormat matches
(
model_gateway/src/routers/openai/mcp/tool_loop.rs)Option X —
ResponseFormatremains non-#[non_exhaustive], so thecompiler still enforces exhaustiveness at every site. Stubs use real
neutral values (no
todo!()/unreachable!()/panic!()):send_tool_call_intermediate_eventImageGenerationCallEvent::GENERATINGsend_tool_call_completion_eventsItemType::IMAGE_GENERATION_CALL→ImageGenerationCallEvent::COMPLETEDstable_streaming_tool_item_id"ig_"non_streaming_tool_item_id_sourcestrip_prefix("fc_"|"call_")armSites using
matches!(...)/_ =>wildcards (harmony streaming,regular gRPC streaming, OpenAI
responses::streaming) do notrequire stubs because the compiler does not enforce exhaustiveness
there. They preserve their existing default behavior; PRs
R6.2/R6.3/R6.4 own extending them.
Why / How
rather than inventing a new one — reviewers only need to verify
the pattern was applied consistently.
#[non_exhaustive]so the compiler keeps enforcing coverage atevery site as new hosted tools are added in the future.
result: Stringfalls back to an empty stringwhen the MCP server returns no image data; per-router code in
R6.2/R6.3/R6.4 owns surfacing a user-visible error status.
compact_image_generation_outputis needed so multi-turn storagedoesn't balloon with base64 image payloads; the input-side
ResponseInputOutputItem::ImageGenerationCallalready allowsresultto be absent (id-only reference), so this is aspec-compatible storage optimization.
Non-goals for R6.1
Explicitly left for the follow-up PRs so this PR stays small and
reviewable:
the shared emitter provides. Harmony and regular gRPC streaming
paths are untouched.
BuiltinToolTypevariant — operators register their image-gen MCP server through
existing config mechanisms.
crates/protocols/src/responses.rs(T4 owns thosetypes).
is being handled separately in PR fix(harmony): shrink BUILTIN_TOOLS to the gpt-oss-native tool set #1353.
model_gateway/src/routers/common/tool_overrides.rs—reviewed and deemed out-of-scope for R6.1 infrastructure; if
caller-pinned
ResponseTool::ImageGeneration(...)config mergingis needed, it'll land as a follow-up PR or inside R6.2/R6.3/R6.4.
Test plan
cargo check -p openai-protocol -p smg-mcp -p smg --lib --tests --benchescargo test -p openai-protocol -p smg-mcp -p smg --lib— 855tests pass, 8 new image_generation tests green:
-
test_image_generation_transform_direct_fields-
test_image_generation_transform_b64_json_alias-
test_image_generation_transform_embedded_openai_response-
test_image_generation_transform_missing_image_data-
test_compact_image_generation_output_strips_base64-
test_compact_image_generation_output_is_noop_for_other_items-
test_collect_builtin_routing_image_generation- serde round-trips for the three new enum variants
cargo fmt --allcargo clippy -p openai-protocol -p smg-mcp -p smg --lib --tests --benches -- -D warningsChecklist
Refs: R6 in
.claude/_audit/responses-api-gap-audit.mdSummary by CodeRabbit