Uh oh!
There was an error while loading. Please reload this page.
test(e2e): C8 structured outputs / JSON mode passthrough - #172
Conversation
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Free Run ID: 📒 Files selected for processing (1)
Note 🎁 Summarized by CodeRabbit FreeYour organization has reached its limit of developer seats under the Pro Plan. For new users, CodeRabbit will generate a high-level summary and a walkthrough for each pull request. For a comprehensive line-by-line review, please add seats to your subscription by visiting https://app.coderabbit.ai/login.If you believe this is a mistake and have available seats, please assign one to the pull request author through the subscription management page using the link above. Comment |
There was a problem hiding this comment.
Pull request overview
Adds end-to-end coverage for OpenAI Chat Completions structured outputs (“JSON mode”) to ensure the gateway forwards response_format through to the upstream unchanged, using the real openai Node SDK + the existing OpenAI mock upstream harness.
Changes:
- Introduces a new e2e suite that exercises
/v1/chat/completionswithresponse_format: { type: "json_object" }. - Adds a second e2e case for
response_format: { type: "json_schema", json_schema: { name, schema, strict: true } }and asserts upstream body preservation.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| test("response_format: { type: 'json_schema', json_schema: {...} } forwarded verbatim", async (ctx) => { | ||
| if (!etcdReachable || !app || !upstream) { | ||
| ctx.skip(); | ||
| return; | ||
| } | ||
| const client = new OpenAI({ | ||
| apiKey: CALLER_PLAINTEXT, | ||
| baseURL: `${app.proxyUrl}/v1`, | ||
| maxRetries: 0, | ||
| }); | ||
| // OpenAI's structured-outputs json_schema descriptor per |
| const baseline = upstream.receivedRequests.length; | ||
| await client.chat.completions.create({ | ||
| model: "jsonmode-e2e", | ||
| messages: [ | ||
| { role: "user", content: "Return facts about SF as JSON." }, | ||
| ], | ||
| response_format: responseFormat, | ||
| }); |
Audit on commit d04abe8 returned 0 HIGH, 1 MEDIUM (rebase-after-#169 infra fix), 3 LOW. Applying LOW-1 here: LOW-1: case 2 (json_schema) asserted only response_format fields on the upstream side. A regression that mangled the rest of the request body while preserving response_format would not surface. Added cross-check: pin `model` translation (display name → upstream model_name) and the user message reaches upstream verbatim — same defense case 1's byte-for-byte content assertion provides on the response side. LOW-2 (streaming + response_format coverage gap) and LOW-3 (case 1 comment polish) deferred. MEDIUM-1 (rebase on #169 once it merges to inherit waitConfig budget bump + maxForks reduction) addressed by waiting for #169 to land and rebasing before merge.
Audit on commit d04abe8 returned 0 HIGH, 1 MEDIUM (rebase-after-#169 infra fix), 3 LOW. Applying LOW-1 here: LOW-1: case 2 (json_schema) asserted only response_format fields on the upstream side. A regression that mangled the rest of the request body while preserving response_format would not surface. Added cross-check: pin `model` translation (display name → upstream model_name) and the user message reaches upstream verbatim — same defense case 1's byte-for-byte content assertion provides on the response side. LOW-2 (streaming + response_format coverage gap) and LOW-3 (case 1 comment polish) deferred. MEDIUM-1 (rebase on #169 once it merges to inherit waitConfig budget bump + maxForks reduction) addressed by waiting for #169 to land and rebasing before merge.
263aec4 to
fa0c4d0Compare| const baseline = upstream.receivedRequests.length; | ||
| await client.chat.completions.create({ | ||
| model: "jsonmode-e2e", | ||
| messages: [ | ||
| { role: "user", content: "Return facts about SF as JSON." }, | ||
| ], | ||
| response_format: responseFormat, | ||
| }); |
Third C8 sub-area covered. The first two (#170 tools-cross-provider, #171 vision input parser) surfaced product gaps and are held back pending fixes. JSON mode is the simpler request-passthrough field — no new content-block parsing, no cross-provider translation — and it works. Two cases pinned: 1. `response_format: { type: "json_object" }` — caller asserts upstream-side body has `response_format: {type: "json_object"}` verbatim, and the JSON-shape content reaches the SDK byte-for-byte and parses as valid JSON. 2. `response_format: { type: "json_schema", json_schema: {name, schema, strict} }` — caller asserts the full descriptor (name, full JSON schema, `strict: true`) reaches upstream verbatim. A regression that dropped any sub-field — especially `strict` — would relax upstream's constraints and break schema-conforming callers. Both cases are real production paths: every modern agent / RAG framework that depends on parseable structured output sets `response_format` on every chat completion. A regression that dropped this field would leave the model in default text mode and silently break every JSON-output caller. References: - OpenAI structured-outputs guide <https://platform.openai.com/docs/guides/structured-outputs> - OpenAI Chat Completions response_format spec <https://platform.openai.com/docs/api-reference/chat/create#chat-create-response_format> Refs #151
Audit on commit d04abe8 returned 0 HIGH, 1 MEDIUM (rebase-after-#169 infra fix), 3 LOW. Applying LOW-1 here: LOW-1: case 2 (json_schema) asserted only response_format fields on the upstream side. A regression that mangled the rest of the request body while preserving response_format would not surface. Added cross-check: pin `model` translation (display name → upstream model_name) and the user message reaches upstream verbatim — same defense case 1's byte-for-byte content assertion provides on the response side. LOW-2 (streaming + response_format coverage gap) and LOW-3 (case 1 comment polish) deferred. MEDIUM-1 (rebase on #169 once it merges to inherit waitConfig budget bump + maxForks reduction) addressed by waiting for #169 to land and rebasing before merge.
fa0c4d0 to
518d5feCompareUh oh!
There was an error while loading. Please reload this page.
Summary
Third sub-area of #151's C8 row (multimodal/tools). The first two surfaced product gaps:
JSON mode is the simpler request-passthrough field — no new content-block parsing, no cross-provider translation needed, just propagation of the `response_format` field.
What's pinned
Why this matters
Every modern agent / RAG framework that depends on parseable structured output sets `response_format` on every chat completion. A regression that dropped this field would leave the model in default text mode and silently break every JSON-output caller — the symptom is "I expected JSON but got prose" with no error envelope.
Source-blind discipline
Every assertion derives from external contracts:
No internal Rust paths or struct field names referenced.
Test plan
Refs #151.