Skip to content

Fix partial sparse vector configuration updates in local mode - #1533

Open
wcsnbilil wants to merge 1 commit into
qdrant:devfrom
wcsnbilil:codex/fix-local-sparse-config-updates
Open

wcsnbilil wants to merge 1 commit into
qdrant:devfrom
wcsnbilil:codex/fix-local-sparse-config-updates

Conversation

@wcsnbilil

Copy link
Copy Markdown

Updating sparse vector settings in local mode currently replaces the whole configuration. An index-only update can silently remove modifier=IDF: with two one-term points, the same query changes from ~0.6931472 to 1.0. Empty updates also clear settings, and modifier-only updates discard the index configuration.

Merge non-null fields into an owned copy of the existing configuration, including fields inside index. This matches the server's partial-update behavior. Explicit False, 0, and Modifier.NONE remain valid updates.

Seven new regression cases cover omitted/empty fields, nested index updates, explicit IDF disabling, and caller/returned-object isolation. All seven failed before the fix.

Validation

  • Focused regression + config isolation: 10 passed on Python 3.12 / Pydantic 2.13.5 and Python 3.10 / Pydantic 1.10.26.
  • Verified index-only, empty, and modifier-only updates against a real Qdrant 1.19.1 server over REST; configurations and scores match the fixed local behavior.
  • Async local client test, close/reopen persistence smoke, module doctest, targeted mypy, and all configured pre-commit hooks passed.
  • Broader offline suite on Windows/Pydantic 2: 181 passed, 4 SQLite file-lock failures. All four reproduce on unchanged dev.
  • Pydantic 1 local suite: 116 passed, 30 fixture setup errors in test_referenced_vectors (integer values rejected as strict floats). The same 30 errors reproduce on unchanged dev.
  • The full Docker/embedding integration suite was not run.

All Submissions

  • Branch created from and targeting dev.
  • Followed the README development instructions and installed pre-commit hooks.
  • Checked existing issues and pull requests for overlapping changes.

Changes to Core Features

  • Explained the behavior and reason for the change.
  • Added regression tests.
  • Ran relevant tests locally, with limitations recorded above.

Developed and reviewed with Codex assistance.

@wcsnbilil
wcsnbilil requested a review from joein as a code owner October 5, 2026 11:19
@netlify

netlify Bot commented Oct 5, 2026 •

Copy link
Copy Markdown

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

Name Link
🔨 Latest commit 807270f
🔍 Latest deploy log https://app.netlify.com/projects/poetic-froyo-8baba7/deploys/6ac387d835ec4b0009ad23ca
😎 Deploy Preview https://deploy-preview-1533--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 5, 2026

Copy link
Copy Markdown

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

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: 63e4eaaa-333e-40b4-8746-7b4638517ec3
📥 Commits

Reviewing files that changed from the base of the PR and between d64b76d and 807270f.

📒 Files selected for processing (2)
  • qdrant_client/local/local_collection.py
  • qdrant_client/local/tests/test_sparse_config_updates.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

Walkthrough

LocalCollection.update_sparse_vectors_config now applies supplied non-null values while retaining unspecified sparse-vector settings. Added local-mode tests cover partial index updates, modifier changes, query scores, and isolation from mutations to caller and returned configurations.

Priority: ➖ Normal

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

Change: Bug fix

Merge Risk: ⚪ Minimal · up to 80727

The local update now preserves omitted sparse-vector settings, and no concrete merge-blocking issue remains in the supplied evidence.

Security Architecture Review

Security architecture risk: ⚪ Minimal · up to 80727

The change preserves unspecified settings without expanding access or authority. Configuration updates remain confined to the selected collection and use owned copies before publication. No material security risk was identified in the reviewed change.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — The changed operation affects the named existing sparse-vector configuration within the collection selected by the local client. A request can update multiple entries through the existing dispatcher, but this PR adds no cross-collection target or wider authority.

Trust Boundaries and Controls

  • observed — Backend selection and collection lookup are unchanged. The method retains its existing vector-membership check and configuration-copy boundary; caller-controlled index values are merged into owned state rather than retaining the caller's index object.

Resilience and Maintainability Implications

  • observed — The existing dispatcher can leave earlier updates applied in memory if a later update fails before saving. Metadata writes are not transactional. These failure-containment limitations predate the PR and are not newly introduced architecture concerns.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 7 functions across 2 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main change: fixing partial sparse-vector configuration updates in local mode.
Description check ✅ Passed The description explains the configuration-update problem, the intended merge behavior, the regression tests, and validation results.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR
  • Autopilot · 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