Skip to content

Do not return UnparsedObject when matching nested oneOf schema#4096

Open
ddadamhooper wants to merge 1 commit into
masterfrom
adam.hooper/nested-oneof
Open

Do not return UnparsedObject when matching nested oneOf schema#4096
ddadamhooper wants to merge 1 commit into
masterfrom
adam.hooper/nested-oneof

Conversation

@ddadamhooper

Copy link
Copy Markdown
Contributor

What does this PR do?

Removes some UnparsedObjects that come from oneOf.

I found that #4062 was failing CI because of an underlying reason: if a oneOf contains a "nested" oneOf, the nested one is always marked as "parsed" even if it isn't. The outcome: any oneOf that itself contained a (direct) unparsed oneOf branch was being parsed as UnparsedObject.

(This seems backwards: why does an inner "faux-parsed" oneOf branch mean the outer oneOf fails? Isn't "parsed" going to give false positives, not false negatives? No: a oneOf must match exactly one branch; if we accidentally mark the faux-parsed oneOf branch as parsed, then two branches are marked as parsed and the outer oneOf is therefore unmatched.)

Why isn't a oneOf marked as unparsed when no branch matches? Because we try to be forward-compatible with future objects (see #944). But it turns out we've been shipping UnparsedObject for years. I've unit-tested a 2023 class (introduced in #1843) as part of the fix.

Additional Notes

It's hard to choose where to draw the line here -- which part of a request should be marked UnparsedObject and which part should succeed? I didn't try to formalize it; I just focused on ensuring that some valid objects get marked as parsed where they weren't before.

Review checklist

Please check relevant items below:

  • This PR includes all newly recorded cassettes for any modified tests.
  • This PR does not rely on API client schema changes.
    • The CI should be fully passing.
  • Or, this PR relies on API schema changes and this is a Draft PR that includes tests.
    • Note: CI shouldn't be run on this Draft PR, as its expected to fail without the corresponding schema changes.

@ddadamhooper ddadamhooper added the changelog/Fixed Fixed features results into a bug fix version bump label Jul 20, 2026
@ddadamhooper

Copy link
Copy Markdown
Contributor Author

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Bravo.

Reviewed commit: f03b2c0fc7

ℹ️ 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".

@ddadamhooper
ddadamhooper marked this pull request as ready for review July 20, 2026 17:33
@ddadamhooper
ddadamhooper requested review from a team as code owners July 20, 2026 17:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

changelog/Fixed Fixed features results into a bug fix version bump

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant