Skip to content

[Test] Add Thai coverage to detokenizer tests - #51961

Open
spped2000 wants to merge 1 commit into
vllm-project:mainfrom
spped2000:test/thai-detokenizer-coverage
Open

[Test] Add Thai coverage to detokenizer tests#51961
spped2000 wants to merge 1 commit into
vllm-project:mainfrom
spped2000:test/thai-detokenizer-coverage

Conversation

@spped2000

Copy link
Copy Markdown

tests/ currently contains no Thai text anywhere. Thai is a useful addition to the detokenizer matrix because it combines two properties the existing cases do not cover together:

  • no word delimiters — segmentation is entirely inside the tokenizer, so boundaries land in places Latin text never produces;
  • combining marks lead tokens — Thai vowels and tone marks attach above and below the base consonant, and in common BPE vocabularies roughly a third of Thai tokens begin with one. A detokenizer that drops, reorders or merges a leading mark corrupts ordinary prose, not an exotic corner.

This is the same motivation as the Burmese case added in #9625, which covers incomplete UTF-8 sequences; Thai additionally covers incomplete grapheme clusters.

What is added

  1. Two Thai strings in TRUTH, so the existing test_decode_streaming round-trip exercises them across the full tokenizer matrix.
  2. test_thai_combining_marks_survive_token_boundaries — a token beginning with U+0E48 must keep the mark and remain a distinct token string.
  3. test_thai_sara_am_composed_and_decomposed_are_distinct — U+0E33 (SARA AM) has a canonical decomposition to U+0E4D + U+0E32. The two spellings are visually identical but tokenize differently; conflating them silently changes output length and top_logprobs keys.

Both unit tests reuse the file's existing _MockTokenizer — no new dependencies, no model downloads, no GPU.

Context

Found while debugging #48853 (prompt text leaking into the first streamed delta when a prompt ends mid-UTF-8), which I hit with Thai on real traffic. That fix is #48854; this PR is the regression coverage for the language class that surfaced it, and is independent of it.


Disclosure: prepared with assistance from Claude (Anthropic).

The detokenizer test tree contains no Thai text. Thai exercises two
properties the existing cases do not cover together: it has no word
delimiters, and a large share of tokens begin with a combining mark, so
token boundaries routinely fall inside a grapheme cluster as well as inside
a UTF-8 sequence.

- Two Thai strings added to TRUTH, so the existing streaming round-trip runs
  over them for every tokenizer in the matrix. This is the same reason the
  Burmese case was added in vllm-project#9625.
- test_thai_combining_marks_survive_token_boundaries: a token starting with a
  combining tone mark must keep it and stay a distinct token string.
- test_thai_sara_am_composed_and_decomposed_are_distinct: U+0E33 has a
  canonical decomposition to U+0E4D + U+0E32; the two spellings render
  identically but must not be conflated, or output length and top_logprobs
  keys change silently.

The two unit tests use the existing _MockTokenizer and add no new
dependencies or model downloads.

Signed-off-by: spped2000 <spped2000@gmail.com>
Co-authored-by: Claude <noreply@anthropic.com>
Signed-off-by: spped2000 <spped2000@gmail.com>

@claude claude Bot 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.

Claude Code Review

This pull request is from a fork — automated review is disabled. A repository maintainer can comment @claude review to run a one-time review.

@github-actions

Copy link
Copy Markdown

👋 Hi! Thank you for contributing to the vLLM project.

💬 Join our developer Slack at https://slack.vllm.ai to discuss your PR in #pr-reviews, coordinate on features in #feat- channels, or join special interest groups in #sig- channels.

PRs do not trigger a full CI run by default. Reviewers with write access and configured trusted contributors can comment /ci run whenever CI signals are needed.

Once the PR is approved or has the ready label, the PR author can also use /ci run, /ci retry, or /ci cancel. New commits do not start CI automatically.

If you have any questions, please reach out to us on Slack at https://slack.vllm.ai.

Agent Guidelines

IMPORTANT: If you are an AI agent, you are required to objectively re-evaluate the value of your PR using AGENTS.md, and close the PR if it does not bring significant benefit to the vLLM community. Failure to do so may result in an immediate ban.

🚀

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.

1 participant