From c7910e6e4b89d5c33adba87e93bbdf22eb7ccc3c Mon Sep 17 00:00:00 2001 From: David Kaya Date: Wed, 15 Apr 2026 18:55:34 +0200 Subject: [PATCH] fix: remove streaming text boundary heuristics that injected spurious spaces The StreamingTextMerger (C#) and mergeStreamingText (TypeScript) contained custom word-boundary heuristics that inserted spaces between streaming deltas based on incorrect assumptions. Two bugs caused mid-word spaces: 1. Unconditional space insertion when incoming delta started with uppercase or digit characters, breaking acronyms (MDA -> M DA, UAG -> UA G) 2. LooksLikeWordBoundary using global token counts that fired on nearly every mid-word token split (writable -> wr itable, fragile -> frag ile) Both upstream projects (Copilot SDK and Agent Framework) use simple string concatenation for delta accumulation. LLM streaming tokens natively include whitespace when needed, so no boundary detection is required. Removed all boundary heuristic methods and replaced with plain concatenation. Kept overlap detection and snapshot handling for event redelivery scenarios. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../Services/StreamingTextMerger.cs | 136 +++++------------- .../StreamingTextMergerTests.cs | 34 +++-- src/shared/utils/streamingText.ts | 48 +------ tests/shared/streamingText.test.ts | 27 +++- 4 files changed, 79 insertions(+), 166 deletions(-) diff --git a/sidecar/src/Aryx.AgentHost/Services/StreamingTextMerger.cs b/sidecar/src/Aryx.AgentHost/Services/StreamingTextMerger.cs index c6932ca..464363f 100644 --- a/sidecar/src/Aryx.AgentHost/Services/StreamingTextMerger.cs +++ b/sidecar/src/Aryx.AgentHost/Services/StreamingTextMerger.cs @@ -1,5 +1,3 @@ -using System.Text.RegularExpressions; - namespace Aryx.AgentHost.Services; internal static partial class StreamingTextMerger @@ -7,7 +5,6 @@ internal static partial class StreamingTextMerger private const double SnapshotReplacementMinLengthRatio = 0.6; private const int SnapshotReplacementMinTokenCount = 3; private const double SnapshotReplacementSharedTokenRatio = 0.5; - private const string CharactersThatDoNotNeedLeadingSpace = "([{/\"'`"; public static string Merge(string current, string incoming) { @@ -32,51 +29,7 @@ internal static partial class StreamingTextMerger return incoming; } - return current + ResolveBoundarySeparator(current, incoming) + incoming; - } - - private static bool TryMergeSnapshotVariants(string current, string incoming, out string merged) - { - if (incoming.StartsWith(current, StringComparison.Ordinal) - || incoming.Contains(current, StringComparison.Ordinal)) - { - merged = incoming; - return true; - } - - if (current.Contains(incoming, StringComparison.Ordinal)) - { - merged = current; - return true; - } - - merged = string.Empty; - return false; - } - - private static bool TryMergeByOverlap(string current, string incoming, out string merged) - { - int overlapLength = ComputeSuffixPrefixOverlap(current, incoming); - if (overlapLength == 0) - { - merged = string.Empty; - return false; - } - - merged = current + incoming[overlapLength..]; - return true; - } - - private static string ResolveBoundarySeparator(string current, string incoming) - { - if (ShouldInsertNewlineBoundary(current, incoming)) - { - return "\n"; - } - - return ShouldInsertSpaceBoundary(current, incoming) - ? " " - : string.Empty; + return current + incoming; } private static int ComputeSuffixPrefixOverlap(string current, string incoming) @@ -125,54 +78,6 @@ internal static partial class StreamingTextMerger && incomingTokens.Count >= SnapshotReplacementMinTokenCount; } - private static bool ShouldInsertNewlineBoundary(string current, string incoming) - { - return !current.EndsWith('\n') - && MarkdownBlockPrefixRegex().IsMatch(incoming.TrimStart()); - } - - private static bool ShouldInsertSpaceBoundary(string current, string incoming) - { - char lastCharacter = current[^1]; - char firstCharacter = incoming[0]; - if (HasExistingBoundary(lastCharacter, firstCharacter) - || CharactersThatDoNotNeedLeadingSpace.Contains(lastCharacter)) - { - return false; - } - - if (ClosingPunctuationRegex().IsMatch(incoming)) - { - return false; - } - - return StartsLikeASeparatedInlineFragment(firstCharacter, incoming) - || LooksLikeWordBoundary(current, incoming); - } - - private static bool HasExistingBoundary(char lastCharacter, char firstCharacter) - { - return char.IsWhiteSpace(lastCharacter) || char.IsWhiteSpace(firstCharacter); - } - - private static bool StartsLikeASeparatedInlineFragment(char firstCharacter, string incoming) - { - return MarkdownInlinePrefixRegex().IsMatch(incoming) - || char.IsUpper(firstCharacter) - || char.IsDigit(firstCharacter); - } - - private static bool LooksLikeWordBoundary(string current, string incoming) - { - string[] currentTokens = Tokenize(current).ToArray(); - string[] incomingTokens = Tokenize(incoming).ToArray(); - string firstIncomingToken = incomingTokens.FirstOrDefault() ?? string.Empty; - - return currentTokens.Length >= 2 - && incomingTokens.Length >= 2 - && firstIncomingToken.Length >= 2; - } - private static IEnumerable Tokenize(string value) { return TokenRegex() @@ -181,15 +86,38 @@ internal static partial class StreamingTextMerger .Where(token => token.Length > 0); } - [GeneratedRegex("[a-z0-9]+", RegexOptions.IgnoreCase)] - private static partial Regex TokenRegex(); + private static bool TryMergeSnapshotVariants(string current, string incoming, out string merged) + { + if (incoming.StartsWith(current, StringComparison.Ordinal) + || incoming.Contains(current, StringComparison.Ordinal)) + { + merged = incoming; + return true; + } - [GeneratedRegex(@"^[.,!?;:%)\]}]")] - private static partial Regex ClosingPunctuationRegex(); + if (current.Contains(incoming, StringComparison.Ordinal)) + { + merged = current; + return true; + } - [GeneratedRegex(@"^[*_`~\[]")] - private static partial Regex MarkdownInlinePrefixRegex(); + merged = string.Empty; + return false; + } - [GeneratedRegex(@"^(?:#{1,6}\s|[-*+]\s|\d+\.\s|>\s|```)", RegexOptions.Singleline)] - private static partial Regex MarkdownBlockPrefixRegex(); + private static bool TryMergeByOverlap(string current, string incoming, out string merged) + { + int overlapLength = ComputeSuffixPrefixOverlap(current, incoming); + if (overlapLength == 0) + { + merged = string.Empty; + return false; + } + + merged = current + incoming[overlapLength..]; + return true; + } + + [System.Text.RegularExpressions.GeneratedRegex("[a-z0-9]+", System.Text.RegularExpressions.RegexOptions.IgnoreCase)] + private static partial System.Text.RegularExpressions.Regex TokenRegex(); } diff --git a/sidecar/tests/Aryx.AgentHost.Tests/StreamingTextMergerTests.cs b/sidecar/tests/Aryx.AgentHost.Tests/StreamingTextMergerTests.cs index ded8ef0..07bd2f7 100644 --- a/sidecar/tests/Aryx.AgentHost.Tests/StreamingTextMergerTests.cs +++ b/sidecar/tests/Aryx.AgentHost.Tests/StreamingTextMergerTests.cs @@ -37,22 +37,36 @@ public sealed class StreamingTextMergerTests Assert.Equal(incoming, StreamingTextMerger.Merge(current, incoming)); } - [Fact] - public void Merge_InsertsWhitespaceWhenSnapshotLikeUpdatesWouldOtherwiseGlueWordsTogether() + [Theory] + [InlineData("requires all wr", "itable fields", "requires all writable fields")] + [InlineData("becomes frag", "ile for clients", "becomes fragile for clients")] + [InlineData("Endpoint (domain) uniqu", "eness across tenants", "Endpoint (domain) uniqueness across tenants")] + [InlineData("The doc says \"wildc", "ards are allowed\"", "The doc says \"wildcards are allowed\"")] + [InlineData("What wildcard syntax supported (*.cont", "oso.com? contoso.* ?)", "What wildcard syntax supported (*.contoso.com? contoso.* ?)")] + [InlineData("How does Pur", "view match traffic", "How does Purview match traffic")] + [InlineData("more M", "DA properties", "more MDA properties")] + [InlineData("does UA", "G normalize them?", "does UAG normalize them?")] + public void Merge_DoesNotInjectSpacesIntoSplitWords(string current, string incoming, string expected) { - Assert.Equal( - "How about The **Ashen Crown** feels", - StreamingTextMerger.Merge("How about", "The **Ashen Crown** feels")); - Assert.Equal( - "The **Ashen Crown** feels classic and timeless.", - StreamingTextMerger.Merge("The **Ashen Crown** feels", "classic and timeless.")); + Assert.Equal(expected, StreamingTextMerger.Merge(current, incoming)); } [Fact] - public void Merge_InsertsNewlineBeforeStreamedMarkdownBlockMarkers() + public void Merge_PreservesWhitespaceAlreadyPresentInDelta() + { + Assert.Equal( + "How about The **Ashen Crown** feels", + StreamingTextMerger.Merge("How about", " The **Ashen Crown** feels")); + Assert.Equal( + "The **Ashen Crown** feels classic and timeless.", + StreamingTextMerger.Merge("The **Ashen Crown** feels", " classic and timeless.")); + } + + [Fact] + public void Merge_PreservesNewlineAlreadyPresentInDelta() { Assert.Equal( "If you want, I can also give you\n- darker titles", - StreamingTextMerger.Merge("If you want, I can also give you", "- darker titles")); + StreamingTextMerger.Merge("If you want, I can also give you", "\n- darker titles")); } } diff --git a/src/shared/utils/streamingText.ts b/src/shared/utils/streamingText.ts index 33fd2a7..9e34acd 100644 --- a/src/shared/utils/streamingText.ts +++ b/src/shared/utils/streamingText.ts @@ -5,52 +5,6 @@ function tokenize(value: string): string[] { .filter((token) => token.length > 0); } -function shouldInsertNewlineBoundary(current: string, incoming: string): boolean { - if (current.endsWith('\n')) { - return false; - } - - return /^(?:#{1,6}\s|[-*+]\s|\d+\.\s|>\s|```)/.test(incoming.trimStart()); -} - -function shouldInsertSpaceBoundary(current: string, incoming: string): boolean { - const lastChar = current.at(-1); - const firstChar = incoming[0]; - if (!lastChar || !firstChar) { - return false; - } - - if (/\s/.test(lastChar) || /\s/.test(firstChar) || /[([{/"'`]/.test(lastChar)) { - return false; - } - - if (/^[.,!?;:%)\]}]/.test(firstChar)) { - return false; - } - - if (/^[*_`~\[]/.test(firstChar) || /^[A-Z0-9]/.test(firstChar)) { - return true; - } - - const currentTokens = tokenize(current); - const incomingTokens = tokenize(incoming); - const firstIncomingToken = incomingTokens[0] ?? ''; - - return currentTokens.length >= 2 && incomingTokens.length >= 2 && firstIncomingToken.length >= 2; -} - -function appendWithNaturalBoundary(current: string, incoming: string): string { - if (shouldInsertNewlineBoundary(current, incoming)) { - return `${current}\n${incoming}`; - } - - if (shouldInsertSpaceBoundary(current, incoming)) { - return `${current} ${incoming}`; - } - - return current + incoming; -} - function computeSuffixPrefixOverlap(current: string, incoming: string): number { const maxOverlap = Math.min(current.length, incoming.length); for (let length = maxOverlap; length > 0; length -= 1) { @@ -109,5 +63,5 @@ export function mergeStreamingText(current: string, incoming: string): string { return incoming; } - return appendWithNaturalBoundary(current, incoming); + return current + incoming; } diff --git a/tests/shared/streamingText.test.ts b/tests/shared/streamingText.test.ts index 2544e21..66de246 100644 --- a/tests/shared/streamingText.test.ts +++ b/tests/shared/streamingText.test.ts @@ -26,17 +26,34 @@ describe('streaming text merge', () => { expect(mergeStreamingText(current, incoming)).toBe(incoming); }); - test('inserts whitespace when snapshot-like updates would otherwise glue words together', () => { - expect(mergeStreamingText('How about', 'The **Ashen Crown** feels')).toBe( + test.each([ + ['requires all wr', 'itable fields', 'requires all writable fields'], + ['becomes frag', 'ile for clients', 'becomes fragile for clients'], + ['Endpoint (domain) uniqu', 'eness across tenants', 'Endpoint (domain) uniqueness across tenants'], + ['The doc says "wildc', 'ards are allowed"', 'The doc says "wildcards are allowed"'], + [ + 'What wildcard syntax supported (*.cont', + 'oso.com? contoso.* ?)', + 'What wildcard syntax supported (*.contoso.com? contoso.* ?)', + ], + ['How does Pur', 'view match traffic', 'How does Purview match traffic'], + ['more M', 'DA properties', 'more MDA properties'], + ['does UA', 'G normalize them?', 'does UAG normalize them?'], + ])('does not inject spaces into split words: %s + %s', (current, incoming, expected) => { + expect(mergeStreamingText(current, incoming)).toBe(expected); + }); + + test('preserves whitespace already present in delta', () => { + expect(mergeStreamingText('How about', ' The **Ashen Crown** feels')).toBe( 'How about The **Ashen Crown** feels', ); - expect(mergeStreamingText('The **Ashen Crown** feels', 'classic and timeless.')).toBe( + expect(mergeStreamingText('The **Ashen Crown** feels', ' classic and timeless.')).toBe( 'The **Ashen Crown** feels classic and timeless.', ); }); - test('inserts a newline before streamed markdown block markers', () => { - expect(mergeStreamingText('If you want, I can also give you', '- darker titles')).toBe( + test('preserves newline already present in delta', () => { + expect(mergeStreamingText('If you want, I can also give you', '\n- darker titles')).toBe( 'If you want, I can also give you\n- darker titles', ); });