Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

Learnt from: haiqi96
PR: y-scope/clp#0
File: :0-0
Timestamp: 2025-07-29T14:04:13.769Z
Learning: User haiqi96 requested creating a GitHub issue to document a bug fix from PR #1136, which addressed MySQL compatibility issues with invalid SQL CAST operations in the WebUI component.

components/webui/client/src/sql-parser/index.ts (3)

67-75: JSDoc should reference the SQL‑specific error type.

- * @throws {SyntaxError} with line, column, and message details if a syntax error is found.
+ * @throws {SqlSyntaxError} with line, column, and message details if a syntax error is found.

---

125-129: Update exports to use the renamed error type.

 export {
     buildSearchQuery,
-    SyntaxError,
+    SqlSyntaxError,
     validate,
 };

---

85-97: Keep original SQL syntax error type when rethrowing; update JSDoc.

Preserve SqlSyntaxError so callers can distinguish parse failures; fix the @throws tag accordingly.

- * @throws {SyntaxError} if any of the input is not valid.
+ * @throws {SqlSyntaxError} if any of the input is not valid.
@@
     try {
         validate(sqlString);
     } catch (err: unknown) {
-        throw new Error(`The constructed SQL is not valid: ${sqlString}`, {cause: err});
+        if (err instanceof SqlSyntaxError) {
+            throw err;
+        }
+        throw new Error(`The constructed SQL is not valid: ${sqlString}`, {cause: err});
     }

components/webui/client/src/pages/SearchPage/SearchControls/Presto/BuildSqlTestingInputs.tsx (2)

59-69: Associate labels with inputs and constrain LIMIT to numeric.

Fixes a11y violations and reduces invalid input.

-            <label>select:</label>
-            <input name={"selectItemList"}/>
-            <label>from:</label>
-            <input name={"relationList"}/>
-            <label>where:</label>
-            <input name={"booleanExpression"}/>
-            <label>order:</label>
-            <input name={"sortItemList"}/>
-            <label>limit:</label>
-            <input name={"limitValue"}/>
+            <label>
+                select:
+                <input name="selectItemList" placeholder="col1, col2, t.*" />
+            </label>
+            <label>
+                from:
+                <input name="relationList" placeholder="table1 t, table2" />
+            </label>
+            <label>
+                where:
+                <input name="booleanExpression" placeholder="t.col = 1 AND ..." />
+            </label>
+            <label>
+                order:
+                <input name="sortItemList" placeholder="col1 DESC, col2" />
+            </label>
+            <label>
+                limit:
+                <input name="limitValue" type="number" inputMode="numeric" min="0" />
+            </label>

---

46-57: Guard submit with try/catch and gate debug logs.

Unhandled exceptions from parsing crash the UI; keep console noise out of production.

             onSubmit={(ev: FormEvent<HTMLFormElement>) => {
                 ev.preventDefault();
-                const formData = new FormData(ev.target as HTMLFormElement);
-                const props: Static<typeof BuildSearchQueryPropsSchema> = Value.Parse(
-                    BuildSearchQueryPropsSchema,
-                    Object.fromEntries(formData),
-                );
-
-                const sqlString = buildSearchQuery(props);
-                console.log(`SQL: ${sqlString}`);
-                handlePrestoQuerySubmit({queryString: sqlString});
+                try {
+                    const formData = new FormData(ev.target as HTMLFormElement);
+                    const props: Static<typeof BuildSearchQueryPropsSchema> = Value.Parse(
+                        BuildSearchQueryPropsSchema,
+                        Object.fromEntries(formData),
+                    );
+                    const sqlString = buildSearchQuery(props);
+                    if ("production" !== process.env.NODE_ENV) {
+                        // eslint-disable-next-line no-console
+                        console.log(`SQL: ${sqlString}`);
+                    }
+                    handlePrestoQuerySubmit({queryString: sqlString});
+                } catch (err) {
+                    // eslint-disable-next-line no-console
+                    console.error(err);
+                    alert("Invalid SQL fragments. Please check your inputs.");
+                }
             }}

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

components/webui/client/src/sql-parser/index.ts (4)

115-119: Preserve the specific SQL error type when rethrowing.

Let callers catch SqlSyntaxError reliably; wrap only non‑syntax errors.

-    } catch (err: unknown) {
-        throw new Error(`The constructed SQL is not valid: ${sqlString}`, {cause: err});
-    }
+    } catch (err: unknown) {
+        if (err instanceof SqlSyntaxError) throw err;
+        throw new Error(`The constructed SQL is not valid: ${sqlString}`, {cause: err});
+    }

---

67-76: JSDoc must reference the renamed error type.

Same as above; update the @throws tag to SqlSyntaxError.

---

105-113: Prefer direct undefined checks over typeof string comparisons.

Cleaner TS; no behaviour change.

-    if ("undefined" !== typeof booleanExpression) {
+    if (undefined !== booleanExpression) {
@@
-    if ("undefined" !== typeof sortItemList) {
+    if (undefined !== sortItemList) {
@@
-    if ("undefined" !== typeof limitValue) {
+    if (undefined !== limitValue) {

---

104-113: Defensively reject empty strings for optional fragments (enforce API contract).

If someone bypasses the UI schema, "" will emit broken clauses. Guard early.

-    if ("undefined" !== typeof booleanExpression) {
-        sqlString += ` WHERE ${booleanExpression}`;
-    }
-    if ("undefined" !== typeof sortItemList) {
-        sqlString += ` ORDER BY ${sortItemList}`;
-    }
-    if ("undefined" !== typeof limitValue) {
-        sqlString += ` LIMIT ${limitValue}`;
-    }
+    if (undefined !== booleanExpression) {
+        if ("" === booleanExpression) throw new Error("booleanExpression must be undefined or a valid fragment");
+        sqlString += ` WHERE ${booleanExpression}`;
+    }
+    if (undefined !== sortItemList) {
+        if ("" === sortItemList) throw new Error("sortItemList must be undefined or a valid fragment");
+        sqlString += ` ORDER BY ${sortItemList}`;
+    }
+    if (undefined !== limitValue) {
+        if ("" === limitValue) throw new Error("limitValue must be undefined or a valid fragment");
+        sqlString += ` LIMIT ${limitValue}`;
+    }

components/webui/client/src/pages/SearchPage/SearchControls/Presto/BuildSqlTestingInputs.tsx (2)

60-71: Associate labels with inputs and constrain LIMIT to numeric.

Fixes a11y errors (Biome) and prevents non‑numeric limit.

-            <label>select:</label>
-            <input name={"selectItemList"}/>
-            <label>from:</label>
-            <input name={"relationList"}/>
-            <label>where:</label>
-            <input name={"booleanExpression"}/>
-            <label>order:</label>
-            <input name={"sortItemList"}/>
-            <label>limit:</label>
-            <input name={"limitValue"}/>
+            <label>
+                select:
+                <input name="selectItemList" placeholder="col1, col2, t.*" required />
+            </label>
+            <label>
+                from:
+                <input name="relationList" placeholder="table1 t, table2" required />
+            </label>
+            <label>
+                where:
+                <input name="booleanExpression" placeholder="t.col = 1 AND ..." />
+            </label>
+            <label>
+                order:
+                <input name="sortItemList" placeholder="col1 DESC, col2" />
+            </label>
+            <label>
+                limit:
+                <input name="limitValue" type="number" inputMode="numeric" min="1" step="1" />
+            </label>

---

47-58: Wrap parse/build in try/catch; don’t crash the UI on invalid input.

Uncaught exceptions here will break the page.

-            onSubmit={(ev: FormEvent<HTMLFormElement>) => {
+            onSubmit={(ev: FormEvent<HTMLFormElement>) => {
                 ev.preventDefault();
-                const formData = new FormData(ev.target as HTMLFormElement);
-                const props: Static<typeof BuildSearchQueryPropsSchema> = Value.Parse(
-                    BuildSearchQueryPropsSchema,
-                    Object.fromEntries(formData),
-                );
-
-                const sqlString = buildSearchQuery(props);
-                console.log(`SQL: ${sqlString}`);
-                handlePrestoQuerySubmit({queryString: sqlString});
+                try {
+                    const formData = new FormData(ev.target as HTMLFormElement);
+                    const props = Value.Parse(
+                        BuildSearchQueryPropsSchema,
+                        Object.fromEntries(formData),
+                    );
+                    const sqlString = buildSearchQuery(props);
+                    if ("production" !== process.env.NODE_ENV) {
+                        // eslint-disable-next-line no-console
+                        console.log(`SQL: ${sqlString}`);
+                    }
+                    handlePrestoQuerySubmit({queryString: sqlString});
+                } catch (err) {
+                    // eslint-disable-next-line no-console
+                    console.error(err);
+                    alert("Invalid SQL fragments. Please check your inputs.");
+                }
             }}

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

Learnt from: hoophalab
PR: y-scope/clp#1304
File: components/webui/client/src/sql-parser/index.ts:98-115
Timestamp: 2025-09-17T22:51:15.765Z
Learning: In the CLP webui codebase, when designing APIs that accept optional string parameters, developers should omit the value (pass undefined) rather than pass empty strings. The API contract should be clear that empty strings are not valid inputs - only undefined should be used to indicate omission of optional clauses.

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.

components/webui/client/src/pages/SearchPage/SearchControls/Presto/BuildSqlTestingInputs.tsx (4)

47-58: Catch and surface parse/build errors; don’t crash the UI.

Wrap Value.Parse/buildSearchQuery in try/catch, dev‑gate logs, and bail out on error. This prevents UX breakage on invalid fragments.

         <form
-            onSubmit={(ev: FormEvent<HTMLFormElement>) => {
+            onSubmit={(ev: FormEvent<HTMLFormElement>) => {
                 ev.preventDefault();
                 const formData = new FormData(ev.target as HTMLFormElement);
-                const props: Static<typeof BuildSearchQueryPropsSchema> = Value.Parse(
-                    BuildSearchQueryPropsSchema,
-                    Object.fromEntries(formData),
-                );
-
-                const sqlString = buildSearchQuery(props);
-                console.log(`SQL: ${sqlString}`);
-                handlePrestoQuerySubmit({queryString: sqlString});
+                try {
+                    const props = Value.Parse(
+                        BuildSearchQueryPropsSchema,
+                        Object.fromEntries(formData),
+                    );
+                    const sqlString = buildSearchQuery(props);
+                    if ("production" !== process.env.NODE_ENV) {
+                        console.log(`SQL: ${sqlString}`);
+                    }
+                    handlePrestoQuerySubmit({queryString: sqlString});
+                } catch (err) {
+                    console.error(err);
+                    alert("Invalid SQL fragments. Please check your inputs.");
+                }
             }}
         >

---

9-11: Type with the exported API type; drop Static to prevent drift.

Keep UI strictly aligned with the parser’s public surface.

-import {
-    Static,
-    Type,
-} from "@sinclair/typebox";
+import {Type} from "@sinclair/typebox";
 import {Value} from "@sinclair/typebox/value";
 
-import {buildSearchQuery} from "../../../../sql-parser";
+import {buildSearchQuery, type BuildSearchQueryProps} from "../../../../sql-parser";
@@
-                const props: Static<typeof BuildSearchQueryPropsSchema> = Value.Parse(
+                const props: BuildSearchQueryProps = Value.Parse(
                     BuildSearchQueryPropsSchema,
                     Object.fromEntries(formData),
                 );

Also applies to: 50-53, 3-7

---

60-71: Fix a11y label association; constrain limit as numeric; mark required fields.

Resolves Biome errors and improves UX.

-            <label>select:</label>
-            <input name={"selectItemList"}/>
-            <label>from:</label>
-            <input name={"relationList"}/>
-            <label>where:</label>
-            <input name={"booleanExpression"}/>
-            <label>order:</label>
-            <input name={"sortItemList"}/>
-            <label>limit:</label>
-            <input name={"limitValue"}/>
+            <label>
+                select:
+                <input name="selectItemList" placeholder="col1, col2, t.*" required />
+            </label>
+            <label>
+                from:
+                <input name="relationList" placeholder="table1 t, table2" required />
+            </label>
+            <label>
+                where:
+                <input name="booleanExpression" placeholder="t.col = 1 AND ..." />
+            </label>
+            <label>
+                order:
+                <input name="sortItemList" placeholder="col1 DESC, col2" />
+            </label>
+            <label>
+                limit:
+                <input name="limitValue" type="number" inputMode="numeric" min="0" step="1" />
+            </label>

---

39-44: Tighten JSDoc phrasing and return tag.

 /**
- * Returns input boxes to test `buildSearchQuery`.
+ * Renders input boxes to test `buildSearchQuery`.
  *
- * @return
+ * @returns JSX.Element
  */

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

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.