Skip to content

Conversation

@nirinchev
Copy link
Collaborator

While testing things out, I came across a corner case where running an aggregation on a cluster that has both vector and atlas search indexes results in an error being thrown when trying to extract the filterable fields from the index definition. The problem was caused by the fact we were assuming we're always operating on a vector search definition, but in reality, we could be working with an atlas search one.

@nirinchev nirinchev requested a review from a team as a code owner November 11, 2025 12:56
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR fixes a bug where the code incorrectly attempted to extract filterable fields from all search indexes, including lexical (Atlas Search) indexes, when it should only process vector search indexes. The fix ensures that only vector search indexes are considered when validating filter fields in aggregation pipelines.

Key changes:

  • Added filtering logic to exclude non-vector search indexes before validation
  • Renamed parameters to clarify that only vector search indexes are expected
  • Added comprehensive test coverage for the mixed index scenario

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
src/helpers/assertVectorSearchFilterFieldsAreIndexed.ts Added type field to VectorSearchIndex type and filtering logic to exclude lexical search indexes; renamed parameter for clarity
src/tools/mongodb/read/aggregate.ts Added filtering to exclude non-vector search indexes before passing to validation function; updated parameter name
tests/unit/helpers/assertVectorSearchFilterFieldsAreIndexed.test.ts Updated all test cases with renamed parameter and added new test case for mixed vector and Atlas search indexes

Copy link
Collaborator

@himanshusinghs himanshusinghs left a comment

Choose a reason for hiding this comment

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

Looks good, left a note about duplicate filtering.

@coveralls
Copy link
Collaborator

Pull Request Test Coverage Report for Build 19330851331

Details

  • 4 of 5 (80.0%) changed or added relevant lines in 2 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-0.004%) to 80.167%

Changes Missing Coverage Covered Lines Changed/Added Lines %
src/tools/mongodb/read/aggregate.ts 0 1 0.0%
Totals Coverage Status
Change from base Build 19307445981: -0.004%
Covered Lines: 6487
Relevant Lines: 7979

💛 - Coveralls

@nirinchev nirinchev merged commit ecbcfa6 into main Nov 13, 2025
21 checks passed
@nirinchev nirinchev deleted the ni/assert-atlas-search branch November 13, 2025 12:34
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.

4 participants