weiqingy commented on code in PR #1097:
URL: https://github.com/apache/flink-agents/pull/1097#discussion_r4007009467
##########
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:
Good catch, thanks. Fixed in 8d763889 by enabling
`FLATTENED_ENUMS_FROM_JSONPROPERTY` and `FLATTENED_ENUMS_FROM_JSONVALUE`. The
new `testDerivedSchemaFollowsJacksonEnumValues` reads every listed value back
with a plain `ObjectMapper`, for one `@JsonProperty` enum and one `@JsonValue`
enum. #1098 has the same fix in d233bead, and I'll add it to #1117 too.
For the shared config, I'd lean toward a separate PR once these land. There
is no shared chat-model module yet, and the recipes differ a bit per provider
(map values, closed objects, `$ref` handling). Adding it here would also tie
#1098 and #1117 to this PR. The follow-up would move Ollama, Gemini, Bedrock
and watsonx onto one config, and fix the same enum gap in Ollama on main. Does
that plan work for you?
##########
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:
Thanks for the pointer. Added in 3f82ff90. While checking, I went through
all the Bedrock model cards and found 28 more models that document structured
outputs on `bedrock-runtime`, including Claude Sonnet 4.6. I added those too,
so the list has 41 ids now.
`capableModels()` covers the direct ids, and the prefix test now runs `us.`,
`eu.`, `apac.`, `au.`, `jp.` and `global.` against
`anthropic.claude-opus-4-6-v1`.
--
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]
