Skip to content

fix(db): return the newest events, not an arbitrary slice - #476

Open
avionicharshit-byte wants to merge 1 commit into
Openpanel-dev:mainfrom
avionicharshit-byte:fix/query-events-order-by
Open

fix(db): return the newest events, not an arbitrary slice#476
avionicharshit-byte wants to merge 1 commit into
Openpanel-dev:mainfrom
avionicharshit-byte:fix/query-events-order-by

Conversation

@avionicharshit-byte

@avionicharshit-byte avionicharshit-byte commented Sep 2, 2026

Copy link
Copy Markdown

fixes #475.

queryEventsCore ended with .limit(input.limit ?? 20).execute() and no ORDER BY, so clickhouse returned whatever it read first. events sort on (project_id, toDate(created_at), created_at, name) partitioned by month, so a bare limit hands back an arbitrary slice of the window. every caller labels those rows as recent: the mcp tool passes sortedBy: 'created_at desc', zLimit tells the model results are ranked, and the agent tools call them recent events.

sorting before the limit. clickhouse can read the sort key in reverse for this, so it is not a full sort of the window.

measured on a local clickhouse with the same sort key and partitioning, 90 days of one profile's events, one per day, limit 5. before: 2026-09-01, then 2026-07-01 through 2026-07-04. after: 2026-09-01 back to 2026-08-28.

the query builder is split out so the shape can be tested, same as buildProfileMetricsSql. the ordering test fails on main.

Summary by CodeRabbit

  • Improvements

    • Event results are now consistently sorted from newest to oldest.
    • Event queries continue to support filtering and configurable result limits, with a default limit of 20.
  • Tests

    • Added coverage for event query ordering, filtering, limits, and ClickHouse compatibility.

@coderabbitai

coderabbitai Bot commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Team

Run ID: a01cfc68-72e8-48b7-b27a-66df28419ebf

📥 Commits

Reviewing files that changed from the base of the PR and between bad75bd and f48aa6d.

📒 Files selected for processing (2)
  • packages/db/src/services/event.service.ts
  • packages/db/src/services/query-events-sql.test.ts

Included review availability: Your plan provides up to 8 included reviews per hour; 6 remain after this review.


📝 Walkthrough

Walkthrough

The event service now exposes a reusable query builder. Event results are ordered by created_at descending before the limit is applied. New SQL-shape tests validate ordering, limits, filters, and ClickHouse parsing.

Changes

Event query ordering

Layer / File(s) Summary
Build and validate ordered event queries
packages/db/src/services/event.service.ts, packages/db/src/services/query-events-sql.test.ts
buildQueryEventsQuery returns the unexecuted event query. queryEventsCore executes the builder. Queries now apply created_at DESC before the default or supplied limit. Tests verify SQL ordering, limits, filters, and ClickHouse parsing.

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: ⚪ Minimal · up to f48aa

The change makes limited event queries return the newest records as expected by callers, with no identified user-impacting or production merge-blocking risk remaining beyond normal checks and review.

Suggested reviewers: lindesvard, niajkitir

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 2 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly describes the primary change: returning newest events instead of an arbitrary limited slice.
Linked Issues check ✅ Passed The change satisfies issue #475 by applying created_at DESC ordering before the limit, extracting the query builder for validation, and adding tests for ordering, limits, filters, and ClickHouse SQL v…
Out of Scope Changes check ✅ Passed All changes are directly related to issue #475. The query-builder extraction and SQL-shape tests support the required ordering fix and introduce no unrelated behavior.
Full details: Linked Issues check

Explanation

The change satisfies issue #475 by applying created_at DESC ordering before the limit, extracting the query builder for validation, and adding tests for ordering, limits, filters, and ClickHouse SQL validity.

  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@avionicharshit-byte
avionicharshit-byte marked this pull request as ready for review September 2, 2026 15:05
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.

query_events and GET /:projectId/events return an arbitrary slice, not the newest events

1 participant