fix(responses): HERMES_DISABLE_REASONING_INCLUDE env-var escape hatch - #25189
Open
nnnet wants to merge 15 commits into
Open
fix(responses): HERMES_DISABLE_REASONING_INCLUDE env-var escape hatch#25189nnnet wants to merge 15 commits into
nnnet wants to merge 15 commits into
Conversation
Two related issues in the gateway voice-input path:
1. After ``_enrich_message_with_transcription`` the gateway also sent a
hardcoded English notice via ``_stt_adapter.send()``. TTS would read
it aloud while the LLM produced its own (correctly localized) reply
from the enriched prompt — two messages, wrong language, double UX.
2. The enrichment templates themselves talked extensively about STT
setup and pretended that a separate help message had been sent:
[The user sent a voice message but I can't listen to it right now
— no STT provider is configured. A direct message has already
been sent to the user with setup instructions. You have a skill
called hermes-agent-setup that can help users configure ...]
That text becomes part of the persisted conversation history. Once
it appears in any past turn, the LLM keeps the topic alive: every
subsequent voice message — even one transcribed successfully —
provokes a reply about "configuring STT" instead of an answer to
what the user actually said. Observed in production with
``gpt-5-nano``: after a single STT failure the assistant volunteered
Whisper / Vosk setup instructions on every voice turn for the rest
of the session.
Fix:
- Drop the standalone hardcoded send (lines 6802-6829).
- Rewrite the four enrichment templates to be minimal and neutral:
success becomes a plain quoted line; every failure branch becomes a
single bracketed neutral marker. No mentions of providers, setup,
skills, or "a message has been sent".
- Move the operator-facing failure cause to a ``logger.info`` line so
it stays diagnosable in container logs without leaking into the
LLM-visible prompt.
Adds ``.github/workflows/sync.yml`` that fast-forwards the fork's
``main`` branch from ``NousResearch/hermes-agent:main`` every hour and
on manual ``workflow_dispatch``.
Uses GitHub's ``/repos/{owner}/{repo}/merge-upstream`` REST endpoint,
which performs a fast-forward when there are no conflicts and reports
``merge_type=none`` (with an "up-to-date" message) when the fork is
already in sync. Real merge conflicts surface as ``merge_type=none``
with a different message and fail the workflow — the run log shows the
full API response for triage.
No personal access token (PAT) is required: the default
``GITHUB_TOKEN`` issued to the workflow has ``contents: write`` on
this fork (set explicitly in the workflow's ``permissions:`` block)
and the ``merge-upstream`` endpoint is designed for exactly this case.
The workflow is gated by ``if: github.repository != 'NousResearch/hermes-agent'``
so it is a no-op if this file ever lands in the canonical upstream.
The default TTS voice (``en-US-AriaNeural``, English-only) silently
mangles non-English replies. Edge-tts has no language auto-detection:
it tries to read Russian/Chinese/Japanese/Korean/Arabic text using
English phonemes, drops characters it cannot map, and produces a clip
that matches no language well. Users see a long, on-topic text reply
and hear a short, garbled, partly-English audio clip — the two
disagree, and the audio sometimes contains only the *Latin* fragments
of the text expanded according to English orthographic rules.
Concrete production case (gpt-5-nano, Russian voice mode):
Text reply: "Анекдот: Почему программисты путают Хэллоуин и
Рождество? Потому что Oct 31 = Dec 25."
Audio: "October 31st equals December 25th."
Fix this on two fronts:
1. **Multilingual default**: switch ``DEFAULT_EDGE_VOICE`` to
``en-US-AvaMultilingualNeural``. This single voice handles ~50
languages with mid-sentence switching; it is the safest out-of-box
choice and immediately resolves the symptom above for users who
never touch the TTS config. ``hermes setup`` now writes the
multilingual voice into new ``~/.hermes/config.yaml`` files.
2. **Per-language voice routing**: a new optional
``tts.<provider>.voice_by_language`` config block routes each TTS
call to a voice that matches the detected script of the text. For
users who want native single-language voices (e.g.
``ru-RU-SvetlanaNeural`` for Russian, ``zh-CN-XiaoxiaoNeural`` for
Chinese), this is opt-in and 100% backward compatible.
Detection is a tiny Unicode-block heuristic — no new pip
dependency. It identifies the five non-Latin scripts where an
English voice produces total garbage (ru/zh/ja/ko/ar). Anything
else (English/German/Spanish/French/Polish/...) falls back to the
default voice, which is now multilingual.
3. **Environment-variable overrides**: every module-level default in
``tools/tts_tool.py`` (provider names, model ids, voice ids, base
URLs, per-provider max-text-length caps, language-detection
threshold, Gemini sample rate / channel layout, etc.) is now read
through ``get_env_value()`` with ``HERMES_TTS_*`` env vars. Hard
defaults remain as the second argument so the module still works
without any env-var configuration.
Wired into ``_generate_edge_tts`` only; other cloud providers
(OpenAI/Gemini/ElevenLabs/Mistral) already use multilingual models and
do not need per-call voice swapping. Local providers
(piper/neutts/kittentts) tie voice and language at the model level
and are out of scope for this change.
The Docker image and the base pip install intentionally exclude the
heavy ``[voice]`` extra (faster-whisper pulls in ctranslate2 +
onnxruntime), and ``LAZY_DEPS["stt.faster_whisper"]`` is declared in
``tools/lazy_deps.py`` for exactly this scenario. However, no call site
references that key, so a user who sets ``stt.provider: local`` always
hits "STT provider 'local' configured but unavailable" — the lazy-install
path is wired up everywhere except the one place it's needed.
Mirror the pattern used by ``gateway/platforms/{telegram,slack,...}.py``
and other ``tools/`` modules: when the user has explicitly opted into
local STT and the wheel isn't present, call
``lazy_deps.ensure("stt.faster_whisper", prompt=False)``, re-check the
import, then continue.
Auto-detect (no explicit ``stt.provider``) deliberately does NOT
lazy-install — that would impose a multi-MB native-wheel download on
users who never asked for local STT. Only the explicit ``provider:
local`` path triggers it.
When ``model.base_url`` is set to a native vendor endpoint (e.g. ``https://api.openai.com/v1``) and ``model.provider`` is left at ``auto``, ``hermes_cli.auth.resolve_provider()`` falls through to its ``"openrouter"`` default — which is correct for credential resolution but produces the misleading user-facing label "(openrouter)" in the context-compression warning even though the request never touches OpenRouter. Translate the displayed ``_main_provider`` label from the live ``self.base_url`` when it points at a known vendor host. Routing is unchanged — this is cosmetic only, but actively confuses users trying to debug provider issues when the label disagrees with the URL. Uses ``base_url_host_matches`` so subdomain-spoof URLs like ``api.openai.com.attacker.com`` are not mislabelled as openai.
When ``hermes_cli.auth.resolve_provider()`` returns ``"openrouter"`` (its catch-all for ``OPENAI_API_KEY``-only configs) but the live ``model.base_url`` is set to a native vendor host such as ``https://api.openai.com/v1``, requests bypass the aggregator and go straight to the vendor — which rejects the aggregator-style ``openai/gpt-4o-mini`` slug with: HTTP 400: The requested model 'openai/gpt-4o-mini' does not exist The existing ``run_agent.HermesAgent`` constructor only calls ``normalize_model_for_provider()`` when the provider is **not** an aggregator (see L1320), so the prefix survives. Extend that branch: when the provider is an aggregator AND the base_url matches a known native vendor host, strip the matching ``<vendor>/`` prefix from ``self.model`` before any kwargs are built. Uses ``base_url_host_matches`` so subdomain-spoof URLs cannot bypass the guard. Only the prefix is touched — the rest of the model id flows through unmodified.
The Responses API rejects ``include: ["reasoning.encrypted_content"]``
on non-reasoning models with:
HTTP 400: Encrypted content is not supported with this model.
param='include', code='model_not_found'
Users targeting a non-reasoning OpenAI model (e.g. ``gpt-4o-mini`` via
``base_url: https://api.openai.com/v1``) currently have no way to opt
out other than switching to a reasoning model.
Add ``HERMES_DISABLE_REASONING_INCLUDE=true`` (~/.hermes/.env) that
suppresses both the ``include`` parameter and the ``reasoning`` block
from the outgoing Responses-API kwargs. Default behavior — sending
``include=["reasoning.encrypted_content"]`` on every Responses-API call
— is preserved when the env var is unset or falsy.
Touches two call sites:
- ``agent/transports/codex.py`` — main agent's Responses API transport.
- ``agent/auxiliary_client.py:690`` — Codex auxiliary client path.
A long-term fix is automatic detection of the reasoning family in
``agent/model_metadata.py``, but the existing tests pin the current
unconditional behavior, so the env-var escape hatch is the smallest
change that addresses the user-visible breakage without revising
~30 pinned test expectations.
Collaborator
|
Resubmission of #25102 (closed without merge). Competing PRs for the same root cause (#23450):
This PR adds an env-var escape hatch ( |
Contributor
|
Thanks for documenting a concrete Responses-API failure and identifying both main and auxiliary construction paths. Problems
Suggested changes
Automated hermes-sweeper review. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
agent/transports/codex.pyandagent/auxiliary_client.pyunconditionally seton every Responses-API call whenever
reasoning_enabledis true (the default). The parameter is only valid for reasoning-capable models (o1/o3/o4-mini/gpt-5*). For every other OpenAI model the API returns:Users running Hermes against
api.openai.comwith a non-reasoning model (e.g.gpt-4o-minifor cost reasons) currently have no way to opt out — every request fails.Add an env-var escape hatch:
HERMES_DISABLE_REASONING_INCLUDE=truesuppresses bothincludeandreasoningin the outgoing Responses-API kwargs. Default behaviour is preserved when the env var is unset / falsy.Repro (end-to-end on the fork's Docker setup)
~/.hermes/.env:OPENAI_API_KEY=sk-...~/.hermes/config.yaml:docker compose restart gatewaycurl -s -X POST http://127.0.0.1:8642/v1/responses -H "Authorization: Bearer $API_SERVER_KEY" -H "Content-Type: application/json" -d '{"model":"hermes-agent","input":"Say OK","conversation":"repro","stream":false}'Observed:
Expected: 200 OK with a normal reply.
Root cause
agent/transports/codex.py:109,124andagent/auxiliary_client.py:690setinclude=["reasoning.encrypted_content"]wheneverreasoning_enabledisTrue(the default). There is no model-family check before the include is added, so non-reasoning models hit the OpenAI 400.Fix
Add
_reasoning_include_disabled()inagent/transports/codex.py:Gate both call sites (codex transport
build_kwargs, auxiliary client codex-responses path) onnot _reasoning_include_disabled(). When the flag is on, bothkwargs["include"]andkwargs["reasoning"]are skipped — equivalent to running without the reasoning add-ons.Why an env var, not auto-detection of the reasoning family?
Roughly 30 tests across
tests/agent/test_auxiliary_client.py,tests/agent/transports/test_codex_transport.py,tests/run_agent/test_provider_parity.py, and friends prescriptively assertinclude == ["reasoning.encrypted_content"]whenever a reasoning code path runs. Addingagent.model_metadata.is_reasoning_model()and switching the transports to consult it would require updating every one of those expectations — a much larger refactor and a riskier PR.The env var is opt-in and 100% backward-compatible, which lets users unblock themselves today without us having to coordinate a test-rewrite. A follow-up PR can introduce automatic detection in
agent/model_metadata.pyand deprecate the env var once the rewrite lands.Testing
End-to-end verified on the fork's Docker setup:
Test 1 — sanity (default reasoning model, env unset)
model.default: gpt-5-nano(reasoning-capable),HERMES_DISABLE_REASONING_INCLUDEunset./v1/responses→ 200,"OK"returned, no regression.include=["reasoning.encrypted_content"]is still sent.Test 2.1 — repro of the bug (non-reasoning model, env unset)
model.default: gpt-4o-mini(non-reasoning),HERMES_DISABLE_REASONING_INCLUDEunset./v1/responses→Test 2.2 — fix verified (non-reasoning model, env set)
HERMES_DISABLE_REASONING_INCLUDE=trueexported into the gateway container.docker exec hermes printenv HERMES_DISABLE_REASONING_INCLUDE→true./v1/responses→HTTP 400/Encrypted contententries during the test.Plus the in-process unit-style check from the original branch:
_reasoning_include_disabled()returnsFalseby default andTruefor any of{"1", "true", "yes", "on"}. With the env set,ResponsesApiTransport.build_kwargs("gpt-4o-mini", ...)returns kwargs that contain noincludeand noreasoningblock.Breaking changes
None. Default behaviour is unchanged. The env var is opt-in.
Follow-up
A separate PR can introduce
agent.model_metadata.is_reasoning_model(model_id) -> booland switch the transports to consult it automatically, at which point the env var becomes a forced override (e.g. for users behind proxies that munge requests). I'd like to land this escape hatch first because it unblocks anyone hitting the bug today without touching the prescriptive tests.PR by Claude Code on behalf of @nnnet. Tracks internal bug tracker entry BUG-2.