Skip to content

fix(session-cost-usage): keep emoji / surrogate pairs intact during first user message truncation#101659

Closed
wings1029 wants to merge 1 commit into
openclaw:mainfrom
wings1029:fix/utf16-safe-discover-first-user-message
Closed

fix(session-cost-usage): keep emoji / surrogate pairs intact during first user message truncation#101659
wings1029 wants to merge 1 commit into
openclaw:mainfrom
wings1029:fix/utf16-safe-discover-first-user-message

Conversation

@wings1029

@wings1029 wings1029 commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

What Problem This Solves

discoverAllSessions in src/infra/session-cost-usage.ts truncates the first user message with .slice(0, 100), which counts UTF-16 code units. When an emoji (e.g. 🦞) straddles the cut boundary at position 100, .slice() produces a lone surrogate (e.g. \uD83E) that renders as � in session titles, session list API responses, and usage CLI output.

The file already imports truncateUtf16Safe (added by #101517 for loadSessionLogs), but the two .slice(0, 100) calls in discoverAllSessions were missed.

Why This Change Was Made

Replace .slice(0, 100) with truncateUtf16Safe(text, 100) so the truncation avoids splitting surrogate pairs at the boundary. This matches:

  • The same file loadSessionLogs already using truncateUtf16Safe (line 2756)
  • The 12 other UTF-16 safe truncation PRs merged in July 2026

User Impact

Session titles derived from the first user message (when no explicit label is set) will no longer contain � replacement characters when an emoji falls near the 100-char boundary.

Evidence

Real environment tested: local OpenClaw source checkout, Node v24.

Exact steps after this patch:

// Simulate: user first message = "a" + 50×🦞 = 101 code units (>100)
// .slice(0, 100) cuts between the surrogates of the last emoji
const msg = "a" + "🦞".repeat(50);
const before = msg.slice(0, 100);
const loneHighSurrogate = /[\uD800-\uDBFF](?![\uDC00-\uDFFF])/.test(before);
console.log("BEFORE: last code unit = U+" + before.charCodeAt(99).toString(16), "lone surrogate:", loneHighSurrogate);
// BEFORE: last code unit = U+d83e lone surrogate: true

Regression test: pnpm test -- src/infra/session-cost-usage.test.ts

 ✓ does not split surrogate pairs when truncating the first user message during discovery
 Test Files  1 passed (1)
      Tests  68 passed (68)

What was not tested: other truncation sites in the same file remain unchanged and are already covered by existing tests.

AI-assisted.

@wings1029 @claude
fix(session-cost-usage): keep emoji / surrogate pairs intact during f…
e17feaa 
…irst user message truncation

The discoverAllSessions helper in session-cost-usage.ts truncates the
first user message with .slice(0, 100), which counts UTF-16 code units.
When an emoji straddles the cut boundary, .slice() produces a lone
surrogate that renders as � in session titles and usage API output.

Replace .slice(0, 100) with the already-imported truncateUtf16Safe()
to avoid splitting surrogate pairs at the boundary.

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

clawsweeper Bot commented Jul 7, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Codex review: needs real behavior proof before merge. Reviewed July 7, 2026, 9:00 AM ET / 13:00 UTC.

Summary
The PR replaces UTF-16 code-unit slicing with surrogate-safe truncation for first user messages discovered from session transcripts and adds a regression test.

PR surface: Source 0, Tests +41. Total +41 across 2 files.

Reproducibility: yes. from source inspection: current main slices first user messages by UTF-16 code units at 100, and an emoji crossing that boundary leaves a dangling surrogate. I did not run a live reproduction in this read-only review, but the path and boundary condition are deterministic.

Review metrics: none identified.

Stored data model
Persistent data-model change detected: serialized state: src/infra/session-cost-usage.test.ts. Confirm migration or upgrade compatibility proof before merge.

Merge readiness
Overall: 🦪 silver shellfish
Proof: 🦪 silver shellfish
Patch quality: 🐚 platinum hermit
Result: blocked until real behavior proof from a real setup 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 output or logs from an after-fix real OpenClaw run that creates or reads a session with the boundary emoji and shows no replacement character.
  • After adding proof, update the PR body so ClawSweeper re-runs automatically, or ask a maintainer to comment @clawsweeper re-review if it does not.

Proof guidance:

  • [P1] Needs real behavior proof before merge: The PR body includes a source-level surrogate simulation and regression-test output, but no after-fix real OpenClaw session/list/usage run outside tests; add redacted terminal output or logs showing the changed behavior before merge. 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] The contributor proof is currently source simulation plus regression-test output, so it does not yet show the after-fix OpenClaw session/list/usage behavior outside tests.
  • [P1] GitHub reports the PR branch as behind the current base; exact-head checks should rerun after refresh or through the merge queue before landing.

Maintainer options:

  1. Decide the mitigation before merge
    Land the helper-based truncation at the two missed discovery sites after the contributor adds redacted real behavior proof and exact-head merge gates pass.
  2. Pause or close
    Do not merge this PR until maintainers decide whether the risk is worth taking.

Next step before merge

  • [P1] No automated code repair is needed; the next action is contributor-provided real behavior proof followed by normal exact-head maintainer review.

Security
Cleared: The diff only changes local string truncation and a colocated test, with no new dependency, workflow, credential, file-permission, or code-execution surface.

Review details

Best possible solution:

Land the helper-based truncation at the two missed discovery sites after the contributor adds redacted real behavior proof and exact-head merge gates pass.

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

Yes from source inspection: current main slices first user messages by UTF-16 code units at 100, and an emoji crossing that boundary leaves a dangling surrogate. I did not run a live reproduction in this read-only review, but the path and boundary condition are deterministic.

Is this the best way to solve the issue?

Yes. Using the existing shared UTF-16-safe truncation helper at the two missed discovery sites preserves the current code-unit cap and is narrower than adding a new spread/code-point truncation path.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 4e29b2f5ab4b.

Label changes

Label changes:

  • add P2: The PR addresses a narrow user-visible text corruption bug in session title and usage surfaces with limited blast radius.
  • add rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🦪 silver shellfish 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: The PR body includes a source-level surrogate simulation and regression-test output, but no after-fix real OpenClaw session/list/usage run outside tests; add redacted terminal output or logs showing the changed behavior before merge. 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:

  • P2: The PR addresses a narrow user-visible text corruption bug in session title and usage surfaces with limited blast radius.
  • rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🦪 silver shellfish 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: The PR body includes a source-level surrogate simulation and regression-test output, but no after-fix real OpenClaw session/list/usage run outside tests; add redacted terminal output or logs showing the changed behavior before merge. 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 0, Tests +41. Total +41 across 2 files.

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

What I checked:

Likely related people:

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: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. P2 Normal backlog priority with limited blast radius. labels Jul 7, 2026
@steipete steipete mentioned this pull request Jul 7, 2026
Merged
@steipete

steipete commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

Closed as superseded by #101685, landed in 176fee5d071dd03f6cbbc54b029f23380b04e911. The replacement carries this useful UTF-16 boundary fix on current main, uses the existing shared helper, expands caller-level coverage, and preserves your commit credit in the consolidated commit. Thank you, @wings1029, for finding and fixing this.

@steipete steipete closed this Jul 7, 2026
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

P2 Normal backlog priority with limited blast radius. rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. size: XS status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@wings1029 @steipete