Skip to content

fix(types): reject non-string values for DSL terms - #1944

Open
hsusul wants to merge 2 commits into
dottxt-ai:mainfrom
hsusul:fix/reject-non-string-regex-fields
Open

fix(types): reject non-string values for DSL terms#1944
hsusul wants to merge 2 commits into
dottxt-ai:mainfrom
hsusul:fix/reject-non-string-regex-fields

Conversation

@hsusul

@hsusul hsusul commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

What

DSL Term objects can be used directly as Pydantic field types. They generate a JSON Schema field of {"type": "string", "pattern": ...}, but Term.validate() converted every input with str(value) before applying the regex and then returned the original input unchanged.

Matching non-string values therefore passed validation and stayed non-strings in the resulting model:

from pydantic import BaseModel
from outlines.types import Regex

class Payload(BaseModel):
    value: Regex(r"[0-9]+")

Payload(value=123).value      # 123, an int

The consequence is not confined to the model instance. The field's declared schema says string, so the model serializes to JSON that its own schema rejects:

Payload.model_json_schema()["properties"]["value"]
# {"pattern": "([0-9]+)", "type": "string"}

Payload(value=123).model_dump_json()
# '{"value":123}'
# jsonschema.validate(...) -> ValidationError: 123 is not of type 'string'

That same schema is what is handed to providers for structured output.

It also is not limited to numeric input. Because the guard was str(value), any object with a cooperative __str__ passed and was stored unconverted:

class Sneaky:
    def __str__(self): return "123"

Payload(value=Sneaky()).value   # accepted, still a Sneaky instance

Fix

Require values passed to Term.validate() to be strings before applying the compiled regular expression, using the existing is_str_instance helper so that str subclasses remain valid.

This aligns the runtime with the signature the method has carried since the DSL was introduced in da1b0db5 (def validate(self, value: str) -> str); the str() call was added in that same commit and reads as defensive rather than as an intended coercion feature.

Behaviour change

This is a behaviour change, not only a bug fix. Term.validate() also backs the built-in terms in outlines.types, which are Regex instances. Terms whose pattern happens to match the str() form of their Python counterpart previously accepted that value and now reject it:

annotation input before after
types.integer 123 accepted, stays int ValidationError
types.number 1.5 accepted, stays float ValidationError
types.boolean True accepted, stays bool ValidationError
types.integer "123" accepted accepted

Anyone annotating a Pydantic field with types.integer, types.number, or types.boolean and passing the corresponding Python value will see a ValidationError where the value was previously accepted.

Two notes on how large that surface actually is:

  • The terms already did not accept natural Python values in general. types.string is Regex(r'"[^"]*"'), so Model(field="abc") raises on main today and only '"abc"' passes. types.integer already rejected 123.0 while accepting 123. The leniency was incidental to str(), not designed.
  • There is no user-facing documentation example annotating a Pydantic field with these terms. They appear only in docs/guide/architecture.md, describing the internal Python-type-to-term mapping.

The alternative resolution is coercion: keep the str(value) match and return the string, so Payload(value=123) yields "123". That fixes the stated defect with a smaller compatibility surface. I did not take it because the accepted inputs are not only numbers — coercing would silently replace an unrelated object with "123" in the Sneaky case above, where the current behaviour at least leaves the object intact to be noticed. Happy to switch if maintainers prefer the coercing variant.

Scope

Term.validate() is reachable only through __get_pydantic_core_schema__ and __get_validator__. Nothing else in src/ calls it and no test calls it directly. Term.matches(), to_regex(), regex generation, and the model-provider integrations are unchanged — this affects only what happens when an already-produced value is handed to a Pydantic model, not constrained generation.

Tests

Added a regression test verifying that:

  • a matching string is accepted and retained as a string
  • a matching integer is rejected with a Pydantic ValidationError
  • a str subclass is still accepted, pinning that the guard is an isinstance check rather than an exact type check

Validation performed:

  • tests/types/test_dsl.py: 60 passed
  • tests/types: 261 passed
  • all configured pre-commit hooks passed, including mypy and Ruff
  • git diff --check passed

The full provider and model integration suite was not run locally because it requires heavyweight and environment-specific dependencies.

@github-actions

Copy link
Copy Markdown

📚 Documentation preview: https://dottxt-ai.github.io/outlines/pr-preview/pr-1944/

Preview updates automatically with each commit.

Comment thread src/outlines/types/dsl.py
pattern = to_regex(self)
compiled = re.compile(pattern)
if not compiled.fullmatch(str(value)):
if not is_str_instance(value) or not compiled.fullmatch(value):

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.

matches() still does fullmatch(str(value)) a few lines below, so Regex("[0-9]+").matches(123) remains true while validate(123) now rejects it. Should these agree on rejecting non-strings?

@ErenAta16 ErenAta16 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.

The defect is real and the reasoning holds. Ran it on a clone of main at be2cd15 against 4a05a12.

The declared schema is unambiguous about what the field is:

Regex("[0-9]+") as a pydantic field
  -> {"pattern": "([0-9]+)", "title": "Field", "type": "string"}

and on main the model happily contradicts it:

Model(field="123")  -> accepted, type=str, value='123'
Model(field=123)    -> accepted, type=int, value=123

So a model that advertises "type": "string" hands back an int. On this branch the second case raises, non-matching strings are still rejected as before, and a str subclass still passes, which is the case a naive type(value) is str check would have broken:

Model(field=MyStr("123"))  -> accepted, type=MyStr
Model(field="abc")         -> ValidationError

is_str_instance being the guard rather than an exact type check is the right call there.

The thing I would want settled before merge is the blast radius, which is wider than the description implies. The same validate serves the built-in terms, so those stop accepting their natural Python values:

                     main                       this PR
types.integer  123   accepted (stays int)       ValidationError
types.number   1.5   accepted (stays float)     ValidationError
types.boolean  True  accepted (stays bool)      ValidationError
types.integer  "123" accepted                   accepted

Whether that is correct depends on what types.integer is meant to be. As a regex term whose schema is {"type": "string", "pattern": ...} it is self-consistent to take strings only. As a name that reads like an integer field, Model(field=123) raising is going to surprise people. I checked the docs and there is no user-facing example annotating a pydantic field with types.integer, it only shows up in architecture.md describing the internal mapping, so this is not breaking a documented pattern. It is still a public name in outlines.types though, and this is a behaviour change rather than a pure bug fix for anyone who found it.

Worth noting the original complaint was about the returned value, not about acceptance:

This allowed matching non-string values to pass validation while remaining non-strings in the resulting model.

Rejecting is one way to resolve that. Coercing is the other: keep the str(value) match and return the string when it matches, so the field ends up a str consistent with the declared schema and Model(field=123) yields "123" instead of raising. That fixes the stated defect, the model no longer contains a non-string, with a much smaller compatibility surface. The cost is a silent 123 -> "123" conversion, which is its own kind of surprise.

I lean toward the strict version you have written, because silent coercion in a validator is harder to debug than a rejection and the schema really does say string. But it is a deliberate choice rather than an obvious one, and the PR description currently presents it as only a bug fix. Saying explicitly that built-in numeric and boolean terms no longer accept their Python values, and getting a maintainer to confirm that is intended, would be the thing that unblocks this.

One factual point in favour of merging either way: this touches validation only, not generation. I confirmed the generated regex for these terms is unchanged, so nothing about constrained output moves. The change is scoped to what happens when you hand an already-produced value to a pydantic model.

Test looks right for the strict reading. If you keep it, a case pinning that a str subclass is still accepted would be worth adding, since that is the part of the guard most likely to be tightened by accident later.

@hsusul

hsusul commented Aug 1, 2026

Copy link
Copy Markdown
Contributor Author

Thanks — fair read, and you're right the blast radius belongs in the description. Reproduced your table independently at cb095ba.

Two things keep me on the strict version rather than coercion:

The bad value doesn't stay contained — the model serializes to JSON its own schema rejects. M(field=123).model_dump_json() gives {"field":123}, and jsonschema says 123 is not of type 'string'. That schema is what gets handed to providers.

And it isn't only ints. str(value) admits anything with a cooperative str:

class Sneaky:
def str(self): return "123"

M2(field=Sneaky()) # main: accepted, type=Sneaky
123 -> "123" is a defensible coercion; Sneaky() -> "123" isn't — coercion makes that case worse, since today the object at least survives to be noticed.

Also worth noting the terms already don't take natural Python values: types.string is Regex(r'"[^"]*"'), so Model(field="abc") raises on main today.

Added the str subclass test and rewrote the description with the behaviour-change table.

@ErenAta16 ErenAta16 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.

Both points check out on my side, and they settle it. Dropping the coercion suggestion.

The containment argument is the one I had not followed through:

M(field=123)                  accepted on main, field stays int
M(field=123).model_dump_json()   {"field":123}
jsonschema.validate(that, M.model_json_schema())
    ValidationError: 123 is not of type 'string'

A model that serialises to something its own generated schema rejects is worse than I was treating it. I was thinking about the value sitting in an attribute; the schema is what goes to the provider, so the bad value does not stay local.

The Sneaky case is the part that actually kills coercion:

class Sneaky:
    def __str__(self): return "123"

M(field=Sneaky())    # main: accepted, type = Sneaky

str(value) is not a narrow int-to-str convenience, it accepts anything with a cooperative __str__. Coercing 123 to "123" is defensible in isolation; coercing an arbitrary object to "123" and storing that is not, and it would convert a case that today survives visibly into one that silently looks correct. So coercion does not just fail to fix the problem, it removes the evidence.

That leaves the compatibility surface as the only open question, and you have already agreed to put it in the description. With the reasoning above in there too, the built-in numeric and boolean terms no longer accepting their Python values reads as the intended consequence of a schema-conformance fix rather than as collateral damage.

Reproducing the table independently at cb095ba7 rather than taking mine is the right instinct, and it is why the Sneaky case surfaced at all. Nothing further from me.

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.

3 participants