luigidemasi commented on code in PR #27494:
URL: https://github.com/apache/camel/pull/27494#discussion_r4208620217
##########
components/camel-ai/camel-semantic/src/main/java/org/apache/camel/semantic/yaml/SemanticDefinitionDeserializer.java:
##########
@@ -69,127 +79,150 @@ public void preParse(YamlDeserializationContext dc, Node
root) {
if (!(root instanceof SequenceNode sequence)) {
return;
}
- Map<String, SemanticQuestion> definitions = new LinkedHashMap<>();
+ Map<String, SemanticEvaluation> definitions = new LinkedHashMap<>();
for (Node node : sequence.getValue()) {
if (!(node instanceof MappingNode mapping)) {
- // Leave malformed entries to the route loader without
replacing the resource's questions.
+ // Leave malformed entries to the route loader without
replacing the resource's evaluations.
return;
}
for (NodeTuple tuple : mapping.getValue()) {
if ("semantic".equals(asText(tuple.getKeyNode()))) {
- read(dc.getCamelContext(),
tuple.getValueNode()).forEach((name, question) -> {
- if (definitions.putIfAbsent(name, question) != null) {
+ read(dc.getCamelContext(),
tuple.getValueNode()).forEach((name, evaluation) -> {
+ if (definitions.putIfAbsent(name, evaluation) != null)
{
throw new YamlDeserializationException(
- tuple.getValueNode(), "Duplicate semantic
question: " + name);
+ tuple.getValueNode(), "Duplicate semantic
evaluation: " + name);
}
});
}
}
}
CamelContext context = dc.getCamelContext();
- SemanticQuestions questions = definitions.isEmpty()
- ?
context.getCamelContextExtension().getContextPlugin(SemanticQuestions.class)
- : SemanticQuestions.get(context);
- if (questions != null) {
+ SemanticEvaluations evaluations = definitions.isEmpty()
+ ?
context.getCamelContextExtension().getContextPlugin(SemanticEvaluations.class)
+ : SemanticEvaluations.get(context);
+ if (evaluations != null) {
try {
- questions.replace(dc.getResource(), definitions);
+ evaluations.replace(dc.getResource(), definitions);
} catch (IllegalArgumentException e) {
throw new YamlDeserializationException(root, e.getMessage(),
e);
}
}
}
- private static Map<String, SemanticQuestion> read(CamelContext context,
Node node) {
+ private static Map<String, SemanticEvaluation> read(CamelContext context,
Node node) {
Map<String, Node> semantic = fields(node, "semantic declaration");
- if (!semantic.keySet().equals(Set.of("question"))) {
- throw new YamlDeserializationException(node, "Semantic declaration
requires only question");
+ for (String field : semantic.keySet()) {
+ if (!Set.of("evaluation", "expert", "state").contains(field)) {
+ throw new YamlDeserializationException(
+ semantic.get(field), "Unknown property '" + field + "'
in semantic declaration");
+ }
}
- Map<String, SemanticQuestion> result = new LinkedHashMap<>();
- fields(semantic.get("question"), "semantic questions").forEach((name,
definition) -> {
+ if (!semantic.containsKey("evaluation")) {
+ throw new YamlDeserializationException(node, "Semantic declaration
requires evaluation");
+ }
+ Map<String, SemanticEvaluation> result = new LinkedHashMap<>();
+ fields(semantic.get("evaluation"), "semantic
evaluations").forEach((name, definition) -> {
if (name.isBlank()) {
- throw new YamlDeserializationException(definition, "Semantic
question requires a nonblank name");
+ throw new YamlDeserializationException(definition, "Semantic
evaluation requires a nonblank name");
+ }
+ Map<String, Node> values = fields(definition, "semantic evaluation
'" + name + "'");
+ for (String common : List.of("expert", "state")) {
+ if (!values.containsKey(common) &&
semantic.containsKey(common)) {
+ values.put(common, semantic.get(common));
+ }
}
- Map<String, Node> values = fields(definition, "semantic question
'" + name + "'");
- String expert = values.containsKey("expert") ?
asText(values.get("expert")) : "default/automatic";
+ String expert = "default/automatic";
try {
- result.put(name, readQuestion(context, name, expert,
definition, values));
+ if (values.containsKey("expert")) {
+ expert = "invalid expert reference";
+ expert = asText(values.get("expert"));
+ }
+ result.put(name, readEvaluation(context, name, expert,
definition, values));
} catch (IllegalArgumentException | InvalidNodeTypeException e) {
throw new YamlDeserializationException(
- definition, "Invalid semantic question '" + name + "':
" + e.getMessage()
+ definition, "Invalid semantic evaluation '" + name +
"': " + e.getMessage()
+ " (expert '" + expert + "')",
e);
}
});
return result;
}
- private static SemanticQuestion readQuestion(
+ private static SemanticEvaluation readEvaluation(
CamelContext context, String name, String expert, Node definition,
Map<String, Node> values) {
- String description = "semantic question '" + name + "'";
+ String description = "semantic evaluation '" + name + "'";
String expertContext = " (expert '" + expert + "')";
values.forEach((field, value) -> {
if (!FIELDS.contains(field)) {
throw new YamlDeserializationException(
value, "Unknown property '" + field + "' in " +
description + expertContext);
}
});
- if (!values.containsKey("type")) {
- throw new YamlDeserializationException(definition, "Semantic
question type is required: " + name + expertContext);
+ if (values.containsKey("type") == values.containsKey("operation")) {
+ throw new YamlDeserializationException(
+ definition, "Specify exactly one operation or type: " +
name + expertContext);
}
- SemanticQuestion.Type type = enumeration(values.get("type"), name,
expert, "type", SemanticQuestion.Type.class);
- Map<String, String> criteria = new LinkedHashMap<>();
- List<String> levels = List.of();
- if (values.containsKey("criteria")) {
- if (type == SemanticQuestion.Type.SCORE) {
- levels =
asSequenceNode(values.get("criteria")).getValue().stream().map(YamlDeserializerSupport::asText)
- .toList();
- } else {
- fields(values.get("criteria"), "criteria for " + description +
expertContext)
- .forEach((key, value) -> criteria.put(key,
asText(value)));
- }
+ String operation = values.containsKey("operation")
+ ? asText(values.get("operation"))
+ : asText(values.get("type")).toLowerCase(Locale.ROOT);
+ SemanticEvaluationBuilder builder = new
SemanticEvaluationBuilder().operation(operation)
+
.expert(asText(values.get("expert"))).state(asText(values.get("state")));
+ if (values.containsKey("parameters")) {
+ fields(values.get("parameters"), "evaluation parameters")
+ .forEach((key, value) -> builder.parameter(key,
value(context, value)));
}
- if (type != SemanticQuestion.Type.BOOLEAN &&
(values.containsKey("threshold") || values.containsKey("uncertainty")
- || values.containsKey("uncertaintyPolicy"))) {
- throw new YamlDeserializationException(
- definition, "Threshold and uncertainty policy require a
boolean question: " + name + expertContext);
+ for (String field : List.of("instructions", "criteria", "threshold",
"uncertainty", "uncertaintyPolicy")) {
+ Node node = values.get(field);
+ if (node == null) {
+ continue;
+ }
+ switch (field) {
+ case "instructions" -> builder.instructions(asText(node));
+ case "threshold", "uncertainty" -> {
+ try {
+ builder.parameter(field,
Double.valueOf(context.resolvePropertyPlaceholders(asText(node))));
+ } catch (NumberFormatException invalid) {
+ throw new YamlDeserializationException(
+ node,
+ "Invalid numeric value for '" + field + "' in
" + description + expertContext);
+ }
+ }
+ case "uncertaintyPolicy" ->
builder.uncertaintyPolicy(asText(node));
+ case "criteria" -> {
+ if (node instanceof MappingNode) {
+ Map<String, String> criteria = new LinkedHashMap<>();
+ fields(node, "criteria").forEach((key, child) ->
criteria.put(key, asText(child)));
+ builder.parameter(field, criteria);
+ } else if (node instanceof SequenceNode sequence) {
+ builder.parameter(field,
+
sequence.getValue().stream().map(SemanticDefinitionDeserializer::asText).toList());
+ } else {
+ builder.parameter(field, value(context, node));
+ }
+ }
+ default -> throw new IllegalStateException(field);
+ }
}
- SemanticQuestion.UncertaintyPolicy policy =
values.containsKey("uncertaintyPolicy")
- ? enumeration(values.get("uncertaintyPolicy"), name, expert,
"uncertaintyPolicy",
- SemanticQuestion.UncertaintyPolicy.class)
- : SemanticQuestion.UncertaintyPolicy.FAIL;
- return new SemanticQuestion(
- type, asText(values.get("instructions")),
asText(values.get("state")),
- criteria, levels, number(context, values, name, expert,
"threshold", 0.5),
- number(context, values, name, expert, "uncertainty", 0),
policy, asText(values.get("expert")));
+ return builder.build(context);
}
- private static <T extends Enum<T>> T enumeration(Node node, String
question, String expert, String field, Class<T> type) {
- try {
- return asEnum(node, type);
- } catch (InvalidEnumException e) {
- throw new YamlDeserializationException(
- node, "Invalid value for '" + field + "' in semantic
question '" + question + "': " + asText(node)
- + " (expert '" + expert + "')",
- e);
+ private static Object value(CamelContext context, Node node) {
+ if (node instanceof MappingNode) {
+ Map<String, Object> result = new LinkedHashMap<>();
+ fields(node, "parameter map").forEach((key, child) ->
result.put(key, value(context, child)));
+ return result;
}
- }
-
- private static double number(
- CamelContext context, Map<String, Node> values, String question,
String expert, String name, double fallback) {
- if (!values.containsKey(name)) {
- return fallback;
+ if (node instanceof SequenceNode sequence) {
+ return sequence.getValue().stream().map(child -> value(context,
child)).toList();
}
- Node node = values.get(name);
- String raw = asText(node);
- try {
- return
Double.parseDouble(context.resolvePropertyPlaceholders(raw));
- } catch (NumberFormatException e) {
- throw new YamlDeserializationException(
- node,
- "Invalid numeric value for '" + name + "' in semantic
question '" + question + "': " + raw
- + " (expert '" + expert + "')",
- e);
+ if (Tag.FLOAT.equals(node.getTag()) || NUMBER.equals(node.getTag())) {
+ try {
+ return new
BigDecimal(context.resolvePropertyPlaceholders(asText(node)));
+ } catch (NumberFormatException invalid) {
+ throw new YamlDeserializationException(node, "Invalid numeric
parameter");
+ }
}
+ return new
StandardConstructor(LoadSettings.builder().build()).constructSingleDocument(Optional.of(node));
Review Comment:
The allocation is real, but a static `StandardConstructor` would introduce
shared mutable state. The pinned SnakeYAML Engine version is 3.1.1, using the
`v2` package namespace. Its `BaseConstructor` maintains `constructedObjects`,
`recursiveObjects`, `maps2fill` and `sets2fill`; clearing state between calls
permits sequential reuse, not concurrent use from different context/reload
threads.
I have kept construction local. This runs during declaration loading rather
than message evaluation. Reuse confined to a single parse would be a possible
optimization if parsing allocations become a measured concern, but the proposed
static singleton is not safe.
_AI-generated by Codex on behalf of @luigidemasi._
##########
components/camel-ai/camel-semantic/src/main/java/org/apache/camel/semantic/SemanticEvaluation.java:
##########
@@ -0,0 +1,104 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.camel.semantic;
+
+import java.math.BigDecimal;
+import java.math.BigInteger;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.LinkedHashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+
+/** Immutable evaluation declaration. The expert defines the operation,
parameter vocabulary and result semantics. */
+public final class SemanticEvaluation {
+ private static final Set<Class<?>> NUMBER_TYPES = Set.of(Byte.class,
Short.class, Integer.class, Long.class,
+ Float.class, Double.class, BigInteger.class, BigDecimal.class);
+
+ private final String operation;
+ private final String expert;
+ private final String state;
+ private final Map<String, Object> parameters;
+
+ public SemanticEvaluation(String operation, String expert, String state,
Map<String, ?> parameters) {
+ if (operation == null || operation.isBlank()) {
+ throw new IllegalArgumentException("Evaluation operation is
required");
+ }
+ if (expert != null && expert.isBlank()) {
+ throw new IllegalArgumentException("Evaluation expert must not be
blank");
+ }
+ if (state != null && state.isBlank()) {
+ throw new IllegalArgumentException("Evaluation state selector must
not be blank");
+ }
+ this.operation = operation;
+ this.expert = expert;
+ this.state = state;
+ this.parameters = immutableMap(parameters == null ? Map.of() :
parameters);
+ }
+
+ static Map<String, Object> immutableMap(Map<String, ?> values) {
+ Map<String, Object> copy = new LinkedHashMap<>();
+ values.forEach((name, value) -> {
+ if (name == null || name.isBlank()) {
+ throw new IllegalArgumentException("Parameter names must not
be blank");
+ }
+ copy.put(name, immutableValue(value));
+ });
+ return Collections.unmodifiableMap(copy);
+ }
+
+ private static Object immutableValue(Object value) {
+ if (value instanceof Map<?, ?> map) {
+ Map<String, Object> copy = new LinkedHashMap<>();
+ map.forEach((key, entry) -> {
+ if (!(key instanceof String name)) {
+ throw new IllegalArgumentException("Parameter maps require
string keys");
+ }
+ copy.put(name, entry);
+ });
+ return immutableMap(copy);
Review Comment:
Applied in
[d338ec439c24](https://github.com/apache/camel/commit/d338ec439c247a869f0f54b88e8dab71e06f9389).
Nested maps now validate their keys and recursively copy values in one pass,
then return an unmodifiable map. This removes the intermediate map and second
traversal while preserving the existing blank-key and string-key validation.
The tests also verify that nested maps and lists remain immutable and detached
from caller-owned containers.
_AI-generated by Codex on behalf of @luigidemasi._
--
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]
