[gshtrasberg]

Overview

deepseek_formatter_for is unconditionally called, so models like inkling end up falling back to jinja

Key Changes

There is already support for InklingFormatter, but it is only used in native_formatter_for, which would fall back to deepseek_formatter_for in case the model is not "inkling_mm_model"

Validation

Running an inkling model.
Before the change Jinja template is used
After the change native inkling formatter is used, which emits the compact tool JAON

Summary by CodeRabbit

• Updates
  • Prompt formatting now considers both model type and display name when selecting a native formatter. Existing Kimi-specific formatting and the Hugging Face template fallback remain unchanged. Models handled by the native formatter selection may receive different prompt formatting as a result.

[copy-pr-bot]

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

[coderabbitai]

Walkthrough

The prompt formatter selection now uses native_formatter_for instead of deepseek_formatter_for. The existing Kimi selector and Hugging Face template fallback remain.

Changes

Prompt formatter selection

Layer / File(s)	Summary
Use the native formatter selector lib/llm/src/preprocessor/prompt.rs	The renderer imports native_formatter_for and uses it in prompt_formatter_from_mdc instead of deepseek_formatter_for.

Estimated code review effort: 1 (Trivial) | ~3 minutes

Merge Risk: Low · up to 419ec

Inkling models now use the native formatter, which is the stated goal of the PR. Deployments that supply a custom chat template for Inkling will no longer have it applied. Owners should confirm that this is acceptable before merge.

Pre-merge checks: 5

Check name	Status	Explanation
Title check		The title clearly and concisely identifies the main change: handling the Inkling formatter.
Description check		The description includes Overview, Key Changes, and Validation sections. It explains the Inkling formatter issue and the observed behavior before and after the change. The Issues and Ongoing Work sect…
Docstring Coverage		Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat…
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.

Full details: Description check

Explanation

The description includes Overview, Key Changes, and Validation sections. It explains the Inkling formatter issue and the observed behavior before and after the change. The Issues and Ongoing Work section and checklist are missing, but the description is otherwise sufficiently complete.

Full details: Docstring Coverage

Explanation

Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 1 functions across 1 files.

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Tip

🎯 Customize your risk assessments

Guide how CodeRabbit assesses merge risk and security architecture risk. Highlight sensitive changes and explain your security boundaries using Merge Risk Instructions and Security Risk Instructions in your review settings. Security architecture risk assessments require the Advanced plan or above.

✅ Automatic approvals, with your rules

Let CodeRabbit approve pull requests once required threads are resolved, pre-merge checks pass, and your risk limits are met. Enable Automatic approvals in review settings, with overrides for teams or file paths.

CodeRabbit Verify

• Run CodeRabbit Verify to test the changes in this pull request.

[coderabbitai]

Additional comments: 1

lib/llm/src/preprocessor/prompt.rs-418-418: 1

418-418: Major · Functional Correctness · Quick win

Unverified finding
Verification ran but could not confirm this finding. It is shown for review, not as a verified issue.

Preserve explicit custom templates for Inkling.

native_formatter_for returns an Inkling formatter from model_type alone. This return occurs before mdc.chat_template_file is read. The model-card loader accepts an optional custom template, so an Inkling deployment with a custom template now bypasses that template and uses the native formatter instead. (raw.githubusercontent.com)

Skip native selection when mdc.chat_template_file is custom. Then let the existing fallback load the custom artifact.

Proposed fix

+    let has_custom_template = mdc
+        .chat_template_file
+        .as_ref()
+        .is_some_and(|artifact| artifact.is_custom());
+
-    if let Some(formatter) = native_formatter_for(&model_type_lower, &display_name_lower) {
-        return Ok(formatter);
+    if !has_custom_template {
+        if let Some(formatter) = native_formatter_for(&model_type_lower, &display_name_lower) {
+            return Ok(formatter);
+        }
     }

As per coding guidelines, preserve explicit prompt semantics through the preprocessing path.

Source: Coding guidelines

Review info

Run configuration

• Source mode: Auto
• Repository: ai-dynamo/dynamo
• Branch: main
• File: .coderabbit.yaml
• Commit: 3aafbef
• Layers: Repository YAML
• Review profile: CHILL
• Plan: Enterprise
• Run ID: eeb6ce11-45ed-4623-a1c6-3ddadee1b841

Commits

Reviewing files that changed from the base of the PR and between 3aafbef and 419ec25.

Files selected for processing: 1

• lib/llm/src/preprocessor/prompt.rs

Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 11 remain after this review.

[github-actions]

👋 Hi gshtrasberg! Thank you for contributing to ai-dynamo/dynamo.

Just a reminder: The NVIDIA Test Github Validation CI runs an essential subset of the testing framework to quickly catch errors.Your PR reviewers may elect to test the changes comprehensively before approving your changes.

🚀

[furionw]

Thank you. LGTM. Asking @KrishnanPrash for a 2nd look

---

native_formatter_for wraps the existing DeepSeek selector and adds one Inkling case.

Its logic is essentially:

fn native_formatter_for(model_type, display_name) -> Option<PromptFormatter> {
    if model_type == "inkling_mm_model" {
        return Some(InklingFormatter);
    }

    deepseek_formatter_for(model_type, display_name)
}

https://github.com/ai-dynamo/frontend-crates/blob/5ecc47ff825c86608a6c5c06c72489f89c5c8c0e/renderer/src/template.rs#L96-L110

[KrishnanPrash]

AFTER (append to lib/llm/tests/preprocessor.rs):
#[test]
fn inkling_model_type_selects_native_formatter() {
    let model_dir = tempfile::tempdir().unwrap();
    let source = std::path::Path::new("tests/data/sample-models/mock-llama-3.1-8b-instruct");
    for filename in [
        "config.json",
        "generation_config.json",
        "tokenizer_config.json",
        "tokenizer.json",
    ] {
        std::fs::copy(source.join(filename), model_dir.path().join(filename)).unwrap();
    }
    let config_path = model_dir.path().join("config.json");
    let mut config: serde_json::Value =
        serde_json::from_slice(&std::fs::read(&config_path).unwrap()).unwrap();
    config["model_type"] = serde_json::json!("inkling_mm_model");
    std::fs::write(&config_path, serde_json::to_vec(&config).unwrap()).unwrap();

    let mdc = ModelDeploymentCard::load_from_disk(model_dir.path(), None).unwrap();
    let PromptFormatter::OAI(formatter) = prompt_formatter_from_mdc(&mdc).unwrap();
    let request: NvCreateChatCompletionRequest = serde_json::from_value(serde_json::json!({
        "model": "inkling",
        "messages": [{"role": "user", "content": "hi"}],
        "tools": [{"type": "function", "function": {
            "name": "get_weather",
            "parameters": {"type": "object", "properties": {"city": {"type": "string"}}}
        }}]
    }))
    .unwrap();

    assert_eq!(
        formatter.render(&request).unwrap(),
        "<|message_system|>tool_declare<|content_xml|>[{\"description\":\"\",\"name\":\"get_weather\",\"parameters\":{\"properties\":{\"city\":{\"type\":\"string\"}},\"type\":\"object\"},\"type\":\"function\"}]<|end_message|><|message_system|><|content_text|>Thinking effort level: 0.9<|end_message|><|message_user|><|content_text|>hi<|end_message|><|message_model|>"
    );
}

Nothing in lib/llm checks which formatter this call returns for a native-formatter model_type, so a revert of this line would go unnoticed. This test passes at this head and fails with the old deepseek_formatter_for call, where the mock card renders its Llama template.

[KrishnanPrash]

if let Some(formatter) = native_formatter_for(&model_type_lower, &display_name_lower) {
    return Ok(formatter);
}

Follow-up idea: this returns before chat_template_file is read, so --custom-jinja-template no longer applies to Inkling, though its help promises precedence (DeepSeek and Kimi K3 already do this). Worth a warning in a separate PR? Happy to leave it as-is.