Skip to content

Conversation

xiaohei520321
Copy link
Contributor

Summary

Fixes vector search functionality in Cosmos DB NoSQL connector that was failing with syntax errors and "One of the input values is invalid" errors.

Changes

  • Fixed VectorDistance function call to use correct 2-parameter syntax instead of 4 parameters
  • Removed RANK keyword from ORDER BY clause to fix SQL syntax error
  • Removed distance function parameter setting as it should be configured in vector index, not query
  • Added proper handling for empty where clauses

Testing

Fixes #13028

- Fixed VectorDistance function call to use correct 2-parameter syntax instead of 4 parameters
- Removed RANK keyword from ORDER BY clause to fix SQL syntax error
- Removed distance function parameter setting as it should be configured in vector index, not query
- Resolves syntax errors and 'One of the input values is invalid' errors in vector search

Fixes microsoft#13028
@moonbox3 moonbox3 added the python Pull requests for the Python Semantic Kernel label Sep 3, 2025
@github-actions github-actions bot changed the title Fix Cosmos DB NoSQL vector search functionality (issue #13028) Python: Fix Cosmos DB NoSQL vector search functionality (issue #13028) Sep 3, 2025
@moonbox3
Copy link
Collaborator

moonbox3 commented Sep 3, 2025

Python Test Coverage

Python Test Coverage Report •
FileStmtsMissCoverMissing
connectors
   azure_cosmos_db.py40116658%181, 198, 204–206, 316, 318, 393, 397, 413–416, 418–421, 423–424, 437–440, 443–445, 451–452, 454, 461–463, 467–469, 492–494, 500–502, 508–509, 511, 516, 533, 608, 625, 628–629, 637, 647–651, 708, 711–712, 749, 751, 764–765, 769–771, 782–785, 788–789, 791–792, 798–803, 808–810, 812–816, 820–821, 829–834, 853–855, 857–860, 868, 875–880, 882–896, 898–909, 912, 914–915, 918–919, 922, 924–929, 935, 939, 963, 991–992, 996–1001, 1093–1098
TOTAL27115472782% 

Python Unit Test Overview

Tests Skipped Failures Errors Time
3719 22 💤 0 ❌ 0 🔥 1m 37s ⏱️

@moonbox3
Copy link
Collaborator

moonbox3 commented Sep 4, 2025

@xiaohei520321 thanks for the fix. Please install the pre-commit locally so it can run the necessary checks. See this DEV_SETUP guide, specifically:

uv run pre-commit install -c python/.pre-commit-config.yaml

Currently, there's a ruff formatting error that needs to be fixed.

- Removed unused json import as detected by ruff
- The json module is no longer needed after fixing VectorDistance function calls
@xiaohei520321
Copy link
Contributor Author

@xiaohei520321 thanks for the fix. Please install the pre-commit locally so it can run the necessary checks. See this DEV_SETUP guide, specifically:

uv run pre-commit install -c python/.pre-commit-config.yaml

Currently, there's a ruff formatting error that needs to be fixed.

ok, I will run pre-commit and fix the error

@xiaohei520321 xiaohei520321 marked this pull request as ready for review September 8, 2025 04:12
@xiaohei520321 xiaohei520321 requested a review from a team as a code owner September 8, 2025 04:12
@moonbox3
Copy link
Collaborator

moonbox3 commented Sep 8, 2025

@xiaohei520321 it doesn't look like the pre-commit is being run properly. There are still ruff formatting errors that must be fixed. See these error logs.

Jian Wang added 2 commits September 8, 2025 21:42
- Fix implicit string concatenation in distance_clause assignment
- Use parentheses for proper multi-line string formatting
- Resolves syntax error that prevented hybrid search functionality
- Update uv.lock after running pre-commit checks

The original code had invalid Python syntax for multi-line string
concatenation which caused the hybrid search feature to fail entirely.
@xiaohei520321
Copy link
Contributor Author

@xiaohei520321 it doesn't look like the pre-commit is being run properly. There are still ruff formatting errors that must be fixed. See these error logs.

I pull main branch and merged main branch, and also run pre-commit again to solve code style, it triggered a lots of changes in uv.lock. And I commit the uv.lock changes also. Is it right now ?

@xiaohei520321
Copy link
Contributor Author

@xiaohei520321 please read the following Contributor License Agreement(CLA). If you agree with the CLA, please reply with the following information.

@microsoft-github-policy-service agree [company="{your company}"]

Options:

  • (default - no company specified) I have sole ownership of intellectual property rights to my Submissions and I am not making Submissions in the course of work for my employer.
@microsoft-github-policy-service agree
  • (when company given) I am making Submissions in the course of work for my employer (or my employer has intellectual property rights in my Submissions by contract or applicable law). I have permission from my employer to make Submissions and enter into this Agreement on behalf of my employer. By signing below, the defined term “You” includes me and my employer.
@microsoft-github-policy-service agree company="Microsoft"

Contributor License Agreement

@microsoft-github-policy-service agree company="Microsoft"

@moonbox3 moonbox3 enabled auto-merge September 11, 2025 03:30
@moonbox3 moonbox3 added this pull request to the merge queue Sep 11, 2025
Merged via the queue into microsoft:main with commit 1404a72 Sep 11, 2025
28 of 29 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
python Pull requests for the Python Semantic Kernel
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Python: it is a bug: missing a comma
3 participants