Skip to content

fix(run_agent): gate reasoning.available on reasoning_content, not content - #24566

Closed
wesleysimplicio wants to merge 1 commit into
NousResearch:mainfrom
wesleysimplicio:fix/cx04-issue-24518-reasoning-available-wrong-source
Closed

fix(run_agent): gate reasoning.available on reasoning_content, not content#24566
wesleysimplicio wants to merge 1 commit into
NousResearch:mainfrom
wesleysimplicio:fix/cx04-issue-24518-reasoning-available-wrong-source

Conversation

@wesleysimplicio

@wesleysimplicio wesleysimplicio commented May 12, 2026

Copy link
Copy Markdown
Contributor

What does this PR do?

reasoning.available was emitting the final assistant reply text instead of the model's actual reasoning/chain-of-thought block. External UIs that display a separate "thinking" pane and a message bubble received identical text in both.

Root cause

reasoning.available was emitting the final assistant reply text instead of the model's actual reasoning/chain-of-thought block. External UIs that display a separate "thinking" pane and a message bubble received identical text in both.

Reported in #24518.

Fix

Decoupled the two code paths completely:

  1. Subagent _thinking relay — still reads content (correct: relays what subagents say to parent), gated on _delegate_depth > 0 (unchanged behaviour).
  2. reasoning.available — now reads getattr(msg, "reasoning_content", None) exclusively and fires only when that field is non-empty.
# AFTER
if self.tool_progress_callback:
    if msg.content and getattr(self, '_delegate_depth', 0) > 0:
        _think_text = msg.content.strip()
        _think_text = re.sub(r'</?(?:REASONING_SCRATCHPAD|think|reasoning)>', '', _think_text).strip()
        first_line = _think_text.split('
')[0][:80] if _think_text else ""
        if first_line:
            try:
                self.tool_progress_callback("_thinking", first_line)
            except Exception:
                pass
    _reasoning_text = (getattr(msg, "reasoning_content", None) or "").strip()
    if _reasoning_text:
        try:
            self.tool_progress_callback("reasoning.available", "_thinking", _reasoning_text[:500], None)
        except Exception:
            pass

NormalizedResponse.reasoning_content is already a property backed by provider_data["reasoning_content"] — no changes needed in agent/transports/types.py.

Why this shape

This shape mirrors #29640 so reviewers can quickly compare scope, root cause, fix, tests, and related context without having to decode a custom PR description.

Tests

  • Veja a descrição original preservada abaixo para detalhes de validação, testes e notas de verificação.
Original body

Related PRs / issues

Closes #24518

Original body

Summary

reasoning.available was emitting the final assistant reply text instead of the model's actual reasoning/chain-of-thought block. External UIs that display a separate "thinking" pane and a message bubble received identical text in both.

What Changed

  • Standardized this PR body to the current Hermes Turbo template.
  • Preserved the original detailed description below for reference.

Fluxo

A mudança continua seguindo o fluxo original descrito na seção preservada abaixo, sem ampliar o escopo funcional deste PR.

Visão

A padronização melhora a revisão, reduz ruído e evita deriva de formatação entre PRs abertos.

Test Plan

  • Veja a descrição original preservada abaixo para detalhes de validação, testes e notas de verificação.
Original body

What does this PR do?

Problem

reasoning.available was emitting the final assistant reply text instead of the model's actual reasoning/chain-of-thought block. External UIs that display a separate "thinking" pane and a message bubble received identical text in both.

Reported in #24518.

Root cause

The callback block in run_agent.py shared _think_text (read from assistant_message.content) across two branches via an implicit coupling:

# BEFORE (buggy)
if msg.content and ...:
    _think_text = msg.content.strip()
    ...
    self.tool_progress_callback("reasoning.available", "_thinking", _think_text[:500], None)

reasoning_content (the actual model reasoning block stored in provider_data) was never consulted. When a model returned content = "final reply" with no separate reasoning_content, the event fired carrying "final reply" — duplicating the message bubble text in the reasoning pane.

Fix

Decoupled the two code paths completely:

  1. Subagent _thinking relay — still reads content (correct: relays what subagents say to parent), gated on _delegate_depth > 0 (unchanged behaviour).
  2. reasoning.available — now reads getattr(msg, "reasoning_content", None) exclusively and fires only when that field is non-empty.
# AFTER
if self.tool_progress_callback:
    if msg.content and getattr(self, '_delegate_depth', 0) > 0:
        _think_text = msg.content.strip()
        _think_text = re.sub(r'</?(?:REASONING_SCRATCHPAD|think|reasoning)>', '', _think_text).strip()
        first_line = _think_text.split('
')[0][:80] if _think_text else ""
        if first_line:
            try:
                self.tool_progress_callback("_thinking", first_line)
            except Exception:
                pass
    _reasoning_text = (getattr(msg, "reasoning_content", None) or "").strip()
    if _reasoning_text:
        try:
            self.tool_progress_callback("reasoning.available", "_thinking", _reasoning_text[:500], None)
        except Exception:
            pass

NormalizedResponse.reasoning_content is already a property backed by provider_data["reasoning_content"] — no changes needed in agent/transports/types.py.

Tests

New regression suite tests/run_agent/test_reasoning_available_event_24518.py (5 tests):

Test Verifies
test_event_not_emitted_when_reasoning_content_absent Primary regression: event must NOT fire when reasoning_content is None
test_event_emitted_with_reasoning_content_when_present Event fires with correct payload from reasoning_content, not content
test_event_payload_truncated_at_500_chars Long reasoning_content is truncated to 500 chars
test_subagent_thinking_relay_unaffected _thinking relay for subagents (delegate_depth > 0) still uses content
test_normalized_response_reasoning_content_property NormalizedResponse.reasoning_content property contract

All 5 pass. Full reasoning test suite (54 tests) green.

Visual

flowchart TD
    CB[tool_progress_callback exists?]
    CB -->|yes| DA{delegate_depth > 0
AND content non-empty?}
    DA -->|yes| TH[emit _thinking
first line of content]
    DA -->|no| RC{reasoning_content
non-empty?}
    TH --> RC
    RC -->|yes| RA[emit reasoning.available
with reasoning_content:500]
    RC -->|no| END[no event]
    CB -->|no| END
Loading

Closes #24518

Solution Sketch

  • fix the root cause in the touched subsystem instead of layering a broad workaround around the symptom
  • keep surrounding behavior stable and avoid unrelated refactors while the area is under review
  • prove the change with focused checks on the exact path that regressed

Related Issue

Closes #24518

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

  • preserved the existing technical rationale and validation notes inside the template body
  • scoped this PR description to the implementation already present on the branch
  • aligned the delivery format with .github/PULL_REQUEST_TEMPLATE.md

How to Test

  1. Review the existing validation notes preserved in this PR body.
  2. Run the focused checks for the touched area.
  3. Confirm the scoped change still behaves as described above.

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:

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 — or N/A
  • I've updated tool descriptions/schemas if I changed tool behavior — or N/A

Screenshots / Logs

  • N/A.

Generated by Hermes Turbo


Generated by Hermes Turbo

…ntent

The reasoning.available event was sourced from assistant_message.content
(the visible reply text) instead of assistant_message.reasoning_content
(the actual model reasoning block).  External UIs that render both a
thinking pane and a message bubble received identical text in both, making
the reasoning panel useless and the UX confusing.

Root cause: the callback block at run_agent.py:14582 read:

    _think_text = assistant_message.content.strip()
    ...
    elif _think_text:
        tool_progress_callback("reasoning.available", "_thinking", _think_text[:500], None)

Fix: decouple the two emitters:

1. Subagent delegation (_thinking) — unchanged: still relays first line
   of assistant_message.content to the parent for progress display.

2. reasoning.available — now gates on reasoning_content:

    _reasoning_text = (getattr(assistant_message, "reasoning_content", None) or "").strip()
    if _reasoning_text:
        tool_progress_callback("reasoning.available", "_thinking", _reasoning_text[:500], None)

   For models without a separate reasoning_content field the event is
   simply not emitted, which is correct — the reply text is not reasoning.

Closes NousResearch#24518

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings May 12, 2026 21:49

Copilot AI 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.

Pull request overview

Fixes incorrect reasoning.available event payloads so downstream UIs don’t show the final assistant reply duplicated in both the “thinking” pane and the normal message bubble. The change aligns the event to exclusively use the model-provided structured reasoning_content field (when present), while keeping the subagent _thinking relay behavior separate.

Changes:

  • Update run_agent.py to emit reasoning.available only when assistant_message.reasoning_content is non-empty, instead of sourcing from assistant_message.content.
  • Preserve subagent delegation “thinking relay” behavior by continuing to use visible content for _thinking when delegate_depth > 0.
  • Add a regression test suite covering emission/non-emission, payload correctness, and truncation behavior.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
run_agent.py Decouples subagent _thinking relay from reasoning.available, and gates reasoning.available on reasoning_content only.
tests/run_agent/test_reasoning_available_event_24518.py Adds regression tests for correct reasoning.available sourcing, truncation, and non-emission when reasoning is absent.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

from __future__ import annotations

import re
from types import SimpleNamespace
Comment on lines +74 to +92
# Production code block (verbatim copy for isolation):
if agent.tool_progress_callback:
if msg.content and getattr(agent, '_delegate_depth', 0) > 0:
_think_text = msg.content.strip()
_think_text = re.sub(
r'</?(?:REASONING_SCRATCHPAD|think|reasoning)>', '', _think_text
).strip()
first_line = _think_text.split('\n')[0][:80] if _think_text else ""
if first_line:
try:
agent.tool_progress_callback("_thinking", first_line)
except Exception:
pass
_reasoning_text = (getattr(msg, "reasoning_content", None) or "").strip()
if _reasoning_text:
try:
agent.tool_progress_callback("reasoning.available", "_thinking", _reasoning_text[:500], None)
except Exception:
pass
@wesleysimplicio

Copy link
Copy Markdown
Contributor Author

Closing — PR has merge conflicts that can't be auto-resolved. The codebase has evolved past this fix. Re-opening with a fresh rebase welcome if the issue is still open.

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 P2 Medium — degraded but workaround exists type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

api_server: reasoning.available event carries final response text instead of reasoning_content

3 participants