Skip to content

Commit 6c9bd82

Browse files
fix: correct metadata timing in chunkers and add boundary tests
Fix metadata accumulation timing bugs in ElementsChunker and DocumentTokenChunker where AccumulateMetadata was called before determining which chunk the element's content contributes to. When a Commit/FinalizeChunk happens before the new element adds content (table pre-commit, non-table overflow, exact-fill boundary), the metadata was incorrectly applied to the previous chunk. ElementsChunker fixes: - Branch 1 (fits): accumulate right before appending - Branch 2 (table): use flag, accumulate before first table content append to _currentChunk, after any pre-commit or row-level commit - Branch 3 (non-table too big): use flag, accumulate when index > 0 (first content contribution in the while loop) DocumentTokenChunker fixes: - Use flag to defer accumulation until first content contribution - In while loop: accumulate only when index > 0 - After while loop: accumulate if not yet done (element fits entirely) New boundary tests (6 tests): - Previous element fills chunk, next element metadata on new chunk - Non-table element too large, metadata on correct chunks - Table pre-commit: table metadata not on pre-committed chunk - DocumentTokenChunker boundary with large filler element - DocumentTokenChunker with overlap enabled - Table split across chunks: first chunk gets metadata Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 742877a commit 6c9bd82

3 files changed

Lines changed: 248 additions & 4 deletions

File tree

src/Libraries/Microsoft.Extensions.DataIngestion/Chunkers/DocumentTokenChunker.cs

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -56,10 +56,9 @@ public override async IAsyncEnumerable<IngestionChunk<string>> ProcessAsync(Inge
5656
continue;
5757
}
5858

59-
AccumulateMetadata(element, ref accumulatedMetadata);
60-
6159
int contentToProcessTokenCount = _tokenizer.CountTokens(elementContent!, considerNormalization: false);
6260
ReadOnlyMemory<char> contentToProcess = elementContent.AsMemory();
61+
bool elementMetadataAccumulated = false;
6362
while (stringBuilderTokenCount + contentToProcessTokenCount >= _maxTokensPerChunk)
6463
{
6564
int index = _tokenizer.GetIndexByTokenCount(
@@ -69,6 +68,13 @@ public override async IAsyncEnumerable<IngestionChunk<string>> ProcessAsync(Inge
6968
out int _,
7069
considerNormalization: false);
7170

71+
// Accumulate metadata the first time this element contributes content.
72+
if (!elementMetadataAccumulated && index > 0)
73+
{
74+
AccumulateMetadata(element, ref accumulatedMetadata);
75+
elementMetadataAccumulated = true;
76+
}
77+
7278
unsafe
7379
{
7480
fixed (char* ptr = &MemoryMarshal.GetReference(contentToProcess.Span))
@@ -82,6 +88,12 @@ public override async IAsyncEnumerable<IngestionChunk<string>> ProcessAsync(Inge
8288
contentToProcessTokenCount = _tokenizer.CountTokens(contentToProcess.Span, considerNormalization: false);
8389
}
8490

91+
// Accumulate metadata if the element only contributed content after the loop.
92+
if (!elementMetadataAccumulated)
93+
{
94+
AccumulateMetadata(element, ref accumulatedMetadata);
95+
}
96+
8597
_ = stringBuilder.Append(contentToProcess);
8698
stringBuilderTokenCount += contentToProcessTokenCount;
8799
}

src/Libraries/Microsoft.Extensions.DataIngestion/Chunkers/ElementsChunker.cs

Lines changed: 24 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -68,17 +68,18 @@ internal IEnumerable<IngestionChunk<string>> Process(IngestionDocument document,
6868
continue; // An image can come with Markdown, but no AlternativeText or Text.
6969
}
7070

71-
AccumulateMetadata(element, ref accumulatedMetadata);
72-
7371
int elementTokenCount = CountTokens(semanticContent.AsSpan());
7472
if (elementTokenCount + totalTokenCount <= _maxTokensPerChunk)
7573
{
74+
// Element fits in the current chunk — accumulate its metadata here.
75+
AccumulateMetadata(element, ref accumulatedMetadata);
7676
totalTokenCount += elementTokenCount;
7777
AppendNewLineAndSpan(_currentChunk, semanticContent.AsSpan());
7878
}
7979
else if (element is IngestionDocumentTable table)
8080
{
8181
ValueStringBuilder tableBuilder = new(initialCapacity: 8000);
82+
bool tableMetadataAccumulated = false;
8283

8384
try
8485
{
@@ -116,6 +117,13 @@ internal IEnumerable<IngestionChunk<string>> Process(IngestionDocument document,
116117
// We append the table as long as it's not just the header.
117118
if (rowIndex != 1)
118119
{
120+
// Accumulate metadata before first table content append.
121+
if (!tableMetadataAccumulated)
122+
{
123+
AccumulateMetadata(element, ref accumulatedMetadata);
124+
tableMetadataAccumulated = true;
125+
}
126+
119127
AppendNewLineAndSpan(_currentChunk, tableBuilder.AsSpan(0, tableLength - Environment.NewLine.Length));
120128
}
121129

@@ -140,6 +148,12 @@ internal IEnumerable<IngestionChunk<string>> Process(IngestionDocument document,
140148
totalTokenCount += lastRowTokens;
141149
}
142150

151+
// Accumulate metadata before appending remaining table content.
152+
if (!tableMetadataAccumulated)
153+
{
154+
AccumulateMetadata(element, ref accumulatedMetadata);
155+
}
156+
143157
AppendNewLineAndSpan(_currentChunk, tableBuilder.AsSpan(0, tableLength - Environment.NewLine.Length));
144158
}
145159
finally
@@ -150,6 +164,7 @@ internal IEnumerable<IngestionChunk<string>> Process(IngestionDocument document,
150164
else
151165
{
152166
ReadOnlySpan<char> remainingContent = semanticContent.AsSpan();
167+
bool elementMetadataAccumulated = false;
153168

154169
while (!remainingContent.IsEmpty)
155170
{
@@ -173,6 +188,13 @@ internal IEnumerable<IngestionChunk<string>> Process(IngestionDocument document,
173188
tokenCount = CountTokens(remainingContent.Slice(0, index));
174189
}
175190

191+
// Accumulate metadata the first time this element contributes content.
192+
if (!elementMetadataAccumulated)
193+
{
194+
AccumulateMetadata(element, ref accumulatedMetadata);
195+
elementMetadataAccumulated = true;
196+
}
197+
176198
totalTokenCount += tokenCount;
177199
ReadOnlySpan<char> spanToAppend = remainingContent.Slice(0, index);
178200
AppendNewLineAndSpan(_currentChunk, spanToAppend);

test/Libraries/Microsoft.Extensions.DataIngestion.Tests/Chunkers/ChunkerMetadataPropagationTests.cs

Lines changed: 210 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -312,4 +312,214 @@ public async Task SectionChunker_TableWithMetadata_PropagatesMetadata()
312312
Assert.True(chunk.HasMetadata);
313313
Assert.Equal("table", chunk.Metadata["element_type"]);
314314
}
315+
316+
[Fact]
317+
public async Task SectionChunker_PreviousElementFillsChunk_NextElementMetadataOnNewChunk()
318+
{
319+
// First element exceeds the chunk limit, so it fills chunk 0 and overflows into chunk 1.
320+
// Second element is small and goes into the last chunk.
321+
// Each element has a unique metadata key — verify they end up on the correct chunks.
322+
string fillerText = string.Join(" ", Enumerable.Repeat("word", 600));
323+
var filler = new IngestionDocumentParagraph(fillerText);
324+
filler.Metadata["filler_key"] = "from_filler";
325+
326+
var nextElement = new IngestionDocumentParagraph("Next element content here.");
327+
nextElement.Metadata["next_key"] = "from_next";
328+
329+
var doc = new IngestionDocument("doc");
330+
doc.Sections.Add(new IngestionDocumentSection { Elements = { filler, nextElement } });
331+
332+
var chunker = CreateSectionChunker(maxTokensPerChunk: 200);
333+
var chunks = await chunker.ProcessAsync(doc).ToListAsync();
334+
335+
Assert.True(chunks.Count >= 2);
336+
337+
// First chunk must have filler metadata (it contributed content to this chunk)
338+
Assert.True(chunks[0].HasMetadata);
339+
Assert.Equal("from_filler", chunks[0].Metadata["filler_key"]);
340+
Assert.False(chunks[0].Metadata.ContainsKey("next_key"));
341+
342+
// The last chunk must have the next element's metadata
343+
var lastChunk = chunks[chunks.Count - 1];
344+
Assert.True(lastChunk.HasMetadata);
345+
Assert.Equal("from_next", lastChunk.Metadata["next_key"]);
346+
}
347+
348+
[Fact]
349+
public async Task SectionChunker_NonTableElementTooLargeForCurrentChunk_MetadataOnCorrectChunks()
350+
{
351+
// Two large elements with the same metadata key but different values.
352+
// Each element exceeds chunk limit. Verify first-wins semantics per chunk:
353+
// - Chunks containing elem1 content get elem1's metadata (only the first such chunk)
354+
// - Chunks containing elem2 content get elem2's metadata (only the first such chunk)
355+
var elem1 = new IngestionDocumentParagraph(string.Join(" ", Enumerable.Repeat("alpha", 300)));
356+
elem1.Metadata["source"] = "elem1";
357+
358+
var elem2 = new IngestionDocumentParagraph(string.Join(" ", Enumerable.Repeat("beta", 300)));
359+
elem2.Metadata["source"] = "elem2";
360+
361+
var doc = new IngestionDocument("doc");
362+
doc.Sections.Add(new IngestionDocumentSection { Elements = { elem1, elem2 } });
363+
364+
var chunker = CreateSectionChunker(maxTokensPerChunk: 200);
365+
var chunks = await chunker.ProcessAsync(doc).ToListAsync();
366+
367+
Assert.True(chunks.Count >= 3);
368+
369+
// First chunk: elem1's metadata (elem1 contributes content)
370+
Assert.Equal("elem1", chunks[0].Metadata["source"]);
371+
372+
// Find the first chunk that contains elem2's content
373+
var firstElem2Chunk = chunks.First(c => c.Content.Contains("beta"));
374+
Assert.True(firstElem2Chunk.HasMetadata);
375+
Assert.Equal("elem2", firstElem2Chunk.Metadata["source"]);
376+
}
377+
378+
[Fact]
379+
public async Task SectionChunker_TablePreCommit_TableMetadataNotOnPreviousChunk()
380+
{
381+
// Previous content fills most of the chunk. Table header doesn't fit, forcing a pre-commit.
382+
// Table metadata must go on the chunk with the table, not the pre-committed chunk.
383+
// Use different metadata keys to distinguish elements.
384+
var filler = new IngestionDocumentParagraph(string.Join(" ", Enumerable.Repeat("fill", 500)));
385+
filler.Metadata["paragraph_key"] = "paragraph_value";
386+
387+
var cells = new IngestionDocumentElement?[2, 2]
388+
{
389+
{ new IngestionDocumentParagraph("Col1"), new IngestionDocumentParagraph("Col2") },
390+
{ new IngestionDocumentParagraph("Val1"), new IngestionDocumentParagraph("Val2") }
391+
};
392+
var table = new IngestionDocumentTable("| Col1 | Col2 |\n| --- | --- |\n| Val1 | Val2 |", cells);
393+
table.Metadata["table_key"] = "table_value";
394+
395+
var doc = new IngestionDocument("doc");
396+
doc.Sections.Add(new IngestionDocumentSection { Elements = { filler, table } });
397+
398+
var chunker = CreateSectionChunker(maxTokensPerChunk: 200);
399+
var chunks = await chunker.ProcessAsync(doc).ToListAsync();
400+
401+
Assert.True(chunks.Count >= 2);
402+
403+
// Find the chunk containing table content
404+
var tableChunk = chunks.FirstOrDefault(c => c.Content.Contains("Col1") || c.Content.Contains("Val1"));
405+
Assert.NotNull(tableChunk);
406+
407+
// The table chunk must have the table's metadata
408+
Assert.True(tableChunk!.HasMetadata);
409+
Assert.Equal("table_value", tableChunk.Metadata["table_key"]);
410+
411+
// Chunks before the table chunk should NOT have table metadata
412+
int tableChunkIndex = chunks.IndexOf(tableChunk);
413+
for (int i = 0; i < tableChunkIndex; i++)
414+
{
415+
Assert.False(chunks[i].Metadata.ContainsKey("table_key"),
416+
$"Chunk {i} should not have table metadata");
417+
}
418+
}
419+
420+
[Fact]
421+
public async Task DocumentTokenChunker_PreviousElementFillsChunk_NextElementMetadataOnNewChunk()
422+
{
423+
// First element exceeds chunk limit, second element is small.
424+
// Each has unique keys — verify correct chunk association.
425+
string fillerText = string.Join(" ", Enumerable.Repeat("word", 600));
426+
var filler = new IngestionDocumentParagraph(fillerText);
427+
filler.Metadata["filler_key"] = "from_filler";
428+
429+
var nextElement = new IngestionDocumentParagraph("Next element with metadata.");
430+
nextElement.Metadata["next_key"] = "from_next";
431+
432+
var doc = new IngestionDocument("doc");
433+
doc.Sections.Add(new IngestionDocumentSection { Elements = { filler, nextElement } });
434+
435+
var chunker = CreateDocumentTokenChunker(maxTokensPerChunk: 200);
436+
var chunks = await chunker.ProcessAsync(doc).ToListAsync();
437+
438+
Assert.True(chunks.Count >= 2);
439+
440+
// First chunk must have filler metadata
441+
Assert.True(chunks[0].HasMetadata);
442+
Assert.Equal("from_filler", chunks[0].Metadata["filler_key"]);
443+
Assert.False(chunks[0].Metadata.ContainsKey("next_key"));
444+
445+
// The last chunk must have the next element's metadata
446+
var lastChunk = chunks[chunks.Count - 1];
447+
Assert.True(lastChunk.HasMetadata);
448+
Assert.Equal("from_next", lastChunk.Metadata["next_key"]);
449+
}
450+
451+
[Fact]
452+
public async Task DocumentTokenChunker_WithOverlap_PropagatesMetadata()
453+
{
454+
var tokenizer = TiktokenTokenizer.CreateForModel("gpt-4o");
455+
var chunker = new DocumentTokenChunker(new(tokenizer) { MaxTokensPerChunk = 200, OverlapTokens = 50 });
456+
457+
string text1 = string.Join(" ", Enumerable.Repeat("alpha", 300));
458+
var para1 = new IngestionDocumentParagraph(text1);
459+
para1.Metadata["section"] = "intro";
460+
461+
string text2 = string.Join(" ", Enumerable.Repeat("beta", 100));
462+
var para2 = new IngestionDocumentParagraph(text2);
463+
para2.Metadata["section"] = "body";
464+
465+
var doc = new IngestionDocument("doc");
466+
doc.Sections.Add(new IngestionDocumentSection { Elements = { para1, para2 } });
467+
468+
var chunks = await chunker.ProcessAsync(doc).ToListAsync();
469+
470+
Assert.True(chunks.Count >= 2);
471+
472+
// First chunk should have intro metadata
473+
Assert.True(chunks[0].HasMetadata);
474+
Assert.Equal("intro", chunks[0].Metadata["section"]);
475+
}
476+
477+
[Fact]
478+
public async Task SectionChunker_TableSplitAcrossChunks_FirstChunkGetsMetadata()
479+
{
480+
// A large table that spans multiple chunks — only the first chunk containing table content gets metadata
481+
int rowCount = 30;
482+
int colCount = 3;
483+
var cells = new IngestionDocumentElement?[rowCount, colCount];
484+
cells[0, 0] = new IngestionDocumentParagraph("HeaderColumn1");
485+
cells[0, 1] = new IngestionDocumentParagraph("HeaderColumn2");
486+
cells[0, 2] = new IngestionDocumentParagraph("HeaderColumn3");
487+
488+
// Build a proper markdown string that's long enough to exceed the token limit
489+
var mdBuilder = new System.Text.StringBuilder();
490+
mdBuilder.AppendLine("| HeaderColumn1 | HeaderColumn2 | HeaderColumn3 |");
491+
mdBuilder.AppendLine("| --- | --- | --- |");
492+
493+
for (int i = 1; i < rowCount; i++)
494+
{
495+
string c1 = $"Row{i} first column value with extra text to increase token count";
496+
string c2 = $"Row{i} second column value with extra text to increase token count";
497+
string c3 = $"Row{i} third column value with extra text to increase token count";
498+
cells[i, 0] = new IngestionDocumentParagraph(c1);
499+
cells[i, 1] = new IngestionDocumentParagraph(c2);
500+
cells[i, 2] = new IngestionDocumentParagraph(c3);
501+
mdBuilder.AppendLine($"| {c1} | {c2} | {c3} |");
502+
}
503+
504+
var table = new IngestionDocumentTable(mdBuilder.ToString(), cells);
505+
table.Metadata["element_type"] = "data_table";
506+
507+
var doc = new IngestionDocument("doc");
508+
doc.Sections.Add(new IngestionDocumentSection { Elements = { table } });
509+
510+
var chunker = CreateSectionChunker(maxTokensPerChunk: 200);
511+
var chunks = await chunker.ProcessAsync(doc).ToListAsync();
512+
513+
Assert.True(chunks.Count > 1, $"Table should span multiple chunks but got {chunks.Count}");
514+
515+
// First chunk gets table metadata
516+
Assert.True(chunks[0].HasMetadata);
517+
Assert.Equal("data_table", chunks[0].Metadata["element_type"]);
518+
519+
// Subsequent table chunks do NOT get metadata (cleared on commit, first-wins)
520+
for (int i = 1; i < chunks.Count; i++)
521+
{
522+
Assert.False(chunks[i].HasMetadata, $"Chunk {i} should not have metadata");
523+
}
524+
}
315525
}

0 commit comments

Comments
 (0)