Skip to content

refactor: replace bypass_render_with_parser with TinkerChatTemplateParser#489

Draft
listar2000 wants to merge 3 commits into
mainfrom
refactor/tinker-chat-template-parser
Draft

refactor: replace bypass_render_with_parser with TinkerChatTemplateParser#489
listar2000 wants to merge 3 commits into
mainfrom
refactor/tinker-chat-template-parser

Conversation

@listar2000

Copy link
Copy Markdown
Collaborator

Summary

  • Introduce TinkerChatTemplateParser (rllm/parser/tinker_parser.py) wrapping tinker-cookbook Renderer as a ChatTemplateParser
  • Refactor TinkerEngine to always use TinkerChatTemplateParser, removing the bypass_render_with_parser flag and all conditional branching
  • Remove dead utility functions (_convert_openai_messages, _prepare_messages_with_tools, _parse_tinker_message) from tinker_engine.py
  • Replace engine-level tests with tests for actual utility functions still in use, and add tool-related tests to test_tinker_parser.py
  • Remove bypass_render_with_parser from YAML configs and shell scripts

Depends on: #488 (first commit is shared — diff will clean up after #488 merges)

Test plan

  • pytest tests/engine/test_tinker_engine.py — 19 tests pass
  • pytest tests/parser/test_tinker_parser.py — 25 tests pass (14 existing + 11 new)
  • End-to-end GPU test with Tinker backend

🤖 Generated with Claude Code

listar2000 and others added 3 commits April 5, 2026 22:02
…rollout/

The experimental rollout engines (RolloutEngine, ModelOutput, TinkerEngine,
VerlEngine, Completer, TITOCompleter, types) are now the canonical
implementations in rllm/engine/rollout/. The experimental versions were
strict supersets of the non-experimental ones, with additional support for
TITO (token-in-token-out), weight version tracking, and training pipeline
integration.

Key changes:
- Move all engine classes and types from rllm/experimental/rollout/ to
  rllm/engine/rollout/ (the canonical location)
- Add backward-compat shim in rllm/experimental/rollout/__init__.py that
  re-exports from the canonical location with a DeprecationWarning
- Update all 24+ consumer imports from rllm.experimental.rollout to
  rllm.engine.rollout
- Standardize on .is_validation (not .validate) as the attribute name
  across all rollout engine consumers
- OpenAIEngine and FireworksEngine remain in rllm/engine/rollout/ and
  inherit the enhanced base RolloutEngine automatically

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…rser

Introduce TinkerChatTemplateParser from the refactor-chat-parser branch,
which wraps a tinker-cookbook Renderer as a ChatTemplateParser. This
eliminates the dual-path bypass_render_with_parser flag on TinkerEngine:

- TinkerEngine now always creates a TinkerChatTemplateParser(renderer)
  as its chat_parser, removing the conditional ChatTemplateParser vs
  raw-renderer branching in __init__, _get_model_response, and
  assemble_model_output
- Remove bypass_render_with_parser parameter from TinkerEngine and all
  callers (configs, examples, tests, shell scripts)
- Add TinkerChatTemplateParser to rllm/parser/ exports
- Add test_tinker_parser.py with comprehensive parser tests

The TinkerChatTemplateParser provides:
- parse() for prompt string generation via the renderer
- build_prompt() returning ModelInput (preserves image chunks for VLM)
- parse_completion() for structured output extraction (content, reasoning,
  tool_calls) via renderer.parse_response()
- tokenize_and_mask() / tokenize_and_mask_cumulative() for SFT training
- Full tool calling support via renderer.create_conversation_prefix_with_tools

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
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