feat(storage): implement milvus vector store #744

chengjoey · 2025-11-25T03:50:07Z

No description provided.

codecov · 2025-11-25T03:53:36Z

Codecov Report

❌ Patch coverage is 85.19448% with 118 lines in your changes missing coverage. Please review.
✅ Project coverage is 87.58866%. Comparing base (7d13294) to head (9ba3bfe).
⚠️ Report is 3 commits behind head on main.

Files with missing lines	Patch %	Lines
knowledge/vectorstore/milvus/milvus.go	82.77592%	57 Missing and 46 partials ⚠️
...nowledge/vectorstore/milvus/condition_converter.go	84.04255%	9 Missing and 6 partials ⚠️

Additional details and impacted files

@@                 Coverage Diff                 @@
##                main        #744         +/-   ##
===================================================
+ Coverage   87.39556%   87.58866%   +0.19310%     
===================================================
  Files            278         285          +7     
  Lines          36146       37361       +1215     
===================================================
+ Hits           31590       32724       +1134     
- Misses          2977        3002         +25     
- Partials        1579        1635         +56

Flag	Coverage Δ
unittests	`87.58866% <85.19448%> (+0.19310%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

hyprh · 2025-11-26T11:17:47Z

knowledge/vectorstore/milvus/milvus.go

+
+func (vs *VectorStore) queryMetadataBatch(ctx context.Context, limit, offset int, ids []string, filter map[string]any) (map[string]vectorstore.DocumentMetadata, error) {
+	filterExpr := ""
+	if len(ids) > 0 {


这两个条件是不是用 and 的关系也可以，现在是互斥的？

done, 可以用and，改成用searchfilter重新构建语句了

hyprh · 2025-11-26T11:33:24Z

knowledge/vectorstore/milvus/milvus.go

+		annReqs = append(annReqs, annReq)
+	}
+
+	hybridOption := client.NewHybridSearchOption(vs.option.collectionName, vs.getMaxResults(query.Limit), annReqs...)


milvus 默认使用的RRFReranker ，这里返回的score会归一化吗

看了下milvus请求的构建和文档，如果没有指定的话，应该没有使用RRFreranker
我另外加了reranker option，可以自定义配置

nomand-zc · 2025-11-27T01:57:49Z

knowledge/vectorstore/milvus/condition_converter.go

+		return "", nil
+	}
+
+	switch condition.Operator {


这里的and 和 or条件构建函数是不是可以合并成一个函数，以及下面的 ==、!=、 >、>= 、< 、<= 这些也可以合并成一个函数。这里的代码重复率太高了。可以参考：tcvector

nomand-zc · 2025-11-27T01:59:29Z

knowledge/vectorstore/milvus/condition_converter.go

+// milvusFilterConverter converts searchfilter conditions to Milvus expressions
+type milvusFilterConverter struct{}
+
+func (c *milvusFilterConverter) Convert(condition *searchfilter.UniversalFilterCondition) (string, error) {


这里最好加一个recover 处理一下可能的panic。可以参考 tcvector的实现

nomand-zc · 2025-11-27T02:01:01Z

knowledge/vectorstore/milvus/condition_converter.go

+		return c.convertInCondition(condition)
+	case searchfilter.OperatorNotIn:
+		return c.convertNotInCondition(condition)
+	case searchfilter.OperatorLike:


是不是少了一个not like 的实现

好像也还少了一个 searchfilter.OperatorBetween

done,还有between

nomand-zc · 2025-11-27T02:10:19Z

knowledge/vectorstore/milvus/options.go

需要实现一下docBuilder, 方便支持用户自定义存储结构的转换处理。参考

这里其实已经实现了，不需要docBuilder，在创建存储collection和查询的时候都已经支持自定义存储结构

这里是指用户不是通过框架来创建的collection，自己提前创建非标准的collection (这里主要是业务可能有自己独立的离线知识库处理流程，只用knowledge的retriever能力)。所以需要一个WithDocBuilder，让用户把非标的document存储结构转换成标准结构
可以看下docBuilder使用例子

理解了，加上docBuilder了，单元测试里也加上了

nomand-zc · 2025-11-27T02:19:38Z

storage/milvus/milvus.go

+)
+
+// Client is the interface for the milvus client.
+type Client interface {


这里看着都是按照v2的返回值定义的interface。会有和v1有兼容性问题吗？或者将来出v3是不是有可能接口兼容不了呢？这里是不是应该定义和版本无关的接口。

这里没有啥v1，之前的milvus-go-sdk已经废弃了，以后都会在milvus仓库维护了
这里主要是为了实现mock client，和milvus server本身没有啥兼容性问题

nomand-zc · 2025-11-27T02:47:38Z

knowledge/vectorstore/milvus/condition_converter.go

这里用过滤器模版是不是会好点

nomand-zc · 2025-11-27T03:15:05Z

knowledge/vectorstore/milvus/milvus.go

+		conditions = append(conditions, fmt.Sprintf("%s in [%s]", vs.option.idField, strings.Join(idFilters, ", ")))
+	}
+
+	if len(filter.Metadata) > 0 {


filter.Metadata 以及 filter.IDs的条件构建是不是可以统一转换到filter.FilterCondition中，交给Converter统一来处理

nomand-zc · 2025-11-27T03:16:24Z

knowledge/vectorstore/milvus/milvus.go

+	return maxResults
+}
+
+func (vs *VectorStore) buildMetadataFilter(filter map[string]any) (string, error) {


这里的格式转换建议统一放到Converter中来处理。filter.FilterCondition中可能也会存在metadata.xx的条件

已经没有buildMetadataFilter了，全部交由Converter来处理

nomand-zc · 2025-11-27T05:10:08Z

knowledge/vectorstore/milvus/condition_converter.go

+
+func (c *milvusFilterConverter) convertCondition(condition *searchfilter.UniversalFilterCondition) (string, error) {
+	if condition == nil {
+		return "", nil


考虑与其他db的对齐一下，return error？

nomand-zc · 2025-11-27T05:14:31Z

knowledge/vectorstore/milvus/condition_converter.go

+
+func (c *milvusFilterConverter) convertAndCondition(condition *searchfilter.UniversalFilterCondition) (string, error) {
+	if condition.Value == nil {
+		return "", nil


不符合预期的条件或者value，建议都返回error。尽量别把error隐藏了

nomand-zc · 2025-11-27T08:16:56Z

knowledge/vectorstore/milvus/condition_converter.go

+		}
+	}()
+
+	if condition == nil {


这个条件判断貌似可以去掉， convertCondition方法中已经做了统一判断

这里是不是搞反了，应该保留convertCondition中的判断，去掉Convert中的判断。因为AND OR逻辑条件时会递归调用convertCondition，所以这里统一在convertCondition做判断处理会完整一点

Signed-off-by: joey <[email protected]>

nomand-zc · 2025-11-27T13:02:12Z

knowledge/vectorstore/milvus/condition_converter.go

+// - String values must be enclosed in quotes
+// - Numeric values (int, float) should not be quoted
+// - Boolean values should not be quoted
+func (c *milvusFilterConverter) formatValue(value interface{}) string {


time.Time类型需要做特殊处理吗？

nomand-zc · 2025-11-27T13:05:46Z

knowledge/vectorstore/milvus/condition_converter.go

+// - String values must be enclosed in quotes
+// - Numeric values (int, float) should not be quoted
+// - Boolean values should not be quoted
+func (c *milvusFilterConverter) formatValue(value interface{}) string {


milvus文档这里使用过滤器模版是不是在性能和value兼容性上会好点？

对的，用过滤器模版会好点，这个我改下

chengjoey force-pushed the feat/storage-milvus branch from 4829443 to 28587e5 Compare November 25, 2025 09:38

hyprh reviewed Nov 26, 2025

View reviewed changes

chengjoey force-pushed the feat/storage-milvus branch 2 times, most recently from 5e08ae2 to 196c8b0 Compare November 26, 2025 15:38

nomand-zc reviewed Nov 27, 2025

View reviewed changes

knowledge/vectorstore/milvus/condition_converter.go

Copy link

Contributor

nomand-zc Nov 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这里用过滤器模版是不是会好点

nomand-zc reviewed Nov 27, 2025

View reviewed changes

chengjoey force-pushed the feat/storage-milvus branch from 196c8b0 to 137f586 Compare November 27, 2025 07:25

nomand-zc reviewed Nov 27, 2025

View reviewed changes

feat(storage): implement milvus vector store

9ba3bfe

Signed-off-by: joey <[email protected]>

chengjoey force-pushed the feat/storage-milvus branch from 137f586 to 9ba3bfe Compare November 27, 2025 08:52

nomand-zc reviewed Nov 27, 2025

View reviewed changes

feat(storage): implement milvus vector store #744

Are you sure you want to change the base?

feat(storage): implement milvus vector store #744

Uh oh!

Conversation

chengjoey commented Nov 25, 2025

Uh oh!

codecov bot commented Nov 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nomand-zc Nov 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nomand-zc Nov 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nomand-zc Nov 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nomand-zc Nov 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

codecov bot commented Nov 25, 2025 •

edited

Loading

nomand-zc Nov 27, 2025 •

edited

Loading

nomand-zc Nov 27, 2025 •

edited

Loading

nomand-zc Nov 27, 2025 •

edited

Loading

nomand-zc Nov 27, 2025 •

edited

Loading