feat(webui): Remove limit from guided UI and set default limit in guided queries to 1000 results. by davemarco · Pull Request #1398 · y-scope/clp

davemarco · 2025-10-09T15:24:30Z

Description

Removes limit, and reallocates space in grid to fill new empty space

Checklist

The PR satisfies the contribution guidelines.
This is a breaking change and that has been indicated in the PR title, OR this isn't a
breaking change.
Necessary docs have been updated, OR no docs need to be updated.

Validation performed

Ran query

Summary by CodeRabbit

Refactor
- Simplified Presto guided search by removing the per-query Limit control; queries now run with a standard limit of 1000 results.
- Streamlined state usage for improved performance without changing behaviour.
Style
- Updated layout of Presto guided controls for clearer arrangement of fields (Where now spans two columns; Timestamp Key and Order positions adjusted).
- Improved automatic row sizing for a more consistent UI.

coderabbitai · 2025-10-09T15:32:58Z

Walkthrough

Removed user-configurable LIMIT from Presto guided search. Deleted Limit UI component and related state fields/setters, updated CSS grid classes, adjusted GuidedControls to omit Limit, refactored RunButton to use individual selectors, and hardcoded a DEFAULT_SEARCH_LIMIT (1000) in query construction. Minor whitespace/formatting tweaks elsewhere.

Changes

Cohort / File(s)	Summary
Remove Limit UI component `components/webui/client/src/pages/SearchPage/SearchControls/Presto/GuidedControls/Limit.tsx`	Deleted the Limit component that rendered a LIMIT Select and updated search state.
GuidedControls layout updates `components/webui/client/src/pages/SearchPage/SearchControls/Presto/GuidedControls/index.module.css`	Switched grid-template-rows to grid-auto-rows; removed .limit; added .timestampKey and .order; adjusted grid-column spans (.where span 2, .timestampKey span 1, .order span 2).
GuidedControls usage cleanup `components/webui/client/src/pages/SearchPage/SearchControls/Presto/GuidedControls/index.tsx`	Removed Limit import and JSX usage.
Presto search state changes `components/webui/client/src/pages/SearchPage/SearchState/Presto/index.tsx`	Removed limit from state interface/defaults and removed updateLimit setter. No new fields added.
Guided run button selector refactor `components/webui/client/src/pages/SearchPage/SearchControls/Presto/SqlSearchButton/RunButton/GuidedRunButton.tsx`	Replaced single state destructure with individual selectors for select, from, timestampKey; logic unchanged.
Query builder default limit `components/webui/client/src/pages/SearchPage/SearchControls/Presto/presto-guided-search-requests.ts`	Added DEFAULT_SEARCH_LIMIT=1000; removed use of state.limit; always applies fixed LIMIT in built query string.
Formatting only `components/webui/client/src/sql-parser/typings.ts`	Added a blank line; no semantic changes.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant U as User
  participant UI as GuidedControls UI
  participant RB as GuidedRunButton
  participant QS as buildPrestoGuidedQueries
  participant S as Presto Backend

  U->>UI: Provide select/from/where/timestampKey/order
  U->>RB: Click Run
  RB->>QS: Build queries with state values
  note over QS: Apply DEFAULT_SEARCH_LIMIT (1000)<br/>No user-provided limit
  QS-->>RB: searchQueryString (with LIMIT 1000)
  RB->>S: Submit query
  S-->>RB: Results
  RB-->>U: Display results

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check	✅ Passed	The title clearly describes the primary change by stating that the guided UI limit has been removed and a default limit of 1000 results is now applied in queries, which directly matches the changes in the files and objectives of the pull request. It is concise, specific, and free of extraneous details or vague language.
Docstring Coverage	✅ Passed	No functions found in the changes. Docstring coverage check skipped.

✨ Finishing touches

📝 Generate docstrings

🧪 Generate unit tests (beta)

Create PR with unit tests
Post copyable unit tests in a comment

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.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

coderabbitai

Actionable comments posted: 2

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)
88-88: Coding guideline violation: Use explicit comparison.

The negation operator !isQueryReady violates the coding guideline. Use false === isQueryReady instead.

As per coding guidelines.

Apply this diff:
-                disabled={!isQueryReady ||
+                disabled={false === isQueryReady ||
                     searchUiState === SEARCH_UI_STATE.QUERY_ID_PENDING}

📜 Review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f8fcd0e and ea35716.

📒 Files selected for processing (7)

components/webui/client/src/pages/SearchPage/SearchControls/Presto/GuidedControls/Limit.tsx (0 hunks)
components/webui/client/src/pages/SearchPage/SearchControls/Presto/GuidedControls/index.module.css (2 hunks)
components/webui/client/src/pages/SearchPage/SearchControls/Presto/GuidedControls/index.tsx (0 hunks)
components/webui/client/src/pages/SearchPage/SearchControls/Presto/SqlSearchButton/RunButton/GuidedRunButton.tsx (2 hunks)
components/webui/client/src/pages/SearchPage/SearchState/Presto/index.tsx (0 hunks)
components/webui/client/src/sql-parser/index.ts (2 hunks)
components/webui/client/src/sql-parser/typings.ts (1 hunks)

💤 Files with no reviewable changes (3)

components/webui/client/src/pages/SearchPage/SearchState/Presto/index.tsx
components/webui/client/src/pages/SearchPage/SearchControls/Presto/GuidedControls/Limit.tsx
components/webui/client/src/pages/SearchPage/SearchControls/Presto/GuidedControls/index.tsx

🧰 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/typings.ts
components/webui/client/src/pages/SearchPage/SearchControls/Presto/SqlSearchButton/RunButton/GuidedRunButton.tsx
components/webui/client/src/sql-parser/index.ts

🧬 Code graph analysis (1)

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

components/webui/client/src/sql-parser/typings.ts (1)

DEFAULT_SEARCH_LIMIT (30-30)

⏰ 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). (2)

GitHub Check: conventional-commits
GitHub Check: package-image

🔇 Additional comments (8)

components/webui/client/src/sql-parser/typings.ts (2)

21-24: LGTM! Well-documented default limit constant.

The constant definition is clear, well-documented, and the value of 1000 aligns with the PR objectives to provide a sensible default for search queries.

30-30: LGTM! Proper export of the constant.

The export statement correctly makes the constant available for use in other modules.

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

14-14: LGTM! Correct import of the default limit constant.

The import properly brings in the DEFAULT_SEARCH_LIMIT constant for use in the query building logic.

118-120: LGTM! Default limit logic is correctly implemented.

The else clause properly applies the default limit of 1000 when no explicit limit is provided. This ensures all search queries have a bounded result set, which is a sensible default behaviour.

components/webui/client/src/pages/SearchPage/SearchControls/Presto/SqlSearchButton/RunButton/GuidedRunButton.tsx (3)

36-36: LGTM: Correct usage of comparison.

The comparison follows the coding guideline by using false === isQueryReady rather than !isQueryReady.

As per coding guidelines.

41-41: Verify the intentional use of getState() for fresh state values.

Fetching where and orderBy via getState() at click time ensures the latest values are used, rather than capturing them at render time. Confirm this behaviour is intentional, as it differs from the pattern used for select, from, and timestampKey, which are captured via selectors and included in the dependency array.

58-66: Default limit of 1000 is already applied in buildSearchQuery (index.ts:119). No further action required.

components/webui/client/src/pages/SearchPage/SearchControls/Presto/GuidedControls/index.module.css (1)

18-21: New .timestampKey placement looks good

Span 1 and flex container are consistent with the grid.

hoophalab

lgtm. Only one nitpick.

Validations: limit input is removed and the results are limited to 1000

coderabbitai

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 0ec3324 and 7e43349.

📒 Files selected for processing (2)

components/webui/client/src/pages/SearchPage/SearchControls/Presto/presto-guided-search-requests.ts (3 hunks)
components/webui/client/src/sql-parser/typings.ts (1 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/presto-guided-search-requests.ts
components/webui/client/src/sql-parser/typings.ts

⏰ 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: rust-checks (ubuntu-24.04)
GitHub Check: rust-checks (ubuntu-22.04)
GitHub Check: lint-check (ubuntu-24.04)

🔇 Additional comments (2)

components/webui/client/src/pages/SearchPage/SearchControls/Presto/presto-guided-search-requests.ts (2)

69-69: LGTM! State destructuring correctly updated.

The removal of limit from the destructured state aligns with the PR objective to remove the limit control from the guided UI.

88-88: LGTM! Default limit correctly hardcoded.

The implementation now always includes a LIMIT clause with the default value of 1000, which aligns with the PR objectives. This is a behaviour change from the previous conditional inclusion, but it's intentional and was previously discussed in reviews. Based on learnings.

hoophalab

All good, except one more nitpick: There is one unintentionally added empty line.

Co-authored-by: hoophalab <200652805+hoophalab@users.noreply.github.com>

junhaoliao

deferring to @hoophalab 's review

…ded queries to 1000 results. (y-scope#1398)

latest

231fbfb

davemarco added 2 commits October 9, 2025 12:05

Merge branch 'main' into rmlimit

0c9f08c

Merge branch 'main' into rmlimit

ea35716

davemarco marked this pull request as ready for review October 9, 2025 18:09

davemarco requested a review from a team as a code owner October 9, 2025 18:09

davemarco requested a review from hoophalab October 9, 2025 18:11

coderabbitai Bot reviewed Oct 9, 2025

View reviewed changes

Comment thread ...ents/webui/client/src/pages/SearchPage/SearchControls/Presto/GuidedControls/index.module.css

Comment thread ...ents/webui/client/src/pages/SearchPage/SearchControls/Presto/GuidedControls/index.module.css

merge

0ec3324

hoophalab previously approved these changes Oct 10, 2025

View reviewed changes

Comment thread ...nts/webui/client/src/pages/SearchPage/SearchControls/Presto/presto-guided-search-requests.ts

latest

7e43349

davemarco dismissed hoophalab’s stale review via 7e43349 October 10, 2025 18:27

davemarco requested a review from hoophalab October 10, 2025 18:27

coderabbitai Bot reviewed Oct 10, 2025

View reviewed changes

Comment thread ...nts/webui/client/src/pages/SearchPage/SearchControls/Presto/presto-guided-search-requests.ts

hoophalab previously approved these changes Oct 14, 2025

View reviewed changes

Comment thread components/webui/client/src/sql-parser/typings.ts Outdated

Update components/webui/client/src/sql-parser/typings.ts

266ce07

Co-authored-by: hoophalab <200652805+hoophalab@users.noreply.github.com>

davemarco dismissed hoophalab’s stale review via 266ce07 October 14, 2025 15:55

hoophalab approved these changes Oct 14, 2025

View reviewed changes

Merge branch 'main' into rmlimit

9127729

junhaoliao approved these changes Oct 14, 2025

View reviewed changes

hoophalab merged commit 6623829 into y-scope:main Oct 14, 2025
22 checks passed

junhaoliao pushed a commit to junhaoliao/clp that referenced this pull request May 17, 2026

feat(webui): Remove limit from guided UI and set default limit in gui…

afa3ac4

…ded queries to 1000 results. (y-scope#1398)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(webui): Remove limit from guided UI and set default limit in guided queries to 1000 results.#1398

feat(webui): Remove limit from guided UI and set default limit in guided queries to 1000 results.#1398
hoophalab merged 7 commits into
y-scope:mainfrom
davemarco:rmlimit

davemarco commented Oct 9, 2025 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Oct 9, 2025 •

edited

Loading

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

Uh oh!

hoophalab left a comment

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

hoophalab left a comment

Uh oh!

Uh oh!

junhaoliao left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

davemarco commented Oct 9, 2025 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Checklist

Validation performed

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Oct 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Sequence Diagram(s)

Estimated code review effort

Pre-merge checks and finishing touches

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

hoophalab left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

hoophalab left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

junhaoliao left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

davemarco commented Oct 9, 2025 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Oct 9, 2025 •

edited

Loading