gnodet commented on code in PR #25273:
URL: https://github.com/apache/camel/pull/25273#discussion_r3690224510
##########
components/camel-ai/camel-langchain4j-embeddingstore/src/main/java/org/apache/camel/component/langchain4j/embeddingstore/LangChain4jEmbeddingStoreProducer.java:
##########
@@ -109,63 +110,145 @@ public void process(Exchange exchange) throws Exception {
}
/**
- * Adds an embedding to the store with optional text segment.
+ * Adds embeddings to the store with optional text segments and
caller-supplied IDs.
*
* <p>
- * Expects the following headers:
+ * Supports both single and batch operations:
+ * </p>
+ *
+ * <p>
+ * <b>Single operation</b> - when the {@code
CamelLangChain4jEmbeddingsEmbedding} header contains a single
+ * {@link Embedding}:
* </p>
* <ul>
- * <li>{@code CamelLangchain4jEmbeddingEmbedding} - The embedding vector
(required)</li>
- * <li>{@code CamelLangchain4jEmbeddingTextSegment} - Associated text
segment (optional)</li>
+ * <li>With caller-supplied ID header ({@code
CamelLangchain4jEmbeddingStoreEmbeddingId}): calls
+ * {@code add(id, embedding)}</li>
+ * <li>With text segment header: calls {@code add(embedding,
textSegment)}</li>
+ * <li>Without text segment: calls {@code add(embedding)}</li>
* </ul>
*
* <p>
- * Returns the generated embedding ID in the message body.
+ * <b>Batch operation</b> - when the {@code
CamelLangChain4jEmbeddingsEmbeddings} header contains a
+ * {@code List<Embedding>}:
* </p>
+ * <ul>
+ * <li>With IDs header ({@code
CamelLangchain4jEmbeddingStoreEmbeddingIds}) and text segments body: calls
+ * {@code addAll(ids, embeddings, textSegments)}</li>
+ * <li>With text segments body: calls {@code addAll(embeddings,
textSegments)}</li>
+ * <li>Without text segments: calls {@code addAll(embeddings)}</li>
+ * </ul>
*
* @param exchange the Camel exchange containing the embedding data
* @throws Exception if the add operation fails
*/
+ @SuppressWarnings("unchecked")
private void add(Exchange exchange) throws Exception {
final Message in = exchange.getMessage();
+ EmbeddingStore<TextSegment> store =
getEndpoint().getConfiguration().getEmbeddingStore();
+
+ // Check for batch embeddings header first
+ List<Embedding> embeddings =
in.getHeader(LangChain4jEmbeddingsHeaders.EMBEDDINGS, List.class);
+ if (embeddings != null) {
+ addBatch(in, store, embeddings);
+ return;
+ }
+ // Single embedding path
if (in.getHeader(LangChain4jEmbeddingsHeaders.EMBEDDING) == null) {
throw new NoSuchHeaderException(
"The embedding is a required header for ADD operations",
exchange,
LangChain4jEmbeddingsHeaders.EMBEDDING);
}
Embedding embedding =
in.getHeader(LangChain4jEmbeddingsHeaders.EMBEDDING, Embedding.class);
+
+ // Check for caller-supplied ID
+ String callerId =
in.getHeader(LangChain4jEmbeddingStoreHeaders.EMBEDDING_ID, String.class);
String id;
- if (in.getHeader(LangChain4jEmbeddingsHeaders.TEXT_SEGMENT) != null) {
+ if (callerId != null) {
+ store.add(callerId, embedding);
+ id = callerId;
+ } else if (in.getHeader(LangChain4jEmbeddingsHeaders.TEXT_SEGMENT) !=
null) {
TextSegment text =
in.getHeader(LangChain4jEmbeddingsHeaders.TEXT_SEGMENT, TextSegment.class);
- id =
getEndpoint().getConfiguration().getEmbeddingStore().add(embedding, text);
+ id = store.add(embedding, text);
} else {
- id =
getEndpoint().getConfiguration().getEmbeddingStore().add(embedding);
+ id = store.add(embedding);
}
- Message out = exchange.getMessage();
- out.setBody(id);
+ in.setBody(id);
+ }
+
+ @SuppressWarnings("unchecked")
+ private void addBatch(Message in, EmbeddingStore<TextSegment> store,
List<Embedding> embeddings) {
+ List<String> callerIds =
in.getHeader(LangChain4jEmbeddingStoreHeaders.EMBEDDING_IDS, List.class);
+ Object body = in.getBody();
+ List<TextSegment> textSegments = null;
+
+ if (body instanceof List && !((List<?>) body).isEmpty() && ((List<?>)
body).get(0) instanceof TextSegment) {
+ textSegments = (List<TextSegment>) body;
+ }
+
+ List<String> ids;
+ if (callerIds != null && textSegments != null) {
+ store.addAll(callerIds, embeddings, textSegments);
+ ids = callerIds;
+ } else if (textSegments != null) {
+ ids = store.addAll(embeddings, textSegments);
+ } else {
+ ids = store.addAll(embeddings);
+ }
+
+ in.setBody(ids);
}
/**
- * Removes an embedding from the store by its ID.
+ * Removes embeddings from the store. Supports multiple removal strategies:
*
- * <p>
- * Expects the embedding ID as the message body (String).
- * </p>
+ * <ul>
+ * <li><b>By filter</b>: when the {@code
CamelLangchain4jEmbeddingStoreFilter} header is set, removes all embeddings
+ * matching the filter via {@code removeAll(Filter)}</li>
+ * <li><b>By ID list</b>: when the body is a {@code Collection<String>},
removes all specified embeddings via
+ * {@code removeAll(Collection)}</li>
+ * <li><b>By single ID</b>: when the body is a single {@code String},
removes that embedding via
+ * {@code remove(id)}</li>
+ * <li><b>Clear all</b>: when the body is null/empty and no filter is set,
removes all embeddings from the store via
Review Comment:
**[HIGH]** When the body is null/empty and no filter is set, this falls
through to `store.removeAll()`, silently clearing the entire embedding store.
Previously, `remove(null)` would call `ensureNotBlank(id, "id")` in
langchain4j's default `EmbeddingStore.remove()`, which threw an
`IllegalArgumentException` — a safe failure mode.
A destructive "clear all" operation should require explicit intent, not be
the default fallback when nothing else matches. Consider requiring a dedicated
action (e.g., `CLEAR`) or a confirmation header like
`CamelLangchain4jEmbeddingStoreClearAll=true`.
##########
components/camel-ai/camel-langchain4j-embeddingstore/src/main/java/org/apache/camel/component/langchain4j/embeddingstore/LangChain4jEmbeddingStoreProducer.java:
##########
@@ -109,63 +110,145 @@ public void process(Exchange exchange) throws Exception {
}
/**
- * Adds an embedding to the store with optional text segment.
+ * Adds embeddings to the store with optional text segments and
caller-supplied IDs.
*
* <p>
- * Expects the following headers:
+ * Supports both single and batch operations:
+ * </p>
+ *
+ * <p>
+ * <b>Single operation</b> - when the {@code
CamelLangChain4jEmbeddingsEmbedding} header contains a single
+ * {@link Embedding}:
* </p>
* <ul>
- * <li>{@code CamelLangchain4jEmbeddingEmbedding} - The embedding vector
(required)</li>
- * <li>{@code CamelLangchain4jEmbeddingTextSegment} - Associated text
segment (optional)</li>
+ * <li>With caller-supplied ID header ({@code
CamelLangchain4jEmbeddingStoreEmbeddingId}): calls
+ * {@code add(id, embedding)}</li>
+ * <li>With text segment header: calls {@code add(embedding,
textSegment)}</li>
+ * <li>Without text segment: calls {@code add(embedding)}</li>
* </ul>
*
* <p>
- * Returns the generated embedding ID in the message body.
+ * <b>Batch operation</b> - when the {@code
CamelLangChain4jEmbeddingsEmbeddings} header contains a
+ * {@code List<Embedding>}:
* </p>
+ * <ul>
+ * <li>With IDs header ({@code
CamelLangchain4jEmbeddingStoreEmbeddingIds}) and text segments body: calls
+ * {@code addAll(ids, embeddings, textSegments)}</li>
+ * <li>With text segments body: calls {@code addAll(embeddings,
textSegments)}</li>
+ * <li>Without text segments: calls {@code addAll(embeddings)}</li>
+ * </ul>
*
* @param exchange the Camel exchange containing the embedding data
* @throws Exception if the add operation fails
*/
+ @SuppressWarnings("unchecked")
private void add(Exchange exchange) throws Exception {
final Message in = exchange.getMessage();
+ EmbeddingStore<TextSegment> store =
getEndpoint().getConfiguration().getEmbeddingStore();
+
+ // Check for batch embeddings header first
+ List<Embedding> embeddings =
in.getHeader(LangChain4jEmbeddingsHeaders.EMBEDDINGS, List.class);
+ if (embeddings != null) {
+ addBatch(in, store, embeddings);
+ return;
+ }
+ // Single embedding path
if (in.getHeader(LangChain4jEmbeddingsHeaders.EMBEDDING) == null) {
throw new NoSuchHeaderException(
"The embedding is a required header for ADD operations",
exchange,
LangChain4jEmbeddingsHeaders.EMBEDDING);
}
Embedding embedding =
in.getHeader(LangChain4jEmbeddingsHeaders.EMBEDDING, Embedding.class);
+
+ // Check for caller-supplied ID
+ String callerId =
in.getHeader(LangChain4jEmbeddingStoreHeaders.EMBEDDING_ID, String.class);
String id;
- if (in.getHeader(LangChain4jEmbeddingsHeaders.TEXT_SEGMENT) != null) {
+ if (callerId != null) {
+ store.add(callerId, embedding);
+ id = callerId;
+ } else if (in.getHeader(LangChain4jEmbeddingsHeaders.TEXT_SEGMENT) !=
null) {
TextSegment text =
in.getHeader(LangChain4jEmbeddingsHeaders.TEXT_SEGMENT, TextSegment.class);
Review Comment:
**[MEDIUM]** When `callerIds != null` but `textSegments == null`, the code
falls through to `store.addAll(embeddings)` which generates new IDs — silently
discarding the user-provided IDs.
The langchain4j API has no `addAll(ids, embeddings)` overload without text
segments, but silently ignoring the user's IDs is a subtle bug. Consider either:
- Looping with `add(id, embedding)` to honor the IDs
- Throwing an `IllegalArgumentException` explaining that caller-supplied IDs
require text segments for batch operations
No test covers this specific edge case.
##########
components/camel-ai/camel-langchain4j-embeddings/src/main/java/org/apache/camel/component/langchain4j/embeddings/LangChain4jEmbeddingsProducer.java:
##########
@@ -55,4 +68,30 @@ public void process(Exchange exchange) throws Exception {
message.setHeader(LangChain4jEmbeddingsHeaders.TEXT_SEGMENT, in);
message.setHeader(LangChain4jEmbeddingsHeaders.EMBEDDING,
result.content());
}
+
+ private void processBatch(Exchange exchange, EmbeddingModel model, Message
message, List<Object> bodyList)
+ throws Exception {
+ // Convert each element to TextSegment using the type converter
Review Comment:
**[MEDIUM]** FQCN used: `new java.util.ArrayList<>()`. Per project
conventions, add `import java.util.ArrayList;` and use `new
ArrayList<>(bodyList.size())`.
##########
components/camel-ai/camel-langchain4j-embeddings/src/main/java/org/apache/camel/component/langchain4j/embeddings/LangChain4jEmbeddingsHeaders.java:
##########
@@ -35,6 +35,10 @@ public class LangChain4jEmbeddingsHeaders {
@Metadata(description = "Embedding representation of a text", javaType =
"dev.langchain4j.data.embedding.Embedding")
public static final String EMBEDDING =
CamelLangchain4jAttributes.CAMEL_LANGCHAIN4J_EMBEDDING;
+ @Metadata(description = "List of embeddings from a batch embedAll
operation",
Review Comment:
**[MEDIUM]** The existing `EMBEDDING`, `VECTOR`, and `TEXT_SEGMENT`
constants all reference `CamelLangchain4jAttributes` in `core/camel-api`
because they are shared across the embeddings and embeddingstore components.
This new `EMBEDDINGS` constant is also used cross-component (set in the
embeddings producer, read in the embeddingstore producer) but uses a hardcoded
string instead. For consistency, a new constant should be added to
`CamelLangchain4jAttributes`.
--
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]
