Skip to content

fix: add per-model structured output opt-out - #1424

Merged
Wibias merged 3 commits into
devfrom
agent/fix-chat-structured-output-optout
Aug 11, 2026
Merged

fix: add per-model structured output opt-out#1424
Wibias merged 3 commits into
devfrom
agent/fix-chat-structured-output-optout

Conversation

@Ingwannu

@Ingwannu Ingwannu commented Aug 10, 2026

Copy link
Copy Markdown
Owner

Summary

  • Preserve response_format translation as the default for routed openai-chat models.
  • Add an explicit provider/model compatibility escape hatch, noStructuredOutputModels, so only exact listed model IDs omit the field.
  • Validate, normalize, persist, and expose the setting through the management configuration surfaces.
  • Cover both Responses and Chat Completions ingress and document the exact-match compatibility boundary in all five reference locales.

Closes #1415

Verification

  • taskset -c 0,1 nice -n 10 bun test tests/chat-completions-endpoint.test.ts tests/management-provider-validation.test.ts tests/openai-chat-hardening.test.ts — 139 pass, 0 fail on exact current head.
  • taskset -c 0,1 nice -n 10 bun run typecheck — passed on exact current head.
  • taskset -c 0,1 nice -n 10 bun run privacy:scan — passed on exact current head.
  • cd docs-site && taskset -c 0,1 nice -n 10 bun install --frozen-lockfile && taskset -c 0,1 nice -n 10 bun run build — passed, 221 pages built on exact current head.
  • git diff --check — passed.
  • Earlier full-suite verification before the review update: 10,680 pass, 10 skip, with one unrelated codex-shim fixture-isolation baseline failure that reproduced unchanged on its base. Refreshed exact-head platform CI is running after the rebase.
  • Exact tested head: a607411453d1a568e6cff93e6bb3c55520b11260, rebased onto dev dd0078416e1ac1efb70fcf088f49f11af6517db0.

Checklist

  • Scope stays focused and avoids unrelated cleanup.
  • Docs or release notes were updated when needed.
  • Security-sensitive changes were reviewed for secrets, auth, and unsafe defaults.

@coderabbitai

coderabbitai Bot commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

The provider configuration adds noStructuredOutputModels. Configuration management validates, normalizes, persists, and exposes the list. The openai-chat adapter omits response_format for exact matching models. Tests and documentation cover the behavior.

Changes

Structured-output compatibility

Layer / File(s) Summary
Provider contract and normalization
src/types.ts, src/config.ts
OcxProviderConfig now supports noStructuredOutputModels?: string[]. Validation rejects non-arrays and blank or non-string entries. Normalization trims values and removes duplicates.
Provider management wiring
src/server/auth-cors.ts, src/server/management/provider-routes.ts, tests/management-provider-validation.test.ts
Provider PATCH requests accept, normalize, persist, and clear the setting. Provider GET and safeConfigDTO responses expose it. Tests cover validation, normalization, persistence, and null clearing.
Chat request suppression and coverage
src/adapters/openai-chat.ts, tests/chat-completions-endpoint.test.ts, tests/openai-chat-hardening.test.ts, docs-site/src/content/docs/reference/proxy-formats.md, structure/04_transports-and-sidecars.md
The adapter omits response_format for listed model IDs and preserves existing formatting for other models. Tests cover Chat Completions, Responses API routing, supported models, and colon-suffixed identifiers.
Configuration reference updates
docs-site/src/content/docs/*/reference/configuration/providers.md
Provider references document noStructuredOutputModels and exact-match behavior in five languages.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Sequence Diagram(s)

sequenceDiagram
  participant ChatClient
  participant OpenAIChatAdapter
  participant RoutedProvider
  ChatClient->>OpenAIChatAdapter: Send structured-output request
  OpenAIChatAdapter->>OpenAIChatAdapter: Check noStructuredOutputModels
  OpenAIChatAdapter->>RoutedProvider: Forward request without response_format for matching model
  RoutedProvider-->>ChatClient: Return response
Loading

Possibly related PRs

  • lidge-jun/opencodex#985: Both changes modify openai-chat structured-output response_format handling. PR #985 adds forwarding, while this change adds model-specific suppression.

Suggested reviewers: lidge-jun, wibias

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 45.45% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed The changes address issue #1415 by omitting response_format for exact configured models while preserving default structured-output translation.
Out of Scope Changes check ✅ Passed The implementation, validation, management support, documentation, and tests are all directly related to the requested provider/model compatibility opt-out.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: adding a per-model structured-output opt-out.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch agent/fix-chat-structured-output-optout

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions github-actions Bot added the bug Something isn't working label Aug 10, 2026
@github-actions

Copy link
Copy Markdown
Contributor

Deterministic PR hygiene checks passed.

@Wibias Wibias left a comment

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.

Request changes based on a full review of the current head.

Merge blockers / required fixes:

  1. noStructuredOutputModels is documented and implemented as an exact per-model escape hatch, but the adapter uses modelInList(). That helper intentionally family-matches colon-tagged variants (foo also matches foo:variant). This can disable response_format for sibling/variant models that were not explicitly listed, which breaks the mixed-capability-gateway contract this PR is introducing. Use exact equality for this field (for example list.includes(parsed.modelId)) or a dedicated exact-match helper, and add a negative regression proving foo does not match foo:structured.

  2. Persisted config.json and Management API PATCH normalize this field differently. PATCH trims and deduplicates entries, but schema/config loading only verifies that entry.trim() is nonblank and preserves the original string. A manually persisted value such as " deepseek-v4-flash " is accepted but will not match the runtime model id. Normalize trim/dedupe at the config boundary so every management surface has identical semantics.

I did not find a security vulnerability in this PR, but the first issue is a direct contract violation and should be fixed before merge. After fixes, refresh onto current dev and rerun exact-head CI.

@Ingwannu
Ingwannu force-pushed the agent/fix-chat-structured-output-optout branch from 9c88233 to 1a4524d Compare August 11, 2026 04:45
@Ingwannu

Copy link
Copy Markdown
Owner Author

Addressed both blockers on the current head 1a4524df9 and rebased the branch onto dev 7779c0574.

  • noStructuredOutputModels now uses exact model-id equality, with a regression proving test-model does not match test-model:structured.
  • persisted config and Management PATCH now share one trim/deduplicate normalizer, with a load-boundary regression for hand-edited config.json.

Verification on the rebased head:

  • bun test tests/openai-chat-hardening.test.ts tests/management-provider-validation.test.ts — 88 passed, 0 failed
  • bun run typecheck — passed
  • bun run privacy:scan — passed
  • git diff --check — passed

@Wibias please re-review the exact head after CI completes.

@Ingwannu
Ingwannu requested a review from Wibias August 11, 2026 04:45
@Wibias
Wibias marked this pull request as ready for review August 11, 2026 05:26

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@docs-site/src/content/docs/reference/configuration/providers.md`:
- Line 106: Update the noStructuredOutputModels? reference row in
docs-site/src/content/docs/reference/configuration/providers.md:106-106 to state
that entries are exact model IDs, response_format is omitted only for exact
matches, and structured-output translation remains enabled for other openai-chat
models. Apply the same clarification in
docs-site/src/content/docs/ja/reference/configuration/providers.md:96-96,
docs-site/src/content/docs/ko/reference/configuration/providers.md:96-96,
docs-site/src/content/docs/ru/reference/configuration/providers.md:109-109, and
docs-site/src/content/docs/zh-cn/reference/configuration/providers.md:96-96
using each document’s language.

In `@tests/management-provider-validation.test.ts`:
- Around line 147-152: Remove the duplicated type-assertion fragments in
tests/management-provider-validation.test.ts at lines 147-152 and 1166-1169:
keep only one closing `} as OcxConfig) as ...` assertion at the first site and
one `providers: Record<string, { noStructuredOutputModels?: string[] }>;`
declaration at the second, restoring valid TypeScript syntax.
🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 003f6489-ec7f-49b5-97ab-2a328cbdbc35

📥 Commits

Reviewing files that changed from the base of the PR and between 7779c05 and 1a4524d.

📒 Files selected for processing (15)
  • docs-site/src/content/docs/ja/reference/configuration/providers.md
  • docs-site/src/content/docs/ko/reference/configuration/providers.md
  • docs-site/src/content/docs/reference/configuration/providers.md
  • docs-site/src/content/docs/reference/proxy-formats.md
  • docs-site/src/content/docs/ru/reference/configuration/providers.md
  • docs-site/src/content/docs/zh-cn/reference/configuration/providers.md
  • src/adapters/openai-chat.ts
  • src/config.ts
  • src/server/auth-cors.ts
  • src/server/management/provider-routes.ts
  • src/types.ts
  • structure/04_transports-and-sidecars.md
  • tests/chat-completions-endpoint.test.ts
  • tests/management-provider-validation.test.ts
  • tests/openai-chat-hardening.test.ts

Comment thread docs-site/src/content/docs/reference/configuration/providers.md Outdated
Comment thread tests/management-provider-validation.test.ts
@Ingwannu
Ingwannu force-pushed the agent/fix-chat-structured-output-optout branch from 1a4524d to a607411 Compare August 11, 2026 05:37
@Ingwannu

Copy link
Copy Markdown
Owner Author

Updated #1424 on latest dev with the remaining documentation clarification.

  • All five provider-reference locales now state that noStructuredOutputModels contains exact model IDs, only an exact requested-model match omits response_format, and every other openai-chat model keeps structured-output translation.
  • I verified the second CodeRabbit finding against current head and did not change it: the alleged duplicated type-assertion fragments are not present at either cited location. tests/management-provider-validation.test.ts:147-152 and :1166-1169 each contain one valid declaration, and the file parses/runs successfully.

Verification on exact head a607411453d1a568e6cff93e6bb3c55520b11260:

  • focused provider/adapter suites: 139 pass, 0 fail
  • typecheck: passed
  • privacy scan: passed
  • documentation build: 221 pages
  • diff check: passed

The branch was rebased onto current dev (dd0078416, which already includes #1428). Leaving approval and merge to an independent maintainer after the refreshed exact-head CI completes.

@Wibias Wibias left a comment

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.

Approving. Earlier blockers (exact includes match instead of modelInList, trim/dedupe at the config boundary) are fixed on this head, docs now state the exact-match contract, and the remaining CodeRabbit duplicate-cast note is a false positive against the current test file.

@Wibias

Wibias commented Aug 11, 2026

Copy link
Copy Markdown
Collaborator

Thanks @Ingwannu — this was useful because it adds a precise per-model escape hatch for gateways that reject response_format, without turning off structured-output translation for every other openai-chat model. Exact-match semantics plus shared trim/dedupe between config load and Management PATCH keep hand-edited and API-written lists behaving the same at runtime.

Merging now.

@Wibias
Wibias merged commit 9f545f2 into dev Aug 11, 2026
26 of 29 checks passed
@Wibias
Wibias deleted the agent/fix-chat-structured-output-optout branch August 11, 2026 05:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants