Skip to content

fix: sort filter keys alphabetically - #3397

Merged
kodiakhq[bot] merged 4 commits into
mainfrom
aaron/alphabetize-keys
Oct 19, 2026
Merged

kodiakhq[bot] merged 4 commits into
mainfrom
aaron/alphabetize-keys

Conversation

@knudtty

@knudtty knudtty commented Oct 16, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Sorts keys in NestedFilterGroups alphabetically.

Screenshots or video

Before
image

After
image

References

  • Linear Issue: Closes HDX-5754

@changeset-bot

changeset-bot Bot commented Oct 16, 2026 •

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 7c3e6d2

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 3 packages
Name Type
@hyperdx/app Patch
@hyperdx/api Patch
@hyperdx/otel-collector Patch

Not sure what this means? Click here to learn what changesets are.

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

@vercel

vercel Bot commented Oct 16, 2026 •

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated
hyperdx-oss Ready Ready Preview Oct 19, 2026 2:37pm UTC
hyperdx-storybook Ready Ready Preview Oct 19, 2026 2:37pm UTC

Request Review

@github-actions github-actions Bot added the review/tier-2 Low risk — AI review + quick human skim label Oct 16, 2026
@github-actions

github-actions Bot commented Oct 16, 2026 •

Copy link
Copy Markdown
Contributor

🔵 Tier 2 — Low Risk

Small, isolated change with no API route or data model modifications.

Why this tier:

  • Standard feature/fix — introduces new logic or modifies core functionality

Review process: AI review + quick human skim (target: 5–15 min). Reviewer validates AI assessment and checks for domain-specific concerns.
SLA: Resolve within 4 business hours.

Stats
  • Production files changed: 2
  • Production lines changed: 28 (+ 29 in test files, excluded from tier calculation)
  • Branch: aaron/alphabetize-keys
  • Author: knudtty

To override this classification, remove the review/tier-2 label and apply a different review/tier-* label. Manual overrides are preserved on subsequent pushes.

Comment thread packages/app/src/components/DBSearchPageFilters/NestedFilterGroup.tsx Outdated
@github-actions

github-actions Bot commented Oct 16, 2026 •

Copy link
Copy Markdown
Contributor

PR Review

If you are a coding agent acting for the author, read this first.
Nothing below blocks merge automatically; a maintainer decides. What they will expect:

  • 🔴 critical or 🟠 major in code this PR adds or changes: fix before asking for review.
  • 🔵 minor in code this PR changes: your call. Fix if small, otherwise reply.
  • Anything about surrounding code, or asking you to widen the change (hoist a helper,
    dedupe with another file, fix other call sites): never fix here, whatever the
    severity. Reply with one sentence; if it is critical, say so plainly so a human sees it.

One commit per review round. After two rounds, stop and ask a maintainer to review
scope rather than addressing more automated findings. Full rule: AGENTS.md.

3 finding(s): 🔴 0 critical · 🟠 0 major · 🔵 3 minor

3 posted as inline comment(s) on the changed lines.


Severity is the reviewer's own estimate and is used for ordering, not filtering. No finding blocks merge automatically; a maintainer decides.

@github-actions

Copy link
Copy Markdown
Contributor

Duplicate code analysis

No new duplicate code introduced by this PR.


jscpd compared this branch against origin/main
across app, api and common-utils. Only new clones are listed; the codebase
currently holds 125 in total (1.69%).
Run yarn dupes locally to reproduce, or yarn dupes:all for the full picture.

@greptile-apps

greptile-apps Bot commented Oct 16, 2026 •

Copy link
Copy Markdown
Contributor

RetriggerView in GreptileConfidence Score: 5/5

[Medium impact] The PR appears safe to merge; no actionable issues remain.

Summary

Sorts nested filter keys alphabetically, with natural number ordering.

  • Nested filter keys appear in alphabetical order within each group.

Reviews (6) · Last reviewed commit: "Merge branch 'main' into aaron/alphabeti..." · Reviewed by Greptile

@github-actions

github-actions Bot commented Oct 16, 2026 •

Copy link
Copy Markdown
Contributor

Deep Review

✅ No critical issues found.

This is a small, well-scoped change: a new pure helper sortNestedFacetKeys that sorts keys within each nested Map/JSON filter group alphabetically, called in the shownFacets memo ahead of the existing priority sorts. The comparator was traced and is a valid total preorder (both-non-nested → 0, nested-vs-non-nested → non-nested first), the non-null assertions are safe because every nested baseName is populated before the comparator runs, and the ES2019 stable sort preserves within-group alphabetical order through the five subsequent equal-priority sorts. The prior Greptile comment (comment restating the code at the call site) was already addressed — the redundant comment was removed in 7c3e6d26.

🟡 P2 — recommended

  • packages/app/src/components/DBSearchPageFilters/utils.test.ts:166 — the new behavior is covered by a single flat-array unit test; there is no test confirming within-group alphabetical order survives the five subsequent priority sorts (primary-key, pinned, shared-pinned, checked, range) that run on _facets after sortNestedFacetKeys.
    • Fix: add a test exercising a nested group where one child is checked or pinned, asserting that child surfaces first within its group while the remaining children stay alphabetical after all sorts.
    • correctness, testing
🔵 P3 nitpicks (2)
  • packages/app/src/components/DBSearchPageFilters/utils.ts:191 — parseMapFieldName is invoked twice per comparison inside facets.sort, i.e. O(n log n) regex/parse calls, plus once per element to build groupOrder.
    • Fix: precompute each facet's parsed {baseName, propertyPath} once into a map keyed by key and read from it inside the comparator.
  • packages/app/src/components/DBSearchPageFilters/utils.ts:194 — the Number(!!nestedA) - Number(!!nestedB) branch that orders nested vs non-nested entries is terse and reads obliquely at the call site.
    • Fix: replace with an explicit nestedA ? 1 : -1-style expression or a short comment stating non-nested facets sort ahead of nested ones.

Reviewers (9): correctness, testing, maintainability, project-standards, kieran-typescript, performance, previous-comments, agent-native, learnings-researcher.

Testing gaps:

  • No coverage for a nested group mixing checked/pinned and plain children (within-group order after priority sorts).
  • No test for numeric-aware ordering within a group (e.g. item2 vs item10) to lock in Intl.Collator({numeric: true}) behavior beyond the single attr9/attr10 case.

No finding blocks merge automatically. A maintainer will expect P0/P1 findings in code this PR changes to be fixed; P2/P3 are your call -- fix or reply. Never fix findings about surrounding code here; reply instead. Do not widen the PR. How to respond

@github-actions

github-actions Bot commented Oct 16, 2026 •

Copy link
Copy Markdown
Contributor

E2E Test Results

✅ All tests passed • 436 passed • 1 skipped • 1342s

Status Count
✅ Passed 436
❌ Failed 0
⚠️ Flaky 1
⏭️ Skipped 1

Tests ran across 4 shards in parallel.

View full report →

Comment thread packages/app/src/components/DBSearchPageFilters/NestedFilterGroup.tsx Outdated
Comment thread packages/app/src/components/__tests__/DBSearchPageFilters.test.tsx Outdated
@knudtty
knudtty force-pushed the aaron/alphabetize-keys branch from 6a351e8 to 5133ebe Compare October 16, 2026 20:10
Comment thread packages/app/src/components/DBSearchPageFilters.tsx
Comment thread packages/app/src/components/DBSearchPageFilters/utils.ts
Comment thread packages/app/src/components/DBSearchPageFilters/utils.test.ts
Comment thread packages/app/src/components/DBSearchPageFilters.tsx Outdated
Comment thread packages/app/src/components/DBSearchPageFilters.tsx
Comment thread packages/app/src/components/DBSearchPageFilters/utils.test.ts
Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
Comment thread packages/app/src/components/DBSearchPageFilters.tsx
Comment thread packages/app/src/components/DBSearchPageFilters/utils.ts
Comment thread packages/app/src/components/DBSearchPageFilters/utils.test.ts
@kodiakhq
kodiakhq Bot merged commit 8b92b7a into main Oct 19, 2026
28 checks passed
@kodiakhq
kodiakhq Bot deleted the aaron/alphabetize-keys branch October 19, 2026 14:43

This branch was successfully deployed

2 active deployments
Preview – hyperdx-oss — 352ead13 Deployed Oct 19, 2026 by vercel[bot]
Preview – hyperdx-storybook — 352ead13 Deployed Oct 19, 2026 by vercel[bot]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

automerge review/tier-2 Low risk — AI review + quick human skim

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants