-
Notifications
You must be signed in to change notification settings - Fork 83
feat(webui): Add hover messages for ANTLR parsing errors in Monaco single-line editor. #1569
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Co-authored-by: hoophalab <[email protected]>
WalkthroughThis pull request adds SQL validation to the SqlInput component with inline error visualization in the Monaco editor. It introduces a ValidationError interface, refactors error handling in the sql-parser to accumulate errors, and wires validation functions (validateSelectItemList, validateBooleanExpression, validateSortItemList) to SqlInput components via a new validateFn prop. Monaco hover features are enabled to display error messages. Changes
Sequence DiagramsequenceDiagram
participant User
participant SqlInput
participant Validator
participant Editor
participant Monaco
User->>SqlInput: Input SQL text
SqlInput->>Validator: Call validateFn(sqlString)
Validator->>Validator: Parse and accumulate errors
Validator-->>SqlInput: Return ValidationError[]
SqlInput->>Editor: Transform errors to decorations
Editor->>Monaco: Apply decorations (squiggly underlines)
Monaco-->>User: Display inline errors with hover messages
User->>Monaco: Hover over error
Monaco-->>User: Show escaped error message tooltip
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20–25 minutes
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (7)
components/webui/client/src/components/SqlEditor/monaco-loader.ts(1 hunks)components/webui/client/src/components/SqlInput/index.tsx(4 hunks)components/webui/client/src/components/SqlInput/utils.ts(1 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/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/sql-parser/index.tscomponents/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/components/SqlEditor/monaco-loader.tscomponents/webui/client/src/components/SqlInput/utils.tscomponents/webui/client/src/pages/SearchPage/SearchControls/Presto/GuidedControls/Select.tsx
🧠 Learnings (2)
📚 Learning: 2025-07-21T14:37:57.648Z
Learnt from: davemarco
Repo: y-scope/clp PR: 1108
File: components/webui/client/src/components/SqlEditor/monaco-config.ts:6-8
Timestamp: 2025-07-21T14:37:57.648Z
Learning: In Monaco Editor configurations for SQL editing, the import of "monaco-editor/esm/vs/editor/editor.all.js" is required for SQL autocomplete functionality to work properly, even when using "editor.api" and "sql.contribution.js". Removing "editor.all.js" breaks the autocomplete feature.
Applied to files:
components/webui/client/src/components/SqlEditor/monaco-loader.ts
📚 Learning: 2025-08-26T13:45:46.445Z
Learnt from: davemarco
Repo: y-scope/clp PR: 1244
File: components/webui/client/package.json:29-29
Timestamp: 2025-08-26T13:45:46.445Z
Learning: The "color" package in components/webui/client/package.json is used in the SqlEditor component to convert Ant Design token colors to hex format for Monaco editor themes, specifically for the disabled state styling.
Applied to files:
components/webui/client/src/components/SqlEditor/monaco-loader.ts
🧬 Code graph analysis (5)
components/webui/client/src/sql-parser/index.ts (2)
components/webui/client/src/sql-parser/generated/SqlLexer.ts (1)
SqlLexer(15-1482)components/webui/client/src/sql-parser/generated/SqlParser.ts (1)
SqlParser(19-11121)
components/webui/client/src/pages/SearchPage/SearchControls/Presto/GuidedControls/Where.tsx (1)
components/webui/client/src/sql-parser/index.ts (1)
validateBooleanExpression(241-241)
components/webui/client/src/components/SqlInput/index.tsx (4)
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/common/src/utility-types.ts (1)
Nullable(3-3)components/webui/client/src/components/SqlInput/utils.ts (1)
escapeHoverMarkdown(13-13)
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/pages/SearchPage/SearchControls/Presto/GuidedControls/Select.tsx (1)
components/webui/client/src/sql-parser/index.ts (1)
validateSelectItemList(242-242)
⏰ 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)
- GitHub Check: build (macos-15)
- GitHub Check: lint-check (macos-15)
- GitHub Check: lint-check (ubuntu-24.04)
🔇 Additional comments (9)
components/webui/client/src/components/SqlEditor/monaco-loader.ts (1)
7-8: LGTM! Hover imports correctly added.These imports enable the hover tooltip functionality needed for displaying validation error messages in the Monaco editor.
components/webui/client/src/pages/SearchPage/SearchControls/Presto/GuidedControls/Where.tsx (1)
3-3: LGTM! Boolean expression validation correctly integrated.The WHERE clause now validates user input as a boolean expression, providing inline error feedback.
Also applies to: 28-28
components/webui/client/src/components/SqlInput/utils.ts (1)
1-13: LGTM! Hover text escaping correctly implemented.The function correctly escapes angle brackets to prevent Monaco from interpreting them as HTML tags in hover tooltips, which is essential for displaying syntax error tokens like
<EOF>.components/webui/client/src/components/SqlInput/index.tsx (4)
19-24: LGTM! SqlInputProps correctly extends SqlEditorProps.The new
SqlInputPropstype properly adds the optionalvalidateFnprop while preserving all existingSqlEditorPropsfunctionality.
34-35: LGTM! Refs correctly typed as Nullable.Both refs are properly typed to handle the initial null state before the editor is ready.
66-108: Validation effect correctly implemented.The useEffect properly:
- Waits for editor initialization
- Lazily creates the decorations collection
- Clears decorations when validator is absent or input is empty
- Maps validation errors to Monaco decorations with hover messages
- Uses correct dependencies
The implementation ensures error highlighting and hover tooltips work seamlessly.
132-132: Good addition: disabling quick suggestions for single-line input.Setting
quickSuggestions: falseprevents Monaco from showing autocomplete dropdowns while typing, which is appropriate for a single-line constrained input where the validation decorations are the primary feedback mechanism.components/webui/client/src/pages/SearchPage/SearchControls/Presto/GuidedControls/Select.tsx (1)
3-3: LGTM! Select item list validation correctly integrated.The SELECT clause now validates user input as a select item list, providing inline error feedback for syntax issues.
Also applies to: 28-28
components/webui/client/src/pages/SearchPage/SearchControls/Presto/GuidedControls/OrderBy.tsx (1)
3-3: LGTM! Sort item list validation correctly integrated.The ORDER BY clause now validates user input as a sort item list, providing inline error feedback for syntax issues.
Also applies to: 28-28
| const token = offendingSymbol as unknown as Token; | ||
| const startColumn = token.start + 1; | ||
| const endColumn = token.stop + 2; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
Verify type safety of Token cast and properties.
The cast offendingSymbol as unknown as Token and subsequent access to token.start and token.stop could fail at runtime if the offending symbol is not actually a Token or lacks these properties. ANTLR4's error listener can receive different symbol types depending on the error context.
Consider adding runtime checks:
- const token = offendingSymbol as unknown as Token;
- const startColumn = token.start + 1;
- const endColumn = token.stop + 2;
+ const token = offendingSymbol as unknown as Token;
+ const startColumn = token?.start !== undefined ? token.start + 1 : column;
+ const endColumn = token?.stop !== undefined ? token.stop + 2 : column + 1;Alternatively, verify with the ANTLR4 documentation that offendingSymbol is always a Token for lexer/parser errors and document this assumption.
I'll verify this review comment by examining the code context and investigating ANTLR4's error listener specifications.
---
🏁 Script executed:
```shell
#!/bin/bash
# Inspect the file around lines 40-42 to understand the context
cat -n components/webui/client/src/sql-parser/index.ts | sed -n '1,60p'
Length of output: 1806
🌐 Web query:
ANTLR4 error listener offendingSymbol type Token properties
💡 Result:
offendingSymbol (the Token passed to ANTLRErrorListener.syntaxError) implements org.antlr.v4.runtime.Token and exposes these useful properties/methods:
- getText() — token text. [1]
- getType() — token type (int). [1]
- getLine() — 1-based line number. [1]
- getCharPositionInLine() — char offset in line (0-based). [1]
- getStartIndex() / getStopIndex() — start/end char indexes in input (may return -1 if not available). [1]
- getTokenIndex() — index of token in token stream (or -1 if synthetic). [1]
- getChannel() — token channel (DEFAULT/HIDDEN etc.). [1]
- getTokenSource() / getInputStream() — source lexer/token stream and underlying CharStream. [1]
In an error listener you typically do:
public void syntaxError(Recognizer r, Object offendingSymbol, int line, int pos, String msg, RecognitionException e) {
Token t = (Token) offendingSymbol;
String text = t.getText();
int type = t.getType();
int start = t.getStartIndex();
int stop = t.getStopIndex();
int tokenIndex = t.getTokenIndex();
// ...
}
The listener contract and the meaning of offendingSymbol are documented on the ANTLR error-listener API. [2]
Sources: ANTLR Token API (methods listed above). [1] ANTLR error-listener / BaseErrorListener docs (offendingSymbol description). [2]
I need to verify the JavaScript/TypeScript ANTLR4 Token interface, as the web search returned Java documentation. Let me investigate further.
---
🏁 Script executed:
```shell
#!/bin/bash
# Search for Token interface definition or usage patterns in the codebase
rg -n "Token\." components/webui/client/src/sql-parser/ -A2 -B2
Length of output: 1108
🏁 Script executed:
#!/bin/bash
# Check if there are other error listener implementations or Token property accesses
rg -n "\.start|\.stop" components/webui/client/src/sql-parser/ -B3 -A1Length of output: 3932
🏁 Script executed:
#!/bin/bash
# Look for package.json to see antlr4 version
find components/webui/client -name "package.json" -exec cat {} \; | grep -A5 -B5 antlr4 || echo "antlr4 not in found package.json"Length of output: 350
🌐 Web query:
antlr4 npm JavaScript Token interface properties start stop
💡 Result:
Short answer: In the JS/TypeScript ANTLR runtimes tokens expose startIndex and stopIndex (character offsets); parser contexts expose start and stop Token objects. Use token.startIndex / token.stopIndex for character positions and ctx.start / ctx.stop for the tokens that bracket a rule. [1][2][3]
Sources
- CommonToken / Token definitions in the antlr4/ANTLR4‑TS TypeScript typings (start/stop → startIndex/stopIndex). [1]
- ParserRuleContext API: contexts have start and stop Token properties. [2]
- Note: rule $stop may be null in embedded actions (see discussion). [3]
Use correct Token property names: startIndex and stopIndex.
The review comment correctly identifies a type safety concern, but the underlying issue is more critical than suggested. In the JavaScript ANTLR4 runtime (version 4.13.2), Token objects expose startIndex and stopIndex properties, not start and stop. The current code will fail at runtime attempting to access undefined properties.
Update lines 41–42 to use:
const startColumn = token.startIndex + 1;
const endColumn = token.stopIndex + 2;The double cast bypasses TypeScript but does not prevent the runtime failure. No optional chaining fallback is needed if you correct the property names.
🤖 Prompt for AI Agents
In components/webui/client/src/sql-parser/index.ts around lines 40 to 42, the
code casts offendingSymbol to Token and accesses non-existent properties
token.start and token.stop which will be undefined at runtime; replace those
property accesses with the correct ANTLR Token properties token.startIndex and
token.stopIndex and remove the unnecessary double-cast so that you compute
startColumn as token.startIndex + 1 and endColumn as token.stopIndex + 2.
| const validateSelectItemList = (sqlString: string): ValidationError[] => { | ||
| const {parser, syntaxErrorListener} = setupParser(sqlString); | ||
| parser.standaloneSelectItemList(); | ||
|
|
||
| return syntaxErrorListener.errors; | ||
| }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹 Nitpick | 🔵 Trivial
Consider adding error handling to validation functions.
The validation functions don't have try-catch blocks. If the parser encounters an unexpected error (beyond syntax errors), it will propagate to the caller and potentially crash the UI.
Consider wrapping validation logic:
const validateSelectItemList = (sqlString: string): ValidationError[] => {
+ try {
const {parser, syntaxErrorListener} = setupParser(sqlString);
parser.standaloneSelectItemList();
return syntaxErrorListener.errors;
+ } catch (error) {
+ console.error("Unexpected validation error:", error);
+ return [{
+ line: 1,
+ column: 0,
+ startColumn: 0,
+ endColumn: sqlString.length,
+ message: "Unexpected validation error occurred"
+ }];
+ }
};Apply similar pattern to validateBooleanExpression and validateSortItemList.
Also applies to: 111-116, 124-129
🤖 Prompt for AI Agents
components/webui/client/src/sql-parser/index.ts lines 98-103 (and similarly
111-116, 124-129): the validation functions call the parser directly and can
throw unexpected exceptions; wrap the parser invocation in a try-catch, keep the
existing setupParser and return of syntaxErrorListener.errors in the normal
path, and in the catch block log the caught error (or forward it to the app
logger) and return a ValidationError[] containing a single ValidationError
describing an internal parser/validation failure so the UI doesn't crash; apply
the same try-catch pattern to validateBooleanExpression and
validateSortItemList.
Description
Add hover feature for ANTLR error. Also reworked highlight to use a different monaco API, since the marker api was not very flexible.
Checklist
breaking change.
Validation performed
Hover box appears
Summary by CodeRabbit
Release Notes
New Features