Skip to content

fix(ollama): preserve multi-byte UTF-8 content split across stream chunks - #11921

Merged
diegosouzapw merged 1 commit into
diegosouzapw:release/v3.8.51from
pacocartones:fix/ollama-multibyte-utf8-boundary
Aug 30, 2026
Merged

diegosouzapw merged 1 commit into
diegosouzapw:release/v3.8.51from
pacocartones:fix/ollama-multibyte-utf8-boundary

Conversation

@pacocartones

Copy link
Copy Markdown
Contributor

Summary

  • The Ollama-compatible streaming transform (transformToOllama in open-sse/utils/ollamaTransform.ts) decoded every upstream network chunk with a fresh new TextDecoder().decode(chunk) and no { stream: true }. A multi-byte UTF-8 character (CJK, emoji) split across a chunk boundary was decoded as two incomplete halves and turned into U+FFFD replacement characters in the NDJSON delivered to Ollama-format clients. The line-level buffer reassembles split SSE lines, but the corruption happens one layer below at byte decode, so it cannot recover it.
  • Fixed by hoisting a single persistent TextDecoder and decoding with { stream: true }, so pending bytes carry across chunks. This matches the pattern already used by every other streaming transform/executor in open-sse/ — e.g. responsesTransformer.ts (which comments "persistent decoder with { stream: true } carries pending bytes between chunks"), stream.ts, and the *-web executors. ollamaTransform.ts was the only streaming transform not doing this.

Related Issues

  • No linked issue — unreported bug found while reviewing the streaming transforms.

Validation

  • Change type: routing (SSE stream transform)
  • Focused test: tests/unit/ollama-transform.test.ts — the new case fails on the pre-fix code ('���好世界🌍', AssertionError) and passes after; full file 10/10 with the fix.
  • npm run lint / full unit suite / coverage / build — not run locally (dependencies not installed); CI (docs: official 'golden path' contribution guide by change type #8329) runs them on this PR. The change is a 3-line decoder hoist with no new imports.
  • Reconciled with the current active release base (branched from main tip d5dfcff); focused test rerun afterward.
  • Production-code change includes a new automated test in this PR.

Tests Added Or Updated

  • tests/unit/ollama-transform.test.ts — new test "transformToOllama preserves multi-byte UTF-8 content split across chunks": splits a CJK+emoji delta at a UTF-8 continuation byte (0x80–0xBF) across two enqueued chunks and asserts exact reassembly with no U+FFFD. Verified failing on the pre-fix code and passing after.

Coverage Notes

  • Touches open-sse/utils/ollamaTransform.ts; the new test exercises the changed decode path (chunk-boundary reassembly) directly.

Reviewer Notes

  • Behaviour change is limited to byte decoding; SSE line framing, tool-call aggregation, and error/non-SSE passthrough are unchanged.

pacocartones added a commit to pacocartones/OmniRoute that referenced this pull request Aug 28, 2026
@pacocartones
pacocartones force-pushed the fix/ollama-multibyte-utf8-boundary branch from ef259df to 25d8074 Compare August 28, 2026 16:34
…unks

transformToOllama decoded each network chunk with a fresh, non-streaming
TextDecoder, so a multi-byte UTF-8 sequence (CJK, emoji) split across a chunk
boundary was corrupted into U+FFFD. Use one persistent decoder with
{ stream: true }, matching every other streaming transform in open-sse/.
@pacocartones
pacocartones force-pushed the fix/ollama-multibyte-utf8-boundary branch from 25d8074 to 0c2a36d Compare August 29, 2026 02:48
@pacocartones

Copy link
Copy Markdown
Contributor Author

CI note — none of the red checks are introduced by this change (an ollama stream-decoder fix touching open-sse/utils/ollamaTransform.ts + its test). Build was cancelled by the workflow's fail-fast, not a compile error (log: "The operation was canceled"). Vitest (MCP / autoCombo / UI components) fails on tests/unit/ui/use-provider-models-auto-fetch.test.tsx — a provider-models UI test this diff does not touch, and the job is green on main; it is flaky here (the run also logs ECONNREFUSED and a better-sqlite3 binding fallback). Protocol Clients E2E is advisory. Rebased onto current main (green) so the diff is minimal.

@diegosouzapw
diegosouzapw changed the base branch from main to release/v3.8.51 August 30, 2026 08:16
@diegosouzapw
diegosouzapw merged commit 41c6135 into diegosouzapw:release/v3.8.51 Aug 30, 2026
34 of 39 checks passed
muhamadgalihsaputra pushed a commit to niyatna/NiyatnaRoute that referenced this pull request Sep 27, 2026
…unks (diegosouzapw#11921)

Boarded with 8 other PRs in one combined worktree: typecheck:core, check:file-size, check:changelog-integrity, check:complexity, check:cognitive-complexity, check:cycles, check-native-deps all green. Retargeted from main to release/v3.8.51. Confirmed ollamaTransform.ts was the only streaming transform not using a persistent { stream: true } decoder — matches the pattern already established in responsesTransformer.ts. TDD repro included. Thanks for finding an unreported bug.
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