Skip to content

fix(telegram): deliver durable reasoning replies#95051

Closed
vincentkoc wants to merge 2 commits into
mainfrom
qa-sre-telegram-reasoning-delivery-20260619
Closed

fix(telegram): deliver durable reasoning replies#95051
vincentkoc wants to merge 2 commits into
mainfrom
qa-sre-telegram-reasoning-delivery-20260619

Conversation

@vincentkoc

Copy link
Copy Markdown
Member

Summary:

  • allow Telegram durable isReasoning payloads through the shared reply dispatcher while keeping generic-channel suppression intact
  • convert Telegram durable reasoning payloads into reasoning-lane text before outbound normalization, then strip the isReasoning marker before durable send
  • add shared-dispatch and Telegram regression coverage for /reasoning on

Fixes #94937

Verification:

@vincentkoc
vincentkoc marked this pull request as ready for review June 19, 2026 17:34
@vincentkoc vincentkoc self-assigned this Jun 19, 2026
@openclaw-barnacle openclaw-barnacle Bot added channel: telegram Channel integration: telegram size: S maintainer Maintainer-authored PR labels Jun 19, 2026
@clawsweeper

clawsweeper Bot commented Jun 19, 2026

Copy link
Copy Markdown
Contributor

Codex review: found issues before merge. Reviewed June 21, 2026, 5:39 PM ET / 21:39 UTC.

Summary
The PR allows Telegram isReasoning block and final payloads through shared dispatch, converts them into Telegram reasoning-lane text before durable send, strips the marker, and adds regression tests for /reasoning on.

PR surface: Source +36, Tests +128. Total +164 across 4 files.

Reproducibility: yes. Current main suppresses durable isReasoning block and final payloads before Telegram delivery, and the linked issue provides concrete /reasoning on steps with live context.

Review metrics: 2 noteworthy metrics.

  • Reasoning Payload Gate: 1 payload class newly allowed. Telegram isReasoning payloads now leave shared dispatch, so downstream delivery and TTS behavior must be checked before merge.
  • TTS-Exposed Paths: 2 paths still exposed. Final replies and all-mode block replies can invoke shared TTS before Telegram strips the reasoning marker.

Root-cause cluster
Relationship: fixed_by_candidate
Canonical: #94937
Summary: This PR is a candidate fix for the canonical Telegram /reasoning on durable-thinking message-loss issue; stacked and duplicate PRs overlap, while the DeepSeek boundary issue is adjacent after delivery works.

Members:

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

Merge readiness
Overall: 🧂 unranked krab
Proof: 🐚 platinum hermit ✨ media proof bonus
Patch quality: 🧂 unranked krab
Result: blocked by patch quality or review findings.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

  • Bypass shared TTS for final isReasoning payloads.
  • [P2] Bypass shared TTS for block isReasoning payloads when TTS mode is all.
  • [P2] Add focused regression tests for both TTS cases.

Mantis proof suggestion
A native Telegram proof would directly show durable Thinking and answer delivery with no duplicate output or TTS media on Thinking. A maintainer can ask Mantis to capture proof by posting this exact PR comment:

@openclaw-mantis telegram desktop proof: verify /reasoning on sends one Thinking message and one answer on Telegram with TTS enabled, and no audio/media is attached to Thinking.

Risk before merge

  • [P1] TTS-enabled Telegram users can receive spoken/audio media for Thinking messages because final isReasoning payloads are newly delivered but still enter shared final TTS first.
  • [P1] When messages.tts.mode is all, block reasoning payloads can still enter the immediate block TTS path before Telegram converts them to the reasoning lane.
  • [P1] The available live screenshots prove durable Thinking delivery and no duplicate visible text, but not a TTS-enabled run with no audio/media attached to Thinking.

Maintainer options:

  1. Skip TTS For Reasoning Payloads (recommended)
    Exclude isReasoning payloads from shared final and block TTS before Telegram delivery, then cover both paths with focused tests.
  2. Accept Spoken Thinking Risk
    Maintainers could intentionally land this branch as-is, but TTS-enabled Telegram users may hear Thinking content rather than only seeing it in the reasoning lane.
  3. Fold The Capability Follow-up Later
    Keep this PR focused on the durable-delivery bug and evaluate the capability-gate follow-up separately after the TTS behavior is safe.
Copy recommended automerge instruction
@clawsweeper automerge

Special instructions:
Keep Telegram `isReasoning` payloads out of shared TTS for final replies and all-mode block replies before Telegram delivery, preserve generic non-Telegram suppression, and add regression coverage for final reasoning with TTS enabled plus block reasoning when `messages.tts.mode` is `all`.

Next step before merge

  • [P2] This maintainer-labeled PR needs human-owned review or an explicit maintainer repair command because the remaining blocker is a narrow but merge-critical TTS behavior change on the PR branch.

Security
Cleared: The diff changes TypeScript reply-dispatch logic and tests only, with no dependency, workflow, secret, install, or package-publishing changes.

Review findings

  • [P1] Bypass final TTS for reasoning payloads — src/auto-reply/reply/dispatch-from-config.ts:3281
  • [P1] Bypass block TTS for reasoning payloads — src/auto-reply/reply/dispatch-from-config.ts:3109
Review details

Best possible solution:

Land the Telegram durable-reasoning fix only after isReasoning payloads bypass shared TTS in final and all-mode block paths, with focused regression coverage on the final branch.

Do we have a high-confidence way to reproduce the issue?

Yes. Current main suppresses durable isReasoning block and final payloads before Telegram delivery, and the linked issue provides concrete /reasoning on steps with live context.

Is this the best way to solve the issue?

No as written. The Telegram pass-through and marker stripping are the right narrow delivery shape, but reasoning payloads must bypass shared TTS before this is the best fix.

Full review comments:

  • [P1] Bypass final TTS for reasoning payloads — src/auto-reply/reply/dispatch-from-config.ts:3281
    This newly allows Telegram reasoning finals through shared dispatch, but the final loop still sends them into sendFinalPayload, which applies shared TTS before Telegram strips isReasoning. A TTS-enabled Telegram user can get audio attached to a Thinking message; skip TTS for reasoning finals and cover that path.
    Confidence: 0.89
  • [P1] Bypass block TTS for reasoning payloads — src/auto-reply/reply/dispatch-from-config.ts:3109
    The second commit keeps reasoning out of accumulated final-fallback TTS, but block isReasoning payloads still continue to the normal block TTS call. With messages.tts.mode: "all", Thinking blocks can still be synthesized unless the block path also excludes reasoning payloads.
    Confidence: 0.86

Overall correctness: patch is incorrect
Overall confidence: 0.88

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 108d6d7eca00.

Label changes

Label justifications:

  • P2: This is a bounded Telegram message-delivery bug fix with clear remaining blockers but no crash, data-loss, or security-bypass signal.
  • merge-risk: 🚨 message-delivery: The diff changes whether Telegram reasoning payloads are suppressed, delivered, or delivered with unintended TTS media.
  • rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🐚 platinum hermit and patch quality is 🧂 unranked krab.
  • status: ⏳ waiting on author: ClawSweeper has contributor-facing work open and is waiting for author action. Sufficient (screenshot): Live Telegram screenshots in the comments show durable Thinking blocks and final answers delivered once after the fix; they do not cover the remaining TTS-enabled blocker.
  • proof: sufficient: Contributor real behavior proof is sufficient. Live Telegram screenshots in the comments show durable Thinking blocks and final answers delivered once after the fix; they do not cover the remaining TTS-enabled blocker.
  • proof: 📸 screenshot: Contributor real behavior proof includes screenshot evidence. Live Telegram screenshots in the comments show durable Thinking blocks and final answers delivered once after the fix; they do not cover the remaining TTS-enabled blocker.
  • mantis: telegram-visible-proof: Mantis should capture Telegram visible proof. The PR changes visible Telegram /reasoning on chat behavior, and the final safe branch should be easy to demonstrate in Telegram Desktop.
Evidence reviewed

PR surface:

Source +36, Tests +128. Total +164 across 4 files.

View PR surface stats
Area Files Added Removed Net
Source 2 44 8 +36
Tests 2 128 0 +128
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 4 172 8 +164

Acceptance criteria:

  • [P1] node scripts/run-vitest.mjs src/auto-reply/reply/dispatch-from-config.test.ts.
  • [P1] node scripts/run-vitest.mjs extensions/telegram/src/bot-message-dispatch.test.ts.

What I checked:

Likely related people:

  • vincentkoc: Authored this PR and recent commits in shared reply dispatch and Telegram dispatch paths central to the reasoning-delivery change. (role: current PR author and recent central dispatch/Telegram contributor; confidence: high; commits: 8ba7e4463812, 54ab69d23bbb, f90ec6d7bea7; files: src/auto-reply/reply/dispatch-from-config.ts, extensions/telegram/src/bot-message-dispatch.ts)
  • obviyus: Recent Telegram dispatch history shows progress-preview and delivery changes in the same dispatcher module that now routes visible reasoning blocks. (role: recent Telegram delivery contributor; confidence: medium; commits: a8b5f5d5518b, 6fd6bddb927c, f4dee9957480; files: extensions/telegram/src/bot-message-dispatch.ts)
  • steipete: Recent TTS config and speech-core history covers the shared TTS behavior that remains a blocker for reasoning payloads. (role: recent TTS and speech-core contributor; confidence: medium; commits: f5b6a977d743, 4df95d3c3fd6, e1c88d4425bc; files: src/tts/tts-config.ts, packages/speech-core/src/tts.ts)
  • amknight: Recent shared dispatch work changed final-delivery state in the same dispatchReplyFromConfig area that now sends allowed reasoning finals through TTS. (role: recent shared reply-dispatch contributor; confidence: low; commits: eab22a911a2d; files: src/auto-reply/reply/dispatch-from-config.ts)
What the crustacean ranks mean
  • 🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
  • 🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
  • 🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
  • 🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
  • 🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
  • 🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
  • 🌊 off-meta tidepool: rating does not apply to this item.

Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

How this review workflow works
  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

@clawsweeper clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. mantis: telegram-visible-proof Mantis should capture Telegram visible proof. P2 Normal backlog priority with limited blast radius. merge-risk: 🚨 message-delivery 🚨 May drop, duplicate, misroute, suppress, or wrongly target messages. labels Jun 19, 2026
@Marvinthebored

Copy link
Copy Markdown
Contributor

Courtesy proof for #95051 — verified on a live OpenClaw 2026.6.9-beta.1 instance with this PR's branch deployed, model deepseek-v4-flash, /reasoning on, on Telegram:

Thinking now persists as a durable standalone message (it was dropped entirely before this PR).
No duplication — the thinking block and the answer each render once per turn; no double-send.

Turn 1 (plain reasoning) and Turn 2 (repeat-prompt) — both show the persisted Thinking block:

turn 1
turn 2

(deepseek-direct is just a local openai-completions config alias to the deepseek API used for the test.)

One small observation for a possible follow-up: the durable Thinking block currently lands after the final answer rather than before it. That looks like the same root cause as #95280 — deepseek emits no discrete thinking_end at the reasoning→answer transition — which #95298 seals. Happy to help wire that in if useful.

cc @Peetiegonzalez

@Peetiegonzalez

Copy link
Copy Markdown
Contributor

Yo, Space minster... Human here...

Super happy to see this direction, I had two PRs ready but you literally gazumped both of them overnight. Fixing two issues I filed, so that's amazing. And it reduces my commit-footprint hugely when I finally land that giga-PR.

Just a teeny tiny suggestion, there was one variable in your PR that was hardcoded that I thought might be better type checked. That's all. My incoming stream is counting on you.

you're our only hope. VK1
luv u babe.

@clawsweeper clawsweeper Bot added proof: 📸 screenshot Contributor real behavior proof includes screenshot evidence. and removed status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. labels Jun 21, 2026
@clawsweeper clawsweeper Bot added status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. and removed rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. labels Jun 21, 2026
@fuller-stack-dev

Copy link
Copy Markdown
Member

Opened #97875 as the narrower canonical upstream fix for the durable thinking-lane part of this PR.

That PR keeps the shared dispatcher generic by adding reasoningPayloadsEnabled, opts Telegram in only when its resolved reasoning level is not off, preserves default suppression for channels without a reasoning lane, and bypasses shared TTS for reasoning payloads.

I would route the Telegram durable reasoning fix through #97875 and retire or rebase this branch onto it if there is still extra Telegram-specific proof to preserve.

@fuller-stack-dev

Copy link
Copy Markdown
Member

Closing this as superseded by the narrower canonical fix that landed in #97875.

#97875 merged as 455f813d6ee606864e6db17a51ad6e7adfd13786 and covers the durable Telegram reasoning path this PR was trying to repair:

  • shared reasoning payloads remain suppressed by default for generic channels
  • Telegram opts into durable reasoning payloads only when it has a deliverable reasoning lane
  • /reasoning on, separate streamed reasoning lanes, and progress-stream suppression are covered by tests
  • CI run 28411526182 was green, including QA Smoke CI and check-test-types; latest Real behavior proof run 28411676681 passed

Thanks for pushing this path forward; the smaller upstream boundary fix is now on main.

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

Labels

channel: telegram Channel integration: telegram maintainer Maintainer-authored PR mantis: telegram-visible-proof Mantis should capture Telegram visible proof. merge-risk: 🚨 message-delivery 🚨 May drop, duplicate, misroute, suppress, or wrongly target messages. P2 Normal backlog priority with limited blast radius. proof: 📸 screenshot Contributor real behavior proof includes screenshot evidence. proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. size: S status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

/reasoning on drops the thinking lane on Telegram — renders under /reasoning stream, lost under on

4 participants