Skip to content

Commit 3318699

Browse files
Douglas Blankclaude
andcommitted
fix(admin): derive EM experiment KPI total from bucketed counts, not metadata
The EM EXPERIMENT_COUNT total (and its workspace roll-up) was sourced from `numberOfExperiments` metadata while the chart series was built from experiments bucketed by start_server_timestamp. When those disagree, the adoption KPI total didn't equal the cumulative chart sum. Derive `total` from the same counts map that feeds the series so they always match. Add a regression test covering the metadata-disagrees case. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent a825fcd commit 3318699

2 files changed

Lines changed: 56 additions & 15 deletions

File tree

cometx/cli/admin_growth_report.py

Lines changed: 14 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -851,17 +851,21 @@ def _collect_em(self, workspaces):
851851
ws_experiment_total = 0
852852
ws_counts: dict = defaultdict(int)
853853

854-
for project in tqdm(
855-
projects, desc=f"EM {ws}", unit="proj", leave=False
856-
):
854+
for project in tqdm(projects, desc=f"EM {ws}", unit="proj", leave=False):
857855
proj_name = project.get("projectName")
858856
try:
859857
# Fetch the project's experiments once and reuse the list
860858
# for both the creation proxy and the over-time series.
861859
experiments = self.api.get_experiments(ws, proj_name) or []
862860
created = self._em_project_created(project, experiments)
863861
counts = self._em_experiment_counts(experiments)
864-
total = project.get("numberOfExperiments", sum(counts.values()))
862+
# Derive the KPI total from the SAME bucketed counts that
863+
# feed the chart series -- NOT the `numberOfExperiments`
864+
# metadata, which can disagree with the experiments that
865+
# actually carry a `start_server_timestamp`. This keeps the
866+
# EM adoption KPI total (and its workspace roll-up) exactly
867+
# equal to the cumulative chart sum.
868+
total = sum(counts.values())
865869

866870
events.append(
867871
CreationEvent(
@@ -1017,9 +1021,7 @@ def _collect_opik(self, workspaces):
10171021

10181022
ws_counts: dict = defaultdict(float)
10191023

1020-
for project in tqdm(
1021-
projects, desc=f"Opik {ws}", unit="proj", leave=False
1022-
):
1024+
for project in tqdm(projects, desc=f"Opik {ws}", unit="proj", leave=False):
10231025
try:
10241026
if project.created_at is not None:
10251027
events.append(
@@ -1045,9 +1047,9 @@ def _collect_opik(self, workspaces):
10451047
counts: dict = defaultdict(float)
10461048
for result in resp.results or []:
10471049
for dp in result.data or []:
1048-
counts[
1049-
format_time_key(dp.time, self.units)
1050-
] += _as_float(dp.value)
1050+
counts[format_time_key(dp.time, self.units)] += _as_float(
1051+
dp.value
1052+
)
10511053

10521054
usage.append(
10531055
UsageMetric(
@@ -1128,8 +1130,7 @@ def _collect_mpm(self, workspaces):
11281130
for ws_entry in all_workspaces
11291131
# The MPM inventory shape is not verifiable live; guard against
11301132
# malformed elements so one bad entry can't crash the report.
1131-
if isinstance(ws_entry, dict)
1132-
and ws_entry.get("workspaceName") in requested
1133+
if isinstance(ws_entry, dict) and ws_entry.get("workspaceName") in requested
11331134
]
11341135
for ws_entry in tqdm(selected_entries, desc="MPM workspaces", unit="ws"):
11351136
ws = ws_entry.get("workspaceName")
@@ -1138,9 +1139,7 @@ def _collect_mpm(self, workspaces):
11381139
ws_counts: dict = defaultdict(float)
11391140
ws_total = 0.0
11401141

1141-
for model in tqdm(
1142-
models, desc=f"MPM {ws}", unit="model", leave=False
1143-
):
1142+
for model in tqdm(models, desc=f"MPM {ws}", unit="model", leave=False):
11441143
if not isinstance(model, dict):
11451144
continue
11461145
model_name = model.get("modelName")

tests/unit/test_admin_growth_report.py

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -212,6 +212,48 @@ def test_collect_em_usage_metrics_experiment_count_and_registry_snapshot():
212212
assert not any(m.metric.startswith("REGISTRY") and m.series for m in usage)
213213

214214

215+
def test_collect_em_kpi_total_matches_chart_sum_when_metadata_disagrees():
216+
# Regression: the EXPERIMENT_COUNT total must come from the SAME bucketed
217+
# timestamps that feed the chart series, NOT from `numberOfExperiments`.
218+
# Here the metadata (10) disagrees with the experiments that actually carry
219+
# a start_server_timestamp (3), so the old fallback would have produced a
220+
# KPI total that didn't equal the cumulative chart sum.
221+
from cometx.cli.admin_growth_report import GrowthReporter
222+
223+
api = MagicMock()
224+
api._client.get_from_endpoint.return_value = {
225+
"projects": [
226+
{
227+
"projectName": "proj1",
228+
"projectId": "p1",
229+
"workspaceName": "ws1",
230+
"numberOfExperiments": 10, # metadata, intentionally != 3
231+
"lastUpdated": 1700000000000,
232+
}
233+
]
234+
}
235+
api.get_experiments.return_value = [
236+
MagicMock(start_server_timestamp=1695000000000),
237+
MagicMock(start_server_timestamp=1695100000000),
238+
MagicMock(start_server_timestamp=1695200000000),
239+
]
240+
api.get_registry_model_names.return_value = []
241+
reporter = GrowthReporter(api, window="7d", units="month", platforms="em")
242+
_events, usage = reporter._collect_em(["ws1"])
243+
244+
proj_metric = next(
245+
m for m in usage if m.metric == "EXPERIMENT_COUNT" and m.project == "proj1"
246+
)
247+
ws_total = next(
248+
m for m in usage if m.metric == "EXPERIMENT_COUNT" and m.project is None
249+
)
250+
# total is derived from counts (3), NOT numberOfExperiments (10)
251+
assert proj_metric.value == 3
252+
assert ws_total.value == 3
253+
# the KPI total equals the cumulative chart sum for the workspace
254+
assert ws_total.value == sum(v for _k, v in ws_total.series)
255+
256+
215257
def test_collect_em_respects_limit_on_workspaces():
216258
from cometx.cli.admin_growth_report import GrowthReporter
217259

0 commit comments

Comments
 (0)