Skip to content

fix(context): count fullwidth chars in token estimates#96442

Merged
vincentkoc merged 1 commit into
openclaw:mainfrom
lin-hongkuan:codex/cjk-chars-fullwidth-token-estimate
Jun 25, 2026
Merged

fix(context): count fullwidth chars in token estimates#96442
vincentkoc merged 1 commit into
openclaw:mainfrom
lin-hongkuan:codex/cjk-chars-fullwidth-token-estimate

Conversation

@lin-hongkuan

@lin-hongkuan lin-hongkuan commented Jun 24, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

What Problem This Solves

estimateStringChars() inflates CJK/Hangul/Japanese text before downstream token estimates divide by CHARS_PER_TOKEN_ESTIMATE, but it did not include East Asian fullwidth forms. Fullwidth text such as ABC123 was counted like ordinary ASCII, so budget and pruning estimates undercounted that content.

Why This Change Was Made

The estimator now includes the Unicode fullwidth ranges U+FF01-U+FF60 and U+FFE0-U+FFE6 in the same weighted path used for other East Asian scripts. Ordinary ASCII remains unchanged.

Focused tests cover fullwidth letters, numbers, and punctuation.

User Impact

Context budgeting and token estimate surfaces are less likely to undercount fullwidth CJK-style text, reducing the chance of overlarge prompts slipping past preflight estimates.

Evidence

Direct behavior probe:

$ node --import tsx -e "import('./src/utils/cjk-chars.ts').then(({ estimateStringChars }) => console.log(JSON.stringify(['ABC123', 'ABC123', 'hello,world'].map((text) => ({ text, chars: text.length, estimated: estimateStringChars(text) })))))"
[{"text":"ABC123","chars":6,"estimated":6},{"text":"ABC123","chars":6,"estimated":24},{"text":"hello,world","chars":11,"estimated":14}]

Targeted test:

$ node scripts/run-vitest.mjs run src/utils/cjk-chars.test.ts

 RUN  v4.1.8 C:/Users/lin20/Documents/New project 3/openclaw


 Test Files  1 passed (1)
      Tests  18 passed (18)
   Start at  21:24:39
   Duration  274ms (transform 32ms, setup 0ms, import 54ms, tests 16ms, environment 0ms)

[test] starting test/vitest/vitest.unit-fast.config.ts
[test] passed 1 Vitest shard in 6.14s

Formatting:

$ node_modules\.bin\oxfmt.cmd --check src/utils/cjk-chars.ts src/utils/cjk-chars.test.ts
Checking formatting...

All matched files use the correct format.
Finished in 1188ms on 2 files using 32 threads.

Whitespace:

$ git diff --check

AI-assisted

Prepared with Codex. I reviewed the change, understand the touched code path, and kept the PR focused on the bug described above.

@lin-hongkuan

lin-hongkuan commented Jun 24, 2026

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented Jun 24, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

@clawsweeper

clawsweeper Bot commented Jun 24, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Codex review: needs maintainer review before merge. Reviewed June 25, 2026, 2:32 AM ET / 06:32 UTC.

Summary
The PR adds East Asian fullwidth Unicode ranges to estimateStringChars() and adds unit coverage for fullwidth letters, numbers, and punctuation.

PR surface: Source +1, Tests +7. Total +8 across 2 files.

Reproducibility: yes. Source inspection of current main shows the shared regex omits the fullwidth Unicode ranges, so fullwidth forms are counted like ordinary text before downstream token division.

Review metrics: none identified.

Merge readiness
Overall: 🦞 diamond lobster
Proof: 🦞 diamond lobster
Patch quality: 🦞 diamond lobster
Result: ready for maintainer review.

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

Next step before merge

  • No automated repair lane is needed because the branch is narrow, proof-supplied, and no actionable defect was found; remaining action is ordinary maintainer review and merge gating.

Security
Cleared: The diff only changes a TypeScript regex and unit tests; it does not touch dependencies, secrets, CI, packaging, permissions, or code execution surfaces.

Review details

Best possible solution:

Merge this focused shared-estimator update after normal maintainer review and required CI completion, keeping caller-specific budgeting logic unchanged.

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

Yes. Source inspection of current main shows the shared regex omits the fullwidth Unicode ranges, so fullwidth forms are counted like ordinary text before downstream token division.

Is this the best way to solve the issue?

Yes. Updating the shared estimateStringChars() classifier is the narrow maintainable fix because multiple context-budgeting callers already depend on that helper.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 56d95b18f4bc.

Label changes

Label changes:

  • add rating: 🦞 diamond lobster: Overall readiness is 🦞 diamond lobster; proof is 🦞 diamond lobster and patch quality is 🦞 diamond lobster.
  • remove rating: 🐚 platinum hermit: Current PR rating is rating: 🦞 diamond lobster, so this older rating label is no longer current.

Label justifications:

  • P2: This fixes a real context-budget estimation bug with limited blast radius and no evidence of an urgent active outage.
  • rating: 🦞 diamond lobster: Overall readiness is 🦞 diamond lobster; proof is 🦞 diamond lobster and patch quality is 🦞 diamond lobster.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (terminal): The PR body includes after-fix terminal output for the estimator plus targeted unit-test output, and the live Real behavior proof check passed for the current head.
  • proof: sufficient: Contributor real behavior proof is sufficient. The PR body includes after-fix terminal output for the estimator plus targeted unit-test output, and the live Real behavior proof check passed for the current head.
Evidence reviewed

PR surface:

Source +1, Tests +7. Total +8 across 2 files.

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

What I checked:

  • Current-main behavior: Current main's NON_LATIN_RE covers CJK, Hangul, Hiragana, Katakana, compatibility ideographs, and Extension B ranges, but not U+FF01-U+FF60 or U+FFE0-U+FFE6; fullwidth ASCII-style forms therefore follow ordinary string-length weighting. (src/utils/cjk-chars.ts:25, 56d95b18f4bc)
  • PR implementation: The PR head adds \uFF01-\uFF60 and \uFFE0-\uFFE6 to the existing shared estimator regex without adding a parallel estimator or caller-local policy. (src/utils/cjk-chars.ts:25, cb7849795b67)
  • Focused test coverage: The patch adds direct assertions for fullwidth letters, numbers, and punctuation while keeping ordinary ASCII behavior covered by existing tests. (src/utils/cjk-chars.test.ts:49, cb7849795b67)
  • Shared caller surface: estimateStringChars() is consumed by gateway transcript usage estimates, context pruning, preemptive compaction, memory chunking, and OpenAI-compatible input budgeting, so the shared helper is the right owner boundary for this correction. (src/agents/embedded-agent-runner/run/preemptive-compaction.ts:46, 56d95b18f4bc)
  • Sibling range precedent: The UI markdown URL-boundary classifier already treats U+FF01-U+FF60 as part of its CJK/fullwidth boundary detection, supporting the same fullwidth-family treatment in the estimator. (ui/src/ui/markdown.ts:338, 56d95b18f4bc)
  • Real behavior proof: The PR body includes after-fix terminal output for the estimator and targeted test output; live checks also show the Real behavior proof check passed for the current PR head. (cb7849795b67)

Likely related people:

  • AaronLuo00: Merged PR history shows this author added src/utils/cjk-chars.ts, its tests, and memory chunking use of the CJK-aware estimator. (role: introduced shared estimator; confidence: high; commits: 8cb4a020f81c, 983016ae5b9c, 60926f76bbb3; files: src/utils/cjk-chars.ts, src/utils/cjk-chars.test.ts, packages/memory-host-sdk/src/host/internal.ts)
  • Takhoffman: Merged PR history for the CJK context-pruning fix shows this user moved the pruner toward the shared estimator path that this PR now extends. (role: adjacent context-pruning owner; confidence: medium; commits: cf9a2cf9aa7d, 637137d5bf12; files: src/agents/agent-hooks/context-pruning/pruner.ts, src/utils/cjk-chars.ts)
  • steipete: Live PR metadata shows this user merged the original shared-estimator PR and local history shows adjacent OpenAI-compatible budgeting work was committed by this user. (role: merger and adjacent area contributor; confidence: medium; commits: 390241ed34c4, c61ecdabdaaf; files: src/utils/cjk-chars.ts, src/agents/openai-transport-stream.ts)
  • rendrag-git: Git history shows this user authored recent OpenAI-compatible context-window budgeting code that consumes estimateStringChars() for non-Latin prompt pressure. (role: recent adjacent contributor; confidence: medium; commits: c61ecdabdaaf; files: src/agents/openai-transport-stream.ts, src/agents/openai-transport-stream.test.ts)
  • Shakker: Current-main blame points the present checked-out estimator and several caller files to a recent repository rewrite/import commit, though earlier PR history is a stronger feature-ownership signal. (role: current-line provenance; confidence: low; commits: a49816ffbb52; files: src/utils/cjk-chars.ts, src/utils/cjk-chars.test.ts, src/agents/openai-transport-stream.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 proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. P2 Normal backlog priority with limited blast radius. labels Jun 24, 2026
@vincentkoc
vincentkoc force-pushed the codex/cjk-chars-fullwidth-token-estimate branch from 188a0c4 to cb78497 Compare June 25, 2026 06:26
@clawsweeper clawsweeper Bot added rating: 🦞 diamond lobster Very strong PR readiness with only minor maintainer review expected. and removed rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. labels Jun 25, 2026
@vincentkoc
vincentkoc merged commit 31e941c into openclaw:main Jun 25, 2026
105 of 107 checks passed
@vincentkoc

vincentkoc commented Jun 25, 2026

Copy link
Copy Markdown
Member

Merged via squash.

github-actions Bot pushed a commit to Desicool/openclaw that referenced this pull request Jun 26, 2026
@lin-hongkuan
fix(context): count fullwidth chars in token estimates (openclaw#96442)
cf78f7e
QiuYuang pushed a commit to QiuYuang/openclaw that referenced this pull request Jul 1, 2026
@lin-hongkuan @QiuYuang
fix(context): count fullwidth chars in token estimates (openclaw#96442)
1a86bcc
@revision-co-ltd revision-co-ltd mentioned this pull request Jul 10, 2026
Open
This was referenced Jul 13, 2026
Closed
Open
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. proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🦞 diamond lobster Very strong PR readiness with only minor maintainer review expected. size: XS status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@lin-hongkuan @vincentkoc