Skip to content

fix(analytics): get_analytics start_time/end_time filters returned zero rows#751

Open
cb1kenobi wants to merge 5 commits into
mainfrom
fix/analytics-start-end-time-compound-key
Open

fix(analytics): get_analytics start_time/end_time filters returned zero rows#751
cb1kenobi wants to merge 5 commits into
mainfrom
fix/analytics-start-end-time-compound-key

Conversation

@cb1kenobi
Copy link
Copy Markdown
Member

Summary

get_analytics returned an empty array whenever start_time or end_time was supplied. list_metrics(['custom']) was silently broken in the same way.

Root cause

hdb_analytics stores its primary key id as the compound array [timestamp, nodeId] (see resources/analytics/write.tsstoreMetric). But the read path was passing scalar timestamps as the bound for greater_than_equal / less_than / between conditions on id. Harper's key encoding sorts scalar numbers and arrays into different ranges, so a scalar bound lands past the array key space and the range iterator returns nothing. The response had been hiding this by post-processing result.id = result.id[0] before returning to the client, so users saw scalar ids in responses and naturally assumed they could filter on them directly.

Fix

Wrap each bound as a single-element array — e.g. value: [startTime]. Because [X] sorts before [X, anything] in the compound key ordering, this gives the correct inclusive lower bound and exclusive upper bound semantics against the [timestamp, nodeId] key space.

While I was in there:

  • Collapsed the prior if (startTime && endTime) / else branching into two independent if blocks. The both-bounds case used to go through between (inclusive end); it now uses less_than (exclusive end), matching the single-bound branch. Since the old code returned zero rows in real use, no caller can have depended on the inclusive-end behavior.
  • Switched the truthiness check from if (startTime) to != null so a 0 timestamp is accepted.
  • Same one-line fix applied to listMetrics' customWindow cutoff condition, which was scalar against the same compound key.

Where to look

  • The behavior change from between (inclusive end) to half-open [start, end) is the only semantic shift. Since the previous behavior was broken, this is a fresh choice rather than a regression — but worth a glance to confirm half-open is what we want for analytics queries.
  • Two separate id conditions vs one between — I don't believe search.ts collapses two same-attribute primary-key conditions into a single range scan, but the cost is negligible for this read path. Flagging in case there's a known reason to prefer between.

Tests

  • Added 5 unit tests covering get(): no time filter, startTime only, endTime only, both bounds, and startTime: 0 (the falsy-edge fix).
  • Updated 3 existing listMetrics tests to assert the wrapped-array bound.

Cross-model review

Reviewed via Gemini CLI — no blockers; the one note was a pre-existing return-type looseness on get() (Promise<Metric[]> vs the actual async iterable when coalesceTime is set), which is out of scope.

Generated by Claude (Opus 4.7).

…lytics filter

The hdb_analytics table stores its primary key as the compound array
[timestamp, nodeId], but get_analytics was passing scalar timestamps for
start_time/end_time/customWindow filters. Harper's key encoding sorts scalar
numbers and arrays into different ranges, so the range iterator landed past
all rows and returned zero results. The response had been hiding this by
post-processing result.id down to result.id[0] before returning to the client.

Wrap each bound as a single-element array so the comparison stays inside the
array key space ([X] sorts before [X, anything]). Also collapsed the
start && end branching to two independent ifs and switched the truthiness
check to != null so a 0 timestamp is treated as valid.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@claude
Copy link
Copy Markdown
Contributor

claude Bot commented May 23, 2026

Reviewed; no blockers found.

@cb1kenobi cb1kenobi changed the title fix(analytics): get_analytics start_time/end_time filters returned zero rows WIP: fix(analytics): get_analytics start_time/end_time filters returned zero rows May 23, 2026
@cb1kenobi cb1kenobi marked this pull request as ready for review May 23, 2026 03:37
@cb1kenobi cb1kenobi changed the title WIP: fix(analytics): get_analytics start_time/end_time filters returned zero rows fix(analytics): get_analytics start_time/end_time filters returned zero rows May 23, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant