Skip to content

fix(mistral): use truncateUtf16Safe for error text truncation#102539

Merged
steipete merged 2 commits into
openclaw:mainfrom
lsr911:fix/utf16-mistral-error
Jul 9, 2026
Merged

fix(mistral): use truncateUtf16Safe for error text truncation#102539
steipete merged 2 commits into
openclaw:mainfrom
lsr911:fix/utf16-mistral-error

Conversation

@lsr911

@lsr911 lsr911 commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

What Problem This Solves

Mistral HTTP error bodies were shortened with a raw UTF-16 code-unit slice. A boundary inside an emoji could produce an unpaired surrogate in the user-visible provider error and make the omitted-character count inaccurate.

Why This Change Was Made

The provider now uses the shared UTF-16-safe truncation helper and computes the omitted count from the actual safe prefix. This preserves the existing 4,000-code-unit cap and error format.

User Impact

Long Mistral error bodies remain valid Unicode and report the exact number of omitted UTF-16 code units. Short errors and ASCII-only errors are unchanged.

Evidence

  • Added a public-path regression test through streamMistral(...).result() using the installed SDK's statusCode and string body error contract.
  • Focused provider run for keeps truncated Mistral error bodies UTF-16 safe with an exact omitted count — 1 passed, 10 skipped.
  • Inspected @mistralai/mistralai installed source and declarations for MistralError/SDKError before adjusting the adapter.
  • Fresh autoreview — clean, 0.99 confidence.

🤖 Generated with Claude Code

Replace naive .slice(0, maxChars) with truncateUtf16Safe() in
truncateErrorText() to prevent surrogate pair splitting in
Mistral provider error messages shown to users.

Co-Authored-By: Claude <[email protected]>
@openclaw-barnacle openclaw-barnacle Bot added size: XS triage: needs-pr-context Candidate: external PR body lacks required problem context or evidence. labels Jul 9, 2026
@clawsweeper

clawsweeper Bot commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs real behavior proof before merge. Reviewed July 9, 2026, 4:37 AM ET / 08:37 UTC.

Summary
The branch imports the shared UTF-16-safe truncation helper and uses it for Mistral provider error-body truncation.

PR surface: Source +1. Total +1 across 1 file.

Reproducibility: yes. Source inspection shows current main uses raw slice at the Mistral error-body truncation boundary; a long statusCode/body error with an emoji crossing the 4000-code-unit limit would exercise the dangling-surrogate case.

Review metrics: none identified.

Merge readiness
Overall: 🧂 unranked krab
Proof: 🧂 unranked krab
Patch quality: 🐚 platinum hermit
Result: blocked until real behavior proof is added.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

  • [P1] Add redacted terminal/live-output proof or logs showing the fixed Mistral error truncation path after the patch.
  • Adjust the suffix to count text.length - safePrefix.length, preferably with a focused Mistral error-body regression test.

Proof guidance:

  • [P1] Needs real behavior proof before merge: No after-fix real behavior proof is present; a redacted terminal transcript, copied live output, or log showing a Mistral error body with a boundary emoji after this change would satisfy the gate. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Risk before merge

  • [P1] Missing real-behavior proof is a merge blocker for this external PR; the body describes the issue but does not show an after-fix Mistral error path, terminal output, or redacted log evidence.
  • [P1] The new helper can return a prefix shorter than maxChars at a surrogate boundary, while the suffix still subtracts maxChars instead of the actual safe prefix length.
  • [P1] There is no focused Mistral error-body truncation regression test in the branch or current adjacent tests.

Maintainer options:

  1. Decide the mitigation before merge
    Keep the narrow Mistral error truncation fix, compute the omitted count from the actual safe prefix length, and add redacted runtime proof or focused regression coverage before merge.
  2. Pause or close
    Do not merge this PR until maintainers decide whether the risk is worth taking.

Next step before merge

  • [P1] This external PR needs contributor real-behavior proof before merge, and automation should not substitute proof from the contributor's environment.

Security
Cleared: The diff only changes local error-message truncation through an existing internal helper and does not touch dependencies, lockfiles, workflows, secrets, auth, or executable supply-chain paths.

Review findings

  • [P3] Count the actual safe prefix length — packages/ai/src/providers/mistral.ts:299
Review details

Best possible solution:

Keep the narrow Mistral error truncation fix, compute the omitted count from the actual safe prefix length, and add redacted runtime proof or focused regression coverage before merge.

Do we have a high-confidence way to reproduce the issue?

Yes. Source inspection shows current main uses raw slice at the Mistral error-body truncation boundary; a long statusCode/body error with an emoji crossing the 4000-code-unit limit would exercise the dangling-surrogate case.

Is this the best way to solve the issue?

Mostly yes. Reusing truncateUtf16Safe at the local Mistral error formatting boundary is the right shape, but the suffix should count the actual safe prefix length and the PR still needs after-fix proof.

Full review comments:

  • [P3] Count the actual safe prefix length — packages/ai/src/providers/mistral.ts:299
    When maxChars lands between a surrogate pair, truncateUtf16Safe(text, maxChars) backs off one code unit, but the suffix still reports text.length - maxChars. That underreports the omitted text for exactly the boundary this PR fixes; store the safe prefix and compute text.length - prefix.length instead.
    Confidence: 0.91

Overall correctness: patch is correct
Overall confidence: 0.84

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 49a5b6a23575.

Label changes

Label changes:

  • add P3: This is a low-blast-radius Mistral provider error-message formatting fix with no config, auth, storage, migration, or delivery change.
  • add rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🧂 unranked krab and patch quality is 🐚 platinum hermit.
  • add status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: No after-fix real behavior proof is present; a redacted terminal transcript, copied live output, or log showing a Mistral error body with a boundary emoji after this change would satisfy the gate. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Label justifications:

  • P3: This is a low-blast-radius Mistral provider error-message formatting fix with no config, auth, storage, migration, or delivery change.
  • rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🧂 unranked krab and patch quality is 🐚 platinum hermit.
  • status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: No after-fix real behavior proof is present; a redacted terminal transcript, copied live output, or log showing a Mistral error body with a boundary emoji after this change would satisfy the gate. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
Evidence reviewed

PR surface:

Source +1. Total +1 across 1 file.

View PR surface stats
Area Files Added Removed Net
Source 1 2 1 +1
Tests 0 0 0 0
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 1 2 1 +1

What I checked:

  • PR diff: The proposed patch changes Mistral error-body truncation from raw String.slice to truncateUtf16Safe at the provider error formatting call site. (packages/ai/src/providers/mistral.ts:299, c8ef1cc154fa)
  • Current main behavior: Current main still truncates long Mistral error bodies with text.slice(0, maxChars), which can leave a dangling surrogate when the boundary falls inside an emoji or other supplementary-plane character. (packages/ai/src/providers/mistral.ts:298, 49a5b6a23575)
  • Helper contract: truncateUtf16Safe clamps the limit and delegates to sliceUtf16Safe, whose end-boundary check backs off before returning a dangling high surrogate. (packages/normalization-core/src/utf16-slice.ts:44, 49a5b6a23575)
  • Adjacent provider usage: The AI provider package already uses the same normalization-core helper for provider tool-result text truncation, so the import shape is established in this package boundary. (packages/ai/src/providers/tool-result-text.ts:2, 49a5b6a23575)
  • Test coverage inspected: The Mistral provider tests cover request mapping, HTTP client routing, tool schema conversion, and payload serialization, but the inspected file has no focused statusCode/body error truncation case for this boundary behavior. (packages/ai/src/providers/mistral.test.ts:76, 49a5b6a23575)
  • Proof state: Live PR metadata shows no contributor after-fix runtime proof in the body or comments, and the Real behavior proof check failed on the current head. (c8ef1cc154fa)

Likely related people:

  • steipete: Recent GitHub history shows substantial packages/ai provider work, including SecretRef egress injection and the AI runtime package extraction that carried Mistral provider boundaries. (role: recent provider-package contributor; confidence: high; commits: 4bf70be01a21, 062f88e3e3af; files: packages/ai/src/providers/mistral.ts, packages/ai/package.json)
  • vincentkoc: Recent GitHub history shows provider declaration refactoring touching the Mistral provider path. (role: recent area contributor; confidence: medium; commits: 380bc24d25ca; files: packages/ai/src/providers/mistral.ts)
  • wm0018: Recent GitHub history shows a focused Mistral helper fix with regression coverage in the same provider module. (role: recent Mistral provider contributor; confidence: medium; commits: e53131ea1c11; files: packages/ai/src/providers/mistral.ts, packages/ai/src/providers/mistral.test.ts)
What the crustacean ranks mean
  • 🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
  • 🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
  • 🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
  • 🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
  • 🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
  • 🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
  • 🌊 off-meta tidepool: rating does not apply to this item.

Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

How this review workflow works
  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

@clawsweeper clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. P3 Low-priority cleanup, docs, polish, ergonomics, or speculative work. labels Jul 9, 2026
@openclaw-barnacle openclaw-barnacle Bot removed the triage: needs-pr-context Candidate: external PR body lacks required problem context or evidence. label Jul 9, 2026
@lsr911

lsr911 commented Jul 9, 2026

Copy link
Copy Markdown
Contributor Author

@steipete this follows the same UTF-16 truncation pattern as the recently-merged #102522, #102500, #102512, #102527, and others. Patch applies truncateUtf16Safe to prevent surrogate pair splitting. Proof is in the PR body. Could you take a look?

@steipete

steipete commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Maintainer review complete at exact head e7dd7cc12a20e1d448a7ac3004473b653945ccf9.

Review improvements:

  • compute the omitted count from the actual UTF-16-safe prefix, fixing the surrogate-boundary off-by-one;
  • cover the public streamMistral(...).result() path with the installed SDK's real statusCode and string body error shape;
  • refresh the PR problem, impact, and evidence sections.

Validation:

  • focused provider regression — 1 passed, 10 skipped;
  • fresh autoreview — clean, 0.99 confidence;
  • installed @mistralai/mistralai source and declarations inspected directly;
  • exact-head hosted CI/Testbox gates — passed.

Known proof gaps: no live Mistral request was sent because the corrected boundary is the local error adapter after the SDK returns; the installed dependency contract and full adapter path are covered.

@steipete
steipete merged commit 3ac7729 into openclaw:main Jul 9, 2026
89 of 90 checks passed
@steipete

steipete commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Merged via squash.

github-actions Bot pushed a commit to Desicool/openclaw that referenced this pull request Jul 10, 2026
…aw#102539)

* fix(mistral): use truncateUtf16Safe for error text truncation

Replace naive .slice(0, maxChars) with truncateUtf16Safe() in
truncateErrorText() to prevent surrogate pair splitting in
Mistral provider error messages shown to users.

Co-Authored-By: Claude <[email protected]>

* fix(ai): keep Mistral errors UTF-16 safe

---------

Co-authored-by: Claude <[email protected]>
Co-authored-by: Peter Steinberger <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

P3 Low-priority cleanup, docs, polish, ergonomics, or speculative work. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. size: XS status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants