Skip to content

feat: surface streamed thinking content as a Discord blockquote#139

Open
marvin-69-jpg wants to merge 2 commits intoopenabdev:mainfrom
marvin-69-jpg:feat/surface-thinking-content
Open

feat: surface streamed thinking content as a Discord blockquote#139
marvin-69-jpg wants to merge 2 commits intoopenabdev:mainfrom
marvin-69-jpg:feat/surface-thinking-content

Conversation

@marvin-69-jpg
Copy link
Copy Markdown
Contributor

Summary

When a Claude (or any other ACP backend) agent enters extended thinking mode, the only visible signal in Discord today is the 🤔 emoji on the bot's message. The actual thinking content streamed by the agent over the ACP protocol (agent_thought_chunk events) is parsed and discarded:

"agent_thought_chunk" => {
    Some(AcpEvent::Thinking)   // ← unit variant, no payload
}
AcpEvent::Thinking => {
    reactions.set_thinking().await;   // ← only the emoji
}

This PR surfaces that content as a blockquote at the top of the streaming Discord message, similar to how the native Claude Code CLI shows thinking blocks above tool calls and the final answer:

> 🤔 Thinking
> Let me start by reading the README and checking my notes.
>
> There's a local commit ahead of origin/main, I should reset to
> origin/main first before branching.

🔧 `git fetch && git checkout main && git pull --ff-only`...

(final answer streams in here)

Discord users — especially in multi-user team channels where the bot is doing non-trivial work — get a much clearer picture of what the agent is reasoning about, before the answer arrives.

Implementation

  • AcpEvent::Thinking carries the delta string instead of being a unit variant.
  • classify_notification reads update.content.text (the same shape agent_message_chunk already uses).
  • stream_prompt accumulates thinking deltas into a thought_buf alongside the existing text_buf, and re-renders the message on each event.
  • compose_display gains a thought parameter and emits a blockquote-prefixed section above the tool list when thought is non-empty.

The block-boundary heuristic

claude-agent-acp emits both thinking (the start of a new block) and thinking_delta (continuation) as the same agent_thought_chunk shape with no marker for the boundary:

// claude-agent-acp/dist/acp-agent.js, around line 1463
case "thinking":
case "thinking_delta":
    update = {
        sessionUpdate: "agent_thought_chunk",
        content: { type: "text", text: chunk.thinking },
    };

When the model produces several thinking blocks in a row separated only by tool calls, the deltas concatenate into a wall of text like ".There's a local commit" with no whitespace between sentences. We work around this with a small heuristic in needs_thinking_separator:

fn needs_thinking_separator(prev: &str, new: &str) -> bool {
    let last = prev.chars().last()?;
    let first = new.chars().next()?;
    let sentence_end = matches!(last, '.' | '!' | '?' | '。' | '!' | '?');
    let starts_word = first.is_alphabetic();
    sentence_end && starts_word
}

If the previous chunk ends in sentence-terminating punctuation and the new chunk starts with a letter (no leading whitespace), insert a \n\n. This is imperfect but covers the common case without requiring upstream protocol changes — token-level deltas within a single block almost always include leading whitespace, so the heuristic doesn't over-correct.

A cleaner long-term fix would be for claude-agent-acp to thread the content_block_start event through to agent_thought_chunk (or add a block_start: true flag in the update payload). Happy to follow up upstream if maintainers prefer that route.

Files changed

  • src/acp/protocol.rsAcpEvent::Thinking(String); classify_notification extracts text from agent_thought_chunk.
  • src/discord.rs — new thought_buf in stream_prompt; new needs_thinking_separator helper; compose_display takes thought and renders blockquote.

+75 / -8.

Compatibility

  • Backwards compatible with any ACP backend that doesn't emit thinking events at all (the Thinking variant is just never produced; compose_display skips the blockquote when thought_buf is empty).
  • Preset-agnostic: this is a render-side change inside discord.rs, not gated on agent.preset.
  • No new dependencies.

Testing

Running in production for the past day against a multi-user Discord channel mixing English and Traditional Chinese conversations. The heuristic correctly inserts paragraph breaks between distinct thinking blocks and leaves token-level deltas inside a single block alone.

Note about base branch

This PR is layered on top of #138 (fix: dedupe tool call display by toolCallId and sanitize titles) — both PRs touch the AcpEvent enum and the compose_display signature, and #138 is the more critical bug fix. I've based this branch on fix/dedupe-tool-call-display so they apply cleanly together; if you'd rather merge them in the opposite order I can rebase. The diff stat above is for the thinking change alone, not cumulative.

claude-agent-acp emits multiple events for the same tool invocation as
the input fields stream in: a `tool_call` with a placeholder title
("Terminal" / "Edit" / etc.) followed by one or more `tool_call_update`
events that refine the title to the real command. The current handler
pushes a new line on every event without deduping, so the message ends
up with orphaned placeholder lines that never get updated:

    🔧 `Terminal`...
    ❌
    🔧 `cd /home/node/work && git status`...
    ✅

It also passes the raw title through inline-code formatting without
flattening newlines or escaping embedded backticks. Discord's
single-backtick inline-code spans render on a single line only, so
multi-line shell commands (heredocs, &&-chained commands written across
lines) appear truncated mid-render with the inline-code span breaking
on the first newline.

Fix:

* Carry the ACP toolCallId through AcpEvent::ToolStart / ToolDone so the
  renderer can pin updates to the same entry.
* Replace `tool_lines: Vec<String>` with `Vec<ToolEntry>` (id, title,
  state). ToolStart updates the slot in place if the id is already
  present; ToolDone preserves the existing title when the Done event
  omits one (which it routinely does in claude-agent-acp updates).
* Add a `sanitize_title` helper that flattens \n to " ; " and rewrites
  embedded backticks so they can't break the surrounding inline-code
  span.
* Each tool now renders via ToolEntry::render() which picks the icon
  from the state — no more brittle substring-based line lookups against
  the placeholder title.

Tested in production against a multi-user Discord channel running heavy
git/gh workflows with multi-line bash commands.
Currently the only signal Discord users get when an agent enters
extended thinking mode is the 🤔 emoji on the bot's message. The
actual thinking content delivered via `agent_thought_chunk` is parsed
and discarded — `classify_notification` returns `AcpEvent::Thinking`
with no payload, and the `Thinking` arm in stream_prompt only flips
the reaction emoji.

This patch surfaces that content as a blockquote at the top of the
streaming Discord message, similar to how the native Claude Code CLI
shows thinking blocks above tool calls and the final answer:

  > 🤔 Thinking
  > Let me start by reading the README and checking my notes.
  >
  > There's a local commit ahead of origin/main, I should reset to
  > origin/main first before branching.

Implementation:

* `AcpEvent::Thinking` carries the delta string instead of being a
  unit variant; `classify_notification` reads `update.content.text`
  (the same shape `agent_message_chunk` already uses).
* `stream_prompt` accumulates thinking deltas into a `thought_buf`
  alongside the existing `text_buf`, and re-renders the message on
  each event.
* `compose_display` gains a `thought` parameter and emits a
  blockquote-prefixed section above the tool list when thought is
  non-empty.

claude-agent-acp emits both `thinking` (block start) and
`thinking_delta` (continuation) as the same `agent_thought_chunk`
shape, with no marker for block boundaries. When the model produces
several thinking blocks in a row separated only by tool calls, the
deltas concatenate into a wall of text like ".There's a local commit"
with no whitespace between sentences. We work around this with a
small heuristic in `needs_thinking_separator`: if the previous chunk
ends in sentence-terminating punctuation (`. ! ? 。 ! ?`) and the
new chunk starts with a letter (no leading whitespace), insert a
paragraph break. This is imperfect but covers the common case
without requiring upstream protocol changes.

Tested in production for the past day against a multi-user Discord
channel with both English and Traditional Chinese conversations; the
heuristic correctly inserts breaks between distinct thinking blocks
and leaves token-level deltas inside a single block alone.
Copy link
Copy Markdown
Collaborator

@chaodu-agent chaodu-agent left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 Needs Rebase + Rework — Thinking-content feature is valuable, but the branch is stale and duplicates code already merged to main.

Baseline Check (Step 0)
Field Value
State OPEN
Mergeable CONFLICTING
Created 2026-04-08 (23 days ago)
Last commit 2026-04-08
Author @brettchien
Labels closing-soon
Base branch fix/dedupe-tool-call-display (PR #138)

Main has: AcpEvent::Thinking (unit variant, no payload), ToolEntry/ToolState/sanitize_title in adapter.rs, compose_display with 4-param signature (tool_lines, text, streaming, tool_display). Thinking events trigger 🤔 emoji only — content is discarded.

Net-new: AcpEvent::Thinking(String) payload, thought_buf accumulation, blockquote rendering ("> 🤔 Thinking"), needs_thinking_separator() heuristic.

四問框架

1. 解決什麼問題

When an agent enters extended thinking mode, the only visible signal is a 🤔 emoji. The actual thinking content streamed via agent_thought_chunk events is parsed and discarded.

2. 怎麼解決

Changes AcpEvent::Thinking to carry the delta string, accumulates thinking deltas into thought_buf, renders as a blockquote above the tool list in Discord messages.

3. 考慮過什麼

Block-boundary heuristic (needs_thinking_separator) handles the case where claude-agent-acp emits both thinking and thinking_delta as the same shape with no boundary marker.

4. 最佳方案嗎

The feature is great UX, but the implementation needs rebasing — most of the tool-tracking infra it adds already landed on main.

🟢 INFO

  • Surfacing thinking content as a blockquote is a great UX improvement.
  • needs_thinking_separator() is a thoughtful heuristic for detecting block boundaries.
  • Graceful fallback: Thinking("") still triggers the 🤔 reaction.
  • Good defensive coding: ToolDone without prior ToolStart is handled.

🟡 NIT

  1. Hardcoded blockquote header"> 🤔 _Thinking_\n" is hardcoded. Main's config already has emoji_thinking — consider using it for consistency.
  2. No truncation on thought_buf — Extended thinking sessions could produce very long blockquotes that hit Discord's 2000-char message limit.
  3. Visibility mismatchToolEntry and ToolState are declared pub in discord.rs but were pub(crate) / private in main's adapter.rs.

🔴 SUGGESTED CHANGES

  1. Merge conflict — needs rebase. Main already has ToolEntry, ToolState, sanitize_title, and id-based tool dedup in src/adapter.rs (merged from PR #138 or similar). This PR re-introduces all of that in src/discord.rs. After rebase, the only net-new code should be: (1) AcpEvent::Thinking(String) in protocol.rs, (2) thought_buf accumulation + needs_thinking_separator(), (3) blockquote rendering in compose_display.

  2. compose_display signature mismatch — Main's compose_display takes (tool_lines, text, streaming, tool_display). The PR's version takes (tool_lines, thought, text) — dropping streaming and tool_display which control compact/full/none tool rendering and the "typing…" indicator. These features would regress. The thought parameter needs to be added to main's existing signature.

  3. protocol.rs duplication — The Thinking → Thinking(String) change is the core of this PR and is correct, but the diff also re-introduces ToolStart { title }ToolStart { id, title } which main already has. After rebase this should be a one-line change.

@chaodu-agent
Copy link
Copy Markdown
Collaborator

超渡法師 Review — PR #139

1. What problem does it solve?

When an ACP backend agent enters extended thinking mode, the only visible signal in Discord is a 🤔 emoji reaction. The actual thinking content streamed via agent_thought_chunk events is parsed but immediately discarded. Users have no visibility into what the agent is reasoning about.

2. How does it solve it?

  • protocol.rsAcpEvent::Thinking becomes AcpEvent::Thinking(String), extracting update.content.text from agent_thought_chunk events.
  • discord.rs — Adds thought_buf to accumulate thinking deltas. needs_thinking_separator() heuristic detects block boundaries using sentence-terminating punctuation (including CJK: 。!?). compose_display renders thinking as > 🤔 _Thinking_ blockquote above tool list and answer text.

3. What was considered?

4. Is this the best approach?

The feature design is solid — blockquote rendering of thinking content is the right UX pattern for Discord. The heuristic is a pragmatic workaround for a protocol gap. However, the implementation needs a rebase.


Traffic Light

🟢 INFO — Excellent UX improvement

  • Thinking content as blockquotes gives users real-time visibility into agent reasoning.
  • needs_thinking_separator() is well-reasoned with good CJK punctuation support.
  • Graceful degradation: Thinking("") still triggers 🤔 reaction; backends without thinking events are unaffected.
  • PR description is exceptionally thorough — explains the protocol gap, heuristic limitations, and upstream fix path.

🔴 SUGGESTED CHANGES

  1. Rebase required — PR is in CONFLICTING merge state. Main's codebase has been significantly refactored since the branch was created. Streaming logic (stream_prompt, compose_display, ToolEntry, ToolState) moved from discord.rs to adapter.rs. The PR branch doesn't have adapter.rs at all. After rebase, the diff should be ~30-40 lines net-new.

  2. compose_display signature mismatch — Main's compose_display takes (tool_lines, text, streaming, tool_display) with compact/full/none tool rendering modes. The PR's version takes (tool_lines, thought, text), dropping streaming and tool_display. Merging as-is would regress the tool display feature. Add thought as a 5th parameter to main's existing signature.

  3. protocol.rs diff is inflated — The diff re-introduces tool_id extraction and ToolStart { id, title } / ToolDone { id, title, status } changes already on main (from fix: dedupe tool call display by toolCallId and sanitize titles #138). After rebase, the protocol.rs change should be ~12 lines.

🟡 NIT

  1. No thought_buf truncation — Extended thinking can produce thousands of tokens. The blockquote will eventually exceed Discord's 2000-char limit. Consider truncating to show only the last N characters during streaming.
  2. Hardcoded emoji"> 🤔 _Thinking_" is hardcoded, but main's config already has emoji_thinking. Use the configured emoji for consistency.

Verdict

🔴 Changes requested — Great feature, needs rebase. The thinking blockquote is the right UX pattern. After rebasing onto current main, port the thinking changes to adapter.rs (where compose_display now lives). The net-new diff should be ~30-40 lines.

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

Labels

closing-soon PR missing Discord Discussion URL — will auto-close in 3 days needs-rebase pending-contributor

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants