renderers <-> v1: serializable RenderedTokens#1471
Merged
Conversation
PR #1414 surfaced the renderer's per-token prompt attribution onto every renderer-client rollout's TrajectoryStepTokens as a live RenderedTokens dataclass. v1 then ran State.assert_serializable over the full state, which json.dumps-walks state["trajectory"] and raises TypeError on the dataclass -- every v1 + renderer rollout failed at that gate. Convert via dataclasses.asdict at the parse_response_tokens sidecar handoff so the trajectory carries the dict form already. This also matches prime-rl's _step_sft_mask consumer contract (prompt_attribution: dict | None, accessed via ["message_indices"] / ["is_content"]) -- previously prime-rl would have raised on dict access against the dataclass too. Downstream consumers rehydrate via RenderedTokens(**d). multi_modal_data has the same latent JSON-gate problem but is intentionally left as a dataclass: save_utils._delta_intermediate_mm_data reads its fields via getattr, so converting it here would silently break the per-step delta optimization for multimodal trajectories. Separate fix. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Two regression tests that catch the PR #1414 v1 serialization bug deterministically (the previous tests only checked parse-boundary output, missing the actual v1 failure path): 1. test_parsed_prompt_attribution_survives_v1_assert_serializable Reproduces v1/runtime.py::submit_model_request exactly -- parse_response_tokens -> serializable() -> state["trajectory"] -> State.assert_serializable(). Pre-fix this raised TypeError on the RenderedTokens dataclass; post-fix it passes and the state round-trips through json.dumps cleanly. Verified A/B: reverting the parse-boundary asdict call makes this test fail with the exact pre-fix TypeError. 2. test_parsed_prompt_attribution_survives_orchestrator_msgpack Locks the verifiers -> orchestrator -> trainer wire contract: msgpack-pack the parsed step with the env_worker encoder (msgpack_encoder), unpack, and assert prime-rl's _step_sft_mask access pattern (prompt_attribution["message_indices"] / ["is_content"]) works on the result. Also checks RenderedTokens(**d) rehydrates losslessly for typed consumers. prime-rl's own test_trajectories.py uses _StubAttribution stubs that bypass the parse pipe, so it produced no A/B signal on either branch. These verifiers-side tests do. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
ApprovabilityVerdict: Needs human review Unable to check for correctness in b2b4cbf. You can customize Macroscope's approvability policy. Learn more. |
Inline the asdict conversion (2 lines) instead of a helper with a 12-line docstring; tighten the TrajectoryStepTokens comment to one line. Drop the msgpack regression test -- msgpack's encoder already had a dataclass fallback, so it passed on both branches and provided no A/B signal. Slim the v1 regression test docstring and drop the redundant json.loads(json.dumps(...)) assertion (assert_serializable already exercises that path). Restore the "carries" test to its original shape with a single dataclasses.asdict equality check. Net: +37 LOC instead of +181, same coverage. The v1 regression test still catches the original TypeError when the asdict line is reverted. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 7eba25d. Configure here.
dataclasses.asdict(RenderedTokens) recurses into the multi_modal_data field, whose mm_items[modality][i] holds raw HF processor tensors (pixel_values, image_grid_thw). Those aren't JSON-native, so the asdict conversion alone leaves State.assert_serializable still raising TypeError on multimodal rollouts — same failure mode as before the parse-boundary fix. Zero multi_modal_data via dataclasses.replace before asdict so the tensors never enter the dict (and aren't deep-copied along the way). The canonical mm payload is preserved at out["multi_modal_data"], which has its own non-JSON transport (msgpack with custom encoder on the wire; explicit trajectory allow-list past the JSONL save gate). No data loss. The existing carry test asserts equality against dataclasses.asdict(attribution) on a text-only attribution where multi_modal_data defaults to None — both sides hold a None for that key, so the test still passes unchanged. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
After the parse-boundary asdict conversion, TrajectoryStepTokens ["prompt_attribution"] is a plain dict mirroring the RenderedTokens field names — not the dataclass itself. One-word swap in the type comment: "sidecar" → "fields as a dict". Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
hallerite
approved these changes
May 27, 2026
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.

Description
Type of Change
Testing
uv run pytestlocally.Checklist
Note
Low Risk
Localized serialization change in token parsing with tests; only consumers expecting a RenderedTokens object on trajectory steps need to rehydrate from dict.
Overview
parse_response_tokensnow stores rendererprompt_attributionon trajectory step tokens as a JSON-safe dict (dataclasses.asdict), stripping nestedmulti_modal_datafrom that dict because tensors already live onout["multi_modal_data"]. The sidecar is still moved offResponseTokens(not duplicated), matchingmulti_modal_datapolicy.Callers that read
tokens["prompt_attribution"]get a dict instead of aRenderedTokensinstance; docs note rehydration viaRenderedTokens(**d). Tests assert dict equality, dict-shaped truncation fields, and a v1 regression thatState.assert_serializable()succeeds when attribution is on a trajectory step.Reviewed by Cursor Bugbot for commit b2b4cbf. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Serialize
prompt_attributionas a plain dict inparse_response_tokensparse_response_tokensin response_utils.py now converts aRenderedTokensdataclass to a plain dict viadataclasses.asdict, clearingmulti_modal_databefore conversion to avoid non-serializable values.RenderedTokens(**d). The originaltokens.prompt_attributionis still cleared (move semantics preserved).prompt_attributionpassesState.assert_serializable.RenderedTokensinstance fromprompt_attributionwill now receive a plain dict.Macroscope summarized b2b4cbf.