Skip to content

feat(transport): config-driven providers.<name>.extra_body override - #21554

Closed
Abd0r wants to merge 3 commits into
NousResearch:mainfrom
Abd0r:feat/configurable-provider-extra-body
Closed

Abd0r wants to merge 3 commits into
NousResearch:mainfrom
Abd0r:feat/configurable-provider-extra-body

Conversation

@Abd0r

@Abd0r Abd0r commented May 7, 2026

Copy link
Copy Markdown
Contributor

What does this PR do?

Closes #8160.

OpenAI-compatible endpoints regularly take non-standard request fields (enable_thinking, top_k, repetition_penalty, vendor-specific options). Hardcoding them per-provider in chat_completions.py doesn't scale; users want a way to pin these once in config.yaml.

Concrete use case from the issue: DashScope-hosted Qwen3 models default to reasoning mode (~3.5s overhead vs ~0.9s on plain calls). DashScope accepts enable_thinking: false as an OpenAI-compat extra_body field to disable it cleanly. Today there's no way to pin that without editing Hermes source.

This PR adds:

providers:
  alibaba-coding-plan:
    extra_body:
      enable_thinking: false
  dashscope:
    extra_body:
      enable_thinking: false

The block is read at request build time and merged into the request's extra_body after profile defaults + caller-level extra_body_additions but before per-call request_overrides, so:

  • Profile defaults (e.g. Nous tags) still apply.
  • User config wins over caller-level additions (the user's "always do this for this provider" intent beats per-callsite defaults).
  • Per-call request_overrides.extra_body still wins (treated as the user's explicit per-call instruction, more specific than standing config).

Related Issue

Closes #8160.

Type of Change

  • ✨ New feature (non-breaking change that adds functionality)
  • 📝 Documentation update
  • ✅ Tests (adding test coverage)

Changes Made

  • agent/transports/chat_completions.py —
    • New module-level helper _load_provider_extra_body_override(provider_name) reads ~/.hermes/config.yaml → providers.<name>.extra_body. Returns {} on any failure (missing config, missing key, non-dict value, load_config exception) so callers can always merge safely.
    • The override merge is wired into BOTH extra_body assembly paths inside build_kwargs: the legacy path (no profile) and the profile-based path. Each path builds extra_body from different sources, so the helper is invoked in each at the right precedence point.
  • tests/agent/transports/test_chat_completions_provider_extra_body.py (new) — 12 tests:
    • 7 unit tests on _load_provider_extra_body_override covering the failure modes (blank name, no providers section, missing entry, missing extra_body, non-dict shape, load_config exception, and the happy-path "returns a copy, not the original" guarantee).
    • 5 integration tests on build_kwargs — the DashScope use case, config-vs-additions precedence, no-emission when nothing is configured, merging with existing profile-driven extra_body (Nous tags), and graceful behavior when the configured provider name doesn't match the call.
  • website/docs/user-guide/configuration.md — new "Provider extra_body" subsection documenting the schema, precedence, and the limitation noted below.

How to Test

pytest tests/agent/transports/test_chat_completions_provider_extra_body.py \
  -o "addopts=-m 'not integration'" -v
# → 12 passed

pytest tests/agent/transports/ tests/providers/ \
  -o "addopts=-m 'not integration'" -q
# → 260+ passed (no regressions)

End-to-end (the DashScope case):

  1. Add to ~/.hermes/config.yaml:
    providers:
      alibaba-coding-plan:
        extra_body:
          enable_thinking: false
  2. Run hermes chat --provider alibaba-coding-plan against a thinking-mode Qwen3 model.
  3. Check that turn latency drops back to non-thinking baseline (~0.9s vs ~3.5s for trivial prompts).
  4. Confirm DashScope is honoring the field via the request log: extra_body should contain enable_thinking: false.

Checklist

Code

  • I've read the Contributing Guide
  • My commit messages follow Conventional Commits (feat(transport):)
  • I searched for existing PRs to make sure this isn't a duplicate
  • My PR contains only changes related to the provider extra_body config override
  • I've run the new tests (12 passed) and the broader transport + provider test suites (260+ passed, no regressions)
  • I've added tests for my changes (12 new tests covering the helper's failure modes, the DashScope use case, precedence, and the merge with profile-driven extra_body)
  • No platform-specific code

Documentation & Housekeeping

  • Updated website/docs/user-guide/configuration.md
  • N/A — no cli-config.yaml.example changes (the new key is documented in configuration.md and is purely additive)
  • N/A — no architectural / workflow changes
  • N/A — config-only feature, platform-neutral
  • N/A — no tool schema changes

Notes for reviewers

  • Out of scope for this PR: the auxiliary client path (get_auxiliary_extra_body() in agent/auxiliary_client.py) is intentionally not changed. It's a separate code path with its own provider-detection mechanics (no current global tracking of the auxiliary provider name) and warrants a follow-up. The primary win for the DashScope use case — long agent turns wasting ~3s per call on thinking mode — lives on the main chat_completions path this PR covers. Mentioned in the docs as a known limitation.
  • Native paths (anthropic_messages, codex_responses) are not affected — they don't use extra_body in the same shape, so a future PR would need a parallel mechanism for them.
  • Precedence design rationale: I considered three orders (config-first, additions-first, request-overrides-first). The chosen order — profile < additions < config < request_overrides — treats the YAML config as the user's standing intent that beats code-default extra_body_additions, while still letting an explicit per-call CLI / API request_overrides win because that's even more specific. Tests pin the precedence so a future refactor can't accidentally flip it.

@alt-glitch alt-glitch added type/feature New feature or request comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint area/config Config system, migrations, profiles P3 Low — cosmetic, nice to have labels May 7, 2026
@gavin-jack

Copy link
Copy Markdown

Update: I've identified a different root cause in my case.

All 6 profiles (default, coding, knowledge, manager, siper, search) share the same HERMES_HOME (~/.hermes) because none of them have hermes_home set individually in their profile config. This means they all write to the same gateway.pid file.

When the default profile's gateway starts with --replace, it reads the shared PID file and finds the PID of another profile's gateway (e.g., coding). It attempts to kill that process. The other profile's gateway doesn't respond expectedly (because it's managed by the profile system, not by --replace), so the new default gateway gives up and returns False → exit 1. systemd sees exit 1 and restarts → loop.

The exit-code fix in this PR would help, but the deeper issue is that --replace should not attempt to replace processes from different profiles. A simple fix is to check the profile name stored in the PID file and only replace if it matches.

Workaround that resolved it: stop and disable the default systemd service (hermes-gateway.service). The 5 agent profiles each manage their own gateway process, so the default systemd instance is redundant and causes conflicts.

@Abd0r

Abd0r commented May 8, 2026

Copy link
Copy Markdown
Contributor Author

Thanks @gavin-jack - I think this comment was meant for #21555 (the gateway --replace / launchd exit-code PR). Worth re-posting there so the deeper fix idea (profile-aware PID file check) doesn't get lost. The PRs are right next to each other so easy to mix up.

@teknium1

Copy link
Copy Markdown
Collaborator

Thanks for the focused configuration-based approach. The underlying need remains real for first-class providers, but the current implementation needs rework against the newer provider-resolution path.

Problems

  • dashscope is an alias of the canonical alibaba profile (plugins/model-providers/alibaba/__init__.py:6-13). The added profile-path lookup keys config by profile.name, so the documented providers.dashscope.extra_body example would not apply after normal alias resolution.
  • Current main resolves configured custom-endpoint request bodies during agent setup (agent/agent_init.py:241-257, :1653-1666) and the transport merges request_overrides.extra_body at agent/transports/chat_completions.py:601-608. Loading config from the transport on every request duplicates that resolution path.
  • providers: is currently the keyed schema for named custom endpoints (hermes_cli/config.py:4888-4947), so a built-in-provider override needs an explicit schema contract rather than ambiguous coexistence.

Suggested changes

  • Resolve and canonicalize built-in provider overrides once upstream, then merge them into request_overrides; add canonical/alias precedence tests.
  • Preserve the existing named-custom-provider behavior documented at website/docs/integrations/providers.md:1197-1223.

This is an automated hermes-sweeper review.

@teknium1 teknium1 added sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-broad Sweeper blast radius: broad — a core path most sessions hit labels Jul 13, 2026
Addresses @teknium1 hermes-sweeper review on NousResearch#21554.

- Resolve first-class / built-in provider extra_body overrides once during
  agent setup into request_overrides (transport already merges those).
- Canonicalize aliases: providers.dashscope.extra_body applies when the
  session provider is alibaba (and vice versa). Exact session key wins
  over canonical/alias siblings.
- Explicit schema: URL-bearing providers.* entries stay named custom
  endpoints; partial entries (extra_body only, no api/base_url/url) are
  the built-in override contract.
- Preserve existing custom-provider merge + caller precedence.
- Docs + alias/precedence/custom-path tests.
@Abd0r

Abd0r commented Jul 14, 2026 •

Copy link
Copy Markdown
Contributor Author

Rework for hermes-sweeper review (local, ready to push)

Addressed the three review points against current main (not the May transport-side loader):

Changes

  1. Resolve once upstream — built-in / first-class providers.<name>.extra_body is merged into request_overrides during agent setup (agent/agent_init.py), same path custom endpoints already use. Transport keeps only its existing request_overrides.extra_body merge (no per-request config reload).
  2. Alias canonicalization — providers.dashscope.extra_body applies when the session provider is alibaba (and the reverse). Lookup order: exact session string → canonical profile name → other aliases. Exact key wins if both exist.
  3. Explicit schema — URL-bearing providers.* entries remain named custom endpoints (custom path). Partial entries with only extra_body (no api/base_url/url) are the built-in override contract. Documented under website/docs/integrations/providers.md.

Tests

tests/agent/test_builtin_provider_extra_body.py + existing custom-provider suite — 21 passed.

Push blocker

Branch tip is rebased on current main. Force-pushing to Abd0r:feat/configurable-provider-extra-body is rejected because the OAuth token lacks the workflow scope (main carries CI workflow history the old tip did not). Local commit: 16e7f225f.

To publish from a machine with workflow scope:

gh auth refresh -h github.com -s workflow
cd /path/to/hermes-agent
git fetch origin
# branch feat/configurable-provider-extra-body @ 16e7f225f
git push fork feat/configurable-provider-extra-body --force-with-lease

H.A.M Fixed by H.A.M · status by H.A.M
(code + tests complete locally; fork force-push blocked on missing workflow OAuth scope)

@Abd0r
Abd0r force-pushed the feat/configurable-provider-extra-body branch from 5d1bd7f to 16e7f22 Compare July 14, 2026 15:14
@Abd0r

Abd0r commented Aug 6, 2026

Copy link
Copy Markdown
Contributor Author

Closing this PR per author request. Thanks!

@Abd0r Abd0r closed this Aug 6, 2026
@Abd0r
Abd0r deleted the feat/configurable-provider-extra-body branch August 6, 2026 18:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/config Config system, migrations, profiles comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint P3 Low — cosmetic, nice to have sweeper:blast-broad Sweeper blast radius: broad — a core path most sessions hit sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades type/feature New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Feature request: Configurable extra_body per provider (enable_thinking=false for DashScope)

4 participants