fix(helpers): preserve constrained object oneOf schemas - #2159
Conversation
![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: e0c9161b3f
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| 'dependentRequired', | ||
| 'dependentSchemas', | ||
| ].some((keyword) => record[keyword] !== undefined); | ||
| if (!hasObjectShape || !hasOwnObjectConstraints) { |
There was a problem hiding this comment.
When this condition is false, the oneOf is intentionally left in place, but rewriteLocalRefsIntoMovedOneOfBranches() still runs unconditionally afterward and rewrites every local $ref that traverses any actual oneOf array to anyOf. A constrained object schema that also has an alias such as { "$ref": "#/properties/value/oneOf/0" } will therefore keep value.oneOf but rewrite the alias to #/properties/value/anyOf/0, causing ref validation to reject the schema (or leaving a dangling pointer). The rewrite needs to be limited to the oneOf records that are actually moved.
Useful? React with 👍 / 👎.
| 'properties', | ||
| 'required', |
There was a problem hiding this comment.
If a converter emits a redundant object union wrapper with properties: {} or required: [], these entries make hasOwnObjectConstraints true, so the oneOf is preserved instead of going through the existing object-union wrapper normalization that strips those empty placeholders. ensureStrictJsonSchema() then treats the wrapper as an object schema and adds additionalProperties: false, which forbids the properties declared by every object branch and turns a previously-valid union into an unsatisfiable schema. Empty properties/required should be normalized before deciding the wrapper has its own constraints.
Useful? React with 👍 / 👎.
| if (!hasObjectShape || !hasOwnObjectConstraints) { | ||
| // `false` can never validate, so it cannot overlap another oneOf | ||
| // branch. Keep it in place until the existing anyOf normalization runs | ||
| // so local refs into surviving branch indices can be rewritten before | ||
| // the impossible alternatives are removed. | ||
| const possibleBranches = record['oneOf'].filter((branch) => branch !== false); | ||
| if (!areOneOfBranchesMutuallyExclusive(possibleBranches, normalizedSchema)) { | ||
| throw new OpenAIError( | ||
| 'Standard JSON Schema generated a `oneOf` whose branches are not provably mutually exclusive. OpenAI strict schemas do not support `oneOf`; use `anyOf` or add a discriminator with distinct literal values.', | ||
| ); | ||
| } | ||
| oneOfSchemas.push(record); |
There was a problem hiding this comment.
For object schemas that now take the preserve path, false alternatives are no longer filtered because the filtering only happens inside the conversion-to-anyOf branch. A constrained object oneOf such as [false, { type: 'object', ... }] is semantically equivalent to the real branch, but strictification will recurse into the preserved false branch and reject it with Expected object schema but got boolean, regressing the existing false-branch handling for this newly preserved category.
Useful? React with 👍 / 👎.
| 'properties', | ||
| 'required', |
There was a problem hiding this comment.
When a schema has its own properties/required and oneOf object branches with branch-specific fields, preserving the oneOf lets ensureStrictJsonSchema() add additionalProperties: false to the outer wrapper. If those branch-only fields are not also declared in the wrapper's own properties, the new outer closure forbids fields like foo/bar, so the helper can emit an unsatisfiable schema instead of failing closed as the previous anyOf normalization did. Preserve these wrappers only when the parent object closure is already semantically safe.
Useful? React with 👍 / 👎.
1 participant
Fixes #2152.
The Standard Schema helper rewrites every provably exclusive
oneOfintoanyOf. For a schema node that already has its own object constraints, this changes the schema into a shape that the strictifier rejects, even though the originaloneOfis accepted.This change:
oneOfon object-shaped nodes with their own object constraints;type: object, object properties, andoneOf.Validation:
git diff --checkpassed.