chore: promote staging to main - #722
Conversation
Rebuilt analytics.custom_events and swapped it in via EXCHANGE TABLES ON CLUSTER: 106.01 MiB -> 59.54 MiB (-43.9%), 433 -> 32 parts, 315 -> 12 partitions. Drops delivery_id entirely. Four days of production query_log show zero SELECTs against it; nothing in the codebase reads it. Its real uses are write-path only - an in-memory dedup key in event-service and the Kafka message key in producer - and the table is ReplicatedMergeTree, not Replacing, so it was never a storage-level dedup key either. It was 37.6 MiB of incompressible SHA-256 hex, 36% of the table. Basket still generates it for Kafka keying; Vector has skip_unknown_fields so the now-unknown field is ignored on insert. Partitioning moves toDate -> toYYYYMM. Daily partitions held ~8.9k rows, about one granule each, which set a floor of one granule read per partition in range: a 31-day scan read 257,516 rows and now reads 16,384 (15.7x). Codecs measured in isolation on a full copy rather than combined: properties ZSTD(1) -> ZSTD(6) (-13.3%); LowCardinality on owner_id (-95%) and website_id. Left timestamp on Delta - DoubleDelta measured 28% worse because timestamp is third in the sort key and only monotonic within runs. Left Nullable on anonymous_id/session_id - removing it measured no gain. The previous table is retained as analytics.custom_events_v2 pending verification.
Bumps every pin that names a runtime version: the packageManager field in the root and the three apps that carry their own, bun-version across ten workflow steps, and the oven/bun base images in all eight Dockerfiles. @types/bun stays on 1.4.0 because 1.4.1 is not published; the types trail the runtime release. Two changes in 1.4.1 touch this repo and are worth watching on the first CI run. localhost now bypasses the system resolver, and the health check talks to services on localhost. TLS verification now reads the URL hostname rather than the Host header, and the links and basket producers reach redpanda over TLS. (cherry picked from commit 375a3b2)
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: ASSERTIVE Plan: Team Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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. Comment |
Greptile SummaryThis PR promotes several staging changes: it upgrades build and runtime environments to Bun 1.4.1, changes the
Confidence Score: 4/5This PR should not merge until the custom-events schema change has a tracked forward migration and the explicit-type repository requirement is satisfied. Existing ClickHouse tables will ignore the edited bootstrap DDL, leaving environments on the old partition and column shape and causing verification drift; the new test also violates an explicit repository typing rule. Files Needing Attention: packages/db/src/clickhouse/schema/analytics/core/custom_events.sql; packages/ai/src/query/builder-execution.test.ts Important Files Changed
Reviews (1): Last reviewed commit: "chore(deps): upgrade bun to 1.4.1" | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| ) | ||
| ENGINE = ReplicatedMergeTree('/clickhouse/tables/{shard}/analytics_custom_events', '{replica}') | ||
| PARTITION BY toDate(timestamp) | ||
| PARTITION BY toYYYYMM(timestamp) |
There was a problem hiding this comment.
This changes the reference table to monthly partitions and removes delivery_id, but existing installations will not receive those changes. clickhouse:init executes CREATE TABLE IF NOT EXISTS, which is a no-op for a deployed table, and this PR does not include the required forward migration. Environments that were not manually migrated will retain the old schema, fail schema verification, and miss the intended partition optimization. Add a tracked shadow-table migration that can be applied consistently to every environment.
Knowledge Base Used: Data storage and schema
| events: unknown[]; | ||
| session_id: string; | ||
| web_vitals: unknown[]; |
There was a problem hiding this comment.
The new test declares events and web_vitals as unknown[]. This violates the repository directive to use proper explicit types instead of any, unknown, or never, and it prevents TypeScript from detecting malformed query output. Define explicit tuple types for the activity and web-vital rows. This repository requirement must be satisfied before merging.
Context Used: Basic guidelines for the project so vibe coders do... (source)
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
|
The latest updates on your projects. Learn more about Unkey Deploy
|
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
2 issues found and verified against the latest diff
Confidence score: 2/5
packages/db/src/clickhouse/schema/analytics/core/custom_events.sqlomitsdelivery_ideven though fresh-bootstrap delivery sends it, which can break custom-event ingestion; restore the column or coordinate producer and migration changes.- The same reference DDL change will not alter the existing
analytics.custom_eventstable, leaving production on the old schema after promotion; add and run a forward shadow-table/backfill/swap migration before updating the reference.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/db/src/clickhouse/schema/analytics/core/custom_events.sql">
<violation number="1" location="packages/db/src/clickhouse/schema/analytics/core/custom_events.sql:13">
P1: On a fresh bootstrap, custom-event delivery sends `delivery_id` but this definition no longer declares it. Restore the column here; removing it would require a coordinated producer and migration change.</violation>
<violation number="2" location="packages/db/src/clickhouse/schema/analytics/core/custom_events.sql:21">
P2: Changing this reference DDL does not repartition the existing `analytics.custom_events` table. Add and run a forward shadow-table/backfill/swap migration before updating the reference, otherwise the promoted production table remains daily-partitioned.</violation>
</file>
Shadow auto-approve: would not auto-approve because issues were found.
Re-trigger cubic
| `profile_id` String DEFAULT '', | ||
| `delivery_id` String DEFAULT '' CODEC(ZSTD(1)), | ||
| `source` LowCardinality(Nullable(String)), | ||
| `profile_id` String DEFAULT '' CODEC(ZSTD(1)), |
There was a problem hiding this comment.
P1: On a fresh bootstrap, custom-event delivery sends delivery_id but this definition no longer declares it. Restore the column here; removing it would require a coordinated producer and migration change.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/db/src/clickhouse/schema/analytics/core/custom_events.sql, line 13:
<comment>On a fresh bootstrap, custom-event delivery sends `delivery_id` but this definition no longer declares it. Restore the column here; removing it would require a coordinated producer and migration change.</comment>
<file context>
@@ -1,24 +1,23 @@
- `profile_id` String DEFAULT '',
- `delivery_id` String DEFAULT '' CODEC(ZSTD(1)),
+ `source` LowCardinality(Nullable(String)),
+ `profile_id` String DEFAULT '' CODEC(ZSTD(1)),
INDEX idx_event_name event_name TYPE bloom_filter(0.01) GRANULARITY 1,
INDEX idx_namespace namespace TYPE bloom_filter(0.01) GRANULARITY 1,
</file context>
| `profile_id` String DEFAULT '' CODEC(ZSTD(1)), | |
| `profile_id` String DEFAULT '' CODEC(ZSTD(1)), | |
| `delivery_id` String DEFAULT '' CODEC(ZSTD(1)), |
| ) | ||
| ENGINE = ReplicatedMergeTree('/clickhouse/tables/{shard}/analytics_custom_events', '{replica}') | ||
| PARTITION BY toDate(timestamp) | ||
| PARTITION BY toYYYYMM(timestamp) |
There was a problem hiding this comment.
P2: Changing this reference DDL does not repartition the existing analytics.custom_events table. Add and run a forward shadow-table/backfill/swap migration before updating the reference, otherwise the promoted production table remains daily-partitioned.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/db/src/clickhouse/schema/analytics/core/custom_events.sql, line 21:
<comment>Changing this reference DDL does not repartition the existing `analytics.custom_events` table. Add and run a forward shadow-table/backfill/swap migration before updating the reference, otherwise the promoted production table remains daily-partitioned.</comment>
<file context>
@@ -1,24 +1,23 @@
)
ENGINE = ReplicatedMergeTree('/clickhouse/tables/{shard}/analytics_custom_events', '{replica}')
-PARTITION BY toDate(timestamp)
+PARTITION BY toYYYYMM(timestamp)
ORDER BY (owner_id, event_name, timestamp)
SETTINGS index_granularity = 8192
</file context>
There was a problem hiding this comment.
1 existing issue remains and no new issues found across 6 files (changes from recent commits).
Confidence score: 3/5
packages/db/src/clickhouse/schema/analytics/core/custom_events.sqlleaves the old Keeper path on existing clusters, soclickhouse:initcan report schema drift and skip the intended repartition migration; add an explicit upgrade path for existing clusters and verify the migration completes.
Shadow auto-approve: would not auto-approve. Auto-approval blocked by 2 unresolved issues from previous reviews.
Re-trigger cubic
There was a problem hiding this comment.
All reported issues were addressed across 6 files (changes from recent commits).
Shadow auto-approve: would not auto-approve because issues were found.
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
There was a problem hiding this comment.
1 issue found across 2 files (changes from recent commits).
Confidence score: 3/5
packages/db/src/clickhouse/migrations/20260904_custom_events_repartitioning.mdleavesanalytics.custom_events_v2stranded after section-7 recovery while writers resume on the legacy table, so new events can be absent from the final schema; ensure recovery resumes writes to the final schema or explicitly reconciles the cutover before proceeding.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/db/src/clickhouse/migrations/20260904_custom_events_repartitioning.md">
<violation number="1" location="packages/db/src/clickhouse/migrations/20260904_custom_events_repartitioning.md:557">
P2: After a section-7 recovery, writers resume on the recovered legacy `analytics.custom_events`, but `analytics.custom_events_v2` (final schema) is stranded at the recovery cutover and receives no new rows. So by the time you "return to this release," the two tables do not have equal row multisets: every row committed since the recovery exists only in the legacy `custom_events`. The documented forward `EXCHANGE TABLES` cannot merge those rows into the new canonical table — the exchange only swaps names — so either the equal-multiset check fails and the operator is stuck with no given next step, or the operator forces the exchange and the post-recovery writes are dropped when the retained legacy table is deleted. The return procedure must first copy the post-recovery delta into `analytics.custom_events_v2` (the same direction-1 delta copy used in section 7 step 3, with the recovered legacy table as the extra_rows source and `custom_events_v2` as the target) before dropping the views and exchanging forward.</violation>
</file>
Shadow auto-approve: would not auto-approve because issues were found.
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
There was a problem hiding this comment.
All reported issues were addressed across 3 files (changes from recent commits).
Shadow auto-approve: would not auto-approve because issues were found.
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
There was a problem hiding this comment.
0 issues found across 4 files (changes from recent commits).
Confidence score: 5/5
- Automated review surfaced no issues in the provided summaries.
- No files require special attention.
Shadow auto-approve: would not auto-approve. Auto-approval blocked by 2 unresolved issues from previous reviews.
Re-trigger cubic
1 participant
Summary
Validation
bun run lintbun run check-typesbun run testSummary by cubic
Promotes staging's release set to main with monthly
analytics.custom_eventspartitions, the profile-query alias fix, a Vector canary update, and Bun 1.4.1. Custom events no longer storedelivery_id; Basket keeps it only for Kafka keys and ClickHouse dedup tokens while stripping it from direct ClickHouse fallback rows, and non-custom Kafka keys are unchanged. Profile queries now include telemetry for aliases established only by custom events.Migration
analytics.custom_events_v2for verification and rollback.delivery_id, so Kafka payloads need no change.properties.canary_id.Dependencies
Written for commit 5684a40. Summary will update on new commits.