Skip to content

fix(telegram): clear split reasoning previews#94631

Closed
hugenshen wants to merge 1 commit into
openclaw:mainfrom
hugenshen:codex/fix-80862-telegram-reasoning-preview
Closed

fix(telegram): clear split reasoning previews#94631
hugenshen wants to merge 1 commit into
openclaw:mainfrom
hugenshen:codex/fix-80862-telegram-reasoning-preview

Conversation

@hugenshen

Copy link
Copy Markdown
Contributor

Summary

  • Fixes Telegram /reasoning stream preview cleanup when a reasoning segment ends and a later reasoning segment starts.
  • Clears the existing reasoning draft preview before forceNewMessage() resets the stream state, preserving the old preview id long enough for Telegram cleanup.
  • Keeps the repair local to the Telegram reasoning draft lifecycle; answer/tool progress draft behavior is intentionally unchanged.
  • Adds a focused regression that models the real id-dropping behavior of forceNewMessage() and proves cleanup happens before the next reasoning preview update.

Linked context

Closes #80862

Related #80862

Was this requested by a maintainer or owner?

No direct maintainer request; ClawSweeper marked the issue source-reproducible and queueable.

Real behavior proof (required for external PRs)

  • Behavior addressed: Telegram reasoning previews could be orphaned when /reasoning stream split into a later reasoning message because forceNewMessage() reset the draft stream before the old preview id was cleaned up.
  • Real environment tested: macOS local checkout, Node/Vitest Telegram extension test lane.
  • Exact steps or command run after this patch: node scripts/run-vitest.mjs extensions/telegram/src/bot-message-dispatch.test.ts extensions/telegram/src/draft-stream.test.ts
  • Evidence after fix: Test Files 2 passed (2); Tests 176 passed (176); [test] passed 1 Vitest shard
  • Observed result after fix: The new regression verifies the prior reasoning preview is cleared before forceNewMessage() and before the second reasoning preview update.
  • What was not tested: Live Telegram DM proof with real /reasoning stream; no Telegram credentials were used locally.
  • Proof limitations or environment constraints: The fix is covered by source-level Telegram dispatcher tests, but live transport proof should still be collected by a maintainer or credentialed environment before landing if required.
  • Before evidence (optional but encouraged): Issue Telegram uses multiple sendMessage previews instead of editMessageText, leaving stale reasoning messages #80862 reports repeated sendMessage reasoning previews with no matching editMessageText or deleteMessage; source showed forceNewMessage() dropped the old stream message id before cleanup.

Tests and validation

  • node scripts/run-vitest.mjs extensions/telegram/src/bot-message-dispatch.test.ts extensions/telegram/src/draft-stream.test.ts
  • git diff --check -- extensions/telegram/src/bot-message-dispatch.ts extensions/telegram/src/bot-message-dispatch.test.ts
  • pnpm exec oxfmt --check --threads=1 extensions/telegram/src/bot-message-dispatch.ts extensions/telegram/src/bot-message-dispatch.test.ts
  • node scripts/run-oxlint.mjs extensions/telegram/src/bot-message-dispatch.ts extensions/telegram/src/bot-message-dispatch.test.ts
  • .agents/skills/autoreview/scripts/autoreview --mode local --prompt "Review the staged/working-tree changes for issue #80862 only..."

Regression coverage added:

  • Added a dispatcher test for onReasoningStream -> onReasoningEnd -> onReasoningStream that uses a draft stream whose forceNewMessage() clears the message id, matching the real draft stream behavior.

What failed before this fix, if known?

  • The second reasoning stream could call forceNewMessage() before cleanup, causing the previous Telegram reasoning preview message id to be lost and leaving the old preview in chat.

If no test was added, why not?

  • Not applicable; focused regression coverage was added.

Risk checklist

Did user-visible behavior change? (Yes/No)

Yes. Telegram reasoning preview messages should no longer remain stale when reasoning is split into a later preview message.

Did config, environment, or migration behavior change? (Yes/No)

No.

Did security, auth, secrets, network, or tool execution behavior change? (Yes/No)

No.

What is the highest-risk area?

Telegram draft preview lifecycle ordering for reasoning-only stream splits.

How is that risk mitigated?

The change is scoped to the reasoning lane's split path and uses existing draft-stream clear() cleanup before starting the next preview message. Existing dispatcher and draft-stream focused tests pass, and autoreview reported no accepted/actionable findings.

Current review state

What is the next action?

Open PR from codex/fix-80862-telegram-reasoning-preview and request maintainer review; collect live Telegram proof if reviewers require transport-level validation.

What is still waiting on author, maintainer, CI, or external proof?

CI and optional live Telegram DM proof are still pending after PR creation.

Which bot or reviewer comments were addressed?

Addressed the ClawSweeper issue analysis that identified onReasoningEnd -> forceNewMessage() orphaning the reasoning preview cleanup.

@openclaw-barnacle openclaw-barnacle Bot added channel: telegram Channel integration: telegram size: XS triage: mock-only-proof Candidate: PR proof only shows tests, mocks, snapshots, lint, typecheck, or CI. labels Jun 18, 2026
@clawsweeper

clawsweeper Bot commented Jun 18, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs real behavior proof before merge. Reviewed June 20, 2026, 8:19 PM ET / 00:19 UTC.

Summary
The PR clears the active Telegram reasoning preview before forcing a new reasoning draft message and adds a regression test for split reasoning streams.

PR surface: Source +11, Tests +30. Total +41 across 2 files.

Reproducibility: yes. at source level. Current main sets splitReasoningOnNextStream on reasoning end, then calls forceNewMessage before clearing the old Telegram preview id; I did not run a live Telegram DM reproduction in this read-only review.

Review metrics: 1 noteworthy metric.

  • Live Telegram proof: 0 live transport runs supplied. The diff changes visible Telegram streaming cleanup, and the matching maintainer note requires real Telegram proof before merge.

Root-cause cluster
Relationship: fixed_by_candidate
Canonical: #80862
Summary: This PR is the active candidate fix for the open Telegram stale reasoning-preview issue; earlier same-root-cause PRs are closed unmerged and not safe canonical landing paths.

Members:

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

Merge readiness
Overall: 🦪 silver shellfish
Proof: 🦪 silver shellfish
Patch quality: 🦞 diamond lobster
Result: blocked until real behavior proof from a real setup is added.

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

Rank-up moves:

  • [P1] Add redacted live Telegram DM/Desktop proof showing the old reasoning preview clears and the next preview appears.
  • Update the PR body after adding proof so ClawSweeper re-runs automatically, or ask a maintainer to comment @clawsweeper re-review.

Proof guidance:

  • [P1] Needs real behavior proof before merge: The PR body supplies local Vitest/mock proof and explicitly says no live Telegram credentials or DM proof were used; add redacted screenshot/video, terminal live output, or logs and update the PR body for re-review.

Mantis proof suggestion
A native Telegram recording would directly prove whether split reasoning clears the old preview while showing the next one. A maintainer can ask Mantis to capture proof by posting this exact PR comment:

@openclaw-mantis telegram desktop proof: verify `/reasoning stream` split reasoning clears the old preview and shows the next reasoning preview without leaving stale messages.

Risk before merge

  • [P1] Merging without a live Telegram DM/Desktop run leaves the visible delivery behavior unverified: the old reasoning preview must disappear and the next reasoning preview must still appear in a real chat.

Maintainer options:

  1. Collect live Telegram proof (recommended)
    Use Telegram Desktop, Mantis, or an equivalent redacted live run to show split /reasoning stream clears the old preview and displays the next preview before merge.
  2. Accept the proof gap
    A maintainer can intentionally merge on source and regression coverage while owning the remaining live-transport uncertainty.

Next step before merge

  • [P1] Manual proof handling is needed because automation cannot supply the contributor's live Telegram evidence, and no concrete code repair was identified.

Security
Cleared: No security or supply-chain concern found; the diff is limited to Telegram TypeScript control flow and a colocated regression test.

Review details

Best possible solution:

Land the focused Telegram reasoning-lane cleanup after redacted live Telegram proof, or explicit maintainer acceptance of the proof gap, confirms split reasoning previews no longer leave stale messages.

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

Yes, at source level. Current main sets splitReasoningOnNextStream on reasoning end, then calls forceNewMessage before clearing the old Telegram preview id; I did not run a live Telegram DM reproduction in this read-only review.

Is this the best way to solve the issue?

Yes for the code shape. Clearing the reasoning lane before forceNewMessage is a narrow fix for the identified lifecycle bug and keeps answer/tool draft behavior unchanged; live Telegram proof remains the merge gate.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 6c2c43d63f4c.

Label changes

Label justifications:

  • P2: The PR targets a concrete Telegram channel bug with visible stale duplicate reasoning previews but limited blast radius.
  • merge-risk: 🚨 message-delivery: The diff changes Telegram preview cleanup and message-creation ordering, so real chat proof is needed to ensure previews are neither left stale nor dropped.
  • rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🦪 silver shellfish and patch quality is 🦞 diamond lobster.
  • status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: The PR body supplies local Vitest/mock proof and explicitly says no live Telegram credentials or DM proof were used; add redacted screenshot/video, terminal live output, or logs and update the PR body for re-review.
  • mantis: telegram-visible-proof: Mantis should capture Telegram visible proof. The PR changes visible Telegram reasoning-preview chat behavior that can be demonstrated in a short Telegram Desktop recording.
Evidence reviewed

PR surface:

Source +11, Tests +30. Total +41 across 2 files.

View PR surface stats
Area Files Added Removed Net
Source 1 13 2 +11
Tests 1 30 0 +30
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 2 43 2 +41

What I checked:

Likely related people:

  • vincentkoc: Current-main blame for the implicated reasoning split branch, rotate helper, and draft-stream cleanup contract points to Vincent Koc's recent commits on this path. (role: recent area contributor; confidence: high; commits: ab165d119cc7, 4954d025e21e, 844f405ac1be; files: extensions/telegram/src/bot-message-dispatch.ts, extensions/telegram/src/draft-stream.ts)
  • obviyus: History shows Ayaan Zaidi introduced and refined the split Telegram reasoning and answer draft stream behavior that this PR repairs. (role: feature-history contributor; confidence: medium; commits: ab256b8ec71f, 8b1fe0d1e240, 575bd77196c3; files: src/telegram/bot-message-dispatch.ts, src/telegram/bot-message-dispatch.test.ts, src/telegram/draft-stream.ts)
  • steipete: Recent Telegram plugin refactors and draft-stream/test hardening touched the same dispatcher and streaming-preview surfaces. (role: adjacent area contributor; confidence: medium; commits: 03a43fe23157, ab96520bbad5, 3451159174c8; files: extensions/telegram/src/bot-message-dispatch.ts, extensions/telegram/src/draft-stream.ts, extensions/telegram/src/bot-message-dispatch.test.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: 🦪 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. mantis: telegram-visible-proof Mantis should capture Telegram visible proof. labels Jun 18, 2026
@clawsweeper clawsweeper Bot added 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
@hugenshen hugenshen closed this Jun 21, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

channel: telegram Channel integration: telegram 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. rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. size: XS status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. triage: mock-only-proof Candidate: PR proof only shows tests, mocks, snapshots, lint, typecheck, or CI.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Telegram uses multiple sendMessage previews instead of editMessageText, leaving stale reasoning messages

1 participant