Repository navigation
fix(proxy): preserve configured chat template defaults - #1725
Conversation
| kwargs["messages"] = prepared_messages | ||
|
|
||
| defaults = dict( | ||
| getattr(_engine.config.agent, "chat_template_kwargs", {}) if _engine else {} |
There was a problem hiding this comment.
The advertised configured defaults are unreachable through supported configuration paths. AgentConfig does not declare chat_template_kwargs, so structured YAML and CLI loading reject the key, while dataclass RPC serialization drops dynamically attached attributes. This lookup therefore resolves to an empty mapping in production even though the new test injects the missing field through SimpleNamespace.
There was a problem hiding this comment.
Fixed in e27d251, included in the new head f332351. The field is now declared on AgentConfig and both CLI references are regenerated, so it is part of structured configuration and dataclass serialization rather than a dynamically attached attribute. I verified real AgentConfig construction through OmegaConf, in addition to the existing proxy suite (41 passed). The previous mock-based check did hide this omission; no additional UT file has been added.
| defaults = dict( | ||
| getattr(_engine.config.agent, "chat_template_kwargs", {}) if _engine else {} | ||
| ) | ||
| extra_body = dict(kwargs.get("extra_body") or {}) |
There was a problem hiding this comment.
This assumes extra_body remains nested in the request body. The pinned OpenAI SDK flattens extra_body into top-level JSON fields, so only chat_template_kwargs is recovered here and every unrelated extension field is later removed by the unsupported-argument filter. The claimed preservation therefore does not hold for the standard client path.
There was a problem hiding this comment.
Confirmed the SDK flattening with an actual AsyncOpenAI serialized request passed through the proxy request-preparation function. I narrowed the PR description and scope: this change reconstructs chat_template_kwargs only, not arbitrary SDK extension fields. Arguments supported by the AReaL client signature retain their existing forwarding behavior; unsupported top-level fields remain filtered. Other fields already present in an explicitly nested raw extra_body are left untouched, but that is not a generic preservation guarantee for the standard SDK wire format. I am keeping generic extension passthrough outside this PR rather than bypassing the existing argument filter.
| request_template = { | ||
| **session_template, | ||
| **(extra_body.get("chat_template_kwargs") or {}), | ||
| **(kwargs.pop("chat_template_kwargs", None) or {}), |
There was a problem hiding this comment.
Thinking aliases are normalized against configured defaults but not across the nested extra_body and top-level request sources. When those two request layers use different aliases, both values survive the merge; downstream code prioritizes enable_thinking, so the higher-priority request setting can be ignored.
There was a problem hiding this comment.
Fixed in e27d251. Each source layer now replaces the entire lower-priority thinking-alias family when it supplies a non-None thinking value, rather than only overriding identical keys. Explicit session thinking pins are reapplied afterward as before. False/off remain valid overrides, while None neither clears a valid lower-layer setting nor creates a session pin. One-off checks covered the cross-layer alias conflict, None/False handling, session pinning, and unchanged input dictionaries; the SDK-flattened template path was also verified.
Integrate current main without rewriting published history.
Description
Follow-up to #1702 (Arena proxy-based rollouts).
Declare
AgentConfig.chat_template_kwargsso structured configuration anddataclass serialization carry proxy chat-template defaults. Regenerate both CLI
references alongside the schema change.
Merge configured defaults, session metadata, and request template settings.
Valid thinking aliases override the whole alias family from lower-priority
sources;
Noneis not an override, whileFalseandoffare preserved. Anexplicit session thinking mode remains authoritative.
The OpenAI SDK flattens
extra_bodyinto top-level JSON. This PR reconstructschat_template_kwargsfrom that form and also handles explicitly nested rawrequests. It does not provide generic passthrough of arbitrary SDK extension
fields. Supported client arguments keep their existing path; unsupported fields
remain filtered. Other fields already inside a raw nested
extra_bodyare notmodified by the template merge.
No legacy group or reward-normalization compatibility flags are included.
Related Issue
Follow-up to #1702. No separate issue.
Type of Change
Checklist
pre-commit run --all-files)main/review-prcommand/create-prBreaking Change Details (if applicable): None.
Additional Context
The additional UT file was removed at the author's request. Validation includes:
pytest -q tests/experimental/openai/test_proxy_rollout_server.py: 41 passed.AgentConfigand OmegaConf for configuration,thinking precedence,
None/False, session pinning, and input non-mutation.verifying flattened template settings, supported arguments and filtering of
unsupported extensions. No external model endpoint was called.
formatting passed. Lint, formatting and license checks pass. Local full-tree
hooks remain limited by Python 3.10 and the installed Torch's FSDP API; the
local CLI-generation hook has the same import limitation.
No full distributed training regression is claimed. Updates are new commits and
ordinary pushes; published history has not been rewritten.