Skip to content

feat(milvus): make distance metric type configurable#6579

Open
gaurav0107 wants to merge 2 commits into
FlowiseAI:mainfrom
gaurav0107:fix/6033-milvus-metric-type-is-not-configurable
Open

feat(milvus): make distance metric type configurable#6579
gaurav0107 wants to merge 2 commits into
FlowiseAI:mainfrom
gaurav0107:fix/6033-milvus-metric-type-is-not-configurable

Conversation

@gaurav0107

Copy link
Copy Markdown

Summary

The Milvus vector store node hardcoded the index metric_type to L2 when it
created the collection index during upsert, even though the similarity-search
path already read the metric from indexCreateParams.metric_type (and applied a
metric-specific score normalization). Because of this, collections that use
COSINE or IP could not be created from Flowise, and querying a collection
configured with a non-L2 metric produced metric-type mismatch errors.

This PR adds a Metric Type option to the node (L2 / COSINE / IP, default L2)
and threads the selected metric into:

  • index creation on upsert (MilvusUpsert.addVectors now uses
    indexCreateParams.metric_type instead of a hardcoded MetricType.L2), and
  • the search path (init sets indexCreateParams.metric_type) so score
    normalization matches the chosen metric.

L2 remains the default and the new input is optional, so existing flows keep
their current behavior.

A small exported helper resolveMilvusMetricType normalizes the input value
(case-insensitive, L2 fallback for empty/unknown), which keeps the mapping
unit-testable without a live Milvus server.

Verification

  • Added packages/components/nodes/vectorstores/Milvus/Milvus.test.ts (jest):
    • resolveMilvusMetricType maps L2/COSINE/IP (any case) correctly and
      falls back to L2 for empty/undefined/unknown input.
    • The node exposes a metricType options input with L2/COSINE/IP and an
      L2 default, and the input is optional.
  • jest (targeted): 6 passed, 6 total.
  • eslint and prettier --check on the changed files: clean.

Closes #6033

The Milvus node hardcoded the index metric_type to L2 when creating the
collection index on upsert, while the similarity-search path already read
the metric from indexCreateParams. As a result, collections using COSINE
or IP could not be created from Flowise and queries against them failed
with metric-type mismatch errors.

Add a 'Metric Type' option (L2 / COSINE / IP, default L2) and thread the
selected metric into index creation on upsert and into the search-side
indexCreateParams so score normalization matches. L2 remains the default,
so existing flows are unchanged.

Closes FlowiseAI#6033

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request introduces support for configuring the distance metric type (L2, COSINE, or IP) in the Milvus vector store node, along with corresponding unit tests. The feedback suggests initializing indexCreateParams safely using object spread if it is undefined to ensure the user-selected metric type is not silently ignored during upsert and search paths. Additionally, casting the metric type to the MetricType enum is recommended to prevent TypeScript compilation errors.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment on lines +272 to +274
if (vectorStore.indexCreateParams) {
vectorStore.indexCreateParams.metric_type = metricType
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

If vectorStore.indexCreateParams is undefined, the check if (vectorStore.indexCreateParams) will prevent setting the metric type, causing it to silently fall back to the default (L2) and ignoring the user's configuration. It is safer to initialize indexCreateParams if it is not already defined. Spreading undefined is safe and does not require a fallback.

Suggested change
if (vectorStore.indexCreateParams) {
vectorStore.indexCreateParams.metric_type = metricType
}
vectorStore.indexCreateParams = {
...vectorStore.indexCreateParams,
metric_type: metricType
}
References
  1. Spreading null or undefined within an object literal is safe and does not require a ?? {} fallback, as it evaluates to an empty object.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Addressed in 5e153b0: indexCreateParams is now built with object spread so the selected metric is always applied.

Comment on lines +359 to +361
if (vectorStore.indexCreateParams) {
vectorStore.indexCreateParams.metric_type = metricType
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

Similarly, if vectorStore.indexCreateParams is undefined, the user-selected metric type will be ignored during the search path initialization. Initializing it safely ensures the metric type is always applied. Spreading undefined is safe and does not require a fallback.

Suggested change
if (vectorStore.indexCreateParams) {
vectorStore.indexCreateParams.metric_type = metricType
}
vectorStore.indexCreateParams = {
...vectorStore.indexCreateParams,
metric_type: metricType
}
References
  1. Spreading null or undefined within an object literal is safe and does not require a ?? {} fallback, as it evaluates to an empty object.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Addressed in 5e153b0: same spread fix applied on the search path so the metric is never silently dropped.

index_name: `myindex_${Date.now().toString()}`,
index_type: IndexType.AUTOINDEX,
metric_type: MetricType.L2
metric_type: this.indexCreateParams?.metric_type ?? MetricType.L2

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

In TypeScript, this.indexCreateParams?.metric_type is typed as a string (or 'L2' | 'COSINE' | 'IP'), whereas createIndex from @zilliz/milvus2-sdk-node expects the MetricType enum. To prevent potential compilation errors under strict type-checking configurations, cast the value to MetricType.

Suggested change
metric_type: this.indexCreateParams?.metric_type ?? MetricType.L2
metric_type: (this.indexCreateParams?.metric_type as MetricType) ?? MetricType.L2

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Addressed in 5e153b0: cast the value to MetricType at the createIndex call site for strict type-checking. Thanks for the review.

Address review feedback: build indexCreateParams with object spread so the
selected metric type is always applied even if the property is initially
undefined (instead of being silently dropped by a truthiness guard), and
cast the metric to MetricType at the createIndex call site for strict
type-checking configurations.
@gaurav0107 gaurav0107 marked this pull request as ready for review June 29, 2026 21:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Milvus metric type is not configurable

1 participant