fix: keep inter-sentence whitespace when a sentence ends with a closing quote

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

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.

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",

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