fix(responses): preserve chat template arguments

[glamr-agent]

Summary

• Accept chat_template_args on POST /v1/responses, including chat_template_kwargs as an alias, and preserve the map when the request is converted to the internal chat-completion format.
• Apply the same nested chat_template rejection used by POST /v1/chat/completions before dispatching the request.
• Honor force_nonempty_content when a non-streaming reasoning-only response is aggregated, so reasoning is moved into content when requested.
• Requests that omit chat-template arguments keep the existing behavior.

Closes #12284

Validation

• git diff --check
• No tests were run for this revision. The HTTP path, a live engine, and end-to-end force_nonempty_content behavior were not exercised.

[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.

[github-actions]

👋 Hi glamr-agent! 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.

🚀

[glamr-agent]

Automated evidence record — validation complete

Validation status: complete

Evidence summary: [1/1 validated]

AI review assessment (advisory — not a merge authorization): sound. An AI agent
judged the change logically sound on the recorded evidence. CI and a human reviewer hold
merge authority. The review raised two nit findings, neither blocking: (1) /v1/responses
does not run ValidateRequest, so the nested-chat_template guard does not fire on this
path — a gap that pre-dates the change but which this change makes reachable, and which
overlaps open PR #12809; (2) a superseded evidence row printed a green banner over a
failing test result, an artifact of 2>/dev/null | tail -3 under set -e; that row is
not the deciding row and the failure was never reproduced across ~55 further attempts.

Validation result: complete — pass. The deciding run is a set -euo pipefail script
with no pipes and no stderr suppression, at SHA 04a6d670ffc83488b841e557cea7afc613559cbb
with a clean tree. All four sections of recipe 02-rust-cargo-check ran: cargo fmt --all --check and git diff --check both exit 0; cargo check --workspace --all-targets
and the python-bindings manifest both finished; cargo clippy --workspace --all-targets --no-deps -- -D warnings finished with no error under the repo-pinned toolchain 1.96.1;
cargo test -p dynamo-llm --lib gave 1976 passed; 0 failed; 5 ignored, with all four
new tests ok, and --test test_chat_template_args gave 3 passed; 0 failed.

Evidence audit: complete [1/1 validated] — the evidence table is grounded in recorded
runs. The tests were shown non-tautological by applying only the new tests to unmodified
base production code, where 3 of 4 go red at named panic sites — covering both halves of
the fix, the serde declaration and the forwarding. The clippy toolchain question was
resolved empirically: the sandbox RUSTUP_TOOLCHAIN=1.93.1 overrides the repo's
rust-toolchain.toml pin of 1.96.1, and the resulting unknown lint: clippy::manual_option_zip was reproduced at unmodified base, proving pre-existence rather
than inferring it. Re-running under the declared toolchain used byte-identical flags.

Scope of the claim: nothing was exercised over HTTP or against a running engine. The
evidence shows the field binds from JSON, survives the TryFrom, and lands on the chat
request — not that a live /v1/responses request renders a different prompt end to end.

Evidence [1/1 validated]

Generated from validation/registry.jsonl — do not edit by hand.

Recipe	Status	Command	Evidence	Note
02-rust-cargo-check	validated	bash /tmp/recipe02-final.sh	validation/logs/2026-08-07T23-13-07.157Z-bash-94f7.log

[glamr-agent]

plan.md

# Plan — Preserve `chat_template_args` when converting Responses requests to Chat Completions

Route: implementation
Template: model-parser-investigation
Engine: vllm

## User intent

GitHub issue [ai-dynamo/dynamo#12284](https://github.com/ai-dynamo/dynamo/issues/12284)
asks that a `/v1/responses` request carrying `chat_template_args` keep those
arguments when Dynamo converts it into its internal Chat Completions request.
Today the conversion throws them away. The issue gives the exact synthetic
repro:

```json
{
  "model": "dummy-model",
  "input": "hello",
  "chat_template_args": { "enable_thinking": true }
}
```

and the exact expectation: the converted chat request must still carry
`chat_template_args.enable_thinking == true`. This matters because for several
model families the reasoning and tool-formatting behavior is driven entirely
through chat-template arguments; a Responses client currently has no way to
reach that switch, while a Chat Completions client does. The issue was
confirmed by a maintainer (`michaelfeil`, 2026-07-28: "yes please, i con
confirm this is a correct issue") and is still open.

So the caller wants a code change: make the Responses path accept the field and
forward it, plus a regression test shaped like the issue's repro.

## Non-goals

- Not touching the Chat Completions side. `NvCreateChatCompletionRequest`
  already has `chat_template_args`, already accepts the `chat_template_kwargs`
  alias, and already validates it. Nothing there needs to move.
- Not adding `chat_template_args` to the upstream `CreateResponse` type. That
  type lives in the published `dynamo-protocols` crate (pinned `= "5.1.0"`), is
  not vendored in this repo, and cannot be edited from here. See Discovery.
- Not routing the field through `NvExt`. `NvExt` is `deny_unknown_fields` and
  is a different, already-crowded extension surface; the issue asks for a
  root-level request field mirroring the Chat one.
- Not changing `normalize_reasoning_template_args` / `thinking` handling, and
  not implementing `thinking_token_budget` — that is someone else's open PR
  (#12624), described below.
- Not fixing the separate question of whether the *converted* chat request gets
  run through `ValidateRequest` on the Responses path. That is open PR #12809's
  subject. Our change must not depend on it, and must not pre-empt it.
- No engine work, no runtime/serving behavior change, no GPU claim.

## Discovery

Everything below was read in the checkout at
`/home/sandbox/workspace/wi-20260807T212155Z-12284/repo`, at
`4f32fc683 fix: Add guided decoding json schema validation (#12795)`.

### 1. The exact conversion site

`lib/llm/src/protocols/openai/responses/mod.rs`

- Line 673: `impl TryFrom<NvCreateResponse> for NvCreateChatCompletionRequest`.
  This is the single conversion function.
- Lines 800–811 are the returned struct literal, and **line 806 is the
  defect**:

  ```rust
              common: Default::default(),
              nvext: resp.nvext,
              chat_template_args: None,      // <-- line 806
              thinking: None,
  ```

  The field is hard-coded to `None` rather than derived from the incoming
  request.

`lib/llm/src/protocols/unified.rs:193` —
`impl TryFrom<NvCreateResponse> for UnifiedRequest` builds a
`ResponsesContext` and then delegates with
`let inner: NvCreateChatCompletionRequest = req.try_into()?;`. That is the same
`TryFrom`, so fixing the one site above also fixes the HTTP path; there is no
second conversion to keep in sync. `ResponsesContext` (unified.rs:111) carries
only `previous_response_id`, `truncation`, `reasoning`, `include`, `store` — it
is not a place to stash template args.

`lib/llm/src/http/service/openai.rs` — the live wiring:
`handler_responses` (2902) → `responses()` (2976); inside `responses()`,
line 3065 converts to `UnifiedRequest`, line 3083 unwraps to the chat request,
line 3084 calls `normalize_chat_reasoning_template_args(&mut chat_request)`.
That normalizer *writes into* `chat_template_args`, so once the conversion
populates the map, the existing downstream machinery already consumes it.

`lib/llm/src/protocols/unified.rs:494` — the `OAIChatLikeRequest` impl exposes
`fn chat_template_args(&self) -> Option<&HashMap<String, serde_json::Value>>`
returning `self.inner.chat_template_args.as_ref()`. This is what the prompt
renderer reads, and it is why forwarding at line 806 is sufficient to reach the
template.

### 2. Does the Responses type already carry the field? **No — and this is the load-bearing finding.**

`NvCreateResponse` is declared at `lib/llm/src/protocols/openai/responses/mod.rs:48-66`
and has exactly **two** fields:

```rust
#[derive(ToSchema, Serialize, Deserialize, Validate, Debug, Clone)]
pub struct NvCreateResponse {
    #[serde(flatten)]
    #[schema(value_type = Object)]
    pub inner: dynamo_protocols::types::responses::CreateResponse,

    #[serde(skip_serializing_if = "Option::is_none")]
    #[schema(value_type = Object)]
    pub nvext: Option<NvExt>,
}
```

There is no `chat_template_args`. The flattened `inner` does not supply one
either: `dynamo_protocols::types::responses::CreateResponse` is an external
published crate (root `Cargo.toml` pins `dynamo-protocols` at `= "5.1.0"`; the
source is not vendored in this repo). I read the published crate source
(`dynamo-protocols-5.1.0/src/types/responses/mod.rs`, `pub struct CreateResponse`
at line 403, 27 fields) out-of-tree and confirmed it has no `chat_template_args`
and no `chat_template_kwargs`.

**This materially changes the size of the change.** It is not a one-line swap
of `None` for `resp.chat_template_args`. It is two parts:

1. add the field to `NvCreateResponse` so the JSON in the issue deserializes at
   all (today it is silently ignored — the struct is *not*
   `deny_unknown_fields`, so the request parses and the argument vanishes
   without any error), and
2. forward it at line 806.

The Chat-side declaration to mirror is `lib/llm/src/protocols/openai/chat_completions.rs:100-107`:

```rust
    #[serde(
        default,
        skip_serializing_if = "Option::is_none",
        alias = "chat_template_kwargs"
    )]
    pub chat_template_args: Option<std::collections::HashMap<String, serde_json::Value>>,
```

### 3. Blast radius of adding a field

`grep -rn "NvCreateResponse {" --include=*.rs .` → **30** literal construction
sites, in exactly two files:
`lib/llm/src/protocols/openai/responses/mod.rs` (~28, all after the
`#[cfg(test)]` at line 1196, e.g. the `make_response_with_input` helper at 1212)
and `lib/llm/src/http/service/openai.rs` (the `make_base_request()` helper at
4684–4692, inside the `#[cfg(test)]` that starts at 4327). **Every one is test
code.** No production caller constructs `NvCreateResponse` by literal; it only
ever arrives from `serde`. That keeps the change contained.

`NvCreateResponse` currently derives `ToSchema, Serialize, Deserialize,
Validate, Debug, Clone` — notably **not** `Default`, which is why the existing
test literals all spell both fields out.

### 4. Existing tests — what is already covered, so we do not duplicate

`lib/llm/tests/test_chat_template_args.rs` has three tests:
`test_chat_template_args`, `test_chat_template_kwargs_alias`,
`test_both_fields_fails`. All three deserialize
**`NvCreateChatCompletionRequest`** only. **None of them touches the Responses
type or the Responses→Chat conversion.** So a Responses-path regression test is
genuinely new coverage, not a restatement of existing coverage.

Related validation code: `lib/llm/src/protocols/openai/validate.rs:818-829`
defines `validate_chat_template_args`, which rejects a nested `chat_template`
key. Its only caller is `NvCreateChatCompletionRequest::validate()`
(`chat_completions.rs:539`). There is **no `impl ValidateRequest for
NvCreateResponse`**, so whether that guard fires on the Responses path depends
on whether the converted chat request is validated — which is precisely the gap
open PR #12809 addresses. We should not try to solve that here; our field
simply flows into the same slot that #12809 will start validating.

### 5. PR and history survey

`git log --since=2026-05-01 -- lib/llm/src/protocols/openai/responses/mod.rs lib/llm/src/protocols/openai/chat_completions.rs lib/llm/src/protocols/unified.rs`
shows steady churn on these files but nothing that adds `chat_template_args` to
the Responses path. Confirmed directly against the working tree: line 806 is
still `chat_template_args: None,` and the struct at 48–66 still has two fields.

Host searches run with `gh pr list --repo ai-dynamo/dynamo --state all --search …`
over `chat_template_args`, `responses chat template`, `12284`,
`enable_thinking`, `preserve chat_template_args`, plus
`--state open --search responses` and `--author rmccorm4`. Results:

- **PR #12477 — `fix(responses): preserve chat_template_args during Responses
  to Chat Completions conversion`**, by `rmccorm4` (Ryan McCormick), branch
  `rmccormick/responses-preserve-chat-template-args`, base `main`, body "Fixes
  #12284". Opened 2026-07-31T06:15:21Z, **CLOSED 2026-07-31T07:03:32Z** — about
  48 minutes later, by the author. `mergedAt: null`, `mergeCommit: null`; the
  timeline shows zero human reviews and zero review comments, only bot labels
  and a coverage bot reporting 100% patch coverage. Its diff is the same
  two-part shape described above (+65/−1 in `responses/mod.rs`, +1 in
  `http/service/openai.rs`) with tests
  `test_responses_chat_template_args_forwarded` and
  `test_responses_chat_template_args_absent`. **Verified against the current
  checkout: none of it is on `main`.** Per the planner contract, a
  closed-unmerged PR is not proof the issue is resolved, so there is **no
  `Disposition: already-resolved`** here. Issue #12284 itself is still `OPEN`.
- **No open PR anywhere implements this.** That is a clean negative, not an
  unchecked one — every search above returned either #12477 or unrelated work.
- **PR #12624 (OPEN)** — `feat(llm): support root-level thinking_token_budget
  for chat and responses`, by `flpanbin`. It does *not* fix this issue, but it
  edits the same struct and inserts a line immediately after the untouched
  `chat_template_args: None,` in the same literal. Worth naming as a textual
  merge-conflict adjacency for whoever rebases second; not a reason to stand
  down.
- **PR #12809 (OPEN)** — `rmccorm4`, validating converted protocol requests;
  touches `validate.rs` / `chat_completions.rs`. Adjacent, complementary, not
  competing.
- This factory (`glamr-agent`) has open PRs #12571, #12412, #12614, #12472 —
  **none for issue #12284**. There is no factory-owned request to maintain,
  which is why the route is `implementation` and not `babysitter`.

### 6. Environment

`compute-env.md`: one A100-SXM4-80GB, CUDA 12.9, vLLM 0.22.0 in-sandbox, **no
Docker daemon**, and `cargo` on `PATH`. The change is pure Rust in `lib/llm`;
nothing here needs the GPU, Docker, Python, or Kubernetes.

## Chosen approach

Two edits, both in `lib/llm/src/protocols/openai/responses/mod.rs`, plus test
fallout.

**1. Add the field to `NvCreateResponse` (lines 48–66).** Mirror the Chat
declaration exactly so the two APIs behave identically:

```rust
    /// Extra args to pass to the chat template rendering context.
    /// Also accepts "chat_template_kwargs" as an alias, matching
    /// `NvCreateChatCompletionRequest`.
    #[serde(
        default,
        skip_serializing_if = "Option::is_none",
        alias = "chat_template_kwargs"
    )]
    pub chat_template_args: Option<std::collections::HashMap<String, serde_json::Value>>,
```

Keeping the `chat_template_kwargs` alias is deliberate: a client that already
sends the vLLM-style spelling to `/v1/chat/completions` should not have to
learn a different spelling for `/v1/responses`. `default` +
`skip_serializing_if` means absent input stays `None` and round-tripped output
gains no new key — the wire format is unchanged for every request that does not
use the feature.

Note the field must sit **outside** the `#[serde(flatten)] inner`, as a sibling
of `nvext`. Because `inner` is flattened, an unknown top-level key is currently
swallowed; declaring the field is what makes serde bind it.

**2. Forward it at line 806.** Replace `chat_template_args: None,` with
`chat_template_args: resp.chat_template_args,`. `resp` is consumed by value in
this `TryFrom`, so this is a move, no clone. Nothing downstream needs to
change: `normalize_chat_reasoning_template_args` at `openai.rs:3084` already
merges into whatever map is present, and `OAIChatLikeRequest::chat_template_args`
at `unified.rs:494` already reads it.

**3. Handle the 30 literal sites.** All are test-only. Derive `Default` on
`NvCreateResponse` (`dynamo_protocols::CreateResponse` already derives
`Default`, and `Option<NvExt>` is `Default`, so this is free) and let the test
literals use `..Default::default()` where convenient; otherwise add the
explicit `chat_template_args: None,`. Either is acceptable — the printer should
pick whichever produces the smaller, more readable diff, and `cargo check
--workspace --all-targets` is what proves all 30 were found. This is the
"optional rather than required" call the contract asks about: the new field is
an `Option` with `#[serde(default)]`, so no *runtime* caller and no other crate
is forced to change; only in-crate test literals move, and only because Rust
struct literals are exhaustive.

**4. The regression test.** Put it next to the conversion, in the existing
`#[cfg(test)]` module of `responses/mod.rs` (that module already owns the
conversion tests and the `make_response_with_input` helper). Shape it exactly
like the issue's repro so it is a behavior test, not a mirror:

- deserialize the issue's literal JSON (`model` `dummy-model`, `input`
  `"hello"`, `chat_template_args: {"enable_thinking": true}`) into
  `NvCreateResponse` via `serde_json::from_value`/`from_str` — i.e. exercise the
  real wire path, not a hand-built struct;
- run the real `NvCreateChatCompletionRequest::try_from(resp)`;
- assert the converted chat request's `chat_template_args` contains
  `enable_thinking == true`.

**Why this is not tautological:** against the pre-change code the test fails
twice over — the JSON key does not bind to any field (so the deserialized value
carries nothing), and even if it did, the conversion hard-codes `None`, so the
assertion on the converted request fails. Revert either edit and the test goes
red. That satisfies `learnings/no-tautological-tests.md`.

Add the two controls the template asks for, both cheap:

- **absent-field control:** a Responses request with no `chat_template_args`
  converts to a chat request whose `chat_template_args` is `None` — proves we
  did not start synthesizing an empty map, which would change
  `skip_serializing_if` behavior and the rendered prompt.
- **alias control:** the same JSON spelled `chat_template_kwargs` produces the
  same converted result — proves the alias is wired, matching the Chat side and
  the coverage already in `lib/llm/tests/test_chat_template_args.rs` for chat.

A malformed-input control (`chat_template_args` as a string rather than an
object) is optional; serde already rejects it with a type error and the
behavior is inherited, so it is worth at most one line and should not be padded
into more.

We are **not** helping an existing request instead of writing our own: the only
request that ever did this work is closed-unmerged with no reviews on it, and
there is nothing open to review. Writing the change here is the correct move,
and it should be attributed as a fresh implementation rather than a revival of
#12477 — though the printer may reasonably arrive at a near-identical diff,
since there is essentially one way to add and forward a struct field.

## Rejected alternatives

- **Edit `dynamo_protocols::types::responses::CreateResponse` instead.** That
  is where an OpenAI-shaped Responses field would ideally live, but the crate
  is an external published dependency pinned at `= "5.1.0"` and is not vendored
  here. Changing it means a release of another repo and a version bump before
  this issue could close. Rejected as out of scope and out of reach.
- **Carry the args inside `nvext`.** `NvExt` is `deny_unknown_fields` and would
  need its own field anyway, the HTTP layer's
  `validate_response_unsupported_fields` (`openai.rs:3289`) already polices
  `nvext.extra_fields`, and the issue explicitly describes a root-level
  request field. It would also diverge from the Chat spelling that clients
  already use. Rejected.
- **Store the args on `ResponsesContext` in `unified.rs`.** That struct exists
  to preserve Responses-only state that has no Chat equivalent
  (`previous_response_id`, `store`, …) for the response-shaping trip back.
  `chat_template_args` has an exact Chat equivalent and needs to travel
  *forward* into the chat request, so it belongs on the request type. Rejected.
- **Make the field non-`Option` (a plain `HashMap`, defaulting to empty).**
  This would let us drop the `Option` juggling, but it changes serialization
  (an empty map would need its own `skip_serializing_if`), diverges from the
  Chat field's type, and would make the "absent stays absent" control
  meaningless. Rejected in favor of matching the Chat side exactly.
- **Fix only line 806 without touching the struct.** Cannot compile — there is
  no `resp.chat_template_args` to read. Recording it because the issue's own
  wording ("the Responses conversion still initializes `chat_template_args` as
  `None`") invites exactly this misreading, and a printer who tries it will
  waste a cycle.
- **Add a `ValidateRequest` impl for `NvCreateResponse` at the same time** so
  the nested-`chat_template` guard fires on this path. Tempting and one line,
  but it overlaps open PR #12809 and would widen this diff into someone else's
  in-flight design. Rejected; noted for the reviewer instead.

## Validation strategy

This change is a Rust struct field and a Rust `TryFrom` arm in `lib/llm`. It
makes no runtime or serving claim: the observable contract — "a Responses
request with `chat_template_args` converts to a chat request that still has
them" — is fully decided at the conversion boundary and is provable by a unit
test that runs the real deserializer and the real conversion. So the ladder is
deliberately short and entirely runnable; nothing here is inspected when it
could be executed.

**`02-rust-cargo-check`** is the whole ladder, and all four of its sections
apply:

- *Section 1 (format)* — `git diff --check` and `cargo fmt --all --check`.
- *Section 2 (compile)* — `cargo check --workspace --all-targets`, plus
  `cargo check --manifest-path lib/bindings/python/Cargo.toml --all-targets`.
  `--all-targets` is what matters here: all 30 `NvCreateResponse` literals are
  inside `#[cfg(test)]` blocks, and a plain `--workspace` build would compile
  none of them. This is the step that proves every construction site was found
  — exactly the "did you find every caller" role the contract assigns it.
- *Section 3 (lint)* — `cargo clippy --workspace --all-targets --no-deps -- -D warnings`.
- *Section 4 (unit tests)* — `cargo test -p dynamo-llm --lib`, which runs the
  new conversion tests in `responses/mod.rs`, plus the existing
  `lib/llm/tests/test_chat_template_args.rs` integration tests to show the Chat
  path did not regress.

The evidence chain: Section 4 green with the new
`chat_template_args`-forwarded test is the direct proof of the issue's stated
expectation; Section 2 green at `--workspace --all-targets` is the proof that
adding a field to a 30-literal struct broke no caller anywhere in the
workspace or in the separately-rooted Python bindings; Sections 1 and 3 are the
lint gate. The validator should, where practical, also record the pre-change
red — stash the two production edits, keep the test, and show it failing — since
that is the cleanest possible demonstration that the test is not a tautology.

Requires: CPU only. No GPU, no Docker, no Kubernetes, no engine process, no
network beyond the crate cache. Everything runs in this sandbox.

**Recipes deliberately not nominated:**

- `07-agg-smoke` / `08-disagg-pair-smoke` / `09-gpu-pytest` — this change adds
  no runtime behavior an engine could demonstrate. Standing up vLLM to observe
  a struct field survive a `TryFrom` would burn real GPU minutes and prove
  strictly less than the unit test. Declined on purpose.
- `00-dynamo-editable-install` — needed only when a selected recipe imports
  `dynamo.*` from Python. None does; the ladder is pure `cargo`.
- `01-python-lint` / `03-python-unit-tests-mocker` / `04-python-runtime-lint` —
  no Python is touched.
- `05-code-inspection` — everything in scope executes; an inspection pass would
  be a worse form of the same evidence.
- `06-dockerfile-build` — no Docker daemon in this sandbox and no container
  change; `N/A`, and per the recipe a missing Docker socket never blocks
  Recipe 02.
- `10-perf-benchmark`, `11-go-operator-tests`, `12-helm-chart-render`,
  `13`–`16` — no performance claim, no Go, no charts, no SGLang/TRT-LLM.

```validation-recipes
02-rust-cargo-check
```

## Required deliverables

Per the `model-parser-investigation` overlay, this work item produces the
standard packet set under the work-item root:

- `plan.md` — this file (planner).
- `change.md` and `change.diff` — the printer's narrative and the committed
  diff. The diff should touch `lib/llm/src/protocols/openai/responses/mod.rs`
  (struct at 48–66, conversion at 806, new tests in the `#[cfg(test)]` module
  from 1196) and, only if the `Default`-derive route is not taken,
  `lib/llm/src/http/service/openai.rs` (the `make_base_request()` test helper
  at 4684–4692). Commit locally with `git commit --signoff` on branch
  `fix/preserve-chat-template-args-responses--7ac08df9fc39` off `main`. The
  printer does not publish.
- `change-validation.md` — the validator's recorded run of
  `02-rust-cargo-check`, all four sections, ending in exactly
  `## Verdict: pass`, `fail`, or `blocked`.
- `review.md` — the reviewer's assessment, ending in exactly
  `## Assessment: sound` or `needs_changes`. The reviewer should specifically
  confirm that the new test fails against the old behavior, that an absent
  `chat_template_args` still yields `None` rather than an empty map, and that
  the Chat Completions path is untouched.
- The publisher pushes the branch and opens a **new** pull request against
  `main` (no review request is recorded for this work item, so nothing exists
  to push onto). The description should reference issue #12284 and should also
  mention PR #12624 as a likely textual conflict in the same struct literal.

[glamr-agent]

change.md

# change.md — Preserve `chat_template_args` on the Responses → Chat conversion

Branch: `fix/preserve-chat-template-args-responses--7ac08df9fc39`, off `main` at
`4f32fc683`.

Commit: **`04a6d670ffc83488b841e557cea7afc613559cbb`** — a single signed-off
commit, `fix(responses): preserve chat_template_args during Responses to Chat
conversion`. DCO verified: author `svc-glamr@nvidia.com` and the
`Signed-off-by:` trailer email match.

Diff: 2 files, +105/−1.

## What changed, per file

### `lib/llm/src/protocols/openai/responses/mod.rs`

Three things, all in this one file.

**1. New field on `NvCreateResponse` (struct at lines 48–66 pre-change).** Added
`pub chat_template_args: Option<HashMap<String, serde_json::Value>>` as a sibling
of `nvext`, outside the `#[serde(flatten)] inner`, with the same serde attributes
the Chat Completions request uses (`default`, `skip_serializing_if =
"Option::is_none"`, `alias = "chat_template_kwargs"`). This is the load-bearing
half: before it, the top-level JSON key was swallowed by the flattened `inner`
and never reached the struct at all, so the request parsed successfully and the
argument vanished silently.

I used the crate-local `HashMap` (already imported at the top of the file, line 6)
rather than spelling `std::collections::HashMap` inline as the plan sketched —
`cargo fmt` and the file's own conventions both prefer the imported name, and the
type is identical.

**2. Forwarding in the conversion.** In `impl TryFrom<NvCreateResponse> for
NvCreateChatCompletionRequest` (line 673 pre-change), the returned struct literal
had `chat_template_args: None` hard-coded; it is now
`chat_template_args: resp.chat_template_args`. `resp` is taken by value, so this
is a move — no clone. This is now line 822.

**3. Test literals and four new tests.** Twenty-two `NvCreateResponse { … }`
literals in the `#[cfg(test)]` module each gained an explicit
`chat_template_args: None,`. The four new tests sit next to the existing
conversion tests, after `test_store_mapped_to_chat_completion_request`:

- `test_responses_chat_template_args_forwarded_to_chat_completion` — the issue's
  exact repro. Deserializes `{"model":"dummy-model","input":"hello",
  "chat_template_args":{"enable_thinking":true}}` into `NvCreateResponse` via
  `serde_json::from_value`, runs the real `TryFrom`, and asserts the converted
  chat request's `chat_template_args["enable_thinking"] == true`.
- `test_responses_chat_template_kwargs_alias_forwarded_to_chat_completion` — the
  alias control: the vLLM-style `chat_template_kwargs` spelling produces the same
  result.
- `test_responses_without_chat_template_args_converts_to_none` — the absent-field
  control: no key in, `None` out (not an empty map).
- `test_responses_chat_template_args_rejects_non_object` — malformed-input
  control: a string instead of an object is a serde type error. One assertion,
  deliberately not padded.

The tests deserialize from JSON rather than building the struct by hand, because
half the defect lives in serde binding; a hand-built struct literal would skip
the part that was actually broken.

### `lib/llm/src/http/service/openai.rs`

One line: the `make_base_request()` test helper (line 4684) gained
`chat_template_args: None,`. Test-only; no production code in this file
constructs `NvCreateResponse` by literal.

## The tests are not tautological — measured, not asserted

I reverted just the forwarding line (`resp.chat_template_args` → `None`) and
re-ran, keeping the tests and the struct field:

```
test test_responses_chat_template_args_forwarded_to_chat_completion ... FAILED
test test_responses_chat_template_kwargs_alias_forwarded_to_chat_completion ... FAILED
test test_responses_chat_template_args_rejects_non_object ... ok
test result: FAILED. 1 passed; 2 failed
```

Panic at `responses/mod.rs:1336` — the `.expect("chat_template_args must survive
…")`. Both positive tests go red on *either* production edit alone; removing the
struct field would fail them at deserialization instead. I restored the line and
reconfirmed green before committing. The working tree is clean at the commit.

## Validation actually run

All in `/home/sandbox/workspace/wi-20260807T212155Z-12284/repo`, environment as
given (I did not set `CARGO_TARGET_DIR`).

| Command | Result |
|---|---|
| `git diff --check` | clean, exit 0 |
| `cargo fmt --all --check` | **pass**, exit 0, no output |
| `cargo check --workspace --all-targets` | **pass** — `Finished dev profile … in 1m 55s` |
| `cargo check --manifest-path lib/bindings/python/Cargo.toml --all-targets` | **pass** — `Finished dev profile … in 1m 39s` |
| `cargo test -p dynamo-llm --lib` | **pass** — `1976 passed; 0 failed; 5 ignored` |
| `cargo test -p dynamo-llm --test test_chat_template_args` | **pass** — `3 passed; 0 failed` (Chat path unregressed) |
| `cargo clippy --workspace --all-targets --no-deps -- -D warnings` | **fails, pre-existing on `main`** — see below |

The `--all-targets` check is the one that proves I found every construction site:
all 23 literals are inside `#[cfg(test)]` and a plain `--workspace` build compiles
none of them.

Targeted run of the new tests:

```
test protocols::openai::responses::tests::test_responses_chat_template_args_rejects_non_object ... ok
test protocols::openai::responses::tests::test_responses_chat_template_kwargs_alias_forwarded_to_chat_completion ... ok
test protocols::openai::responses::tests::test_responses_chat_template_args_forwarded_to_chat_completion ... ok
test protocols::openai::responses::tests::test_responses_without_chat_template_args_converts_to_none ... ok
```

### The clippy failure is not mine

```
error: unknown lint: `clippy::manual_option_zip`
    --> lib/llm/src/discovery/model_manager.rs:1898:17
     = note: `-D unknown-lints` implied by `-D warnings`
```

`model_manager.rs` is not in my diff. I verified this is pre-existing rather than
assuming it: `git stash` back to the clean branch point, re-ran `cargo clippy -p
dynamo-llm --all-targets --no-deps -- -D warnings`, and got the identical error.
The installed clippy is newer than the repo expects and has renamed that lint
(`manual_option_zip` → `manual_option_as_slice`). Per
`learnings/format-changed-only.md` — a linter complaining about a line I did not
touch is not mine to fix — I left it alone and recorded it. The validator will hit
the same wall on Section 3 of Recipe 02; it is an environment/toolchain skew, not
a defect in this change.

## Things I tried that did not work

**The bulk edit of the 23 test literals, first attempt.** I wrote a brace-matching
Python script keyed on the regex `NvCreateResponse\s*\{` and it over-matched: it
also fired on `fn make_response_with_input(text: &str) -> NvCreateResponse {` and
on `impl NvExtProvider for NvCreateResponse {`-style blocks, so it injected
`chat_template_args: None,` into function bodies and trait impls. Caught it by
reading the output rather than trusting the count (26 + 1 edits, where I expected
~23). Reverted both files with `git checkout --`, confirmed the tree was clean, and
redid it with a guard: only treat a match as a struct literal when the text right
after the brace starts with `inner: CreateResponse`, which is true of every real
literal and of nothing else. That gave 22 + 1 = 23, matching the grep count of
construction sites, and `cargo check --all-targets` then compiled clean — which is
the actual proof that all sites were found and none were corrupted.

## Scope discipline

Non-goals from the plan, all honoured. The Chat Completions side is untouched
(`chat_completions.rs` is not in the diff). Nothing was added to
`dynamo_protocols::types::responses::CreateResponse` — it is an external crate
pinned at `= "5.1.0"` and not vendored here. Nothing routed through `NvExt` or
`ResponsesContext`. I did **not** add an `impl ValidateRequest for
NvCreateResponse`, even though it is one line and would make the nested-
`chat_template` guard fire on this path, because that overlaps open PR #12809 —
flagging it for the reviewer as deliberate omission rather than oversight.

One adjacency worth naming for whoever rebases second: open PR #12624
(`thinking_token_budget`) inserts a line into the same struct literal immediately
after the `chat_template_args:` line I changed. That is a textual conflict, not a
semantic one.

## Compatibility

Per `lib/llm/CLAUDE.md`, this is a cross-process wire surface. The new field is
`Option` with `#[serde(default)]`, so a previous-version client that sends no
`chat_template_args` is unaffected, and `skip_serializing_if` means a
round-tripped request gains no new key. A previous-version frontend receiving the
field ignores it exactly as it does today. No shim is needed and none was added —
this is a purely additive optional field in both directions.

MR URL: https://github.com/ai-dynamo/dynamo/pull/12860

[glamr-agent]

change-validation.md

# change-validation.md — Recipe 02-rust-cargo-check

Validated commit `04a6d670ffc83488b841e557cea7afc613559cbb` on branch
`fix/preserve-chat-template-args-responses--7ac08df9fc39`, off `main` at `4f32fc683`.
Working tree clean before, during (all destructive experiments were done in a scratch
worktree outside the repo), and after. The plan names exactly one recipe,
`02-rust-cargo-check`; all four of its sections apply and all four ran, at full
strength, on this host. Every command went through the recorder.

## The three questions worth answering

Sections 1, 2 and 4 were unremarkable — they compile, they format, they pass. The work
of this validation was in three places: whether the clippy failure the printer reported
was really pre-existing, whether the new tests actually measure anything, and whether
one of my own recorded runs could be trusted. The third one is the reason this file is
longer than it would otherwise be.

## Section 3: the clippy failure was real, pre-existing, and avoidable — the printer's
## diagnosis was inverted

The printer reported `cargo clippy --workspace --all-targets --no-deps -- -D warnings`
failing with `unknown lint: clippy::manual_option_zip` at
`lib/llm/src/discovery/model_manager.rs:1898`, and asserted this was pre-existing on
`main` and caused by a clippy *newer* than the repo expects.

Pre-existing: confirmed, three independent ways. The `#[allow(clippy::manual_option_zip)]`
exists verbatim at base commit `4f32fc683` and is nowhere in `change.diff` — the diff
touches only `protocols/openai/responses/mod.rs` and `http/service/openai.rs`. It was
introduced by `18b6e7eed feat(llm): route requests through encode workers (#11460)`. And
I recorded a clippy run in a detached worktree at unmodified `4f32fc683` with no change
applied: identical error, identical exit 101. That is the empirical proof, not an
inference from the diff.

The cause, however, is the opposite of what was claimed. `rust-toolchain.toml` pins
`channel = "1.96.1"`, but the sandbox exports `RUSTUP_TOOLCHAIN=1.93.1`, which overrides
the repo pin — so the *older* toolchain was running and simply did not know the lint
name. I isolated this to toolchain skew with a two-line probe crate: 1.93.1 rejects the
name, 1.96.1 accepts it. The lint was renamed `manual_option_zip` → `manual_option_as_slice`
in the interval.

That distinction matters because it changes the disposition. A newer-than-expected clippy
would be an environment wall to be recorded and lived with. An override of the repo's own
declared toolchain is a wall I can simply step around: I installed 1.96.1 and ran
Section 3 as `cargo +1.96.1 clippy --workspace --all-targets --no-deps -- -D warnings` —
**exit 0, green**. Same lint set, same `--all-targets` scope, same `-D warnings`. Nothing
was weakened; the flags are byte-identical and the only change is running the toolchain
the repository asks for. So Section 3 needs no disposition at all. It is green, and the
printer's "the validator will hit the same wall" prediction did not hold.

## The tests measure something — and more than the printer showed

The printer's pre-change red reverted only the forwarding line
(`resp.chat_template_args` → `None`), which turned 2 of the 4 new tests red. That is a
real demonstration but it only exercises half the change. The load-bearing half is the
*declaration* of the field on `NvCreateResponse`: because `inner` is `#[serde(flatten)]`,
an undeclared top-level key is silently swallowed rather than rejected, which is why the
bug was invisible in the first place.

So I ran the stronger variant in the scratch worktree: true base production code — no
struct field, no forwarding — with only the four new tests added on top. **3 of 4 go red**,
and the third one is the interesting one:
`test_responses_chat_template_args_rejects_non_object` fails with
`assertion failed: err.is_err()`. At base, sending a string where an object belongs is
accepted, because the flattened `inner` swallows it. That single assertion the printer
described as "deliberately not padded" turns out to be the one that pins the serde half
of the defect. Both halves are therefore independently necessary, measured rather than
asserted.

This also satisfies the paired positive/negative requirement on new test infrastructure:
the same deserialization path was exercised on an input it accepts (the `chat_template_args`
and `chat_template_kwargs` object forms, which reach the conversion and survive it) and on
an input it rejects (the non-object form, which is a serde type error), with both directions
recorded as command logs rather than claimed in prose.

## One of my own recorded runs was vacuously green — and I am flagging it rather than
## quietly superseding it

An earlier consolidated run of mine (`2026-07T22-45-59Z`) printed
`=== ALL FOUR SECTIONS GREEN ===` and exited 0 over a log that contains
`test result: FAILED. 1962 passed; 14 failed`. The wrapper used `set -e` with
`cargo test -p dynamo-llm --lib 2>/dev/null | tail -3`; because the failing command's
stdout was piped, the shell only ever saw `tail`'s status, and the `2>/dev/null` discarded
the per-test detail. That entry was, at the time, the most recent registry line for this
recipe — i.e. the row customs would have used to decide the recipe. A green verdict resting
on it would have been worthless, and it is exactly the failure mode a validator exists to
catch, so it does not get to disappear silently.

I chased the 14 failures rather than assuming them away. Standalone
`cargo test -p dynamo-llm --lib` is green (`1976 passed; 0 failed; 5 ignored`) on every
isolated attempt — I recorded ten consecutive runs with the exit code preserved, all
exit 0, and the four new tests `ok` in all ten. Beyond that I ran roughly forty-five more
under deliberately adversarial conditions: four-way concurrent test processes over the
shared `CARGO_TARGET_DIR`, and test runs racing a concurrent workspace clippy on a
*different* toolchain (a mass recompile into the same target dir, which is what the
original masked run was actually doing). The 14-failure event did not reproduce. The
only failure I ever reproduced was a single instance of
`kv_router::indexer::tests::concurrent_rank_reset_waits_for_all_local_tiers` under two
simultaneous test processes — a timing-sensitive test in the KV-router indexer, a
subsystem this diff does not touch.

I am not going to over-claim here. I could not reproduce the exact 14-failure event, and
the log that would have named those 14 tests was destroyed by my own `2>/dev/null`. What
I can say with evidence: the failures were concentrated in a run that shared a target
directory with a concurrently-executing cross-toolchain compile; the identified flaky test
belongs to an unrelated subsystem; and across ~55 subsequent runs no test in
`protocols::openai::responses` or `http::service::openai` failed even once. The honest
characterization is environmental contention in my own harness, not a defect in the change
— stated as a judgement with its supporting evidence, not as a certainty.

The final recorded run for this recipe re-runs all four sections consolidated with
`set -euo pipefail`, no pipes and no stderr suppression on anything that can fail, so a
non-zero exit genuinely propagates. It is green end to end: clean tree at
`04a6d670f`, both `cargo check` invocations, clippy at 1.96.1 with `-D warnings`,
`1976 passed; 0 failed`, and `test_chat_template_args` `3 passed; 0 failed`. Grepping
that log for `FAILED` returns nothing.

## Coverage, and what was not proven

The `--all-targets` compile is what actually proves the printer found every construction
site: all 23 `NvCreateResponse` literals live inside `#[cfg(test)]`, so a plain
`--workspace` build compiles none of them and would have hidden a miss. It compiles clean,
so there is no missed site.

The Chat Completions path is unregressed — `cargo test -p dynamo-llm --test test_chat_template_args`
passes its 3 tests, and `chat_completions.rs` is not in the diff.

What this recipe does not establish: nothing was exercised over HTTP or against a running
engine. The evidence is that the field binds from JSON, survives the `TryFrom`, and arrives
on the chat request — not that a live `/v1/responses` request renders a different prompt
end to end. That is the correct scope for the recipe the plan selected, but it is the
boundary of the claim. I also note, without treating it as a defect, that the printer
deliberately omitted an `impl ValidateRequest for NvCreateResponse` to avoid overlapping
open PR #12809; that omission is outside what Recipe 02 can adjudicate and is the
reviewer's call.

## Verdict: pass

All four sections of `02-rust-cargo-check` ran green at full strength on the printer's
commit, with no disposition required — including Section 3, once run under the toolchain
`rust-toolchain.toml` actually pins. The new tests were shown to fail against true base
production code (3 of 4 red, covering both the serde-declaration and forwarding halves),
so the green is measured rather than tautological. The one materially misleading artifact
in the evidence trail was my own masked consolidated run; it has been investigated,
explained, superseded by an honest run that cannot mask a failure, and left in the record
rather than buried. The branch is exactly as the printer left it: `04a6d670f`, clean tree,
scratch worktree removed.

[glamr-agent]

review.md

🤖 **Automated AI review — advisory.** An AI agent's judgment of whether this change is logically sound on the evidence; not a merge authorization. CI, customs, and a human reviewer hold that.

## Assessment: sound

The change is small, correct, and matches the defect it claims to fix. Both halves of the
fix are necessary and both were shown red at true base. The deciding evidence row is
honest and shows the change actually exercised. Two findings, both `nit`; neither blocks.

## Findings

| # | Severity | Location | Claim |
|---|---|---|---|
| 1 | nit | `lib/llm/src/protocols/openai/responses/mod.rs:822` | Forwarding makes `chat_template_args` reachable on `/v1/responses`, but that path never runs `ValidateRequest`, so the nested-`chat_template` guard does not fire there — a payload rejected on `/v1/chat/completions` is now accepted on `/v1/responses`. |
| 2 | nit | `validation/logs/2026-08-07T22-45-59.829Z-bash-d811.log` (registry row 12) | A superseded evidence row printed a green banner over `test result: FAILED. 1962 passed; 14 failed`; the identity of those 14 tests is permanently unrecoverable and the event was never reproduced. |

### Finding 1 — the nested-`chat_template` guard does not cover the Responses path

**Consequence.** After this change, `POST /v1/responses` with
`{"chat_template_args": {"chat_template": "<attacker-supplied Jinja>"}}` reaches the
renderer unguarded. The byte-identical payload on `POST /v1/chat/completions` is rejected.

**Evidence, by file:line:**

- `lib/llm/src/protocols/openai/validate.rs:818-829` — `validate_chat_template_args` is
  the guard. Its own doc comment states the consequence: "A nested `chat_template`
  bypasses Dynamo's top-level rejection and is promoted into the rendered template, so
  block it for every chat processor."
- `lib/llm/src/protocols/openai/chat_completions.rs:536-539` — the guard's **sole**
  caller is `impl ValidateRequest for NvCreateChatCompletionRequest`. There is no
  `impl ValidateRequest for NvCreateResponse`.
- `lib/llm/src/http/service/openai.rs:2542` — the chat handler reaches it via
  `validate_chat_completion_fields_generic(&request)`, which calls `request.validate()`
  at line 2860.
- `lib/llm/src/http/service/openai.rs:2976-3110` — the `responses()` handler calls only
  `validate_response_unsupported_fields(&request)` (defined at 3289) and
  `normalize_chat_reasoning_template_args(&mut chat_request)` (3084). A scan of the whole
  handler body finds no `.validate()` and no `validate_chat_completion_fields_generic`
  call.
- `lib/llm/src/engines.rs:423-436` — `ValidateEngine<E>::generate` would call
  `request.validate()`, but a repo-wide grep for `ValidateEngine` returns only
  `engines.rs` itself (definition plus one test at line 630). No production wiring, so it
  does not close the gap.

**Why this is a note and not a scope demand.** The change does not create the missing
`impl`; the gap pre-dates it. But the honest characterisation is that the gap was *moot*
before — the field could never be populated on this path, because the flattened `inner`
swallowed the key — and this change makes it *live*. That is worth a human's attention
even though `plan.md` names it a non-goal owned by open PR #12809, and `change.md`
already flags the omission as deliberate. The correct disposition is a one-line
`impl ValidateRequest for NvCreateResponse` delegating to
`validate::validate_chat_template_args(self.chat_template_args.as_ref())`, landed on
whichever of the two branches merges second. Not requested here.

### Finding 2 — one superseded evidence row was vacuously green

`validation/registry.jsonl` row 12 recorded exit 0 for a consolidated run whose log
contains `test result: FAILED. 1962 passed; 14 failed` followed by
`=== ALL FOUR SECTIONS GREEN ===`. The cause is mechanical and the validator diagnosed it
correctly: `cargo test … 2>/dev/null | tail -3` under `set -e` exposes only `tail`'s
status, and the `2>/dev/null` destroyed the per-test detail.

This does **not** meet the `evidence-rows-must-show-execution` condition that would force
a `failing` disposition, because row 12 is not the deciding row. The deciding row is 14,
and its log is clean (see the evidence audit below). Row 12's absence of a disposition is
therefore not a finding against the gate.

The residual worth naming: the identity of those 14 tests is gone and cannot be
recovered. The validator ran ~55 further attempts, including four-way concurrent test
processes over a shared `CARGO_TARGET_DIR` and runs racing a cross-toolchain workspace
compile, and did not reproduce the event; the only failure reproduced at all was
`kv_router::indexer::tests::concurrent_rank_reset_waits_for_all_local_tiers` under two
simultaneous test processes — a timing-sensitive test in a subsystem this diff does not
touch. "Environmental contention in the harness" is a reasonable judgement on that
evidence and was stated as a judgement rather than a certainty. A human should know the
event exists and was never explained.

## Evidence audit

**Deciding row.** `validation/registry.jsonl` index 14,
`validation/logs/2026-08-07T23-13-07.157Z-bash-94f7.log` (3546 lines), exit 0, produced
by a `set -euo pipefail` script with no pipes and no stderr suppression on anything that
can fail. Header confirms SHA `04a6d670ffc83488b841e557cea7afc613559cbb` and a clean
tree. All four sections of `02-rust-cargo-check` ran:

- Section 1 (fmt + `git diff --check`) — both exit 0.
- Section 2 — `cargo check --workspace --all-targets` and the python-bindings manifest
  both `Finished`.
- Section 3 — `cargo +1.96.1 clippy --workspace --all-targets --no-deps -- -D warnings`,
  `Finished … in 1m 46s`, no error.
- Section 4 — lines 3141-3145 show all four new tests `ok`; line 3530
  `test result: ok. 1976 passed; 0 failed; 5 ignored`; line 3543
  `test result: ok. 3 passed; 0 failed` for `test_chat_template_args`.
- `grep -c FAILED` over the whole log → 0.

The row is `validated` and its log shows execution and pass. No contradiction.

**Toolchain (lead 2) — legitimate.** `rust-toolchain.toml` pins `channel = "1.96.1"`; the
sandbox exports `RUSTUP_TOOLCHAIN=1.93.1`, which overrides the repo pin. Under 1.93.1 the
workspace fails with `unknown lint: clippy::manual_option_zip` at
`lib/llm/src/discovery/model_manager.rs:1898` — a file absent from the diff, and the
`#[allow]` was introduced by `18b6e7eed`. The validator reproduced that failure at
unmodified base (registry row 8, exit 101), which is the empirical proof of
pre-existence rather than an inference from the diff. Re-running as `cargo +1.96.1` uses
byte-identical flags (`--workspace --all-targets --no-deps -- -D warnings`) and the same
lint set; nothing was weakened. Restoring the toolchain the repository itself declares is
not papering over a failure, and I agree with the validator's correction of the printer's
inverted diagnosis (the installed clippy was *older*, not newer).

**Non-tautology (plan confirmation a) — proven.** `validation/logs/2026-08-07T22-37-09.924Z-bash-9e8f.log`
(registry row 10) applies **only** the 65 lines of new tests to true base production code
— no struct field, no forwarding. 3 of 4 go red at named panic sites:
`responses/mod.rs:1319`, `:1338`, and `:1365`
(`test_responses_chat_template_args_rejects_non_object`, `assertion failed: err.is_err()`).
That third failure is the load-bearing one: it shows the *serde declaration* half is
independently necessary, because at base the flattened `inner` accepts a string where an
object belongs. The printer's own weaker red (forwarding line only, 2 of 4) exercised
half the change; the validator's stronger red covers both halves. This satisfies
`no-tautological-tests` on evidence, not assertion, and supplies the paired
positive/negative on the same deserialization path.

**Absent field yields `None` (plan confirmation b) — direct.**
`test_responses_without_chat_template_args_converts_to_none` deserializes a payload with
no key and asserts `chat_req.chat_template_args.is_none()` — not an empty map. Shown `ok`
in the deciding log.

**Chat Completions path untouched (plan confirmation c) — confirmed.**
`chat_completions.rs` does not appear in `change.diff`, and
`cargo test -p dynamo-llm --test test_chat_template_args` is `3 passed; 0 failed` at line
3543 of the deciding log.

**Non-goals — clean.** The diff touches exactly two files: `responses/mod.rs` and one
test-helper line at `lib/llm/src/http/service/openai.rs:4690`. No `chat_completions.rs`,
no `NvExt`, no `ResponsesContext`, no addition to the pinned external
`dynamo_protocols::types::responses::CreateResponse`, no engine, runtime, or GPU work. No
out-of-scope edits.

**Correctness of the field itself.** The new declaration at `responses/mod.rs:67-81`
mirrors `chat_completions.rs:100-107` attribute-for-attribute (`default`,
`skip_serializing_if = "Option::is_none"`, `alias = "chat_template_kwargs"`); the only
difference is the imported `HashMap` versus the inline `std::collections::HashMap`, which
is the same type. Forwarding at line 822 is a move out of a by-value `resp`, no clone.
Per `lib/llm/CLAUDE.md` this is a cross-process wire surface: the field is `Option` with
`#[serde(default)]` and `skip_serializing_if`, so an N-1 client that omits it is
unaffected and a round-trip gains no new key — purely additive in both directions, no
shim needed.

**Coverage of construction sites.** All 23 `NvCreateResponse` literals live inside
`#[cfg(test)]`, so a plain `--workspace` build compiles none of them. The
`--all-targets` compile in Section 2 is what actually proves no site was missed, and it
is green.

**Compute environment.** No remote compute, SSH, or Slurm was involved; the recipe is
CPU-only and ran locally. Nothing in `compute-env.md` is contradicted.

## What this review does not establish

Nothing was exercised over HTTP or against a running engine. The evidence shows the field
binds from JSON, survives the `TryFrom`, and lands on the chat request — not that a live
`/v1/responses` request renders a different prompt end to end. That is the correct scope
for the single recipe the plan selected, and the validator states the same boundary, but
it is the boundary of the claim.

[coderabbitai]

Walkthrough

Changes

Responses-to-Chat argument preservation

Layer / File(s)	Summary
Request contract and conversion lib/llm/src/protocols/openai/responses/mod.rs	NvCreateResponse now accepts optional chat_template_args and the chat_template_kwargs alias. Responses-to-Chat conversion forwards the value.
Conversion validation and fixtures lib/llm/src/protocols/openai/responses/mod.rs, lib/llm/src/http/service/openai.rs	Tests cover forwarding, alias deserialization, omission, and rejection of non-object values. Existing fixtures initialize the new field.

Estimated code review effort: 2 (Simple) | ~10 minutes

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	The changes satisfy issue #12284 by deserializing chat_template_args, forwarding it during conversion, and testing populated and aliased values.
Out of Scope Changes check	✅ Passed	The changes are limited to the requested field binding, conversion forwarding, related fixtures, and focused tests.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Description check	✅ Passed	The description explains the fix, lists validation, and links issue #12284; it does not identify where the reviewer should start.
Title check	✅ Passed	The title clearly and concisely describes preserving chat template arguments in the Responses endpoint.

---

_{Comment @coderabbitai help to get the list of available commands.}

[devin-ai-integration]

Devin Review found 1 potential issue.

View 1 additional finding in Devin Review.

[glamr-agent]

Automated CI watch — passed

CI result for head commit 18abdd44a5c85852999f992bc075453003e35dbc: passed.

Observed at 2026-08-08T01:12Z, after a bounded poll of the GitHub check-runs and
commit-status APIs for this commit. All check runs reached a terminal state.

• 20 check runs concluded success, including rust-clippy and rust-tests
  across ., lib/bindings/python, lib/bindings/kvbm, and
  lib/runtime/examples; plus pre-commit, codeowners, copyright-checks,
  changed-files, lychee, DCO, and pre-merge-status-check.
• 5 check runs concluded skipped because the change touches no files in their
  scope: snapshot, operator, Fern Configuration Check,
  Fern Broken Links Check, Docs Website Composition Check.
• 0 check runs failed, errored, timed out, or were cancelled.
• Combined commit status: success (CodeRabbit).

This head supersedes 04a6d670f, which was also green. The difference is
18abdd44a, the repair-round-1 commit that applies the nested-chat_template
guard on the Responses path; see the round-1 summary comment for its checks.

This covers the pre-merge checks that run automatically on a fork pull request.
The repository's full CI suite runs only after a maintainer comments
/ok to test <sha>; that suite is outside what this watch observed.

[glamr-agent]

Automated repair round 1 — chat_template_args guard on the Responses path

Finding addressed: the Devin review thread on
lib/llm/src/protocols/openai/responses/mod.rs — extra template arguments sent to
/v1/responses were forwarded without the nested-chat_template check that
/v1/chat/completions applies, so {"chat_template_args": {"chat_template": "..."}}
was rejected on one endpoint and accepted on the other.

The finding is accurate. NvCreateChatCompletionRequest::validate calls
validate::validate_chat_template_args, and the chat handler runs it via
validate_chat_completion_fields_generic. The Responses handler converts to a chat
request but never calls validate() on the result. Before this PR the field was always
None there, so the gap was inert; forwarding resp.chat_template_args made it
reachable.

Fix: new validate_response_chat_template_args in
lib/llm/src/http/service/openai.rs calls the same shared validator on the Responses
request and maps failure to a 400 carrying the standard VALIDATION_PREFIX. It is
invoked in the responses handler immediately after
validate_response_unsupported_fields, so the identical payload now gets the identical
rejection on both endpoints. Two unit tests cover rejection of the nested key and
acceptance of ordinary args such as enable_thinking.

Calling the shared validator directly was preferred over implementing ValidateRequest
for NvCreateResponse or running the whole converted chat request through
validate_chat_completion_fields_generic: the broader change is the subject of open PR
#12809, and pre-empting it here would
alter validation behavior for Responses fields this PR does not touch.

Commit: 18abdd44a — fix(responses): reject nested chat_template in chat_template_args
(signed off, fast-forward on 04a6d670f).

Checks actually run on the new head, clean tree:

Check	Result
cargo fmt --all --check	exit 0
git diff --check	exit 0
cargo check --workspace --all-targets	exit 0
cargo test -p dynamo-llm --lib	1978 passed; 0 failed; 5 ignored (1976 before; +2 new)
cargo +1.96.1 clippy -p dynamo-llm --all-targets --no-deps -- -D warnings	exit 0

The workspace-wide check was run rather than a crate-scoped one because the change adds
a new public function in a crate other crates depend on.

Note on clippy: under this host's ambient default toolchain (1.93.1) the run fails on
unknown lint: clippy::manual_option_zip at
lib/llm/src/discovery/model_manager.rs:1898. That is pre-existing on main, lives in
a file this PR does not touch, and does not reproduce under the repo-pinned toolchain
1.96.1 from rust-toolchain.toml; the rust-clippy CI job was green on the previous
head for the same reason.

[MatejKosec]

/ok to test a84c79f

[MatejKosec]

/ok to test 2602817

[datadog-official]

🔄 Datadog auto-retried 10 jobs - 10 passed on retry

🎯 Code Coverage (details)
• Patch Coverage: 100.00%
• Overall Coverage: 45.92% (-6.48%)

_{This comment will be updated automatically if new data arrives.
🔗 Commit SHA: c25f056 | Docs | Datadog PR Page | Give us feedback!}

[MatejKosec]

/ok to test c25f056

[MatejKosec]

/ok to test 2f3ecc4

[rmccorm4]

FYI duplicate PR here: #13624

[ayaangazali]

I owe you a heads up on this one, and an apology.

I opened #13624 for the same problem on 2026-08-20 and it merged this morning. Yours predates mine by thirteen days and covers the same three files. I did not see it before opening mine, and that is a miss on my part, not a judgement about which approach was better.

The reason I missed it is worth stating so it does not look careless. My duplicate check searches open PR bodies for a reference to the issue number, in this case #12284. This PR does not cite that issue, so a number search never surfaced it. A keyword search for the symbol would have. I have since found two other cases where the same blind spot hid an owning PR from me, and I have changed the check.

What merged covers the forwarding and the alias, same as yours: chat_template_args on NvCreateResponse with chat_template_kwargs accepted as an alias, preserved through the conversion to the internal chat-completion request, plus honouring force_nonempty_content when a reasoning-only response is aggregated.

On the one piece of yours that is not literally in my diff, the nested chat_template rejection: I checked whether that left a gap on main rather than assuming it did, because if forwarding landed without the guard that would be a template-injection hole and worth fixing immediately. It does not. The Responses handler calls chat_request.validate() after conversion at lib/llm/src/http/service/openai.rs:3404, and NvCreateChatCompletionRequest::validate already calls validate::validate_chat_template_args. I ran it against current main to be sure:

POST-shaped NvCreateResponse with chat_template_args = {"chat_template": "{% for _ in range(10**9) %}x{% endfor %}"}
  -> converted to NvCreateChatCompletionRequest
  -> validate() = Err("`chat_template` is not supported inside `chat_template_args`")

So the rejection you wanted is in effect on /v1/responses today. Your validate_response_chat_template_args still differs in that it fails earlier, on the Responses request, which would produce a Responses-shaped error rather than one raised after conversion. That is a real if small difference and I did not want to wave it away as redundant on your behalf, so I will leave it to you and the maintainers whether it is worth keeping as a focused follow-up.

Sorry for the duplicated effort.

[rmccorm4]

Superseded by #13624