test: add provider bridge conformance replay suite - #2261
Conversation
lsm
left a comment
lsm
left a comment
There was a problem hiding this comment.
🤖 Review by GLM-5.1[1m] (GLM / Zhipu)
Model: GLM-5.1[1m] | Client: NeoKai | Provider: GLM (Zhipu)
Recommendation: APPROVE
Reviewed the full 1205-line suite against all three bridge implementations (line-by-line), not just the diff. Every assertion traces to real computed behavior — no bugs locked in, and the tests exercise the SDK/UI-visible contract (ordered Anthropic event sequence, block indices, delta contents, stop_reasons, usage) rather than internal helpers.
What I verified
- Gates: 51/51 tests pass (ran directly);
tsc --noEmitexit 0;check:test-qualityaudit clean. Lint is N/A — oxlint ignores*.test.tsvia.oxlintrc.jsonignorePatterns. Knip unaffected (no new prod exports). - Section A — OpenAI Chat bridge: canonical event sequence;
reasoning_content→ thinking block (index 0) strictly before text (index 1);delta.reasoningcorrectly NOT translated; tool_call arg fragments collapse to oneinput_json_deltaat flush; stop_reason map (stop→end_turn, length→max_tokens, content_filter→end_turn, absent→end_turn); malformed-JSON skip, choices-less usage harvest, split-frame reassembly, no-trailing-newline flush, non-SSE-200 error;ceil(len/4)usage heuristic floored at 1;buildChatRequestflag gating (reasoning_effort/tools/stream_options). - Section B — Responses (Codex) bridge:
response.reasoning_summary_part.addedopens thinking @0,summary_text.donecloses, text @1; empty-block path;function_call_arguments.done→ tool_use + dedup vsresponse.completed; monotonic block indices;response.incomplete→max_tokens; usage aliases (prompt_tokens/completion_tokens) +reasoning_tokens;onReasoningItemsencrypted round-trip; resilience;response.failed→error with no message_delta;code:'429'→rate_limit_error. - Section C — OAuth request variants: drives the real
createOpenAIResponsesBridgeServerwith a capturing upstreamfetch(the only layer auth routing lives in). api_key →api.openai.com/v1/responses+ Bearer +includeadmitsreasoning.summary_text+ keepsmax_output_tokens; chatgpt_oauth →chatgpt.com/backend-api/codex/responses+ChatGPT-Account-ID(capitalID) + dropssummary_text/max_output_tokens/parallel_tool_calls; FedRAMP header; effort bands incl. the model-cap (gpt-4o caps to high); 401 refresh-once + retried Bearer. - Section D — Anthropic pass-through: byte-for-byte verbatim forwarding (text/event-stream skips the JSON pre-buffer);
anthropic-*header forwarding + dualx-api-key/Bearer; 200-with-JSON rate-limit body → retryable 429;count_tokensURL built once (suffix-strip handles a baseUrl already ending in/v1/messages).
The replay strategy is sound: response-side tests drive the canonical transforms directly with synthetic Response bodies (no network, fully deterministic), while the OAuth/pass-through tests go through the real server with a mocked upstream fetch. All seven task-named gap categories are covered.
Non-blocking observations (optional coverage refinements, not defects)
These are finite-suite edge sub-cases the task scope did not require; none affects correctness of what IS asserted:
response.incomplete+ tool calls together (tool_use currently wins over max_tokens) isn't exercised —server.ts:1244.- The api_key path also emits
parallel_tool_calls:false; onlymax_output_tokensis asserted on that path. - FedRAMP/
ChatGPT-Account-IDheaders are asserted only on the positive chatgpt_oauth path, not their absence on the api_key path. content_block_stopindex values aren't directly asserted (start indices + event sequence are).
Zero blocking findings. Self-PR so posting as COMMENT — recommendation is APPROVE.
![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: 5e5760501a
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| headers: { | ||
| 'Content-Type': 'application/json', | ||
| 'anthropic-beta': 'interleaved-thinking-2025-05-14', | ||
| 'anthropic-version': '2023-06-01', |
There was a problem hiding this comment.
This request sends the bridge's default anthropic-version value and the test never asserts the captured version header. If forwarding of anthropic-version is removed, the bridge will still supply 2023-06-01 from its default and this claimed “every anthropic-* request header” conformance test will continue to pass. Send a distinguishable non-default value and assert that exact value upstream.
Useful? React with 👍 / 👎.
| tools: [{ name: 't', description: '', input_schema: { type: 'object' } }], | ||
| tool_choice: { type: 'auto' as const }, | ||
| }; | ||
| const req = _openAIChatBridgeTesting.buildChatRequest(body, 'm', false, false, true); |
There was a problem hiding this comment.
Despite the test title, this request contains no thinking configuration and there is no assertion on req.reasoning_effort, so the field is undefined regardless of whether reasoning is correctly forwarded. A regression that accidentally couples reasoning translation to toolUseSupported would therefore pass this suite. Include a thinking budget in this case and assert the resulting effort while tool support is disabled.
Useful? React with 👍 / 👎.
| let capturedUrl = ''; | ||
| const fetchImpl = async (url: string) => { | ||
| capturedUrl = url; |
There was a problem hiding this comment.
The test title promises that count_tokens is routed exactly once, but capturedUrl only records the most recent invocation. If the bridge begins issuing the same upstream request twice, both calls overwrite this variable with the expected URL and the test still passes, missing duplicated billing or side effects. Track the fetch call count and assert that it is one in addition to checking the URL.
Useful? React with 👍 / 👎.
| expect(blocksOfType(events, 'thinking')).toHaveLength(1); | ||
| expect(deltasOfType(events, 'thinking_delta')).toHaveLength(0); |
There was a problem hiding this comment.
This test claims the empty thinking block is opened and closed, but it only checks for a content_block_start and the absence of deltas. If the bridge stops emitting content_block_stop for reasoning blocks, the resulting Anthropic stream is malformed while this test still passes. Assert the matching stop event and block index, ideally before the final message events.
Useful? React with 👍 / 👎.
| // Standard API path keeps max_output_tokens / parallel_tool_calls. | ||
| expect(caps.body.max_output_tokens).toBe(1024); |
There was a problem hiding this comment.
The comment states that the standard API path retains both max_output_tokens and parallel_tool_calls, but this case only asserts the former. There is no other test for the standard-path value, so a regression that omits parallel_tool_calls: false—allowing an upstream default to enable parallel calls the bridge does not intend—would pass, even though the OAuth counterpart explicitly verifies omission. Assert that the captured standard request contains parallel_tool_calls: false.
Useful? React with 👍 / 👎.
🤖 CI failure root-cause (follow-up) — GLM-5.1[1m]
The red Evidence
Likely mechanism (for whoever fixes the flake)The Recommendation
|
Replays synthetic/captured provider payloads through the OpenAI Chat, OpenAI Responses (Codex), and Anthropic-Messages pass-through bridges, asserting the exact ordered Anthropic event sequence + block structure the UI consumes. Covers the gap categories the per-bridge unit tests leave coarse: reasoning_content/thinking blocks, include arrays, OAuth-gated request variants (api_key vs chatgpt_oauth), stop_reason + usage mapping, and malformed/partial-chunk resilience.
5e57605 to
30fea2b
Compare
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
1 participant
Replays synthetic provider payloads through the OpenAI Chat, OpenAI Responses (Codex), and Anthropic-Messages pass-through bridges, asserting the exact ordered Anthropic event sequence the UI consumes. Fills the gap categories the per-bridge unit tests leave coarse:
reasoning_content/thinking blocks,includearrays, OAuth-gated request variants (api_key vs chatgpt_oauth URL/headers), stop_reason + usage mapping, and malformed/partial-chunk resilience.The replay drives the canonical stream transforms (
streamChatToAnthropic/streamResponsesToAnthropic) directly with synthetic upstreamResponsebodies — noBun.serve, no real network — so it is fully deterministic. The1-coreshard globs the directory, so CI picks the new file up automatically.