Skip to content

add trimSentences option to SentenceTokenizer, to let users choose to preserve whitespace - #760

Merged
Hugo-ter-Doest merged 5 commits into
NaturalNode:masterfrom
jeremybmerrill:master
May 22, 2025
Merged

add trimSentences option to SentenceTokenizer, to let users choose to preserve whitespace#760
Hugo-ter-Doest merged 5 commits into
NaturalNode:masterfrom
jeremybmerrill:master

Conversation

@jeremybmerrill

Copy link
Copy Markdown
Contributor

I use natural downstream of [https://github.com/run-llama/LlamaIndexTS/blob/189d8a83ac8c7d0f8ac9402f24fd1516569555dd/packages/core/src/node-parser/sentence-splitter.ts](LlamaIndex's sentence-splitter) -- which aims to split natural language text into chunks of a (roughly) arbitrary-size, preferring to split at sentence boundaries. I use that in https://github.com/jeremybmerrill/meaningfully, a desktop tool for semantic search across CSVs containing text.

LlamaIndex appears to have a bug whereby text chunks end up with sentences joined without a space, e.g. This is a sentence.This is also a sentence.. This is a problem because that violates English's orthography rules.

The problem occurs because LlamaIndex naively joins sentences with an empty string -- even though natural has trimmed its output sentences of any leading/trailing whitespace.

This PR adds a trimSentences option to SentenceTokenizer's constructor() that, when set to false, doesn't trim the whitespace from output sentences, instead leaving it intact. As a result, when LlamaIndex splits and re-joins the sentences, proper whitespace is preserved. The option defaults to true, which means that the existing behavior is retained when code downstream of natural is left unchanged.

I've added a passing test.

@coveralls

coveralls commented May 21, 2025

Copy link
Copy Markdown

Pull Request Test Coverage Report for Build 15187783489

Details

  • 3 of 4 (75.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-0.008%) to 87.132%

Changes Missing Coverage Covered Lines Changed/Added Lines %
lib/natural/tokenizers/sentence_tokenizer.js 3 4 75.0%
Totals Coverage Status
Change from base Build 10317973660: -0.008%
Covered Lines: 8933
Relevant Lines: 9903

💛 - Coveralls

@Hugo-ter-Doest
Hugo-ter-Doest merged commit 014c934 into NaturalNode:master May 22, 2025
@jeremybmerrill

Copy link
Copy Markdown
Contributor Author

Thanks very much @Hugo-ter-Doest !

jeremybmerrill added a commit to jeremybmerrill/meaningfully that referenced this pull request Jun 7, 2025
@jeremybmerrill

Copy link
Copy Markdown
Contributor Author

@Hugo-ter-Doest This might be my mistake (I only pretend to know typescript!) but I think https://github.com/NaturalNode/natural/blob/master/lib/natural/tokenizers/index.d.ts#L139 needs to be updated to know about this option for the SentenceTokenizer constructor.

@Hugo-ter-Doest

Copy link
Copy Markdown
Collaborator

You are right! Thanks!

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.

3 participants