Reverse Chat Template Fallback Order

[sayanshaw24]

This change now ensures we try to parse the chat template with Minja first, and then falls back on the native implementation.

[hanbitmyths]

Do we need a clean-up for some specialized handling in case that minja works, so we keep codes clean?

[sayanshaw24]

Do we need a clean-up for some specialized handling in case that minja works, so we keep codes clean?

You mean like this: https://github.com/microsoft/onnxruntime-extensions/pull/963/files#diff-660d41852d9740562f27b085a10a08b04b635625f45367300c30206c907b3f5aR816, or something more?

If Minja works, we get a chat template root from minja::Parser::parse that we can then call render on. If not, we attempt the native impl and finally return a kOrtxErrorInvalidArgument if that fails as well.

This PR simply switches fallback order plus any respective changes needed and ensures the status is returned correctly, it shouldn't make the code messier.

[hanbitmyths]

Do we need a clean-up for some specialized handling in case that minja works, so we keep codes clean?

You mean like this: https://github.com/microsoft/onnxruntime-extensions/pull/963/files#diff-660d41852d9740562f27b085a10a08b04b635625f45367300c30206c907b3f5aR816, or something more?

If Minja works, we get a chat template root from minja::Parser::parse that we can then call render on. If not, we attempt the native impl and finally return a kOrtxErrorInvalidArgument if that fails as well.

This PR simply switches fallback order plus any respective changes needed and ensures the status is returned correctly, it shouldn't make the code messier.

What I meant is to remove a model specific code in the file like TokenizerImpl::Phi3_5ChatTemplate if minja works fine with Phi3.5 chat template. We don't need to do it in this PR, but I'd like to know this makes sense and we can clean up unnecessary codes.

[sayanshaw24]

Do we need a clean-up for some specialized handling in case that minja works, so we keep codes clean?

You mean like this: https://github.com/microsoft/onnxruntime-extensions/pull/963/files#diff-660d41852d9740562f27b085a10a08b04b635625f45367300c30206c907b3f5aR816, or something more?
If Minja works, we get a chat template root from minja::Parser::parse that we can then call render on. If not, we attempt the native impl and finally return a kOrtxErrorInvalidArgument if that fails as well.
This PR simply switches fallback order plus any respective changes needed and ensures the status is returned correctly, it shouldn't make the code messier.

What I meant is to remove a model specific code in the file like TokenizerImpl::Phi3_5ChatTemplate if minja works fine with Phi3.5 chat template. We don't need to do it in this PR, but I'd like to know this makes sense and we can clean up unnecessary codes.

Oh I see, sorry did not understand what you meant before - yes absolutely we should, but I want to implement multimodal support and run some tests before we do, in case we need some of the TokenizerImpl functions to add on to model-agnostic multimodal support. Like you said, can do that in a separate PR once multimodal work is sorted.

[hanbitmyths]

Do we have any test cases to catch exception due to minja failure?