Skip to content

Fix off-by-one in is_partial_stop causing false positives#3848

Open
Chessing234 wants to merge 1 commit intolm-sys:mainfrom
Chessing234:fix/is-partial-stop-off-by-one
Open

Fix off-by-one in is_partial_stop causing false positives#3848
Chessing234 wants to merge 1 commit intolm-sys:mainfrom
Chessing234:fix/is-partial-stop-off-by-one

Conversation

@Chessing234
Copy link
Copy Markdown

Summary

is_partial_stop checks whether the tail of the streaming output is a partial match for a stop string. The loop range range(0, min(len(output), len(stop_str))) has two bugs:

  1. i=0 on first iteration: output[-0:] in Python evaluates to output[0:] — the entire string, not an empty suffix. So stop_str.startswith(output) is tested, returning True whenever the full output happens to be a prefix of the stop string. This incorrectly suppresses streaming chunks.

  2. Exclusive upper bound: The largest valid suffix (of length min(len(output), len(stop_str))) is never checked.

Fix: Change range(0, ...) to range(1, ... + 1) so the loop checks suffixes of length 1 through min(len(output), len(stop_str)).

You can verify in a Python REPL:

>>> s = "hello"
>>> s[-0:]
'hello'  # entire string, not empty
>>> s[-1:]
'o'      # correct: last 1 char

Test plan

  • Verified output[-0:] returns full string in Python REPL
  • Existing tests in tests/test_utils.py should continue to pass

🤖 Generated with Claude Code

The range started at 0, so on the first iteration output[-0:]
evaluates to the entire string (Python treats -0 as 0). This made
stop_str.startswith(output) return True whenever the full output
happened to be a prefix of the stop string, incorrectly suppressing
streaming chunks. The exclusive upper bound also skipped the longest
valid suffix check.

Fix: start the range at 1 and add 1 to the upper bound.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant