Add Telegram progress preview flows#83847
Conversation
|
Codex review: needs real behavior proof before merge. Workflow note: Future ClawSweeper reviews update this same comment in place. How this review workflow works
Summary Reproducibility: yes. for the PR blocker: source inspection shows the PR formatter emits PR rating Rank-up moves:
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. PR egg Where did the egg go?
Real behavior proof Mantis proof suggestion Risk before merge
Maintainer options:
Next step before merge Security Review findings
Review detailsBest possible solution: Fix the remaining Telegram progress assertion, rerun the targeted touched-surface tests, and land only after maintainers accept the wording change with redacted live Telegram proof. Do we have a high-confidence way to reproduce the issue? Yes for the PR blocker: source inspection shows the PR formatter emits Is this the best way to solve the issue? No, not yet; the direction is plausible, but the patch needs the remaining Telegram test expectation fixed and live Telegram proof before it is the best mergeable solution. Label justifications:
Full review comments:
Overall correctness: patch is incorrect Acceptance criteria:
What I checked:
Likely related people:
Codex review notes: model gpt-5.5, reasoning high; reviewed against 6fcfeed5dc67. |
obviyus
left a comment
There was a problem hiding this comment.
Thanks for the cleanup here. One request before merge: can you wire the label animation into the production Telegram native sendMessageDraft path too, or narrow the PR wording/tests so they do not imply native draft animation ships there?
Right now the dev working-final flow animates native drafts, but production native tool-progress formatting does not pass labelFrame, so sendMessageDraft appears to keep a stable label. Since this PR is UX-facing, the demo flow and production behavior should match.
|
Addressed in Current behavior is now static labels end-to-end: production Telegram native Verification after the cleanup included focused Telegram flow/dispatch tests, cross-channel reasoning/progress tests, and final autoreview clean against In plain english, I decided not to whole animated ellipsis thing after our convo about it potentially backing up other edits since Telegram has a 1RPS limit on updates. |
Summary
thinking-final/working-finalpreview flowsThinkingandWorking, with blank-line-separated progress details and no label animationworking-finalflow through nativesendMessageDraftwhile keeping the preview fixture aligned with production static-label behaviorThinking...replies are not swallowed, with Discord/Feishu/Telegram parser follow-throughVerification
git diff --checknode scripts/run-vitest.mjs test/scripts/channel-message-flows.test.tsnode scripts/run-vitest.mjs extensions/telegram/src/bot-message-dispatch.test.ts test/scripts/channel-message-flows.test.tsnode scripts/run-vitest.mjs src/plugin-sdk/reply-payload.test.ts src/agents/tools/message-tool.test.ts extensions/telegram/src/reasoning-lane-coordinator.test.ts test/scripts/channel-message-flows.test.ts --runnode scripts/run-vitest.mjs extensions/discord/src/monitor/message-handler.process.test.ts extensions/discord/src/chunk.test.ts src/plugin-sdk/channel-streaming.test.ts src/infra/heartbeat-runner.returns-default-unset.test.ts extensions/feishu/src/reply-dispatcher.test.ts extensions/mattermost/src/mattermost/reply-delivery.test.ts extensions/whatsapp/src/auto-reply/deliver-reply.test.ts --runAUTOREVIEW_AUTO_TESTS=0 .agents/skills/autoreview/scripts/autoreview --mode branch --base origin/main --reviewer codex --fallback-reviewer noneReal behavior proof
Behavior addressed: Telegram progress previews can be locally exercised with flow fixtures, native Telegram tool-progress previews use
sendMessageDraft, and progress/reasoning labels now render as staticWorking/Thinkinglabels.Real environment tested: Local source checkout using the repo Vitest wrapper and autoreview.
Exact steps or command run after this patch:
git diff --check;node scripts/run-vitest.mjs test/scripts/channel-message-flows.test.ts;node scripts/run-vitest.mjs extensions/telegram/src/bot-message-dispatch.test.ts test/scripts/channel-message-flows.test.ts;node scripts/run-vitest.mjs src/plugin-sdk/reply-payload.test.ts src/agents/tools/message-tool.test.ts extensions/telegram/src/reasoning-lane-coordinator.test.ts test/scripts/channel-message-flows.test.ts --run;node scripts/run-vitest.mjs extensions/discord/src/monitor/message-handler.process.test.ts extensions/discord/src/chunk.test.ts src/plugin-sdk/channel-streaming.test.ts src/infra/heartbeat-runner.returns-default-unset.test.ts extensions/feishu/src/reply-dispatcher.test.ts extensions/mattermost/src/mattermost/reply-delivery.test.ts extensions/whatsapp/src/auto-reply/deliver-reply.test.ts --run;AUTOREVIEW_AUTO_TESTS=0 .agents/skills/autoreview/scripts/autoreview --mode branch --base origin/main --reviewer codex --fallback-reviewer none.Evidence after fix: Focused touched-surface runs passed, including 86 Telegram flow/dispatch tests, 180 reasoning/flow tests, and 232 cross-channel reasoning/progress tests; final autoreview reported
autoreview clean: no accepted/actionable findings reported.Observed result after fix: The dev flow runner supports Telegram
thinking-finaland nativesendMessageDraftworking-final, both production and fixture progress labels are static, and formatted reasoning filters no longer treat visibleThinking...answers as hidden reasoning.What was not tested: No live Telegram network send was rerun after the final native/static-label cleanup; earlier local previews drove the UX iteration.