feat(metrics): declarative YAML metric registry (#1974)

[cyberantonz]

Closes #1974. Part of #1803 (Phase B).

What

One declarative registry (src/backend/services/analytics/src/domain/metric_definitions/registry.yaml) is now the single source of truth for the sanctioned metric-definition seed — a sources list and a metrics list. It replaces the code-literal BUILTIN_SOURCES / BUILTIN_METRICS arrays in builtin.rs.

The registry was generated from the const arrays, so it is byte-for-byte parity (5 sources, 59 metrics). It is embedded at build time (include_str!) and deserialized once via OnceLock into the seed types, exposed as builtin_sources() / builtin_metrics(). The startup reconciler reads it through those accessors, so reconcile semantics (idempotent additive upserts + disable-missing) are unchanged. The registry invariant tests parse the same embedded file, so a malformed or drifted registry fails the build.

Aligns with the adopted semantic-layer design

This PR is the first Phase-1 step of the semantic-layer target architecture adopted in #2184 (docs/domain/semantic-layer/specs/ — PRD cpt-semantic-layer-fr-definitions-as-data, DESIGN cpt-semantic-layer-component-definition-store): metric definitions authored as data, replacing Rust constants.

Per that design, the registry's current observation-relation shape (source_ref, per-measure evidence_granularity, reconcile into metric_source_measures) is intentionally transitional — it is rewritten to the dataset/measure/metric domain model at the compiler-first cutover (target Phases 2–3). So this PR deliberately does not reshape the schema; it moves authoring to data in the existing shape, which is the low-risk first step the design prescribes. Migration cost stays near zero because builtin rows are seed-reconciled.

Scope

• FE: untouched — it already renders from the metric_definitions catalog API and live peer percentiles and holds no per-metric thresholds, so the "FE thresholds" collapse was already done.
• Legacy metric_catalog / metric_threshold: untouched. It is an orphaned, frozen subsystem (no live consumer — /v1/catalog/get_metrics is not called by the FE), so it is not folded into the new registry. Its retirement is tracked separately ([pres] Retire the orphaned legacy metric_catalog/metric_threshold subsystem #2167).

Docs

• Updated the governing metrics DESIGN (builtin.rs -> registry.yaml) in "Builtin Seed Reconciliation" and "Adding a Metric".
• Extended the registered presentation PRD/DESIGN with cpt-presentation-fr-metric-registry + cpt-presentation-component-metric-registry.
• The adopted semantic-layer PRD/DESIGN (docs(semantic-layer): adopt target architecture for Phase B #2184) record this registry as their Phase-1 down-payment.

Verification

• cargo test -p analytics: 561 passed, 0 failed (registry invariant tests validate the YAML-loaded data, incl. every_source_declares_at_least_one_measure).
• cargo clippy -p analytics: clean.
• cfs per-artifact validation of the presentation PRD/DESIGN: green; zero new whole-registry errors.

🤖 Generated with Claude Code

Summary by CodeRabbit

• New Features

  • Added a centralized metric registry covering AI usage, Git, collaboration, task tracking, and wiki activity.
  • Expanded metrics for adoption, delivery, collaboration, workflow, quality, and contribution insights.
  • Metric definitions are synchronized at startup, with missing definitions safely disabled.

• Reliability

  • Added validation to detect inconsistent, incomplete, or unsupported metric definitions.

• Documentation

  • Updated product and design documentation to describe the registry, supported scope, validation, and deferred work.

[coderabbitai]

Warning

Review limit reached

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

Next review available in: 55 minutes

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

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 Plus

Run ID: 940727cc-0584-4962-83e8-bf14ddefab94

📥 Commits

Reviewing files that changed from the base of the PR and between 27ce76e and 7119ad7.

📒 Files selected for processing (2)

• tests/stand/ui/pages/team_view.py
• tests/stand/ui/test_seeded_data_visible.py

📝 Walkthrough

Walkthrough

The analytics service now loads builtin sources and metrics from an embedded registry.yaml. Rust types deserialize the registry, tests validate its invariants, and seed reconciliation uses accessor-backed definitions. Design and product documents describe the registry scope and workflows.

Changes

Declarative metric registry

Layer / File(s)	Summary
Registry contract and declarations src/backend/services/analytics/Cargo.toml, src/backend/services/analytics/src/domain/metric_definitions/{definition.rs,builtin.rs,registry.yaml}	Added YAML deserialization support and defined five sources and 59 metrics in registry.yaml.
Registry loading and invariant validation src/backend/services/analytics/src/domain/metric_definitions/builtin.rs	Added embedded lazy parsing, public registry accessors, and tests for registry structure, references, computations, uniqueness, and presentation formats.
Builtin reconciliation integration src/backend/services/analytics/src/domain/metric_definitions/seeds.rs, docs/domain/{metrics,presentation-layer}/specs/{DESIGN.md,PRD.md}	Updated reconciliation to consume parsed definitions and documented the registry, startup behavior, and addition workflows.
Sample service configuration cleanup docker-compose.yml	Removed the duplicate AUTH_MODE environment entry and updated the related comment.

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
  participant registry as Embedded registry.yaml
  participant analytics as Analytics metric definitions
  participant database as Service database
  registry->>analytics: Provides sources and metrics
  analytics->>analytics: Deserializes and validates definitions
  analytics->>database: Reconciles builtin sources, measures, and metrics
  analytics->>database: Disables missing builtin measures

Loading

Possibly related PRs

• constructorfabric/insight#1656: Introduced the builtin registry that this change migrates to YAML.
• constructorfabric/insight#2003: Overlaps in builtin definitions and evidence metadata.
• constructorfabric/insight#2072: Overlaps in metric definition types and seed integration.

Suggested reviewers: ktursunov

🚥 Pre-merge checks | ✅ 2 | ❌ 3

❌ Failed checks (3 warnings)

Check name	Status	Explanation	Resolution
Linked Issues check	⚠️ Warning	The PR implements the analytics seed registry but does not collapse frontend thresholds and catalog specifications as requested by issue #1974.	Implement the frontend threshold and catalog integration, or split the issue and update its scope to cover only the analytics registry.
Out of Scope Changes check	⚠️ Warning	The docker-compose.yml AUTH_MODE cleanup is unrelated to the metric registry objectives in issue #1974.	Remove the unrelated docker-compose.yml change or move it to a separate pull request.
Docstring Coverage	⚠️ Warning	Docstring coverage is 44.44% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly identifies the primary change: introducing a declarative YAML metric registry.

✨ 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

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

Actionable comments posted: 4

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

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

245-255: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Guard the empty-measures case, which the registry migration makes reachable.

If a source declares measures: [], measure_keys is empty, placeholders is an empty string, and the SQL becomes measure_key NOT IN (). MariaDB rejects that as a syntax error, so startup reconciliation fails.

Before this PR the measure lists were Rust array literals. Now they come from registry.yaml, where measures: [] is valid YAML and no registry test forbids it. disable_missing at lines 275-280 already guards its own empty case, so this inline loop is the only unguarded site.

Add the guard, and add a registry invariant test that requires at least one measure per source.

🐛 Proposed fix

     for builtin_source in builtin_sources() {
         let source_id = fetch_source_id(db, &builtin_source.source.key).await?;
         let measure_keys = builtin_source
             .measures
             .iter()
             .map(String::as_str)
             .collect::<Vec<_>>();
-        let placeholders = vec!["?"; measure_keys.len()].join(", ");
-        let sql = format!(
-            "UPDATE metric_source_measures SET is_enabled = FALSE \
-             WHERE source_id = ? AND is_enabled = TRUE \
-               AND measure_key NOT IN ({placeholders})"
-        );
+        let base_sql = "UPDATE metric_source_measures SET is_enabled = FALSE \
+                        WHERE source_id = ? AND is_enabled = TRUE";
+        let sql = if measure_keys.is_empty() {
+            base_sql.to_owned()
+        } else {
+            let placeholders = vec!["?"; measure_keys.len()].join(", ");
+            format!("{base_sql} AND measure_key NOT IN ({placeholders})")
+        };

In builtin.rs tests:

#[test]
fn every_source_declares_at_least_one_measure() {
    for builtin_source in builtin_sources() {
        assert!(
            !builtin_source.measures.is_empty(),
            "source must declare at least one measure: {:?}",
            builtin_source.source.key
        );
    }
}

🤖 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/seeds.rs` around
lines 245 - 255, Guard the inline reconciliation SQL around
measure_keys/placeholders so empty measure lists do not generate NOT IN (); skip
the update when builtin_source.measures is empty, while preserving the existing
behavior for non-empty lists. In the builtin.rs test module, add
every_source_declares_at_least_one_measure to assert each builtin source has at
least one measure and identify the source key in failures.

🧹 Nitpick comments (2)

src/backend/services/analytics/src/domain/metric_definitions/builtin.rs (2)

152-154: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Move REGISTRY_YAML to the module top with the other constants.

The coding guidelines require constants at the module top, grouped. REGISTRY_YAML and the REGISTRY static sit between the type definitions and the accessors.

As per coding guidelines: "Define constants at module top, group them, and use unit-suffixed names such as _BYTES, _SECS, and _DAYS."

🤖 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/builtin.rs`
around lines 152 - 154, Move the REGISTRY_YAML constant to the module’s
top-level constants group, alongside the other constants, while leaving REGISTRY
and the accessor implementations unchanged.

Source: Coding guidelines

---

199-206: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Registry shape tests assert without per-case messages. The uniqueness and snake_case asserts report only a line number on failure, so a duplicate or malformed key in a 59-metric registry is hard to locate. The coding guidelines require per-case assertion messages.

• src/backend/services/analytics/src/domain/metric_definitions/builtin.rs#L199-L206: bind the key to a local and add messages such as "source key must be snake_case: {key:?}" and "duplicate source key: {key:?}".
• src/backend/services/analytics/src/domain/metric_definitions/builtin.rs#L223-L244: add the same messages to the measure and dimension asserts in measure_and_dimension_keys_are_unique_per_source, and to the uniqueness assert in metric_keys_are_unique_and_shaped.

As per coding guidelines: "Make tests read as specifications: use table-driven loops with per-case assertion messages such as "should reject: {input:?}"".

🤖 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/builtin.rs`
around lines 199 - 206, The registry shape tests need per-case assertion
messages identifying the failing key. In
src/backend/services/analytics/src/domain/metric_definitions/builtin.rs lines
199-206, bind each source key locally and include it in the snake_case and
duplicate-key assertions; in lines 223-244, add equivalent key-specific messages
to the measure and dimension assertions in
measure_and_dimension_keys_are_unique_per_source and the uniqueness assertion in
metric_keys_are_unique_and_shaped.

Source: Coding guidelines

🤖 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 `@docs/domain/metrics/specs/DESIGN.md`:
- Around line 363-366: Update the documentation around builtin_metrics() to
narrow the build-failure claim so it excludes undetected mistyped optional key
names, or add deny_unknown_fields to the relevant seed structs and retain the
claim only once unknown keys are rejected. Align the wording with the behavior
covered by the builtin.rs registry tests and the consolidated
deny_unknown_fields guidance.

In `@src/backend/services/analytics/src/domain/metric_definitions/builtin.rs`:
- Around line 83-85: Move evidence granularity ownership into the registry: in
src/backend/services/analytics/src/domain/metric_definitions/builtin.rs lines
83-85, replace BuiltinSource.measures entries with a MeasureSeed containing key
and required evidence_granularity, derive deserialization with snake_case
handling for EvidenceGranularity, and remove the hardcoded match fallback in
BuiltinSource::evidence_granularity. In docs/domain/metrics/specs/DESIGN.md
lines 636-637, retain step 5 as registry-driven; in lines 657-662, make the same
case 3 step 2 correction and clarify that Rust changes are still required even
though backend enum or table-name changes are not.
- Around line 110-150: Reject unknown YAML keys during registry deserialization
by adding deny-unknown-field validation to MetricSeed, InputSeed, Registry,
SourceSeed, and BuiltinSource in
src/backend/services/analytics/src/domain/metric_definitions/builtin.rs:65-81
and 110-150, and to ValueTransform in
src/backend/services/analytics/src/domain/metric_definitions/definition.rs:118-119
alongside its default attribute. The claim in
docs/domain/metrics/specs/DESIGN.md:363-366 requires no direct change once
validation is enabled.

In `@src/backend/services/analytics/src/domain/metric_definitions/registry.yaml`:
- Line 507: Update the user-facing label values for all collab metric
definitions in the registry so they use sentence casing, including the listed
Messages, Channel, Files, Meeting, email, and sharing labels. Preserve each
existing short_label value unchanged.

---

Outside diff comments:
In `@src/backend/services/analytics/src/domain/metric_definitions/seeds.rs`:
- Around line 245-255: Guard the inline reconciliation SQL around
measure_keys/placeholders so empty measure lists do not generate NOT IN (); skip
the update when builtin_source.measures is empty, while preserving the existing
behavior for non-empty lists. In the builtin.rs test module, add
every_source_declares_at_least_one_measure to assert each builtin source has at
least one measure and identify the source key in failures.

---

Nitpick comments:
In `@src/backend/services/analytics/src/domain/metric_definitions/builtin.rs`:
- Around line 152-154: Move the REGISTRY_YAML constant to the module’s top-level
constants group, alongside the other constants, while leaving REGISTRY and the
accessor implementations unchanged.
- Around line 199-206: The registry shape tests need per-case assertion messages
identifying the failing key. In
src/backend/services/analytics/src/domain/metric_definitions/builtin.rs lines
199-206, bind each source key locally and include it in the snake_case and
duplicate-key assertions; in lines 223-244, add equivalent key-specific messages
to the measure and dimension assertions in
measure_and_dimension_keys_are_unique_per_source and the uniqueness assertion in
metric_keys_are_unique_and_shaped.

🪄 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 Plus

Run ID: 84192fad-d1f9-4ea9-884c-b4155779b35f

📥 Commits

Reviewing files that changed from the base of the PR and between 98bd0c5 and 25f86469ac7232b6a4f33381648d020b4b975d77.

⛔ Files ignored due to path filters (1)

• src/backend/Cargo.lock is excluded by !**/*.lock

📒 Files selected for processing (8)

• docs/domain/metrics/specs/DESIGN.md
• docs/domain/presentation-layer/specs/DESIGN.md
• docs/domain/presentation-layer/specs/PRD.md
• src/backend/services/analytics/Cargo.toml
• 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/registry.yaml
• src/backend/services/analytics/src/domain/metric_definitions/seeds.rs