Skip to content
Merged
Show file tree
Hide file tree
Changes from 26 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 2 additions & 6 deletions components/webui/client/src/App.tsx
Original file line number Diff line number Diff line change
@@ -1,20 +1,16 @@
import {RouterProvider} from "react-router";

import {
QueryClient,
QueryClientProvider,
} from "@tanstack/react-query";
import {QueryClientProvider} from "@tanstack/react-query";
import {ReactQueryDevtools} from "@tanstack/react-query-devtools";
import {ConfigProvider} from "antd";

import queryClient from "./config/queryClient";
import router from "./router";
import THEME_CONFIG from "./theme";

import "@ant-design/v5-patch-for-react-19";


const queryClient = new QueryClient();

/**
* Renders Web UI app.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import axios, {
import {
ExtractStreamResp,
QUERY_JOB_TYPE,
} from "../typings/query";
} from "../../typings/query";


interface SubmitExtractStreamJobProps {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import axios, {AxiosResponse} from "axios";

import {Nullable} from "../typings/common";
import {Nullable} from "../../typings/common";


// eslint-disable-next-line no-warning-comments
Expand Down
11 changes: 10 additions & 1 deletion components/webui/client/src/components/DashboardCard/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ interface DashboardCardProps {
titleColor?: string;
backgroundColor?: string;
children?: React.ReactNode;
isLoading?: boolean;
}

/**
Expand All @@ -23,13 +24,21 @@ interface DashboardCardProps {
* @param props.titleColor
* @param props.backgroundColor
* @param props.children
* @param props.isLoading
* @return
*/
const DashboardCard = ({title, titleColor, backgroundColor, children}: DashboardCardProps) => {
const DashboardCard = ({
title,
titleColor,
backgroundColor,
children,
isLoading = false,
}: DashboardCardProps) => {
return (
<Card
className={styles["card"] || ""}
hoverable={true}
loading={isLoading}
style={{backgroundColor}}
>
<div className={styles["cardContent"]}>
Expand Down
4 changes: 4 additions & 0 deletions components/webui/client/src/components/StatCard/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ interface StatCardProps {
backgroundColor?: string;
statSize?: string;
statColor?: string;
isLoading?: boolean;
}

/**
Expand All @@ -27,6 +28,7 @@ interface StatCardProps {
* @param props.backgroundColor
* @param props.statSize
* @param props.statColor
* @param props.isLoading
* @return
*/
const StatCard = ({
Expand All @@ -36,9 +38,11 @@ const StatCard = ({
backgroundColor,
statSize,
statColor,
isLoading = false,
}: StatCardProps) => {
const props: DashboardCardProps = {
title,
isLoading,
...(titleColor ?
{titleColor} :
{}),
Expand Down
14 changes: 14 additions & 0 deletions components/webui/client/src/config/queryClient.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
import {QueryClient} from "@tanstack/react-query";


const DEFAULT_REFETCH_INTERVAL_MILLIS = 10_000;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 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,
},
},
});
Comment on lines +6 to +12
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 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.

Suggested change
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.


export default queryClient;
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import StatCard from "../../../components/StatCard";
interface DetailsCardProps {
title: string;
stat: string;
isLoading: boolean;
}

/**
Expand All @@ -14,12 +15,14 @@ interface DetailsCardProps {
* @param props
* @param props.title
* @param props.stat
* @param props.isLoading
* @return
*/
const DetailsCard = ({title, stat}: DetailsCardProps) => {
const DetailsCard = ({title, stat, isLoading}: DetailsCardProps) => {
Copy link
Contributor

@hoophalab hoophalab Jul 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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:

  1. Combine all three into one: DetailsCard as all cards requires title and stat/content
  2. DetailsCard accepts title as a property
  3. DetailsCard accepts content text (i.e. No timestamp data and 0.00%) as react node in children, so that space saving can have a different fontsize.
  4. two options:
    1. All other cards uses DetailsCard directly and isLoading becomes a page state in zustand store
    2. Have a ...props parameter in all concrete cards such as File Message... which are directed to DetailsCard

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did also think these cards can be refactored when working on the code. But probably a different PR.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

okay this is reasonable, but you will have to merge into this PR

const {token} = theme.useToken();
return (
<StatCard
isLoading={isLoading}
stat={stat}
statColor={token.colorTextSecondary}
statSize={"1.4rem"}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,18 +5,21 @@ import DetailsCard from "./DetailsCard";

interface FilesProps {
numFiles: Nullable<number>;
isLoading: boolean;
}

/**
* Renders the files statistic.
*
* @param props
* @param props.numFiles
* @param props.isLoading
* @return
*/
const Files = ({numFiles}: FilesProps) => {
const Files = ({numFiles, isLoading}: FilesProps) => {
return (
<DetailsCard
isLoading={isLoading}
stat={(numFiles ?? 0).toString()}
title={"Files"}/>
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,18 +5,21 @@ import DetailsCard from "./DetailsCard";

interface MessagesProps {
numMessages: Nullable<number>;
isLoading: boolean;
}

/**
* Renders the messages statistic.
*
* @param props
* @param props.numMessages
* @param props.isLoading
* @return
*/
const Messages = ({numMessages}: MessagesProps) => {
const Messages = ({numMessages, isLoading}: MessagesProps) => {
return (
<DetailsCard
isLoading={isLoading}
stat={(numMessages ?? 0).toString()}
title={"Messages"}/>
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ const DATE_FORMAT = "MMMM D, YYYY";
interface TimeRangeProps {
beginDate: Dayjs;
endDate: Dayjs;
isLoading: boolean;
}

/**
Expand All @@ -16,9 +17,10 @@ interface TimeRangeProps {
* @param props
* @param props.beginDate
* @param props.endDate
* @param props.isLoading
* @return
*/
const TimeRange = ({beginDate, endDate}: TimeRangeProps) => {
const TimeRange = ({beginDate, endDate, isLoading}: TimeRangeProps) => {
let stat;
if (beginDate.isValid() && endDate.isValid()) {
stat = `${beginDate.format(DATE_FORMAT)} - ${endDate.format(DATE_FORMAT)}`;
Expand All @@ -28,6 +30,7 @@ const TimeRange = ({beginDate, endDate}: TimeRangeProps) => {

return (
<DetailsCard
isLoading={isLoading}
stat={stat}
title={"Time Range"}/>
);
Expand Down
100 changes: 35 additions & 65 deletions components/webui/client/src/pages/IngestPage/Details/index.tsx
Original file line number Diff line number Diff line change
@@ -1,104 +1,74 @@
import {
useCallback,
useEffect,
useState,
} from "react";

import dayjs, {Dayjs} from "dayjs";
import {Nullable} from "src/typings/common";
import {useQuery} from "@tanstack/react-query";
import dayjs from "dayjs";

import {querySql} from "../../../api/sql";
import {
CLP_STORAGE_ENGINES,
SETTINGS_STORAGE_ENGINE,
} from "../../../config";
import useIngestStatsStore from "../ingestStatsStore";
import {fetchDatasetNames} from "../../SearchPage/SearchControls/Dataset/sql";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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 -10

Length 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/src

Fix 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.

import Files from "./Files";
import styles from "./index.module.css";
import Messages from "./Messages";
import {
DetailsResp,
getDetailsSql,
DETAILS_DEFAULT,
fetchClpDetails,
fetchClpsDetails,
} from "./sql";
import TimeRange from "./TimeRange";


/**
* Default state for details.
*/
const DETAILS_DEFAULT = Object.freeze({
beginDate: dayjs(null),
endDate: dayjs(null),

numFiles: 0,
numMessages: 0,
});

/**
* Renders grid with compression details.
*
* @return
*/
const Details = () => {
const {refreshInterval} = useIngestStatsStore();
const [beginDate, setBeginDate] = useState<Dayjs>(DETAILS_DEFAULT.beginDate);
const [endDate, setEndDate] = useState<Dayjs>(DETAILS_DEFAULT.endDate);
const [numFiles, setNumFiles] = useState<Nullable<number>>(DETAILS_DEFAULT.numFiles);
const [numMessages, setNumMessages] = useState<Nullable<number>>(DETAILS_DEFAULT.numMessages);
const {data: datasetNames = [], isSuccess: isSuccessDatasetNames} = useQuery({
queryKey: ["datasets"],
queryFn: fetchDatasetNames,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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' -n

Length 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.tsx

Length 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.

Suggested change
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.

enabled: CLP_STORAGE_ENGINES.CLP_S === SETTINGS_STORAGE_ENGINE,
});

/**
* Fetches details stats from the server.
*
* @throws {Error} If the response is undefined.
*/
const fetchDetailsStats = useCallback(() => {
querySql<DetailsResp>(getDetailsSql()).then((resp) => {
const [details] = resp.data;
if ("undefined" === typeof details) {
throw new Error("Details response is undefined");
const {data: details = DETAILS_DEFAULT, isPending} = useQuery({
queryKey: [
"details",
datasetNames,
],
queryFn: async () => {
if (CLP_STORAGE_ENGINES.CLP === SETTINGS_STORAGE_ENGINE) {
return fetchClpDetails();
}
setBeginDate(dayjs.utc(details.begin_timestamp));
setEndDate(dayjs.utc(details.end_timestamp));
setNumFiles(details.num_files);
setNumMessages(details.num_messages);
})
.catch((e: unknown) => {
console.error("Failed to fetch details stats", e);
});
}, []);


useEffect(() => {
fetchDetailsStats();
const intervalId = setInterval(fetchDetailsStats, refreshInterval);

return () => {
clearInterval(intervalId);
};
}, [
refreshInterval,
fetchDetailsStats,
]);
return fetchClpsDetails(datasetNames);
},
enabled: CLP_STORAGE_ENGINES.CLP === SETTINGS_STORAGE_ENGINE || isSuccessDatasetNames,
});

if (CLP_STORAGE_ENGINES.CLP === SETTINGS_STORAGE_ENGINE) {
return (
<div className={styles["detailsGrid"]}>
<div className={styles["timeRange"]}>
<TimeRange
beginDate={beginDate}
endDate={endDate}/>
beginDate={dayjs.utc(details.begin_timestamp)}
endDate={dayjs.utc(details.end_timestamp)}
isLoading={isPending}/>
</div>
<Messages numMessages={numMessages}/>
<Files numFiles={numFiles}/>
<Messages
isLoading={isPending}
numMessages={details.num_messages}/>
<Files
isLoading={isPending}
numFiles={details.num_files}/>
</div>
);
}

return (
<div>
<TimeRange
beginDate={beginDate}
endDate={endDate}/>
beginDate={dayjs.utc(details.begin_timestamp)}
endDate={dayjs.utc(details.end_timestamp)}
isLoading={isPending}/>
</div>
);
Comment on lines 67 to 73
Copy link
Contributor

@hoophalab hoophalab Jul 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Quick question: I might be missing some context here. Why don’t we show Messages and Files for CLP-s? @davemarco

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The clp-s storage engine doesn't currently keep track of the number of log events and files in the archives.

};
Expand Down
Loading