Skip to content

perf(analytics): precompute provider state - #2228

Merged
Asherlc merged 2 commits into
mainfrom
Asherlc/fix-sentry-7632766197
Jul 28, 2026
Merged

Asherlc merged 2 commits into
mainfrom
Asherlc/fix-sentry-7632766197

Conversation

@Asherlc

@Asherlc Asherlc commented Jul 27, 2026 •

Copy link
Copy Markdown
Owner

Summary

  • add an aggregate ClickHouse projection that maintains exact latest deletion state per provider record
  • make provider_stats prefer the projection while retaining the existing exact fallback semantics
  • add real-ClickHouse regression coverage for cross-part resurrection, planner selection, bounded-memory counting, and explicit historical materialization
  • document the production evidence, rollout, and verification steps for DOFEK-SERVER-5A

Root cause

Dirty-provider discovery was already bounded to one provider, but the exact metric-stream count still rebuilt an argMax hash entry for each of Apple Motion's 33.7 million record IDs. That query exceeded the unchanged four-minute worker ceiling. The existing provider projection pruned the range but did not precompute current state.

Validation

  • DBT_CLICKHOUSE_SCHEMA=analytics_lint pnpm lint
  • pnpm tsc --noEmit
  • cd packages/server && pnpm tsc --noEmit
  • cd packages/web && pnpm tsc --noEmit
  • pnpm test (14,097 passed)
  • focused unit suite (24 passed)
  • focused real-ClickHouse integration suite (10 passed)
  • pnpm setup-db
  • pnpm analytics:build (38 models)

Production rollout

Migration 0061 adds the projection definition only. After deployment, materialize by_provider_current_state explicitly, monitor system.mutations, verify no active part lacks the projection, and observe a complete analytics/cache cycle before resolving Sentry. No timeout or retry ceiling changes are included.

Summary by Sourcery

Introduce a ClickHouse aggregate projection and query settings to precompute and preferentially use provider current-state metric-stream counts, improving performance and documenting the associated production incident and rollout.

Enhancements:

  • Add a by-provider current-state aggregate projection to the metric_stream table and wire it into the provider_stats read model and ClickHouse bootstrap/migration registry.
  • Adjust provider_stats query settings to optimize in-order aggregation and prefer the new current-state projection without making it mandatory.
  • Extend analytics and ClickHouse documentation to describe the new projection, its materialization process, and how provider inventory counting uses it.

Documentation:

  • Document the new provider current-state aggregate projection, its role in metric-stream inventory counting, and detailed rollout and validation steps for the related production incident.

Tests:

  • Add and update real-ClickHouse integration tests and SQL model tests to cover projection materialization, planner selection, cross-part resurrection correctness, and bounded-memory exact recount behavior.

Summary by cubic

Precomputes provider current-state for metric-stream records with a ClickHouse projection and makes analytics.provider_stats prefer it, preventing provider inventory recount timeouts (refs DOFEK-SERVER-5A).

  • New Features

    • Added ingest.metric_stream aggregate projection by_provider_current_state that stores mergeable argMax(is_deleted, (version, ingested_at)) per (user_id, provider_id, id); included in ClickHouse bootstrap and migration registry.
    • Updated analytics.provider_stats to prefer the projection with optimize_aggregation_in_order = 1 and preferred_optimize_projection_name = 'by_provider_current_state', retaining exact fallback semantics.
    • Hardened real-ClickHouse tests for tombstone/live correctness across parts, EXPLAIN-verified planner selection (projection preferred, not required), and bounded-memory exact recounts.
    • Expanded docs with projection rationale and production rollout details tied to DOFEK-SERVER-5A.
  • Migration

    • Migration 0061_provider_current_state_projection adds the projection definition only.
    • After deploy, materialize historical parts: ALTER TABLE ingest.metric_stream MATERIALIZE PROJECTION by_provider_current_state; then monitor system.mutations and verify no active parts lack the projection.
    • Observe a full analytics + cache cycle under the existing 4‑minute ceiling before resolving DOFEK-SERVER-5A.

Written for commit dcaf177. Summary will update on new commits.

Review in cubic

Provider inventory rebuilt per-ID latest-row state across tens of millions of metric-stream records and exceeded the worker timeout. Add an aggregate projection; historical parts still require explicit post-deploy materialization.\n\nRefs DOFEK-SERVER-5A
Copilot AI review requested due to automatic review settings July 27, 2026 23:51
@cursor

cursor Bot commented Jul 27, 2026

Copy link
Copy Markdown

Bugbot is not enabled for your account, so this pull request was not reviewed.

Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs.

@codereviewbot-ai

Copy link
Copy Markdown

🤖 Review skipped: Repository rate limit exceeded. Free accounts are limited to 2 reviews per 4 hours per repository. Upgrade to a paid plan for unlimited reviews.

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry @Asherlc, you have reached your weekly rate limit of 500000 diff characters.

Please try again later or upgrade to continue using Sourcery

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@coderabbitai

coderabbitai Bot commented Jul 27, 2026 •

Copy link
Copy Markdown

Warning

Review limit reached

@Asherlc, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 2 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: d300c0bf-9222-4d71-8667-7f01b798e1da

📥 Commits

Reviewing files that changed from the base of the PR and between 5ee2b62 and dcaf177.

📒 Files selected for processing (12)
  • analytics/README.md
  • analytics/models/read_models/provider_stats.sql
  • analytics/models/read_models/provider_stats.sql.test.ts
  • docs/clickhouse-metric-stream.md
  • docs/production-incident-baseline.md
  • packages/server/src/repositories/provider-stats-read-model.integration.test.ts
  • src/db/clickhouse-migrations/0061_provider_current_state_projection.integration.test.ts
  • src/db/clickhouse-migrations/0061_provider_current_state_projection.ts
  • src/db/clickhouse-migrations/registry.test.ts
  • src/db/clickhouse-migrations/registry.ts
  • src/db/clickhouse.test.ts
  • src/metric-stream/clickhouse-table.ts

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.

@sourcery-ai

sourcery-ai Bot commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

Reviewer's Guide

Adds a ClickHouse aggregate projection that precomputes latest provider-level deletion state for metric_stream rows, wires provider_stats to prefer this projection with in-order aggregation, and introduces regression tests plus documentation and migration wiring to ensure correct rollout and bounded-memory exact counts.

Flow diagram for migration 0061 provider current-state projection rollout

flowchart TD
  Dev[Developer creates migration 0061_provider_current_state_projection] --> Deploy[Deploy runs ALTER TABLE ingest.metric_stream ADD PROJECTION by_provider_current_state]
  Deploy --> NewParts[New metric_stream parts populate projection automatically]
  Operator[Operator runs MATERIALIZE PROJECTION by_provider_current_state] --> Materialize[ClickHouse rewrites historical parts]
  Materialize --> VerifyParts[Check system.parts for missing_projection_parts = 0]
  VerifyParts --> ProviderStatsQuery[provider_stats query with preferred_optimize_projection_name = by_provider_current_state]
  ProviderStatsQuery --> BoundedCount[Exact metric_stream counts within bounded memory]
Loading

File-Level Changes

Change Details Files
Introduce a by_provider_current_state aggregate projection on ingest.metric_stream and wire it into migrations and bootstrap SQL.
  • Define METRIC_STREAM_PROVIDER_CURRENT_STATE_PROJECTION and its argMax-based projection SQL for latest is_deleted per (user_id, provider_id, id).
  • Extend ingest.metric_stream CREATE TABLE statement to include the by_provider_current_state projection alongside existing projections.
  • Add migration 0061_provider_current_state_projection that ALTERs ingest.metric_stream to add the projection if missing and register it in the migration registry and tests.
  • Update ClickHouse bootstrap tests to assert that the metric_stream table definition includes the new projection.
src/metric-stream/clickhouse-table.ts
src/db/clickhouse-migrations/0061_provider_current_state_projection.ts
src/db/clickhouse-migrations/0061_provider_current_state_projection.integration.test.ts
src/db/clickhouse-migrations/registry.ts
src/db/clickhouse-migrations/registry.test.ts
src/db/clickhouse.test.ts
Make provider_stats prefer the new projection and validate planner behavior, bounded memory usage, and correctness via integration tests.
  • Update provider_stats dbt model query_settings to enable optimize_aggregation_in_order and preferred_optimize_projection_name = 'by_provider_current_state'.
  • Extend provider_stats.sql.test.ts to assert the new ClickHouse settings are present in the rendered SQL.
  • Refine provider-stats read model integration helper to parameterize projection behavior and attach ClickHouse SETTINGS for projection forcing, aggregation-in-order, and max_memory_usage.
  • Adjust existing metric-stream counting test to account for cross-part tombstone-then-live resurrection and expect count 2 instead of 1.
  • Add an EXPLAIN-based test to confirm the planner selects the by_provider_current_state projection when preferred but not forced.
  • Add a high-cardinality stress test inserting 50k metric_stream IDs and asserting the exact recount succeeds within a 32 MiB memory setting.
analytics/models/read_models/provider_stats.sql
analytics/models/read_models/provider_stats.sql.test.ts
packages/server/src/repositories/provider-stats-read-model.integration.test.ts
Document the new projection, its rollout, and the associated production incident and validation strategy.
  • Expand ClickHouse metric stream docs to describe the by_provider_current_state projection, its role in exact provider inventory counts, and the explicit MATERIALIZE PROJECTION rollout steps and verification queries.
  • Update analytics README to explain how provider_stats uses the aggregate projection to avoid rebuilding high-cardinality latest-row state on each run.
  • Add a new incident entry detailing DOFEK-SERVER-5A, its root cause in provider inventory aggregation, the projection-based fix, and required production validation and rollout steps.
docs/clickhouse-metric-stream.md
analytics/README.md
docs/production-incident-baseline.md

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

perf(analytics): precompute provider current-state via ClickHouse projection

🐞 Bug fix 🧪 Tests 📝 Documentation ⚙️ Configuration changes 🕐 40+ Minutes

Grey Divider

AI Description

• Add a ClickHouse aggregate projection for per-provider latest metric-stream deletion state.
• Make provider_stats prefer the projection while preserving exact fallback semantics.
• Add real-ClickHouse regression tests and operational docs for safe rollout/materialization.
Diagram

graph TD
A["dbt: provider_stats"] --> B[("CH table: metric_stream")] --> C[("Projection: current_state")]
D["Migration 0061"] --> B
E["CH integration tests"] --> A
F["Rollout docs"] --> D
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Materialized view / rollup table for current-state IDs
  • ➕ More explicit lifecycle management than projections (easier to reason about backfills).
  • ➕ Can choose a different primary key/order_by to optimize the exact query pattern.
  • ➖ Additional table(s) and write amplification; more plumbing in ingestion.
  • ➖ Harder to keep exactly consistent with the base table without careful MV semantics.
2. Approximate counting (e.g., uniq* variants) for metric_stream inventory
  • ➕ Much faster and lower memory for high-cardinality providers.
  • ➕ Avoids reliance on argMax latest-row semantics.
  • ➖ Breaks existing “exact inventory” semantics and tombstone/resurrection correctness.
  • ➖ UI/analytics would need product sign-off on approximate values.
3. Increase query resources (timeout/memory/threads) for provider_stats
  • ➕ Minimal code/DDL changes; reduces operational complexity.
  • ➕ No need to manage projection materialization.
  • ➖ Masks the underlying scaling issue; risks repeated incidents as data grows.
  • ➖ Resource increases can impact cluster stability and cost; still slow.

Recommendation: The PR’s approach (aggregate projection + preferred planner selection + exact fallback) is the best tradeoff: it preserves correctness while turning a per-refresh high-cardinality argMax build into a merge of precomputed states. Keeping the projection “preferred” (not forced) limits correctness coupling during rollout, while docs/tests explicitly cover the operational requirement to materialize historical parts.

Files changed (12) +379 / -10

Bug fix (1) +3 / -1
provider_stats.sqlPrefer by_provider_current_state projection for exact metric-stream counts +3/-1

Prefer by_provider_current_state projection for exact metric-stream counts

• Adds ClickHouse query settings to enable in-order aggregation and prefer the 'by_provider_current_state' projection. This targets the expensive 'metric_stream_current' argMax aggregation without changing query semantics.

analytics/models/read_models/provider_stats.sql

Tests (5) +225 / -5
provider_stats.sql.test.tsAssert new provider_stats ClickHouse query settings in generated SQL +4/-0

Assert new provider_stats ClickHouse query settings in generated SQL

• Extends the SQL snapshot-style unit test to require 'optimize_aggregation_in_order' and 'preferred_optimize_projection_name'. Ensures future edits don’t silently drop the projection preference.

analytics/models/read_models/provider_stats.sql.test.ts

provider-stats-read-model.integration.test.tsReal-ClickHouse coverage for projection correctness, planner selection, and memory bounds +98/-4

Real-ClickHouse coverage for projection correctness, planner selection, and memory bounds

• Enhances the integration suite to count through the projection, including cross-part resurrection (tombstone then later live row). Adds an 'EXPLAIN projections=1' assertion that the planner selects 'by_provider_current_state' when preferred, and enforces a bounded-memory exact recount via 'max_memory_usage'.

packages/server/src/repositories/provider-stats-read-model.integration.test.ts

0061_provider_current_state_projection.integration.test.tsIntegration test for adding and materializing the current-state projection +110/-0

Integration test for adding and materializing the current-state projection

• Introduces a real-ClickHouse migration test that creates historical parts, applies migration 0061, asserts parts initially lack the projection, then materializes it and verifies exact argMax-based live counting with forced projection usage.

src/db/clickhouse-migrations/0061_provider_current_state_projection.integration.test.ts

registry.test.tsValidate registry includes migration 0061 and its projection definition +12/-1

Validate registry includes migration 0061 and its projection definition

• Updates registry tests to assert migration 0061 exists, contains the expected 'ADD PROJECTION' statement, and is ordered as the latest migration.

src/db/clickhouse-migrations/registry.test.ts

clickhouse.test.tsAssert ClickHouse bootstrap DDL includes the new projection +1/-0

Assert ClickHouse bootstrap DDL includes the new projection

• Extends the ClickHouse bootstrap SQL test to expect the 'PROJECTION by_provider_current_state' definition in the ingest metric_stream table DDL.

src/db/clickhouse.test.ts

Documentation (3) +118 / -4
README.mdExplain provider inventory’s use of current-state aggregate projection +8/-1

Explain provider inventory’s use of current-state aggregate projection

• Updates the analytics overview to describe how provider inventory uses the 'by_provider_current_state' aggregate projection. Clarifies that counts merge precomputed argMax state instead of rebuilding per-ID state from the raw stream, and links to ClickHouse projection docs.

analytics/README.md

clickhouse-metric-stream.mdDocument projection-backed exact provider inventory and rollout steps +58/-3

Document projection-backed exact provider inventory and rollout steps

• Documents that exact metric-stream counts prefer the 'by_provider_current_state' aggregate projection and explains why it retains tombstone/live replacement correctness. Adds an operator-focused rollout section describing 'MATERIALIZE PROJECTION', mutation monitoring, and part verification queries.

docs/clickhouse-metric-stream.md

production-incident-baseline.mdAdd DOFEK-SERVER-5A incident write-up and verification checklist +52/-0

Add DOFEK-SERVER-5A incident write-up and verification checklist

• Adds a production incident entry describing the ClickHouse timeout, evidence, root cause, and mitigation via the current-state projection. Captures validation steps and explicit post-deploy materialization/verification requirements.

docs/production-incident-baseline.md

Other (3) +33 / -0
0061_provider_current_state_projection.tsAdd migration 0061 to define by_provider_current_state projection +18/-0

Add migration 0061 to define by_provider_current_state projection

• Adds a ClickHouse migration that performs 'ALTER TABLE ingest.metric_stream ADD PROJECTION IF NOT EXISTS by_provider_current_state (...)'. The migration only adds the definition; historical materialization remains an explicit operator action.

src/db/clickhouse-migrations/0061_provider_current_state_projection.ts

registry.tsRegister migration 0061 in ClickHouse migration list +2/-0

Register migration 0061 in ClickHouse migration list

• Imports and appends the 0061 migration factory to the migration registry so it runs during ClickHouse bootstrap/migration execution.

src/db/clickhouse-migrations/registry.ts

clickhouse-table.tsDefine by_provider_current_state aggregate projection in metric_stream DDL +13/-0

Define by_provider_current_state aggregate projection in metric_stream DDL

• Adds a new projection constant and definition that computes 'argMax(is_deleted, tuple(version, ingested_at))' grouped by '(user_id, provider_id, id)'. Wires it into the 'ingest.metric_stream' CREATE TABLE statement alongside existing provider projections.

src/metric-stream/clickhouse-table.ts

@github-actions

github-actions Bot commented Jul 27, 2026 •

Copy link
Copy Markdown
Contributor

Storybook previews for 5f8fb989 are ready:

This comment updates automatically on each PR push.

@qodo-code-review

qodo-code-review Bot commented Jul 27, 2026 •

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📜 Skill insights (0)

Context used
✅ Compliance rules (platform): 181 rules

Grey Divider


Remediation recommended

1. Flaky parts-count assertion ✓ Resolved 🐞 Bug ☼ Reliability
Description
0061_provider_current_state_projection.integration.test.ts asserts there are exactly 3 active
parts missing the projection, but ClickHouse background merges can change the number of active parts
between inserts and the assertion. This makes the test non-deterministic and prone to CI flakes.
Code

src/db/clickhouse-migrations/0061_provider_current_state_projection.integration.test.ts[R76-78]

+    expect(projectionPartSchema.parse(await beforeMaterialization.json())).toEqual([
+      { missing_projection_parts: 3 },
+    ]);
Relevance

⭐⭐⭐ High

Team often accepts reducing test brittleness/flakiness; fixed-count assertions on async processes
are typically relaxed.

PR-#1971
PR-#2219

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The test performs multiple inserts before adding the projection, then queries `system.parts WHERE
active and expects a fixed count; since active` parts can be merged asynchronously, the exact
count is not stable even if the underlying behavior is correct.

src/db/clickhouse-migrations/0061_provider_current_state_projection.integration.test.ts[45-79]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
The test assumes `missing_projection_parts` is exactly `3`, but the number of *active* parts can shrink due to background merges, making the exact count unreliable.

### Issue Context
The test’s real invariant is: parts that existed before `ADD PROJECTION` should initially lack the projection, and after `MATERIALIZE PROJECTION` the projection-backed query should be correct.

### Fix Focus Areas
- src/db/clickhouse-migrations/0061_provider_current_state_projection.integration.test.ts[45-79]

### Suggested change
- Replace the strict equality assertion with a looser invariant, e.g.:
 - parse the row and assert `missing_projection_parts > 0` (or `>= 1`).
- Keep the later correctness assertion (count == 2) to validate the projection works after materialization.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


2. Hardcoded projection name ✓ Resolved 🐞 Bug ⚙ Maintainability
Description
provider-stats-read-model.integration.test.ts hardcodes by_provider_current_state in query
SETTINGS and the EXPLAIN assertion instead of using the exported
METRIC_STREAM_PROVIDER_CURRENT_STATE_PROJECTION. This can drift if the projection name is ever
changed and repeats a previously accepted issue pattern in the same test area.
Code

packages/server/src/repositories/provider-stats-read-model.integration.test.ts[R110-115]

+  const projectionSettings =
+    projectionMode === "force"
+      ? `force_optimize_projection = 1,
+    force_optimize_projection_name = 'by_provider_current_state'`
+      : "preferred_optimize_projection_name = 'by_provider_current_state'";
+
Relevance

⭐⭐⭐ High

Direct accepted precedent in same file: avoid hardcoding projection names; use exported constant to
prevent drift.

PR-#2048

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The test hardcodes the projection name even though the codebase defines a shared constant for it,
and other new code in this PR already uses that constant—making this duplication unnecessary and
fragile.

packages/server/src/repositories/provider-stats-read-model.integration.test.ts[99-133]
packages/server/src/repositories/provider-stats-read-model.integration.test.ts[219-255]
src/metric-stream/clickhouse-table.ts[7-16]
src/db/clickhouse-migrations/0061_provider_current_state_projection.integration.test.ts[1-7]
PR-#2048

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
The integration test hardcodes the projection name (`'by_provider_current_state'`) in multiple places, which can drift from the canonical exported constant and cause misleading failures during renames.

### Issue Context
The repo already exports `METRIC_STREAM_PROVIDER_CURRENT_STATE_PROJECTION` and uses it in other tests/migrations; this test should reuse it for SETTINGS generation and the EXPLAIN assertion.

### Fix Focus Areas
- packages/server/src/repositories/provider-stats-read-model.integration.test.ts[99-133]

### Suggested change
- Import `METRIC_STREAM_PROVIDER_CURRENT_STATE_PROJECTION` from `src/metric-stream/clickhouse-table.ts`.
- Replace the hardcoded strings in `projectionSettings` with string interpolation using the constant.
- Replace the hardcoded EXPLAIN substring check to use the same constant (e.g., `ReadFromMergeTree (${METRIC_STREAM_PROVIDER_CURRENT_STATE_PROJECTION})`).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Qodo Logo

@Asherlc
Asherlc enabled auto-merge (squash) July 28, 2026 00:00
@codereviewbot-ai

codereviewbot-ai Bot commented Jul 28, 2026 •

Copy link
Copy Markdown

LGTM!

This PR adds the by_provider_current_state aggregate projection on metric_stream and configures provider_stats read model to prefer it (preferred_optimize_projection_name = 'by_provider_current_state').

Summary of Changes:

  1. ClickHouse Table & Migration (0061_provider_current_state_projection): Added projection definition METRIC_STREAM_PROVIDER_CURRENT_STATE_PROJECTION to clickhouse-table.ts and created migration 0061 to add the projection by_provider_current_state to ingest.metric_stream.
  2. dbt Read Model (provider_stats.sql): Configured optimize_aggregation_in_order = 1 and preferred_optimize_projection_name = 'by_provider_current_state' in query settings.
  3. Tests & Coverage:
    • Added unit tests in provider_stats.sql.test.ts to assert the query settings are present.
    • Updated integration tests in provider-stats-read-model.integration.test.ts to test projection usage, live version count accuracy, and bounded memory budget execution.
    • Reused constant METRIC_STREAM_PROVIDER_CURRENT_STATE_PROJECTION across test files.
    • Added integration test for migration 0061 and updated registry tests.

🤖 Reviewed by codereviewbot.ai - Catch bugs before your team does.

@Asherlc
Asherlc merged commit 5ec8074 into main Jul 28, 2026
104 checks passed
@Asherlc
Asherlc deleted the Asherlc/fix-sentry-7632766197 branch July 28, 2026 00:46
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.

2 participants