## Issue
Closes#6068
## Change
Three related fixes to how every `EmbeddingStore` handles the edges of
`addAll` and `removeAll`. Each
one existed in isolation in a dozen-plus stores because the interface
never said what the right
answer was.
### 1. `addAll` validated sizes only after returning on empty input
```java
if (isNullOrEmpty(ids) || isNullOrEmpty(embeddings)) {
log.info("Empty embeddings - no ops");
return;
}
ensureTrue(ids.size() == embeddings.size(), "ids size is not equal to
embeddings size");
```
A mismatch where one of the two lists was empty never reached the check:
the call was silently
ignored, nothing was stored, and nothing was thrown. Every other
mismatch shape, for example 2 ids
against 3 embeddings, threw `IllegalArgumentException`.
`ChromaEmbeddingStore` (#5913) and `PineconeEmbeddingStore` already call
core's
`ValidationUtils.ensureConsistentSizes` before the guard. The remaining
15 stores now do the same:
```java
ensureConsistentSizes(ids, embeddings, embedded);
if (isNullOrEmpty(embeddings)) {
return;
}
```
After the check the two sizes are equal, so testing `embeddings` alone
preserves the empty-input
no-op. Matching lists behave exactly as before.
### 2. `removeAll(ids)` rejected what `addAll` ignored
19 stores called `ensureNotEmpty(ids, "ids")`, so the same user got an
`IllegalArgumentException` for
removing nothing and a silent no-op for adding nothing. Removing nothing
discards nothing, so there
is nothing to warn about:
```java
if (isNullOrEmpty(ids)) {
return;
}
```
This matches the JDK collection methods these APIs mirror
(`list.addAll(emptyList())`,
`map.putAll(emptyMap())`), and it matters in practice because empty
batches are a normal outcome of
filtering upstream, not a programming error. Callers no longer need an
`isEmpty()` guard around every
call. Removing a *single* embedding still rejects a blank id:
`remove(null)` has no sensible "remove
nothing" reading.
### 3. A no-op is not news
16 `log.info("Empty embeddings - no ops")`-style lines across 13 modules
announced that nothing
happened, at the level most applications ship to production, inside
methods that are typically called
in a loop. They are gone, along with the 7 loggers left unused by their
removal.
### Contract
`EmbeddingStore#addAll(List, List, List)` and `#removeAll(Collection)`
never documented any of this,
which is why every store invented its own rules. Their Javadoc now
states it: the lists are
positional and must agree in size, a `null` list of IDs or embeddings
counts as an empty list, a
`null` list of embedded contents is accepted, and having nothing to add
or remove is a no-op.
### Tests
`EmbeddingStoreAddAllContract` and `EmbeddingStoreRemoveAllContract` are
new test interfaces in
`langchain4j-core`'s test-jar. They are interfaces rather than base
classes so a store can implement
both, which a single superclass could not express:
```java
class MilvusEmbeddingStoreContractTest
implements EmbeddingStoreAddAllContract, EmbeddingStoreRemoveAllContract
{
@Override
public EmbeddingStore<TextSegment> embeddingStore() {
return mock(MilvusEmbeddingStore.class,
withSettings().defaultAnswer(CALLS_REAL_METHODS));
}
}
```
Validation and the early return both happen before a store touches its
backend, so these run offline
with no container and no credentials, and a store that failed to return
early would reach an
unconnected backend and fail the test. 22 stores implement one or both
contracts. The bespoke
`PineconeEmbeddingStoreTest` is removed, since the shared contract
covers all of its cases and adds
one for null ids.
`EmbeddingStoreWithRemovalIT`, which 29 store ITs extend, asserted the
old reject-on-empty behaviour;
its two tests now assert that the store is left unchanged instead.
```
langchain4j-core: Tests run: 1247, Failures: 0, Errors: 0, Skipped: 5
langchain4j: Tests run: 1333, Failures: 0, Errors: 0, Skipped: 0
22 store modules: all green
revapi: no API differences reported
```
## Breaking Changes
No API signatures change; the behaviour changes below are all at the
edges of `addAll` and
`removeAll`. No caller inside LangChain4j produces any of the affected
input: `add`, `addAll` and
`EmbeddingStoreIngestor` always pass consistent lists.
**1. A size mismatch where one list is empty or `null` now throws (all
stores)**
| `ids` | `embeddings` | `embedded` | before | after |
| --- | --- | --- | --- | --- |
| `[]` or `null` | `[e]` | `null` | returned, nothing stored | throws |
| `["id"]` | `[]` or `null` | `null` | returned | throws |
| `[]` | `[]` | `[s]` | returned | throws |
If you relied on the no-op, keep the lists consistent or skip the call:
```java
// before: silently did nothing when the lists disagreed
store.addAll(ids, embeddings, segments);
// after: the lists must describe the same entries
if (!embeddings.isEmpty()) {
store.addAll(ids, embeddings, segments);
}
```
Empty on every side is still a no-op, with `null` or empty lists,
exactly as before.
**2. `removeAll(ids)` no longer throws on empty or `null` input**
```java
// before: IllegalArgumentException("ids cannot be null or empty")
// after: no-op, the store is left unchanged
store.removeAll(emptyList());
```
This is a relaxation, so existing working code keeps working. Only code
that *depended* on the
exception is affected:
```java
// before: relied on removeAll to reject an empty batch
try {
store.removeAll(idsToDelete);
} catch (IllegalArgumentException e) {
log.warn("nothing to delete");
}
// after: check it yourself if you care about the distinction
if (idsToDelete.isEmpty()) {
log.warn("nothing to delete");
} else {
store.removeAll(idsToDelete);
}
```
**3. Five stores gain size validation they never had**
`couchbase` and `vespa` never compared `ids` to `embeddings`; `qdrant`,
`milvus` and `milvus-v2` had
no size validation at all. Mismatched lists there previously threw
`IndexOutOfBoundsException` from
inside the driver, or wrote partial data. They now fail up front with a
clear message.
**4. `coherence`: an empty `segments` list is no longer treated as "no
segments"**
```java
// before: stored 2 embeddings without text segments
// after: throws, "embeddings size (2) is not equal to embedded size
(0)"
store.addAll(ids, embeddings, List.of());
// pass null when there are no segments
store.addAll(ids, embeddings, null);
```
**5. Two exception messages changed** (they now report the actual sizes,
from core's helper)
- `couchbase`: `embedded and ids have different sizes` -> `embeddings
size (2) is not equal to embedded size (1)`
- `vespa`: `The list of ids and embeddings must have the same size` ->
`ids size (1) is not equal to embeddings size (2)`
**6. Empty-input INFO logging is gone.** If you relied on `Empty
embeddings - no ops` appearing in
your logs, it no longer does.
## Out of scope
Deliberately not touched, listed here so they are not mistaken for
oversights:
- `addAll` in stores that are stricter than the contract and reject a
`null` `embedded` outright:
`cassandra`, `astradb`, `oracle`, `tablestore`, `in-memory`. Applying
the helper would relax them
and then fail later with a `NullPointerException`, so they need a guard
rather than a swap. Their
`removeAll` is aligned by this PR.
- `addAll` in stores that deviate on purpose: `azure-cosmos-nosql`
(full-text mode accepts empty
`embeddings` with non-empty `embedded`) and `weaviate` (accepts `null`
`ids` and generates them).
- `HibernateEmbeddingStore.addAll(embeddings, embedded)`, the
two-argument overload, has the same
ordering on a method with a different contract (the IDs are generated).
## General checklist
- [ ] There are no breaking changes (API, behaviour)
- [X] I have added unit and/or integration tests for my change
- [X] The tests cover both positive and negative cases
- [X] I have manually run all the unit and integration tests in the
module I have added/changed, and they are all green
- [X] I have manually run all the unit and integration tests in the
[core](https://github.com/langchain4j/langchain4j/tree/main/langchain4j-core)
and
[main](https://github.com/langchain4j/langchain4j/tree/main/langchain4j)
modules, and they are all green
- [ ] I have added/updated the
[documentation](https://github.com/langchain4j/langchain4j/tree/main/docs/docs)
- [ ] I have added an example in the [examples
repo](https://github.com/langchain4j/langchain4j-examples) (only for
"big" features)
- [ ] I have added/updated [Spring Boot
starter(s)](https://github.com/langchain4j/langchain4j-spring) (if
applicable)
---------
Co-authored-by: Dmytro Liubarskyi <ljubarskij@gmail.com>
Subhash Polisetti
208dc4553a