Repository navigation
feat: report media placeholder positions in logical prompts; drop mm_token_type_id_map - #211
Draft
faresobeid wants to merge 1 commit into
Draft
faresobeid wants to merge 1 commit into
faresobeid wants to merge 1 commit into
Conversation
…token_type_id_map Renderers that place images record PlaceholderRange where they emitted each placeholder in every mode, including process_multimodal=False (one logical token per image). Bridges carry previous placeholders forward instead of counting placeholder token ids. generate() attaches each raw media part's placeholder range to its content part, so the engine can expand media only there. The token-id based mm_token_type_id_map is removed; is_multimodal is an explicit class attribute.
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
<|image_pad|>is an ordinary token id the policy can sample (about 1 in 300k tokens on Qwen3.5-4B VLM RL). Tool or user text containing the literal string also tokenizes to it. Several consumers identify images by that token id:MultimodalRenderer.mm_token_type_id_map, which verifiers uses to buildmm_token_type_ids;previous_ids.count(image_pad);A stray placeholder token then becomes an extra "image".
Design
PlaceholderRange(offset, length)at emission, in every mode. Withprocess_multimodal=Falsethis is one logical token per image, for qwen3_vl, qwen3.5/3.6/3.8, kimi_k25, GLM-5.3 Flash and Muse Glimmer. Nemotron 3.5 records its<image>tokens: its template places images as<image>text, and HF treats a typed<image>the same way.emit_specialorplaceholder, never text.previous_multi_modal_data.mm_placeholdersin both modes, and seedadd_vision_idcounts from them instead of counting token ids.generate(process_multimodal=False)attaches each raw media part's range to its content part ("placeholder": {"offset", "length"}). It raises if the counts differ. A prebuilt prompt with no multi-modal data sends no ranges, which keeps today's behavior.mm_token_type_id_mapis removed from the protocol and from every renderer. This is a clean break, with no shim.is_multimodal = Trueis now an explicit class attribute, because the runtime protocol check needed a discriminating member. Org-wide code search found consumers only in verifiers (updated in the linked PR) and inPrimeIntellect-ai/internal-prime-rl-elie-test, a test fork, which is flagged.Validation
tests/test_placeholders.py, Qwen3.5-0.8B and Qwen3-VL-4B:<|image_pad|>in the previous completion and a literal one in the next user text produce exactly 2 placeholders, each right after<|vision_start|>;tests/test_client.py test_placeholders test_glm53_flash test_muse_glimmer test_nemotron35_vlm test_multimodal test_bridge test_qwen38 test_is_content test_incremental: 940 passed, 48 skipped. Main gives 938 passed on the same files minus the new one.ruff check/ruff format --check: clean.Stack / merge order
deps/renderersto this branch)The verifiers PR needs this renderers change. Bump the verifiers
renderersfloor after a release.