fix: improves table formatting in the playground and adds Jest tests#8743
Conversation
- Introduced Jest configuration file to set up testing environment with TypeScript support and JSDOM. - Added setupTests.ts for global test configurations, including mocks for ResizeObserver and IntersectionObserver. - Updated package.json and package-lock.json to include Jest and related dependencies. - Implemented utility functions for processing markdown content, including handling tables and <think> tags. - Added comprehensive tests for markdown utility functions to ensure proper functionality.
- Removed frontend-related targets from the main Makefile and created a new Makefile.frontend to manage frontend-specific commands. - Updated the main Makefile to include a reference to the new frontend Makefile and added a help message for frontend commands. - This restructuring improves organization and clarity for managing backend and frontend build processes.
|
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 main Makefile was refactored to delegate all frontend-related targets to a new Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Makefile
participant Makefile.frontend
User->>Makefile: make build_frontend (or other frontend target)
Makefile->>Makefile.frontend: Delegates frontend target execution
Makefile.frontend-->>User: Runs appropriate frontend build/test/format command
sequenceDiagram
participant Developer
participant Jest
participant markdownUtils
Developer->>Jest: Run unit tests
Jest->>markdownUtils: Call isMarkdownTable / cleanupTableEmptyCells / preprocessChatMessage
markdownUtils-->>Jest: Return processed results
Jest-->>Developer: Report test results
Suggested labels
✨ Finishing Touches🧪 Generate Unit Tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
src/frontend/src/utils/markdownUtils.ts (2)
8-13: Consider enhancing the regex pattern for more robust table detection.The current regex
/\|.*\|.*\n\s*\|[\s\-:]+\|/mmay not handle all valid markdown table formats, particularly tables with minimal content or different spacing patterns.Consider this more comprehensive approach:
-export const isMarkdownTable = (text: string): boolean => { - if (!text?.trim()) return false; - - // Single regex to detect markdown table with header separator - return /\|.*\|.*\n\s*\|[\s\-:]+\|/m.test(text); -}; +export const isMarkdownTable = (text: string): boolean => { + if (!text?.trim()) return false; + + const lines = text.trim().split('\n'); + + // Look for at least 2 lines with pipes and a separator line + for (let i = 0; i < lines.length - 1; i++) { + if (lines[i].includes('|') && /^\s*\|[\s\-:]*\|\s*$/.test(lines[i + 1])) { + return true; + } + } + + return false; +};
40-52: Consider handling edge cases in think tag replacement.The current replacement wraps
<think>tags in backticks, but this may not be ideal if the tags are already within code blocks or have nested backticks.Consider adding more robust tag handling:
export const preprocessChatMessage = (text: string): string => { - // Handle <think> tags - let processed = text - .replace(/<think>/g, "`<think>`") - .replace(/<\/think>/g, "`</think>`"); + // Handle <think> tags - avoid double-wrapping if already in backticks + let processed = text + .replace(/(?<!`)<think>(?!`)/g, "`<think>`") + .replace(/(?<!`)<\/think>(?!`)/g, "`</think>`"); // Clean up tables if present if (isMarkdownTable(processed)) { processed = cleanupTableEmptyCells(processed); } return processed; };src/frontend/src/setupTests.ts (1)
33-62: Consider making console suppression more flexible.The current implementation suppresses very specific warnings, but this approach may become brittle as React versions change or new warnings are introduced.
Consider a more flexible approach:
-beforeAll(() => { - console.error = (...args) => { - if ( - typeof args[0] === "string" && - args[0].includes("Warning: ReactDOM.render is deprecated") - ) { - return; - } - originalError.call(console, ...args); - }; - - console.warn = (...args) => { - if ( - typeof args[0] === "string" && - args[0].includes("componentWillReceiveProps has been renamed") - ) { - return; - } - originalWarn.call(console, ...args); - }; -}); +beforeAll(() => { + // Suppress known test environment warnings + const suppressedPatterns = [ + /Warning: ReactDOM\.render is deprecated/, + /componentWillReceiveProps has been renamed/, + /componentWillUpdate has been renamed/ + ]; + + console.error = (...args) => { + if (typeof args[0] === "string" && + suppressedPatterns.some(pattern => pattern.test(args[0]))) { + return; + } + originalError.call(console, ...args); + }; + + console.warn = (...args) => { + if (typeof args[0] === "string" && + suppressedPatterns.some(pattern => pattern.test(args[0]))) { + return; + } + originalWarn.call(console, ...args); + }; +});
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
src/frontend/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (9)
Makefile(4 hunks)Makefile.frontend(1 hunks)src/frontend/jest.config.js(1 hunks)src/frontend/package.json(3 hunks)src/frontend/src/modals/IOModal/components/chatView/chatMessage/chat-message.tsx(0 hunks)src/frontend/src/modals/IOModal/components/chatView/chatMessage/components/edit-message.tsx(2 hunks)src/frontend/src/setupTests.ts(1 hunks)src/frontend/src/utils/__tests__/markdownUtils.test.ts(1 hunks)src/frontend/src/utils/markdownUtils.ts(1 hunks)
💤 Files with no reviewable changes (1)
- src/frontend/src/modals/IOModal/components/chatView/chatMessage/chat-message.tsx
🧰 Additional context used
📓 Path-based instructions (6)
`src/frontend/**/*.{ts,tsx,js,jsx}`: All React and TypeScript/JavaScript source files for the frontend must reside under src/frontend/ and use .ts, .tsx, .js, or .jsx extensions.
src/frontend/**/*.{ts,tsx,js,jsx}: All React and TypeScript/JavaScript source files for the frontend must reside under src/frontend/ and use .ts, .tsx, .js, or .jsx extensions.
📄 Source: CodeRabbit Inference Engine (.cursor/rules/frontend_development.mdc)
List of files the instruction was applied to:
src/frontend/src/modals/IOModal/components/chatView/chatMessage/components/edit-message.tsxsrc/frontend/src/setupTests.tssrc/frontend/jest.config.jssrc/frontend/src/utils/markdownUtils.tssrc/frontend/src/utils/__tests__/markdownUtils.test.ts
`src/frontend/**/*.{css,scss,json}`: All CSS, SCSS, and JSON files for the frontend must be located under src/frontend/.
src/frontend/**/*.{css,scss,json}: All CSS, SCSS, and JSON files for the frontend must be located under src/frontend/.
📄 Source: CodeRabbit Inference Engine (.cursor/rules/frontend_development.mdc)
List of files the instruction was applied to:
src/frontend/package.json
`src/frontend/package*.json`: Frontend package configuration files must be named package.json or package-lock.json and reside in src/frontend/.
src/frontend/package*.json: Frontend package configuration files must be named package.json or package-lock.json and reside in src/frontend/.
📄 Source: CodeRabbit Inference Engine (.cursor/rules/frontend_development.mdc)
List of files the instruction was applied to:
src/frontend/package.json
`src/frontend/src/utils/**/*`: Utility functions must be placed under src/frontend/src/utils/.
src/frontend/src/utils/**/*: Utility functions must be placed under src/frontend/src/utils/.
📄 Source: CodeRabbit Inference Engine (.cursor/rules/frontend_development.mdc)
List of files the instruction was applied to:
src/frontend/src/utils/markdownUtils.tssrc/frontend/src/utils/__tests__/markdownUtils.test.ts
`src/frontend/**/*.test.{ts,tsx,js,jsx}`: Frontend test files should use the .test.{ts,tsx,js,jsx} suffix.
src/frontend/**/*.test.{ts,tsx,js,jsx}: Frontend test files should use the .test.{ts,tsx,js,jsx} suffix.
📄 Source: CodeRabbit Inference Engine (.cursor/rules/testing.mdc)
List of files the instruction was applied to:
src/frontend/src/utils/__tests__/markdownUtils.test.ts
`Makefile`: Use make backend to start the FastAPI backend, make format_backend t...
Makefile: Use make backend to start the FastAPI backend, make format_backend to format Python code, make lint to run linting checks, and make unit_tests to run backend unit tests.
📄 Source: CodeRabbit Inference Engine (.cursor/rules/backend_development.mdc)
List of files the instruction was applied to:
Makefile
🧬 Code Graph Analysis (1)
src/frontend/src/utils/__tests__/markdownUtils.test.ts (1)
src/frontend/src/utils/markdownUtils.ts (3)
isMarkdownTable(8-13)cleanupTableEmptyCells(18-35)preprocessChatMessage(40-52)
🪛 checkmake (0.2.2)
Makefile
[warning] 1-1: Missing required phony target "clean"
(minphony)
[warning] 1-1: Missing required phony target "test"
(minphony)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: autofix
🔇 Additional comments (12)
src/frontend/src/utils/markdownUtils.ts (1)
18-35: LGTM! Well-structured table cleanup logic.The function correctly identifies and preserves table structure while removing empty data rows. The approach of splitting cells and checking for content is sound.
src/frontend/src/modals/IOModal/components/chatView/chatMessage/components/edit-message.tsx (2)
2-2: LGTM! Clean refactoring to use centralized utilities.The import change properly leverages the new markdown utilities module, promoting code reuse and maintainability.
25-26: LGTM! Comment accurately reflects enhanced functionality.The updated comment correctly describes both the think tag handling and table cleanup functionality provided by the new utility.
src/frontend/package.json (2)
96-97: LGTM! Standard Jest testing scripts added.The test scripts follow Jest conventions and provide both single-run testing and watch mode functionality.
123-123: Verify Jest dependency versions for compatibility.The Jest ecosystem dependencies should be compatible with each other and the project's TypeScript/React setup.
#!/bin/bash # Description: Check for potential version compatibility issues with Jest dependencies echo "Checking Jest dependencies compatibility..." # Check if jest and @jest/types versions are compatible jest_version=$(jq -r '.devDependencies.jest' src/frontend/package.json) jest_types_version=$(jq -r '.devDependencies["@jest/types"]' src/frontend/package.json) echo "Jest version: $jest_version" echo "@jest/types version: $jest_types_version" # Check ts-jest compatibility with jest ts_jest_version=$(jq -r '.devDependencies["ts-jest"]' src/frontend/package.json) echo "ts-jest version: $ts_jest_version" # Verify TypeScript version compatibility typescript_version=$(jq -r '.devDependencies.typescript' src/frontend/package.json) echo "TypeScript version: $typescript_version"Also applies to: 141-142, 150-150
src/frontend/src/setupTests.ts (1)
1-31: LGTM! Comprehensive browser API mocking for Jest environment.The mocks for ResizeObserver, IntersectionObserver, and matchMedia are essential for testing React components that use these browser APIs, and the implementations are complete and correct.
src/frontend/jest.config.js (2)
1-20: LGTM! Comprehensive and well-configured Jest setup.The configuration properly handles TypeScript compilation, module aliasing, CSS imports, and test file patterns. The transform ignore patterns correctly exclude node_modules while allowing transformation of ES modules and testing library packages.
19-19: Verify transform ignore pattern effectiveness.The current pattern may not cover all edge cases for ES modules in node_modules.
#!/bin/bash # Description: Check for potential ES modules in node_modules that might need transformation echo "Checking for ES modules in node_modules that might require transformation..." # Look for package.json files with "type": "module" or "module" field fd -t f "package.json" src/frontend/node_modules/ --exec jq -r 'select(.type == "module" or .module != null) | .name' 2>/dev/null | head -10 # Check for .mjs files that might need transformation fd -e mjs . src/frontend/node_modules/ | head -5src/frontend/src/utils/__tests__/markdownUtils.test.ts (1)
1-216: Excellent comprehensive test suite with thorough coverage.This test suite demonstrates excellent testing practices:
- Complete function coverage: All three utility functions are thoroughly tested
- Edge case handling: Tests cover empty input, whitespace, invalid formats, and complex scenarios
- Integration testing:
preprocessChatMessagetests verify the integration of think tag replacement and table cleanup- Clear test organization: Well-structured describe blocks with descriptive test names
- Proper file placement: Correctly placed in
__tests__directory following coding guidelinesThe test cases effectively validate both happy path and edge case scenarios, ensuring robust utility functions.
Makefile (2)
48-48: Good UX improvement for discoverability.Adding the reference to frontend help commands improves discoverability for developers who might not be aware of the new frontend Makefile structure.
524-524: Excellent architectural decision to modularize frontend operations.The inclusion of
Makefile.frontendis a clean way to separate concerns while maintaining backward compatibility. This approach:
- Keeps the main Makefile focused on backend operations
- Delegates all frontend complexity to a dedicated file
- Maintains existing workflow compatibility
- Improves maintainability and organization
Makefile.frontend (1)
1-206: Outstanding comprehensive frontend Makefile with excellent organization.This frontend Makefile demonstrates exceptional attention to detail and developer experience:
Strong Architecture:
- Clear section organization (dependencies, build, development, code quality, testing)
- Proper variable definitions and phony target declarations
- Good error handling with informative messages
Comprehensive Testing Support:
- Extensive Jest testing modes (watch, coverage, verbose, CI, bail, silent)
- Targeted testing capabilities (specific files, patterns, snapshots)
- Playwright e2e testing integration
- Coverage reporting with browser opening functionality
Developer Experience:
- Detailed help system with categorized commands
- Dependency checking with automatic installation
- Cross-platform compatibility (open/xdg-open detection)
- Catch-all rule (lines 125-126) to properly handle make arguments
Best Practices:
- Consistent error handling and user feedback
- Clean separation between different types of operations
- Proper use of make variables and functions
This significantly enhances the frontend development workflow and testing capabilities.
ogabrielluiz
left a comment
There was a problem hiding this comment.
This is really nice.
…8743) * feat(tests): add Jest configuration and setup for testing environment - Introduced Jest configuration file to set up testing environment with TypeScript support and JSDOM. - Added setupTests.ts for global test configurations, including mocks for ResizeObserver and IntersectionObserver. - Updated package.json and package-lock.json to include Jest and related dependencies. - Implemented utility functions for processing markdown content, including handling tables and <think> tags. - Added comprehensive tests for markdown utility functions to ensure proper functionality. * refactor(makefile): separate frontend commands into a dedicated Makefile - Removed frontend-related targets from the main Makefile and created a new Makefile.frontend to manage frontend-specific commands. - Updated the main Makefile to include a reference to the new frontend Makefile and added a help message for frontend commands. - This restructuring improves organization and clarity for managing backend and frontend build processes.
…angflow-ai#8743) * feat(tests): add Jest configuration and setup for testing environment - Introduced Jest configuration file to set up testing environment with TypeScript support and JSDOM. - Added setupTests.ts for global test configurations, including mocks for ResizeObserver and IntersectionObserver. - Updated package.json and package-lock.json to include Jest and related dependencies. - Implemented utility functions for processing markdown content, including handling tables and <think> tags. - Added comprehensive tests for markdown utility functions to ensure proper functionality. * refactor(makefile): separate frontend commands into a dedicated Makefile - Removed frontend-related targets from the main Makefile and created a new Makefile.frontend to manage frontend-specific commands. - Updated the main Makefile to include a reference to the new frontend Makefile and added a help message for frontend commands. - This restructuring improves organization and clarity for managing backend and frontend build processes.
…8743) * feat(tests): add Jest configuration and setup for testing environment - Introduced Jest configuration file to set up testing environment with TypeScript support and JSDOM. - Added setupTests.ts for global test configurations, including mocks for ResizeObserver and IntersectionObserver. - Updated package.json and package-lock.json to include Jest and related dependencies. - Implemented utility functions for processing markdown content, including handling tables and <think> tags. - Added comprehensive tests for markdown utility functions to ensure proper functionality. * refactor(makefile): separate frontend commands into a dedicated Makefile - Removed frontend-related targets from the main Makefile and created a new Makefile.frontend to manage frontend-specific commands. - Updated the main Makefile to include a reference to the new frontend Makefile and added a help message for frontend commands. - This restructuring improves organization and clarity for managing backend and frontend build processes.
…8743) * feat(tests): add Jest configuration and setup for testing environment - Introduced Jest configuration file to set up testing environment with TypeScript support and JSDOM. - Added setupTests.ts for global test configurations, including mocks for ResizeObserver and IntersectionObserver. - Updated package.json and package-lock.json to include Jest and related dependencies. - Implemented utility functions for processing markdown content, including handling tables and <think> tags. - Added comprehensive tests for markdown utility functions to ensure proper functionality. * refactor(makefile): separate frontend commands into a dedicated Makefile - Removed frontend-related targets from the main Makefile and created a new Makefile.frontend to manage frontend-specific commands. - Updated the main Makefile to include a reference to the new frontend Makefile and added a help message for frontend commands. - This restructuring improves organization and clarity for managing backend and frontend build processes.
JIRA TICKET
https://datastax.jira.com/browse/LFOSS-1468?atlOrigin=eyJpIjoiNTE0N2Y4ZTMwNWNhNDcxMTg3YTQ4MDQxNzc3NmRiMGQiLCJwIjoiaiJ9
BUG
Screen.Recording.2025-06-26.at.11.32.10.AM.mov
FIX
Screen.Recording.2025-06-26.at.10.38.55.AM.mov
This pull request introduces significant changes to the
Makefilestructure and adds new testing configurations for the frontend. The main changes involve splitting frontend-related commands into a separateMakefile.frontend, adding Jest testing support for the frontend, and cleaning up theMakefileto improve modularity and maintainability.Makefile Refactoring and Frontend Modularization:
install_frontend,build_frontend,format_frontend, etc.) from the mainMakefileand moved them into a newMakefile.frontendfor better separation of concerns. The mainMakefilenow includes the frontend-specific makefile. [1] [2] [3] [4] [5] [6]Frontend Testing Enhancements:
jest.config.js) to enable unit testing for the frontend, including support for TypeScript and mocking utilities.package.jsonto include Jest dependencies and scripts for running tests (test,test:watch) and coverage. [1] [2] [3]setupTests.ts) to mock browser-specific APIs (e.g.,ResizeObserver,IntersectionObserver) and suppress irrelevant warnings during tests.Codebase Cleanup and Improvements:
preprocessChatMessagefunction fromedit-message.tsxand replaced it with a utility import for better reusability. [1] [2]chat-message.tsxto clean up the code.These changes improve the maintainability of the build system, enhance the frontend testing workflow, and clean up unused or redundant code.
Summary by CodeRabbit
New Features
Refactor
Tests
Chores