Skip to content

fix: validate slice conditions inside local formula queries - #1597

Open
digit50 wants to merge 1 commit into
qdrant:devfrom
digit50:fix/formula-slice-validation
Open

digit50 wants to merge 1 commit into
qdrant:devfrom
digit50:fix/formula-slice-validation

Conversation

@digit50

@digit50 digit50 commented Oct 16, 2026

Copy link
Copy Markdown

closes #1595

In local mode a SliceCondition used inside a FormulaQuery was no longer validated after the slice range checks moved from check_condition into validate_filter, because formula conditions are evaluated through check_condition and never pass through validate_filter. As a result Slice(total=0) raised ZeroDivisionError and an index outside 0..total was accepted and scored as false, while the server rejects both.

This change adds validate_expression in qdrant_client/hybrid/formula.py, which walks the formula once and sends every condition it finds through validate_filter, the same check request filters already use. LocalCollection._rescore_with_formula calls it before scoring any point, so a formula given in a nested prefetch is covered too. The async local client shares LocalCollection, so it gets the same fix.

The new test test_formula_rejects_invalid_slice in qdrant_client/local/tests/test_formula.py runs the example from the issue against an in-memory client: Slice(total=0, index=0) and Slice(total=2, index=5), each placed directly in the formula and inside a Filter under a NegExpression, must raise ValueError naming the bad parameter. All four cases fail on dev and pass with the change.

All Submissions:

  • Contributions should target the dev branch. Did you create your branch from dev?
  • Have you followed the guidelines in our Contributing document?
  • Have you checked to ensure there aren't other open Pull Requests for the same update/change?

Changes to Core Features:

  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Have you written new tests for your core changes, as applicable?
  • Have you successfully ran tests with your changes locally?

Local mode stopped validating a SliceCondition used inside a FormulaQuery once the slice checks moved into validate_filter, so total=0 raised ZeroDivisionError and an out of range index was accepted. The formula expression is now walked once before scoring and each condition in it goes through validate_filter, the same check used for request filters.
@digit50
digit50 requested a review from joein as a code owner October 16, 2026 14:24
@netlify

netlify Bot commented Oct 16, 2026 •

Copy link
Copy Markdown

✅ Deploy Preview for poetic-froyo-8baba7 ready!

Name Link
🔨 Latest commit 62f29fd
🔍 Latest deploy log https://app.netlify.com/projects/poetic-froyo-8baba7/deploys/6ad233a3ef12a6bd8d123b1b
😎 Deploy Preview https://deploy-preview-1597--poetic-froyo-8baba7.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@coderabbitai

coderabbitai Bot commented Oct 16, 2026

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 0fbc7b50-4b48-4642-8bbf-267466a50b3b
📥 Commits

Reviewing files that changed from the base of the PR and between 0eb4d9e and 62f29fd.

📒 Files selected for processing: 3
  • qdrant_client/hybrid/formula.py
  • qdrant_client/local/local_collection.py
  • qdrant_client/local/tests/test_formula.py

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 3 remain after this review.

📝 Walkthrough

The local collection now validates formula expressions before rescoring. Condition expressions use validate_filter, and supported compound and decay expressions are traversed recursively. Tests cover slice conditions with a zero total or an out-of-range index, both directly and inside a negated filter.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix · Severity of issue fixed: Medium

Suggested reviewers: shashvat-singham

Merge Risk: ⚪ Minimal · up to 62f29

No merge-blocking risk is identified; merge after normal checks.

Pre-merge checks: Passed 4 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 40.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 5 functions across 3 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks: 4 passed
Check name Status Explanation
Description check Passed The description clearly explains the invalid slice validation regression, the implementation, the affected clients, and the added tests. It is directly related to the changeset.
Title check Passed The title clearly and concisely identifies the primary change: validating slice conditions inside local formula queries.
Linked Issues check Passed Issue #1595 requires local FormulaQuery requests to reject invalid SliceCondition values, including conditions inside filters and nested expressions, with ValueError messages that identify the invalid…
Out of Scope Changes check Passed The reviewed changes are limited to formula-condition validation, the call site before local formula scoring, and focused regression tests. These changes directly implement issue #1595 and provide req…
Full details: Linked Issues check

Explanation

Issue #1595 requires local FormulaQuery requests to reject invalid SliceCondition values, including conditions inside filters and nested expressions, with ValueError messages that identify the invalid value. The PR adds validate_expression, recursively traverses the supported formula expressions, and sends each condition to validate_filter. LocalCollection._rescore_with_formula runs this validation before scoring, which covers nested prefetch formulas and the shared async local implementation. The added test covers Slice(total=0, index=0) and Slice(total=2, index=5) both directly and under a NegExpression filter, and checks the ValueError messages.

Full details: Out of Scope Changes check

Explanation

The reviewed changes are limited to formula-condition validation, the call site before local formula scoring, and focused regression tests. These changes directly implement issue #1595 and provide required automated coverage. No unrelated change is demonstrated.

  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

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.

1 participant