Skip to content

fix(transports): promote Ollama reasoning field to content when content is empty - #46489

Open
liuhao1024 wants to merge 1 commit into
NousResearch:mainfrom
liuhao1024:fix/ollama-reasoning-content-fallback
Open

fix(transports): promote Ollama reasoning field to content when content is empty#46489
liuhao1024 wants to merge 1 commit into
NousResearch:mainfrom
liuhao1024:fix/ollama-reasoning-content-fallback

Conversation

@liuhao1024

Copy link
Copy Markdown
Contributor

What does this PR do?

Promotes Ollama's reasoning field to content when the model returns an empty content string. This fixes Ollama reasoning models (deepseek-r1, qwen3.5, etc.) returning blank responses — the agent loop previously retried 3 times and gave up because it only read content, while Ollama puts the actual output in a separate reasoning field.

Related Issue

Fixes #46131

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)

Changes Made

  • agent/transports/chat_completions.py: Added model_extra fallback for the reasoning field (symmetric with existing reasoning_content fallback for DeepSeek/Moonshot). Added reasoning-to-content promotion when content is empty/blank but reasoning has text.
  • tests/agent/transports/test_chat_completions.py: Added 3 tests — Ollama reasoning promotion, model_extra fallback, and non-override guard.

How to Test

  1. Run pytest tests/agent/transports/test_chat_completions.py -xvs -k "ollama_reasoning or non_empty_content_not_overridden" — all 3 new tests should pass
  2. Run full transport test suite: pytest tests/agent/transports/test_chat_completions.py -x — all 84 tests should pass (zero regression)
  3. With a local Ollama instance running a reasoning model (e.g. deepseek-r1), send a chat message via Hermes — the response should now display correctly instead of appearing blank

Checklist

Code

  • I've read the Contributing Guide
  • My commit messages follow Conventional Commits (fix(scope):, feat(scope):, etc.)
  • I searched for existing PRs to make sure this isn't a duplicate
  • My PR contains only changes related to this fix/feature (no unrelated commits)
  • I've run pytest tests/ -q and all tests pass
  • I've added tests for my changes (required for bug fixes, strongly encouraged for features)
  • I've tested on my platform: macOS

Documentation & Housekeeping

  • I've updated relevant documentation (README, docs/, docstrings) — or N/A
  • I've updated cli-config.yaml.example if I added/changed config keys — or N/A
  • I've updated CONTRIBUTING.md or AGENTS.md if I changed architecture or workflows — or N/A
  • I've considered cross-platform impact (Windows, macOS) per the compatibility guide — N/A (transport-layer change, no platform-specific code)
  • I've updated tool descriptions/schemas if I changed tool behavior — or N/A

Code Intelligence

  • Analyzed: agent/transports/chat_completions.py::normalize_response (callers: 2, flows: 3)
  • Analyzed: agent/transports/types.py::NormalizedResponse.reasoning (callers: 2)
  • Blast radius: LOW — fallback only activates when content is empty AND reasoning has text; existing providers with non-empty content are unaffected
  • Related patterns: mirrors existing refusalcontent promotion (lines 684-699), reasoning_contentprovider_data preservation (lines 654-662)

…nt is empty

Ollama reasoning models (deepseek-r1, qwen3.5, etc.) return content=""
with the actual output in a separate ``reasoning`` field. The transport
only read ``content``, so the agent loop treated every response as empty
and retried 3 times before giving up.

Two changes:
1. Add model_extra fallback for ``reasoning`` (symmetric with existing
   ``reasoning_content`` fallback for DeepSeek/Moonshot).
2. When content is empty/blank but reasoning has text, promote reasoning
   to content so the agent loop receives a usable response.
@alt-glitch alt-glitch added type/bug Something isn't working P3 Low — cosmetic, nice to have comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint provider/ollama Ollama / local models labels Jun 15, 2026

@tonydwb tonydwb left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Code Review Summary

Verdict: Approved

Fix promotes Ollama reasoning field to content when content is empty. Clean fix for an edge case. No security concerns.


Reviewed by Hermes Agent

@teknium1 teknium1 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for tracing the non-streaming reasoning shape and adding focused coverage.

Problems

  • agent/transports/chat_completions.py:707 promotes any blank-content chat_completions response with reasoning into visible content. Current main instead treats structured reasoning with no visible content as a thinking-only turn and continues generation (agent/conversation_loop.py:5011-5040). The shared message builder intentionally keeps reasoning separate because putting it in content leaks it to delivery, transcript, replay, compression, and titles (agent/chat_completion_helpers.py:1082-1092).

Suggested changes

  • Retain the model_extra["reasoning"] extraction, but preserve it as reasoning so the existing continuation path can handle it. If Ollama has a distinct contract where the field is final answer text, make that behavior provider-scoped and cover it with an end-to-end test.

Automated hermes-sweeper review.

# Ollama reasoning models return content="" with the actual output in
# the ``reasoning`` field. Promote reasoning to content so the agent
# loop does not treat it as an empty response and retry 3 times.
if not (isinstance(content, str) and content.strip()) and isinstance(reasoning, str) and reasoning.strip():

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This executes for every OpenAI-compatible chat-completions provider, not just Ollama. Promoting reasoning here bypasses main's thinking-only continuation (agent/conversation_loop.py:5011) and makes hidden reasoning final visible content; please keep this as structured reasoning unless an explicitly scoped Ollama contract proves it is answer text.

@teknium1 teknium1 added sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state 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 14, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint P3 Low — cosmetic, nice to have provider/ollama Ollama / local models 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 sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Fix: Ollama reasoning models return empty content — send reasoning_effort to disable thinking

4 participants