gnodet-bot commented on code in PR #28892:
URL: https://github.com/apache/camel/pull/28892#discussion_r4444230607
##########
components/camel-aws/camel-aws-bedrock/src/main/java/org/apache/camel/component/aws2/bedrock/runtime/BedrockProducer.java:
##########
@@ -499,14 +527,32 @@ private void setResponseTextByModelFamily(String modelId,
InvokeModelResponse re
}
}
+ /**
+ * The kind of model, by its vendor/family name, when the model does not
generate text, or null for a text model.
+ */
+ private static String nonTextModelKind(String modelId) {
+ if (modelId.startsWith("stability.") ||
modelId.contains("image-generator") || modelId.contains("-canvas")) {
+ return "an image generation model";
+ } else if (modelId.contains("embed")) {
+ return "an embedding model";
+ } else if (modelId.contains("rerank")) {
+ return "a rerank model";
+ } else if (modelId.contains("-reel") || modelId.contains("-sonic")) {
+ return "a video/speech generation model";
+ }
+ return null;
+ }
+
private void setTitanText(InvokeModelResponse result, Message message) {
message.setBody(result.body().asUtf8String());
}
private void setAi21Text(InvokeModelResponse result, Message message)
throws JsonProcessingException {
ObjectMapper mapper = new ObjectMapper();
JsonNode jsonString = mapper.readTree(result.body().asUtf8String());
Review Comment:
<!-- gnodet-bot-review -->
`setAi21Text` is being modified by this PR, but it still creates a fresh
`ObjectMapper` on every call:
```java
ObjectMapper mapper = new ObjectMapper();
JsonNode jsonString = mapper.readTree(result.body().asUtf8String());
```
The class already has a static `MAPPER` field at line 73. Please reuse it:
```suggestion
JsonNode jsonString = MAPPER.readTree(result.body().asUtf8String());
```
Note: the same redundant allocation exists in `setAnthropicText`,
`setMistralText`, `setCohereText`, and `setLlamaText`. Since this PR is already
touching this area, consider fixing all of them in one pass.
##########
components/camel-aws/camel-aws-bedrock/src/main/java/org/apache/camel/component/aws2/bedrock/runtime/BedrockProducer.java:
##########
@@ -826,17 +901,25 @@ private void converseStream(Exchange exchange) throws
InvalidPayloadException {
}
// Optional: Guardrail configuration (use
GuardrailStreamConfiguration for streaming)
- GuardrailStreamConfiguration guardrailConfig
- =
exchange.getMessage().getHeader(BedrockConstants.GUARDRAIL_CONFIG,
- GuardrailStreamConfiguration.class);
- if (ObjectHelper.isNotEmpty(guardrailConfig)) {
- builder.guardrailConfig(guardrailConfig);
+ Object guardrailConfig =
exchange.getMessage().getHeader(BedrockConstants.GUARDRAIL_CONFIG);
+ if (guardrailConfig instanceof GuardrailStreamConfiguration
streamConfig) {
+ builder.guardrailConfig(streamConfig);
+ } else if (guardrailConfig instanceof GuardrailConfiguration
config) {
+ // the GuardrailConfiguration of the converse operation, as
the docs name it
+ builder.guardrailConfig(GuardrailStreamConfiguration.builder()
+ .guardrailIdentifier(config.guardrailIdentifier())
+ .guardrailVersion(config.guardrailVersion())
+ .trace(config.traceAsString())
+ .build());
} else if
(ObjectHelper.isNotEmpty(getConfiguration().getGuardrailIdentifier())) {
Review Comment:
<!-- gnodet-bot-review -->
The `GuardrailConfiguration`→`GuardrailStreamConfiguration` auto-conversion
path omits `streamProcessingMode`:
```java
builder.guardrailConfig(GuardrailStreamConfiguration.builder()
.guardrailIdentifier(config.guardrailIdentifier())
.guardrailVersion(config.guardrailVersion())
.trace(config.traceAsString())
.build()); // ← streamProcessingMode not set
```
The endpoint-config path (the `else if` branch that follows) explicitly sets
it:
```java
.streamProcessingMode(getConfiguration().isGuardrailTrace()
? GuardrailStreamProcessingMode.ASYNC
: GuardrailStreamProcessingMode.SYNC)
```
The SDK will default `streamProcessingMode` to whatever its zero-value is
when converting from a header-supplied `GuardrailConfiguration`, which may
silently diverge from the user's intent. At minimum document why the omission
is intentional; otherwise add the equivalent logic here.
--
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]
