-
-
Notifications
You must be signed in to change notification settings - Fork 266
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: simplify rag tool by simply calling gptme-rag via subprocess #316
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 Looks good to me! Reviewed everything up to cd31215 in 50 seconds
More details
- Looked at
588
lines of code in6
files - Skipped
0
files when reviewing. - Skipped posting
2
drafted comments based on config settings.
1. gptme/tools/rag.py:76
- Draft comment:
Consider handlingFileNotFoundError
in_run_rag_cmd
to log an appropriate error message whengptme-rag
is not found. - Reason this comment was not posted:
Confidence changes required:50%
The function_run_rag_cmd
ingptme/tools/rag.py
does not handle the case where thegptme-rag
command is not found. This could lead to an unhandledFileNotFoundError
. It would be better to catch this exception and log an appropriate error message.
2. gptme/tools/rag.py:161
- Draft comment:
Handle the case where_run_rag_cmd(cmd)
raises an exception inrag_enhance_messages
to ensure the function returns a valid message even if the command fails. - Reason this comment was not posted:
Confidence changes required:50%
Therag_enhance_messages
function ingptme/tools/rag.py
uses_run_rag_cmd(cmd).stdout
to get the command output, but if the command fails, it will raise an exception and not return any output. It would be better to handle this case and return an empty string or a default message instead.
Workflow ID: wflow_SkjNFJ2IZh0kF9tS
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet
mode, and more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 Looks good to me! Incremental review on 89d29e7 in 17 seconds
More details
- Looked at
32
lines of code in1
files - Skipped
0
files when reviewing. - Skipped posting
1
drafted comments based on config settings.
1. pyproject.toml:68
- Draft comment:
The 'gptme-rag' dependency is removed from the extras but still present in the 'tool.poetry.dependencies' section. Consider removing it from here as well for consistency. - Reason this comment was not posted:
Comment did not seem useful.
Workflow ID: wflow_xzLx3uQDUZx8HDhM
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet
mode, and more.
Codecov ReportAttention: Patch coverage is
✅ All tests successful. No failed tests found.
Additional details and impacted files@@ Coverage Diff @@
## master #316 +/- ##
==========================================
- Coverage 73.56% 72.37% -1.20%
==========================================
Files 68 67 -1
Lines 4975 4908 -67
==========================================
- Hits 3660 3552 -108
- Misses 1315 1356 +41
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
❌ Changes requested. Incremental review on e929f30 in 35 seconds
More details
- Looked at
46
lines of code in1
files - Skipped
0
files when reviewing. - Skipped posting
1
drafted comments based on config settings.
1. tests/test_tools_rag.py:25
- Draft comment:
The docstring is inconsistent with the test logic. It should reflect that the test checks enhancement without RAG available, not with. - Reason this comment was not posted:
Comment was on unchanged code.
Workflow ID: wflow_Klu0CfrvB49cf7wR
Want Ellipsis to fix these issues? Tag @ellipsis-dev
in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet
mode, and more.
Gets rid of a lot of unnecessary complexity.
Important
Simplifies RAG tool by using
gptme-rag
CLI via subprocess, removing internal RAG management.gptme-rag
CLI calls inrag.py
._rag_context.py
, eliminating internal RAG management classes and functions.rag_enhance_messages()
to usegptme-rag
CLI for context enhancement.gptme-rag
frompyproject.toml
dependencies.test_tools_rag.py
to mocksubprocess.run
forgptme-rag
CLI calls.This description was created by
for e929f30. It will automatically update as commits are pushed.