Skip to content

fix(models): avoid overflow when splitting large batches - #5015

Open
xiashao wants to merge 1 commit into
Tencent:mainfrom
xiashao:fix/chunkslice-large-batch
Open

xiashao wants to merge 1 commit into
Tencent:mainfrom
xiashao:fix/chunkslice-large-batch

Conversation

@xiashao

@xiashao xiashao commented Nov 2, 2026

Copy link
Copy Markdown
Contributor

Description

ChunkSlice can panic for a valid positive batch size close to math.MaxInt: the capacity expression adds the batch size to the slice length and overflows. This is reachable through BATCH_EMBED_SIZE; for three input texts, setting it to the platform's maximum integer panics before the embedding request is sent.

Compute the batch count without adding two potentially large integers, and bound each end index by the remaining input length before adding it to the current position. Ordinary chunk order, slice views, empty-input handling, and the existing non-positive-size panic are unchanged. This complements #4870, which addresses non-positive embedding settings.

Type of Change

  • 🐛 Bug fix
  • 🧪 Test

Related Issue

None; found while checking batch-size boundaries.

Testing

  • Before the fix, both the direct ChunkSlice regression and the real pooled-embedding path panic for a maximum positive batch size.
  • The new tests cover empty, normal, partial, oversized and near-maximum batch sizes, non-positive sizes, and a pooled embedding request with three texts.
  • Passed: go test ./internal/models/utils ./internal/models/embedding -count=1 -timeout=2m
  • Passed: go test -race ./internal/models/utils ./internal/models/embedding -count=1 -timeout=2m
  • Passed: go vet ./internal/models/utils ./internal/models/embedding
  • Passed: go build ./internal/models/utils ./internal/models/embedding ./internal/application/service/retriever
  • Passed: golangci-lint run --new-from-rev=origin/main --timeout=5m ./internal/models/utils ./internal/models/embedding (v2.12.2; 0 issues).
  • Full-repository checks and live embedding-provider tests were not run.

Checklist

  • git diff --check origin/main...HEAD passes
  • Changed source files are formatted
  • Targeted tests for changed packages/components pass
  • Diff-scoped lint passes
  • Self-reviewed the code
  • Added regression tests

Documentation and screenshots: not applicable. Diff: +71/-6 (77 changed lines including tests).

This branch has not been deployed

No deployments
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