feat: unified metrics system — typed registry, generic result runtime, AI metrics

[aleksdotbar]

What

Introduces the unified metrics system: metrics are authored, requested, and rendered through one structured path instead of ad-hoc per-metric gold views and catalog seeds. The AI metrics are the first family on it.

Architecture

• Observation contract — managed dbt gold models emit normalized source measure observations (insight.ai_metric_observations): tenant, entity, date, measure_key, value, dimension tuples. Observations belong to measures, not metrics.
• Typed registry (MariaDB) — metric_sources → metric_source_measures / metric_source_dimensions → metric_definitions + input-role mappings. Product definitions are tenant-overridable; disabled/schema-error rows degrade to 400, never 500.
• Builtin seed reconciler — definitions live in one code registry (builtin.rs) converged at startup (disable-not-delete); migrations own schema only. Warm environments converge to the registry state on every deploy.
• Generic runtime — POST /v1/metric-results validates (request caps, per-entity-type id normalization, dimension grants), compiles one ClickHouse query per metric view (sum/ratio × period/timeseries/peer/breakdown), densifies, and enforces all-or-nothing row caps. No metric-key-specific branches. The computation vocabulary is closed and fully executable — nothing is stored before it executes; extending it is one coordinated change (spec variant, compiler arm, DB enum, response tag) documented in the design spec.
• Schema validator — background probes mark definitions ok/error/unchecked per emitted measures and dimension coverage; transient probe failures never overwrite established status; quiet (filtered) measures downgrade to unchecked, never error.

Vendor-neutral gold

Gold models read only class-contract fields. The AI class contracts gain connector-declared semantics: tool_label/surface_label (each connector owns its display name), conversation_count (populated only by sources that actually report conversations), and an activity invariant — a class row exists only for real activity, enforced by each staging model's emission filter and guarded by data-quality tests. Gold derives active days from row existence and contains no vendor columns, tool names, or label maps.

AI metrics on the system

ai.accepted_lines, ai.removed_lines, ai.active_days, ai.cost, ai.accepted_edit_actions, ai.tool_acceptance_rate, ai.assistant_messages, ai.assistant_actions, ai.dev_conversations, ai.chat_assistant_conversations — with tool/surface dimensions and org-unit peer comparison. ai.active_days and ai.cost span dev and assistant tools.

Authoring standard

docs/domain/metrics/specs/DESIGN.md is the system contract and includes an Adding a Metric guide (three cases: existing measure / new measure / new source) plus the rules that hold for every case. Root AGENTS.md routes all metric work there. The legacy path (ad-hoc insight.* gold views + metric_catalog seed migrations) is frozen for new metrics.

Validation

• cargo test -p analytics: 397 passed — 60+ new tests for this system (registry invariants, compiler SQL shapes and parameter order, bucket enumeration vs ClickHouse week/month semantics, densification/zero-fill, definition precedence and fallback, request caps, DDL CHECK↔enum sync); the pre-existing suite stays green.
• dbt parse clean; gold models resolve to insight.* with lineage to the AI silver classes; contract-column parity verified across all staging models in both class tags.
• Placeholder DDL extended additively on the canonical script (new columns only; existing types and compose-seed compatibility untouched).

Follow-ups

• Frontend switch to /v1/metric-results (metric collections), then removal of the legacy AI gold views and catalog seeds.
• Performance levers when data warrants: same-view query batching in the compiler, cohort view materialization, observation view → incremental table (all local changes by design).
• Extend the e2e metric-test harness to the new endpoint.
• Decide the custom-range-picker policy vs the 400-day period cap: clamp the picker or raise the cap (config); team timeseries bucket auto-selection must also factor entity count against the row cap.

🤖 Generated with Claude Code

Summary by CodeRabbit

• New Features
  • Added a POST /v1/metric-results analytics endpoint to compute metric results across period, timeseries, peer, and breakdown views.
  • Expanded the metrics system with a typed, unified metrics registry plus automatic reconciliation of built-in metric definitions.
  • Ingestion now carries new AI class label fields (tool_label, surface_label) and conversation_count end-to-end.
• Bug Fixes
  • Improved metric results robustness: stricter request validation, clearer unknown/missing dimension behavior, zero-fill handling, and enforced response size limits.
• Documentation
  • Published/updated metrics domain guidance and the technical design contract for metrics definition, validation, and computation.

[coderabbitai]

Warning

Review limit reached

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

Next review available in: 57 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: defaults

Review profile: CHILL

Plan: Pro

Run ID: b778a326-75b2-4579-888c-0cad00cfb7a1

📥 Commits

Reviewing files that changed from the base of the PR and between deab55e and b05e5f9.

📒 Files selected for processing (9)

• charts/insight/templates/clickhouse-migrate-job.yaml
• docs/components/backend/analytics/openapi.json
• docs/domain/metrics/specs/DESIGN.md
• src/backend/services/analytics/src/api/metric_results.rs
• src/ingestion/connectors/ai/chatgpt-team/descriptor.yaml
• src/ingestion/connectors/ai/claude-enterprise/descriptor.yaml
• src/ingestion/connectors/ai/claude-team/descriptor.yaml
• src/ingestion/scripts/apply-ch-migrations.sh
• src/ingestion/scripts/migrations/20260707000000_ai_class_label_backfill.sql

📝 Walkthrough

Walkthrough

Adds a typed metrics registry and metric-results runtime in the analytics backend, plus dbt and silver/gold ingestion changes that emit the metric observations, labels, and cohorts those metrics consume.

Changes

Metrics Engine Backend

Layer / File(s)	Summary
Docs and wiring AGENTS.md, docs/domain/README.md, docs/domain/metrics/README.md, docs/domain/metrics/specs/DESIGN.md, src/backend/services/analytics/src/domain/mod.rs, src/backend/services/analytics/src/domain/metric_definitions/README.md, src/backend/services/analytics/src/domain/metric_definitions/mod.rs, src/backend/services/analytics/src/domain/metric_results/mod.rs, src/backend/services/analytics/src/domain/metric_results/view.rs, src/backend/services/analytics/src/domain/metric_results/dto.rs, src/backend/services/analytics/src/api/mod.rs, src/backend/services/analytics/src/gear.rs	Adds metrics domain docs, module exports, API route registration, and startup/migrate hooks for builtin reconciliation and metric validation.
Definition registry and seeds src/backend/services/analytics/src/domain/metric_definitions/definition.rs, src/backend/services/analytics/src/domain/metric_definitions/builtin.rs, src/backend/services/analytics/src/domain/metric_definitions/error_code.rs	Defines metric domain enums and structs, builtin source and metric seeds, schema error codes, and their invariant tests.
Repository and builtin reconciliation src/backend/services/analytics/src/domain/metric_definitions/repository.rs, src/backend/services/analytics/src/domain/metric_definitions/seeds.rs	Loads and classifies metric definitions from the database, writes schema status updates, and reconciles builtin sources, measures, dimensions, and metric definitions.
Schema validator and migration src/backend/services/analytics/src/domain/metric_definitions/validator.rs, src/backend/services/analytics/src/migration/m20260625_000001_metric_definitions.rs, src/backend/services/analytics/src/migration/mod.rs	Validates managed metric schemas against ClickHouse and adds the forward-only migration plus registry checks for the metric definition tables.
Metric results domain and API src/backend/services/analytics/src/domain/metric_results/validation.rs, src/backend/services/analytics/src/domain/metric_results/compiler.rs, src/backend/services/analytics/src/domain/metric_results/builder.rs, src/backend/services/analytics/src/api/metric_results.rs	Adds request validation and row-limit projection, SQL compilation for period/timeseries/breakdown/peer views, view builders with row-limit enforcement, and the API handler that runs the compiled queries.

Ingestion Labels and Metric Observations

Layer / File(s)	Summary
Silver labels and contracts src/ingestion/connectors/ai/.../*.sql, src/ingestion/silver/ai/*	Adds tool and surface labels plus conversation counts across connector models, updates silver schema contracts, and changes incremental schema handling.
Gold observations, cohorts, and macros src/ingestion/dbt/dbt_project.yml, src/ingestion/dbt/macros/metric_observation_measures.sql, src/ingestion/gold/*, src/ingestion/dbt/tests/*	Adds dbt macros, gold observation and cohort views, schema contracts, model path updates, and tests for the gold layer.
Placeholders and backfill src/ingestion/scripts/create-bronze-placeholders.sh, src/ingestion/scripts/migrations/20260707000000_ai_class_label_backfill.sql	Extends bronze placeholder table creation and reconciliation, and backfills label columns for historical silver rows.

Estimated code review effort: 5 (Critical) | ~120 minutes

Possibly related issues

• INGEST 1: Unified metric system (Metrics catalog) #1561 — Implements the same unified metrics system: typed definitions, normalized observations, and a generic /v1/metric-results runtime/API.
• INGEST 1: Unified metric system — test scope #1676 — Covers the runtime, definitions, reconciliation, and validation layers that this change set adds.

Possibly related PRs

• constructorfabric/insight#1548 — Both PRs touch src/backend/services/analytics/src/api/mod.rs and the analytics API surface around the new metric-results endpoint.
• constructorfabric/insight#1597 — Both PRs modify the ClickHouse placeholder DDL/reconciliation logic in src/ingestion/scripts/create-bronze-placeholders.sh.
• constructorfabric/insight#1509 — Both PRs modify the Claude Enterprise dbt incremental usage model.

Suggested reviewers: mitasovr, cyberantonz, ktursunov

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately summarizes the main changes: a typed metrics registry, generic results runtime, and initial AI metrics support.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch feat/unified-metrics

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

Actionable comments posted: 5

🧹 Nitpick comments (5)

src/backend/services/analytics/src/domain/metric_definitions/mod.rs (1)

8-14: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Inconsistent re-export surface.

MetricInputRole, MetricComputation, SourceKind, MetricBase, and MetricInput are all public types in definition.rs but aren't re-exported here, while CohortSource, ComputationSpec, MetricDefinition, MetricDirection, MetricFormat, and ObservationSource are. Consumers outside this module (e.g. the metric-results compiler/builder that need to match on MetricInputRole) will have to reach into definition:: directly instead of the module root, which is workable but inconsistent.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/backend/services/analytics/src/domain/metric_definitions/mod.rs` around
lines 8 - 14, The module root re-export list is incomplete and leaves several
public definition types only reachable through definition::, creating an
inconsistent public API. Update the pub use block in metric_definitions::mod to
also re-export MetricInputRole, MetricComputation, SourceKind, MetricBase, and
MetricInput alongside the existing types, so consumers like the metric-results
compiler/builder can import everything from the module root consistently.

src/ingestion/scripts/create-bronze-placeholders.sh (1)

420-434: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoff

Consider extracting the placeholder-reconciliation guard into a helper.

The count(placeholder) → if == "1" reconcile else skip idiom is now duplicated across class_ai_dev_usage, class_ai_assistant_usage, class_people, and mtr_git_person_weekly. A small helper (e.g. reconcile_if_placeholder <db> <table> <<'SQL' … SQL) would DRY this up and keep the count-query/comment filter consistent as the list grows. Not required for this PR.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/ingestion/scripts/create-bronze-placeholders.sh` around lines 420 - 434,
The placeholder-reconciliation logic is duplicated across multiple table blocks,
using the same count/comment guard followed by conditional schema updates.
Consider extracting this pattern into a helper in create-bronze-placeholders.sh,
such as a reconcile_if_placeholder function that takes the database/table and
runs the SQL only when the table matches the placeholder marker. Reuse that
helper for class_ai_dev_usage, class_ai_assistant_usage, class_people, and
mtr_git_person_weekly to keep the guard/query logic consistent and easier to
extend.

src/backend/services/analytics/src/api/metric_results.rs (1)

36-51: 🧹 Nitpick | 🔵 Trivial

Errors don't short-circuit concurrent view execution.

stream::iter(tasks).map(...).buffer_unordered(QUERY_CONCURRENCY).collect::<Vec<_>>().await waits for every task to finish (success or failure) before the first Err is surfaced via result? at line 49. With up to 50 metrics × 4 views, a request that fails validation-adjacent execution early still lets the remaining ClickHouse queries run to completion under the hood (throttled to 4 concurrent). Bounded impact given QUERY_CONCURRENCY = 4 and existing row caps, but worth considering try_for_each_concurrent-style early bailout if this becomes a hot failure path.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/backend/services/analytics/src/api/metric_results.rs` around lines 36 -
51, Concurrent view execution currently buffers all task results before checking
for errors in the metric_results flow, so failures in execute_task do not stop
later ClickHouse work early. Update the task execution path in the metric
results handler to short-circuit on the first error, using an early-bailing
concurrent pattern instead of collecting into a Vec first, and keep the existing
result placement logic in views_by_metric and the execute_task call site aligned
with that change.

src/backend/services/analytics/src/domain/metric_results/validation.rs (1)

325-350: 🧹 Nitpick | 🔵 Trivial

Breakdown views have no upfront row-limit projection.

Unlike Period/Peer/Timeseries, ValidatedMetricView::Breakdown { .. } => {} contributes nothing to the projected count, so oversized breakdown requests aren't rejected until after the ClickHouse GROUP BY entity_id, dim_* query (compiler.rs) runs to (near) completion and enforce_row_limit catches it post-hoc. This is workable given today's low-cardinality AI dimensions (tool/surface), but as more dimensions are registered this becomes a way to force expensive aggregations before rejection. Worth a bound (e.g., cap on entity_ids × requested-dimension count for breakdown views, or an upfront COUNT(DISTINCT ...) probe) if breakdown dimensions grow beyond a handful of low-cardinality values.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/backend/services/analytics/src/domain/metric_results/validation.rs`
around lines 325 - 350, The projected row-limit check in
validate_projected_row_limit currently ignores ValidatedMetricView::Breakdown {
.. }, so oversized breakdown requests can slip through until after the
ClickHouse query runs. Update validate_projected_row_limit to account for
Breakdown views by adding a conservative upfront bound based on req.entity_ids
and the requested breakdown dimensions, or by introducing a lightweight precheck
before compiler.rs executes the GROUP BY path. Keep the fix localized to
validate_projected_row_limit and the Breakdown match arm so row-limit
enforcement happens before expensive aggregation work.

src/backend/services/analytics/src/gear.rs (1)

139-159: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Duplicated single-tenant warehouse-id parsing logic.

The trim+is_empty filter chain here duplicates warehouse_tenant_id() in api/metric_results.rs. Consider extracting a shared helper (e.g. MetricResultsConfig::effective_single_tenant_warehouse_id(&self) -> Option<&str>) so the startup guard and the runtime read path can never diverge on what counts as "set".

♻️ Proposed shared helper

+impl MetricResultsConfig {
+    pub fn effective_single_tenant_warehouse_id(&self) -> Option<&str> {
+        self.single_tenant_warehouse_id
+            .as_deref()
+            .map(str::trim)
+            .filter(|id| !id.is_empty())
+    }
+}

Then both gear.rs and api/metric_results.rs call cfg.metric_results.effective_single_tenant_warehouse_id().

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/backend/services/analytics/src/gear.rs` around lines 139 - 159, The
single-tenant warehouse-id parsing logic is duplicated between the startup guard
and the runtime metric-results read path, which can cause them to drift. Extract
the trimming/non-empty check into a shared helper on MetricResultsConfig, such
as effective_single_tenant_warehouse_id(), and have both gear.rs and
api/metric_results.rs use it. Update the existing warehouse_tenant handling in
gear.rs to call the helper so the “set” semantics stay identical everywhere.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/backend/services/analytics/src/domain/metric_results/builder.rs`:
- Around line 110-126: build_peer_view currently returns only the rows from
ClickHouse, so it can drop requested entity_ids and break positional alignment
with the request. Update build_peer_view to take the request context like
build_period_view does, then densify against req.entity_ids by producing one
PeerValueDto per requested id, inserting a default entry with zero/nil values
when no row exists. Thread req through the execute_task call site in
api/metric_results.rs and use the existing build_peer_view and PeerValueDto
symbols to keep peer results aligned with the requested ids.

In `@src/backend/services/analytics/src/domain/metric_results/validation.rs`:
- Around line 207-221: The Peer metric view path in validate_metric_view_request
only normalizes an explicit cohort_key and never verifies it matches
def.base.peer_cohort_key. Update the MetricViewRequest::Peer branch to compare
any provided cohort_key against the metric’s declared peer cohort (similar to
the allowed_dimension validation elsewhere), and return a
MetricError::invalid_argument with a field violation when it does not match.
Keep the existing defaulting behavior when cohort_key is omitted, but reject
unsupported explicit values in
validate_metric_view_request/ValidatedMetricView::Peer.

In `@src/ingestion/silver/ai/class_ai_assistant_usage.sql`:
- Line 8: The incremental class_ai_assistant_usage model will still leave
tool_label and surface_label NULL for existing rows, so the new not_null tests
will fail. Update the dbt flow around class_ai_assistant_usage and its upstream
staging models to backfill these fields for historical data, either by
full-refreshing the affected models or by adding a backfill step before
enforcing the new constraints. Ensure the fix covers the incremental logic in
class_ai_assistant_usage and any staging models that populate these labels.

In `@src/ingestion/silver/ai/class_ai_dev_usage.sql`:
- Line 8: The `append_new_columns` change in `class_ai_dev_usage` leaves
existing `tool_label` values NULL for historical rows, so the `not_null`
contract still fails. Update the `class_ai_dev_usage` model and its upstream
`silver:class_ai_dev_usage` builders to backfill `tool_label` for existing
records before enforcing the constraint, and ensure the same fix is applied
wherever this column is introduced so the historical data is rebuilt with
non-NULL values.

In `@src/ingestion/silver/ai/schema.yml`:
- Around line 63-69: Add a backfill/full-refresh step before enabling the new
not_null test on tool_label in the schema for the silver AI models. Since the
incremental models use on_schema_change='append_new_columns', update the
relevant incremental model(s) to populate historical rows with tool_label (and
surface_label where applicable) or document/trigger a coordinated full refresh
so existing records are repaired before the tests run.

---

Nitpick comments:
In `@src/backend/services/analytics/src/api/metric_results.rs`:
- Around line 36-51: Concurrent view execution currently buffers all task
results before checking for errors in the metric_results flow, so failures in
execute_task do not stop later ClickHouse work early. Update the task execution
path in the metric results handler to short-circuit on the first error, using an
early-bailing concurrent pattern instead of collecting into a Vec first, and
keep the existing result placement logic in views_by_metric and the execute_task
call site aligned with that change.

In `@src/backend/services/analytics/src/domain/metric_definitions/mod.rs`:
- Around line 8-14: The module root re-export list is incomplete and leaves
several public definition types only reachable through definition::, creating an
inconsistent public API. Update the pub use block in metric_definitions::mod to
also re-export MetricInputRole, MetricComputation, SourceKind, MetricBase, and
MetricInput alongside the existing types, so consumers like the metric-results
compiler/builder can import everything from the module root consistently.

In `@src/backend/services/analytics/src/domain/metric_results/validation.rs`:
- Around line 325-350: The projected row-limit check in
validate_projected_row_limit currently ignores ValidatedMetricView::Breakdown {
.. }, so oversized breakdown requests can slip through until after the
ClickHouse query runs. Update validate_projected_row_limit to account for
Breakdown views by adding a conservative upfront bound based on req.entity_ids
and the requested breakdown dimensions, or by introducing a lightweight precheck
before compiler.rs executes the GROUP BY path. Keep the fix localized to
validate_projected_row_limit and the Breakdown match arm so row-limit
enforcement happens before expensive aggregation work.

In `@src/backend/services/analytics/src/gear.rs`:
- Around line 139-159: The single-tenant warehouse-id parsing logic is
duplicated between the startup guard and the runtime metric-results read path,
which can cause them to drift. Extract the trimming/non-empty check into a
shared helper on MetricResultsConfig, such as
effective_single_tenant_warehouse_id(), and have both gear.rs and
api/metric_results.rs use it. Update the existing warehouse_tenant handling in
gear.rs to call the helper so the “set” semantics stay identical everywhere.

In `@src/ingestion/scripts/create-bronze-placeholders.sh`:
- Around line 420-434: The placeholder-reconciliation logic is duplicated across
multiple table blocks, using the same count/comment guard followed by
conditional schema updates. Consider extracting this pattern into a helper in
create-bronze-placeholders.sh, such as a reconcile_if_placeholder function that
takes the database/table and runs the SQL only when the table matches the
placeholder marker. Reuse that helper for class_ai_dev_usage,
class_ai_assistant_usage, class_people, and mtr_git_person_weekly to keep the
guard/query logic consistent and easier to extend.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 5f1cd9c7-0c73-4474-9106-9386cc0f134f

📥 Commits

Reviewing files that changed from the base of the PR and between 17940f4 and 3a8dbf6.

📒 Files selected for processing (46)

• AGENTS.md
• docs/components/backend/analytics/openapi.json
• docs/domain/README.md
• docs/domain/metrics/README.md
• docs/domain/metrics/specs/DESIGN.md
• src/backend/services/analytics/src/api/metric_results.rs
• src/backend/services/analytics/src/api/mod.rs
• src/backend/services/analytics/src/config.rs
• src/backend/services/analytics/src/domain/metric_definitions/README.md
• src/backend/services/analytics/src/domain/metric_definitions/builtin.rs
• src/backend/services/analytics/src/domain/metric_definitions/definition.rs
• src/backend/services/analytics/src/domain/metric_definitions/error_code.rs
• src/backend/services/analytics/src/domain/metric_definitions/mod.rs
• src/backend/services/analytics/src/domain/metric_definitions/repository.rs
• src/backend/services/analytics/src/domain/metric_definitions/seeds.rs
• src/backend/services/analytics/src/domain/metric_definitions/validator.rs
• src/backend/services/analytics/src/domain/metric_results/builder.rs
• src/backend/services/analytics/src/domain/metric_results/compiler.rs
• src/backend/services/analytics/src/domain/metric_results/dto.rs
• src/backend/services/analytics/src/domain/metric_results/mod.rs
• src/backend/services/analytics/src/domain/metric_results/validation.rs
• src/backend/services/analytics/src/domain/metric_results/view.rs
• src/backend/services/analytics/src/domain/mod.rs
• src/backend/services/analytics/src/gear.rs
• src/backend/services/analytics/src/migration/m20260625_000001_metric_definitions.rs
• src/backend/services/analytics/src/migration/mod.rs
• src/ingestion/connectors/ai/chatgpt-team/dbt/chatgpt_team__ai_assistant_usage.sql
• src/ingestion/connectors/ai/chatgpt-team/dbt/chatgpt_team__ai_dev_usage.sql
• src/ingestion/connectors/ai/claude-admin/dbt/claude_admin__ai_dev_usage.sql
• src/ingestion/connectors/ai/claude-enterprise/dbt/claude_enterprise__ai_assistant_usage.sql
• src/ingestion/connectors/ai/claude-enterprise/dbt/claude_enterprise__ai_dev_usage.sql
• src/ingestion/connectors/ai/claude-team/dbt/claude_team__ai_dev_usage.sql
• src/ingestion/connectors/ai/cursor/dbt/cursor__ai_dev_usage.sql
• src/ingestion/connectors/ai/github-copilot/dbt/copilot__ai_dev_usage.sql
• src/ingestion/dbt/dbt_project.yml
• src/ingestion/dbt/macros/metric_observation_measures.sql
• src/ingestion/dbt/tests/ai/assert_ai_assistant_usage_rows_active.sql
• src/ingestion/dbt/tests/ai/assert_ai_dev_usage_rows_active.sql
• src/ingestion/dbt/tests/gold/assert_metric_entity_cohorts_unique.sql
• src/ingestion/gold/ai_metric_observations.sql
• src/ingestion/gold/metric_entity_cohorts_current.sql
• src/ingestion/gold/schema.yml
• src/ingestion/scripts/create-bronze-placeholders.sh
• src/ingestion/silver/ai/class_ai_assistant_usage.sql
• src/ingestion/silver/ai/class_ai_dev_usage.sql
• src/ingestion/silver/ai/schema.yml

[coderabbitai]

🧹 Nitpick comments (1)

src/backend/services/analytics/src/domain/metric_results/compiler.rs (1)

308-313: 🚀 Performance & Scalability | 🔵 Trivial

Cohort join fan-out may be costly for large peer cohorts.

LEFT JOIN peers ON peers.cohort_id = targets.cohort_id materializes a targets × peers cross-product per cohort before the GROUP BY. For a target set within a large org-unit cohort (thousands of observed members), this expands to a large intermediate before aggregation. Correctness is fine; this is a scaling note. Consider pre-aggregating cohort percentiles once per cohort_id and joining the target rows to the aggregated result, so peer stats are computed once per cohort rather than once per (target, peer) pair.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/backend/services/analytics/src/domain/metric_results/compiler.rs` around
lines 308 - 313, The cohort percentile query in compiler.rs is doing a costly
target-to-peer fan-out by joining peers directly to targets before aggregation.
Update the SQL-building logic around the query that uses targets, peers, and
GROUP BY targets.entity_id, target_values.value so peer statistics are computed
once per cohort_id first, then join that pre-aggregated cohort result back to
the target rows. Use the existing metric query compiler path in compiler.rs to
keep correctness while reducing the intermediate cross-product.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@src/backend/services/analytics/src/domain/metric_results/compiler.rs`:
- Around line 308-313: The cohort percentile query in compiler.rs is doing a
costly target-to-peer fan-out by joining peers directly to targets before
aggregation. Update the SQL-building logic around the query that uses targets,
peers, and GROUP BY targets.entity_id, target_values.value so peer statistics
are computed once per cohort_id first, then join that pre-aggregated cohort
result back to the target rows. Use the existing metric query compiler path in
compiler.rs to keep correctness while reducing the intermediate cross-product.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 506df536-9cb5-4d93-bacc-5e83ab7b07c0

📥 Commits

Reviewing files that changed from the base of the PR and between 50a03b8 and 119a6ca.

📒 Files selected for processing (6)

• docs/domain/metrics/specs/DESIGN.md
• src/backend/services/analytics/src/api/metric_results.rs
• src/backend/services/analytics/src/domain/metric_results/compiler.rs
• src/backend/services/analytics/src/domain/metric_results/validation.rs
• src/ingestion/dbt/tests/ai/assert_ai_class_labels_nonempty.sql
• src/ingestion/scripts/migrations/20260707000000_ai_class_label_backfill.sql

✅ Files skipped from review due to trivial changes (1)

• docs/domain/metrics/specs/DESIGN.md

🚧 Files skipped from review as they are similar to previous changes (2)

• src/backend/services/analytics/src/api/metric_results.rs
• src/backend/services/analytics/src/domain/metric_results/validation.rs