[glanikali]

Summary

• Update estimate_tokens to count Unicode characters instead of UTF-8 bytes, matching the
  documented ceil(chars / 4) heuristic.
• Add regression coverage for non-ASCII input. Fixes estimate_tokens docstring says chars/4 but code divides byte length #3359.

Test plan

• cargo fmt --all && cargo clippy --all-targets && cargo test
• Manual testing: rtk proxy printf '你好\n' output inspected with an isolated tracking
  database; input and output were each estimated as one token.

Important: All PRs must target the develop branch (not master).
See CONTRIBUTING.md for details.

[CLAassistant]

All committers have signed the CLA.

[EvolveAegis]

Confirmed independently by running the real estimate_tokens (not a mock) on six strings spanning CJK, emoji, and mixed. For every non-ASCII case in that corpus, rtk_est == ceil(bytes/4) and rtk_est != ceil(chars/4), confirming the unit is bytes, not chars, contradicting the docstring's tokens = ceil(chars / 4). This PR's text.chars().count().div_ceil(4) is the correction our PoC pointed to; the added "你好" test is a genuine regression witness (old code returns 2, new code returns 1). Suggestion: our PoC diverged across four categories (12-char CJK, 32-char CJK, 5 emoji, ASCII-CJK-emoji mix); adding an emoji and a mixed case would widen the net beyond a single CJK pair. Heads-up: even corrected, ceil(chars/4) still underestimates real CJK tokens vs tiktoken (3 vs 15 for 12 chars); that is the future-tokenizer issue, out of scope here.

[glanikali]

Confirmed independently by running the real estimate_tokens (not a mock) on six strings spanning CJK, emoji, and mixed. For every non-ASCII case in that corpus, rtk_est == ceil(bytes/4) and rtk_est != ceil(chars/4), confirming the unit is bytes, not chars, contradicting the docstring's tokens = ceil(chars / 4). This PR's text.chars().count().div_ceil(4) is the correction our PoC pointed to; the added "你好" test is a genuine regression witness (old code returns 2, new code returns 1). Suggestion: our PoC diverged across four categories (12-char CJK, 32-char CJK, 5 emoji, ASCII-CJK-emoji mix); adding an emoji and a mixed case would widen the net beyond a single CJK pair. Heads-up: even corrected, ceil(chars/4) still underestimates real CJK tokens vs tiktoken (3 vs 15 for 12 chars); that is the future-tokenizer issue, out of scope here.

Thanks for independently validating this. I’ve added regression cases covering five emoji and a mixed ASCII/CJK/emoji string. Both distinguish character counting from the previous UTF-8 byte counting. Agreed that matching model-specific tokenizers is separate future work and out of scope for this fix.

[rtk-release-bot]

This pull request has been automatically marked as stale due to 90 days of inactivity. If this pull request is still relevant, please leave any comment (for example, "bump"), and we'll keep it open. Your contribution is very much appreciated — we're sorry we haven't been able to review it yet.

[rtk-release-bot]

Closing this pull request after a prolonged period of inactivity. If this is still relevant, please ask for it to be reopened. Thank you!