fix: sorting issue in query management page #8305

nikhilsaikethe · 2025-09-05T10:20:29Z

User description

This PR fixes the issue that is related to sorting doesn't work properly for more number of records for execution duration and query range

PR Type

Bug fix, Enhancement

Description

Centralize duration utilities in zincutils
Fix sorting by using raw seconds
Render durations via table slots
Improve query range formatting

Diagram Walkthrough

flowchart LR
  utils["zincutils.ts: add time helpers"] -- "import/use" --> runningList["RunningQueriesList.vue: slot-based render"]
  utils -- "import/use" --> summaryList["SummaryList.vue: slot-based render"]
  runningQueries["RunningQueries.vue: data shaping"] -- "use raw values" --> tables["Tables sort correctly"]

File Walkthrough

Relevant files

Enhancement

zincutils.ts `Add shared time/duration utilities` web/src/utils/zincutils.ts Add `localTimeToMicroseconds` helper. Add `getDuration` returning seconds and formatted. Export these utilities for reuse.	+23/-0
RunningQueriesList.vue `Render durations via slots using utils` web/src/components/queries/RunningQueriesList.vue Add table slots to format `duration` and `queryRange`. Import and expose `getDuration` and `durationFormatter`. Display duration from `created_at` via util.	+13/-0
SummaryList.vue `Format summary durations with table slots` web/src/components/queries/SummaryList.vue Add slots to format `duration` and `queryRange`. Import and expose `durationFormatter`.	+12/-0

Bug fix

RunningQueries.vue `Use raw values and shared duration util` web/src/components/queries/RunningQueries.vue Import `getDuration` from utils. Keep `duration` and `queryRange` as raw values. Remove local `getDuration` impl. Adjust pagination code formatting.	+6/-14

greptile-apps

Greptile Summary

This PR addresses sorting issues in the query management page by implementing proper data formatting for duration and query range columns. The changes introduce two new utility functions in zincutils.ts: localTimeToMicroseconds() to get current time in microseconds, and getDuration(createdAt) to calculate duration differences with formatted output.

The fix modifies three Vue components that display query data. In RunningQueriesList.vue and SummaryList.vue, custom cell templates are added that use the new utility functions to format duration and query range values for display. The core data structure changes are in RunningQueries.vue, where the duration and query range fields now store raw numeric values instead of pre-formatted strings.

This approach separates data storage from presentation - tables now store sortable numeric values (timestamps, seconds) while the display components format these values into human-readable strings using durationFormatter() and getDuration(). This ensures JavaScript's numeric sorting works correctly for large datasets while maintaining user-friendly display formatting.

Confidence score: 2/5

This PR has significant implementation inconsistencies that could cause runtime issues
Score lowered due to incorrect duration field assignment and potential data flow problems between components
Pay close attention to RunningQueries.vue data assignment logic and verify the removed getDuration function doesn't break existing functionality

_{4 files reviewed, 4 comments}

_{Edit Code Review Bot Settings | Greptile}

greptile-apps · 2025-09-05T10:21:06Z

web/src/utils/zincutils.ts

+
+export const localTimeToMicroseconds = () => {
+  // Create a Date object representing the current local time
+  var date = new Date();


style: Use const instead of var for better scoping

Suggested change

var date = new Date();

const date = new Date();

greptile-apps · 2025-09-05T10:21:07Z

web/src/utils/zincutils.ts

+  var date = new Date();
+
+  // Get the timestamp in milliseconds
+  var timestampMilliseconds = date.getTime();


style: Use const instead of var for better scoping

Suggested change

var timestampMilliseconds = date.getTime();

const timestampMilliseconds = date.getTime();

greptile-apps · 2025-09-05T10:21:08Z

web/src/utils/zincutils.ts

+  var timestampMilliseconds = date.getTime();
+
+  // Convert milliseconds to microseconds
+  var timestampMicroseconds = timestampMilliseconds * 1000;


style: Use const instead of var for better scoping

Suggested change

var timestampMicroseconds = timestampMilliseconds * 1000;

const timestampMicroseconds = timestampMilliseconds * 1000;

greptile-apps · 2025-09-05T10:21:22Z

web/src/components/queries/RunningQueries.vue

-          duration: getDuration(row?.created_at).duration,
-          queryRange: queryRange(row?.query?.start_time, row?.query?.end_time)
-            .duration,
+          duration: row.created_at,


logic: Storing row.created_at as duration seems incorrect - this is a timestamp, not a calculated duration. Should this be getDuration(row.created_at).durationInSeconds?

Suggested change

duration: row.created_at,

duration: getDuration(row.created_at).durationInSeconds,

Thanks for the correct suggestion , I have changed that

github-actions · 2025-09-05T10:21:39Z

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
🧪 No relevant tests
🔒 No security concerns identified
⚡ Recommended focus areas for review Possible Issue The 'duration' field is now set to 'row.created_at' (a timestamp) and rendered via slot using 'getDuration(props.row.created_at).duration' elsewhere; ensure consistent units (microseconds vs milliseconds) and that sorting uses raw seconds consistently. Verify that 'created_at' is indeed in microseconds; otherwise duration and sorting will be incorrect. rows.sort((a: any, b: any) => b.created_at - a.created_at); return rows.map((row: any, index) => { return { "#": index < 9 ? `0${index + 1}` : index + 1, user_id: row?.user_id, org_id: row?.org_id, duration: row.created_at, queryRange: queryRange(row?.query?.start_time, row?.query?.end_time).queryRangeInSeconds, status: row?.status, Precision/Overflow 'localTimeToMicroseconds' multiplies milliseconds by 1000; in JS number range, large microsecond values can lose precision. Confirm consumers only rely on seconds (integers) for logic/sorting, or consider using BigInt to avoid precision issues. export const localTimeToMicroseconds = () => { // Create a Date object representing the current local time var date = new Date(); // Get the timestamp in milliseconds var timestampMilliseconds = date.getTime(); // Convert milliseconds to microseconds var timestampMicroseconds = timestampMilliseconds * 1000; return timestampMicroseconds; }; Dead Code A local 'localTimeToMicroseconds' helper remains in this component while a centralized version exists in 'zincutils'. Consider removing the local duplicate to avoid divergence. // Get the timestamp in milliseconds var timestampMilliseconds = date.getTime(); // Convert milliseconds to microseconds var timestampMicroseconds = timestampMilliseconds * 1000; return timestampMicroseconds; };

github-actions · 2025-09-05T10:22:08Z

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
Possible issue	Use computed duration value The `duration` field is now set to `row.created_at` (a timestamp) which breaks the intended display and sorting. Use the computed duration value (in seconds or formatted) consistently with the table cell that renders it. web/src/components/queries/RunningQueries.vue [734-744] return rows.map((row: any, index) => { + const { durationInSeconds } = getDuration(row?.created_at); return { "#": index < 9 ? `0${index + 1}` : index + 1, user_id: row?.user_id, org_id: row?.org_id, - duration: row.created_at, + duration: durationInSeconds, queryRange: queryRange(row?.query?.start_time, row?.query?.end_time).queryRangeInSeconds, status: row?.status, work_group: row?.work_group, stream_type: row?.stream_type, actions: "true", + }; +}); Suggestion importance[1-10]: 7 __ Why: In the new mapping, `duration` is set to `row.created_at`, which is a timestamp and mismatches the intended semantics; using `getDuration(...).durationInSeconds` aligns with rendering and sorting needs. However, the PR also moves duration formatting/rendering to the list component, so impact is moderate rather than critical.	Medium
General	Clarify and harden timestamp helper `Date.getTime()` returns epoch milliseconds in UTC, not local time; the "local" wording is misleading. Also use `const` and return a number type consistently to avoid accidental mutation. web/src/utils/zincutils.ts [1240-1251] -export const localTimeToMicroseconds = () => { - // Create a Date object representing the current local time - var date = new Date(); - - // Get the timestamp in milliseconds - var timestampMilliseconds = date.getTime(); - - // Convert milliseconds to microseconds - var timestampMicroseconds = timestampMilliseconds * 1000; - - return timestampMicroseconds; +export const localTimeToMicroseconds = (): number => { + const timestampMilliseconds = Date.now(); + return timestampMilliseconds * 1000; }; Suggestion importance[1-10]: 6 __ Why: Replacing manual `Date` handling with `Date.now()` and clarifying return type improves clarity and immutability without changing behavior; the "local" naming is indeed misleading as it returns epoch ms. This is a minor but correct improvement.	Low
General	Guard formatter inputs Ensure `props.row.duration` and `props.row.queryRange` are numeric seconds, since upstream mapping now passes raw values. If they may be undefined, guard to prevent runtime errors during formatting. web/src/components/queries/SummaryList.vue [62-71] <template #body-cell-duration="props"> <q-td :props="props"> - {{ durationFormatter(props.row.duration) }} + {{ typeof props.row.duration === 'number' ? durationFormatter(props.row.duration) : '-' }} </q-td> </template> <template #body-cell-queryRange="props"> <q-td :props="props"> - {{ durationFormatter(props.row.queryRange) }} + {{ typeof props.row.queryRange === 'number' ? durationFormatter(props.row.queryRange) : '-' }} </q-td> </template> Suggestion importance[1-10]: 5 __ Why: Adding simple type guards prevents runtime errors if `duration` or `queryRange` are undefined or non-numeric, which is plausible given upstream changes. It's a safe, minor robustness improvement.	Low

github-actions bot added the ☢️ Bug Something isn't working label Sep 5, 2025

greptile-apps bot reviewed Sep 5, 2025

View reviewed changes

github-actions bot added the Review effort 2/5 label Sep 5, 2025

bjp232004 approved these changes Sep 5, 2025

View reviewed changes

nikhilsaikethe force-pushed the fix/sorting-in-query-management-page branch 4 times, most recently from dc1197a to d0a69a9 Compare September 11, 2025 12:46

nikhilsaikethe added 2 commits September 11, 2025 22:03

fix: sorting issue in query management page

9371fca

fix: greptile suggestions

5a4afbd

nikhilsaikethe force-pushed the fix/sorting-in-query-management-page branch from d0a69a9 to 5a4afbd Compare September 11, 2025 16:33

nikhilsaikethe merged commit 7bf9770 into main Sep 11, 2025
49 of 52 checks passed

nikhilsaikethe deleted the fix/sorting-in-query-management-page branch September 11, 2025 17:32

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix: sorting issue in query management page #8305

fix: sorting issue in query management page #8305

Uh oh!

nikhilsaikethe commented Sep 5, 2025 •

edited by github-actions bot

Loading

Uh oh!

greptile-apps bot left a comment

Uh oh!

greptile-apps bot Sep 5, 2025

Uh oh!

greptile-apps bot Sep 5, 2025

Uh oh!

greptile-apps bot Sep 5, 2025

Uh oh!

greptile-apps bot Sep 5, 2025

Uh oh!

nikhilsaikethe Sep 11, 2025

Uh oh!

github-actions bot commented Sep 5, 2025

Uh oh!

github-actions bot commented Sep 5, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

	var timestampMilliseconds = date.getTime();
	const timestampMilliseconds = date.getTime();

	var timestampMicroseconds = timestampMilliseconds * 1000;
	const timestampMicroseconds = timestampMilliseconds * 1000;

	duration: row.created_at,
	duration: getDuration(row.created_at).durationInSeconds,

fix: sorting issue in query management page #8305

fix: sorting issue in query management page #8305

Uh oh!

Conversation

nikhilsaikethe commented Sep 5, 2025 • edited by github-actions bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

User description

PR Type

Description

Diagram Walkthrough

File Walkthrough

Uh oh!

greptile-apps bot left a comment

Choose a reason for hiding this comment

Greptile Summary

Confidence score: 2/5

Uh oh!

greptile-apps bot Sep 5, 2025

Choose a reason for hiding this comment

Uh oh!

greptile-apps bot Sep 5, 2025

Choose a reason for hiding this comment

Uh oh!

greptile-apps bot Sep 5, 2025

Choose a reason for hiding this comment

Uh oh!

greptile-apps bot Sep 5, 2025

Choose a reason for hiding this comment

Uh oh!

nikhilsaikethe Sep 11, 2025

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Sep 5, 2025

PR Reviewer Guide 🔍

Uh oh!

github-actions bot commented Sep 5, 2025

PR Code Suggestions ✨

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

nikhilsaikethe commented Sep 5, 2025 •

edited by github-actions bot

Loading