fix(gateway): use code-point budget for fallback chunk split boundary - #62901
fix(gateway): use code-point budget for fallback chunk split boundary#62901sprmn24 wants to merge 1 commit into
Conversation
Duplicate of #42523 — same |
teknium1
left a comment
There was a problem hiding this comment.
Thanks for isolating the fallback boundary mismatch. The proposed _cp_budget slice is correct for _split_text_chunks(): _custom_unit_to_cp() explicitly returns the largest code-point offset fitting the custom length budget (gateway/platforms/base.py:167-183).
Problems
- The same unsafe fallback remains in the existing-message streaming overflow loop. It computes
_cp_budgetatgateway/stream_consumer.py:701-704, but falls back to_safe_limitas a Python slice offset atgateway/stream_consumer.py:705-707. - This PR adds no UTF-16 regression coverage. The existing fallback test (
tests/gateway/test_stream_consumer.py:778-805) uses a MagicMock/defaultlen()path and cannot assert the custom-unit chunk invariant.
Suggested changes
- Add a direct UTF-16 no-newline regression asserting every
_split_text_chunks()result satisfiesutf16_len(chunk) <= limitand round-trips the input. - Apply the same
_cp_budgetboundary correction to the sibling streaming-edit fallback atgateway/stream_consumer.py:705-706.
Automated hermes-sweeper review.
| if split_at < limit // 2: | ||
| split_at = limit | ||
| if split_at < _cp_budget // 2: | ||
| split_at = _cp_budget |
There was a problem hiding this comment.
Please add a direct UTF-16 regression for this branch: an emoji-only, no-newline input should produce only chunks whose utf16_len is within limit. The current fallback tests exercise default len() behavior and would not catch this unit/code-point mismatch.
435dfc6 to
c5c9383
Compare
fix(gateway): use _cp_budget boundary in UTF-16 chunk split fallback
c5c9383 to
7f9710e
Compare
What does this PR do?
_split_text_chunks()correctly calculates_cp_budget(code-point boundary) via_custom_unit_to_cp()for adapters using custom length functions (e.g. UTF-16 unit counters). However, the fallback path when no suitable newline is found was usinglimit(raw platform units) instead of_cp_budget(code points) as the split boundary.For adapters measuring UTF-16 units, emoji and other astral-plane characters count as 2 units each, so slicing at
limitinstead of_cp_budgetcan produce chunks that substantially exceedMAX_MESSAGE_LENGTHand fail delivery.This PR fixes both the split boundary and the too-short newline guard to consistently use
_cp_budget.Related Issue
None
Type of Change
Changes Made
gateway/stream_consumer.pyL977-978: changedsplit_at < limit // 2→split_at < _cp_budget // 2andsplit_at = limit→split_at = _cp_budgetin_split_text_chunks()How to Test
len_fn(chunk) <= limitChecklist
pytest tests/ -qpassedDocumentation & Housekeeping
Screenshots / Logs
N/A