yunfengzhou-hub opened a new pull request, #1280: URL: https://github.com/apache/flink-agents/pull/1280
Linked issue: none, every commit here is a hotfix. ### Purpose of change Three vector store defects where the code does not do what its own declaration promises. - **Elasticsearch.** The constructor rejected `num_candidates >= k`, which Elasticsearch requires, and accepted `num_candidates < k`, which Elasticsearch refuses at query time. A store-level `k` also never took effect, although its javadoc and the parameter table both describe it as a default a query can override. - **Mem0.** `pyproject.toml` declares `mem0ai>=0.1.43`, but `_query_embedding` calls `store.search(query=..., vectors=[...])` and `VectorStoreBase.search` only takes `vectors` from 0.1.74 on. An install inside `[0.1.43, 0.1.74)` fails every vector query with `TypeError`. - **Filter DSL.** The documentation promised that any implementation receiving an unsupported operator raises `NotImplementedError` or `UnsupportedOperationException`. Only Chroma and Milvus do. #### Runtime flow These two sub-headings cover the Elasticsearch change, the only one that alters observable behavior. The constructor reads `k` and `num_candidates` from the descriptor and rejects one combination of them. At query time `BaseVectorStore.query` merges `getStoreKwargs()` with the query's own arguments and passes the result to `queryEmbedding`, which reads `k` back out of that map. A store-level `k` therefore reaches a query only under the key `queryEmbedding` reads. #### Key decisions The validation now rejects `num_candidates < k`. Elasticsearch documents `num_candidates` as required to be at least `k`, so the rejected combination is the one that cannot succeed. `getStoreKwargs` now exposes the store-level `k` as `k` rather than `top_k`. `top_k` is read only by `S3VectorsVectorStore`, from its own arguments; nothing read it on the Elasticsearch path. For Mem0 the floor is raised rather than a compatibility branch added. Only `search` differs across the boundary; the six other methods the integration calls have identical signatures on both sides. A branch would wrap one call to support versions where vector search cannot work. For the Filter DSL the documentation is corrected rather than the four backends. Local validation would change which exception a caller catches, across four modules in two languages, and Mem0 forwards `filters` to a provider the caller chooses, so its semantics are not ours to enforce. ### Behavioral Semantics Applies to the Elasticsearch change. The Mem0 floor is packaging metadata and the Filter DSL edit is prose. #### Interaction decisions | store `k` | store `num_candidates` | query `k` in `extraArgs` | Result | |---|---|---|---| | unset | any | unset | no check; `queryEmbedding` falls back to `max(1, limit)` | | set | unset | unset | no check; store `k` reaches `queryEmbedding` | | set | set, `>= k` | unset | constructs; store `k` reaches `queryEmbedding` | | set | set, `< k` | unset | constructor throws `IllegalArgumentException` | | any | any | set | the query value wins; `query` merges `extraArgs` over the store kwargs | #### Behavioral contracts 1. A descriptor with `num_candidates >= k` constructs successfully. 2. A descriptor with `num_candidates < k` is rejected at construction. 3. A store-level `k` reaches `queryEmbedding` when the query does not override it. 4. A `k` in the query's `extraArgs` overrides the store-level one. 5. Omitting either argument skips the check. 6. `getStoreKwargs` no longer contains `top_k`. #### Failure behavior Contract 2 raises `IllegalArgumentException` from the constructor. That is the same point in the lifecycle as before, with the condition inverted: an invalid combination now fails when the resource is built rather than as an Elasticsearch 400 at query time. Nothing else in the change raises, falls back or retries. #### Compatibility impact A configuration that previously constructed and then failed at query time now fails at construction; one that previously failed at construction now works. A caller who set a store-level `k` and saw it ignored now gets it applied, which changes result counts. A caller who always passed `k` per query sees no difference. `getStoreKwargs` is public, so a caller reading `top_k` from the Elasticsearch store now gets nothing; no code in this repository did. Raising the Mem0 floor makes an install that would resolve inside `[0.1.43, 0.1.74)` fail dependency resolution instead of failing at query time. Installs on 0.1.74 or later are unaffected. ### Tests | Contract | Tests | |---|---| | 1, `num_candidates > k` | `acceptsNumCandidatesGreaterThanK` | | 1, `num_candidates == k` | `acceptsNumCandidatesEqualToK` | | 2 | `rejectsNumCandidatesLessThanK` | | 3 and 6 | `exposesStoreLevelKUnderTheKeyQueryReads` | | 4 | none | | 5 | none | All four ran against the unfixed source first: the two acceptance tests and the store-kwargs test failed, and the rejection test reported no throwable. Restoring `top_k` in place of `k` fails only the store-kwargs test. The Elasticsearch module runs 10 tests and the `api` module 769, both with 0 failures. Spotless, Apache RAT and a Hugo 0.110.0 site build all pass. Not verified: - Contracts 4 and 5 live in `queryEmbedding`, which needs a live cluster. The module's existing test is gated on `ES_HOST` and was skipped. - No query ran against a server, so the claim that Elasticsearch refuses `num_candidates < k` rests on its documented requirement, not an observed 400. - No install was resolved against a mem0ai in `[0.1.43, 0.1.74)` to observe the `TypeError`. - The Filter DSL wording was checked by reading all six backends, not by exercising them. <details> <summary>Supporting evidence</summary> - mem0ai boundary: the published wheels for 0.1.43, 0.1.50, 0.1.60, 0.1.66, 0.1.70 and 0.1.73 all declare `VectorStoreBase.search(self, query, limit=5, filters=None)`; 0.1.74 and 1.0.11 declare `search(self, query, vectors, limit=5, filters=None)`. The base class is the right layer to check because the integration resolves a configurable `provider` through `VectorStoreFactory`. - Elasticsearch `num_candidates`: the vendor documentation states it must be greater than or equal to `k`. - Filter DSL audit: Chroma raises `NotImplementedError` for a `$`-prefixed key or a dict value; Milvus raises `UnsupportedOperationException` for a null or `Map` value. Elasticsearch and OpenSearch serialise the value into a `term` query without inspecting it. S3Vectors applies `String.valueOf` to it, so an operator form becomes a literal that silently matches nothing. Mem0 passes `filters` through to the configured provider. - Elasticsearch null values: `filtersToBoolMustJson` builds `Map.of(field, value)`, which throws `NullPointerException` on a null value. Not addressed here, and independent of the operator-form contract. </details> ### API No public API signatures change. `getStoreKwargs` keeps its signature and returns one different key for the Elasticsearch store. ### Documentation - [ ] `doc-needed` - [ ] `doc-not-needed` - [x] `doc-included` ### Was this patch authored or co-authored using generative AI tooling? - [x] Yes - [ ] No Generated-by: Qoder 0.4.4 (Qwen 3.8-Max) -- 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]
