feat(index): add optional Milvus derived index backend - #332
Conversation
70476b6 to
c7db5bd
Compare
|
Pushed a small follow-up that normalizes Milvus COSINE and BM25 score semantics across Milvus Lite and Zilliz Cloud. Milvus Lite reports COSINE as distance while Zilliz Cloud reports similarity through the same field, so the backend now normalizes both to the LanceDB-style distance contract expected by the recall layer. BM25 scores are also normalized to positive higher-is-better values. Additional validation:
|
b184fad to
9758ba2
Compare
|
Pushed a follow-up to narrow the COSINE score workaround to Milvus Lite 3.0 / 3.0.0 only, with a link to the upstream Milvus Lite tracking issue: milvus-io/milvus-lite#343 For Milvus server, Zilliz Cloud, and future Milvus Lite versions, the backend now treats COSINE raw distance as similarity and converts it to the LanceDB-style distance contract expected by the recall layer. Re-ran the Lite + Zilliz smoke and the focused test suite after the change. |
|
Pushed one follow-up fix commit after running a larger Milvus smoke matrix. Changes:
Validation run:
|
zc277584121
changed the title
Signed-off-by: Cheney Zhang <chen.zhang@zilliz.com>
708e8e9 to
ecd6ea1
Compare
|
Thank you for the depth of work here, and for the follow-up commits — the COSINE normalisation, the topK We have finished evaluating and cannot merge this as it stands. Two of the reasons are upstream Milvus Lite Scope note: we only evaluated embedded Milvus Lite ( Part 1 — Upstream blockersThese decide whether the backend can ship at all, independently of code quality. U1. Search-time
|
| Engine / index | best match ranked first | mean similarity of returned rank-1 | p50 latency |
|---|---|---|---|
| LanceDB, no index (what EverOS runs today) | 100% | 1.0000 | 81.1 ms |
LanceDB IVF_FLAT, nprobes=1 |
100% | 1.0000 | 2.2 ms |
LanceDB IVF_FLAT, nprobes=256 |
100% | 1.0000 | 14.7 ms |
Milvus Lite FLAT (exact — control) |
100% | 1.0000 | 202.4 ms |
Milvus Lite AUTOINDEX (= HNSW, this PR) |
36.5% | 0.8624 | 30.8 ms |
Four controls narrow the cause down:
- Not our measurement. Milvus Lite's own
FLATscores 100% on the same harness, and Milvus's reported
similarities agree with ours to 1.4e-06, so the id mapping is sound. - Not our embeddings being ANN-hostile. LanceDB's
IVF_FLATreaches 100% atnprobes=1in 2.2 ms. - Not insufficient build parameters.
M=48withefConstruction=512moves nothing. - The only effective lever is search-time
ef, which is the dropped parameter. For contrast,nprobes
on the LanceDB side moves latency from 2.2 ms to 14.7 ms across 1 → 256, so that knob is connected.
Our embeddings have a high similarity floor — random pairs average 0.6463 against a mean top-10 similarity of
0.8060 — so HNSW's greedy descent settles in the dense 0.78 shell and never reaches the isolated exact match.
A larger ef is what escapes it. Top-1 carries real weight for us: dedup, "have I already recorded this",
exact recall of a specific fact.
That leaves FLAT as the only usable dense path on Lite, and FLAT is 202 ms against 81 ms for our current
default (and 2.2 ms once LanceDB has an IVF index).
If there is a configuration we have missed that fixes this, we would genuinely welcome it — the reproduction
scripts are small and we are happy to share them.
U2. Embedded Milvus Lite is single-process, and we cannot work around it
Embedded Milvus Lite takes an exclusive file lock on data_dir. A second process gets
DataDirLockedError: another process holds the lock → ConnectionConfigException: Open local milvus failed.
EverOS explicitly supports a CLI process alongside a running server on the same memory root — see
entrypoints/cli/commands/cascade.py::_runtime:
The CLI piggybacks on the same singletons as the running daemon (lazy + process-wide), so if a server
happens to be running on the same memory root, both share state correctly.
Cascade runs inside the API server process (CascadeLifespanProvider), so under [index] backend = "milvus"
running everos cascade sync while the server is up fails. LanceDB supports this: with one process appending
rows and another repeatedly opening the table and counting, both succeed on lancedb 0.34.0.
We did verify that Lite's gRPC server mode (milvus-lite server --data-dir X --port P, clients on
http://127.0.0.1:P) allows concurrent client processes. We are not going to adopt it. Running as a
single process is a deliberate design choice for EverOS; adding a resident sidecar raises deployment and
operational complexity beyond what this project has the maintenance capacity for. So this one has no path
forward on our side.
Part 2 — Implementation issues
Recorded in full because they are worth fixing regardless of Part 1, and because several of them would apply
to any future backend.
Blocking
B1. Collections have no schema definition — the LanceDB models are reflected at runtime.
_build_collection_schema() walks BaseLanceTable.model_fields and infers Milvus types with
_is_datetime / _is_list / _is_float / _is_int, falling back to VARCHAR(65535);
_to_milvus_record() / _restore_row() convert rows by the same inferred rules. Consequences: the LanceDB
Pydantic model becomes the de-facto cross-backend interface (MilvusRepoBase[T: BaseLanceTable]), any field
type we add later is silently coerced to a string until an insert fails, and special cases are keyed on field
names (if name == "subject_vector": continue, the _ms suffix convention).
Please introduce a backend-neutral schema description that both backends render from. We are deliberately
not asking for a hand-written milvus/tables/ mirroring lancedb/tables/ — two parallel definitions kept
in sync by hand would diverge silently on a missed field, which is no better than today.
B2. Multiple dense vector fields per collection must be supported.
subject_vector is dropped from the schema and the domain layer compensates:
if active_backend() == "milvus":
return await self.dense_recall(vector, where, limit=limit)That path is live — agentic.py:118 calls dense_recall_subject_as_child. The stated reason ("the Milvus
backend stores one dense vector field per row") is a design choice rather than a Milvus constraint: this PR
already puts a dense vector field and N <field>__sparse fields in the same collection.
This matters beyond one field. Multi-vector rows (primary + subject vector, multi-view embeddings,
multi-route recall) are a recurring requirement from our algorithm layer, and a backend that cannot express
them forces every future retrieval design to be shaped around the storage engine. Please store
subject_vector as a second dense field, and make the schema builder handle an arbitrary number of vector
fields rather than special-casing names.
B3. One filter DSL, one compiler, N renderers.
The same filter semantics now exist in three places:
| Implementation | Location |
|---|---|
| LanceDB SQL compiler | memory/search/filters.py (_OP_MAP, TIMESTAMP '...') |
| Milvus expression compiler | same file (_MILVUS_OP_MAP, _render_str_literal, _column_for_backend) |
| Regex transpiler | infra/persistence/milvus/repository.py::_lancedb_expr_to_milvus |
BackendFilters also compiles both backends on every search and uses one, and adding a third backend would
mean editing the domain layer again. created_at → created_at_ms is a consequence of storing datetimes as
INT64 in Milvus, so it belongs to a Milvus renderer, not to memory/.
The original seam is ours — compile_filters() has always lived in the domain layer and emitted DataFusion
SQL directly — so we are not asking you to absorb our debt. We will land that refactor as a separate PR and
ask you to rebase onto it. What we need agreement on is the target shape, because the Milvus repo has to be
written against a predicate object rather than a str:
memory/search/filters.pykeeps validation/normalisation only (ALLOWED_FIELDS,RESERVED_FIELDS,
operator whitelist, type coercion) and returns a backend-neutral predicate.FilterNodein
memory/search/dto.pyalready provides the AST.- Repo methods take that predicate, not a string.
- Each backend package owns a renderer holding its operator table, literal escaping and column mapping.
active_backend()is resolved once, in the repo factory, and never appears undermemory/.- Internal predicates (
find_by_owner,find_by_session,fetch_by_entry_ids, …) build predicates too,
which removes the need for_lancedb_expr_to_milvusentirely.
B4. The regex transpiler rewrites string literals.
converted = re.sub(r"(?<![!<>=])=(?!=)", "==", converted)
converted = re.sub(r"\btimestamp\b", "timestamp_ms", converted)The substitution is applied to the whole expression, quoted regions included. session_id is a
client-supplied string and base64-style ids commonly end in =, so session_id = 'abc=' becomes
session_id == 'abc==' and silently matches nothing. Live call paths: find_by_owner, find_by_session,
find_by_parent, find_by_owner_entry, get_by_id(id_field != "id"), and the hand-built predicate in
recall/episode.py::fetch_by_entry_ids. B3 removes this function; until then it is a data-correctness bug.
B5. The new backend has zero CI coverage.
tests/unit/test_infra/test_milvus/test_repo.py is guarded by
skipif(importlib.util.find_spec("pymilvus") is None), CI runs uv sync --frozen without extras, and
pymilvus is not in the dev dependency group — so none of the 758 lines of the new repository execute in
CI. All five CI jobs are ubuntu-latest, so there is no Windows-runner obstacle to adding it.
We have a precedent for exactly this in the same pyproject.toml:
# Tracing tests must actually run (no skip-when-absent), so the optional
# [otel] stack is always present in the dev / CI environment.Please add pymilvus[milvus-lite] to the dev group, and parameterise the cross-backend behavioural tests
so the same assertions run against both backends.
B6. ensure_collection() runs on every operation.
add, upsert, update, count, get_by_id, _query_raw (so every find_*), sparse_search,
dense_search, delete and delete_by_md_path all call it, and it issues has_collection() followed by
describe_collection() — two extra metadata calls per read and per write.
Please mirror the LanceDB side: create collections and verify schemas once at startup (the lifespan
already calls startup() → ensure_business_indexes() + verify_business_schemas()), then serve operations
from a process-wide cache.
B7. VARCHAR(65535) and 512-char array elements are hard caps the default backend does not have.
_build_collection_schema applies max_length=65_535 to every non-specialised string field and
max_length=512 / max_capacity=256 to arrays. Long episode or knowledge bodies that write fine on LanceDB
would fail to insert here. Please validate before write with a diagnosable error and document the limit —
otherwise "switch the backend" silently becomes "some documents no longer persist".
Non-blocking
find_where_paginatedreturnstotal = len(raw), capped bymax_fetch; the LanceDB chassis returns the
predicate's truecount_rows(core/persistence/lancedb/repository.py:381). Use the same exactcount(*)
query you already use incount().fetch_all_for_ownerchanged from an unbounded scan tolimit=10_000while the docstring still says
"Nolimit". This one affects the default LanceDB path (agentic.py:197→acluster_retrieve), and
LanceDB has no implicit limit on a plainquery().where().to_list()— verified on 0.34.0. 10k is enough at
our current scale; please make the cap a named constant, log a warning when it is hit, and fix the
docstring.update()pulls whole rows (1024-dim vector included) to patch one field and re-upserts, capped silently
at 10k;delete_by_md_path()fetches rows only to count them. Milvus has no nativeUPDATE, so
read-modify-write is forced — the issues are the silent cap and the read-just-to-count. For reference the
LanceDB side pushestable.update(...)down and readsnum_deleted_rowsstraight offtable.delete(...).
Every current call site matches a single row, so this is low-risk today.[milvus] dimension = 1024duplicates_DIM = 1024inlancedb/tables/*.py; these will drift.- Milvus Lite returns negative BM25 distances and
abs()restores the ordering, but_scoreends up on a
different scale than LanceDB's. Fusion is RRF so ranking is unaffected; worth documenting that_scoreis
not comparable across backends. - The PR also wires up Milvus server / Zilliz Cloud through
[milvus] uri/token/db_name. Since we
are only evaluating the embedded path, that branch would ship untested — either add coverage or mark it
explicitly unsupported.
Part 3 — Other upstream findings, for the record
Measured on pymilvus 3.0.0 + milvus-lite 3.1.1. Not blocking by themselves, but they belong in the docs if
this backend ever ships.
| Finding | Evidence |
|---|---|
| BM25 IDF statistics are segment-local, not global | Two shards where the same term has opposite local rarity (shard A: 10 docs, 1 contains zebra; shard B: 1000 docs, 500 contain it; globally common at 501/1010). LanceDB scores both identically — ratio 1.0000, global IDF. Milvus Lite scores the shard-A document 2.8745× higher. Ranking from the sparse route then depends on which segment a row landed in; RRF reads ranks so it cannot undo this |
| Exact search is slower than the default backend | FLAT 202 ms vs LanceDB 81 ms on 117k × 1024. _build_local_mask runs a per-row Python loop on every search() call, before handing off to faiss. The cost is per call, and one EverOS request issues several — sparse + dense per route, one call per BM25_FIELDS entry, an extra atomic_fact dense recall in hierarchical mode, multiplied again by sub-queries in agentic mode. Batching would amortise it, but each route submits a single vector |
| The embedded engine has no Windows support | milvus-lite>=2.4.0; sys_platform != "win32" and extra == "milvus-lite" in the pymilvus dependency metadata — on Windows everos[milvus] installs only the client. This conflicts with native-Windows support we need downstream |
| No PQ / binary / fp16 / int8 vectors | Package METADATA → Known Limitations. No impact on us today |
2.x .db files cannot be opened by 3.x, with no automatic migration, and pymilvus declares milvus-lite>=2.4.0 with no upper bound |
pymilvus 3.0.0 dependency metadata. An unpinned upgrade can strand an existing local index |
One correction to an earlier concern of ours: we could not reproduce the COSINE/L2 semantic mismatch from
milvus-lite#343 on 3.1.1 — same=1.0,
orthogonal=0.0, and L2 returns squared distance, all matching the server. Your decision to scope the
workaround to {"3.0", "3.0.0"} is correct.
What happens next
We are not merging this PR. U1 and U2 are the deciding factors and neither is something you can fix here.
If milvus-lite's search-time parameters get connected upstream, we would be glad to re-run the measurements —
the harness is reproducible and we will share it on request. The single-process constraint would still need a
separate answer, since the sidecar route is closed for us.
On our side we will land the filter-abstraction refactor described in B3 as its own PR, independently of the
Milvus backend, because that seam is our own debt and the next backend of any kind will need it.
Thanks again for the work and for the patience through a long review.
Reproduction details for the recall measurement in U1Posting the full procedure so you can check our work or run it against your own data. We went through three Setup
Vectors were L2-normalised after export (raw norms ranged 0.616–19.558), so cosine equals inner product. Similarity profile of the corpus — this is what invalidated our first two metrics:
Mean similarity between random pairs is 0.6463. Ranks 2–10 span 0.004; the rank-10 / rank-11 gap is 0.0002; Metric 1 — recall@10 by id overlap. Discarded.Counting how many of the returned 10 ids appear in the ground-truth 10 gave HNSW 0.3135. That metric needs a The disqualifying check: exact engines do not score 1.0 on it either.
We inspected the 44 "missed" ground-truth items in the LanceDB run individually: every one had a similarity Widening the tolerance confirms it:
And the mean similarity of what HNSW returns divided by the mean similarity of the ideal set is 0.9805. Metric 2 — quality ratio. Informative but not decisive.0.9805 says the returned set is equivalent on average, but averages hide position. Working backwards from Metric 3 — does the engine surface the unambiguous best match? Used.Every query has an exact duplicate at similarity 1.0000 with the runner-up at 0.7871 — a 0.21 gap. This
The two HNSW percentages are identical because when it reaches the exact match it ranks it first, and Controls
What remains is search-time Script for the final metricAssumes import os, statistics, tempfile
import numpy as np
from pymilvus import DataType, MilvusClient
TOPK = 10
corpus = np.load("corpus.npy")
queries = np.load("queries.npy")
n, dim = corpus.shape
sims_all = queries @ corpus.T # unit vectors -> cosine
best = sims_all.max(axis=1) # the unambiguous target per query
def run(label, index_type):
client = MilvusClient(uri=os.path.join(tempfile.mkdtemp(), "t.db"))
s = MilvusClient.create_schema(auto_id=False, enable_dynamic_field=False)
s.add_field("id", DataType.INT64, is_primary=True)
s.add_field("vector", DataType.FLOAT_VECTOR, dim=dim)
ip = client.prepare_index_params()
ip.add_index(field_name="vector", index_type=index_type, metric_type="COSINE")
client.create_collection("t", schema=s, index_params=ip, consistency_level="Session")
for st in range(0, n, 5000):
c = corpus[st:st + 5000]
client.insert("t", [{"id": int(st + j), "vector": c[j].tolist()}
for j in range(len(c))])
client.flush("t")
in_top10 = at_rank1 = 0
r1 = []
for qi in range(len(queries)):
res = client.search("t", data=[queries[qi].tolist()], anns_field="vector",
limit=TOPK, output_fields=["id"],
search_params={"metric_type": "COSINE"})
got = [r["entity"]["id"] for r in res[0]]
sims = sims_all[qi, got]
target = best[qi] - 1e-5
in_top10 += int((sims >= target).any())
at_rank1 += int(sims[0] >= target)
r1.append(float(sims[0]))
m = len(queries)
print(f"{label}: in_top10={in_top10/m:.3f} rank1={at_rank1/m:.3f} "
f"mean_rank1_sim={statistics.mean(r1):.4f}")
client.close()
run("AUTOINDEX", "AUTOINDEX")
run("FLAT", "FLAT")The remaining scripts (corpus export, tolerance sweep, id-mapping check, LanceDB One clarification on
|
| Family | Search-time knob |
|---|---|
HNSW (graph) — what AUTOINDEX resolves to on milvus-lite 3.x (index/factory.py:91, faiss branch, and faiss-cpu is a hard dependency so it always applies) |
ef — candidate queue length during the descent |
| IVF (clustering) — what we used on the LanceDB side | nprobes — how many partitions to scan |
So the LanceDB nprobes sweep is not evidence about ef directly; it is there to show that on comparable
data an ANN index reaches 100% and that its search-time knob responds, which is what isolates the milvus-lite
behaviour.
2 participants
Summary
everos[milvus]and[index] backend = "milvus".<root>/.index/milvus/milvus.db, plus Milvus server or Zilliz Cloud via[milvus] uri,token, anddb_name.MilvusClient, explicit schemas,AUTOINDEX, native BM25 functions, dense COSINE search, and backend-specific filter rendering.Design Notes
subject_vectorrecall falls back to the main episode vector on Milvus because the Milvus backend stores one dense vector field per row.Docs
[index]/[milvus]configuration and the local Lite default.Tests
uv run python -m compileall -q src/everosuv run ruff check src/everos/infra/persistence/milvus/repository.py src/everos/memory/search/filters.py tests/unit/test_memory/test_search/test_recall_episode.py tests/unit/test_memory/test_search/test_recall_knowledge_topic.py tests/unit/test_memory/test_search/test_manager.py tests/unit/test_memory/test_get/test_manager.pyuv run lint-importsgit diff --checkuv run pytest tests/unit/test_config/test_settings.py tests/unit/test_core/test_persistence/test_memory_root.py tests/unit/test_memory/test_search/test_filters.py tests/unit/test_infra/test_milvus/test_repo.py tests/unit/test_memory/test_search/test_recall_episode.py tests/unit/test_memory/test_search/test_recall_knowledge_topic.py tests/unit/test_memory/test_search/test_recall_profile.py tests/unit/test_memory/test_search/test_recall_atomic_fact.py tests/unit/test_memory/test_search/test_recall_agent_skill.py tests/unit/test_memory/test_get/test_manager.py tests/unit/test_memory/test_search/test_manager.py tests/unit/test_scripts/test_check_datetime_discipline.py::test_real_repo_passes_discipline_gateuv run pytest tests/unit: 1483 passed, 20 failed due to Python 3.14 rejecting invalid escape sequences in the installedjiebapackage during tokenizer import.Related to #326.