fix(analytics): gate the per-tenant metric filter behind config (default off)

[cyberantonz]

Problem

#1967 replaced the metric compiler's shared-WHERE no-op with a hard tenant_id = <session tenant> on every observation and cohort read (metric_where, shared_observation_where, and the peer-cohort CTEs). It binds ctx.subject_tenant_id() — the JWT tenant.

But the observation tenant_id is a connector-stamped Nullable(String) carried verbatim bronze → class_* → gold; it is not yet aligned to the JWT tenant representation (that alignment is #1829). So the exact match now filters out every row: existing metrics return no data, with no error. Before #1967 the no-op masked this.

Fix

Gate the injection behind a new metric_catalog.enforce_tenant_scope flag, defaulting to false — there is no tenant isolation in the bronze/silver/gold data today, so enforcing here only empties reads.

• Handler sets the flag from config onto the validated request.
• Compiler selects the term via a tenant_predicate(enforce) helper:
  • true → tenant_id = ? (the [pres] Flat tenant data-row filter #1967 behavior)
  • false → (tenant_id = ? OR 1 = 1) — a tautology that still binds the same one placeholder, so param arity is identical in both modes (no bind-order risk); this is the pre-[pres] Flat tenant data-row filter #1967 behavior.
• Env override: APP__gears__analytics__config__metric_catalog__enforce_tenant_scope=true.

Flip the flag on per environment once the ingest tenant is aligned to the JWT tid (#1829).

Tests

cargo test -p analytics green (533 passing). Enforcement stays covered — the compiler fixtures pin enforce_tenant_scope: true, so the existing "tenant predicate leads every read" assertions are unchanged. A new test covers the bypass form and asserts placeholder arity is unchanged in both modes. cargo clippy -p analytics --all-targets clean.

Notes

• No e2e regression: the metric e2e suite seeds and queries with the harness tenant, so an un-enforced (more permissive) filter still returns those rows; no metric e2e asserts cross-tenant emptiness.
• The DESIGN's tenant-isolation guarantee (cpt-presentation-component-metric-compiler / the isolation NFR) becomes config-gated. I'll reconcile that wording in the presentation spec once the in-flight [pres] Contract version stamp #1969/[pres] Query console FE #1970 spec PRs land, to avoid three branches editing DESIGN.md concurrently.

Refs #1967, #1829

Summary by CodeRabbit

• New Features
  • Added a configurable tenant-scoping option for metric results.
  • Metric queries can now restrict observations to the applicable tenant when enabled.
  • Supports consistent scoping across time series, rankings, breakdowns, histograms, peer comparisons, and shared observations.
  • Tenant scoping is disabled by default for backward compatibility.
• Bug Fixes
  • Improved consistency in tenant filtering and query parameter handling across metric result types.

[coderabbitai]

📝 Walkthrough

Walkthrough

Metric result requests now receive tenant-scope enforcement from metric catalog configuration. Query compilation applies exact or bypass tenant predicates across metric and peer queries while preserving SQL parameter ordering. Fixtures and tests cover both enforcement modes.

Changes

Metric tenant-scope control

Layer / File(s)	Summary
Request scope contract and configuration src/backend/services/analytics/src/config.rs, src/backend/services/analytics/src/domain/metric_results/validation.rs, src/backend/services/analytics/src/api/metric_results.rs	Adds enforce_tenant_scope to configuration and validated requests. The API handler copies the configured value before authorization and query processing.
Tenant predicate query compilation src/backend/services/analytics/src/domain/metric_results/compiler.rs	Applies configurable tenant predicates to timeseries, ranking, capped-timeseries, breakdown, histogram, shared observation, and peer queries. Centralizes peer cohort parameter binding.
Fixture and bypass coverage src/backend/services/analytics/src/domain/metric_results/batch.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/validation.rs	Updates request fixtures and verifies disabled tenant scoping, tautological predicates, retained tenant parameters, and placeholder alignment.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Sequence Diagram(s)

sequenceDiagram
  participant MetricResultsAPI
  participant MetricCatalogConfig
  participant MetricResultsCompiler
  participant SQLQuery
  MetricResultsAPI->>MetricCatalogConfig: Read enforce_tenant_scope
  MetricResultsAPI->>MetricResultsCompiler: Pass validated request
  MetricResultsCompiler->>SQLQuery: Build tenant-scoped metric and peer predicates

Loading

Possibly related PRs

• constructorfabric/insight#1656: Extends the same metric results validation, API, compiler, and fixture paths.
• constructorfabric/insight#1701: Introduces related metric-results batching and compiler paths.
• constructorfabric/insight#2086: Modifies the same request validation and tenant-scoping predicates.

Suggested reviewers: ktursunov, aleksdotbar

🚥 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 clearly and concisely describes gating per-tenant metric filtering behind configuration with the default disabled.
Docstring Coverage	✅ Passed	Docstring coverage is 80.00% which is sufficient. The required threshold is 80.00%.
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

---

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]

🧹 Nitpick comments (1)

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

88-94: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Remove source comments that duplicate rollout context.

Move environment-variable instructions and rollout rationale to configuration documentation. Remove issue references from source. Keep only a concise // INVARIANT: comment when code and a specification-named test cannot express the rule.

• src/backend/services/analytics/src/config.rs#L88-L94: move the field rationale and environment-variable path out of the service source.
• src/backend/services/analytics/src/domain/metric_results/validation.rs#L41-L43: remove the field documentation comment.
• src/backend/services/analytics/src/domain/metric_results/validation.rs#L186-L188: remove the initialization comment.
• src/backend/services/analytics/src/domain/metric_results/compiler.rs#L511-L512: remove the parameter-order comment.
• src/backend/services/analytics/src/domain/metric_results/compiler.rs#L754-L755: remove the private-helper documentation comment.
• src/backend/services/analytics/src/domain/metric_results/compiler.rs#L837-L845: reduce this to a concise invariant without issue references.
• src/backend/services/analytics/src/domain/metric_results/compiler.rs#L850-L852: remove the bypass comment.
• src/backend/services/analytics/src/domain/metric_results/compiler.rs#L1253-L1255: remove the test implementation comment.
• src/backend/services/analytics/src/domain/metric_results/compiler.rs#L1268-L1268: remove the test implementation comment.

As per coding guidelines, “Use comments only when code cannot express the reason” and keep issue context in issues or PRs.

🤖 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/config.rs` around lines 88 - 94, Remove
rollout rationale, environment-variable guidance, issue references, and
implementation comments at src/backend/services/analytics/src/config.rs:88-94,
src/backend/services/analytics/src/domain/metric_results/validation.rs:41-43 and
186-188, and
src/backend/services/analytics/src/domain/metric_results/compiler.rs:511-512,
754-755, 850-852, 1253-1255, and 1268-1268; move configuration guidance to
documentation. At compiler.rs:837-845, retain only a concise // INVARIANT:
comment expressing the rule without issue references.

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.

Nitpick comments:
In `@src/backend/services/analytics/src/config.rs`:
- Around line 88-94: Remove rollout rationale, environment-variable guidance,
issue references, and implementation comments at
src/backend/services/analytics/src/config.rs:88-94,
src/backend/services/analytics/src/domain/metric_results/validation.rs:41-43 and
186-188, and
src/backend/services/analytics/src/domain/metric_results/compiler.rs:511-512,
754-755, 850-852, 1253-1255, and 1268-1268; move configuration guidance to
documentation. At compiler.rs:837-845, retain only a concise // INVARIANT:
comment expressing the rule without issue references.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 19ba34fd-4597-4f98-8c6d-1e00ff69fd9d

📥 Commits

Reviewing files that changed from the base of the PR and between 87fd1d6 and 4682d9d.

📒 Files selected for processing (6)

• src/backend/services/analytics/src/api/metric_results.rs
• src/backend/services/analytics/src/config.rs
• src/backend/services/analytics/src/domain/metric_results/batch.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/validation.rs