Skip to content

fix(tts): strip every reasoning-tag variant before speech, not just <think> - #77079

Open
Drexuxux wants to merge 2 commits into
NousResearch:mainfrom
Drexuxux:drex/tts-reasoning-variants
Open

Drexuxux wants to merge 2 commits into
NousResearch:mainfrom
Drexuxux:drex/tts-reasoning-variants

Conversation

@Drexuxux

@Drexuxux Drexuxux commented Aug 2, 2026

Copy link
Copy Markdown
Contributor

What

The two TTS text cleaners only removed <think>...</think>:

  • strip_nonspoken_blocks (batch normalizer — every auto-TTS / voice / tool path via prepare_spoken_text)
  • SentenceChunker (the streamed speaker pipeline + speak-stream WebSocket)

Every other reasoning scrubber in the codebase — agent/think_scrubber.py (_OPEN_TAG_NAMES) and strip_think_blocks — covers five variants: <think>, `")
before: 'The answer is 42.' ← spoken
after : 'The answer is 42.'

SentenceChunker, "hidden" split across deltas
before: reasoning spoken
after : held until close, stripped


The raw deltas genuinely reach TTS unscrubbed: the gateway tees the same raw `text` to both the display consumer and the streaming-TTS consumer (`gateway/run.py`), so the TTS side must scrub on its own.

## Fix

Share one canonical tag set (kept in sync with `think_scrubber._OPEN_TAG_NAMES`) across both cleaners:

- `strip_nonspoken_blocks` strips all five variants, closed and unterminated, case-insensitively.
- `SentenceChunker` strips them across deltas and holds a delta back on **any** unclosed reasoning tag — a new `has_unclosed_reasoning_tag` probe — not just `<think>`.

No behaviour change for text without reasoning tags; the `<think>` path is unchanged.

## Tests

`tests/tools/test_tts_text_normalize.py` and `tests/tools/test_tts_streaming.py`:

- every variant (`<thinking>`/`<reasoning>`/`<thought>`/`<REASONING_SCRATCHPAD>`), closed and unterminated, is never spoken — parametrized
- case-insensitive (`<THINKING>`, `<Reasoning>`)
- `SentenceChunker` strips non-`<think>` tags split across deltas and holds on an unclosed tag
- the `<think>` path still passes (unchanged)

scripts/run_tests.sh tests/tools/test_tts_text_normalize.py tests/tools/test_tts_streaming.py

with the fix

=== Summary: 13 passed (text_normalize) / all SentenceChunker green ===

without the fix — genuine behavioural failures (not import errors)

test_tts_text_normalize.py 9 failed ( params still pass — already covered)
test_tts_streaming.py 5 failed


Broad TTS sweep — `tests/tools/test_tts_*`, `tests/gateway/test_*tts*`, speak-stream: **81 passed, 1 failed**, the one failure being a pre-existing flaky wall-clock timing test (`test_hybrid_prefetch_fires_http_immediately`) that also fails on a clean checkout. Zero new failures.

…think>

The two TTS text cleaners only removed <think>...</think>:
strip_nonspoken_blocks (batch normalizer, all auto-TTS / voice / tool paths)
and SentenceChunker (the streamed speaker + speak-stream WebSocket). Every
other reasoning scrubber in the codebase — agent/think_scrubber.py and
strip_think_blocks — covers <think>, <thinking>, <reasoning>, <thought> and
<REASONING_SCRATCHPAD>.

Models that don't use <think> — Gemini and Gemma (including the Vertex
OpenAI-compatible path), GLM, and others — emit <thinking>/<reasoning>/
<thought>. Their reasoning reached the speech provider verbatim and was read
aloud, defeating the whole point of these cleaners ("blocks that must never
reach a speech provider", NousResearch#34213). The streaming chunker was worse: no
re.IGNORECASE, so even <THINK> leaked, and its open-tag hold-back only knew
about <think>.

Share one canonical tag set (kept in sync with think_scrubber._OPEN_TAG_NAMES)
across both cleaners:
- strip_nonspoken_blocks strips all five variants, closed and unterminated,
  case-insensitively.
- SentenceChunker strips them across deltas and holds a delta back on any
  unclosed reasoning tag (new has_unclosed_reasoning_tag probe), not just
  <think>.

No behaviour change for text without reasoning tags; the <think> path is
unchanged.
@alt-glitch alt-glitch added type/bug Something isn't working P3 Low — cosmetic, nice to have tool/tts Text-to-speech and transcription labels Aug 2, 2026

@teknium1 teknium1 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.

Thanks for covering the non-variants; the underlying current-main gap is real: `tools/tts_text_normalize.py:215` and `tools/tts_streaming.py:86-107` only handle.

Problems

  • The amended SentenceChunker.flush() at PR tools/tts_streaming.py:137 strips only closed pairs. After feed("secret") holds the buffer, flush() returns the still-open raw tail. This reaches synthesis because gateway/streaming_tts_consumer.py:182 enqueues every flushed clause.
  • hermes_cli/web_server.py:4644 independently recognizes only lowercase `` before an idle force-flush, so the speak-stream path does not preserve the new variants across an idle gap.

Suggested changes

  • Drop unterminated reasoning tails in flush() and add parameterized flush/end-of-stream tests for every supported tag.
  • Replace the WebSocket's literal `` idle guard with the same variant-aware probe.
  • Avoid a second tag-list source: agent/think_scrubber.py:79-85 already owns the current set.

Automated hermes-sweeper review.

Comment thread tools/tts_streaming.py
@@ -119,7 +137,7 @@ def feed(self, delta: str) -> List[str]:

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.

Blocking: this only removes closed pairs. feed("secret") returns [] via the new unclosed-tag guard, but an end-of-stream or idle flush() returns the raw open tail and the gateway synthesizes it. Discard or remove an unclosed reasoning suffix here, and add a regression test that calls flush() after every unclosed variant.

Comment thread tools/tts_text_normalize.py Outdated
# Reasoning blocks: models with ``/reasoning show`` enabled emit reasoning
# blocks in the final assistant message. Users want to SEE reasoning, not
# hear it read aloud (#34213). Cover every tag variant the canonical scrubber
# recognises (``agent/think_scrubber.py`` ``_OPEN_TAG_NAMES``) — not just

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.

Suggestion: this is a second manually maintained source for the tag set, while agent/think_scrubber.py:79-85 already defines it. Please expose/import one shared constant so future reasoning-tag additions cannot silently diverge between display and TTS suppression.

@teknium1 teknium1 added sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Aug 2, 2026
Review follow-ups on the reasoning-variant fix:

1. SentenceChunker.flush() stripped only closed pairs, so when feed() held a
   delta back on an open reasoning tag and the stream then ended (or idled
   out) with no close, flush() returned the raw open tail — and the gateway
   (streaming_tts_consumer) synthesises every flushed clause. Drop the
   unterminated block on flush too, via a shared
   strip_unterminated_reasoning() helper.

2. The speak-stream idle force-flush guard in web_server.py recognised only a
   lowercase <think open tag, so the WebSocket path didn't hold non-<think>
   variants across an idle gap. Replace the literal check with the same
   has_unclosed_reasoning_tag() probe.

3. Single source of truth for the tag set: agent/think_scrubber.py now exposes
   REASONING_TAG_NAMES; tts_text_normalize imports it instead of keeping a
   second hand-maintained copy, so a future variant can't be scrubbed for
   display yet spoken aloud.

Tests: parameterized flush / end-of-stream coverage for every tag (open tail
dropped, visible prefix kept), split-across-deltas unterminated case, and a
guard that the TTS tag set is the canonical scrubber's object.
@andrexibiza

Copy link
Copy Markdown
Contributor

Verification comment (Vox Lockin lane 10 — adversarial check)

Verified this PR against current origin/main (70db671). Verdict: mergeable, CI green, covers the reasoning-tag class — no duplicate opened.

Premise confirmed on main: both TTS cleaners strip only <think>:

  • tools/tts_text_normalize.py _THINK_BLOCK_RE (batch normalizer used by prepare_spoken_text)
  • tools/tts_streaming.py SentenceChunker._THINK_BLOCK_RE (streamed speaker + speak-stream WS)

so a model emitting <thinking>/<reasoning>/<thought> leaks raw tags into speech.

Checked against this PR's head 8137545377:

  • Tag set drawn from agent/think_scrubber.REASONING_TAG_NAMES (single source of truth) ✓
  • strip_reasoning_blocks / strip_unterminated_reasoning / has_unclosed_reasoning_tag shared by both cleaners — no drift possible ✓
  • Unterminated-tail handling on flush() closes the streaming cut-off hole ✓
  • Regression tests added in tests/tools/test_tts_streaming.py + tests/tools/test_tts_text_normalize.py

CI: all required checks pass (run 30799200885; ruff, footguns, check-attribution, Docker builds green).

Composition note (lane 10): this PR is orthogonal to my streaming-truncation work (#78234) — different code paths (tag stripping vs. length splitting). No conflicts expected.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

P3 Low — cosmetic, nice to have 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 tool/tts Text-to-speech and transcription type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants