luigidemasi commented on code in PR #27494:
URL: https://github.com/apache/camel/pull/27494#discussion_r4217010355
##########
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:
Resolving with the explanation above: the pinned SnakeYAML Engine 3.1.1
constructor owns mutable construction maps, sets and deferred-fill lists, so a
shared static instance is unsafe across concurrent loaders. Construction
remains local to declaration parsing; it is not on the message-evaluation path.
The suggested singleton is intentionally not applied.
_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:
Confirmed fixed in
[d338ec439c24](https://github.com/apache/camel/commit/d338ec439c247a869f0f54b88e8dab71e06f9389).
Nested maps validate keys and recursively copy values in one traversal, then
return an unmodifiable map. The intermediate map and second traversal are gone,
while immutability and invalid-key regression coverage remain.
_AI-generated by Codex on behalf of @luigidemasi._
##########
components/camel-ai/camel-semantic/src/main/java/org/apache/camel/semantic/SemanticEvaluationBuilder.java:
##########
@@ -0,0 +1,184 @@
+/*
+ * 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.util.ArrayList;
+import java.util.LinkedHashMap;
+import java.util.List;
+import java.util.Locale;
+import java.util.Map;
+
+import org.apache.camel.CamelContext;
+import org.apache.camel.util.StringHelper;
+
+/** Fluent definition of an expert-owned evaluation. All DSLs build the same
immutable declaration. */
+public final class SemanticEvaluationBuilder {
+ private final SemanticEvaluationsBuilder parent;
+ private final Map<String, Object> parameters = new LinkedHashMap<>();
+ private final Map<String, String> criteria = new LinkedHashMap<>();
+ private final List<String> levels = new ArrayList<>();
+ private String threshold;
+ private String uncertainty;
+ private boolean normalizeUncertaintyPolicy;
+ private String operation;
+ private String expert;
+ private String state;
+
+ /** Create a standalone declaration, completed with {@link
#build(CamelContext)}. */
+ public SemanticEvaluationBuilder() {
+ this(null);
+ }
+
+ SemanticEvaluationBuilder(SemanticEvaluationsBuilder parent) {
+ this.parent = parent;
+ }
+
+ public SemanticEvaluationBuilder operation(String operation) {
+ this.operation = operation;
+ return this;
+ }
+
+ /** Select an instruction-driven operation by type, such as boolean,
choice or score. */
+ public SemanticEvaluationBuilder type(String type) {
+ return operation(type.toLowerCase(Locale.ROOT));
+ }
+
+ public SemanticEvaluationBuilder expert(String expert) {
+ this.expert = expert;
+ return this;
+ }
+
+ String getExpert() {
+ return expert != null ? expert : parent != null ? parent.getExpert() :
null;
+ }
+
+ public SemanticEvaluationBuilder state(String state) {
+ this.state = state;
+ return this;
+ }
+
+ public SemanticEvaluationBuilder parameter(String name, Object value) {
+ if (parameters.containsKey(name)) {
+ throw new IllegalArgumentException("Duplicate semantic parameter:
" + name);
+ }
+ parameters.put(name, value);
+ return this;
+ }
+
+ public SemanticEvaluationBuilder parameters(Map<String, ?> values) {
+ values.forEach(this::parameter);
+ return this;
+ }
+
+ public SemanticEvaluationBuilder instructions(String instructions) {
+ return parameter("instructions", instructions);
+ }
+
+ public SemanticEvaluationBuilder criterion(String name, String
description) {
+ if (criteria.containsKey(name)) {
+ throw new IllegalArgumentException("Duplicate semantic criterion:
" + name);
+ }
+ criteria.put(name, description);
+ return this;
+ }
+
+ public SemanticEvaluationBuilder level(String level) {
+ levels.add(level);
+ return this;
+ }
+
+ public SemanticEvaluationBuilder threshold(double threshold) {
+ return parameter("threshold", threshold);
+ }
+
+ public SemanticEvaluationBuilder threshold(String threshold) {
+ this.threshold = threshold;
+ return this;
+ }
+
+ public SemanticEvaluationBuilder uncertainty(double uncertainty) {
+ return parameter("uncertainty", uncertainty);
+ }
+
+ public SemanticEvaluationBuilder uncertainty(String uncertainty) {
+ this.uncertainty = uncertainty;
+ return this;
+ }
+
+ public SemanticEvaluationBuilder uncertaintyPolicy(String policy) {
+ parameter("uncertaintyPolicy", policy);
+ normalizeUncertaintyPolicy = true;
+ return this;
+ }
+
+ public SemanticEvaluationsBuilder end() {
+ if (parent == null) {
+ throw new IllegalStateException("Complete a standalone declaration
with build(context)");
+ }
+ return parent;
+ }
+
+ public void register() {
+ end().register();
+ }
+
+ /** Build an immutable declaration, resolving placeholders in parameter
values while retaining their types. */
+ public SemanticEvaluation build(CamelContext context) {
+ Map<String, Object> values = new LinkedHashMap<>(parameters);
+ if (!criteria.isEmpty() || !levels.isEmpty()) {
+ if (values.containsKey("criteria") || !criteria.isEmpty() &&
!levels.isEmpty()) {
+ throw new IllegalArgumentException("Duplicate parameter
'criteria'");
+ }
+ values.put("criteria", !criteria.isEmpty() ? criteria : levels);
+ }
+ for (String numeric : List.of("threshold", "uncertainty")) {
+ String text = numeric.equals("threshold") ? threshold :
uncertainty;
+ if (text != null) {
+ if (values.containsKey(numeric)) {
+ throw new IllegalArgumentException("Duplicate parameter '"
+ numeric + "'");
+ }
+ try {
+ values.put(numeric,
Double.valueOf(context.resolvePropertyPlaceholders(text)));
+ } catch (NumberFormatException invalid) {
+ throw new IllegalArgumentException("Parameter '" + numeric
+ "' must be a valid number");
+ }
+ }
+ }
+ values.replaceAll((name, value) -> resolve(context, value));
+ if (normalizeUncertaintyPolicy && values.get("uncertaintyPolicy")
instanceof String policy) {
+ values.put("uncertaintyPolicy",
+
StringHelper.asEnumConstantValue(policy).toLowerCase(Locale.ROOT).replace('_',
'-'));
+ }
+ return new SemanticEvaluation(
+ operation, getExpert(), state != null ? state : parent != null
? parent.getState() : null, values);
+ }
+
+ private static Object resolve(CamelContext context, Object value) {
+ if (value instanceof String text) {
+ return context.resolvePropertyPlaceholders(text);
+ }
+ if (value instanceof Map<?, ?> map) {
+ Map<Object, Object> resolved = new LinkedHashMap<>();
Review Comment:
Resolving with the existing checked-validation path. `resolve()` also
accepts arbitrary Java builder maps; casting their keys to `String` here can
throw `ClassCastException`. The intermediate `Map<Object, Object>` lets the
common immutable declaration reject invalid keys with its intended
`IllegalArgumentException` and evaluation context.
`nonStringNestedKeysFailWithValidationError()` covers this case.
_AI-generated by Codex on behalf of @luigidemasi._
##########
components/camel-ai/camel-semantic/src/test/java/org/apache/camel/semantic/SemanticInitializationTest.java:
##########
@@ -77,6 +76,38 @@ void concurrentQuestionRegistryCreationReturnsOneInstance()
throws Exception {
}
}
+ @Test
+ void cachedRegistryLookupDoesNotWaitForAnotherContext() throws Exception {
+ ExecutorService callers = Executors.newFixedThreadPool(2);
+ CountDownLatch entered = new CountDownLatch(1);
+ CountDownLatch release = new CountDownLatch(1);
+ try (var creating = new DefaultCamelContext(); var cached = new
DefaultCamelContext()) {
+ var expected = SemanticEvaluations.get(cached);
+
creating.getCamelContextExtension().lazyAddContextPlugin(SemanticEvaluations.class,
() -> {
+ entered.countDown();
+ try {
+ assertThat(release.await(30, TimeUnit.SECONDS)).isTrue();
+ } catch (InterruptedException e) {
+ Thread.currentThread().interrupt();
+ throw new IllegalStateException(e);
+ }
+ return null;
Review Comment:
The test deliberately covers cached lookup independence, while simultaneous
first creation still uses the private lock; it does not claim that first
creation is lock-free. Concurrent creation for one context has its own test.
The null-supplier intent is documented in
[384c0e7f7de1](https://github.com/apache/camel/commit/384c0e7f7de14a473b201a4ef05f6d8ee0a0423d),
and
[96e6d465f016](https://github.com/apache/camel/commit/96e6d465f016856fb43c7e95b8e67818cda8112d)
adds a descriptive assertion: after the gated lookup leaves the plugin absent,
`get()` creates the registry. Resolving with this clarified scope.
_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]
