components/log-viewer-webui/client/src/index.tsx (1)

9-10: Consider gracefully handling potential null references.

While the non-null assertion operator (!) informs TypeScript that the element will never be null, it could still lead to runtime errors if the element does not actually exist. It might be safer to throw an error or provide a fallback to handle this scenario more robustly.

Below is a sample approach:

-const root = createRoot(document.getElementById("root")!);
+const rootElement = document.getElementById("root");
+if (false == rootElement) {
+    throw new Error("Root element not found in DOM");
+}
+const root = createRoot(rootElement);

components/log-viewer-webui/client/src/App.tsx (1)

10-10: Confirm JSDoc return annotation.

Removing the explicit return type is often fine in TypeScript since return types can be inferred. However, please ensure that your documentation accurately reflects the function’s intended return structure.

components/log-viewer-webui/client/src/api/query.ts (1)

29-44: Enhance error handling and request cancellation.

While submitExtractStreamJob is straightforward, consider adding optional request cancellation (via Axios.cancelToken) or error handling to avoid unhandled promise rejections if the server or network fails.

components/log-viewer-webui/client/src/ui/QueryStatus.tsx (1)

29-29: Document return values more explicitly.
Please clarify the returned type or structure to help future readers understand the component’s outcome.

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

components/log-viewer-webui/client/tsconfig.node.json (2)

3-7: Consider relocating tsBuildInfo and reviewing skipLibCheck setting

Two suggestions for improvement:

1. Consider moving tsBuildInfoFile outside of node_modules to avoid CI/CD caching issues. A better location would be .ts-cache or similar.
2. The skipLibCheck: true setting might hide type issues in dependencies. Consider setting it to false if build time permits.

-    "tsBuildInfoFile": "./node_modules/.tmp/tsconfig.node.tsbuildinfo",
+    "tsBuildInfoFile": "./.ts-cache/tsconfig.node.tsbuildinfo",

---

16-21: Consider enabling additional strict checks

The current strict checks are good, but consider enabling these additional TypeScript checks for better type safety:

• noImplicitReturns: Ensure all code paths return a value
• exactOptionalPropertyTypes: More precise optional property types
• noPropertyAccessFromIndexSignature: Prevent typos in property access

     "strict": true,
     "noUnusedLocals": true,
     "noUnusedParameters": true,
     "noFallthroughCasesInSwitch": true,
-    "noUncheckedSideEffectImports": true
+    "noUncheckedSideEffectImports": true,
+    "noImplicitReturns": true,
+    "exactOptionalPropertyTypes": true,
+    "noPropertyAccessFromIndexSignature": true

🧰 Tools

🪛 Biome (1.9.4)

[error] 16-16: JSON standard does not allow comments.

(parse)

---

[error] 17-17: End of file expected

Use an array for a sequence of values: [1, 2]

(parse)

---

[error] 17-17: End of file expected

Use an array for a sequence of values: [1, 2]

(parse)

---

[error] 17-17: End of file expected

Use an array for a sequence of values: [1, 2]

(parse)

---

[error] 17-17: End of file expected

Use an array for a sequence of values: [1, 2]

(parse)

---

[error] 18-18: End of file expected

Use an array for a sequence of values: [1, 2]

(parse)

---

[error] 18-18: End of file expected

Use an array for a sequence of values: [1, 2]

(parse)

---

[error] 18-18: End of file expected

Use an array for a sequence of values: [1, 2]

(parse)

---

[error] 18-18: End of file expected

Use an array for a sequence of values: [1, 2]

(parse)

---

[error] 19-19: End of file expected

Use an array for a sequence of values: [1, 2]

(parse)

---

[error] 19-19: End of file expected

Use an array for a sequence of values: [1, 2]

(parse)

---

[error] 19-19: End of file expected

Use an array for a sequence of values: [1, 2]

(parse)

---

[error] 19-19: End of file expected

Use an array for a sequence of values: [1, 2]

(parse)

---

[error] 20-20: End of file expected

Use an array for a sequence of values: [1, 2]

(parse)

---

[error] 20-20: End of file expected

Use an array for a sequence of values: [1, 2]

(parse)

---

[error] 20-20: End of file expected

Use an array for a sequence of values: [1, 2]

(parse)

---

[error] 20-20: End of file expected

Use an array for a sequence of values: [1, 2]

(parse)

---

[error] 21-21: End of file expected

Use an array for a sequence of values: [1, 2]

(parse)

---

[error] 21-21: End of file expected

Use an array for a sequence of values: [1, 2]

(parse)

---

[error] 21-21: End of file expected

Use an array for a sequence of values: [1, 2]

(parse)

components/log-viewer-webui/client/tsconfig.app.json (2)

18-23: Consider additional type-checking options

The current linting configuration is good, but could be enhanced with these additional checks:

 {
   "compilerOptions": {
     /* Linting */
     "strict": true,
     "noUnusedLocals": true,
     "noUnusedParameters": true,
     "noFallthroughCasesInSwitch": true,
-    "noUncheckedSideEffectImports": true
+    "noUncheckedSideEffectImports": true,
+    "noImplicitReturns": true,
+    "exactOptionalPropertyTypes": true,
+    "noPropertyAccessFromIndexSignature": true
   }
 }

🧰 Tools

🪛 Biome (1.9.4)

[error] 18-18: JSON standard does not allow comments.

(parse)

---

[error] 19-19: End of file expected

Use an array for a sequence of values: [1, 2]

(parse)

---

[error] 19-19: End of file expected

Use an array for a sequence of values: [1, 2]

(parse)

---

[error] 19-19: End of file expected

Use an array for a sequence of values: [1, 2]

(parse)

---

[error] 19-19: End of file expected

Use an array for a sequence of values: [1, 2]

(parse)

---

[error] 20-20: End of file expected

Use an array for a sequence of values: [1, 2]

(parse)

---

[error] 20-20: End of file expected

Use an array for a sequence of values: [1, 2]

(parse)

---

[error] 20-20: End of file expected

Use an array for a sequence of values: [1, 2]

(parse)

---

[error] 20-20: End of file expected

Use an array for a sequence of values: [1, 2]

(parse)

---

[error] 21-21: End of file expected

Use an array for a sequence of values: [1, 2]

(parse)

---

[error] 21-21: End of file expected

Use an array for a sequence of values: [1, 2]

(parse)

---

[error] 21-21: End of file expected

Use an array for a sequence of values: [1, 2]

(parse)

---

[error] 21-21: End of file expected

Use an array for a sequence of values: [1, 2]

(parse)

---

[error] 22-22: End of file expected

Use an array for a sequence of values: [1, 2]

(parse)

---

[error] 22-22: End of file expected

Use an array for a sequence of values: [1, 2]

(parse)

---

[error] 22-22: End of file expected

Use an array for a sequence of values: [1, 2]

(parse)

---

[error] 22-22: End of file expected

Use an array for a sequence of values: [1, 2]

(parse)

---

[error] 23-23: End of file expected

Use an array for a sequence of values: [1, 2]

(parse)

---

[error] 23-23: End of file expected

Use an array for a sequence of values: [1, 2]

(parse)

---

[error] 23-23: End of file expected

Use an array for a sequence of values: [1, 2]

(parse)

---

25-25: Consider more specific include/exclude paths

The current configuration includes all files under src. Consider adding an exclude array to filter out test files and other non-app code:

 {
   "compilerOptions": {
     // ... existing options ...
   },
-  "include": ["src"]
+  "include": ["src"],
+  "exclude": ["src/**/*.test.ts", "src/**/*.test.tsx", "src/**/*.spec.ts", "src/**/*.spec.tsx"]
 }

🧰 Tools

🪛 Biome (1.9.4)

[error] 25-25: End of file expected

Use an array for a sequence of values: [1, 2]

(parse)

---

[error] 25-25: End of file expected

Use an array for a sequence of values: [1, 2]

(parse)

---

[error] 25-25: End of file expected

Use an array for a sequence of values: [1, 2]

(parse)

---

[error] 25-26: End of file expected

Use an array for a sequence of values: [1, 2]

(parse)

components/log-viewer-webui/client/vite.config.ts (1)

1-18: Consider environment variables for development and production differences.

Currently, the server proxy is hard-coded to http://localhost:3000/query. You may want to employ environment variables (e.g., process.env.NODE_ENV for dev or production) or a .env file to manage different URLs. This would enhance deploy-time flexibility.

components/log-viewer-webui/client/package.json (2)

5-5: Use a more descriptive entry name if desired.

While "main": "src/main.tsx" is valid, you could consider using a more descriptive name to indicate the application’s entry point. This is purely a stylistic choice that might help distinguish from other modules.

---

15-22: Document newly introduced dependencies.

The new dependencies (@emotion/*, axios, etc.) should be noted in the project’s documentation to inform future maintainers of their usage. This is especially important when adopting UI libraries like MUI Joy.

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

#!/bin/bash
# Description: Check for potential type issues in declaration files
# Look for .d.ts files with @ts-ignore or @ts-nocheck
rg -t ts "(@ts-ignore|@ts-nocheck)" --glob "*.d.ts"

#!/bin/bash
# Let's check for declaration files first to understand the scope
fd "\.d\.ts$"

# Also check package.json to understand dependencies that might need type checking
fd "package.json$" --exec cat {}

Taskfile.yml (1)

250-257: Add comments to clarify the build tools in use.

While the build commands are well-structured, it would be helpful to add comments indicating that these are Vite and webpack builds respectively.

Apply this diff to improve clarity:

      - |-
        cd "client"
+       # Build the client using Vite
        PATH="{{.G_NODEJS_22_BIN_DIR}}":$PATH npm run build -- \
          --outDir "{{.OUTPUT_DIR}}/client"
      - |-
        cd "yscope-log-viewer"
+       # Build the log viewer using webpack
        PATH="{{.G_NODEJS_22_BIN_DIR}}":$PATH npm run build -- \
          --output-path "{{.OUTPUT_DIR}}/yscope-log-viewer"

components/log-viewer-webui/client/src/api/query.ts (2)

9-16: Consider adding JSDoc comments to interface fields.

While the interface is well-structured, adding documentation for each field would improve code maintainability and make the purpose of each field clearer to other developers.

 interface ExtractStreamResp {
+    /** Unique identifier for the extraction job */
     _id: string;
+    /** Starting message index in the stream */
     begin_msg_ix: number;
+    /** Ending message index in the stream */
     end_msg_ix: number;
+    /** Indicates if this is the final chunk of the stream */
     is_last_chunk: boolean;
+    /** File path to the extracted stream */
     path: string;
+    /** Identifier of the stream being extracted */
     stream_id: string;
 }

---

19-44: Consider enhancing error handling and configuration.

Two suggestions for improvement:

1. Add explicit error handling with typed error responses
2. Consider making the API endpoint URL configurable

Example implementation:

interface ExtractStreamError {
    code: string;
    message: string;
}

const API_ENDPOINTS = {
    extractStream: '/query/extract-stream'
} as const;

const submitExtractStreamJob = async (
    extractJobType: QUERY_JOB_TYPE,
    streamId: string,
    logEventIdx: number,
    onUploadProgress: (progressEvent: AxiosProgressEvent) => void,
): Promise<AxiosResponse<ExtractStreamResp>> => {
    try {
        return await axios.post(
            API_ENDPOINTS.extractStream,
            {
                extractJobType,
                streamId,
                logEventIdx,
            },
            {onUploadProgress}
        );
    } catch (error) {
        if (axios.isAxiosError<ExtractStreamError>(error)) {
            // Handle specific API errors
            throw new Error(`Extract stream failed: ${error.response?.data.message}`);
        }
        throw error;
    }
};

components/log-viewer-webui/client/src/ui/Loading.tsx (5)

24-30: LGTM! Well-structured TypeScript interfaces.

The type definitions are clear and comprehensive. The use of the Nullable type for errorMsg shows good type reuse practices.

Consider making the props interfaces readonly to prevent accidental mutations:

-interface LoadingStepProps {
+interface LoadingStepProps {
+    readonly description: string;
+    readonly isActive: boolean;
+    readonly isError: boolean;
+    readonly label: string;
+    readonly stepIndicatorText: number | string;
 }

-interface LoadingProps {
+interface LoadingProps {
+    readonly currentState: QUERY_LOADING_STATE;
+    readonly errorMsg: Nullable<string>;
 }

Also applies to: 84-87

---

33-41: Enhance JSDoc documentation.

The JSDoc comments could be more descriptive and include return type information.

Consider updating the documentation:

 /**
- * Renders a step with a label and description.
+ * Renders a loading step with a label, description, and customizable indicator.
  *
  * @param props
  * @param props.description - Description text for the loading step
  * @param props.isActive - Whether this step is currently active
  * @param props.isError - Whether this step is in an error state
  * @param props.label - Label text for the loading step
  * @param props.stepIndicatorText - Text or number to display in the step indicator
- * @return
+ * @returns {JSX.Element} A styled step component
  */

---

Line range hint 50-57: Simplify color logic using ternary operator.

The color logic can be more concise while maintaining readability.

Consider this refactor:

-    let color: DefaultColorPalette = isActive ?
-        "primary" :
-        "neutral";
-
-    if (isError) {
-        color = "danger";
-    }
+    const color: DefaultColorPalette = isError ? "danger" : (isActive ? "primary" : "neutral");

---

131-134: Use TypeScript nullish checks.

Replace explicit null comparisons with TypeScript's nullish checking operator for better idiomatic TypeScript code.

Consider this refactor:

-                    determinate={null !== errorMsg}
-                    color={null === errorMsg ?
+                    determinate={!!errorMsg}
+                    color={errorMsg === null ?

---

101-102: Optimize steps array generation with useMemo.

The steps array is recreated on every render. Consider memoizing it to improve performance.

Consider this refactor:

-    const steps: React.ReactNode[] = [];
-    QUERY_LOADING_STATE_VALUES.forEach((state) => {
+    const steps = React.useMemo(() => {
+        const stepsArray: React.ReactNode[] = [];
+        QUERY_LOADING_STATE_VALUES.forEach((state) => {

Then wrap the entire steps generation logic in the useMemo hook with appropriate dependencies:

    }, [currentState, errorMsg]);

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

components/log-viewer-webui/client/src/typings/query.ts (2)

2-3: Consider reducing extra empty lines.

There are two consecutive empty lines after the import statement. For consistency, consider keeping just one empty line.

import {Type} from "@sinclair/typebox";

-

---

74-77: Fix capitalization in label text.

There's a minor capitalization inconsistency in the label text.

-            label: "Submitting query Job",
+            label: "Submitting query job",

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

Taskfile.yml (1)

270-278: Refactor build commands using a loop for clarity and consistency.
The updated for: loop that iterates over the client and yscope-log-viewer builds introduces explicit build flags ("--emptyOutDir --outDir" for the client and "--output-path" for yscope-log-viewer). Please confirm that these flags are precisely what is required for the respective build processes with Vite and that the YAML indentation meets the required syntax. This approach improves clarity in the build process.

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

components/log-viewer-webui/client/src/ui/QueryStatus.tsx (1)

79-80: ⚠️ Potential issue

Sanitize URL parameters to prevent XSS.

The URL construction should sanitize the parameters to prevent potential XSS attacks.

-                window.location.href = `/log-viewer/index.html?filePath=${data.path}` +
-                    `#logEventNum=${innerLogEventNum}`;
+                const sanitizeParam = (param: string): string => {
+                    return encodeURIComponent(param.replace(/[<>'"]/g, ''));
+                };
+                window.location.href = `/log-viewer/index.html?filePath=${sanitizeParam(data.path)}` +
+                    `#logEventNum=${sanitizeParam(String(innerLogEventNum))}`;

components/log-viewer-webui/client/src/ui/QueryStatus.tsx (2)

83-93: Simplify error message extraction from Axios errors.

The nested conditionals for extracting error messages can be simplified for better readability.

-                let msg = "Unknown error.";
-                if (isAxiosError<{message: string}>(e)) {
-                    msg = e.message;
-                    if ("undefined" !== typeof e.response) {
-                        if ("undefined" !== typeof e.response.data.message) {
-                            msg = e.response.data.message;
-                        } else {
-                            msg = e.response.statusText;
-                        }
-                    }
-                }
+                let msg = "Unknown error.";
+                if (isAxiosError<{message: string}>(e)) {
+                    if (e.response?.data?.message) {
+                        msg = e.response.data.message;
+                    } else if (e.response?.statusText) {
+                        msg = e.response.statusText;
+                    } else {
+                        msg = e.message;
+                    }
+                }

---

86-87: Use consistent null checks.

Following the coding guidelines, prefer using Yoda conditions for consistency.

-                    if ("undefined" !== typeof e.response) {
-                        if ("undefined" !== typeof e.response.data.message) {
+                    if ("undefined" !== typeof e.response) {
+                        if ("undefined" !== typeof e.response.data.message) {

Note: This is already using the guideline syntax. The alternative approach would be to use optional chaining as shown in the refactoring suggestion above, which would be even cleaner.

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

components/log-viewer-webui/client/src/ui/QueryStatus.tsx (1)

81-83: ⚠️ Potential issue

Sanitize URL parameters to prevent potential XSS.

The URL construction does not sanitize the parameters, which could potentially lead to XSS vulnerabilities.

Consider sanitizing the parameters before constructing the URL:

- const innerLogEventNum = parseResult.logEventIdx - data.begin_msg_ix + 1;
- window.location.href = `/log-viewer/index.html?filePath=${data.path}` +
-     `#logEventNum=${innerLogEventNum}`;
+ const innerLogEventNum = parseResult.logEventIdx - data.begin_msg_ix + 1;
+ // Sanitize parameters to prevent XSS
+ const sanitizedPath = encodeURIComponent(data.path);
+ const sanitizedLogEventNum = encodeURIComponent(String(innerLogEventNum));
+ window.location.href = `/log-viewer/index.html?filePath=${sanitizedPath}` +
+     `#logEventNum=${sanitizedLogEventNum}`;

components/log-viewer-webui/client/src/ui/QueryStatus.tsx (1)

67-77: Type assertion could be improved for better type safety.

The type assertion on line 71 is necessary but could potentially be handled in a more type-safe manner.

Consider using a type guard or validation step to ensure type safety without using the as keyword:

- EXTRACT_JOB_TYPE[parseResult.type as keyof typeof EXTRACT_JOB_TYPE],
+ // Verify that parseResult.type is a valid key in EXTRACT_JOB_TYPE
+ ((type: string): number => {
+   if (!(type in EXTRACT_JOB_TYPE)) {
+     throw new Error(`Invalid extract job type: ${type}`);
+   }
+   return EXTRACT_JOB_TYPE[type as keyof typeof EXTRACT_JOB_TYPE];
+ })(parseResult.type),

Alternatively, if you're confident the validation from TypeBox ensures parseResult.type will always be valid:

- // `parseResult.type` must be valid key since parsed using with typebox type
- // `ExtractJobSearchParams`.
- EXTRACT_JOB_TYPE[parseResult.type as keyof typeof EXTRACT_JOB_TYPE],
+ // TypeBox validation ensures parseResult.type is valid
+ EXTRACT_JOB_TYPE[parseResult.type as keyof typeof EXTRACT_JOB_TYPE],

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro