Skip to content
Merged
Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
de62478
test(metrics): migrate e2e coverage to unified metrics
aleksdotbar Jul 17, 2026
3a204d8
Merge branch 'main' into test/unified-metric-e2e-coverage
aleksdotbar Jul 17, 2026
59c57fb
Merge branch 'main' into test/unified-metric-e2e-coverage
aleksdotbar Jul 18, 2026
f65ba6a
Merge remote-tracking branch 'origin/main' into test/unified-metric-e…
aleksdotbar Jul 20, 2026
e0f4823
fix(metrics): stabilize unified e2e coverage
aleksdotbar Jul 20, 2026
76444d2
Merge remote-tracking branch 'origin/test/unified-metric-e2e-coverage…
aleksdotbar Jul 20, 2026
1bfae36
test(metrics): address unified e2e review findings
aleksdotbar Jul 20, 2026
3840b64
fix(metrics): align fixture schemas with bronze types
aleksdotbar Jul 20, 2026
6ed1746
Merge remote-tracking branch 'origin/main' into test/unified-metric-e…
aleksdotbar Jul 21, 2026
0a7021b
Merge branch 'main' into test/unified-metric-e2e-coverage
aleksdotbar Jul 21, 2026
24f844e
Merge remote-tracking branch 'origin/main' into test/unified-metric-e…
aleksdotbar Jul 22, 2026
86eb3c5
Merge remote-tracking branch 'origin/test/unified-metric-e2e-coverage…
aleksdotbar Jul 22, 2026
1a6c1e6
Merge branch 'main' into test/unified-metric-e2e-coverage
aleksdotbar Jul 22, 2026
46eb848
fix(e2e): add file_extension/change_type to git file-changes silver p…
aleksdotbar Jul 22, 2026
61adcd2
Merge remote-tracking branch 'origin/test/unified-metric-e2e-coverage…
aleksdotbar Jul 22, 2026
f83fdab
Merge branch 'main' into test/unified-metric-e2e-coverage
aleksdotbar Jul 22, 2026
1651a2d
Merge branch 'main' into test/unified-metric-e2e-coverage
aleksdotbar Jul 22, 2026
7ce63ca
fix(e2e): add destination_branch to git pull-requests silver placeholder
aleksdotbar Jul 22, 2026
d35e98b
Merge remote-tracking branch 'origin/test/unified-metric-e2e-coverage'
aleksdotbar Jul 22, 2026
302de58
test(metrics): expect null for empty-window metric values
aleksdotbar Jul 22, 2026
dd81147
Merge branch 'main' into test/unified-metric-e2e-coverage
aleksdotbar Jul 22, 2026
ce8bdc7
test(metrics): cover git.lines_removed and git.merge_rate breakdown
aleksdotbar Jul 22, 2026
8f1ef38
Merge remote-tracking branch 'origin/test/unified-metric-e2e-coverage…
aleksdotbar Jul 22, 2026
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions .github/workflows/e2e-bronze-to-api.yml
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ name: E2E — Bronze to API
# The metric-coverage-gate and api-endpoint-coverage-gate jobs then each
# analyse their own lane's artifact as a pure-Python check (no Docker, no app
# boot). See
# src/ingestion/tests/e2e/lib/{metric_coverage,collect_metrics,api_coverage}.py.
# src/ingestion/tests/e2e/lib/{metric_coverage,collect_metric_definitions,api_coverage}.py.
#
# Specs: docs/domain/bronze-to-api-e2e/specs/
# Code: src/ingestion/tests/e2e/
Expand Down Expand Up @@ -188,7 +188,7 @@ jobs:
# `./e2e.sh test meta/`.
#
# This session ALSO emits BOTH .artifacts/ files while analytics is up:
# • catalog_metrics.json (lib/collect_metrics.py)
# • metric_definitions.json (lib/collect_metric_definitions.py)
# • observed_endpoints.json (conftest.pytest_sessionfinish — httpx hook
# ledger)
# but only the lane-relevant one is consumed downstream: this api lane
Expand Down Expand Up @@ -294,7 +294,7 @@ jobs:
# `./e2e.sh test meta/`.
#
# This session ALSO emits BOTH .artifacts/ files while analytics is up:
# • catalog_metrics.json (lib/collect_metrics.py)
# • metric_definitions.json (lib/collect_metric_definitions.py)
# • observed_endpoints.json (conftest.pytest_sessionfinish — httpx hook
# ledger)
# but only the lane-relevant one is consumed downstream: this metrics
Expand Down Expand Up @@ -371,10 +371,10 @@ jobs:
run: |
pip install --quiet pyyaml
python3 src/ingestion/tests/e2e/lib/metric_coverage.py \
--universe-file coverage-inputs/catalog_metrics.json | tee metric_coverage.md
--universe-file coverage-inputs/metric_definitions.json | tee metric_coverage.md
rc=${PIPESTATUS[0]}
if [ -n "${GITHUB_STEP_SUMMARY:-}" ]; then cat metric_coverage.md >> "$GITHUB_STEP_SUMMARY"; fi
if [ "$rc" -ne 0 ]; then echo "::error::metric coverage gate failed — a metric_key is value-tested by no test and not in SKIP_TABLES/SKIP_LIST (or a skip is stale). See the report."; fi
if [ "$rc" -ne 0 ]; then echo "::error::metric coverage gate failed — builtin metric coverage is incomplete or stale. See the report."; fi
exit "$rc"

api-endpoint-coverage-gate:
Expand Down
4 changes: 2 additions & 2 deletions src/ingestion/dbt/macros/git_file_category.sql
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,9 @@
than an empty group `(?i)()`, which would match every path and flag all
files as vendored. `[^\s\S]` is the RE2-safe never-match (RE2 has no
lookahead), doubled-escaped for the ClickHouse string literal. #}
'[^\\s\\S]'
{{ return("'[^\\\\s\\\\S]'") }}
{%- else -%}
{{ "'(?i)(" ~ (patterns | join('|')) ~ ")'" }}
{{ return("'(?i)(" ~ (patterns | join('|')) ~ ")'") }}
{%- endif -%}
{%- endmacro %}

Expand Down
1 change: 1 addition & 0 deletions src/ingestion/silver/git/class_git_commits.sql
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
-- depends_on: {{ ref('github__commits') }}
-- depends_on: {{ ref('bitbucket_cloud__commits') }}
-- depends_on: {{ ref('gitlab__commits') }}
{{ config(
Expand Down
1 change: 1 addition & 0 deletions src/ingestion/silver/git/class_git_file_changes.sql
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
-- depends_on: {{ ref('github__file_changes') }}
-- depends_on: {{ ref('bitbucket_cloud__file_changes') }}
-- depends_on: {{ ref('gitlab__file_changes') }}
{{ config(
Expand Down
1 change: 1 addition & 0 deletions src/ingestion/silver/git/class_git_pull_requests.sql
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
-- depends_on: {{ ref('github__pull_requests') }}
-- depends_on: {{ ref('bitbucket_cloud__pull_requests') }}
-- depends_on: {{ ref('gitlab__pull_requests') }}
{{ config(
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
-- depends_on: {{ ref('github__pull_requests_comments') }}
-- depends_on: {{ ref('bitbucket_cloud__pull_requests_comments') }}
-- depends_on: {{ ref('gitlab__pull_requests_comments') }}
{{ config(
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
-- depends_on: {{ ref('github__pull_requests_commits') }}
-- depends_on: {{ ref('bitbucket_cloud__pull_requests_commits') }}
-- depends_on: {{ ref('gitlab__pull_requests_commits') }}
{{ config(
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
-- depends_on: {{ ref('github__pull_requests_reviewers') }}
-- depends_on: {{ ref('bitbucket_cloud__pull_requests_reviewers') }}
-- depends_on: {{ ref('gitlab__pull_requests_reviewers') }}
{{ config(
Expand Down
1 change: 1 addition & 0 deletions src/ingestion/silver/git/class_git_repositories.sql
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
-- depends_on: {{ ref('github__repositories') }}
-- depends_on: {{ ref('bitbucket_cloud__repositories') }}
-- depends_on: {{ ref('gitlab__repositories') }}
{{ config(
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
-- depends_on: {{ ref('github__repository_branches') }}
-- depends_on: {{ ref('bitbucket_cloud__repository_branches') }}
-- depends_on: {{ ref('gitlab__repository_branches') }}
{{ config(
Expand Down
65 changes: 25 additions & 40 deletions src/ingestion/tests/e2e/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,8 @@
Test framework that exercises the full data path:

```
metrics/<name>.test.yaml (bronze records) → bronze tables → dbt staging/silver →
ClickHouse migration gold-views → analytics HTTP (POST /v1/metrics/queries) → expect rules
metrics/<name>.test.yaml (bronze records) → bronze tables → dbt staging/silver/gold →
analytics HTTP (POST /v1/metric-results) → expect rules
```

Airbyte / Kestra / Argo are NOT exercised — bronze is seeded by direct INSERT of the
Expand Down Expand Up @@ -82,9 +82,9 @@ e2e/
│ ├── migration_applier.py # applies src/ingestion/scripts/migrations/*.sql
│ ├── analytics.py # builds + spawns the analytics binary
│ ├── worker.py # WorkerContext (resolves pytest-xdist worker id)
│ ├── metric_coverage.py # metric-coverage gate: SKIP_TABLES + SKIP_LIST (--universe-file)
│ ├── metric_coverage.py # builtin metric coverage gate
│ ├── api_coverage.py # endpoint-coverage report + httpx recording hook
│ ├── collect_metrics.py # script: snapshot the metric catalog → .artifacts/
│ ├── collect_metric_definitions.py # snapshot enabled builtin definitions
│ └── config.py # session config (ports, random creds)
├── seed/
│ └── metrics.yaml # optional test-specific metric overrides (default: empty)
Expand All @@ -94,31 +94,18 @@ e2e/

## Metric coverage gate

A job (`metric-coverage-gate`) in the **E2E — Bronze to API** workflow, *not* a pytest test. The `e2e-metrics` lane runs the metrics/ suite and, while analytics is up, snapshots the metric catalog (`POST /v1/catalog/get_metrics`) to `.artifacts/catalog_metrics.json` (uploaded as `coverage-inputs-metrics`); the gate job then checks every product `metric_key` the catalog exposes is value-asserted by a test or covered by a `SKIP_TABLES`/`SKIP_LIST` entry — pure Python, no Docker, no second app boot.
A job (`metric-coverage-gate`) in the **E2E — Bronze to API** workflow checks enabled builtin product definitions. The metrics lane snapshots `metric_key`, computation, dimensions, and cohort metadata to `.artifacts/metric_definitions.json`; the gate then verifies every builtin has period, peer, and timeseries assertions, plus breakdown assertions for declared dimensions and histogram assertions for median metrics.

Locally, after a run:

```bash
./e2e.sh test metrics/ # runs the metrics suite (emits both .artifacts files; only catalog_metrics.json feeds this gate)
./e2e.sh test metrics/ # runs the metrics suite and writes metric_definitions.json
./e2e.sh gates metrics # metric gate only, against .artifacts/ (in the runner image; no DB)
```

`./e2e.sh gates` with no argument runs both gates (handy after running both suites locally; see [API endpoint coverage gate](#api-endpoint-coverage-gate) below for the api/-only equivalent). `gates api` / `gates metrics` run one gate against one artifact each — that per-lane shape is what mirrors the two independent CI jobs, each of which only ever needs its own lane's artifact.

The verdict per **metric_key** (each individual number) is **binary**:

- **value-tested** — a `metrics/*.test.yaml` asserts it (`find: {metric_key: …}` paired with `equal`/`assert`) → **PASS**
- **skip-listed** (in the inline `SKIP_LIST` in [`lib/metric_coverage.py`](lib/metric_coverage.py)) → **PASS** (baseline)
- **neither** → **FAIL** — a number nobody validates must get an assertion or a `SKIP_LIST` entry.

Catalog keys are dotted (`collab_bullet_rows.m365_emails_sent`); a test asserts the bare response key (`m365_emails_sent`). The column suffix is unique across the catalog, so the gate maps bare→dotted by suffix (a future collision raises). `SKIP_LIST` is the accepted baseline and single source of truth (no side-car file — just `(metric_key, reason)`). Kept honest: a **stale** entry (key no longer in the catalog), a **redundant** one (now value-tested), or a test asserting a **non-catalog** key (typo / unseeded → matches 0 rows) all fail. PASS iff no FAILs.

```bash
# ad hoc against a running analytics (instead of the collected artifact):
ANALYTICS_URL=http://localhost:18081 python3 lib/metric_coverage.py
```

Coverage is **per metric_key**, so every number on a bullet is validated independently — one tested key of a metric does not cover the rest. Today: **44/96** value-tested; the rest are skip-listed with a reason (`reachable — …` entries are the backlog where fixtures already exist).
Missing views, unknown asserted keys, unknown requested keys, and stale fixtures fail. There are no legacy suffix mappings or skip lists.
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Resolved

## API endpoint coverage gate

Expand All @@ -133,7 +120,7 @@ Locally, after a run:

Per-status-code coverage is **reported, not enforced**: the report renders an endpoints × registered-status-codes table (`✓` observed · `✗` declared but not yet observed · `·` excluded · blank = not declared) and an overall coverage percentage. A code is *coverable* — and so counts toward the percentage — only if a black-box rig can produce it: `coverable(op) = declared(op) − {codes ≥ 500} − UNIVERSAL_BOILERPLATE{401,429} − BLOCKED[op]`. `BLOCKED` absorbs the committed spec's `.standard_errors` over-declaration (#1669) plus pinned rig/product limits, and a `·` code that becomes observed (or a `BLOCKED` op dropped from the spec) is surfaced as a non-blocking advisory so the list stays honest.

The [`api/`](api/) contract suite covers all 21 spec operations — one module per path group (`test_metrics.py`, `test_metric_thresholds.py`, `test_admin_thresholds.py`, `test_catalog.py`, `test_columns.py`, `test_persons.py`, `test_metric_results.py`), one test per (path, method, status-code) case, from self-cleaning fixtures (`api/conftest.py`) — so `SKIP_LIST` is empty and adding a spec operation without a test fails the gate as MISSING. Spec/product gaps are pinned by **strict xfails** rather than fixed here: #1663 (legacy threshold reads 500 once a row exists), #1664 (duplicate admin create answers 500 instead of the declared 409), and #1670 (off-schema legacy body answers a non-canonical 422, not the intended 400). `POST /v1/metric-results` — the unified-metric compute endpoint added by the `feat/unified-metrics` merge (#1656) — is covered on its deterministic error paths (400 empty/bad-period/unknown-key, 415 wrong content-type); its 200 happy-path needs seeded unified-metric observation data and shows as a reported `✗` gap until that fixture lands.
The [`api/`](api/) contract suite covers all 21 spec operations — one module per path group (`test_metrics.py`, `test_metric_thresholds.py`, `test_admin_thresholds.py`, `test_catalog.py`, `test_columns.py`, `test_persons.py`, `test_metric_results.py`), one test per (path, method, status-code) case, from self-cleaning fixtures (`api/conftest.py`). The declarative metrics suite covers successful `/v1/metric-results` computation for every builtin metric.

## Ports (loopback only)

Expand All @@ -153,16 +140,17 @@ These ports avoid conflict with a local gitops dev cluster (which forwards 8123

## `cases` / `expect` (declarative YAML rig)

Tests are `metrics/**/*.test.yaml`; each `case` POSTs a batch to `/v1/metrics/queries` and checks an `expect` list of rules. A rule selects with `in` (batch result by `id`) + an exact-equality `find` (`{field: value}`), then asserts via `equal` (subset of fields, exact / `null`) or `assert` (a CEL boolean). Anything richer than equality (inequalities, counts, predicates) goes in a CEL `assert` — the rig deliberately has no second selector language. See the [yaml-rig FEATURE](../../../../docs/domain/bronze-to-api-e2e/specs/feature-yaml-rig/FEATURE.md) and the `/metric` skill.
Tests are `metrics/**/*.test.yaml`; each case posts to `/v1/metric-results`. Rules select a full builtin `metric` key and `view`, then optionally use `find` for a row and `equal` for exact or null values. Use CEL `assert` for nested timeseries points, dimensions, histogram bins, counts, and predicates.

Variables available in an `assert` (CEL) expression — assembled in `lib/expect_engine.py::evaluate_case` (the `bindings` dict), converted to CEL in `_eval_cel`:

| Binding | Value | Present when |
|---------|-------|--------------|
| `it` | the single row matched by `find` | only with `find` (else `null`) |
| `items` | the selected result's `items` array | a result is selected (`in` or sole query) |
| `result` | the selected batch result `{id, status, metric_id, items, page_info}` | a result is selected |
| `results` | the full `results[]` of the batch | always |
| `items` | values or series from the selected view | a metric view is selected |
| `view` | the selected view object | a metric view is selected |
| `metric` | the selected metric result | a metric is selected |
| `metrics` | all metric results | always |
| `status` | the batch HTTP status code (int) | always |

CEL is strictly typed and will not compare an `int` to a `double`. Bindings are passed through unchanged, so when a metric value may be integral (e.g. `40`) and you compare against a fractional literal, cast it: `double(it.value) > 39.5`. `status` and `size(...)` are integers — compare them with integer literals. Use `equal` for exact / `null` comparisons (it uses Python `==`).
Expand All @@ -171,25 +159,22 @@ CEL is strictly typed and will not compare an `int` to a `double`. Bindings are

`assert` expressions are written in **CEL — the [Common Expression Language](https://github.com/google/cel-spec)** (the same expression language used by Kubernetes admission policies and Envoy). It is a small, side-effect-free language for boolean/value expressions over structured data: no statements, no loops, no I/O — an expression is evaluated against the bindings above and must return a boolean. The rig evaluates it with the [`cel-python`](https://pypi.org/project/cel-python/) library (`celpy`) in `lib/expect_engine.py::_eval_cel`.

Operators: `== != < <= > >=`, `&& || !`, `+ - * / %`, `in`, ternary `cond ? a : b`. Field/index access: `it.value`, `result.status`, `items[0]`. Useful built-ins & macros: `size(x)`, `has(x.field)`, `x.exists(e, <pred>)`, `x.all(e, <pred>)`, `x.filter(e, <pred>)`, `x.map(e, <expr>)`, string `.startsWith()/.endsWith()/.contains()/.matches(re)`.
Operators: `== != < <= > >=`, `&& || !`, `+ - * / %`, `in`, ternary `cond ? a : b`. Field/index access: `it.value`, `view.values`, `items[0]`. Useful built-ins & macros: `size(x)`, `has(x.field)`, `x.exists(e, <pred>)`, `x.all(e, <pred>)`, `x.filter(e, <pred>)`, `x.map(e, <expr>)`, string `.startsWith()/.endsWith()/.contains()/.matches(re)`.

Examples:

```yaml
- assert: "status == 200" # batch HTTP code
- in: collaboration
assert: "result.status == 'ok'" # this query's own status
- in: collaboration
assert: "size(items) == 20" # row count
- in: collaboration
find: { metric_key: m365_emails_sent }
assert: "double(it.value) > 39.5 && double(it.value) < 40.5" # cast to double for fractional compare
- in: collaboration
find: { metric_key: slack_dm_ratio }
assert: "it.value == null" # explicit null
- assert: "results.exists(r, r.status == 'error')" # any query in the batch failed?
- in: collaboration
assert: "items.all(r, r.range_min <= r.value)" # invariant across all rows
- assert: "status == 200"
- metric: collab.emails_sent
view: period
find: { entity_id: erin@example.com }
equal: { value: 50 }
- metric: collab.messages_sent
view: timeseries
assert: "items.exists(s, s.points.exists(p, double(p.value) > 0.0))"
- metric: git.lines_added
view: breakdown
assert: "items.exists(v, v.dimensions.exists(d, d.key == 'category' && d.value == 'code'))"
```

Prefer `equal` for exact / `null` checks (it uses Python `==`, so `40 == 40.0` and `value: null` work directly); reach for `assert` when you need inequalities, counts, or cross-row predicates.
12 changes: 3 additions & 9 deletions src/ingestion/tests/e2e/api/test_metric_results.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,22 +3,16 @@
POST /v1/metric-results 200 compute · 400 (empty/bad-period/unknown-key) · 415 wrong-ct

Added when the `feat/unified-metrics` merge (#1656) introduced this operation to
the committed spec. It computes results for CATALOG metrics (`metric_key` +
computation spec over the observation gold-views) — a different system from the
legacy `query_ref` metrics the `/v1/metrics` CRUD and the scratch fixtures use.
the committed spec. It computes builtin metrics over unified observation models.

The endpoint validates its request body in `domain::metric_results::validate_request`
BEFORE touching ClickHouse, so the whole 400 family is deterministic in the rig:
an empty `metrics`, a malformed/reversed `period`, and an unknown `metric_key`
(which is a 400 via `unavailable`, NOT a 404) all reject up front. Wrong
Content-Type is a 415 at the `axum::Json` extractor.

The 200 happy-path is deferred: a deterministic compute needs a seeded catalog
metric whose observation gold-view is built in the rig, which this branch does
not yet stand up — the coverage report marks metric-results' 200 as a `✗` gap
until that fixture lands. The endpoint-coverage gate blocks on an *unexercised*
operation, not on that per-code gap, so these error-path cases are enough to
keep the new endpoint covered.
The declarative metric suite exercises deterministic 200 responses for every
builtin metric; this module retains the endpoint contract error cases.
"""

from __future__ import annotations
Expand Down
Loading
Loading