Skip to content

fix(python): validate query topk#616

Open
HosniBelfeki wants to merge 2 commits into
alibaba:mainfrom
HosniBelfeki:fix-python-query-topk-validation
Open

fix(python): validate query topk#616
HosniBelfeki wants to merge 2 commits into
alibaba:mainfrom
HosniBelfeki:fix-python-query-topk-validation

Conversation

@HosniBelfeki

Copy link
Copy Markdown
Contributor

Summary

Validate Collection.query(topk=...) before building the query context.

The Python wrapper already has _require_positive_integer() for group-by limits, but plain query() passed invalid topk values through to the executor/native layer. This change rejects 0, negative values, None, and booleans with a clear Python-side ValueError.

Tests

  • python -m ruff check python\zvec\model\collection.py python\tests\test_collection.py
  • python -m py_compile python\zvec\model\collection.py python\tests\test_collection.py

Copilot AI review requested due to automatic review settings July 23, 2026 22:05
@HosniBelfeki
HosniBelfeki requested a review from Cuiyus as a code owner July 23, 2026 22:05

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Adds Python-side validation for Collection.query(topk=...) to reject invalid values early (before query context construction / native execution), aligning query() behavior with existing group-by limit validation.

Changes:

  • Validate topk via _require_positive_integer(topk, "topk") at the start of Collection.query().
  • Add a unit test to ensure invalid topk values raise a clear ValueError and do not call the executor.

Reviewed changes

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

File Description
python/zvec/model/collection.py Adds _require_positive_integer check for topk in Collection.query() before building QueryContext.
python/tests/test_collection.py Adds parametrized test covering invalid topk values and asserts the executor is not invoked.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot AI review requested due to automatic review settings July 25, 2026 18:48

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

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

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.

3 participants