Conversation
|
Hi @zjncs, thank you for your first Pull Request! 🎉 🙌 Join Developer CommunityThanks so much for your contribution! We'd love to invite you to join the official CoPaw developer group! You can find the Discord and DingTalk group links under the "Developer Community" section on our docs page: We truly appreciate your enthusiasm—and look forward to your future contributions! 😊 We'll review your PR soon. |
There was a problem hiding this comment.
Code Review
This pull request introduces a comprehensive set of DingTalk tools for AI table and document operations, supported by a new centralized DingTalkOpenAPIClient that handles token caching, retries, and error management. Additionally, it enhances the system's robustness by implementing memory fallback mechanisms and improved error handling for malformed session states. A critical performance issue was identified in the DingTalk tool implementations, where the repeated instantiation of HTTP sessions and API clients for every request negates the benefits of token caching and increases latency.
| async with aiohttp.ClientSession(timeout=_HTTP_TIMEOUT) as session: | ||
| client = DingTalkOpenAPIClient( | ||
| client_id=client_id, | ||
| client_secret=client_secret, | ||
| http_session=session, | ||
| ) |
There was a problem hiding this comment.
Creating a new aiohttp.ClientSession and DingTalkOpenAPIClient for each tool call is inefficient and defeats the purpose of the token caching implemented in DingTalkOpenAPIClient.
This will result in:
- Increased latency due to new TCP connections for every request.
- Unnecessary calls to the DingTalk
accessTokenendpoint for every tool use, which can lead to performance issues and hitting API rate limits.
It is recommended to refactor this to reuse a single aiohttp.ClientSession and DingTalkOpenAPIClient instance across all DingTalk tool calls. The implementation in src/copaw/app/channels/dingtalk/channel.py provides a good example of managing a shared client instance.
Description
This PR adds DingTalk AI table and document operation support to CoPaw as built-in agent tools.
For DingTalk AI tables, this PR adds built-in tools for:
For DingTalk documents, this PR adds built-in tools for:
This PR also introduces a shared DingTalk OpenAPI client used by both the DingTalk channel and the new tools. The client now handles access token caching, JSON request handling, transient retry for safe reads, and boolean query param normalization.
The document MVP scope intentionally excludes template listing and rich-text/body editing APIs, because they were not needed for the validated MVP path.
Related Issue: Relates to #2291
Security Considerations: Reuses the existing DingTalk
client_id/client_secretfrom agent channel configuration. Credentials are not exposed as tool parameters. Requests are sent through the DingTalk OpenAPI client with existing token handling.Type of Change
Component(s) Affected
Checklist
pre-commit run --all-fileslocally and it passespytestor as relevant) and they passTesting
Tested locally with:
python3 -m py_compilefor the changed DingTalk tool/client/config filespython3 -m pytest -q tests/unit/agents/tools/test_dingtalk_tools.py tests/unit/app/channels/dingtalk/test_openapi_client.py tests/unit/app/test_chat_api.py tests/unit/agents/test_memory_fallback.pyAlso verified against a real DingTalk app configuration:
Local Verification Evidence
python3 -m pytest -q tests/unit/agents/tools/test_dingtalk_tools.py \ tests/unit/app/channels/dingtalk/test_openapi_client.py \ tests/unit/app/test_chat_api.py \ tests/unit/agents/test_memory_fallback.py 29 passed, 2 warnings in 2.53s