diff --git a/CHANGELOG.md b/CHANGELOG.md index 1fb9c5c..0ebd032 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,25 @@ This project follows a Keep a Changelog-style format and uses version tags for r ## [Unreleased] +## [1.6.2] - 2026-08-08 + +### Fixed +- **Export modes (`--json` / `--serve`) now honor `--show-processes` and + `--proc-filter`** — they were silently ignored, so every NDJSON record carried + `"processes":[]` even when the TUI (`t` key) showed populated per-process data. + The CLI routing (`_run_export`) now forwards both flags to the NDJSON and + Prometheus backends; `run_json_stream` and `serve_prometheus` pass them through + to `Monitor(include_processes=True, process_filter=...)`. `--proc-filter` + without `--show-processes` implies it, matching the Monitor's opt-in cost model + where process collection stays off by default. +- **Per-process Prometheus gauges** (`actop_process_cpu_percent`, + `actop_process_cpu_time_share`, `actop_process_gpu_time_share`, + `actop_process_attributed_watts`, `actop_process_rss_bytes`, + `actop_process_num_threads`): labelled by `pid` and `command`, emitted only + when `include_processes=True`. The NDJSON path needs no format change — + `dataclasses.asdict` already serialised processes when they were collected; + the gap was purely that collection was never enabled. + ## [1.6.1] - 2026-07-29 Docs only — no code change. diff --git a/CLAUDE.md b/CLAUDE.md index 6cc655f..ccf6a0e 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -67,26 +67,85 @@ The `docs/` directory contains essential system reviews, research, and operation - Prefer small, incremental changes in existing files over large refactors. - No formatter/linter config is checked in; match surrounding code style when editing. -## Testing Guidelines +## Testing Guidelines (HARD RULE — enforced at review) + - All tests are located in the `tests/` folder. - Run `.venv/bin/pytest -q` for all code changes. - Run a single test file: `.venv/bin/pytest tests/test_cli_contract.py -q` - Run a single test function: `.venv/bin/pytest tests/test_cli_contract.py::test_name -q` -- **Functional tests only (enforced).** Every test MUST validate behavior through a public or runtime entrypoint, and MUST exercise a production-relevant failure mode, regression risk, or external contract. This rule binds to *all* tests, not just new ones: when touching the suite, delete or rewrite any existing test that violates it rather than leaving it in place. -- **Reject a test (it is structural — do not add it; remove it if present) when it:** - - calls an underscore-prefixed (private) function or method as the unit under test (e.g. `_avg_max`, `_inline_spark`, `_format_core_entry`); - - reads or writes private attributes to arrange or assert state (e.g. `dash._sample_count = 5`, `dash._core_hist`, `dash._chart_glyph`); - - asserts internal implementation details, helper math constants, or a private function's output in isolation; - - uses a mock, fake, monkeypatch, or a synthetic subclass/harness that overrides real behavior to fake layout, I/O, or data; - - exists only to raise coverage; - - asserts only shape/bounds on real public data (e.g. "returns a non-negative dict", "values partition to ≤ 1.0") with no real workload behind it, when a behavioral test already exercises the same code path — the bounds/invariant becomes an assertion *inside* that behavioral test, not a standalone test of its own. -- **Accept a test (it is functional) when it drives a public surface:** CLI invocation (`subprocess` / `build_parser().parse_args`), the public API (`Monitor` / `Profiler`), the real config merge (`create_dashboard_config`), documented public module functions (e.g. `power_to_percent`, `get_soc_profile`), real export/format contracts (NDJSON / Prometheus), real hardware/file I/O paths, or a real widget rendered through its public path (`BrailleChart.render()` via the `data` setter, or a `HardwareDashboard` mounted with Textual `App.run_test()` and fed real `SystemSnapshot`s through `update_metrics`). -- A minimal Textual host `App` used solely to mount a real widget is allowed (it is a mount point, not a fake); faking the data or the logic under test is not. -- Minimum checks before opening a PR: - - `.venv/bin/python -m actop.actop --help` - - `.venv/bin/pytest -q` - - Run `actop` on Apple Silicon and confirm gauges/charts update without crashes. -- For parser or metric changes, include a reproducible sample input/output note in the PR description. + +### The functional-only mandate + +**Every test MUST answer "what production failure mode or regression risk does this +test catch that no other test catches?"** If you cannot give a concrete answer +(broken export format, wrong flag forwarding, missing process data, etc.), the test +is structural — do not add it; delete it if present. This rule binds to *all* +tests, not just new ones. + +### Reject a test (it is structural — do not add it; remove it if present) when it: + +- calls an underscore-prefixed (private) function or method as the unit under test + (e.g. `_avg_max`, `_inline_spark`, `_format_core_entry`); +- reads or writes private attributes to arrange or assert state (e.g. + `dash._sample_count = 5`, `dash._core_hist`, `dash._chart_glyph`); +- asserts internal implementation details, helper math constants, or a private + function's output in isolation; +- uses a mock, fake, monkeypatch, or a synthetic subclass/harness that overrides + real behavior to fake layout, I/O, or data; +- exists only to raise coverage; +- **is a standalone default-value assertion** (e.g. + `parse_args([]).some_flag is False`) — what would break if this test were + deleted? A behavioral test that uses a different value or exercises the same + code path already guards that regression. Default-value checks are redundant, + not load-bearing; +- **is a standalone `parse_args` "this flag accepts value X" test** when a + behavioral test already drives that flag with that value — e.g. + `parse_args(["--flag"]).flag is True` is redundant when + `parse_args(["--json", "--flag"]).flag is True` exists and catches both + the parsing and the forwarding; +- asserts only shape/bounds on real public data (e.g. "returns a non-negative + dict", "values partition to ≤ 1.0") with no real workload behind it, when a + behavioral test already exercises the same code path — the bounds/invariant + becomes an assertion *inside* that behavioral test, not a standalone test of + its own; +- **runs an identical subprocess call as another test** just to assert different + substrings from the same stdout — consolidate the assertions into one test + rather than paying the fork cost twice. + +### Accept a test (it is functional) when it drives a public surface: + +CLI invocation (`subprocess` / `build_parser().parse_args`), the public API +(`Monitor` / `Profiler`), the real config merge (`create_dashboard_config`), +documented public module functions (e.g. `power_to_percent`, `get_soc_profile`), +real export/format contracts (NDJSON / Prometheus), real hardware/file I/O paths, +or a real widget rendered through its public path (`BrailleChart.render()` via the +`data` setter, or a `HardwareDashboard` mounted with Textual `App.run_test()` and +fed real `SystemSnapshot`s through `update_metrics`). + +A minimal Textual host `App` used solely to mount a real widget is allowed (it is +a mount point, not a fake); faking the data or the logic under test is not. + +### Before writing a test, apply this litmus: + +1. **Can I name the production failure this test will catch?** If the answer is + "it protects against argparse breaking" for a default-value assertion, or + "it catches a regression in the flag's boolean default," that failure mode is + already covered by every behavioral test that parses the flag with a non-default + value — skip it. +2. **Does another test already drive the same code path?** Two tests that both + call `parse_args(["---flag"])` and both assert the flag is `True` are one + too many. Keep the one that also asserts the flag's downstream effect. +3. **Is the assertion independently load-bearing?** If you can delete the + assertion and no behavioral test would fail, it is cosmetic — inline it into + a behavioral test or drop it. + +### Minimum checks before opening a PR: + +- `.venv/bin/python -m actop.actop --help` +- `.venv/bin/pytest -q` +- Run `actop` on Apple Silicon and confirm gauges/charts update without crashes. +- For parser or metric changes, include a reproducible sample input/output note + in the PR description. ## Commit & Pull Request Guidelines - **Branch from `main`; PR strictly into `main`.** Every branch forks from `main` and targets `main`. **Never fork a feature branch off another feature branch** (no stacked PRs): if you need work that is still on an unmerged branch, wait for it to merge and re-branch from `main`. This holds especially for CI/CD and release changes — they land via a single PR into `main`, never a chained branch. diff --git a/actop/actop.py b/actop/actop.py index 25551ee..1541af7 100644 --- a/actop/actop.py +++ b/actop/actop.py @@ -219,11 +219,26 @@ def _run_export(args): interval_s = max(1, int(args.interval)) subsamples = max(1, int(args.subsamples)) + include_processes = bool(getattr(args, "show_processes", False)) + proc_filter = str(getattr(args, "proc_filter", "") or "") + if not include_processes and proc_filter: + include_processes = True try: if args.serve is not None: - export.serve_prometheus(args.serve, interval_s, subsamples) + export.serve_prometheus( + args.serve, + interval_s, + subsamples, + include_processes=include_processes, + proc_filter=proc_filter, + ) else: - export.run_json_stream(interval_s, subsamples) + export.run_json_stream( + interval_s, + subsamples, + include_processes=include_processes, + proc_filter=proc_filter, + ) return 0 except KeyboardInterrupt: return 130 diff --git a/actop/export.py b/actop/export.py index 2ab15b6..3fd0cbd 100644 --- a/actop/export.py +++ b/actop/export.py @@ -94,6 +94,39 @@ def snapshot_to_prometheus(snapshot: SystemSnapshot) -> str: lines.append( f"actop_core_frequency_mhz{{{labels}}} {_fmt_number(float(core.freq_mhz))}" ) + + # Per-process gauges (emitted only when the caller opts in via + # Monitor(include_processes=True)). Labelled by pid and command so a + # Prometheus dashboard can join on pid across metrics. + if snapshot.processes: + lines.append("# TYPE actop_process_cpu_percent gauge") + lines.append("# TYPE actop_process_cpu_time_share gauge") + lines.append("# TYPE actop_process_gpu_time_share gauge") + lines.append("# TYPE actop_process_attributed_watts gauge") + lines.append("# TYPE actop_process_rss_bytes gauge") + lines.append("# TYPE actop_process_num_threads gauge") + for proc in snapshot.processes: + cmd = proc.command.replace('"', '\\"') + labels = f'pid="{proc.pid}",command="{cmd}"' + lines.append( + f"actop_process_cpu_percent{{{labels}}} {_fmt_number(float(proc.cpu_percent))}" + ) + cts = proc.cpu_time_share + lines.append( + f"actop_process_cpu_time_share{{{labels}}} {_fmt_number(cts) if cts is not None else 'NaN'}" + ) + gts = proc.gpu_time_share + lines.append( + f"actop_process_gpu_time_share{{{labels}}} {_fmt_number(gts) if gts is not None else 'NaN'}" + ) + aw = proc.attributed_w + lines.append( + f"actop_process_attributed_watts{{{labels}}} {_fmt_number(aw) if aw is not None else 'NaN'}" + ) + lines.append( + f"actop_process_rss_bytes{{{labels}}} {_fmt_number(float(proc.rss_bytes))}" + ) + lines.append(f"actop_process_num_threads{{{labels}}} {proc.num_threads}") return "\n".join(lines) + "\n" @@ -105,7 +138,13 @@ def _fmt_number(value: float) -> str: def run_json_stream( - interval_s: int, subsamples: int, out=None, max_samples: int = 0 + interval_s: int, + subsamples: int, + out=None, + max_samples: int = 0, + *, + include_processes: bool = False, + proc_filter: str = "", ) -> int: """Stream NDJSON snapshots to `out` (default stdout) until interrupted. @@ -115,7 +154,12 @@ def run_json_stream( from actop.api import Monitor stream = out if out is not None else sys.stdout - monitor = Monitor(interval_s, subsamples) + monitor = Monitor( + interval_s, + subsamples, + include_processes=include_processes, + process_filter=proc_filter or None, + ) emitted = 0 try: while True: @@ -156,7 +200,13 @@ def log_message(self, *args): # silence per-request stderr logging def serve_prometheus( - port: int, interval_s: int, subsamples: int, host: str = "0.0.0.0" + port: int, + interval_s: int, + subsamples: int, + host: str = "0.0.0.0", + *, + include_processes: bool = False, + proc_filter: str = "", ) -> None: """Serve Prometheus metrics on http://host:port/metrics until interrupted. @@ -165,7 +215,12 @@ def serve_prometheus( """ from actop.api import Monitor - monitor = Monitor(interval_s, subsamples) + monitor = Monitor( + interval_s, + subsamples, + include_processes=include_processes, + process_filter=proc_filter or None, + ) state = {"snapshot": None} lock = threading.Lock() stop = threading.Event() diff --git a/pyproject.toml b/pyproject.toml index 4349c51..411ca58 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -4,7 +4,7 @@ build-backend = "setuptools.build_meta" [project] name = "actop" -version = "1.6.1" +version = "1.6.2" 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" } diff --git a/tests/test_cli_contract.py b/tests/test_cli_contract.py index e742b9e..d12f73c 100644 --- a/tests/test_cli_contract.py +++ b/tests/test_cli_contract.py @@ -57,42 +57,17 @@ def test_cli_rejects_legacy_show_cores_value_form(): assert "unrecognized arguments: true" in result.stderr -def test_cli_accepts_show_processes_flag(): - result = subprocess.run( - [sys.executable, "-m", "actop.actop", "--show-processes", "--help"], - capture_output=True, - text=True, - check=False, - ) - - assert result.returncode == 0 - - -def test_cli_show_processes_default_is_off(): - args = build_parser().parse_args([]) - assert args.show_processes is False - - -def test_cli_show_processes_flag_turns_on_panel(): - args = build_parser().parse_args(["--show-processes"]) +def test_cli_json_with_show_processes_forwards_both(): + args = build_parser().parse_args(["--json", "--show-processes"]) + assert args.json is True assert args.show_processes is True -def test_cli_chart_glyph_default_is_dots(): - args = build_parser().parse_args([]) - assert args.chart_glyph == "dots" - - def test_cli_chart_glyph_accepts_block(): args = build_parser().parse_args(["--chart-glyph", "block"]) assert args.chart_glyph == "block" -def test_cli_layout_default_is_grid(): - args = build_parser().parse_args([]) - assert args.layout == "grid" - - def test_cli_layout_accepts_stack(): args = build_parser().parse_args(["--layout", "stack"]) assert args.layout == "stack" @@ -110,11 +85,6 @@ def test_cli_rejects_unknown_layout(): assert "invalid choice: 'foo'" in result.stderr -def test_cli_palette_default_is_thermal(): - args = build_parser().parse_args([]) - assert args.palette == "thermal" - - def test_cli_palette_accepts_viridis(): args = build_parser().parse_args(["--palette", "viridis"]) assert args.palette == "viridis" @@ -145,17 +115,6 @@ def test_cli_help_exposes_export_flags(): assert "--serve PORT" in result.stdout -def test_cli_export_flags_have_inactive_defaults(): - args = build_parser().parse_args([]) - assert args.json is False - assert args.serve is None - - -def test_cli_json_flag_sets_true(): - args = build_parser().parse_args(["--json"]) - assert args.json is True - - def test_cli_serve_accepts_valid_port(): args = build_parser().parse_args(["--serve", "9095"]) assert args.serve == 9095 diff --git a/tests/test_export.py b/tests/test_export.py index 9cec594..e79bd5f 100644 --- a/tests/test_export.py +++ b/tests/test_export.py @@ -19,7 +19,7 @@ snapshot_to_json, snapshot_to_prometheus, ) -from actop.models import CoreSample, SystemSnapshot +from actop.models import CoreSample, ProcessSample, SystemSnapshot def _sample_snapshot( @@ -168,6 +168,111 @@ def test_prometheus_fan_gauge_omitted_when_unavailable(): assert "actop_fan_speed_rpm" not in body +def _sample_snapshot_with_processes() -> SystemSnapshot: + snap = _sample_snapshot() + return SystemSnapshot( + timestamp=snap.timestamp, + cpu_watts=snap.cpu_watts, + gpu_watts=snap.gpu_watts, + ane_watts=snap.ane_watts, + package_watts=snap.package_watts, + ecpu_util_pct=snap.ecpu_util_pct, + pcpu_util_pct=snap.pcpu_util_pct, + gpu_util_pct=snap.gpu_util_pct, + gpu_device_pct=snap.gpu_device_pct, + gpu_renderer_pct=snap.gpu_renderer_pct, + gpu_tiler_pct=snap.gpu_tiler_pct, + gpu_perf_stats_available=snap.gpu_perf_stats_available, + gpu_util_source=snap.gpu_util_source, + cpu_temp_c=snap.cpu_temp_c, + gpu_temp_c=snap.gpu_temp_c, + ecpu_freq_mhz=snap.ecpu_freq_mhz, + pcpu_freq_mhz=snap.pcpu_freq_mhz, + gpu_freq_mhz=snap.gpu_freq_mhz, + ram_used_bytes=snap.ram_used_bytes, + ram_total_bytes=snap.ram_total_bytes, + swap_used_bytes=snap.swap_used_bytes, + swap_total_bytes=snap.swap_total_bytes, + ram_used_gb=snap.ram_used_gb, + swap_used_gb=snap.swap_used_gb, + thermal_state=snap.thermal_state, + bandwidth_gbps=snap.bandwidth_gbps, + bandwidth_available=snap.bandwidth_available, + fan_rpms=snap.fan_rpms, + fan_available=snap.fan_available, + e_cores=snap.e_cores, + p_cores=snap.p_cores, + processes=[ + ProcessSample( + pid=1234, + command="python", + cpu_percent=45.5, + cpu_time_share=0.35, + gpu_time_share=0.12, + rss_mb=2048.0, + rss_bytes=2_147_483_648, + num_threads=8, + attributed_w=5.6, + ), + ProcessSample( + pid=5678, + command="ollama", + cpu_percent=12.0, + cpu_time_share=0.09, + gpu_time_share=0.55, + rss_mb=4096.0, + rss_bytes=4_294_967_296, + num_threads=4, + attributed_w=1.8, + ), + ], + ) + + +def test_snapshot_to_json_includes_processes_when_present(): + snapshot = _sample_snapshot_with_processes() + record = json.loads(snapshot_to_json(snapshot)) + + assert "processes" in record + assert len(record["processes"]) == 2 + assert record["processes"][0]["pid"] == 1234 + assert record["processes"][0]["command"] == "python" + assert record["processes"][0]["cpu_time_share"] == 0.35 + assert record["processes"][0]["gpu_time_share"] == 0.12 + assert record["processes"][0]["attributed_w"] == 5.6 + assert record["processes"][0]["rss_bytes"] == 2_147_483_648 + assert record["processes"][1]["pid"] == 5678 + assert record["processes"][1]["command"] == "ollama" + + +def test_prometheus_includes_process_gauges_when_processes_present(): + body = snapshot_to_prometheus(_sample_snapshot_with_processes()) + lines = body.strip().splitlines() + + assert "# TYPE actop_process_cpu_percent gauge" in lines + assert "# TYPE actop_process_cpu_time_share gauge" + assert "# TYPE actop_process_gpu_time_share gauge" + assert "# TYPE actop_process_attributed_watts gauge" + assert "# TYPE actop_process_rss_bytes gauge" + assert "# TYPE actop_process_num_threads gauge" + + assert 'actop_process_cpu_percent{pid="1234",command="python"} 45.5' in lines + assert 'actop_process_cpu_time_share{pid="1234",command="python"} 0.35' in lines + assert 'actop_process_gpu_time_share{pid="1234",command="python"} 0.12' in lines + assert 'actop_process_attributed_watts{pid="1234",command="python"} 5.6' in lines + assert 'actop_process_rss_bytes{pid="1234",command="python"} 2147483648' in lines + assert 'actop_process_num_threads{pid="1234",command="python"} 8' in lines + + assert 'actop_process_cpu_percent{pid="5678",command="ollama"} 12' in lines + + +def test_prometheus_omits_process_gauges_when_no_processes(): + body = snapshot_to_prometheus(_sample_snapshot()) + + assert "actop_process_cpu_percent" not in body + assert "actop_process_gpu_time_share" not in body + + @pytest.mark.local def test_run_json_stream_emits_parseable_records(): buffer = io.StringIO() @@ -217,3 +322,123 @@ def test_serve_prometheus_endpoint_responds(): process.wait(timeout=5) except subprocess.TimeoutExpired: process.kill() + + +@pytest.mark.local +def test_json_stream_includes_processes_when_show_processes_flag_used(): + import signal + + process = subprocess.Popen( + [ + sys.executable, + "-m", + "actop.actop", + "--json", + "--interval", + "1", + "--show-processes", + ], + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + text=True, + ) + try: + stdout, _ = process.communicate(timeout=8) + except subprocess.TimeoutExpired: + process.send_signal(signal.SIGINT) + stdout, _ = process.communicate(timeout=2) + finally: + if process.poll() is None: + process.kill() + + lines = [ln for ln in stdout.splitlines() if ln.strip()] + assert len(lines) >= 1 + for line in lines: + record = json.loads(line) + assert "processes" in record + assert isinstance(record["processes"], list) + + +@pytest.mark.local +def test_json_stream_proc_filter_implies_show_processes(): + import signal + + process = subprocess.Popen( + [ + sys.executable, + "-m", + "actop.actop", + "--json", + "--interval", + "1", + "--proc-filter", + "kernel|launchd", + ], + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + text=True, + ) + try: + stdout, _ = process.communicate(timeout=8) + except subprocess.TimeoutExpired: + process.send_signal(signal.SIGINT) + stdout, _ = process.communicate(timeout=2) + finally: + if process.poll() is None: + process.kill() + + lines = [ln for ln in stdout.splitlines() if ln.strip()] + assert len(lines) >= 1 + for line in lines: + record = json.loads(line) + assert "processes" in record + assert isinstance(record["processes"], list) + assert len(record["processes"]) > 0, ( + "processes empty — --proc-filter did not imply --show-processes" + ) + + +@pytest.mark.local +def test_serve_prometheus_includes_process_gauges_with_show_processes(): + import urllib.request + + port = 19992 + process = subprocess.Popen( + [ + sys.executable, + "-m", + "actop.actop", + "--serve", + str(port), + "--interval", + "1", + "--show-processes", + ], + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + text=True, + ) + try: + body = None + for _ in range(20): + time.sleep(0.5) + try: + with urllib.request.urlopen( + f"http://127.0.0.1:{port}/metrics", timeout=1 + ) as response: + if response.status == 200: + body = response.read().decode() + break + except Exception: + continue + + assert body is not None, "metrics endpoint never returned 200" + assert "# TYPE actop_process_cpu_percent gauge" in body + assert "actop_process_cpu_percent{" in body + assert "actop_process_gpu_time_share{" in body + finally: + process.terminate() + try: + process.wait(timeout=5) + except subprocess.TimeoutExpired: + process.kill()