[train] feat: Multiple Linear Chains for Gateway Sessions#66
Conversation
…eway sessions - Introduced `enable_multiple_chains` configuration option in GatewayActorConfig to toggle multiple chain support. - Updated GatewayManager and GatewayActor to handle multiple chains. - Enhanced GatewaySession to manage active chains, including selection, materialization, and closure of chains. - Implemented chain state management with ChainState and MaterializedChain data classes. - Refactored message encoding and response handling to accommodate multiple chains. - Added methods for computing message prefix hashes and managing chain compatibility.
There was a problem hiding this comment.
Code Review
This pull request introduces support for managing multiple active linear trajectory chains per gateway session, allowing for branching conversations (e.g., subagents) within a single session. The changes span the framework configuration, gateway actor, and session management, backed by a comprehensive suite of unit tests. The feedback identifies a potential issue where the calculated remaining response budget could become negative if the generated response exceeds the budget, which may cause LLM backends to crash; clamping this value to a minimum of zero is recommended.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
…ro and update MessageCodec for dynamic system prompts
- Removed enable_multiple_chains parameter from session and gateway configurations. - Simplified GatewaySession to handle multiple chains without conditional checks. - Updated MessageCodec to derive system prompts from processors when available. - Consolidated trajectory handling to streamline the materialization of active chains. - Enhanced tests for multiple chains to ensure consistent behavior and coverage. - Removed legacy code related to single chain handling, improving clarity and maintainability.
- Introduced `enable_parallel_session_generation` and `ignore_cch_for_prefix_hash` flags in GatewayActorConfig to control session behavior. - Updated GatewaySession to handle parallel backend generation and manage inflight generations. - Enhanced tests to cover new functionality, including concurrent requests and validation of boolean flags. - Modified message prefix hash computation to optionally ignore CCH markers for prefix comparisons. - Ensured backward compatibility while implementing new features.
…n decode failures
…and_trajectory_assembly
…and related configurations
| f"got {type(value).__name__}" | ||
| ) | ||
| return value | ||
|
|
There was a problem hiding this comment.
consider inline this helper function. For example:
enable_parallel_session_generation = af_cfg.get("enable_parallel_session_generation", False)
if not isinstance(enable_parallel_session_generation, bool):
raise ValueError(...)
| # present: encode_incremental() slices processor-produced ids by this length on the | ||
| # default path, and a tokenizer-derived prefix would mis-slice the continuation delta | ||
| # whenever the processor adds BOS / multimodal-aware prefix tokens the bare tokenizer | ||
| # never emits. |
There was a problem hiding this comment.
This docstr is redundant imo. The code is self-explanatory enough
|
|
||
| omitted = codec.encode_incremental([{"role": "user", "content": "delta"}]) | ||
| assert _PlainTokenizer().decode(omitted) == "user:delta\nassistant:" | ||
|
|
There was a problem hiding this comment.
The first several test cases seem redundant, and some seem artificial (e.g., adding an extra bos in the processor, and monkeypatch). Consider removing them
| return await self._run_generation_prepared(payload, request_context, backend) | ||
|
|
||
| async with self.generation_lock: | ||
| return await self._run_generation_prepared(payload, request_context, backend) |
There was a problem hiding this comment.
Is generation_lock still necessary?
| materialized_trajectory = None | ||
| image_data = None | ||
| video_data = None | ||
| incoming_message_prefix_hashes = self._compute_message_prefix_hashes(messages) |
There was a problem hiding this comment.
consider inline this helper _compute_message_prefix_hashes
…d remove unused function
| candidates = [ | ||
| chain | ||
| for chain in self.active_chains | ||
| if self._is_chain_request_compatible( |
| ("enable_parallel_session_generation", 1), | ||
| ], | ||
| ) | ||
| def test_build_gateway_manager_rejects_non_bool_m2_flags(flag_name, bad_value): |
|
|
||
|
|
||
| @pytest.mark.asyncio | ||
| async def test_generate_sequences_preserves_sorted_trajectory_order_and_rewards_final_target(fake_tq): |
There was a problem hiding this comment.
Is this really needed? We already have test cases for session order, TQ order, and reward target
|
|
||
|
|
||
| @pytest.mark.asyncio | ||
| async def test_generate_sequences_preserves_normal_multiple_chain_order_and_rewards_final_main(fake_tq): |
There was a problem hiding this comment.
Is this really needed? focus on functionalities that are yet covered by other tests
|
|
||
|
|
||
| @pytest.mark.asyncio | ||
| async def test_score_trajectories_uses_last_finalized_trajectory_as_reward_target(): |
| enable_parallel_session_generation: bool = False, | ||
| ): | ||
| """Create an active session bound to a handle and model codec.""" | ||
| if type(enable_parallel_session_generation) is not bool: |
There was a problem hiding this comment.
this has already been validated in config.py
- Removed the `enable_parallel_session_generation` flag from session creation and related tests. - Updated session handling to support concurrent backend generation without the need for the flag. - Refactored tests to align with the new session management approach, ensuring proper handling of concurrent requests and chain reservations. - Enhanced the `GatewaySession` class to manage reserved chains and streamline the generation process. - Adjusted the codec and framework to accommodate the removal of the parallel generation flag while maintaining functionality.
| # Same-session requests overlap backend generation and commit in backend | ||
| # completion order. The framework currently scores session_trajectories[-1] | ||
| # and broadcasts that reward, so concurrent siblings share one reward target. | ||
| return await self._run_generation_prepared(payload, request_context, backend) |
| chain | ||
| for chain in self.active_chains | ||
| if chain.chain_id not in self.reserved_chain_ids | ||
| and self._is_chain_request_compatible( |
There was a problem hiding this comment.
this helper can be replaced by chain.active_tool_schemas == tools
| incoming_message_prefix_hashes=list(incoming_message_prefix_hashes), | ||
| logprobs_complete=logprobs_complete, | ||
| ) | ||
|
|
There was a problem hiding this comment.
move these public api to the beginning of this class, they are currently interrupting the code reading flow. For example:
init
run_generation
set_reward_info
finalize
abort
snapshot_state
_prepare_generation_inputs
_select_chain
...
| else: | ||
| buffer = self._copy_trajectory_buffer(selected_chain.buffer) | ||
| image_data, video_data = self._copy_chain_media(selected_chain) | ||
| logprobs_complete = selected_chain.logprobs_complete |
There was a problem hiding this comment.
Could we avoid carrying logprobs_complete through both ChainState and EncodedData? logprobs is already a session-level sampling invariant (rollout.calculate_log_probs is forwarded through the session sampling params), and it is not request-overridable by default.
When the effective sampling params request logprobs, a missing or length-mismatched output.log_probs seems like a backend contract violation and should fail fast. When logprobs are disabled, the trajectory can simply expose response_logprobs=None. The buffer can then keep the simpler invariant that response_logprobs is either empty or aligned with response_ids, without duplicating completeness state across prepare, commit, and materialization. Incremental masked tokens should use the same effective logprobs flag when appending 0.0.
| logprobs_complete=logprobs_complete, | ||
| ) | ||
|
|
||
| if incremental_messages: |
There was a problem hiding this comment.
This if-check can be removed. Relevant variables are initialized as empty:
incremental_ids = []
new_image_data = None
new_video_data = None
| sampling_params = self._codec.build_sampling_params(payload) | ||
| remaining_response_budget = ( | ||
| self._response_length - len(buffer.response_mask) if self._response_length is not None else None | ||
| max(0, self._response_length - len(buffer.response_mask)) if self._response_length is not None else None |
There was a problem hiding this comment.
this is not needed, since self._response_length < len(buffer.response_mask) is already gated by
if self._response_length is not None
and len(buffer.response_mask) + len(incremental_ids) >= self._response_length
| stop_reason=output.stop_reason, | ||
| ) | ||
| except Exception as e: | ||
| raise HTTPException(status_code=500, detail=f"{e.__class__.__name__}: {e}") from e |
There was a problem hiding this comment.
This exception handler is not quite needed. I will also add a global handler for internal errors in the Anthropic PR
| sampling_params=encoded.sampling_params, | ||
| image_data=encoded.image_data, | ||
| video_data=encoded.video_data, | ||
| image_data=self._copy_media_list(encoded.image_data), |
There was a problem hiding this comment.
Why do we need copying here?
…ains # Conflicts: # tests/uni_agent/gateway/test_gateway_actor_on_cpu.py # uni_agent/gateway/gateway.py # uni_agent/gateway/session/codec.py # uni_agent/gateway/session/protocol.py # uni_agent/gateway/session/session.py
| "messages": [_normalize_message(message) for message in messages], | ||
| "tools": tools, | ||
| "chat_template_kwargs": dict(chat_template_kwargs) if chat_template_kwargs else {}, | ||
| "chat_template_kwargs": {}, |
There was a problem hiding this comment.
This can be removed altogether, along with the chat_template_kwargs fields in TypedDict
Summary
Implements per-session multiple linear trajectory chains for
GatewaySession. Multiple chains are now the default and only session state model: live chains are stored inactive_chains, closed chains inmaterialized_chains, and the previous single-active-chain state is removed.This lets subagent/system splits, context compaction, repeated prompts, retries, and best-of-N-style sibling requests coexist without overwriting unrelated trajectory state. Same-session backend generation can overlap by default; request preparation, decoding, commits, and lifecycle transitions remain serialized by the session lock.
What's in this PR
Multiple-chain state and selection
(updated_seq, chain_id)as the deterministic tie-break for equally long compatible siblings.Lifecycle and trajectory materialization
Trajectoryschema.response_idsor emitted asNone.Default same-session backend concurrency
request_lock; reservations are released on success, failure, decode failure, cancellation, or late terminal-state rejection.session_idas the LLM client sticky-routing key. The LLM client still creates a unique server request ID for each generation call.Codec, sampling, and framework integration
tool_call_idin prefix identity, so rewritten committed IDs split safely.data.apply_chat_template_kwargsconsistently for full and incremental encoding; request-levelchat_template_kwargsis not part of the public request contract.Tests
CPU coverage spans:
Representative validation for the current PR head:
Design
The core rule is: reuse a chain's token buffer only when the incoming normalized message history proves it is a safe continuation of that chain; otherwise create a sibling chain and preserve the existing state.
Backend generation is the only phase allowed to overlap within a session. Reserving selected chain IDs prevents two concurrent requests from committing over the same chain. Requests that cannot select a reserved chain prepare independently and materialize as siblings. Completion order defines
order_seq, which also defines the final trajectory used by the existing session-level reward policy.Scope / deferrals
enable_multiple_chainsorenable_parallel_session_generationfeature flag; multiple chains and backend overlap are the default session model.ignore_cch_for_prefix_hashcompatibility mode; historicalcch=changes remain ordinary semantic prefix changes.Trajectory.extra_fields.Trajectoryor TransferQueue schema changes.chat_template_kwargs; chat-template configuration remains gateway/model scoped.