Skip to content

fix: count literal special-token strings instead of crashing - #13014

Closed
milangeorge2000 wants to merge 2 commits into
deepset-ai:mainfrom
milangeorge2000:fix/12869-tiktoken-special-tokens
Closed

milangeorge2000 wants to merge 2 commits into
deepset-ai:mainfrom
milangeorge2000:fix/12869-tiktoken-special-tokens

Conversation

@milangeorge2000

Copy link
Copy Markdown

Haystack PR — Fixes #12869

TiktokenCounter.count() used Encoding.encode(), which raises ValueError on text containing special-token markers like <|endoftext|>. Since allowed_special is empty at this call site, the strict check can never emit a special token — it only turns ordinary source text into a crash.

Changes

  • Count with encode_ordinary() instead of encode(), matching the earlier #12853 fix for DocumentSplitter/RecursiveDocumentSplitter.
  • Updated the _FakeEncoder test double (it now stubs encode_ordinary) and added a regression test with a literal <|endoftext|> marker.
  • Added a release note per repo process.

Verification (observed)

  • Real encoder: encode() raises ValueError: ... disallowed special token ...; encode_ordinary() returns 9 tokens for the same string.
  • test/token_counters/: 45 passed, 1 skipped via Hatch.
  • hatch run fmt: all checks passed.

TiktokenCounter.count() used Encoding.encode(), which raises ValueError on text containing special-token markers like <|endoftext|>. Since allowed_special is empty here, the strict check can never emit a special token - it only turns ordinary source text into a crash. Switch to encode_ordinary(), matching the deepset-ai#12853 fix for the document splitters. Adds a regression test and release note. Fixes deepset-ai#12869.
@milangeorge2000
milangeorge2000 requested a review from a team as a code owner September 29, 2026 06:50
@milangeorge2000
milangeorge2000 requested review from bogdankostic and removed request for a team September 29, 2026 06:50
@vercel

vercel Bot commented Sep 29, 2026

Copy link
Copy Markdown
Contributor

Someone is attempting to deploy a commit to the deepset Team on Vercel.

A member of the Team first needs to authorize it.

@github-actions

Copy link
Copy Markdown
Contributor

Hi @milangeorge2000, thanks for your interest in contributing to Haystack! 🙏

⚠️ Issue #12869 is already being addressed by open pull request(s) #12871, #12876, #12879, #12888, #13009. Before opening a PR for an issue, please check whether a PR is already linked to it, and consider contributing to the existing PR instead. We may close duplicate PRs to keep the review queue manageable.

This is an automated message to help us keep the review queue healthy.

@CLAassistant

CLAassistant commented Sep 29, 2026 •

Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
1 out of 2 committers have signed the CLA.

✅ milangeorge2000
❌ milan varghese george


milan varghese george seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You have signed the CLA already but the status is still pending? Let us recheck it.

@HaystackBot

Copy link
Copy Markdown
Contributor

Hi @milangeorge2000, thanks a lot for your contribution! 🙏

We noticed that the Contributor License Agreement (CLA) check (license/cla) hasn't passed yet, so we've temporarily moved this PR to draft and paused the review assignment.

To get your PR reviewed, please sign the CLA via the link in the license/cla check below (or in the CLA bot comment). As soon as the check turns green, this PR will automatically be marked ready for review again and a reviewer will be re-assigned.

@HaystackBot
HaystackBot removed the request for review from bogdankostic September 29, 2026 07:56
@HaystackBot HaystackBot added the cla-pending PR is in draft until the contributor signs the CLA label Sep 29, 2026
@HaystackBot
HaystackBot marked this pull request as draft September 29, 2026 07:56
@julian-risch

Copy link
Copy Markdown
Member

Thank you for your efforts! We're closing this PR because it duplicates #13009, which makes the same one-line fix to TiktokenCounter. This PR is also still a draft with the CLA unsigned, and its release note fails our backtick check.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cla-pending PR is in draft until the contributor signs the CLA topic:tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

TiktokenCounter raises ValueError on literal special-token strings like <|endoftext|>

4 participants