feat(webui): Highlight Presto SQL syntax errors for guided UI inputs with ANTLR. #1533
Conversation
WalkthroughAdds client-side SQL validation: SqlInput gains a validateFn prop and editor ref to apply Monaco markers; sql-parser now exposes three validators returning ValidationError[] (select, boolean, sort); Select, Where, and OrderBy components pass the appropriate validators into SqlInput. Changes
Sequence Diagram(s)sequenceDiagram
participant User as User
participant SqlInput as SqlInput (wrapper)
participant Editor as Monaco Editor (model)
participant Validator as Validator Function
User->>SqlInput: Type or update SQL text
SqlInput->>SqlInput: useEffect detects value change
SqlInput->>Editor: Get model & current text
alt validateFn present and text non-empty
SqlInput->>Validator: validateFn(sqlString)
Validator-->>SqlInput: ValidationError[]
SqlInput->>Editor: Map errors to Monaco markers and setModelMarkers
else no validateFn or empty text
SqlInput->>Editor: Clear Monaco markers
end
Editor-->>User: Render diagnostics/markers
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~30 minutes
Pre-merge checks and finishing touches✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: ASSERTIVE Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used📓 Path-based instructions (1)**/*.{cpp,hpp,java,js,jsx,tpp,ts,tsx}⚙️ CodeRabbit configuration file
Files:
🧠 Learnings (1)📚 Learning: 2025-06-09T17:48:56.024ZApplied to files:
🧬 Code graph analysis (1)components/webui/client/src/components/SqlInput/index.tsx (3)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
🔇 Additional comments (5)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![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
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (2)
components/webui/client/src/sql-parser/generated/SqlLexer.tsis excluded by!**/generated/**components/webui/client/src/sql-parser/generated/SqlParser.tsis excluded by!**/generated/**
📒 Files selected for processing (6)
components/webui/client/src/components/SqlInput/index.tsx(3 hunks)components/webui/client/src/pages/SearchPage/SearchControls/Presto/GuidedControls/OrderBy.tsx(2 hunks)components/webui/client/src/pages/SearchPage/SearchControls/Presto/GuidedControls/Select.tsx(2 hunks)components/webui/client/src/pages/SearchPage/SearchControls/Presto/GuidedControls/Where.tsx(2 hunks)components/webui/client/src/sql-parser/Sql.g4(1 hunks)components/webui/client/src/sql-parser/index.ts(4 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{cpp,hpp,java,js,jsx,tpp,ts,tsx}
⚙️ CodeRabbit configuration file
- Prefer
false == <expression>rather than!<expression>.
Files:
components/webui/client/src/pages/SearchPage/SearchControls/Presto/GuidedControls/Where.tsxcomponents/webui/client/src/components/SqlInput/index.tsxcomponents/webui/client/src/pages/SearchPage/SearchControls/Presto/GuidedControls/OrderBy.tsxcomponents/webui/client/src/pages/SearchPage/SearchControls/Presto/GuidedControls/Select.tsxcomponents/webui/client/src/sql-parser/index.ts
🧬 Code graph analysis (3)
components/webui/client/src/components/SqlInput/index.tsx (2)
components/webui/client/src/components/SqlEditor/index.tsx (2)
SqlEditorProps(144-144)SqlEditorType(145-145)components/webui/client/src/sql-parser/index.ts (1)
ValidationError(249-249)
components/webui/client/src/pages/SearchPage/SearchControls/Presto/GuidedControls/OrderBy.tsx (1)
components/webui/client/src/sql-parser/index.ts (1)
validateSortItemList(243-243)
components/webui/client/src/sql-parser/index.ts (1)
components/webui/client/src/sql-parser/generated/SqlParser.ts (1)
SqlParser(19-11121)
🪛 GitHub Actions: clp-lint
components/webui/client/src/sql-parser/index.ts
[error] 8-8: ESLint: 'Token' is defined but never used. (no-unused-vars)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: package-image
🔇 Additional comments (13)
components/webui/client/src/pages/SearchPage/SearchControls/Presto/GuidedControls/OrderBy.tsx (2)
3-3: LGTM!The import is correctly sourced and aligns with the new validation architecture.
28-28: LGTM!The validator is correctly wired into SqlInput, enabling client-side validation for ORDER BY expressions.
components/webui/client/src/pages/SearchPage/SearchControls/Presto/GuidedControls/Where.tsx (2)
3-3: LGTM!The import is correctly sourced from the sql-parser module.
28-28: LGTM!The validator integration enables real-time syntax validation for WHERE clause expressions.
components/webui/client/src/pages/SearchPage/SearchControls/Presto/GuidedControls/Select.tsx (2)
3-3: LGTM!The import is correctly sourced and follows the established pattern.
28-28: LGTM!The validator correctly enables syntax validation for SELECT item lists.
components/webui/client/src/sql-parser/Sql.g4 (1)
5-23: LGTM!The grammar rules correctly define standalone parsing contexts for SELECT items, boolean expressions, and sort items. The EOF markers ensure complete input validation, which is essential for guided UI controls.
components/webui/client/src/components/SqlInput/index.tsx (3)
2-10: LGTM!The additional imports support the validation feature correctly.
17-22: LGTM!The type definition is well-structured and properly documented.
30-60: LGTM!The editor reference management is correctly implemented for validation purposes.
components/webui/client/src/sql-parser/index.ts (3)
22-28: LGTM!The
ValidationErrorinterface is well-structured and provides all necessary fields for editor markers.
42-48: Verify column calculations for Monaco compatibility.The column calculation logic uses 1-based indexing, which is correct for Monaco editor. However, ensure that
e.offendingToken.startande.offendingToken.stoprepresent character positions (not token indices) and that the+1and+2adjustments produce the expected highlighting ranges in the editor.
87-136: LGTM!The
setupParserhelper and three validator functions are well-structured and follow a consistent pattern. The validators correctly invoke the corresponding ANTLR grammar rules for standalone expression validation.
|
still dependent on previous PR being merged after release, but I think @hoophalab u can start looking at |
hoophalab
left a comment
hoophalab
left a comment
There was a problem hiding this comment.
lgtm. some cosmetic comments.
Validation: If a syntax error is detectable by the grammar checker, a red marker will appear in the SQL input field.
Co-authored-by: hoophalab <[email protected]>
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/components/SqlInput/index.tsx(3 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{cpp,hpp,java,js,jsx,tpp,ts,tsx}
⚙️ CodeRabbit configuration file
- Prefer
false == <expression>rather than!<expression>.
Files:
components/webui/client/src/components/SqlInput/index.tsx
🧠 Learnings (1)
📚 Learning: 2025-06-09T17:48:56.024Z
Learnt from: junhaoliao
Repo: y-scope/clp PR: 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.
Applied to files:
components/webui/client/src/components/SqlInput/index.tsx
🧬 Code graph analysis (1)
components/webui/client/src/components/SqlInput/index.tsx (3)
components/webui/client/src/components/SqlEditor/index.tsx (2)
SqlEditorProps(144-144)SqlEditorType(145-145)components/webui/client/src/sql-parser/index.ts (1)
ValidationError(254-254)components/webui/common/src/utility-types.ts (1)
Nullable(3-3)
🪛 GitHub Actions: clp-lint
components/webui/client/src/components/SqlInput/index.tsx
[warning] 2-2: Run autofix to sort these imports! simple-import-sort/imports
[error] 8-8: There should be no space after '{' @stylistic/object-curly-spacing
[error] 8-8: There should be no space before '}' @stylistic/object-curly-spacing
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: package-image
🔇 Additional comments (3)
components/webui/client/src/components/SqlInput/index.tsx (3)
19-24: LGTM!The type definition is well-structured with clear documentation. The optional
validateFnprop maintains backward compatibility while enabling the new validation feature.
32-34: LGTM!The props destructuring cleanly separates the validation function from editor props, and the ref is properly typed with
Nullable<SqlEditorType>.
138-138: LGTM!Exporting
SqlInputPropsallows consumers to properly type their components when using thevalidateFnprop.
| import { | ||
| useCallback, | ||
| useEffect, | ||
| useRef, | ||
| } from "react"; |
There was a problem hiding this comment.
Fix import sorting to resolve pipeline failure.
The pipeline reports an import sorting violation that must be resolved before merge.
Run the autofix command to sort the imports:
npm run lint:fixOr manually reorder if needed.
🧰 Tools
🪛 GitHub Actions: clp-lint
[warning] 2-2: Run autofix to sort these imports! simple-import-sort/imports
🤖 Prompt for AI Agents
In components/webui/client/src/components/SqlInput/index.tsx around lines 2 to
6, import statements are not sorted which triggers the lint pipeline; run the
autofix to reorder imports (npm run lint:fix) or manually reorder the imports to
match the project's ESLint/organize-imports rules (group external packages, then
local imports, alphabetize within groups), save the file, and commit the change
to clear the import-sorting violation.
| // Validate SQL and update markers whenever value changes | ||
| useEffect(() => { | ||
| const editor = editorRef.current; | ||
| if (null === editor) { | ||
| return; | ||
| } | ||
|
|
||
| const model = editor.getModel(); | ||
| if (null === model) { | ||
| return; | ||
| } | ||
|
|
||
| const value = editorProps.value ?? ""; | ||
|
|
||
| // Clear markers if no validation function or empty/whitespace-only input | ||
| if ("undefined" === typeof validateFn || "" === value.trim()) { | ||
| monaco.editor.setModelMarkers(model, "sql-parser", []); | ||
|
|
||
| return; | ||
| } | ||
|
|
||
| const errors = validateFn(value); | ||
| const markers: monaco.editor.IMarkerData[] = errors.map((error) => ({ | ||
| endColumn: error.endColumn, | ||
| endLineNumber: error.line, | ||
| message: error.message, | ||
| severity: monaco.MarkerSeverity.Error, | ||
| startColumn: error.startColumn, | ||
| startLineNumber: error.line, | ||
| })); | ||
|
|
||
| monaco.editor.setModelMarkers(model, "sql-parser", markers); | ||
| }, [editorProps.value, | ||
| validateFn]); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
LGTM with optional error-handling suggestion.
The validation logic correctly applies Monaco markers and follows the coding guidelines with explicit comparisons. The useEffect dependencies are accurate.
Optionally, consider wrapping the validator call in a try-catch to handle unexpected parse errors gracefully:
const errors = validateFn(value);
+ try {
+ errors = validateFn(value);
+ } catch (e) {
+ console.error("Validation failed:", e);
+ return;
+ }
const markers: monaco.editor.IMarkerData[] = errors.map((error) => ({This would prevent the component from crashing if the validator throws an unexpected error.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In components/webui/client/src/components/SqlInput/index.tsx around lines 64 to
97, the call to validateFn(value) is not guarded against runtime exceptions
which could crash the component; wrap the validator invocation in a try-catch,
on error log or report the exception (e.g., console.error or a telemetry call),
clear any existing markers via monaco.editor.setModelMarkers(model,
"sql-parser", []) and return so the component remains stable, and keep the
existing marker-setting logic in the try block for successful validations.
junhaoliao
left a comment
junhaoliao
left a comment
There was a problem hiding this comment.
deferring to @hoophalab 's review
3 participants
Description
Adds monaco markers to single line editor. Markers are generated using ANTLR grammar. Errors show up as red underline for the user. Grammar is different for each SQL field
Checklist
breaking change.
Validation performed
Red lines show up under parsing error
Summary by CodeRabbit