Skip to content

fix(filters): sort comparator for LowCardinality fields never returns 0#2690

Merged
teeohhem merged 3 commits into
hyperdxio:mainfrom
tsushanth:fix-sort-comparator-lc-fields
Jul 21, 2026
Merged

fix(filters): sort comparator for LowCardinality fields never returns 0#2690
teeohhem merged 3 commits into
hyperdxio:mainfrom
tsushanth:fix-sort-comparator-lc-fields

Conversation

@tsushanth

Copy link
Copy Markdown
Contributor

Summary

  • In DBSearchPageFilters/hooks.ts, the .sort() comparator that orders fields by LowCardinality-first returned 1 for every pair that wasn't (a=LC, b=!LC) — including when both fields share the same cardinality.
  • A comparator that returns a non-zero value for equal elements violates the antisymmetry contract and produces non-deterministic ordering within each group (LC and non-LC fields get shuffled/reversed relative to their original positions).
  • Fixed by computing the sort key as (isLC(b) ? 1 : 0) - (isLC(a) ? 1 : 0), which correctly returns -1, 0, or 1 for all three cases.

Before:

return isLowCardinality(a.type) && !isLowCardinality(b.type) ? -1 : 1;
// returns 1 when both LC, both non-LC, or b=LC/a=!LC — never 0

After:

return (isLowCardinality(b.type) ? 1 : 0) - (isLowCardinality(a.type) ? 1 : 0);
// returns -1 (a=LC, b=!LC), 1 (a=!LC, b=LC), 0 (both same)

Test plan

  • Open the filter sidebar on a search page with a source that has both LowCardinality and non-LowCardinality string columns
  • Confirm LowCardinality fields appear before non-LowCardinality fields
  • Confirm fields within each group maintain a stable relative order across re-renders

… equal elements

The comparator previously returned 1 (b before a) for any pair that was
not (a=LC, b=!LC), including when both fields have the same cardinality.
A comparator that never returns 0 is not a valid comparator — it violates
the antisymmetry requirement and can produce non-deterministic ordering
across JS engines.

Fixed by computing the sort key as the difference of boolean-to-int
conversions, which correctly returns -1/0/1 for all three cases.
@vercel

vercel Bot commented Jul 20, 2026

Copy link
Copy Markdown

@sushanth9 is attempting to deploy a commit to the HyperDX Team on Vercel.

A member of the Team first needs to authorize it.

@changeset-bot

changeset-bot Bot commented Jul 20, 2026

Copy link
Copy Markdown

⚠️ No Changeset found

Latest commit: a859182

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@greptile-apps

greptile-apps Bot commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR fixes the field ordering comparator in the search filter sidebar.

  • Updates the LowCardinality-first sort in DBSearchPageFilters/hooks.ts.
  • Returns equal comparison results for fields in the same cardinality group.
  • Preserves stable relative ordering within LowCardinality and non-LowCardinality groups.

Confidence Score: 5/5

This looks safe to merge.

  • No blocking issues found in the changed code.

Important Files Changed

Filename Overview
packages/app/src/components/DBSearchPageFilters/hooks.ts Fixes the LowCardinality-first comparator so equal groups keep their relative order.

Reviews (3): Last reviewed commit: "style(app): format sort comparator to sa..." | Re-trigger Greptile

@teeohhem
teeohhem merged commit a14c2c0 into hyperdxio:main Jul 21, 2026
24 of 27 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants