tests/benchmarks: Add VectorType deserialization benchmarks and expand test coverage

[mykaul]

Summary

• Add VectorType deserialization benchmark harness testing 4 strategies across multiple vector sizes and types
• Expand benchmark configurations to include larger vector sizes and more type combinations
• Add unit test coverage for variable-size VectorType Cython fallback and numpy large vector deserialization

Commits (3)

1. benchmarks: Add VectorType deserialization performance benchmark

New benchmarks/vector_deserialize.py (320 lines) testing:

• 4 strategies: VectorType.deserialize(), raw struct.unpack, numpy.frombuffer().tolist(), Cython DesVectorType
• Vector sizes: 3, 4, 128, 384, 768, 1536 (float); 128 (double, int)
• Iteration counts scaled by vector size for stable measurements

2. benchmarks: expand vector sizes

Add double[768], double[1536], int32[64] configurations.

3. tests: add coverage for variable-size VectorType Cython fallback and numpy large vector deserialization

• Test that DesVectorType raises ValueError for variable-size subtypes (UTF8Type) while pure Python handles them
• Exercise the numpy deserialization path for 64-element vectors across float, double, int32, int64
• Also fixes a NotImplemented -> NotImplementedError typo in an integration test assertion

No production code changes — benchmark and test files only.

Note: enabling vector integration tests on Scylla 2025.4+ was previously listed here, but that work actually landed separately (already on master before this branch was created) and isn't part of this PR — removed from this description to avoid confusion.

[Copilot]

Pull request overview

Adds new benchmark and test coverage around VectorType deserialization, and refreshes integration test formatting to support vector-related testing scenarios.

Changes:

• Add a new benchmarks/vector_deserialize.py harness comparing multiple vector deserialization strategies across sizes/types.
• Add unit tests for VectorType large-vector deserialization and intended Cython fallback behavior.
• Reformat/clean up tests/integration/standard/test_types.py (imports/string literals/line wrapping) and keep vector test class enabled via @requires_vector_type.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 6 comments.

File	Description
tests/unit/test_types.py	Adds new unit tests for vector deserialization behavior (including a Cython-deserializer expectation).
tests/integration/standard/test_types.py	Largely formatting/refactoring; keeps/organizes vector integration tests under @requires_vector_type.
benchmarks/vector_deserialize.py	New benchmark script to measure vector deserialization performance across approaches and configurations.

---

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

[coderabbitai]

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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.
Title check	✅ Passed	The title matches the main change: adding VectorType benchmarks and broader test coverage.
Description check	✅ Passed	The description summarizes the changes, commits, and rationale well, though the checklist items are not filled out.

---

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

[Copilot]

Pull request overview

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

Comments suppressed due to low confidence (5)

benchmarks/vector_serialize.py:166

• This function/docstring claims to benchmark BoundStatement.bind() end-to-end, but the implementation never calls bind() and instead manually appends vector_type.serialize(...). This makes the benchmark label misleading and can misinform readers about measured overhead. Either (a) call the real BoundStatement.bind() path using your mocked PreparedStatement, or (b) rename the function and update the docstring/output text to reflect that it benchmarks a minimal/manual serialization path rather than bind().

def benchmark_bind_statement(vector_type, values, iterations=10000):
    """Benchmark BoundStatement.bind() end-to-end with 1 vector column.

    This simulates the full bind path for a prepared statement with a single
    vector column, including column metadata lookup and serialization.
    """
    from unittest.mock import MagicMock

    try:
        from cassandra.query import BoundStatement, PreparedStatement, UNSET_VALUE
    except ImportError:
        return None, None, None

    # Create a mock PreparedStatement with one vector column
    col_meta_mock = MagicMock()
    col_meta_mock.keyspace_name = "test_ks"
    col_meta_mock.table_name = "test_table"
    col_meta_mock.name = "vec_col"
    col_meta_mock.type = vector_type

    prepared = MagicMock(spec=PreparedStatement)
    prepared.protocol_version = 4
    prepared.column_metadata = [col_meta_mock]
    prepared.column_encryption_policy = None
    prepared.routing_key_indexes = None
    prepared.is_idempotent = False
    prepared.result_metadata = None
    prepared.keyspace = "test_ks"

    start = time.perf_counter()
    for _ in range(iterations):
        bs = BoundStatement.__new__(BoundStatement)
        bs.prepared_statement = prepared
        bs.values = []
        bs.raw_values = [values]
        # Inline the core serialization path (no CE policy)
        bs.values.append(vector_type.serialize(values, 4))
    end = time.perf_counter()

benchmarks/vector_serialize.py:37

• float_pack, double_pack, and int32_pack are imported but not used anywhere in this benchmark file. Removing unused imports avoids confusion about which serialization strategy is being measured and keeps the benchmark focused.

from cassandra.cqltypes import FloatType, DoubleType, Int32Type, lookup_casstype
from cassandra.marshal import float_pack, double_pack, int32_pack

benchmarks/vector_deserialize.py:121

• The struct.unpack(format_str, ...) call inside the tight loop includes per-iteration overhead related to format handling/caching, which can skew benchmark results (especially for smaller vectors). For a fairer measurement of unpacking itself (and to match the serialize benchmark), precompile once with struct.Struct(format_str) outside the loop and call unpacker.unpack(serialized_data) inside the loop.

    start = time.perf_counter()
    for _ in range(iterations):
        result = list(struct.unpack(format_str, serialized_data))
    end = time.perf_counter()

tests/unit/test_types.py:562

• Asserting on a specific error-message substring makes this test brittle (minor wording changes will fail the test even if behavior is correct). Prefer asserting via assertRaisesRegex on a stable/intentional fragment, or validate only the exception type (and possibly an error code/attribute if available) while keeping the behavior check (Cython path raises; Python path works).

        with self.assertRaises(ValueError) as cm:
            des_text.deserialize_bytes(data, 5)
        self.assertIn("variable-size subtype", str(cm.exception))

tests/unit/test_types.py:578

• The test name references 'numpy' even though the test explicitly states there is no numpy-specific path being exercised. Renaming the test to reflect what it actually validates (e.g., large fixed-size numeric vector deserialization correctness) will reduce confusion while still keeping the forward-looking intent in the docstring if desired.

    def test_vector_numpy_large_deserialization(self):
        """
        Test that large (>= 32 element) vectors are correctly deserialized for all
        supported fixed-size numeric subtypes.

        Note: VectorType.deserialize() has no numpy-specific fast path today -- this
        exercises the general-purpose Python deserialization code with vectors large
        enough to be representative of real embedding sizes. The name/threshold is
        forward-looking, matching a numpy-accelerated path that may land separately;
        until then this simply guards the correctness of the existing implementation
        at that size.

[Copilot]

🟡 Not ready to approve

The bind benchmark is inaccurate, and multiple advertised test behaviors are not actually exercised.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.

Review details

Comments suppressed due to low confidence (7)

tests/unit/test_types.py:571

• Despite the test name and PR description, this test never imports NumPy or invokes a NumPy-backed path; it only calls the existing pure-Python VectorType.deserialize(). Therefore it does not add the advertised NumPy deserialization coverage. Either exercise the actual NumPy implementation or rename/reframe this as generic large-vector coverage.

    def test_vector_numpy_large_deserialization(self):

benchmarks/vector_serialize.py:165

• This does not benchmark BoundStatement.bind() at all: the timed loop bypasses both the constructor and bind() and directly calls vector_type.serialize(). Consequently the reported “end-to-end” timing omits metadata iteration, value validation, unset handling, and the other bind-path work in cassandra/query.py:626-714. Please construct a fully initialized statement and invoke bind([values]) in the timed loop, or rename this as a serialization-wrapper benchmark.

        bs.values.append(vector_type.serialize(values, 4))

benchmarks/vector_serialize.py:263

• The ratio is inverted for the label: baseline_time / per_op is a speedup factor, whereas overhead relative to baseline is per_op / baseline_time. As written, a slower bind path is reported as less than 1x overhead.

        speedup = baseline_time / per_op
        print(
            f"   Total: {elapsed:.4f}s, Per-op: {per_op:.2f} us, Overhead vs baseline: {speedup:.2f}x"

tests/unit/test_types.py:563

• The PR description says this coverage verifies that DesVectorType raises ValueError for a variable-size subtype, but these assertions instead require GenericDeserializer and exercise no error. The dedicated DesVectorType still lives in the unmerged companion PR, so the advertised regression coverage is absent here. Either test the claimed behavior together with that implementation or update this PR’s stated scope.

This issue also appears on line 571 of the same file.

        des_text = find_deserializer(vt_text)
        self.assertIsInstance(des_text, GenericDeserializer)

tests/unit/test_types.py:627

• _vector_struct does not exist anywhere in this tree, so this explanation describes a nonexistent path. Remove that sentence or explicitly identify it as behavior from a separate future change.

        # ShortType struct.unpack works for small vectors via _vector_struct.

benchmarks/vector_deserialize.py:20

• The current implementation is a per-element Python deserialization loop (cassandra/cqltypes.py:1449-1458); it uses neither batch struct.unpack nor NumPy. Labeling the baseline this way makes the benchmark comparison misleading.

1. Current implementation (Python with struct.unpack/numpy)

tests/integration/standard/test_types.py:584

• This hunk only fixes the exception class; it does not enable vector tests. Scylla 2025.4 vector-test enablement already landed on the base branch in commit 0b1802b7, under tests/integration/__init__.py, so the PR description and commit list incorrectly present it as part of this change. Please remove that stale claim from the PR metadata.

            raise NotImplementedError('Missing datatype not implemented: {}'.format(

• Files reviewed: 4/4 changed files
• Comments generated: 0 new
• Review effort level: Medium

We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.

[coderabbitai]

Actionable comments posted: 3

🧹 Nitpick comments (2)

benchmarks/vector_serialize.py (1)

67-99: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Duplicated subtype→format-string dispatch.

Same if/elif dispatch pattern as benchmarks/vector_deserialize.py's benchmark_struct_optimization/benchmark_numpy_optimization; see the consolidated note for a shared fix across both files.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@benchmarks/vector_serialize.py` around lines 67 - 99, Remove the duplicated
subtype-to-format-string dispatch from benchmark_struct_pack and reuse the
shared helper or mapping established for benchmark_struct_optimization and
benchmark_numpy_optimization in benchmarks/vector_deserialize.py. Preserve the
existing unsupported-subtype return behavior and the format strings used for
supported FloatType, DoubleType, and Int32Type values.

benchmarks/vector_deserialize.py (1)

93-168: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Duplicated subtype→format dispatch logic.

The FloatType/DoubleType/Int32Type/LongType if/elif chain is repeated almost verbatim between benchmark_struct_optimization (Lines 99-116) and benchmark_numpy_optimization (Lines 140-157), and again in benchmarks/vector_serialize.py's benchmark_struct_pack. A small helper mapping subtype → (struct format char, numpy dtype) would remove the triplication and reduce the risk of the branches silently drifting out of sync if a new type is added.

♻️ Suggested consolidation

+_TYPE_INFO = {
+    FloatType: (">{}f", ">f4"),
+    DoubleType: (">{}d", ">f8"),
+    Int32Type: (">{}i", ">i4"),
+    LongType: (">{}q", ">i8"),
+}
+
+
+def _resolve_type_info(subtype):
+    for cass_type, info in _TYPE_INFO.items():
+        if subtype is cass_type or (isinstance(subtype, type) and issubclass(subtype, cass_type)):
+            return info
+    return None, None

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@benchmarks/vector_deserialize.py` around lines 93 - 168, Consolidate the
repeated subtype dispatch used by benchmark_struct_optimization,
benchmark_numpy_optimization, and vector_serialize.py’s benchmark_struct_pack
into one shared helper or mapping. Have it resolve FloatType, DoubleType,
Int32Type, and LongType—including supported subclasses—to their struct format
character and NumPy dtype, while preserving the existing unsupported-type
behavior and using the shared result in all three benchmarks.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@benchmarks/vector_serialize.py`:
- Around line 129-171: Update benchmark_bind_statement to invoke the actual
BoundStatement.bind() path for each iteration instead of manually constructing
the instance and calling vector_type.serialize. Preserve the prepared statement
setup and timing scope so the benchmark measures end-to-end binding overhead,
and return the bound value from the real bind result.
- Around line 102-127: Update benchmark_cython_serializer to guard both
find_serializer(vector_type) and the serializer.serialize(values,
protocol_version) call against missing or incompatible optional Cython APIs.
Return the existing (None, None, None) skip tuple when lookup or serialization
cannot be used, while preserving normal timing and result behavior for a valid
SerVectorType serializer.

In `@tests/unit/test_types.py`:
- Around line 562-569: Update the round-trip assertion in the
GenericDeserializer test to invoke des_text.deserialize(data, 5) instead of
vt_text.deserialize(data, 5), while keeping the existing serialized input and
expected result unchanged.

---

Nitpick comments:
In `@benchmarks/vector_deserialize.py`:
- Around line 93-168: Consolidate the repeated subtype dispatch used by
benchmark_struct_optimization, benchmark_numpy_optimization, and
vector_serialize.py’s benchmark_struct_pack into one shared helper or mapping.
Have it resolve FloatType, DoubleType, Int32Type, and LongType—including
supported subclasses—to their struct format character and NumPy dtype, while
preserving the existing unsupported-type behavior and using the shared result in
all three benchmarks.

In `@benchmarks/vector_serialize.py`:
- Around line 67-99: Remove the duplicated subtype-to-format-string dispatch
from benchmark_struct_pack and reuse the shared helper or mapping established
for benchmark_struct_optimization and benchmark_numpy_optimization in
benchmarks/vector_deserialize.py. Preserve the existing unsupported-subtype
return behavior and the format strings used for supported FloatType, DoubleType,
and Int32Type values.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 2b761866-b7e7-4783-96c7-ba4752403b96

📥 Commits

Reviewing files that changed from the base of the PR and between 9b5b037 and 7a35bb1.

📒 Files selected for processing (4)

• benchmarks/vector_deserialize.py
• benchmarks/vector_serialize.py
• tests/integration/standard/test_types.py
• tests/unit/test_types.py

[Copilot]

🟡 Not ready to approve

Advertised Cython and NumPy paths are not exercised, and one benchmark reports an inverted runtime ratio.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.

Review details

Comments suppressed due to low confidence (4)

tests/unit/test_types.py:610

• This calls the pure-Python VectorType.deserialize directly and never imports NumPy or selects DesVectorType, so it remains green if the advertised NumPy path is unavailable or broken. Add coverage through the NumPy-enabled production deserializer once that path is present, or rename this test and update the PR description to describe it as generic large-vector coverage.

        vt_float = VectorType.apply_parameters(["FloatType", vector_size], {})
        packed = struct.pack(">%df" % vector_size, *float_values)
        result = vt_float.deserialize(packed, 5)

tests/unit/test_types.py:581

• On builds with the compiled extension, GenericDeserializer.deserialize is cdef, so this call always raises AttributeError and the fallback invokes VectorType.deserialize directly. A regression in the selected Cython deserializer/from-binary path would therefore still pass this test, contrary to the stated coverage. Exercise the selected deserializer through a real Cython parser/helper instead of replacing it with the pure-Python call.

This issue also appears on line 608 of the same file.

        try:
            result = des_text.deserialize(data, 5)
        except AttributeError:
            result = vt_text.deserialize(data, 5)

benchmarks/vector_serialize.py:287

• The ratio is inverted for an overhead comparison: if binding takes three times as long as serialization, this reports 0.33x overhead. Divide end-to-end time by baseline time and label it as a runtime ratio so larger values correctly indicate added cost.

        speedup = baseline_time / per_op
        print(
            f"   Total: {elapsed:.4f}s, Per-op: {per_op:.2f} us, Overhead vs baseline: {speedup:.2f}x"

tests/integration/standard/test_types.py:584

• This hunk only corrects the exception class; it does not enable vector integration tests. The actual gate is requires_vector_type in tests/integration/__init__.py:304-306, and that file has no change in this PR. Either include the intended gating change or remove the “enable vector integration tests on Scylla 2025.4+” claim from the PR description.

            raise NotImplementedError('Missing datatype not implemented: {}'.format(

• Files reviewed: 4/4 changed files
• Comments generated: 0 new
• Review effort level: Medium

We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.

[Copilot]

🟡 Not ready to approve

The new tests do not exercise the claimed Cython fallback or NumPy deserialization paths.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.

Review details

Comments suppressed due to low confidence (4)

tests/unit/test_types.py:610

• VectorType.deserialize() is the pure-Python path, so this test never reaches the NumPy fast path described in the PR; it will pass even if NumPy-backed vector deserialization is broken. Exercise the selected DesVectorType through find_deserializer(...).deserialize_bytes(...) for these subtypes (skipping when unavailable), or rename this as generic large-vector coverage and remove the NumPy coverage claim.

        vt_float = VectorType.apply_parameters(["FloatType", vector_size], {})
        packed = struct.pack(">%df" % vector_size, *float_values)
        result = vt_float.deserialize(packed, 5)

tests/unit/test_types.py:640

• This note is inaccurate: the variable-size path handles correctly vint-prefixed ShortType vectors, and _vector_struct does not exist in this codebase. The omission is specifically because this test constructs contiguous fixed-width payloads, while ShortType vector elements use length prefixes.

        # ShortType skipped: serial_size() returns None (pre-existing bug),
        # so VectorType.deserialize takes the variable-size path which fails.
        # ShortType struct.unpack works for small vectors via _vector_struct.

benchmarks/vector_deserialize.py:20

• The current VectorType.deserialize() implementation does not use struct.unpack or NumPy; it slices each element and calls the subtype deserializer. Labeling it this way obscures what the baseline actually measures and duplicates the two explicit strategies below.

1. Current implementation (Python with struct.unpack/numpy)

tests/unit/test_types.py:581

• This does not exercise the selected GenericDeserializer: with the current extension, deserialize is not Python-callable, so every run catches AttributeError and calls VectorType.deserialize directly. A regression in GenericDeserializer.deserialize/from_binary would therefore still pass. Add a Cython helper that invokes from_binary with des_text, as done in tests/unit/cython/types_testhelper.pyx, and assert that result here.

        try:
            result = des_text.deserialize(data, 5)
        except AttributeError:
            result = vt_text.deserialize(data, 5)

• Files reviewed: 4/4 changed files
• Comments generated: 0 new
• Review effort level: Medium

We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.

[mykaul]

Two follow-up fixes pushed (commit 771ed4df1):

1. Fixed the mislabeled benchmark ratio flagged across 3 review passes (benchmarks/vector_serialize.py, benchmark_bind_statement): the "Overhead vs baseline" figure was computed with the same speedup = baseline_time / per_op formula used for genuinely-faster paths, which reads backwards for a path that's slower than baseline (e.g. reporting 0.33x for something that's actually ~3x slower). Now reports overhead = per_op / baseline_time and labels it explicitly as "Nx slower (+N%)".
2. Corrected the PR description: it previously listed "Enable vector integration tests on Scylla 2025.4+" as a summary bullet and commit 3 of 4, but that work landed separately on master before this branch existed and isn't part of this PR. Updated to accurately reflect the 3 real commits.