Skip to content

chat_template.hpp improvements #16

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
ochafik merged 3 commits into from
Dec 30, 2024
Merged

chat_template.hpp improvements #16

ochafik merged 3 commits into from
Dec 30, 2024

Conversation

@ochafik
Copy link
Contributor

@ochafik ochafik commented Dec 29, 2024

  • Robust system message support detection
  • Robust arguments-as-object detection
  • Json fix in python expectations logic

@ochafik ochafik marked this pull request as ready for review December 30, 2024 01:01
@ochafik ochafik merged commit 58f0ca6 into main Dec 30, 2024
7 checks passed
@ochafik ochafik deleted the tool-call branch December 30, 2024 01:02
ochafik referenced this pull request in ochafik/minja Nov 2, 2025
@ochafik
Merge pull request #16 from google/tool-call
158ba16 
chat_template.hpp improvements
CISC pushed a commit to aldehir/minja that referenced this pull request Dec 20, 2025
@ochafik @claude
Fix CI: Windows encoding, sanitizers, cppcheck, and test issues (goog…
aa530c2 
…le#15)

## Summary

This PR fixes multiple CI issues to get all builds passing on Windows,
macOS, and Linux.

## Changes

### Workflow Fixes
- **Branch trigger**: Changed from `master` to `main`
- **Sanitizer exclusions**: Added exclusions for MSVC ARM64 builds
(address/thread/undefined sanitizers not supported)

### Build Fixes
- **Disabled clang-tidy for address sanitizer builds**: Avoids GCC
`-Wno-maybe-uninitialized` flag incompatibility with clang-tidy
- **Disabled cppcheck on Windows**: Fixes `std.cfg` not found error
- **Added `-Wa,-mbig-obj` for MinGW Debug builds**: Fixes COFF section
limit exceeded error (>65535 sections)

### Python/Encoding Fixes
- **Added `PYTHONIOENCODING=utf-8`** to Configure and Test steps for
Windows Unicode support
- **Added `encoding='utf-8'`** to all file operations in
`fetch_templates_and_goldens.py`
- **Added `newline='\n'`** to force Unix line endings in generated files

### Test Fixes
- **Normalize actual template output**: Apply `normalize_newlines()` to
actual output in tests
- **Windows blank line workaround**: Added `collapse_blank_lines()` for
Windows due to a known issue where C++ minja outputs fewer newlines than
Python Jinja2 (tracked in google#16)

## Related Issues
- google#16 - Windows: C++ minja outputs fewer newlines than Python Jinja2

## Test Plan
- [x] All 28 CI jobs pass (Windows, macOS, Linux with various sanitizers
and build types)

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---------

Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>
aldehir added a commit to aldehir/minja that referenced this pull request Jan 1, 2026
@aldehir
Replace std::regex_replace when stripping leading/trailing space (goo…
c5a97c1 
…gle#21)

Fixes google#16

Looks like `std::regex_replace()` does not respect anchors, at least not
in Windows.

**Minimal reproducing example (Microsoft (R) C/C++ Optimizing Compiler
Version 19.44.35221 for x64)**
```cpp
#include <iostream>
#include <regex>

int main() {
    auto text = "\nthis contains\n\nmultiple\nline\n\nbreaks\n\n";

    std::cout << "== Leading ==\n";
    auto bad = std::regex_replace(text, std::regex(R"(^\s)"), "");
    std::cout << "Bad: " << bad << "\n";
    std::cout << "==\n";

    std::string good = text;
    good.erase(0, good.find_first_not_of(" \t\r\n"));
    std::cout << "Good: " << good << "\n";
    std::cout << "==\n";

    std::cout << "== Trailing ==\n";
    bad = std::regex_replace(text, std::regex(R"(\s$)"), "");
    std::cout << "Bad: " << bad << "\n";
    std::cout << "==\n";

    good = text;
    auto pos = good.find_last_not_of(" \t\n\r\f\v");
    good.resize(pos == std::string::npos ? 0 : pos + 1);
    std::cout << "Good: " << good << "\n";
    std::cout << "==\n";
}
```
```
== Leading ==
Bad: this contains
multiple
line
breaks

==
Good: this contains

multiple
line

breaks


==
== Trailing ==
Bad: 
this contains
multiple
line
breaks
==
Good: 
this contains

multiple
line

breaks
==
```

Passes all the tests, excluding the gated templates I don't have.

```
$ ctest -R test-supported-template -j 24
...
100% tests passed, 0 tests failed out of 220

Total Test time (real) =  32.38 sec

The following tests did not run:
         11 - test-supported-template-google-gemma-7b-it (Skipped)
         12 - test-supported-template-CohereForAI-c4ai-command-r-plus (Skipped)
         14 - test-supported-template-meta-llama-Llama-3.2-3B-Instruct (Skipped)
         15 - test-supported-template-meta-llama-Llama-3.1-8B-Instruct (Skipped)
         16 - test-supported-template-meta-llama-Meta-Llama-3-8B-Instruct (Skipped)
         18 - test-supported-template-meta-llama-Llama-2-7b-chat-hf (Skipped)
         54 - test-supported-template-CohereForAI-aya-expanse-8b (Skipped)
         55 - test-supported-template-databricks-dbrx-instruct (Skipped)
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@ochafik