feat(grpc-harmony): wire image_generation streaming events (R6.3) - #1359
Merged
Merged
Conversation
Replaces the implicit stub in Harmony's streaming router that gated argument-streaming on `matches!(response_format, Some(Passthrough) | None)`. The negative pattern silently routed `ImageGenerationCall` down the builtin structured-event path by accident of pattern exhaustion. R6.1 (#1355) landed the shared emitter helpers (`emit_tool_call_in_progress` / `emit_tool_call_searching` / `emit_tool_call_completed` + `emit_image_generation_partial_image`, plus the `image_generation_call` entries in `type_str_for_format` / `output_item_type_for_format` / `allocate_output_index`) — but the gRPC Harmony router still identified builtins via the opaque negative match. This PR is R6.3 of 4, wiring the intent explicitly for Harmony. Harmony dispatches hosted-tool calls (web_search / code_interpreter / file_search / image_generation) through the gateway's MCP layer, not through gpt-oss native builtin tokens. The streaming events for `image_generation_call` therefore fire when the Harmony parser surfaces the commentary-channel tool call, while the actual image generation runs via `execute_mcp_tools`. Argument streaming must stay skipped for all four builtins so the shared emitter's structured events are the single source of progress signal. Changes (model_gateway/src/routers/grpc/harmony/streaming.rs): - Introduce `streams_arguments(Option<&ResponseFormat>) -> bool` as a private exhaustive classifier. `None` (function_call) and `Some(Passthrough)` (mcp_call) stream arguments; all four hosted builtins — WebSearchCall, CodeInterpreterCall, FileSearchCall, ImageGenerationCall — do not. Using `match` instead of `matches!` forces a compile error if a new `ResponseFormat` variant lands without a classification decision. - Replace the four `matches!(response_format, Some(Passthrough) | None)` sites in `process_decode_stream` with `streams_arguments(...)` calls, swapping the terse opaque gate for named intent: * ~L744-L761: initial `arguments.delta` for mcp_call / function_call * ~L771-L785: continuing `arguments.delta` during streaming * ~L838-L860: `arguments.done` in the Complete branch * ~L976-L996: `arguments.done` in the extracted-commentary branch Behavior for WebSearchCall / CodeInterpreterCall / FileSearchCall / ImageGenerationCall is identical to before (all four skip argument streaming); the change is documentation and exhaustiveness, so future readers and the compiler both see the intent. - Expand the surrounding comments to name the four builtins and explain that their progress surfaces via the `*.in_progress` / `*.searching` / `*.generating` / `*.completed` structured events emitted by the shared `ResponseStreamEventEmitter`. - Add a `#[cfg(test)]` unit test `streams_arguments_matches_passthrough_and_function_only` that iterates every `ResponseFormat` variant (plus `None`) and asserts the classification. The inner `match format { ... }` is exhaustive and will fail to compile if a new variant is added without being placed on one side of the `true` / `false` split. Scope: strictly `model_gateway/src/routers/grpc/harmony/streaming.rs`. `model_gateway/src/routers/grpc/common/responses/streaming.rs` is R6.1's territory (and is shared by the regular router). The regular router's per-site wiring lives in R6.4. The OpenAI (HTTP) router's per-site wiring lives in R6.2. No changes outside the Harmony router. Gates (with CARGO_TARGET_DIR=/Users/simolin/.cargo/target): - cargo check -p smg --lib — clean - cargo test -p smg --lib — 617 passed, 4 ignored, 0 failed (including the new routers::grpc::harmony::streaming::tests::streams_arguments_matches_passthrough_and_function_only) - cargo fmt --all — no diffs - cargo clippy -p smg --lib --tests -- -D warnings — clean Refs: R6 (image_generation hosted-tool plumbing), R6.1 (#1355) Signed-off-by: Simo Lin <linsimo.mark@gmail.com> Co-authored-by: Tingting Zhou <zhoutt96@gmail.com>
|
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)
📝 WalkthroughWalkthroughThis PR introduces a centralized Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 a streams_arguments helper function to centralize the logic for determining whether a tool call should stream arguments or emit structured events, refactoring the HarmonyStreamingProcessor accordingly. A unit test was added to verify this logic; however, feedback suggests that the test's reliance on a manually maintained array of enum variants could bypass checks for newly added variants, potentially undermining the goal of compile-time exhaustiveness.
Comment on lines
+1116
to
+1122
| for format in [ | ||
| ResponseFormat::Passthrough, | ||
| ResponseFormat::WebSearchCall, | ||
| ResponseFormat::CodeInterpreterCall, | ||
| ResponseFormat::FileSearchCall, | ||
| ResponseFormat::ImageGenerationCall, | ||
| ] { |
Contributor
There was a problem hiding this comment.
The test relies on a manually maintained array of ResponseFormat variants. While the match statement on line 1123 is exhaustive, a developer might update the match but forget to add the new variant to this array. This results in the test passing without exercising the new variant, which undermines the goal of using compile-time exhaustiveness checks to catch specification divergences. Please ensure the test structure prioritizes fail-fast behavior or compile-time safety to stay consistent with the repository's approach to external API types.
References
- Prioritize fail-fast behavior and leverage compile-time exhaustiveness checks for external specifications to ensure divergences are surfaced early.
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Clean refactor. The exhaustive match in streams_arguments plus the test lock are the right approach for ensuring new ResponseFormat variants get explicitly classified. All four call sites are mechanically equivalent to the old matches! checks. No issues found.
github-actions
Bot
added
grpc
…xhaustiveness Addresses the gemini-code-assist review concern on PR #1359 that the original test relied on a manually maintained array of `ResponseFormat` variants, which could drift from the enum and bypass the compile-time exhaustiveness guarantee. Changes (model_gateway/src/routers/grpc/harmony/streaming.rs, test mod): - Add a new private `expected_streams_arguments(&ResponseFormat) -> bool` helper inside `mod tests`. It is a wildcard-free `match` that mirrors the production classifier without calling it. Drift between the two surfaces as a runtime assertion failure; a missing variant surfaces as a compile error in this helper (and, transitively, in the test that references every variant by name). - Rewrite `streams_arguments_explicit_variants` to bind each `ResponseFormat::*` variant to a named `let` and assert both `streams_arguments` and `expected_streams_arguments` on it, plus `None`. No `for` loop, no array of variants — every variant is spelled out, so adding a new variant forces every production and test site to be updated explicitly before it compiles. - Rename the test to `streams_arguments_explicit_variants` to reflect the new structure and add a doc comment explaining why both helpers exist (compile-error vs runtime-assertion failure modes are intentionally split). No behavior change. The classifier and the four call sites in `process_decode_stream` are unchanged. Gates (with CARGO_TARGET_DIR=/Users/simolin/.cargo/target): - cargo check -p smg --lib — clean - cargo test -p smg --lib — 617 passed, 4 ignored, 0 failed (including the renamed routers::grpc::harmony::streaming::tests::streams_arguments_explicit_variants) - cargo fmt --all — no diffs - cargo clippy -p smg --lib --tests -- -D warnings — clean Refs: #1359 (R6.3) Signed-off-by: Simo Lin <linsimo.mark@gmail.com> Co-authored-by: Tingting Zhou <zhoutt96@gmail.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Member
Author
|
The
Both assert Scope of this PR is strictly Will re-trigger CI on the next push if needed; not blocking on this. |
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>
This was referenced Apr 24, 2026
1 of 4 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>
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 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
R6.3 of the 4-PR split for the
image_generationhosted-tool audit (R6). Wires the gRPC Harmony router's per-site handling ofImageGenerationCallso it explicitly follows the structured-event path thatWebSearchCall,CodeInterpreterCall, andFileSearchCallalready use. No behavior change for existing hosted tools — this is an explicit-intent + exhaustiveness refactor that removes the implicit stub R6.1 deliberately left in Harmony for per-router PRs.Refs: R6 image_generation integration. Builds on R6.1 (#1355). Sibling PRs: R6.2 (openai HTTP router) and R6.4 (gRPC regular router).
Problem
R6.1 (#1355) landed the shared streaming infrastructure for
image_generation_callinmodel_gateway/src/routers/grpc/common/responses/streaming.rs:ResponseFormat::ImageGenerationCallarms toemit_tool_call_in_progress/emit_tool_call_searching/emit_tool_call_completed.emit_image_generation_partial_image(marked#[expect(dead_code)]until per-router PRs wire it up).image_generation_callentries intype_str_for_format/output_item_type_for_format/allocate_output_index.The gRPC Harmony router's
process_decode_streaminmodel_gateway/src/routers/grpc/harmony/streaming.rsgates argument streaming on a negative pattern:That pattern quietly excluded every hosted built-in — including the newly added
ImageGenerationCall— by pattern exhaustion rather than by deliberate naming. When a newResponseFormatvariant lands (as R6.1 just did), the compiler gives no signal that the variant is unclassified. The behavior was correct by coincidence; the intent was implicit.Solution
Make the classification explicit, named, and compiler-checked — same structured-event path that
WebSearchCall/CodeInterpreterCall/FileSearchCallalready take in the shared emitter.What changed
model_gateway/src/routers/grpc/harmony/streaming.rsAdded a private
streams_arguments(Option<&ResponseFormat>) -> boolclassifier:match(notmatches!) so adding a newResponseFormatvariant is a compile error until explicitly classified. Doc comment names all four hosted builtins and points at the shared emitter helpers (includingemit_image_generation_partial_image).Replaced the four
matches!(response_format, Some(ResponseFormat::Passthrough) | None)call sites withstreams_arguments(response_format.as_ref()):arguments.deltaon new tool call (mcp_call / function_call only).arguments.deltaduring streaming (skip for builtins).arguments.donein theCompletebranch after parser finalize.arguments.donein the fallback branch that extracts commentary from the parser when Complete hadn't emitted tool calls.Rewrote the surrounding comments to name the four hosted builtins (web_search_call, code_interpreter_call, file_search_call, image_generation_call) and point at the structured
*.in_progress/*.searching/*.generating/*.completedevents the sharedResponseStreamEventEmitteremits. Replaces the old opaque "builtin tools" phrasing.Added unit test
streams_arguments_matches_passthrough_and_function_onlyunder#[cfg(test)] mod teststhat iterates everyResponseFormatvariant plusNoneand locks the classification. The innermatch format { ... }inside the test is also exhaustive, so a new variant fails to compile in the test too.Why this shape (not just keep
matches!)The point of R6 is that
image_generationis a first-class hosted built-in. R6.1 added it to every dispatch surface in the shared emitter. Harmony's negativeSome(Passthrough) | Nonegate was the only remaining site whereImageGenerationCalllanded on the correct path by pattern accident. After this PR, the decision is named, compiler-enforced, and explained inline, and the classification is test-locked. The runtime behavior is bit-identical.Harmony-specific nuance (why image_generation rides MCP dispatch on Harmony)
Harmony is designed for gpt-oss, which was not trained on
image_generationas a native builtin (see PR #1353 and the separatefix(harmony): shrink BUILTIN_TOOLS to the gpt-oss-native tool setwork). On the Harmony path,image_generationtool calls come through the gateway-level MCP dispatch (execute_mcp_toolsinharmony/responses/execution.rs), not from model tokens. The streaming events (response.image_generation_call.in_progress/.generating/.completed) fire from the shared emitter when the Harmony parser surfaces a commentary-channel tool call, while the actual image generation runs in the MCP orchestrator. Argument streaming must stay skipped so the structured events are the single source of progress signal — which is exactly whatstreams_arguments(...) == falseenforces forImageGenerationCall.Scope boundary
Hard-limited to
model_gateway/src/routers/grpc/harmony/streaming.rsper the R6.3 task contract.model_gateway/src/routers/grpc/common/responses/streaming.rsis R6.1's territory (landed in feat(gateway): add image_generation infrastructure for hosted-tool MCP plumbing (R6.1) #1355) and is shared by the regular router — untouched here.model_gateway/src/routers/grpc/regular/is R6.4's territory — untouched here.model_gateway/src/routers/openai/mcp/tool_loop.rsis R6.2's territory — untouched here.grep -rnE "ResponseFormat::ImageGenerationCall" model_gateway/src/routers/grpc/harmony/confirmed no other stub sites in the Harmony tree.Test plan
Run from
model_gateway/src/routers/grpc/harmony/withCARGO_TARGET_DIR=/Users/simolin/.cargo/target:cargo check -p smg --lib— cleancargo test -p smg --lib— 617 passed, 4 ignored, 0 failed, including the newrouters::grpc::harmony::streaming::tests::streams_arguments_matches_passthrough_and_function_onlycargo fmt --all— no diffs (re-ran--check)cargo clippy -p smg --lib --tests -- -D warnings— cleanThe unit test double-locks the classification:
streams_argumentsreturnstrueforNoneandSome(Passthrough),falsefor the four hosted builtins.match formatinside the test is itself exhaustive, so any newResponseFormatvariant fails to compile in both the production path and the test.No source behavior change for existing hosted tools;
ImageGenerationCallnow rides the same structured-event path it was already (accidentally) on, but explicitly and compiler-enforced.Refs: #1355 (R6.1)
Summary by CodeRabbit
Refactor
Tests