Skip to content

fix: strip think blocks from reasoning field in extract_content_or_reasoning - #17797

Closed
Ifkellx wants to merge 2 commits into
NousResearch:mainfrom
Ifkellx:fix/title-generator-think-strip
Closed

fix: strip think blocks from reasoning field in extract_content_or_reasoning#17797
Ifkellx wants to merge 2 commits into
NousResearch:mainfrom
Ifkellx:fix/title-generator-think-strip

Conversation

@Ifkellx

@Ifkellx Ifkellx commented Apr 30, 2026

Copy link
Copy Markdown
Contributor

Summary

Fixes session titles showing raw think block content instead of the generated title.

Changes

  • extract_content_or_reasoning: now strips <think> blocks from message.reasoning and reasoning_details before returning
  • title_generator: uses extract_content_or_reasoning() instead of direct .content access for consistent handling
  • 4 new tests covering think block stripping

Root cause

When the auxiliary model returned the title in the message.reasoning structured field instead of message.content, the fallback path returned raw reasoning text including <think> blocks without cleaning them.

Testing

python -m pytest tests/tools/test_llm_content_none_guard.py tests/agent/test_title_generator.py -v

Ifkellx added 2 commits April 30, 2026 05:38
…asoning

- extract_content_or_reasoning now strips <think> blocks from message.reasoning
  and reasoning_details before returning, fixing session titles that showed
  raw think block content
- title_generator now uses extract_content_or_reasoning instead of direct
  .content access, ensuring consistent handling across all response types
- 4 new tests covering think block stripping in content, reasoning field,
  and reasoning_details summary
@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint labels Apr 30, 2026

@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 identifying the structured-reasoning title path. The premise remains valid on current main: agent/title_generator.py:90 reads only message.content, while agent/auxiliary_client.py:7006-7026 already defines the structured-reasoning fallback.

Problems

  • The added fallback at PR agent/auxiliary_client.py:3682 returns joined.strip() when tag removal yields an empty string. A reasoning field containing only `` therefore still leaks the raw block.
  • The title fix is coupled to unrelated MiniMax Coding Plan vision, provider, and web-search work (agent/auxiliary_client.py:2435, tools/web_tools.py:998 in the PR diff). Current main has a user model-provider plugin discovery surface at providers/__init__.py:163-165.

Suggested changes

  • Split out the focused title/extractor/test change.
  • Return the cleaned structured-reasoning value even when empty, and test reasoning containing only a think block.
  • Reuse the canonical scrubber behavior used by agent/title_generator.py:97-98.

Automated hermes-sweeper review.

Comment thread agent/auxiliary_client.py
return re.sub(
r"<(?:think|thinking|reasoning|thought|REASONING_SCRATCHPAD)>"
r".*?"
r"</(?:think|thinking|reasoning|thought|REASONING_SCRATCHPAD)>",

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.

When joined contains only a paired think block, re.sub(...).strip() is empty and this fallback returns the original raw block. Return the cleaned value instead, and add coverage for reasoning that is only ``.

@teknium1 teknium1 added sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jul 12, 2026
@teknium1

Copy link
Copy Markdown
Contributor

Thanks for the contribution. Closing on policy: Hermes does not post-process or repair model output. Extracting titles out of the reasoning field when a model returns content=None is compensating for a model/provider that isn't following the request format — that's the inference engine's job to fix, not ours to paper over. The same policy applies to the think-block stripping on the reasoning field.

The bundled MiniMax Coding Plan vision client is a separate feature that shouldn't ride in a title-fix PR — if you want to pursue it, please open it as its own focused PR so it can be evaluated on its own merits.

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-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants