refactor(chunk): remove the unused size chunker

What does this MR do and why?

This MR removes the size chunker. Nothing builds it: since !686 (merged) the indexer creates only the two code chunkers, and GitLab accepts only those two chunk strategies, so its only remaining caller is one benchmark case. It also panics, never returns or splits multi-byte characters for some inputs, and removing it is simpler than fixing code that nothing runs. Indexing does not change.

Testing

The chunk mode packages pass with -race on Go 1.26.7 and 1.27.3, the benchmark runs its two remaining cases, and make golangci-lint at the pinned 2.11.3 reports 0 issues.

Performance analysis

The removed code ran only in its own tests and one benchmark case, so indexing does the same work as before.

  • This merge request does not introduce a performance regression. If a regression is expected, explain why.
Details: what goes, why it is safe, what stays, checks

What goes. The internal/mode/chunk/chunker/size/ package (chunker.go and chunker_test.go) and the size case of BenchmarkChunker, with its import. After the change, git grep -n -e chunker/size -e sizeChunker finds nothing.

Why it is safe.

  • In the indexer, createChunker builds only the code_bytes chunker (also for an empty strategy) and the code_pre_bert chunker, and answers any other strategy with unknown chunking strategy (chunk.go#L53-79). !686 (merged) replaced the call to the size chunker there with the code chunker.
  • In GitLab, the options sent to the indexer take chunk_strategy from the collection record (indexer.rb#L98-109), whose options are validated against a JSON schema (collection.rb#L25) that allows only null, code_bytes and code_pre_bert (ai_active_context_collection_options.json#L23-34), the same two strategies as Ai::ActiveContext::Embedding::CHUNK_STRATEGIES (embedding.rb#L50-52).
  • Before the change, the only use of the package outside itself was the size case of BenchmarkChunker (chunker_test.go#L21-29).

What stays.

  • The comment in code/chunker.go saying that the chunker falls back to a size chunker: that fallback is the size chunker of gitlab-code-parser, which its C bindings give the split-code chunker (lib.rs#L72-77, at v0.22.0, the version in go.mod), not this package.
  • ChunkOptions, DefaultChunkSize, FileContentType and File.GetFilename, which the code chunker uses too.
  • chunk_overlap: still accepted and checked against the chunk size. Removing it, with its README text, is left for the follow-up issue discussed in this MR.

Earlier version. This MR first fixed the size chunker in two commits (1c33cd43 and 5f716741, still in the MR's versions). After review, this removal replaces them in one commit, so that the changelog, which lists every commit of an MR merged without squash, does not list fixes for code that this MR deletes.

Checks with the change at 9b44f273, on Linux amd64 with Go 1.26.7 (from .tool-versions) unless named, and libicu-dev 76.1:

  • go mod tidy (no change), make format (no file changed), ./scripts/check-go-format.sh, make build, go vet over every package, and ./scripts/check-tool-versions.sh, check-go-build-versions.sh and check-go-mod-version.sh.
  • go test -race -count=1 ./internal/mode/chunk/... with Go 1.26.7 and 1.27.3: every package passes.
  • go test -run '^$' -bench BenchmarkChunker ./internal/mode/chunk/chunker/ with both: the code_bytes and code_pre_bert cases run.
  • make golangci-lint with golangci-lint 2.11.3: 0 issues.
  • make test and make cover without Elasticsearch or Gitaly: every package passes except TestBulkSizeTracking in internal/mode/advanced/elastic, which posts to Elasticsearch at 127.0.0.1:9200 and gets connection refused; that package and its tests import nothing under internal/mode/chunk (go list -deps -test). The tests of the nested third_party/icu module pass with -race.
  • commitlint with the project's .commitlintrc.yml on the commit and on this title: 0 problems.
  • The same checks on the merge of 9b44f273 into main at 79408959, which changed no Go file: same results.

Not run: the integration tests that need make test-infra. None of them can build the removed package.

Edited by José M. Requena Plens

Merge request reports

Loading
Loading