Skip to content

Commit a1b5a8b

Browse files
fix(growth-report): implement PR #26 review findings (dsblank)
- #1 workspace_active_series: report real per-bucket active totals instead of a flat total repeated across the series. - #2 barsLine/lines: add right-axis scale (rAxis) for the secondary rate series and wire hover tooltips (attachTip) with per-column labels; remove dead indexOfKey/keyInRange JS helpers. - #4 service accounts: drop the 'users' container key and match a service account by username OR email (mirror-failure safe). - #5 workspace-growth KPI: surface the membership-proxy basis in the KPI sub ("+N new (est. from earliest member)"). - #6 scoping: add a caveat that per-user totals stay org-wide when workspaces are scoped. - #7 lint/misc: fix Python E501s + stale docstrings; warn on growth series bucket truncation; pass reportMonth as a URL-encoded query param (and stop migrate_users' client dropping params); make _num rounding consistent; drop 7 dead window_start/window_end chart keys. - tests: render coverage for barsLine, groupedBarsH, and that the leaderboards / personal-vs-service sections reach the rendered body. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent 9648b25 commit a1b5a8b

9 files changed

Lines changed: 284 additions & 79 deletions

cometx/cli/admin.py

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -464,7 +464,9 @@ def get_parser_arguments(parser):
464464
)
465465

466466
# growth-report subcommand
467-
growth_report_description = """Generate a cross-platform use-case growth report for one or more workspaces.
467+
growth_report_description = """\
468+
Generate an org-wide people/usage growth report from the admin \
469+
chargeback report.
468470
469471
Arguments:
470472
WORKSPACE (optional, one or more)
@@ -488,7 +490,10 @@ def get_parser_arguments(parser):
488490
"""
489491
growth_parser = subparsers.add_parser(
490492
"growth-report",
491-
help="Generate a cross-platform use-case growth report for one or more workspaces",
493+
help=(
494+
"Generate an org-wide people/usage growth report from the "
495+
"admin chargeback report"
496+
),
492497
description=growth_report_description,
493498
formatter_class=argparse.RawDescriptionHelpFormatter,
494499
)

cometx/cli/admin_growth_render.py

Lines changed: 43 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -21,11 +21,11 @@
2121
calls, no secrets -- `report_data` never carries an API key.
2222
2323
Charts render as plain month-on-month / week-on-week series with horizontal
24-
gridlines for scale. The earlier single-analysis-window overlay (a shaded
25-
`[window_start, window_end]` band with accent-vs-muted fill and start/end
26-
dots) has been removed now that the report is period-over-period; the
27-
`window_start`/`window_end` fields in chart data are retained but no longer
28-
drawn. A window chip is still rendered next to each section title.
24+
gridlines for scale. An earlier single-analysis-window overlay (a shaded
25+
band with start/end dots) was removed once the report became
26+
period-over-period; the now-defunct `window_start`/`window_end` chart-data
27+
fields were dropped with it. A window chip is still rendered next to each
28+
section title.
2929
"""
3030

3131
from __future__ import annotations
@@ -207,17 +207,6 @@ def _fmt(value) -> str:
207207
return node;
208208
}
209209
function fmt(n){ return (n || 0).toLocaleString("en-US"); }
210-
function indexOfKey(points, key){
211-
if(key === undefined || key === null) return -1;
212-
for(var i = 0; i < points.length; i++){ if(points[i].key === key) return i; }
213-
return -1;
214-
}
215-
function keyInRange(key, start, end){
216-
if(key === undefined || key === null) return false;
217-
if(start !== undefined && start !== null && key < start) return false;
218-
if(end !== undefined && end !== null && key > end) return false;
219-
return true;
220-
}
221210
222211
var W = 560, H = 220, P = {t: 16, r: 14, b: 28, l: 44};
223212
@@ -232,7 +221,7 @@ def _fmt(value) -> str:
232221
var guide = el("line", {class: "guide", y1: P.t, y2: H - P.b, x1: 0, x2: 0, opacity: "0"});
233222
svg.appendChild(guide);
234223
function show(col){
235-
tip.textContent = col.key + ": " + fmt(col.value);
224+
tip.textContent = col.label || (col.key + ": " + fmt(col.value));
236225
tip.classList.add("show");
237226
var r = host.getBoundingClientRect();
238227
tip.style.left = (col.x / W) * r.width + "px";
@@ -261,10 +250,22 @@ def _fmt(value) -> str:
261250
}
262251
}
263252
253+
// Right-hand axis ticks (unlabelled gridlines omitted; the left axis owns
254+
// the grid) for a secondary series drawn on its own scale. `suffix` labels
255+
// the units, e.g. "%". Colored to match the series it measures.
256+
function rAxis(svg, yOf, maxVal, iw, suffix, color){
257+
for(var g = 1; g <= 4; g++){
258+
var v = maxVal * g / 4, y = yOf(v);
259+
var t = el("text", {x: P.l + iw + 6, y: y + 3, "text-anchor": "start",
260+
fill: color || tok("--muted"), "font-size": "10", "font-family": "var(--mono)"});
261+
t.textContent = fmt(Math.round(v)) + (suffix || ""); svg.appendChild(t);
262+
}
263+
}
264+
264265
function drawBars(host, data){
265266
data = data || {}; var points = data.points || [];
266267
if(!points.length){ host.innerHTML = "<p class=\\"nodata\\">No data</p>"; return; }
267-
var accent = tok("--accent"), mute = tok("--bar-mute"), band = tok("--accent-soft");
268+
var accent = tok("--accent");
268269
var svg = el("svg", {viewBox: "0 0 " + W + " " + H, role: "img"});
269270
var iw = W - P.l - P.r, ih = H - P.t - P.b, n = points.length;
270271
var max = 1; points.forEach(function(p){ max = Math.max(max, p.value || 0); });
@@ -290,7 +291,6 @@ def _fmt(value) -> str:
290291
data = data || {}; var points = data.points || []; var cats = data.categories || [];
291292
if(!points.length || !cats.length){ host.innerHTML = "<p class=\\"nodata\\">No data</p>"; return; }
292293
var colors = (data.colors && data.colors.length) ? data.colors : ["--accent", "--sdk", "--ok", "--warn"];
293-
var accent = tok("--accent"), band = tok("--accent-soft");
294294
var svg = el("svg", {viewBox: "0 0 " + W + " " + H, role: "img"});
295295
var iw = W - P.l - P.r, ih = H - P.t - P.b, n = points.length;
296296
var max = 1;
@@ -321,6 +321,16 @@ def _fmt(value) -> str:
321321
fill: tok("--muted"), "font-size": "10", "font-family": "var(--mono)"});
322322
xl.textContent = points[i].key; svg.appendChild(xl);
323323
});
324+
// Per-x tooltips: one column spanning the gap around each point, listing
325+
// every category's value at that x.
326+
var labels = data.labels || {};
327+
var slot = n > 1 ? iw / (n - 1) : iw;
328+
var cols = points.map(function(p, i){
329+
var parts = [p.key];
330+
cats.forEach(function(c){ parts.push((labels[c] || c) + " " + fmt((p.values && p.values[c]) || 0)); });
331+
return {x: X(i), x0: X(i) - slot / 2, w: slot, key: p.key, label: parts.join(" \\u00b7 ")};
332+
});
333+
attachTip(host, svg, cols);
324334
host.appendChild(svg);
325335
}
326336
@@ -373,8 +383,13 @@ def _fmt(value) -> str:
373383
if(!points.length || !bars.length){ host.innerHTML = "<p class=\\"nodata\\">No data</p>"; return; }
374384
var barColors = (data.bar_colors && data.bar_colors.length) ? data.bar_colors : ["--ok", "--warn"];
375385
var lineColor = data.line_color || "--accent";
386+
var lineSuffix = data.line_suffix || "";
387+
var barLabels = data.bar_labels || {};
376388
var svg = el("svg", {viewBox: "0 0 " + W + " " + H, role: "img"});
377-
var iw = W - P.l - P.r, ih = H - P.t - P.b, n = points.length;
389+
// Reserve extra right margin for the secondary (line) axis labels when a
390+
// line is present, so they don't collide with the plot area.
391+
var rpad = lineCat ? 34 : 0;
392+
var iw = W - P.l - P.r - rpad, ih = H - P.t - P.b, n = points.length;
378393
var lmax = 1, rmax = 1;
379394
points.forEach(function(p){
380395
bars.forEach(function(b){ lmax = Math.max(lmax, (p.values && p.values[b]) || 0); });
@@ -385,7 +400,9 @@ def _fmt(value) -> str:
385400
var YR = function(v){ return P.t + ih * (1 - v / rmax); };
386401
var cx = function(i){ return P.l + slot * i + slot / 2; };
387402
hGrid(svg, YL, lmax, iw);
403+
if(lineCat) rAxis(svg, YR, rmax, iw, lineSuffix, tok(lineColor));
388404
svg.appendChild(el("line", {class: "axis-base", x1: P.l, x2: P.l + iw, y1: P.t + ih, y2: P.t + ih}));
405+
var cols = [];
389406
points.forEach(function(p, i){
390407
var gx = P.l + slot * i + (slot - group) / 2;
391408
bars.forEach(function(b, bi){
@@ -394,6 +411,11 @@ def _fmt(value) -> str:
394411
svg.appendChild(el("rect", {x: gx + bw * bi, y: P.t + ih - h, width: Math.max(bw - 2, 1),
395412
height: h, rx: "2", fill: tok(barColors[bi % barColors.length])}));
396413
});
414+
// Per-column tooltip label: each bar series + the line value.
415+
var parts = [p.key];
416+
bars.forEach(function(b){ parts.push((barLabels[b] || b) + " " + fmt((p.values && p.values[b]) || 0)); });
417+
if(lineCat){ parts.push((barLabels[lineCat] || lineCat) + " " + fmt((p.values && p.values[lineCat]) || 0) + lineSuffix); }
418+
cols.push({x: cx(i), x0: P.l + slot * i, w: slot, key: p.key, label: parts.join(" \\u00b7 ")});
397419
if(i % Math.max(1, Math.ceil(n / 8)) === 0){
398420
var t = el("text", {x: cx(i), y: H - 8, "text-anchor": "middle", fill: tok("--muted"),
399421
"font-size": "10", "font-family": "var(--mono)"});
@@ -409,6 +431,7 @@ def _fmt(value) -> str:
409431
svg.appendChild(el("circle", {cx: cx(i), cy: YR((p.values && p.values[lineCat]) || 0), r: "2.5", fill: tok(lineColor)}));
410432
});
411433
}
434+
attachTip(host, svg, cols);
412435
host.appendChild(svg);
413436
}
414437

cometx/cli/admin_growth_report.py

Lines changed: 40 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -11,11 +11,13 @@
1111
# Copyright (c) 2024 Cometx Development
1212
# Team. All rights reserved.
1313
# ****************************************
14-
"""cometx admin growth-report — cross-platform use-case growth & rates, per
15-
workspace/department (Opik + EM + MPM), rendered as a self-contained HTML page.
14+
"""cometx admin growth-report — org-wide people/usage growth from the admin
15+
chargeback report, rendered as a self-contained HTML page.
1616
17-
Distinct from `admin usage-report` (experiment counts over time, PDF/Streamlit):
18-
growth-report tracks cross-platform use-case creation growth and rates.
17+
Sources exclusively from `/api/admin/chargeback/report` (admin API key
18+
required): organization overview, users, leaderboards, and a
19+
personal-vs-service-account split. Distinct from `admin usage-report`
20+
(experiment counts over time, PDF/Streamlit).
1921
"""
2022

2123
from __future__ import annotations
@@ -112,11 +114,17 @@ def _extract_service_account_names(payload) -> "set[str] | None":
112114
"""Defensively unwrap the `/admin/service-accounts` response into a
113115
flat set of account names, tolerating several plausible response
114116
shapes since the exact schema isn't documented in this codebase: a
115-
bare list of entries, or a dict wrapping that list under a common
116-
container key (`serviceAccounts`, `accounts`, `users`). Each entry may
117-
be a plain string, or a dict carrying the name under `name`,
118-
`username`, or `email` (mirrors `_extract_licensed_users`'s defensive
119-
style in `admin_growth_users.py`).
117+
bare list of entries, or a dict wrapping that list under a
118+
service-account-specific container key (`serviceAccounts`,
119+
`accounts`). Each entry may be a plain string, or a dict carrying the
120+
name under `name`, `username`, or `email` (mirrors
121+
`_extract_licensed_users`'s defensive style in `admin_growth_users.py`).
122+
123+
A generic `users` key is deliberately NOT accepted: if the endpoint
124+
ever returned the full user roster under it, every user would be
125+
classified as a service account and the report would label that
126+
inversion authoritative ("admin API"). Better to fail the parse and
127+
fall back to the labeled heuristic.
120128
121129
Returns `None` (not an empty set) when the payload cannot be honestly
122130
parsed as a service-account container -- either because its shape
@@ -131,7 +139,7 @@ def _extract_service_account_names(payload) -> "set[str] | None":
131139
container = payload
132140
recognized = isinstance(payload, list)
133141
if isinstance(payload, dict):
134-
for key in ("serviceAccounts", "accounts", "users"):
142+
for key in ("serviceAccounts", "accounts"):
135143
if key in payload:
136144
container = payload[key]
137145
recognized = isinstance(container, list)
@@ -493,8 +501,6 @@ def _build_unified_section(
493501
"labels": {"total": "Total", "active": "Active"},
494502
"colors": ["--sdk", "--ok"],
495503
"points": ws_active_pts,
496-
"window_start": None,
497-
"window_end": None,
498504
},
499505
}
500506
)
@@ -544,12 +550,15 @@ def _build_unified_section(
544550
"points": churn_pts,
545551
"bars": ["added", "deleted"],
546552
"line": "rate",
547-
"bar_labels": {"added": "Added", "deleted": "Deleted"},
553+
"bar_labels": {
554+
"added": "Added",
555+
"deleted": "Deleted",
556+
"rate": "Growth rate",
557+
},
548558
"bar_colors": ["--ok", "--warn"],
549559
"line_label": "Growth rate",
550560
"line_color": "--accent",
551-
"window_start": None,
552-
"window_end": None,
561+
"line_suffix": "%",
553562
},
554563
}
555564
)
@@ -601,7 +610,7 @@ def _build_unified_section(
601610
{
602611
"label": f"New in {self.window or '7d'} (% of base)",
603612
"value": f"{wg['pct']}%",
604-
"sub": f"+{wg['new_in']} new",
613+
"sub": f"+{wg['new_in']} new (est. from earliest member)",
605614
},
606615
{
607616
"label": "Active workspaces %",
@@ -686,8 +695,6 @@ def _build_people_section(self, users, now_ms, window=None):
686695
"labels": {"total": "Total", "active": "Active"},
687696
"colors": ["--sdk", "--ok"],
688697
"points": active_pts,
689-
"window_start": None,
690-
"window_end": None,
691698
},
692699
}
693700
]
@@ -709,7 +716,10 @@ def _build_people_section(self, users, now_ms, window=None):
709716
"id": "chart-people-adoption-rate",
710717
"kind": "lines",
711718
"title": "Adoption rates",
712-
"hint": f"active users / total; active window {self.active_window} · {self._units_adverb()}",
719+
"hint": (
720+
f"active users / total; active window "
721+
f"{self.active_window} · {self._units_adverb()}"
722+
),
713723
"legend": [
714724
{"label": lbl, "color": col} for _, lbl, col in rate_spec
715725
],
@@ -718,8 +728,6 @@ def _build_people_section(self, users, now_ms, window=None):
718728
"labels": {k: lbl for k, lbl, _ in rate_spec},
719729
"colors": [col for _, _, col in rate_spec],
720730
"points": rate_pts,
721-
"window_start": None,
722-
"window_end": None,
723731
},
724732
}
725733
)
@@ -731,7 +739,10 @@ def _build_people_section(self, users, now_ms, window=None):
731739
"id": "chart-people-capability",
732740
"kind": "lines",
733741
"title": "Active users by capability",
734-
"hint": f"active window {self.active_window} · {self._units_adverb()}",
742+
"hint": (
743+
f"active window {self.active_window} · "
744+
f"{self._units_adverb()}"
745+
),
735746
"legend": [
736747
{"label": "EM", "color": "--sdk"},
737748
{"label": "Opik", "color": "--accent"},
@@ -741,8 +752,6 @@ def _build_people_section(self, users, now_ms, window=None):
741752
"labels": {"em": "EM", "opik": "Opik"},
742753
"colors": ["--sdk", "--accent"],
743754
"points": cap_pts,
744-
"window_start": None,
745-
"window_end": None,
746755
},
747756
}
748757
)
@@ -771,8 +780,6 @@ def _build_people_section(self, users, now_ms, window=None):
771780
},
772781
"colors": ["--sdk", "--accent", "--warn"],
773782
"points": em_pts,
774-
"window_start": None,
775-
"window_end": None,
776783
},
777784
}
778785
)
@@ -799,8 +806,6 @@ def _build_people_section(self, users, now_ms, window=None):
799806
},
800807
"colors": ["--accent", "--ok"],
801808
"points": opik_pts,
802-
"window_start": None,
803-
"window_end": None,
804809
},
805810
}
806811
)
@@ -813,8 +818,8 @@ def _build_people_section(self, users, now_ms, window=None):
813818
"kind": "lines",
814819
"title": "Users added vs. deleted",
815820
"hint": (
816-
f"per period · {self._units_adverb()}; deletions reflect soft-deletes "
817-
"still present in the snapshot"
821+
f"per period · {self._units_adverb()}; deletions reflect "
822+
"soft-deletes still present in the snapshot"
818823
),
819824
"legend": [
820825
{"label": "Added", "color": "--ok"},
@@ -825,8 +830,6 @@ def _build_people_section(self, users, now_ms, window=None):
825830
"labels": {"added": "Added", "deleted": "Deleted"},
826831
"colors": ["--ok", "--warn"],
827832
"points": user_churn,
828-
"window_start": None,
829-
"window_end": None,
830833
},
831834
}
832835
)
@@ -839,7 +842,7 @@ def _build_people_section(self, users, now_ms, window=None):
839842
[
840843
u.username,
841844
_num(u.experiment_count),
842-
_num(u.data_logged_mb),
845+
_num(round(u.data_logged_mb)),
843846
_num(u.opik_span_count) if u.opik_span_count is not None else "-",
844847
]
845848
for u in top
@@ -1066,7 +1069,10 @@ def _scope_label(scope, org_workspaces, org_users, scoped_count=None):
10661069
the rendered sections."""
10671070
if scope is not None:
10681071
n = scoped_count if scoped_count is not None else len(scope)
1069-
return f"Scoped to {n} selected workspace(s)"
1072+
return (
1073+
f"Scoped to {n} selected workspace(s) "
1074+
"(per-user totals remain org-wide)"
1075+
)
10701076
if org_workspaces is not None:
10711077
return (
10721078
f"Org-wide: {org_workspaces} workspaces, {org_users} users "

0 commit comments

Comments
 (0)