Skip to content

Support client-side Harmony format parsing and stateful streaming scrubbing - #46330

Open
tsaipifong wants to merge 1 commit into
NousResearch:mainfrom
tsaipifong:feature/client-harmony-parser-scrubber
Open

Support client-side Harmony format parsing and stateful streaming scrubbing#46330
tsaipifong wants to merge 1 commit into
NousResearch:mainfrom
tsaipifong:feature/client-harmony-parser-scrubber

Conversation

@tsaipifong

Copy link
Copy Markdown

Support Client-Side Harmony Format Parsing & Stateful Streaming Scrubbing

This pull request implements support for client-side Harmony format parsing and stateful streaming scrubbing for standard OpenAI-compatible endpoints under the chat_completions transport.

The Problem

When running local/custom models (such as gpt-oss-120b via LM Studio) that generate standard Harmony tag schemas under the permissive chat_completions transport:

  1. At the end of a response (non-streaming): Raw XML-like control tags (e.g. <|channel|> and <|end|>) remained in the user-visible content, and tool calls were not extracted if the model returned them as plain text rather than native structured tool_calls.
  2. During streaming (real-time generation): Raw control tags, internal channel headers, and JSON payloads for tool calls leaked directly into the terminal stream, causing a messy user experience.

The Solution

  1. Client-Side Non-Streaming Parser:

    • Implemented _parse_harmony_tool_calls in ChatCompletionsTransport (chat_completions.py) using a brace-matching parser (_extract_json_object).
    • Supports both tag-style (XML-like <|channel|>) and text-style (leaked to=functions.tool_name) formats.
    • Normalizes these to structured ToolCall objects and cleans the visible text response, setting finish_reason = "tool_calls".
  2. Stateful Streaming Scrubber:

    • Introduced StreamingHarmonyScrubber (agent/harmony_scrubber.py) using a state machine (SCANNING -> IN_HEADER -> IN_CHANNEL_FINAL / IN_CHANNEL_SUPPRESSED) to parse streaming tokens incrementally.
    • Suppresses internal channels (e.g., commentary, analysis, thought) entirely from real-time terminal output, preventing leaked JSON payloads and internal headers from flashing in the terminal.
    • Gracefully handles character-by-character token boundaries and partial tags across delta limits.
    • Integrates the scrubber into agent initialization (agent_init.py), turn context resetting (turn_context.py), and stream callbacks/flushing (run_agent.py).
  3. Robust Test Suite:

    • Added 12 unit tests and 2 agent integration tests in test_harmony_scrubber.py.
    • Added tests in test_turn_context.py and test_chat_completions.py (totaling 136 passing tests).

@alt-glitch alt-glitch added type/feature New feature or request P2 Medium — degraded but workaround exists comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint provider/openai OpenAI / Codex Responses API labels Jun 14, 2026
@teknium1

Copy link
Copy Markdown
Contributor

Thanks for tackling a real compatibility gap: current ChatCompletionsTransport.normalize_response() still returns message.content unchanged (agent/transports/chat_completions.py:736-766), while existing cleanup only handles selected XML forms (agent/agent_runtime_helpers.py:617-692).

Problems

  • The new scrubber is applied through _fire_stream_delta(), but the live path after native tool-call accumulation directly invokes agent.stream_delta_callback(delta.content) at agent/chat_completion_helpers.py:2379-2382. That bypass would still expose Harmony text in that path.
  • The added integration tests call _fire_stream_delta() directly, so they do not exercise the bypass above.

Suggested changes

  • Route both streaming paths through one scrubbed delivery boundary and add a chunked integration test covering native tool-call accumulation followed by Harmony text.
  • Salvage the narrow feature onto the current split streaming modules rather than carrying the whole-file rewrites.

This is an automated hermes-sweeper review.

@teknium1 teknium1 added sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages 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 area/streaming Streaming responses: gateway delivery, provider wire labels Jul 14, 2026
@laurinaitis

Copy link
Copy Markdown
Contributor

I run local models over the same chat_completions transport (Ollama instead of LM Studio) and have been chasing the same leak, so a few things from that side.

The diff is almost entirely line endings. Of the +10,093/−9,525, every removed line reappears identically with CRLF appended - the branch converts run_agent.py, agent_init.py, turn_context.py, chat_completions.py and their tests to CRLF wholesale. The real new code is 568 lines: 185 in agent/harmony_scrubber.py, 136 in its tests, 152 in chat_completions.py, small remainders elsewhere. Renormalising to LF would take the diff from ~19,600 changed lines to under 600 and make it reviewable, and it also makes the sweeper's suggestion (salvage onto the current split streaming modules) much cheaper.

This reaches beyond gpt-oss. Every harmony leak captured in my local message store came from the gemma4 family via Ollama: three on gemma4:26b-a4b-q8 (2026-06-28 → 07-11) and two on gemma4:e4b-q8 (07-13). Two shapes show up, and the scrubber's literal tag match sees neither:

  • a degraded token <channel|> (single pipe - Ollama's partial thinking-parse eats the front of the token) appearing mid-message after ordinary prose;
  • a bare lowercase thought head opening the message, no control tokens at all until a later <channel|> separates reasoning from the answer.

buf_lower.find("<|channel|>") in SCANNING can't hit either one. My own gate only arms at the head of a message, so it misses the mid-message case too - neither of us currently catches those two. Loosening the matcher to tolerate the degraded token would widen the fix to whatever Ollama serves, and I have the shapes written up as fixtures I can hand over if that helps.

The sweeper's bypass point applies to my PR as well. I have #62680 open for the degraded shapes (scrubbing at the persist/CLI/stream sites); its streaming gate also hangs off _fire_stream_delta, and the direct agent.stream_delta_callback(delta.content) call after native tool-call accumulation (agent/chat_completion_helpers.py:3281-3283 on current main) skips both our scrubbers. Whichever PR lands, that path needs routing through the one scrubbed delivery boundary.

The two PRs cover different halves of the bug: yours catches canonical tokens anywhere in the stream and extracts text-form tool calls, mine handles the degraded shapes and the persisted-content path. I'd rather reconcile than land two overlapping scrubbers - happy to rebase mine around yours, or fold my leak fixtures into your tests, whichever direction the maintainers prefer.

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

Labels

area/streaming Streaming responses: gateway delivery, provider wire comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint P2 Medium — degraded but workaround exists provider/openai OpenAI / Codex Responses API 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-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state type/feature New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants