Feature Pack #363
Feature Pack #363
Conversation
WalkthroughThe recent changes encompass a variety of updates across the project. The Changes
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (1)
- docs/app.md (1 hunks)
Additional comments: 1
docs/app.md (1)
- 372-381: The new section headers added to the documentation are indicative of the expanded content and features of the application. It is important to ensure that the content under each new section is comprehensive and aligns with the section headers. Additionally, the use of Chinese language for the headers suggests that the documentation may be multilingual or region-specific, which should be consistent throughout the document.
|
沉淀 |
In this commit, the send_message method in the ChainFunc class is wrapped with a new method to handle sending messages. This is done to improve code organization and readability.
Updated the instructions in the README.md file to use `pdm` as the dependency manager instead of `poetry`. This change was made to prevent version conflicts with the `pydantic^1.9.0` library.
Updated the start commands for llm_sender and llm_receiver in the pm2.json file. Now using "sleep 3" instead of "sleep 6" and "python -m" instead of "poetry run python3".
Updated the start commands for llm_sender and llm_receiver in the pm2.json file. Now using "sleep 3" instead of "sleep 6" and "python -m" instead of "poetry run python3".
…tion and volume setup
…tion and volume setup
chore: Remove unused dependencies in pdm.lock
chore: Remove unused dependencies in pdm.lock
chore: Remove unused dependencies in pdm.lock
♻️ chore(python_test.yml): change pdm sync command
♻️ chore(python_test.yml): change pdm sync command
♻️ chore(python_test.yml): change pdm sync command
♻️ chore(python_test.yml): change pdm sync command
♻️ chore(python_test.yml): change pdm sync command
♻️ chore(python_test.yml): change pdm sync command
- Wrap the sending of a message in the notify_user method of the ChainFunc class.
…or maintenance and 'Slow progress' label for plugin updates The commit adds new labels to improve issue categorization and tracking.
- Fixed line formatting in function.py to improve readability. - Fixed line formatting in receiver_client.py to improve readability and consistency.
Updated the llmkira/task/schema.py file to include additional imports and make some changes to the code.
Adding pre-commit hooks to the project for performing code quality checks using pre-commit framework. This includes hooks for trailing whitespace, end-of-file fixer, YAML validation, and checking added large files. Also includes hooks from ruff-pre-commit for linting and formatting.
There was a problem hiding this comment.
Review Status
Actionable comments generated: 15
Configuration used: CodeRabbit UI
Files ignored due to path filters (4)
docs/SeriveProvider.svgis excluded by:!**/*.svgpdm.lockis excluded by:!**/*.lockpm2.jsonis excluded by:!**/*.jsonpyproject.tomlis excluded by:!**/*.toml
Files selected for processing (23)
- .env.exp (2 hunks)
- .github/release-drafter.yml (1 hunks)
- .github/workflows/python_test.yml (1 hunks)
- .pre-commit-config.yaml (1 hunks)
- Dockerfile (1 hunks)
- README.md (2 hunks)
- README_EN.md (1 hunks)
- docs/app.md (1 hunks)
- llmkira/error.py (4 hunks)
- llmkira/extra/plugins/search.py (11 hunks)
- llmkira/middleware/chain_box/init.py (4 hunks)
- llmkira/middleware/chain_box/schema.py (2 hunks)
- llmkira/middleware/llm_task.py (1 hunks)
- llmkira/receiver/function.py (10 hunks)
- llmkira/receiver/receiver_client.py (2 hunks)
- llmkira/receiver/telegram/init.py (5 hunks)
- llmkira/sdk/cache/redis.py (3 hunks)
- llmkira/sdk/func_calling/schema.py (1 hunks)
- llmkira/sdk/openapi/fuse/init.py (1 hunks)
- llmkira/sender/util_func.py (3 hunks)
- llmkira/task/schema.py (14 hunks)
- tests/test_chain_box.py (2 hunks)
- tests/test_router.py (1 hunks)
Files skipped from review due to trivial changes (4)
- .github/release-drafter.yml
- llmkira/receiver/receiver_client.py
- llmkira/receiver/telegram/init.py
- llmkira/sender/util_func.py
Additional comments: 28
.pre-commit-config.yaml (1)
- 10-18: The addition of the
ruff-pre-commitrepository with hooks for linting and formatting is a good practice for maintaining code quality..github/workflows/python_test.yml (1)
- 13-32: The updates to the workflow file to use PDM for Python setup and dependency installation are consistent with the move towards using PDM as the package manager.
Dockerfile (1)
- 2-31: The changes in the Dockerfile, including the downgrade of the Python version and the switch from Poetry to PDM for package management, are consistent and well-structured.
.env.exp (1)
- 6-18: > Note: This review was outside the patches, so it was mapped to the patch with the greatest overlap. Original lines [1-23]
The updates to the
.env.expfile, including the removal of commented-out configurations and updates to DSN configurations, are clear and should not affect existing functionality if the environment variables are set correctly.tests/test_chain_box.py (1)
- 7-25: > Note: This review was outside the patches, so it was mapped to the patch with the greatest overlap. Original lines [10-37]
The changes in
tests/test_chain_box.py, including the addition of athead_uuidparameter in thetest_schemafunction, are appropriate and align with the updates to theChainclass.llmkira/sdk/cache/redis.py (2)
- 38-40: The reformatting of the
setmethod call inRedisClientWrapperimproves readability and is consistent with best practices.- 75-77: The reformatting of the
lrangemethod call is also an improvement for readability.llmkira/error.py (2)
- 62-62: The use of
MappingDefaultfor safer string formatting in error messages is a good practice to avoid potential security issues with direct string interpolation.- 83-83: The update to the
get_upload_error_messagefunction to useMappingDefaultfor formatting is consistent with the changes made toget_request_error_message.llmkira/middleware/chain_box/schema.py (1)
- 15-27: The addition of the
thead_uuidfield and the updates to the descriptions of existing fields in theChainclass are clear and enhance the documentation of the class.llmkira/middleware/chain_box/__init__.py (3)
- 25-25: Updating the authentication schema version in the
_prefixclass method is a good practice for versioning and maintaining backward compatibility.- 70-70: The change in the
_prefixinstance method to reflect the new authentication schema version is consistent with the class method change.- 55-55: The addition of logging statements to indicate authentication and chain expiration helps with debugging and monitoring the system.
README.md (3)
- 106-106: The update to the testing commands to use
pdmreflects the switch to PDM for dependency management and is consistent with the changes in other files.- 154-157: The instructions for manual installation using
pdmare clear and provide the necessary steps for setting up the project.- 165-165: The recommendation to use
pdmfor dependency management is well-justified and aligns with the project's move to PDM.docs/app.md (1)
- 375-447: The extensive changes in
docs/app.mdprovide a comprehensive overview of the OpenAI Assistant implementation, including discussions on language model advantages, the necessity of strong matching in release chains, and platform-specific support for plugins and channels.llmkira/extra/plugins/search.py (2)
- 15-23: The addition of
# noqa: E402comments to the import statements is a common practice to tell linters to ignore certain rules, in this case, the rule that enforces imports to be at the top of the file. This is acceptable if there's a specific reason for imports to be placed after some initial code (e.g., to avoid circular imports or to set up some environment variables before importing).- 91-140: The update to the
keywordslist in theSearchToolclass is a logical change that seems to be adding more terms to trigger the search functionality. This is a straightforward enhancement to the tool's capabilities.llmkira/sdk/func_calling/schema.py (1)
- 90-91: Converting the
env_help_docsmethod from an instance method to a class method is a good practice when the method does not depend on the instance state. This allows the method to be called on the class itself rather than on an instance, which can be more convenient and appropriate for utility methods like this one.llmkira/receiver/function.py (4)
- 25-27: The
reply_usermethod's signature has been updated to take keyword-only arguments, which is a good practice for clarity and to prevent errors when calling the method with multiple parameters.- 52-60: The
auth_chainmethod's logic for handling chain registration and routing appears to be correct and follows the expected flow for an authentication chain within the application's context.- 66-97: > Note: This review was outside the patches, so it was mapped to the patch with the greatest overlap. Original lines [94-106]
The
resign_chainmethod's additional parameters and logic for chain resigning are appropriate for the context of the application. The method's logic is consistent with the expected behavior for resigning a chain.
- 241-249: > Note: This review was outside the patches, so it was mapped to the patch with the greatest overlap. Original lines [235-246]
The
process_function_callmethod's logic for parsing and running functions based on incoming messages is well-structured and follows the expected flow for processing function calls within the application.llmkira/task/schema.py (4)
- 13-13: The addition of the
shortuuidimport is appropriate for generating unique identifiers.- 129-132: The
task_uuidproperty correctly returns the UUID part of thesign_astuple.- 249-249: The
childmethod correctly increments the first element of thesign_astuple to denote a child node.- 261-261: The
chainmethod correctly increments the first element of thesign_astuple and resets message routing flags.
| @@ -110,7 +110,7 @@ Both the authentication component and the backend need to be implemented by your | |||
|
|
|||
| Read the [🧀 Deployment Documentation](https://llmkira.github.io/Docs/) for more information. | |||
|
|
|||
| Please use `python3 start_sender.py` `python3 start_receiver.py` to test whether it can run normally. | |||
| Please use `poetry run python3 start_sender.py` `poetry run python3 start_receiver.py` to test whether it can run normally. | |||
There was a problem hiding this comment.
The command instructions for running the application have been updated to use poetry, but it's important to ensure that the poetry dependency management tool is mentioned in the prerequisites or installation instructions within the README. If it's not already included, users may encounter issues when trying to run the commands without having poetry installed.
| ) | ||
|
|
||
|
|
||
| @resign_plugin_executor(function=search) | ||
| def search_on_duckduckgo(search_sentence: str, key_words: str = None): | ||
| logger.debug(f"Plugin --search_on_duckduckgo {search_sentence}") | ||
| from duckduckgo_search import DDGS | ||
|
|
There was a problem hiding this comment.
The commented-out code related to NLP optimization and the Sublimate class should be removed if it's no longer needed. Leaving commented-out code can lead to confusion and clutter in the codebase.
- # 内存优化抛弃 NLP
- # from llmkira.sdk.filter import SublimateCommittable suggestion
❗ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| from duckduckgo_search import DDGS # noqa: F401 | ||
|
|
||
| # from llmkira.sdk.filter import Sublimate | ||
| return True | ||
| except ImportError as e: | ||
| logger.warning(f"plugin:package <duckduckgo_search> not found,please install it first:{e}") | ||
| logger.warning( | ||
| f"plugin:package <duckduckgo_search> not found,please install it first:{e}" | ||
| ) |
There was a problem hiding this comment.
The pre_check method is catching an ImportError and logging a warning, but it should also handle the error appropriately by either raising an exception or returning a value that indicates the check failed. This is important for the robustness of the application.
+ return FalseCommittable suggestion
❗ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| from duckduckgo_search import DDGS # noqa: F401 | |
| # from llmkira.sdk.filter import Sublimate | |
| return True | |
| except ImportError as e: | |
| logger.warning(f"plugin:package <duckduckgo_search> not found,please install it first:{e}") | |
| logger.warning( | |
| f"plugin:package <duckduckgo_search> not found,please install it first:{e}" | |
| ) | |
| from duckduckgo_search import DDGS # noqa: F401 | |
| # from llmkira.sdk.filter import Sublimate | |
| return True | |
| except ImportError as e: | |
| logger.warning( | |
| f"plugin:package <duckduckgo_search> not found,please install it first:{e}" | |
| ) | |
| return False |
| async def failed( | ||
| self, | ||
| task: "TaskHeader", | ||
| receiver: "TaskHeader.Location", | ||
| exception, | ||
| env: dict, | ||
| arg: dict, | ||
| pending_task: "TaskBatch", | ||
| refer_llm_result: dict = None, | ||
| **kwargs, | ||
| ): |
There was a problem hiding this comment.
The failed method is designed to handle task failures, but it currently does not seem to handle the exception in any meaningful way. It should at least log the exception or take some action to notify the system of the failure.
+ logger.error(f"Task failed due to exception: {exception}")Committable suggestion
❗ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| async def failed( | |
| self, | |
| task: "TaskHeader", | |
| receiver: "TaskHeader.Location", | |
| exception, | |
| env: dict, | |
| arg: dict, | |
| pending_task: "TaskBatch", | |
| refer_llm_result: dict = None, | |
| **kwargs, | |
| ): | |
| async def failed( | |
| self, | |
| task: "TaskHeader", | |
| receiver: "TaskHeader.Location", | |
| exception, | |
| env: dict, | |
| arg: dict, | |
| pending_task: "TaskBatch", | |
| refer_llm_result: dict = None, | |
| **kwargs, | |
| ): | |
| logger.error(f"Task failed due to exception: {exception}") |
| async def callback( | ||
| self, | ||
| task: "TaskHeader", | ||
| receiver: "TaskHeader.Location", | ||
| env: dict, | ||
| arg: dict, | ||
| pending_task: "TaskBatch", | ||
| refer_llm_result: dict = None, | ||
| **kwargs, | ||
| ): |
There was a problem hiding this comment.
The callback method is currently a stub that simply returns True. It would be beneficial to implement actual logic for the callback or remove it if it's not needed. If the method is required but not yet implemented, consider adding a TODO comment to indicate future work.
+ # TODO: Implement callback logic or remove if not neededCommittable suggestion
❗ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| async def callback( | |
| self, | |
| task: "TaskHeader", | |
| receiver: "TaskHeader.Location", | |
| env: dict, | |
| arg: dict, | |
| pending_task: "TaskBatch", | |
| refer_llm_result: dict = None, | |
| **kwargs, | |
| ): | |
| async def callback( | |
| self, | |
| task: "TaskHeader", | |
| receiver: "TaskHeader.Location", | |
| env: dict, | |
| arg: dict, | |
| pending_task: "TaskBatch", | |
| refer_llm_result: dict = None, | |
| **kwargs, | |
| ): | |
| # TODO: Implement callback logic or remove if not needed |
| @@ -547,7 +556,7 @@ | |||
| 消息标准化 | |||
| """ | |||
| if not _message: | |||
| raise ValueError(f"Message is empty") | |||
| raise ValueError("Message is empty") | |||
There was a problem hiding this comment.
The error message "Message is empty" is repeated here for Discord messages. It should also be made more descriptive.
- raise ValueError("Message is empty")
+ raise ValueError("The provided Discord message is empty and cannot be processed.")Committable suggestion
❗ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| raise ValueError("Message is empty") | |
| raise ValueError("The provided Discord message is empty and cannot be processed.") |
| @@ -630,7 +639,7 @@ | |||
| 消息标准化 | |||
| """ | |||
| if not _message: | |||
| raise ValueError(f"Message is empty") | |||
| raise ValueError("Message is empty") | |||
There was a problem hiding this comment.
Again, the error message "Message is empty" is used for Kook messages. It should be updated for consistency and clarity.
- raise ValueError("Message is empty")
+ raise ValueError("The provided Kook message is empty and cannot be processed.")Committable suggestion
❗ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| raise ValueError("Message is empty") | |
| raise ValueError("The provided Kook message is empty and cannot be processed.") |
| @@ -713,7 +722,7 @@ | |||
| 消息标准化 | |||
| """ | |||
| if not _message: | |||
| raise ValueError(f"Message is empty") | |||
| raise ValueError("Message is empty") | |||
There was a problem hiding this comment.
The error message "Message is empty" is used for Slack messages. Update it to be more descriptive and consistent with the other changes.
- raise ValueError("Message is empty")
+ raise ValueError("The provided Slack message is empty and cannot be processed.")Committable suggestion
❗ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| raise ValueError("Message is empty") | |
| raise ValueError("The provided Slack message is empty and cannot be processed.") |
| def test_schema(): | ||
| r1 = RouterCache(router=[]) | ||
| print("\n") | ||
| print(r1) | ||
| try: | ||
| RouterCache(router=None) | ||
| except Exception: | ||
| pass | ||
| else: | ||
| assert False, "Should Raise Error" | ||
|
|
||
| r3 = Router(from_="from", to_="to", user_id=2200, rules="rule there", method="chat") | ||
| print("\n") | ||
| print(r3) |
There was a problem hiding this comment.
Consider removing the print statements from the test cases. Print statements are generally not used in tests as test runners will typically capture and display output from tests separately.
| if Exception in exclude_exceptions or BaseException in exclude_exceptions: | ||
| raise ValueError("Exception and BaseException cant be exclude") |
There was a problem hiding this comment.
The check for Exception and BaseException in the exclude_exceptions list is a good security practice to prevent broad exception handling, but the error message could be more descriptive.
- raise ValueError("Exception and BaseException cant be exclude")
+ raise ValueError("Exception and BaseException cannot be excluded from the exclude_exceptions list")Committable suggestion
❗ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| if Exception in exclude_exceptions or BaseException in exclude_exceptions: | |
| raise ValueError("Exception and BaseException cant be exclude") | |
| if Exception in exclude_exceptions or BaseException in exclude_exceptions: | |
| raise ValueError("Exception and BaseException cannot be excluded from the exclude_exceptions list") |
📚 docs/app.md
Summary by CodeRabbit
Chores
Refactor
Tests
Documentation