fix: properly handle piped input in prompts #354
Conversation
|
This PR improves upon #344 which added initial support for piped stdin. Instead of treating piped input as a separate message, it now properly integrates it into the first prompt message (or creates a new one if none exists), making the behavior more consistent and intuitive. |
![ellipsis-dev[bot]](https://avatars.githubusercontent.com/in/64358?s=60&v=4){kind=small}
There was a problem hiding this comment.
❌ Changes requested. Reviewed everything up to 040622b in 1 minute and 12 seconds
More details
- Looked at
47lines of code in1files - Skipped
0files when reviewing. - Skipped posting
0drafted comments based on config settings.
Workflow ID: wflow_o043PHngYohJeCKQ
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.
gptme/cli.py
Outdated
| @@ -283,6 +294,7 @@ def main( | |||
| ) | |||
| except RuntimeError as e: | |||
| logger.error(e) | |||
There was a problem hiding this comment.
logger.error(e) is redundant since logger.exception(e) already logs the error message. Consider removing logger.error(e). This applies to line 296 and 297.
Previously, piped input was treated as a separate message instead of being included in the user prompt. This change: - Adds inject_stdin function to handle piped input - Ensures piped input is appended to first prompt or creates new message if none exists - Removes redundant system message creation
ErikBjare
force-pushed
the
dev/fix-piped-input-msgs
branch
from
040622b to
57cdc9f
Compare
There was a problem hiding this comment.
👍 Looks good to me! Incremental review on 57cdc9f in 10 seconds
More details
- Looked at
39lines of code in1files - Skipped
0files when reviewing. - Skipped posting
1drafted comments based on config settings.
1. gptme/cli.py:241
- Draft comment:
Consider movinginject_stdinoutside themainfunction if it needs to be reused elsewhere. However, the current implementation is correct as per the PR description. - Reason this comment was not posted:
Confidence changes required:33%
Theinject_stdinfunction is defined within themainfunction, which is fine for encapsulation, but it could be moved outside if it needs to be reused elsewhere. However, the current implementation is correct as per the PR description.
Workflow ID: wflow_utoXWTgQfPo84pex
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 #354 +/- ##
==========================================
- Coverage 71.15% 70.86% -0.30%
==========================================
Files 68 68
Lines 5228 5234 +6
==========================================
- Hits 3720 3709 -11
- Misses 1508 1525 +17
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Issue
Previously, piped input was being treated as a separate message instead of being included in the user prompt. This could lead to unexpected behavior when using gptme with piped input.
Changes
Testing
Tested with various piped input scenarios:
Important
Introduces
inject_stdinfunction ingptme/cli.pyto handle piped input by appending it to the first prompt or creating a new message, removing redundant system message creation.inject_stdinfunction ingptme/cli.pyto handle piped input.This description was created by
for 57cdc9f. It will automatically update as commits are pushed.