feat: front page v2.0 #304
Conversation
Summary of ChangesHello @tenwanft, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a major overhaul to the application's data loading and display architecture, shifting from a rigid, predefined data structure to a more dynamic and file-path-driven approach. The core changes involve enabling recursive reading of JSONL files, dynamically generating UI components based on the loaded data's schema, and providing file-path-based filtering options. These updates enhance the flexibility and adaptability of the application when handling varied JSONL datasets. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a significant refactoring to support reading and displaying data from nested .jsonl files. This includes new backend logic in Electron's main process and Python scripts to recursively scan directories, and major updates to frontend components like detail-table, filter-cascader, and pieChart to handle the dynamic data structure. The changes make the application much more flexible in handling different data layouts.
My review focuses on improving code maintainability by reducing duplication, increasing robustness by handling potential runtime errors, and cleaning up leftover development artifacts like console.log statements. I've identified a few critical issues that could lead to application crashes and some medium-severity issues related to code quality.
| // 检查是否有二级数据 | ||
| const hasSecondLevel = firstLevelType => { | ||
| return Object.keys(data.name_ratio).some(key => | ||
| return Object.keys(data.type_ratio.content).some(key => |
There was a problem hiding this comment.
This line directly accesses data.type_ratio.content, but content may not exist on type_ratio, which would cause the application to crash. Please add optional chaining or a fallback to prevent this.
| return Object.keys(data.type_ratio.content).some(key => | |
| return Object.keys(data.type_ratio?.content || {}).some(key => |
|
|
||
| //根据筛选获得扇形图的右侧展示的一级目录 | ||
| const firstLevelData = useMemo(()=>{ | ||
| return Object.entries(typeRatio[selected]).map( |
There was a problem hiding this comment.
The useMemo hook for firstLevelData accesses typeRatio[selected]. If data.type_ratio is empty, the selected state will be initialized to '', causing typeRatio[selected] to be undefined. Calling Object.entries(undefined) will crash the application. Please add a fallback to handle this case.
| return Object.entries(typeRatio[selected]).map( | |
| return Object.entries(typeRatio[selected] || {}).map( |
| console.log(filePaths, 'jsonlFilePaths'); | ||
|
|
||
| // 使用 id 作为唯一标识 | ||
| setData(uniqBy(allData, 'id')); |
There was a problem hiding this comment.
Using uniqBy(allData, 'id') assumes that every data item across all .jsonl files will have a unique id property. If an item lacks an id, or if ids are not unique across different files, this could lead to unintentional data loss or incorrect filtering. A more robust approach would be to use a key that is guaranteed to be unique, or to not perform this de-duplication if the data source already guarantees uniqueness.
| async function getAllJsonlFilePathsRecursive( | ||
| dirPath: string | ||
| ): Promise<string[]> { | ||
| const filePaths: string[] = []; | ||
|
|
||
| async function traverseDirectory( | ||
| currentPath: string, | ||
| relativePath: string = '' | ||
| ): Promise<void> { | ||
| try { | ||
| const items = await fs.readdir(currentPath, { | ||
| withFileTypes: true, | ||
| }); | ||
|
|
||
| for (const item of items) { | ||
| const fullPath = path.join(currentPath, item.name); | ||
| const newRelativePath = relativePath | ||
| ? `${relativePath}/${item.name}` | ||
| : item.name; | ||
|
|
||
| if (item.isDirectory()) { | ||
| // 递归遍历子目录 | ||
| await traverseDirectory(fullPath, newRelativePath); | ||
| } else if ( | ||
| item.isFile() && | ||
| item.name.endsWith('.jsonl') && | ||
| item.name !== 'summary.json' | ||
| ) { | ||
| // 添加 jsonl 文件路径（相对路径） | ||
| filePaths.push(newRelativePath); | ||
| } | ||
| } | ||
| } catch (error) { | ||
| console.error(`Error reading directory ${currentPath}:`, error); | ||
| } | ||
| } | ||
|
|
||
| await traverseDirectory(dirPath); | ||
| return filePaths.sort(); | ||
| } | ||
|
|
||
| ipcMain.handle( | ||
| 'get-all-jsonl-file-paths', | ||
| async (event, dirPath: string) => { | ||
| try { | ||
| return await getAllJsonlFilePathsRecursive(dirPath); | ||
| } catch (error) { | ||
| console.error('Error getting all JSONL file paths:', error); | ||
| throw error; | ||
| } | ||
| } | ||
| ); | ||
|
|
||
| // 修改 readAllJsonlFilesRecursive，为每个数据项添加文件路径信息 | ||
| async function readAllJsonlFilesRecursiveWithPath( | ||
| dirPath: string | ||
| ): Promise<any[]> { | ||
| const allData: any[] = []; | ||
|
|
||
| async function traverseDirectory( | ||
| currentPath: string, | ||
| relativePath: string = '' | ||
| ): Promise<void> { | ||
| try { | ||
| const items = await fs.readdir(currentPath, { | ||
| withFileTypes: true, | ||
| }); | ||
|
|
||
| for (const item of items) { | ||
| const fullPath = path.join(currentPath, item.name); | ||
| const newRelativePath = relativePath | ||
| ? `${relativePath}/${item.name}` | ||
| : item.name; | ||
|
|
||
| if (item.isDirectory()) { | ||
| // 递归遍历子目录 | ||
| await traverseDirectory(fullPath, newRelativePath); | ||
| } else if ( | ||
| item.isFile() && | ||
| item.name.endsWith('.jsonl') && | ||
| item.name !== 'summary.json' | ||
| ) { | ||
| // 读取 jsonl 文件（排除 summary.json） | ||
| try { | ||
| const fileContent = await fs.readFile( | ||
| fullPath, | ||
| 'utf-8' | ||
| ); | ||
| const lines = fileContent | ||
| .trim() | ||
| .split('\n') | ||
| .filter(line => line.trim()); | ||
| const parsedData = lines | ||
| .map(line => { | ||
| try { | ||
| const data = JSON.parse(line); | ||
| // 为每个数据项添加文件路径信息 | ||
| return { | ||
| ...data, | ||
| _filePath: newRelativePath, | ||
| }; | ||
| } catch (e) { | ||
| console.error( | ||
| `Error parsing line in ${fullPath}:`, | ||
| e | ||
| ); | ||
| return null; | ||
| } | ||
| }) | ||
| .filter(item => item !== null); | ||
| allData.push(...parsedData); | ||
| } catch (error) { | ||
| console.error( | ||
| `Error reading file ${fullPath}:`, | ||
| error | ||
| ); | ||
| } | ||
| } | ||
| } | ||
| } catch (error) { | ||
| console.error(`Error reading directory ${currentPath}:`, error); | ||
| } | ||
| } | ||
|
|
||
| await traverseDirectory(dirPath); | ||
| return allData; | ||
| } |
There was a problem hiding this comment.
There is significant code duplication between the getAllJsonlFilePathsRecursive and readAllJsonlFilesRecursiveWithPath functions. The directory traversal logic is identical in both. To improve maintainability and adhere to the DRY (Don't Repeat Yourself) principle, I recommend abstracting the traversal logic into a shared helper function. This function could take a callback to process each found file, which would make the code cleaner and easier to manage.
| showHighlight?: boolean; | ||
| } | ||
|
|
||
| //该组件此次迭代该组件暂时不用了 |
There was a problem hiding this comment.
The comment indicates this component is temporarily unused. To keep the codebase clean and maintainable, it's best to remove unused components. Version control can be used to restore it later if needed. Leaving unused code can lead to confusion and maintenance overhead.
| console.log(allData, 'allData'); | ||
| console.log(filePaths, 'jsonlFilePaths'); |
There was a problem hiding this comment.
These console.log statements should be removed before merging to keep the browser console clean in production.
| <div className="space-y-1 max-h-[60vh] overflow-y-auto scrollbar-thin"> | ||
| {firstLevelData?.map((item, index) => { | ||
| const hasChildren = hasSecondLevel(item?.name); | ||
| console.log(hasChildren, 'hasChildren') |
There was a problem hiding this comment.
This console.log statement should be removed from the code.
| const getSecondLevelData = firstLevelType => { | ||
| return Object.entries(data.name_ratio) | ||
| const getSecondLevelData = (firstLevelType: string) => { | ||
| console.log(firstLevelType, 'firstLevelType'); |
There was a problem hiding this comment.
This console.log statement should be removed from the code.
2 participants
No description provided.