Skip to content

Conversation

@joein
Copy link
Member

@joein joein commented Dec 2, 2025

on my machine results:

  • test_client_init - 20s -> 0.3s (do not check client's compatibility with server version)
  • test_sparse_recommend - 21.5s -> 4.2s (remove too big sparse vectors (sparse-code), remove/refactor some tests, cause sparse supports only dot product distance)
  • test_simple_opt_sparse_vectors_search - 3.33s -> 0.7s (replace sparse-code)
  • test_simple_opt_vectors_search - 8.8s -> 5.4s (replace sparse-code)
  • test_simple_search - 8.9s -> 5.2s (replace sparse-code)
  • test_sparse_retrieve - 5.4s -> 1s (replace sparse-code)
    etc.

current overall improvements for tests/integration_test.sh on dev image:
5 min 58s -> 4 min 22s

python3.10 in CI 14min -> 10min 34s

@netlify
Copy link

netlify bot commented Dec 2, 2025

Deploy Preview for poetic-froyo-8baba7 ready!

Name Link
🔨 Latest commit 290c494
🔍 Latest deploy log https://app.netlify.com/projects/poetic-froyo-8baba7/deploys/692e9c214b98470008a2ab83
😎 Deploy Preview https://deploy-preview-1130--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
Copy link

coderabbitai bot commented Dec 2, 2025

📝 Walkthrough

Walkthrough

The PR introduces a new boolean parameter check_compatibility to the QdrantClient constructor and updates tests to pass check_compatibility=False for client instantiation. Numerous congruence tests were modified: all references to a "sparse-code" vector were removed or replaced (typically with "sparse-text"), several recommendation test helpers were renamed/removed, and related query/retrieve/search test paths were adjusted to drop the sparse-code path.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

  • Verify QdrantClient.__init__ signature change and default value for check_compatibility.
  • Inspect compatibility-check gating logic and both initialization paths for correctness.
  • Review all updated tests for consistency with the new vector set (removal/replacement of "sparse-code") and renamed recommendation helpers.
  • Confirm no remaining references to removed methods/attributes and that test expectations align with updated behavior.

Suggested reviewers

  • coszio

Pre-merge checks and finishing touches

❌ 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%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately reflects the main objective of the pull request—optimizing test performance through various changes like disabling compatibility checks and removing sparse-code vectors.
Description check ✅ Passed The PR description clearly explains the performance improvements achieved by skipping compatibility checks and removing large sparse code vectors from tests.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch try-to-speed-up-tests

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 and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (1)
tests/test_qdrant_client.py (1)

2021-2022: Consider adding check_compatibility=False for consistency.

For consistency with the changes in test_client_init() and to avoid an unnecessary compatibility check in this auxiliary REST client, consider:

-        rest_client = QdrantClient()
+        rest_client = QdrantClient(check_compatibility=False)

This is a minor optimization since this client is only used for a quick cross-check when the main client uses gRPC.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 1de2ce2 and 9d6f374.

📒 Files selected for processing (1)
  • tests/test_qdrant_client.py (3 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
tests/test_qdrant_client.py (2)
qdrant_client/qdrant_client.py (1)
  • QdrantClient (27-2489)
qdrant_client/qdrant_remote.py (1)
  • QdrantRemote (45-2665)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
  • GitHub Check: Redirect rules - poetic-froyo-8baba7
  • GitHub Check: Header rules - poetic-froyo-8baba7
  • GitHub Check: Pages changed - poetic-froyo-8baba7
  • GitHub Check: Python 3.12.x on ubuntu-latest test
  • GitHub Check: Python 3.13.x on ubuntu-latest test
  • GitHub Check: Python 3.10.x on ubuntu-latest test
  • GitHub Check: Python 3.11.x on ubuntu-latest test
  • GitHub Check: Python 3.9.x on ubuntu-latest test
🔇 Additional comments (2)
tests/test_qdrant_client.py (2)

106-242: LGTM! Appropriate use of check_compatibility=False in initialization tests.

The changes correctly add check_compatibility=False to all remote client instantiations in test_client_init(). This is appropriate because:

  • This test focuses on verifying URL parsing, prefix handling, and connection parameter setup—not compatibility checking logic
  • Local clients (:memory:, path-based) correctly omit the flag since they don't perform compatibility checks
  • The pattern is consistently applied across all remote client test cases
  • The speed improvement (20s → 0.3s per the PR description) is significant for developer productivity

1822-1826: LGTM! Enhanced test coverage for auth token provider.

These additions improve test coverage by explicitly verifying auth token provider behavior both with and without compatibility checks. The test correctly validates that:

  • Without check_compatibility=False: token provider is called during init for the compatibility check, then again for get_collections() (starting at "token_1")
  • With check_compatibility=False: token provider is only called for get_collections() (starting at "token_0")

This makes the test more comprehensive and less brittle to implementation changes.

Also applies to: 1844-1850

@joein joein changed the title fix: do not try to check compatibility in test_client_init tests: speed up tests Dec 2, 2025
* debug: add durations=0 to pytest

* debug: test only test query group

* debug: try adding payload indexes to query group test

* rollback test launch
@joein joein requested a review from tbung December 2, 2025 08:25
@joein joein merged commit 458646c into dev Dec 4, 2025
14 of 19 checks passed
joein added a commit that referenced this pull request Dec 12, 2025
* fix: do not try to check compatibility in test_client_init

* tests: remove sparse-code vectors (10_000 dim), remove euclid recommend methods as non-applicable for sparse

* tests: add payload indexes to query group test

* debug: add durations=0 to pytest

* debug: test only test query group

* debug: try adding payload indexes to query group test

* rollback test launch
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