This is an automated email from the ASF dual-hosted git repository.
mattcasters pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/hop.git
The following commit(s) were added to refs/heads/main by this push:
new ad599d450f Fixes #8751 : Text chunker: don't emit whitespace-only
chunks (#8832)
ad599d450f is described below
commit ad599d450f231d2f0f21ef7b3b55f82061d3ba44
Author: Bart Maertens <[email protected]>
AuthorDate: Fri Oct 9 22:54:08 2026 +0200
Fixes #8751 : Text chunker: don't emit whitespace-only chunks (#8832)
---
.../pages/pipeline/transforms/textchunker.adoc | 2 ++
.../0115-text-chunker-structure-markdown.hpl | 6 +++++
.../golden-text-chunker-structure-markdown.csv | 3 +++
.../golden-text-chunker-structure-markdown.json | 2 +-
.../0115-text-chunker-structure-markdown UNIT.json | 2 +-
.../pipeline/transforms/chunker/TextChunker.java | 5 ++--
.../chunking/StructureChunkingStrategy.java | 31 +++++++++++++++++-----
.../chunker/messages/messages_en_US.properties | 2 +-
.../transforms/chunker/TextChunkerTest.java | 16 +++++++++++
.../chunking/StructureChunkingStrategyTest.java | 21 +++++++++++++++
10 files changed, 79 insertions(+), 11 deletions(-)
diff --git
a/docs/hop-user-manual/modules/ROOT/pages/pipeline/transforms/textchunker.adoc
b/docs/hop-user-manual/modules/ROOT/pages/pipeline/transforms/textchunker.adoc
index 460ca7f587..92f0a0a85f 100644
---
a/docs/hop-user-manual/modules/ROOT/pages/pipeline/transforms/textchunker.adoc
+++
b/docs/hop-user-manual/modules/ROOT/pages/pipeline/transforms/textchunker.adoc
@@ -106,4 +106,6 @@ The Hop-native parsers make a pipeline, a workflow or a
metadata file retrievabl
Chunk overlap applies to the Character strategy. The Paragraph and Structure
strategies split on natural boundaries, where an overlap would duplicate whole
paragraphs or sections between neighbouring chunks.
+The Structure strategy drops chunks that contain only whitespace. The chunk
index and total chunk count cover only the chunks that are emitted. Text that
yields no chunks produces a single row with an empty chunk and a total of 0,
the same as empty text.
+
Keep *Include metadata* enabled when the chunks are going into a vector store:
the document ID and chunk index are what let a later search merge neighbouring
chunks back into a passage, and re-index a single document without touching the
rest.
diff --git
a/integration-tests/transforms/0115-text-chunker-structure-markdown.hpl
b/integration-tests/transforms/0115-text-chunker-structure-markdown.hpl
index b48f201a46..5b40d5c261 100644
--- a/integration-tests/transforms/0115-text-chunker-structure-markdown.hpl
+++ b/integration-tests/transforms/0115-text-chunker-structure-markdown.hpl
@@ -59,6 +59,12 @@ limitations under the License.
<line>
<item># Title Intro line. ## Section A Body
A. ## Section B Body B.</item>
</line>
+ <line>
+ <item> 	 </item>
+ </line>
+ <line>
+ <item>alpha
beta</item>
+ </line>
</data>
<fields>
<field>
diff --git
a/integration-tests/transforms/datasets/golden-text-chunker-structure-markdown.csv
b/integration-tests/transforms/datasets/golden-text-chunker-structure-markdown.csv
index 1ac9e8448e..115cac4452 100644
---
a/integration-tests/transforms/datasets/golden-text-chunker-structure-markdown.csv
+++
b/integration-tests/transforms/datasets/golden-text-chunker-structure-markdown.csv
@@ -8,3 +8,6 @@ Body A.",1,22,3
"Title > Section B
Body B.",2,45,3
+"",0,0,0
+"alpha
",0,0,2
+" beta",1,400,2
diff --git
a/integration-tests/transforms/metadata/dataset/golden-text-chunker-structure-markdown.json
b/integration-tests/transforms/metadata/dataset/golden-text-chunker-structure-markdown.json
index 6f89051cd6..e514c9b83f 100644
---
a/integration-tests/transforms/metadata/dataset/golden-text-chunker-structure-markdown.json
+++
b/integration-tests/transforms/metadata/dataset/golden-text-chunker-structure-markdown.json
@@ -1,7 +1,7 @@
{
"base_filename": "golden-text-chunker-structure-markdown.csv",
"name": "golden-text-chunker-structure-markdown",
- "description": "Structure chunking of Markdown: one chunk per section,
prefixed with its heading path",
+ "description": "Structure chunking of Markdown: one chunk per section,
prefixed with its heading path; whitespace-only chunks are dropped",
"dataset_fields": [
{
"field_comment": "",
diff --git
a/integration-tests/transforms/metadata/unit-test/0115-text-chunker-structure-markdown
UNIT.json
b/integration-tests/transforms/metadata/unit-test/0115-text-chunker-structure-markdown
UNIT.json
index 84616afe12..50eeeeb011 100644
---
a/integration-tests/transforms/metadata/unit-test/0115-text-chunker-structure-markdown
UNIT.json
+++
b/integration-tests/transforms/metadata/unit-test/0115-text-chunker-structure-markdown
UNIT.json
@@ -35,7 +35,7 @@
],
"input_data_sets": [],
"name": "0115-text-chunker-structure-markdown UNIT",
- "description": "Structure chunking of Markdown: one chunk per section,
prefixed with its heading path",
+ "description": "Structure chunking of Markdown: one chunk per section,
prefixed with its heading path; whitespace-only chunks are dropped",
"trans_test_tweaks": [],
"persist_filename": "",
"pipeline_filename": "./0115-text-chunker-structure-markdown.hpl",
diff --git
a/plugins/transforms/textchunker/src/main/java/org/apache/hop/pipeline/transforms/chunker/TextChunker.java
b/plugins/transforms/textchunker/src/main/java/org/apache/hop/pipeline/transforms/chunker/TextChunker.java
index 0bf89fcae5..612060a191 100644
---
a/plugins/transforms/textchunker/src/main/java/org/apache/hop/pipeline/transforms/chunker/TextChunker.java
+++
b/plugins/transforms/textchunker/src/main/java/org/apache/hop/pipeline/transforms/chunker/TextChunker.java
@@ -124,7 +124,9 @@ public class TextChunker extends
BaseTransform<TextChunkerMeta, TextChunkerData>
String text = data.inputRowMeta.getString(row, data.inputFieldIndex);
String documentIdValue = resolveDocumentId(row);
- if (Utils.isEmpty(text)) {
+ List<Chunk> chunks = Utils.isEmpty(text) ? List.of() : chunk(row, text);
+ if (chunks.isEmpty()) {
+ // Empty text, or text that yields no chunks (e.g. whitespace-only for
Structure).
if (isRowLevel()) {
logRowlevel(BaseMessages.getString(PKG, "TextChunker.Log.EmptyText"));
}
@@ -132,7 +134,6 @@ public class TextChunker extends
BaseTransform<TextChunkerMeta, TextChunkerData>
return;
}
- List<Chunk> chunks = chunk(row, text);
int totalChunks = chunks.size();
// Output each chunk as a new row (must copy row data per chunk; the
buffer is reused).
diff --git
a/plugins/transforms/textchunker/src/main/java/org/apache/hop/pipeline/transforms/chunker/chunking/StructureChunkingStrategy.java
b/plugins/transforms/textchunker/src/main/java/org/apache/hop/pipeline/transforms/chunker/chunking/StructureChunkingStrategy.java
index 7d4901314a..65347dff12 100644
---
a/plugins/transforms/textchunker/src/main/java/org/apache/hop/pipeline/transforms/chunker/chunking/StructureChunkingStrategy.java
+++
b/plugins/transforms/textchunker/src/main/java/org/apache/hop/pipeline/transforms/chunker/chunking/StructureChunkingStrategy.java
@@ -27,6 +27,7 @@ import
org.apache.hop.pipeline.transforms.chunker.document.DocumentParserRegistr
/**
* Structure-aware chunking: parse document into sections, prefix each chunk
with a breadcrumb path,
* and fall back to {@link CharacterChunkingStrategy} when a section exceeds
the size limit.
+ * Whitespace-only chunks are dropped; chunk indexes count only the chunks
that are returned.
*/
public class StructureChunkingStrategy implements ChunkingStrategy {
@@ -52,15 +53,14 @@ public class StructureChunkingStrategy implements
ChunkingStrategy {
List<DocumentNode.DocumentSection> sections = root.flattenSections();
if (sections.isEmpty()) {
- return fallback.chunk(text, maxSize, overlap);
+ return withoutBlankChunks(fallback.chunk(text, maxSize, overlap));
}
List<Chunk> chunks = new ArrayList<>();
- int chunkIndex = 0;
for (DocumentNode.DocumentSection section : sections) {
String sectionText = formatSection(section);
- if (sectionText.isEmpty()) {
+ if (sectionText.isBlank()) {
continue;
}
@@ -68,7 +68,7 @@ public class StructureChunkingStrategy implements
ChunkingStrategy {
chunks.add(
new Chunk(
sectionText,
- chunkIndex++,
+ chunks.size(),
section.getStartPosition(),
section.getStartPosition() + sectionText.length()));
} else {
@@ -78,11 +78,14 @@ public class StructureChunkingStrategy implements
ChunkingStrategy {
int bodyMax = prefix.isEmpty() ? maxSize : Math.max(1, maxSize -
prefix.length());
List<Chunk> split = fallback.chunk(body, bodyMax, overlap);
for (Chunk part : split) {
+ if (part.getContent().isBlank()) {
+ continue;
+ }
String content = prefix.isEmpty() ? part.getContent() : prefix +
part.getContent();
chunks.add(
new Chunk(
content,
- chunkIndex++,
+ chunks.size(),
section.getStartPosition() + part.getStartPosition(),
section.getStartPosition() + part.getEndPosition()));
}
@@ -90,12 +93,28 @@ public class StructureChunkingStrategy implements
ChunkingStrategy {
}
if (chunks.isEmpty()) {
- return fallback.chunk(text, maxSize, overlap);
+ return withoutBlankChunks(fallback.chunk(text, maxSize, overlap));
}
return chunks;
}
+ /** Drops whitespace-only chunks and renumbers the rest from 0. */
+ private static List<Chunk> withoutBlankChunks(List<Chunk> chunks) {
+ List<Chunk> result = new ArrayList<>();
+ for (Chunk chunk : chunks) {
+ if (!chunk.getContent().isBlank()) {
+ result.add(
+ new Chunk(
+ chunk.getContent(),
+ result.size(),
+ chunk.getStartPosition(),
+ chunk.getEndPosition()));
+ }
+ }
+ return result;
+ }
+
private static String formatSection(DocumentNode.DocumentSection section) {
String breadcrumb = section.breadcrumb();
String body = section.getBody();
diff --git
a/plugins/transforms/textchunker/src/main/resources/org/apache/hop/pipeline/transforms/chunker/messages/messages_en_US.properties
b/plugins/transforms/textchunker/src/main/resources/org/apache/hop/pipeline/transforms/chunker/messages/messages_en_US.properties
index ad8baa5616..091974a3eb 100644
---
a/plugins/transforms/textchunker/src/main/resources/org/apache/hop/pipeline/transforms/chunker/messages/messages_en_US.properties
+++
b/plugins/transforms/textchunker/src/main/resources/org/apache/hop/pipeline/transforms/chunker/messages/messages_en_US.properties
@@ -80,7 +80,7 @@ TextChunker.Error.UnknownStrategy=Unknown chunking strategy:
{0}
TextChunker.Log.Initialized=Initialized Text Chunker with strategy: {0}
TextChunker.Log.ProcessedDocument=Processed document with {0} chunks
-TextChunker.Log.EmptyText=Empty text received, emitting a single empty chunk
row
+TextChunker.Log.EmptyText=No text to chunk, emitting a single empty chunk row
TextChunker.Validation.OutputChunkFieldRequired=Output chunk field name must
be specified
TextChunker.Validation.SourceDocumentIdFieldNotFound=Source document ID field
''{0}'' not found in the input stream
diff --git
a/plugins/transforms/textchunker/src/test/java/org/apache/hop/pipeline/transforms/chunker/TextChunkerTest.java
b/plugins/transforms/textchunker/src/test/java/org/apache/hop/pipeline/transforms/chunker/TextChunkerTest.java
index 5d7edbe830..3fe51678be 100644
---
a/plugins/transforms/textchunker/src/test/java/org/apache/hop/pipeline/transforms/chunker/TextChunkerTest.java
+++
b/plugins/transforms/textchunker/src/test/java/org/apache/hop/pipeline/transforms/chunker/TextChunkerTest.java
@@ -143,6 +143,22 @@ class TextChunkerTest {
assertEquals(0L, output.get(0)[totalPos]);
}
+ @Test
+ void emitsASingleRowForWhitespaceOnlyTextWithStructure() throws Exception {
+ TextChunkerMeta meta = new TextChunkerMeta();
+ meta.setDefault();
+ meta.setInputField("text");
+ meta.setChunkingStrategy(ChunkingStrategyType.STRUCTURE);
+
+ run(meta, rowMeta(), List.<Object[]>of(new Object[] {" \n\n \t \n"}));
+
+ int chunkPos = outputRowMeta.indexOfValue("chunk_text");
+ int totalPos = outputRowMeta.indexOfValue("total_chunks");
+ assertEquals(1, output.size());
+ assertEquals("", output.get(0)[chunkPos]);
+ assertEquals(0L, output.get(0)[totalPos]);
+ }
+
private static IRowMeta rowMeta() {
IRowMeta rowMeta = new RowMeta();
rowMeta.addValueMeta(new ValueMetaString("text"));
diff --git
a/plugins/transforms/textchunker/src/test/java/org/apache/hop/pipeline/transforms/chunker/chunking/StructureChunkingStrategyTest.java
b/plugins/transforms/textchunker/src/test/java/org/apache/hop/pipeline/transforms/chunker/chunking/StructureChunkingStrategyTest.java
index 94152ace39..44f5452f83 100644
---
a/plugins/transforms/textchunker/src/test/java/org/apache/hop/pipeline/transforms/chunker/chunking/StructureChunkingStrategyTest.java
+++
b/plugins/transforms/textchunker/src/test/java/org/apache/hop/pipeline/transforms/chunker/chunking/StructureChunkingStrategyTest.java
@@ -76,6 +76,27 @@ class StructureChunkingStrategyTest {
assertEquals(text, chunks.get(0).getContent());
}
+ @Test
+ void whitespaceOnlyTextYieldsNoChunks() {
+ assertTrue(strategy.chunk(" \n\n\t \n", 5, 0,
ContentType.PLAIN).isEmpty());
+ assertTrue(strategy.chunk(" \n\n\t \n", 5, 0,
ContentType.MARKDOWN).isEmpty());
+ }
+
+ @Test
+ void dropsWhitespaceOnlyPartsOfLargeSectionAndRenumbers() {
+ // A preamble without heading has no breadcrumb, so its character-split
parts are emitted as-is.
+ String text = "alpha" + " ".repeat(30) + "beta";
+
+ List<Chunk> chunks = strategy.chunk(text, 10, 0, ContentType.PLAIN);
+
+ assertTrue(chunks.stream().noneMatch(c -> c.getContent().isBlank()));
+ for (int i = 0; i < chunks.size(); i++) {
+ assertEquals(i, chunks.get(i).getIndex());
+ }
+ assertTrue(chunks.get(0).getContent().contains("alpha"));
+ assertTrue(chunks.get(chunks.size() - 1).getContent().contains("beta"));
+ }
+
@Test
void strategyTypeIsStructure() {
assertEquals(ChunkingStrategyType.STRUCTURE, strategy.getType());
