Fix divide by zero error - #2226
Merged
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR hardens the tokenizer batch-encoding path used by the C/C++ API by adding input validation to prevent a divide-by-zero crash when EncodeBatch is called with an empty batch, and adds a regression test to ensure the error is surfaced as an exception.
Changes:
- Add an early
strings.empty()guard inTokenizer::EncodeBatch(std::span<const char*>)to prevent divide-by-zero when computing the output tensor shape. - Add a C API regression test that verifies
EncodeBatch(nullptr, 0)throws instead of crashing.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
src/models/model.cpp |
Adds empty-input validation to prevent a divide-by-zero when shaping the encoded batch tensor. |
test/c_api_tests.cpp |
Adds a regression test ensuring empty-batch encode throws rather than crashing. |
kunal-vaishnavi
approved these changes
Jun 12, 2026
kunal-vaishnavi
enabled auto-merge (squash)
June 13, 2026 23:07
kunal-vaishnavi
disabled auto-merge
June 18, 2026 21:26
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This pull request improves error handling for the
EncodeBatchfunction in the tokenizer by adding input validation and corresponding unit tests. The main goal is to prevent crashes when the function is called with empty input.Error handling improvements:
src/models/model.cpp: Added a check inTokenizer::EncodeBatchto throw astd::runtime_errorif the input string list is empty, preventing a crash when called with no input strings.Testing enhancements:
test/c_api_tests.cpp: Added a new test (EncodeBatchEmptyInputThrows) to verify that callingEncodeBatchwith zero input strings throws astd::runtime_errorinstead of causing a crash.