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', ); });