wenjin272 commented on code in PR #1098:
URL: https://github.com/apache/flink-agents/pull/1098#discussion_r3978484024
##########
integrations/chat-models/gemini/src/main/java/org/apache/flink/agents/integrations/chatmodels/gemini/GeminiChatModelConnection.java:
##########
@@ -283,9 +377,115 @@ private GenerateContentConfig buildConfig(
builder.tools(List.of(convertTools(tools)));
}
+ // Native structured output applies only for a POJO Class schema; any
other schema form,
+ // such as a RowTypeInfo wrapped in OutputSchema, keeps the
prompt-engineering fallback.
+ // Nothing above writes either field this branch sets: the keys read
directly are
+ // temperature and max_output_tokens, and applyAdditionalKwargs
recognizes only top_k,
+ // top_p and stop_sequences, so there is no caller-supplied value to
collide with.
+ //
+ // TODO(#912): the requested strategy is not visible here, so this
re-check cannot tell an
+ // explicit NATIVE request apart from one that merely resolved to
native. A caller asking
+ // for NATIVE on a schema form, a model, or a tool-carrying request
this branch skips
+ // therefore gets an unconstrained response instead of an error. Once
strategy resolution is
+ // wired up, NATIVE must either bypass this capability re-check or
fail explicitly.
+ if (outputSchema instanceof Class
+ && (tools == null || tools.isEmpty())
+ && supportsNativeStructuredOutput(modelName)) {
+ builder.responseMimeType("application/json");
+ builder.responseJsonSchema(toNativeJsonSchema((Class<?>)
outputSchema));
+ }
+
return builder.build();
}
+ // Derives the JSON Schema Gemini's responseJsonSchema field expects from
a POJO class. Gemini
+ // supports a subset of JSON Schema and ignores the keywords outside that
subset server-side
+ // without reporting which, so every setting below is chosen against that
published subset:
+ //
+ // - DRAFT_2020_12 is the draft whose keywords Gemini's supported list
names: $defs and
+ // prefixItems are listed, while the older drafts' definitions and
tuple-form items are not.
+ // - The PLAIN_JSON preset keeps generation to fields. A preset is
mandatory, and the
+ // generator's default one, FULL_DOCUMENTATION, surfaces getters as
properties of their
+ // own, named after the accessor call, e.g. "getSummary()".
+ // - MAP_VALUES_AS_ADDITIONAL_PROPERTIES gives a Map its value schema,
as an
+ // additionalProperties keyword carrying the declared value type.
Dropped from this recipe,
+ // the map instead takes the additionalProperties:false of the option
below and admits no
+ // entries at all.
+ // - FORBIDDEN_ADDITIONAL_PROPERTIES_BY_DEFAULT closes every object. No
Gemini document states
+ // that a schema omitting the keyword is closed, and under ordinary
JSON Schema semantics it
+ // is not, so without it a response may carry an undeclared key that
the ObjectMapper
+ // read-back then rejects. If the service honors the keyword the
object is closed; if it
+ // does not, the keyword is ignored like any other unsupported one,
which is where omitting
+ // it would have left us. It applies to the enclosing object and
leaves a Map's declared
+ // value schema alone.
+ // - Sorting fields before methods and applying no further comparison
leaves properties in
+ // declaration order, which keeps the emitted document stable rather
than alphabetized. It
+ // is not an ordering guarantee: Gemini's ordering knob is the
non-standard propertyOrdering
+ // keyword, which this generator never emits.
+ // - The required check marks every field required except an Optional
one. Gemini treats a
+ // field the schema does not list as required as optional and lets the
model skip it, while
+ // marking everything required would force the fields a caller
declared omissible.
+ // - The Jackson module makes the schema name properties the way Jackson
names them. The
+ // response is read back into the same class with an ObjectMapper, so
a property that
+ // @JsonProperty renames or @JsonIgnore drops has to be stated in the
schema under the name
+ // the mapper reads, or a response that satisfies the schema still
fails to deserialize. It
+ // is applied with no JacksonOption, so it contributes property
naming, visibility and
+ // descriptions only: the required set and the property order stay the
ones configured
+ // above.
+ //
+ // DEFINITION_FOR_MAIN_SCHEMA is deliberately absent. Without it a
recursive type emits
+ // {"$ref": "#"} at the recursion point, which is the form Google's own
recursion example uses.
+ // Enabling it instead produces a $defs entry referencing another $defs
entry, a shape no
+ // published Gemini example demonstrates.
+ private static ObjectNode toNativeJsonSchema(Class<?> schemaClass) {
+ SchemaGeneratorConfigBuilder configBuilder =
+ new SchemaGeneratorConfigBuilder(
+ SchemaVersion.DRAFT_2020_12,
OptionPreset.PLAIN_JSON)
+ .with(Option.MAP_VALUES_AS_ADDITIONAL_PROPERTIES)
+
.with(Option.FORBIDDEN_ADDITIONAL_PROPERTIES_BY_DEFAULT)
+ .with(new JacksonModule());
Review Comment:
Could we align the generated schema with Jackson enum wire values here as
well? A bare `JacksonModule` emits Java enum names rather than `@JsonProperty`
or `@JsonValue` values—for example, `IN_PROGRESS` instead of `in-progress`.
`ChatModelAction` then deserializes the response with a regular `ObjectMapper`,
so a response that satisfies the Gemini schema can still fail locally. This is
the same issue discussed in
[#1097](https://github.com/apache/flink-agents/pull/1097#discussion_r3978366246).
Enabling `JacksonOption.FLATTENED_ENUMS_FROM_JSONPROPERTY` and
`JacksonOption.FLATTENED_ENUMS_FROM_JSONVALUE`, with a round-trip test, should
keep both sides aligned.
--
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]
