[Nantha kumar (Nanduu24)]

Summary

Adds a new agent-framework-milvus package providing a Milvus vector store for Agent Framework, implementing the BaseVectorStore / BaseVectorCollection / BaseVectorSearch interfaces. This makes Milvus — one of the most widely used open-source vector databases — available out of the box alongside the existing Postgres, Qdrant, Redis, MongoDB, SQL Server and other vector-store packages.

It follows the structure of the existing agent-framework-qdrant package (settings via load_settings, a store that mints collection clients, borrowed-vs-managed client ownership, operation_options validation, native filtering).

What's included

• MilvusStore / MilvusCollection / MilvusSettings (+ py.typed).
• Schema from the data model — the key field becomes the primary key (INT64 for int keys, VARCHAR otherwise), data fields become typed scalar fields, and vector fields become FLOAT_VECTOR fields, each with its own index.
• Mappings — DistanceFunction → Milvus metric types (cosine_similarity/cosine_distance → COSINE, dot_prod → IP, euclidean_distance/euclidean_squared_distance → L2); IndexKind → Milvus index types (hnsw, flat, ivf_flat, disk_ann, default → AUTOINDEX).
• Full surface — ensure_collection_exists / collection_exists / ensure_collection_deleted, upsert, get (by key or portable filter), delete, and vector search.
• Portable filters translated to Milvus boolean expressions: eq, ne, gt, gte, lt, lte, between, in, not_in, and and/or/not groups. Operators that Milvus can't implement with correct null/collection semantics raise NotImplementedError rather than silently returning wrong results.
• Connector-side score_threshold that respects each metric's comparison direction (larger-is-closer for COSINE/IP, smaller-is-closer for L2), since core does not post-filter.
• Uses the native AsyncMilvusClient throughout, presenting the framework's async interface without wrapping a synchronous client.
• README, a runnable sample (samples/milvus_vectors.py), and the package registered in the workspace [tool.uv.sources].

Review guide — what to focus on

The areas most worth a careful read, since they are where a vector-store connector most easily goes subtly wrong:

• Existing-collection validation (_validate_existing_collection): when a collection already exists, the connector verifies it matches the data model — key presence and primary flag, INT64-vs-VARCHAR key type, each scalar field's DataType, and each vector field's FLOAT_VECTOR type, dimension, metric (via describe_index), and that it is not left on AUTOINDEX — rather than trusting it blindly.
• Filter translation (eq/ne/gt/gte/lt/lte/between/in/not_in + and/or/not), and the deliberate NotImplementedError for operators Milvus can't express with correct null/collection semantics — the design choice is to fail loudly instead of returning wrong results.
• Dense-vector validation (_validate_dense_vector): rejects non-numeric, bool, NaN/inf, and float32-overflow values before they reach the server.
• score_threshold direction handling per metric, given core does not post-filter results.
• Dependency floor and integration coverage (see Testing): the pymilvus>=2.6.0 floor and the new disposable-Milvus integration job.

Testing

• Unit tests (tests/milvus/): filter translation, settings resolution/precedence, and collection behavior (upsert/get/delete/search, schema building, serialization, lifecycle, client ownership) — hermetic, no live service.
• Integration tests gated behind MILVUS_TEST_URI for a real server, now run in CI against a disposable single-node Milvus (embedded etcd + local storage, no cloud credentials) via a new Python Integration Tests - Milvus job that mirrors the existing Qdrant and MongoDB jobs and feeds the aggregate report and the integration gate.
• A dependency-bounds job installs the declared floor (pymilvus==2.6.0) and runs the unit tests + Pyright against it, so the minimum supported version stays covered.
• Verified end-to-end against a live Milvus database (create → index → upsert → semantic search → portable-filter search → get with vectors → list → delete → lifecycle), all passing.
• ruff check, ruff format --check, and Pyright (strict) are clean on the package; uv lock --locked succeeds.

Motivation

Milvus is a top-tier open-source vector database with no first-party Agent Framework integration. This PR implements the accepted feature request in #3488, with a connector that matches the conventions of the other vector-store packages.

Related issue

Closes #3488 — the agreed Milvus VectorStore connector feature request.

[Copilot]

🟡 Changes recommended

Connector correctness, package registration, CI coverage, integration testing, and documentation issues remain unresolved.

9 open findings

• Concurrent collection creation is not handled atomically · New
• String escaping leaves control characters in expressions · New
• Data-field indexing options are silently ignored · New
• Score threshold lacks finite non-boolean validation · New
• Milvus Lite dependency is missing for documented local paths · New
• Milvus tests are not scheduled for pull requests · New
• Integration tests select no tests · New
• Usage snippet passes invalid upsert and result handling · New
• Alpha package is missing from package status metadata · New

What changed in this PR

Adds a first-party Milvus vector-store connector implementing Agent Framework’s collection, store, filtering, lifecycle, and vector-search abstractions.

Changes:

• Adds Milvus schema, CRUD, filtering, search, settings, and client ownership support.
• Adds package documentation, samples, and unit-test scaffolding.
• Registers and locks the new workspace package and dependencies.

File	Description
python/​uv.lock	Locks Milvus and transitive dependencies.
python/​pyproject.toml	Registers the workspace source.
python/​packages/​milvus/​tests/​milvus/​test_settings.py	Tests settings resolution.
python/​packages/​milvus/​tests/​milvus/​test_filters.py	Tests filter translation.
python/​packages/​milvus/​tests/​milvus/​test_collection.py	Tests collection behavior.
python/​packages/​milvus/​tests/​milvus/​conftest.py	Defines test fixtures.
python/​packages/​milvus/​samples/​README.md	Documents the sample.
python/​packages/​milvus/​samples/​milvus_vectors.py	Demonstrates Milvus usage.
python/​packages/​milvus/​README.md	Documents installation and APIs.
python/​packages/​milvus/​pyproject.toml	Defines package metadata and tasks.
python/​packages/​milvus/​LICENSE	Adds the MIT license.
python/​packages/​milvus/​agent_framework_milvus/​py.typed	Marks typed-package support.
python/​packages/​milvus/​agent_framework_milvus/​_vector_store.py	Implements the connector.
python/​packages/​milvus/​agent_framework_milvus/​__init__.py	Exports public APIs.

🧠 Review effort: Balanced

---

💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.

[Allyson (allyson-tcesp)]

Great! Thx

[Eduard van Valkenburg (eavanvalkenburg)]

/review

[github-actions]

MAF Automated Review — Iteration 1

Result: Findings reported
Scope: full PR (2 commit(s)): d121cfe0e10a, 73796abec852
Model: gpt-5.6-sol

Overview

The review found 2 verified inline finding(s).

Reviewed the supplied pull-request change set across correctness, security/reliability, architecture, and failure behavior.
2 verified findings remained after source verification (2 medium) across 1 file. Details are attached to the affected lines below.

Affected areas: python/packages/milvus/agent_framework_milvus/_vector_store.py

[Nantha kumar (Nanduu24)]

Heads-up on the red Public API Compatibility check — it isn't from this connector. griffe raises AliasResolutionError on agent_framework._workflows._edge.EdgeGroup.builtin_type while loading the base API, because that builtins.type alias is imported inside the EdgeGroup class body (so griffe sees it as a class member). It fails on every PR against main. I opened #9614 with a one-line fix (move the alias to module scope, matching _request_info_mixin.py); once it lands and I re-sync, this check should go green here. The connector's own checks pass.

[Nantha kumar (Nanduu24)]

Hi Eduard van Valkenburg (@eavanvalkenburg) — thanks for merging #9614. With that fix on main and the branch re-synced, the Public API Compatibility check (and all of CI) is now green here: 40 passing / 16 skipped, 0 failing.

The code-review items from your last pass are all addressed and pushed — schema safety (strict validation of an existing collection's key/primary/data/vector field types, dims, and metric), dense-vector validation (rejecting NaN/inf/overflow/non-numeric), and the concurrent-create handling.

The one item still awaiting your steer is live integration coverage. Milvus Lite can't be driven through MILVUS_URI (pymilvus reserves that env var for an http(s) server address and crashes at import on a local .db path), so the cleanest option looks like a docker run standalone Milvus service in the integration job, mirroring the existing Qdrant/MongoDB jobs. Happy to wire that up — just let me know if that approach works for you or if you'd prefer something else.

[Eduard van Valkenburg (eavanvalkenburg)]

Nantha kumar (@Nanduu24) Please address the inline dependency-floor finding, implement the integration-test direction in the existing thread, and add the missing reviewer-focus answer to the PR description.

[Eduard van Valkenburg (eavanvalkenburg)]

Nantha kumar (@Nanduu24) I found two additional current-head blockers:

1. In python/packages/milvus/agent_framework_milvus/_vector_store.py:153, _format_scalar() uses json.dumps() with its default ensure_ascii=True, so filtering for a non-BMP string such as "😀" emits UTF-16 surrogate escapes. Milvus parses string literals through Go's strconv.Unquote, which rejects those escapes. Valid stored strings containing emoji or other supplementary-plane characters therefore fail in eq, ne, in, and not_in filters. Please use json.dumps(value, ensure_ascii=False) and add a non-BMP regression while retaining the existing quote/control-character coverage.

2. In _vector_store.py:176-178, _check_scalar_operand() accepts floats for declared integer fields, but the generated expression preserves floating literals. Milvus's INT64 comparison and membership parser cannot cast them, so portable filters such as Filter("id", "eq", 1.0) and Filter("id", "in", [1.0]) fail instead of matching key 1; fractional ordered comparisons also fail. Please normalize integral floats and translate fractional comparisons exactly, or explicitly reject cases the connector cannot represent, and add integer-field tests for integral and fractional float operands.

Please address both before approval.

[Nantha kumar (Nanduu24)]

Thanks Eduard van Valkenburg (@eavanvalkenburg) — both addressed in d04a8b9:

1. Non-BMP strings: _format_scalar now uses json.dumps(value, ensure_ascii=False), so emoji / supplementary-plane characters stay literal UTF-8 instead of 😀 surrogate escapes (which Go's strconv.Unquote rejects). Quote/backslash/control-character escaping is unchanged. Added a non-BMP regression covering eq and in.

2. Integer-field float operands: operands on INT64 fields are now normalized. Integral floats (1.0) emit integer literals so eq/ne/in/not_in match the key; fractional ordered comparisons are translated exactly (id > 1.5 → id >= 2, id < 1.5 → id <= 1, and likewise for between bounds); fractional equality/membership is rejected, since no integer key can equal a fractional value. Float-declared fields are unaffected. Added integer-field tests for both integral and fractional operands.

Separately, now that the integration job runs against a real server it surfaced a third issue: immediate read-after-write missed fresh data under Milvus's default bounded consistency (Milvus Lite is strongly consistent, so it only showed on the server). The collection is now created with consistency_level=Strong and get/query/search default to Strong — both overridable via operation_options — which targets exactly the reads the integration tests do right after upsert.

65 unit tests pass; ruff and Pyright (strict) are clean.

[Nantha kumar (Nanduu24)]

Status after syncing main into the branch and addressing the new validation checks:

• Python Integration Tests - Milvus and Milvus pymilvus 2.6 floor are green — the disposable standalone-Milvus integration job runs the connector end-to-end, and the strong read-after-write fix resolved the earlier empty-read failures.
• Validate Python release manifest is addressed: registered agent-framework-milvus → agent_framework_milvus:MilvusStore in prepare_release.py SMOKE_IMPORTS (the inventory check requires every workspace distribution to have a reviewed public entry point). Release-validation tests pass locally (88 passed).

One red check, Dependency Bounds Validation, is unrelated to this PR: it fails inside packages/azure-documentdb (a Pyright error at its dependency lower bound, _vector_store.py:902), which this PR does not touch. The repo-wide validate-dependency-bounds-test --package "*" surfaces it on any PR. The Milvus package itself passes its own lower-bound validation (pymilvus==2.6.0 floor job is green). Happy to help if useful, but it looked like a pre-existing issue in that package rather than something this change should carry.

[Eduard van Valkenburg (eavanvalkenburg)]

Nantha kumar (@Nanduu24) Independently created connector-owned AsyncMilvusClient instances can reuse the same global connection alias at the supported pymilvus==2.6.0 floor. Each store then believes it owns the client, but closing one removes the shared connection and breaks the other store’s later operations. Please allocate a unique SDK connection alias for each connector-owned client, or raise the dependency floor to a verified version with reference-counted connection ownership. Add a regression proving that closing one independently created store leaves another store usable.

[Nantha kumar (Nanduu24)]

Good catch — fixed in 45f2619.

Confirmed against the 2.6.0 source: AsyncMilvusClient derives its connection alias from the uri/auth/db_name and reuses any global connection already registered under it (pymilvus.milvus_client._utils.create_connection), so two connector-owned stores at the same server shared one connection — and close() → connections.async_remove_connection(alias) on one tore it down for the other.

I took the unique-alias route rather than raising the floor: each connector-created client now gets alias="agent-framework-milvus-<uuid4>", so it owns an independent connection. Caller-supplied clients are untouched and keep managing their own alias.

Regression added in test_settings.py:

• test_connector_clients_get_unique_connection_alias — two stores get distinct aliases.
• test_closing_one_store_leaves_independent_store_usable — closing one independently created store leaves the other connected (fails without the fix: both collapse to async-http://localhost:19530).

67 unit tests pass, ruff + pyright-strict clean.