Repository: griptape-ai/griptape

Issue:
Chunked text removing the separator character at end of chunk
When chunking text via a separator, the separator isn't included in the chunks.

```python
from griptape.chunkers import ChunkSeparator, TextChunker
from griptape.tokenizers import OpenAiTokenizer
from rich import print

text = "This is a paragraph of text. I'll count to three: one, two three."
tokenizer = OpenAiTokenizer(model="gpt-4o-mini")
max_tokens = 10
chunker = TextChunker(
    max_tokens=max_tokens, separators=[ChunkSeparator(".")], tokenizer=tokenizer
).chunk(text)

print(f"Max Tokens: {max_tokens}")
for i, chunk in enumerate(chunker):
    chunk_size = tokenizer.count_tokens(chunk.value)
    print(f"{chunk_size}: {chunk.value}")
```

This results in:

```bash
Max Tokens: 10
7: This is a paragraph of text.
9: I'll count to three: one, two three
```

But notice the last `.` is not included after the `three`.

Discussion:
@collindutter: @shhlife while I get to the bottom of _why_ this is happening, you can workaround this by setting the chunk separator to:
 `". "`. That is how the [default separators](https://github.com/griptape-ai/griptape/blob/9aee8133647e1c17a3ff83f6691e92724eafefc5/griptape/chunkers/text_chunker.py?plain=1#L8) are configured.

I think this is somehow related to [this line](https://github.com/griptape-ai/griptape/blob/9aee8133647e1c17a3ff83f6691e92724eafefc5/griptape/chunkers/base_chunker.py?plain=1#L93).

Maybe @vasinov can shed some light on this behavior.

Solution patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index b25a058c..f50ac9c8 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -26,6 +26,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - `LocalVectorStoreDriver` returned Entries not containing the namespace.
 - References being lost on Artifacts during chunking.
 - `FootnotePromptResponseRagModule`'s system prompt causing it to not answer even with relevant chunks. 
+- Chunker occasionally dropping suffix chunk separators.
 
 ### Deprecated
 
diff --git a/griptape/chunkers/base_chunker.py b/griptape/chunkers/base_chunker.py
index fa32e559..6f4f9c06 100644
--- a/griptape/chunkers/base_chunker.py
+++ b/griptape/chunkers/base_chunker.py
@@ -53,10 +53,14 @@ class BaseChunker(ABC):
             # Loop through available separators to find the best split.
             for separator in separators:
                 # Split the chunk into subchunks using the current separator.
-                subchunks = list(filter(None, chunk.split(separator.value)))
+                subchunks = chunk.strip().split(separator.value)
 
-                # We can only recurse if there are multiple subchunks.
-                if len(subchunks) > 1:
+                # We should not operate on the filtered, non-empty subchunks because the joins will be incorrect.
+                # However, we only want to process chunks that have multiple non-empty subchunks.
+                # Therefore, we use the non-empty subchunks to decide if we should proceed, but we operate on the original subchunks.
+                non_empty_subchunks = list(filter(None, subchunks))
+
+                if len(non_empty_subchunks) > 1:
                     # Find what combination of subchunks results in the most balanced split of the chunk.
                     midpoint_index = self.__find_midpoint_index(subchunks, half_token_count)
 
@@ -82,13 +86,13 @@ class BaseChunker(ABC):
             return self._chunk_recursively(chunk[:midpoint]) + self._chunk_recursively(chunk[midpoint:])
 
     def __get_subchunks(self, separator: ChunkSeparator, subchunks: list[str], balance_index: int) -> tuple[str, str]:
-        # Create the two subchunks based on the best separator.
+        # Create the two subchunks based on the best separator
         if separator.is_prefix:
-            # If the separator is a prefix, append it before this subchunk.
-            first_subchunk = separator.value + separator.value.join(subchunks[: balance_index + 1])
+            first_subchunk = separator.value.join(subchunks[: balance_index + 1])
+            # We need to manually prepend the separator since join doesn't add it to the first element.
             second_subchunk = separator.value + separator.value.join(subchunks[balance_index + 1 :])
         else:
-            # If the separator is not a prefix, append it after this subchunk.
+            # We need to manually append the separator since join doesn't add it to the last element.
             first_subchunk = separator.value.join(subchunks[: balance_index + 1]) + separator.value
             second_subchunk = separator.value.join(subchunks[balance_index + 1 :])
 


Test patch:
diff --git a/tests/unit/chunkers/test_text_chunker.py b/tests/unit/chunkers/test_text_chunker.py
index ee1b2152..db4d16f8 100644
--- a/tests/unit/chunkers/test_text_chunker.py
+++ b/tests/unit/chunkers/test_text_chunker.py
@@ -114,8 +114,8 @@ class TestTextChunker:
             TextChunker(max_tokens=-1)
 
     def test_tiny_max_tokens(self, chunker):
-        max_tokens = 10
-        chunker.separators = [ChunkSeparator(" ")]
+        max_tokens = 11
+        chunker.separators = [ChunkSeparator(".")]
         chunker.max_tokens = max_tokens
         text = "This is a paragraph of text. I'll count to three: one, two three."
         chunks = chunker.chunk(text)
@@ -123,6 +123,7 @@ class TestTextChunker:
         assert len(chunks) == 2
         for chunk in chunks:
             assert chunker.tokenizer.count_tokens(chunk.value) <= max_tokens
+        assert chunks[-1].value.endswith("one, two three.")
 
     def test_artifact_reference(self, chunker):
         from griptape.common.reference import Reference