Skip to content

fix(compaction): use truncateUtf16Safe for post-compaction context text#102515

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

fix(compaction): use truncateUtf16Safe for post-compaction context text#102515
steipete merged 2 commits into
openclaw:mainfrom
lsr911:fix/utf16-compaction

Conversation

@lsr911

@lsr911 lsr911 commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

What Problem This Solves

The post-compaction context builder uses naive .slice(0, maxContextChars) to truncate context summaries sent to the model. This can split surrogate pairs in compaction context text.

Why This Change Was Made

Replace with truncateUtf16Safe(). Same pattern as #102464, #102467, #102500 (all merged).

Files Changed

File Change
src/auto-reply/reply/post-compaction-context.ts +1 import; .slice(0,N) → truncateUtf16Safe()

🤖 Generated with Claude Code

Replace naive .slice(0, maxContextChars) with truncateUtf16Safe()
to prevent surrogate pair splitting in post-compaction context
summaries sent to the model.

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:30 AM ET / 08:30 UTC.

Summary
The branch replaces post-compaction context .slice(0, maxContextChars) truncation with truncateUtf16Safe and adds a regression test for an emoji boundary case.

PR surface: Source +1, Tests +16. Total +17 across 2 files.

Reproducibility: yes. by source inspection: current main builds post-compaction context from AGENTS.md sections and truncates it with combined.slice(0, maxContextChars), which can return a dangling surrogate half when the limit falls inside a pair. I did not run tests because this review is read-only and must not create checkout artifacts.

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 real behavior proof for the post-compaction context path, such as terminal output from the focused test or a validation transcript showing the emoji boundary case after the fix.
  • Update the PR body after proof is added so ClawSweeper can re-review the current head.

Proof guidance:

  • [P1] Needs real behavior proof before merge: No after-fix real behavior proof is present; add redacted terminal output, copied live output, logs, or a validation transcript, then update the PR body to trigger re-review or ask a maintainer to comment @clawsweeper re-review. Redact private information such as IP addresses, API keys, phone numbers, non-public endpoints, and other private details.

Risk before merge

  • [P1] Contributor real-behavior proof is still absent, so merge should wait for redacted terminal output, copied live output, logs, or a validation transcript showing the after-fix post-compaction context truncation behavior.

Maintainer options:

  1. Decide the mitigation before merge
    Merge the narrow helper replacement once after-fix real behavior proof or a maintainer proof override confirms emoji-containing post-compaction context truncates without a dangling surrogate.
  2. Pause or close
    Do not merge this PR until maintainers decide whether the risk is worth taking.

Next step before merge

  • [P1] There is no narrow code defect for automation to repair; the next action is contributor-provided real behavior proof followed by normal exact-head maintainer review.

Security
Cleared: The diff only imports and calls an existing internal string truncation helper and adds a focused unit test; no security or supply-chain concern was found.

Review details

Best possible solution:

Merge the narrow helper replacement once after-fix real behavior proof or a maintainer proof override confirms emoji-containing post-compaction context truncates without a dangling surrogate.

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

Yes by source inspection: current main builds post-compaction context from AGENTS.md sections and truncates it with combined.slice(0, maxContextChars), which can return a dangling surrogate half when the limit falls inside a pair. I did not run tests because this review is read-only and must not create checkout artifacts.

Is this the best way to solve the issue?

Yes for the code shape: reusing the existing package-exported truncateUtf16Safe helper at the owner-local truncation site is the narrow maintainable fix. Merge readiness still depends on real behavior proof or maintainer proof override.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 5c3c5c0ee49a.

Label changes

Label justifications:

  • P3: This is a small text-truncation correctness fix with limited blast radius and no config, protocol, data, availability, or security surface 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; add redacted terminal output, copied live output, logs, or a validation transcript, then update the PR body to trigger re-review or ask a maintainer to comment @clawsweeper re-review. Redact private information such as IP addresses, API keys, phone numbers, non-public endpoints, and other private details.
Evidence reviewed

PR surface:

Source +1, Tests +16. Total +17 across 2 files.

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

What I checked:

  • Repository policy read: Root AGENTS.md was read fully and applied; no scoped AGENTS.md exists under src/auto-reply/reply, so the root deep-review and PR proof rules govern this item. (AGENTS.md:25)
  • Current main still truncates unsafely: At current main, readPostCompactionContext joins extracted AGENTS.md sections, replaces date placeholders, and uses combined.slice(0, maxContextChars) before appending the truncation marker. (src/auto-reply/reply/post-compaction-context.ts:120, 5c3c5c0ee49a)
  • Runtime caller path checked: The auto-reply runner calls readPostCompactionContext after auto-compaction and enqueues returned text as a system event for the next turn; the memory runner also appends the same refresh prompt into extraSystemPrompt. (src/auto-reply/reply/agent-runner.ts:2393, 5c3c5c0ee49a)
  • Helper contract supports the patch: truncateUtf16Safe floors the requested limit and delegates to sliceUtf16Safe, whose end-boundary check backs off when truncation would leave a dangling high surrogate. (packages/normalization-core/src/utf16-slice.ts:44, 5c3c5c0ee49a)
  • Import path is exported: @openclaw/normalization-core/utf16-slice is an explicit package export, so the PR’s direct helper import matches the package contract. (packages/normalization-core/package.json:47, 5c3c5c0ee49a)
  • Adjacent helper tests exist: The shared UTF-16 helper tests already cover truncating at a surrogate-pair boundary and expect the safe truncation result to avoid returning a dangling half. (packages/normalization-core/src/utf16-slice.test.ts:84, 5c3c5c0ee49a)

Likely related people:

  • vincentkoc: Current blame and commit history show bf3e2f5b80881b713a664f105cf8ce92c359e920 added the post-compaction context module and the raw truncation block. (role: introduced behavior; confidence: high; commits: bf3e2f5b8088; files: src/auto-reply/reply/post-compaction-context.ts)
  • wings1029: Recent current-main history shows this contributor made the analogous startup-context truncation path in src/auto-reply/reply UTF-16 safe. (role: adjacent area contributor; confidence: medium; commits: f0cc5e500f66; files: src/auto-reply/reply/startup-context.ts, src/auto-reply/reply/startup-context.test.ts)
  • lsr911: Although also the PR author, this contributor authored merged current-main UTF-16 truncation fixes on adjacent agent surfaces, so they are connected to the active truncation cleanup pattern beyond this single proposal. (role: recent adjacent contributor; confidence: medium; commits: 472e5167d38a, 4fa3118049c1; files: src/agents/tool-policy-audit.ts, src/agents/harness/native-hook-relay.ts)
  • steipete: The latest PR head adds the focused post-compaction UTF-16 boundary test under this author, and related UTF-16 truncation PRs were recently merged after maintainer proof review. (role: recent reviewer and test author; confidence: medium; commits: 7ac3e6ef5d28, 472e5167d38a, 4fa3118049c1; files: src/auto-reply/reply/post-compaction-context.test.ts, src/agents/tool-policy-audit.ts, src/agents/harness/native-hook-relay.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.
Review history (1 earlier review cycle)
  • reviewed 2026-07-09T07:48:39.579Z sha 3e0b4dd :: needs real behavior proof before merge. :: none

@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
@steipete

steipete commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Land-ready maintainer verification at exact head 7ac3e6ef5d28d878e22a8716878809bc20c85e5c:

  • Post-compaction context keeps the existing aggregation, date replacement, configured cap, and truncation marker.
  • The final prefix selection now preserves whole UTF-16 surrogate pairs.
  • Focused post-compaction context suite: 27 passed.
  • Fresh autoreview: clean (0.99).
  • Exact-head hosted CI/Testbox gates: passed.

The regression reads a real temporary AGENTS.md through readDefaultPostCompactionContext with a 180-code-unit limit. No config/default/docs surface changed.

@steipete
steipete merged commit 49a5b6a into openclaw:main Jul 9, 2026
98 of 99 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
…xt (openclaw#102515)

* fix(compaction): use truncateUtf16Safe for post-compaction context text

Replace naive .slice(0, maxContextChars) with truncateUtf16Safe()
to prevent surrogate pair splitting in post-compaction context
summaries sent to the model.

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

* test(compaction): prove context UTF-16 boundary

---------

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. triage: needs-pr-context Candidate: external PR body lacks required problem context or evidence.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants