[willie-engelbrecht]

Summary

Line colours on panels are assigned by series position, and ClickHouse returns groups in a different order as the time window moves, based on which blocks were processed at that particular point in time. This PR resolves by sorting the series alphabetically first, and then assigning the colour based on the now sorted alphabetical sorted list. Refreshing a graph now keeps the series colours consistent.

Screenshots

Before: Here you can see the colours assigned to each series. When reloading a page, or updating the time interval to a new internal, the series colours are assigned at somewhat random order:

After a second refresh:

And another refresh:

After: Now the colours assigned to each series remains the same, no matter how many times you refresh the page or update the time picker.

How to reproduce

Before:
Open a graph with multiple lines, and change the time picker to "last 15 minutes". Watch the colours selected on the graph. Keep repeating this process and you'll see after a few refreshes that the colours are seemingly assigned in random order to the same series. This is because ClickHouse does not guarantee the order of the returned series.

After: Do the same as above, but now the series colours remain the same no matter how many times you refresh. This creates a richer user experience as a user might expect the graph to remain the same colours as they are used.

[changeset-bot]

🦋 Changeset detected

Latest commit: 39f89a1

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]

@willie-engelbrecht is attempting to deploy a commit to the HyperDX Team on Vercel.

A member of the Team first needs to authorize it.

[github-actions]

Hi @willie-engelbrecht, thanks for the pull request!

Before we review code from a first-time contributor, a maintainer vouches for them, and you're not on our list yet. There's nothing else to file and this PR stays open — a maintainer will look at it, usually within a day or two, and vouch for you right here if it's a genuine attempt at the problem.

To make that quick, check the PR description has:

• The issue it addresses (ideally one labelled Accepting PR) and why the change is needed
• Steps to reproduce the problem and verify the fix, starting from https://play.hyperdx.io or yarn app:dev:local connected to the demo server
• Before/after screenshots if anything in the UI changed

More detail in our contributing guide.

[github-actions]

🟠 major — Colours still depend on position, so they shift whenever the set of series changes

Sorting fixes colours only when the same series come back in a different order. setLineColors (ChartUtils.tsx:637) still hands out colorIndex++ by position, and the set of series often changes when the user moves the time window, which is the repro in the PR description. Example: refresh 1 returns cart, checkout, email, so checkout gets COLORS[1]. Refresh 2 has no rows for cart, so checkout moves to COLORS[0] and email to COLORS[1]. The maxSeries cap causes the same shift when the kept top-N changes. To make a series' colour independent of what else is in the result, pick the palette index from the series key, e.g. hashCode(currentPeriodKey) from @hyperdx/common-utils/dist/core/utils modulo the palette size. Add a test where one series is missing from the second response.

_{Nothing blocks merge automatically, but a maintainer will expect this fixed if it is a real defect in code this PR changes. If it is about surrounding code, reply and say so instead of patching. Do not widen the PR. How to respond}

[github-actions]

🔵 minor — Comparing strings with < puts numeric group values out of order

currentPeriodKey comparison is by UTF-16 code unit, so numeric group-bys (status codes, partition ids, durations) come out as '10', '100', '9', and uppercase names sort before lowercase. The changeset says series are now 'ordered by name', and this comparison decides the series order in the chart, so the user sees this ordering. Use a.currentPeriodKey.localeCompare(b.currentPeriodKey, undefined, { numeric: true }), or an Intl.Collator with numeric: true created once outside the comparator.

_{Advisory. Fix if it is a small defect in code this PR changes; otherwise reply in the thread. Do not widen the PR or touch files it did not already change. How to respond}

[github-actions]

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 · 🟠 1 major · 🔵 2 minor

2 posted as inline comment(s) on the changed lines. 1 listed below.

Findings outside the changed lines

1 minor

• 🔵 packages/cli/src/shared/chartData.ts:342 — CLI port of formatResponseForTimeChart did not get the same tie-break → packages/cli/src/shared/chartData.ts is marked @source packages/app/src/ChartUtils.tsx (formatResponseForTimeChart…). Its sortedSeries sort (lines 342-346) still compares only log-level colour, and colours are then assigned by position (lines 386-391). Terminal tile renders therefore still swap series colours between runs, and the two copies now behave differently. Apply the same key tie-break there, or better, move the shared ordering and colour logic into common-utils.

---

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

[github-actions]

Deep Review

Scope: packages/app/src/ChartUtils.tsx series-sort comparator, its test, and a changeset. Base c17fc16a. Intent: tie-break the time-chart series sort on currentPeriodKey so position-assigned colors stay stable across refreshes despite ClickHouse's non-deterministic row order.

✅ No critical issues found. The comparator is correct: the || short-circuit binds as (colorRankA - colorRankB) || (alphabetical tiebreak), non-log series tie at findIndex === -1 and fall through to the deterministic key compare, and current/previous pairs share a currentPeriodKey so they keep one color via setLineColors regardless of order. Code-unit </> comparison (not localeCompare) is the right choice for cross-environment determinism.

🟡 P2 — recommended

• packages/app/src/ChartUtils.tsx:883 — the fix stabilizes colors only when the series set is identical between refreshes; when a high-cardinality chart trips the series cap, survivor membership is chosen by per-group peak value (peakByGroup), which varies with the time window, so a group entering or leaving the kept set still shifts later series' positions and reassigns their colors.
  • Fix: add a test (and/or scope note) covering the cap-trips case so the stability guarantee's boundary is explicit rather than implied by the fixed-set reorder test alone.
  • _{correctness-reviewer, testing-reviewer}

🔵 P3 nitpicks (2)

• packages/app/src/ChartUtils.tsx:879 — the nested ternary for the string tiebreak is a recognized no-nested-ternary smell; if simplified, keep a code-unit-safe compare (a < b ? -1 : a > b ? 1 : 0 or a small helper) and do not switch to localeCompare, which is locale-sensitive and would reintroduce cross-environment ordering variance.

  • Fix: extract a compareByKey helper or leave as-is; avoid localeCompare.

• packages/app/src/__tests__/ChartUtils.test.ts:978 — the cap-tiebreak test comment ("keep the first two encountered (g0, g1)") now describes alphabetical order rather than response order, since the new sort feeds the cap's index tiebreak; it reads correctly only because the input is already alphabetical.

  • Fix: update the comment to say the survivors are the alphabetically-first groups, or add a shuffled-input case that distinguishes the two.

---

Reviewers (6): correctness, testing, maintainability, project-standards, kieran-typescript, performance.

Testing gaps: New color-stability test covers only row-reorder of a fixed 3-series non-log set; no coverage for the cap-trips path or for log-level-colored series under reordered rows (the latter is partially locked by the updated static log-level expectation).

_{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}