feat(telegram): non-streaming tool+commentary progress accumulator#89890
feat(telegram): non-streaming tool+commentary progress accumulator#89890anagnorisis2peripeteia wants to merge 12 commits into
Conversation
|
Codex review: needs maintainer review before merge. Reviewed June 6, 2026, 2:59 PM ET / 18:59 UTC. Summary PR surface: Source +321, Tests +591, Docs +1, Generated 0. Total +913 across 23 files. Reproducibility: not applicable. as a feature PR. Source inspection confirms current main lacks this off-mode persistence path, and the supplied Telegram proof shows the proposed after-fix behavior. Review metrics: 2 noteworthy metrics.
Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Mantis proof suggestion Risk before merge
Maintainer options:
Next step before merge
Security Review detailsBest possible solution: Land or update the prerequisite stack first, then refresh this branch and either explicitly accept the shared Do we have a high-confidence way to reproduce the issue? Not applicable as a feature PR. Source inspection confirms current main lacks this off-mode persistence path, and the supplied Telegram proof shows the proposed after-fix behavior. Is this the best way to solve the issue? Unclear until maintainer API review. The stream-off accumulator is a plausible narrow Telegram implementation, but the shared AGENTS.md: found and applied where relevant. Codex review notes: model gpt-5.5, reasoning high; reviewed against f2677d55ecf0. Label changesLabel justifications:
Evidence reviewedPR surface: Source +321, Tests +591, Docs +1, Generated 0. Total +913 across 23 files. View PR surface stats
Acceptance criteria:
What I checked:
Likely related people:
What the crustacean ranks mean
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 re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
|
@clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
|
@clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
Inter-tool commentary (assistant text emitted before a tool call, surfaced by the CLI parser as a stream:"item", kind:"preamble" agent event) landed on the agent-event bus with no subscriber and was silently dropped: runCliAgentWithLifecycle bridges the assistant, reasoning, and tool streams to channel callbacks, but the item/preamble stream had no bridge. Add createCommentaryEventBridge to forward it to onItemEvent, so CLI commentary reaches the channel's commentary render hook. This is the CLI-dispatch delivery half of the commentary feature: the parser emission (claude-cli) feeds the bus; this bridge delivers it to the channel. Addresses the claude-cli case of intermediate-text-lost (openclaw#87326 / openclaw#84486).
Commentary lines carry noCompact so the progress-draft renderer does not compact them like tool lines — assistant prose renders in full, spilling to a new message at the channel limit rather than truncating mid-sentence.
Detect text accumulated before tool_use blocks in the Claude CLI streaming parser and emit it as commentary via a new onCommentaryText callback. This enables the same commentary progress display that the Codex backend already provides through preamble item events. - Add onCommentaryText optional callback to createCliJsonlStreamingParser - Flush accumulated assistantText as commentary when content_block_start with tool_use type is encountered - Track last flushed position to avoid duplicate emissions on consecutive tool_use blocks without intervening text - Wire the callback in both execute.ts (regular CLI spawn + live session) and claude-live-session.ts, emitting AgentItemEventData with kind=preamble and progressText - Add 3 test cases covering: text before tool_use, empty text before tool_use, and consecutive tool_use dedup
8692ed4 to
4034483
Compare
|
Updated: stream-off accumulation now works — fixed the double-wrap (tool lines) and the progress-mode guard (commentary), added a guard-free |
|
🦞👀 Command router queued. I will update this comment with the next step. |
aed9df8 to
62fa8b4
Compare
|
🦞👀 Command router queued. I will update this comment with the next step. |
…a media (photo) final
|
Added real-behavior proof for the media-final path (Telegram Desktop, stream-off + persistProgress):
The accumulated progress set is delivered first as a single text-only message (💭 commentary interleaved with 📖 tool reads); the answer's photo ( Note on strict media-only (no answer text): a safety-conscious model won't reliably emit text-free media on command (it treated "reply with only this" as link-laundering and refused), so the no-text branch of the common-path flush is covered deterministically by the dispatch regression @clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
Moving the progress-set flush to the common final path made it fire on the bare absence of an answer-lane draft (!answerLane.stream), which is also true in partial/progress turns that lack a live draft (e.g. selected-quote replies) — so those turns could get a surprise durable progress message. Gate accumulation, suppressed-source callback forwarding, and emission on a single streamOffPersistEnabled = persistProgressEnabled && streamMode === 'off' so the accumulator only ever engages in genuine stream-off turns. Regression: a no-draft partial-mode turn with persistProgress emits no progress set.
|
Fixed the [P2/P1] no-draft leak ( On the strict media-only proof: a safety-conscious model won't emit text-free media on command (asked to "reply with only this," it refused as link-laundering), so a true no-caption media-only final isn't naturally elicitable in the proof harness. The closest real capture is the media-with-caption one above (which proves the progress set stays text-only and the photo lands only on the answer — the payload-isolation fix), and the strict no-text branch of the common-path flush is covered deterministically by the dispatch regression @clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
|
Closing as superseded by #91976, which just landed — with |
Summary
The non-streaming counterpart to
persistProgress: whenstreaming.modeis off, nothing streams during a turn, so tool and commentary progress were lost entirely. This accumulates the commentary + tool-progress lines silently as the turn runs and delivers them as their own message — commentary first, then the tool lines — immediately before the final answer. The user gets the whole progress lane once, when the answer is ready.What it does (with
streaming.mode: "off"+streaming.progress.persistProgress: true):onItemEvent(preamble/commentary) andonToolStart, append to a per-turn log gated onpersistProgress. Nothing is sent mid-turn (stream is off).emitPersistedProgressSetflushes the accumulated lane as its own durable message ahead of the final answer, but only when there is no live draft stream (so the stream-on persist path in feat(streaming): opt-in persistProgress mode for tool+commentary lines #89850 doesn't double-emit).persistProgressis set, so the accumulator actually runs inmode:off.resolveChannelStreamingProgressCommentaryEnabledandresolveChannelStreamingProgressToolProgressEnabled, which read thecommentary/toolProgressopt-ins without the progress-mode guard (the public guarded resolvers only read them in progress mode). The stream-off accumulator gates each lane on its toggle, socommentary:falseortoolProgress:falseomits that lane from the accumulated set; the public guarded resolvers are unchanged (live-draft behavior preserved).Tests
tsgocleanchannel-streaming.test.ts— added a regression for thetoolProgressgate (toolProgress:falseomits the tool lane; guard-free and mode-independent); 29/29 passbot-message-dispatch.test.ts— 103/104 pass; the one failure (records streamed final replies into the prompt context cache) is a pre-existing Windows/tmpstore-roundtrip issue that reproduces on the unmodified base commit (it is green on Linux CI).Proof
Captured via a local recreation of the Mantis (telegram-desktop-proof) flow on crabbox, with
streaming.mode: "off"+persistProgress: true(theseparatestreamkey). Bot DM,claude-clibackend, opus-4.8 / max thinking.Nothing streams during the turn; at final-send the whole lane lands as one set, commentary first, then the tool reads, with the answer as a separate message below it:
Observed:
Motion (silent turn → the set + answer arrive together at the end): recording
Gateway trace confirms the lane accumulated and emitted with no live stream:
emitSet persist=true hasStream=false logLen=4 streamMode=off(1 commentary line + 3 tool lines).Stacking
Built on #89850 (
streaming.progress.persistProgress), which is built on #89834 (claude-cli commentary producer) and #90883 (commentary delivery + render). This branch is rebased on the current #89850 head, so its own diff is just the stream-off accumulator (bot-message-dispatch.ts) + the guard-free resolver (streaming.ts). Merge #90883 → #89834 → #89850 first.Issues
Part of the #87326 progress UX work.