Fix: Added check the type of tokenizer.json's model_merges

[NEWTON MALLICK (N-E-W-T-O-N)]

Modern Tokenizer store merge's value as list of lists of strings. But models like GPT-2 or gemma-2 store as a list of strings
Ex Mistral

Added a check for this fix

[NEWTON MALLICK (N-E-W-T-O-N)]

Mistral

merges": [
      [
        "\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n",
        "\n"
      ],
      [
        "▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁",
        "▁"
      ],
      [

[NEWTON MALLICK (N-E-W-T-O-N)]

Gpt-2

"merges":["Ġ t","Ġ a","h " ...

[NEWTON MALLICK (N-E-W-T-O-N)]

SHALU AGARWAL (@shaaga) Sayan Shaw (@sayanshaw24)

[Sayan Shaw (sayanshaw24)]

Modern Tokenizer store merge's value as list of lists of strings. But models like GPT-2 or gemma-2 store as a list of strings Ex Mistral

Added a check for this fix

awesome, thanks for this! could you add a unit test here as well to ensure expected functionality: https://github.com/microsoft/onnxruntime-extensions/tree/main/test?

[Sayan Shaw (sayanshaw24)]

/azp run onnxruntime-extensions.CI

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[NEWTON MALLICK (N-E-W-T-O-N)]

Using phi-4 as an example

[NEWTON MALLICK (N-E-W-T-O-N)]

The following Functions is only called when use_fast=True used in passed to tokenizer

[Sayan Shaw (sayanshaw24)]

please use test/data/phi-4-base or test/data/phi-4-mini-reasoning, we'd like to minimize tokenizer files checked into the repo; (also, FYI onnxruntime-extensions only needs the tokenizer.json and tokenizer_config.json files to load tokenizers).

[azure-pipelines]

Commenter does not have sufficient privileges for PR 987 in repo microsoft/onnxruntime-extensions

[NEWTON MALLICK (N-E-W-T-O-N)]

Hi Sayan Shaw (@sayanshaw24)
Revert back the change as requested and updated the branch .
Please merge my pr

[Sayan Shaw (sayanshaw24)]

/azp run onnxruntime-extensions.CI

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[NEWTON MALLICK (N-E-W-T-O-N)]

Merge the latest code .Please rerun the pipeline

[Changming Sun (snnn)]

/azp where

[azure-pipelines]

Azure DevOps orgs getting events for this repository:

• aiinfra

[NEWTON MALLICK (N-E-W-T-O-N)]

Hi Sayan Shaw (@sayanshaw24) pr please merge my PR

[Copilot]

Copilot review overview

🟡 Changes recommended

A critical merge-format bug remains, and string-merge coverage is missing.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 1 · 1

Open (2)

• Generator condition always truthy, corrupting string merges · New
• Fixture lacks coverage for list-of-strings merges · New

What changed in this PR

Updates Hugging Face tokenizer conversion to support alternate merge formats and adds Phi-4 tokenizer coverage.

Changes:

• Handles nested-list and string merge representations.
• Adds a local Phi-4 tokenizer regression test.

File	Summary
test/​test_autotokenizer.py	Adds Phi-4 coverage, but does not test string merges. Nit: Add representative string-merge cases.
onnxruntime_extensions/​_hf_cvt.py	Handles tokenizer merge formats. Critical: The generator condition is always truthy and can corrupt string merges.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[NEWTON MALLICK (N-E-W-T-O-N)]

Sayan Shaw (@sayanshaw24) could you please re-review? Your earlier comments are addressed: the tests only use the existing test/data/phi-4-base and test/data/phi-4-mini-reasoning fixtures, and tokenizer_config.json is unchanged. Commit 37f33f6 also fixes the merges format check that Copilot flagged and adds a phi-4-base test for string merges. test_autotokenizer.py passes locally (11 tests). Could you also trigger CI? Thanks!