feat(webui): Add file listing UI for submitting compression jobs for local files.#1292
Conversation
WalkthroughAdds a lazy-loading TreeSelect file browser for compression path selection, introduces a client API to list server directories, and propagates/clears filesystem-backed LogsInputRootDir between controller, client, and server; replaces a textarea paths input with an array-based paths input. Changes
Sequence DiagramsequenceDiagram
participant User as User (Browser)
participant UI as PathsSelectFormItem (TreeSelect)
participant Utils as Path Utils (addServerPrefix / toTreeNode)
participant API as Client API (/api/os/ls)
participant Server as Backend FS service
User->>UI: Expand tree node (e.g., "/")
activate UI
UI->>UI: Check loadedPathsRef (dedupe)
alt already loading/loaded
UI-->>User: Use existing children
else first load
UI->>UI: mark path as loading
UI->>Utils: addServerPrefix(path)
Utils->>API: GET /api/os/ls?path=<encoded>
activate API
API->>Server: Request directory listing
Server-->>API: FileEntry[] response
API-->>Utils: FileListing
deactivate API
Utils->>Utils: toTreeNode(FileEntry[]) -> TreeNode[]
Utils->>UI: return nodes
UI->>UI: merge nodes into treeData, unmark loading
UI-->>User: Render expanded children
end
alt error during load
UI->>UI: unmark loading
UI-->>User: message.error(...)
end
deactivate UI
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~40 minutes
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: ASSERTIVE Plan: Pro 📒 Files selected for processing (3)
🧰 Additional context used📓 Path-based instructions (1)**/*.{cpp,hpp,java,js,jsx,tpp,ts,tsx}⚙️ CodeRabbit configuration file
Files:
🧠 Learnings (4)📚 Learning: 2025-11-29T08:03:34.272ZApplied to files:
📚 Learning: 2025-06-16T13:05:27.349ZApplied to files:
📚 Learning: 2025-10-17T19:59:25.596ZApplied to files:
📚 Learning: 2025-10-07T07:54:32.427ZApplied to files:
🧬 Code graph analysis (1)components/clp-package-utils/clp_package_utils/controller.py (3)
⏰ 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). (5)
🔇 Additional comments (5)
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 |
Resolved conflicts by accepting origin/main changes: - Refactored compression job submission to use common schemas - Updated webui components to use new architecture - Moved compression schema from server to common package - Updated server settings to include archive output configuration - Simplified SearchControls implementation - Updated start_clp.py to use DockerComposeController 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
- Replace http-status-codes with constants from http2 - Update import to use @webui/common/schemas/common - Fix lint errors in os/index.ts 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
…bmission - Restore the TreeSelect-based file browser that was lost during merge - Extract file tree logic into useFileTree custom hook for better organization - Fix all lint and TypeScript compilation errors - Update compression API call to use new schema format with jobId response The file lister provides an interactive tree view for selecting files/directories to compress, replacing the simple text input from origin/main. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
…emTree into a separate module
…or clarity and update related references
…oRemove` for better flexibility
…store and restructure related logic
| server_settings_json_updates["PrestoPort"] = None | ||
|
|
||
| if StorageType.FS == self._clp_config.logs_input.type: | ||
| client_settings_json_updates["LogsInputDir"] = str( |
There was a problem hiding this comment.
I just realized i dont think we need LogsInputDir at all? At first i was confused, but maybe this is correct, and it just for docker mounting? we should probably test with a non-root mount with full package. If that is correct we should remove here and in settings.json
There was a problem hiding this comment.
Right, i was originally treating the user configured path as root but ended up not doing so, so this path is no longer needed. thanks for catching it
| /** | ||
| * Height of the dropdown list in pixels. | ||
| */ | ||
| const LIST_HEIGHT_PX = 512; |
There was a problem hiding this comment.
| const LIST_HEIGHT_PX = 512; | |
| const LIST_HEIGHT_PX = 450; |
There was a problem hiding this comment.
i removed it and now change the height dynamically. see bff4e55
| /** | ||
| * Tree node for Ant Design TreeSelect in simple mode (treeDataSimpleMode). | ||
| */ | ||
| interface TreeNode extends Omit<GetProp<TreeSelectProps, "treeData">[number], "label"> { |
There was a problem hiding this comment.
I prefer the custom type since we are using it for rootNode. Like we are adding more fields that just id
…pages/IngestPage/Compress/PathsSelectFormItem/SwitcherIcon.tsx Co-authored-by: davemarco <83603688+davemarco@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
components/webui/client/src/pages/IngestPage/Compress/PathsSelectFormItem/SwitcherIcon.tsx(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{cpp,hpp,java,js,jsx,tpp,ts,tsx}
⚙️ CodeRabbit configuration file
- Prefer
false == <expression>rather than!<expression>.
Files:
components/webui/client/src/pages/IngestPage/Compress/PathsSelectFormItem/SwitcherIcon.tsx
🧠 Learnings (1)
📓 Common learnings
Learnt from: Bill-hbrhbr
Repo: y-scope/clp PR: 1122
File: components/core/src/clp/clp/CMakeLists.txt:175-195
Timestamp: 2025-07-23T09:54:45.185Z
Learning: In the CLP project, when reviewing CMakeLists.txt changes that introduce new compression library dependencies (BZip2, LibLZMA, LZ4, ZLIB), the team prefers to address conditional linking improvements in separate PRs rather than expanding the scope of focused migration PRs like the LibArchive task-based installation migration.
⏰ 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). (1)
- GitHub Check: package-image
🔇 Additional comments (4)
components/webui/client/src/pages/IngestPage/Compress/PathsSelectFormItem/SwitcherIcon.tsx (4)
1-7: LGTM!The imports are clean and all are used appropriately in the component.
10-12: LGTM!The type derivation is well-structured and ensures type safety by extracting prop types directly from Ant Design's TreeSelectProps.
27-27: LGTM!The default export is appropriate for this component.
22-24: Consider adding an explicit return type annotation.While TypeScript can infer the return type, explicitly annotating it improves code clarity and catches potential type errors earlier.
Apply this diff:
-const SwitcherIcon = ({expanded}: SwitcherIconProps) => (expanded ? +const SwitcherIcon = ({expanded}: SwitcherIconProps): React.ReactElement => (expanded ? <MinusOutlined style={{color: "grey"}}/> : <PlusOutlined style={{color: "grey"}}/>);⛔ Skipped due to learnings
Learnt from: junhaoliao Repo: y-scope/clp PR: 937 File: components/log-viewer-webui/client/src/AntdApp.tsx:16-24 Timestamp: 2025-05-29T20:33:40.653Z Learning: In components/log-viewer-webui React codebase: Return type annotations (like `: JSX.Element`) are unnecessary and not preferred for React components in JSX/TSX files.
| /** | ||
| * Icon component for tree node expand/collapse state. | ||
| * | ||
| * @param props | ||
| * @param props.expanded Whether the node is expanded (passed by Ant Design). | ||
| * @return | ||
| */ |
There was a problem hiding this comment.
Complete the @return JSDoc tag.
The @return tag on line 20 is missing its description. Add a brief description of what the component returns.
Apply this diff:
* @param props
* @param props.expanded Whether the node is expanded (passed by Ant Design).
- * @return
+ * @return A grey plus or minus icon based on the expanded state.
*/📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| /** | |
| * Icon component for tree node expand/collapse state. | |
| * | |
| * @param props | |
| * @param props.expanded Whether the node is expanded (passed by Ant Design). | |
| * @return | |
| */ | |
| /** | |
| * Icon component for tree node expand/collapse state. | |
| * | |
| * @param props | |
| * @param props.expanded Whether the node is expanded (passed by Ant Design). | |
| * @return A grey plus or minus icon based on the expanded state. | |
| */ |
🤖 Prompt for AI Agents
In
components/webui/client/src/pages/IngestPage/Compress/PathsSelectFormItem/SwitcherIcon.tsx
around lines 15 to 21, the JSDoc @return tag is missing its description; update
the JSDoc so the @return line briefly describes what the component returns
(e.g., "JSX.Element — the icon element representing expanded/collapsed state"),
keeping the wording concise and accurate to indicate a React element is
returned.
…electFormItem; remove hardcoded LIST_HEIGHT_PX.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
components/webui/client/src/pages/IngestPage/Compress/PathsSelectFormItem/utils.ts (1)
60-71: Address the prefix matching false positive risk.The
removeServerPrefixfunction at line 66 usesstartsWith()without verifying path-segment boundaries, allowing false matches (e.g.,/mnt/logs-archiveincorrectly matching/mnt/logs). The suggested fix is valid: normalize paths and check for exact prefix match or prefix with trailing slash before stripping.const removeServerPrefix = (serverPath: string): string => { const logsInputRootDir = settings.LogsInputRootDir; if (null === logsInputRootDir) { throw new Error("LogsInputRootDir is not configured."); } + const prefix = logsInputRootDir.endsWith("/") ? + logsInputRootDir.slice(0, -1) : + logsInputRootDir; + const normalized = serverPath.replace(/\/{2,}/g, "/"); - const stripped = serverPath.startsWith(logsInputRootDir) ? - serverPath.slice(logsInputRootDir.length) : - serverPath; + const shouldStrip = normalized === prefix || normalized.startsWith(`${prefix}/`); + const stripped = shouldStrip ? + normalized.slice(prefix.length) : + normalized; return joinPath(ROOT_PATH, stripped); };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (3)
components/webui/client/src/pages/IngestPage/Compress/PathsSelectFormItem/index.tsx(1 hunks)components/webui/client/src/pages/IngestPage/Compress/PathsSelectFormItem/typings.ts(1 hunks)components/webui/client/src/pages/IngestPage/Compress/PathsSelectFormItem/utils.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{cpp,hpp,java,js,jsx,tpp,ts,tsx}
⚙️ CodeRabbit configuration file
- Prefer
false == <expression>rather than!<expression>.
Files:
components/webui/client/src/pages/IngestPage/Compress/PathsSelectFormItem/typings.tscomponents/webui/client/src/pages/IngestPage/Compress/PathsSelectFormItem/index.tsxcomponents/webui/client/src/pages/IngestPage/Compress/PathsSelectFormItem/utils.ts
🧠 Learnings (1)
📓 Common learnings
Learnt from: Bill-hbrhbr
Repo: y-scope/clp PR: 1122
File: components/core/src/clp/clp/CMakeLists.txt:175-195
Timestamp: 2025-07-23T09:54:45.185Z
Learning: In the CLP project, when reviewing CMakeLists.txt changes that introduce new compression library dependencies (BZip2, LibLZMA, LZ4, ZLIB), the team prefers to address conditional linking improvements in separate PRs rather than expanding the scope of focused migration PRs like the LibArchive task-based installation migration.
🧬 Code graph analysis (2)
components/webui/client/src/pages/IngestPage/Compress/PathsSelectFormItem/typings.ts (1)
components/webui/client/src/pages/IngestPage/Compress/PathsSelectFormItem/utils.ts (2)
ROOT_PATH(113-113)TreeNode(109-109)
components/webui/client/src/pages/IngestPage/Compress/PathsSelectFormItem/utils.ts (3)
components/webui/client/src/settings.ts (1)
settings(39-39)components/webui/client/src/pages/IngestPage/Compress/PathsSelectFormItem/typings.ts (1)
ROOT_PATH(20-20)components/webui/common/src/schemas/os.ts (1)
FileEntry(39-39)
⏰ 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). (5)
- GitHub Check: package-image
- GitHub Check: lint-check (ubuntu-24.04)
- GitHub Check: build (ubuntu-24.04)
- GitHub Check: lint-check (macos-15)
- GitHub Check: build (macos-15)
🔇 Additional comments (5)
components/webui/client/src/pages/IngestPage/Compress/PathsSelectFormItem/typings.ts (1)
1-21: LGTM! Clean constants definition.The ROOT_PATH and ROOT_NODE constants are well-defined with appropriate use of Object.freeze for immutability. The structure aligns with the TreeNode type requirements.
components/webui/client/src/pages/IngestPage/Compress/PathsSelectFormItem/index.tsx (2)
57-69: LGTM! Correct use of comparison style.The implementation correctly follows the coding guideline by using
false === existingIds.has(n.id)(line 60) and0 < newNodes.length(line 62) rather than negation operators. The deduplication logic is sound.
85-97: Good error handling with user feedback.The error handling properly logs errors, removes failed paths from the dedupe set, and provides user-friendly error messages via
message.error.components/webui/client/src/pages/IngestPage/Compress/PathsSelectFormItem/utils.ts (2)
14-16: LGTM! Custom TreeNode type is appropriate.The custom TreeNode interface extending Ant Design's type with an explicit
idfield is well-justified given the usage in ROOT_NODE and the tree structure requirements.
101-101: LGTM! Correct comparison style.The code correctly follows the coding guideline by using
false === fileEntry.isExpandableinstead of!fileEntry.isExpandable.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (4)
components/webui/client/src/pages/IngestPage/Compress/PathsSelectFormItem/SwitcherIcon.tsx(1 hunks)components/webui/client/src/pages/IngestPage/Compress/PathsSelectFormItem/index.tsx(1 hunks)components/webui/client/src/pages/IngestPage/Compress/PathsSelectFormItem/typings.ts(1 hunks)components/webui/client/src/pages/IngestPage/Compress/PathsSelectFormItem/utils.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{cpp,hpp,java,js,jsx,tpp,ts,tsx}
⚙️ CodeRabbit configuration file
- Prefer
false == <expression>rather than!<expression>.
Files:
components/webui/client/src/pages/IngestPage/Compress/PathsSelectFormItem/SwitcherIcon.tsxcomponents/webui/client/src/pages/IngestPage/Compress/PathsSelectFormItem/index.tsxcomponents/webui/client/src/pages/IngestPage/Compress/PathsSelectFormItem/typings.tscomponents/webui/client/src/pages/IngestPage/Compress/PathsSelectFormItem/utils.ts
🧠 Learnings (3)
📓 Common learnings
Learnt from: Bill-hbrhbr
Repo: y-scope/clp PR: 1122
File: components/core/src/clp/clp/CMakeLists.txt:175-195
Timestamp: 2025-07-23T09:54:45.185Z
Learning: In the CLP project, when reviewing CMakeLists.txt changes that introduce new compression library dependencies (BZip2, LibLZMA, LZ4, ZLIB), the team prefers to address conditional linking improvements in separate PRs rather than expanding the scope of focused migration PRs like the LibArchive task-based installation migration.
📚 Learning: 2025-05-29T20:33:40.653Z
Learnt from: junhaoliao
Repo: y-scope/clp PR: 937
File: components/log-viewer-webui/client/src/AntdApp.tsx:16-24
Timestamp: 2025-05-29T20:33:40.653Z
Learning: In components/log-viewer-webui React codebase: Return type annotations (like `: JSX.Element`) are unnecessary and not preferred for React components in JSX/TSX files.
Applied to files:
components/webui/client/src/pages/IngestPage/Compress/PathsSelectFormItem/SwitcherIcon.tsx
📚 Learning: 2025-11-29T08:03:34.272Z
Learnt from: junhaoliao
Repo: y-scope/clp PR: 1688
File: components/webui/server/src/routes/api/os/index.ts:46-46
Timestamp: 2025-11-29T08:03:34.272Z
Learning: In the WebUI server (components/webui/server), the file listing API relies on container isolation for security rather than explicit path validation like ALLOWED_BASE_PATH. The service runs in a Docker container with controlled volume mappings, which provides the security boundary.
Applied to files:
components/webui/client/src/pages/IngestPage/Compress/PathsSelectFormItem/utils.ts
🧬 Code graph analysis (2)
components/webui/client/src/pages/IngestPage/Compress/PathsSelectFormItem/index.tsx (4)
components/webui/client/src/pages/IngestPage/Compress/PathsSelectFormItem/typings.ts (2)
TreeNode(25-25)ROOT_NODE(27-27)components/webui/client/src/pages/IngestPage/Compress/PathsSelectFormItem/utils.ts (3)
getListHeight(103-103)addServerPrefix(102-102)toTreeNode(105-105)components/webui/common/src/schemas/os.ts (1)
FileEntry(39-39)components/webui/client/src/api/os/index.ts (1)
listFiles(19-19)
components/webui/client/src/pages/IngestPage/Compress/PathsSelectFormItem/utils.ts (3)
components/webui/client/src/settings.ts (1)
settings(39-39)components/webui/client/src/pages/IngestPage/Compress/PathsSelectFormItem/typings.ts (2)
ROOT_PATH(28-28)TreeNode(25-25)components/webui/common/src/schemas/os.ts (1)
FileEntry(39-39)
⏰ 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). (5)
- GitHub Check: package-image
- GitHub Check: build (ubuntu-24.04)
- GitHub Check: build (macos-15)
- GitHub Check: lint-check (macos-15)
- GitHub Check: lint-check (ubuntu-24.04)
🔇 Additional comments (9)
components/webui/client/src/pages/IngestPage/Compress/PathsSelectFormItem/typings.ts (1)
1-29: LGTM! Well-structured type definitions.The TreeNode interface is properly defined for Ant Design TreeSelect, and using
Object.freezefor ROOT_NODE ensures immutability. The structure is clean and appropriate for the file browser implementation.components/webui/client/src/pages/IngestPage/Compress/PathsSelectFormItem/SwitcherIcon.tsx (1)
1-27: LGTM! Clean icon component implementation.The type extraction from
TreeSelectPropsis elegant, and the component correctly renders the appropriate icon based on the expanded state. The implementation is straightforward and type-safe.components/webui/client/src/pages/IngestPage/Compress/PathsSelectFormItem/index.tsx (4)
40-54: LGTM! Proper state initialization and effect cleanup.The resize listener is correctly registered and cleaned up, and the initial state values are appropriate for lazy loading the tree data.
56-85: LGTM! Solid deduplication and error handling.The ref-based deduplication strategy avoids unnecessary re-renders while preventing duplicate loads. The error handling correctly removes failed paths from the loaded set, allowing retry. Good adherence to the coding guideline with
false === existingIds.has(n.id)on line 62.
87-103: LGTM! Type-safe callbacks with good error handling.The type guard on line 88 correctly follows the coding guideline with
"string" !== typeof value. Error messages are user-friendly and provide appropriate feedback for loading failures.
105-129: LGTM! Properly configured TreeSelect for lazy loading.The TreeSelect configuration is appropriate for multi-select with lazy loading. The combination of
treeCheckable,showCheckedStrategy={TreeSelect.SHOW_PARENT}, andloadDataprovides a good user experience for path selection.components/webui/client/src/pages/IngestPage/Compress/PathsSelectFormItem/utils.ts (3)
10-43: LGTM! Clean utility implementations.
getListHeightprovides a responsive dropdown size clamped to reasonable bounds, andjoinPathcorrectly normalizes POSIX paths by filtering empty segments and collapsing consecutive slashes.
65-79: LGTM! Proper validation and path construction.
addServerPrefixcorrectly validates thatLogsInputRootDiris configured before constructing the full server path. The error message is clear and actionable.
81-98: LGTM! Correct tree node conversion with guideline adherence.Line 93 correctly follows the coding guideline with
false === fileEntry.isExpandable. The function properly maps API responses to TreeSelect nodes.The
parentPathparameter usage (forpId) versusfileEntry.parentPath(for path construction) is intentional and consistent with how it's called in index.tsx line 80, though renaming toparentIdcould improve clarity as noted in previous feedback.
|
|
||
| /** | ||
| * Form item with TreeSelect for selecting file paths for compression. | ||
| * Supports lazy loading and search with auto-expand. |
There was a problem hiding this comment.
| * Supports lazy loading and search with auto-expand. |
davemarco
left a comment
There was a problem hiding this comment.
Okay i am ready to approve this, just want to test the small new height thing when bakers are back up
|
okay i retested height. I approve |
…local files. (y-scope#1292) Co-authored-by: davemarco <83603688+davemarco@users.noreply.github.com>
…local files. (y-scope#1292) Co-authored-by: davemarco <83603688+davemarco@users.noreply.github.com>
…local files. (y-scope#1292) Co-authored-by: davemarco <83603688+davemarco@users.noreply.github.com>
(Items that are
crossed outare to be submitted in future improvement PRs.)Description
This PR adds an interactive file system tree selector for the compression job submission page in the WebUI, replacing the previous text area input with a user-friendly tree-based file browser.
What Changed
Frontend (WebUI Client)
Added Zustand-based state management: ImplementedfsTreeStorefor managing tree state, loading, and expansionsrc/api/os/index.tsfor file system operationsSearch support: Users can type paths to quickly navigate to specific directoriesBackend (WebUI Server)
/api/os/lsendpoint: Lists files and directories at a specified path with proper validationFileListingSchemaandFileEntrySchemafor type-safe responsesLogsInputRootDirconfiguration for server-side path resolutionInfrastructure
CLP_LOGS_INPUT_DIR_HOSTto enable file browsing in containerized environmentscontroller.pyto properly configure client and server settings with logs input pathsChecklist
breaking change.
Validation performed
Manual Testing
Sample logs were stored at
/home/junhao/samples.Validated search functionality by typing paths. e.g./home/junhao/sampAlso, before the "sample" item was selected, tested directory expansion of other nodes (e.g./var) by clicking both expand icons and node titles.Confirmed proper error handling (404 error message) when accessing invalid pathslogs_input.directory: "/home/junhao/samples"and restarting the package: in the popup, only files under "/home/junhao/samples are shown.Summary by CodeRabbit
New Features
Bug Fixes / Improvements
✏️ Tip: You can customize this high-level summary in your review settings.