wenjin272 commented on code in PR #1097:
URL: https://github.com/apache/flink-agents/pull/1097#discussion_r4011393029
##########
integrations/chat-models/bedrock/src/main/java/org/apache/flink/agents/integrations/chatmodels/bedrock/BedrockChatModelConnection.java:
##########
@@ -88,6 +104,80 @@
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 =
+ Set.of(
+ "anthropic.claude-sonnet-4-5-20250929-v1:0",
+ "anthropic.claude-opus-4-5-20251101-v1:0",
+ "anthropic.claude-haiku-4-5-20251001-v1:0",
+ "anthropic.claude-opus-4-6-v1",
+ "anthropic.claude-sonnet-4-6",
+ "deepseek.v3-v1:0",
+ "deepseek.v3.2",
+ "google.gemma-3-12b-it",
+ "google.gemma-3-27b-it",
+ "minimax.minimax-m2",
+ "minimax.minimax-m2.1",
+ "minimax.minimax-m2.5",
+ "mistral.mistral-large-3-675b-instruct",
+ "mistral.devstral-2-123b",
+ "mistral.magistral-small-2509",
+ "mistral.ministral-3-14b-instruct",
+ "mistral.ministral-3-3b-instruct",
+ "mistral.ministral-3-8b-instruct",
+ "mistral.voxtral-mini-3b-2507",
+ "mistral.voxtral-small-24b-2507",
+ "moonshot.kimi-k2-thinking",
+ "moonshotai.kimi-k2.5",
+ "nvidia.nemotron-nano-12b-v2",
+ "nvidia.nemotron-nano-3-30b",
+ "nvidia.nemotron-nano-9b-v2",
+ "nvidia.nemotron-super-3-120b",
+ "openai.gpt-oss-120b-1:0",
+ "openai.gpt-oss-20b-1:0",
+ "openai.gpt-5.6-luna",
Review Comment:
AWS documents Structured outputs as unsupported for GPT-5.6 Luna on
`bedrock-runtime`. Could we remove it from the allowlist? Otherwise its `us.`
and `global.` inference-profile IDs also incorrectly take the native path.
Reference:
https://docs.aws.amazon.com/bedrock/latest/userguide/model-card-openai-gpt-56-luna.html
##########
integrations/chat-models/bedrock/src/main/java/org/apache/flink/agents/integrations/chatmodels/bedrock/BedrockChatModelConnection.java:
##########
@@ -173,19 +356,78 @@ 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, and
+ // list enum constants mapped by @JsonProperty or by a @JsonValue
method under the values
+ // Jackson reads. The response is read back into the same class with
an ObjectMapper, so a
+ // property that @JsonProperty renames or @JsonIgnore drops, and a
mapped enum constant,
+ // have to appear in the schema as the mapper reads them, or a
response that satisfies the
+ // schema still fails to deserialize. An enum annotating only some
constants falls back to
+ // Java names for all of them, so its annotated constants do not read
back. The two enum
Review Comment:
One user-visible limitation remains: for a partially annotated enum,
victools falls back to Java names for the whole enum, so the generated schema
may differ from the values Jackson accepts. Since the existing
structured-output integrations do not yet have user-facing documentation
either, this does not need to block this PR. Could we document it consistently
once the structured-output work across providers is complete, noting that
`@JsonProperty` should be applied to every enum constant?
--
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]
