Skip to content

Python: Fix text chunker merging the last paragraph past max_tokens - #14567

Closed
Franco Ceballos Rastello (xThreeh) wants to merge 1 commit into
microsoft:mainfrom
xThreeh:fix/text-chunker-max-tokens
Closed

Franco Ceballos Rastello (xThreeh) wants to merge 1 commit into
microsoft:mainfrom
xThreeh:fix/text-chunker-max-tokens

Conversation

@xThreeh

Copy link
Copy Markdown

Motivation and Context

Fixes #14566

split_plaintext_paragraph and split_markdown_paragraph can return a paragraph above max_tokens. When the last paragraph is short, _split_text_paragraph merges it into the previous one, but the merge check counts words (split(" ")) instead of using token_counter. A word is usually more than one token, so the merged paragraph can go over the limit. With tiktoken as the counter I saw chunks of up to 314 tokens for max_tokens=256.

Description

  • Build the merged paragraph once and merge only if token_counter(merged) <= max_tokens. The merged text is the same as before; only the condition changes.
  • Add test_split_text_paragraph_evenly_respects_max_tokens. It fails on main (last chunk 16 tokens, limit 15) and passes with this change.
  • Update eight existing tests whose expected last chunk was above max_token_per_line (16 for 15, 18 for 17). Everything before the last chunk is unchanged; the last chunk is now split in two.

pytest tests/unit/text tests/unit/core_plugins passes, mypy is clean on text_chunker.py, and the repo's pre-commit hooks pass on the changed files.

Contribution Checklist

_split_text_paragraph merges a short last paragraph into the previous
one, but the merge check counted words (split(" ")) instead of calling
token_counter. A word is usually more than one token, so the merged
paragraph could exceed max_tokens.

Measure the merged text with token_counter and only merge when it fits.
The merged text itself is unchanged.

Eight existing tests expected a last chunk above max_token_per_line
(16 tokens with a limit of 15, 18 with 17); their last chunk is now
split instead. A new test checks that no paragraph exceeds the limit.

Fixes microsoft#14566
Copilot AI balanced review requested due to automatic review settings October 7, 2026 19:09

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@semantic-kernel-automation semantic-kernel-automation Bot added the python Pull requests for the Python Semantic Kernel label Oct 7, 2026
@xThreeh

Copy link
Copy Markdown
Author

Closing: #14238 already fixes the same merge check in _split_text_paragraph and was opened first. Sorry for the duplicate.

This branch was previously deployed

1 inactive deployment
github-app-auth — cb3b74b0 Deployed Oct 7, 2026 by xThreeh via team_check #600
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

python Pull requests for the Python Semantic Kernel

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Python: Bug: split_plaintext_paragraph / split_markdown_paragraph can return a chunk larger than max_tokens

2 participants