wenjin272 commented on code in PR #1046:
URL: https://github.com/apache/flink-agents/pull/1046#discussion_r3842285953
##########
python/flink_agents/api/agents/types.py:
##########
@@ -65,3 +65,177 @@ def __custom_deserialize(self) -> "OutputSchema":
module = importlib.import_module(output_schema["module"])
self["output_schema"] = getattr(module, output_schema["class"])
return self
+
+
+def render_provider_output_schema(
+ model: type[BaseModel], render: Callable[[type[BaseModel]], dict[str, Any]]
+) -> dict[str, Any]:
+ """Render an output schema for a provider, reporting a render failure
clearly.
+
+ The renderer is supplied by the caller because each chat model translates a
+ schema with its own vendor renderer, and this module may not depend on any
of
+ them. The model's own render runs first so that a model no JSON Schema can
+ express is reported as exactly that: a vendor renderer renders the model
+ itself, so it would otherwise surface the same failure worded as a
translation
+ failure. The vendor render is wrapped in turn because it rejects models the
+ model's own render accepts, such as one carrying an untyped member that
renders
+ to a document with no ``type``.
+
+ A schema that renders but declares no fields is returned as rendered.
Whether
+ such a document is usable is the receiving provider's to judge, and
refusing it
+ here would fail a request that succeeds today. Callers whose deliverable
is the
+ rendered document itself have no provider to defer to and use
+ ``render_constraining_output_schema`` instead.
+
+ Args:
+ model: The model class describing the shape the response must take.
+ render: Renders ``model`` in the wire format the chat model expects.
+
+ Returns:
+ The document ``render`` produced.
+
+ Raises:
+ TypeError: If ``model`` has no JSON Schema, or if ``render`` fails or
returns
+ something other than a document.
+ """
+ return _render_output_schema(model, render, reject_unconstrained=False)
+
+
+def render_constraining_output_schema(
+ model: type[BaseModel], render: Callable[[type[BaseModel]], dict[str, Any]]
+) -> dict[str, Any]:
+ """Render an output schema, refusing one that cannot constrain the
response.
+
+ For a caller that consumes the rendered document itself rather than
sending it
+ to a provider, such as one pasting it into an instruction prompt. A
document
+ describing an object with no fields instructs the model to match nothing,
and
+ every response then satisfies it.
+
+ Whether a model expresses a constraint is a property of the model, not of a
+ provider's wire format, so the check runs against the model's own JSON
Schema
+ and only the returned document comes from ``render``. That keeps the check
+ independent of how any SDK normalizes a schema: one renderer rewrites every
+ map-typed member into an empty object, which is indistinguishable from a
model
+ that declares no fields at all.
+
+ Args:
+ model: The model class describing the shape the response must take.
+ render: Renders ``model`` in the wire format the caller expects.
+
+ Returns:
+ The document ``render`` produced.
+
+ Raises:
+ TypeError: Everything ``render_provider_output_schema`` raises, and
+ additionally if ``model`` renders to a JSON Schema containing an
object
+ that declares no properties and so constrains nothing.
+ """
+ return _render_output_schema(model, render, reject_unconstrained=True)
+
+
+def _render_output_schema(
+ model: type[BaseModel],
+ render: Callable[[type[BaseModel]], dict[str, Any]],
+ *,
+ reject_unconstrained: bool,
+) -> dict[str, Any]:
+ """Back the two public renderers, which differ only in the emptiness
check."""
+ try:
+ document = model.model_json_schema()
+ except Exception as e:
+ msg = (
+ f"Output schema {model.__module__}.{model.__qualname__} cannot be"
+ " rendered as a JSON Schema, so it cannot constrain the response.
Use a"
+ " schema whose fields are all JSON-Schema-renderable, or pass no
output"
+ f" schema. Rendering it reported: {e}"
+ )
+ raise TypeError(msg) from e
+
+ if reject_unconstrained:
+ defs = document.get("$defs")
+ _reject_empty_objects(
+ document, "$", defs if isinstance(defs, dict) else {}, set(), model
+ )
+
+ try:
+ schema = render(model)
+ except Exception as e:
+ msg = (
+ f"Output schema {model.__module__}.{model.__qualname__} cannot be"
+ " translated for this chat model, so it cannot constrain the
response."
+ " Use a schema whose fields are all JSON-Schema-renderable, or
pass no"
+ f" output schema. The renderer reported: {e}"
+ )
+ raise TypeError(msg) from e
+ if not isinstance(schema, dict):
+ msg = (
+ f"Output schema {model.__module__}.{model.__qualname__} rendered
to"
+ f" {type(schema).__name__} rather than a JSON Schema document, so
it"
+ " cannot constrain the response. Supply a renderer that returns a
JSON"
+ " Schema document, or pass no output schema."
+ )
+ raise TypeError(msg)
+ return schema
+
+
+def _reject_empty_objects(
+ node: Any,
+ path: str,
+ defs: dict[str, Any],
+ visited: set[int],
+ model: type[BaseModel],
+) -> None:
+ """Raise if any object below ``node`` declares an empty ``properties``.
+
+ ``properties`` present and empty is an object that admits every response
and
+ rejects none. ``properties`` absent is a free-form map such as
+ ``dict[str, str]``, which is a legitimate constraint and is left alone.
+
+ An empty ``properties`` still constrains something when
``additionalProperties``
+ carries a schema, which bounds every extra member, or ``True``, which
admits
+ them deliberately. Only an absent or ``False`` ``additionalProperties``
leaves
+ the object expressing nothing. The tests are identity comparisons because a
+ JSON Schema document may hold ``{}`` there, which is a legitimate schema
and is
+ falsy in Python.
+
+ Descends through ``properties``, ``items``, ``prefixItems``, a
schema-valued
+ ``additionalProperties``, the ``anyOf``/``oneOf``/``allOf`` branches, and
any
+ ``$defs`` entry a ``$ref`` reaches.
+ """
+ if not isinstance(node, dict) or id(node) in visited:
+ return
+ visited.add(id(node))
+
+ ref = node.get("$ref")
+ if isinstance(ref, str):
+ prefix = "#/$defs/"
+ target = defs.get(ref[len(prefix) :]) if ref.startswith(prefix) else
None
+ _reject_empty_objects(target, path, defs, visited, model)
+ return
+
+ properties = node.get("properties")
+ additional = node.get("additionalProperties")
+ if (
Review 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?
##########
api/src/main/java/org/apache/flink/agents/api/agents/ReActAgent.java:
##########
@@ -69,11 +70,28 @@ public ReActAgent(
jsonSchema = outputSchema.toString();
outputSchema = new OutputSchema((RowTypeInfo) outputSchema);
} else if (outputSchema instanceof Class) {
+ Class<?> schemaClass = (Class<?>) outputSchema;
+ JsonNode schemaNode;
try {
- jsonSchema = mapper.generateJsonSchema((Class<?>)
outputSchema).toString();
- } catch (JsonMappingException e) {
- throw new RuntimeException(e);
+ schemaNode =
mapper.generateJsonSchema(schemaClass).getSchemaNode();
+ } catch (JsonMappingException | IllegalArgumentException e) {
Review 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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
