[api][integrations][java][python] Raise a clear error for an output schema that cannot be rendered - #1046
Open
weiqingy wants to merge 6 commits into
Open
[api][integrations][java][python] Raise a clear error for an output schema that cannot be rendered#1046weiqingy wants to merge 6 commits into
weiqingy wants to merge 6 commits into
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
Open
2 tasks
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.
|
|
||
| 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?
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
2 participants
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 constrains nothing is refused only on the ReAct path, where the rendered document is pasted verbatim into the prompt, so an empty one tells the model to match nothing. At a connection it is a wire format the provider may accept, and it is passed through as before.
Observable changes:
TypeError, cause chainedIllegalArgumentException, cause chained@JsonTypeInfobase with no fields of its own; one shared field renders and is acceptedTypeError, 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:
getMessage()still shows the path even when the error has been buried.The map-member tests pin the distinction the check rests on:
propertiespresent and empty constrains nothing,propertiesabsent is a free-form map and is valid. Inverting that rejects ordinary models.Verified: Python 985 passed, no non-e2e failures. Java
api387, openai 104, anthropic 61, zero failures.spotless:checkandcheck-license.shclean. The 27 e2e failures here are pre-existing, from jars never built in this worktree.tools/lint.shcould not run locally (its Python step needs an editable install this pip is too old to perform).ruff checkandruff format --checkwere run directly on the changed files and are clean.API
Yes. Two helpers in
flink_agents.api.agents.types:render_provider_output_schemafor connections,render_constraining_output_schemafor the prompt path. Two names rather than a flag, so the call site states which behaviour it wants, and neither takes the bare namerender_output_schemaso a future prompt path cannot silently skip the check.OutputSchema.rejectUnconstrainedSchemais package-private, its only caller beingReActAgent.No existing signature changes. Exception types change as listed above.
Documentation
doc-includedWas this patch authored or co-authored using generative AI tooling?
Generated-by: Claude Code v2.1.53 (claude-opus-5[1m])