wenjin272 commented on code in PR #1097:
URL: https://github.com/apache/flink-agents/pull/1097#discussion_r3978366259
##########
integrations/chat-models/bedrock/src/main/java/org/apache/flink/agents/integrations/chatmodels/bedrock/BedrockChatModelConnection.java:
##########
@@ -88,6 +103,51 @@
public class BedrockChatModelConnection extends BaseChatModelConnection {
private static final ObjectMapper MAPPER = new ObjectMapper();
+
+ // Models AWS documents structured-output support for on the
bedrock-runtime endpoint. There is
+ // no single list page: the feature page delegates the per-model answer to
the individual model
+ // cards, where each card carries it as a "Structured outputs" bullet in
the Supported or Not
+ // Supported column of its "Features supported using bedrock-runtime
endpoint" table.
+ //
+ // The ids are the Model ID column of each card's Programmatic Access
table, read from the
+ // bedrock-runtime row. A card commonly prints a different id for
bedrock-mantle and can carry
+ // opposite verdicts for the two, so the endpoint an id was read from is
part of what makes the
+ // entry correct. This connection calls Converse on bedrock-runtime.
+ //
+ // Matching is exact, never by prefix. A Bedrock id already pins the
vendor, the snapshot date
+ // and the version in one string, so there is no alias for a prefix to
cover, and a prefix would
+ // over-capture: "qwen.qwen3" admits qwen.qwen3-vl-235b-a22b, which AWS
documents as not
+ // supported, and "anthropic.claude-sonnet-4" admits
anthropic.claude-sonnet-4-20250514-v1:0,
+ // whose card carries no answer at all. Exact matching also keeps
irregular id shapes correct
+ // with no normalisation rule: mistral.mistral-large-3-675b-instruct
carries no version suffix,
+ // openai.gpt-oss-120b-1:0 carries "-1:0" rather than "-v1:0".
+ //
+ // A card whose capability table carries the bullet in neither column is
undocumented rather
+ // than negative, and is absent from this set for that reason.
+ private static final Set<String> NATIVE_STRUCTURED_OUTPUT_MODELS =
Review Comment:
AWS now documents Structured outputs support for Claude Opus 4.6 and lists
`anthropic.claude-opus-4-6-v1` as its Bedrock model ID, but it is absent from
this allowlist. Consequently, both the base ID and its `us.`, `eu.`, `au.`, and
`global.` inference-profile forms are classified as incapable and fall back to
prompting. Could we add the base ID and cover the direct and prefixed forms in
the capability tests? Reference: [AWS Claude Opus 4.6 model
card](https://docs.aws.amazon.com/en_en/bedrock/latest/userguide/model-card-anthropic-claude-opus-4-6.html).
##########
integrations/chat-models/bedrock/src/main/java/org/apache/flink/agents/integrations/chatmodels/bedrock/BedrockChatModelConnection.java:
##########
@@ -173,19 +326,73 @@ public ChatMessage chat(
}
}
- ConverseRequest request = requestBuilder.build();
+ if (outputSchema instanceof Class &&
supportsNativeStructuredOutput(modelId)) {
+ requestBuilder.outputConfig(nativeOutputConfig((Class<?>)
outputSchema));
+ }
- ConverseResponse response =
- retryExecutor.execute(() -> client.converse(request),
"BedrockConverse");
+ return requestBuilder.build();
+ }
- ChatMessage result = convertResponse(response);
- if (response.usage() != null) {
- result.getExtraArgs().put("model_name", modelId);
- result.getExtraArgs().put("promptTokens",
response.usage().inputTokens().longValue());
- result.getExtraArgs()
- .put("completionTokens",
response.usage().outputTokens().longValue());
- }
- return result;
+ /**
+ * Wraps the schema derived from {@code schemaClass} in the request
element Converse reads it
+ * from.
+ *
+ * <p>Converse takes the schema as serialized text rather than as a
document, unlike the tool
+ * input schema on the same request, so the derived schema is written out
here.
+ */
+ private static OutputConfig nativeOutputConfig(Class<?> schemaClass) {
+ return OutputConfig.builder()
+ .textFormat(
+ OutputFormat.builder()
+ .type(OutputFormatType.JSON_SCHEMA)
+ .structure(
+ OutputFormatStructure.builder()
+ .jsonSchema(
+
JsonSchemaDefinition.builder()
+ .schema(
+
toNativeSchema(schemaClass)
+
.toString())
+ .build())
+ .build())
+ .build())
+ .build();
+ }
+
+ // Derives the JSON schema from a POJO class. Every setting below
addresses a concrete way the
+ // generated schema otherwise fails to constrain generation:
+ //
+ // - DRAFT_2020_12 is the dialect Bedrock validates a schema against, so
the schema
+ // declares it rather than the generator's older default.
+ // - The PLAIN_JSON preset keeps generation to fields. Without a preset,
getters surface as
+ // properties of their own, named after the accessor call, e.g.
"getSummary()".
+ // - The required check marks every field required except an Optional
one. The default marks
+ // nothing required, which lets a model omit fields at will, 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 and visibility
+ // only: the required set stays the one configured above.
+ //
+ // A Map's value schema is deliberately left underived. Bedrock accepts
additionalProperties
+ // only as false, and rejects a schema that carries it as a subschema, so
typing map values
+ // would trade an unconstrained map for a rejected request. A Map field
reaches the model as a
+ // bare object.
+ //
+ // A self-referencing class derives its own field as a reference back to
the schema root,
+ // whatever the required check says. Bedrock does not accept a recursive
schema and rejects the
+ // request before the model runs, so declaring the field Optional does not
rescue it; only
+ // flattening the recursion does.
+ private static JsonNode toNativeSchema(Class<?> schemaClass) {
+ SchemaGeneratorConfigBuilder configBuilder =
+ new SchemaGeneratorConfigBuilder(
+ SchemaVersion.DRAFT_2020_12,
OptionPreset.PLAIN_JSON)
+ .with(new JacksonModule());
Review Comment:
Could we align the generated schema with Jackson's enum wire values? The
bare `JacksonModule` emits Java enum names instead of their `@JsonProperty` or
`@JsonValue` values—for example, `IN_PROGRESS` instead of `in-progress`—so a
schema-valid response can fail during subsequent `ObjectMapper`
deserialization. Ollama and #1098 use similar schema-generation logic, so this
may also be a good opportunity to extract the shared configuration into a
`chat-models/common` component. Would you prefer to introduce that abstraction
here or handle the cross-provider migration separately?
--
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]
