fix(agent): preserve later user message boundaries - #46459
Conversation
tonydwb
left a comment
There was a problem hiding this comment.
Code Review Summary
Verdict: Approved
Fix for agent message boundary preservation: ensures later user message boundaries are not lost in the conversation processing pipeline.
Looks Good
- Clean, well-scoped
- No security or stability concerns
Reviewed by Hermes Agent
teknium1
left a comment
There was a problem hiding this comment.
Thanks for isolating the alternation-repair marker path. The reported behavior is still present on current main: agent/agent_runtime_helpers.py:543-555 merges adjacent user text and rewrites the live message list, called before API-copy construction at agent/conversation_loop.py:783-794.
Problems
- This is not an end-to-end #45560 fix: source turns remain canonically collapsed before persistence. The currently linked competing PR #63298 documents the broader wire-only merge/FIFO approach.
tui_gateway/server.py:5074-5091independently retains one busy queued-prompt slot and merges a second prompt with\n\n; this PR does not cover that path.- The changed tests calculate expected values through
merge_later_user_text, so they do not assert the actual[Later user message]contract.
Suggested changes
- Either scope the PR to the narrow textual-marker behavior, or preserve canonical turns and merge only the API copy.
- Add direct literal-marker and empty-input tests, plus coverage for the TUI busy-queue path if retaining the #45560 linkage.
Automated hermes-sweeper review.
| assert len(messages) == 1 | ||
| assert messages[0]["role"] == "user" | ||
| assert messages[0]["content"] == "first\n\nsecond" | ||
| assert messages[0]["content"] == merge_later_user_text("first", "second") |
There was a problem hiding this comment.
This expected value is computed by the same production helper the test exercises, so it does not pin the required marker. Please add a direct assertion for the literal [Later user message] boundary (and empty-side behavior).
Fixes #45560.
Summary
Verification
uv run --frozen pytest tests/run_agent/test_message_sequence_repair.py tests/gateway/test_active_session_text_merge.py tests/agent/test_anthropic_adapter.py -k "merge or merges_consecutive_user_messages"uv run --frozen ruff check utils.py agent/agent_runtime_helpers.py agent/anthropic_adapter.py gateway/platforms/base.py tests/run_agent/test_message_sequence_repair.py tests/gateway/test_active_session_text_merge.py tests/agent/test_anthropic_adapter.pygit diff --check HEAD^ HEADNote
PytestUnraisableExceptionWarningfrom text-debounce task cleanup outside the changed merge logic.