fix: preserve discriminated-union schema in tool conversion#1104
Open
planetf1 wants to merge 2 commits into
Open
fix: preserve discriminated-union schema in tool conversion#1104planetf1 wants to merge 2 commits into
planetf1 wants to merge 2 commits into
Conversation
A tool parameter typed as a Pydantic discriminated union
(``Annotated[A | B, Field(discriminator="kind")]``, with or without
``| None``) currently collapses to ``{"type": "string"}`` in the schema
emitted by ``convert_function_to_ollama_tool``. Because that schema is
shared by every backend (Ollama, OpenAI, Watsonx, HuggingFace, LiteLLM),
discriminated-union tool parameters are silently broken across the
board: the model sees a string and hallucinates a payload, and
``validate_tool_arguments`` rejects valid dicts.
Pydantic emits the union as ``oneOf`` plus an OAS-3 ``discriminator``
keyword, neither of which is in the JSON Schema subset accepted by
tool-calling APIs. The existing inliner only descends into ``anyOf`` and
``$ref``, so the structure falls through to the primitive-flattening
branch.
Fix: add a pre-pass that flattens the discriminated-union shapes — both
top-level (required) and nested-in-anyOf (Optional) — to plain ``anyOf``
of inlined object schemas, with the OAS ``discriminator`` keyword
stripped. The ``Literal`` constraints on the tag field already carry
the discriminator signal. Also extends ``_is_complex_anyof`` to detect
``oneOf`` branches defensively.
Resolves generative-computing#989.
Assisted-by: Claude Code
Signed-off-by: Nigel Jones <jonesn@uk.ibm.com>
- Rewrite ``_flatten_discriminated_union`` to be non-mutating and update the docstring; the previous "rewrites in place" claim was misleading because the required-union path returned a new dict. Document the single-level limitation (nested discriminated unions are not recursively flattened — tracked alongside generative-computing#911). - Defensively merge ``oneOf`` into any pre-existing top-level ``anyOf`` rather than overwriting, so the helper is safe to call in isolation even on shapes Pydantic does not currently emit. - Drop a redundant ``v.get("anyOf", [])`` default whose key existence was already guaranteed by the surrounding guard. Tests: - ``test_optional_union_strips_discriminator_keyword`` — pin the implicit-strip in the optional path so a refactor can't silently reintroduce the OAS-3 keyword. - ``test_three_way_union_preserves_all_branches`` — three-arm discriminated unions are common in command-pattern tools. - ``test_non_discriminated_optional_unchanged`` — regression guard for the existing ``Optional[Email]`` flow; the new pre-pass must be a no-op. - Tighten ``_has_branch`` to ``anyOf`` only; accepting ``oneOf`` as a fallback would silently mask a regression of the flattening pre-pass. - Move ``import json`` to module top. Assisted-by: Claude Code Signed-off-by: Nigel Jones <jonesn@uk.ibm.com>
This was referenced May 20, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Bug Fix: discriminated-union tool parameters
Type of PR
Description
A tool parameter typed as a Pydantic discriminated union —
Annotated[A | B, Field(discriminator="kind")], with or without| None— currently collapses to{"type": "string"}in the schema emitted byconvert_function_to_ollama_tool.Despite the function name, that schema is consumed by every backend (
as_json_toolis read by Ollama, OpenAI, LiteLLM, Watsonx, and HuggingFace), so discriminated-union tool parameters are silently broken across the board: the model sees a string and hallucinates a payload, andvalidate_tool_argumentsrejects valid dicts.Root cause
Pydantic emits the union as
oneOfplus an OAS-3discriminatorkeyword:Neither
oneOfnordiscriminatoris in the JSON Schema subset accepted by tool-calling APIs. The existing inliner only descends intoanyOfand$ref, so the structure falls through to the primitive-flattening branch and emerges as{"type": "string"}.Fix
Adds a pre-pass that flattens both shapes to plain
anyOfof inlined object branches, with the OASdiscriminatorkeyword stripped. TheLiteralconstraints on the tag field already carry the discriminator signal, so dropping the OAS keyword is a no-op semantically but makes the schema acceptable to tool-calling APIs._flatten_discriminated_unionis non-mutating (returns a new schema) and defensively merges into any pre-existing top-levelanyOfrather than overwriting. Flattening is single-level; nested discriminated unions are tracked alongside the recursive$refresolution work in #911 (follow-up filed as #1105).Before / after
Before (
{"type": "string"}is wrong):After (preserved union,
discriminatorstripped, refs inlined):Testing
Test additions:
test/backends/test_schema_helpers.py— extends_is_complex_anyofcoverage foroneOfbranches (with and withoutdiscriminatormetadata).test/backends/test_discriminated_union_tools.py— new file:{"type":"string"}, bothCatandDogbranches survive as fully-inlined object schemas, OASdiscriminatorkeyword is stripped (asserted on both required and optional paths), no dangling$refleaks, optional variant is removed fromrequired.Annotated[Cat | Dog | Fish, Field(discriminator="kind")]preserves all three branches.Optional[Email]regression guard: the new pre-pass must be a no-op for plain$ref+| Noneshapes that the existing inliner already handles.validate_tool_arguments: accepts valid{"kind":"dog", ...}and{"kind":"cat", ...}payloads, rejects bare strings, rejects dicts missing the discriminator, accepts both omitted and supplied for the optional variant.Local regression:
uv run pytest test/ -m "not qualitative" --ignore=test/stdlib/tools/test_mcp.py→ 1847 passed, 1 pre-existing failure unrelated to this change (test_example_collection_sanity— depends on optional extras likemcpbeing installed).Notes for reviewers
mellea/backends/tools.py) but applies to every tool-calling backend because they all consumeMelleaTool.as_json_tool. No backend-specific changes are needed.oneOf→anyOfrather than the reverse: OpenAI strict-mode tool schema rejectsoneOf; theLiteraltag enforces uniqueness in practice, so the choice is purely about which keyword the consumers accept.discriminator-stripping is a behaviour change for any consumer that may have been relying on it. Grep finds no internal consumer.convert_function_to_ollama_toolis misleading (it is the canonical OpenAI-tool-format converter for all backends), but renaming is a public-API change worth a separate PR.Follow-ups filed during review
oneOf+discriminator+ dangling$refs on the inner field. Related to Support recursive nested-model $ref resolution in Pydantic tool parameter schemas #911.validate_tool_argumentsdoes not enforceLiteral/const/enumconstraints. Pre-existing gap, surfaced more visibly by this PR. Affects everyLiteral[...]tool field, not just discriminated unions.Attribution