fix(feishu): remove streaming card update throttle to prevent duplicates#66148
fix(feishu): remove streaming card update throttle to prevent duplicates#66148SARAMALI15792 wants to merge 1 commit into
Conversation
Greptile SummarySets
Confidence Score: 5/5Safe to merge after dropping the docs/plans files; the one-line code change is correct and low-risk. The substantive change is a single property assignment that correctly disables a throttle mechanism whose flush-timer side was never implemented. All remaining findings are P2: a dead flushTimer field worth cleaning up, and two AI planning documents that shouldn't live in docs/. Neither blocks correctness or safety. docs/plans/2026-04-13-feishu-streaming-throttle-fix.md and docs/plans/2026-04-13-feishu-streaming-throttle-fix-spec.md should be removed before merging. Prompt To Fix All With AIThis is a comment left during a code review.
Path: extensions/feishu/src/streaming-card.ts
Line: 169-172
Comment:
**`flushTimer` is never set**
`flushTimer` is declared, cleared in both `update()` and `close()`, but never actually assigned a `setTimeout` call. The intent was clearly to schedule a deferred flush of `pendingText` after the throttle window expires — without it, any `pendingText` accumulated during a throttle window is only drained when the *next* non-throttled update arrives or when `close()` fires. Setting `updateThrottleMs = 0` avoids this gap entirely, but the dead timer field will confuse anyone who later tries to re-enable throttling (they'll add `= 0` back, the bug reappears, and the timer wiring is still missing). Consider removing `flushTimer` entirely now that throttling is disabled, or add the missing timer assignment if throttling is ever re-enabled.
How can I resolve this? If you propose a fix, please make it concise.
---
This is a comment left during a code review.
Path: docs/plans/2026-04-13-feishu-streaming-throttle-fix.md
Line: 1-5
Comment:
**AI planning artifacts should not be committed**
Both `docs/plans/2026-04-13-feishu-streaming-throttle-fix.md` and `docs/plans/2026-04-13-feishu-streaming-throttle-fix-spec.md` are agentic-worker planning documents (note the `- [ ]` task syntax and "REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development" directive). These are internal scaffolding files, not user-facing documentation. The `docs/` directory is for user-facing content; committing agent-generated planning artifacts alongside code changes adds permanent noise to the repo history and will confuse contributors browsing the docs tree.
How can I resolve this? If you propose a fix, please make it concise.Reviews (1): Last reviewed commit: "fix(feishu): remove streaming card updat..." | Re-trigger Greptile |
| private lastUpdateTime = 0; | ||
| private pendingText: string | null = null; | ||
| private flushTimer: ReturnType<typeof setTimeout> | null = null; | ||
| private updateThrottleMs = 100; // Throttle updates to max 10/sec | ||
| private updateThrottleMs = 0; // No throttling - Feishu API handles rapid updates |
There was a problem hiding this comment.
flushTimer is never set
flushTimer is declared, cleared in both update() and close(), but never actually assigned a setTimeout call. The intent was clearly to schedule a deferred flush of pendingText after the throttle window expires — without it, any pendingText accumulated during a throttle window is only drained when the next non-throttled update arrives or when close() fires. Setting updateThrottleMs = 0 avoids this gap entirely, but the dead timer field will confuse anyone who later tries to re-enable throttling (they'll add = 0 back, the bug reappears, and the timer wiring is still missing). Consider removing flushTimer entirely now that throttling is disabled, or add the missing timer assignment if throttling is ever re-enabled.
Prompt To Fix With AI
This is a comment left during a code review.
Path: extensions/feishu/src/streaming-card.ts
Line: 169-172
Comment:
**`flushTimer` is never set**
`flushTimer` is declared, cleared in both `update()` and `close()`, but never actually assigned a `setTimeout` call. The intent was clearly to schedule a deferred flush of `pendingText` after the throttle window expires — without it, any `pendingText` accumulated during a throttle window is only drained when the *next* non-throttled update arrives or when `close()` fires. Setting `updateThrottleMs = 0` avoids this gap entirely, but the dead timer field will confuse anyone who later tries to re-enable throttling (they'll add `= 0` back, the bug reappears, and the timer wiring is still missing). Consider removing `flushTimer` entirely now that throttling is disabled, or add the missing timer assignment if throttling is ever re-enabled.
How can I resolve this? If you propose a fix, please make it concise.| # Feishu Streaming Card Throttle Fix Implementation Plan | ||
|
|
||
| > **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. | ||
|
|
||
| **Goal:** Remove artificial throttling from Feishu streaming card updates to eliminate duplicate/truncated messages |
There was a problem hiding this comment.
Both docs/plans/2026-04-13-feishu-streaming-throttle-fix.md and docs/plans/2026-04-13-feishu-streaming-throttle-fix-spec.md are agentic-worker planning documents (note the - [ ] task syntax and "REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development" directive). These are internal scaffolding files, not user-facing documentation. The docs/ directory is for user-facing content; committing agent-generated planning artifacts alongside code changes adds permanent noise to the repo history and will confuse contributors browsing the docs tree.
Prompt To Fix With AI
This is a comment left during a code review.
Path: docs/plans/2026-04-13-feishu-streaming-throttle-fix.md
Line: 1-5
Comment:
**AI planning artifacts should not be committed**
Both `docs/plans/2026-04-13-feishu-streaming-throttle-fix.md` and `docs/plans/2026-04-13-feishu-streaming-throttle-fix-spec.md` are agentic-worker planning documents (note the `- [ ]` task syntax and "REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development" directive). These are internal scaffolding files, not user-facing documentation. The `docs/` directory is for user-facing content; committing agent-generated planning artifacts alongside code changes adds permanent noise to the repo history and will confuse contributors browsing the docs tree.
How can I resolve this? If you propose a fix, please make it concise.There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a58b1c6c56
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| @@ -169,7 +169,7 @@ export class FeishuStreamingSession { | |||
| private lastUpdateTime = 0; | |||
| private pendingText: string | null = null; | |||
| private flushTimer: ReturnType<typeof setTimeout> | null = null; | |||
| private updateThrottleMs = 100; // Throttle updates to max 10/sec | |||
| private updateThrottleMs = 0; // No throttling - Feishu API handles rapid updates | |||
There was a problem hiding this comment.
Keep a positive streaming update throttle
Setting updateThrottleMs to 0 makes the throttle guard in update() effectively unreachable, so every queued partial snapshot now performs a full CardKit PUT instead of coalescing updates. In the Feishu reply path, partialUpdateQueue and close() both await these updates, so long responses with many small deltas can accumulate hundreds/thousands of sequential network calls and significantly delay completion (and are much more likely to hit Feishu-side rate limiting). This change removes the only backpressure/coalescing mechanism on this hot path.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b61d251337
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| AGENTS.md |
There was a problem hiding this comment.
Remove trailing newline from CLAUDE.md symlink target
The symlink blob now stores AGENTS.md\n instead of AGENTS.md, so docs/CLAUDE.md resolves to a non-existent path and is effectively broken for tools/readers that follow it. This commit changed only the symlink payload, and adding the newline makes the target invalid even though docs/AGENTS.md exists, so consumers relying on docs/CLAUDE.md will fail to load the intended instructions.
Useful? React with 👍 / 👎.
Fixes openclaw#65993 The 100ms throttle was causing updates to be dropped when blocks arrived faster than the throttle interval. This led to: - Lost content during streaming - Truncated messages when close() was called with pending text - Duplicate messages when fallback sendChunkedTextReply sent full content Feishu Card Kit API is designed for rapid updates (print_frequency_ms: 50). User validated 0ms throttle in production with no issues. Also removes unused flushTimer field that was never assigned.
663f496 to
36ebf31
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 36ebf3149e
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| private pendingText: string | null = null; | ||
| private flushTimer: ReturnType<typeof setTimeout> | null = null; | ||
| private updateThrottleMs = 100; // Throttle updates to max 10/sec | ||
| private updateThrottleMs = 0; // No throttling - Feishu API handles rapid updates |
There was a problem hiding this comment.
Restore coalescing for streaming card partial updates
Setting updateThrottleMs to 0 makes the throttle branch in FeishuStreamingSession.update() unreachable, so every partial snapshot now enqueues a full CardKit PUT instead of collapsing rapid updates. In the Feishu reply path, onPartialReply can emit many snapshots and closeStreaming() waits partialUpdateQueue before finalizing, so long replies will perform a large serial request chain, which materially increases completion latency and makes rate-limit/network failure much more likely under high-chunk streams.
Useful? React with 👍 / 👎.
CI Status UpdateThe failing CI checks are not related to this Feishu fix. This PR:
CI Failures:
These appear to be pre-existing issues from recent main branch changes, not introduced by this PR. Recommendation: This Feishu fix can be merged independently as it's isolated and working correctly. |
🎉 Thank you!Thank you @SARAMALI15792 for the fix! This resolves the Feishu streaming card duplicate/truncation issue we reported in #65993. Your analysis confirmed our root cause finding (the 100ms throttle in ). The fix looks clean and minimal - exactly what's needed. We're currently running with a dist-file patch () as a temporary workaround, so we'll be happy to remove it once this PR is merged! ✅ |
|
Thanks for chasing this. I’m closing this as superseded. The reporter later confirmed the throttle change did not address the root cause. The canonical remaining fix is #68491, which handles the Your investigation helped narrow the failure, but this patch is no longer the merge-safe path. If a new throttle-specific regression shows up on current main, please open a fresh issue/PR with that narrower evidence. |
What does this PR do?
Removes artificial 100ms throttling from Feishu streaming card updates to eliminate duplicate and truncated messages when handling long replies.
Fixes #65993
Problem Statement
Feishu streaming cards produce duplicate or truncated messages when handling long replies (>1000 characters).
Root cause: The
updateThrottleMs = 100throttle inFeishuStreamingSessioncauses updates to be dropped when content blocks arrive faster than 100ms intervals. Whenclose()is called with accumulated unsent text (pendingText), the content is truncated. Then the fallbacksendChunkedTextReplysends the full content, resulting in duplicate messages.User impact:
Why Telegram/Discord unaffected: They use
editMessageTexton a draft message without throttling. Feishu uses throttled card updates viaFeishuStreamingSession.Solution Applied
Changed
updateThrottleMsfrom100to0inextensions/feishu/src/streaming-card.ts:172.Why this solves the bottleneck:
print_frequency_ms: 50)Why this is safe:
Changes
Modified:
extensions/feishu/src/streaming-card.ts:172- ChangedupdateThrottleMsfrom 100 to 0Testing:
Test Plan
Automated:
Manual verification (for maintainers):
streaming: trueandrenderMode: "card"Expected behavior: