Skip to content

fix: add _has_visible_content guard to thinking-prefill retry gate - #64733

Open
mason-jeeves wants to merge 1 commit into
NousResearch:mainfrom
mason-jeeves:fix/thinking-prefill-visible-content-guard
Open

fix: add _has_visible_content guard to thinking-prefill retry gate#64733
mason-jeeves wants to merge 1 commit into
NousResearch:mainfrom
mason-jeeves:fix/thinking-prefill-visible-content-guard

Conversation

@mason-jeeves

Copy link
Copy Markdown

Summary

Adds a _has_visible_content guard to the thinking-prefill retry gate in agent/conversation_loop.py so that reasoning models producing both reasoning tokens AND visible text (even a single character like ".") skip the prefill cascade and deliver their response immediately.

Problem

The thinking-prefill continuation logic (line 5026) checks whether the model produced structured reasoning with no visible text, then prefills to let the model continue. However, it only checks _has_structured — it never verifies that final_response is actually empty.

This means any model with reasoning_tokens > 0 can trigger the prefill branch even when it DID produce visible output. The prefill logic sees structured reasoning and retries regardless:

↻ Thinking-only response — prefilling to continue (1/2)
↻ Thinking-only response — prefilling to continue (2/2)
⚠️ Empty response from model — retrying (1/3)
⚠️ Empty response from model — retrying (2/3)
⚠️ Empty response from model — retrying (3/3)

Fix

Add _has_visible_content that strips think blocks from final_response and checks for non-whitespace content:

_has_visible_content = bool(
    final_response
    and agent._strip_think_blocks(final_response).strip()
)
if _has_structured and not _has_visible_content and agent._thinking_prefill_retries < 2:

This mirrors the existing _truly_empty check at line 5055 which already uses this same pattern — the fix brings the prefill gate into alignment. If the model produced any visible text, we skip the prefill retry and deliver it.

Affected models

Any reasoning model that produces both reasoning tokens and tiny visible responses:

  • DeepSeek-R1 / deepseek-v4-pro — stand-down protocol "." replies
  • OpenAI o-series (o1, o3, o4-mini)
  • Anthropic extended thinking (Claude with thinking)
  • Qwen3 with in-content <think> blocks

Fixes #64732

The thinking-prefill continuation logic checks whether the model
produced structured reasoning with no visible text, then prefills
to let the model continue.  However, it only checked _has_structured
without verifying that final_response is actually empty.

Add a _has_visible_content guard that strips think blocks from
final_response and checks for any non-whitespace content.  This
prevents the 'Thinking-only response' / 'Empty response' retry cascade
when a reasoning model produces both reasoning tokens AND a visible
response (even a single character like '.').

The downstream _truly_empty check already uses this same pattern
— this brings the prefill gate into alignment and ensures the
prefill branch only activates for truly content-less responses.

Fixes cases where models like DeepSeek-R1, OpenAI o-series,
Anthropic extended thinking, and Qwen3 produce tiny visible
responses alongside reasoning tokens.
@alt-glitch alt-glitch added type/bug Something isn't working comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint P2 Medium — degraded but workaround exists labels Jul 15, 2026
@teknium1

Copy link
Copy Markdown
Contributor

Thanks for the focused fix. The current-main prefill gate still checks only _has_structured at agent/conversation_loop.py:5116-5122, while the downstream empty-response path already strips think blocks before deciding content is empty at agent/conversation_loop.py:5150-5157. The proposed condition aligns those two decisions and lets the normal successful-content finalization at agent/conversation_loop.py:5302 deliver visible text.

Problems

  • The PR changes only agent/conversation_loop.py; it adds no regression test. Existing reasoning-prefill tests at tests/run_agent/test_run_agent.py:4484-4532 cover reasoning-only responses, not a response containing both reasoning and visible content.

Suggested changes

  • Add a focused test with reasoning_content plus visible content such as ".", asserting one API call and that no prefill continuation is taken.

Automated hermes-sweeper review.

@GottZ

GottZ commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

This was generated by AI during triage.

Summary

Two PRs address #64732 with the same visible-content guard at the thinking-prefill condition. #64733 isolates that change in agent/conversation_loop.py, while #64952 includes the guard alongside unrelated ACP, model-normalization, schema-sanitizer, and toolset changes.

Related pull requests

Duplicates

#64733 and #64952 duplicate the core #64732 guard; #64952 is the closed, stacked variant, while #64733 isolates the issue-specific change.

Suggested consolidation

Keep #64733 open with a salvage path: add the focused reasoning-plus-visible-content regression test requested by its keep_open review, asserting one API call and no prefill continuation, which also tests the conflicting outer-gate premise raised in the review of #64952. Keep #64952 closed as a duplicate of #64733 because its relevant guard is already isolated there and its remaining diff concerns unrelated changes.

Complex graph

flowchart LR
    classDef open fill:#dbeafe,stroke:#1d4ed8,color:#1e3a8a
    classDef merged fill:#dcfce7,stroke:#15803d,color:#14532d
    classDef closed fill:#e5e7eb,stroke:#6b7280,color:#1f2937
    classDef unverified fill:#f3f4f6,stroke:#9ca3af,color:#374151
    classDef best stroke-width:3px,stroke:#b45309
    classDef target stroke-width:3px,stroke:#4338ca
    I64732(["issue #64732 (open)"])
    subgraph Dup64733 ["PRs duplicating each other"]
        P64733["PR #64733 (open)"]
        P64952["PR #64952 (closed)"]
    end
    P64733 -->|best fix| I64732
    class I64732 open
    class P64733 open
    class P64952 closed
    class P64733 best
    class P64733 target
    click I64732 "https://github.com/NousResearch/hermes-agent/issues/64732"
    click P64733 "https://github.com/NousResearch/hermes-agent/pull/64733"
    click P64952 "https://github.com/NousResearch/hermes-agent/pull/64952"
Loading

Graph: solid arrow = fixes / best fix, dashed arrow = partial or unverified (see edge label); boxed group = PRs duplicating each other; amber border = best fix; indigo border = target; gray node = closed (state tag in the node label).

Cross-PR triage: Reviewed 2 pull requests and 1 issue in this complex. Each diff was read against this issue; Assessment working set: 10 kB of PR diffs, 6 kB of issue/PR text, 1 kB of discussion (3 comments), 3 verify verdicts. verdicts reflect diff content, not PR titles. Part of an automated triage batch.

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 sweeper:blast-broad Sweeper blast radius: broad — a core path most sessions hit 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.

Thinking-prefill retry cascade triggers on reasoning responses with visible content

4 participants