From 26249b24eab140b86b99a607e042a42e32feea6e Mon Sep 17 00:00:00 2001 From: Bob Date: Wed, 26 Aug 2026 15:40:16 +0000 Subject: [PATCH 1/3] feat(vitals): per-version crash/ANR breakdown + stalled-feed warning MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two gaps surfaced by 'are crash rates better on v0.14.0b2?' (ActivityWatch/aw-android#176), which the existing tooling cannot answer. 1. The app-wide daily series cannot attribute a rate to a release. During a partial rollout the aggregate is dominated by the install base still on the old build, so a genuinely fixed version barely moves it for weeks. Add 'vitals.py by-version', which breaks the same metric down by the API's versionCode dimension and compares versions directly. 2. A stalled feed was invisible. _daily_freshness anchors the query to the API's latest available DAILY date; when that date stops advancing the collector still exits 0, rewrites identical rows, and 'git diff --quiet' skips the commit — so the daily job reports success while the series silently stops. Warn on stderr when the feed is more than STALE_FRESHNESS_DAYS behind, so a stall is output rather than absence of it. Add offline tests for the dimensioned query body, per-version row parsing, and the freshness warning. ruff/mypy/pytest all green. --- test_vitals.py | 80 ++++++++++++++++++++++++++ vitals.py | 152 ++++++++++++++++++++++++++++++++++++++++++++++--- 2 files changed, 225 insertions(+), 7 deletions(-) create mode 100644 test_vitals.py diff --git a/test_vitals.py b/test_vitals.py new file mode 100644 index 00000000..f725fbc1 --- /dev/null +++ b/test_vitals.py @@ -0,0 +1,80 @@ +"""Offline tests for the vitals query/parse helpers. + +These cover the pieces that can be wrong without the API ever telling us: +the dimensioned query body, per-version row parsing, and the stalled-feed +warning that exists because a stalled collector otherwise exits 0 silently. +""" +from datetime import date, timedelta + +import vitals + + +END_DT = {"year": 2026, "month": 8, "day": 26} + + +def _row(day: int, version: str | None, value: float | None, + metric: str = "userPerceivedCrashRate") -> dict: + row: dict = {"startTime": {"year": 2026, "month": 8, "day": day}, "metrics": []} + if version is not None: + row["dimensions"] = [{"dimension": "versionCode", "int64Value": version}] + if value is not None: + row["metrics"] = [{"metric": metric, "decimalValue": {"value": str(value)}}] + return row + + +def test_build_body_omits_dimensions_by_default(): + body = vitals._build_body(END_DT, 7, "userPerceivedCrashRate") + assert "dimensions" not in body + + +def test_build_body_includes_requested_dimensions(): + body = vitals._build_body(END_DT, 7, "userPerceivedCrashRate", ("versionCode",)) + assert body["dimensions"] == ["versionCode"] + assert body["timelineSpec"]["startTime"]["day"] == 19 + + +def test_dimension_value_reads_int64(): + assert vitals._dimension_value(_row(20, "1000", 0.05), "versionCode") == "1000" + + +def test_dimension_value_prefers_label_when_it_adds_information(): + row = {"dimensions": [{"dimension": "versionCode", "int64Value": "1000", + "valueLabel": "0.14.0b2"}]} + assert vitals._dimension_value(row, "versionCode") == "1000 (0.14.0b2)" + + +def test_dimension_value_missing_dimension_is_none(): + assert vitals._dimension_value(_row(20, None, 0.05), "versionCode") is None + assert vitals._dimension_value(_row(20, "1000", 0.05), "apiLevel") is None + + +def test_parse_rows_reads_the_requested_metric_only(): + rows = [_row(20, "1000", 0.05), _row(21, "1000", 0.03)] + assert vitals._parse_rows(rows, "userPerceivedCrashRate") == [ + ("2026-08-20", 0.05), + ("2026-08-21", 0.03), + ] + # A different metric name yields no values rather than the wrong number. + assert [v for _, v in vitals._parse_rows(rows, "userPerceivedAnrRate")] == [None, None] + + +def test_fresh_feed_produces_no_warning(): + today = date.today() + assert vitals._freshness_warning( + {"year": today.year, "month": today.month, "day": today.day} + ) is None + + +def test_stalled_feed_warns_with_the_lag(): + stale = date.today() - timedelta(days=vitals.STALE_FRESHNESS_DAYS + 4) + warning = vitals._freshness_warning( + {"year": stale.year, "month": stale.month, "day": stale.day} + ) + assert warning is not None + assert str(stale) in warning + assert "stalled" in warning + + +def test_freshness_warning_tolerates_a_malformed_date(): + assert vitals._freshness_warning({}) is None + assert vitals._freshness_warning({"year": 2026, "month": 13, "day": 40}) is None diff --git a/vitals.py b/vitals.py index 63d5634e..85b45d1e 100644 --- a/vitals.py +++ b/vitals.py @@ -47,6 +47,10 @@ # Consistent key location shared by local runs, agents, and CI. DEFAULT_CREDENTIALS = os.path.expanduser("~/.config/activitywatch/play-sa.json") +# Days behind "today" past which the API's DAILY feed counts as stalled +# rather than normally lagging. +STALE_FRESHNESS_DAYS = 3 + # command -> (metric-set resource, default metric, human label) METRIC_SETS = { "crash-rate": ("crashRateMetricSet", "userPerceivedCrashRate", "User-perceived crash rate"), @@ -80,12 +84,18 @@ def _datetime(d: date, tz: dict | None) -> dict: return out -def _build_body(end_dt: dict, days: int, metric: str) -> dict: - """Build a query body for the DAILY window ending at `end_dt` (a DateTime).""" +def _build_body(end_dt: dict, days: int, metric: str, + dimensions: tuple[str, ...] = ()) -> dict: + """Build a query body for the DAILY window ending at `end_dt` (a DateTime). + + `dimensions` breaks the metric down instead of returning one app-wide + number per day — `versionCode` is the one that answers "is the release we + just shipped better?", which the aggregate cannot (see `by-version`). + """ end = date(end_dt["year"], end_dt["month"], end_dt["day"]) tz = end_dt.get("timeZone") start = end - timedelta(days=days) - return { + body: dict = { "timelineSpec": { "aggregationPeriod": "DAILY", "startTime": _datetime(start, tz), @@ -93,6 +103,9 @@ def _build_body(end_dt: dict, days: int, metric: str) -> dict: }, "metrics": [metric], } + if dimensions: + body["dimensions"] = list(dimensions) + return body def _daily_freshness(base: str, metric_set: str, token: str) -> dict | None: @@ -112,6 +125,25 @@ def _daily_freshness(base: str, metric_set: str, token: str) -> dict | None: return None +def _dimension_value(row: dict, dimension: str) -> str | None: + """The row's value for `dimension`, as a display string. + + The API returns the value under one of several typed keys and may add a + human `valueLabel` (e.g. a version *name* next to a versionCode), so we + prefer the label when present and fall back to the raw value. + """ + for d in row.get("dimensions", []): + if d.get("dimension") != dimension: + continue + label = d.get("valueLabel") + for key in ("stringValue", "int64Value", "value"): + if d.get(key) is not None: + raw = str(d[key]) + return f"{raw} ({label})" if label and label != raw else raw + return str(label) if label else None + return None + + def _parse_rows(rows: list, metric: str) -> list[tuple[str, float | None]]: series = [] for row in rows: @@ -127,13 +159,15 @@ def _parse_rows(rows: list, metric: str) -> list[tuple[str, float | None]]: return series -def fetch_metric(package, metric_set, metric, days, credentials): +def fetch_rows(package, metric_set, metric, days, credentials, + dimensions: tuple[str, ...] = ()): + """Raw rows plus the API's DAILY freshness date, for dimensioned queries.""" token = _access_token(credentials) base = f"{BASE}/apps/{package}" end_dt = _daily_freshness(base, metric_set, token) if not end_dt: raise SystemExit(f"No DAILY freshness available for {metric_set}") - body = _build_body(end_dt, days, metric) + body = _build_body(end_dt, days, metric, dimensions) headers = {"Authorization": f"Bearer {token}"} url = f"{base}/{metric_set}:query" rows, page_token = [], None @@ -146,7 +180,34 @@ def fetch_metric(package, metric_set, metric, days, credentials): page_token = data.get("nextPageToken") if not page_token: break - return _parse_rows(rows, metric) + return rows, end_dt + + +def _freshness_warning(end_dt: dict) -> str | None: + """Warn when the API's own latest DAILY date has fallen behind. + + A stalled feed is otherwise invisible: the collector still exits 0 and + re-writes the same rows, so `git diff --quiet` skips the commit and the + daily job keeps reporting success while the series silently stops. Callers + print this so a stall shows up as output rather than as absence of it. + """ + try: + latest = date(end_dt["year"], end_dt["month"], end_dt["day"]) + except (KeyError, TypeError, ValueError): + return None + lag = (date.today() - latest).days + # Play's DAILY vitals normally land within ~2-3 days; beyond that the feed + # is stalled, not merely lagging. + if lag > STALE_FRESHNESS_DAYS: + return (f"WARNING: latest available DAILY data is {latest} ({lag} days " + f"behind today) — the feed is stalled, not just lagging.") + return None + + +def fetch_metric(package, metric_set, metric, days, credentials): + """The app-wide daily series, plus the API's latest available DAILY date.""" + rows, end_dt = fetch_rows(package, metric_set, metric, days, credentials) + return _parse_rows(rows, metric), end_dt # --- CLI ------------------------------------------------------------------- @@ -205,7 +266,10 @@ def _emit(metric_set, metric, label, package, days, credentials, as_csv, dry_run click.echo(f"POST {BASE}/apps/{package}/{metric_set}:query") click.echo(json.dumps(body, indent=2)) return - series = fetch_metric(package, metric_set, metric, days, credentials) + series, end_dt = fetch_metric(package, metric_set, metric, days, credentials) + warning = _freshness_warning(end_dt) + if warning: + click.echo(warning, err=True) if update_path: n, last = _upsert_csv(update_path, series, metric) click.echo(f"{update_path}: {n} rows (latest {last})") @@ -296,6 +360,80 @@ def errors(package, credentials, issue_type, limit, stacktraces): click.echo() +@cli.command("by-version") +@click.option("--package", default=DEFAULT_PACKAGE, show_default=True) +@click.option("--credentials", envvar="GOOGLE_APPLICATION_CREDENTIALS") +@click.option("--days", default=14, show_default=True, + help="Days of history to compare over.") +@click.option("--kind", type=click.Choice(["crash-rate", "anr-rate"]), + default="crash-rate", show_default=True) +@click.option("--metric", default=None, help="Override the metric name.") +@click.option("--as-csv", "as_csv", is_flag=True, + help="Print 'version,days,mean' rows instead of a table.") +@click.option("--dry-run", is_flag=True, help="Print the API request instead of sending it.") +def by_version(package, credentials, days, kind, metric, as_csv, dry_run): + """Crash/ANR rate broken down by app version. + + The app-wide series in `crash-rate` cannot answer "is the release we just + shipped better?" — a partial rollout is swamped by the install base still + on the old build, so a genuinely fixed version barely moves the aggregate + for weeks. Breaking the same metric down by `versionCode` compares the + versions directly, which is the question people actually mean. + """ + metric_set, default_metric, label = METRIC_SETS[kind] + metric = metric or default_metric + + if dry_run: + import json + today = date.today() + body = _build_body({"year": today.year, "month": today.month, "day": today.day}, + days, metric, ("versionCode",)) + click.echo(f"POST {BASE}/apps/{package}/{metric_set}:query") + click.echo(json.dumps(body, indent=2)) + return + + rows, end_dt = fetch_rows(package, metric_set, metric, days, credentials, + dimensions=("versionCode",)) + warning = _freshness_warning(end_dt) + if warning: + click.echo(warning, err=True) + + # version -> observed daily values (None means "no data that day") + per_version: dict[str, list[float]] = {} + for row in rows: + version = _dimension_value(row, "versionCode") + if version is None: + continue + for _, value in _parse_rows([row], metric): + if value is not None: + per_version.setdefault(version, []).append(value) + + if not per_version: + click.echo("No per-version data returned.") + return + + # Newest versionCode last: sort numerically when we can, else lexically. + def _sort_key(v: str): + head = v.split(" ", 1)[0] + return (0, int(head)) if head.isdigit() else (1, 0) + + summary_rows = [ + (version, len(values), sum(values) / len(values)) + for version, values in sorted(per_version.items(), key=lambda kv: _sort_key(kv[0])) + ] + + if as_csv: + click.echo("version,days,mean") + for version, n, mean in summary_rows: + click.echo(f"{version},{n},{mean!r}") + return + + click.echo(f"{label} ({metric}) by version for {package}, last {days} days") + width = max(len(v) for v, _, _ in summary_rows) + for version, n, mean in summary_rows: + click.echo(f" {version:<{width}} {mean:>8.4%} ({n} day(s) of data)") + + @cli.command("summary") @click.option("--package", default=DEFAULT_PACKAGE, show_default=True) @click.option("--credentials", envvar="GOOGLE_APPLICATION_CREDENTIALS") From e50c0da476e3c7e5622e1edf0c1b63e1d416d4a2 Mon Sep 17 00:00:00 2001 From: Bob Date: Wed, 26 Aug 2026 15:49:58 +0000 Subject: [PATCH 2/3] fix(vitals): stable version grouping + CSV escaping in by-version Two correctness issues found in Greptile review: P1 (unstable version identity): `by-version` keyed `per_version` on the display string from `_dimension_value`, which includes `valueLabel`. When Play omits the label for some rows of the same versionCode the same release splits into separate buckets, corrupting day counts and means. Fix: add `_dimension_raw` that returns only the typed value (no label), group by that as a stable key, and track the best display string in a separate dict. P2 (unescaped CSV): the `--as-csv` branch interpolated version labels directly into an f-string, so a comma or newline in a Play label would produce malformed CSV. Fix: write through `csv.writer`. Tests: 3 new tests covering `_dimension_raw`, stable-grouping behaviour, and CSV escaping with a comma-bearing label (16 pass total). --- test_vitals.py | 78 ++++++++++++++++++++++++++++++++++++++++++++++++++ vitals.py | 52 ++++++++++++++++++++++++++------- 2 files changed, 119 insertions(+), 11 deletions(-) diff --git a/test_vitals.py b/test_vitals.py index f725fbc1..d43f4a4c 100644 --- a/test_vitals.py +++ b/test_vitals.py @@ -33,6 +33,20 @@ def test_build_body_includes_requested_dimensions(): assert body["timelineSpec"]["startTime"]["day"] == 19 +def test_dimension_raw_reads_int64(): + assert vitals._dimension_raw(_row(20, "1000", 0.05), "versionCode") == "1000" + + +def test_dimension_raw_ignores_label(): + row = {"dimensions": [{"dimension": "versionCode", "int64Value": "1000", + "valueLabel": "0.14.0b2"}]} + assert vitals._dimension_raw(row, "versionCode") == "1000" + + +def test_dimension_raw_missing_is_none(): + assert vitals._dimension_raw(_row(20, None, 0.05), "versionCode") is None + + def test_dimension_value_reads_int64(): assert vitals._dimension_value(_row(20, "1000", 0.05), "versionCode") == "1000" @@ -78,3 +92,67 @@ def test_stalled_feed_warns_with_the_lag(): def test_freshness_warning_tolerates_a_malformed_date(): assert vitals._freshness_warning({}) is None assert vitals._freshness_warning({"year": 2026, "month": 13, "day": 40}) is None + + +def _labeled_row(day: int, code: str, label: str | None, value: float | None, + metric: str = "userPerceivedCrashRate") -> dict: + """Like _row but with an explicit valueLabel on the versionCode dimension.""" + row: dict = {"startTime": {"year": 2026, "month": 8, "day": day}, "metrics": []} + dim: dict = {"dimension": "versionCode", "int64Value": code} + if label is not None: + dim["valueLabel"] = label + row["dimensions"] = [dim] + if value is not None: + row["metrics"] = [{"metric": metric, "decimalValue": {"value": str(value)}}] + return row + + +def test_by_version_same_code_different_label_merges(): + """P1: rows with the same versionCode but varying valueLabel must land in one bucket.""" + rows = [ + _labeled_row(20, "1000", "0.14.0b2", 0.05), + _labeled_row(21, "1000", None, 0.03), # label absent on day 21 + _labeled_row(22, "1000", "0.14.0b2", 0.04), + ] + metric = "userPerceivedCrashRate" + per_version: dict = {} + version_labels: dict = {} + for row in rows: + code = vitals._dimension_raw(row, "versionCode") + assert code is not None + display = vitals._dimension_value(row, "versionCode") + if display and display != code: + version_labels[code] = display + for _, val in vitals._parse_rows([row], metric): + if val is not None: + per_version.setdefault(code, []).append(val) + # Must be exactly one bucket keyed by the raw code, not by display strings + assert list(per_version.keys()) == ["1000"] + assert len(per_version["1000"]) == 3 + # Label should have been captured as the full display string + assert version_labels.get("1000") == "1000 (0.14.0b2)" + + +def test_by_version_csv_escapes_comma_in_label(monkeypatch, tmp_path): + """P2: a comma in a version label must not corrupt the CSV output.""" + import io + from click.testing import CliRunner + + # Patch fetch_rows to return a row whose label contains a comma + fake_row = _labeled_row(20, "1000", "label,with,commas", 0.05) + monkeypatch.setattr( + vitals, "fetch_rows", + lambda *a, **kw: ([fake_row], END_DT) + ) + + runner = CliRunner() + result = runner.invoke( + vitals.cli, + ["by-version", "--as-csv", "--package", "net.activitywatch.android"], + ) + assert result.exit_code == 0, result.output + import csv, io as _io + rows = list(csv.reader(_io.StringIO(result.output))) + # Header + one data row; the label's commas must not create extra columns + assert rows[0] == ["version", "days", "mean"] + assert len(rows[1]) == 3 diff --git a/vitals.py b/vitals.py index 85b45d1e..b915d808 100644 --- a/vitals.py +++ b/vitals.py @@ -125,12 +125,32 @@ def _daily_freshness(base: str, metric_set: str, token: str) -> dict | None: return None +def _dimension_raw(row: dict, dimension: str) -> str | None: + """The row's raw (unlabelled) value for `dimension`. + + Use this as a stable grouping key — `_dimension_value` may produce + different strings for the same versionCode when Play omits or changes the + valueLabel between rows, splitting one release across multiple buckets. + """ + for d in row.get("dimensions", []): + if d.get("dimension") != dimension: + continue + for key in ("int64Value", "stringValue", "value"): + if d.get(key) is not None: + return str(d[key]) + label = d.get("valueLabel") + return str(label) if label else None + return None + + def _dimension_value(row: dict, dimension: str) -> str | None: """The row's value for `dimension`, as a display string. The API returns the value under one of several typed keys and may add a human `valueLabel` (e.g. a version *name* next to a versionCode), so we prefer the label when present and fall back to the raw value. + + Do NOT use this as a grouping key — see `_dimension_raw` instead. """ for d in row.get("dimensions", []): if d.get("dimension") != dimension: @@ -398,34 +418,44 @@ def by_version(package, credentials, days, kind, metric, as_csv, dry_run): if warning: click.echo(warning, err=True) - # version -> observed daily values (None means "no data that day") + # Group by raw versionCode (stable key); track best label seen for display. + # Grouping by _dimension_value would split one release into multiple buckets + # if Play omits or changes valueLabel between rows for the same versionCode. per_version: dict[str, list[float]] = {} + version_labels: dict[str, str] = {} for row in rows: - version = _dimension_value(row, "versionCode") - if version is None: + version_code = _dimension_raw(row, "versionCode") + if version_code is None: continue + display = _dimension_value(row, "versionCode") + if display and display != version_code: + version_labels[version_code] = display for _, value in _parse_rows([row], metric): if value is not None: - per_version.setdefault(version, []).append(value) + per_version.setdefault(version_code, []).append(value) if not per_version: click.echo("No per-version data returned.") return # Newest versionCode last: sort numerically when we can, else lexically. - def _sort_key(v: str): - head = v.split(" ", 1)[0] - return (0, int(head)) if head.isdigit() else (1, 0) + def _sort_key(code: str) -> tuple: + return (0, int(code)) if code.isdigit() else (1, 0) + + def _display(code: str) -> str: + return version_labels.get(code, code) summary_rows = [ - (version, len(values), sum(values) / len(values)) - for version, values in sorted(per_version.items(), key=lambda kv: _sort_key(kv[0])) + (_display(code), len(values), sum(values) / len(values)) + for code, values in sorted(per_version.items(), key=lambda kv: _sort_key(kv[0])) ] if as_csv: - click.echo("version,days,mean") + import sys + w = csv.writer(sys.stdout, lineterminator="\n") + w.writerow(["version", "days", "mean"]) for version, n, mean in summary_rows: - click.echo(f"{version},{n},{mean!r}") + w.writerow([version, n, repr(mean)]) return click.echo(f"{label} ({metric}) by version for {package}, last {days} days") From 617a113ef8827698ed859ec575c44dad3044d418 Mon Sep 17 00:00:00 2001 From: Bob Date: Wed, 26 Aug 2026 16:49:10 +0000 Subject: [PATCH 3/3] fix(tests): split imports and remove unused io import for ruff compliance --- test_vitals.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test_vitals.py b/test_vitals.py index d43f4a4c..2a6076af 100644 --- a/test_vitals.py +++ b/test_vitals.py @@ -135,7 +135,8 @@ def test_by_version_same_code_different_label_merges(): def test_by_version_csv_escapes_comma_in_label(monkeypatch, tmp_path): """P2: a comma in a version label must not corrupt the CSV output.""" - import io + import csv + import io as _io from click.testing import CliRunner # Patch fetch_rows to return a row whose label contains a comma @@ -151,7 +152,6 @@ def test_by_version_csv_escapes_comma_in_label(monkeypatch, tmp_path): ["by-version", "--as-csv", "--package", "net.activitywatch.android"], ) assert result.exit_code == 0, result.output - import csv, io as _io rows = list(csv.reader(_io.StringIO(result.output))) # Header + one data row; the label's commas must not create extra columns assert rows[0] == ["version", "days", "mean"]