Skip to content

test: verify Codex image attachments and image tool outputs through Responses (#253) - #263

Open
Zheng-Lu wants to merge 3 commits into
vllm-project:mainfrom
Zheng-Lu:feat/253-verify-codex-images
Open

Zheng-Lu wants to merge 3 commits into
vllm-project:mainfrom
Zheng-Lu:feat/253-verify-codex-images

Conversation

@Zheng-Lu

@Zheng-Lu Zheng-Lu commented Sep 6, 2026

Copy link
Copy Markdown
Contributor

Summary

Closes #253 as a pure gateway PR, per the sequencing agreed in #261: establish and record the gateway's image-input compatibility with the OpenAI Responses API first; #261 then only has to allow the modality in the Codex catalog/launcher and add the client verification.

This PR is no longer stacked on #261. It is rebased on main and carries no launcher, catalog, or codex-smoke.sh changes (those are parked locally and will be added to #261 after this merges).

Scope: image input (input_image parts in messages and in client-executed tool output) through /v1/responses. The image_generation built-in tool is intentionally out of scope; if it is wanted, it will be tracked in a follow-up issue.

Gateway fix

  • Strict Content Validation (Fail-fast): Message content parts that the gateway cannot represent (like input_audio) are now explicitly rejected with an HTTP 400 invalid_request_error (e.g., message content part type `input_audio` is not supported), rather than being silently dropped or forwarded as a synthetic {"type": "unknown"} part. Empty content arrays are similarly rejected.
  • Data Integrity: InputContent::Unknown is now explicitly skipped during serialization (#[serde(skip)]), providing a compile-time guarantee that unmodeled content is "never forwarded, never stored", preventing database corruption.
  • Refusal Support: Modeled the refusal message content type to ensure valid assistant refusal history survives rehydration and replay without being erroneously pruned.

Recorded OpenAI compatibility

  • New recorder script record_image_input_cassettes.sh records the same two-turn conversation against OpenAI and against the gateway pointed at the same gpt-4o upstream, streaming and non-streaming (4 cassettes, 8 turns). Turn 1 sends a committed 64×64 PNG inline as an input_image data URL beside input_text; turn 2 continues it via previous_response_id. Recordings are validated (fixture bytes preserved, previous_response_id chained, completed message present) and staged before promotion.
  • record_cassette.py now accepts --input-file for the first of several turns so a structured opening turn can be continued by previous_response_id in one recording.
  • image_input_test.rs replays both providers and asserts protocol structure only — never model wording:
    • request bodies identical across providers and byte-equal to the committed fixture (the test re-encodes the PNG and checks the data URL);
    • completed response shape: one assistant message with output_text, usage, status: completed;
    • streaming: contiguous sequence_numbers, deltas reconcile with output_text.done, and an identical normalized lifecycle for OpenAI and the gateway (response.created → in_progress → output_item.added:message → content_part.added → output_text.delta → output_text.done → content_part.done → output_item.done:message → response.completed), also asserted on the events the gateway itself emits during replay;
    • continuation: the history the gateway rehydrates and forwards upstream is [user(text + image), assistant(output_text), user(follow-up)] with the image part byte for byte intact.

Deterministic gateway coverage (HTTP + WebSocket)

Mixed text/image ordering, multiple images per message, client-executed view_image tool output, previous_response_id continuation, conversation rehydration, stateless store: false proxying, and compaction of retained image-bearing user messages.

Test Plan

  • cargo fmt -- --check
  • cargo clippy --workspace --all-targets -- -D warnings
  • env -u http_proxy -u HTTP_PROXY -u https_proxy -u HTTPS_PROXY NO_PROXY=127.0.0.1,localhost cargo test --workspace — 1396 passed, 0 failed
  • python3 scripts/validate-cassettes.py — 137/137 cassettes, 304 turns
  • pre-commit run --all-files
  • Cassettes recorded live with OPENAI_API_KEY against api.openai.com (gpt-4o, detail: low) and against a local gateway started with --llm-api-base https://api.openai.com --skip-llm-ready-check, using crates/agentic-server-core/tests/cassettes/record_image_input_cassettes.sh.

@LOGO127

LOGO127 commented Sep 6, 2026

Copy link
Copy Markdown
Contributor

Thanks for picking up #253. I had an unpublished local image/replay candidate and am pausing its publication to avoid duplicating this PR.

There is one integration seam with my #258 (fab52ff): a no-commit merge into your 5318fb5 has conflicts in three core files: types/io/input.rs, types/io/output.rs, and executor/compaction.rs.

The local resolution keeps one shared InputContent::InputFile variant and your unknown-part filtering; it preserves #258's typed-path rejection before inference/compaction, without changing eligible raw-proxy traffic or structured tool-output files. The nested-content regression retains both your file/audio cases and #258's future-content case, so neither assertion is discarded.

rustfmt and the 17 replay-server Python tests pass for that combination. Full Rust verification is still pending: compilation completed, but the run was interrupted by filesystem I/O failures in my local WSL environment. This is not a passing Rust-suite, upstream-CI, or live vision-model result. Neither published branch has changed, and no mixed commit/PR has been created.

I'll keep #258's validation contract and this PR's image-preservation tests together when checking the eventual integration. The original test/replay implementation here remains yours; I am withdrawing the overlapping publication plan, not requesting ownership of #253.

@Zheng-Lu

Zheng-Lu commented Sep 6, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for picking up #253. I had an unpublished local image/replay candidate and am pausing its publication to avoid duplicating this PR.

There is one integration seam with my #258 (fab52ff): a no-commit merge into your 5318fb5 has conflicts in three core files: types/io/input.rs, types/io/output.rs, and executor/compaction.rs.

The local resolution keeps one shared InputContent::InputFile variant and your unknown-part filtering; it preserves #258's typed-path rejection before inference/compaction, without changing eligible raw-proxy traffic or structured tool-output files. The nested-content regression retains both your file/audio cases and #258's future-content case, so neither assertion is discarded.

rustfmt and the 17 replay-server Python tests pass for that combination. Full Rust verification is still pending: compilation completed, but the run was interrupted by filesystem I/O failures in my local WSL environment. This is not a passing Rust-suite, upstream-CI, or live vision-model result. Neither published branch has changed, and no mixed commit/PR has been created.

I'll keep #258's validation contract and this PR's image-preservation tests together when checking the eventual integration. The original test/replay implementation here remains yours; I am withdrawing the overlapping publication plan, not requesting ownership of #253.

Thanks for reaching out.

The resolution strategy you tested locally sounds great: keeping InputContent::InputFile along with the unknown-part filtering, while incorporating #258's typed-path validation and combining the regression test cases, is a very clean and complete solution.

Whenever #258 (or #252) lands, I'm happy to rebase and align with that resolution, or vice versa if this PR merges first. Thanks again for the thoughtful heads-up and collaborative spirit!

@LOGO127

LOGO127 commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

AI-assisted follow-up from LOGO127: I found a separate replay-fixture concurrency issue while adding complementary coverage against your exact head 5318fb5ee3692f2a8053301c0e3ed4f1a0b173ab.

On Windows / Python 3.13.6, the unchanged append_capture implementation loses JSONL records when multiple handler threads append concurrently. A new file-level regression starts 8 writers, each appending 32 records with a 16 KiB payload. All 10 baseline repetitions failed: 208-227 records survived instead of 256. This is a Windows reproduction, not evidence that your Linux Actions job has failed.

A local two-file candidate serializes each record's open/write/flush/close with a process-local lock and adds that regression plus a six-request HTTP check (one recorded turn consumed, one 200 and five 409 responses, all 12 capture records retained). After the fix:

  • All 19 replay tests pass on Windows Python 3.12 and 3.13: your existing 17 tests plus these two additions.
  • The two new cases pass 50 file-level and 20 HTTP repetitions on Python 3.13; these are repeats, not 70 distinct cases.
  • All applicable hooks for the two changed files pass.

No Rust, cassette, model-catalog, or image-validation behavior changes are involved. I have not verified this on Linux, with a live vision model, or across multiple writer processes. The separate #258 integration still needs its full Rust verification.

Would you prefer this fixture fix as a separate follow-up, or folded into #263? I have kept the candidate local pending personal review and scope agreement; no competing #253 PR or signed/public patch has been created.

@LOGO127

LOGO127 commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

AI-assisted scope proposal from LOGO127, following the separate capture-concurrency note above:

I prepared a complementary fixture-only extension against unchanged 5318fb5, without modifying your Rust/image implementation: optional assert-capture --expect-tool-image CALL_ID SHA256 checks structured function/custom tool outputs by call ID and exact ordered PNG digests. It rejects stringified arrays, wrong/duplicate calls, and missing/extra/reordered images. Message attachments cannot satisfy tool-output assertions, and the existing attachment assertion is unchanged.

All 28 replay tests pass on native Windows Python 3.12 and 3.13 (your 17 plus 11 new methods); coverage includes real loopback HTTP capture and Python CLI subprocesses. Applicable three-file hooks pass. This remains a single outgoing-request capture assertion, not a new multi-round runner or evidence of real Codex tool execution, Rust/WebSocket correctness, or live-model understanding. No cassette changed. The earlier two-file concurrency candidate remains separate.

Would maintaining capture assertions and fixture concurrency be a useful complementary slice for me to take on, as a follow-up to your #263? The new extension is still local, pending scope agreement and personal review/sign-off; I have not opened a competing #253 implementation.

@LOGO127

LOGO127 commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

A follow-up closing the local Rust-validation gap in my earlier integration report: the unchanged combination of your 5318fb5 head and #258 fab52ff now passes the full workspace/all-features tests on WSL Linux with Rust 1.98.0: 979 passed, 0 failed, 9 ignored (excluding two duplicate nested subprocess summaries). Strict workspace/all-targets/all-features Clippy with -D warnings also passes.

I verified all 448 source files and executable flags against the original integration archive (tree 5a1628c224a104bc3a056e267c5e51caa06f089b) before running. The three-conflict resolution is unchanged: one shared InputFile variant, your unknown-part filtering, #258's typed-path rejection, and the combined nested-content regressions. The previous I/O failure no longer prevents these checks; no source or cassette changes were needed.

Commands: cargo +1.98.0 test --workspace --all-features --locked --offline --no-fail-fast --quiet and cargo +1.98.0 clippy --workspace --all-targets --all-features --locked --offline -- -D warnings.

This is evidence for that exact local combination, not current-main/upstream CI or live vision-model validation. The ignored tests remain ignored; PostgreSQL acceptance is not claimed. The separate capture-concurrency/assertion proposals are not included. Neither published branch has been changed, and I have not signed or published the mixed integration tree. AI-assisted verification; thanks again for coordinating the resolution.

@Zheng-Lu
Zheng-Lu force-pushed the feat/253-verify-codex-images branch from 5318fb5 to 553ea1e Compare September 14, 2026 15:39
@Zheng-Lu
Zheng-Lu marked this pull request as ready for review September 14, 2026 18:18
@Zheng-Lu
Zheng-Lu force-pushed the feat/253-verify-codex-images branch from 553ea1e to e52cbf4 Compare September 14, 2026 18:41
…llm-project#253)

Message content parts the typed Responses executor cannot convey are now
rejected with a 400 naming the part (`input[i].content[j]` and its type)
instead of being forwarded as a synthetic {"type": "unknown"} part that no
client sent. This extends the existing input_file rejection to unmodeled
part types and to empty part arrays, so one validator owns the rule that a
typed message is never mutated in transit, never means something different
on the typed path than on the raw store=false path, and is never persisted
with content the client did not send.

InputContent gains a hand-written deserializer so an unmodeled part keeps
its type name for the error, and its Unknown variant can no longer
serialize, which makes "never forwarded, never stored" a compile-time
property rather than a convention. `refusal` is modeled as an assistant
history part so OpenAI-style history replays through the typed executor.

Add deterministic gateway coverage for image preservation over HTTP and
WebSocket: mixed text/image ordering, multiple images per message,
client-executed view_image tool output, previous_response_id continuation,
conversation rehydration, stateless store=false proxying, compaction of
retained image-bearing user messages, and rejection of unmodeled or empty
message content before inference and before persistence.

Claude-Session: https://claude.ai/code/session_01EsbCF1Smauv41qnpnqYvfz
Signed-off-by: Zheng Lu <Lz429671594@gmail.com>
The cassette recorder only accepted a structured --input-file for a single
turn, so a conversation that opens with an item array (for example an
input_image part) could not be continued by previous_response_id in the same
recording. Apply the preset input to turn 1 and prompt for later turns.

Claude-Session: https://claude.ai/code/session_01EsbCF1Smauv41qnpnqYvfz
Signed-off-by: Zheng Lu <Lz429671594@gmail.com>
…llm-project#253)

Record the same two-turn image conversation against OpenAI and the gateway
(gateway pointed at the same gpt-4o upstream), streaming and non-streaming:
turn 1 sends a committed 64x64 PNG inline as an input_image part beside
input_text, turn 2 continues it by previous_response_id.

The replay tests compare protocol structure, never model wording: request
bodies identical across providers and equal to the committed fixture, a
completed response with one assistant message and usage, contiguous
sequence numbers and an identical normalized streaming lifecycle, and the
rehydrated history the gateway forwards on continuation carrying the image
part byte for byte.

The recorder script validates the fixture, stages every recording, and
promotes the set only once all selected providers pass.

Claude-Session: https://claude.ai/code/session_01EsbCF1Smauv41qnpnqYvfz
Signed-off-by: Zheng Lu <Lz429671594@gmail.com>
@Zheng-Lu
Zheng-Lu force-pushed the feat/253-verify-codex-images branch from e52cbf4 to fbdd150 Compare September 14, 2026 19:07
@maralbahari

Copy link
Copy Markdown
Collaborator

@Zheng-Lu could you please update the PR title and resolve merge conflict thanks

role: user
type: message
max_output_tokens: 64
model: gpt-4o

@maralbahari maralbahari Sep 17, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

By Agentic API gateway, we always mean client → Agentic API gateway → vLLM inference engine hosting an open-source model. For this PR, must an open-source vision model. The reference path is client → OpenAI Responses API directly.

The current recordings use OpenAI for both paths. That tests the gateway as an OpenAI wrapper and does not verify image handling with vLLM.

Please make the following changes:

  1. Record against the correct backends. Keep direct OpenAI as the reference. Point the gateway at a vLLM-hosted open-source vision model and re-record the gateway cassettes.

  2. Send equivalent inputs to both paths. Use the same image bytes, prompts, and tool definitions. Allow different model names. Compare image preservation, response structure, streaming events, and continuation behavior; generated wording and token counts can differ.

  3. Cover these cases with paired OpenAI and gateway cassettes:

    • A user message containing text and an image.
    • Multiple images mixed with text, preserving their order.
    • A follow-up using previous_response_id, preserving the earlier image in context.
    • A tool returns an image: the model calls a function, the client submits function_call_output containing input_image, and the model processes it in the next turn.

    Record each case in both streaming and non-streaming modes. Tool-returned images currently have mock tests but no recorded coverage. Image-input cases should produce ordinary response events; image-generation events are not required.

  4. Update the replay tests and recording instructions. Remove the hard-coded requirement that both providers use gpt-4o. Document the actual vLLM model and launch configuration, gateway endpoint, and recording command so the comparison is reproducible. Generate all captured request/response YAML through the recorder and validate it before committing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Verify Codex image attachments and image tool outputs through Responses

3 participants