Skip to content

fix(types): JSON-quote date/time/datetime inside DSL containers - #1961

Open
bharadwaj-pendyala wants to merge 5 commits into
dottxt-ai:mainfrom
bharadwaj-pendyala:fix/json-quote-temporal-types-in-containers
Open

fix(types): JSON-quote date/time/datetime inside DSL containers#1961
bharadwaj-pendyala wants to merge 5 commits into
dottxt-ai:mainfrom
bharadwaj-pendyala:fix/json-quote-temporal-types-in-containers

Conversation

@bharadwaj-pendyala

@bharadwaj-pendyala bharadwaj-pendyala commented Jul 25, 2026

Copy link
Copy Markdown

Fixes #1960.

What broke

date, time and datetime inside a DSL container came out unquoted, so the generated regex only matched a string JSON can't parse.

to_regex(python_types_to_terms(List[datetime.date]))
# matches ["2024-01-01"] : False
# matches [2024-01-01]   : True   <- json.loads: Expecting ',' delimiter

Dict made the inconsistency obvious. _handle_dict passes quote_regex=True for keys, so dict[date, str] produced {"2024-01-01":"x"}, while dict[str, date] produced {"d":2024-01-01}. Same type, opposite treatment, one object. The key half landed in #1917 and the value half was missed.

The fix

  • Quote the built-in temporal terms in _ensure_json_quoted, alongside the String literals that already get quoted there.
  • Match them by identity, not by pattern. Regex is a dataclass so equality compares pattern text, which would also grab a user's own Regex(types.date.pattern). There's a test pinning that.
  • Standalone date/time/datetime are untouched, same as the deliberate Python-style types.boolean.

Identity carries no provenance, so list[types.date] (handed in as a term) and list[datetime.date] (from the annotation) produce the same quoted output. That is deliberate and pinned by a test.

Out of scope, and unchanged by this PR:

Test plan

  • pytest tests/types passes (296 tests)
  • 6 new test functions, 10 parametrized items. 9 fail on main and pass here; the tenth is the user-Regex guard, which passes on both by design
  • list/dict/tuple covered for all three temporal types, parametrized
  • terms handed in directly (list[types.date]) covered separately from the annotation route (list[datetime.date]), since a move away from identity matching would drop the first while the second kept passing
  • every container assertion has its negative half: the quoted form matches and the bare form no longer does
  • Optional[date] in a container quotes the date, leaves bare None alone
  • user Regex sharing types.date.pattern still renders bare
  • standalone date/time/datetime still match unquoted
  • pre-commit run --files src/outlines/types/dsl.py tests/types/test_dsl.py clean (mypy, ruff)

Note tests/test_generator.py::test_steerable_generator_init_valid_processor errors on a clean checkout of main too (a DeprecationWarning from tokenizers BPE), unrelated to this change.

list[date], dict[str, date] and tuple[date, ...] generated regexes that
only matched a bare 2024-01-01, which json.loads rejects. Dict keys of
the same type were already quoted by _handle_dict's quote_regex path, so
dict[date, str] and dict[str, date] disagreed about the same type.

Quote the built-in temporal terms in container contexts too. They are
matched by identity rather than by pattern, because Regex equality is
pattern-based and would also capture a user's own term that happens to
reuse types.date.pattern. Standalone date/time/datetime are unchanged.

Fixes dottxt-ai#1960
@bharadwaj-pendyala
bharadwaj-pendyala marked this pull request as ready for review July 25, 2026 18:42
Comment thread src/outlines/types/dsl.py
The docstring described a general "JSON has no literal syntax for it"
rule while the code only covers the temporal terms python_types_to_terms
returns for a Python type. Built-in terms passed straight through, such
as list[types.email], take the isinstance(ptype, Term) shortcut and stay
bare. Point at dottxt-ai#1962 rather than imply they are handled.
Comment thread src/outlines/types/dsl.py

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

Checked this by running both sides rather than reading the diff. Fresh clone of main at be2cd15, then the same script against 08ba48a, comparing re.fullmatch(to_regex(python_types_to_terms(T)), sample) for a set of samples I picked so that the JSON-valid and JSON-invalid form of each is tested separately.

Before, on main:

List[date]              ["2024-01-01"]            match=False   (valid JSON, rejected)
List[date]              [2024-01-01]              match=True    (invalid JSON, accepted)
List[time]              ["20:20:20"]              match=False
List[datetime]          ["2024-01-01 20:20:20"]   match=False
Dict[str, date]         {"k":"2024-01-01"}        match=False
Dict[date, str]         {"2024-01-01":"x"}        match=True    (already worked via quote_regex)

After, on this branch, every one of those flips the right way and Dict[date, str] stays where it was. So the asymmetry the description points at, keys quoted but values not, is real and this closes it.

The part I wanted to be sure about was the condition change from if quote_regex and isinstance(term, Regex) to if isinstance(term, Regex), since read quickly that looks like it drops the guard and would start quoting every bare Regex in list context. It does not, the guard moved into the inner if quote_regex or any(...), but it is worth saying out loud that this is what the nesting does. I pinned it anyway:

List[int]     [1]      match=True     ["1"]   match=False
List[float]   [1.5]    match=True
List[str]     ["a"]    match=True

No movement there, which is the outcome you want.

Identity matching is the right call and the comment explaining why belongs there. python_types_to_terms returns the module-level singletons directly (return types.date at line 764 and its neighbours), so term is types.date holds, and Regex.__eq__ being pattern-based really would have swept up a user term that happened to share the pattern. I also checked the recursion, since Alternatives passes the member through untouched and identity survives that:

List[Optional[date]]              ["2024-01-01"]      match=True
List[List[date]]                  [["2024-01-01"]]    match=True
List[Union[date, time]]           ["2024-01-01"]      match=True

All three were False before. Good that test_e2e_temporal_types_quoted_in_containers pins the standalone case staying bare in the same test, because that is the boundary this change could plausibly have crossed and it did not.

One thing to correct before merge, in the new docstring paragraph:

Built-in terms passed straight through (e.g. list[types.email]) are not covered: they reach this function via the isinstance(ptype, Term) shortcut in python_types_to_terms and stay bare. See issue #1962.

That generalisation is not true for the three types this PR is about. Identity matching does not care which route the term took to get here, so a built-in temporal term passed straight through still gets quoted:

list[types.date]        ["2024-01-01"]            quoted in output: True
list[types.time]        ["20:20:20"]              quoted in output: True
list[types.datetime]    ["2024-01-01 20:20:20"]   quoted in output: True
list[types.email]       ["a@b.co"]                quoted in output: False
list[types.isbn]        ["978-0-596-52068-7"]     quoted in output: False

So the carve-out holds for email and isbn and everything else in types, and not for date, time and datetime. As written, someone reading it would conclude list[types.date] is still broken and would scope #1962 to include a case this PR already fixed. Narrowing it to something like "other built-in terms (e.g. list[types.email]) are not covered; the temporal terms are, since they are matched by identity regardless of how they reach this function" would say what the code does.

Worth adding a test for that route too, list[types.date] alongside list[datetime.date], since right now nothing pins it and it is the kind of thing a later refactor away from identity matching would silently drop.

Smaller point on test_e2e_temporal_types_quoted_in_containers: it asserts the container pattern matches ["2024-01-01"] but not that it stops matching [2024-01-01]. The positive assertion is enough to fail on old code so it is a real RED test, but the negative one is what guards against the quoting being added as an extra alternative rather than replacing the bare form. Cheap to add next to the line that is already there.

Nothing blocking from my side beyond the docstring sentence.

The paragraph added in 08ba48a said built-in terms reaching
`_ensure_json_quoted` through the `isinstance(ptype, Term)` shortcut stay
bare. That holds for `email`, `isbn` and the rest, but not for the three
terms this branch quotes: identity matching doesn't look at how the term
arrived, so `list[types.date]` is quoted just like `list[datetime.date]`.
As written the note would have sent a reader to dottxt-ai#1962 for a case already
fixed here. The first paragraph carried the same route-scoped phrasing
and is narrowed too.

The inline comment claimed `types` was read at call time "rather than at
module level", which contradicts the `import outlines.types as types` at
the top of the file. What can't happen at module level is the attribute
lookup: `outlines/types/__init__.py` imports this module before it binds
`date`/`time`/`datetime`, so hoisting the tuple to a constant would read
names that don't exist yet. Reworded to say that.

Add a parametrized test for the direct-term route across list, dict value
and tuple. Nothing covered it before, and it's what would fail first if
the identity check were swapped for pattern equality. Dict keys are left
out on purpose, since `quote_regex` quotes those either way and the test
would still pass with identity matching gone.
@bharadwaj-pendyala
bharadwaj-pendyala force-pushed the fix/json-quote-temporal-types-in-containers branch from bb8b946 to 2ea6c61 Compare August 1, 2026 01:32

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

On the denylist proposal: it's the right shape, but there's a fifth term that belongs with your four rather than in the "quote it" group. isbn can't be made to work by quoting, for a different reason than the control characters.

Measured on main:

to_regex(python_types_to_terms(isbn))  vs  '9783161484100'      -> True
to_regex(python_types_to_terms(isbn))  vs  '978-3-16-148410-0'  -> True
to_regex(python_types_to_terms(List[isbn]))  vs  '[9783161484100]'    -> False
to_regex(python_types_to_terms(List[isbn]))  vs  '["9783161484100"]'  -> False
to_regex(python_types_to_terms(List[isbn]))  vs  '[]'                 -> False

Nothing matches list[isbn] at all — not the bare form, not the quoted form, not even the empty list. The term carries four $ anchors inside its lookaheads:

(?:ISBN(?:-1[03])?:? )?(?=[0-9X]{10}$|(?=(?:[0-9]+[- ]){3})[- 0-9X]{13}$|97[89][0-9]{10}$|...

and once that sits inside \[(...)\] the $ can't match, so the whole alternation is unsatisfiable. Adding quotes around it doesn't change that.

Worth separating from a "does it contain $" rule, because that would flag the wrong set: email also has two $ characters, but they're inside a character class rather than acting as anchors, and list[email] matches [a@b.com] fine on main. So the predictor is anchor semantics, not the character.

That makes the split five-and-eleven rather than four-and-twelve for the denylist version, and isbn needs the anchors dropped from the term itself — which changes what bare isbn accepts, so it's the same "maintainer's call first" bucket as newline/whitespace/paragraph/sentence.

Keeping this PR scoped to the reported date/time/datetime bug still seems right to me; the above is for whoever picks up #1962.

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

Sanjays2402's reading is right — measured on this branch:

List[datetime.date]  (Python type)      bare=False  quoted=True
List[types.date]     (term passed in)   bare=False  quoted=True
List[types.email]    (term passed in)   bare=True   quoted=False

So the identity check doesn't distinguish "reached here from a Python type" from "handed in as a term". python_types_to_terms(datetime.date) returns the same types.date object the user would pass directly, so term is types.date is True either way, and list[types.date] comes out quoted while list[types.email] — same shape, same isinstance(ptype, Term) shortcut — stays bare.

That means the note about built-in terms passed straight through staying bare doesn't hold for the three this PR names, and the split between types.date and types.email isn't one a caller can predict from anything visible.

It's not fixable by tightening the identity check, because the two paths are genuinely indistinguishable at that point — same object, no provenance. Which I think is the argument for the denylist shape you proposed on #1962: with "quote everything except the JSON scalars", types.date and types.email land in the same bucket by construction and the question stops existing.

Worth noting the current behaviour isn't wrong for list[types.date] — quoting is what it needs. The problem is only that list[types.email] doesn't get the same treatment, so this is an under-reach rather than a regression, and scoping this PR to the reported bug still holds up. The inconsistency argues for #1962 landing after it rather than for changing anything here.

The carve-out paragraph blamed the `isinstance(ptype, Term)` shortcut for
`list[types.email]` staying bare, but the same paragraph then said the
temporal terms take that route and are quoted anyway. The route is not the
reason; the identity tuple is. Say that instead.
@bharadwaj-pendyala

Copy link
Copy Markdown
Author

All three are in.

Docstring. 2ea6c61 narrowed it but it was still wrong, it kept blaming the isinstance(ptype, Term) shortcut for other built-ins staying bare and then said in the next sentence that the temporal terms take that route and get quoted anyway. The route was never the reason, the identity tuple is. Rewritten.

Direct-term test. test_e2e_temporal_term_passed_directly_quoted_in_containers, parametrized over the three terms, list plus dict value plus tuple. Fails on main for all three.

Negative assertion. assert not re.fullmatch(list_pattern, f"[{value}]") sits next to the positive one now, same for dict and tuple.

isbn confirmed, and it does belong with your four. I stripped the anchors to check they're the cause and not just correlated:

Regex(isbn.pattern.replace('$', ''))  in a list, bare    '[9783161484100]'    -> True
Regex(isbn.pattern.replace('$', ''))  in a list, quoted  '["9783161484100"]'  -> True

Unsatisfiable with them, fine without. And your point about email holds, two $ inside [a-z0-9!#$%&'*+/=?^_{|}~-], list[email]matches[a@b.com]onmain`.

One correction, on the evidence not the conclusion. The [] line isn't isbn-specific:

list[int]        '[]'  -> False
list[str]        '[]'  -> False
list[types.isbn] '[]'  -> False

_handle_list builds \[(item)(,\ (item))*\], which needs at least one element, so no list[T] matches []. Worth not carrying into #1962 as an isbn symptom.

Checking the buckets turned up a second thing wrong in #1962, mine this time: the expected-result block says list[bool] matches [true]. types.boolean is (True|False), so it's [True] and [true] is rejected. Python literals, not JSON, separate bug. I've fixed that line and moved isbn into the third bucket, so the split reads eleven-and-five now.

Nothing else outstanding here.

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

All three land, and the docstring is now saying the right thing rather than a narrower version of the wrong thing.

The sentence that matters is the one you rewrote: the match is on identity and carries no notion of how the term arrived, so list[types.date] and list[datetime.date] produce the same output. That is the fact I measured and could not reconcile with the earlier wording, and attributing it to the identity tuple rather than to the isinstance(ptype, Term) route is what makes #1962 scopeable, since it says exactly which built-ins are still uncovered and why.

test_e2e_temporal_term_passed_directly_quoted_in_containers, parametrized across the three terms and covering list, dict value and tuple, pins the route that had no coverage at all. And the negatives are in beside the positives:

assert not _re.fullmatch(list_pattern,  f"[{value}]")
assert not _re.fullmatch(dict_pattern,  f'{{"a":{value}}}')
assert not _re.fullmatch(tuple_pattern, f"({value})")

which is the assertion that catches quoting being added as an extra alternative rather than replacing the bare form. A positive-only test passes either way.

Keeping the identity-versus-equality comment next to the check is worth it for the same reason as before: Regex.__eq__ is pattern-based, so a later "simplification" to == would silently start catching a user's own term that happens to share a pattern, and nothing in the suite would fail.

Nothing further from me.

@bharadwaj-pendyala

Copy link
Copy Markdown
Author

Review here has closed out, so this is ready when you have a slot.

Sanjays2402 and ErenAta16 both went through it by running the branch against main rather than by reading the diff. Two things came out of that and are now fixed. The docstring paragraph about built-in terms was wrong: identity matching carries no provenance, so list[types.date] and list[datetime.date] produce the same output, and attributing the carve-out to the isinstance(ptype, Term) route said something the code does not do. And the container tests were positive-only. test_e2e_temporal_term_passed_directly_quoted_in_containers now pins the direct-term route across list, dict value and tuple, and the negative assertions sit beside the positives, so quoting added as an extra alternative rather than replacing the bare form would fail instead of pass.

Scope is unchanged: date, time and datetime. The other built-in Regex terms are #1962. One of them, isbn, is a different defect that quoting will not fix, since its four $ anchors sit inside lookaheads and leave list[isbn] matching nothing at all, not even []. That belongs in #1962.

Branch is 5 ahead of main and 0 behind, no conflicts.

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.

DSL containers leave date/time/datetime unquoted, producing output json.loads rejects

3 participants