Skip to content

fix: make tokenizer metadata files optional - #693

Open
libaojiang wants to merge 1 commit into
qdrant:mainfrom
libaojiang:fix/optional-tokenizer-metadata
Open

fix: make tokenizer metadata files optional#693
libaojiang wants to merge 1 commit into
qdrant:mainfrom
libaojiang:fix/optional-tokenizer-metadata

Conversation

@libaojiang

Copy link
Copy Markdown

Summary

  • allow tokenizer loading when config.json is absent by resolving the padding token ID from the tokenizer vocabulary
  • allow tokenizer loading when special_tokens_map.json is absent by deriving special tokens from the tokenizer metadata
  • preserve the existing metadata-file behavior and add focused offline regression tests

Testing

  • pytest tests/test_common.py tests/test_preprocessor_utils.py -q (5 passed)
  • pre-commit run --files fastembed/common/preprocessor_utils.py tests/test_preprocessor_utils.py
  • mypy fastembed --disallow-incomplete-defs --disallow-untyped-defs --disable-error-code=import-untyped
  • pyright tests/type_stub.py

Closes #686

All Submissions

  • Followed the contributing guidelines
  • Checked for existing pull requests for the same change
  • Existing relevant tests pass
  • Added tests for the fallback behavior
  • Ran the repository pre-commit hooks

@coderabbitai

coderabbitai Bot commented Aug 28, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 12fd0726-a3b6-4c7d-8580-969507ee2ff4

📥 Commits

Reviewing files that changed from the base of the PR and between c48247f and b41de1f.

📒 Files selected for processing (2)
  • fastembed/common/preprocessor_utils.py
  • tests/test_preprocessor_utils.py

Included review availability: Your plan provides up to 8 included reviews per hour; 7 remain after this review.


📝 Walkthrough

Walkthrough

load_tokenizer now treats config.json and special_tokens_map.json as optional. It resolves pad_token_id from tokenizer configuration or vocabulary data. It derives special-token identifiers from the tokenizer’s added-token decoder. Tests cover missing tokenizer metadata and validate padding and special-token mappings.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Merge Risk: ⚪ Minimal · up to b41de

The change makes two tokenizer metadata files optional while preserving existing behavior and adds focused offline coverage; no actionable merge-blocking risk remains beyond normal checks and review.

🚥 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 5 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 and concisely describes the primary change: making tokenizer metadata files optional.
Description check ✅ Passed The description directly explains the optional metadata behavior, fallback logic, compatibility goals, tests, and validation commands.
Linked Issues check ✅ Passed The changes address issue #686 by making config.json and special_tokens_map.json optional, resolving pad_token_id from tokenizer metadata when needed, deriving special tokens from tokenizer data, and …
Out of Scope Changes check ✅ Passed The implementation and tests remain within the linked issue scope. The import-order change is incidental and does not introduce unrelated functionality.
Full details: Linked Issues check

Explanation

The changes address issue #686 by making config.json and special_tokens_map.json optional, resolving pad_token_id from tokenizer metadata when needed, deriving special tokens from tokenizer data, and adding regression tests.

  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit 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.

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.

[Feature]: make config.json and special_tokens_map.json optional

1 participant