fix(cli): collapse timeline markers and /skill invocations in the resume preview - #71950
fix(cli): collapse timeline markers and /skill invocations in the resume preview#71950pierrenode wants to merge 1 commit into
Conversation
a576677 to
54fd0cb
Compare
|
Thanks — this addresses a verified classic-CLI-only resume projection gap. On current main, The proposed placement after the existing GitHub currently reports this branch as conflicting. Salvage should resolve the local hunk by retaining main's Automated hermes-sweeper review. |
…ume preview _display_resumed_history() (the CLI's "Previous Conversation" recap shown on /resume and hermes --resume) is a separate display projection from tui_gateway/server.py::_history_to_messages (the one desktop/TUI/web read). It special-cases the display_kind values it happens to know about, but two classes of bookkeeping/scaffolding rows fell through to the plain role=="user" branch and rendered as fake You: bubbles: 1. Personality-change/clear marker — tui_gateway.server's _set_personality() writes a plain role=user "[System: ...]" row with no display_kind. _history_to_messages already hides it via _is_display_hidden_marker() (a role + "[System:" prefix check, independent of display_kind); the CLI's recap never picked up the same check. 2. /skill invocations — persisted expanded (activation note plus the entire skill body). _history_to_messages already collapses this onto the invocation the user typed via describe_skill_invocation(); the CLI's recap showed up to 300 raw characters of the expanded body as if the user had written it. This is the third time this general bug class has been closed without covering the CLI path — NousResearch#68665 / NousResearch#69861 fixed the duplicate-bubble symptom on desktop, and a same-day sibling commit widened _history_to_messages for "desktop, TUI, CLI, and web transcripts" even though that function is never on the CLI's `hermes --resume` code path. Fix: - Personality marker: same role == "user" + "[System:" prefix check as _is_display_hidden_marker(), added to _display_resumed_history()'s history loop. Positioned after the display_kind special cases (so model_switch's "◈ model changed" event line is unaffected) and before the plain role == "user" handling. - Skill invocations: describe_skill_invocation() (from agent.skill_commands, the same helper tui_gateway/server.py uses) is called on role=="user" content; a match renders as "◈ skill invoked: /work — fix the leak" instead of the expanded body, consistent with the existing model-switch / async-delegation event lines. Deliberately not importing tui_gateway.server to reuse _is_display_hidden_marker directly — that module is not otherwise on the CLI's import graph and is a large gateway-oriented module. The duplicated prefix check is commented to point back at _is_display_hidden_marker so the two stay in sync if the marker wording/role ever changes. Tests: 4 regression tests (personality-change marker, personality-cleared marker, skill invocation with instruction, bare skill invocation) in tests/cli/test_resume_display.py — all assert the raw scaffolding never appears and the collapsed/hidden form renders correctly, while real conversation turns around them still render. Mutation-verified: each fix reverted independently, confirmed both fail with the exact bug's failure mode.
54fd0cb to
d076478
Compare
|
Rebased onto current `upstream/main` and squashed to a single commit. `hermes_cli/cli_agent_setup_mixin.py` auto-merged cleanly. `tests/cli/test_resume_display.py` had one conflict: the pre-existing test this PR's diff shows as trailing context (`test_tool_messages_hidden`) has been renamed/rewritten upstream to `test_tool_only_message_skipped_by_default` with different content — kept upstream's current version untouched and inserted this PR's 4 new tests immediately before it. Verified the retained tail matches `upstream/main`'s file byte-for-byte from that point on. Targeted suite (19 tests) passes, mutation-verified both fixes independently (disabling the `[System:` prefix check reproduces both personality-marker tests failing with the raw marker text visible in output; disabling the `describe_skill_invocation()` call reproduces both skill-invocation tests failing with the raw skill body visible). Broader sweep (`tests/cli/` filtered to resume/agent_setup/skill_commands, 59 tests, plus the dedicated `describe_skill_invocation` test file, 7 tests) passes clean. Ruff clean. Fresh competitor search found no PR touching this specific gap (the closest hits were unrelated `_display_resumed_history` symptoms — empty messages, hardcoded agent name, terminal escape sanitization). |
Summary
_display_resumed_history()(the CLI's "Previous Conversation" recap shown on/resumeandhermes --resume) is a separate display projection fromtui_gateway/server.py::_history_to_messages(the one desktop/TUI/web read). It special-cases thedisplay_kindvalues it happens to know about, but two classes of bookkeeping/scaffolding rows fall through to the plainrole=="user"branch and render as fakeYou:bubbles:tui_gateway.server's_set_personality()writes a plainrole=user "[System: ...]"row with nodisplay_kind._history_to_messagesalready hides it via_is_display_hidden_marker()(a role +"[System:"prefix check, independent ofdisplay_kind); the CLI's recap never picked up the same check./skillinvocations — persisted expanded (activation note plus the entire skill body)._history_to_messagesalready collapses this onto the invocation the user typed viadescribe_skill_invocation(); the CLI's recap showed up to 300 raw characters of the expanded body as if the user had written it.This is the third time this general bug class has been closed without covering the CLI path — #68665 / #69861 fixed the duplicate-bubble symptom on desktop, and a same-day sibling commit widened
_history_to_messagesfor "desktop, TUI, CLI, and web transcripts" even though that function is never on the CLI'shermes --resumecode path.Fix
role == "user"+"[System:"prefix check as_is_display_hidden_marker(), added to_display_resumed_history()'s history loop. Positioned after thedisplay_kindspecial cases (somodel_switch's "◈ model changed" event line is unaffected) and before the plainrole == "user"handling.describe_skill_invocation()(fromagent.skill_commands, the same helpertui_gateway/server.pyuses) is called onrole=="user"content; a match renders as"◈ skill invoked: /work — fix the leak"instead of the expanded body, consistent with the existing model-switch / async-delegation event lines.Deliberately not importing
tui_gateway.serverto reuse_is_display_hidden_markerdirectly — that module is not otherwise on the CLI's import graph and is a large gateway-oriented module. The duplicated prefix check is commented to point back at_is_display_hidden_markerso the two stay in sync if the marker wording/role ever changes.Changes
hermes_cli/cli_agent_setup_mixin.py: skiprole=="user"rows whose content starts with"[System:"; collapserole=="user"rows matchingdescribe_skill_invocation()to an event line.tests/cli/test_resume_display.py: 4 regression tests (personality-change marker, personality-cleared marker, skill invocation with instruction, bare skill invocation) — all assert the raw scaffolding never appears and the collapsed/hidden form renders correctly, while real conversation turns around them still render.Validation
tests/cli/test_resume_display.py+tests/cli/test_cli_resume_command.py: 69/69 passed.tests/hermes_cli/resume/agent-setup/skill subset: 312/313 passed — the one failure (test_session_platform_env_var) reproduces identically with both commits reverted (pre-existing test-order pollution via env-var leakage between tests, unrelated to this PR).ruff check: clean.