feat: add status emojis to system messages

[0xbrayo]

Important

Add status emojis to system messages by introducing a status attribute in the Message class and updating related functions to handle this attribute.

• Behavior:
  • Add status attribute to Message class in message.py to represent message status as "success", "error", "info", or None.
  • Update format_msgs() in message.py to append status emojis to system messages based on the status attribute.
• Functions:
  • Modify execute() in base.py, execute_patch_impl() in patch.py, execute_python() in python.py, execute_save_impl() and execute_append_impl() in save.py, execute_shell_impl() in shell.py, and execute_tmux() in tmux.py to set the status attribute in Message instances.
• Misc:
  • Update to_dict() and from_toml() in message.py to handle the status attribute.
  • Add status handling in execute_with_confirmation() in ask_execute.py.

^{This description was created by for 05f8220. It will automatically update as commits are pushed.}

[ellipsis-dev]

👍 Looks good to me! Reviewed everything up to 05f8220 in 1 minute and 5 seconds

More details

• Looked at 230 lines of code in 8 files
• Skipped 0 files when reviewing.
• Skipped posting 8 drafted comments based on config settings.

1. gptme/message.py:46

• Draft comment:
  The status attribute is added to the Message class, but the class docstring is not updated to reflect this new attribute. Consider updating the docstring to include status and its possible values.
• Reason this comment was not posted:
  Confidence changes required: 50%
  The PR introduces a new 'status' attribute to the Message class, which is used to add status emojis to system messages. This is a good feature addition, but there are some areas that need attention.

2. gptme/message.py:89

• Draft comment:
  The to_dict method now includes the status attribute. Ensure that any consumers of this dictionary representation are aware of the new status field to avoid potential issues.
• Reason this comment was not posted:
  Confidence changes required: 30%
  The PR introduces a new 'status' attribute to the Message class, which is used to add status emojis to system messages. This is a good feature addition, but there are some areas that need attention.

3. gptme/message.py:226

• Draft comment:
  The use of the walrus operator := is Python 3.8+. Ensure that the project supports this version or higher. If not, consider using a different approach to maintain compatibility.
• Reason this comment was not posted:
  Confidence changes required: 50%
  The PR introduces a new 'status' attribute to the Message class, which is used to add status emojis to system messages. This is a good feature addition, but there are some areas that need attention.

4. gptme/tools/base.py:300

• Draft comment:
  The comment # this works is unnecessary and should be removed to maintain code cleanliness. This applies to similar comments in other files as well.
• Reason this comment was not posted:
  Confidence changes required: 20%
  The PR introduces a new 'status' attribute to the Message class, which is used to add status emojis to system messages. This is a good feature addition, but there are some areas that need attention.

5. gptme/tools/patch.py:261

• Draft comment:
  The message 'No patch provided by the assistant' is marked with status="error". Consider if this should be an info status instead, as it might not be an error condition but rather a lack of input.
• Reason this comment was not posted:
  Confidence changes required: 30%
  The PR introduces a new 'status' attribute to the Message class, which is used to add status emojis to system messages. This is a good feature addition, but there are some areas that need attention.

6. gptme/tools/python.py:119

• Draft comment:
  The message 'Execution aborted: user chose not to run this code.' is marked with status="info". Ensure this is the intended status level, as it might be more appropriate as a warning.
• Reason this comment was not posted:
  Confidence changes required: 30%
  The PR introduces a new 'status' attribute to the Message class, which is used to add status emojis to system messages. This is a good feature addition, but there are some areas that need attention.

7. gptme/tools/save.py:100

• Draft comment:
  The message 'Save aborted: user refused to overwrite the file.' is marked with status="error". Consider if this should be a warning status instead, as it might not be a critical error but a user decision.
• Reason this comment was not posted:
  Confidence changes required: 30%
  The PR introduces a new 'status' attribute to the Message class, which is used to add status emojis to system messages. This is a good feature addition, but there are some areas that need attention.

8. gptme/tools/shell.py:306

• Draft comment:
  The print statements for debugging purposes (e.g., print(f"expecting non-zero return code: {returncode}")) should be removed or replaced with proper logging to maintain code cleanliness and performance.
• Reason this comment was not posted:
  Confidence changes required: 40%
  The PR introduces a new 'status' attribute to the Message class, which is used to add status emojis to system messages. This is a good feature addition, but there are some areas that need attention.

Workflow ID: wflow_eA21jIrCmMrzV4QH

---

You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

[0xbrayo]

#304

[ErikBjare]

I think we can do this without adding msg.status

[ErikBjare]

In gptme-webui I simply check the first line with:

first_line = msg.content.split("\n", 1)[0].lower()
isSuccess = first_line.startswith(["saved", "success", "patch successfully"])
isError = first_line.startswith(["error", "failed"])

Since it's just a design thing, I think such a lightweight approach makes more sense.
The "info" emoji also doesn't make much sense to me (how is it different from no status?)

[0xbrayo]

The "info" emoji also doesn't make much sense to me (how is it different from no status?)

The info emoji was suggested by gptme yeah, I should just scrap it.

[ErikBjare]

"executed", "ran"

I think these do not actually signal success explicitly? (could have non-zero exit codes)

[ErikBjare]

I see tmux gives "missing arguments", the message should probably be changed to "Error: Missing arguments" so the detection can be simplified.
Same for "unknown command" and "no patch".

[0xbrayo]

Done

[0xbrayo]

When a shell command exits with a non-zero code the code is part of the message. Only captures unsuccessful commands. Could change the code to append the return code for each command and strip the zero return codes before displaying the message.

[ErikBjare]

I think "failed to run" might be bad prompting. We should be careful to not infer too much from a non-zero exit code I think. I'd just keep the "Ran" and not try to infer a success/error status for shell commands, for now.

[0xbrayo]

Removed the shell.py changes.

[ErikBjare]

Nice, lets go!