gnodet commented on code in PR #27656:
URL: https://github.com/apache/camel/pull/27656#discussion_r4237569362
##########
components/camel-ai/camel-semantic/src/main/java/org/apache/camel/semantic/internal/SemanticAuditService.java:
##########
@@ -404,7 +471,8 @@ public void complete(String status, String reason,
SemanticResult result) {
}
}
if (enabled) {
- publish(record);
+ fields.putAll(inputFields);
+ publish(inputFields.isEmpty() ? record :
SemanticAuditRecord.fromMap(fields));
Review Comment:
💡 **Minor redundancy:** `SemanticAuditRecord.fromMap(fields)` on the publish
path re-validates all fields and re-runs
`SemanticAuditInputSnapshot.copy(input, MAX_CHARS)` on input that was already
snapshot-copied in `captureInput()` with the per-expert `maxChars`. Both paths
are idempotent and safe, but the double copy is wasteful. Consider a
package-private constructor or factory that accepts a pre-built,
already-validated map, or add a comment explaining the intentional defensive
re-copy.
##########
dsl/camel-jbang/camel-jbang-core/src/main/java/org/apache/camel/dsl/jbang/core/commands/action/CamelSemantic.java:
##########
@@ -94,9 +95,11 @@ protected void render(JsonObject response) {
new
Column().header("REFERENCE").dataAlign(HorizontalAlign.LEFT).with(r -> text(r,
"reference")),
new
Column().header("NAME").dataAlign(HorizontalAlign.LEFT).with(r -> text(r,
"name")),
new
Column().header("ERROR").dataAlign(HorizontalAlign.LEFT).with(r -> text(r,
"error")))));
- printer().println("Default expert: " + text(response,
"defaultExpert"));
- if (response.get("defaultError") != null) {
- printer().println("Default expert error: " +
response.get("defaultError"));
+ String defaultExpert = text(response, "defaultExpert");
+ printer().println("Default expert: " + (defaultExpert.isEmpty() ?
"none" : defaultExpert));
+ String defaultError = text(response, "defaultError");
+ if (!defaultError.isEmpty() && !defaultError.startsWith("Semantic
language requires exactly one eligible expert")) {
Review Comment:
💡 **Fragile string match:** `defaultError.startsWith("Semantic language
requires exactly one eligible expert")` is a hardcoded prefix match on a
server-side error message. If the message wording changes, this suppression
silently breaks (the error appears when there is exactly one expert, which is
arguably the normal case). Consider matching on a structured error code
returned by the producer, or extract this prefix to a shared 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]
