-
Notifications
You must be signed in to change notification settings - Fork 8.3k
fix: Prevent duplicate code blocks during streaming #11261
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThe changes modify how code blocks are detected in the frontend utility. The Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 7✅ Passed checks (7 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. ❌ Your project status has failed because the head coverage (40.63%) is below the target coverage (60.00%). You can increase the head coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## main #11261 +/- ##
==========================================
- Coverage 34.20% 34.05% -0.16%
==========================================
Files 1409 1407 -2
Lines 66882 66640 -242
Branches 9858 9836 -22
==========================================
- Hits 22879 22696 -183
+ Misses 42810 42760 -50
+ Partials 1193 1184 -9
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/frontend/src/utils/__tests__/streaming-bug-simulation.test.ts (1)
81-87: Remove console.log statements from test file.The test file contains multiple
console.logstatements that will clutter the test output. While they may have been useful during development to demonstrate the bug, they should be removed before merging.If you want to preserve this information, consider:
- Adding the details to test descriptions or comments
- Using a testing library's output mechanism if available
- Removing them entirely since the assertions already validate the behavior
♻️ Suggested cleanup
Remove all
console.logstatements from the test file (lines 81-87, 108-110, 134-136, 161-166, 185-187, 200-207).Also applies to: 108-110, 134-136, 161-166, 185-187, 200-207
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/frontend/src/utils/__tests__/codeBlockUtils.test.tssrc/frontend/src/utils/__tests__/streaming-bug-simulation.test.tssrc/frontend/src/utils/codeBlockUtils.ts
🧰 Additional context used
📓 Path-based instructions (5)
src/frontend/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/frontend_development.mdc)
src/frontend/src/**/*.{ts,tsx}: Use React 18 with TypeScript for frontend development
Use Zustand for state management
Files:
src/frontend/src/utils/codeBlockUtils.tssrc/frontend/src/utils/__tests__/streaming-bug-simulation.test.tssrc/frontend/src/utils/__tests__/codeBlockUtils.test.ts
src/frontend/src/utils/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/frontend_development.mdc)
Use named exports for utility functions in the utils/ directory
Files:
src/frontend/src/utils/codeBlockUtils.tssrc/frontend/src/utils/__tests__/streaming-bug-simulation.test.tssrc/frontend/src/utils/__tests__/codeBlockUtils.test.ts
src/frontend/**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/frontend_development.mdc)
Write component tests using React Testing Library with render(), screen, and fireEvent utilities
Files:
src/frontend/src/utils/__tests__/streaming-bug-simulation.test.tssrc/frontend/src/utils/__tests__/codeBlockUtils.test.ts
src/frontend/**/*.{test,spec}.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/testing.mdc)
src/frontend/**/*.{test,spec}.{ts,tsx,js,jsx}: Use@pytest.mark.asynciodecorator for async frontend tests; structure tests to verify component behavior, state updates, and proper cleanup
Frontend tests should validate component rendering, user interactions, state management, and async operations using appropriate testing libraries (React Testing Library, Vitest, Jest, etc.)
Document each frontend test with a clear docstring/comment explaining its purpose, the scenario being tested, and expected outcomes
Files:
src/frontend/src/utils/__tests__/streaming-bug-simulation.test.tssrc/frontend/src/utils/__tests__/codeBlockUtils.test.ts
**/*.test.ts?(x)
📄 CodeRabbit inference engine (Custom checks)
**/*.test.ts?(x): Review test files for excessive use of mocks that may indicate poor test design - check if tests have too many mock objects that obscure what's actually being tested
Warn when mocks are used instead of testing real behavior and interactions, and suggest using real objects or test doubles when mocks become excessive
Ensure mocks are used appropriately for external dependencies only, not for core logic
Frontend test files should follow the naming convention *.test.ts or *.test.tsx using Playwright
Test files should have descriptive test function names that explain what is being tested
Tests should be organized logically with proper setup and teardown
Consider including edge cases and error conditions for comprehensive test coverage
Verify tests cover both positive and negative scenarios where appropriate
Files:
src/frontend/src/utils/__tests__/streaming-bug-simulation.test.tssrc/frontend/src/utils/__tests__/codeBlockUtils.test.ts
🧠 Learnings (6)
📓 Common learnings
Learnt from: CR
Repo: langflow-ai/langflow PR: 0
File: coderabbit-custom-pre-merge-checks-unique-id-file-non-traceable-F7F2B60C-1728-4C9A-8889-4F2235E186CA.txt:0-0
Timestamp: 2025-12-03T18:17:26.561Z
Learning: For new components or functionality, ensure corresponding test files are included in the PR
Learnt from: CR
Repo: langflow-ai/langflow PR: 0
File: .cursor/rules/docs_development.mdc:0-0
Timestamp: 2025-11-24T19:46:26.770Z
Learning: Applies to docs/docs/**/*.{md,mdx} : Code blocks must include a title attribute and specify the language (e.g., ```python title="filename.py")
📚 Learning: 2025-12-03T18:17:26.561Z
Learnt from: CR
Repo: langflow-ai/langflow PR: 0
File: coderabbit-custom-pre-merge-checks-unique-id-file-non-traceable-F7F2B60C-1728-4C9A-8889-4F2235E186CA.txt:0-0
Timestamp: 2025-12-03T18:17:26.561Z
Learning: Applies to **/*.test.ts?(x) : Verify tests cover both positive and negative scenarios where appropriate
Applied to files:
src/frontend/src/utils/__tests__/streaming-bug-simulation.test.tssrc/frontend/src/utils/__tests__/codeBlockUtils.test.ts
📚 Learning: 2025-12-03T18:17:26.561Z
Learnt from: CR
Repo: langflow-ai/langflow PR: 0
File: coderabbit-custom-pre-merge-checks-unique-id-file-non-traceable-F7F2B60C-1728-4C9A-8889-4F2235E186CA.txt:0-0
Timestamp: 2025-12-03T18:17:26.561Z
Learning: Applies to **/*.test.ts?(x) : Consider including edge cases and error conditions for comprehensive test coverage
Applied to files:
src/frontend/src/utils/__tests__/streaming-bug-simulation.test.tssrc/frontend/src/utils/__tests__/codeBlockUtils.test.ts
📚 Learning: 2025-12-03T18:17:26.561Z
Learnt from: CR
Repo: langflow-ai/langflow PR: 0
File: coderabbit-custom-pre-merge-checks-unique-id-file-non-traceable-F7F2B60C-1728-4C9A-8889-4F2235E186CA.txt:0-0
Timestamp: 2025-12-03T18:17:26.561Z
Learning: Applies to **/*.test.ts?(x) : Review test files for excessive use of mocks that may indicate poor test design - check if tests have too many mock objects that obscure what's actually being tested
Applied to files:
src/frontend/src/utils/__tests__/streaming-bug-simulation.test.tssrc/frontend/src/utils/__tests__/codeBlockUtils.test.ts
📚 Learning: 2025-12-03T18:17:26.561Z
Learnt from: CR
Repo: langflow-ai/langflow PR: 0
File: coderabbit-custom-pre-merge-checks-unique-id-file-non-traceable-F7F2B60C-1728-4C9A-8889-4F2235E186CA.txt:0-0
Timestamp: 2025-12-03T18:17:26.561Z
Learning: Applies to **/*.test.ts?(x) : Ensure mocks are used appropriately for external dependencies only, not for core logic
Applied to files:
src/frontend/src/utils/__tests__/streaming-bug-simulation.test.ts
📚 Learning: 2025-11-24T19:47:28.997Z
Learnt from: CR
Repo: langflow-ai/langflow PR: 0
File: .cursor/rules/testing.mdc:0-0
Timestamp: 2025-11-24T19:47:28.997Z
Learning: Applies to src/frontend/**/*.{test,spec}.{ts,tsx,js,jsx} : Document each frontend test with a clear docstring/comment explaining its purpose, the scenario being tested, and expected outcomes
Applied to files:
src/frontend/src/utils/__tests__/codeBlockUtils.test.ts
🧬 Code graph analysis (1)
src/frontend/src/utils/__tests__/codeBlockUtils.test.ts (1)
src/frontend/src/utils/codeBlockUtils.ts (1)
isCodeBlock(18-27)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Test Docker Images / Test docker images
- GitHub Check: Test Starter Templates
🔇 Additional comments (7)
src/frontend/src/utils/__tests__/streaming-bug-simulation.test.ts (3)
1-27: Excellent documentation of the bug and fix.The test file structure clearly demonstrates:
- The original bug behavior with
isCodeBlockOLD- The root cause (newlines being used as a criterion)
- The contrast with the fixed implementation
This serves as valuable documentation for future maintainers.
12-23: Clear demonstration of the buggy behavior.The
isCodeBlockOLDfunction effectively recreates the problematic logic that caused duplicate code blocks during streaming. The inline comment on line 20 clearly identifies the root cause.
60-112: Comprehensive test coverage of the bug.The test cases thoroughly demonstrate both the problematic OLD behavior and the corrected NEW behavior across various streaming scenarios. The side-by-side comparison makes the fix clear.
Based on learnings: The tests appropriately cover both positive (working correctly) and negative (buggy behavior) scenarios.
src/frontend/src/utils/__tests__/codeBlockUtils.test.ts (3)
39-41: Well-documented behavior change.The comment clearly explains why newlines are not used as a criterion, and the accompanying tests validate all combinations of newlines with/without language markers. This ensures the fix works as intended.
Also applies to: 102-125
128-209: Excellent streaming scenario coverage.The new streaming scenarios section comprehensively tests the bug fix across multiple dimensions:
- Streaming chunks with/without markers
- Multiple newline counts
- Incomplete code blocks
- Code-like text without markers
Based on learnings: This section includes appropriate edge cases and validates both positive (correctly identified blocks) and negative (text that should not be blocks) scenarios.
The test at lines 151-170 directly validates the fix for the reported bug, ensuring duplicate blocks are not created during streaming.
1-253: Comprehensive test coverage with clear organization.The test file is well-structured with logical sections covering:
- Positive cases (language markers present)
- Negative cases (no markers)
- Edge cases (null props, empty strings, etc.)
- Streaming scenarios (the core bug fix)
The test suite maintains backward compatibility by preserving all
extractLanguagetests while thoroughly updatingisCodeBlocktests to validate the new marker-based detection logic.Based on learnings: The tests appropriately document each test's purpose through descriptive names and comments, validate both positive and negative scenarios, and include comprehensive edge case coverage.
src/frontend/src/utils/codeBlockUtils.ts (1)
23-26: The regex pattern correctly handles all language identifiers produced by react-markdown.All practical language identifiers in the codebase (python, javascript, typescript, es2020, es6) match the regex
/language-\w+/. The concern about special characters (C++, C#, F#) is theoretical—markdown parsers normalize these to word-character-only formats (cpp, csharp, fsharp). No changes needed.Likely an incorrect or invalid review comment.
lucaseduoli
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
This pull request addresses a bug in code block detection during streaming, which previously caused code blocks to appear duplicated or split. The key fix is to ensure that only elements with a language class or a
data-languageattribute are treated as code blocks, and not those with newlines alone. This prevents fragmented code from being rendered as multiple blocks during streaming. The changes are thoroughly tested and documented, including a simulation of the original bug and its resolution.Bug fix and improved code block detection:
isCodeBlockincodeBlockUtils.tsto only consider elements with a language class ordata-languageattribute as code blocks, removing the previous check for newlines in the content. This prevents duplicate or broken code blocks during streaming.Test improvements and documentation:
codeBlockUtils.test.tsto reflect the new logic, ensuring that newlines alone do not trigger code block rendering and covering various streaming scenarios. [1] [2]streaming-bug-simulation.test.tsthat simulates the original bug, demonstrates the old vs. new behavior, and documents the root cause and fix for future reference.Summary by CodeRabbit
Bug Fixes
Tests
✏️ Tip: You can customize this high-level summary in your review settings.