Skip to content

Fix DSL booleans in JSON containers - #1947

Open
cyphercodes wants to merge 2 commits into
dottxt-ai:mainfrom
cyphercodes:cyphercodes/fix-dsl-bool-json-1942
Open

cyphercodes wants to merge 2 commits into
dottxt-ai:mainfrom
cyphercodes:cyphercodes/fix-dsl-bool-json-1942

Conversation

@cyphercodes

Copy link
Copy Markdown

Summary

Fixes #1942.

This updates DSL container handling so boolean values use JSON literals (true/false) when nested in list, tuple, or dict types, while preserving the existing standalone types.boolean behavior.

Tests

  • pytest tests/types/test_dsl.py -q
  • manual regex smoke for list[bool] and dict[str, bool]

Notes

I used AI assistance to prepare this focused bug fix.

Comment thread src/outlines/types/dsl.py Outdated
return String(f'"{term.value}"')
if isinstance(term, Regex):
if term.pattern == "(True|False)":
return Regex("(true|false)")

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.

This helper is also used for dict keys, so dict[bool, int] will now emit {true: ...}; that is still invalid JSON because object keys must be strings. Could the key path turn these into "true"/"false" instead?

@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 bug is real and I confirmed it by running both sides rather than reading the diff. Clone of main at be2cd15 against 75dcd31:

                       sample        main    this PR   valid JSON
list[bool]             [true]        False   True      yes
list[bool]             [True]        True    False     no
dict[str, bool]        {"k":true}    False   True      yes
dict[str, bool]        {"k":True}    True    False     no

So main generates [True], which no JSON parser accepts, and this branch fixes it. The standalone case stays Python-style in both, which is the boundary that matters:

bool standalone   'True' -> True    'true' -> False

Recursion is covered too, which I was not sure about from the diff alone. list[Optional[bool]] and list[Literal[True]] both reach [true] on this branch and neither did before, so the Alternatives path is fine.

Where I would push back is on how the boolean term is recognised. Matching on term.pattern means any user term that happens to share the spelling gets rewritten:

from outlines.types.dsl import Regex, _ensure_json_quoted
_ensure_json_quoted(Regex("True")).pattern
# main:     'True'
# this PR:  'true'

Same for Regex("False") and Regex("(True|False)"). Someone writing Regex("True") to match the literal word in prose, inside a list[...], silently gets a pattern that no longer matches their input. Regex.__eq__ is pattern-based, so there is nothing distinguishing the library's boolean from a user's identical one once you are looking at the pattern string.

This is worth reconciling with #1961, which is open against this exact function and goes the other way deliberately. It matches the temporal terms by identity and says why in a comment:

Matched by identity, since Regex equality is pattern-based and would also catch a user's own term that happens to share the pattern.

Two changes landing in _ensure_json_quoted with opposite conventions for the same problem would be an awkward place to end up, and they will conflict textually as well since both rewrite the same lines.

The complication, and I think this is why you reached for the pattern check, is that identity alone does not cover Literal[True]. types.boolean is a module-level singleton so term is types.boolean works, but a bool inside a Literal comes from a different place:

# python_types_to_terms, basic type instances
if isinstance(ptype, bool):
    return Regex(str(ptype))

That builds a fresh Regex("True") with no way to tell it apart downstream, which is exactly the term your third branch is catching. So the two cases have genuinely different origins and the pattern check is the only thing that unifies them after the fact.

The cleaner split is to handle each where the intent is still known: match types.boolean by identity here, and deal with the Literal bool at the branch above, where isinstance(ptype, bool) has already told you it is a boolean and not a user pattern that reads like one. That keeps the JSON spelling decision attached to something that actually knows it is looking at a bool, and it means a user's Regex("True") is never a candidate. It also lines this up with how #1961 handles the same shape of problem, so whichever lands second does not have to undo the other's convention.

Smaller note: the branches return Regex("true") as a new term rather than transforming the existing one, so anything a caller might have attached to the original instance is dropped. Not an issue with the current call sites, but it is the same identity-erasing move that makes the false positive above possible, so it is worth being deliberate about.

Tests look fine and test_literal_bool_in_container_uses_json_literals asserting not _re.fullmatch(pattern, "[True]") is the right negative to have. If you keep any form of pattern matching, a test pinning that a user's own Regex("True") survives untouched would be the one guarding the case above.

…ool-json-1942

# Conflicts:
#	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 that case on the branch and it doesn't fire — dict[bool, int] already quotes the key.

Dict[bool, int]  ->  \{("((true|false))":([+-]?(0|[1-9][0-9]*))(,\ "((true|false))":...
      '{true:1}'    matches=False   (invalid JSON)
      '{"true":1}'  matches=True    (valid JSON)
      '{True:1}'    matches=False   (invalid JSON)

_handle_dict passes the key through _ensure_json_quoted(..., quote_regex=True) while the value gets the plain call, with the comment right above saying why:

# JSON object keys must always be quoted strings, so quote the key term
# even when the Python key type isn't `str` (e.g. `Dict[int, str]`).
key_type = _ensure_json_quoted(python_types_to_terms(args[0], ...), quote_regex=True)
value_type = _ensure_json_quoted(python_types_to_terms(args[1], ...))

So the key path was already covered before this PR, and the change to the boolean term flows into it as "true"/"false" rather than bare true/false. Same for Dict[int, str], which is what that branch was originally written for.

The two paths this PR does change come out valid:

Dict[str, bool]  '{"k":true}'  matches=True   valid JSON
List[bool]       '[true]'      matches=True   valid JSON

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.

DSL container types render bool as Python True/False, producing invalid JSON and diverging from the JSON-schema path

3 participants