feat: client-side embedding training & multi-turn rollout support#247
Open
Yunnglin wants to merge 6 commits into
Open
feat: client-side embedding training & multi-turn rollout support#247Yunnglin wants to merge 6 commits into
Yunnglin wants to merge 6 commits into
Conversation
Expose the core-library embedding training (InfoNCE) and trainable multi-turn rollout capabilities through twinkle_client, and clean up the client_generator technical debt. - Remove client_tools/client_generator.py; convert affected client modules (dataloader/dataset/processor/model/sampler) to hand-maintained sources by stripping their AUTO-GENERATED headers. - Extract bridge-token stitching into the shared pure function twinkle_agentic/rollout/bridge.py::extend_with_bridge; MultiTurnRollout now calls it instead of duplicating the logic. - Add ClientMultiTurnRollout (src/twinkle_client/rollout/) driving HTTP sampling via vLLMSampler, reusing ToolManager and extend_with_bridge. - Enhance MockSampler with opt-in multi-turn knobs (new_input_feature, configurable stop_reason / tool_call_text / tool_call_turns) for local CPU-only multi-turn e2e; backward compatible by default. - Add embedding e2e scaffolding + local mock protocol-link validation and GPU-gated numeric/SP-CP tests; add multi-turn property/unit tests, local mock e2e, and GPU-gated alignment / Gateway-logprobs tests. - Document client embedding call sequence and the Tinker Gateway indirect multi-turn approach with its capability boundaries. Local CPU suite: 98 passed, GPU-gated cases skip cleanly.
Contributor
There was a problem hiding this comment.
Code Review
This pull request introduces client-side multi-turn agentic rollout orchestration (ClientMultiTurnRollout) and adds documentation and tests for embedding training and multi-turn tool calling. Feedback focuses on ensuring proper JSON serialization of PyTorch tensors and template outputs in the HTTP client path, as well as removing an unused variable in the mock sampler.
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.
…-multiturn-support
…nt cookbook examples - megatron set_loss: inject _dp_group into loss.process_group so InfonceLoss all-gather stays within DP ranks under PP (prevents NCCL deadlock on earlier pipeline stages that never enter the loss). Verified: pp=2/dp=2 local + server. - dedup _to_plain: remove duplicated helper from twinkle_agentic/rollout/multi_turn.py, import from bridge.py (single source). Also remove unused numpy import. - cookbook: add client embedding training example (cookbook/client/twinkle/self_host/embedding.py) and client multi-turn rollout GRPO example (multi_turn_rollout.py). Both verified against live megatron server (pp=2/dp=2). - tests/config: add no-sampler megatron config for faster embedding-only server boots. - processor/base.py, twinkle_client/rollout, sampler, tests: branch feature updates for embedding + multi-turn client support.
The tuple was defined but never referenced anywhere; the constructor already declares stop_reason / tool_call_text / tool_call_turns as keyword-only args, so Python's native argument binding handles them. Addresses PR #247 review.
- docs(zh/en): Twinkle 客户端文档新增「可训练多轮 Rollout(ClientMultiTurnRollout)」章节;Embedding 文档新增「通过 Twinkle Client 训练」章节 - docs(zh): Tinker 兼容客户端文档移除多轮 rollout 与 embedding 内容(Tinker 协议不支持),精简实现细节叙述,交叉引用改为 markdown 链接 - test: 移除冗余的 GPU/e2e 及重复的多轮测试,保留 CPU 单元测试 (test_bridge / test_client_multi_turn_rollout);删除随之孤立的 e2e 服务配置 - chore: 清理注释/docstring 中残留的 spec 工作流编号
Yunnglin
marked this pull request as ready for review
July 20, 2026 10:26
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.
Summary
Exposes the core-library embedding training (InfoNCE) and trainable multi-turn rollout capabilities through
twinkle_client, and removes theclient_generatortechnical debt. Thetinkerclient is untouched — only documented for its Gateway-based, generation-only multi-turn path.Changes
Technical debt cleanup
client_tools/client_generator.pyand convert the affected client modules (dataloader/dataset/processor/model/sampler) to hand-maintained sources by stripping theirAUTO-GENERATEDheaders (otherwise unchanged).Shared bridge function
twinkle_agentic/rollout/bridge.py::extend_with_bridge.MultiTurnRolloutnow calls it instead of keeping its own copy, so the core-lib and client paths cannot drift.Client multi-turn rollout
ClientMultiTurnRollout(src/twinkle_client/rollout/) driving HTTP sampling viavLLMSampler.sample(), reusingToolManagerandextend_with_bridge.vLLMSamplernow JSON-safes pre-encodedInputFeatureinputs before the POST.Embedding training
processor/base.py: rebuild aposition_idsreference for last-valid-token pooling when HF omitsposition_idsin a standard padded batch.megatron.pyset_loss: inject the DP group intoloss.process_groupso the InfoNCE in-batch-negative all-gather stays within DP ranks under PP (prevents an NCCL deadlock on earlier pipeline stages that never enter the loss).MockSampler multi-turn knobs
new_input_featureplus configurablestop_reason/tool_call_text/tool_call_turnsso a local CPU-only multi-turn e2e can run without a GPU. Backward compatible by default.Tests
Docs & cookbook
Verification