feat: Alt+Enter queues follow-up messages without interrupting agent - #4788
feat: Alt+Enter queues follow-up messages without interrupting agent#4788iRonin wants to merge 5 commits into
Conversation
Code Review: Alt+Enter Queue FeatureThanks for this useful feature! I've reviewed the diff and found a few issues to address: Critical Bugs
Potential Issues
Minor Suggestions
OverallThe feature is well-implemented with good UX feedback (status bar, placeholder text). The above issues are edge cases but should be fixed before merge. |
|
@britrik thank you for review! I was not sure how busy you are but I started using Hermes yesterday and I asked it to add a couple of features from Claude Code and Pi that I missed and it cranked the first PR without even asking lol |
|
Thanks @britrik — great review. Pushed a fix commit (f02bcb1): #1 & #3 (duplicate text / set leak) — fixed with UUID tags. Alt+Enter now wraps each payload as Phantom queue pops — also fixed. The display sync ( #1 (image payload cancel) — was already correct. Both the queuing side and the process_loop side extracted text via #4 (empty text + images) — intentional. Allows queuing an image-only message (attach image, no text, Alt+Enter). Documented in the commit. #5 (emoji fallback) — not changing. The 📬 emoji is used consistently with other indicators already in the codebase (📌 for stash, etc.). |
f02bcb1 to
5ed3e5d
Compare
Addresses review feedback from britrik (NousResearch#4788): - Replace text-based cancellation with UUID tags — identical messages queued twice no longer cancel each other incorrectly - Wrap Alt+Enter payloads as {_followup_tag, payload} dicts so process_loop can identify followup items by ID, not content - Fix phantom _followup_queue pops: display sync now only happens for tagged (Alt+Enter) items, not regular Enter messages - _cancelled_followups stores UUIDs (bounded, auto-discarded on match) Note: the image-payload cancel check was already correct in the original — both sides extracted text via payload[0] — but UUID tagging makes the intent unambiguous regardless of payload shape.
5ed3e5d to
d75001f
Compare
d75001f to
ce7fa7b
Compare
Addresses review feedback from britrik (NousResearch#4788): - Replace text-based cancellation with UUID tags — identical messages queued twice no longer cancel each other incorrectly - Wrap Alt+Enter payloads as {_followup_tag, payload} dicts so process_loop can identify followup items by ID, not content - Fix phantom _followup_queue pops: display sync now only happens for tagged (Alt+Enter) items, not regular Enter messages - _cancelled_followups stores UUIDs (bounded, auto-discarded on match) Note: the image-payload cancel check was already correct in the original — both sides extracted text via payload[0] — but UUID tagging makes the intent unambiguous regardless of payload shape.
Addresses review feedback from britrik (NousResearch#4788): - Replace text-based cancellation with UUID tags — identical messages queued twice no longer cancel each other incorrectly - Wrap Alt+Enter payloads as {_followup_tag, payload} dicts so process_loop can identify followup items by ID, not content - Fix phantom _followup_queue pops: display sync now only happens for tagged (Alt+Enter) items, not regular Enter messages - _cancelled_followups stores UUIDs (bounded, auto-discarded on match) Note: the image-payload cancel check was already correct in the original — both sides extracted text via payload[0] — but UUID tagging makes the intent unambiguous regardless of payload shape.
Addresses review feedback from britrik (NousResearch#4788): - Replace text-based cancellation with UUID tags — identical messages queued twice no longer cancel each other incorrectly - Wrap Alt+Enter payloads as {_followup_tag, payload} dicts so process_loop can identify followup items by ID, not content - Fix phantom _followup_queue pops: display sync now only happens for tagged (Alt+Enter) items, not regular Enter messages - _cancelled_followups stores UUIDs (bounded, auto-discarded on match) Note: the image-payload cancel check was already correct in the original — both sides extracted text via payload[0] — but UUID tagging makes the intent unambiguous regardless of payload shape.
Addresses review feedback from britrik (NousResearch#4788): - Replace text-based cancellation with UUID tags — identical messages queued twice no longer cancel each other incorrectly - Wrap Alt+Enter payloads as {_followup_tag, payload} dicts so process_loop can identify followup items by ID, not content - Fix phantom _followup_queue pops: display sync now only happens for tagged (Alt+Enter) items, not regular Enter messages - _cancelled_followups stores UUIDs (bounded, auto-discarded on match) Note: the image-payload cancel check was already correct in the original — both sides extracted text via payload[0] — but UUID tagging makes the intent unambiguous regardless of payload shape.
Alt+Enter now queues the current input as a follow-up to be sent after the agent finishes responding, instead of inserting a newline. - Alt+Enter → puts message into _pending_input (non-interrupting) - Enter (agent running) → still interrupts via _interrupt_queue - _followup_queue list mirrors pending items for display - Status bar shows 📬 N when follow-ups are queued - Placeholder hints update: shows queue depth while agent runs, and persists after it finishes until queue drains - Ctrl+J remains the newline key for multi-line input Closes: the need for Shift+Enter queue (terminals can't distinguish Shift+Enter from Enter; Alt+Enter is the reliable alternative)
Alt+Up pops the most recently queued follow-up (LIFO) from _followup_queue, appends it to the current input with a newline--- separator, and marks it cancelled so process_loop skips it. Repeated Alt+Up recalls one at a time until queue is empty. _cancelled_followups set is checked in process_loop and discarded on match to avoid sending the recalled message twice.
Addresses review feedback from britrik (NousResearch#4788): - Replace text-based cancellation with UUID tags — identical messages queued twice no longer cancel each other incorrectly - Wrap Alt+Enter payloads as {_followup_tag, payload} dicts so process_loop can identify followup items by ID, not content - Fix phantom _followup_queue pops: display sync now only happens for tagged (Alt+Enter) items, not regular Enter messages - _cancelled_followups stores UUIDs (bounded, auto-discarded on match) Note: the image-payload cancel check was already correct in the original — both sides extracted text via payload[0] — but UUID tagging makes the intent unambiguous regardless of payload shape.
5b53c08 to
ad9b4c8
Compare
Addresses review feedback from britrik (NousResearch#4788): - Replace text-based cancellation with UUID tags — identical messages queued twice no longer cancel each other incorrectly - Wrap Alt+Enter payloads as {_followup_tag, payload} dicts so process_loop can identify followup items by ID, not content - Fix phantom _followup_queue pops: display sync now only happens for tagged (Alt+Enter) items, not regular Enter messages - _cancelled_followups stores UUIDs (bounded, auto-discarded on match) Note: the image-payload cancel check was already correct in the original — both sides extracted text via payload[0] — but UUID tagging makes the intent unambiguous regardless of payload shape.
teknium1
left a comment
There was a problem hiding this comment.
Thanks for the focused follow-up-queue UX work. The requested Alt+Enter behavior is not on current main, but this branch needs adaptation before it can be assessed for merge.
Problems
- The change reassigns Alt+Enter, but current CLI documentation still promises it as a newline binding in
website/docs/user-guide/cli.md:102,:216, and:240, pluswebsite/docs/getting-started/quickstart.md:243andhermes_cli/tips.py:57. Main implements that contract atcli.py:13564-13573. tests/test_cli_followup_queue.pyonly checks initialized attribute shapes. It does not exercise Alt+Enter/Alt+Up, queued payload consumption, cancellation, duplicate text, or image payloads.- GitHub reports this branch as conflicting, and current busy-input routing is now at
cli.py:13502-13560with consumption atcli.py:15157-15338; the existingbusy_input_mode="queue"path atcli.py:13525-13529must remain intact.
Suggested changes
- Reconcile the keybinding/documentation contract and add behavioral keybinding/queue tests while adapting the implementation to the current routing paths.
Automated hermes-sweeper review.
| @@ -0,0 +1,140 @@ | |||
| """Tests for PR #4788 feat/queue-followup. | |||
|
|
|||
| Covers: Alt+Enter queues followup messages. | |||
There was a problem hiding this comment.
This file does not yet cover the stated Alt+Enter behavior: it only asserts constructor state. Please add behavioral tests that invoke the key handler and verify enqueueing, FIFO consumption, Alt+Up cancellation, duplicate text, and image payload cases.
Repurposes Alt+Enter from 'insert newline' to 'queue follow-up message'.
Behaviour:
_pending_input— sent as the next turn, no interruptionUI feedback:
📬 Nwhen follow-ups are queuedEnter to interrupt · Alt+Enter to queue follow-up · 📬 N queued📬 N follow-ups queued — Alt+Enter to add moreImplementation:
_followup_queuelist mirrors_pending_inputfor display (popped inprocess_loopas messages are consumed)busy_input_modeconfig