Skip to content

[Internal] VectorEmbeddingPolicy: Adds EmbeddingSource block to Embedding model#5848

Merged
aavasthy merged 6 commits into
mainfrom
users/aavasthy/vectorembedding
May 14, 2026
Merged

[Internal] VectorEmbeddingPolicy: Adds EmbeddingSource block to Embedding model#5848
aavasthy merged 6 commits into
mainfrom
users/aavasthy/vectorembedding

Conversation

@aavasthy
Copy link
Copy Markdown
Contributor

@aavasthy aavasthy commented May 8, 2026

Pull Request Template

Description

Adds EmbeddingSource block to entries in VectorEmbeddingPolicy, describing where the SDK should call the embedding service for a given vector path — including the source document paths to
embed, the model and deployment name, the service endpoint, and the authentication type (ApiKey or Entra)

Type of change

Please delete options that are not relevant.

  • New feature (non-breaking change which adds functionality)

Closing issues

To automatically close an issue: closes #5840

Comment thread Microsoft.Azure.Cosmos/src/Resource/Settings/Embedding.cs Outdated
Comment thread Microsoft.Azure.Cosmos/src/Resource/Settings/EmbeddingAuthType.cs
ananth7592
ananth7592 previously approved these changes May 11, 2026
Copy link
Copy Markdown
Member

@ananth7592 ananth7592 left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Copy Markdown
Member

@kushagraThapar kushagraThapar left a comment

Choose a reason for hiding this comment

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

Changelog entry is missing, let's add it.

aayush3011
aayush3011 previously approved these changes May 13, 2026
Copy link
Copy Markdown
Member

@aayush3011 aayush3011 left a comment

Choose a reason for hiding this comment

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

LGTM

ananth7592
ananth7592 previously approved these changes May 13, 2026
Copy link
Copy Markdown
Member

@ananth7592 ananth7592 left a comment

Choose a reason for hiding this comment

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

LGTM

@kushagraThapar
Copy link
Copy Markdown
Member

  1. EmbeddingSource has no Equals/GetHashCode, but Embedding.Equals uses object.Equals on it

&& object.Equals(this.EmbeddingSource, that.EmbeddingSource);

Since EmbeddingSource doesn't override Equals, this is reference equality. Two Embedding instances with semantically-identical EmbeddingSource blocks
(different objects) will compare as not-equal. Either implement IEquatable on the new sealed class, or compare field-by-field in
Embedding.Equals.

  1. Silent pre-existing bug fix in Embedding.Equals — behavior change, undocumented On main, Equals had this.Dimensions.Equals(that.Dimensions) (comparing
    Dimensions twice). This PR changes it to this.DistanceFunction.Equals(that.DistanceFunction). Good catch — but previously two embeddings differing only in
    DistanceFunction were considered equal; now they're not. Should be called out in the PR description / changelog so it's not silently shipped.

@aavasthy aavasthy dismissed stale reviews from ananth7592 and aayush3011 via 6556999 May 13, 2026 23:50
@aavasthy
Copy link
Copy Markdown
Contributor Author

  1. EmbeddingSource has no Equals/GetHashCode, but Embedding.Equals uses object.Equals on it

&& object.Equals(this.EmbeddingSource, that.EmbeddingSource);

Since EmbeddingSource doesn't override Equals, this is reference equality. Two Embedding instances with semantically-identical EmbeddingSource blocks (different objects) will compare as not-equal. Either implement IEquatable on the new sealed class, or compare field-by-field in Embedding.Equals.

  1. Silent pre-existing bug fix in Embedding.Equals — behavior change, undocumented On main, Equals had this.Dimensions.Equals(that.Dimensions) (comparing
    Dimensions twice). This PR changes it to this.DistanceFunction.Equals(that.DistanceFunction). Good catch — but previously two embeddings differing only in
    DistanceFunction were considered equal; now they're not. Should be called out in the PR description / changelog so it's not silently shipped.

Fixed in the latest commit. EmbeddingSource now implements IEquatable with field-by-field Equals and GetHashCode (referenced from here https://msdata.visualstudio.com/CosmosDB/_git/CosmosDB?path=/Product/Microsoft.Azure.Documents/SharedFiles/EmbeddingSource.cs&version=GBmaster&line=145&lineEnd=146&lineStartColumn=1&lineEndColumn=1&lineStyle=plain&_a=contents).

On the DistanceFunction fix: keeping it in this PR since Embedding.Equals had to change here anyway to fold in the new EmbeddingSource field.

@aavasthy aavasthy added the auto-merge Enables automation to merge PRs label May 14, 2026
Copy link
Copy Markdown
Member

@kushagraThapar kushagraThapar left a comment

Choose a reason for hiding this comment

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

LGTM, thanks @aavasthy

@aavasthy aavasthy merged commit 88ecaff into main May 14, 2026
32 checks passed
@aavasthy aavasthy deleted the users/aavasthy/vectorembedding branch May 14, 2026 03:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

auto-merge Enables automation to merge PRs

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Embedding V0] VectorEmbeddingPolicy: add EmbeddingSource to Embedding model

4 participants