Add chat template kwargs to C API - #1102
Open
jennyf19 wants to merge 7 commits into
Open
Conversation
Preserve the existing API while allowing typed JSON context values for model-specific chat templates. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: a9f5bae1-1b88-4311-8fac-6224769e0400
|
Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run. |
Run the repository C API build and C++ tests on Jenny's fork while upstream Azure Pipelines await maintainer authorization. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: a9f5bae1-1b88-4311-8fac-6224769e0400
Contributor
There was a problem hiding this comment.
Pull request overview
This PR extends the tokenizer C API to accept additional, typed JSON context values (“template kwargs”) when applying chat templates, while preserving the legacy OrtxApplyChatTemplate entry point.
Changes:
- Adds a new C API function
OrtxApplyChatTemplateWithOptionsthat acceptstemplate_kwargsas a JSON object string. - Updates chat template rendering to merge
template_kwargsinto the Minja context while preventing overrides of core context keys (messages,tools,add_generation_prompt). - Adds new unit tests covering typed kwargs, core-context override prevention, and invalid kwargs handling.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| test/pp_api_test/test_tokenizer_chat.cc | Adds tests for typed template kwargs behavior, override-prevention, and invalid JSON errors. |
| shared/api/tokenizer_impl.h | Extends TokenizerImpl::ApplyChatTemplate signature to accept template_kwargs. |
| shared/api/chat_template.cc | Implements parsing/merging of template_kwargs into the template context with core-key precedence rules. |
| shared/api/c_api_tokenizer.cc | Introduces OrtxApplyChatTemplateWithOptions and routes legacy API through it. |
| include/ortx_tokenizer.h | Exposes the new API in the public header with documentation. |
Suppressed comments (1)
shared/api/c_api_tokenizer.cc:486
- OrtxApplyChatTemplateWithOptions can dereference a null tokenizer when tokenizer == nullptr but template_str != nullptr. The current guard only rejects the case where both tokenizer and template_str are null, but token_ptr is always derived from tokenizer, so this can crash instead of returning kOrtxErrorInvalidArgument.
if (tokenizer == nullptr && template_str == nullptr) {
ReturnableStatus::last_error_message_ = "both tokenizer and template_str are null, no template to apply";
return kOrtxErrorInvalidArgument;
}
if (input == nullptr || output == nullptr) {
ReturnableStatus::last_error_message_ = "Invalid argument";
return kOrtxErrorInvalidArgument;
}
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
added 5 commits
August 17, 2026 14:10
Exercise the same typed false value without relying on the unsupported is false predicate in an explicit template. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: a9f5bae1-1b88-4311-8fac-6224769e0400
The repository C API build and full C++ test suite passed in fork run 32069739093. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: a9f5bae1-1b88-4311-8fac-6224769e0400
Return an invalid-argument error instead of dereferencing a null tokenizer, with a focused regression test and temporary fork validation. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: a9f5bae1-1b88-4311-8fac-6224769e0400
Keep the public contract strict: callers must pass a JSON object or null, never an empty string. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: a9f5bae1-1b88-4311-8fac-6224769e0400
The strict kwargs validation and null-tokenizer regression pass the full C API C++ suite in run 32070849855. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: a9f5bae1-1b88-4311-8fac-6224769e0400
This was referenced Aug 18, 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.
Preserve the existing API while allowing typed JSON context values for model-specific chat templates.
Co-authored-by: Copilot 223556219+Copilot@users.noreply.github.com