[SpencerTorres]

Summary

Fixes a query that was causing the session player to show a warning/error on replay:

Browser SDKs up to @clickhouse/clickstack-browser 1.2.0 restart replay event numbers at 1 on every page load. When a page closed before all chunks of a large event were sent, the replay player joined the leftover chunk with the next page's event of the same number. The joined JSON failed to parse, and that page's full snapshot was dropped, so its part of the replay rendered blank. This was seen in production as Unexpected non-whitespace character after JSON.

Changes

• Chunk grouping (rrwebChunkAssembler.ts): pending chunks are keyed by event number plus full-precision timestamp. All chunks of one event share its timestamp, and different page loads don't. The duplicate-chunk guard is unchanged.
• Replay query (sessions.ts): selects Timestamp and orders by TimestampTime, Timestamp, offset, chunk, so rows of two page loads in the same second no longer interleave. TimestampTime stays first to match the table's sort key. If the session table has no Timestamp column, the query uses the source's own timestamp instead.
• SDK (package.json, yarn.lock): the app's own browser telemetry moves to @clickhouse/clickstack-browser 1.2.1, whose event numbers no longer repeat across page loads (fix: 1.2.1 event sequencing clickstack-browser#18)

No data is lost, this is fixed by this query update + the browser SDK patch I published.

[vercel]

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

Project	Deployment	Actions	Updated
hyperdx-oss	Ready	Preview	Nov 10, 2026 1:56am UTC
hyperdx-storybook	Ready	Preview	Nov 10, 2026 1:56am UTC

[changeset-bot]

🦋 Changeset detected

Latest commit: 3e5aaf2

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]

🔵 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: 3
• Production lines changed: 163 (+ 196 in test files, excluded from tier calculation)
• Branch: spencer/replay-chunk-timestamp-fix
• Author: SpencerTorres

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

[github-actions]

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 110 in total (1.21%).
Run yarn dupes locally to reproduce, or yarn dupes:all for the full picture.

[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.

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

None could be anchored to changed lines; they are listed below.

Findings outside the changed lines

1 minor

• 🔵 packages/app/src/utils/__tests__/rrwebChunkAssembler.test.ts:425 — 'Large event ids' test also passes on the pre-PR assembler, so it guards nothing → toNumber is Number(value), and both 2^52-1 and 2^52 are exact safe integers. The old code used a Map<number, …> key, so these chunks never collided there either. Either remove the test, or make it cover the risk this PR adds: two ids above 2^53 (e.g. '9007199254740993' and '9007199254740992') that round to the same Number. Those collide in the new ${eventId}:${timestamp} key too, unless the key uses the raw row.ev string.

---

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

✅ No critical issues found. This is a well-documented, well-tested bugfix: the chunk assembler now keys pending events by eventId:timestamp, the replay query selects/orders by a full-precision Timestamp, and a fallback keeps replay working when the column is absent. No P0/P1 production-breakage was identified in the diff. The items below are recommended hardening and nits.

🟡 P2 -- recommended

• packages/app/src/sessions.ts:493 -- SESSION_PRECISE_TIMESTAMP_COLUMN = 'Timestamp' bakes an OTel-specific column name into the replay path; a non-OTel session source whose table happens to expose a differently-purposed Timestamp column would be grouped/ordered by the wrong column instead of its mapped event time.
  • Fix: Source the precise-timestamp column from the TSessionSource mapping rather than a hardcoded module constant, or track the coupling as a follow-up tied to the existing // FIXME: add mappings to session source.
  • _{maintainability}

🔵 P3 nitpicks (1)

• packages/app/src/sessions.ts:622 -- the orderBy de-duplication via new Set([source.timestampValueExpression, preciseTimestampExpression]) collapses the two leading sort keys only on byte-identical strings, so two expressions that denote the same column but differ textually would emit a redundant (harmless) sort term; the dedup intent is implicit and brittle to future changes in getFirstTimestampValueExpression.
  • Fix: Derive both sort keys from one source of truth, or add a short comment stating the string-equality dedup is intentional.

---

Reviewers (5 returned, 6 did not report in time): maintainability, performance, previous-comments, agent-native, learnings. Supplemented by orchestrator analysis of the full diff and its dependencies (metadata.getColumn/getColumns, getFirstTimestampValueExpression, the three getRRWebTimestampExpression branch tests). The correctness, adversarial, testing, kieran-typescript, reliability, and project-standards sub-agents were dispatched but had not returned when output was finalized, so coverage of logic-edge-case and test-gap dimensions is partial.

Residual risks:

• The fix relies on the invariant that all chunks of one event carry the identical full-precision Timestamp. If a recorder/SDK ever stamps a single event's chunks with differing timestamps, the eventId:timestamp key would split one event into incomplete entries and drop it. This is an SDK-side guarantee not verifiable from this repo.
• When the session table lacks Timestamp, preciseTimestampExpression falls back to second precision, so two page loads in the same second can still collide on the eventId: key — a documented degraded mode, no worse than the prior behavior but not loud at runtime.
• getRRWebTimestampExpression awaits a metadata.getColumn lookup before each stream fetch; on a cold/expired columns cache this adds one DESCRIBE round-trip to the critical path of the first replay fetch.

Testing gaps:

• No assembler test exercises the empty-timestamp (ts === '') fallback path to pin the documented degraded same-second grouping behavior.
• No coverage for an event split across a LIMIT/offset pagination boundary.
• No test confirms the columns cache stays warm across repeated/polling useRRWebEventStream invocations (the assumption that keeps the metadata lookup off the per-fetch round-trip path).

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

E2E Test Results

✅ All tests passed • 463 passed • 1 skipped • 1032s

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

Tests ran across 4 shards in parallel.

View full report →