Skip to content

fix(deepseek): make extra_body.thinking opt-in to unblock V4 first call (#30818) - #30832

Open
xxxigm wants to merge 2 commits into
NousResearch:mainfrom
xxxigm:fix/30818-deepseek-v4-400
Open

fix(deepseek): make extra_body.thinking opt-in to unblock V4 first call (#30818)#30832
xxxigm wants to merge 2 commits into
NousResearch:mainfrom
xxxigm:fix/30818-deepseek-v4-400

Conversation

@xxxigm

@xxxigm xxxigm commented May 23, 2026

Copy link
Copy Markdown
Contributor

What does this PR do?

Fixes #30818: provider: deepseek with any V4-family model (deepseek-v4-flash, deepseek-v4-pro, …) returns HTTP 400 on the very first message — not on multi-turn, not on tool calls, just a plain "hello" against https://api.deepseek.com. curl with the same key/model succeeds, and switching to provider: custom + api_mode: openai-completions (which bypasses the profile entirely) also succeeds. That points squarely at the profile's per-request extras.

The DeepSeek profile was unconditionally injecting extra_body["thinking"] = {"type": "enabled"} for every V4-family model regardless of the user's reasoning_config. The OpenAI SDK unwraps extra_body into the top-level request body, so DeepSeek's V4 native API saw an unrecognized top-level thinking field and rejected the request with 400.

This PR makes the extra_body.thinking injection opt-in — only forwarded when the user explicitly sets reasoning_config.enabled — so the default path emits no extras and DeepSeek's server-side defaults apply (matching the curl behaviour). Users who explicitly want the Kimi-style toggle still get it.

Related Issue

Closes #30818

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)
  • ✨ New feature (non-breaking change that adds functionality)
  • 🔒 Security fix
  • 📝 Documentation update
  • ✅ Tests (adding or improving test coverage)
  • ♻️ Refactor (no behavior change)
  • 🎯 New skill (bundled or hub)

Changes Made

  • plugins/model-providers/deepseek/__init__.py (+88, −22):

  • tests/providers/test_deepseek_profile_30818.py (+365, new file) — 28 tests across five classes:

    • TestDefaultPathNoThinkingInjected (7): the exact scenario the bug reporter hit. Parametrised over deepseek-v4-flash, -v4-pro, -v4-experimental, -v5-flash (forward compat) and deepseek-reasoner; also covers the reasoning_config={} shape that hermes_cli/config.py produces when the reasoning: section is empty, and pins independence between reasoning.effort and the thinking toggle.
    • TestExplicitThinkingOptIn (8): legacy Kimi-style payload still works for users who configured reasoning_config.enabled explicitly: enabled=True{"type": "enabled"}, enabled=False{"type": "disabled"} AND reasoning_effort is dropped (no point picking an effort when thinking is off), enabled+effort forwards both, enabled-without-effort lets the server pick its default, invalid effort levels are silently dropped.
    • TestV3AndNonThinkingModelsUnchanged (5): V3 chat / coder models stay untouched in BOTH default and opt-in paths.
    • TestProfileMetadataUnchanged (5): static pins so a refactor that breaks profile identity (rename, deletion, base_url change, alias loss, _model_supports_thinking mis-classification) fails here instead of silently breaking every DeepSeek user.
    • TestSourceGuards (2): structural pin — the extra_body['thinking'] assignment must be preceded by the _user_opted_into_thinking_config guard within a 200-char window, so a future refactor that moves the assignment outside the guard would fail here even if it slipped past the behavioural tests above. Also pins that the docstring keeps citing [Bug]: DeepSeek provider returns HTTP 400 on first message with deepseek-v4-flash (v0.14.0) #30818.

How to Test

  1. Run the new regression suite on its own:

    scripts/run_tests.sh tests/providers/test_deepseek_profile_30818.py -v

    Expected: 28 passed.

  2. Run the wider provider + deepseek + runtime sweep to confirm no cross-file regressions:

    scripts/run_tests.sh tests/providers/ \
        tests/run_agent/test_deepseek_reasoning_content_echo.py \
        tests/hermes_cli/test_runtime_provider_resolution.py \
        tests/agent/test_auxiliary_named_custom_providers.py

    Expected: 314 passed.

  3. Manual reproduction of the original bug (mirrors the issue body):

    model:
      default: deepseek-v4-flash
      provider: deepseek
      base_url: https://api.deepseek.com
      api_key: sk-xxxxx

    Run hermes and send "hello".

    • Before this PR: BadRequestError [HTTP 400] on the very first message; the agent aborts with Non-retryable client error (HTTP 400).
    • After this PR: 200 OK; the assistant replies normally. Same config that previously required the provider: custom + api_mode: openai-completions workaround now works directly with provider: deepseek.
  4. Opt-in regression check (preserves the pre-fix behaviour for users who depend on it):

    reasoning:
      enabled: true
      effort: high

    With this config the request body still contains "thinking": {"type": "enabled"} and "reasoning_effort": "high", exactly as before.

Checklist

Code

  • I've read the Contributing Guide
  • My commit messages follow Conventional Commits (fix(deepseek): …, test(deepseek): …)
  • I searched for existing PRs to make sure this isn't a duplicate
  • My PR contains only changes related to this fix (no unrelated commits)
  • I've run scripts/run_tests.sh tests/providers/test_deepseek_profile_30818.py and all tests pass (28/28)
  • I've added tests for my changes (28 new test cases across 5 classes)
  • I've tested on my platform: macOS 15.2 (Darwin 24.6.0), Python 3.12

Documentation & Housekeeping

  • I've updated relevant documentation (README, docs/, docstrings) — module docstring rewritten with full History / Current behavior sections citing [Bug]: DeepSeek provider returns HTTP 400 on first message with deepseek-v4-flash (v0.14.0) #30818
  • I've updated cli-config.yaml.example if I added/changed config keys — N/A (no new config keys; the existing reasoning.enabled / reasoning.effort keys retain their semantics)
  • I've updated CONTRIBUTING.md or AGENTS.md if I changed architecture or workflows — N/A
  • I've considered cross-platform impact (Windows, macOS) per the compatibility guide — pure data-shape change in profile, no platform-specific surface; tests are hermetic (mocked transport, no real network)
  • I've updated tool descriptions/schemas if I changed tool behavior — N/A

Screenshots / Logs

$ scripts/run_tests.sh tests/providers/test_deepseek_profile_30818.py -v
28 passed in 0.11s

$ scripts/run_tests.sh tests/providers/ \
    tests/run_agent/test_deepseek_reasoning_content_echo.py \
    tests/hermes_cli/test_runtime_provider_resolution.py \
    tests/agent/test_auxiliary_named_custom_providers.py
314 passed in 2.23s

xxxigm added 2 commits May 23, 2026 15:36
…ll (NousResearch#30818)

``provider: deepseek`` with any V4 family model (``deepseek-v4-flash``,
``deepseek-v4-pro``, …) returns HTTP 400 on the very first message —
not on multi-turn, not on tool calls, just a plain "hello" against
``https://api.deepseek.com``. ``curl`` with the same key/model
succeeds, and switching to ``provider: custom`` + ``api_mode:
openai-completions`` — which bypasses this profile entirely — also
succeeds. That points squarely at the profile's per-request extras.

The profile was unconditionally injecting

    extra_body["thinking"] = {"type": "enabled" | "disabled"}

for every V4-family model regardless of the user's
``reasoning_config``. The OpenAI SDK unwraps ``extra_body`` into the
top-level request body, so DeepSeek's V4 native API saw an
unrecognized top-level ``thinking`` field and rejected the request.

The reasoning-content echo concern that originally motivated injecting
``extra_body.thinking`` (NousResearch#15700, NousResearch#17212, NousResearch#17825) is already covered on
the RESPONSE side: ``build_assistant_message`` pads assistant tool-call
messages with ``reasoning_content`` whenever the active provider is
DeepSeek thinking mode (see ``_needs_deepseek_tool_reasoning`` in
``run_agent.py``). No request-side flag is needed to keep that path
working.

Behavior after this change:

* Default (``reasoning_config`` not set, or set without an ``enabled``
  key) — emit nothing. DeepSeek applies its server-side defaults and
  the request succeeds. Fixes the symptom in NousResearch#30818.
* Explicit opt-in (``reasoning_config={"enabled": True/False, ...}``) —
  still forward the Kimi-style ``extra_body.thinking`` payload so the
  pre-NousResearch#30818 contract is preserved for users who actually configured
  it.
* ``reasoning_effort`` — forwarded when the user sets
  ``reasoning_config.effort``, regardless of whether they opted into
  the thinking toggle. Effort and thinking are now independent
  parameters (they were coupled before only by accident of the
  unconditional injection above).

Non-thinking models (``deepseek-chat`` / V3 family) remain untouched
no-ops so V3 wire format is unchanged.
…arch#30818)

28 tests across five classes:

* ``TestDefaultPathNoThinkingInjected`` (7) — the exact scenario the
  bug reporter hit: a V4 model with no ``reasoning_config`` (or with
  ``reasoning_config={}``, the shape ``hermes_cli/config.py`` produces
  when the ``reasoning:`` section is empty) emits NO
  ``extra_body.thinking`` and NO ``reasoning_effort``. Parametrised
  over ``deepseek-v4-flash``, ``-v4-pro``, ``-v4-experimental``,
  ``-v5-flash`` (forward compat for the next-gen family) and the
  legacy ``deepseek-reasoner``. Also pins the independence of
  ``reasoning_config.effort`` from the ``thinking`` toggle: setting
  effort alone forwards ``reasoning_effort`` but must not silently
  re-enable the smoking-gun ``extra_body.thinking`` field.

* ``TestExplicitThinkingOptIn`` (8) — the legacy Kimi-style payload
  still works for users who configured ``reasoning_config.enabled``
  explicitly: enabled-True → ``{"type": "enabled"}``, enabled-False →
  ``{"type": "disabled"}`` AND ``reasoning_effort`` is dropped (no
  point picking an effort when thinking is off), enabled-True with
  effort forwards both, enabled-True without effort lets the server
  pick its default, invalid effort levels are silently dropped.

* ``TestV3AndNonThinkingModelsUnchanged`` (5) — V3 chat / coder
  models stay untouched in BOTH the default path and the opt-in
  path (a V3 user with an explicit thinking opt-in must not get
  ``extra_body.thinking`` either, because V3 has no thinking mode
  and forwarding the field would re-introduce the same class of
  HTTP 400 the fix avoids for V4).

* ``TestProfileMetadataUnchanged`` (5) — static pins so a refactor
  that breaks the profile identity (rename, deletion, base_url
  change, alias loss, ``_model_supports_thinking`` mis-classification)
  fails here instead of silently breaking every DeepSeek user.

* ``TestSourceGuards`` (2) — read the profile source and assert the
  structural contract: ``_user_opted_into_thinking_config`` still
  exists and the ``extra_body['thinking']`` assignment is preceded by
  it within a 200-char window. A future refactor that moves the
  assignment outside the guard would fail here even if it happened
  to slip past the behavioural tests above. Also pins that the
  docstring keeps citing NousResearch#30818.
@alt-glitch alt-glitch added type/bug Something isn't working P3 Low — cosmetic, nice to have comp/plugins Plugin system and bundled plugins provider/deepseek DeepSeek API labels May 23, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

Fix PR for #30818. Related to #26648 (merged DeepSeekProfile) and #28945 (DeepSeek V4 reasoning support).

@teknium1 teknium1 added sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jul 13, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/plugins Plugin system and bundled plugins P3 Low — cosmetic, nice to have provider/deepseek DeepSeek API sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: DeepSeek provider returns HTTP 400 on first message with deepseek-v4-flash (v0.14.0)

3 participants