Skip to content

Test refactor PR2: instruct tokenizer tests, request builders, golden registry (OSS-108) - #280

Open
juliendenize wants to merge 2 commits into
test-refactor-pr1-foundationfrom
test-refactor-pr2-instruct
Open

Test refactor PR2: instruct tokenizer tests, request builders, golden registry (OSS-108)#280
juliendenize wants to merge 2 commits into
test-refactor-pr1-foundationfrom
test-refactor-pr2-instruct

Conversation

@juliendenize

@juliendenize juliendenize commented Jul 24, 2026

Copy link
Copy Markdown
Collaborator

PR2 — Instruct + FIM tokenizer tests (OSS-108)

Second PR of the test-suite refactor. Stacked on test-refactor-pr1-foundation (#279); base is that branch, not main. When PR1 merges this rebases onto main. Mostly additive on PR1's files: it modifies quick_vocab's body in tests/utils/tokenizers.py (adds errors="replace", needed by the SPM sentinel merge) and removes now-superseded helpers from tests/integrations/chat_templates/helpers.py. An earlier claim of "AST diff: modified=[], removed=[]" was wrong — that check compared top-level names, not bodies, so it could not detect what it claimed to rule out. All legacy tests stay green; no src/ behaviour changes.

1. Shared request builders — tests/utils/requests/

Mirrors src/mistral_common/protocol/.

  • instruct.py — single source of truth for ChatCompletionRequest conversations. Folds in the integrations fixtures_data.py (REQUEST_* + get_conversations) plus deterministic dummy image/audio inputs, math_interpreter_tools(), and the registry request set.
  • fim.pyFIMRequest builders.
  • tests/integrations/chat_templates/ rewired to consume the shared module (fixtures_data.py shows as a rename). Collected integration ids unchanged from PR1: 271.

Message-literal dedup: 36 distinct messages=[...] literals → 20; the 9 that were duplicated across 22 sites → 0.

2. One golden mechanism

tests/utils/registry.py + one file per tokenizer key:

tests/data/expected/<protocol>/<key>.json      # request + text + token_ids per scenario
tests/data/expected/<protocol>/<key>/*.npz     # processed image arrays, when present

Each record colocates the input with both outputs, so a golden is reviewable — you see the request that produced the ids instead of a bare 314-int list:

{
  "abcd_multi_turn": {
    "request":   { "messages": [ ... ] },
    "text":      "<s>\u2581[INST]\u2581a[/INST]\u2581b</s>...",
    "token_ids": [1, 733, 16289, ...]
  }
}

The stored request is asserted, not decorative — TestInstructGoldens/TestFimGoldens compare each builder's output against it across all 135 scenarios. A decorative copy would drift from the builder while looking authoritative; asserting it makes a builder edit fail loudly and forces a look at whether the tokens should have moved too. The Python builders stay the source of truth for inputs (they are shared with the chat_templates tests and give type checking).

Base64 image/audio payloads are redacted from the stored request: PNG encoding is not byte-stable across zlib versions, so the same pixels serialize differently per Python build and the golden would pass on some CI runners and fail on others (it did, on 3.14). Nothing is lost — processed images stay pinned by the .npz goldens and audio by the token ids.

An earlier iteration had two parallel golden shapes — a per-key matrix and a flat scenarios.json. That was pure duplication, and the matrix structurally could not express v11–v15 because model settings were not part of the key. Collapsed into a single Scenario dataclass + SCENARIOS + PROTOCOLS + SUPPORTED_PROTOCOLS + PROTOCOL_ENCODERS, with model settings baked into the key (v13_think, v15_think, v15_aud, v15_img_think). One key = one tokenizer = one golden directory. All 23 pre-existing scenario values verified byte-identical through the collapse.

  • Token ids and decoded text → JSON, exact equality.
  • Processed image arrays → compressed .npz, compared with np.testing.assert_allclose (arrays stored; floats never hashed).
  • Loaded via session-scoped fixtures in the root conftest.
  • Reviewed-only regeneration script tests/utils/regenerate_registry.py, never run by the suite, so goldens are never auto-overwritten.

135 scenarios across 17 instruct keys and 6 FIM keys.

3. Expected outputs live in JSON, not in test files

Every full-request encode expectation is now golden-backed, and every golden assertion checks both token ids and text — one-sided assertions and inline copies of existing goldens are gone.

before after
inline encode-output literals 10 1
f-string rendered templates 9 0
inline literals duplicating an existing golden 9 0
one-sided golden assertions 15 0

The remaining literals are sub-unit outputs — encode_system_message then decode, and
encode_assistant_message under continue_message — which the request-keyed registry cannot express
without a second mechanism. The encode_assistant_message case previously reused a full-request
golden via [1:] to strip the bos; that coupling was removed, since a prefix-token change would have
silently shifted the slice while the test kept passing.

Migration safety, enforced for every value moved:

  • Each expectation was captured before the edit, then compared byte-for-byte against the regenerated golden. Regeneration inherited values; it never established them.
  • Golden regeneration proven additive only — no pre-existing value was mutated or removed.
  • The pre-migration test files were re-run against unchanged src/ to prove the old expectations still hold, which pins the f-string cases that cannot be compared as literals.

4. Migrated tests

tests/tokenizers/ — v1/v2/v3 (+multimodal), v7, v11, v13, v15, and FIM. tests/tokenizers/conftest.py owns every tokenizer fixture. Adds an executable version × protocol coverage test (TestProtocolCoverage) and TestRefusalScenarios.

FIM inputs widened 2 → 7 (prompt_suffix, prompt_only, empty_suffix, empty_prompt, newline_suffix, leading_space_suffix, unicode_suffix); FIM tests 17 → 155 (70 in test_fim.py plus 85 registry cases).

Legacy tests/test_tokenize_v*.py, test_tokenizer_v*.py, test_fim_tokenizer.py left untouched (retired in a later PR).

5. v2 has no FIM support

Shipped SPM v2 lacks [SUFFIX]/[PREFIX]piece_to_id returns 0 (<unk>) silently, with no exception — and tekken v2 never existed. Rather than invent a refusal assertion for behaviour that does not refuse, SUPPORTED_PROTOCOLS[v2] = {"instruct"} with SILENT_UNSUPPORTED_PROTOCOLS = {(v2, "fim")}. The synthetic tekken v2 FIM key and its goldens were removed.

6. Naming: the sentinel is a SentencePiece artifact

_encode_infilling prefixes and slices [2:] to strip SentencePiece's synthetic . InstructTokenizerV2 is generic over both backends, so tekken inherits a hack it does not need. Test infrastructure renamed accordingly (FIM_SENTINEL*SPM_INFILLING_SENTINEL*, build_fim_capable_* → SPM-artifact naming) so later readers are not misled.

The pinned invariant is now decode(_encode_infilling(x)) == x across whitespace/newline/unicode/punctuation/digit suffixes; len(encode(sentinel)) == 2 is kept as a secondary guard on the hardcoded slice. A tekken characterisation test records the current (lossless but suboptimal) behaviour.

Verification

  • ruff check . + ruff format --check . clean.
  • mypy . clean (142 files).
  • pytest --cov=mistral_common tests/ --ignore=tests/integrations (the gated CI command) → 88.84%, gate 88, exits 0.
  • pytest tests/2583 passed, 17 skipped; whole-suite branch coverage 93.8%.
  • pytest tests/integrations/271 collected ids, unchanged from PR1.
  • pytest --doctest-modules ./src → 52 passed.

Known SPM FIM behaviour, characterised not changed

Two long-standing SentencePiece behaviours are pinned by this suite rather than treated as defects, since mistral-common is the reference tokenizer and its output defines what the models were trained on:

  • _encode_infilling strips one leading space from a FIM suffix (deliberate, per its docstring). tests/tokenizers/test_fim.py asserts the round-trip for tab/newline/unicode/punctuation/digit suffixes and excludes a leading space, with a comment recording why.
  • SPM v2 has no [SUFFIX]/[PREFIX] pieces and encode_fim emits <unk> without raising. Recorded as SUPPORTED_PROTOCOLS[v2] = {"instruct"} plus SILENT_UNSUPPORTED_PROTOCOLS = {(v2, "fim")}.

Linear: OSS-108

@juliendenize
juliendenize force-pushed the test-refactor-pr2-instruct branch from eea5a3e to e52c3a3 Compare July 27, 2026 10:11
@juliendenize
juliendenize force-pushed the test-refactor-pr1-foundation branch from cde98be to b41fb82 Compare July 27, 2026 10:39
@juliendenize
juliendenize force-pushed the test-refactor-pr2-instruct branch from e52c3a3 to c135aa4 Compare July 27, 2026 12:31
@juliendenize
juliendenize force-pushed the test-refactor-pr1-foundation branch from b41fb82 to 1e17969 Compare July 27, 2026 12:45
@juliendenize
juliendenize force-pushed the test-refactor-pr2-instruct branch 3 times, most recently from 2a2ffa5 to 164924f Compare July 27, 2026 19:09
@juliendenize
juliendenize force-pushed the test-refactor-pr1-foundation branch from 1e17969 to 575b378 Compare July 27, 2026 19:39
@juliendenize
juliendenize force-pushed the test-refactor-pr2-instruct branch 2 times, most recently from 4f59961 to b2bc57b Compare July 27, 2026 22:09
@juliendenize
juliendenize force-pushed the test-refactor-pr1-foundation branch 2 times, most recently from 83656e6 to 74c9708 Compare July 27, 2026 22:58
@juliendenize
juliendenize force-pushed the test-refactor-pr2-instruct branch 5 times, most recently from f666452 to 585677d Compare July 28, 2026 12:35
@juliendenize
juliendenize force-pushed the test-refactor-pr1-foundation branch from 74c9708 to 195993c Compare July 28, 2026 14:04
@juliendenize
juliendenize force-pushed the test-refactor-pr2-instruct branch from 585677d to d1cc455 Compare July 28, 2026 14:11
@juliendenize
juliendenize force-pushed the test-refactor-pr1-foundation branch from 195993c to 36253d6 Compare July 28, 2026 16:01
@juliendenize
juliendenize force-pushed the test-refactor-pr2-instruct branch from d1cc455 to 48364b5 Compare July 28, 2026 16:11
@juliendenize
juliendenize force-pushed the test-refactor-pr1-foundation branch from 36253d6 to 687fffd Compare July 28, 2026 18:06
@juliendenize
juliendenize force-pushed the test-refactor-pr2-instruct branch from 48364b5 to bacab9a Compare July 28, 2026 18:20
@juliendenize
juliendenize force-pushed the test-refactor-pr1-foundation branch from 687fffd to d729a36 Compare July 28, 2026 21:44
@juliendenize
juliendenize force-pushed the test-refactor-pr2-instruct branch from bacab9a to 89a7bc7 Compare July 29, 2026 07:48
- tests/utils: versions (TestConfig, version lists, config_id) and tokenizer
  builders (quick_vocab, get_special_tokens, build_tekkenizer, load_sentencepiece,
  write_tekkenizer_model)
- root tests/conftest.py session fixtures (dummy_v3_tekkenizer, spm_v7, make_tekkenizer)
- structured library tests in tests/tokenizers (tekken, sentencepiece) at 100% per-file
- util self-tests in tests/utils_tests
- route legacy test_tekken.py and integrations TestConfig/helpers through shared utils
- coverage config + CI per-file gate and diff-cover
@juliendenize
juliendenize force-pushed the test-refactor-pr1-foundation branch from d729a36 to 39ec63d Compare July 29, 2026 09:47
@juliendenize
juliendenize force-pushed the test-refactor-pr2-instruct branch 3 times, most recently from af42b43 to e0df115 Compare July 29, 2026 12:13
@juliendenize
juliendenize force-pushed the test-refactor-pr2-instruct branch 4 times, most recently from 448d48a to d3ca4a4 Compare July 30, 2026 17:17
@juliendenize
juliendenize force-pushed the test-refactor-pr2-instruct branch from d3ca4a4 to 49d2706 Compare July 30, 2026 17:31
@juliendenize
juliendenize force-pushed the test-refactor-pr1-foundation branch 3 times, most recently from 6dce84e to 748fbe0 Compare August 6, 2026 15:58
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