fix: wire up dereference_refs() in tool schema pipeline#3170
fix: wire up dereference_refs() in tool schema pipeline#3170jlowin merged 2 commits intorelease/2.xfrom
Conversation
WalkthroughThe change adds an optional dereference step to JSON Schema compression and enables it when generating both input and output schemas in ParsedFunction.from_function. Possibly related PRs
🚥 Pre-merge checks | ✅ 6✅ Passed checks (6 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
No actionable comments were generated in the recent review. 🎉 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 |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: eed6a2ae9c
ℹ️ 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".
| if dereference: | ||
| schema = dereference_refs(schema) |
There was a problem hiding this comment.
Preserve discriminator mappings when inlining $ref schemas
Calling compress_schema(..., dereference=True) now runs dereference_refs, which removes $defs after resolving only $ref nodes; this breaks discriminated-union schemas because Pydantic emits discriminator.mapping values like #/$defs/Cat as plain strings, so those mappings become dangling once $defs is stripped. For tools whose input/output annotations use discriminated unions, clients/validators that rely on discriminator.mapping can no longer resolve variants correctly.
Useful? React with 👍 / 👎.
This addresses the issues found in PrefectHQ#3760. Currently, if a self-referencing Pydantic data type is buried in a nested field, calling dereference_refs() introduces a circular reference to the final schema. This results in a serialization error that crashes the server during tools/list requests. I checked v3.2.0 and noticed that dereference=True is no longer being passed to compress_schema(), which essentially reverts PrefectHQ#3170. We should implement this same behavior for the 2.x versions. While the community will eventually migrate to 3.x, I think we still need to pull this fix in now because the bug causes a complete server crash whenever tools/list is called.
PR #2861 backported
dereference_refs()to v2 but never actually called it — the function exists and is tested in isolation, but nothing in the tool schema pipeline invokes it. As a result,$refreferences are sent as-is to MCP clients, breaking clients like VS Code Copilot that don't handle them.This wires it up by adding a
dereferenceparameter tocompress_schema()and enabling it at the two tool schema creation sites (input and output schemas inTool.from_function). Schemas are now inlined before being sent to clients.Closes #3153