From b8d9913fb3433c4e49aa5524e8468898146b6a9a Mon Sep 17 00:00:00 2001 From: Bin Le Date: Wed, 29 Jul 2026 20:33:56 -0400 Subject: [PATCH] Land the layering-cleanup plan and promote its two open items (v1.6.1) docs/TODO-layering-cleanup-2026-07-02.md drove LC-1 through LC-3 (shipped v1.2.4-v1.3.0: SystemSnapshot as sole frame contract, ProcessSample through L2, analytics.py taking AlertEngine/throttle/session-energy out of the widget), but it lived only on the unmerged lc-2-process-sample branch. The code shipped while its rationale stayed unpublished. Landed as the design record, with status verified against main at v1.6.0. Kept sections 1-9 verbatim so they read as the July plan (their line references point at July code, noted in the header); added section 10 for the only open scope: - 10.1 LC-4: watt/GB-s history deques and _avg_max never moved to an analytics.RollingStats. Low priority - relocates working code. - 10.2 Export parity: per-process rows, throttle/alert flags, and session energy are TUI-only and never reach --json/--serve, so profiling a local inference run through the export backends cannot answer which process drew the watts, whether the chip throttled, or the run's total energy. Records both candidate designs, the recommendation, and the per-PID Prometheus cardinality constraint. - 10.3 One acceptance criterion does not literally pass and should be reworded, not "fixed": tui/app.py imports get_soc_info for a single construction-time call. The criterion targeted per-frame L1 acquisition in the view, which is gone; beating the grep would add indirection for no layering gain. Both open items promoted into TODO-architecture-roadmap.md, which is where open work is tracked. Docs only - no code change. 95 passed (-m "not local"), ruff clean. Co-Authored-By: Claude Opus 5 (1M context) --- CHANGELOG.md | 37 +++ docs/TODO-architecture-roadmap.md | 3 + docs/TODO-layering-cleanup-2026-07-02.md | 384 +++++++++++++++++++++++ pyproject.toml | 2 +- 4 files changed, 425 insertions(+), 1 deletion(-) create mode 100644 docs/TODO-layering-cleanup-2026-07-02.md diff --git a/CHANGELOG.md b/CHANGELOG.md index 9e0d74b..1fb9c5c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,43 @@ This project follows a Keep a Changelog-style format and uses version tags for r ## [Unreleased] +## [1.6.1] - 2026-07-29 + +Docs only — no code change. + +### Added +- **Landed `docs/TODO-layering-cleanup-2026-07-02.md`, the design record behind + LC-1→LC-3** (shipped v1.2.4–v1.3.0: `SystemSnapshot` as the sole frame + contract, `ProcessSample` through L2, and `analytics.py`'s `AlertEngine` / + throttle / session-energy move out of the widget). The plan drove all three + releases but lived only on an unmerged branch, so the code shipped while its + rationale stayed unpublished — the violation inventory, per-violation fix + design, and sequencing are now in the repo instead of one branch tip. + + §§1–9 are the July plan verbatim, so their line references point at July code. + A new **§10** records status verified against `main`, and is the only open + scope. +- **Two roadmap items promoted out of that plan** into + `docs/TODO-architecture-roadmap.md`: + - **Export parity** (§10.2) — per-process rows, throttle/alert flags, and + session energy are still TUI-only and never reach `--json` / `--serve`, so + profiling a local inference run through the export backends cannot answer + which process drew the watts, whether the chip throttled, or what the run + cost in energy. Both candidate designs and the recommendation are recorded, + along with the per-PID Prometheus cardinality constraint that keeps process + data NDJSON-only either way. + - **LC-4** (§10.1) — the watt/GB-s history deques and `_avg_max` reducer never + moved to an `analytics.RollingStats`. Explicitly low priority: it relocates + working code, and the reason to do it is export parity needing the same + aggregates outside the TUI. + + §10.3 records one acceptance criterion that does not literally pass and should + be reworded rather than "fixed": `tui/app.py` still imports `get_soc_info` for + a single construction-time call that builds `DashboardConfig`. The criterion + targeted per-frame L1 acquisition in the view, which is gone; routing that one + call through another module to satisfy a grep would add indirection for no + layering gain. + ## [1.6.0] - 2026-07-29 Reading-plane audit §8: adopt the GPU driver's `IOAccelerator` diff --git a/docs/TODO-architecture-roadmap.md b/docs/TODO-architecture-roadmap.md index 6f6c3e1..326eaa4 100644 --- a/docs/TODO-architecture-roadmap.md +++ b/docs/TODO-architecture-roadmap.md @@ -11,6 +11,9 @@ Completed work is **not** tracked here — its as-built design is folded into `d * [ ] **Net / disk I/O via native ctypes** → **`docs/TODO-net-disk-io-2026-07-02.md`.** Moderate effort; deliberately widens scope. **Feasibility spike complete (2026-07-02)** — verified on-device (M4 Max, unprivileged) and cross-checked against `mactop`'s shipped implementation; the full impl-ready design (exact syscalls, struct layouts, IOKit matching strings, property keys, aggregation, and where each plugs into the sampler/models/TUI/API/export layers) lives in that dedicated plan. Summary of the verified approach: **network** via `getifaddrs()`/`freeifaddrs()` walking `AF_LINK` entries and casting `ifa_data` → `struct if_data` (one new ctypes binding in `native_sys.py`; the originally-guessed `net.link.generic.system.stats` MIB does not exist on-device); **disk** via IOKit `IOServiceMatching("AppleAPFSVolume")` summing each volume's `Statistics` dict, with `IOServiceMatching("IOBlockStorageDriver")` as the non-APFS fallback (read the way `gpu_registry.py` walks `IOAccelerator` — no new IOKit binding classes); both **aggregated** as `(current_total - previous_total) / elapsed_seconds`, the same delta-over-interval shape as `sampler._compute_bandwidth_gbps` (§3.5). Update `DESIGN-system.md` §3.6 to drop the non-goal framing once it ships. * **Why a must-have despite being single-peer breadth:** per `docs/REVIEW-architecture-comparison.md` §4 this is *single-peer breadth* (mactop-only; macmon deliberately omits it), **not** a converged peer expectation. We keep it as a conscious bet on narrowing the gap to the breadth leader, tracked openly rather than silently declined. Post-launch — launch gates on none of it. +* [ ] **Export parity for processes / alerts / session energy** → **`docs/TODO-layering-cleanup-2026-07-02.md` §10.2.** The layering cleanup (LC-1→LC-3, shipped v1.2.4–v1.3.0) made `SystemSnapshot` the sole frame contract, but three data points the TUI shows still never reach `--json` / `--serve`: per-process rows (the export loops build `Monitor` without `include_processes=True`), the throttle/alert flags, and cumulative session energy (both produced by `analytics.AlertEngine`, which only the dashboard widget drives). **Why it matters:** someone profiling a local inference run through the export backends gets watts and utilization but cannot answer *which process drew them*, *did the chip throttle*, or *what did the run cost in total energy* — the questions that profiling session exists to ask. That plan's §10.2 records both candidate designs (opt-in export flags vs. moving `AlertEngine` behind `Monitor`), the recommendation, and the per-PID Prometheus cardinality constraint that keeps process data NDJSON-only either way. +* [ ] **LC-4 — rolling native-unit stats out of the widget** → **`docs/TODO-layering-cleanup-2026-07-02.md` §10.1.** The watt/GB-s history deques and the `_avg_max` reducer behind the `avg N · max N` suffixes still live in `HardwareDashboard`; `analytics.RollingStats` was never built. Deliberately low priority — it relocates working code and fixes no user-visible defect. The reason to do it is the item above, which needs those same aggregates outside the TUI. + --- ## Nice-to-Have — Distribution & UX diff --git a/docs/TODO-layering-cleanup-2026-07-02.md b/docs/TODO-layering-cleanup-2026-07-02.md new file mode 100644 index 0000000..29146b7 --- /dev/null +++ b/docs/TODO-layering-cleanup-2026-07-02.md @@ -0,0 +1,384 @@ +# TODO: Layering Cleanup — TUI ← Data Points ← Data APIs + +Status: **LC-1 / LC-2 / LC-3 shipped; LC-4 open** · Created: 2026-07-02 · +Status verified against `main` 2026-07-29 (v1.6.0) +Scope: `models.py`, `api.py`, `utils.py`, `soc_profiles.py`, `config.py`, new +`analytics.py`, `tui/*`; export extensions are a follow-up. No native-binding +(`ioreport.py`/`smc.py`/`gpu_registry.py`/`native_sys.py`) changes. + +> **Why this doc landed late.** The plan was written 2026-07-02 and drove LC-1 +> through LC-3, but it lived only on the `lc-2-process-sample` branch and was +> never merged — the code shipped while the plan stayed unpublished. It is landed +> here as the design record for work already in `main`, plus the three items that +> genuinely remain (**§10**). Read §§1–9 as the originating plan (kept verbatim, +> so its line references point at July code, not current line numbers) and §10 as +> the only open scope. + +--- + +## 1. The intended layering + +actop's architecture factors cleanly into three layers; each violation below +is a place where the implementation crosses a boundary the design implies: + +- **L1 — data APIs (acquisition):** `ioreport.py`, `smc.py`, + `gpu_registry.py`, `native_sys.py`, `sampler.py`, and `utils.py`'s raw + queries (`sysctl`, `system_profiler`, RAM/process collection). +- **L2 — data points (typed contract + delivery):** `models.py` + (`SystemSnapshot`, `CoreSample`), `api.py` (`Monitor`/`Profiler`/ + `AsyncMonitor`), `soc_profiles.py`, `power_scaling.py`. +- **L3 — presentation:** `tui/app.py`, `tui/widgets.py`, `export.py`. + +**Contract:** L3 consumes *only* L2 types. L2 derives every data point a +consumer needs. L1 is invisible above L2. + +**Why it matters concretely:** `export.py` — the other L3 consumer — can only +emit what `SystemSnapshot` carries, which is why exports today have no +processes, no alerts, no throttle flag, no session energy: those data points +are trapped inside the TUI. Symmetrically, every violation below is also the +reason some test must mount a widget instead of asserting against the API. + +## 2. Violations inventory (verified against code, 2026-07-02) + +| # | Violation | Evidence | Consequence | +|---|---|---|---| +| V1 | TUI imports L1 directly and stitches its own data bundle | `tui/app.py:18-23` imports `get_ram_metrics_dict`, `get_top_processes`, `get_soc_info`, `attribute_power`; worker at `app.py:326-342` ships `MetricsUpdated(snapshot, ram, processes)` — two untyped dicts beside the snapshot (`widgets.py:264-271`); `ActopApp.__init__` re-calls `get_soc_info()` (`app.py:275-283`) | `SystemSnapshot` is not the data contract; three parallel channels into the view | +| V2 | RAM sampled twice per frame via two paths | `api.py:96` (inside `get_snapshot`) **and** `app.py:332`; snapshot carries only `ram_used_gb`/`swap_used_gb` (`models.py:34-35`) while the TUI needs `total_GB`/`used_percent`/`swap_total_GB` from the raw dict (`widgets.py:521,633-636`) | Incomplete model → duplicate acquisition → two sources of truth for one metric in one frame; snapshot RAM fields are dead weight to the TUI | +| V3 | Processes never enter L2 | `get_top_processes` returns `{"cpu": [dict], "memory": [dict]}` with implicit string keys (`cpu_time_share`, `gpu_time_share`, `rss_mb`, `num_threads`) consumed at `app.py:489-520`; no `ProcessSample` model, no `Monitor`/`Profiler` path | TUI shows data a public-API user cannot obtain; dict keys form an unanchored `utils`↔`app` contract | +| V4 | Domain analytics implemented in the view | Throttle detection `_domain_throttling` + gates (`widgets.py:278-294`); alert engine + sustain counters `_compute_alerts` (`widgets.py:825-920`); session energy `_session_joules` (`widgets.py:563-565,922-927`) self-described as "mirroring `Profiler.total_package_joules`"; ANE% derived at render time (`widgets.py:520`); BW/package normalizations (`widgets.py:297-315`) | Hardware judgments (throttling, alerts) invisible to API/export; energy integration duplicated across layers; `ane_util_pct` exists only inside the TUI | +| V5 | Per-process power attribution executes at render time | `sort_processes` (`app.py:41-73`) and `_refresh_process_table` (`app.py:489-533`) call `attribute_power` during table refresh; comment at `app.py:498`: "computed here because the TUI owns cpu_watts/gpu_watts" | The watts↔process join can only happen in the view *because* the inputs arrive on different channels (V1/V3); Σ-reconciliation is more domain math in the same place | +| V6 | `utils.py` is a layer sandwich | L1 acquisition (`get_ram_metrics_dict`, `get_top_processes`), platform discovery (`get_soc_info`), and pure L2 domain math (`attribute_power`, `utils.py:131`) in one module, imported by both `api.py` and `tui/app.py` | Module has no assignable layer; domain math travels with acquisition code | +| V7 | Hardware constant outside the SoC-profile layer | `ane_max_power = 8.0` hardcoded in `config.py:50`, identical across M1→M4 | The ANE data point's denominator is a UI-config constant instead of per-SoC reference data (`soc_profiles.py` is where every other reference wattage lives) | +| V8 | View keeps its own native-unit data store | Watt/GB-s histories `_cpu_w_hist`/`_gpu_w_hist`/`_pkg_w_hist`/`_bw_gbps_hist` (`widgets.py:379-390`) + `_avg_max` (`widgets.py:714-732`) | Data aggregation (rolling watt stats) duplicates what L2 history (`Profiler`, `to_pandas`) exists for; chart-*percent* deques are legitimate presentation and stay | + +## 3. Fix design, per violation + +### F1 — `SystemSnapshot` becomes the single frame contract (V1, V2) + +- Extend `SystemSnapshot` (`models.py`) with `ram_total_gb: float`, + `ram_used_percent: float`, `swap_total_gb: float` (defaulted `0.0` — + existing call sites stay valid, matching the `*_max_freq_mhz` precedent at + `models.py:39-44`). +- `_sample_to_snapshot` (`api.py:12`) maps them from the ram dict it already + receives (`api.py:59-60` pattern). +- TUI: delete the second `get_ram_metrics_dict()` call (`app.py:332`); + `widgets.py` reads RAM/swap from the snapshot. Delete the `ram` field + from `MetricsUpdated`. **Deleting that field forces migrating *all four* + ram-dict reads in `widgets.py` in this same PR**, not just the RAM label: + - `514` `ram = message.ram` (the binding itself — remove it); + - `521` `ram.get("used_percent")` → `s.ram_used_percent`; + - `633-644` RAM label (`total_GB`/`used_GB`/`swap_used_GB`/`swap_total_GB`) + → `s.ram_total_gb`/`s.ram_used_gb`/`s.swap_used_gb`/`s.swap_total_gb`; + - `575` swap-history append (`ram.get("swap_used_GB")`) → `s.swap_used_gb`; + - `890` `ram.get("swap_total_GB")` inside `_compute_alerts` → + `s.swap_total_gb` (that function only *moves* in LC-3, but its ram read + must switch to the snapshot field now). + + Confirmed the mapping is total: `get_ram_metrics_dict` returns exactly + `total_GB`/`used_percent`/`swap_total_GB`/`swap_used_GB`, so the three new + fields plus existing `swap_used_gb` cover every read. +- `ActopApp.__init__`: stop re-calling `get_soc_info()`; thread the two + missing display fields (`name`, `gpu_core_count`) through + `DashboardConfig` (add `chip_name: str`, `gpu_core_count: int` — computed + once in `create_dashboard_config`, which already receives `soc_info`). + +### F2 — `ProcessSample` + processes through L2 (V3, V5) + +- New `ProcessSample` dataclass in `models.py`: `pid`, `command`, + `cpu_percent`, `cpu_time_share: float | None`, + `gpu_time_share: float | None`, `rss_mb`, `num_threads`, and + `attributed_w: float | None` (None ⇔ no CPU delta yet — today's `"–"` + cell, `app.py:507-508`). +- **Decision (firm): single list.** Snapshot carries `processes: + list[ProcessSample]` (default empty) sorted by the acquisition default (CPU). + The current dict's `"memory"` ordering becomes a plain `sorted(..., + key=lambda p: p.rss_mb, reverse=True)` in the L3 consumer (`sort_processes`); + the dual-list shape is deleted, not carried into the model. +- Collection is opt-in to keep `Monitor` cheap for API users: + `Monitor(interval_s, subsamples, include_processes=False, + process_limit=50, process_filter=None)` sets defaults; `get_snapshot(*, + process_filter=None)` overrides the filter per call (see the firm decision + below). When enabled, `get_snapshot()` + calls `get_top_processes` and computes `attributed_w` **in L2** — it has + `cpu_watts`/`gpu_watts` and the shares in the same scope, dissolving the + `app.py:498` comment. +- **Decision (firm): live filter via per-call arg.** `get_snapshot(*, + process_filter=None)` takes the pattern each call; the worker loop passes + `self._filter_regex` every tick (preserving the `app.py:443-444` liveness — + the worker already re-reads that attribute each iteration). No mutable + `Monitor` attribute, no locking — same guarantees as today. `process_limit` + stays `50` (matches today's `process_display_count`; the TUI's + table-height re-limit at `app.py:486` remains a pure display concern, so no + behavior change). +- `attribute_power` moves out of `utils.py` (see F4); `sort_processes` + (`app.py:41-73`) loses its watts parameters and becomes pure ordering over + `ProcessSample.attributed_w` — it stays in the TUI (ordering for display is + presentation). Σ-shown subtitle becomes `sum(p.attributed_w for visible)` + — arithmetic over L2-computed values, acceptable in L3. +- `MetricsUpdated` shrinks to `MetricsUpdated(snapshot)` — completing F1. + +### F3 — `analytics.py`: alerts, throttling, session energy (V4) + +Extend `actop/analytics.py` (L2 — imports `models`, `power_scaling`, never +`tui/*`). **The module is created in LC-2, not here:** F4 moves +`attribute_power` into it, so it already exists with that function by the time +LC-3 starts; LC-3 adds the analytics below to it: + +- `domain_throttling(util, freq, max_freq, temp, thermal_state, *, + util_gate, temp_gate, freq_percent)` — moved from `widgets.py:282-294` + with its gate constants; plus `bandwidth_percent(snapshot, max_total_bw)` + and `package_power_percent(snapshot, package_ref_w)` from + `widgets.py:297-315` (signatures take explicit refs, not the TUI config + object). +- `class AlertEngine`: constructed from the threshold values + (`alert_*` fields of `DashboardConfig` — pass values, not the config type, + so `analytics` stays TUI-config-agnostic). `feed(snapshot) -> + AlertFrame` where `AlertFrame` is a small frozen dataclass: + `thermal_alert: bool`, `cpu_throttle: bool`, `gpu_throttle: bool`, + `bw_alert: bool`, `pkg_alert: bool`, `swap_alert: bool`, + `swap_rise_gb: float`, `session_energy_j: float`. It owns the sustain + counters, swap history, and the energy integral — everything now in + `widgets.py:825-920` + `563-565`. Snapshot deltas use + `snapshot.timestamp` for the energy dt (more honest than the TUI's + `interval` approximation at `widgets.py:563-565`). Note the **first + `feed()` contributes 0 J** (no prior timestamp to diff against), a slight + change from today's fixed-interval first frame — call it out in the + CHANGELOG alongside the dt-source note in §7. +- The **status string stays a TUI concern**: `_compute_alerts` shrinks to + `frame = self._alert_engine.feed(s)` + token formatting + (`widgets.py:895-920` retained). Session-energy formatting + (`_format_session_energy`) reads `frame.session_energy_j`. +- ANE utilization becomes a data point: `SystemSnapshot.ane_util_pct` + computed in `_sample_to_snapshot` from `ane_watts` and the SoC profile's + ANE reference (F5); `widgets.py:520` reads the field. +- Deletion rule (conformance): the widget-local copies + (`_domain_throttling`, `_bandwidth_percent`, `_package_power_percent`, + counters, `_session_joules`) are **deleted**, not shimmed. + +### F4 — split `utils.py` along the L1/L2 seam (V6) + +Minimal-churn split (repo style: small incremental moves, no big-bang): + +- `attribute_power` (pure domain math) → `analytics.py`. **This move creates + `analytics.py`** (in LC-2); LC-3 extends the same module. Update the two + importers (`api.py` after F2; `app.py` import disappears entirely with F2). +- `utils.py` keeps acquisition + platform discovery (single L1 role); its + docstring states the layer assignment. A rename/further split (e.g. + `system_metrics.py`) is **not** worth the churn — record as non-goal. + +### F5 — ANE reference power into `soc_profiles.py` (V7) + +- Add `ane_max_w` as the trailing field on `SocProfile` (`ane_max_w: float = + 8.0`), plus every one of the 16 `KNOWN_SOC_PROFILES`, `GENERIC_APPLE_SILICON_ + PROFILE`, the 4 `TIER_FALLBACKS`, `_copy_with_name` (must copy the new + field), and the `get_soc_info` dict (`utils.py:91-101`) — all defaulting to + the current `8.0` (behavior-preserving; per-generation refinements are a + separate research task — the field creates the slot). Frozen-dataclass call + sites stay valid because profiles pass `name` positionally then kwargs, so a + trailing defaulted field does not shift any positional arg. +- `create_dashboard_config` reads it from `soc_info` instead of the literal + (`config.py:50`); `get_soc_profile` consumers unchanged. +- **`ane_util_pct` needs an L2 ANE reference that L2 does not have today.** + `_sample_to_snapshot(sample, ram, interval_s)` and `Monitor` carry no SoC + profile — the ANE denominator currently lives only in + `DashboardConfig.ane_max_power` (`config.py:50`), which L2 never sees (the + widget does `s.ane_watts / cfg.ane_max_power` at `widgets.py:520`). **Fix + in this PR:** `Monitor.__init__` acquires `ane_max_w` once (call + `get_soc_info()` and read the new key, storing `self._ane_max_w`) and passes + it into `_sample_to_snapshot`, which sets + `ane_util_pct = clamp_percent(ane_watts / ane_max_w * 100)`. `widgets.py:520` + then reads `s.ane_util_pct` and drops its `cfg.ane_max_power` divide. This is + the one place LC-1 is most likely to stall — resolve the plumbing before + writing the field. + +### F6 — rolling native-unit stats out of the widget (V8) — optional, last + +- `analytics.RollingStats(window)` fed per snapshot; exposes + `avg/max` per metric (watts, GB/s, percents). Widget keeps only + chart-percent deques + per-core presentation history. +- Lowest payoff of the set; schedule opportunistically (it mostly relocates + working code). If skipped, V8 stays documented here as accepted debt. + +## 4. Delivery plan & sequencing with the other open TODOs + +Every PR: branch from `main` → PR into `main` (no stacking), version bump + +CHANGELOG, `ruff check --fix . && ruff format .`, `pytest -q`, on-device run. + +| PR | Content | Bump | +|---|---|---| +| **LC-1** ✅ shipped v1.2.4 (#21) | F1 (snapshot RAM completion, kill duplicate sampling, config chip fields) + F5 (`ane_max_w` slot) + `ane_util_pct` field | patch | +| **LC-2** ✅ shipped v1.3.0 | F2 (`ProcessSample`, opt-in collection, L2 attribution) + F4 (`attribute_power` move → `analytics.py`); `MetricsUpdated` → snapshot-only | **minor** (public API addition: `ProcessSample`, `Monitor(include_processes=...)`) | +| **LC-3** ✅ shipped | F3 (extend `analytics.py`: `AlertEngine`, throttling, session energy; widget analytics deleted). Note `analytics.py` now **exists** (created in LC-2 with `attribute_power`); LC-3 adds the alert/throttle/energy analytics to it | patch | +| **LC-4** *(optional)* — **open, see §10** | F6 (`RollingStats`) | patch | + +**Revised master queue across all open plans:** + +1. `TODO-convergence-quick-wins` **Feature B** (fan `F{n}Mx`) — orthogonal, already first. +2. **LC-1 → LC-2 → LC-3** (this plan). +3. `TODO-tui-layout-redesign` **PR1 → PR2** — lands on a cleaner base: + `MetricsUpdated(snapshot)` only, and PR1's `AlertsComputed` message + carries formatting of an `AlertFrame` instead of re-homed widget math. + **Update that doc's §5.1-6/§5.2-1 wording when LC-3 merges.** +4. `TODO-convergence-quick-wins` **Feature A** (palette cycle) — after the + layout settles (bindings/help/compose churn zone). +5. LC-4 + net/disk I/O plan (`TODO-net-disk-io`) thereafter; note net/disk + should add its data points to `SystemSnapshot` *from the start* under + this plan's contract. + +Rationale for slotting LC before the layout redesign: PR1 currently plans to +move `#status-line` + invent `AlertsComputed` around the *old* in-widget +alert math; doing LC-3 first means that move relocates a thin formatter, not +an analytics engine — strictly less churn than either order reversed. + +## 5. Test plan (functional only, per CLAUDE.md) + +- **LC-1**: `Monitor().get_snapshot()` carries `ram_total_gb > 0`, + `0 ≤ ram_used_percent ≤ 100`, `ane_util_pct` consistent with `ane_watts` + (`@pytest.mark.local`, real hardware — extend `tests/test_api.py`). + `create_dashboard_config` exposes `chip_name`/`gpu_core_count` + (`tests/test_config.py`, real merge). Dashboard: existing + `tests/test_dashboard_metrics.py` harness updated for the + `MetricsUpdated` shape; RAM row asserts against snapshot-fed values. + `get_soc_profile` returns `ane_max_w` for known + tier-fallback chips + (`tests/test_soc_profiles.py`). +- **LC-2**: `Monitor(include_processes=True).get_snapshot().processes` is a + non-empty `list[ProcessSample]` with a plausible self-process entry + (`local`); `attributed_w` partition property (Σ over all ≤ cpu+gpu watts) + asserted *inside* that behavioral test, not standalone. Rewrite + `tests/test_per_process_power.py`'s table assertions against typed samples + through the same public `ActopApp.run_test()` path it already uses; delete + any assertions that only pinned the old dict keys. +- **LC-3**: `AlertEngine` driven with sequences of real-shaped + `SystemSnapshot`s through public `feed()`: sustain-threshold behavior + (N-1 hot frames → no alert; N → alert), swap-rise, energy monotonicity; + the TUI-rendered status line still shows `THERMAL`/`THROTTLING` tokens via + the existing mounted-dashboard test. Delete/rewrite any test that reached + the old widget-internal analytics. +- **CI parity**: hardware-dependent additions marked `@pytest.mark.local` + (CI runs `-m "not local"`). + +## 6. Docs to sync (with the PR that changes each fact) + +- `docs/DESIGN-system.md`: data-flow section — snapshot as sole frame + contract, `analytics.py` layer, opt-in process collection. +- `CLAUDE.md` + `README.md`: module tables (add `analytics.py`), data-flow + paragraph, Python-API snippet if it gains `include_processes`. +- `docs/REVIEW-architecture-comparison.md`: public-API row (processes + + alerts now API-accessible — a differentiator vs both peers). +- `docs/TODO-architecture-roadmap.md`: insert this plan into the priority + list (queue in §4 above). +- `docs/TODO-tui-layout-redesign-2026-07-02.md`: §5.1-6/§5.2-1 reference + `AlertFrame`; §4 table row for `MetricsUpdated`. +- Export follow-up (out of scope here): once LC-2/LC-3 land, `export.py` can + emit processes/alerts/energy — file under the roadmap when planned. + +## 7. Risks & mitigations + +| Risk | Mitigation | +|---|---| +| `SystemSnapshot` constructor-arg growth breaks external callers | All new fields defaulted (repo precedent `models.py:39-48`); construction via `Monitor` unaffected | +| Opt-in process collection changes TUI timing (process walk now inside `get_snapshot`) | Same total work per frame as today (the walk just moves from `app.py:334` into the same worker's `get_snapshot` call); verify frame cadence on-device in LC-2 | +| Live filter-regex mutation raced through to L2 | Keep today's semantics: the worker re-reads the filter each tick (`app.py:443-444`); Monitor takes the pattern per call or reads a plain attribute — no locking added, same guarantees as now | +| `AlertEngine` timestamp-based energy differs slightly from interval-based | Difference is a correctness *improvement*; note in CHANGELOG; `Profiler.total_package_joules` should adopt the same dt source for consistency | +| Two plans editing `widgets.py`/`app.py` in flight | Sequencing in §4 is strict; layout PR1 does not start until LC-3 merges | + +## 8. Acceptance criteria + +1. `grep -n "from actop.utils import" actop/tui/*.py` returns nothing — + the TUI imports only `models`/`api`/`config`/`analytics`/`power_scaling` + types (plus Textual/stdlib). +2. One `get_ram_metrics_dict()` call per frame, inside L2. +3. `MetricsUpdated` carries exactly one field: the `SystemSnapshot`. +4. `Monitor(include_processes=True)` exposes typed, watt-attributed + processes; export-visible parity gap for processes/alerts documented as + follow-up. +5. No throttle/alert/energy math remains in `tui/`. Gate precisely — a bare + grep for `sustain` false-positives on surviving legitimate refs (the help + text at `app.py:157`, and `cfg.alert_*` reads that stay in the L3 token + formatter at `widgets.py:913,917`). Grep instead for the internals being + removed: `_THROTTLE_UTIL_GATE`, `_THROTTLE_TEMP_C`, `_session_joules`, + `_high_bw_counter`, `_high_pkg_counter`, `_throttle_cpu_counter`, + `_throttle_gpu_counter`, `_domain_throttling`, `_bandwidth_percent`, + `_package_power_percent` under `tui/` → all must return nothing. +6. `pytest -q` green locally; `-m "not local"` green; ruff clean; TUI + visually unchanged after each LC PR (these are refactors — captures + compared before/after per the runbook in the layout plan §8). + +## 9. Non-goals + +- Renaming/splitting `utils.py` beyond the `attribute_power` move. +- Export format changes (follow-up). +- New metrics (net/disk, fan max — separate plans). +- Per-generation `ane_max_w` values (slot created, research separate). +- Textual/TUI layout changes (deliberately zero visual delta; the layout + redesign plan owns those). + +--- + +## 10. Remaining work (verified against `main` at v1.6.0, 2026-07-29) + +LC-1/LC-2/LC-3 are in `main`: `MetricsUpdated` carries exactly the snapshot +(§8-3), and every internal listed in §8-5 is gone from `tui/` — `analytics.py` +holds `attribute_power`, `domain_throttling`, `bandwidth_percent`, +`package_power_percent`, `AlertFrame`, and `AlertEngine`. Three items are open. + +### 10.1 LC-4 / F6 / V8 — native-unit rolling stats still live in the widget + +**Open, accepted debt.** `HardwareDashboard` still owns the watt and GB/s +history deques (`_cpu_w_hist`, `_gpu_w_hist`, `_pkg_w_hist`, `_bw_gbps_hist`) +and the `_avg_max` reducer behind the `avg N · max N` suffixes. F6's +`analytics.RollingStats` was never built. + +Still the lowest-payoff item in the set — it relocates working code rather than +fixing a user-visible defect or unblocking a consumer. Priority is genuinely low; +the reason to do it is §10.2, which needs the same numbers outside the TUI. + +### 10.2 Export parity gap — processes, alerts, and session energy are TUI-only + +**Open, and the highest-value item here.** §8-4 deferred this as "documented +follow-up"; the documentation is this section. `export.py` emits nothing for the +three data points the TUI computes or opts into: + +| Data point | Where it exists | Why export misses it | +|---|---|---| +| Processes | `SystemSnapshot.processes` | On the snapshot, so `snapshot_to_dict` would carry it — but `run_json_stream` / `serve_prometheus` construct `Monitor(interval_s, subsamples)` without `include_processes=True`, so the list is always empty | +| Alerts / throttle flags | `analytics.AlertFrame`, via `AlertEngine.feed()` | `AlertEngine` is instantiated by the dashboard widget; no export path drives it | +| Session energy | `AlertFrame.session_energy_j` | Same — the integral is only advanced by the TUI's per-frame `feed()` | + +This is the concrete cost the plan's §1 predicted ("`export.py` can only emit what +`SystemSnapshot` carries"). It matters most for actop's core audience: a user +profiling a local inference run via `--json` or `--serve` gets watts and +utilization but cannot see which process drew them, whether the chip throttled, +or total energy for the run — exactly the questions a profiling session asks. + +Two viable shapes, not yet decided: + +1. **Opt-in flags on the export entry points** — `--json-processes` / + `--serve-processes` pass `include_processes=True`, and the run loops own an + `AlertEngine` whose `AlertFrame` is merged into the emitted record. Smallest + change; leaves `SystemSnapshot` alone. +2. **Alerts become snapshot fields** — `AlertEngine` moves behind `Monitor`, so + `SystemSnapshot` carries the throttle/alert booleans and session energy, and + every consumer (TUI, NDJSON, Prometheus) reads one contract. Truer to §1's + thesis, but makes `Monitor` stateful across calls and needs a decision about + whether an API user wants alert sustain-counting they did not configure. + +Prefer (1) unless a consumer materially needs alerts on the typed model — +`AlertEngine` is stateful and threshold-configured, which sits awkwardly inside a +per-frame snapshot. Note the per-PID Prometheus cardinality constraint recorded in +the v1.5.0 notes: process data stays NDJSON-only either way, never a labelled +gauge. + +### 10.3 §8-1's literal grep does not pass — by design, not a defect + +`grep -n "from actop.utils import" actop/tui/*.py` still matches: +`tui/app.py` imports `get_soc_info` for the one call at construction that builds +`DashboardConfig` (`create_dashboard_config(args, get_soc_info())`). + +The acceptance criterion was written to kill **per-frame** L1 acquisition in the +view, and that is gone: nothing in the render loop touches L1. What remains is a +single startup call to obtain the platform info the config merge requires, which +has nowhere better to live — `create_dashboard_config` takes `soc_info` as a +parameter precisely so the caller supplies it. **Treat §8-1 as satisfied in +substance and reword it rather than "fixing" the import**; routing it through +another module to beat a grep would add indirection for no layering gain. diff --git a/pyproject.toml b/pyproject.toml index 641a5bb..4349c51 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -4,7 +4,7 @@ build-backend = "setuptools.build_meta" [project] name = "actop" -version = "1.6.0" +version = "1.6.1" description = "Apple Silicon (M1–M4) power, GPU, ANE & memory-bandwidth monitor — sudoless TUI + Python API for profiling local LLM / MLX / CoreML inference" readme = "README.md" license = { text = "MIT" }