Skip to content

Keep declaration order in Foundation Models schemas - #337

Merged
mattt merged 4 commits into
huggingface:mainfrom
akbashev:fix/schema-property-order
Oct 17, 2026
Merged

mattt merged 4 commits into
huggingface:mainfrom
akbashev:fix/schema-property-order

Conversation

@akbashev

Copy link
Copy Markdown
Contributor

Foundation Models generates an object's properties in the order its schema lists them. The conversion went through JSONValue and JSONSchema, whose decoding loses key order, so properties reached the model shuffled. A container's items could come before what the container is, and generation ran away nesting.

The conversion now goes straight from GenerationSchema's nodes, which keep propertyOrder.

Foundation Models generates an object's properties in its schema's order.
The conversion went through JSONValue and JSONSchema, whose decoding loses
key order, so properties reached the model shuffled: a container's items
could come before what the container is, and generation ran away nesting.
Convert straight from GenerationSchema's nodes, which keep propertyOrder.
@mattt

mattt commented Oct 17, 2026

Copy link
Copy Markdown
Collaborator

Thanks, @akbashev. Converting straight from the nodes makes sense to me.

One catch: with this change, the JSONSchema path that #332 fixes isn't used anymore, and the .ref case in convertToDynamicSchema(_ node:) drops name the same way the old code did. So if both PRs merge, #332's fix is lost — and there's no conflict to warn us. Could you carry that fix over here (keep a named schema whose one choice is the reference), with a test at the GenerationSchema level?

Two smaller things:

  • Int(minimum) truncates a fractional minimum and traps on a non-finite or very large one.
  • appearInOrder doesn't look like it's used anywhere.

Converting from GenerationSchema's nodes, a named definition that is only
a reference, such as a one-choice anyOf, dropped its name as the JSONSchema
path did before huggingface#332, so the references to it were undefined. It now stays
a named schema whose one choice is the reference. An integer's bounds
become the whole numbers within them, and a bound no Int holds, such as a
non-finite or very large one, sets no guide instead of trapping. The
unused appearInOrder test helper is gone.
The stand-in value for a number with no usable output took Int(minimum),
which traps on a minimum that isn't finite or that Int can't hold. It's
now the smallest whole number the minimum allows, or the nearest Int when
there's none; a stand-in decimal ignores a minimum that isn't finite.
@akbashev

Copy link
Copy Markdown
Contributor Author

Thanks, @mattt.

  • Keep the name of a schema that only refers to another #332's fix is carried over: in the node conversion, a named definition that's only a reference keeps its name. convertNamedOneChoiceReferenceFromGenerationSchema builds it from a GenerationSchema and fails without the fix (undefinedReferences ["Action"]).
  • Integer bounds now round inward to whole numbers. A bound no Int can hold sets no guide, instead of truncating or trapping. The same trap was also in the stand-in value for an integer (Int(num.minimum ?? 0)), so that's fixed too, with tests.
  • appearInOrder is removed.

One question: the JSONSchema path is now used only by its own tests. Would you like me to remove it here, or in a follow-up?

# Conflicts:
#	Tests/AnyLanguageModelTests/DynamicSchemaConversionTests.swift
@akbashev

Copy link
Copy Markdown
Contributor Author

Merged main in: it conflicted with #332's test in the same file. Both tests are kept.

@mattt

mattt commented Oct 17, 2026

Copy link
Copy Markdown
Collaborator

@akbashev Let's do that in a follow-up, so this PR stays focused on the ordering.

@mattt
mattt merged commit 6ab97b0 into huggingface:main Oct 17, 2026
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants