diff --git a/src/frontend/src/utils/__tests__/codeBlockUtils.test.ts b/src/frontend/src/utils/__tests__/codeBlockUtils.test.ts index 7e5045197987..16f40369acf3 100644 --- a/src/frontend/src/utils/__tests__/codeBlockUtils.test.ts +++ b/src/frontend/src/utils/__tests__/codeBlockUtils.test.ts @@ -36,15 +36,9 @@ describe("codeBlockUtils", () => { expect(result).toBe(true); }); - it("should_return_true_when_content_has_newlines", () => { - const result = isCodeBlock(undefined, {}, "line1\nline2\nline3"); - expect(result).toBe(true); - }); - - it("should_return_true_when_content_has_single_newline", () => { - const result = isCodeBlock(undefined, {}, "line1\nline2"); - expect(result).toBe(true); - }); + // Note: Content with newlines alone is NOT considered a code block. + // This is intentional to prevent duplicate code block rendering during streaming. + // react-markdown identifies code blocks through language class or data-language attribute. it("should_return_true_when_multiple_conditions_are_met", () => { const result = isCodeBlock( @@ -99,32 +93,119 @@ describe("codeBlockUtils", () => { expect(result).toBe(true); }); - it("should_handle_content_with_carriage_return_only", () => { - // \r alone should not be treated as newline for code block - const result = isCodeBlock(undefined, {}, "line1\rline2"); + it("should_handle_null_props_gracefully", () => { + // TypeScript would prevent this, but testing runtime safety + const result = isCodeBlock(undefined, null as any, "code"); expect(result).toBe(false); }); - it("should_handle_content_with_crlf", () => { - const result = isCodeBlock(undefined, {}, "line1\r\nline2"); - expect(result).toBe(true); + it("should_return_false_for_content_with_newlines_but_no_language_indicator", () => { + // Newlines alone should NOT make content a code block + // This prevents duplicate rendering during streaming + const result = isCodeBlock(undefined, {}, "line1\nline2\nline3"); + expect(result).toBe(false); }); - it("should_handle_content_with_trailing_newline_only", () => { - const result = isCodeBlock(undefined, {}, "single line\n"); + it("should_return_true_for_content_with_newlines_AND_language_class", () => { + const result = isCodeBlock( + "language-python", + {}, + "def hello():\n print('world')", + ); expect(result).toBe(true); }); - it("should_handle_content_with_leading_newline_only", () => { - const result = isCodeBlock(undefined, {}, "\nsingle line"); + it("should_return_true_for_content_with_newlines_AND_data_language", () => { + const result = isCodeBlock( + undefined, + { "data-language": "python" }, + "def hello():\n print('world')", + ); expect(result).toBe(true); }); + }); - it("should_handle_null_props_gracefully", () => { - // TypeScript would prevent this, but testing runtime safety - const result = isCodeBlock(undefined, null as any, "code"); + describe("streaming scenarios", () => { + it("should_not_treat_partial_streaming_content_as_block", () => { + // During streaming, content may arrive with newlines but without + // proper language indicators. This should NOT be treated as a block. + const partialContent = "def hello():\n print('world')"; + const result = isCodeBlock(undefined, {}, partialContent); expect(result).toBe(false); }); + + it("should_treat_properly_marked_streaming_content_as_block", () => { + // When react-markdown properly identifies code, it adds language class + const partialContent = "def hello():\n print('world')"; + const result = isCodeBlock("language-python", {}, partialContent); + expect(result).toBe(true); + }); + + it("should_handle_code_block_without_language_via_data_language", () => { + // Code blocks without language (```) get data-language attribute + const content = "some code\nwith newlines"; + const result = isCodeBlock(undefined, { "data-language": "" }, content); + expect(result).toBe(true); + }); + + it("should_prevent_duplicate_blocks_during_streaming", () => { + // This test documents the bug fix: + // During streaming, if content with newlines was treated as a code block, + // it would cause the same code to appear as multiple blocks. + // By only using language class and data-language, we prevent this. + + // Scenario: code is being streamed and arrives in chunks with newlines + const streamingChunk1 = + "output_data = {\n 'total_iterations': iteration_count,"; + const streamingChunk2 = + "\n 'termination_reason': termination_reason,\n 'results': results,"; + + // Without proper language marker, these should NOT be code blocks + expect(isCodeBlock(undefined, {}, streamingChunk1)).toBe(false); + expect(isCodeBlock(undefined, {}, streamingChunk2)).toBe(false); + + // With proper language marker, they should be code blocks + expect(isCodeBlock("language-python", {}, streamingChunk1)).toBe(true); + expect(isCodeBlock("language-python", {}, streamingChunk2)).toBe(true); + }); + + it("should_handle_multiline_code_consistently_regardless_of_newline_count", () => { + // All these should behave the same - only language marker matters + const singleLine = "print('hello')"; + const twoLines = "x = 1\ny = 2"; + const manyLines = "x = 1\ny = 2\nz = 3\nprint(x + y + z)"; + + // Without language marker, all should be inline + expect(isCodeBlock(undefined, {}, singleLine)).toBe(false); + expect(isCodeBlock(undefined, {}, twoLines)).toBe(false); + expect(isCodeBlock(undefined, {}, manyLines)).toBe(false); + + // With language marker, all should be blocks + expect(isCodeBlock("language-python", {}, singleLine)).toBe(true); + expect(isCodeBlock("language-python", {}, twoLines)).toBe(true); + expect(isCodeBlock("language-python", {}, manyLines)).toBe(true); + }); + + it("should_not_create_block_from_text_that_looks_like_code", () => { + // Text content that happens to look like code should not become a block + // unless properly marked by the markdown parser + const codelikeText = ` + "Condition threshold reached" + if iteration_count >= condition_threshold + else "Max iterations reached" + `; + expect(isCodeBlock(undefined, {}, codelikeText)).toBe(false); + }); + + it("should_handle_incomplete_code_blocks_during_streaming", () => { + // When streaming, code blocks may be incomplete (missing closing ```) + // The parser may not assign language class yet + const incompleteBlock = "def hello():\n print('world')"; + expect(isCodeBlock(undefined, {}, incompleteBlock)).toBe(false); + + // Once properly parsed, it will have the language class + expect(isCodeBlock("language-python", {}, incompleteBlock)).toBe(true); + }); }); }); diff --git a/src/frontend/src/utils/__tests__/streaming-bug-simulation.test.ts b/src/frontend/src/utils/__tests__/streaming-bug-simulation.test.ts new file mode 100644 index 000000000000..91483c414c06 --- /dev/null +++ b/src/frontend/src/utils/__tests__/streaming-bug-simulation.test.ts @@ -0,0 +1,214 @@ +/** + * This test simulates the bug reported by Rodrigo: + * - During streaming, code blocks appeared duplicated/split into 2 blocks + * - When leaving and returning to playground, it rendered correctly + * + * Root cause: The old isCodeBlock() used `hasNewlines` as a criterion, + * which caused content with newlines to be treated as code blocks + * even without proper language markers. + */ + +// Simulate the OLD buggy implementation +function isCodeBlockOLD( + className: string | undefined, + props: Record | undefined, + content: string, +): boolean { + const languageMatch = /language-(\w+)/.exec(className ?? ""); + const hasLanguageClass = !!languageMatch; + const hasDataLanguage = "data-language" in (props ?? {}); + const hasNewlines = content.includes("\n"); // BUG: This caused the issue! + + return hasLanguageClass || hasDataLanguage || hasNewlines; +} + +// Import the NEW fixed implementation +import { isCodeBlock as isCodeBlockNEW } from "../codeBlockUtils"; + +describe("Streaming Bug Simulation - Rodrigo's Report", () => { + describe("Scenario: Code being streamed in chunks", () => { + /** + * During streaming, react-markdown processes content progressively. + * The markdown parser may create multiple elements when + * the code block is incomplete or being updated. + * + * Example markdown being streamed: + * ```python + * output_data = { + * "total_iterations": iteration_count, + * "termination_reason": termination_reason, + * } + * ``` + * + * During streaming, this might be split into fragments by the parser. + */ + + const streamingFragment1 = `output_data = { + "total_iterations": iteration_count,`; + + const streamingFragment2 = ` + "termination_reason": termination_reason, + "results": results, +}`; + + const completeCode = `output_data = { + "total_iterations": iteration_count, + "termination_reason": termination_reason, + "results": results, +}`; + + describe("OLD buggy behavior (before fix)", () => { + it("should_incorrectly_treat_streaming_fragments_as_code_blocks", () => { + // Without language class or data-language, but WITH newlines + // OLD implementation would return TRUE (BUG!) + + const fragment1IsBlock = isCodeBlockOLD( + undefined, + {}, + streamingFragment1, + ); + const fragment2IsBlock = isCodeBlockOLD( + undefined, + {}, + streamingFragment2, + ); + + // BUG: Both fragments are treated as separate code blocks! + expect(fragment1IsBlock).toBe(true); // Wrong! Should be false + expect(fragment2IsBlock).toBe(true); // Wrong! Should be false + + // This caused the visual bug: 2 separate code blocks rendered + console.log("OLD BEHAVIOR (BUGGY):"); + console.log( + `Fragment 1 is block: ${fragment1IsBlock} (should be false)`, + ); + console.log( + `Fragment 2 is block: ${fragment2IsBlock} (should be false)`, + ); + }); + + it("should_show_why_duplicate_blocks_appeared", () => { + // Simulating what happens during streaming: + // 1. First chunk arrives with newlines -> treated as block + // 2. Second chunk arrives with newlines -> treated as ANOTHER block + // Result: 2 code blocks instead of 1! + + const chunks = [ + "def hello():\n print('world')", + "\n\nself.status = f'Completed'", + "\n\nreturn Data(data=output_data)", + ]; + + const blocksCreated = chunks.filter((chunk) => + isCodeBlockOLD(undefined, {}, chunk), + ); + + // BUG: All 3 chunks become separate blocks! + expect(blocksCreated.length).toBe(3); + console.log( + `OLD: ${blocksCreated.length} blocks created from ${chunks.length} chunks`, + ); + }); + }); + + describe("NEW fixed behavior (after fix)", () => { + it("should_not_treat_streaming_fragments_as_code_blocks_without_language_marker", () => { + // Without language class or data-language + // NEW implementation correctly returns FALSE + + const fragment1IsBlock = isCodeBlockNEW( + undefined, + {}, + streamingFragment1, + ); + const fragment2IsBlock = isCodeBlockNEW( + undefined, + {}, + streamingFragment2, + ); + + // FIXED: Neither fragment is treated as a code block + expect(fragment1IsBlock).toBe(false); + expect(fragment2IsBlock).toBe(false); + + console.log("NEW BEHAVIOR (FIXED):"); + console.log(`Fragment 1 is block: ${fragment1IsBlock} (correct!)`); + console.log(`Fragment 2 is block: ${fragment2IsBlock} (correct!)`); + }); + + it("should_only_create_one_block_when_properly_marked", () => { + // When react-markdown properly identifies the code block, + // it adds the language class. Only then should it be a block. + + const chunks = [ + "def hello():\n print('world')", + "\n\nself.status = f'Completed'", + "\n\nreturn Data(data=output_data)", + ]; + + // Without language markers: no blocks + const blocksWithoutMarker = chunks.filter((chunk) => + isCodeBlockNEW(undefined, {}, chunk), + ); + expect(blocksWithoutMarker.length).toBe(0); + + // With language marker: all become blocks (as expected when properly parsed) + const blocksWithMarker = chunks.filter((chunk) => + isCodeBlockNEW("language-python", {}, chunk), + ); + expect(blocksWithMarker.length).toBe(3); + + console.log( + `NEW: ${blocksWithoutMarker.length} blocks without marker (correct: 0)`, + ); + console.log( + `NEW: ${blocksWithMarker.length} blocks with marker (correct: all)`, + ); + }); + + it("should_handle_complete_code_correctly_when_streaming_ends", () => { + // When streaming completes and markdown is re-parsed, + // the code block gets proper language class + + // During streaming (no marker yet) + const duringStreaming = isCodeBlockNEW(undefined, {}, completeCode); + expect(duringStreaming).toBe(false); + + // After streaming (parser adds language class) + const afterStreaming = isCodeBlockNEW( + "language-python", + {}, + completeCode, + ); + expect(afterStreaming).toBe(true); + + console.log("Complete code handling:"); + console.log(`During streaming (no marker): ${duringStreaming}`); + console.log(`After streaming (with marker): ${afterStreaming}`); + }); + }); + + describe("Visual comparison of the bug", () => { + it("should_demonstrate_the_difference", () => { + const testContent = `"Condition threshold reached" +if iteration_count >= condition_threshold +else "Max iterations reached"`; + + const oldResult = isCodeBlockOLD(undefined, {}, testContent); + const newResult = isCodeBlockNEW(undefined, {}, testContent); + + console.log("\n=== BUG DEMONSTRATION ==="); + console.log("Content with newlines but NO language marker:"); + console.log( + `OLD isCodeBlock(): ${oldResult} -> Would render as code block (BUG)`, + ); + console.log( + `NEW isCodeBlock(): ${newResult} -> Correctly NOT a code block (FIXED)`, + ); + + expect(oldResult).toBe(true); // Bug + expect(newResult).toBe(false); // Fixed + }); + }); + }); +}); diff --git a/src/frontend/src/utils/codeBlockUtils.ts b/src/frontend/src/utils/codeBlockUtils.ts index f8284050e3b0..34fc1a9c18dc 100644 --- a/src/frontend/src/utils/codeBlockUtils.ts +++ b/src/frontend/src/utils/codeBlockUtils.ts @@ -5,19 +5,25 @@ * A code element is considered a block if any of the following conditions are met: * 1. It has a language class (e.g., "language-python") * 2. It has the "data-language" attribute (from some markdown parsers) - * 3. The content contains newlines (multi-line code) + * + * Note: We intentionally do NOT use newlines as a criterion for detecting code blocks. + * During streaming, react-markdown may create multiple code elements for a single + * code block, and using newlines would cause each fragment to be rendered as a + * separate block, resulting in duplicated/broken code block rendering. + * + * @param className - CSS class name that may contain language identifier + * @param props - Element props that may contain data-language attribute + * @param _content - Unused. Kept for backward compatibility with existing call sites. */ export function isCodeBlock( className: string | undefined, props: Record | undefined, - content: string, + _content?: string, ): boolean { - const languageMatch = /language-(\w+)/.exec(className ?? ""); - const hasLanguageClass = !!languageMatch; + const hasLanguageClass = /language-\w+/.test(className ?? ""); const hasDataLanguage = "data-language" in (props ?? {}); - const hasNewlines = content.includes("\n"); - return hasLanguageClass || hasDataLanguage || hasNewlines; + return hasLanguageClass || hasDataLanguage; } /**