feat(webui): Show query speed in native search status. by hoophalab · Pull Request #1429 · y-scope/clp

hoophalab · 2025-10-16T20:27:06Z

Description

This PR fetches the query duration and bytes read from the clp database, and shows the result in the UI.

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

Query speed is shown correctly for clp and clps.

Summary by CodeRabbit

New Features
- Added query performance metrics to search results: displays query latency (seconds) and throughput (MB/s) when a search completes and specific engine conditions are met.

coderabbitai · 2025-10-16T20:27:32Z

Walkthrough

Adds a QuerySpeed React component and a utils module to fetch query bytes/duration via SQL, displays latency and MB/s when a search finishes, and integrates QuerySpeed into QueryStatus rendering for the DONE state when engine differs.

Changes

Cohort / File(s)	Summary
New component: QuerySpeed `components/webui/client/src/pages/SearchPage/SearchControls/QueryStatus/QuerySpeed/index.tsx`	Adds a React component subscribing to shared store (searchUiState, searchJobId, cachedDataset); uses useQuery to call `fetchQuerySpeed(dataset, jobId)` when UI state is DONE; formats and renders latency and speed (MB/s). Default export.
New utilities: query speed SQL `components/webui/client/src/pages/SearchPage/SearchControls/QueryStatus/QuerySpeed/utils.ts`	Adds `buildQuerySpeedSql(datasetName, jobId)` to construct SQL (selects bytes and duration from query_tasks / query_jobs using `clp_archives` or `clp_${dataset}_archives`) and `fetchQuerySpeed(datasetName, jobId)` which runs `querySql`, validates response, and returns bytes/duration. Exports `fetchQuerySpeed`.
Modified integration: QueryStatus `components/webui/client/src/pages/SearchPage/SearchControls/QueryStatus/index.tsx`	Imports and conditionally renders `QuerySpeed` in the DONE branch when the active Presto engine differs from configured engine; placed alongside results text.

Sequence Diagram(s)

sequenceDiagram
    participant QS as QueryStatus
    participant QSP as QuerySpeed
    participant Store as GlobalStore
    participant API as SQL/API

    QS->>QSP: render when searchUiState == DONE && engine mismatch
    activate QSP
    QSP->>Store: read(searchUiState, searchJobId, cachedDataset)

    alt UI state == DONE
        QSP->>API: fetchQuerySpeed(cachedDataset, searchJobId)
        activate API
        API->>API: buildQuerySpeedSql(...) (choose table: clp_archives or clp_${dataset}_archives)
        API->>API: querySql(sql)
        API-->>QSP: { bytes, duration }
        deactivate API

        QSP->>QSP: compute latency and MB/s (formatSizeInBytes, truncate latency)
        QSP-->>QS: render "in {latency}s ({speed} MB/s)"
    else
        QSP-->>QS: no output
    end
    deactivate QSP

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 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 PR title "feat(webui): Show query speed in native search status." directly and accurately describes the main objective of the changeset. The PR introduces a new QuerySpeed component, associated utility functions, and integrates them into the search status UI to display query performance metrics (latency and speed in MB/s). The title is concise, specific, and uses clear terminology that immediately conveys the feature being added without vague language or unnecessary details. A teammate reviewing the commit history would quickly understand that this change adds query speed visibility to the native search status.
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: 1

♻️ Duplicate comments (2)

components/webui/client/src/pages/SearchPage/SearchControls/QueryStatus/QuerySpeed/utils.ts (1)

13-39: SQL injection vulnerability already flagged—awaiting fix.

This comment reiterates the extensively discussed SQL injection vulnerability in the past reviews. The string interpolation of datasetName (line 16) and jobId (line 38) without parameterization creates a critical security risk, particularly since datasetName is user-controlled via UI selection.
components/webui/client/src/pages/SearchPage/SearchControls/QueryStatus/QuerySpeed/index.tsx (1)
29-32: Division by zero produces Infinity—guard against zero duration.

Line 31 divides bytes by duration without checking if duration is zero. When a query completes instantly or has zero recorded duration, this calculation produces Infinity, resulting in display text like "Query took 0.000 seconds (Infinity MB/s)."

Apply this diff to guard against division by zero:
         queryFn: async () => {
             const {bytes, duration} = await fetchQuerySpeed(cachedDataset ?? "", searchJobId ?? "");
-            return {latency: duration, speed: bytes / duration};
+            return {latency: duration, speed: duration > 0 ? bytes / duration : 0};
         },

📜 Review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between be5e1f6 and ab061fe.

📒 Files selected for processing (3)

components/webui/client/src/pages/SearchPage/SearchControls/QueryStatus/QuerySpeed/index.tsx (1 hunks)
components/webui/client/src/pages/SearchPage/SearchControls/QueryStatus/QuerySpeed/utils.ts (1 hunks)
components/webui/client/src/pages/SearchPage/SearchControls/QueryStatus/index.tsx (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/QueryStatus/index.tsx
components/webui/client/src/pages/SearchPage/SearchControls/QueryStatus/QuerySpeed/utils.ts
components/webui/client/src/pages/SearchPage/SearchControls/QueryStatus/QuerySpeed/index.tsx

🧬 Code graph analysis (2)

components/webui/client/src/pages/SearchPage/SearchControls/QueryStatus/index.tsx (2)

components/webui/client/src/pages/SearchPage/SearchState/typings.ts (1)

SEARCH_UI_STATE (31-31)

components/webui/client/src/config/index.ts (1)

SETTINGS_QUERY_ENGINE (27-27)

components/webui/client/src/pages/SearchPage/SearchControls/QueryStatus/QuerySpeed/utils.ts (2)

components/webui/client/src/settings.ts (1)

settings (36-36)

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

querySql (14-14)

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

GitHub Check: package-image
GitHub Check: rust-checks (ubuntu-22.04)
GitHub Check: rust-checks (ubuntu-24.04)
GitHub Check: rust-checks (macos-15)
GitHub Check: lint-check (macos-15)

davemarco

small changes but approve

davemarco · 2025-10-21T18:03:40Z

i'm okay with code - @junhaoliao can approve query

coderabbitai

Actionable comments posted: 1

♻️ Duplicate comments (1)

components/webui/client/src/pages/SearchPage/SearchControls/QueryStatus/QuerySpeed/utils.ts (1)

13-39: SQL injection vulnerability already extensively discussed.

This issue has been thoroughly reviewed in previous comments. The vulnerability is confirmed: datasetName (user-controlled) and jobId are interpolated directly into SQL without parameterisation, and the server executes the raw query string. Parameterised queries or proper escaping should be implemented at the server level.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 2473795 and f21b4e0.

📒 Files selected for processing (1)

components/webui/client/src/pages/SearchPage/SearchControls/QueryStatus/QuerySpeed/utils.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/QueryStatus/QuerySpeed/utils.ts

junhaoliao

the query lgtm. deferring to @davemarco 's review for the rest of the code

junhaoliao · 2025-10-21T19:10:56Z

+        settings.SqlDbClpArchivesTableName :
+        `${settings.SqlDbClpTablePrefix}${datasetName}_archives`;
+
+    return `WITH qt AS (


the query lgtm

* 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>

feat(webui): Show query speed in native search status.

be5e1f6

hoophalab requested a review from a team as a code owner October 16, 2025 20:27

hoophalab requested a review from davemarco October 16, 2025 20:27

This comment was marked as resolved.

Sign in to view

davemarco reviewed Oct 17, 2025

View reviewed changes

Comment thread components/webui/client/src/pages/SearchPage/SearchControls/QueryStatus/QuerySpeed/utils.ts

davemarco requested changes Oct 17, 2025

View reviewed changes

davemarco requested a review from junhaoliao October 17, 2025 18:00

hoophalab added 2 commits October 20, 2025 12:38

merge

a15d14d

address comments

ab061fe

hoophalab requested a review from davemarco October 20, 2025 16:43

coderabbitai Bot reviewed Oct 20, 2025

View reviewed changes

Comment thread components/webui/client/src/pages/SearchPage/SearchControls/QueryStatus/index.tsx Outdated

davemarco previously approved these changes Oct 21, 2025

View reviewed changes

address comments

2473795

hoophalab dismissed davemarco’s stale review via 2473795 October 21, 2025 16:24

hoophalab requested a review from davemarco October 21, 2025 16:24

hoophalab added 2 commits October 21, 2025 14:12

update sql

d4517cf

Merge branch 'main' into speed

f21b4e0

coderabbitai Bot reviewed Oct 21, 2025

View reviewed changes

Comment thread components/webui/client/src/pages/SearchPage/SearchControls/QueryStatus/QuerySpeed/utils.ts

junhaoliao approved these changes Oct 21, 2025

View reviewed changes

hoophalab merged commit 13f2cfd into y-scope:main Oct 21, 2025
23 checks passed

hoophalab deleted the speed branch October 23, 2025 19:06

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

feat(webui): Show query speed in native search status. (y-scope#1429)

27ea881

Conversation

hoophalab commented Oct 16, 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 16, 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!

This comment was marked as resolved.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

davemarco left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

davemarco commented Oct 21, 2025

Uh oh!

coderabbitai Bot 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!

junhaoliao Oct 21, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

hoophalab commented Oct 16, 2025 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Oct 16, 2025 •

edited

Loading