Skip to content

feat: Add HuggingFace-style skip_special_tokens and batch_decode support (#403) - #1186

Open
lakshitsoni26 wants to merge 2 commits into
mlfoundations:mainfrom
lakshitsoni26:feature/tokenizer-decode-improvements
Open

feat: Add HuggingFace-style skip_special_tokens and batch_decode support (#403)#1186
lakshitsoni26 wants to merge 2 commits into
mlfoundations:mainfrom
lakshitsoni26:feature/tokenizer-decode-improvements

Conversation

@lakshitsoni26

@lakshitsoni26 lakshitsoni26 commented Jun 28, 2026

Copy link
Copy Markdown
  • Fixes Improve tokenizer decode #403 by adding batch_decode, skip_special_tokens, and clean_up_tokenization_spaces to the tokenizer API.
  • Implemented a clean_up_tokenization helper to exactly mimic HuggingFace's internal string-cleaning logic (stripping spurious spaces before punctuation).
  • Added batch_decode and the new decoding arguments to SimpleTokenizer and TikTokenTokenizer.
  • Added pass-through wrappers for the new arguments in HFTokenizer and SigLipTokenizer.
  • Bug Fix: Fixed an edge case where padding tokens could accidentally bleed into the output string in SimpleTokenizer.
  • Testing: Added comprehensive unit tests in tests/test_tokenizer_decode.py covering all tokenizer variants (100% test coverage for the new features).
  • Zero Regressions: Preserves exact backwards compatibility by defaulting the new boolean arguments to False across all signatures.
Screenshot 2026-06-28 at 11 28 12 AM Screenshot 2026-06-28 at 11 29 22 AM Screenshot 2026-06-28 at 2 03 25 PM

@rwightman

Copy link
Copy Markdown
Collaborator

@lakshitsoni26 so there are some risks here, and decisions to be made, it's not a 'zero regression' case... there's also some shifting ground re other significant additions (e.g. #2713) so am going to defer tokenizer cleanup until a bit later

@lakshitsoni26

Copy link
Copy Markdown
Author

Understood! Thanks for taking a look. I'll leave this PR open for now — just ping me whenever the dust settles on #2713 and you are ready to revisit the tokenizer cleanup. Happy to update this PR to resolve any conflicts then!

@ErenAta16 ErenAta16 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.

Checked the special-token handling across all four tokenizer classes since that's where a change like this usually breaks.

SimpleTokenizer.all_special_ids is built from ['<start_of_text>', '<end_of_text>'] + additional_special_tokens, and both are already present in self.decoder (they're added to vocab before the encoder/decoder are built), so skip_special_tokens=True's truncate-at-eot-then-filter-by-all_special_ids correctly strips both, including any user-supplied additional special tokens, not just the two built-in ones.

TikTokenTokenizer.decode's non-skip branch (the one that renders special tokens as literal strings like <|endoftext|> instead of dropping them) hardcodes exactly three ids via if/elif, I checked this against self.all_special_ids = [eot_token_id, pad_token_id, bos_token_id] from __init__ and it's the same three, so nothing falls through the elif chain and gets silently dropped.

The switch from self.decoder[token] to self.decoder.get(token, '') in SimpleTokenizer.decode changes a KeyError into a silent empty string for a truly out-of-vocab id. Given the new batch_decode is clearly meant to run on raw padded tensors, that's the right direction, an id you didn't expect (e.g., stray padding) shouldn't crash decode, but flagging it since it is a behavior change for any existing caller that relied on the KeyError to catch a bad token id.

clean_up_tokenization matches the standard HF replacement list exactly (checked against the reference set of ten replacements), and the test_clean_up_tokenization case traces correctly through the chain by hand.

Good, well-tested addition, no correctness issues found.

@rwightman

Copy link
Copy Markdown
Collaborator

Exploring a different approach to unifying tokenizer interface, aligning options a bit closer to HF while keeping defaults that shouldn't break compat in #1196 ... still pondering aspects of both of these

@lakshitsoni26

Copy link
Copy Markdown
Author

Thanks for the update and for linking #1196! I just took a look at the refactor, and unifying the backend decode helpers while strictly preserving the backwards-compatible defaults is definitely the right architectural approach here.

Since #1196 comprehensively handles the HuggingFace-style decoding we were aiming for, I'm more than happy to close this PR in favor of yours once you are ready to merge it.

Please let me know if you'd like me to help test any of the edge cases on #1196 in the meantime!

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.

Improve tokenizer decode

3 participants