tool-call: refactor common chat / tool-call api (+ tests / fixes)

[ochafik]

Refactoring of chat / tool-call logic (follow up to #11016) along the lines of @ggerganov's suggestions (ref):

• Move all common_chat_* from common.* to common/chat.*
• Moved minja headers to common/minja/*, now only imported from chat.cpp
• Removed json.hpp dep from chat.hpp (and test-chat.cpp only uses it to normalize arguments during comparisons)
  • Added common_chat_tool struct + refined common_chat_msg to accept multipart content, tool name, tool call id to stop depending on json
• common_chat_apply_template becomes common_chat_templates_apply
• Added common helpers to convert oaicompat JSON to / from common_chat_tool / common_chat_msg vectors
  • Templated signature for oaicompat converters so they can be called w/ strings... or preparsed json (from utils.hpp, to avoid needless round-trip of json dump / parse)

Also some fixes:

• Fixed test-chat-template + cleaned up double-bos avoidance hack introduced in tool-call: allow --chat-template chatml w/ --jinja, default to chatml upon parsing issue, avoid double bos #11616 (was preventing insertion of bos/eos tags inside the template)
• Fixed & tested --jinja w/o tool call w/ grammar or json_schema
• Added tests for array message.content
• Fixed clang-tidy lints in chat files

cc/ @bandoti

TODO:

• Add some missing tests (json<->tools converter, array message.content)
• Wire some logs / debug info I've commented out
• Ensure no regression in legacy array message.content

[bandoti]

Working through the MCP implementation I added a similar structure to this one. It would be good to normalize the two. Here's what I ended up adding:

    struct tool {
        struct param {
            std::string name;
            std::string type;
            std::string description;
        };
        std::string tool_name;
        std::string tool_description;
        std::vector<param> params;
        std::vector<std::string> required_params;
    };

I think instead of having parameters as an opaque JSON object it would be worthwhile to expand it out into a vector of parameters (or something similar) so that the entire structure is accessible without JSON, making the conversion between MCP to openai-compat more seamless.

[bandoti]

However, it is not that difficult to convert to the JSON parameters from the structure above if the change would require significant alterations in the template logic in this PR. I am happy to make the change in the #11556 😊

[ochafik]

We should not assume parameters is an object schema, it could technically be a {"type": "string"}. Llama 3.2 seems to handle object, array and string for the ipython tool for instance

see message loop of llama 3.2 's template

{%- for message in messages %}
    {%- if not (message.role == 'ipython' or message.role == 'tool' or 'tool_calls' in message) %}
        {{- '<|start_header_id|>' + message['role'] + '<|end_header_id|>\n\n'+ message['content'] | trim + '<|eot_id|>' }}
    {%- elif 'tool_calls' in message %}
        {%- if not message.tool_calls|length == 1 %}
            {{- raise_exception("This model only supports single tool-calls at once!") }}
        {%- endif %}
        {%- set tool_call = message.tool_calls[0].function %}
        {{- '<|start_header_id|>assistant<|end_header_id|>\n\n' -}}
        {{- '{"name": "' + tool_call.name + '", ' }}
        {{- '"parameters": ' }}
        {{- tool_call.arguments | tojson }}
        {{- "}" }}
        {{- "<|eot_id|>" }}
    {%- elif message.role == "tool" or message.role == "ipython" %}
        {{- "<|start_header_id|>ipython<|end_header_id|>\n\n" }}
        {%- if message.content is mapping or message.content is iterable %}
            {{- message.content | tojson }}
        {%- else %}
            {{- message.content }}
        {%- endif %}
        {{- "<|eot_id|>" }}
    {%- endif %}
{%- endfor %}

Not sure either we should describe any possible JSON schema as a fixed data structure, although that would be quite fun and might guide improvements of the native json schema conversion coverage.

[ngxson]

I'm wondering, is it possible (after this refactoring) to finally remove this std::unique_ptr?

[ochafik]

Good point, probably, I'll look into it as follow up

[ngxson]

This check is redundant, right?

Suggested change

	} else if (!content.is_null()) {
	} else {

[ochafik]

It's not: content can be set to string, array or null (or else that branch throws)

[ochafik]

(oai returns content: null when there are tool_calls)

[ngxson]

Not 100% sure about this, but at least for deepseek models, the reasoning_content will be ignored for input messages

[ochafik]

Indeed, and actually even if we were, say, to turn reasoning_content back to <think> tags inside the content, their template would explicitly filter it out:

        {%- set content = message['content'] -%}
        {%- if '</think>' in content -%}
            {%- set content = content.split('</think>')[-1] -%}
        {%- endif -%}
        {{- '<｜Assistant｜>' + content + '<｜end▁of▁sentence｜>' -}}

(but anyone can write a template that does differently, for instance if we wanted to avoid invalidating the KV cache)