fix: handle boolean JSON Schema nodes in JsonSchemaTransformer#4772
fix: handle boolean JSON Schema nodes in JsonSchemaTransformer#4772guoyangzhen wants to merge 8 commits intopydantic:mainfrom
Conversation
JSON Schema allows boolean values as schemas: true means accept anything
(equivalent to {}) and false means accept nothing. The _handle method
assumed schema is always a dict, causing AttributeError: bool has no
attribute get when encountering boolean schema nodes.
Fixes pydantic#4771
There was a problem hiding this comment.
📝 Info: items: false skipped by falsy walrus check in _handle_array, but functionally correct
At line 154, if items := schema.get('items'): will skip items: false because False is falsy. This means _handle(False) is never called for this case. However, since _handle(False) returns False immediately (line 86-87), the end result is identical — the value is preserved as-is. This is consistent with how additionalProperties: false is handled in _handle_object (line 137, explicit isinstance(bool) check). The inconsistency in approach is worth noting but doesn't cause incorrect behavior.
Was this helpful? React with 👍 or 👎 to provide feedback.
Addresses Devin review finding. transform() implementations (OpenAI, Anthropic, Google) call dict methods like schema.pop() which crashes on bool values. Boolean schemas should be returned as-is since they have no sub-schemas to recurse into.
|
Good catch from Devin Review! Fixed — boolean schemas are now returned directly from The |
Addresses Devin Review finding: when prefer_inlined_defs=True and a
$ref resolves to a boolean $defs entry (valid JSON Schema), the while
loop would crash on bool.get('$ref'). Now breaks out of the loop and
returns the boolean schema directly.
Co-Authored-By: Devin <devin-ai-integration[bot]@users.noreply.github.com>
|
Fixed the to boolean schema issue in 41e5f86. Added boolean check inside while loop after resolution. Good catch! |
- Add isinstance(schema, bool) check at top of _handle - In _handle_union, return boolean directly instead of bool|dict merge Addresses Devin Review: _handle_union crashes with TypeError when a single union member is a boolean schema
|
Fixed in 076daf5. Two changes:
Good catch on the |
Devin correctly identified that _handle_union can return a bool (e.g.,
when a union has a single boolean member like {'oneOf': [True]}),
which then crashes when self.transform() is called on it.
Added isinstance(schema, bool) guard after _handle_union calls in _handle
to short-circuit before calling transform().
When _handle_union returns a bool schema, the early return was skipping refs_stack cleanup, causing stale entries that could trigger false circular reference detection on subsequent resolutions. Addressing Devin Review feedback.
|
Fixed the When # Before
if isinstance(schema, bool):
return schema # skips refs_stack cleanup!
# After
if isinstance(schema, bool):
if nested_refs > 0:
self.refs_stack = self.refs_stack[:-nested_refs]
return schemaGood catch from Devin Review! |
|
Thanks for the thorough review! I've reviewed the remaining findings:
def test_boolean_schema_in_union():
"""Test boolean schemas inside anyOf/oneOf unions."""
class PassthroughTransformer(JsonSchemaTransformer):
def transform(self, schema: dict[str, Any]) -> dict[str, Any]:
return schema
# oneOf with boolean members
schema = {"oneOf": [True, {"type": "string"}]}
result = PassthroughTransformer(schema).walk()
assert "oneOf" in result
assert True in result["oneOf"]
assert {"type": "string"} in result["oneOf"]
# single-element oneOf resolving to bool
schema2 = {"oneOf": [True]}
result2 = PassthroughTransformer(schema2).walk()
assert result2 is True |
|
Thanks Devin! Good point about union-level booleans. The current code handles them correctly through the recursive I can add a test covering cases like |
|
@guoyangzhen Thanks for working on this but we don't love fully automated PRs (which this one is judging by the 🦞 emoji). Is there a human in the loop here? :) In any case, please fix test coverage. |
|
Hi DouweM — yes, there's a human in the loop here. I use automation to assist with code analysis and drafting, but I review every change before submitting and I'm actively engaged in responding to feedback. I'll add test coverage for union-level boolean schemas (e.g. |
…f handler When anyOf simplifies to a single member (e.g., anyOf: [False] → False), the subsequent oneOf handler would crash on bool.pop(). Add isinstance check between the two union handlers. Also add comprehensive test coverage for union-level boolean schemas: - Boolean schemas in / - Deeply nested boolean schemas - Boolean schemas as additionalProperties - Boolean schemas in prefixItems (tuple validation)
|
Hi DouweM — I've pushed additional test coverage as requested: New tests added:
Bug fix discovered while adding tests: All 8 tests pass locally. Would appreciate another look when you get a chance. |
There was a problem hiding this comment.
🚩 Downstream walk() overrides assume dict return type
The OpenAIJsonSchemaTransformer.walk() override at pydantic_ai_slim/pydantic_ai/profiles/openai.py:230 calls result.pop('$ref', None) and result.update(...) on the return value of super().walk(). The AnthropicJsonSchemaTransformer.walk() at pydantic_ai_slim/pydantic_ai/providers/anthropic.py:126 passes the result to transform_schema(). Both would crash if walk() returned a boolean. In practice, these transformers only process Pydantic-generated schemas (which always have object or $ref roots), so the boolean case is currently unreachable through these code paths. However, the type contract of walk() has silently changed with this PR — the return type annotation still says JsonSchema (i.e., dict[str, Any]) but the method can now return bool. This mismatch could cause issues if the API is used more broadly in the future.
(Refers to line 57)
Was this helpful? React with 👍 or 👎 to provide feedback.
| if isinstance(schema, bool): | ||
| if nested_refs > 0: | ||
| self.refs_stack = self.refs_stack[:-nested_refs] | ||
| return schema |
There was a problem hiding this comment.
🟡 walk() crashes with TypeError when _handle returns bool and $defs are present
The walk() method at pydantic_ai_slim/pydantic_ai/_json_schema.py:64-65 assumes handled (the return value of _handle) is always a dict, but with the new bool handling, _handle can now return a boolean. When the root schema resolves to a bool (e.g., {'anyOf': [True], '$defs': {'Foo': {'type': 'string'}}}) and self.defs is non-empty, line 65 executes handled['$defs'] = ... on a boolean, raising TypeError: 'bool' object does not support item assignment. While unlikely with Pydantic-generated schemas (which always have object roots), this is a valid JSON Schema pattern that the PR intends to support.
Prompt for agents
In pydantic_ai_slim/pydantic_ai/_json_schema.py, in the walk() method (lines 57-83), add a bool guard after the _handle call on line 62 and before the if/elif that accesses handled as a dict on line 64. For example:
handled = self._handle(schema)
if isinstance(handled, bool):
return handled
if not self.prefer_inlined_defs and self.defs:
handled['$defs'] = {k: self._handle(v) for k, v in self.defs.items()}
...
This ensures that when the root schema resolves to a boolean, it is returned directly without attempting dict operations on it.
Was this helpful? React with 👍 or 👎 to provide feedback.
Fixes #4771
Problem
JSON Schema allows boolean values as schemas:
true= accept anything (equivalent to{})false= accept nothing (equivalent to{"not": {}})JsonSchemaTransformer._handle()callsschema.get("type")which assumesschemais always a dict. When a boolean schema node liketrueappears inproperties,items, etc., the walker crashes:Fix
Add an
isinstance(schema, bool)check at the start of_handle(). Boolean schemas are passed directly totransform()(same as schemas with no specifictype). This is correct per the JSON Schema spec where boolean schemas have no sub-schemas to recurse into.Test
Added
test_boolean_schema_nodesthat verifies bothtrueandfalseboolean nodes are preserved when walking a schema containing them inproperties.