fix(examples): support recursive structured output schemas - #844
fix(examples): support recursive structured output schemas#844charle-z wants to merge 5 commits into
Conversation
|
@codex review |
|
Codex Review: Didn't find any major issues. Nice work! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
|
Security review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
|
@codex review Please re-review the current head |
|
Codex Review: Didn't find any major issues. More of your lovely PRs please. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
|
Security review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
sylvesterkaczmarek
left a comment
There was a problem hiding this comment.
Rechecked the current head. Keeping refs enabled fixes the recursion problem, and expanding only the root while carrying $defs forward preserves the self-reference instead of flattening the schema. The metadata copy also avoids dropping schema/id/anchor. Looks good from this side.
|
@jbeckwith-oai when you have a chance, could you take a maintainer look at this one? The current head ( |
Summary
Make the Structured Outputs schema helper support recursive Go types without changing the example's object-shaped root schema.
Fixes #492.
Problem
The example configures
invopop/jsonschemawithDoNotReference: true. That forces referenced types to be inlined. A self-referential type therefore has no cycle breaker and schema reflection recurses until the Go runtime exhausts the goroutine stack.Simply re-enabling references avoids the recursion, but
jsonschemanormally represents the root as a$refinto$defs, while the Structured Outputs example needs an object-shaped root.Fix
Keep references enabled and expand only the reflector's local root
$ref:AllowAdditionalProperties: falseremains unchanged;$defsis retained, so nested and recursive definitions can continue to use$ref;$schema,$id,$anchor) is preserved;json.RawMessageconversion remains in place so large integer constraints keep their exact JSON representation.This deliberately does not use
ExpandedStruct: true: revalidation againstinvopop/jsonschemav0.14.0 showed that for the recursive fixture it expands the root while omitting$defs, leaving the emitted nested$refdangling.The duplicated Structured Outputs snippet in
README.mduses the same logic.Regression coverage
The recursive fixture verifies that:
type: "object";$defsis present;childrenitem is a self-reference into$defs.The existing object-shape and exact 64-bit integer enum tests remain intact.
Validation
Current head:
32823ea5723614ad1053f3caaa4cc1bc6686dfe3.Local / isolated validation:
examplestest module: pass;go vet ./...: pass;git diff --check: pass;upstream/mainat37c0f0ab07fbe5f5b2775c12ca3500acd0caad8c: clean; examples tests and root build pass on the merged tree.Fork CI on the exact head is green for the executable gates:
check-go-mod: pass;The upstream workflows for this external-contributor head remain
action_required; that is an approval gate, not a technical test failure.