fix(acp): route <think> reasoning content to agent_thought_chunk - #108
Merged
Conversation
session/prompt was streaming everything as agent_message_chunk, so a provider's <think>...</think> reasoning leaked into the assistant's visible message instead of the ACP schema's dedicated agent_thought_chunk update kind. Track in-think state per session (file-based, since run_turn_streaming_with_provider's callback type is fixed at (d.Step) -> [io] Unit and can't be widened to [io, sql] for just this caller), split each streamed chunk on the tags, and route Thought/Msg segments to the right update kind. Also strips a duplicated leading <think> (some providers double-emit the opening marker), verified against the literal boundary chunk that surfaced it in live testing. Fixes #107
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
session/promptstreamed every text delta asagent_message_chunk, so a provider's<think>...</think>reasoning content leaked into the assistant's visible message instead of the ACP schema's dedicatedagent_thought_chunkupdate kind./tmp/lex-acp-think-<session_id>, sincerun_turn_streaming_with_provider's callback type is fixed at(d.Step) -> [io] Unitshared across mcp_main.lex/TUI/client_protocol.lex and can't be widened to[io, sql]for just this caller) and a recursive splitter (split_think) that segments each streamed chunk across<think>/</think>boundaries, routingThoughtsegments toagent_thought_chunkandMsgsegments toagent_message_chunk.<think>marker — some providers double-emit the opening tag — caught via live testing against a real reasoning model.Fixes #107
Test plan
lex check src/server/client_protocol.lex— oklex fmt --check src/server/client_protocol.lex— formattedlex checkover every tracked.lexfile — all passlex test— 4/4 passedsplit_thinkcases (plain text, close-then-open, tag spanning a chunk boundary, open-with-no-close, close-with-no-open, doubled leading<think>, and the exact literal<think>Theboundary chunk from the original bug report) — all correctinitialize→session/new→session/prompt→session/closeflow against a local reasoning model over litellm): before the fix, 100% of streamed text — including reasoning — arrived asagent_message_chunk; after the fix, reasoning arrives as genuineagent_thought_chunkframes and zero<think>/</think>substrings leak into any chunk's text across a 244-frame run