Skip to content

fix(cli): prepend turn notes to multimodal messages#37080

Closed
helix4u wants to merge 1 commit into
NousResearch:mainfrom
helix4u:fix/cli-multimodal-turn-notes
Closed

fix(cli): prepend turn notes to multimodal messages#37080
helix4u wants to merge 1 commit into
NousResearch:mainfrom
helix4u:fix/cli-multimodal-turn-notes

Conversation

@helix4u

@helix4u helix4u commented Jun 2, 2026

Copy link
Copy Markdown
Contributor

What does this PR do?

Fixes a CLI crash when a user switches model mid-session and then sends an image attachment as the next turn.

The CLI queues one-shot notes after commands such as /model and /reload-skills so the model sees the updated session state on the next user turn. That path assumed the next message was always a string. Native image attachment routing can instead turn the next message into an OpenAI-style content-parts list, so the CLI crashed while trying to concatenate the note with a list.

This adds a small helper that prepends queued CLI notes to either plain text messages or native multimodal content lists. For content lists, it prefixes the first text part, or inserts a text part before image parts when no text part exists.

Related Issue

No issue filed. Reported on Discord by privacydied.

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)
  • ✨ New feature (non-breaking change that adds functionality)
  • 🔒 Security fix
  • 📝 Documentation update
  • ✅ Tests (adding or improving test coverage)
  • ♻️ Refactor (no behavior change)
  • 🎯 New skill (bundled or hub)

Changes Made

  • cli.py: add _prepend_turn_note_to_message() for one-shot CLI notes on string and native multimodal messages.
  • cli.py: use the helper for pending model-switch notes and pending skills-reload notes.
  • tests/cli/test_cli_image_command.py: add regression coverage for text messages, native image messages with a text part, and image-only content-part lists.

How to Test

  1. Start Hermes on one model, then run /model gpt-5.5 --provider openai-codex.
  2. Attach or paste an image as the next message.
  3. Confirm the CLI does not crash with TypeError: can only concatenate str (not "list") to str and the image turn reaches the model.

Focused local validation:

  • scripts/run_tests.sh -j 4 tests/cli/test_cli_image_command.py — 12 passed
  • scripts/run_tests.sh -j 4 tests/cli/test_cli_reload_skills.py — 3 passed
  • git diff --check -- cli.py tests/cli/test_cli_image_command.py — clean

Checklist

Code

  • I've read the Contributing Guide
  • My commit messages follow Conventional Commits (fix(scope):, feat(scope):, etc.)
  • I searched for existing PRs to make sure this isn't a duplicate
  • My PR contains only changes related to this fix/feature (no unrelated commits)
  • I've run pytest tests/ -q and all tests pass
  • I've added tests for my changes (required for bug fixes, strongly encouraged for features)
  • I've tested on my platform: WSL2 / Linux 6.6.87.2

Documentation & Housekeeping

  • I've updated relevant documentation (README, docs/, docstrings) — or N/A
  • I've updated cli-config.yaml.example if I added/changed config keys — or N/A
  • I've updated CONTRIBUTING.md or AGENTS.md if I changed architecture or workflows — or N/A
  • I've considered cross-platform impact (Windows, macOS) per the compatibility guide — or N/A
  • I've updated tool descriptions/schemas if I changed tool behavior — or N/A

For New Skills

N/A

Screenshots / Logs

Original crash:

TypeError: can only concatenate str (not "list") to str

Crash site was the pending model-switch note prepend path when the next message was a native multimodal content-parts list.

xxxigm added a commit to xxxigm/hermes-agent that referenced this pull request Jun 2, 2026
Adopt the cleaner handling from PR NousResearch#37080: coerce/strip the note and
skip the extra newlines when the underlying message (or text part) is
empty, while keeping the safer fail-open behavior for unknown shapes.

@tonydwb tonydwb left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review Summary

Verdict: Approved

Review Notes

  • PR #37080fix(cli): prepend turn notes to multimodal messages by @helix4u

✅ Looks Good

  • Same root cause as #37081 (sibling fix) — both independently address the same str + list TypeError. This PR's _prepend_turn_note_to_message() handles the same cases correctly.
  • Good test coverage: 3 test cases in existing test file, covering str, multimodal with text part, and image-only lists.
  • Non-mutating: Original message preserved.
  • Clean scope: Only 2 files changed (cli.py + test file), 67 additions.

⚠️ Note on Duplication

  • This PR and #37081 fix the same bug independently. Both solutions are correct. The team should pick one or merge the approaches. This PR uses a simpler helper name (_prepend_turn_note_to_message with args (note, message)) while #37081 uses _prepend_note_to_message with args (message, note).

Reviewed by Hermes Agent

@helix4u helix4u closed this Jun 2, 2026
@helix4u

helix4u commented Jun 2, 2026

Copy link
Copy Markdown
Contributor Author

Looks like #37081 landed on the same root cause after this one. It has the same fix shape plus a few extra edge-case tests, so I’m fine with using that one instead if maintainers prefer the more complete test coverage.

teknium1 pushed a commit that referenced this pull request Jun 2, 2026
Adopt the cleaner handling from PR #37080: coerce/strip the note and
skip the extra newlines when the underlying message (or text part) is
empty, while keeping the safer fail-open behavior for unknown shapes.
changman pushed a commit to changman/hermes-agent that referenced this pull request Jun 10, 2026
Adopt the cleaner handling from PR NousResearch#37080: coerce/strip the note and
skip the extra newlines when the underlying message (or text part) is
empty, while keeping the safer fail-open behavior for unknown shapes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants