feat: migrate readline to prompt_toolkit, with many features and fixes #244
Conversation
Codecov ReportAttention: Patch coverage is
✅ All tests successful. No failed tests found.
Additional details and impacted files@@ Coverage Diff @@
## master #244 +/- ##
==========================================
- Coverage 72.77% 72.01% -0.77%
==========================================
Files 67 67
Lines 4951 5138 +187
==========================================
+ Hits 3603 3700 +97
- Misses 1348 1438 +90
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
ErikBjare
force-pushed
the
dev/prompt-toolkit
branch
from
059b4d4 to
94e29dd
Compare
- Add better path completion with syntax highlighting - Add command completion - Add LLM suggestions support - Improve history handling - Remove readline dependency
![ellipsis-dev[bot]](https://avatars.githubusercontent.com/in/64358?s=60&v=4){kind=small}
There was a problem hiding this comment.
👍 Looks good to me! Reviewed everything up to b36e785 in 50 seconds
More details
- Looked at
501lines of code in8files - Skipped
0files when reviewing. - Skipped posting
2drafted comments based on config settings.
1. gptme/chat.py:278
- Draft comment:
LLM suggestions are not implemented yet asenabledis set toFalse. This is a planned feature according to the PR description. - Reason this comment was not posted:
Confidence changes required:50%
Theget_suggestionsfunction is currently not implemented, as indicated by theenabledflag being set toFalse. This means LLM suggestions are not functional yet, which is a planned feature according to the PR description.
2. gptme/dirs.py:13
- Draft comment:
get_readline_history_fileis unused and can be removed after migrating toprompt_toolkit. - Reason this comment was not posted:
Comment was not on a valid diff hunk.
Workflow ID: wflow_nqBXPEs5HizUO3Qi
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
There was a problem hiding this comment.
👍 Looks good to me! Incremental review on d70449e in 9 seconds
More details
- Looked at
37lines of code in1files - Skipped
0files when reviewing. - Skipped posting
0drafted comments based on config settings.
Workflow ID: wflow_ZuwclAysSAGSKdT7
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
…d paths, misc refactor and fixes
ErikBjare
force-pushed
the
dev/prompt-toolkit
branch
from
026123c to
51c2c62
Compare
There was a problem hiding this comment.
👍 Looks good to me! Incremental review on 026123c in 49 seconds
More details
- Looked at
630lines of code in7files - Skipped
0files when reviewing. - Skipped posting
4drafted comments based on config settings.
1. gptme/chat.py:272
- Draft comment:
Theget_suggestionsfunction is removed, but its functionality is not replaced or refactored elsewhere. This might lead to missing LLM suggestions, which is a feature mentioned in the PR description. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable:
The comment is speculative because it assumes the removal ofget_suggestionsis problematic without evidence from the code changes. The rules state to avoid speculative comments and to only comment if there is a definite issue. Since the diff does not show any evidence that the removal causes a problem, the comment should likely be removed.
I might be missing the context of the PR description, which could provide more information about the necessity of theget_suggestionsfeature. However, the rules specify to focus on the code changes and not external context.
Even if the PR description mentions the feature, the code changes themselves do not show evidence of an issue caused by the removal, so the comment remains speculative.
The comment should be deleted because it is speculative and lacks strong evidence from the code changes that the removal ofget_suggestionsis an issue.
2. gptme/message.py:20
- Draft comment:
Therich_to_strfunction is moved togptme/util/prompt.py, but the import statement here is not updated. This will lead to an import error. - Reason this comment was not posted:
Comment looked like it was already resolved.
3. gptme/util/ask_execute.py:18
- Draft comment:
Thecreate_prompt_sessionfunction is removed, but its usage here is not updated to useget_prompt_session. This will lead to a function not found error. - Reason this comment was not posted:
Comment looked like it was already resolved.
4. gptme/util/ask_execute.py:108
- Draft comment:
Thecreate_prompt_sessionfunction is removed, but its usage here is not updated to useget_prompt_session. This will lead to a function not found error. - Reason this comment was not posted:
Marked as duplicate.
Workflow ID: wflow_XI52u9A0oIaH4AGY
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
There was a problem hiding this comment.
👍 Looks good to me! Incremental review on 51c2c62 in 1 minute and 28 seconds
More details
- Looked at
630lines of code in7files - Skipped
0files when reviewing. - Skipped posting
4drafted comments based on config settings.
1. gptme/chat.py:272
- Draft comment:
Theget_suggestionsfunction is removed, but its functionality is not replaced or refactored elsewhere. This could lead to missing functionality if it was used elsewhere in the codebase. - Reason this comment was not posted:
Comment did not seem useful.
2. gptme/message.py:19
- Draft comment:
Therich_to_strfunction is moved fromgptme/util/__init__.pytogptme/util/prompt.py, but the import ingptme/message.pyis not updated. This will cause an import error. - Reason this comment was not posted:
Comment looked like it was already resolved.
3. gptme/util/ask_execute.py:18
- Draft comment:
Thecreate_prompt_sessionfunction is renamed toget_prompt_session, but the import ingptme/util/ask_execute.pyis not updated. This will cause an import error. - Reason this comment was not posted:
Comment looked like it was already resolved.
4. gptme/util/prompt.py:387
- Draft comment:
Theget_suggestionsfunction is removed, but its functionality is not replaced or refactored elsewhere. This could lead to missing functionality if it was used elsewhere in the codebase. - Reason this comment was not posted:
Marked as duplicate.
Workflow ID: wflow_DcPpFv6Qp7a72idl
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
There was a problem hiding this comment.
❌ Changes requested. Incremental review on fc0ec3f in 30 seconds
More details
- Looked at
32lines of code in1files - Skipped
0files when reviewing. - Skipped posting
0drafted comments based on config settings.
Workflow ID: wflow_CaQFNkiLqMQ2ijCA
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.
There was a problem hiding this comment.
👍 Looks good to me! Incremental review on e2f11e5 in 27 seconds
More details
- Looked at
220lines of code in2files - Skipped
0files when reviewing. - Skipped posting
1drafted comments based on config settings.
1. gptme/util/prompt.py:144
- Draft comment:
logger.debug(f"File exists in current directory: {path_str}")
- Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable:
The comment suggests a change that directly contradicts the code change made in the diff. The author likely had a reason for changing the log level from debug to warning, possibly to highlight the importance of the log message. Without strong evidence that this change is incorrect, the comment should be removed.
I might be missing the context of why the log level was changed, but the comment does not provide any justification for reverting it. The author might have intended to make the log message more prominent.
The lack of justification in the comment for reverting the log level change suggests that it is not a strong comment. The author likely had a reason for the change, and without evidence to the contrary, the comment should be removed.
Remove the comment as it suggests reverting a deliberate code change without justification.
Workflow ID: wflow_Rg54OmUlSw7wMMrw
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
There was a problem hiding this comment.
👍 Looks good to me! Incremental review on d4ae5dc in 14 seconds
More details
- Looked at
13lines of code in1files - Skipped
0files when reviewing. - Skipped posting
1drafted comments based on config settings.
1. gptme/util/prompt.py:399
- Draft comment:
The condition was corrected to check if the parent directory does not exist before creating it. This is a necessary fix to ensure the directory is created when it doesn't exist. - Reason this comment was not posted:
Confidence changes required:0%
The condition in theget_prompt_sessionfunction was incorrectly checking if the parent directory exists before creating it. The fix in the PR correctly changes the condition to check if the parent directory does not exist before creating it.
Workflow ID: wflow_yN337Q87YXWA3Kla
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
There was a problem hiding this comment.
👍 Looks good to me! Incremental review on f9b4fb6 in 14 seconds
More details
- Looked at
12lines of code in1files - Skipped
0files when reviewing. - Skipped posting
1drafted comments based on config settings.
1. tests/test_prompt.py:117
- Draft comment:
Consider automating thecheck_cwd()call to ensure the current working directory is always validated. The current comment is not actionable. - Reason this comment was not posted:
Confidence changes required:50%
The comment suggests automating thecheck_cwd()call, which is a good idea for ensuring the current working directory is always checked. However, the comment itself is not actionable and should be removed or replaced with a more specific implementation note.
Workflow ID: wflow_kSfQhhQmhikom59i
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
There was a problem hiding this comment.
👍 Looks good to me! Incremental review on 77f9978 in 13 seconds
More details
- Looked at
24lines of code in1files - Skipped
0files when reviewing. - Skipped posting
1drafted comments based on config settings.
1. gptme/prompts.py:247
- Draft comment:
The change fromdatetime.utcnow()todatetime.now(timezone.utc)is a good practice for timezone-aware datetime objects. This ensures that the datetime object is aware of its timezone, which can prevent potential bugs related to naive datetime objects. - Reason this comment was not posted:
Confidence changes required:0%
The change fromdatetime.utcnow()todatetime.now(timezone.utc)is correct and aligns with best practices for timezone-aware datetime objects.
Workflow ID: wflow_P1wU6w0o8LdxNDKy
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
There was a problem hiding this comment.
👍 Looks good to me! Incremental review on c46bc28 in 12 seconds
More details
- Looked at
15lines of code in1files - Skipped
0files when reviewing. - Skipped posting
0drafted comments based on config settings.
Workflow ID: wflow_Zh89Gv9mX5bQeOqN
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
ErikBjare
changed the title
Fixes #243
Important
Migrates input handling from
readlinetoprompt_toolkit, enhancing completion, history, and multi-line input features, and fixes existing issues.readlinetoprompt_toolkitinchat.pyandcli.py.prompt_input()inchat.py.prompt_toolkit.GptmeCompleterinprompt.pyfor command, path, and LLM suggestions.create_prompt_session()andget_input()inprompt.pyfor session management and input retrieval.get_pt_history_file()indirs.pyfor managingprompt_toolkithistory files.readline.pyand related imports.PROMPT_USERinconstants.pyto remove non-printable character markers.ask_execute.pyto usecreate_prompt_session()for user prompts.This description was created by
for c46bc28. It will automatically update as commits are pushed.