Repository navigation
[api][integrations][java][python] Raise a clear error for an output schema that cannot be rendered - #1046
Merged
Conversation
… constrain a response An output schema that pydantic cannot render surfaces a raw PydanticInvalidForJsonSchema naming a pydantic-internal type, with no mention of the caller's model or what to do about it. A schema that renders to an object declaring no properties is worse: it is accepted, and the caller receives an unconstrained response that looks schema-conforming. Add render_output_schema, which validates the model's own JSON Schema, then returns the injected renderer's output. Both steps raise TypeError with the original cause chained. The check rejects an object whose properties is present and empty, and accepts one where properties is absent, since absent denotes a free-form map. It runs against model_json_schema rather than the renderer's output because anthropic.transform_schema collapses every map-typed member into the same shape a field-less model produces, which would reject valid schemas. The renderer is injected as a callable so that api/ does not import openai or anthropic. No call site uses the helper yet. Generated-by: Claude Code v2.1.53 (claude-opus-5[1m])
…rain a response Route every Python output-schema render through render_output_schema, so a schema pydantic cannot render, or one that renders to an object declaring no properties, raises a clear TypeError naming the schema and the offending path instead of a raw pydantic error or a silently unconstrained request. The three connections differ because their pre-existing precedence differs. Anthropic lets a caller-supplied output_config win, so the schema is not rendered at all in that case; raising on a value that is about to be discarded would be a false failure. OpenAI overwrites a caller-supplied response_format, so an unconstrained schema really does reach the provider today, and raising is the point. Azure treats a caller response_format as a conflict, and that check now runs before the render so the caller still gets the conflict message. Hoisting the Azure check trades one behavior for another rather than restoring the old one: with a caller-supplied response_format, a field-less schema again raises the conflict as it did before, while an unrenderable schema now raises the conflict instead of a raw pydantic error. The conflict is the outer fault, so reporting it first is the more actionable message. At the Anthropic site a model pydantic renders but the vendor renderer rejects now raises TypeError where it raised ValueError. That is the one site where the new exception is not a subtype of the old. Generated-by: Claude Code v2.1.53 (claude-opus-5[1m])
…nstrain a response The Java mirror of the Python helper: walk a rendered schema and throw IllegalArgumentException when it contains an object declaring no properties, naming the offending path. Such a schema admits every response and rejects none, so a caller receives an unconstrained answer that looks conforming. An object whose properties is present and empty is rejected. One where properties is absent is accepted, because absent is what Jackson emits for a free-form map such as Map<String,String>. Inverting that distinction rejects ordinary models. The walk descends properties and items only. Measured against Jackson 2.18.2, generateJsonSchema emits no anyOf, oneOf, allOf, prefixItems, $ref or $defs, so descending them would be unreachable code. It carries no cycle guard: Jackson overflows the stack on a cyclic POJO before returning a node, and never shares node instances between siblings. Two shapes are rejected that a reader might not expect, both documented on the method. A @JsonAnyGetter class renders byte-identically to an unrenderable member, and a @JsonTypeInfo base carrying no fields of its own renders empty. No call site uses the check yet. Generated-by: Claude Code v2.1.53 (claude-opus-5[1m])
…onse ReActAgent pastes the rendered schema verbatim into the instruction prompt, so a schema that fails to render, or renders to an object declaring nothing, sends the model an instruction to match nothing. Validate it before building the prompt and raise IllegalArgumentException naming the offending path. The catch is widened rather than replaced. JsonMappingException is reachable through two colliding annotated getters, and IllegalArgumentException is what Jackson throws for a POJO it cannot serialize as an object. The schema check runs outside that catch: it throws IllegalArgumentException itself, and catching it there would bury the path in getCause(). The tests assert the absence of a cause rather than message text, because a wrapper that interpolates getMessage() still shows the path. The three connections are deliberately left alone. Validating them with Jackson would refuse schemas the providers accept: a @JsonTypeInfo member renders empty under Jackson while the SDK ships a full anyOf union, and Jackson throws on field-less, colliding-property and self-referential POJOs that the SDKs render and send today. Validating the SDK's own schema is not possible either, since it renders a map member and an unrenderable member identically. A @JsonTypeInfo POJO whose base carries no fields of its own is refused here. One shared field on the base renders and is accepted. Generated-by: Claude Code v2.1.53 (claude-opus-5[1m])
…path The issue asks for a clear error when an output schema cannot be rendered. Refusing a schema that renders successfully but expresses no constraint went beyond that, and after the Java side was scoped back it left the two languages disagreeing: a field-less model was refused at a Python connection and sent at the Java one, though pydantic, to_strict_json_schema and transform_schema all render it without complaint. No vendor difference explained the divergence. Python's three connections now keep the render-failure wrap and drop the constraint check. The check remains on the ReAct path, where the rendered schema is pasted verbatim into the instruction prompt, so an empty one instructs the model to match nothing. At a connection it is a wire format the provider is free to accept. One private body now sits behind two public names: render_provider_output_schema for connections, and render_constraining_output_schema for the prompt path. Neither is the bare render_output_schema, so a future prompt path cannot reach for an obvious name and silently lose the check. The Java check drops to package-private, its only caller being ReActAgent. Generated-by: Claude Code v2.1.53 (claude-opus-5[1m])
…put schema The chat contract said nothing about what a connection does with an output schema, so the rules lived only in private helper docstrings. State them where a caller reads them. A RowTypeInfo is skipped by a connection that translates natively, and the caller keeps the prompt-engineering fallback. That is deliberate and permanent, and it is a different case from a connection with no native translation at all, which rejects every schema it is given. Confusing the two was easy and the docstrings did not help. A schema that cannot be rendered raises with the cause chained. Two asymmetries are stated rather than smoothed over: that error is Python-only at connections, because the Java SDKs always render something, and the second raise site is per-provider, since a model with an Any-typed field is sent by OpenAI and refused by Anthropic. A schema that renders but expresses no constraint is accepted at a connection and rejected only on the ReAct prompt path, where the rendered document is pasted into the prompt. Documentation only. No code or test changes. Generated-by: Claude Code v2.1.53 (claude-opus-5[1m])
github-actions
Bot
added
doc-included
Closed
2 tasks done
wenjin272
reviewed
Aug 24, 2026
wenjin272
left a comment
wenjin272
left a comment
Contributor
There was a problem hiding this comment.
Thanks for working on this and for improving the error reporting! I left two comments below.
| } catch (JsonMappingException e) { | ||
| throw new RuntimeException(e); | ||
| schemaNode = mapper.generateJsonSchema(schemaClass).getSchemaNode(); | ||
| } catch (JsonMappingException | IllegalArgumentException e) { |
Contributor
There was a problem hiding this comment.
Could we also handle self-referential POJOs here? Jackson may throw StackOverflowError while generating their schemas, which currently leaks as a raw error. Please catch StackOverflowError specifically and add a regression test.
Collaborator
Author
There was a problem hiding this comment.
Thanks for the review!
Done. Turned out it isn't only direct self-reference: a mutual cycle, A holding a B that holds an A, blows the stack the same way. So the message talks about the schema referring back to itself rather than naming the direct case.
I gave it its own catch clause rather than adding to the existing one, since StackOverflowError carries no message and folding it in would have printed reported: null.
|
|
||
| properties = node.get("properties") | ||
| additional = node.get("additionalProperties") | ||
| if ( |
Contributor
There was a problem hiding this comment.
This check does not match JSON Schema semantics. For example, a field-less BaseModel with ConfigDict(extra="forbid") renders as {"type": "object", "properties": {}, "additionalProperties": false}, which accepts only {} and is therefore strongly constrained, but this branch rejects it. Conversely, omitting additionalProperties is semantically equivalent to allowing it, yet the omitted form is rejected while explicit true or {} is accepted. The check also misses a genuinely unconstrained RootModel[Any], which renders without type or properties. Since properties == {} is not a reliable test, and #985 only concerns render failures, shall we remove the unconstrained-schema check and handle that policy separately?
Collaborator
Author
There was a problem hiding this comment.
Yes, removed, in both languages.
Your examples hold up. I ran them against pydantic 2.11.4: the extra="forbid" case was being rejected even though it only admits {}, and RootModel[Any] passed straight through because the check tested type == "object". It was turning away the strictest schema and waving through the emptiest one.
Java had the same check and never looked at additionalProperties at all, so that went too. OutputSchema.java is back to its state on main.
Render failures are untouched, which is the part #985 was actually about.
Two knock-ons outside your comment, so they don't surprise you in the diff: with one renderer left the two long names collapsed into render_output_schema, which touches the imports in the three integrations, and the ReAct tests that asserted rejection now assert the opposite.
… check An output schema that renders but declares no fields was refused on the ReAct prompt path in both languages. The predicate did not match JSON Schema semantics: a field-less model forbidding extra fields renders with additionalProperties false and admits only the empty object, the tightest constraint expressible, and was refused, while a root model of any type renders carrying neither type nor properties, constrains nothing, and passed. Omitting additionalProperties is equivalent to allowing it, yet the omitted form was refused and the explicit form accepted. Such a schema is now returned as rendered and reaches the prompt. Reporting for a schema that cannot be rendered is unchanged, in both languages. The two Python renderers collapse into render_output_schema, since the pair existed only to separate the two policies. OutputSchema.java returns to its previous state. The Java message for an unsupported output schema now names the type it received, matching its Python counterpart. Generated-by: Claude Code v2.1.53 (claude-opus-5[1m])
Jackson has no cycle guard when generating a schema, so a class that reaches itself through its own members recurses until the stack is gone. The resulting StackOverflowError is an Error rather than an Exception, so it passed the catch guarding the generator and left the constructor raw, naming no schema and carrying no message of its own. It is now caught and reported like any other schema that cannot be rendered, naming the class and the self-reference. Direct and mutual recursion both reach it. The clause is separate from the one beside it because the error carries no message to quote. Pydantic renders a self-referential model through $defs and $ref and terminates, so Python needs no equivalent handling. A test pins that. Generated-by: Claude Code v2.1.53 (claude-opus-5[1m])
github-actions
Bot
added
doc-included
Collaborator
Author
|
The one failing check isn't related to this PR.
I'll re-run it once the rest of the run settles. |
wenjin272
approved these changes
Aug 31, 2026
wenjin272
left a comment
wenjin272
left a comment
Contributor
There was a problem hiding this comment.
Thanks for addressing my comments. LGTM
pranavshuklaa
pushed a commit
to pranavshuklaa/flink-agents
that referenced
this pull request
Sep 19, 2026
…chema that cannot be rendered (apache#1046) Generated-by: Claude Code v2.1.53 (claude-opus-5[1m])
1 task done
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Linked issue: #985
Purpose of change
An output schema that cannot be rendered as JSON Schema fails opaquely. Python leaks a raw
PydanticInvalidForJsonSchemanaming a pydantic-internal type, never the caller's model. Java throws past a catch clause too narrow to see it.Per the decision on the issue, such a schema is now refused with a clear, cause-chained error naming the schema.
RowTypeInfostays a fallback and is documented as one on the publicchatcontract, which previously said nothing about how a connection treats each kind of schema.A schema that renders but declares no fields is returned as rendered, in both languages. Whether such a document is usable belongs to whatever consumes it.
Observable changes:
TypeError, cause chainedIllegalArgumentException, cause chainedError, so it escaped the catch entirely. Now reported as a schema that refers back to itselfTypeError, notValueError. Only site where the new type is not a subtype of the oldoutput_config, so a discarded schema can no longer fail the callresponse_formatconflict is reported before the renderThe Java connections are deliberately untouched. Validating them with Jackson would refuse schemas the providers accept: a
@JsonTypeInfomember renders empty under Jackson while the SDK ships a fullanyOfunion, and Jackson throws on field-less, colliding-property and self-referential POJOs that the SDKs send today. Validating the SDK's own schema is no better, since it renders a map member and an unrenderable member identically. Probed with 15 hostile classes, the SDKs never failed to render, so there is no failure there to report.That leaves one asymmetry, imposed by the libraries rather than chosen: pydantic raises where the Java SDKs do not.
Not in scope: no change to
RowTypeInfohandling, no new dependency, no payload change for a schema that renders, andoutput_schemastill is not threaded from the runtime to a connection in either language.Tests
ReActAgentTestandtest_react_agent.pyare the first unit coverage ofReActAgentin either language.Per site: an unrenderable schema raises with the cause chained, a
dict/Mapmember is accepted and still sent whole, and a normal schema produces the payload it produced before.Two assertions are load-bearing:
StackOverflowError, which is the regression it exists to catch.$ref. A model that merely nests another model has$defsbut no top-level$ref, so the weaker guards pass on a non-cyclic model and would miss a regression.Both languages pin that an unsupported output schema type names the type it received, so neither message can silently stop reporting it.
Verified. Java:
mvn clean test -pl api380 passed, openai 104, anthropic 61, zero failures,spotless:checkclean. Python:pytest flink_agents --ignore=flink_agents/e2e_tests -m "not integration"940 passed, 13 skipped.check-license.shclean. The e2e failures in this worktree are pre-existing, from jars never built here.tools/lint.shcould not run locally (its Python step needs an editable install this pip is too old to perform).ruff check, which is what that step runs, is clean on every changed file.API
Yes. One helper in
flink_agents.api.agents.types:render_output_schema, used by the three chat model connections and by the ReAct prompt path. It renders through a callable the caller supplies, and reports a render failure clearly.No Java helper is added.
OutputSchema.javais unchanged.No existing signature changes. Exception types change as listed above, and two error messages change wording: the Java unsupported-type message now names the type, and the Python translation failure no longer describes its caller as a chat model, since the prompt path is also a caller.
Documentation
doc-includedWas this patch authored or co-authored using generative AI tooling?
Generated-by: Claude Code v2.1.53 (claude-opus-5[1m])