feat(webui): Add drawer to display guided query and errors.#1421
Conversation
WalkthroughAdds caching of generated guided query strings during guided runs, introduces an OpenQueryDrawerButton to open a query drawer with contextual tooltip, and updates Presto search completion handling to record error name/msg and conditionally notify based on SQL interface. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant GuidedRunButton
participant PrestoState as Presto Search State
participant PrestoBackend as Presto Backend
User->>GuidedRunButton: Click "Run (Guided)"
GuidedRunButton->>GuidedRunButton: Build search & timeline queries
GuidedRunButton->>PrestoState: updateCachedGuidedSearchQueryString(searchQueryString)
GuidedRunButton->>PrestoBackend: Execute queries
PrestoBackend-->>GuidedRunButton: Results / status
sequenceDiagram
autonumber
participant PrestoBackend as Engine
participant Hook as useUpdateStateWithMetadata
participant Store as Search Store
participant UI as Notifications/Drawer
rect rgb(246,250,255)
Engine-->>Hook: completion signal (DONE | FAILED) + payload
alt signal == DONE
Hook->>Store: updateSearchUiState(DONE)
Hook->>Store: updateNumSearchResultsMetadata(...)
else signal == FAILED
Hook->>Store: updateErrorName(...)
Hook->>Store: updateErrorMsg(...)
alt errorName == USER_CANCELLED
Hook->>Store: updateSearchUiState(DONE)
else sqlInterface == PRESTO_SQL_INTERFACE.FREEFORM
Hook->>Store: updateSearchUiState(FAILED)
Hook->>UI: show error notification
else
Hook->>Store: updateSearchUiState(FAILED)
note right of UI: no notification for non-FREEFORM
end
end
end
sequenceDiagram
autonumber
actor User
participant OpenBtn as OpenQueryDrawerButton
participant PrestoState as Presto Search State
participant Drawer as Query Drawer
User->>OpenBtn: Click
OpenBtn->>PrestoState: updateQueryDrawerOpen(true)
PrestoState-->>Drawer: open
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~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.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
components/webui/client/src/pages/SearchPage/SearchControls/Presto/SqlSearchButton/RunButton/GuidedRunButton.tsx (1)
95-96: Follow coding guideline: Replace!isQueryReadywithfalse === isQueryReady.Line 95 uses
!isQueryReady, which violates the coding guideline that prefersfalse == <expression>over!<expression>. Line 51 already follows this pattern withfalse === isQueryReady.As per coding guidelines.
Apply this diff to align with the guideline:
- disabled={!isQueryReady || + disabled={false === isQueryReady || searchUiState === SEARCH_UI_STATE.QUERY_ID_PENDING}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (11)
components/webui/client/src/pages/SearchPage/SearchControls/Presto/SqlSearchButton/RunButton/GuidedRunButton.tsx(3 hunks)components/webui/client/src/pages/SearchPage/SearchControls/Presto/presto-guided-search-requests.ts(2 hunks)components/webui/client/src/pages/SearchPage/SearchControls/QueryStatus/OpenQueryDrawerButton.tsx(1 hunks)components/webui/client/src/pages/SearchPage/SearchControls/QueryStatus/QueryDrawer.module.css(1 hunks)components/webui/client/src/pages/SearchPage/SearchControls/QueryStatus/QueryDrawer.tsx(1 hunks)components/webui/client/src/pages/SearchPage/SearchControls/QueryStatus/index.tsx(2 hunks)components/webui/client/src/pages/SearchPage/SearchResults/SearchResultsTimeline/Presto/PrestoResultsTimeline.tsx(2 hunks)components/webui/client/src/pages/SearchPage/SearchState/Presto/index.tsx(4 hunks)components/webui/client/src/pages/SearchPage/SearchState/useUpdateStateWithMetadata.ts(3 hunks)components/webui/client/src/sql-parser/index.ts(1 hunks)components/webui/client/src/sql-parser/typings.ts(1 hunks)
🔥 Files not summarized due to errors (8)
- components/webui/client/src/pages/SearchPage/SearchState/Presto/index.tsx: Error: Server error: no LLM provider could handle the message
- components/webui/client/src/pages/SearchPage/SearchControls/QueryStatus/index.tsx: Error: Server error: no LLM provider could handle the message
- components/webui/client/src/sql-parser/index.ts: Error: Server error: no LLM provider could handle the message
- components/webui/client/src/pages/SearchPage/SearchResults/SearchResultsTimeline/Presto/PrestoResultsTimeline.tsx: Error: Server error: no LLM provider could handle the message
- components/webui/client/src/pages/SearchPage/SearchControls/QueryStatus/QueryDrawer.tsx: Error: Server error: no LLM provider could handle the message
- components/webui/client/src/pages/SearchPage/SearchControls/QueryStatus/QueryDrawer.module.css: Error: Server error: no LLM provider could handle the message
- components/webui/client/src/pages/SearchPage/SearchControls/Presto/presto-guided-search-requests.ts: Error: Server error: no LLM provider could handle the message
- components/webui/client/src/sql-parser/typings.ts: Error: Server error: no LLM provider could handle the message
🧰 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/SearchState/Presto/index.tsxcomponents/webui/client/src/pages/SearchPage/SearchControls/QueryStatus/index.tsxcomponents/webui/client/src/sql-parser/index.tscomponents/webui/client/src/pages/SearchPage/SearchControls/QueryStatus/OpenQueryDrawerButton.tsxcomponents/webui/client/src/pages/SearchPage/SearchResults/SearchResultsTimeline/Presto/PrestoResultsTimeline.tsxcomponents/webui/client/src/pages/SearchPage/SearchControls/QueryStatus/QueryDrawer.tsxcomponents/webui/client/src/pages/SearchPage/SearchControls/Presto/presto-guided-search-requests.tscomponents/webui/client/src/sql-parser/typings.tscomponents/webui/client/src/pages/SearchPage/SearchControls/Presto/SqlSearchButton/RunButton/GuidedRunButton.tsxcomponents/webui/client/src/pages/SearchPage/SearchState/useUpdateStateWithMetadata.ts
🧬 Code graph analysis (4)
components/webui/client/src/pages/SearchPage/SearchState/Presto/index.tsx (1)
components/webui/common/src/utility-types.ts (1)
Nullable(3-3)
components/webui/client/src/pages/SearchPage/SearchControls/QueryStatus/index.tsx (2)
components/webui/client/src/config/index.ts (1)
SETTINGS_QUERY_ENGINE(27-27)components/webui/client/src/pages/SearchPage/SearchState/typings.ts (1)
SEARCH_UI_STATE(31-31)
components/webui/client/src/pages/SearchPage/SearchResults/SearchResultsTimeline/Presto/PrestoResultsTimeline.tsx (2)
components/webui/client/src/pages/SearchPage/SearchControls/TimeRangeInput/utils.tsx (1)
TIME_RANGE_OPTION(130-130)components/webui/client/src/pages/SearchPage/SearchControls/Presto/presto-guided-search-requests.ts (1)
buildPrestoGuidedQueries(212-212)
components/webui/client/src/pages/SearchPage/SearchControls/QueryStatus/QueryDrawer.tsx (1)
components/webui/client/src/pages/SearchPage/SearchState/typings.ts (1)
SEARCH_UI_STATE(31-31)
🔇 Additional comments (15)
components/webui/client/src/pages/SearchPage/SearchControls/Presto/presto-guided-search-requests.ts (1)
122-146: Clearing guided-error state before resubmission looks goodResetting the cached error name/message prior to launching the next query prevents stale errors from surfacing in the new drawer—nice guard against user confusion.
components/webui/client/src/sql-parser/typings.ts (1)
24-24: No update neededWhitespace-only adjustment looks fine.
components/webui/client/src/pages/SearchPage/SearchResults/SearchResultsTimeline/Presto/PrestoResultsTimeline.tsx (1)
16-51: Nice state-store integrationStoring the freshly built guided search string alongside the zoom-triggered query keeps the drawer data consistent with what actually ran, and using
usePrestoSearchState.getState()lines up with the existing store pattern here.components/webui/client/src/sql-parser/index.ts (1)
113-117: Formatting tweak looks goodExplicit newlines ahead of
ORDER BYandLIMITkeep the generated SQL neatly structured without altering behaviour. ✅components/webui/client/src/pages/SearchPage/SearchState/Presto/index.tsx (1)
11-108: State wiring looks solid.
The cached guided query, error metadata, and drawer toggle flag are properly defaulted and exposed through focused setters, so the drawer can reflect submitted queries without stale inputs.components/webui/client/src/pages/SearchPage/SearchControls/QueryStatus/OpenQueryDrawerButton.tsx (2)
1-10: LGTM!The imports are appropriate and necessary for the component's functionality.
19-26: LGTM!The hook usage and conditional tooltip logic are implemented correctly and follow React best practices.
components/webui/client/src/pages/SearchPage/SearchControls/Presto/SqlSearchButton/RunButton/GuidedRunButton.tsx (3)
41-42: LGTM: State updater retrieval follows established pattern.The retrieval of
updateCachedGuidedSearchQueryStringis consistent with other state accessors in the file.
77-84: LGTM: Dependency array correctly updated.The addition of
updateCachedGuidedSearchQueryStringto theuseCallbackdependency array is correct.
72-76: No null/undefined risk forsearchQueryString.
buildPrestoGuidedQueriesalways returns a non-null string (it throws on missing inputs), andupdateCachedGuidedSearchQueryString’s signature enforces astringparameter.components/webui/client/src/pages/SearchPage/SearchControls/QueryStatus/index.tsx (1)
35-59: Nice conditional exposure of the guided drawer controlsGating the drawer trigger to only appear once a job has been launched—and only for guided Presto runs—keeps the UI tidy and prevents confusing entry points when the feature is irrelevant. Looks solid.
components/webui/client/src/pages/SearchPage/SearchState/useUpdateStateWithMetadata.ts (4)
10-11: LGTM! Imports are appropriate.The new imports for Presto search state and SQL interface typings are necessary for the conditional error handling introduced below.
29-33: LGTM! State extraction is well-structured.Destructuring the necessary state update functions from both stores is appropriate for the error handling and conditional notification logic.
85-87: Good fix: Previously missing dependency added.Adding
updateSearchUiStateto the dependency array corrects a previous omission. This ensures the effect re-runs appropriately when the state updater changes, improving correctness.
92-92: LGTM! Explicit export improves clarity.The explicit named export is a good practice that makes the module's public API clear and supports better tree-shaking.
hoophalab
left a comment
There was a problem hiding this comment.
Looks good! I only have two minor UI suggestions.
Validation: The Presto errors from the server are shown in the drawer.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
components/webui/client/src/pages/SearchPage/SearchControls/QueryStatus/Results.tsx(2 hunks)components/webui/client/src/pages/SearchPage/SearchControls/QueryStatus/index.tsx(2 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/QueryStatus/Results.tsxcomponents/webui/client/src/pages/SearchPage/SearchControls/QueryStatus/index.tsx
🧬 Code graph analysis (1)
components/webui/client/src/pages/SearchPage/SearchControls/QueryStatus/index.tsx (2)
components/webui/client/src/config/index.ts (1)
SETTINGS_QUERY_ENGINE(27-27)components/webui/client/src/pages/SearchPage/SearchState/typings.ts (1)
SEARCH_UI_STATE(31-31)
🔇 Additional comments (8)
components/webui/client/src/pages/SearchPage/SearchControls/QueryStatus/Results.tsx (2)
3-3: LGTM! Import simplification is appropriate.The removal of unused imports (
GetProps,Texttypings) aligns with the simplified component logic.
34-41: LGTM! Hardcoded styling simplifies the component.Hardcoding
type="secondary"is appropriate since the parentQueryStatuscomponent now handles state-specific text styling and conditionally renders this component.components/webui/client/src/pages/SearchPage/SearchControls/QueryStatus/index.tsx (6)
3-12: LGTM! Import additions support the new drawer functionality.The new imports for configuration, Presto state, and drawer components are appropriate for the feature.
29-31: LGTM! Clear and correct condition for guided Presto queries.The
isPrestoGuidedcomputation correctly identifies when the drawer functionality should be available.
95-113: LGTM! FAILED state correctly omits result count.The FAILED state appropriately excludes the
<Results/>component, showing only the search job ID and failure message. This addresses the past review discussion.
34-34: LGTM! Fragment usage removes unnecessary DOM wrapper.Replacing the
<div>wrapper with a React Fragment (<>) is a good improvement that reduces unnecessary DOM nesting.Also applies to: 122-122
121-121: LGTM! Conditional QueryDrawer rendering is appropriate.The
QueryDraweris correctly rendered only whenisPrestoGuidedis true, maintaining consistency with the drawer button visibility.
66-66: Add null-safety checks to searchJobId template string interpolation to match established patterns in the codebase.The
searchJobIdis correctly typed asstring | nullin the search store and only becomes non-null when a search is submitted. While the state machine ensuressearchJobIdis set before transitioning to QUERYING, DONE, or FAILED states, TypeScript cannot enforce this invariant automatically.Inconsistency found:
QueryDrawer.tsx(line 37) already usessearchJobId ?? ""for safe interpolation, butQueryStatuslines 66, 88, and 110 lack this defensive pattern. Add null coalescing to align with the established convention:
- Line 66:
{- search job ${searchJobId ?? "unknown"} found}- Line 88:
{- search job ${searchJobId ?? "unknown"} found}- Line 110:
{- search job ${searchJobId ?? "unknown"}`}This improves clarity and maintains consistency with defensive patterns used elsewhere (e.g.,
CancelButton.tsxincludes explicit null checks despite state guarantees).
kirkrodrigues
left a comment
There was a problem hiding this comment.
Deferring to @hoophalab's review.
* feat(webui): Add drawer to display guided query and errors. (y-scope#1421) * docs: Add Slack community invite badge to home page README. (y-scope#1418) * refactor(clp-package): Simplify StrEnum and Path serialization via Annotated serializers. (y-scope#1417) Co-authored-by: Junhao Liao <junhao@junhao.ca> Co-authored-by: Junhao Liao <junhao.liao@yscope.com> * build(clp-package): Adopt uv + hatchling as the build and packaging backend for Python components (resolves y-scope#1396); Upgrade dependencies for Python components. (y-scope#1405) * chore(docker): Add `--link` flags to COPY/ADD operations for improved build performance (fixes y-scope#1408). (y-scope#1411) * fix(ci): Correctly update and restore cache of `lint:check-cpp-lint-static-full`'s generated files (fixes y-scope#1419): (y-scope#1430) - Save cache entries using unique key per entry. - Restore latest entries using key prefix. - Avoid using outputs from optionally-run `restore` task. * fix(clp-rust-utils): Use AWS SDK default configuration with latest behavior version for S3 client. (y-scope#1445) Co-authored-by: Junhao Liao <junhao.liao@yscope.com> * refactor(clp-package): Remove unused `python-dotenv` dependency and related imports (fixes y-scope#1443). (y-scope#1444) * fix(webui): Submit queries that failed ANTLR validation to Presto. (y-scope#1450) * feat(clp-s): Explicitly reject unstructured log inputs during compression. (y-scope#1434) * feat(webui): Show query speed in native search status. (y-scope#1429) * fix(job-orchestration): Make `tag_ids` a required `list[int]` for compatibility with Spider compressor. (y-scope#1453) * feat(clp-mcp-server): Add log viewer links to query results for displaying in LLM output. (y-scope#1454) Co-authored-by: Junhao Liao <junhao.liao@yscope.com> * feat(ci): Add tasks for checking and updating Rust lock file (`Cargo.lock`); Add check to GH workflow. (y-scope#1448) * feat(webui): Trigger submit action when pressing Enter on Monaco single line editor. (y-scope#1459) * Add filters. * Update cargo lock. --------- Co-authored-by: davemarco <83603688+davemarco@users.noreply.github.com> Co-authored-by: Abigail Matthews <abigail.v.matthews@gmail.com> Co-authored-by: sitaowang1998 <sitaowang1998@outlook.com> Co-authored-by: Junhao Liao <junhao@junhao.ca> Co-authored-by: Junhao Liao <junhao.liao@yscope.com> Co-authored-by: kirkrodrigues <2454684+kirkrodrigues@users.noreply.github.com> Co-authored-by: Devin Gibson <gibber9809@users.noreply.github.com> Co-authored-by: hoophalab <200652805+hoophalab@users.noreply.github.com> Co-authored-by: Huangshi Tian <All-less@users.noreply.github.com>
* feat(webui): Add drawer to display guided query and errors. (y-scope#1421) * docs: Add Slack community invite badge to home page README. (y-scope#1418) * refactor(clp-package): Simplify StrEnum and Path serialization via Annotated serializers. (y-scope#1417) Co-authored-by: Junhao Liao <junhao@junhao.ca> Co-authored-by: Junhao Liao <junhao.liao@yscope.com> * build(clp-package): Adopt uv + hatchling as the build and packaging backend for Python components (resolves y-scope#1396); Upgrade dependencies for Python components. (y-scope#1405) * chore(docker): Add `--link` flags to COPY/ADD operations for improved build performance (fixes y-scope#1408). (y-scope#1411) * fix(ci): Correctly update and restore cache of `lint:check-cpp-lint-static-full`'s generated files (fixes y-scope#1419): (y-scope#1430) - Save cache entries using unique key per entry. - Restore latest entries using key prefix. - Avoid using outputs from optionally-run `restore` task. * fix(clp-rust-utils): Use AWS SDK default configuration with latest behavior version for S3 client. (y-scope#1445) Co-authored-by: Junhao Liao <junhao.liao@yscope.com> * refactor(clp-package): Remove unused `python-dotenv` dependency and related imports (fixes y-scope#1443). (y-scope#1444) * fix(webui): Submit queries that failed ANTLR validation to Presto. (y-scope#1450) * feat(clp-s): Explicitly reject unstructured log inputs during compression. (y-scope#1434) * feat(webui): Show query speed in native search status. (y-scope#1429) * fix(job-orchestration): Make `tag_ids` a required `list[int]` for compatibility with Spider compressor. (y-scope#1453) * feat(clp-mcp-server): Add log viewer links to query results for displaying in LLM output. (y-scope#1454) Co-authored-by: Junhao Liao <junhao.liao@yscope.com> * feat(ci): Add tasks for checking and updating Rust lock file (`Cargo.lock`); Add check to GH workflow. (y-scope#1448) * feat(webui): Trigger submit action when pressing Enter on Monaco single line editor. (y-scope#1459) * Add filters. * Update cargo lock. --------- Co-authored-by: davemarco <83603688+davemarco@users.noreply.github.com> Co-authored-by: Abigail Matthews <abigail.v.matthews@gmail.com> Co-authored-by: sitaowang1998 <sitaowang1998@outlook.com> Co-authored-by: Junhao Liao <junhao@junhao.ca> Co-authored-by: Junhao Liao <junhao.liao@yscope.com> Co-authored-by: kirkrodrigues <2454684+kirkrodrigues@users.noreply.github.com> Co-authored-by: Devin Gibson <gibber9809@users.noreply.github.com> Co-authored-by: hoophalab <200652805+hoophalab@users.noreply.github.com> Co-authored-by: Huangshi Tian <All-less@users.noreply.github.com>
* feat(webui): Add drawer to display guided query and errors. (y-scope#1421) * docs: Add Slack community invite badge to home page README. (y-scope#1418) * refactor(clp-package): Simplify StrEnum and Path serialization via Annotated serializers. (y-scope#1417) Co-authored-by: Junhao Liao <junhao@junhao.ca> Co-authored-by: Junhao Liao <junhao.liao@yscope.com> * build(clp-package): Adopt uv + hatchling as the build and packaging backend for Python components (resolves y-scope#1396); Upgrade dependencies for Python components. (y-scope#1405) * chore(docker): Add `--link` flags to COPY/ADD operations for improved build performance (fixes y-scope#1408). (y-scope#1411) * fix(ci): Correctly update and restore cache of `lint:check-cpp-lint-static-full`'s generated files (fixes y-scope#1419): (y-scope#1430) - Save cache entries using unique key per entry. - Restore latest entries using key prefix. - Avoid using outputs from optionally-run `restore` task. * fix(clp-rust-utils): Use AWS SDK default configuration with latest behavior version for S3 client. (y-scope#1445) Co-authored-by: Junhao Liao <junhao.liao@yscope.com> * refactor(clp-package): Remove unused `python-dotenv` dependency and related imports (fixes y-scope#1443). (y-scope#1444) * fix(webui): Submit queries that failed ANTLR validation to Presto. (y-scope#1450) * feat(clp-s): Explicitly reject unstructured log inputs during compression. (y-scope#1434) * feat(webui): Show query speed in native search status. (y-scope#1429) * fix(job-orchestration): Make `tag_ids` a required `list[int]` for compatibility with Spider compressor. (y-scope#1453) * feat(clp-mcp-server): Add log viewer links to query results for displaying in LLM output. (y-scope#1454) Co-authored-by: Junhao Liao <junhao.liao@yscope.com> * feat(ci): Add tasks for checking and updating Rust lock file (`Cargo.lock`); Add check to GH workflow. (y-scope#1448) * feat(webui): Trigger submit action when pressing Enter on Monaco single line editor. (y-scope#1459) * Add filters. * Update cargo lock. * Stupid fix --------- Co-authored-by: davemarco <83603688+davemarco@users.noreply.github.com> Co-authored-by: Abigail Matthews <abigail.v.matthews@gmail.com> Co-authored-by: sitaowang1998 <sitaowang1998@outlook.com> Co-authored-by: Junhao Liao <junhao@junhao.ca> Co-authored-by: Junhao Liao <junhao.liao@yscope.com> Co-authored-by: kirkrodrigues <2454684+kirkrodrigues@users.noreply.github.com> Co-authored-by: Devin Gibson <gibber9809@users.noreply.github.com> Co-authored-by: hoophalab <200652805+hoophalab@users.noreply.github.com> Co-authored-by: Huangshi Tian <All-less@users.noreply.github.com>
Description
Adds a drawer and a button to open drawer to UI.
Drawer is only shown for submitted queries and not the current inputs to the UI. After some thinking, it wouldn't make sense to show the previous error on the new query. With live-approach, there are also issues with the timestamp being out-of-date, and will not match submitted query. It could also show a nonsensical query if user puts bad inputs.
The approach of showing submitted query solves issues.
Checklist
breaking change.
Validation performed
Tested query and error
Summary by CodeRabbit
New Features
Improvements
Bug Fixes