[karl-power]

Summary

Clicking a search result could open the wrong row's details when the table persists only one of _block_number / _block_offset.

• Since fix(app): resolve row details with PK + whichever block column the table persists #3364 the side panel used a key-only lookup (primary key + partition + whichever block column exists) when at least one block column was persisted. One block column is not unique: _block_offset repeats across insert blocks, _block_number across the rows of one block, and the primary key often doesn't break the tie (the OTel exporter default keys on the second-precision TimestampTime).
• Since feat(app): fall back to server MergeTree settings for block columns #3530 the block columns are also detected from server-level merge_tree_settings, so on servers that enable only one of them, every table without explicit settings takes this path.
• The key-only lookup now requires both block columns. A lone block column is still appended to the full-row lookup as a tiebreaker.
• Tables that persist only one block column return to the full-row lookup and lose the fix(app): resolve row details with PK + whichever block column the table persists #3364 speed-up. A _part + _part_offset lookup is the follow-up to restore it.

How to reproduce

Starting point: yarn dev. Set the server default on the dev ClickHouse and restart it:

docker exec <ch-server> sh -c 'echo "<clickhouse><merge_tree><enable_block_offset_column>1</enable_block_offset_column></merge_tree></clickhouse>" > /etc/clickhouse-server/config.d/block_offset.xml'
docker restart <ch-server>

Create an exporter-shaped table and insert three rows as separate inserts, two in the same second:

CREATE TABLE default.repro_logs (
  Timestamp DateTime64(9), TimestampTime DateTime DEFAULT toDateTime(Timestamp),
  ServiceName LowCardinality(String), SeverityText LowCardinality(String), Body String,
  LogAttributes Map(LowCardinality(String), String)
) ENGINE = MergeTree PARTITION BY toDate(TimestampTime)
PRIMARY KEY (ServiceName, TimestampTime) ORDER BY (ServiceName, TimestampTime, Timestamp);

INSERT INTO default.repro_logs (Timestamp, ServiceName, SeverityText, Body) VALUES ('2026-11-05 09:00:39.845', 'saruman', 'INFO', 'Response x_span_id=ESLMARS7L7');
INSERT INTO default.repro_logs (Timestamp, ServiceName, SeverityText, Body) VALUES ('2026-11-05 09:00:39.100', 'saruman', 'INFO', 'Request x_span_id=ESOPOZ7FCV');
INSERT INTO default.repro_logs (Timestamp, ServiceName, SeverityText, Body) VALUES ('2026-11-05 09:00:36.909', 'saruman', 'INFO', 'Request x_span_id=ESLMARS7L7');

Add a log source for default.repro_logs (timestamp column TimestampTime), then search it for "ESLMARS7L7" over a range covering 2026-11-05 09:00 UTC.

On main:

1. Click the Response x_span_id=ESLMARS7L7 row a few times.
2. The side panel shows Request x_span_id=ESOPOZ7FCV about half the time. The query in system.query_log is WHERE ServiceName='saruman' AND TimestampTime=... AND toDate(TimestampTime)=... AND _block_offset='0' LIMIT 1.

On this branch:

1. Click the same row. The panel always shows Response x_span_id=ESLMARS7L7; the query also filters on Timestamp, SeverityText and Body.

How to test on Vercel preview

N/A — needs a ClickHouse server default; the demo server's user cannot read system.merge_tree_settings.

References

• Related PRs: feat(app): fall back to server MergeTree settings for block columns #3530, fix(app): resolve row details with PK + whichever block column the table persists #3364, feat: only use pk + block ids to resolve a row #2320, fix: use block_number/block_offset to uniquely identify log rows #2071

[vercel]

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

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

[changeset-bot]

🦋 Changeset detected

Latest commit: 02e09c8

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: 1
• Production lines changed: 33 (+ 86 in test files, excluded from tier calculation)
• Branch: claude/row-lookup-requires-both-block-columns
• Author: karl-power

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

[github-actions]

PR Review

✅ No issues found.

[github-actions]

Deep Review

✅ No critical issues found.

The change is a narrowly-scoped correctness fix: the fast key-only row-details lookup (rowKeyColumns as a Set) is now used only when a table persists both _block_number and _block_offset; a lone block column (plus __hdx_id) is appended to the full-row lookup as a tiebreaker only. The core logic was traced across all cardinality cases (0/1/2 block columns), the __hdx_id interaction, full-row uniqueness restoration via useRowWhere, and the useMemo dependency array — all sound. Tests were updated to match the new semantics.

🔵 P3 nitpicks (1)

• packages/app/src/components/DBRowTable.tsx:1637 — hasBothBlockColumns reads as "exactly two" but is derived from blockColumns.length === BLOCK_COLUMN_SETTINGS.length ("all persisted"); if a third entry is ever added to BLOCK_COLUMN_SETTINGS the name silently becomes misleading.
  • Fix: Add a brief comment noting the boolean means "all configured block columns are persisted," or keep the name aligned with the array semantics.

---

Reviewers (2): correctness, maintainability.

Testing gaps:

• No test exercises useRowWhere end-to-end to confirm a lone block column actually appears in the emitted WHERE clause when rowKeyColumns is undefined; current tests assert only the select / rowKeyColumns shape, not the generated SQL.

⚠️ Coverage note: This report was finalized before the adversarial, testing, kieran-typescript, performance, project-standards, and previous-comments reviewers returned. The two reviewers that completed (correctness, maintainability) found no must-fix issues. The prior PR comments are all automated bot output (Vercel, changeset-bot, triage, jscpd, e2e) with no substantive human feedback to verify.

_{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 • 461 passed • 1 skipped • 1347s

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

Tests ran across 4 shards in parallel.

View full report →