diff --git a/haystack/components/preprocessors/sentence_tokenizer.py b/haystack/components/preprocessors/sentence_tokenizer.py index 209cfb993f..a438b3c72a 100644 --- a/haystack/components/preprocessors/sentence_tokenizer.py +++ b/haystack/components/preprocessors/sentence_tokenizer.py @@ -42,6 +42,9 @@ QUOTE_SPANS_RE = re.compile(r'"[^"]*"|\'[^\']*\'') +# Closing brackets and quotes that can trail a sentence ending, as in `He said "Hi." Bye.` or `(Hi.) Bye.` +SENTENCE_CLOSING_CHARS_RE = r"[\)\]}\"'’”»]*" + if nltk_imports.is_successful(): def load_sentence_tokenizer( @@ -105,8 +108,10 @@ def period_context_re(self) -> re.Pattern: self._period_context_fmt % { "NonWord": self._re_non_word_chars, - # SentEndChars might be followed by closing brackets, so we match them here. - "SentEndChars": self._re_sent_end_chars + r"[\)\]}]*", + # SentEndChars might be followed by closing brackets or quotes, so we match them here. + # If we don't, the whitespace after e.g. `."` is behind the closing quote, the pattern + # above can't reach it and it ends up in none of the sentences we return. + "SentEndChars": self._re_sent_end_chars + SENTENCE_CLOSING_CHARS_RE, }, re.UNICODE | re.VERBOSE, ) @@ -201,6 +206,10 @@ def _needs_join( start, end = span next_start, next_end = next_span + # with keep_white_spaces=True a span also covers the whitespace up to the next sentence, the rules below + # look at where the sentence itself ends + end = start + len(text[start:end].rstrip()) + # sentence. sentence"\nsentence -> no split (end << quote_end) # sentence.", sentence -> no split (end < quote_end) # sentence?", sentence -> no split (end < quote_end) @@ -214,6 +223,16 @@ def _needs_join( # question is cited return True + # sentence.", sentence -> no split (end == quote_end): widening the closing-char class (see + # period_context_re) moves the boundary just past the closing quote instead of leaving it inside, + # so continuation punctuation directly after the quote must still join rather than start a sentence. + if any( + quote_start < end == quote_end and quote_end < len(text) and text[quote_end] in ",;:—" + for quote_start, quote_end in quote_spans + ): + # e.g. `He said "Hi.", then left.` -> the punctuation continues the sentence + return True + if re.search(r"(^|\n)\s*\d{1,2}\.$", text[start:end]) is not None: # sentence ends with a numeration return True diff --git a/releasenotes/notes/keep-white-space-after-closing-quote-99c6f2411842fd8f.yaml b/releasenotes/notes/keep-white-space-after-closing-quote-99c6f2411842fd8f.yaml new file mode 100644 index 0000000000..5905c5cd67 --- /dev/null +++ b/releasenotes/notes/keep-white-space-after-closing-quote-99c6f2411842fd8f.yaml @@ -0,0 +1,10 @@ +--- +fixes: + - | + Fixed ``SentenceSplitter`` losing the whitespace between two sentences when the first one ends with a closing + quote, as in ``He said "Hi." Bye.``. Those characters were missing from the chunk text and shifted the + ``split_idx_start`` offset of every following chunk, so chunks could no longer be mapped back onto the original + text. This affects all components that split on sentences, such as ``DocumentSplitter``, + ``RecursiveDocumentSplitter``, ``MarkdownHeaderSplitter`` and ``EmbeddingBasedDocumentSplitter``. Chunk + boundaries change for text that contains quoted sentences, so re-indexing an existing corpus produces different + chunks than before. diff --git a/test/components/preprocessors/test_document_splitter.py b/test/components/preprocessors/test_document_splitter.py index 6b4be79967..f267afd145 100644 --- a/test/components/preprocessors/test_document_splitter.py +++ b/test/components/preprocessors/test_document_splitter.py @@ -696,6 +696,24 @@ def test_run_split_by_sentence_4(self) -> None: assert documents[2].meta["split_id"] == 2 assert documents[2].meta["split_idx_start"] == text.index(documents[2].content) + def test_run_split_by_sentence_quoted_text_keeps_offsets_aligned(self) -> None: + document_splitter = DocumentSplitter( + split_by="sentence", + split_length=1, + split_overlap=0, + split_threshold=0, + language="en", + use_split_rules=True, + extend_abbreviations=True, + ) + text = 'One. He said "Two." Three.' + documents = document_splitter.run(documents=[Document(content=text)])["documents"] + + assert "".join(document.content for document in documents) == text + for document in documents: + start = document.meta["split_idx_start"] + assert text[start : start + len(document.content)] == document.content + def test_run_split_by_word_respect_sentence_boundary(self) -> None: document_splitter = DocumentSplitter( split_by="word", diff --git a/test/components/preprocessors/test_sentence_tokenizer.py b/test/components/preprocessors/test_sentence_tokenizer.py index fbae255eee..a4edea4189 100644 --- a/test/components/preprocessors/test_sentence_tokenizer.py +++ b/test/components/preprocessors/test_sentence_tokenizer.py @@ -104,6 +104,62 @@ def test_quote_spans_regex(): assert len(matches5) == 0 +@pytest.mark.parametrize( + "text", + [ + 'He said "Two." Three.', + "He said 'Two.' Three.", + "He said “Two.” Three.", + "He said ‘Two.’ Three.", + "Il a dit «Deux.» Trois.", + 'He shouted "Stop!" Three.', + "He said (two.) Three.", # brackets are already handled, this is the control case + ], +) +def test_split_sentences_keeps_white_spaces_after_a_closing_quote(text: str) -> None: + splitter = SentenceSplitter(language="en", keep_white_spaces=True) + sentences = splitter.split_sentences(text) + + # no character of the original text is lost + assert "".join(sentence["sentence"] for sentence in sentences) == text + + # and the spans still tile the text, so they can be mapped back onto it + assert sentences[0]["start"] == 0 + assert sentences[-1]["end"] == len(text) + for index in range(1, len(sentences)): + assert sentences[index]["start"] == sentences[index - 1]["end"] + + +def test_split_sentences_keeps_a_cited_question_joined() -> None: + # a quoted question is not a sentence boundary, the split rules must keep joining it + text = 'She asked "Are you sure?" Then she left.' + splitter = SentenceSplitter(language="en", keep_white_spaces=True) + + sentences = splitter.split_sentences(text) + + assert [sentence["sentence"] for sentence in sentences] == [text] + + +@pytest.mark.parametrize( + "text", + [ + 'He said "Hi.", then left.', # comma directly after the closing quote + "He said 'Hi.', then left.", # single quotes + 'He said "Hi."; then left.', # semicolon + 'He said "Hi.": then left.', # colon + 'He said "Hi."—then left.', # em dash + ], +) +def test_split_sentences_keeps_a_quote_with_trailing_punctuation_joined(text: str) -> None: + # a closing quote directly followed by continuation punctuation is not a sentence boundary; widening + # the closing-char class must not turn e.g. `.",` into a split that would start a chunk with a comma + splitter = SentenceSplitter(language="en", keep_white_spaces=True) + + sentences = splitter.split_sentences(text) + + assert [sentence["sentence"] for sentence in sentences] == [text] + + def test_split_sentences_performance() -> None: # make sure our regex is not vulnerable to Regex Denial of Service (ReDoS) # https://owasp.org/www-community/attacks/Regular_expression_Denial_of_Service_-_ReDoS