Skip to content

fix: preserve ACP multimodal content during persistence#44249

Open
Rivuza wants to merge 1 commit into
NousResearch:mainfrom
Rivuza:fix/acp-preserve-multimodal-user-content
Open

fix: preserve ACP multimodal content during persistence#44249
Rivuza wants to merge 1 commit into
NousResearch:mainfrom
Rivuza:fix/acp-preserve-multimodal-user-content

Conversation

@Rivuza

@Rivuza Rivuza commented Jun 11, 2026

Copy link
Copy Markdown

Summary

  • Keep text-only persist_user_message overrides from replacing multimodal current-turn user content
  • Preserve ACP image/image_url blocks through early crash-resilience persistence so they can reach the provider request
  • Add regression coverage for text override behavior and multimodal preservation

Test Plan

  • /home/rivuza/hermes-agent/.venv/bin/python -m pytest tests/run_agent/test_run_agent.py::test_persist_user_message_override_rewrites_text_turns tests/run_agent/test_run_agent.py::test_persist_user_message_override_preserves_multimodal_turns -q
  • /home/rivuza/hermes-agent/.venv/bin/python -m pytest tests/run_agent/test_run_agent.py -q

Closes #44242

@Rivuza
fix: preserve multimodal user content during persistence
ac8d236 
Avoid applying text-only persist_user_message overrides to multimodal current-turn user messages. Early crash-resilience persistence mutates the same messages list later used for the API call, so clobbering list content drops ACP image blocks before model dispatch.\n\nAdd regression coverage for both text override behavior and multimodal preservation.\n\nCloses NousResearch#44242
@liuhao1024

liuhao1024 commented Jun 11, 2026

Copy link
Copy Markdown
Contributor

Verification review — reviewed the diff and tests; no issues found.

The guard if isinstance(msg.get("content"), list): return correctly prevents the text-only persistence override from clobbering multimodal content-part lists. The unit tests directly exercise _apply_persist_user_message_override for both text-rewriting and multimodal-skip paths.

Note: #44248 addresses the same issue (#44242) with an equivalent guard. Its test is more end-to-end — it drives _persist_session and verifies that (a) the API-bound message keeps the image part and (b) the DB persistence still redacts to "[screenshot]". Both PRs are clean; the maintainer can pick either. If this one lands first, #44248 can be closed (and vice versa).

@alt-glitch alt-glitch added type/bug Something isn't working P3 Low — cosmetic, nice to have comp/agent Core agent loop, run_agent.py, prompt builder comp/acp Agent Communication Protocol adapter duplicate This issue or pull request already exists labels Jun 11, 2026
@alt-glitch

alt-glitch commented Jun 11, 2026

Copy link
Copy Markdown
Collaborator

Duplicate of #44247 — same approach (isinstance(content, list) guard in _apply_persist_user_message_override to skip the text-only override for multimodal turns), both fixing #44242. #44247 is the earliest open PR of the trio (#44248 is the other dup).

@alt-glitch alt-glitch mentioned this pull request Jun 11, 2026
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

comp/acp Agent Communication Protocol adapter comp/agent Core agent loop, run_agent.py, prompt builder duplicate This issue or pull request already exists P3 Low — cosmetic, nice to have type/bug Something isn't working

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

ACP image content blocks dropped before API call (persist_user_message override clobbers multimodal content)

3 participants

@Rivuza @liuhao1024 @alt-glitch