Skip to content

[serge] Fix 6 integration tests for model mluke failing with other (other (6)) - #48385

Closed
sergereview[bot] wants to merge 1 commit into
mainfrom
serge/fix/itf-174023b85761-93ad72e1
Closed

sergereview[bot] wants to merge 1 commit into
mainfrom
serge/fix/itf-174023b85761-93ad72e1

Conversation

@sergereview

@sergereview sergereview Bot commented Aug 28, 2026 •

Copy link
Copy Markdown
Contributor

CPU CI GPU run-slow

Original CI failure

  • Failure group: 6 integration tests for model mlukefailing withother (other (6))
  • tests/models/mluke/test_tokenization_mluke.py::MLukeTokenizerIntegrationTests::test_entity_span_classification_no_padding_or_truncation [multi-gpu] (other, seen 7/7)
CI traceback — tests/models/mluke/test_tokenization_mluke.py::MLukeTokenizerIntegrationTests::test_entity_span_classification_no_padding_or_truncation
(line 572)  AssertionError: '<s> [33 chars]e spoken by about 128 million people, primarily in Japan .</s>' != '<s> [33 chars]e spoken by about 128 million people, primarily in Japan.</s>'

Where to watch it:

Relates to #48384

The MLuke integration tests fail because:

  1. Entity markers are emitted as <s>/</s> instead of <ent>/<ent2>. The hub config can place other tokens (e.g. <s>, </s>) at the front of extra_special_tokens, so extra_special_tokens_ids[0/1] no longer point to the entity markers.
  2. Tokenizing entity spans piecewise makes the SentencePiece/Metaspace backend prepend ▁ to punctuation that follows an entity boundary (e.g. Japan . instead of Japan.).

The patch:

  • Moves <ent> and <ent2> to the front of extra_special_tokens so the hard-coded indices are reliable.
  • Tokenizes the full text once and computes entity token spans from the encoding offsets, avoiding spurious ▁ prefixes on trailing punctuation.
  • Corrects _convert_token_to_id / _convert_id_to_token so added tokens (like <ent>) are not shifted by fairseq_offset.

No modular_*.py exists for mluke, so tokenization_mluke.py is edited directly.


✅ Verified on GPU

serge ran the targeted @slow test(s) on a GPU runner and opened this PR only after they passed with this patch. Each targeted test was run 5× on both the pre-patch and patched trees to rule out flakiness — the result held on every run.


This change was produced automatically by serge from a CI failure report. The patch was generated by an LLM and applied by serge; review before merging.

serge v0.1.0 · model: moonshotai/Kimi-K2.7-Code · 51 LLM turns · 51 tool calls · 261.8s · 2196431 in / 50080 out tokens

@sergereview
sergereview Bot marked this pull request as ready for review August 28, 2026 01:44
@github-actions
github-actions Bot requested a review from ArthurZucker August 28, 2026 01:45
@github-actions

Copy link
Copy Markdown
Contributor

[For maintainers] Suggested jobs to run (before merge)

run-slow: mluke

@github-actions

Copy link
Copy Markdown
Contributor

CI recap

Dashboard: View test results in Grafana
Latest run: 33133799356
Result: success | Grafana metrics are not available yet.

@HuggingFaceDocBuilderDev

Copy link
Copy Markdown

The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update.

@ydshieh

ydshieh commented Aug 28, 2026 •

Copy link
Copy Markdown
Collaborator

MLuke tokenizer integration test failures — findings

We investigated the long-running failures of the following three tests:

  • tests/models/mluke/test_tokenization_mluke.py::MLukeTokenizerIntegrationTests::test_entity_classification_no_padding_or_truncation
  • tests/models/mluke/test_tokenization_mluke.py::MLukeTokenizerIntegrationTests::test_entity_pair_classification_no_padding_or_truncation
  • tests/models/mluke/test_tokenization_mluke.py::MLukeTokenizerIntegrationTests::test_entity_span_classification_no_padding_or_truncation

Timeline

Period Status Error
before 2025-11-28 ✅ passing —
2025-11-28 → 2025-12-09 ❌ AttributeError: MLukeTokenizer has no attribute additional_special_tokens_ids
2025-12-10 → 2026-02-03 no CI data for this test —
2026-02-10 → 2026-03-02 ❌ AttributeError: MLukeTokenizer has no attribute additional_special_tokens_ids
2026-03-03 → today (191 failures) ❌ AssertionError: wrong decode output

Root cause 1 — AttributeError from #40936

PR #40936 ("rm slow tokenizers", merged 2025-11-27) introduced extra_special_tokens as the v5 replacement and removed additional_special_tokens_ids from the base tokenizer. MLukeTokenizer still referenced self.additional_special_tokens_ids internally, causing the AttributeError.


Root cause 2 — different outputs after #44362

PR #44362 ("Fix MLukeTokenizer AttributeError post-v5 refactor", merged 2026-03-02) by @harshaljanjani fixed the AttributeError, allowing the test to run again — but we observed that the entity special tokens <ent> and <ent2> are now decoded as <s> and </s> respectively, which differs from the previous behavior.

Verified on the runner by checking out the commit before and after #44362:

Before #44362: AttributeError

After #44362 (current main):

test_entity_classification_no_padding_or_truncation:

ACTUAL:   '<s> Japanese is an<s> East Asian language<s> spoken by about 128 million people, primarily in Japan.</s>'
EXPECTED: '<s> Japanese is an<ent>East Asian language<ent>spoken by about 128 million people, primarily in Japan.</s>'

test_entity_pair_classification_no_padding_or_truncation:

ACTUAL:   '<s><s> Japanese<s> is an East Asian language spoken by about 128 million people, primarily in</s> Japan</s> .</s>'
EXPECTED: '<s><ent>Japanese<ent>is an East Asian language spoken by about 128 million people, primarily in<ent2>Japan<ent2>.</s>'

test_entity_span_classification_no_padding_or_truncation — unrelated pre-existing mismatch (Japan . vs Japan.), not caused by either PR.


@itazap — since this originates from the v5 tokenizer refactor in #40936, do you think we should fix the decode behavior so <ent>/<ent2> are correctly decoded again, or is the new output the expected behavior under v5 and we should update the golden strings in the tests?

@ydshieh
ydshieh requested a review from itazap August 28, 2026 09:46
@itazap itazap mentioned this pull request Aug 29, 2026

@itazap itazap left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#48408

this is the pr I had! I was waiting to see if the contributor opening the issue would open a fix, but did not

@tarekziade

Copy link
Copy Markdown
Collaborator

Closing as stale: all 6 mluke failures cleared on main on 2026-10-08 and the tests pass in the 2026-10-10 nightly.

Part of a clean-up of the serge PR queue: https://huggingface.co/spaces/huggingface/serge-pr-queue-review

@tarekziade tarekziade closed this Oct 11, 2026
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.

4 participants