Skip to content

feat: client-side embedding training & multi-turn rollout support#247

Open
Yunnglin wants to merge 6 commits into
mainfrom
feat/client-embedding-multiturn-support
Open

feat: client-side embedding training & multi-turn rollout support#247
Yunnglin wants to merge 6 commits into
mainfrom
feat/client-embedding-multiturn-support

Conversation

@Yunnglin

@Yunnglin Yunnglin commented Jul 14, 2026

Copy link
Copy Markdown
Collaborator

Summary

Exposes the core-library embedding training (InfoNCE) and trainable multi-turn rollout capabilities through twinkle_client, and removes the client_generator technical debt. The tinker client is untouched — only documented for its Gateway-based, generation-only multi-turn path.

Changes

Technical debt cleanup

  • Remove client_tools/client_generator.py and convert the affected client modules (dataloader / dataset / processor / model / sampler) to hand-maintained sources by stripping their AUTO-GENERATED headers (otherwise unchanged).

Shared bridge function

  • Extract bridge-token stitching into the shared pure function twinkle_agentic/rollout/bridge.py::extend_with_bridge. MultiTurnRollout now calls it instead of keeping its own copy, so the core-lib and client paths cannot drift.

Client multi-turn rollout

  • Add ClientMultiTurnRollout (src/twinkle_client/rollout/) driving HTTP sampling via vLLMSampler.sample(), reusing ToolManager and extend_with_bridge. vLLMSampler now JSON-safes pre-encoded InputFeature inputs before the POST.

Embedding training

  • processor/base.py: rebuild a position_ids reference for last-valid-token pooling when HF omits position_ids in a standard padded batch.
  • megatron.py set_loss: inject the DP group into loss.process_group so 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

  • Add opt-in new_input_feature plus configurable stop_reason / tool_call_text / tool_call_turns so a local CPU-only multi-turn e2e can run without a GPU. Backward compatible by default.

Tests

  • Embedding: CPU-only mock protocol/link validation; GPU-gated real-model loss finiteness, embedding-patch auto-rollback, bare-library parity, pos_sim upward trend, and SP/CP consistency.
  • Multi-turn: CPU-only property/unit tests, a local mock-sampler HTTP e2e, and GPU-gated behavioural-alignment + Gateway no-logprobs tests.

Docs & cookbook

  • Client embedding training example and client multi-turn rollout (GRPO) example, both verified against a live megatron server (pp=2/dp=2).
  • Document the client embedding call sequence and the Tinker Gateway indirect multi-turn approach with its capability boundaries.

Verification

  • Local CPU suite passes; GPU-gated cases skip cleanly on machines without a GPU.

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.

@gemini-code-assist gemini-code-assist Bot left a comment

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.

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.

Comment thread src/twinkle_agentic/rollout/bridge.py
Comment thread src/twinkle_client/rollout/multi_turn.py
Comment thread src/twinkle_client/rollout/multi_turn.py
Comment thread src/twinkle/server/sampler/backends/mock_sampler.py Outdated
Yunnglin added 5 commits July 15, 2026 14:59
…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
Yunnglin marked this pull request as ready for review July 20, 2026 10:26
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