Skip to content

Keep multimodal content inline in v1 rollouts - #2417

Open
eligotts wants to merge 8 commits into
mainfrom
feat/v1-inline-mm-vllm
Open

Keep multimodal content inline in v1 rollouts#2417
eligotts wants to merge 8 commits into
mainfrom
feat/v1-inline-mm-vllm

Conversation

@eligotts

@eligotts eligotts commented Aug 20, 2026

Copy link
Copy Markdown
Contributor

Summary

  • keep base64 image content in canonical v1 message nodes instead of carrying processed multimodal sidecars
  • use renderer logical prompt tokens only for bridge extension, then store the effective expanded prompt tail returned by vLLM
  • preserve the renderer-stamped multimodal token map from main and derive token types from the effective vLLM sequence
  • remove multimodal kwargs and tensor metadata bookkeeping from graph, trace, and training client state
  • preserve strict prefix validation and response masking across multimodal tool turns

Stack

Validation

  • uv run --locked pytest -q tests/v1 passed (live API-key tests skipped)
  • focused graph, renderer-client, multimodal-types, and types suite (58 passed)
  • pre-push Markdown, Ruff check/format, and ty hooks passed

Note

Keep multimodal content inline in v1 rollouts using renderer_token_ids

  • Replaces per-node multi_modal_data with a parallel renderer_token_ids stream on MessageNode and TurnTokens, so multimodal content stays inline in the token flow rather than being passed as a separate blob.
  • _commit_turn in graph.py now validates that vLLM prompt tokens exactly extend the stored rollout prefix in both logical and renderer token spaces, raising ValueError on mismatch. Nodes persist both token_ids and renderer_token_ids.
  • TrainClient.get_response in train.py bridges over the turn tail only, derives routed_experts_prompt_start from turn.path_len, and passes process_multimodal to render/generate. Renderers without supports_process_multimodal error when multimodal processing is disabled.
  • Branch in trace.py no longer exposes aggregated multi_modal_data; mm_token_type_ids is computed without node-level multimodal data.
  • Bumps aiohttp dependency to >=3.14.1 in pyproject.toml.
  • Behavioral Change: removes multi_modal_data from TurnTokens, MessageNode, and Branch; any out-of-tree code reading these fields will break. _commit_turn now raises ValueError('vLLM prompt tokens do not exactly extend the stored rollout prefix') on prefix mismatch where previously it did not.

Macroscope summarized 9070ddd.


Note

Medium Risk
Changes rollout token identity, multimodal render/generate wiring, and strict prefix validation on turn commit—misaligned renderer/vLLM sequences will now fail rollouts instead of silently reusing stale prefixes.

Overview
Multimodal v1 rollouts no longer carry processed image tensors on nodes or turns. TurnTokens.multi_modal_data, MessageNode.multi_modal_data, and Branch.multi_modal_data are removed; images stay in message content while training uses the expanded vLLM prompt in token_ids and a separate logical renderer stream via renderer_prompt_ids / renderer_token_ids (with logical_ids and None meaning “same as expanded”).

Train inference bridging is updated to render the full prompt but bridge from previous_renderer_token_ids() and the incremental wire_tail, passing process_multimodal (and process_multimodal=False on render/bridge when images are present). Renderers without supports_process_multimodal fail fast on image prompts. TurnTokens from generate now includes renderer_prompt_ids.

Turn commit and prefix reuse compare both expanded and logical prefixes; when renderer_prompt_ids is supplied, a non-extending vLLM prefix raises ValueError. Span attribution and renderer_path_len follow the logical stream when expanded and logical prompts differ (e.g. duplicated image placeholder tokens). mm_token_type_ids is derived from the stored token map on the effective sequence, not merged node tensors.

Adds a graph test for canonical expanded tokens vs bridge logical tokens and bumps aiohttp to >=3.14.1.

Reviewed by Cursor Bugbot for commit 9070ddd. Bugbot is set up for automated code reviews on this repo. Configure here.

Latest main sync (2026-08-28)

  • Merged main through 5f24fb843, including the newer typed ACP/RLM changes.
  • Validation: focused graph, renderer-client, multimodal-message, and client-type tests passed; all pre-push Markdownlint, Ruff, formatting, and ty checks passed.

Processing flag follow-up

  • Replaced the redundant raw_multimodal caller flag with process_multimodal; render, bridge, and generate now share one processing switch.
  • The focused test suite and all pre-push checks passed.

Naming cleanup

  • Uses explicit has_images and process_multimodal locals and checks supports_process_multimodal.
  • The focused suite and all pre-push checks passed.

Explicit logical token state

  • MessageNode.renderer_token_ids is now list[int] | None: None means identical to expanded token_ids, while [] remains a real empty logical slice.
  • All per-node truthiness fallbacks use logical_ids; previous_token_ids() is now previous_renderer_token_ids().
  • The focused graph test, Ruff, formatting, ty, and pre-push checks passed.

Runtime dependency alignment

  • Raised the aiohttp runtime lower bound to >=3.14.1, matching the web.RequestKey API used by the interception server.
  • The focused suite and all pre-push checks passed.

@eligotts
eligotts marked this pull request as ready for review August 28, 2026 22:49
@macroscopeapp

macroscopeapp Bot commented Aug 28, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Not approved

Macroscope's review found this PR not approvable — This PR changes established multimodal inference and rollout-training behavior across the client, token graph, and trace models rather than adding an isolated option. It also carries an unresolved compatibility risk in the inference call path, so the runtime contract should be reviewed before merging.

Not approved because:

  • 1 blocking correctness issue found at or above your repo's Minimum Blocking Severity

Adjust the Minimum Blocking Severity for this repo — including turning it Off — in Settings. You can add or adjust custom eligibility rules. Learn more.

prompt_attribution=prompt_attribution,
tools=wire_tools,
sampling_params=sampling_params,
process_multimodal=process_multimodal,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟠 High clients/train.py:435

Every TrainClient.get_response call raises TypeError before inference because the pinned renderers==0.1.10 generate() does not accept process_multimodal, including text-only requests. Remove this unsupported keyword (or upgrade the renderer dependency) so requests reach the inference endpoint.

-                    process_multimodal=process_multimodal,
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @verifiers/v1/clients/train.py around line 435:

Every `TrainClient.get_response` call raises `TypeError` before inference because the pinned `renderers==0.1.10` `generate()` does not accept `process_multimodal`, including text-only requests. Remove this unsupported keyword (or upgrade the renderer dependency) so requests reach the inference endpoint.

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