Skip to content

fix(dingtalk): add AI Card QPS token-bucket throttle - #17365

Open
spike2204 wants to merge 1 commit into
NousResearch:mainfrom
spike2204:fix/dingtalk-card-qps-throttle
Open

fix(dingtalk): add AI Card QPS token-bucket throttle#17365
spike2204 wants to merge 1 commit into
NousResearch:mainfrom
spike2204:fix/dingtalk-card-qps-throttle

Conversation

@spike2204

Copy link
Copy Markdown

Summary

DingTalk's AI Card streaming API enforces a 50 QPS limit per robot. Without client-side throttling, bursts of concurrent conversations trigger HTTP 429 errors that abort card updates mid-stream.

Changes

  • Add _CardTokenBucket class — async token-bucket rate limiter with configurable QPS, burst capacity, and max wait timeout
  • Instantiate process-wide _CARD_BUCKET shared across all adapter instances
  • Guard every _card_create, _card_update, _card_close call with await _CARD_BUCKET.acquire()

Related

Part of the DingTalk adapter enhancement series — see #12769 for the umbrella PR.

DingTalk's interactive-card PUT API enforces ~20 QPS per tenant;
exceeding it returns 403 and drops the card update.

- Add _CardTokenBucket — async token-bucket rate limiter (20 QPS)
  with automatic 2s exponential backoff on 403 responses
- Add per-card 800ms minimum interval between non-finalize edits
  (matches openclaw-connector reply-dispatcher.ts:103)
- Per-chat error-send cooldown (60s) to avoid spamming users
- Finalize edits are NEVER throttled — dropping them would leave
  the card stuck in streaming state
- Global bucket shared across all adapters in the process to #
@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/gateway Gateway runner, session dispatch, delivery platform/dingtalk DingTalk adapter labels Apr 29, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

Overlaps with the card QPS throttle component of #14333. Part of umbrella #12769.

@spike2204

Copy link
Copy Markdown
Author

Hi @alt-glitch, thanks for the review and labels! 🙏

You're right — this PR extracts the card QPS throttle component from #14333. To clarify:

@PeterGuy326 and @spike2204 are from the same team — the DingTalk (钉钉) Open Platform team at Alibaba. We're the team that builds and maintains the DingTalk stream SDK, AI Card APIs, and Robot OpenAPI that this adapter integrates with.

The original #14333 bundled three independent fixes (websockets proxy + card QPS + inbound queue) into a single stacked PR. We've re-submitted them as 3 separate, self-contained PRs (#17364, #17365, #17366) against main so each can be reviewed and merged independently.

About this specific fix: The 50 QPS limit on AI Card streaming APIs is an actual platform-enforced rate limit. We've observed HTTP 429 storms in production when 10+ concurrent conversations update cards simultaneously. The token-bucket approach here mirrors what we recommend in our own DingTalk AI Card best practices documentation.

Happy to address any feedback — the original #14333 can be closed once these granular PRs are reviewed. 🚀

@Librazy

Librazy commented May 11, 2026

Copy link
Copy Markdown

@Librazy

Librazy commented May 11, 2026

Copy link
Copy Markdown

And _CARD_BUCKET and all throttle related codes are missing.

@teknium1 teknium1 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for splitting out the DingTalk rate-limit work. The current active adapter still calls the streaming update API directly at plugins/platforms/dingtalk/adapter.py:1214, so the underlying concern remains relevant.

Problems

  • The PR references _CARD_EDIT_THROTTLE_MS, _CARD_BUCKET, and _CARD_API_QPS_BACKOFF_MS, but none is defined in its base or diff. The first non-final edit and every streamed update would therefore fail at runtime.
  • The edited path, gateway/platforms/dingtalk.py, was migrated to plugins/platforms/dingtalk/adapter.py by 560010547. The current card create, deliver, and stream paths are at adapter.py:1078, :1118, and :1214, respectively.
  • No tests accompany the limiter; existing lifecycle coverage is in tests/gateway/test_dingtalk.py:952-1087.

Suggested changes

  • Reimplement the complete limiter in the bundled plugin and add hermetic concurrency/backoff/finalize tests there.

Automated hermes-sweeper review.

now_ms = int(datetime.now(tz=timezone.utc).timestamp() * 1000)
if not finalize:
last_ms = self._card_last_edit_ms.get(message_id, 0)
if now_ms - last_ms < _CARD_EDIT_THROTTLE_MS:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

_CARD_EDIT_THROTTLE_MS is not defined in this PR or its base. A non-final edit will raise NameError; include the limiter constants and implementation in this self-contained PR.

await self._card_sdk.streaming_update_with_options_async(
stream_request, stream_headers, runtime
)
await _CARD_BUCKET.acquire()

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

_CARD_BUCKET is also not defined in this PR or its base, so every streamed card update will raise NameError. Please add the token-bucket implementation and test its acquire/backoff behavior.

@alt-glitch alt-glitch added type/feature New feature or request and removed type/bug Something isn't working labels Jul 12, 2026
@teknium1 teknium1 added sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jul 12, 2026
@alt-glitch alt-glitch added P3 Low — cosmetic, nice to have and removed P2 Medium — degraded but workaround exists sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades labels Jul 12, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/gateway Gateway runner, session dispatch, delivery P3 Low — cosmetic, nice to have platform/dingtalk DingTalk adapter sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages type/feature New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants