Skip to content

fix(telegram): keep tool progress after non-final commentary#90997

Closed
wsyjh8 wants to merge 1 commit into
openclaw:mainfrom
wsyjh8:fix/telegram-90962-commentary-tool-progress
Closed

fix(telegram): keep tool progress after non-final commentary#90997
wsyjh8 wants to merge 1 commit into
openclaw:mainfrom
wsyjh8:fix/telegram-90962-commentary-tool-progress

Conversation

@wsyjh8

@wsyjh8 wsyjh8 commented Jun 6, 2026

Copy link
Copy Markdown
Contributor

Summary

In non-persist progress streaming mode with streaming.progress.commentary
enabled, Telegram drops the tool-progress lines that follow inter-tool
commentary: the live progress draft shows the model's "Let me check X…"
commentary, but the subsequent 📖 Read … / tool lines never appear. Every
other streaming channel keeps tool progress and commentary coexisting.

Root cause

Non-final inter-tool commentary suppressed the progress draft via
prepareAnswerLaneForText. In progress mode commentary arrives as assistant
partial text (onPartialReply → ingestDraftLaneSegments); for an answer-lane
segment that path calls prepareAnswerLaneForText →
rotateAnswerLaneAfterToolProgress, which clear()s the draft and calls
suppressProgressDraftState(). The next tool's pushStreamToolProgress is then
dropped inside the compositor on the progressSuppressed guard, so the
tool-progress lines after commentary vanish.

(For precision: the lane is not "finalized" — rotateAnswerLaneAfterToolProgress
resets finalized to false and suppresses the compositor; the drop is the
progressSuppressed guard, not the finalized guard.)

Fix

In progress mode, when commentary is enabled, route non-final answer-lane
partial text into the shared progress draft's commentary lane
(progressDraft.pushCommentaryProgress, keyed per assistant message) so
commentary and tool progress accumulate together in one open draft instead of
tearing the lane down. This reuses the same commentary lane the onItemEvent
preamble path already uses.

Deliberately unchanged:

  • The pushStreamToolProgress finalized / final-delivery guard — it is correct
    for real final delivery and is not relaxed.
  • The final-answer cleanup path — the draft still rotates into its own final
    message on completion.
  • No new config field or accessor; the branch is gated on the existing
    progressDraft.commentaryProgressEnabled.

Trigger conditions / scope

Reproduces only when all three hold within one turn:

  1. streaming.progress.commentary enabled (default: off)
  2. non-persist progress streaming mode
  3. inter-tool commentary and tool calls interleave in the turn

Default config is unaffected. The diverting branch is gated on commentary
being enabled; with commentary off, the answer-lane path is unchanged from main
(prepareAnswerLaneForText), so users on defaults see no behavior change.

Testing

  • New dispatch regression in bot-message-dispatch.test.ts drives the
    commentary → tool → commentary → tool interleave and asserts every
    tool-progress line (and the commentary) stays in the draft. It fails on main
    (the second tool line is dropped) and passes with this change.
  • Green locally: extensions/telegram/src/bot-message-dispatch.test.ts,
    src/plugin-sdk/channel-streaming.test.ts,
    extensions/discord/src/monitor/message-handler.process.test.ts; plus
    tsgo:extensions (+ :test), lint:extensions, and oxfmt --check clean.
  • Observed a pre-existing, unrelated failure in the prompt-context-cache test
    under Windows (/tmp path resolution); it reproduces on clean main and is
    out of scope for this PR.
  • Live Telegram Desktop capture to follow once the fix(cli): bridge inter-tool commentary events to channel progress #90883 producer path lands —
    that's what surfaces real inter-tool commentary to reproduce this end to end.

Relation to #90883

This is the Telegram consumer-side fix and is independent of #90883 (the
commentary producer). It should land first: once #90883 merges, every
user with commentary + non-persist progress would immediately hit this clobber,
so shipping this guard ahead of it prevents the regression.

Fixes #90962

@wsyjh8
fix(telegram): keep tool progress after non-final commentary
4b42898 
Root cause: non-final inter-tool commentary suppressed the progress draft via
prepareAnswerLaneForText. In non-persist progress streaming mode (with
streaming.progress.commentary enabled) commentary arrives as assistant partial
text (onPartialReply -> ingestDraftLaneSegments), whose answer-lane rotation in
rotateAnswerLaneAfterToolProgress called suppressProgressDraftState. The next
tool's pushStreamToolProgress was then dropped on the compositor's
progressSuppressed guard, so the tool-progress lines after commentary vanished.

Fix: in progress mode, route non-final answer-lane partial text into the shared
progress draft's commentary lane (progressDraft.pushCommentaryProgress, keyed
per assistant message) so commentary and tool progress accumulate in one open
draft instead of tearing the lane down. This reuses the same commentary lane the
onItemEvent preamble path already uses. The pushStreamToolProgress finalized /
final-delivery guard and the final-answer cleanup path are left unchanged, and
no new config is added.

Adds a dispatch regression covering the commentary -> tool -> commentary -> tool
interleave that asserts every tool-progress line stays in the draft.

This is the Telegram consumer-side fix, independent of openclaw#90883 (the commentary
producer); it should land first so enabling commentary + non-persist progress
does not immediately regress.

Fixes openclaw#90962

Signed-off-by: Jason Yao <[email protected]>
@openclaw-barnacle openclaw-barnacle Bot added channel: telegram Channel integration: telegram size: S triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup. labels Jun 6, 2026
@clawsweeper

clawsweeper Bot commented Jun 6, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Codex review: needs real behavior proof before merge. Reviewed June 21, 2026, 2:57 PM ET / 18:57 UTC.

Summary
The PR routes Telegram progress-mode answer-lane partial text into the shared commentary progress draft when commentary progress is enabled and adds a dispatcher regression test for commentary/tool interleaving.

PR surface: Source +17, Tests +48. Total +65 across 2 files.

Reproducibility: yes. for the source path: current main can suppress the progress compositor when answer-lane partial text follows tool progress, and the suppressed compositor then drops later tool progress. I did not run a fresh live Telegram reproduction in this read-only review.

Review metrics: 1 noteworthy metric.

  • Progress Commentary Scope: 1 gated path added; default path unchanged. The diff changes only commentary-enabled progress handling while the reported default/off interleave may still drop later tool progress.

Root-cause cluster
Relationship: fixed_by_candidate
Canonical: #90962
Summary: This PR is the candidate fix for the open Telegram progress/commentary issue, but scope and proof remain unresolved.

Members:

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

Merge readiness
Overall: 🧂 unranked krab
Proof: 🧂 unranked krab
Patch quality: 🦐 gold shrimp
Result: blocked until real behavior proof is added.

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

Rank-up moves:

  • Resolve whether the default commentary-off interleave is in scope and update the branch or linked issue accordingly.
  • [P1] Add redacted real Telegram proof showing commentary and later tool progress remain visible in the approved scope.
  • Rebase or refresh the branch against current main before final review.

Proof guidance:

  • [P1] Needs real behavior proof before merge: No after-fix real Telegram proof is present for this PR; add redacted Telegram Desktop, bot-to-bot transcript, terminal/live output, or logs and update the PR body for re-review.

Mantis proof suggestion
A native Telegram recording would directly show whether commentary and later tool-progress lines remain visible together in one draft. A maintainer can ask Mantis to capture proof by posting this exact PR comment:

@openclaw-mantis telegram desktop proof: verify progress-mode commentary and later tool progress stay visible together in one Telegram draft.

Risk before merge

  • [P1] The PR changes only the commentary-enabled path; if the linked bug scope includes default commentary-off untagged partial text, later Telegram tool progress can still be dropped.
  • [P1] The Telegram maintainer note requires real Telegram proof for streaming changes, and this PR body still says live Telegram Desktop capture is to follow.
  • [P1] The branch is stale against current main and GitHub currently reports mergeability as unknown, so maintainers need a rebase or refreshed review before trusting the exact merge result.

Maintainer options:

  1. Resolve Default Progress Scope (recommended)
    Decide whether the fix must cover commentary-off/default untagged partial text, then update the branch and regression test to match that scope.
  2. Accept Commentary-Enabled Scope
    Maintainers can explicitly accept this as a commentary-enabled-only fix and leave the default/off behavior to the linked issue or a separate PR.
  3. Pause For Live Shape Proof
    Hold the PR until a real provider plus Telegram run proves whether the untagged inter-tool partial text path is user-facing and how it should render.

Next step before merge

  • [P1] The remaining action is maintainer scope judgment plus contributor proof and rebase, not a narrow autonomous repair on the current branch.

Security
Cleared: The diff only changes Telegram dispatch TypeScript and a colocated test, with no dependency, workflow, lockfile, secret, package-resolution, or supply-chain surface.

Review findings

  • [P1] Cover the default progress interleave — extensions/telegram/src/bot-message-dispatch.ts:1171
Review details

Best possible solution:

Land a maintainer-approved Telegram progress-mode fix for the intended partial-text scope, with regression coverage and redacted live Telegram proof showing later tool progress remains visible.

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

Yes for the source path: current main can suppress the progress compositor when answer-lane partial text follows tool progress, and the suppressed compositor then drops later tool progress. I did not run a fresh live Telegram reproduction in this read-only review.

Is this the best way to solve the issue?

No as-is: the patch is a plausible commentary-enabled fix, but it is too narrow if the approved scope includes the default commentary-off interleave or suppressing untagged tool-use assistant text instead.

Full review comments:

  • [P1] Cover the default progress interleave — extensions/telegram/src/bot-message-dispatch.ts:1171
    This branch only diverts answer-lane partial text when progressDraft.commentaryProgressEnabled is true, but Telegram docs keep commentary off by default and the contributor reports the same untagged interleave still drops the later tool line with commentary off. Cover the default/off path or explicitly narrow the linked issue scope before merge.
    Confidence: 0.86

Overall correctness: patch is incorrect
Overall confidence: 0.86

AGENTS.md: found and applied where relevant.

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

Label changes

Label justifications:

  • P2: This is a bounded Telegram progress streaming bug with visible channel impact but no core outage, security issue, or data loss.
  • merge-risk: 🚨 message-delivery: The PR changes visible Telegram progress-draft delivery while a default-path tool-progress drop and live transport proof gap remain unresolved.
  • rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🧂 unranked krab and patch quality is 🦐 gold shrimp.
  • status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: No after-fix real Telegram proof is present for this PR; add redacted Telegram Desktop, bot-to-bot transcript, 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 progress-draft rendering that can be demonstrated in a short Telegram Desktop recording.
Evidence reviewed

PR surface:

Source +17, Tests +48. Total +65 across 2 files.

View PR surface stats
Area Files Added Removed Net
Source 1 17 0 +17
Tests 1 48 0 +48
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 2 65 0 +65

What I checked:

Likely related people:

  • snowzlmbot: Authored the merged current-main Telegram progress placeholder work that rewrote the same dispatcher and progress compositor surface. (role: recent area contributor; confidence: medium; commits: 6441e5646594; files: extensions/telegram/src/bot-message-dispatch.ts, src/channels/progress-draft-compositor.ts)
  • obviyus: Authored the earlier Telegram draft streaming feature and appears repeatedly in the Telegram dispatcher history for streaming and preview behavior. (role: Telegram streaming feature owner; confidence: high; commits: 6edb512efaeb, 575bd77196; files: extensions/telegram/src/bot-message-dispatch.ts)
  • vincentkoc: Authored channel streaming config normalization and recent release/current-main work around the same Telegram streaming configuration surface. (role: channel streaming config contributor; confidence: medium; commits: 0fdf9e874b7e, c645ec4555c0; files: extensions/telegram/src/bot-message-dispatch.ts, docs/channels/telegram.md)
  • anagnorisis2peripeteia: Authored the merged CLI commentary bridge that feeds commentary events into channel progress paths and opened the canonical linked issue. (role: adjacent commentary delivery contributor; confidence: medium; commits: 2858c629bd0f; files: src/auto-reply/reply/agent-runner-cli-dispatch.ts, src/auto-reply/reply/agent-runner-execution.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. P2 Normal backlog priority with limited blast radius. merge-risk: 🚨 message-delivery 🚨 May drop, duplicate, misroute, suppress, or wrongly target messages. labels Jun 6, 2026
@849261680 849261680 mentioned this pull request Jun 6, 2026
Open
3 tasks
@wsyjh8

wsyjh8 commented Jun 6, 2026

Copy link
Copy Markdown
Contributor Author

Correction after a deeper producer-side trace

A closer look at where this fix's inputs actually come from shows the PR's stated
trigger — and its "default config is unaffected" claim — are partly wrong. Details
and a scope question below.

1. Corrected trigger

  • Phase-tagged commentary routes via onItemEvent/preamble → pushCommentaryProgress,
    which already coexists with tool progress — so it is not what this PR fixes.
    (Verified for CLI/fix(cli): bridge inter-tool commentary events to channel progress #90883: read its diff and cherry-picked it onto the fix branch;
    codex extensions/codex/src/app-server/event-projector.ts:482-490: read.)
  • The path this PR actually touches is untagged inter-tool assistant text reaching
    onPartialReply     (non-OpenAI-Responses providers, e.g. Anthropic). This part is
    inferred from code, not yet confirmed with a live run: resolveAssistantMessagePhase
    (src/shared/chat-message-content.ts:70-96) returns undefined without a
    phase/textSignature, so the unconditional if (deliveryPhase === "commentary") return;
    at src/agents/embedded-agent-subscribe.handlers.messages.ts:678 doesn't fire; and
    src/agents/embedded-agent-subscribe.handlers.lifecycle.test.ts:469 shows pre-tool text
    captured as streamed (assistantTexts: ["Initial analysis…"]) for a stopReason:"toolUse"
    message — though that test asserts the turn is marked abandoned.

2. Gating gap (retraction) — confirmed by a dispatcher run

The fix is gated on progressDraft.commentaryProgressEnabled, so it only covers
streaming.progress.commentary: on. The trigger above is commentary-agnostic. Driving the
dispatcher with commentary off and the untagged sequence (onPartialReply text →
onToolStart → onPartialReply text → onItemEvent tool), the draft renders only the
first tool line, then clear()s, and the second tool line is dropped — the branch
falls through to prepareAnswerLaneForText(). So I'm retracting the PR body's
"default config is unaffected / this is the fix": the default (and highest-frequency)
case is not fixed here.

3. Scope question

Before I finish this, could you confirm #90962's intended scope:

  • (a) commentary-on subset only (keep the gate), or
  • (b) all inter-tool partial answer text (drop the gate so the default config is covered)?

Note I haven't confirmed live that a shipped provider actually emits untagged inter-tool text
through onPartialReply in a real turn — it's inferred from the phase resolver + line-678 +
lifecycle.test.ts:469. If in practice nothing reaches this path, that reframes #90962
entirely, so I'd value your read on whether this is a real user-facing path or a latent one.

Going by its title, #71589 ("suppress tool-use assistant text", open) may point the
opposite way — suppressing the inter-tool text this PR surfaces. I haven't read its diff;
flagging in case surface-vs-suppress is unreconciled.

4. Next step

Once you confirm direction I'll adjust accordingly (drop/keep the gate per (a)/(b)) and add
an extracted-unit before/after proof driven by the real Anthropic onPartialReply event
shape, rather than re-skinning the existing unit test.

@clawsweeper clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. and removed rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. labels Jun 15, 2026
This was referenced Jun 19, 2026
Closed
Merged
Closed
@clawsweeper clawsweeper Bot mentioned this pull request Jul 2, 2026
Merged
@obviyus

obviyus commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

Closing as superseded by #98907, which landed on main as dae86e6.

Decision: close in favor of the broader root-cause fix.

Why: this PR patched the commentary-vs-tool-progress loss narrowly and had drifted into conflict with main. #98907 fixes the same loss at the lifecycle level — Telegram now consumes the shared progress-draft compositor (lane prefixes, markdown rendering, parse_mode=HTML transport) and keeps a single stationary window message across commentary→tool handovers — and ships with regressions plus live Telegram Desktop / Mantis proof. The specific scenario this PR targeted is covered by the landed test "uses one stationary window message across a multi-boundary turn (commentary→tool→commentary→tool→final)" in extensions/telegram/src/bot-message-dispatch.test.ts.

Thanks @wsyjh8 for identifying and attacking this — the report and PR helped pin the canonical issue (#90962). If you see any remaining gap on a build containing dae86e6, please open a fresh issue with a capture.

@obviyus obviyus closed this Jul 3, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

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: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. size: S status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Telegram: inter-tool commentary clobbers tool progress in non-persist progress mode (diverges from other streaming channels)

2 participants

@wsyjh8 @obviyus