Skip to content

fix(tokenizer): do not prepend a BOS token the tokenizer does not have - #1634

Open
Chinmayrawat15 wants to merge 1 commit into
TransformerLensOrg:dev-4.xfrom
Chinmayrawat15:fix/tokenizer-bos-prepend
Open

fix(tokenizer): do not prepend a BOS token the tokenizer does not have#1634
Chinmayrawat15 wants to merge 1 commit into
TransformerLensOrg:dev-4.xfrom
Chinmayrawat15:fix/tokenizer-bos-prepend

Conversation

@Chinmayrawat15

Copy link
Copy Markdown

Description

Follow-up to #1628. That issue fixed the removal side (get_tokens_with_bos_removed, #1629) and deliberately left the prepend side alone, noting that the root-cause fix "needs the prepend path hardened first". This is that hardening.

get_input_with_manually_prepended_bos concatenates bos_token + input. With no BOS token that is None + str:

TypeError: unsupported operand type(s) for +: 'NoneType' and 'str'

With no BOS token there is nothing to prepend, so the helper now returns the input unchanged — the same shape of fix #1629 applied to the mirror-image helper.

How it is reached

Both normal paths are safe today. setup_tokenizer (_hf_format.py:376) and HookedTransformer.set_tokenizer (HookedTransformer.py:801) each backfill bos_token = eos_token, so a booted model never hands None to this helper.

The gap is the one #1628 identified. A bridge built via build_bridge_from_module(tokenizer=None) and given a tokenizer afterwards takes the initial-assignment branch at bridge_core.py:113, so configure_tokenizer — and with it that backfill — never runs, and bos_token stays None. The stale tokenizer_prepends_bos=True currently keeps to_tokens out of the prepend branch. Correcting that flag, which is exactly what fixing #1628's root cause does, routes to_tokens(prepend_bos=True) straight into this helper.

from transformers import AutoModelForCausalLM, AutoTokenizer
from transformer_lens.model_bridge.sources._bridge_builder import build_bridge_from_module

model = AutoModelForCausalLM.from_pretrained("roneneldan/TinyStories-1M")
bridge = build_bridge_from_module(
    model, architecture="GPTNeoForCausalLM", hf_config=model.config, tokenizer=None
)

# Initial assignment per bridge_core.py:113 -> configure_tokenizer is skipped.
bridge.tokenizer = AutoTokenizer.from_pretrained("google-bert/bert-base-cased")
assert bridge.tokenizer.bos_token is None

# What fixing the root cause would set, since detection correctly reports False here.
bridge.cfg.tokenizer_prepends_bos = False

bridge.to_tokens("hello world")

Before: TypeError: unsupported operand type(s) for +: 'NoneType' and 'str'.
After: [[101, 19082, 1362, 102]][CLS] hello world [SEP], with [CLS] intact.

So this unblocks the bridge_core.py:113 fix rather than competing with it.

Scope

The guard lives in the shared helper, so all three call sites are covered in one place: transformer_bridge.py:717, HookedTransformer.py:850, and remote_bridge.py:216. The HT-to-Bridge mirroring rule in AGENTS.md is satisfied by construction.

bos_token is now typed Optional[str]. That part is load-bearing, not cosmetic: the repo's beartype instrumentation rejects None against the old str annotation, so the runtime guard alone would still fail under test.

Related to #1628 — this does not close it, since #1629 covers the removal side. I asked on the issue whether this warranted its own issue or a PR referencing that one; opening it here as the latter, happy to move it if a maintainer prefers.

Based on dev-4.x to match #1628 and #1629. Happy to retarget to dev if that is preferred — the only difference is that Optional is not yet imported in tokenize_utils.py there.

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

Checklist:

  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • I have not rewritten tests relating to key interfaces which would affect backward compatibility

get_input_with_manually_prepended_bos concatenated bos_token + input
unconditionally, which is None + str for a tokenizer with no BOS token and
raises TypeError naming neither the tokenizer nor the flag that caused it.
With no BOS token there is nothing to prepend, so return the input unchanged.

Reached when a tokenizer skips setup_tokenizer's bos_token = eos_token
backfill, which is the initial-assignment branch at bridge_core.py:113, and
tokenizer_prepends_bos is then corrected to False. That is the follow-up
scoped out of TransformerLensOrg#1628; hardening it here unblocks the root-cause fix.

The guard sits in the shared helper, so all three call sites are covered:
transformer_bridge.py:717, HookedTransformer.py:850, remote_bridge.py:216.
bos_token widens to Optional[str] because beartype rejects None against the
old str annotation, so the runtime guard alone would still fail under test.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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