fix(new-webui): Add support for querying metadata from multiple datasets (fixes #1024). #1042
Conversation
## Walkthrough
This change refactors data-fetching logic in several ingestion and statistics components to use React Query hooks for declarative data management, replacing manual polling and local state. It introduces new utility modules for SQL queries and result processing, adds loading state props to various UI cards, and centralizes QueryClient configuration.
## Changes
| File(s) | Change Summary |
|---------------------------------------------------------------------------------------------------------|------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|
| `App.tsx`, `config/queryClient.ts` | Centralized QueryClient instantiation in a new config module and updated the app to import and use this shared instance. |
| `components/DashboardCard/index.tsx`, `components/StatCard/index.tsx` | Added optional `isLoading` prop to card components and passed loading state to underlying UI elements. |
| `pages/IngestPage/Details/DetailsCard.tsx`, `.../Files.tsx`, `.../Messages.tsx`, `.../TimeRange.tsx` | Extended props interfaces and components to accept and propagate an `isLoading` boolean for loading state indication. |
| `pages/IngestPage/SpaceSavings/CompressedSize.tsx`, `.../UncompressedSize.tsx` | Added `isLoading` prop to compressed/uncompressed size card components and propagated to child components. |
| `pages/IngestPage/Details/index.tsx`, `.../Jobs/index.tsx`, `.../SpaceSavings/index.tsx` | Refactored from manual polling and state management to React Query hooks for data fetching, caching, and loading state; removed local state and interval logic. |
| `pages/IngestPage/Details/sql.ts`, `.../SpaceSavings/sql.ts` | Added interfaces, constants, and functions for modular, multi-dataset-aware SQL query construction and execution, including error handling and default values. |
| `pages/SearchPage/SearchControls/Dataset/index.tsx` | Removed dependency on polling interval and store for dataset names; simplified dataset fetching logic. |
| `api/query/index.ts`, `api/search/index.ts` | Adjusted import paths for typings with no functional changes. |
| `clp_package_utils/scripts/start_clp.py`, `public/settings.json`, `config/sql-table-suffix.ts`, `settings.ts` | Simplified and standardized SQL table naming logic for CLP storage engine; added new config key for table prefix; introduced enum for table suffixes; updated client settings JSON and script to reflect these changes. |
## Sequence Diagram(s)
```mermaid
sequenceDiagram
participant UI as UI Component
participant QueryClient
participant ReactQuery
participant SQLUtil as SQL Utility
participant Backend
UI->>ReactQuery: useQuery(queryKey, fetchFn)
ReactQuery->>QueryClient: Check cache for data
alt Cache miss or stale
ReactQuery->>SQLUtil: fetchClpDetails / fetchClpsDetails / fetchClpSpaceSavings / fetchClpsSpaceSavings
SQLUtil->>Backend: querySql(SQL)
Backend-->>SQLUtil: SQL result
SQLUtil-->>ReactQuery: Parsed result (with defaults if needed)
ReactQuery->>QueryClient: Store result in cache
end
ReactQuery-->>UI: Provide data and isLoading flag
UI->>UI: Render with data or loading statePossibly related PRs
Suggested reviewers
Learnt from: gibber9809 Learnt from: Bill-hbrhbr Learnt from: davemarco Learnt from: junhaoliao Learnt from: junhaoliao Learnt from: junhaoliao |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
components/log-viewer-webui/client/src/pages/IngestPage/Details/sql.ts (1)
69-110: Verify dataset name validation to prevent SQL injection.Similar to the SpaceSavings module, this function directly interpolates dataset names into SQL strings (
clp_${name}_archives,clp_${name}_files). The same SQL injection concerns apply here.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
components/log-viewer-webui/client/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (19)
components/log-viewer-webui/client/package.json(1 hunks)components/log-viewer-webui/client/src/AntdApp.tsx(2 hunks)components/log-viewer-webui/client/src/api/sql.ts(1 hunks)components/log-viewer-webui/client/src/components/DashboardCard/index.tsx(2 hunks)components/log-viewer-webui/client/src/components/StatCard/index.tsx(3 hunks)components/log-viewer-webui/client/src/config/queryClient.ts(1 hunks)components/log-viewer-webui/client/src/pages/IngestPage/Details/DetailsCard.tsx(2 hunks)components/log-viewer-webui/client/src/pages/IngestPage/Details/Files.tsx(1 hunks)components/log-viewer-webui/client/src/pages/IngestPage/Details/Messages.tsx(1 hunks)components/log-viewer-webui/client/src/pages/IngestPage/Details/TimeRange.tsx(3 hunks)components/log-viewer-webui/client/src/pages/IngestPage/Details/index.tsx(1 hunks)components/log-viewer-webui/client/src/pages/IngestPage/Details/sql.ts(3 hunks)components/log-viewer-webui/client/src/pages/IngestPage/Jobs/index.tsx(3 hunks)components/log-viewer-webui/client/src/pages/IngestPage/SpaceSavings/CompressedSize.tsx(1 hunks)components/log-viewer-webui/client/src/pages/IngestPage/SpaceSavings/UncompressedSize.tsx(1 hunks)components/log-viewer-webui/client/src/pages/IngestPage/SpaceSavings/index.tsx(2 hunks)components/log-viewer-webui/client/src/pages/IngestPage/SpaceSavings/sql.ts(2 hunks)components/log-viewer-webui/client/src/pages/IngestPage/ingestStatsStore.ts(0 hunks)components/log-viewer-webui/client/src/pages/SearchPage/SearchControls/Dataset/sql.ts(1 hunks)
💤 Files with no reviewable changes (1)
- components/log-viewer-webui/client/src/pages/IngestPage/ingestStatsStore.ts
🧰 Additional context used
📓 Path-based instructions (1)
`**/*.{cpp,hpp,java,js,jsx,tpp,ts,tsx}`: - Prefer `false == ` rather than `!`.
**/*.{cpp,hpp,java,js,jsx,tpp,ts,tsx}: - Preferfalse == <expression>rather than!<expression>.
⚙️ Source: CodeRabbit Configuration File
List of files the instruction was applied to:
components/log-viewer-webui/client/src/config/queryClient.tscomponents/log-viewer-webui/client/src/AntdApp.tsxcomponents/log-viewer-webui/client/src/components/StatCard/index.tsxcomponents/log-viewer-webui/client/src/api/sql.tscomponents/log-viewer-webui/client/src/pages/SearchPage/SearchControls/Dataset/sql.tscomponents/log-viewer-webui/client/src/pages/IngestPage/Details/DetailsCard.tsxcomponents/log-viewer-webui/client/src/pages/IngestPage/SpaceSavings/UncompressedSize.tsxcomponents/log-viewer-webui/client/src/pages/IngestPage/Details/TimeRange.tsxcomponents/log-viewer-webui/client/src/pages/IngestPage/SpaceSavings/CompressedSize.tsxcomponents/log-viewer-webui/client/src/components/DashboardCard/index.tsxcomponents/log-viewer-webui/client/src/pages/IngestPage/Details/Messages.tsxcomponents/log-viewer-webui/client/src/pages/IngestPage/Details/index.tsxcomponents/log-viewer-webui/client/src/pages/IngestPage/Details/Files.tsxcomponents/log-viewer-webui/client/src/pages/IngestPage/SpaceSavings/index.tsxcomponents/log-viewer-webui/client/src/pages/IngestPage/Jobs/index.tsxcomponents/log-viewer-webui/client/src/pages/IngestPage/SpaceSavings/sql.tscomponents/log-viewer-webui/client/src/pages/IngestPage/Details/sql.ts
🧠 Learnings (18)
📓 Common learnings
Learnt from: junhaoliao
PR: y-scope/clp#596
File: components/log-viewer-webui/client/src/api/query.js:16-23
Timestamp: 2024-11-21T15:51:33.203Z
Learning: In `components/log-viewer-webui/client/src/api/query.js`, the `ExtractJsonResp` type definition is accurate as-is and does not require modification. When suggesting changes to type definitions, ensure they align with the server-side definitions, referencing the source code if necessary.
Learnt from: junhaoliao
PR: y-scope/clp#939
File: components/package-template/src/etc/clp-config.yml:64-64
Timestamp: 2025-06-22T04:01:43.409Z
Learning: The new webui architecture uses Fastify with Pino logging instead of the previous Winston-based logging system that was removed during the webui refactoring.
Learnt from: junhaoliao
PR: y-scope/clp#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.
Learnt from: davemarco
PR: y-scope/clp#892
File: components/log-viewer-webui/client/src/api/socket/MongoCollectionSocket.ts:27-31
Timestamp: 2025-05-09T19:15:26.180Z
Learning: For the MongoDB real-time updates implementation in components/log-viewer-webui/client/src/api/socket, a socket singleton pattern is used where a single shared socket connection is maintained rather than creating multiple connections. The socket lifecycle is managed centrally, with unsubscription handling in the useCursor React hook's cleanup function.
Learnt from: junhaoliao
PR: y-scope/clp#596
File: components/log-viewer-webui/client/src/api/query.js:35-41
Timestamp: 2024-11-19T19:52:43.429Z
Learning: For internal APIs in `components/log-viewer-webui/client/src/api/query.js`, runtime validation of parameters may not be necessary since the APIs are not exposed to end users, and JsDoc type annotations may be sufficient.
Learnt from: junhaoliao
PR: y-scope/clp#962
File: components/log-viewer-webui/client/src/pages/SearchPage/SearchQueryStatus/Results.tsx:22-26
Timestamp: 2025-06-09T17:15:30.309Z
Learning: When using Zustand stores in React components, always use specific selectors (e.g., `useStore(state => state.property)`) instead of destructuring the entire state object to prevent unnecessary re-renders. This is especially important as state stores grow in complexity.
Learnt from: junhaoliao
PR: y-scope/clp#962
File: components/log-viewer-webui/client/src/pages/SearchPage/SearchQueryStatus/Results.tsx:22-26
Timestamp: 2025-06-09T17:15:30.309Z
Learning: When using Zustand stores in React components, always use specific selectors (e.g., `useStore(state => state.property)`) instead of destructuring the entire state object to prevent unnecessary re-renders. This is especially important as state stores grow in complexity.
components/log-viewer-webui/client/package.json (5)
Learnt from: junhaoliao
PR: y-scope/clp#596
File: components/log-viewer-webui/client/src/api/query.js:16-23
Timestamp: 2024-11-21T15:51:33.203Z
Learning: In `components/log-viewer-webui/client/src/api/query.js`, the `ExtractJsonResp` type definition is accurate as-is and does not require modification. When suggesting changes to type definitions, ensure they align with the server-side definitions, referencing the source code if necessary.
Learnt from: junhaoliao
PR: y-scope/clp#939
File: components/package-template/src/etc/clp-config.yml:64-64
Timestamp: 2025-06-22T04:01:43.409Z
Learning: The new webui architecture uses Fastify with Pino logging instead of the previous Winston-based logging system that was removed during the webui refactoring.
Learnt from: junhaoliao
PR: y-scope/clp#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.
Learnt from: junhaoliao
PR: y-scope/clp#596
File: components/log-viewer-webui/client/src/api/query.js:35-41
Timestamp: 2024-11-19T19:52:43.429Z
Learning: For internal APIs in `components/log-viewer-webui/client/src/api/query.js`, runtime validation of parameters may not be necessary since the APIs are not exposed to end users, and JsDoc type annotations may be sufficient.
Learnt from: davemarco
PR: y-scope/clp#797
File: components/log-viewer-webui/client/src/components/Layout/MainLayout.tsx:2-5
Timestamp: 2025-04-08T22:32:05.366Z
Learning: In this codebase using React Router v7.4.1, components should be imported directly from "react-router" (e.g., `import { Link, Outlet } from "react-router";`) rather than from "react-router-dom" as was common in previous versions of React Router.
components/log-viewer-webui/client/src/config/queryClient.ts (1)
Learnt from: junhaoliao
PR: y-scope/clp#596
File: components/log-viewer-webui/client/src/api/query.js:16-23
Timestamp: 2024-11-21T15:51:33.203Z
Learning: In `components/log-viewer-webui/client/src/api/query.js`, the `ExtractJsonResp` type definition is accurate as-is and does not require modification. When suggesting changes to type definitions, ensure they align with the server-side definitions, referencing the source code if necessary.
components/log-viewer-webui/client/src/AntdApp.tsx (6)
Learnt from: junhaoliao
PR: y-scope/clp#596
File: components/log-viewer-webui/client/src/api/query.js:16-23
Timestamp: 2024-11-21T15:51:33.203Z
Learning: In `components/log-viewer-webui/client/src/api/query.js`, the `ExtractJsonResp` type definition is accurate as-is and does not require modification. When suggesting changes to type definitions, ensure they align with the server-side definitions, referencing the source code if necessary.
Learnt from: junhaoliao
PR: y-scope/clp#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.
Learnt from: davemarco
PR: y-scope/clp#797
File: components/log-viewer-webui/client/src/components/Layout/MainLayout.tsx:2-5
Timestamp: 2025-04-08T22:32:05.366Z
Learning: In this codebase using React Router v7.4.1, components should be imported directly from "react-router" (e.g., `import { Link, Outlet } from "react-router";`) rather than from "react-router-dom" as was common in previous versions of React Router.
Learnt from: davemarco
PR: y-scope/clp#797
File: components/log-viewer-webui/client/src/routes/routes.tsx:1-1
Timestamp: 2025-04-04T15:15:59.432Z
Learning: In React Router v7, `createBrowserRouter` and other router primitives should be imported from `react-router` package, not from `react-router-dom` as in previous versions. This represents a significant change in the library's structure where router primitives are now in the core package.
Learnt from: junhaoliao
PR: y-scope/clp#596
File: components/log-viewer-webui/client/src/api/query.js:35-41
Timestamp: 2024-11-19T19:52:43.429Z
Learning: For internal APIs in `components/log-viewer-webui/client/src/api/query.js`, runtime validation of parameters may not be necessary since the APIs are not exposed to end users, and JsDoc type annotations may be sufficient.
Learnt from: davemarco
PR: y-scope/clp#797
File: components/log-viewer-webui/client/src/components/Layout/MainLayout.tsx:2-5
Timestamp: 2025-04-08T22:32:05.367Z
Learning: React Router v7.4.1 uses a different package structure than previous versions. Components can be imported directly from "react-router" (e.g., `import { Link, Outlet } from "react-router";`) rather than from "react-router-dom" as was common in previous versions. Alternatively, DOM-specific components can also be imported using deep imports like `import { Link } from "react-router/dom";`.
components/log-viewer-webui/client/src/components/StatCard/index.tsx (1)
Learnt from: junhaoliao
PR: y-scope/clp#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.
components/log-viewer-webui/client/src/api/sql.ts (1)
Learnt from: junhaoliao
PR: y-scope/clp#596
File: components/log-viewer-webui/client/src/api/query.js:16-23
Timestamp: 2024-11-21T15:51:33.203Z
Learning: In `components/log-viewer-webui/client/src/api/query.js`, the `ExtractJsonResp` type definition is accurate as-is and does not require modification. When suggesting changes to type definitions, ensure they align with the server-side definitions, referencing the source code if necessary.
components/log-viewer-webui/client/src/pages/IngestPage/Details/DetailsCard.tsx (2)
Learnt from: junhaoliao
PR: y-scope/clp#596
File: components/log-viewer-webui/client/src/api/query.js:16-23
Timestamp: 2024-11-21T15:51:33.203Z
Learning: In `components/log-viewer-webui/client/src/api/query.js`, the `ExtractJsonResp` type definition is accurate as-is and does not require modification. When suggesting changes to type definitions, ensure they align with the server-side definitions, referencing the source code if necessary.
Learnt from: junhaoliao
PR: y-scope/clp#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.
components/log-viewer-webui/client/src/pages/IngestPage/SpaceSavings/UncompressedSize.tsx (2)
Learnt from: junhaoliao
PR: y-scope/clp#596
File: components/log-viewer-webui/client/src/api/query.js:16-23
Timestamp: 2024-11-21T15:51:33.203Z
Learning: In `components/log-viewer-webui/client/src/api/query.js`, the `ExtractJsonResp` type definition is accurate as-is and does not require modification. When suggesting changes to type definitions, ensure they align with the server-side definitions, referencing the source code if necessary.
Learnt from: junhaoliao
PR: y-scope/clp#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.
components/log-viewer-webui/client/src/pages/IngestPage/Details/TimeRange.tsx (2)
Learnt from: junhaoliao
PR: y-scope/clp#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.
Learnt from: junhaoliao
PR: y-scope/clp#596
File: components/log-viewer-webui/client/src/api/query.js:16-23
Timestamp: 2024-11-21T15:51:33.203Z
Learning: In `components/log-viewer-webui/client/src/api/query.js`, the `ExtractJsonResp` type definition is accurate as-is and does not require modification. When suggesting changes to type definitions, ensure they align with the server-side definitions, referencing the source code if necessary.
components/log-viewer-webui/client/src/pages/IngestPage/SpaceSavings/CompressedSize.tsx (2)
Learnt from: junhaoliao
PR: y-scope/clp#596
File: components/log-viewer-webui/client/src/api/query.js:16-23
Timestamp: 2024-11-21T15:51:33.203Z
Learning: In `components/log-viewer-webui/client/src/api/query.js`, the `ExtractJsonResp` type definition is accurate as-is and does not require modification. When suggesting changes to type definitions, ensure they align with the server-side definitions, referencing the source code if necessary.
Learnt from: junhaoliao
PR: y-scope/clp#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.
components/log-viewer-webui/client/src/components/DashboardCard/index.tsx (1)
Learnt from: junhaoliao
PR: y-scope/clp#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.
components/log-viewer-webui/client/src/pages/IngestPage/Details/Messages.tsx (2)
Learnt from: junhaoliao
PR: y-scope/clp#596
File: components/log-viewer-webui/client/src/api/query.js:16-23
Timestamp: 2024-11-21T15:51:33.203Z
Learning: In `components/log-viewer-webui/client/src/api/query.js`, the `ExtractJsonResp` type definition is accurate as-is and does not require modification. When suggesting changes to type definitions, ensure they align with the server-side definitions, referencing the source code if necessary.
Learnt from: junhaoliao
PR: y-scope/clp#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.
components/log-viewer-webui/client/src/pages/IngestPage/Details/index.tsx (3)
Learnt from: junhaoliao
PR: y-scope/clp#596
File: components/log-viewer-webui/client/src/api/query.js:16-23
Timestamp: 2024-11-21T15:51:33.203Z
Learning: In `components/log-viewer-webui/client/src/api/query.js`, the `ExtractJsonResp` type definition is accurate as-is and does not require modification. When suggesting changes to type definitions, ensure they align with the server-side definitions, referencing the source code if necessary.
Learnt from: junhaoliao
PR: y-scope/clp#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.
Learnt from: junhaoliao
PR: y-scope/clp#596
File: components/log-viewer-webui/client/src/api/query.js:35-41
Timestamp: 2024-11-19T19:52:43.429Z
Learning: For internal APIs in `components/log-viewer-webui/client/src/api/query.js`, runtime validation of parameters may not be necessary since the APIs are not exposed to end users, and JsDoc type annotations may be sufficient.
components/log-viewer-webui/client/src/pages/IngestPage/Details/Files.tsx (2)
Learnt from: junhaoliao
PR: y-scope/clp#596
File: components/log-viewer-webui/client/src/api/query.js:16-23
Timestamp: 2024-11-21T15:51:33.203Z
Learning: In `components/log-viewer-webui/client/src/api/query.js`, the `ExtractJsonResp` type definition is accurate as-is and does not require modification. When suggesting changes to type definitions, ensure they align with the server-side definitions, referencing the source code if necessary.
Learnt from: junhaoliao
PR: y-scope/clp#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.
components/log-viewer-webui/client/src/pages/IngestPage/SpaceSavings/index.tsx (4)
Learnt from: junhaoliao
PR: y-scope/clp#596
File: components/log-viewer-webui/client/src/api/query.js:16-23
Timestamp: 2024-11-21T15:51:33.203Z
Learning: In `components/log-viewer-webui/client/src/api/query.js`, the `ExtractJsonResp` type definition is accurate as-is and does not require modification. When suggesting changes to type definitions, ensure they align with the server-side definitions, referencing the source code if necessary.
Learnt from: junhaoliao
PR: y-scope/clp#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.
Learnt from: junhaoliao
PR: y-scope/clp#962
File: components/log-viewer-webui/client/src/pages/SearchPage/SearchQueryStatus/Results.tsx:22-26
Timestamp: 2025-06-09T17:15:30.309Z
Learning: When using Zustand stores in React components, always use specific selectors (e.g., `useStore(state => state.property)`) instead of destructuring the entire state object to prevent unnecessary re-renders. This is especially important as state stores grow in complexity.
Learnt from: junhaoliao
PR: y-scope/clp#962
File: components/log-viewer-webui/client/src/pages/SearchPage/SearchQueryStatus/Results.tsx:22-26
Timestamp: 2025-06-09T17:15:30.309Z
Learning: When using Zustand stores in React components, always use specific selectors (e.g., `useStore(state => state.property)`) instead of destructuring the entire state object to prevent unnecessary re-renders. This is especially important as state stores grow in complexity.
components/log-viewer-webui/client/src/pages/IngestPage/Jobs/index.tsx (4)
Learnt from: junhaoliao
PR: y-scope/clp#596
File: components/log-viewer-webui/client/src/api/query.js:16-23
Timestamp: 2024-11-21T15:51:33.203Z
Learning: In `components/log-viewer-webui/client/src/api/query.js`, the `ExtractJsonResp` type definition is accurate as-is and does not require modification. When suggesting changes to type definitions, ensure they align with the server-side definitions, referencing the source code if necessary.
Learnt from: junhaoliao
PR: y-scope/clp#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.
Learnt from: junhaoliao
PR: y-scope/clp#596
File: components/log-viewer-webui/client/src/api/query.js:35-41
Timestamp: 2024-11-19T19:52:43.429Z
Learning: For internal APIs in `components/log-viewer-webui/client/src/api/query.js`, runtime validation of parameters may not be necessary since the APIs are not exposed to end users, and JsDoc type annotations may be sufficient.
Learnt from: davemarco
PR: y-scope/clp#797
File: components/log-viewer-webui/client/src/components/Layout/MainLayout.tsx:2-5
Timestamp: 2025-04-08T22:32:05.366Z
Learning: In this codebase using React Router v7.4.1, components should be imported directly from "react-router" (e.g., `import { Link, Outlet } from "react-router";`) rather than from "react-router-dom" as was common in previous versions of React Router.
components/log-viewer-webui/client/src/pages/IngestPage/SpaceSavings/sql.ts (1)
Learnt from: junhaoliao
PR: y-scope/clp#596
File: components/log-viewer-webui/client/src/api/query.js:16-23
Timestamp: 2024-11-21T15:51:33.203Z
Learning: In `components/log-viewer-webui/client/src/api/query.js`, the `ExtractJsonResp` type definition is accurate as-is and does not require modification. When suggesting changes to type definitions, ensure they align with the server-side definitions, referencing the source code if necessary.
components/log-viewer-webui/client/src/pages/IngestPage/Details/sql.ts (1)
Learnt from: junhaoliao
PR: y-scope/clp#596
File: components/log-viewer-webui/client/src/api/query.js:16-23
Timestamp: 2024-11-21T15:51:33.203Z
Learning: In `components/log-viewer-webui/client/src/api/query.js`, the `ExtractJsonResp` type definition is accurate as-is and does not require modification. When suggesting changes to type definitions, ensure they align with the server-side definitions, referencing the source code if necessary.
🪛 GitHub Actions: clp-lint
components/log-viewer-webui/client/src/AntdApp.tsx
[error] 4-4: ESLint: Unable to resolve path to module '@tanstack/react-query-devtools' (import/no-unresolved)
🔇 Additional comments (33)
components/log-viewer-webui/client/package.json (1)
22-22: Good addition for data fetching and caching.The React Query dependency is a solid choice for managing server state and data fetching. The version ^5.81.4 is appropriate and aligns with the refactoring objectives described in the PR.
components/log-viewer-webui/client/src/pages/SearchPage/SearchControls/Dataset/sql.ts (1)
20-25: Well-structured SQL query construction.The SQL query is properly constructed using template literals with enum values, which prevents SQL injection risks. The query structure is clear and follows good practices with explicit column selection and ordering.
components/log-viewer-webui/client/src/config/queryClient.ts (1)
4-12: Appropriate React Query configuration.The 10-second stale time is a reasonable default that balances data freshness with performance. The configuration is clean and follows React Query best practices.
components/log-viewer-webui/client/src/pages/IngestPage/Details/DetailsCard.tsx (1)
9-9: Clean implementation of loading state support.The addition of the
isLoadingprop is well-implemented with proper TypeScript typing, JSDoc documentation, and correct prop forwarding. This change supports the broader React Query integration effectively.Also applies to: 18-18, 21-21, 25-25
components/log-viewer-webui/client/src/pages/IngestPage/Details/TimeRange.tsx (3)
8-12: LGTM! Clean addition of loading state prop.The addition of the
isLoadingboolean prop to the interface follows the consistent pattern across the IngestPage components and properly supports the React Query integration.
19-21: JSDoc properly updated for new parameter.Good practice updating the JSDoc to document the new
isLoadingparameter.
23-23: Proper destructuring and prop forwarding.The component correctly accepts the
isLoadingprop and forwards it to theDetailsCardcomponent as expected.components/log-viewer-webui/client/src/pages/IngestPage/SpaceSavings/UncompressedSize.tsx (2)
5-8: LGTM! Consistent loading state prop addition.The
isLoadingprop addition follows the same pattern as other components in this refactor, maintaining consistency across the codebase.
18-25: Proper implementation of loading state forwarding.The component correctly accepts and forwards the
isLoadingprop toDetailsCard, and the JSDoc documentation has been appropriately updated.components/log-viewer-webui/client/src/components/StatCard/index.tsx (3)
11-19: LGTM! Well-designed optional loading prop.The addition of the optional
isLoadingprop maintains backward compatibility while enabling loading state support where needed.
34-42: Good use of default parameter value.Setting
isLoading = falseas the default parameter value is a clean approach that maintains backward compatibility and follows React best practices.
43-45: Proper prop forwarding to DashboardCard.The
isLoadingprop is correctly added to theDashboardCardPropsobject and passed down to the child component.components/log-viewer-webui/client/src/AntdApp.tsx (2)
21-29: LGTM! Proper React Query provider setup.The QueryClientProvider correctly wraps the existing providers and the ReactQueryDevtools are appropriately configured with
initialIsOpen={false}for a clean development experience.
7-7: Clean queryClient import.Good practice importing the configured queryClient from the config module for centralized configuration management.
components/log-viewer-webui/client/src/pages/IngestPage/Details/Files.tsx (2)
6-9: LGTM! Completes the consistent loading state pattern.The addition of the
isLoadingprop maintains consistency with the other IngestPage detail components, completing the unified loading state implementation.
19-25: Proper implementation matching sibling components.The component correctly implements the same loading state pattern as
TimeRange,Messages, and other detail components, ensuring consistent behaviour across the IngestPage details UI.components/log-viewer-webui/client/src/pages/IngestPage/SpaceSavings/CompressedSize.tsx (1)
5-25: LGTM! Clean loading state integration.The loading state prop is properly typed, documented, and propagated to the child component. This follows the established pattern for the React Query migration.
components/log-viewer-webui/client/src/pages/IngestPage/Details/Messages.tsx (1)
6-26: LGTM! Consistent loading state pattern.The implementation mirrors the pattern used in other components, with proper TypeScript typing and JSDoc documentation. The loading state is correctly propagated to the DetailsCard.
components/log-viewer-webui/client/src/components/DashboardCard/index.tsx (1)
11-55: LGTM! Well-designed loading state integration.The optional
isLoadingprop with a sensible default maintains backward compatibility while enabling loading states throughout the UI. The integration with Ant Design's Card loading functionality is proper and the JSDoc documentation is comprehensive.components/log-viewer-webui/client/src/pages/IngestPage/SpaceSavings/index.tsx (2)
28-47: LGTM! Well-structured React Query implementation.The conditional query execution logic is well-designed:
- Dataset names query is properly enabled only for CLP_S storage engine
- Space savings query has correct dependencies and enabling conditions
- Query keys are properly structured for cache invalidation when datasetNames change
The loading state propagation to child components is consistent and appropriate.
63-75: Consistent loading state propagation.The
isPendingstate from React Query is properly passed down to all child components (StatCard, UncompressedSize, CompressedSize), ensuring a unified loading experience across the space savings section.components/log-viewer-webui/client/src/pages/IngestPage/Details/index.tsx (2)
26-45: LGTM! Consistent React Query pattern.The implementation follows the same well-structured pattern as the SpaceSavings component:
- Proper conditional dataset fetching for CLP_S storage engine
- Details query correctly depends on datasetNames for cache invalidation
- Query enabling logic is sound for both storage engine types
The consistency across components enhances maintainability.
47-74: Proper loading state handling across both storage engine paths.Both the CLP and CLP_S conditional render paths correctly propagate the
isPendingloading state to all child components (TimeRange, Messages, Files), ensuring consistent loading behavior regardless of storage engine configuration.components/log-viewer-webui/client/src/pages/IngestPage/Jobs/index.tsx (2)
33-41: LGTM! Clean React Query implementation.The transition from manual state management to React Query is well executed. The static query key, async query function, and proper data transformation all follow React Query best practices.
50-50: Good integration of loading state.Properly using React Query's
isPendingto control the table's loading state.components/log-viewer-webui/client/src/pages/IngestPage/SpaceSavings/sql.ts (5)
11-22: Well-defined interface and sensible defaults.The
SpaceSavingsIteminterface clearly defines the expected data structure, andSPACE_SAVINGS_DEFAULTprovides appropriate fallback values for empty data scenarios.
89-97: Appropriate error handling for React Query integration.The function correctly validates the query result and throws descriptive errors when data is missing, allowing React Query to properly handle failed states.
104-123: Well-structured fetch functions with appropriate optimizations.The functions correctly handle different storage engines, with
fetchClpsSpaceSavingsincluding a smart optimization to return default values immediately when no datasets are provided.
125-133: Comprehensive and well-organized exports.All necessary types and functions are properly exported for external consumption.
54-80: Verify dataset name validation to prevent SQL injection.The function directly interpolates dataset names into SQL strings (
clp_${name}_archives). Ensure that dataset names are properly validated/sanitized upstream to prevent SQL injection attacks.#!/bin/bash # Search for dataset name validation or sanitization in the codebase rg -A 5 -B 5 "dataset.*name.*valid|sanitize.*dataset|escape.*dataset" --type ts --type jscomponents/log-viewer-webui/client/src/pages/IngestPage/Details/sql.ts (3)
14-29: Appropriate interface design with nullable timestamps.The use of
Nullable<number>for timestamps is correct since begin/end times may not exist when no data is available. Default values are sensible.
119-153: Consistent error handling and execution patterns.The functions follow the same well-established patterns as the SpaceSavings module, providing consistency across the codebase. The empty dataset array optimization in
fetchClpsDetailsis appropriate.
155-163: Comprehensive exports maintaining consistency.The export structure matches the SpaceSavings module pattern, providing good consistency across the codebase.
| const querySql = async <T>(queryString: string) => { | ||
| return axios.post<T>("/query/sql", {queryString}); | ||
| }; |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Add input validation and consider SQL injection risks.
The function directly passes the queryString to the server without any validation. While this is an internal API, consider adding basic validation to prevent potential issues.
Additionally, the JSDoc is incomplete - the @return tag should specify the return type:
/**
* Query the SQL server with the queryString.
*
* @param queryString
- * @return
+ * @return Promise<AxiosResponse<T>> The axios response containing the query results
*/📝 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.
| const querySql = async <T>(queryString: string) => { | |
| return axios.post<T>("/query/sql", {queryString}); | |
| }; | |
| /** | |
| * Query the SQL server with the queryString. | |
| * | |
| * @param queryString | |
| * @return Promise<AxiosResponse<T>> The axios response containing the query results | |
| */ | |
| const querySql = async <T>(queryString: string) => { | |
| return axios.post<T>("/query/sql", {queryString}); | |
| }; |
🤖 Prompt for AI Agents
In components/log-viewer-webui/client/src/api/sql.ts around lines 10 to 12, the
querySql function lacks input validation and has incomplete JSDoc. Add basic
validation to check that queryString is a non-empty string and does not contain
suspicious patterns that could lead to SQL injection. Also, complete the JSDoc
by specifying the exact return type of the function, indicating it returns a
Promise of the axios post response with generic type T.
| const fetchDatasetNames = async (): Promise<string[]> => { | ||
| const resp = await querySql<DatasetItem[]>(GET_DATASETS_SQL); | ||
| return resp.data.map((dataset) => dataset.name); | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider adding error handling for the async operation.
The function lacks explicit error handling, which could lead to unhandled promise rejections if the SQL query fails.
Consider wrapping the implementation with try-catch or let the calling code handle errors with proper React Query error boundaries:
const fetchDatasetNames = async (): Promise<string[]> => {
+ try {
const resp = await querySql<DatasetItem[]>(GET_DATASETS_SQL);
return resp.data.map((dataset) => dataset.name);
+ } catch (error) {
+ console.error('Failed to fetch dataset names:', error);
+ throw error;
+ }
};Alternatively, document that error handling is expected to be managed by React Query's error handling mechanisms.
📝 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.
| const fetchDatasetNames = async (): Promise<string[]> => { | |
| const resp = await querySql<DatasetItem[]>(GET_DATASETS_SQL); | |
| return resp.data.map((dataset) => dataset.name); | |
| }; | |
| const fetchDatasetNames = async (): Promise<string[]> => { | |
| try { | |
| const resp = await querySql<DatasetItem[]>(GET_DATASETS_SQL); | |
| return resp.data.map((dataset) => dataset.name); | |
| } catch (error) { | |
| console.error('Failed to fetch dataset names:', error); | |
| throw error; | |
| } | |
| }; |
🤖 Prompt for AI Agents
In
components/log-viewer-webui/client/src/pages/SearchPage/SearchControls/Dataset/sql.ts
around lines 36 to 39, the async function fetchDatasetNames does not handle
errors from the SQL query, risking unhandled promise rejections. To fix this,
wrap the querySql call and subsequent data processing in a try-catch block to
catch and handle errors explicitly, or alternatively, document clearly that
error handling is delegated to React Query's error boundaries so callers are
aware of the expectation.
| import {QueryClientProvider} from "@tanstack/react-query"; | ||
| import {ReactQueryDevtools} from "@tanstack/react-query-devtools"; |
There was a problem hiding this comment.
🧩 Verification correct
Fix the import resolution issue.
The ESLint pipeline failure indicates that the @tanstack/react-query-devtools import cannot be resolved. Ensure this package is properly installed in package.json.
🏁 Script executed:
#!/bin/bash
# Check if react-query-devtools is installed as a dependency
cat components/log-viewer-webui/client/package.json | jq '.dependencies["@tanstack/react-query-devtools"], .devDependencies["@tanstack/react-query-devtools"]'Length of output: 170
Fix the import resolution issue.
The ESLint pipeline failure indicates that the @tanstack/react-query-devtools import cannot be resolved. Ensure this package is properly installed in package.json.
#!/bin/bash
# Check if react-query-devtools is installed as a dependency
cat components/log-viewer-webui/client/package.json | jq '.dependencies["@tanstack/react-query-devtools"], .devDependencies["@tanstack/react-query-devtools"]'🧰 Tools
🪛 GitHub Actions: clp-lint
[error] 4-4: ESLint: Unable to resolve path to module '@tanstack/react-query-devtools' (import/no-unresolved)
🤖 Prompt for AI Agents
In components/log-viewer-webui/client/src/AntdApp.tsx around lines 3 to 4, the
import of @tanstack/react-query-devtools cannot be resolved, causing ESLint
pipeline failure. Verify that @tanstack/react-query-devtools is listed as a
dependency or devDependency in components/log-viewer-webui/client/package.json.
If it is missing, add it by running the appropriate package manager install
command (e.g., npm install or yarn add) for @tanstack/react-query-devtools, then
rerun the build to confirm the import resolves correctly.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
components/log-viewer-webui/client/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (1)
components/log-viewer-webui/client/package.json(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: junhaoliao
PR: y-scope/clp#939
File: components/package-template/src/etc/clp-config.yml:64-64
Timestamp: 2025-06-22T04:01:43.409Z
Learning: The new webui architecture uses Fastify with Pino logging instead of the previous Winston-based logging system that was removed during the webui refactoring.
Learnt from: junhaoliao
PR: y-scope/clp#596
File: components/log-viewer-webui/client/src/api/query.js:16-23
Timestamp: 2024-11-21T15:51:33.203Z
Learning: In `components/log-viewer-webui/client/src/api/query.js`, the `ExtractJsonResp` type definition is accurate as-is and does not require modification. When suggesting changes to type definitions, ensure they align with the server-side definitions, referencing the source code if necessary.
Learnt from: junhaoliao
PR: y-scope/clp#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.
Learnt from: davemarco
PR: y-scope/clp#892
File: components/log-viewer-webui/client/src/api/socket/MongoCollectionSocket.ts:27-31
Timestamp: 2025-05-09T19:15:26.180Z
Learning: For the MongoDB real-time updates implementation in components/log-viewer-webui/client/src/api/socket, a socket singleton pattern is used where a single shared socket connection is maintained rather than creating multiple connections. The socket lifecycle is managed centrally, with unsubscription handling in the useCursor React hook's cleanup function.
Learnt from: junhaoliao
PR: y-scope/clp#596
File: components/log-viewer-webui/client/src/api/query.js:35-41
Timestamp: 2024-11-19T19:52:43.429Z
Learning: For internal APIs in `components/log-viewer-webui/client/src/api/query.js`, runtime validation of parameters may not be necessary since the APIs are not exposed to end users, and JsDoc type annotations may be sufficient.
Learnt from: junhaoliao
PR: y-scope/clp#962
File: components/log-viewer-webui/client/src/pages/SearchPage/SearchQueryStatus/Results.tsx:22-26
Timestamp: 2025-06-09T17:15:30.309Z
Learning: When using Zustand stores in React components, always use specific selectors (e.g., `useStore(state => state.property)`) instead of destructuring the entire state object to prevent unnecessary re-renders. This is especially important as state stores grow in complexity.
Learnt from: junhaoliao
PR: y-scope/clp#962
File: components/log-viewer-webui/client/src/pages/SearchPage/SearchQueryStatus/Results.tsx:22-26
Timestamp: 2025-06-09T17:15:30.309Z
Learning: When using Zustand stores in React components, always use specific selectors (e.g., `useStore(state => state.property)`) instead of destructuring the entire state object to prevent unnecessary re-renders. This is especially important as state stores grow in complexity.
components/log-viewer-webui/client/package.json (5)
Learnt from: junhaoliao
PR: y-scope/clp#596
File: components/log-viewer-webui/client/src/api/query.js:16-23
Timestamp: 2024-11-21T15:51:33.203Z
Learning: In `components/log-viewer-webui/client/src/api/query.js`, the `ExtractJsonResp` type definition is accurate as-is and does not require modification. When suggesting changes to type definitions, ensure they align with the server-side definitions, referencing the source code if necessary.
Learnt from: junhaoliao
PR: y-scope/clp#939
File: components/package-template/src/etc/clp-config.yml:64-64
Timestamp: 2025-06-22T04:01:43.409Z
Learning: The new webui architecture uses Fastify with Pino logging instead of the previous Winston-based logging system that was removed during the webui refactoring.
Learnt from: junhaoliao
PR: y-scope/clp#596
File: components/log-viewer-webui/client/src/api/query.js:35-41
Timestamp: 2024-11-19T19:52:43.429Z
Learning: For internal APIs in `components/log-viewer-webui/client/src/api/query.js`, runtime validation of parameters may not be necessary since the APIs are not exposed to end users, and JsDoc type annotations may be sufficient.
Learnt from: junhaoliao
PR: y-scope/clp#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.
Learnt from: davemarco
PR: y-scope/clp#797
File: components/log-viewer-webui/client/src/components/Layout/MainLayout.tsx:2-5
Timestamp: 2025-04-08T22:32:05.366Z
Learning: In this codebase using React Router v7.4.1, components should be imported directly from "react-router" (e.g., `import { Link, Outlet } from "react-router";`) rather than from "react-router-dom" as was common in previous versions of React Router.
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: lint-check (ubuntu-latest)
🔇 Additional comments (1)
components/log-viewer-webui/client/package.json (1)
22-22: Addition of@tanstack/react-querylooks solid
v5 works with React 19, and the caret range allows non-breaking patches. No red flags.
| "@mui/joy": "^5.0.0-beta.51", | ||
| "@sinclair/typebox": "^0.34.25", | ||
| "@tanstack/react-query": "^5.81.4", | ||
| "@tanstack/react-query-devtools": "^5.81.4", |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Shift DevTools to devDependencies to avoid bloating the prod bundle
@tanstack/react-query-devtools is purely a debugging aid; keeping it in dependencies ships the library to all end-users. Move it to devDependencies (or conditionally import it) to trim bundle size.
@@
"@mui/joy": "^5.0.0-beta.51",
"@sinclair/typebox": "^0.34.25",
"@tanstack/react-query": "^5.81.4",
- "@tanstack/react-query-devtools": "^5.81.4",
@@
"zustand": "^5.0.3"
},
"devDependencies": {
+ "@tanstack/react-query-devtools": "^5.81.4",
"@types/node": "22.15.31",📝 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.
| "@tanstack/react-query-devtools": "^5.81.4", | |
| // … other dependencies … | |
| "@mui/joy": "^5.0.0-beta.51", | |
| "@sinclair/typebox": "^0.34.25", | |
| "@tanstack/react-query": "^5.81.4", | |
| "zustand": "^5.0.3" | |
| }, | |
| "devDependencies": { | |
| "@tanstack/react-query-devtools": "^5.81.4", | |
| "@types/node": "22.15.31" | |
| // … other devDependencies … | |
| } |
🤖 Prompt for AI Agents
In components/log-viewer-webui/client/package.json at line 23, the package
@tanstack/react-query-devtools is listed under dependencies, which causes it to
be included in the production bundle unnecessarily. Move this package from the
dependencies section to the devDependencies section to ensure it is only used
during development and not shipped to end-users, thereby reducing the production
bundle size.
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (3)
components/webui/client/src/pages/IngestPage/SpaceSavings/index.tsx (2)
9-9: Fix the unresolved import path.Same issue as in the Details component - the import path
'../../SearchPage/SearchControls/Dataset/sql'cannot be resolved. This suggests a consistent issue across multiple files that needs to be addressed.
30-30: Address the unsafe assignment warning.Same TypeScript ESLint error as in the Details component regarding unsafe assignment of the
fetchDatasetNamesfunction result. This indicates a systemic type safety issue that should be resolved.components/webui/client/src/pages/IngestPage/Details/sql.ts (1)
69-84: Same SQL injection vulnerability as in SpaceSavings file.Dataset names are directly interpolated into SQL strings in both archive and file queries, creating the same security risk identified in the previous file.
The same input validation approach should be applied here:
const buildMultiDatasetDetailsSql = (datasetNames: string[]): string => { + // Validate dataset names to prevent SQL injection + const validatedNames = datasetNames.map(name => { + if (!/^[a-zA-Z0-9_]+$/.test(name)) { + throw new Error(`Invalid dataset name: ${name}`); + } + return name; + }); + - const archiveQueries = datasetNames.map((name) => ` + const archiveQueries = validatedNames.map((name) => ` // ... rest of function - const fileQueries = datasetNames.map((name) => ` + const fileQueries = validatedNames.map((name) => `
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
components/webui/client/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (17)
components/webui/client/package.json(1 hunks)components/webui/client/src/App.tsx(2 hunks)components/webui/client/src/api/sql.ts(1 hunks)components/webui/client/src/components/DashboardCard/index.tsx(2 hunks)components/webui/client/src/components/StatCard/index.tsx(3 hunks)components/webui/client/src/config/queryClient.ts(1 hunks)components/webui/client/src/pages/IngestPage/Details/DetailsCard.tsx(2 hunks)components/webui/client/src/pages/IngestPage/Details/Files.tsx(1 hunks)components/webui/client/src/pages/IngestPage/Details/Messages.tsx(1 hunks)components/webui/client/src/pages/IngestPage/Details/TimeRange.tsx(3 hunks)components/webui/client/src/pages/IngestPage/Details/index.tsx(1 hunks)components/webui/client/src/pages/IngestPage/Details/sql.ts(3 hunks)components/webui/client/src/pages/IngestPage/Jobs/index.tsx(3 hunks)components/webui/client/src/pages/IngestPage/SpaceSavings/CompressedSize.tsx(1 hunks)components/webui/client/src/pages/IngestPage/SpaceSavings/UncompressedSize.tsx(1 hunks)components/webui/client/src/pages/IngestPage/SpaceSavings/index.tsx(2 hunks)components/webui/client/src/pages/IngestPage/SpaceSavings/sql.ts(2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
`**/*.{cpp,hpp,java,js,jsx,tpp,ts,tsx}`: - Prefer `false == ` rather than `!`.
**/*.{cpp,hpp,java,js,jsx,tpp,ts,tsx}: - Preferfalse == <expression>rather than!<expression>.
⚙️ Source: CodeRabbit Configuration File
List of files the instruction was applied to:
components/webui/client/src/config/queryClient.tscomponents/webui/client/src/api/sql.tscomponents/webui/client/src/components/StatCard/index.tsxcomponents/webui/client/src/components/DashboardCard/index.tsxcomponents/webui/client/src/pages/IngestPage/Details/TimeRange.tsxcomponents/webui/client/src/pages/IngestPage/SpaceSavings/CompressedSize.tsxcomponents/webui/client/src/pages/IngestPage/Jobs/index.tsxcomponents/webui/client/src/App.tsxcomponents/webui/client/src/pages/IngestPage/Details/Files.tsxcomponents/webui/client/src/pages/IngestPage/Details/DetailsCard.tsxcomponents/webui/client/src/pages/IngestPage/Details/index.tsxcomponents/webui/client/src/pages/IngestPage/SpaceSavings/UncompressedSize.tsxcomponents/webui/client/src/pages/IngestPage/Details/Messages.tsxcomponents/webui/client/src/pages/IngestPage/SpaceSavings/index.tsxcomponents/webui/client/src/pages/IngestPage/Details/sql.tscomponents/webui/client/src/pages/IngestPage/SpaceSavings/sql.ts
🧠 Learnings (13)
📓 Common learnings
Learnt from: junhaoliao
PR: y-scope/clp#939
File: components/package-template/src/etc/clp-config.yml:64-64
Timestamp: 2025-06-22T04:01:43.409Z
Learning: The new webui architecture uses Fastify with Pino logging instead of the previous Winston-based logging system that was removed during the webui refactoring.
components/webui/client/package.json (3)
Learnt from: junhaoliao
PR: y-scope/clp#596
File: components/log-viewer-webui/client/src/api/query.js:16-23
Timestamp: 2024-11-21T15:51:33.203Z
Learning: In `components/log-viewer-webui/client/src/api/query.js`, the `ExtractJsonResp` type definition is accurate as-is and does not require modification. When suggesting changes to type definitions, ensure they align with the server-side definitions, referencing the source code if necessary.
Learnt from: junhaoliao
PR: y-scope/clp#939
File: components/package-template/src/etc/clp-config.yml:64-64
Timestamp: 2025-06-22T04:01:43.409Z
Learning: The new webui architecture uses Fastify with Pino logging instead of the previous Winston-based logging system that was removed during the webui refactoring.
Learnt from: davemarco
PR: y-scope/clp#797
File: components/log-viewer-webui/client/src/components/Layout/MainLayout.tsx:2-5
Timestamp: 2025-04-08T22:32:05.366Z
Learning: In this codebase using React Router v7.4.1, components should be imported directly from "react-router" (e.g., `import { Link, Outlet } from "react-router";`) rather than from "react-router-dom" as was common in previous versions of React Router.
components/webui/client/src/config/queryClient.ts (1)
Learnt from: junhaoliao
PR: y-scope/clp#596
File: components/log-viewer-webui/client/src/api/query.js:16-23
Timestamp: 2024-11-21T15:51:33.203Z
Learning: In `components/log-viewer-webui/client/src/api/query.js`, the `ExtractJsonResp` type definition is accurate as-is and does not require modification. When suggesting changes to type definitions, ensure they align with the server-side definitions, referencing the source code if necessary.
components/webui/client/src/api/sql.ts (1)
Learnt from: junhaoliao
PR: y-scope/clp#596
File: components/log-viewer-webui/client/src/api/query.js:16-23
Timestamp: 2024-11-21T15:51:33.203Z
Learning: In `components/log-viewer-webui/client/src/api/query.js`, the `ExtractJsonResp` type definition is accurate as-is and does not require modification. When suggesting changes to type definitions, ensure they align with the server-side definitions, referencing the source code if necessary.
components/webui/client/src/components/DashboardCard/index.tsx (1)
Learnt from: junhaoliao
PR: y-scope/clp#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.
components/webui/client/src/pages/IngestPage/Details/TimeRange.tsx (2)
Learnt from: junhaoliao
PR: y-scope/clp#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.
Learnt from: junhaoliao
PR: y-scope/clp#596
File: components/log-viewer-webui/client/src/api/query.js:16-23
Timestamp: 2024-11-21T15:51:33.203Z
Learning: In `components/log-viewer-webui/client/src/api/query.js`, the `ExtractJsonResp` type definition is accurate as-is and does not require modification. When suggesting changes to type definitions, ensure they align with the server-side definitions, referencing the source code if necessary.
components/webui/client/src/pages/IngestPage/SpaceSavings/CompressedSize.tsx (1)
Learnt from: junhaoliao
PR: y-scope/clp#596
File: components/log-viewer-webui/client/src/api/query.js:16-23
Timestamp: 2024-11-21T15:51:33.203Z
Learning: In `components/log-viewer-webui/client/src/api/query.js`, the `ExtractJsonResp` type definition is accurate as-is and does not require modification. When suggesting changes to type definitions, ensure they align with the server-side definitions, referencing the source code if necessary.
components/webui/client/src/pages/IngestPage/Jobs/index.tsx (4)
Learnt from: junhaoliao
PR: y-scope/clp#596
File: components/log-viewer-webui/client/src/api/query.js:16-23
Timestamp: 2024-11-21T15:51:33.203Z
Learning: In `components/log-viewer-webui/client/src/api/query.js`, the `ExtractJsonResp` type definition is accurate as-is and does not require modification. When suggesting changes to type definitions, ensure they align with the server-side definitions, referencing the source code if necessary.
Learnt from: junhaoliao
PR: y-scope/clp#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.
Learnt from: junhaoliao
PR: y-scope/clp#596
File: components/log-viewer-webui/client/src/api/query.js:35-41
Timestamp: 2024-11-19T19:52:43.429Z
Learning: For internal APIs in `components/log-viewer-webui/client/src/api/query.js`, runtime validation of parameters may not be necessary since the APIs are not exposed to end users, and JsDoc type annotations may be sufficient.
Learnt from: davemarco
PR: y-scope/clp#797
File: components/log-viewer-webui/client/src/components/Layout/MainLayout.tsx:2-5
Timestamp: 2025-04-08T22:32:05.366Z
Learning: In this codebase using React Router v7.4.1, components should be imported directly from "react-router" (e.g., `import { Link, Outlet } from "react-router";`) rather than from "react-router-dom" as was common in previous versions of React Router.
components/webui/client/src/App.tsx (5)
Learnt from: davemarco
PR: y-scope/clp#797
File: components/log-viewer-webui/client/src/components/Layout/MainLayout.tsx:2-5
Timestamp: 2025-04-08T22:32:05.366Z
Learning: In this codebase using React Router v7.4.1, components should be imported directly from "react-router" (e.g., `import { Link, Outlet } from "react-router";`) rather than from "react-router-dom" as was common in previous versions of React Router.
Learnt from: junhaoliao
PR: y-scope/clp#596
File: components/log-viewer-webui/client/src/api/query.js:16-23
Timestamp: 2024-11-21T15:51:33.203Z
Learning: In `components/log-viewer-webui/client/src/api/query.js`, the `ExtractJsonResp` type definition is accurate as-is and does not require modification. When suggesting changes to type definitions, ensure they align with the server-side definitions, referencing the source code if necessary.
Learnt from: junhaoliao
PR: y-scope/clp#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.
Learnt from: davemarco
PR: y-scope/clp#797
File: components/log-viewer-webui/client/src/routes/routes.tsx:1-1
Timestamp: 2025-04-04T15:15:59.432Z
Learning: In React Router v7, `createBrowserRouter` and other router primitives should be imported from `react-router` package, not from `react-router-dom` as in previous versions. This represents a significant change in the library's structure where router primitives are now in the core package.
Learnt from: davemarco
PR: y-scope/clp#797
File: components/log-viewer-webui/client/src/components/Layout/MainLayout.tsx:2-5
Timestamp: 2025-04-08T22:32:05.367Z
Learning: React Router v7.4.1 uses a different package structure than previous versions. Components can be imported directly from "react-router" (e.g., `import { Link, Outlet } from "react-router";`) rather than from "react-router-dom" as was common in previous versions. Alternatively, DOM-specific components can also be imported using deep imports like `import { Link } from "react-router/dom";`.
components/webui/client/src/pages/IngestPage/Details/DetailsCard.tsx (1)
Learnt from: junhaoliao
PR: y-scope/clp#596
File: components/log-viewer-webui/client/src/api/query.js:16-23
Timestamp: 2024-11-21T15:51:33.203Z
Learning: In `components/log-viewer-webui/client/src/api/query.js`, the `ExtractJsonResp` type definition is accurate as-is and does not require modification. When suggesting changes to type definitions, ensure they align with the server-side definitions, referencing the source code if necessary.
components/webui/client/src/pages/IngestPage/Details/index.tsx (2)
Learnt from: junhaoliao
PR: y-scope/clp#596
File: components/log-viewer-webui/client/src/api/query.js:16-23
Timestamp: 2024-11-21T15:51:33.203Z
Learning: In `components/log-viewer-webui/client/src/api/query.js`, the `ExtractJsonResp` type definition is accurate as-is and does not require modification. When suggesting changes to type definitions, ensure they align with the server-side definitions, referencing the source code if necessary.
Learnt from: junhaoliao
PR: y-scope/clp#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.
components/webui/client/src/pages/IngestPage/SpaceSavings/UncompressedSize.tsx (2)
Learnt from: junhaoliao
PR: y-scope/clp#596
File: components/log-viewer-webui/client/src/api/query.js:16-23
Timestamp: 2024-11-21T15:51:33.203Z
Learning: In `components/log-viewer-webui/client/src/api/query.js`, the `ExtractJsonResp` type definition is accurate as-is and does not require modification. When suggesting changes to type definitions, ensure they align with the server-side definitions, referencing the source code if necessary.
Learnt from: junhaoliao
PR: y-scope/clp#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.
components/webui/client/src/pages/IngestPage/Details/sql.ts (1)
Learnt from: junhaoliao
PR: y-scope/clp#596
File: components/log-viewer-webui/client/src/api/query.js:16-23
Timestamp: 2024-11-21T15:51:33.203Z
Learning: In `components/log-viewer-webui/client/src/api/query.js`, the `ExtractJsonResp` type definition is accurate as-is and does not require modification. When suggesting changes to type definitions, ensure they align with the server-side definitions, referencing the source code if necessary.
🪛 GitHub Actions: clp-lint
components/webui/client/src/pages/IngestPage/Details/index.tsx
[error] 8-8: ESLint: Unable to resolve path to module '../../SearchPage/SearchControls/Dataset/sql' (import/no-unresolved)
[error] 28-28: ESLint: Unsafe assignment of an error typed value (@typescript-eslint/no-unsafe-assignment)
components/webui/client/src/pages/IngestPage/SpaceSavings/index.tsx
[error] 9-9: ESLint: Unable to resolve path to module '../../SearchPage/SearchControls/Dataset/sql' (import/no-unresolved)
[error] 30-30: ESLint: Unsafe assignment of an error typed value (@typescript-eslint/no-unsafe-assignment)
🔇 Additional comments (30)
components/webui/client/src/config/queryClient.ts (1)
1-14: LGTM! Clean React Query configuration.The QueryClient configuration is well-structured with a sensible 10-second stale time that balances data freshness with performance. The use of underscore number separators enhances readability.
components/webui/client/src/api/sql.ts (1)
4-12: LGTM! Clean and reusable SQL query utility.The generic function design is excellent and provides a consistent interface for SQL queries. The lack of explicit error handling is appropriate since React Query will handle errors at the hook level.
components/webui/client/package.json (1)
21-22: React Query v5.81.4 is secure; optional upgrade to v5.81.5The added React Query dependencies have no known security issues in version 5.81.4. To stay up-to-date:
- @tanstack/react-query v5.81.4 has no reported vulnerabilities.
- The latest release is v5.81.5 (June 27, 2025).
Consider updating both entries in package.json:
- "@tanstack/react-query": "^5.81.4", - "@tanstack/react-query-devtools": "^5.81.4", + "@tanstack/react-query": "^5.81.5", + "@tanstack/react-query-devtools": "^5.81.5",components/webui/client/src/pages/IngestPage/Details/TimeRange.tsx (1)
11-11: LGTM! Proper loading state integration.The loading state prop is correctly added to the interface, documented in JSDoc, and properly propagated to the child component. The implementation follows consistent patterns for loading state management.
Also applies to: 20-20, 23-23, 33-33
components/webui/client/src/components/StatCard/index.tsx (1)
18-18: LGTM! Well-implemented optional loading state prop.The optional
isLoadingprop is properly typed, documented, and defaulted tofalsefor backward compatibility. The prop propagation toDashboardCardfollows the established pattern correctly.Also applies to: 31-31, 41-41, 45-45
components/webui/client/src/components/DashboardCard/index.tsx (1)
16-16: LGTM! Well-implemented loading state support.The optional
isLoadingprop with sensible default value maintains backward compatibility while enabling loading state control. The integration with Ant Design's Card loading attribute is appropriate.Also applies to: 27-27, 35-36, 41-41
components/webui/client/src/App.tsx (1)
3-4: LGTM! Proper React Query integration.The setup follows React Query best practices with QueryClientProvider at the root level and devtools for debugging. The component hierarchy is correct and enables React Query context throughout the app.
Also applies to: 7-7, 21-28
components/webui/client/src/pages/IngestPage/SpaceSavings/CompressedSize.tsx (1)
7-7: LGTM! Consistent loading state propagation.The required
isLoadingprop is properly integrated and propagated to the DetailsCard component, maintaining consistency with the loading state management pattern.Also applies to: 15-15, 18-18, 21-21
components/webui/client/src/pages/IngestPage/Details/DetailsCard.tsx (1)
9-9: LGTM! Proper loading state forwarding.The required
isLoadingprop is correctly implemented and forwarded to the StatCard component, maintaining the loading state propagation chain.Also applies to: 18-18, 21-21, 25-25
components/webui/client/src/pages/IngestPage/Details/Messages.tsx (1)
8-8: LGTM! Consistent loading state integration.The required
isLoadingprop follows the established pattern and is properly propagated to DetailsCard. The existing message handling logic remains appropriately unchanged.Also applies to: 16-16, 19-19, 22-22
components/webui/client/src/pages/IngestPage/Details/Files.tsx (1)
6-26: Clean implementation of loading state support.The addition of the
isLoadingprop follows a consistent pattern across components and is properly documented and propagated to theDetailsCardcomponent.components/webui/client/src/pages/IngestPage/SpaceSavings/UncompressedSize.tsx (1)
5-25: Consistent implementation of loading state support.The changes match the established pattern for adding loading state support across components, with proper documentation and prop propagation.
components/webui/client/src/pages/IngestPage/Jobs/index.tsx (3)
1-4: Good React Query integration.The import changes properly bring in the necessary React Query functionality and centralized SQL query API.
33-41: Well-structured React Query implementation.The
useQueryhook is properly configured with:
- Clear query key for caching
- Async query function that fetches and transforms data
- Good default value to prevent undefined errors
The data transformation flow is clean and follows the established pattern.
50-50: Proper loading state management.Using
isPendingfrom React Query provides a more reliable loading state compared to manual state management.components/webui/client/src/pages/IngestPage/Details/index.tsx (3)
26-30: Good conditional query implementation.The dataset names query is properly conditionally enabled based on the storage engine configuration, which prevents unnecessary queries when using CLP storage engine.
32-45: Well-structured dependency management.The details query properly depends on the dataset names and includes appropriate enabling conditions. The storage engine branching logic is clear and handles both CLP and CLP_S scenarios.
52-62: Proper loading state propagation.The
isPendingstate is correctly passed asisLoadingto all child components, ensuring consistent loading state representation throughout the UI.components/webui/client/src/pages/IngestPage/SpaceSavings/index.tsx (3)
28-32: Consistent conditional query pattern.The dataset names query implementation is consistent with the Details component, properly conditionally enabled based on storage engine configuration.
34-47: Well-structured space savings query.The query implementation follows the established pattern with proper dependency management and storage engine branching logic. The enabling conditions ensure queries only run when appropriate.
63-63: Comprehensive loading state propagation.The
isPendingstate is properly propagated to all relevant components (StatCard, UncompressedSize, CompressedSize), ensuring a consistent loading experience across the space savings section.Also applies to: 71-71, 75-75
components/webui/client/src/pages/IngestPage/SpaceSavings/sql.ts (4)
9-12: Well-structured interface definition.The
SpaceSavingsIteminterface appropriately represents the expected SQL query result structure with clear property names and correct types.
17-20: Good defensive programming with default values.The
SPACE_SAVINGS_DEFAULTconstant provides sensible fallback values when no data is available, which aligns with the defensive programming approach used in the fetch functions.
87-95: Proper error handling and type safety.The
executeSpaceSavingsQueryfunction correctly handles the case where the query returns no data and provides clear error messages. The type checking follows good practices.
113-121: Well-implemented conditional logic for multi-dataset scenarios.The
fetchClpsSpaceSavingsfunction properly handles the edge case where no datasets are provided by returning default values, which aligns with the PR objective of handling systems without datasets.components/webui/client/src/pages/IngestPage/Details/sql.ts (5)
14-19: Appropriate use of nullable types for optional data.The
DetailsIteminterface correctly usesNullable<number>for timestamps and counts that may not be available, providing proper type safety for potentially missing data.
24-29: Sensible default values for missing data.The
DETAILS_DEFAULTconstant provides appropriate fallback values, with null for timestamps (indicating no data) and 0 for counts (indicating empty state).
86-109: Complex but logically sound multi-dataset query structure.The nested query approach correctly combines data from multiple datasets by:
- Unioning archive queries to find global min/max timestamps
- Unioning file queries to sum counts across datasets
- Cross-joining the results to produce final statistics
The logic appears sound for aggregating details across multiple datasets.
119-127: Consistent error handling pattern.The
executeDetailsQueryfunction follows the same error handling pattern as the SpaceSavings equivalent, providing consistency across the codebase.
145-153: Proper handling of empty dataset scenarios.The
fetchClpsDetailsfunction correctly returns default values when no datasets are provided, maintaining consistency with the SpaceSavings implementation and supporting the PR objective.
| const [numMessages, setNumMessages] = useState<Nullable<number>>(DETAILS_DEFAULT.numMessages); | ||
| const {data: datasetNames = [], isSuccess: isSuccessDatasetNames} = useQuery({ | ||
| queryKey: ["datasets"], | ||
| queryFn: fetchDatasetNames, |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Address the unsafe assignment warning.
The TypeScript ESLint error suggests there's an unsafe assignment of an error-typed value. Ensure that the fetchDatasetNames function returns the expected type and handle potential errors appropriately.
🏁 Script executed:
#!/bin/bash
# Check the return type of fetchDatasetNames function
ast-grep --pattern 'export $_ fetchDatasetNames'Length of output: 50
🏁 Script executed:
#!/bin/bash
# Locate all references and the definition of fetchDatasetNames
rg 'fetchDatasetNames' -nLength of output: 792
🏁 Script executed:
#!/bin/bash
# Display lines around the usage of fetchDatasetNames in the Details page
sed -n '1,200p' components/webui/client/src/pages/IngestPage/Details/index.tsxLength of output: 2314
Specify useQuery generics to avoid unsafe unknown assignment
The @typescript-eslint/no-unsafe-assignment error is due to useQuery inferring data as unknown by default. Since fetchDatasetNames returns Promise<string[]>, you should explicitly type the hook so that data is string[].
• In components/webui/client/src/pages/IngestPage/Details/index.tsx (around line 28) update the hook:
- const { data: datasetNames = [], isSuccess: isSuccessDatasetNames } = useQuery({
+ const { data: datasetNames = [], isSuccess: isSuccessDatasetNames } = useQuery<string[], Error>({
queryKey: ["datasets"],
queryFn: fetchDatasetNames,
enabled: CLP_STORAGE_ENGINES.CLP_S === SETTINGS_STORAGE_ENGINE,
});Optionally, apply the same change in components/webui/client/src/pages/IngestPage/SpaceSavings/index.tsx.
This ensures datasetNames is typed correctly and prevents unsafe assignment.
📝 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.
| queryFn: fetchDatasetNames, | |
| const { data: datasetNames = [], isSuccess: isSuccessDatasetNames } = useQuery<string[], Error>({ | |
| queryKey: ["datasets"], | |
| queryFn: fetchDatasetNames, | |
| enabled: CLP_STORAGE_ENGINES.CLP_S === SETTINGS_STORAGE_ENGINE, | |
| }); |
🧰 Tools
🪛 GitHub Actions: clp-lint
[error] 28-28: ESLint: Unsafe assignment of an error typed value (@typescript-eslint/no-unsafe-assignment)
🤖 Prompt for AI Agents
In components/webui/client/src/pages/IngestPage/Details/index.tsx at line 28,
the useQuery hook is currently untyped, causing data to default to unknown and
triggering a no-unsafe-assignment error. Fix this by specifying the generic type
parameter for useQuery as string[], matching the Promise<string[]> return type
of fetchDatasetNames. This will ensure datasetNames is correctly typed as
string[] and prevent unsafe assignment errors. Optionally, apply the same
generic typing fix to the useQuery hook in
components/webui/client/src/pages/IngestPage/SpaceSavings/index.tsx.
| import useIngestStatsStore from "../ingestStatsStore"; | ||
| import {querySql} from "../sqlConfig"; | ||
| import {fetchDatasetNames} from "../../SearchPage/SearchControls/Dataset/sql"; | ||
| import Files from "./Files"; |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Fix the unresolved import path.
The ESLint error indicates that the import path '../../SearchPage/SearchControls/Dataset/sql' cannot be resolved. Verify that this file exists and the import path is correct relative to the current file.
🏁 Script executed:
#!/bin/bash
# Verify the import path exists and check the correct relative path
fd -t f "sql.ts" | grep -E "(SearchPage|Dataset)" | head -10Length of output: 146
#!/bin/bash
# Locate the IngestPage Details index file to confirm its path
fd -t f index.tsx | grep -E "IngestPage/Details"🏁 Script executed:
#!/bin/bash
# Find the path of the IngestPage Details index file and verify the import target
file=$(fd "Details/index.tsx" | head -n1)
echo "Found index.tsx at: $file"
dir=$(dirname "$file")
import_rel="../../SearchPage/SearchControls/Dataset/sql.ts"
echo "Checking existence of import target: $dir/$import_rel"
if [ -f "$dir/$import_rel" ]; then
echo "✅ Import target exists at $dir/$import_rel"
else
echo "❌ Import target missing at $dir/$import_rel"
fi
# List all sql.ts occurrences for context
echo "All sql.ts files in project:"
fd -t f "sql.ts"Length of output: 1036
🏁 Script executed:
#!/bin/bash
# 1. Locate the IngestPage Details index file
echo "Searching for Details/index.tsx…"
fd -t f index.tsx | grep -E "IngestPage/Details"
# 2. Find where fetchDatasetNames is defined in the repo
echo "Looking for fetchDatasetNames definitions…"
rg "fetchDatasetNames" -n .Length of output: 1088
#!/bin/bash
# Display the top of Details/sql.ts
echo "=== components/webui/client/src/pages/IngestPage/Details/sql.ts ==="
sed -n '1,20p' "$(fd -t f sql.ts | grep 'webui/client/src/pages/IngestPage/Details/sql.ts')"
# Display the top of api/sql.ts to see if fetchDatasetNames is exported
echo -e "\n=== components/webui/client/src/api/sql.ts ==="
sed -n '1,20p' "$(fd -t f sql.ts | grep 'webui/client/src/api/sql.ts')"#!/bin/bash
# Locate and display the Details/sql.ts file
details_sql=$(fd -t f sql.ts | grep 'webui/client/src/pages/IngestPage/Details/sql.ts')
echo "=== $details_sql ==="
sed -n '1,50p' "$details_sql"
# Locate and inspect client api/sql.ts for fetchDatasetNames export
api_sql=$(fd -t f sql.ts | grep 'webui/client/src/api/sql.ts')
echo -e "\n=== $api_sql ==="
grep -R "export.*fetchDatasetNames" -n "$api_sql" || echo "No fetchDatasetNames export found in api/sql.ts"
# Search all of webui/client for fetchDatasetNames occurrences
echo -e "\n=== All fetchDatasetNames in webui/client ==="
grep -R "fetchDatasetNames" -n components/webui/client/srcFix broken import path for fetchDatasetNames
The import
import {fetchDatasetNames} from "../../SearchPage/SearchControls/Dataset/sql";does not resolve because there is no pages/SearchPage/SearchControls/Dataset/sql.ts in the webui client. Update to the actual module that exports fetchDatasetNames. For example, if it lives in your shared API:
• File: components/webui/client/src/pages/IngestPage/Details/index.tsx (line 8)
• File: components/webui/client/src/pages/IngestPage/SpaceSavings/index.tsx (line 9)
Suggested diff:
- import {fetchDatasetNames} from "../../SearchPage/SearchControls/Dataset/sql";
+ import {fetchDatasetNames} from "../../../api/sql";Adjust the path according to where fetchDatasetNames is truly exported.
🧰 Tools
🪛 GitHub Actions: clp-lint
[error] 8-8: ESLint: Unable to resolve path to module '../../SearchPage/SearchControls/Dataset/sql' (import/no-unresolved)
🤖 Prompt for AI Agents
In components/webui/client/src/pages/IngestPage/Details/index.tsx at line 8, the
import path for fetchDatasetNames is incorrect because the referenced file does
not exist. Locate the correct module that exports fetchDatasetNames, such as a
shared API or another valid path, and update the import statement to use that
correct path to fix the broken import.
| return ` | ||
| SELECT | ||
| CAST( | ||
| COALESCE( | ||
| SUM(${CLP_ARCHIVES_TABLE_COLUMN_NAMES.UNCOMPRESSED_SIZE}), | ||
| 0 | ||
| ) AS UNSIGNED | ||
| ) AS total_uncompressed_size, | ||
| CAST( | ||
| COALESCE( | ||
| SUM(${CLP_ARCHIVES_TABLE_COLUMN_NAMES.SIZE}), | ||
| 0 | ||
| ) AS UNSIGNED | ||
| ) AS total_compressed_size | ||
| FROM ( | ||
| ${archiveQueries.join("\nUNION ALL\n")} | ||
| ) AS archives_combined | ||
| `; |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Consider performance implications of large UNION ALL queries.
While the UNION ALL approach works well for a limited number of datasets (as mentioned in the PR description), it could become performance-intensive with many datasets since it generates a query with multiple subqueries.
Consider monitoring query performance and establishing limits:
🏁 Script executed:
#!/bin/bash
# Description: Check for existing dataset count limits or performance considerations in codebase
# Search for dataset count limits or performance-related comments
rg -i "dataset.*limit|performance.*dataset|max.*dataset" --type ts --type js
# Look for similar UNION ALL patterns that might have performance considerations
rg -A 5 -B 5 "UNION ALL" --type ts --type jsLength of output: 2641
Add safeguards and monitoring for large UNION ALL queries
No existing dataset‐count limits or performance guards were found in the codebase. To prevent potential slowdowns as the number of archives grows, consider:
- Introducing a configurable maximum number of datasets allowed in
archiveQueries - Emitting telemetry or logging query execution time to identify regressions
- Falling back to batched queries or a summary table if the dataset count exceeds the limit
- Documenting this behaviour in the code (e.g. JSDoc or README) so other teams are aware of the constraints
🤖 Prompt for AI Agents
In components/webui/client/src/pages/IngestPage/SpaceSavings/sql.ts around lines
60 to 77, add a configurable maximum limit on the number of datasets in
archiveQueries to prevent performance issues with large UNION ALL queries.
Implement telemetry or logging to capture query execution times for monitoring.
If the dataset count exceeds the limit, modify the code to fall back to batched
queries or use a summary table. Also, add clear documentation in the code
comments or README explaining these safeguards and constraints for future
maintainers.
| const buildMultiDatasetSpaceSavingsSql = (datasetNames: string[]): string => { | ||
| const archiveQueries = datasetNames.map((name) => ` | ||
| SELECT | ||
| ${CLP_ARCHIVES_TABLE_COLUMN_NAMES.UNCOMPRESSED_SIZE}, | ||
| ${CLP_ARCHIVES_TABLE_COLUMN_NAMES.SIZE} | ||
| FROM clp_${name}_archives | ||
| `); |
There was a problem hiding this comment.
Potential SQL injection vulnerability with dataset name interpolation.
The dataset names are directly interpolated into SQL strings without validation or sanitisation. If dataset names come from user input or external sources, this could lead to SQL injection attacks.
Consider adding input validation for dataset names:
const buildMultiDatasetSpaceSavingsSql = (datasetNames: string[]): string => {
+ // Validate dataset names to prevent SQL injection
+ const validatedNames = datasetNames.map(name => {
+ if (!/^[a-zA-Z0-9_]+$/.test(name)) {
+ throw new Error(`Invalid dataset name: ${name}`);
+ }
+ return name;
+ });
+
- const archiveQueries = datasetNames.map((name) => `
+ const archiveQueries = validatedNames.map((name) => `🤖 Prompt for AI Agents
In components/webui/client/src/pages/IngestPage/SpaceSavings/sql.ts around lines
52 to 58, dataset names are directly interpolated into SQL strings, risking SQL
injection. To fix this, implement strict validation to ensure datasetNames
contain only safe characters (e.g., alphanumeric and underscores) before
interpolation. Reject or sanitize any names that do not meet these criteria to
prevent injection vulnerabilities.
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
components/webui/client/src/pages/SearchPage/SearchControls/Dataset/sql.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
`**/*.{cpp,hpp,java,js,jsx,tpp,ts,tsx}`: - Prefer `false == ` rather than `!`.
**/*.{cpp,hpp,java,js,jsx,tpp,ts,tsx}: - Preferfalse == <expression>rather than!<expression>.
⚙️ Source: CodeRabbit Configuration File
List of files the instruction was applied to:
components/webui/client/src/pages/SearchPage/SearchControls/Dataset/sql.ts
🧠 Learnings (1)
📓 Common learnings
Learnt from: junhaoliao
PR: y-scope/clp#939
File: components/package-template/src/etc/clp-config.yml:64-64
Timestamp: 2025-06-22T04:01:43.409Z
Learning: The new webui architecture uses Fastify with Pino logging instead of the previous Winston-based logging system that was removed during the webui refactoring.
Learnt from: junhaoliao
PR: y-scope/clp#596
File: components/log-viewer-webui/client/src/api/query.js:16-23
Timestamp: 2024-11-21T15:51:33.203Z
Learning: In `components/log-viewer-webui/client/src/api/query.js`, the `ExtractJsonResp` type definition is accurate as-is and does not require modification. When suggesting changes to type definitions, ensure they align with the server-side definitions, referencing the source code if necessary.
Learnt from: davemarco
PR: y-scope/clp#892
File: components/log-viewer-webui/client/src/api/socket/MongoCollectionSocket.ts:27-31
Timestamp: 2025-05-09T19:15:26.180Z
Learning: For the MongoDB real-time updates implementation in components/log-viewer-webui/client/src/api/socket, a socket singleton pattern is used where a single shared socket connection is maintained rather than creating multiple connections. The socket lifecycle is managed centrally, with unsubscription handling in the useCursor React hook's cleanup function.
🔇 Additional comments (3)
components/webui/client/src/pages/SearchPage/SearchControls/Dataset/sql.ts (3)
1-1: LGTM: Clean import structure.The relative import path is correctly structured for the project architecture.
20-25: LGTM: Well-structured SQL query.The query uses parameterized references through constants/enums, preventing SQL injection. The formatting is clear and the ORDER BY clause ensures consistent results.
7-7: Consider using PascalCase for consistency.The constant name uses PascalCase mixed with underscores, which is inconsistent. Consider using either pure PascalCase or SCREAMING_SNAKE_CASE for constants.
-const SqlDbClpDatasetsTableName = "clp_datasets"; +const SQL_DB_CLP_DATASETS_TABLE_NAME = "clp_datasets";⛔ Skipped due to learnings
Learnt from: LinZhihao-723 PR: y-scope/clp#873 File: components/core/src/clp/ffi/ir_stream/search/QueryHandlerImpl.hpp:320-324 Timestamp: 2025-05-05T01:12:18.561Z Learning: In the CLP codebase, the `m_case_sensitive_search` flag is used only for actual string value comparisons during query evaluation, not for schema key name matching. Schema keys are always compared case-sensitively regardless of this flag's setting.Learnt from: haiqi96 PR: y-scope/clp#594 File: components/clp-package-utils/clp_package_utils/scripts/native/del_archives.py:90-96 Timestamp: 2024-11-15T16:22:33.635Z Learning: In the CLP codebase, `table_prefix` is a trusted configuration value and can be safely embedded into SQL queries.Learnt from: junhaoliao PR: y-scope/clp#988 File: components/log-viewer-webui/client/src/components/QueryBox/InputWithCaseSensitive/CaseSenstiveToggle.tsx:34-37 Timestamp: 2025-06-09T17:48:56.024Z Learning: In the y-scope/clp project, prefer `false == <expression>` rather than `!<expression>` for boolean expressions in TypeScript/JavaScript files, as specified in the coding guidelines.Learnt from: Bill-hbrhbr PR: y-scope/clp#868 File: components/clp-py-utils/clp_py_utils/clp_metadata_db_utils.py:141-144 Timestamp: 2025-05-05T16:32:55.163Z Learning: The column metadata table (created by `_create_column_metadata_table`) is only needed for dataset-specific workflows in `CLP_S` and is obsolete for non-dataset workflows.
| enum CLP_DATASETS_TABLE_COLUMN_NAMES { | ||
| NAME = "name", | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Improve enum naming convention.
The enum name uses SCREAMING_SNAKE_CASE which is typically reserved for constants. TypeScript enums conventionally use PascalCase.
-enum CLP_DATASETS_TABLE_COLUMN_NAMES {
+enum ClpDatasetsTableColumnNames {
NAME = "name",
}🤖 Prompt for AI Agents
In components/webui/client/src/pages/SearchPage/SearchControls/Dataset/sql.ts
around lines 13 to 15, rename the enum CLP_DATASETS_TABLE_COLUMN_NAMES from
SCREAMING_SNAKE_CASE to PascalCase, such as ClpDatasetsTableColumnNames, to
follow TypeScript enum naming conventions.
| interface DatasetItem { | ||
| [CLP_DATASETS_TABLE_COLUMN_NAMES.NAME]: string; | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Simplify interface definition.
Using the enum as a key is unusual and makes the interface less readable. Consider using the string literal directly since there's only one property.
-interface DatasetItem {
- [CLP_DATASETS_TABLE_COLUMN_NAMES.NAME]: string;
-}
+interface DatasetItem {
+ name: string;
+}📝 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.
| interface DatasetItem { | |
| [CLP_DATASETS_TABLE_COLUMN_NAMES.NAME]: string; | |
| } | |
| interface DatasetItem { | |
| name: string; | |
| } |
🤖 Prompt for AI Agents
In components/webui/client/src/pages/SearchPage/SearchControls/Dataset/sql.ts
around lines 27 to 29, the DatasetItem interface uses an enum value as a key,
which reduces readability. Replace the enum key with the direct string literal
for the property name to simplify the interface definition and improve clarity.
| /** | ||
| * Fetches all datasets names from the `clp_datasets` table. | ||
| * | ||
| * @return | ||
| */ | ||
| const fetchDatasetNames = async (): Promise<string[]> => { | ||
| const resp = await querySql<DatasetItem[]>(GET_DATASETS_SQL); | ||
| return resp.data.map((dataset) => dataset.name); | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add error handling and improve documentation.
The function lacks error handling and the JSDoc comment is incomplete. Consider adding error handling for robustness and complete the documentation.
/**
* Fetches all datasets names from the `clp_datasets` table.
*
- * @return
+ * @returns Promise that resolves to an array of dataset names
+ * @throws Error if the SQL query fails or returns invalid data
*/
-const fetchDatasetNames = async (): Promise<string[]> => {
- const resp = await querySql<DatasetItem[]>(GET_DATASETS_SQL);
- return resp.data.map((dataset) => dataset.name);
-};
+const fetchDatasetNames = async (): Promise<string[]> => {
+ try {
+ const resp = await querySql<DatasetItem[]>(GET_DATASETS_SQL);
+ if (!resp.data || !Array.isArray(resp.data)) {
+ throw new Error("Invalid response format from dataset query");
+ }
+ return resp.data.map((dataset) => dataset.name);
+ } catch (error) {
+ console.error("Failed to fetch dataset names:", error);
+ throw error;
+ }
+};📝 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.
| /** | |
| * Fetches all datasets names from the `clp_datasets` table. | |
| * | |
| * @return | |
| */ | |
| const fetchDatasetNames = async (): Promise<string[]> => { | |
| const resp = await querySql<DatasetItem[]>(GET_DATASETS_SQL); | |
| return resp.data.map((dataset) => dataset.name); | |
| }; | |
| /** | |
| * Fetches all datasets names from the `clp_datasets` table. | |
| * | |
| * @returns Promise that resolves to an array of dataset names | |
| * @throws Error if the SQL query fails or returns invalid data | |
| */ | |
| const fetchDatasetNames = async (): Promise<string[]> => { | |
| try { | |
| const resp = await querySql<DatasetItem[]>(GET_DATASETS_SQL); | |
| if (!resp.data || !Array.isArray(resp.data)) { | |
| throw new Error("Invalid response format from dataset query"); | |
| } | |
| return resp.data.map((dataset) => dataset.name); | |
| } catch (error) { | |
| console.error("Failed to fetch dataset names:", error); | |
| throw error; | |
| } | |
| }; |
🤖 Prompt for AI Agents
In components/webui/client/src/pages/SearchPage/SearchControls/Dataset/sql.ts
around lines 31 to 39, the fetchDatasetNames function lacks error handling and
has an incomplete JSDoc comment. Add try-catch to handle potential errors from
the querySql call and return an empty array or rethrow the error as appropriate.
Complete the JSDoc comment by specifying the return type and describing the
function's behavior including error scenarios.
| console.error("Failed to fetch space savings stats", e); | ||
| }); | ||
| }, []); | ||
| const {data: datasetNames = [], isSuccess: isSuccessDatasetNames} = useQuery({ |
There was a problem hiding this comment.
for reviewer, this enabled looks a bit weird, but this is there recommendation in the docs for dependant queries. https://tanstack.com/query/latest/docs/framework/react/guides/dependent-queries
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
hoophalab
left a comment
hoophalab
left a comment
There was a problem hiding this comment.
@kirkrodrigues raised some concerns about the number of UI bugs we've seen over the past couple of weeks.
Instead of giving an easy approval, I will share suggestions that I believe will improve the modularity of the web UI.
I noticed a few UI state issues, but I'm not sure if they're introduced in this PR (by react-query) or not. So here's the plan:
- Let’s start by addressing the coding style issues. I’m seriously concerned that if we leave them unchecked, they’ll lead to more and more UI bugs over time -- so please make sure to fix them.
- Once #1050 is merged, I'll take another look and see if those UI state issues still persist.
Normally, a PR shouldn’t mix refactoring with new features, but I’m okay with including the refactoring here since the PR is already at +482 lines.
| * @return | ||
| */ | ||
| const DetailsCard = ({title, stat}: DetailsCardProps) => { | ||
| const DetailsCard = ({title, stat, isLoading}: DetailsCardProps) => { |
There was a problem hiding this comment.
This inheritance-style dependencies is too crazy.
We have
DetailsCard -> StatCard -> DashboardCard
You are passing isLoading all the way down to DashboardCard which is a typical problem of inheritance
Suggestion:
- Combine all three into one:
DetailsCardas all cards requires title and stat/content DetailsCardacceptstitleas a propertyDetailsCardaccepts content text (i.e.No timestamp dataand0.00%) as react node in children, so that space saving can have a different fontsize.- two options:
- All other cards uses
DetailsCarddirectly andisLoadingbecomes a page state in zustand store - Have a
...propsparameter in all concrete cards such asFileMessage... which are directed toDetailsCard
- All other cards uses
There was a problem hiding this comment.
I did also think these cards can be refactored when working on the code. But probably a different PR.
There was a problem hiding this comment.
Can we create a quick refactor PR to address this before merging? It shouldn’t take long — I’m happy to submit one and you review that. However, we shouldn't let the current isLoading solution make it into the main branch.
There was a problem hiding this comment.
okay this is reasonable, but you will have to merge into this PR
| import axios from "axios"; | ||
|
|
||
|
|
||
| /** | ||
| * Query the SQL server with the queryString. | ||
| * | ||
| * @param queryString | ||
| * @return | ||
| */ | ||
| const querySql = async <T>(queryString: string) => { | ||
| return axios.post<T>("/query/sql", {queryString}); | ||
| }; | ||
|
|
||
| export {querySql}; |
There was a problem hiding this comment.
The original idea of having separate sql.ts is: the developer of ingestion page don't need to touch any other code -- every change can be done in src/pages/IngestPage.
However, this idea doesn't work. Since you are already create an src/api folder, then let's do it properly:
- create a API module that one-to-one maps all restful endpoint to a typescript function.
- create a backend manager class on top of the thin api that includes all shared fetchers such as
fetchDatasetNames.
There was a problem hiding this comment.
for 1. i think the src/api is already the api module no?.
for 2. I dont think we need a class since there is no saved state. But i think we can move shared fetchers like fetchDatasetNames to src/api/ maybe in a datasets.ts file
There was a problem hiding this comment.
for 1. i think the src/api is already the api module no?
yes, but everything is mixed now. Can we create a module one-to-one maps each endpoint that fastify exposes?
for 2. I dont think we need a class since there is no saved state. But i think we can move shared fetchers like fetchDatasetNames to src/api/ maybe in a datasets.ts file
yes, it doesn't need to be a class. but we should move shared fetchers to src/api/
There was a problem hiding this comment.
for 1. do you mean like a new folder for each. I think that is okay
There was a problem hiding this comment.
okay I prefer one as lesser of two evils
If we go with that approach, then I feel it's best to:
-
We keep
src/api/search.ts,src/api/query.ts, andsrc/api/sql.tsas they are. -
We move the other fetchers into
src/fetchers.ts.
There was a problem hiding this comment.
what about src/api/fetchers.ts?
There was a problem hiding this comment.
I think the fetchers are making requests so okay to be in the api folder, and keep one file out of the top level code
There was a problem hiding this comment.
Yeah, sure. Let's put them in src/api/fetchers.ts.
Also, I’m thinking about adding a generic fetchSqlFirstRow function. A sketch
const fetchSqlFirstRow = async <T>(queryString: string): Promise<T> => {
const resp = await querySql<T[]>(queryString);
const [firstRow] = resp.data;
if ("undefined" === typeof firstRow) {
throw new Error(`SQL ${queryString} does not contain data.`);
}
return firstRow;
};
This way, we can keep all the build*Sql functions inside src/pages/IngestPage which aligns with Junhao’s original idea.
There was a problem hiding this comment.
i dont think this works since some like the dataset fetch multiple rows.
| export type {SpaceSavingsItem}; | ||
| export { | ||
| buildMultiDatasetSpaceSavingsSql, | ||
| executeSpaceSavingsQuery, | ||
| fetchClpSpaceSavings, | ||
| fetchClpsSpaceSavings, | ||
| getSpaceSavingsSql, | ||
| SPACE_SAVINGS_DEFAULT, |
There was a problem hiding this comment.
Take a look at sql.ts — for some features, it just builds a SQL query string, and for others, it actually executes the query. This kind of inconsistency makes the interface confusing.
One idea: move all of this logic into the "backend manager class" mentioned in another comment. That way, it’s all in one place with a clear and consistent responsibility.
Also, consider doing the same for the sql.ts file in IngestPage. Move its logic into the backend manager as well, and only expose fetch* functions instead of build*Sql ones.
There was a problem hiding this comment.
I think these are different from the dataset query since they are not shared. Junhao original idea for the widget is that they can send a self-contained SQL query, so in some sense it makes sense these fetches at here and only the query/sql is in api module.
However, i do think i exported too many functions. I think we only need to export fetchClpSpaceSavings and fetchClpsSpaceSavings, the others can be removed
There was a problem hiding this comment.
Ok, I think there are two layers of abstractions.
(1) submitExtractStreamJob, querySql ..., which only calls axios.get/axios.post
(2) fetchClpSpaceSavings, fetchClpsSpaceSavings, fetchDatasetNames build the query and returns the result as a promise.
We could either
-
move both functions in (1) (2) to
client/src/api, which obviously deviates from Junhao's original idea -
move functions in (1) to
client/src/apiand keep functions in (2) inclient/src/pages/*
However, we shouldn't put fetchDatasetNames in client/src/api while leave fetchClpSpaceSavings, fetchClpsSpaceSavings in client/src/pages/*.
If we choose method 2., then there will should be two fetchDatasetNames separate functions -- one in card, another one in search page
There was a problem hiding this comment.
okay I prefer one as lesser of two evils
There was a problem hiding this comment.
After looking at this further I think its weird to have the build* in the pages code, and the fetch* in the api folder. I actually think the current approach is not that bad, and importing the fetch datasets from the datasets folder is not great but is not that terrible.
| import axios from "axios"; | ||
|
|
||
|
|
||
| /** | ||
| * Query the SQL server with the queryString. | ||
| * | ||
| * @param queryString | ||
| * @return | ||
| */ | ||
| const querySql = async <T>(queryString: string) => { | ||
| return axios.post<T>("/query/sql", {queryString}); | ||
| }; | ||
|
|
||
| export {querySql}; |
There was a problem hiding this comment.
for 1. i think the src/api is already the api module no?.
for 2. I dont think we need a class since there is no saved state. But i think we can move shared fetchers like fetchDatasetNames to src/api/ maybe in a datasets.ts file
| export type {SpaceSavingsItem}; | ||
| export { | ||
| buildMultiDatasetSpaceSavingsSql, | ||
| executeSpaceSavingsQuery, | ||
| fetchClpSpaceSavings, | ||
| fetchClpsSpaceSavings, | ||
| getSpaceSavingsSql, | ||
| SPACE_SAVINGS_DEFAULT, |
There was a problem hiding this comment.
I think these are different from the dataset query since they are not shared. Junhao original idea for the widget is that they can send a self-contained SQL query, so in some sense it makes sense these fetches at here and only the query/sql is in api module.
However, i do think i exported too many functions. I think we only need to export fetchClpSpaceSavings and fetchClpsSpaceSavings, the others can be removed
| * @return | ||
| */ | ||
| const DetailsCard = ({title, stat}: DetailsCardProps) => { | ||
| const DetailsCard = ({title, stat, isLoading}: DetailsCardProps) => { |
There was a problem hiding this comment.
I did also think these cards can be refactored when working on the code. But probably a different PR.
|
@davemarco we need to resolve conflicts as #1050 is merged |
|
fyi @hoophalab - I merged main into this, and it looks like it still works |
| <div> | ||
| <TimeRange | ||
| beginDate={beginDate} | ||
| endDate={endDate}/> | ||
| beginDate={dayjs.utc(details.begin_timestamp)} | ||
| endDate={dayjs.utc(details.end_timestamp)} | ||
| isLoading={isPending}/> | ||
| </div> | ||
| ); |
There was a problem hiding this comment.
Quick question: I might be missing some context here. Why don’t we show Messages and Files for CLP-s? @davemarco
There was a problem hiding this comment.
The clp-s storage engine doesn't currently keep track of the number of log events and files in the archives.
|
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (3)
components/webui/client/src/pages/SearchPage/SearchControls/Dataset/sql.ts (3)
27-31: JSDoc still incomplete & function lacks basic error handling.
The earlier review already pointed this out; nothing has changed. Please finish the@returnsdescription and wrap the body in atry / catchso upstream callers can handle failures gracefully.
8-10: Enum naming still violates TypeScript conventions.
CLP_DATASETS_TABLE_COLUMN_NAMESremains in SCREAMING_SNAKE_CASE instead of PascalCase (ClpDatasetsTableColumnNames) as previously suggested.
22-24: Interface key remains unnecessarily indirect.
Using the enum value as a computed key hurts readability; a plainname: stringfield would suffice.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
components/webui/client/src/pages/IngestPage/Details/sql.ts(3 hunks)components/webui/client/src/pages/SearchPage/SearchControls/Dataset/sql.ts(2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
`**/*.{cpp,hpp,java,js,jsx,tpp,ts,tsx}`: - Prefer `false == ` rather than `!`.
**/*.{cpp,hpp,java,js,jsx,tpp,ts,tsx}: - Preferfalse == <expression>rather than!<expression>.
⚙️ Source: CodeRabbit Configuration File
List of files the instruction was applied to:
components/webui/client/src/pages/SearchPage/SearchControls/Dataset/sql.tscomponents/webui/client/src/pages/IngestPage/Details/sql.ts
🧠 Learnings (3)
📓 Common learnings
Learnt from: gibber9809
PR: y-scope/clp#504
File: components/core/src/clp_s/search/kql/CMakeLists.txt:29-29
Timestamp: 2024-10-22T15:36:04.655Z
Learning: When reviewing pull requests, focus on the changes within the PR and avoid commenting on issues outside the scope of the PR.
Learnt from: Bill-hbrhbr
PR: y-scope/clp#1036
File: components/clp-py-utils/clp_py_utils/clp_metadata_db_utils.py:204-211
Timestamp: 2025-07-03T12:58:18.407Z
Learning: In the CLP codebase, the validate_and_cache_dataset function in components/clp-py-utils/clp_py_utils/clp_metadata_db_utils.py uses in-place updates of the existing_datasets set parameter rather than returning a new set, as preferred by the development team.
Learnt from: davemarco
PR: y-scope/clp#892
File: components/log-viewer-webui/client/src/api/socket/MongoCollectionSocket.ts:27-31
Timestamp: 2025-05-09T19:15:26.180Z
Learning: For the MongoDB real-time updates implementation in components/log-viewer-webui/client/src/api/socket, a socket singleton pattern is used where a single shared socket connection is maintained rather than creating multiple connections. The socket lifecycle is managed centrally, with unsubscription handling in the useCursor React hook's cleanup function.
Learnt from: junhaoliao
PR: y-scope/clp#596
File: components/log-viewer-webui/client/src/api/query.js:16-23
Timestamp: 2024-11-21T15:51:33.203Z
Learning: In `components/log-viewer-webui/client/src/api/query.js`, the `ExtractJsonResp` type definition is accurate as-is and does not require modification. When suggesting changes to type definitions, ensure they align with the server-side definitions, referencing the source code if necessary.
components/webui/client/src/pages/SearchPage/SearchControls/Dataset/sql.ts (6)
Learnt from: junhaoliao
PR: y-scope/clp#596
File: components/log-viewer-webui/client/src/api/query.js:16-23
Timestamp: 2024-11-21T15:51:33.203Z
Learning: In `components/log-viewer-webui/client/src/api/query.js`, the `ExtractJsonResp` type definition is accurate as-is and does not require modification. When suggesting changes to type definitions, ensure they align with the server-side definitions, referencing the source code if necessary.
Learnt from: Bill-hbrhbr
PR: y-scope/clp#1036
File: components/clp-py-utils/clp_py_utils/clp_metadata_db_utils.py:204-211
Timestamp: 2025-07-03T12:58:18.407Z
Learning: In the CLP codebase, the validate_and_cache_dataset function in components/clp-py-utils/clp_py_utils/clp_metadata_db_utils.py uses in-place updates of the existing_datasets set parameter rather than returning a new set, as preferred by the development team.
Learnt from: junhaoliao
PR: y-scope/clp#988
File: components/log-viewer-webui/client/src/components/QueryBox/InputWithCaseSensitive/CaseSenstiveToggle.tsx:34-37
Timestamp: 2025-06-09T17:48:56.024Z
Learning: In the y-scope/clp project, prefer `false == <expression>` rather than `!<expression>` for boolean expressions in TypeScript/JavaScript files, as specified in the coding guidelines.
Learnt from: jackluo923
PR: y-scope/clp#1054
File: components/core/tools/scripts/lib_install/musllinux_1_2/install-packages-from-source.sh:6-8
Timestamp: 2025-07-01T14:51:19.172Z
Learning: In CLP installation scripts within `components/core/tools/scripts/lib_install/`, maintain consistency with existing variable declaration patterns across platforms rather than adding individual improvements like `readonly` declarations.
Learnt from: jackluo923
PR: y-scope/clp#1054
File: components/core/tools/scripts/lib_install/musllinux_1_2/install-packages-from-source.sh:6-8
Timestamp: 2025-07-01T14:51:19.172Z
Learning: In CLP installation scripts within `components/core/tools/scripts/lib_install/`, maintain consistency with existing variable declaration patterns across platforms rather than adding individual improvements like `readonly` declarations.
Learnt from: LinZhihao-723
PR: y-scope/clp#873
File: components/core/src/clp/ffi/ir_stream/search/QueryHandlerImpl.hpp:320-324
Timestamp: 2025-05-05T01:12:18.561Z
Learning: In the CLP codebase, the `m_case_sensitive_search` flag is used only for actual string value comparisons during query evaluation, not for schema key name matching. Schema keys are always compared case-sensitively regardless of this flag's setting.
components/webui/client/src/pages/IngestPage/Details/sql.ts (1)
Learnt from: junhaoliao
PR: y-scope/clp#596
File: components/log-viewer-webui/client/src/api/query.js:16-23
Timestamp: 2024-11-21T15:51:33.203Z
Learning: In `components/log-viewer-webui/client/src/api/query.js`, the `ExtractJsonResp` type definition is accurate as-is and does not require modification. When suggesting changes to type definitions, ensure they align with the server-side definitions, referencing the source code if necessary.
🧬 Code Graph Analysis (1)
components/webui/client/src/pages/IngestPage/Details/sql.ts (2)
components/webui/client/src/pages/IngestPage/sqlConfig.ts (2)
CLP_ARCHIVES_TABLE_COLUMN_NAMES(40-40)CLP_FILES_TABLE_COLUMN_NAMES(41-41)components/webui/client/src/api/sql/index.ts (1)
querySql(14-14)
⏰ 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: lint-check (ubuntu-latest)
- GitHub Check: lint-check (macos-latest)
🔇 Additional comments (4)
components/webui/client/src/pages/SearchPage/SearchControls/Dataset/sql.ts (1)
13-15: Comment wording fix looks good.The update to “dataset names” clarifies the comment—no further action required.
components/webui/client/src/pages/IngestPage/Details/sql.ts (3)
14-19: Interface definition looks solid.The
DetailsIteminterface is well-structured with appropriate nullable types for timestamps and numeric fields.
24-29: Default values are appropriate.The
DETAILS_DEFAULTconstant provides sensible fallback values, withnullfor timestamps (indicating no data) and0for counts.
148-150: Good defensive programming for empty datasets.The early return with default values when no datasets are provided prevents unnecessary SQL query execution and potential errors.
| if ("undefined" === typeof detailsResult) { | ||
| throw new Error("Details result does not contain data."); | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Error message could be more descriptive.
The generic error message doesn't provide context about which query failed or why, making debugging difficult.
Consider making the error message more specific:
- if ("undefined" === typeof detailsResult) {
- throw new Error("Details result does not contain data.");
- }
+ if ("undefined" === typeof detailsResult) {
+ throw new Error("Details query returned no results. The query may have failed or the database may be empty.");
+ }📝 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.
| if ("undefined" === typeof detailsResult) { | |
| throw new Error("Details result does not contain data."); | |
| } | |
| if ("undefined" === typeof detailsResult) { | |
| throw new Error("Details query returned no results. The query may have failed or the database may be empty."); | |
| } |
🤖 Prompt for AI Agents
In components/webui/client/src/pages/IngestPage/Details/sql.ts around lines 122
to 124, the error message thrown when detailsResult is undefined is too generic.
Update the error message to include specific context such as the name of the
query or operation that failed and possibly why it might have failed, to make
debugging easier and more informative.
| FROM clp_${name}_archives | ||
| `); | ||
|
|
||
| type DetailsResp = DetailsItem[]; | ||
| const fileQueries = datasetNames.map((name) => ` | ||
| SELECT | ||
| COUNT(DISTINCT ${CLP_FILES_TABLE_COLUMN_NAMES.ORIG_FILE_ID}) AS num_files, | ||
| CAST( | ||
| COALESCE(SUM(${CLP_FILES_TABLE_COLUMN_NAMES.NUM_MESSAGES}), 0) AS INTEGER | ||
| ) AS num_messages | ||
| FROM clp_${name}_files | ||
| `); |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Hard-coded table naming convention may be brittle.
The table names are constructed using string interpolation with a fixed pattern (clp_${name}_archives, clp_${name}_files). This could break if the naming convention changes.
Consider extracting the table name construction into a utility function or configuration:
+const getArchiveTableName = (datasetName: string) => `clp_${datasetName}_archives`;
+const getFilesTableName = (datasetName: string) => `clp_${datasetName}_files`;
const buildMultiDatasetDetailsSql = (datasetNames: string[]): string => {
const archiveQueries = datasetNames.map((name) => `
SELECT
MIN(${CLP_ARCHIVES_TABLE_COLUMN_NAMES.BEGIN_TIMESTAMP}) AS begin_timestamp,
MAX(${CLP_ARCHIVES_TABLE_COLUMN_NAMES.END_TIMESTAMP}) AS end_timestamp
- FROM clp_${name}_archives
+ FROM ${getArchiveTableName(name)}
`);This would make the naming convention more maintainable and centralized.
📝 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.
| FROM clp_${name}_archives | |
| `); | |
| type DetailsResp = DetailsItem[]; | |
| const fileQueries = datasetNames.map((name) => ` | |
| SELECT | |
| COUNT(DISTINCT ${CLP_FILES_TABLE_COLUMN_NAMES.ORIG_FILE_ID}) AS num_files, | |
| CAST( | |
| COALESCE(SUM(${CLP_FILES_TABLE_COLUMN_NAMES.NUM_MESSAGES}), 0) AS INTEGER | |
| ) AS num_messages | |
| FROM clp_${name}_files | |
| `); | |
| // centralize table‐name construction for maintainability | |
| const getArchiveTableName = (datasetName: string) => `clp_${datasetName}_archives`; | |
| const getFilesTableName = (datasetName: string) => `clp_${datasetName}_files`; | |
| const buildMultiDatasetDetailsSql = (datasetNames: string[]): string => { | |
| const archiveQueries = datasetNames.map((name) => ` | |
| SELECT | |
| MIN(${CLP_ARCHIVES_TABLE_COLUMN_NAMES.BEGIN_TIMESTAMP}) AS begin_timestamp, | |
| MAX(${CLP_ARCHIVES_TABLE_COLUMN_NAMES.END_TIMESTAMP}) AS end_timestamp | |
| FROM ${getArchiveTableName(name)} | |
| `); | |
| const fileQueries = datasetNames.map((name) => ` | |
| SELECT | |
| COUNT(DISTINCT ${CLP_FILES_TABLE_COLUMN_NAMES.ORIG_FILE_ID}) AS num_files, | |
| CAST( | |
| COALESCE(SUM(${CLP_FILES_TABLE_COLUMN_NAMES.NUM_MESSAGES}), 0) AS INTEGER | |
| ) AS num_messages | |
| FROM clp_${name}_files | |
| `); | |
| // ...rest of function... | |
| `; |
🤖 Prompt for AI Agents
In components/webui/client/src/pages/IngestPage/Details/sql.ts around lines 74
to 84, the table names are hard-coded using string interpolation with a fixed
pattern, which is brittle. Refactor by extracting the table name construction
logic into a dedicated utility function or configuration that takes the dataset
name and returns the correctly formatted table name. Replace all direct string
interpolations with calls to this utility to centralize and simplify future
updates to the naming convention.
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
components/webui/client/src/config/queryClient.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
`**/*.{cpp,hpp,java,js,jsx,tpp,ts,tsx}`: - Prefer `false == ` rather than `!`.
**/*.{cpp,hpp,java,js,jsx,tpp,ts,tsx}: - Preferfalse == <expression>rather than!<expression>.
⚙️ Source: CodeRabbit Configuration File
List of files the instruction was applied to:
components/webui/client/src/config/queryClient.ts
🧠 Learnings (2)
📓 Common learnings
Learnt from: gibber9809
PR: y-scope/clp#504
File: components/core/src/clp_s/search/kql/CMakeLists.txt:29-29
Timestamp: 2024-10-22T15:36:04.655Z
Learning: When reviewing pull requests, focus on the changes within the PR and avoid commenting on issues outside the scope of the PR.
Learnt from: Bill-hbrhbr
PR: y-scope/clp#1036
File: components/clp-py-utils/clp_py_utils/clp_metadata_db_utils.py:204-211
Timestamp: 2025-07-03T12:58:18.407Z
Learning: In the CLP codebase, the validate_and_cache_dataset function in components/clp-py-utils/clp_py_utils/clp_metadata_db_utils.py uses in-place updates of the existing_datasets set parameter rather than returning a new set, as preferred by the development team.
Learnt from: davemarco
PR: y-scope/clp#892
File: components/log-viewer-webui/client/src/api/socket/MongoCollectionSocket.ts:27-31
Timestamp: 2025-05-09T19:15:26.180Z
Learning: For the MongoDB real-time updates implementation in components/log-viewer-webui/client/src/api/socket, a socket singleton pattern is used where a single shared socket connection is maintained rather than creating multiple connections. The socket lifecycle is managed centrally, with unsubscription handling in the useCursor React hook's cleanup function.
Learnt from: junhaoliao
PR: y-scope/clp#596
File: components/log-viewer-webui/client/src/api/query.js:16-23
Timestamp: 2024-11-21T15:51:33.203Z
Learning: In `components/log-viewer-webui/client/src/api/query.js`, the `ExtractJsonResp` type definition is accurate as-is and does not require modification. When suggesting changes to type definitions, ensure they align with the server-side definitions, referencing the source code if necessary.
components/webui/client/src/config/queryClient.ts (1)
Learnt from: junhaoliao
PR: y-scope/clp#596
File: components/log-viewer-webui/client/src/api/query.js:16-23
Timestamp: 2024-11-21T15:51:33.203Z
Learning: In `components/log-viewer-webui/client/src/api/query.js`, the `ExtractJsonResp` type definition is accurate as-is and does not require modification. When suggesting changes to type definitions, ensure they align with the server-side definitions, referencing the source code if necessary.
⏰ 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: lint-check (macos-latest)
- GitHub Check: lint-check (ubuntu-latest)
🔇 Additional comments (2)
components/webui/client/src/config/queryClient.ts (2)
1-1: LGTM - Clean import statementThe import follows standard React Query practices and is correctly structured.
14-14: LGTM - Proper default exportThe default export is appropriate for this configuration module and follows standard practices.
| import {QueryClient} from "@tanstack/react-query"; | ||
|
|
||
|
|
||
| const DEFAULT_REFETCH_INTERVAL_MILLIS = 10_000; |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Consider if 10-second refetch interval is appropriate for all queries
The constant is well-named and formatted correctly. However, a 10-second global refetch interval might be aggressive for all query types. Consider if different query types might benefit from different intervals.
🤖 Prompt for AI Agents
In components/webui/client/src/config/queryClient.ts at line 4, the constant
DEFAULT_REFETCH_INTERVAL_MILLIS is set to 10,000 milliseconds (10 seconds) for
all queries, which may be too frequent for some query types. Refactor the code
to allow different refetch intervals based on query type or context by defining
multiple interval constants or configuring intervals per query, and update the
query client setup to use these differentiated intervals instead of a single
global value.
| const queryClient = new QueryClient({ | ||
| defaultOptions: { | ||
| queries: { | ||
| refetchInterval: DEFAULT_REFETCH_INTERVAL_MILLIS, | ||
| }, | ||
| }, | ||
| }); |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Well-structured QueryClient configuration
The centralized configuration approach is excellent for maintaining consistency across the application. The current minimal configuration focusing on refetch interval is appropriate for the current refactoring.
Future enhancements could include additional default options such as retry logic, cache time, or stale time if needed:
const queryClient = new QueryClient({
defaultOptions: {
queries: {
refetchInterval: DEFAULT_REFETCH_INTERVAL_MILLIS,
+ retry: 3,
+ staleTime: 5_000,
},
},
});📝 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.
| const queryClient = new QueryClient({ | |
| defaultOptions: { | |
| queries: { | |
| refetchInterval: DEFAULT_REFETCH_INTERVAL_MILLIS, | |
| }, | |
| }, | |
| }); | |
| const queryClient = new QueryClient({ | |
| defaultOptions: { | |
| queries: { | |
| refetchInterval: DEFAULT_REFETCH_INTERVAL_MILLIS, | |
| retry: 3, | |
| staleTime: 5_000, | |
| }, | |
| }, | |
| }); |
🤖 Prompt for AI Agents
In components/webui/client/src/config/queryClient.ts around lines 6 to 12, the
QueryClient is correctly configured with a centralized refetchInterval. No
changes are needed now, but consider adding future enhancements like retry
logic, cache time, or stale time as additional defaultOptions to improve query
behavior consistency.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
components/webui/client/src/pages/IngestPage/SpaceSavings/sql.ts (1)
53-79: Potential SQL injection vulnerability and performance concerns.The dataset names are directly interpolated into SQL strings without validation, and the UNION ALL approach may have performance implications with many datasets.
This issue was previously identified in past review comments regarding SQL injection vulnerability with dataset name interpolation and performance implications of large UNION ALL queries.
components/webui/client/src/pages/IngestPage/Details/sql.ts (2)
70-111: Critical security vulnerability: SQL injection risk in dataset name interpolation.The function directly interpolates dataset names into SQL queries without validation or sanitization, creating a potential SQL injection vulnerability if dataset names come from untrusted sources.
Based on the retrieved learnings, the team prefers parameterized queries and input validation. Consider adding dataset name validation:
const buildMultiDatasetDetailsSql = (datasetNames: string[]): string => { + // Validate dataset names to prevent SQL injection + const validNamePattern = /^[a-zA-Z0-9_-]+$/; + const invalidNames = datasetNames.filter(name => !validNamePattern.test(name)); + if (invalidNames.length > 0) { + throw new Error(`Invalid dataset names: ${invalidNames.join(', ')}`); + } + const archiveQueries = datasetNames.map((name) => `Additionally, the hard-coded table naming convention should be extracted into utility functions for better maintainability:
+const getArchiveTableName = (datasetName: string) => + `${settings.SqlDbClpTablePrefix}${datasetName}_${SqlTableSuffix.ARCHIVES}`; +const getFilesTableName = (datasetName: string) => + `${settings.SqlDbClpTablePrefix}${datasetName}_${SqlTableSuffix.FILES}`; const buildMultiDatasetDetailsSql = (datasetNames: string[]): string => { const archiveQueries = datasetNames.map((name) => ` SELECT MIN(${CLP_ARCHIVES_TABLE_COLUMN_NAMES.BEGIN_TIMESTAMP}) AS begin_timestamp, MAX(${CLP_ARCHIVES_TABLE_COLUMN_NAMES.END_TIMESTAMP}) AS end_timestamp - FROM ${settings.SqlDbClpTablePrefix}${name}_${SqlTableSuffix.ARCHIVES} + FROM ${getArchiveTableName(name)} `);
123-125: Improve error message specificity for better debugging.The generic error message doesn't provide context about which operation failed, making debugging difficult.
Consider making the error message more descriptive:
- if ("undefined" === typeof detailsResult) { - throw new Error("Details result does not contain data."); - } + if ("undefined" === typeof detailsResult) { + throw new Error("Details query returned no results. The query may have failed or the database may be empty."); + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (6)
components/clp-package-utils/clp_package_utils/scripts/start_clp.py(1 hunks)components/webui/client/public/settings.json(1 hunks)components/webui/client/src/config/sql-table-suffix.ts(1 hunks)components/webui/client/src/pages/IngestPage/Details/sql.ts(3 hunks)components/webui/client/src/pages/IngestPage/SpaceSavings/sql.ts(2 hunks)components/webui/client/src/settings.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
`**/*.{cpp,hpp,java,js,jsx,tpp,ts,tsx}`: - Prefer `false == ` rather than `!`.
**/*.{cpp,hpp,java,js,jsx,tpp,ts,tsx}: - Preferfalse == <expression>rather than!<expression>.
⚙️ Source: CodeRabbit Configuration File
List of files the instruction was applied to:
components/webui/client/src/config/sql-table-suffix.tscomponents/webui/client/src/settings.tscomponents/webui/client/src/pages/IngestPage/Details/sql.tscomponents/webui/client/src/pages/IngestPage/SpaceSavings/sql.ts
🧠 Learnings (7)
📓 Common learnings
Learnt from: gibber9809
PR: y-scope/clp#504
File: components/core/src/clp_s/search/kql/CMakeLists.txt:29-29
Timestamp: 2024-10-22T15:36:04.655Z
Learning: When reviewing pull requests, focus on the changes within the PR and avoid commenting on issues outside the scope of the PR.
Learnt from: Bill-hbrhbr
PR: y-scope/clp#1036
File: components/clp-py-utils/clp_py_utils/clp_metadata_db_utils.py:204-211
Timestamp: 2025-07-03T12:58:18.407Z
Learning: In the CLP codebase, the validate_and_cache_dataset function in components/clp-py-utils/clp_py_utils/clp_metadata_db_utils.py uses in-place updates of the existing_datasets set parameter rather than returning a new set, as preferred by the development team.
Learnt from: davemarco
PR: y-scope/clp#892
File: components/log-viewer-webui/client/src/api/socket/MongoCollectionSocket.ts:27-31
Timestamp: 2025-05-09T19:15:26.180Z
Learning: For the MongoDB real-time updates implementation in components/log-viewer-webui/client/src/api/socket, a socket singleton pattern is used where a single shared socket connection is maintained rather than creating multiple connections. The socket lifecycle is managed centrally, with unsubscription handling in the useCursor React hook's cleanup function.
Learnt from: junhaoliao
PR: y-scope/clp#596
File: components/log-viewer-webui/client/src/api/query.js:16-23
Timestamp: 2024-11-21T15:51:33.203Z
Learning: In `components/log-viewer-webui/client/src/api/query.js`, the `ExtractJsonResp` type definition is accurate as-is and does not require modification. When suggesting changes to type definitions, ensure they align with the server-side definitions, referencing the source code if necessary.
components/webui/client/public/settings.json (1)
Learnt from: haiqi96
PR: y-scope/clp#594
File: components/clp-package-utils/clp_package_utils/scripts/native/del_archives.py:90-96
Timestamp: 2024-11-15T16:22:33.635Z
Learning: In the CLP codebase, `table_prefix` is a trusted configuration value and can be safely embedded into SQL queries.
components/webui/client/src/config/sql-table-suffix.ts (1)
Learnt from: haiqi96
PR: y-scope/clp#594
File: components/clp-package-utils/clp_package_utils/scripts/native/del_archives.py:90-96
Timestamp: 2024-11-15T16:22:33.635Z
Learning: In the CLP codebase, `table_prefix` is a trusted configuration value and can be safely embedded into SQL queries.
components/webui/client/src/settings.ts (1)
Learnt from: haiqi96
PR: y-scope/clp#594
File: components/clp-package-utils/clp_package_utils/scripts/native/del_archives.py:90-96
Timestamp: 2024-11-15T16:22:33.635Z
Learning: In the CLP codebase, `table_prefix` is a trusted configuration value and can be safely embedded into SQL queries.
components/clp-package-utils/clp_package_utils/scripts/start_clp.py (10)
Learnt from: Bill-hbrhbr
PR: y-scope/clp#1036
File: components/clp-py-utils/clp_py_utils/clp_metadata_db_utils.py:204-211
Timestamp: 2025-07-03T12:58:18.407Z
Learning: In the CLP codebase, the validate_and_cache_dataset function in components/clp-py-utils/clp_py_utils/clp_metadata_db_utils.py uses in-place updates of the existing_datasets set parameter rather than returning a new set, as preferred by the development team.
Learnt from: Bill-hbrhbr
PR: y-scope/clp#1004
File: components/clp-package-utils/clp_package_utils/scripts/native/decompress.py:139-144
Timestamp: 2025-06-24T08:54:14.438Z
Learning: In the CLP codebase, the get_orig_file_id function signature was changed after a recent merge to no longer accept a dataset parameter, making previous suggestions that reference this parameter invalid.
Learnt from: Bill-hbrhbr
PR: y-scope/clp#868
File: components/clp-py-utils/clp_py_utils/clp_metadata_db_utils.py:141-144
Timestamp: 2025-05-05T16:32:55.163Z
Learning: The column metadata table (created by `_create_column_metadata_table`) is only needed for dataset-specific workflows in `CLP_S` and is obsolete for non-dataset workflows.
Learnt from: haiqi96
PR: y-scope/clp#594
File: components/clp-package-utils/clp_package_utils/scripts/native/del_archives.py:104-110
Timestamp: 2024-11-15T16:21:52.122Z
Learning: In `clp_package_utils/scripts/native/del_archives.py`, when deleting archives, the `archive` variable retrieved from the database is controlled and is always a single string without path components. Therefore, it's acceptable to skip additional validation checks for directory traversal in this context.
Learnt from: haiqi96
PR: y-scope/clp#594
File: components/clp-package-utils/clp_package_utils/scripts/del_archives.py:56-65
Timestamp: 2024-11-18T16:49:20.248Z
Learning: When reviewing wrapper scripts in `components/clp-package-utils/clp_package_utils/scripts/`, note that it's preferred to keep error handling simple without adding extra complexity.
Learnt from: jackluo923
PR: y-scope/clp#1054
File: components/core/tools/scripts/lib_install/musllinux_1_2/install-packages-from-source.sh:6-8
Timestamp: 2025-07-01T14:51:19.172Z
Learning: In CLP installation scripts within `components/core/tools/scripts/lib_install/`, maintain consistency with existing variable declaration patterns across platforms rather than adding individual improvements like `readonly` declarations.
Learnt from: jackluo923
PR: y-scope/clp#1054
File: components/core/tools/scripts/lib_install/musllinux_1_2/install-packages-from-source.sh:6-8
Timestamp: 2025-07-01T14:51:19.172Z
Learning: In CLP installation scripts within `components/core/tools/scripts/lib_install/`, maintain consistency with existing variable declaration patterns across platforms rather than adding individual improvements like `readonly` declarations.
Learnt from: haiqi96
PR: y-scope/clp#594
File: components/clp-package-utils/clp_package_utils/scripts/native/del_archives.py:90-96
Timestamp: 2024-11-15T16:22:33.635Z
Learning: In the CLP codebase, `table_prefix` is a trusted configuration value and can be safely embedded into SQL queries.
Learnt from: Bill-hbrhbr
PR: y-scope/clp#1050
File: components/clp-package-utils/clp_package_utils/scripts/search.py:100-106
Timestamp: 2025-07-03T20:10:43.789Z
Learning: In the current CLP codebase implementation, dataset validation using validate_dataset() is performed within the native scripts (like clp_package_utils/scripts/native/search.py) rather than at the wrapper script level, where the native scripts handle their own parameter validation.
Learnt from: Bill-hbrhbr
PR: y-scope/clp#1050
File: components/clp-package-utils/clp_package_utils/scripts/search.py:100-106
Timestamp: 2025-07-03T20:10:43.789Z
Learning: In the current CLP codebase implementation, dataset validation using validate_dataset() is performed within the native scripts (like clp_package_utils/scripts/native/search.py) rather than at the wrapper script level, where the native scripts handle their own parameter validation.
components/webui/client/src/pages/IngestPage/Details/sql.ts (13)
Learnt from: junhaoliao
PR: y-scope/clp#596
File: components/log-viewer-webui/client/src/api/query.js:16-23
Timestamp: 2024-11-21T15:51:33.203Z
Learning: In `components/log-viewer-webui/client/src/api/query.js`, the `ExtractJsonResp` type definition is accurate as-is and does not require modification. When suggesting changes to type definitions, ensure they align with the server-side definitions, referencing the source code if necessary.
Learnt from: gibber9809
PR: y-scope/clp#672
File: components/core/src/clp_s/indexer/MySQLIndexStorage.cpp:30-38
Timestamp: 2025-01-23T17:08:55.566Z
Learning: SQL identifiers (table names, column names) in the codebase are currently from trusted sources and directly interpolated into queries. However, the team prefers to implement sanitization for robustness, even for trusted inputs.
Learnt from: Bill-hbrhbr
PR: y-scope/clp#831
File: components/job-orchestration/job_orchestration/scheduler/compress/compression_scheduler.py:0-0
Timestamp: 2025-04-17T16:55:23.796Z
Learning: In the CLP project, SQL queries should use parameterized queries with placeholders (%s) and pass values as a tuple to `db_cursor.execute()` to prevent SQL injection, rather than directly interpolating values into the query string.
Learnt from: Bill-hbrhbr
PR: y-scope/clp#1036
File: components/clp-py-utils/clp_py_utils/clp_metadata_db_utils.py:204-211
Timestamp: 2025-07-03T12:58:18.407Z
Learning: In the CLP codebase, the validate_and_cache_dataset function in components/clp-py-utils/clp_py_utils/clp_metadata_db_utils.py uses in-place updates of the existing_datasets set parameter rather than returning a new set, as preferred by the development team.
Learnt from: Bill-hbrhbr
PR: y-scope/clp#1036
File: components/clp-py-utils/clp_py_utils/clp_metadata_db_utils.py:188-212
Timestamp: 2025-07-03T12:59:46.638Z
Learning: In CLP schedulers (compression and query), runtime dataset validation is required; the helper validate_and_cache_dataset keeps a local cache and only queries the DB on cache misses because dataset additions are rare.
Learnt from: Bill-hbrhbr
PR: y-scope/clp#868
File: components/clp-py-utils/clp_py_utils/clp_metadata_db_utils.py:141-144
Timestamp: 2025-05-05T16:32:55.163Z
Learning: The column metadata table (created by `_create_column_metadata_table`) is only needed for dataset-specific workflows in `CLP_S` and is obsolete for non-dataset workflows.
Learnt from: haiqi96
PR: y-scope/clp#594
File: components/clp-package-utils/clp_package_utils/scripts/native/del_archives.py:90-96
Timestamp: 2024-11-15T16:22:33.635Z
Learning: In the CLP codebase, `table_prefix` is a trusted configuration value and can be safely embedded into SQL queries.
Learnt from: Bill-hbrhbr
PR: y-scope/clp#1004
File: components/clp-package-utils/clp_package_utils/scripts/native/decompress.py:139-144
Timestamp: 2025-06-24T08:54:14.438Z
Learning: In the CLP codebase, the get_orig_file_id function signature was changed after a recent merge to no longer accept a dataset parameter, making previous suggestions that reference this parameter invalid.
Learnt from: jackluo923
PR: y-scope/clp#1054
File: components/core/tools/scripts/lib_install/musllinux_1_2/install-packages-from-source.sh:6-8
Timestamp: 2025-07-01T14:51:19.172Z
Learning: In CLP installation scripts within `components/core/tools/scripts/lib_install/`, maintain consistency with existing variable declaration patterns across platforms rather than adding individual improvements like `readonly` declarations.
Learnt from: jackluo923
PR: y-scope/clp#1054
File: components/core/tools/scripts/lib_install/musllinux_1_2/install-packages-from-source.sh:6-8
Timestamp: 2025-07-01T14:51:19.172Z
Learning: In CLP installation scripts within `components/core/tools/scripts/lib_install/`, maintain consistency with existing variable declaration patterns across platforms rather than adding individual improvements like `readonly` declarations.
Learnt from: Bill-hbrhbr
PR: y-scope/clp#868
File: components/clp-package-utils/clp_package_utils/scripts/start_clp.py:869-870
Timestamp: 2025-06-10T05:56:56.572Z
Learning: The team prefers keeping simple conditional logic inline rather than extracting it into helper functions, especially for straightforward operations like string concatenation based on storage engine type.
Learnt from: jackluo923
PR: y-scope/clp#1054
File: components/core/tools/scripts/lib_install/musllinux_1_2/install-prebuilt-packages.sh:27-32
Timestamp: 2025-07-07T17:41:15.655Z
Learning: In CLP installation scripts, consistency across platform scripts is prioritized over defensive programming improvements. For example, when extracting Task binaries with tar in `install-prebuilt-packages.sh`, the extraction pattern should remain consistent with other platform scripts rather than adding defensive flags like `--strip-components=1` to handle potential tarball layout changes.
Learnt from: haiqi96
PR: y-scope/clp#620
File: components/core/src/clp/clo/clo.cpp:198-198
Timestamp: 2024-12-02T22:36:17.847Z
Learning: The existing references to `file_split_id` in the codebase are acceptable and do not need to be changed.
components/webui/client/src/pages/IngestPage/SpaceSavings/sql.ts (8)
Learnt from: junhaoliao
PR: y-scope/clp#596
File: components/log-viewer-webui/client/src/api/query.js:35-41
Timestamp: 2024-11-19T19:52:43.429Z
Learning: For internal APIs in `components/log-viewer-webui/client/src/api/query.js`, runtime validation of parameters may not be necessary since the APIs are not exposed to end users, and JsDoc type annotations may be sufficient.
Learnt from: Bill-hbrhbr
PR: y-scope/clp#1036
File: components/clp-py-utils/clp_py_utils/clp_metadata_db_utils.py:188-212
Timestamp: 2025-07-03T12:59:46.638Z
Learning: In CLP schedulers (compression and query), runtime dataset validation is required; the helper validate_and_cache_dataset keeps a local cache and only queries the DB on cache misses because dataset additions are rare.
Learnt from: gibber9809
PR: y-scope/clp#672
File: components/core/src/clp_s/indexer/MySQLIndexStorage.cpp:30-38
Timestamp: 2025-01-23T17:08:55.566Z
Learning: SQL identifiers (table names, column names) in the codebase are currently from trusted sources and directly interpolated into queries. However, the team prefers to implement sanitization for robustness, even for trusted inputs.
Learnt from: Bill-hbrhbr
PR: y-scope/clp#831
File: components/job-orchestration/job_orchestration/scheduler/compress/compression_scheduler.py:0-0
Timestamp: 2025-04-17T16:55:23.796Z
Learning: In the CLP project, SQL queries should use parameterized queries with placeholders (%s) and pass values as a tuple to `db_cursor.execute()` to prevent SQL injection, rather than directly interpolating values into the query string.
Learnt from: Bill-hbrhbr
PR: y-scope/clp#1036
File: components/clp-py-utils/clp_py_utils/clp_metadata_db_utils.py:204-211
Timestamp: 2025-07-03T12:58:18.407Z
Learning: In the CLP codebase, the validate_and_cache_dataset function in components/clp-py-utils/clp_py_utils/clp_metadata_db_utils.py uses in-place updates of the existing_datasets set parameter rather than returning a new set, as preferred by the development team.
Learnt from: Bill-hbrhbr
PR: y-scope/clp#868
File: components/clp-package-utils/clp_package_utils/scripts/start_clp.py:869-870
Timestamp: 2025-06-10T05:56:56.572Z
Learning: The team prefers keeping simple conditional logic inline rather than extracting it into helper functions, especially for straightforward operations like string concatenation based on storage engine type.
Learnt from: Bill-hbrhbr
PR: y-scope/clp#1036
File: components/clp-package-utils/clp_package_utils/general.py:564-579
Timestamp: 2025-06-28T07:10:47.295Z
Learning: The validate_dataset function in components/clp-package-utils/clp_package_utils/general.py is designed to be called once upon function startup for dataset validation, not repeatedly during execution, making caching optimizations unnecessary.
Learnt from: junhaoliao
PR: y-scope/clp#596
File: components/log-viewer-webui/client/src/api/query.js:16-23
Timestamp: 2024-11-21T15:51:33.203Z
Learning: In `components/log-viewer-webui/client/src/api/query.js`, the `ExtractJsonResp` type definition is accurate as-is and does not require modification. When suggesting changes to type definitions, ensure they align with the server-side definitions, referencing the source code if necessary.
🧬 Code Graph Analysis (1)
components/clp-package-utils/clp_package_utils/scripts/start_clp.py (1)
components/clp-py-utils/clp_py_utils/clp_metadata_db_utils.py (3)
get_archives_table_name(226-227)get_files_table_name(238-239)get_datasets_table_name(234-235)
⏰ 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: lint-check (ubuntu-latest)
- GitHub Check: lint-check (macos-latest)
🔇 Additional comments (14)
components/webui/client/src/settings.ts (1)
10-10: LGTM - Clean type definition addition.The new
SqlDbClpTablePrefixproperty is well-named and consistent with other SQL-related properties in the Settings type.components/webui/client/public/settings.json (1)
7-7: LGTM - Appropriate default configuration value.The default table prefix "clp_" is reasonable and consistent with CLP naming conventions.
components/webui/client/src/config/sql-table-suffix.ts (1)
1-7: LGTM - Well-structured enum for table suffix standardization.The
SqlTableSuffixenum provides a clean way to standardize table suffix references across the application, improving maintainability and consistency.components/clp-package-utils/clp_package_utils/scripts/start_clp.py (1)
887-887: LGTM - Proper addition of table prefix to client settings.Adding the table prefix to the client settings enables frontend components to construct table names dynamically.
components/webui/client/src/pages/IngestPage/SpaceSavings/sql.ts (4)
10-21: LGTM - Well-structured type definitions and defaults.The
SpaceSavingsIteminterface andSPACE_SAVINGS_DEFAULTconstant provide clear contracts and sensible fallback values.
29-44: LGTM - Clean SQL query construction for single dataset.The single dataset query builder is well-structured with proper NULL handling using
COALESCE.
88-96: LGTM - Robust query execution with proper error handling.The
executeSpaceSavingsQueryfunction provides good error handling and type safety for query results.
103-122: LGTM - Clean API design with appropriate fallback handling.Both fetch functions provide clear abstractions with proper handling of edge cases (empty dataset arrays return defaults).
components/webui/client/src/pages/IngestPage/Details/sql.ts (6)
3-4: LGTM: Clean imports for new functionality.The imports are appropriately scoped and support the new SQL query building and execution features.
15-30: LGTM: Well-structured interface and default values.The
DetailsIteminterface correctly defines nullable types for timestamps and counts, and theDETAILS_DEFAULTconstant provides sensible fallback values.
120-128: LGTM: Well-structured query execution with proper error handling.The function correctly uses the
querySqlutility and provides appropriate error handling for empty results.
135-138: LGTM: Clean single-dataset fetch implementation.The function properly delegates to the query builder and execution functions.
146-154: LGTM: Proper handling of empty dataset scenario.The function correctly returns default values when no datasets are provided, following the coding guideline by using
0 === datasetNames.lengthinstead of!datasetNames.length.
156-161: LGTM: Clean export structure.The exports are well-organized and provide the necessary public API for the module.
components/clp-package-utils/clp_package_utils/scripts/start_clp.py
Outdated
Show resolved
Hide resolved
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/config/sql-table-suffix.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
`**/*.{cpp,hpp,java,js,jsx,tpp,ts,tsx}`: - Prefer `false == ` rather than `!`.
**/*.{cpp,hpp,java,js,jsx,tpp,ts,tsx}: - Preferfalse == <expression>rather than!<expression>.
⚙️ Source: CodeRabbit Configuration File
List of files the instruction was applied to:
components/webui/client/src/config/sql-table-suffix.ts
🧠 Learnings (2)
📓 Common learnings
Learnt from: gibber9809
PR: y-scope/clp#504
File: components/core/src/clp_s/search/kql/CMakeLists.txt:29-29
Timestamp: 2024-10-22T15:36:04.655Z
Learning: When reviewing pull requests, focus on the changes within the PR and avoid commenting on issues outside the scope of the PR.
Learnt from: Bill-hbrhbr
PR: y-scope/clp#1036
File: components/clp-py-utils/clp_py_utils/clp_metadata_db_utils.py:204-211
Timestamp: 2025-07-03T12:58:18.407Z
Learning: In the CLP codebase, the validate_and_cache_dataset function in components/clp-py-utils/clp_py_utils/clp_metadata_db_utils.py uses in-place updates of the existing_datasets set parameter rather than returning a new set, as preferred by the development team.
Learnt from: davemarco
PR: y-scope/clp#892
File: components/log-viewer-webui/client/src/api/socket/MongoCollectionSocket.ts:27-31
Timestamp: 2025-05-09T19:15:26.180Z
Learning: For the MongoDB real-time updates implementation in components/log-viewer-webui/client/src/api/socket, a socket singleton pattern is used where a single shared socket connection is maintained rather than creating multiple connections. The socket lifecycle is managed centrally, with unsubscription handling in the useCursor React hook's cleanup function.
Learnt from: junhaoliao
PR: y-scope/clp#596
File: components/log-viewer-webui/client/src/api/query.js:16-23
Timestamp: 2024-11-21T15:51:33.203Z
Learning: In `components/log-viewer-webui/client/src/api/query.js`, the `ExtractJsonResp` type definition is accurate as-is and does not require modification. When suggesting changes to type definitions, ensure they align with the server-side definitions, referencing the source code if necessary.
components/webui/client/src/config/sql-table-suffix.ts (1)
Learnt from: haiqi96
PR: y-scope/clp#594
File: components/clp-package-utils/clp_package_utils/scripts/native/del_archives.py:90-96
Timestamp: 2024-11-15T16:22:33.635Z
Learning: In the CLP codebase, `table_prefix` is a trusted configuration value and can be safely embedded into SQL queries.
⏰ 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: lint-check (macos-latest)
- GitHub Check: lint-check (ubuntu-latest)
| export enum SqlTableSuffix { | ||
| ARCHIVES = "archives", | ||
| FILES = "files", | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Prefer const enum or a literal-union object to drop the runtime payload
A const enum (or an as const object + type alias) offers the same type-safety without emitting the enum lookup object in the compiled bundle, shaving a few bytes off every load and matching our “config is compile-time only” philosophy.
-export enum SqlTableSuffix {
+export const enum SqlTableSuffix {
ARCHIVES = "archives",
FILES = "files",
}If isolatedModules is enabled, fall back to a frozen object:
export const SQL_TABLE_SUFFIX = {
ARCHIVES: "archives",
FILES: "files",
} as const;
export type SqlTableSuffix = typeof SQL_TABLE_SUFFIX[keyof typeof SQL_TABLE_SUFFIX];🤖 Prompt for AI Agents
In components/webui/client/src/config/sql-table-suffix.ts around lines 4 to 7,
replace the current enum SqlTableSuffix with either a const enum or a frozen
object with an as const assertion to avoid emitting the enum object at runtime.
This reduces bundle size and aligns with compile-time only config philosophy. If
isolatedModules is enabled, use the frozen object approach with a type alias for
SqlTableSuffix to maintain type safety without runtime overhead.
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
components/clp-package-utils/clp_package_utils/scripts/start_clp.py(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: gibber9809
PR: y-scope/clp#504
File: components/core/src/clp_s/search/kql/CMakeLists.txt:29-29
Timestamp: 2024-10-22T15:36:04.655Z
Learning: When reviewing pull requests, focus on the changes within the PR and avoid commenting on issues outside the scope of the PR.
Learnt from: Bill-hbrhbr
PR: y-scope/clp#1036
File: components/clp-py-utils/clp_py_utils/clp_metadata_db_utils.py:204-211
Timestamp: 2025-07-03T12:58:18.407Z
Learning: In the CLP codebase, the validate_and_cache_dataset function in components/clp-py-utils/clp_py_utils/clp_metadata_db_utils.py uses in-place updates of the existing_datasets set parameter rather than returning a new set, as preferred by the development team.
Learnt from: davemarco
PR: y-scope/clp#892
File: components/log-viewer-webui/client/src/api/socket/MongoCollectionSocket.ts:27-31
Timestamp: 2025-05-09T19:15:26.180Z
Learning: For the MongoDB real-time updates implementation in components/log-viewer-webui/client/src/api/socket, a socket singleton pattern is used where a single shared socket connection is maintained rather than creating multiple connections. The socket lifecycle is managed centrally, with unsubscription handling in the useCursor React hook's cleanup function.
Learnt from: junhaoliao
PR: y-scope/clp#596
File: components/log-viewer-webui/client/src/api/query.js:16-23
Timestamp: 2024-11-21T15:51:33.203Z
Learning: In `components/log-viewer-webui/client/src/api/query.js`, the `ExtractJsonResp` type definition is accurate as-is and does not require modification. When suggesting changes to type definitions, ensure they align with the server-side definitions, referencing the source code if necessary.
components/clp-package-utils/clp_package_utils/scripts/start_clp.py (11)
Learnt from: Bill-hbrhbr
PR: y-scope/clp#1036
File: components/clp-py-utils/clp_py_utils/clp_metadata_db_utils.py:204-211
Timestamp: 2025-07-03T12:58:18.407Z
Learning: In the CLP codebase, the validate_and_cache_dataset function in components/clp-py-utils/clp_py_utils/clp_metadata_db_utils.py uses in-place updates of the existing_datasets set parameter rather than returning a new set, as preferred by the development team.
Learnt from: Bill-hbrhbr
PR: y-scope/clp#1004
File: components/clp-package-utils/clp_package_utils/scripts/native/decompress.py:139-144
Timestamp: 2025-06-24T08:54:14.438Z
Learning: In the CLP codebase, the get_orig_file_id function signature was changed after a recent merge to no longer accept a dataset parameter, making previous suggestions that reference this parameter invalid.
Learnt from: Bill-hbrhbr
PR: y-scope/clp#868
File: components/clp-py-utils/clp_py_utils/clp_metadata_db_utils.py:141-144
Timestamp: 2025-05-05T16:32:55.163Z
Learning: The column metadata table (created by `_create_column_metadata_table`) is only needed for dataset-specific workflows in `CLP_S` and is obsolete for non-dataset workflows.
Learnt from: haiqi96
PR: y-scope/clp#594
File: components/clp-package-utils/clp_package_utils/scripts/del_archives.py:56-65
Timestamp: 2024-11-18T16:49:20.248Z
Learning: When reviewing wrapper scripts in `components/clp-package-utils/clp_package_utils/scripts/`, note that it's preferred to keep error handling simple without adding extra complexity.
Learnt from: haiqi96
PR: y-scope/clp#594
File: components/clp-package-utils/clp_package_utils/scripts/native/del_archives.py:104-110
Timestamp: 2024-11-15T16:21:52.122Z
Learning: In `clp_package_utils/scripts/native/del_archives.py`, when deleting archives, the `archive` variable retrieved from the database is controlled and is always a single string without path components. Therefore, it's acceptable to skip additional validation checks for directory traversal in this context.
Learnt from: jackluo923
PR: y-scope/clp#1054
File: components/core/tools/scripts/lib_install/musllinux_1_2/install-packages-from-source.sh:6-8
Timestamp: 2025-07-01T14:51:19.172Z
Learning: In CLP installation scripts within `components/core/tools/scripts/lib_install/`, maintain consistency with existing variable declaration patterns across platforms rather than adding individual improvements like `readonly` declarations.
Learnt from: jackluo923
PR: y-scope/clp#1054
File: components/core/tools/scripts/lib_install/musllinux_1_2/install-packages-from-source.sh:6-8
Timestamp: 2025-07-01T14:51:19.172Z
Learning: In CLP installation scripts within `components/core/tools/scripts/lib_install/`, maintain consistency with existing variable declaration patterns across platforms rather than adding individual improvements like `readonly` declarations.
Learnt from: haiqi96
PR: y-scope/clp#594
File: components/clp-package-utils/clp_package_utils/scripts/native/del_archives.py:90-96
Timestamp: 2024-11-15T16:22:33.635Z
Learning: In the CLP codebase, `table_prefix` is a trusted configuration value and can be safely embedded into SQL queries.
Learnt from: Bill-hbrhbr
PR: y-scope/clp#1050
File: components/clp-package-utils/clp_package_utils/scripts/search.py:100-106
Timestamp: 2025-07-03T20:10:43.789Z
Learning: In the current CLP codebase implementation, dataset validation using validate_dataset() is performed within the native scripts (like clp_package_utils/scripts/native/search.py) rather than at the wrapper script level, where the native scripts handle their own parameter validation.
Learnt from: Bill-hbrhbr
PR: y-scope/clp#1050
File: components/clp-package-utils/clp_package_utils/scripts/search.py:100-106
Timestamp: 2025-07-03T20:10:43.789Z
Learning: In the current CLP codebase implementation, dataset validation using validate_dataset() is performed within the native scripts (like clp_package_utils/scripts/native/search.py) rather than at the wrapper script level, where the native scripts handle their own parameter validation.
Learnt from: Bill-hbrhbr
PR: y-scope/clp#1036
File: components/clp-package-utils/clp_package_utils/general.py:564-579
Timestamp: 2025-06-28T07:10:47.295Z
Learning: The validate_dataset function in components/clp-package-utils/clp_package_utils/general.py is designed to be called once upon function startup for dataset validation, not repeatedly during execution, making caching optimizations unnecessary.
🧬 Code Graph Analysis (1)
components/clp-package-utils/clp_package_utils/scripts/start_clp.py (1)
components/clp-py-utils/clp_py_utils/clp_metadata_db_utils.py (3)
get_archives_table_name(226-227)get_files_table_name(238-239)get_datasets_table_name(234-235)
⏰ 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: lint-check (macos-latest)
- GitHub Check: lint-check (ubuntu-latest)
🔇 Additional comments (1)
components/clp-package-utils/clp_package_utils/scripts/start_clp.py (1)
878-880: Call signature mismatch resolved – good catchSwitching to
get_*_table_name(table_prefix, None)brings the call back in line with the helper’s two-argument signature that was flagged earlier. 👍
| "SqlDbClpArchivesTableName": archives_table_name, | ||
| "SqlDbClpDatasetsTableName": get_datasets_table_name(table_prefix), | ||
| "SqlDbClpFilesTableName": get_files_table_name(table_prefix, dataset), | ||
| "SqlDbClpFilesTableName": files_table_name, | ||
| "SqlDbClpTablePrefix": table_prefix, | ||
| "SqlDbCompressionJobsTableName": COMPRESSION_JOBS_TABLE_NAME, |
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
There was a problem hiding this comment.
Looks good to me!
There’s a conflict showing up on GitHub, but I think that file was only moved in this PR.
Validation Steps Performed
-
Run
clp-text:- Compressed
hive-24h i-00c90a0f/ - All cards displayed correctly
- Compressed
-
Run
clp-json:- Compressed
spark-event-logsandpostgresqllogs across several databases - All cards rendered as expected
- Compressed
-
If the data fetching fails, all cards display default zero values
Todos
The current implementation passes the isLoading parameter through multiple layers down to DashboardCard. This approach is not ideal and will be refactored in an upcoming PR.
|
@hoophalab - merged i main and there was a conflict. I need you to reapprove |
hoophalab
left a comment
hoophalab
left a comment
There was a problem hiding this comment.
lgtm
all the validation steps passed after the merge
| if StorageEngine.CLP_S == clp_config.package.storage_engine: | ||
| dataset = CLP_DEFAULT_DATASET_NAME | ||
| archives_table_name = "" | ||
| files_table_name = "" | ||
| else: |
There was a problem hiding this comment.
We may want to pass null values in the future, but this would require changes in settings.json's typing and proper null value handling, instead of relying on the implicit conversion ${null} == "null".
@coderabbitai can you create an issue on this?
There was a problem hiding this comment.
🧩 Analysis chain
I'll create a GitHub issue to track this improvement for proper null value handling in settings.json.
🏁 Script executed:
#!/bin/bash
# Create GitHub issue for null value handling improvement
gh issue create \
--title "Improve null value handling in settings.json instead of implicit string conversion" \
--body "## Problem
Currently, the codebase relies on implicit conversion where \`\${null} == \"null\"\` when handling null values in settings.json. This approach is not robust and may cause issues in the future.
## Context
In PR #1042, the code was changed to set \`archives_table_name\` and \`files_table_name\` to empty strings for CLP_S storage engine instead of using null values. While this works for the current implementation, it highlights the need for proper null value handling.
## Proposed Solution
- Update settings.json typing to properly handle null values
- Implement proper null value handling instead of relying on implicit string conversion
- Consider how null values should be properly serialized and deserialized
## References
- PR: https://github.com/y-scope/clp/pull/1042
- Comment: https://github.com/y-scope/clp/pull/1042#discussion_r2195280156
- Requested by: hoophalab" \
--assignee hoophalab
Length of output: 1068
@hoophalab I've created issue #1087 to track this improvement: #1087
The issue documents the concern about the current implicit conversion approach where ${null} == "null" and proposes implementing proper null value handling in settings.json typing instead of relying on empty strings as a workaround. This will help ensure more robust handling of null values in the future.
🧬 Code Graph Analysis Results
Relevant snippets from components/clp-package-utils/clp_package_utils/scripts/start_clp.py (entire file)
- The file defines a CLI script to start various CLP components as Docker containers.
- It uses a
CLPConfigobject loaded from a YAML config file and validates credentials and configuration. - Components include DB, Queue, Redis, Results Cache, Compression Scheduler, Query Scheduler, Workers, Reducer, and WebUI.
- Each component has a dedicated
start_<component>function that:- Checks if the container already exists or is running.
- Validates configuration and directories.
- Sets up Docker mounts and environment variables.
- Constructs and runs a
docker runcommand to start the container.
- The
start_webuifunction reads and updates twosettings.jsonfiles (client and server) with configuration values before starting the WebUI container. - The
update_settings_objectfunction recursively updates a nested dictionary with new values, raising an error if keys are invalid. - The
read_and_update_settings_jsonfunction reads a JSON file, applies updates viaupdate_settings_object, and returns the updated dictionary. - The
mainfunction parses CLI arguments, loads and validates the config, generates container config and mounts, and starts the requested components. - The script handles environment variables and mounts for AWS authentication, including special handling for S3 storage and AWS credentials.
- The user comment about passing null values and handling them properly in
settings.jsonrelates to thestart_webuifunction where settings JSON files are updated with values that may currently rely on implicit conversion ofnullto string"null".
Relevant snippet from components/clp-package-utils/clp_package_utils/general.py (lines 216-285)
def generate_container_config(
clp_config: CLPConfig, clp_home: pathlib.Path
) -> Tuple[CLPConfig, CLPDockerMounts]:
"""
Copies the given config and sets up mounts mapping the relevant host paths into the container
:param clp_config:
:param clp_home:
:return: The container config and the mounts.
"""
container_clp_config = clp_config.copy(deep=True)
docker_mounts = CLPDockerMounts(clp_home, CONTAINER_CLP_HOME)
if StorageType.FS == clp_config.logs_input.type:
input_logs_dir = clp_config.logs_input.directory.resolve()
container_clp_config.logs_input.directory = (
CONTAINER_INPUT_LOGS_ROOT_DIR / input_logs_dir.relative_to(input_logs_dir.anchor)
)
docker_mounts.input_logs_dir = DockerMount(
DockerMountType.BIND, input_logs_dir, container_clp_config.logs_input.directory, True
)
if not is_path_already_mounted(
clp_home, CONTAINER_CLP_HOME, clp_config.data_directory, container_clp_config.data_directory
):
docker_mounts.data_dir = DockerMount(
DockerMountType.BIND, clp_config.data_directory, container_clp_config.data_directory
)
if not is_path_already_mounted(
clp_home, CONTAINER_CLP_HOME, clp_config.logs_directory, container_clp_config.logs_directory
):
docker_mounts.logs_dir = DockerMount(
DockerMountType.BIND, clp_config.logs_directory, container_clp_config.logs_directory
)
if not is_path_already_mounted(
clp_home,
CONTAINER_CLP_HOME,
clp_config.archive_output.get_directory(),
container_clp_config.archive_output.get_directory(),
):
docker_mounts.archives_output_dir = DockerMount(
DockerMountType.BIND,
clp_config.archive_output.get_directory(),
container_clp_config.archive_output.get_directory(),
)
if not is_path_already_mounted(
clp_home,
CONTAINER_CLP_HOME,
clp_config.stream_output.get_directory(),
container_clp_config.stream_output.get_directory(),
):
docker_mounts.stream_output_dir = DockerMount(
DockerMountType.BIND,
clp_config.stream_output.get_directory(),
container_clp_config.stream_output.get_directory(),
)
# Only create the mount if the directory exists
if clp_config.aws_config_directory is not None:
container_clp_config.aws_config_directory = CONTAINER_AWS_CONFIG_DIRECTORY
docker_mounts.aws_config_dir = DockerMount(
DockerMountType.BIND,
clp_config.aws_config_directory,
container_clp_config.aws_config_directory,
)
return container_clp_config, docker_mounts- This function creates a deep copy of the CLP config and adjusts paths for container mounts.
- It returns the container config and a
CLPDockerMountsobject with Docker mount specifications. - This is relevant because the container config is used in
start_webuiand other start functions.
Relevant snippet from components/clp-package-utils/clp_package_utils/general.py (lines 90-100)
class CLPDockerMounts:
def __init__(self, clp_home: pathlib.Path, docker_clp_home: pathlib.Path):
self.input_logs_dir: typing.Optional[DockerMount] = None
self.clp_home: typing.Optional[DockerMount] = DockerMount(
DockerMountType.BIND, clp_home, docker_clp_home
)
self.data_dir: typing.Optional[DockerMount] = None
self.logs_dir: typing.Optional[DockerMount] = None
self.archives_output_dir: typing.Optional[DockerMount] = None
self.stream_output_dir: typing.Optional[DockerMount] = None
self.aws_config_dir: typing.Optional[DockerMount] = None- Defines a container for Docker mount points used in the start scripts.
Relevant snippet from components/clp-package-utils/clp_package_utils/general.py (lines 68-87)
class DockerMount:
def __init__(
self,
type: DockerMountType,
src: pathlib.Path,
dst: pathlib.Path,
is_read_only: bool = False,
):
self.__type = type
self.__src = src
self.__dst = dst
self.__is_read_only = is_read_only
def __str__(self):
mount_str = (
f"type={DOCKER_MOUNT_TYPE_STRINGS[self.__type]},src={self.__src},dst={self.__dst}"
)
if self.__is_read_only:
mount_str += ",readonly"
return mount_str- Represents a Docker mount option, used to generate
--mountarguments fordocker run.
Relevant snippet from components/clp-py-utils/clp_py_utils/s3_utils.py (lines 97-170)
def generate_container_auth_options(
clp_config: CLPConfig, component_type: str
) -> Tuple[bool, List[str]]:
"""
Generates Docker container authentication options for AWS S3 access based on the given type.
Handles authentication methods that require extra configuration (profile, env_vars).
:param clp_config: CLPConfig containing storage configurations.
:param component_type: Type of calling container (compression, log_viewer, or query).
:return: Tuple of (whether aws config mount is needed, credential env_vars to set).
:raises: ValueError if environment variables are not set correctly.
"""
output_storages_by_component_type: List[Union[S3Storage, FsStorage]] = []
input_storage_needed = False
if component_type in (
COMPRESSION_SCHEDULER_COMPONENT_NAME,
COMPRESSION_WORKER_COMPONENT_NAME,
):
output_storages_by_component_type = [clp_config.archive_output.storage]
input_storage_needed = True
elif component_type in (WEBUI_COMPONENT_NAME,):
output_storages_by_component_type = [clp_config.stream_output.storage]
elif component_type in (
QUERY_SCHEDULER_COMPONENT_NAME,
QUERY_WORKER_COMPONENT_NAME,
):
output_storages_by_component_type = [
clp_config.archive_output.storage,
clp_config.stream_output.storage,
]
else:
raise ValueError(f"Component type {component_type} is not valid.")
config_mount = False
add_env_vars = False
for storage in output_storages_by_component_type:
if StorageType.S3 == storage.type:
auth = storage.s3_config.aws_authentication
if AwsAuthType.profile == auth.type:
config_mount = True
elif AwsAuthType.env_vars == auth.type:
add_env_vars = True
if input_storage_needed and StorageType.S3 == clp_config.logs_input.type:
auth = clp_config.logs_input.aws_authentication
if AwsAuthType.profile == auth.type:
config_mount = True
elif AwsAuthType.env_vars == auth.type:
add_env_vars = True
credentials_env_vars = []
if add_env_vars:
access_key_id = os.getenv(AWS_ENV_VAR_ACCESS_KEY_ID)
secret_access_key = os.getenv(AWS_ENV_VAR_SECRET_ACCESS_KEY)
if access_key_id and secret_access_key:
credentials_env_vars.extend(
(
f"{AWS_ENV_VAR_ACCESS_KEY_ID}={access_key_id}",
f"{AWS_ENV_VAR_SECRET_ACCESS_KEY}={secret_access_key}",
)
)
else:
raise ValueError(
f"{AWS_ENV_VAR_ACCESS_KEY_ID} and {AWS_ENV_VAR_SECRET_ACCESS_KEY} "
"environment variables not set"
)
if os.getenv(AWS_ENV_VAR_SESSION_TOKEN):
raise ValueError(
f"{AWS_ENV_VAR_SESSION_TOKEN} not supported for environmental variable credentials."
)
return (config_mount, credentials_env_vars)- Generates Docker environment variables and mounts for AWS S3 authentication based on the component type and config.
- Used in
start_webuiand other start functions to properly configure AWS credentials inside containers.
Relevant snippet from components/clp-package-utils/clp_package_utils/general.py (lines 524-533)
def validate_webui_config(
clp_config: CLPConfig,
client_settings_json_path: pathlib.Path,
server_settings_json_path: pathlib.Path,
):
for path in [client_settings_json_path, server_settings_json_path]:
if not path.exists():
raise ValueError(f"{WEBUI_COMPONENT_NAME} {path} is not a valid path to settings.json")
validate_port(f"{WEBUI_COMPONENT_NAME}.port", clp_config.webui.host, clp_config.webui.port)- Validates that the WebUI settings JSON files exist and that the configured port is valid.
- Called in
start_webuibefore updating settings JSON files.
Relevant snippet from components/clp-py-utils/clp_py_utils/clp_config.py (lines 588-773)
- The
CLPConfigclass is a Pydantic model representing the entire CLP configuration. - It includes nested configs for database, queue, redis, reducer, results cache, compression scheduler, query scheduler, workers, webui, archive output, stream output, data and logs directories, and AWS config directory.
- It has methods to:
- Make paths absolute relative to CLP_HOME.
- Validate logs input, archive output, stream output, data directory, logs directory, and AWS config directory.
- Load credentials from a credentials file.
- Dump the config to a primitive dictionary suitable for serialization.
- The
webuiconfig is part of this model and is used instart_webui.
Summary of relevance to user comment
- The user comment concerns passing null values in
settings.jsonfiles used by the WebUI component. - The
start_webuifunction instart_clp.pyreads, updates, and writes back these JSON files (client/settings.jsonandserver/dist/server/settings.json). - The update is done via
update_settings_objectwhich replaces values in the JSON dict. - Currently, some values may be set to Python
Nonewhich when serialized to JSON becomesnull, but the comment suggests that implicit conversion${null} == "null"is relied upon and proper handling or typing changes insettings.jsonmay be needed. - The
start_webuifunction also handles AWS authentication environment variables and mounts, which may affect how null or missing values are handled. - The
CLPConfigmodel and validation functions ensure the config is valid before starting the WebUI. - The
generate_container_configfunction creates a container-specific config with paths adjusted for Docker mounts, which is used instart_webui. - The
generate_container_auth_optionsfunction manages AWS auth environment variables and mounts for the container.
These snippets provide the full context to understand how settings.json is updated and how null values might be passed or handled, as well as the configuration and container setup relevant to the WebUI component.
[End of relevant snippets.]
🧠 Learnings used
Learnt from: Bill-hbrhbr
PR: y-scope/clp#1050
File: components/clp-package-utils/clp_package_utils/scripts/search.py:100-106
Timestamp: 2025-07-03T20:10:43.789Z
Learning: In the current CLP codebase implementation, dataset validation using validate_dataset() is performed within the native scripts (like clp_package_utils/scripts/native/search.py) rather than at the wrapper script level, where the native scripts handle their own parameter validation.
Learnt from: Bill-hbrhbr
PR: y-scope/clp#1050
File: components/clp-package-utils/clp_package_utils/scripts/search.py:100-106
Timestamp: 2025-07-03T20:10:43.789Z
Learning: In the current CLP codebase implementation, dataset validation using validate_dataset() is performed within the native scripts (like clp_package_utils/scripts/native/search.py) rather than at the wrapper script level, where the native scripts handle their own parameter validation.
Learnt from: Bill-hbrhbr
PR: y-scope/clp#1036
File: components/clp-py-utils/clp_py_utils/clp_metadata_db_utils.py:204-211
Timestamp: 2025-07-03T12:58:18.407Z
Learning: In the CLP codebase, the validate_and_cache_dataset function in components/clp-py-utils/clp_py_utils/clp_metadata_db_utils.py uses in-place updates of the existing_datasets set parameter rather than returning a new set, as preferred by the development team.
Learnt from: Bill-hbrhbr
PR: y-scope/clp#1036
File: components/clp-package-utils/clp_package_utils/general.py:564-579
Timestamp: 2025-06-28T07:10:47.295Z
Learning: The validate_dataset function in components/clp-package-utils/clp_package_utils/general.py is designed to be called once upon function startup for dataset validation, not repeatedly during execution, making caching optimizations unnecessary.
Learnt from: Bill-hbrhbr
PR: y-scope/clp#868
File: components/clp-py-utils/clp_py_utils/clp_metadata_db_utils.py:141-144
Timestamp: 2025-05-05T16:32:55.163Z
Learning: The column metadata table (created by `_create_column_metadata_table`) is only needed for dataset-specific workflows in `CLP_S` and is obsolete for non-dataset workflows.
Learnt from: Bill-hbrhbr
PR: y-scope/clp#1004
File: components/clp-package-utils/clp_package_utils/scripts/native/decompress.py:139-144
Timestamp: 2025-06-24T08:54:14.438Z
Learning: In the CLP codebase, the get_orig_file_id function signature was changed after a recent merge to no longer accept a dataset parameter, making previous suggestions that reference this parameter invalid.
Learnt from: haiqi96
PR: y-scope/clp#594
File: components/clp-package-utils/clp_package_utils/scripts/native/del_archives.py:90-96
Timestamp: 2024-11-15T16:22:33.635Z
Learning: In the CLP codebase, `table_prefix` is a trusted configuration value and can be safely embedded into SQL queries.
Learnt from: gibber9809
PR: y-scope/clp#504
File: components/core/src/clp_s/search/kql/CMakeLists.txt:29-29
Timestamp: 2024-10-22T15:36:04.655Z
Learning: When reviewing pull requests, focus on the changes within the PR and avoid commenting on issues outside the scope of the PR.
|
asking @junhaoliao for approval since @hoophalab dosent have write access |
junhaoliao
left a comment
junhaoliao
left a comment
There was a problem hiding this comment.
Deferring to @hoophalab 's review
davemarco
changed the title
4 participants
This PR is blocked by #1004 / #1050. We cannot test fix for UI, until multiple datasets can be ingested
Description
PR #868 modified the names of the metadata tables leading to issue #1024. The table names were changed, breaking the ui.
The pr is fixes the issue by first querying all datasets, then issuing a large runtime generated query that includes a subquery for each dataset, and finally combines the results using UNION ALL. The large query should be more performant than sending multiple queries for each dataset.
This is even more complicated since clp does not have datasets, so in that case, the dataset query is not sent, and the old query is used.
The solution is a bit crude, but should be fine for a limited number of datasets. I will create an issue about another solution we discussed where we add a dataset column instead of renaming the tables.
Checklist
breaking change.
Validation performed
Tested clp and clp-s(with 1 dataset). Need to wait #1004 to test multiple dataset, but it should support it
Summary by CodeRabbit
New Features
Refactor
Chores