Skip to content

fix: add NVIDIA API host to reasoning_content allowlist for DeepSeek V4 models - #914

Closed
SnotacusNexus wants to merge 3 commits into
Twigpine:mainfrom
SnotacusNexus:main
Closed

SnotacusNexus wants to merge 3 commits into
Twigpine:mainfrom
SnotacusNexus:main

Conversation

@SnotacusNexus

@SnotacusNexus SnotacusNexus commented Apr 27, 2026 •

Copy link
Copy Markdown

Summary

  • Adds integrate.api.nvidia.com to the reasoning_content allowlist so DeepSeek models (deepseek-ai/deepseek-v4-flash, deepseek-ai/deepseek-v4-pro) hosted on NVIDIA's API don't get a 400 error on tool-call rounds
  • Adds a history-based fallback (hasThinkingBlockInHistory) so unlisted providers that already returned a thinking block continue to echo reasoning_content correctly
  • Adds providerSupportsReasoning() as a unified entry point combining explicit host matching + history detection, with a carve-out for api.openai.com to avoid false positives

Impact

  • user-facing: fixes API Error: 400 {"error":{"message":"The reasoning_content in the thinking mode must be passed back to the API."}} when using DeepSeek models (deepseek-ai/deepseek-v4-flash, deepseek-ai/deepseek-v4-pro) through integrate.api.nvidia.com
  • developer/maintainer: the two-pronged approach (allowlist + history heuristic) means new reasoning providers work without code changes as long as they already returned a thinking block on the first response

Testing

  • bun run build
  • bun run smoke
  • focused tests: bun test src/services/api/openaiShim.test.ts — blocked by pre-existing bun:bundle import issue in slowOperations.ts

Notes

  • provider/model path tested: NVIDIA API (integrate.api.nvidia.com) with deepseek-ai/deepseek-v4-flash and deepseek-ai/deepseek-v4-pro
  • follow-up work or known limitations: none

@SnotacusNexus SnotacusNexus changed the title fix: add NVIDIA API host to reasoning_content allowlist for DeepSeek models fix: add NVIDIA API host to reasoning_content allowlist for DeepSeek V4 models Apr 27, 2026
@jatmn

jatmn commented Apr 27, 2026

Copy link
Copy Markdown
Collaborator

#910 adds per provider/gateway flags for this i think. or at least has the framework to adapt it cleanly.
I would test you change concepts against that pr and see if it works or not.

kevincodex1
kevincodex1 previously approved these changes Apr 27, 2026
gnanam1990
gnanam1990 previously approved these changes Apr 28, 2026

@gnanam1990 gnanam1990 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice — adding NVIDIA's integrate.api.nvidia.com to the allowlist and the unified providerSupportsReasoning() helper is a clean refactor. Explicit api.openai.com carve-out to avoid false positives is a good touch.

LGTM. Once the bun:bundle smoke is unblocked, a small unit test for hasThinkingBlockInHistory would be a nice follow-up.

@SnotacusNexus
SnotacusNexus dismissed stale reviews from kevincodex1 and gnanam1990 via ad5bc2a May 1, 2026 19:33
@kevincodex1

Copy link
Copy Markdown
Member

hello bro @SnotacusNexus we just merged the registry PR for providers, kindly fix conflicts and this is good to go

@Vasanthdev2004 Vasanthdev2004 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Targeted maintainer triage review of the current head ($short).

Verdict: Needs changes

Blocking issue:

  1. GitHub reports this branch as DIRTY / conflicting with main, so it cannot be merged or final-approved as-is. Please rebase or merge latest main, resolve the conflicts, and rerun the relevant checks.

I did not do a full code review because the current branch state is not mergeable. Happy to re-review once the branch is clean.

@jatmn

jatmn commented May 13, 2026

Copy link
Copy Markdown
Collaborator

Verdict

I think we should close this PR rather than merge it.

Findings

1. Blocking: the added Webpack workflow is unrelated and failing

The PR adds a new Node/Webpack workflow that runs npm install and npx webpack across Node 18/20/22. All three jobs from this new workflow are failing, while the existing project checks pass. This is outside the stated bug fix, duplicates CI surface, and should not ride along with an OpenAI shim provider fix.

2. Blocking: the OpenAI shim changes are obsolete after #910

#910 merged descriptor-backed runtime provider metadata and current main now routes OpenAI shim behavior through resolveOpenAIShimRuntimeContext(). That path already sets preserveReasoningContent, requireReasoningContentOnAssistantMessages, reasoningContentFallback: '', DeepSeek-compatible thinking format, and store stripping for model ids containing deepseek. That covers NVIDIA-hosted model names such as deepseek-ai/deepseek-v4-flash and deepseek-ai/deepseek-v4-pro without adding another host allowlist in openaiShim.ts.

3. Design risk: the history-based fallback would send reasoning_content to arbitrary unknown providers

The proposed hasThinkingBlockInHistory() fallback changes the gate from provider/route/model metadata to "any non-OpenAI host if prior history contains a thinking block." The surrounding code explicitly treats this field as provider-specific because strict endpoints can reject unknown fields. A previous thinking block is not proof that the next target accepts OpenAI-shaped reasoning_content; it can also appear after profile switches, resumed conversations, or gateway changes. The descriptor/model capability path from #910 is the safer home for this behavior.

4. Mergeability: GitHub reports the branch as CONFLICTING

Even ignoring the design issues above, this branch is not mergeable into current main.

Recommendation

Close PR #914. If there is still a real NVIDIA DeepSeek V4 gap after #910, the useful follow-up should be a small fresh PR against current main, probably in the integration metadata/catalog area rather than openaiShim.ts, with a focused regression test proving integrate.api.nvidia.com plus deepseek-ai/deepseek-v4-* resolves to reasoning-content preservation. The unrelated Webpack workflow should be dropped entirely.

Notes

@jatmn jatmn closed this May 13, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants