Skip to content

fix: warn when a string schema uses a format that is not built in - #969

Open
adimalkar wants to merge 1 commit into
mlc-ai:mainfrom
adimalkar:fix/warn-unknown-string-format
Open

adimalkar wants to merge 1 commit into
mlc-ai:mainfrom
adimalkar:fix/warn-unknown-string-format

Conversation

@adimalkar

Copy link
Copy Markdown
Contributor

Fixes #967

What

A string schema whose format isn't one of the built-in formats compiled without any message, and the format was then ignored. {"type": "string", "format": "non_existing_format"} accepts "hello", so a constraint the schema asked for disappears and nobody notices.

This PR logs a warning when that happens:

json_schema_converter.cc:1155: Warning: format "non_existing_format" is not supported; ignoring format

Why a warning and not an error

The issue asks for a compile-time error. I went with a warning, as explained in #967 (comment):

Glad to switch to an error, or an error only under strict_mode, if maintainers would rather.

How

The warning lives in SchemaParser::ParseString, next to the minLength/maxLength warning from #896, for the same reason that one does: every converter goes through ParseString, including the XML tool-calling converter, which overrides GenerateString. It reuses JSONSchemaConverter::IsBuiltinFormat. It's an unconditional XGRAMMAR_LOG(WARNING) rather than WarnUnsupportedKeywords, because nothing calls that function with verbose=true, so its warnings never print.

The grammar doesn't change.

Tests

Added to tests/python/test_json_schema_converter.py, next to the #896 tests and using the same _compile_and_capture helper:

  • an unknown format warns: alone, as password, combined with pattern, and combined with maxLength
  • the built-in formats (email, date, date-time, uuid, uri) don't warn
  • the XML tool-calling path (qwen_xml) warns too
  • an unknown-format string accepts exactly what a plain string accepts, which pins the "grammar unchanged" behaviour

Locally, tests/python/test_json_schema_converter.py gives 627 passed. The whole tests/python directory gives 4524 passed. The only 26 failures are in test_builtin_structural_tag_alignment.py and come from the optional cohere_melody package, which isn't installed here. pre-commit and ruff check pass.

Copilot AI balanced review requested due to automatic review settings October 9, 2026 02:28

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

Only the built-in formats compile to a regex. Any other format was
skipped by GenerateString without a word, so the string was generated
unconstrained and the dropped keyword could not be noticed (mlc-ai#967).

Warn in SchemaParser::ParseString, where the minLength/maxLength
warning from mlc-ai#896 lives, so the JSON and XML tool-calling converters
both report it. The grammar is unchanged: format is an annotation by
default in JSON Schema 2020-12, and schema generators emit formats
xgrammar does not implement (pydantic's password, binary, path), so
rejecting them would break schemas that compile today.
@Seven-Streams
Seven-Streams force-pushed the fix/warn-unknown-string-format branch from b1a836b to 9ae4fc1 Compare October 17, 2026 14:42
Copilot AI balanced review requested due to automatic review settings October 17, 2026 14:42

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

An organization owner can choose the repository organization as the billing source for reviews requested by members with a paid Copilot license. Learn about Copilot code review billing.

This branch has not been deployed

No deployments
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.

[Bug] Unknown string format compiles successfully and accepts arbitrary strings

3 participants