Skip to content

fix: preserve ColBERT tokenizer truncation direction - #801

Merged
joein merged 2 commits into
qdrant:mainfrom
j45856021-dev:fix/cpu-embedding-utilities
Oct 28, 2026
Merged

joein merged 2 commits into
qdrant:mainfrom
j45856021-dev:fix/cpu-embedding-utilities

Conversation

@j45856021-dev

Copy link
Copy Markdown
Contributor

ColBERT loads its document and query tokenizers through load_tokenizer, then reapplies the marker-adjusted context limit. Calling enable_truncation without direction resets both to right truncation, undoing the direction resolved by #775. This also affects JinaColBERT, LateOn and mLateOn, which share the loader.

Pass the already-resolved direction to both limit updates. Context-limit resolution, marker reservation, query expansion, and default/right truncation are unchanged.

The regressions use real local WordLevel tokenizer files and lazy/local-path initialization, without an ONNX session. For example, with left truncation, a 40-token limit and word0 through word49, ColBERT should retain word13 through word49 between its special tokens; unchanged main instead retains word0 through word36.

Validation

  • Before the fix: 16 failures, 16 passing controls across both tokenizers and all four model classes. Only the left-truncation cases fail.
  • After the fix: 40 passed, including the 32 new cases and eight existing model-free controls. Tests check exact tokens, IDs, attention masks, marker-adjusted lengths, short inputs, and tokenizer reloads.
  • Pre-commit-pinned Ruff 0.3.4 lint/format, the CI mypy command (66 files), and the CI Pyright command pass. Pyright was run with the test virtual environment activated; an initial unactivated invocation's 12 unknown-member diagnostics disappear on both unchanged and patched source when that interpreter is selected.
  • Sdist/wheel build passes; the wheel's changed module matches the tested source byte-for-byte.
python -m pytest tests/test_common.py \
  tests/test_late_interaction_embeddings.py::test_get_embedding_size \
  tests/test_late_interaction_embeddings.py::test_colbert_tokenizer_preserves_truncation_direction -q

Environment: macOS arm64, Python 3.13.16, tokenizers 0.23.2, NumPy 2.5.3. Validation is tokenizer-only: no model-weight downloads, pretrained inference, full model suite, or OS/Python matrix. Pre-commit hooks were not installed; their pinned Ruff checks ran directly.

All Submissions

AI disclosure: investigation, implementation, tests and this PR text were developed autonomously with AI assistance. No human pre-review is claimed.

@j45856021-dev
j45856021-dev requested a review from joein as a code owner October 11, 2026 04:26
@coderabbitai

coderabbitai Bot commented Oct 11, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review.

Recent review info
Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 501da462-585b-47a2-8e24-35e6a4cad33c
Commits

Reviewing files that changed from the base of the PR and between 6dd1129 and f1716c4.

Files selected for processing: 1
  • tests/test_late_interaction_embeddings.py

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

Walkthrough

Colbert._load_tokenizer now passes the resolved truncation direction to both tokenizers when reducing the maximum length for reserved marker tokens. A new test checks that document and query tokenizers retain left truncation after loading a configured tokenizer.

Priority: Normal

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

Change: Bug fix

Possibly related PRs

  • qdrant/fastembed#841: Preserves the serialized truncation strategy when load_tokenizer reapplies a context limit, a related tokenizer reconfiguration change.
Merge Risk: Minimal · up to f1716

No actionable merge-blocking risk remains for this narrowly scoped tokenizer fix.

Pre-merge checks: Passed 5
Check name Status Explanation
Title check Passed The title clearly and concisely describes the main change: preserving ColBERT tokenizer truncation direction.
Description check Passed The description directly explains the truncation-direction bug, affected model classes, implementation, regression tests, validation results, and known validation limits.
Docstring Coverage Passed Docstring coverage is inconclusive: files from earlier reviews still need to be rechecked within the check's analysis limits. Not blocking.
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.
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.

@Coding-Professional Coding-Professional 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.

I reviewed the tokenizer change and ran the PR's regression test against this exact commit. Preserving the direction already resolved by load_tokenizer is the right fix when ColBERT reserves marker space; the tests cover document and query tokenizers, left and right truncation, and all four affected model classes.

Local validation: 33 passed (test_colbert_tokenizer_preserves_truncation_direction plus test_get_embedding_size). This was tokenizer-only; I did not run model inference or the full suite.

j45856021-dev and others added 2 commits October 28, 2026 15:12
Keep the resolved direction when reserving marker-token space for document and query tokenizers. Cover the shared ColBERT-family loader with local-tokenizer regressions and right-truncation controls.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@joein
joein force-pushed the fix/cpu-embedding-utilities branch from 6dd1129 to f1716c4 Compare October 28, 2026 08:15
@joein

joein commented Oct 28, 2026

Copy link
Copy Markdown
Member

thank you for the fix!

@joein
joein merged commit 45f3802 into qdrant:main Oct 28, 2026
12 checks passed
@Coding-Professional

Copy link
Copy Markdown

@joein, Please assign me any issue if possible for you.

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