Skip to content

fix(embedding): reject non-positive batch sizes before pooling - #4870

Merged
lyingbug merged 1 commit into
Tencent:mainfrom
xiashao:fix/embedding-batch-size
Nov 2, 2026
Merged

lyingbug merged 1 commit into
Tencent:mainfrom
xiashao:fix/embedding-batch-size

Conversation

@xiashao

@xiashao xiashao commented Oct 29, 2026

Copy link
Copy Markdown
Contributor

Description

Setting BATCH_EMBED_SIZE to 0 or a negative integer currently makes BatchEmbedWithPool panic in utils.ChunkSlice, rather than returning an error to ingestion or Wiki callers.

Validate the parsed value before mapping inputs or submitting work to the pool. Invalid values now return a configuration error with no provider requests. Positive values, the default batch size of 5, and existing integer-parse errors retain their current behavior.

The production fix is three lines and adds no shared state, dependencies or API fields.

Type of Change

  • 🐛 Bug fix
  • 🧪 Test

Related Issue

None; found while checking batch configuration handling. This does not change the retry/pacing work in #4127.

Testing

  • Before the fix, TestPoolRejectsInvalidBatchSize failed with chunkSize must be greater than 0 for both 0 and -1.
  • The regression uses the existing pooled-embedder HTTP fixture, checks error/nil results and verifies that invalid configuration sends no upstream requests. It also preserves the malformed-integer path.
  • Passed: go test ./internal/models/embedding -count=1 -timeout=2m
  • Passed: go test -race ./internal/models/embedding -count=1 -timeout=2m
  • Passed: go vet ./internal/models/embedding
  • Passed: go build ./internal/models/embedding
  • Passed: golangci-lint run --new-from-rev=origin/main --timeout=5m ./internal/models/embedding (v2.12.2; 0 issues).
  • Full-repository tests were not run. The existing live Qwen parity test is opt-in and was skipped because its endpoint/fixture environment variables were unset; local package checks use controlled HTTP fixtures.

Checklist

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

Documentation and screenshots: not applicable. Total diff: +29/-0 (29 changed lines including tests).

@lyingbug
lyingbug merged commit 07f36ed into Tencent:main Nov 2, 2026
4 checks passed
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.

2 participants