fix(aux): default Codex reasoning effort to medium when extra_body.reasoning.effort is falsy by briandevans · Pull Request #17012 · NousResearch/hermes-agent

briandevans · 2026-04-28T13:19:18Z

Summary

Fall back to "medium" when auxiliary.<task>.extra_body.reasoning.effort is null / "" / 0 instead of forwarding the falsy value to Codex (which 400s).
Brings _CodexCompletionsAdapter.create() (PR feat(aux): translate extra_body.reasoning into Codex Responses API #17004) into parity with agent/transports/codex.py::build_kwargs(), which the new adapter is explicitly documented to mirror.

The bug

PR #17004 (391f1ca) wired extra_body.reasoning through to the Codex Responses API:

# agent/auxiliary_client.py — current
effort = reasoning_cfg.get("effort", "medium")
if effort == "minimal":
    effort = "low"
resp_kwargs["reasoning"] = {"effort": effort, "summary": "auto"}

The two-arg dict.get only fills in the default when the key is absent. If the user has the key but with a falsy value — which is what effort: ~ or effort: (no value) produces in YAML — effort is bound to None / "" and goes out as:

"reasoning": {"effort": null, "summary": "auto"}

The Codex Responses API rejects that with HTTP 400 (Invalid value for parameter "reasoning.effort"), so the auxiliary call fails on a config shape that the main agent accepts cleanly.

The main-agent path the new adapter is mirroring (agent/transports/codex.py:79-90) uses a truthy check:

reasoning_effort = "medium"
...
elif reasoning_config.get("effort"):    # truthy — None/""/0 fall through
    reasoning_effort = reasoning_config["effort"]

so falsy values stay at the "medium" default. The same config that works for the main agent's reasoning_config should work for the auxiliary adapter's extra_body.reasoning.

The fix

One-line change in _CodexCompletionsAdapter.create():

effort = reasoning_cfg.get("effort") or "medium"

The or "medium" mirrors the truthy check from the main-agent transport — None, "", 0 all coalesce to the default rather than getting forwarded to Codex. Truthy strings ("low", "medium", "high", "minimal", etc.) behave exactly as before, and the existing minimal → low clamp still runs after this.

Test plan

Focused regression tests: two new cases in TestCodexAdapterReasoningTranslation cover effort: None and effort: "" and assert the outgoing request is {"effort": "medium", "summary": "auto"}.
Regression guard: with the production fix reverted, both new tests fail with {'effort': None} != {'effort': 'medium'} and {'effort': ''} != {'effort': 'medium'}. With the fix applied, all 11 tests in the class pass.
Adjacent suite: tests/agent/test_auxiliary_client.py — 108 passed.
Transport adjacency: tests/agent/transports/test_codex_transport.py + test_chat_completions.py — 73 passed.

Follows on from feat(aux): translate extra_body.reasoning into Codex Responses API #17004 (just merged in 391f1ca) — the new adapter explicitly documents that it "Mirrors agent/transports/codex.py::build_kwargs()", so any config-handling divergence is a real bug, not just a hypothetical edge.
The 9 tests added in feat(aux): translate extra_body.reasoning into Codex Responses API #17004 cover the fully-omitted, fully-disabled, and valid-effort cases; they don't exercise the falsy-but-present shape that this PR closes.

Copilot

Pull request overview

Fixes Codex auxiliary request construction so extra_body.reasoning.effort defaults to "medium" when the key is present but falsy (e.g., None/""), matching the main Codex transport behavior and avoiding Codex 400s.

Changes:

Coalesce falsy extra_body.reasoning.effort values to "medium" in _CodexCompletionsAdapter.create().
Add regression tests covering effort=None and effort="" to ensure the outgoing reasoning payload is valid.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File	Description
`agent/auxiliary_client.py`	Adjusts reasoning effort defaulting to avoid forwarding invalid falsy values to Codex.
`tests/agent/test_auxiliary_client.py`	Adds focused regression tests for falsy-but-present `effort` values.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

+        """Parity with agent/transports/codex.py::build_kwargs() — falsy
+        ``effort`` (None / empty / 0) keeps the default ``medium`` instead
+        of being forwarded to Codex.  Codex rejects ``{"effort": null}``
+        with HTTP 400 (Invalid value for parameter `reasoning.effort`)."""


briandevans · 2026-04-28T13:26:31Z

@copilot Finding addressed in commit e4f83c01b:

Finding (tests/agent/test_auxiliary_client.py:1642 — docstring lists 0 among falsy values but only None/"" tested): Real gap — the docstring explicitly names 0 as part of the falsy contract that falls back to medium, so the test class should pin that case too. Added test_reasoning_effort_zero_falls_back_to_medium mirroring the structure of the None/"" variants. 12/12 in TestCodexAdapterReasoningTranslation pass.

briandevans · 2026-04-28T13:42:11Z

CI audit — all 35 test job failures on commit e4f83c01b are pre-existing baselines on clean origin/main (391f1ca1f). Zero failures intersect with touched code (agent/auxiliary_client.py::_CodexCompletionsAdapter.create reasoning-effort fallback + tests/agent/test_auxiliary_client.py).

Same baseline set observable on every other PR opened today (#16851, #16902, #16990, …) — test_anthropic_adapter::test_custom_base_url, test_copilot_acp_client::test_read_text_file_redacts_sensitive_content, test_config_env_expansion::*, test_session_split_brain_11016::* (timeouts), test_resolve_path::test_relative_path_uses_terminal_cwd, etc.

Locally on this branch the focused suite is green:

tests/agent/test_auxiliary_client.py -k codex_reasoning -- 5 / 5 pass

including the falsy-effort coverage extension (None, "", 0, False).

…asoning.effort is falsy auxiliary.<task>.extra_body.reasoning, but the new translation path in _CodexCompletionsAdapter.create() reads the effort with ``reasoning_cfg.get("effort", "medium")``. That returns the configured value verbatim when the key is present, so ``effort: null`` / ``effort: ""`` (both common YAML shapes) flow through as ``{"effort": null, "summary": "auto"}`` and Codex rejects the request with "Invalid value for parameter ``reasoning.effort``". agent/transports/codex.py::build_kwargs() — which the new adapter is documented to mirror — uses a truthy check (``elif reasoning_config.get("effort"):``) so the same falsy values keep the "medium" default. Switch the auxiliary adapter to the same ``or "medium"`` truthy form so identical config produces identical requests on both paths. - [x] Two new regression tests cover ``effort: None`` and ``effort: ""`` and assert the request goes out as ``{"effort": "medium", "summary": "auto"}``. - [x] Old behaviour fails the new tests (``{'effort': None} != {'effort': 'medium'}``); fixed behaviour passes all 11 tests in the ``TestCodexAdapterReasoningTranslation`` class. - [x] Adjacent suites green: ``tests/agent/test_auxiliary_client.py`` (108 passed) and ``tests/agent/transports/test_codex_transport.py + test_chat_completions.py`` (73 passed). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Copilot review on PR NousResearch#17012 noted the docstring/comment lists `0` among the falsy effort values that fall back to `medium`, but the existing regression tests only cover `None` and `""`. Add the third case to lock in the full contract. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

briandevans · 2026-04-29T18:16:00Z

Rebased onto current origin/main (5a61c116e) — was 183 commits stale and conflicted with the new TestVisionAutoSkipsKimiCoding class added by #17076.

Conflict resolution: kept all four kimi-coding vision-skip tests intact and inserted the three reasoning-effort fallback tests (null / "" / 0) inside TestCodexAdapterReasoningTranslation where they belong, before the new vision class. No code-path changes.

Verified: tests/agent/test_auxiliary_client.py::TestCodexAdapterReasoningTranslation 12/12 passing locally (10 pre-existing + 3 new), TestVisionAutoSkipsKimiCoding 4/4 passing.

Copilot review on PR #17012 noted the docstring/comment lists `0` among the falsy effort values that fall back to `medium`, but the existing regression tests only cover `None` and `""`. Add the third case to lock in the full contract. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Copilot review on PR NousResearch#17012 noted the docstring/comment lists `0` among the falsy effort values that fall back to `medium`, but the existing regression tests only cover `None` and `""`. Add the third case to lock in the full contract. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Copilot AI review requested due to automatic review settings April 28, 2026 13:19

Copilot started reviewing on behalf of briandevans April 28, 2026 13:19 View session

Copilot AI reviewed Apr 28, 2026

View reviewed changes

alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/agent Core agent loop, run_agent.py, prompt builder labels Apr 28, 2026

briandevans and others added 2 commits April 29, 2026 11:14

briandevans force-pushed the fix/aux-codex-reasoning-effort-defaults-17004 branch from e4f83c0 to 0185fbf Compare April 29, 2026 18:15

teknium1 mentioned this pull request May 5, 2026

fix(aux): default Codex reasoning effort to medium when falsy (salvage #17012) #20210

Merged

teknium1 closed this in #20210 May 5, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(aux): default Codex reasoning effort to medium when extra_body.reasoning.effort is falsy#17012

fix(aux): default Codex reasoning effort to medium when extra_body.reasoning.effort is falsy#17012
briandevans wants to merge 2 commits into
NousResearch:mainfrom
briandevans:fix/aux-codex-reasoning-effort-defaults-17004

briandevans commented Apr 28, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

briandevans commented Apr 28, 2026

Uh oh!

briandevans commented Apr 28, 2026

Uh oh!

briandevans commented Apr 29, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

briandevans commented Apr 28, 2026

Summary

The bug

The fix

Test plan

Related

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

briandevans commented Apr 28, 2026

Uh oh!

briandevans commented Apr 28, 2026

Uh oh!

briandevans commented Apr 29, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants