Skip to content

fix(agent): sanitize surrogate characters from Ollama model output - #5176

Closed
tmchow wants to merge 1 commit into
NousResearch:mainfrom
tmchow:fix/5059-surrogate-sanitization
Closed

fix(agent): sanitize surrogate characters from Ollama model output#5176
tmchow wants to merge 1 commit into
NousResearch:mainfrom
tmchow:fix/5059-surrogate-sanitization

Conversation

@tmchow

@tmchow tmchow commented Apr 5, 2026

Copy link
Copy Markdown
Contributor

What does this PR do?

Fixes three gaps in the existing surrogate character sanitization that cause UnicodeEncodeError crashes when using Ollama-hosted models (Kimi K2.5, GLM-5, Qwen). These models can return invalid surrogate code points (U+D800-U+DFFF) that crash json.dumps() during UTF-8 serialization.

The existing _sanitize_surrogates() and _sanitize_messages_surrogates() functions work correctly, but they're not called in the right places:

  1. No proactive sanitization before the API call (line 6848). Surrogates from prior model output or tool results survive in api_messages and crash on the next turn. Fixed by calling _sanitize_messages_surrogates(api_messages) after _sanitize_api_messages().

  2. API responses stored without sanitization (line 5242). Surrogates from model output persist in session history via _build_assistant_message(). Fixed by sanitizing content and reasoning_text before building the message dict.

  3. Retry path sanitized the wrong variable (line 7368). The UnicodeEncodeError handler called _sanitize_messages_surrogates(messages) but the API call uses api_messages (a separate list). The retry would fail again with the same error. Fixed by sanitizing both api_messages and messages.

Related Issue

Fixes #5059

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

  • run_agent.py:6848: Added _sanitize_messages_surrogates(api_messages) call after _sanitize_api_messages() for proactive surrogate cleaning before the API call
  • run_agent.py:5242: Sanitize assistant_message.content and reasoning_text with _sanitize_surrogates() before storing in message history
  • run_agent.py:7368: Changed _sanitize_messages_surrogates(messages) to also sanitize api_messages so the retry actually cleans the list sent to the API

How to Test

  1. Set up Hermes with an Ollama-hosted model (GLM-5 or Kimi K2.5)
  2. Ask the model to process a large file or have a long conversation that produces surrogate characters
  3. Verify no UnicodeEncodeError crash occurs
  4. Run pytest tests/test_surrogate_sanitization.py to verify existing tests still pass

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: macOS 15 (Darwin 25.3.0)

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

This contribution was developed with AI assistance (Claude Code).

Compound Engineering

Models served through Ollama (Kimi K2.5, GLM-5, Qwen) can return
invalid surrogate code points (U+D800-U+DFFF) that crash json.dumps()
during UTF-8 serialization.

Three gaps in the existing surrogate handling:

1. No proactive sanitization before the API call. Surrogates from prior
   model output or tool results survive in api_messages and crash on
   the next turn. Fixed by calling _sanitize_messages_surrogates() on
   api_messages after _sanitize_api_messages().

2. API responses stored without sanitization. Surrogates from model
   output persist in session history via _build_assistant_message().
   Fixed by sanitizing content and reasoning_text before building the
   message dict.

3. Retry path sanitized the wrong variable. The UnicodeEncodeError
   handler at line ~7366 called _sanitize_messages_surrogates(messages)
   but the API call uses api_messages (a separate list). The retry
   would fail again with the same error. Fixed by sanitizing both
   api_messages and messages.

Fixes NousResearch#5059
@tmchow

tmchow commented Apr 5, 2026

Copy link
Copy Markdown
Contributor Author

Closing in favor of #5074 which addresses the same issue and was submitted earlier. Nice fix @ygd58. Missed yours before i started my fix!

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.

[Bug]: Surrogate Code Points Crash UTF-8 Serialization with Ollama Models

1 participant