fix(anthropic): forward structured output schemas - #1464
Conversation
|
✅ Deterministic PR hygiene checks passed. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAnthropic structured-output requests now normalize JSON Schemas, preserve references and definitions, merge the result into ChangesAnthropic schema output
Estimated code review effort: 3 (Moderate) | ~30 minutes Possibly related PRs
Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant ResponsesRequest
participant buildRequest
participant normalizeAnthropicOutputSchema
participant AnthropicAPI
ResponsesRequest->>buildRequest: provide JSON Schema text format
buildRequest->>normalizeAnthropicOutputSchema: normalize schema
normalizeAnthropicOutputSchema-->>buildRequest: return normalized schema
buildRequest->>AnthropicAPI: send merged output_config
sequenceDiagram
participant AnthropicRequest
participant formatFromOutputConfig
participant parseRequest
participant ResponsesRequest
AnthropicRequest->>formatFromOutputConfig: provide output_config.format
formatFromOutputConfig-->>parseRequest: return validated text format
parseRequest->>ResponsesRequest: set body.text.format and options.textFormat
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
✅ READY
Review readiness checklist
✅ 4/4 boxes ticked. This pull request is already Ready for Review. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@src/adapters/anthropic-output-schema.ts`:
- Around line 33-43: Update normalizeSubschema so $defs are normalized and
attached to normalized before handling the $ref return; when both exist, return
normalized containing both $defs and $ref instead of returning only the
reference. Add a regression test covering a root schema with a `#/`$defs/...
reference and verify the emitted schema preserves its definitions.
🪄 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: 2301959a-1347-40ec-a07d-877e314e339a
📒 Files selected for processing (3)
src/adapters/anthropic-output-schema.tssrc/adapters/anthropic.tstests/anthropic-reasoning.test.ts
Ingwannu
left a comment
Ingwannu
left a comment
There was a problem hiding this comment.
The direction is valuable and the current transform is much closer to the real Anthropic contract than silently dropping structured output. I compared the implementation with Anthropic's current TypeScript SDK transform and rechecked exact PR head e600c877: six adjacent parser/adapter/Chat/compaction suites pass 181/181, and typecheck plus privacy scan pass. I did not find a credential or destination-boundary problem.
I am requesting changes for three completion blockers:
- The PR promises both Responses
text.formatand Chat Completionsresponse_format, but the new behavior tests callparseRequest()with a Responses body only. Add a regression that starts from the Chat Completions JSON Schema shape and proves the Anthropic request receives the expectedoutput_config.formatafter the real inbound conversion. This boundary matters becausesrc/chat/inbound.tsownsname,description,strict, and missing-schema normalization independently from the Anthropic adapter. The test should make explicit that Anthropic receives only its supported{ type: "json_schema", schema }shape and that adaptiveoutput_config.effortis preserved. - This is user-visible adapter behavior and a non-trivial 125-line compatibility transform. Update
docs-site/src/content/docs/reference/adapters.mdwith the supported Anthropic structured-output behavior and its limitations, and record the decision in the applicablestructure/document using the repository's[Decision Log]format. In particular, document why OpenCodex mirrors the SDK subset locally, why unsupported constraints are preserved in descriptions, why root$defsis retained beside a root$refeven though the current SDK transform returns on$ref, and the maintenance/tradeoff of copying that transform. - The head is currently three commits ahead of and three commits behind current
dev, while the full suite did not finish in the author's two attempts and the four readiness boxes remain open. Rebase onto currentdev, rerun the focused tests/typecheck/privacy/docs build, then complete the readiness checklist only after exact-head CI is actually green.
Once those coverage, documentation, and exact-head gates are complete, this should remain a strong merge candidate. Please keep it draft until then.
f35d1da to
b6b8dac
Compare
|
|
|
|
|
Tip For best results, initiate chat on the files or code changes.
You are interacting with an AI system. |
There was a problem hiding this comment.
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/adapters.md`:
- Around line 77-84: Update the structured-output documentation in
docs-site/src/content/docs/reference/adapters.md lines 77-84 to state that
structured output preserves compatible adaptive-thinking output_config.effort.
Also update structure/04_transports-and-sidecars.md lines 551-565 to document
that format merges into, rather than replaces, the existing adaptive-thinking
output configuration, while keeping output_config.effort compatible with the
structured-output format.
In `@src/adapters/anthropic-output-schema.ts`:
- Around line 47-56: Update the normalization logic around normalizeSubschema so
it removes only the composition keyword selected by the branch, preserving
unselected anyOf, oneOf, or allOf entries in schema for the fallback guidance
path. Ensure schemas containing multiple composition keywords retain every
unselected constraint, and add a regression test covering at least two
composition keywords.
🪄 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: ca8cb91f-9737-4ee9-96bb-4b1f5d601fc0
📒 Files selected for processing (4)
docs-site/src/content/docs/reference/adapters.mdsrc/adapters/anthropic-output-schema.tsstructure/04_transports-and-sidecars.mdtests/anthropic-reasoning.test.ts
|
|
|
|
b6b8dac to
b9f846a
Compare
|
Tip For best results, initiate chat on the files or code changes.
You are interacting with an AI system. |
|
Tip For best results, initiate chat on the files or code changes.
You are interacting with an AI system. |
|
Tip For best results, initiate chat on the files or code changes.
You are interacting with an AI system. |
b9f846a to
a093091
Compare
|
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@src/claude/inbound.ts`:
- Around line 72-76: Update formatFromOutputConfig to validate format.schema
with the existing structured-output schema validator before returning the
translated json_schema format; preserve valid root $ref and $defs schemas, and
return undefined for unsupported schemas. Add an inbound regression test
covering an invalid schema such as one containing only description.
🪄 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: 787264c3-862f-4e59-b0a5-0a9ee32c6b43
📒 Files selected for processing (5)
docs-site/src/content/docs/reference/adapters.mdsrc/claude/inbound.tsstructure/04_transports-and-sidecars.mdtests/anthropic-reasoning.test.tstests/claude-inbound.test.ts
|
✅ Action performedReview finished.
|
|
✅ Action performedReview finished.
|
|
|
|
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@src/claude/inbound.ts`:
- Around line 76-81: Update the JSON Schema return in the inbound format
conversion around isAnthropicOutputSchema to include a deterministic name such
as “response” alongside type and schema. Add or update the corresponding tests
in claude-inbound.test.ts to assert the complete final wire shape includes this
name.
🪄 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: d969d54c-9b52-455d-a231-5eda578fecef
📒 Files selected for processing (3)
src/adapters/anthropic-output-schema.tssrc/claude/inbound.tstests/claude-inbound.test.ts
|
|
|
|
|
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@tests/claude-inbound.test.ts`:
- Line 184: Add a deterministic `name` field to the JSON Schema format emitted
by the inbound translator, updating the relevant logic in `inbound.ts`; then
update the `referenced.text` expectation in the test to assert the same name
alongside `type` and `schema`.
🪄 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: 95c1ab53-5f99-497b-8d62-cf45657d23c4
📒 Files selected for processing (3)
src/adapters/anthropic-output-schema.tssrc/claude/inbound.tstests/claude-inbound.test.ts
Ingwannu
left a comment
Ingwannu
left a comment
There was a problem hiding this comment.
Follow-up on exact head 3e42026f: the author has addressed the first two clusters from my requested-changes review. The real Chat Completions inbound conversion now has regression coverage, compatible adaptive effort is preserved, and the public docs plus [Decision Log] explain the local Anthropic SDK subset and root $defs divergence. The later inbound validation and deterministic name coverage are also present, and no unresolved review threads remain.
The remaining blocker is still the exact-head integration gate: this head is 45 commits behind current dev@849ab5e35, and the status rollup contains only hygiene/target checks, not the required cross-platform suite. Please rebase onto current dev and obtain green exact-head required CI. Until then my existing changes-requested state remains intentional; I am not asking for additional feature scope.
Wibias
left a comment
Wibias
left a comment
There was a problem hiding this comment.
Re-review on head 3e42026f against current dev@849ab5e35.
The earlier review findings around root $defs, composition fallback, Chat Completions coverage, inbound schema validation, deterministic Responses naming, docs, and the adaptive-thinking merge are addressed. I found one remaining integration blocker in the routed Claude Messages path.
handleClaudeMessagesWithBudget() extracts ocx-route / ocx-effort before anthropicToResponsesTranslation(). When an effort override is present it currently does:
anthropicBody.output_config = { effort: effortOverride };That replaces the caller's entire output_config. A valid request carrying both output_config.format and the generated ocx-route + ocx-effort directives therefore loses the structured-output format before the new inbound translator can preserve it. The adapter-level merge added by this PR never sees the format.
Please merge the effort override into an existing object instead of replacing it, preserving output_config.format, and add an endpoint-level regression for a routed Claude Messages request that has structured output plus the route/effort directives. The final Anthropic wire body should contain both the overridden effort and the JSON-schema format.
Separately, this head is still 45 base commits behind current dev, and the exact-head Cross-platform CI / React Doctor runs are action_required with zero jobs. Rebase and exact-head CI are still required after the code fix.
3 participants
Summary
output_config.format.$defsbeside a root$ref, merge structured-output configuration with adaptive-thinkingoutput_config.effort, and round-trip Messages structured output through stored OAuth.Verification
bun test tests/anthropic-reasoning.test.ts tests/claude-inbound.test.ts tests/claude-messages-endpoint.test.ts tests/responses-parser.test.ts tests/responses-parser-malformed-content.test.ts tests/chat-completions-endpoint.test.ts tests/responses-compaction.test.ts tests/responses-compaction-routing.test.ts(270 passed)bun test tests/api-storage-policy-put-race.test.ts(1 passed)bun run typecheckbun run privacy:scancd docs-site && bun install --frozen-lockfile && bun run buildOutput.objectrequest through the patched server on port 10101 usinganthropic/claude-sonnet-5Checklist
Review readiness checklist
This PR stays in draft until every box below is ticked. Tick all four boxes once the requirements are met:
All CI tests are green on my local testing.
I pushed my PR to the latest dev commit.
I resolved all correct Codex and CodeRabbit findings.
My PR is ready for review.
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Tests