feat: deterministic sampling with adaptive sample size

[alex-fedotyev]

Summary

Closes #1827

Replaces non-deterministic ORDER BY rand() with deterministic cityHash64(SpanId) sampling and introduces sampling configuration constants.

What this PR does

• Deterministic sampling: ORDER BY cityHash64(SpanId) instead of rand() — same data always produces the same sample, so results are stable across re-renders
• Named constants: SAMPLE_SIZE, STABLE_SAMPLE_EXPR replace hardcoded 1000 and 'rand()' in query configs
• Adaptive sizing foundation: computeEffectiveSampleSize() function with MIN_SAMPLE_SIZE/MAX_SAMPLE_SIZE/SAMPLE_RATIO constants, exported and tested (6 unit tests)

What this PR does NOT do (follow-up)

• Count query for adaptive sizing: Wiring computeEffectiveSampleSize into the actual queries requires adding a lightweight count() query. This is deferred to keep this PR focused on the deterministic sampling change.
• Dynamic column detection: STABLE_SAMPLE_EXPR uses SpanId which is trace-specific. Event Deltas currently only renders on the traces search page where SpanId is always present. If the feature expands to logs/metrics, this should be parameterized per source (documented in code comment).

Test plan

• Same data + same hover always highlights the same heatmap cells (deterministic)
• Run npx jest src/components/__tests__/deltaChartSampling.test.ts — 6 tests pass

🤖 Generated with Claude Code

[vercel]

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

Project	Deployment	Actions	Updated (UTC)
hyperdx-oss	Ready	Preview, Comment	Mar 5, 2026 4:24pm

[changeset-bot]

🦋 Changeset detected

Latest commit: 980ab65

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

[github-actions]

Claude finished @alex-fedotyev's task in 58s —— View job

---

Code Review

• ⚠️ Dead code / incomplete wiring — STABLE_SAMPLE_EXPR, computeEffectiveSampleSize, and all sampling constants in deltaChartUtils.ts are only referenced in the test file. They're never imported into DBDeltaChart.tsx or any query builder, so the deterministic sampling and adaptive sizing described in the PR description aren't actually active yet. → Wire these into the actual query (e.g. the ORDER BY / LIMIT in the delta chart's ClickHouse query), or hold back until they're connected.

• 🔒 Hardcoded SpanId column — STABLE_SAMPLE_EXPR = 'cityHash64(SpanId)' will silently produce wrong results (or a ClickHouse error) on log/metric sources that don't have a SpanId column. → Use the source's spanIdExpression dynamically, or pick a universally-present column (e.g. cityHash64(timestamp, RowID)) as the stable hash key.

• ⚠️ Unrelated changes in the same PR — The onAddFilter prop added to DBDeltaChart.tsx (lines 47–55, 221–229, 421, 458) has nothing to do with sampling. This mixes two separate features, making review and rollback harder. → Split into its own PR, or clearly call this out in the description.

[github-actions]

E2E Test Results

✅ All tests passed • 86 passed • 3 skipped • 941s

Status	Count
✅ Passed	86
❌ Failed	0
⚠️ Flaky	2
⏭️ Skipped	3

Tests ran across 4 shards in parallel.

View full report →

[github-actions]

Claude finished @alex-fedotyev's task in 39s —— View job

---

Code Review

• ❌ computeEffectiveSampleSize is imported but never called (DBDeltaChart.tsx:25) → All three queries still use the hardcoded SAMPLE_SIZE (1000) constant; the adaptive sizing is dead code. The count query described in the PR summary also doesn't exist. Either wire up the count query + call computeEffectiveSampleSize with its result, or remove the import and the function until it's ready.

• 🔒 STABLE_SAMPLE_EXPR = 'cityHash64(SpanId)' hardcodes a trace-only column (deltaChartUtils.ts:306) → This will cause a ClickHouse error or silent wrong results on log/metric sources that don't have SpanId. Use a universally-present column (e.g., cityHash64(toString(timestamp))) or derive the expression dynamically from the source's schema.

[github-actions]

Claude finished @alex-fedotyev's task in 37s —— View job

---

Code Review

• ❌ computeEffectiveSampleSize is never called — the three queries all use the static SAMPLE_SIZE = 1000. The PR description promises "adaptive sample sizing" but it's not wired up. Either call it with a count query result or remove it until ready. Fix this →

• ⚠️ STABLE_SAMPLE_EXPR = 'cityHash64(SpanId)' is trace-only (deltaChartUtils.ts:309) — will cause a ClickHouse error on log/metric sources without a SpanId column. Use a universally-present column like cityHash64(toString(timestamp)), or accept the current scope limitation with a clear code comment (one already exists, which is good, but the risk is real if the component is ever reused).

[alex-fedotyev]

@claude - repeat code review against the updated PR description and give aligned feedback

[github-actions]

Claude finished @alex-fedotyev's task in 1m 17s —— View job

---

Code Review (aligned with PR description)

• Read changed files
• Post aligned review

---

The latest commit (70023920) cleaned up the previously flagged unused import of computeEffectiveSampleSize. Here's a review aligned with what the PR actually claims to do and explicitly defers:

What this PR does well:

• All 3 queries in DBDeltaChart.tsx correctly swap ORDER BY rand() → ORDER BY cityHash64(SpanId) DESC and 1000 → SAMPLE_SIZE — the core deterministic sampling is live and working.
• Named constants replace magic numbers throughout, making future adjustments trivial.
• The SpanId scope limitation is clearly documented inline (deltaChartUtils.ts:298–304) — no silent failure risk for the current use case (traces-only page).
• computeEffectiveSampleSize is exported, tested (6 passing tests), and not imported in production code — this correctly matches the PR's stated intent to defer the count query wiring.

One genuine concern not previously raised:

• ⚠️ ORDER BY … DESC LIMIT is a biased top-N, not a statistical sample (DBDeltaChart.tsx:141, 196, 205) — ORDER BY cityHash64(SpanId) DESC LIMIT 1000 always returns the same 1000 rows: those with the largest hash values. For the stated goal of stable hover highlights across re-renders, this is exactly right. But it's worth knowing this isn't a statistically representative sample — if the data is large enough that 1000 rows is a meaningful subset, the selected rows are always a fixed population (highest-hash spans), not a random draw. This could matter if, e.g., certain span types cluster in hash space. For the current heatmap use case it's acceptable, but worth a brief comment like // always selects the same top-N rows by hash — biased but stable across renders.

Previous review concerns vs. current state:

Concern	Status
computeEffectiveSampleSize imported but unused	✅ Fixed in latest commit
Adaptive sizing not wired	✅ Explicitly deferred — PR description is clear
STABLE_SAMPLE_EXPR / SpanId scope	✅ Documented with code comment; scoped risk is acceptable
Unrelated onAddFilter changes	Not present in current diff — may have been on an older version

Overall the implementation matches the PR description accurately. The only call-out is the "biased sample" nuance, which is a minor documentation gap rather than a bug.

[github-actions]

Claude finished @alex-fedotyev's task in 22s —— View job

---

Code Review

✅ No critical issues found.

The implementation matches the PR description accurately:

• ORDER BY cityHash64(SpanId) DESC LIMIT 1000 is correctly wired into all 3 queries
• computeEffectiveSampleSize is exported/tested but intentionally not called yet (deferred per PR description)
• SpanId scope limitation is documented inline

One minor note: ORDER BY cityHash64(SpanId) DESC LIMIT 1000 is a deterministic top-N (always the same 1000 rows), not a statistical sample. This is correct for stable hover highlights but worth knowing if representative sampling is ever needed.

[pulpdrew]

This should refer to the source's spanIdExpression - it may not always be SpanId. If the user has a custom schema, I suspect this would break.

[alex-fedotyev]

Good catch — fixed in 980ab65. Replaced the hardcoded STABLE_SAMPLE_EXPR constant with getStableSampleExpression(spanIdExpression?) that accepts the source's spanIdExpression (passed from DBSearchHeatmapChart which already has the source). Falls back to rand() when not available.

[github-actions]

Claude finished @alex-fedotyev's task in 38s —— View job

---

Code Review

✅ No critical issues. The implementation matches the PR description accurately.

• getStableSampleExpression(spanIdExpression?) correctly uses the source's spanIdExpression (passed from DBSearchHeatmapChart.tsx:122) and falls back to rand() — the previously flagged hardcoded SpanId concern is resolved.
• All 3 queries in DBDeltaChart.tsx use stableSampleExpr and SAMPLE_SIZE — deterministic sampling is wired and active.
• computeEffectiveSampleSize is exported + tested but intentionally not called — matches the PR's stated deferral.
• JSDoc comments are now on each constant as suggested by the reviewer.

One minor note (not blocking): ORDER BY cityHash64(spanId) DESC LIMIT 1000 is a deterministic top-N, not a statistically representative sample — it always selects the same highest-hash rows. This is the correct behavior for stable hover highlights, but worth a brief inline comment if representative sampling is ever a future goal.