From 07db206feb856329bdd9225e0e1b890caa19af67 Mon Sep 17 00:00:00 2001 From: Chang Liu Date: Wed, 22 Jul 2026 16:00:02 -0400 Subject: [PATCH 1/4] =?UTF-8?q?feat(cli):=20add=20`results=20matrix`=20beh?= =?UTF-8?q?avior=C3=97arm=20comparison=20table?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds `assert-ai results matrix` rendering a 2D pivot (rows=behavior, cols=arm/variant, cells=metric rate) over multiple runs, for the N-arms × M-behaviors comparison used by the 1-behavior-per-yaml pattern. Pivots on each run's config.yaml behavior.name and an arm label derived from the run id. Includes --suite auto-expand, --json, and tests. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: d429caee-f2da-4fb0-8b2c-d0eff7cf40a3 --- assert_ai/cli.py | 176 ++++++++++++++++++++++++++ tests/test_cli_results_matrix.py | 205 +++++++++++++++++++++++++++++++ 2 files changed, 381 insertions(+) create mode 100644 tests/test_cli_results_matrix.py diff --git a/assert_ai/cli.py b/assert_ai/cli.py index 72cd1f5c..7eb3f04e 100644 --- a/assert_ai/cli.py +++ b/assert_ai/cli.py @@ -374,6 +374,13 @@ def _dimension_rate(metrics: dict[str, Any], metric: str) -> float | None: return float(rate) if isinstance(rate, (int, float)) else None +def _run_dimension_rate(run_summary: dict[str, Any], metric: str) -> float | None: + prompt_rate = _dimension_rate(run_summary.get("prompt_metrics") or {}, metric) + if prompt_rate is not None: + return prompt_rate + return _dimension_rate(run_summary.get("scenario_metrics") or {}, metric) + + def _reject_ordinal_compare(run_summaries: Iterable[dict[str, Any]], metric: str) -> None: for run_summary in run_summaries: for key in ("prompt_metrics", "scenario_metrics"): @@ -514,6 +521,62 @@ def _load_run_summary(run_dir: Path) -> dict[str, Any] | None: } +def _run_behavior_name(run_dir: Path, suite_id: str) -> str: + config_path = run_dir / "config.yaml" + config = yaml.safe_load(config_path.read_text(encoding="utf-8")) if config_path.exists() else None + behavior = config.get("behavior") if isinstance(config, dict) else None + if isinstance(behavior, dict) and isinstance(behavior.get("name"), str) and behavior.get("name"): + return behavior["name"] + + manifest = load_json(run_dir / "manifest.json") + manifest_candidates: list[Any] = [] + if isinstance(manifest, dict): + manifest_behavior = manifest.get("behavior") + manifest_candidates.append(manifest.get("behavior_name")) + if isinstance(manifest_behavior, dict): + manifest_candidates.append(manifest_behavior.get("name")) + manifest_config = manifest.get("config") + if isinstance(manifest_config, dict): + manifest_behavior = manifest_config.get("behavior") + if isinstance(manifest_behavior, dict): + manifest_candidates.append(manifest_behavior.get("name")) + for candidate in manifest_candidates: + if isinstance(candidate, str) and candidate: + return candidate + return suite_id + + +def _run_arm_label(run_id: str, suite_id: str) -> str: + prefix = f"{suite_id}-" + if run_id.startswith(prefix): + return run_id[len(prefix):] or run_id + if "-" in run_id: + return run_id.rsplit("-", 1)[-1] or run_id + return run_id + + +def _ordered_arm_labels(arms: Iterable[str]) -> list[str]: + known_order = {"baseline": 0, "prompted": 1, "acs": 2} + return sorted( + arms, + key=lambda arm: ( + 0 if arm.lower() in known_order else 1, + known_order.get(arm.lower(), 0), + arm.lower(), + ), + ) + + +def _parse_suite_run_arg(suite_run: str) -> tuple[str, str]: + parts = suite_run.strip("/").split("/") + if len(parts) == 1: + return parts[0], "run-1" + if len(parts) == 2: + return parts[0], parts[1] + _error(f"Invalid format: '{suite_run}'. Use SUITE/RUN (e.g., my-suite/run-1).") + raise AssertionError("unreachable") + + def _count_test_case_types(path: Path) -> tuple[int, int]: rows = load_jsonl(path) prompt_count = 0 @@ -1237,6 +1300,119 @@ def _run_within_suite_compare( console.print(delta_table) +@results.command("matrix", short_help="Compare behaviors across arms as a matrix") +@click.argument("suite_runs", nargs=-1) +@click.option( + "--suite", + "suites", + multiple=True, + shell_complete=_complete_suite, + help="Suite ID under artifacts/results. May be repeated; expands to all runs with scores.", +) +@click.option( + "--results-dir", + type=click.Path(path_type=Path), + default=DEFAULT_RESULTS_DIR, + show_default=True, + help="Results root to inspect.", +) +@click.option( + "--metric", + default=DEFAULT_COMPARE_METRIC, + show_default=True, + help="Judge dimension to compare.", +) +@click.option("--json", "as_json", is_flag=True, help="Emit machine-readable JSON instead of tables.") +@click.option("--no-color", is_flag=True, help="Disable colored terminal output.") +def results_matrix( + suite_runs: tuple[str, ...], + suites: tuple[str, ...], + results_dir: Path, + metric: str, + as_json: bool, + no_color: bool, +): + """Render a behavior x arm pivot over multiple runs.""" + if not suites and len(suite_runs) < 2: + _error("Provide at least two SUITE/RUN arguments to compare, or use --suite SUITE.") + + results_root = _resolve_results_dir(results_dir) + resolved_suite_runs: list[tuple[str, str]] = [_parse_suite_run_arg(suite_run) for suite_run in suite_runs] + for suite in suites: + suite_dir = results_root / suite + if not suite_dir.exists(): + _error(f"Suite not found: {suite}") + for child in sorted(suite_dir.iterdir()): + if child.is_dir() and (child / "scores.jsonl").exists(): + resolved_suite_runs.append((suite, child.name)) + + if len(resolved_suite_runs) < 2: + _error("Provide at least two runs with scores to compare.") + + run_summaries: list[dict[str, Any]] = [] + behaviors: list[str] = [] + arms: list[str] = [] + cells: dict[str, dict[str, float | None]] = {} + seen_behaviors: set[str] = set() + seen_arms: set[str] = set() + + for suite_id, run_id in resolved_suite_runs: + run_dir = results_root / suite_id / run_id + if not run_dir.exists(): + _error(f"Not found: {suite_id}/{run_id}") + run_summary = _load_run_summary(run_dir) + if run_summary is None: + _error(f"No scores in {suite_id}/{run_id}") + run_summaries.append(run_summary) + + behavior = _run_behavior_name(run_dir, suite_id) + arm = _run_arm_label(run_id, suite_id) + if behavior not in seen_behaviors: + behaviors.append(behavior) + seen_behaviors.add(behavior) + if arm not in seen_arms: + arms.append(arm) + seen_arms.add(arm) + + cells.setdefault(behavior, {})[arm] = _run_dimension_rate(run_summary, metric) + + _reject_ordinal_compare(run_summaries, metric) + arms = _ordered_arm_labels(arms) + + if as_json: + _echo_json({ + "metric": metric, + "behaviors": behaviors, + "arms": arms, + "cells": { + behavior: { + arm: cells.get(behavior, {}).get(arm) + for arm in arms + } + for behavior in behaviors + }, + }) + return + + console = _console(no_color=no_color) + table = Table( + title=f"Behavior × arm matrix ({_metric_label(metric)})", + box=None, + show_header=True, + show_edge=False, + pad_edge=False, + ) + table.add_column("Behavior", style="cyan", no_wrap=True) + for arm in arms: + table.add_column(arm, style="white", no_wrap=True) + for behavior in behaviors: + row = [behavior] + for arm in arms: + row.append(_fmt_percent(cells.get(behavior, {}).get(arm))) + table.add_row(*row) + console.print(table) + + @results.command("compare-suites", short_help="Compare runs across different suites (e.g., approach A vs B vs C)") @click.argument("suite_runs", nargs=-1) @click.option( diff --git a/tests/test_cli_results_matrix.py b/tests/test_cli_results_matrix.py new file mode 100644 index 00000000..3a37c7dd --- /dev/null +++ b/tests/test_cli_results_matrix.py @@ -0,0 +1,205 @@ +# Copyright (c) Microsoft Corporation. +# Licensed under the MIT License. + +import json +from pathlib import Path +from typing import Any + +from click.testing import CliRunner + +from assert_ai.cli import cli + + +def _write_jsonl(path: Path, rows: list[dict[str, Any]]) -> None: + path.write_text("\n".join(json.dumps(row) for row in rows) + "\n", encoding="utf-8") + + +def _score_row(policy_violation: bool) -> dict[str, Any]: + return { + "judge_status": "ok", + "target": "test-target", + "judge_model": "test-judge", + "verdict": { + "dimensions": { + "policy_violation": policy_violation, + "overrefusal": False, + }, + "node_judgments": [], + }, + } + + +def _make_run( + results_root: Path, + suite_id: str, + run_id: str, + behavior_name: str | None, + policy_violations: list[bool], +) -> None: + run_dir = results_root / suite_id / run_id + run_dir.mkdir(parents=True) + (run_dir / "manifest.json").write_text( + json.dumps({"status": "completed", "stages": {"judge": "completed"}}), + encoding="utf-8", + ) + (run_dir / "config.yaml").write_text( + "\n".join([ + "behavior:", + f" name: {behavior_name}", + ]) if behavior_name is not None else "behavior: {}\n", + encoding="utf-8", + ) + _write_jsonl(run_dir / "scores.jsonl", [_score_row(value) for value in policy_violations]) + + +def test_results_matrix_json_renders_two_behaviors_by_two_arms(tmp_path: Path) -> None: + results_root = tmp_path / "results" + _make_run(results_root, "behavior-a", "behavior-a-baseline", "behavior_a", [True, False]) + _make_run(results_root, "behavior-a", "behavior-a-prompted", "behavior_a", [False, False]) + _make_run(results_root, "behavior-b", "behavior-b-baseline", "behavior_b", [True, True]) + _make_run(results_root, "behavior-b", "behavior-b-prompted", "behavior_b", [False, True]) + + result = CliRunner().invoke( + cli, + [ + "results", + "matrix", + "behavior-a/behavior-a-baseline", + "behavior-a/behavior-a-prompted", + "behavior-b/behavior-b-baseline", + "behavior-b/behavior-b-prompted", + "--results-dir", + str(results_root), + "--json", + ], + ) + + assert result.exit_code == 0, result.output + assert json.loads(result.output) == { + "metric": "policy_violation", + "behaviors": ["behavior_a", "behavior_b"], + "arms": ["baseline", "prompted"], + "cells": { + "behavior_a": {"baseline": 0.5, "prompted": 0.0}, + "behavior_b": {"baseline": 1.0, "prompted": 0.5}, + }, + } + + +def test_results_matrix_missing_cell_renders_null_and_dash(tmp_path: Path) -> None: + results_root = tmp_path / "results" + _make_run(results_root, "behavior-a", "behavior-a-baseline", "behavior_a", [True]) + _make_run(results_root, "behavior-a", "behavior-a-prompted", "behavior_a", [False]) + _make_run(results_root, "behavior-b", "behavior-b-baseline", "behavior_b", [False]) + + args = [ + "results", + "matrix", + "behavior-a/behavior-a-baseline", + "behavior-a/behavior-a-prompted", + "behavior-b/behavior-b-baseline", + "--results-dir", + str(results_root), + ] + runner = CliRunner() + + json_result = runner.invoke(cli, [*args, "--json"]) + assert json_result.exit_code == 0, json_result.output + payload = json.loads(json_result.output) + assert payload["cells"]["behavior_b"]["prompted"] is None + + text_result = runner.invoke(cli, [*args, "--no-color"]) + assert text_result.exit_code == 0, text_result.output + assert "behavior_b" in text_result.output + assert "-" in text_result.output + + +def test_results_matrix_suite_auto_expand_matches_explicit_args(tmp_path: Path) -> None: + results_root = tmp_path / "results" + _make_run(results_root, "suite-a", "suite-a-baseline", "behavior_a", [True, False]) + _make_run(results_root, "suite-a", "suite-a-prompted", "behavior_a", [False, False]) + + runner = CliRunner() + explicit = runner.invoke( + cli, + [ + "results", + "matrix", + "suite-a/suite-a-baseline", + "suite-a/suite-a-prompted", + "--results-dir", + str(results_root), + "--json", + ], + ) + expanded = runner.invoke( + cli, + [ + "results", + "matrix", + "--suite", + "suite-a", + "--results-dir", + str(results_root), + "--json", + ], + ) + + assert explicit.exit_code == 0, explicit.output + assert expanded.exit_code == 0, expanded.output + assert json.loads(explicit.output) == json.loads(expanded.output) + + +def test_results_matrix_repeated_suite_expands_multiple_suites_with_known_arm_order(tmp_path: Path) -> None: + results_root = tmp_path / "results" + _make_run(results_root, "suite-a", "suite-a-acs", "behavior_a", [False]) + _make_run(results_root, "suite-a", "suite-a-baseline", "behavior_a", [True]) + _make_run(results_root, "suite-b", "suite-b-prompted", "behavior_b", [True, False]) + _make_run(results_root, "suite-b", "suite-b-acs", "behavior_b", [False, False]) + _make_run(results_root, "suite-b", "suite-b-baseline", "behavior_b", [True, True]) + + result = CliRunner().invoke( + cli, + [ + "results", + "matrix", + "--suite", + "suite-a", + "--suite", + "suite-b", + "--results-dir", + str(results_root), + "--json", + ], + ) + + assert result.exit_code == 0, result.output + payload = json.loads(result.output) + assert payload["behaviors"] == ["behavior_a", "behavior_b"] + assert payload["arms"] == ["baseline", "prompted", "acs"] + assert payload["cells"] == { + "behavior_a": {"baseline": 1.0, "prompted": None, "acs": 0.0}, + "behavior_b": {"baseline": 1.0, "prompted": 0.5, "acs": 0.0}, + } + + +def test_results_matrix_behavior_name_falls_back_to_suite_id(tmp_path: Path) -> None: + results_root = tmp_path / "results" + _make_run(results_root, "fallback-suite", "fallback-suite-baseline", None, [True]) + _make_run(results_root, "fallback-suite", "fallback-suite-prompted", None, [False]) + + result = CliRunner().invoke( + cli, + [ + "results", + "matrix", + "fallback-suite/fallback-suite-baseline", + "fallback-suite/fallback-suite-prompted", + "--results-dir", + str(results_root), + "--json", + ], + ) + + assert result.exit_code == 0, result.output + assert json.loads(result.output)["behaviors"] == ["fallback-suite"] From b02773e97feb2825dd8888d4972caa7988584fca Mon Sep 17 00:00:00 2001 From: Chang Liu Date: Thu, 6 Aug 2026 16:06:28 -0400 Subject: [PATCH 2/4] feat(cli): matrix leads with the permissibility split `policy_violation` unions permissible and impermissible behaviors, so ranking behaviors by it can order them by the wrong thing entirely. On real runs the two halves diverge sharply -- a behavior can carry a high union rate made up almost wholly of mishandled *permissible* work while another with a lower union rate is nearly all genuine impermissible failure. A behavior x arm matrix is precisely the surface where that ordering matters, since its whole purpose is to say which behavior is worst. So the matrix now defaults to the impermissible half whenever every run reports the split, matching the supersede rule `results list` and `results status` already follow. It requires *all* runs to have it rather than any: one run contributing an impermissible-only rate while another contributes the union would put non-comparable numbers in the same table, which is worse than falling back to the union everywhere. Runs without a taxonomy -- including quality suites that repurpose `policy_violation` for non-safety failures -- keep reporting the union. Fixes a bug in the process. The split is derived from node judgments plus the taxonomy and is stored as a top-level rate, not under `dimensions`, so `_run_dimension_rate` could not see it. Passing `--metric policy_violation_not_permissible` resolved and *labelled* correctly and then rendered every cell as `-`, which reads as "no violations" rather than "not wired up". Both spellings are now accepted: the viewer-facing metric name and the artifact rate key. Also notes the denominators. Each half is scored only over the rows where a behavior in that bucket was relevant, so the halves differ from each other and from `policy_violation` -- on the career-health CV-injection baseline the impermissible half is 4/21 while the permissible half is 16/25. Without saying so, a reader will try to add them and find they do not reconcile to the union. Six tests, each verified to fail against the previous behavior. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 7cb46daf-b5ce-4ad5-a85d-977737e5c02b --- assert_ai/cli.py | 84 ++++++++++++++-- tests/test_cli_results_matrix.py | 165 +++++++++++++++++++++++++++++++ 2 files changed, 243 insertions(+), 6 deletions(-) diff --git a/assert_ai/cli.py b/assert_ai/cli.py index faf3308b..92331785 100644 --- a/assert_ai/cli.py +++ b/assert_ai/cli.py @@ -142,6 +142,24 @@ def _fmt_percent(value: Optional[float]) -> str: "permissible_policy_violation_rate", ) +# Canonical metric name -> the top-level rate key it is stored under. The split +# is derived rather than judged, so it never appears in ``dimensions``; both +# spellings are accepted so ``--metric`` works with either the viewer-facing +# name or the artifact key. +_PERMISSIBILITY_RATE_KEY_BY_METRIC = { + "policy_violation_not_permissible": "not_permissible_policy_violation_rate", + "policy_violation_permissible": "permissible_policy_violation_rate", + "not_permissible_policy_violation_rate": "not_permissible_policy_violation_rate", + "permissible_policy_violation_rate": "permissible_policy_violation_rate", +} + +#: The half of the split the matrix leads with. ``policy_violation`` unions +#: permissible and impermissible behaviors, so ranking behaviors by it can order +#: them by the wrong thing entirely -- a behavior can carry a high union rate +#: made up almost wholly of mishandled *permissible* work while another with a +#: lower union rate is nearly all genuine impermissible failure. +_MATRIX_SPLIT_METRIC = "policy_violation_not_permissible" + def _has_permissibility_split(*metric_sets: Any) -> bool: """True when any of ``metric_sets`` reports the permissibility split. @@ -439,10 +457,28 @@ def _dimension_rate(metrics: dict[str, Any], metric: str) -> float | None: def _run_dimension_rate(run_summary: dict[str, Any], metric: str) -> float | None: - prompt_rate = _dimension_rate(run_summary.get("prompt_metrics") or {}, metric) + prompt_metrics = run_summary.get("prompt_metrics") or {} + scenario_metrics = run_summary.get("scenario_metrics") or {} + + # The permissibility split is not a judged dimension -- it is derived from + # node judgments plus the taxonomy and stored as a top-level rate on the + # metrics dict, not under ``dimensions``. Looking it up like an ordinary + # dimension therefore returns None, and the matrix renders an empty cell + # under a correct-looking "Impermissible behavior violated" heading, which + # reads as "no violations" rather than "not wired up". + split_key = _PERMISSIBILITY_RATE_KEY_BY_METRIC.get(metric) + if split_key is not None: + for metrics in (prompt_metrics, scenario_metrics): + if isinstance(metrics, dict) and split_key in metrics: + rate = metrics.get(split_key) + if isinstance(rate, (int, float)): + return float(rate) + return None + + prompt_rate = _dimension_rate(prompt_metrics, metric) if prompt_rate is not None: return prompt_rate - return _dimension_rate(run_summary.get("scenario_metrics") or {}, metric) + return _dimension_rate(scenario_metrics, metric) def _reject_ordinal_compare(run_summaries: Iterable[dict[str, Any]], metric: str) -> None: @@ -1438,9 +1474,12 @@ def _run_within_suite_compare( ) @click.option( "--metric", - default=DEFAULT_COMPARE_METRIC, - show_default=True, - help="Judge dimension to compare.", + default=None, + help=( + "Judge dimension to compare. Defaults to the impermissible half of the " + "permissibility split when every run reports it, otherwise " + f"'{DEFAULT_COMPARE_METRIC}'." + ), ) @click.option("--json", "as_json", is_flag=True, help="Emit machine-readable JSON instead of tables.") @click.option("--no-color", is_flag=True, help="Disable colored terminal output.") @@ -1448,7 +1487,7 @@ def results_matrix( suite_runs: tuple[str, ...], suites: tuple[str, ...], results_dir: Path, - metric: str, + metric: str | None, as_json: bool, no_color: bool, ): @@ -1476,6 +1515,7 @@ def results_matrix( seen_behaviors: set[str] = set() seen_arms: set[str] = set() + loaded: list[tuple[str, str, dict[str, Any]]] = [] for suite_id, run_id in resolved_suite_runs: run_dir = results_root / suite_id / run_id if not run_dir.exists(): @@ -1493,7 +1533,28 @@ def results_matrix( if arm not in seen_arms: arms.append(arm) seen_arms.add(arm) + loaded.append((behavior, arm, run_summary)) + + # Resolve the default only once every run is loaded: it depends on whether + # they all carry the split. Requiring *all* of them keeps the matrix from + # mixing halves -- one run contributing an impermissible-only rate while + # another contributes the union would put non-comparable numbers in the same + # table, which is worse than falling back to the union everywhere. + if metric is None: + metric = ( + _MATRIX_SPLIT_METRIC + if run_summaries + and all( + _has_permissibility_split( + run_summary.get("prompt_metrics"), + run_summary.get("scenario_metrics"), + ) + for run_summary in run_summaries + ) + else DEFAULT_COMPARE_METRIC + ) + for behavior, arm, run_summary in loaded: cells.setdefault(behavior, {})[arm] = _run_dimension_rate(run_summary, metric) _reject_ordinal_compare(run_summaries, metric) @@ -1532,6 +1593,17 @@ def results_matrix( table.add_row(*row) console.print(table) + if metric in _PERMISSIBILITY_RATE_KEY_BY_METRIC: + # Each half is scored only over the rows where a behavior in that bucket + # was relevant, so the two halves have different denominators from each + # other and from `policy_violation`. Say so, or a reader will try to add + # them and find they do not reconcile to the union. + console.print( + "[dim]Rate is over rows where a behavior in this bucket was relevant, " + "not all scored rows. The two halves of the split therefore have " + "different denominators and do not sum to policy_violation.[/dim]" + ) + @results.command("compare-suites", short_help="Compare runs across different suites (e.g., approach A vs B vs C)") @click.argument("suite_runs", nargs=-1) diff --git a/tests/test_cli_results_matrix.py b/tests/test_cli_results_matrix.py index 3a37c7dd..9bd6c0ad 100644 --- a/tests/test_cli_results_matrix.py +++ b/tests/test_cli_results_matrix.py @@ -203,3 +203,168 @@ def test_results_matrix_behavior_name_falls_back_to_suite_id(tmp_path: Path) -> assert result.exit_code == 0, result.output assert json.loads(result.output)["behaviors"] == ["fallback-suite"] + + +# --- permissibility split ------------------------------------------------- +# +# `policy_violation` unions permissible and impermissible behaviors, so ranking +# behaviors by it can order them by the wrong thing entirely. The matrix +# therefore leads with the impermissible half when every run reports it. +# +# The split is derived from node judgments plus the taxonomy and is stored as a +# top-level rate, not under `dimensions`. Asking for it used to render an empty +# cell under a correct-looking "Impermissible behavior violated" heading, which +# reads as "no violations" rather than "not wired up". + + +def _split_score_row(*, impermissible: bool, permissible: bool) -> dict[str, Any]: + """A row whose node judgments cover both buckets.""" + return { + "judge_status": "ok", + "target": "test-target", + "judge_model": "test-judge", + "verdict": { + "dimensions": { + "policy_violation": impermissible or permissible, + "overrefusal": False, + }, + "node_judgments": [ + {"node_index": 0, "node_name": "must never", "relevant": True, "violated": impermissible}, + {"node_index": 1, "node_name": "allowed", "relevant": True, "violated": permissible}, + ], + }, + } + + +def _make_split_run( + results_root: Path, + suite_id: str, + run_id: str, + behavior_name: str, + rows: list[tuple[bool, bool]], + *, with_taxonomy: bool = True, +) -> None: + run_dir = results_root / suite_id / run_id + run_dir.mkdir(parents=True) + (run_dir / "manifest.json").write_text( + json.dumps({"status": "completed", "stages": {"judge": "completed"}}), encoding="utf-8" + ) + (run_dir / "config.yaml").write_text(f"behavior:\n name: {behavior_name}\n", encoding="utf-8") + if with_taxonomy: + (run_dir.parent / "taxonomy.json").write_text( + json.dumps({ + "behavior_categories": [ + {"name": "must never", "permissible": False}, + {"name": "allowed", "permissible": True}, + ] + }), + encoding="utf-8", + ) + _write_jsonl( + run_dir / "scores.jsonl", + [_split_score_row(impermissible=i, permissible=p) for i, p in rows], + ) + + +def test_matrix_defaults_to_the_impermissible_half_when_every_run_has_the_split(tmp_path: Path) -> None: + results_root = tmp_path / "results" + # 1 of 4 impermissible, 3 of 4 permissible: the union would rank this high + # for the wrong reason. + _make_split_run(results_root, "beh", "beh-baseline", "beh", + [(True, True), (False, True), (False, True), (False, False)]) + _make_split_run(results_root, "beh", "beh-governed", "beh", + [(False, False), (False, False), (False, True), (False, False)]) + + result = CliRunner().invoke( + cli, ["results", "matrix", "--suite", "beh", "--results-dir", str(results_root), "--json"] + ) + + assert result.exit_code == 0, result.output + payload = json.loads(result.output) + assert payload["metric"] == "policy_violation_not_permissible" + assert payload["cells"]["beh"]["baseline"] == 0.25 + assert payload["cells"]["beh"]["governed"] == 0.0 + + +def test_matrix_split_cells_are_populated_not_dashes(tmp_path: Path) -> None: + """The regression: the metric resolved and labelled, but every cell was None.""" + results_root = tmp_path / "results" + _make_split_run(results_root, "beh", "beh-baseline", "beh", [(True, False), (False, True)]) + _make_split_run(results_root, "beh", "beh-governed", "beh", [(False, False), (False, True)]) + + for metric in ("policy_violation_not_permissible", "policy_violation_permissible"): + result = CliRunner().invoke( + cli, + ["results", "matrix", "--suite", "beh", "--results-dir", str(results_root), + "--metric", metric, "--json"], + ) + assert result.exit_code == 0, result.output + cells = json.loads(result.output)["cells"]["beh"] + assert all(value is not None for value in cells.values()), (metric, cells) + + +def test_matrix_accepts_the_artifact_key_spelling_of_the_split(tmp_path: Path) -> None: + results_root = tmp_path / "results" + _make_split_run(results_root, "beh", "beh-baseline", "beh", [(True, False), (False, False)]) + _make_split_run(results_root, "beh", "beh-governed", "beh", [(False, False), (False, False)]) + + result = CliRunner().invoke( + cli, + ["results", "matrix", "--suite", "beh", "--results-dir", str(results_root), + "--metric", "not_permissible_policy_violation_rate", "--json"], + ) + + assert result.exit_code == 0, result.output + assert json.loads(result.output)["cells"]["beh"]["baseline"] == 0.5 + + +def test_matrix_falls_back_to_policy_violation_without_a_taxonomy(tmp_path: Path) -> None: + """Quality suites repurpose policy_violation and have no taxonomy; they must + keep reporting the union rather than a table of blanks.""" + results_root = tmp_path / "results" + _make_split_run(results_root, "beh", "beh-baseline", "beh", + [(True, False), (False, False)], with_taxonomy=False) + _make_split_run(results_root, "beh", "beh-governed", "beh", + [(False, False), (False, False)], with_taxonomy=False) + + result = CliRunner().invoke( + cli, ["results", "matrix", "--suite", "beh", "--results-dir", str(results_root), "--json"] + ) + + assert result.exit_code == 0, result.output + payload = json.loads(result.output) + assert payload["metric"] == "policy_violation" + assert payload["cells"]["beh"]["baseline"] == 0.5 + + +def test_matrix_does_not_mix_halves_when_only_some_runs_have_the_split(tmp_path: Path) -> None: + """One run contributing an impermissible-only rate while another contributes + the union would put non-comparable numbers in one table.""" + results_root = tmp_path / "results" + _make_split_run(results_root, "with-tax", "with-tax-baseline", "beh_a", [(True, False)]) + _make_split_run(results_root, "no-tax", "no-tax-baseline", "beh_b", + [(True, False)], with_taxonomy=False) + + result = CliRunner().invoke( + cli, + ["results", "matrix", "with-tax/with-tax-baseline", "no-tax/no-tax-baseline", + "--results-dir", str(results_root), "--json"], + ) + + assert result.exit_code == 0, result.output + assert json.loads(result.output)["metric"] == "policy_violation" + + +def test_matrix_warns_that_split_halves_have_different_denominators(tmp_path: Path) -> None: + """Each half is scored only over rows where its bucket was relevant, so the + halves do not sum to policy_violation.""" + results_root = tmp_path / "results" + _make_split_run(results_root, "beh", "beh-baseline", "beh", [(True, False), (False, True)]) + _make_split_run(results_root, "beh", "beh-governed", "beh", [(False, False), (False, True)]) + + result = CliRunner().invoke( + cli, ["results", "matrix", "--suite", "beh", "--results-dir", str(results_root)] + ) + + assert result.exit_code == 0, result.output + assert "denominator" in result.output.lower() From 6572ec3d76b2faa18cc90d8b392ff26fd768be3b Mon Sep 17 00:00:00 2001 From: Chang Liu Date: Tue, 11 Aug 2026 18:25:08 -0400 Subject: [PATCH 3/4] fix(cli): pool prompt and scenario rows in the matrix `_run_dimension_rate` returned whichever half was present first -- prompt if it existed, scenario only as a fallback. Runs that have both silently reported half their data, with nothing on screen to say so. The halves are not interchangeable. On the career-health CV-injection baseline the prompt rows score 64% and the scenario rows 88%, so the matrix showed 64% for a run that is 76% overall (38/50). Cells were understated by 12 points, and the error is invisible: a plausible number in a well-formed table. That is the same failure the permissibility work in this PR is meant to address -- a figure that looks authoritative while measuring something narrower than the reader assumes -- so leaving it in place would undercut the change. Both halves are now pooled, and pooled from counts rather than by averaging the two rates. Averaging is wrong whenever the halves differ in size: 1/1 and 1/9 is 2/10, not the 55.6% the mean of 100% and 11.1% would give. The permissibility split pools the same way, from the bucket detail already stored alongside each rate, falling back to the stored rate when an older artifact lacks the detail. Verified against real runs: the union now reports 76.0% and the impermissible half 47.8% (22/46), both matching `results status` and `compute_policy_violation_by_permissibility`. Three tests, all verified to fail against the previous behavior. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 7cb46daf-b5ce-4ad5-a85d-977737e5c02b --- assert_ai/cli.py | 73 ++++++++++++++++++-- tests/test_cli_results_matrix.py | 110 +++++++++++++++++++++++++++++++ 2 files changed, 178 insertions(+), 5 deletions(-) diff --git a/assert_ai/cli.py b/assert_ai/cli.py index 92331785..a945f3d0 100644 --- a/assert_ai/cli.py +++ b/assert_ai/cli.py @@ -153,6 +153,13 @@ def _fmt_percent(value: Optional[float]) -> str: "permissible_policy_violation_rate": "permissible_policy_violation_rate", } +# The bucket detail that carries the counts behind each stored rate, so prompt +# and scenario halves can be pooled from counts rather than averaged. +_PERMISSIBILITY_BUCKET_BY_RATE_KEY = { + "not_permissible_policy_violation_rate": "policy_violation_on_not_permissible", + "permissible_policy_violation_rate": "policy_violation_on_permissible", +} + #: The half of the split the matrix leads with. ``policy_violation`` unions #: permissible and impermissible behaviors, so ranking behaviors by it can order #: them by the wrong thing entirely -- a behavior can carry a high union rate @@ -456,9 +463,47 @@ def _dimension_rate(metrics: dict[str, Any], metric: str) -> float | None: return float(rate) if isinstance(rate, (int, float)) else None +def _pooled_rate(pairs: Iterable[tuple[int, int]]) -> float | None: + """Pool ``(flagged, scored)`` pairs into one rate. + + Rates must be recombined from counts, never averaged: prompt and scenario + halves routinely differ in both rate and size, so the mean of two rates is + not the rate of the whole. + """ + flagged = scored = 0 + for hit, total in pairs: + flagged += hit + scored += total + return flagged / scored if scored else None + + +def _binary_counts(summary: Any) -> tuple[int, int] | None: + """Return ``(flagged, scored)`` for a binary dimension summary.""" + if not isinstance(summary, dict) or summary.get("kind") == "ordinal": + return None + counts = summary.get("counts") + if not isinstance(counts, dict): + return None + flagged = counts.get(1, counts.get("1", 0)) + passed = counts.get(0, counts.get("0", 0)) + if not isinstance(flagged, int) or not isinstance(passed, int): + return None + total = flagged + passed + return (flagged, total) if total else None + + def _run_dimension_rate(run_summary: dict[str, Any], metric: str) -> float | None: + """Rate for ``metric`` across a run's prompt and scenario rows. + + Both halves are pooled. Reporting whichever half happened to be present + first silently drops the other, and they are not interchangeable -- on the + career-health CV-injection baseline the prompt rows score 64% and the + scenario rows 88%, so a prompt-only cell understates the run by 12 points + with nothing on screen to say half the data was excluded. + """ prompt_metrics = run_summary.get("prompt_metrics") or {} scenario_metrics = run_summary.get("scenario_metrics") or {} + halves = [m for m in (prompt_metrics, scenario_metrics) if isinstance(m, dict)] # The permissibility split is not a judged dimension -- it is derived from # node judgments plus the taxonomy and stored as a top-level rate on the @@ -468,13 +513,31 @@ def _run_dimension_rate(run_summary: dict[str, Any], metric: str) -> float | Non # reads as "no violations" rather than "not wired up". split_key = _PERMISSIBILITY_RATE_KEY_BY_METRIC.get(metric) if split_key is not None: - for metrics in (prompt_metrics, scenario_metrics): - if isinstance(metrics, dict) and split_key in metrics: - rate = metrics.get(split_key) - if isinstance(rate, (int, float)): - return float(rate) + bucket_key = _PERMISSIBILITY_BUCKET_BY_RATE_KEY[split_key] + pairs = [ + counts + for metrics in halves + if (counts := _binary_counts(metrics.get(bucket_key))) is not None + ] + if pairs: + return _pooled_rate(pairs) + # Fall back to the stored rate when counts are unavailable -- an older + # artifact may carry the rate without the bucket detail. + for metrics in halves: + rate = metrics.get(split_key) + if isinstance(rate, (int, float)): + return float(rate) return None + pairs = [ + counts + for metrics in halves + if isinstance(metrics.get("dimensions"), dict) + and (counts := _binary_counts(metrics["dimensions"].get(metric))) is not None + ] + if pairs: + return _pooled_rate(pairs) + prompt_rate = _dimension_rate(prompt_metrics, metric) if prompt_rate is not None: return prompt_rate diff --git a/tests/test_cli_results_matrix.py b/tests/test_cli_results_matrix.py index 9bd6c0ad..67d7d0c7 100644 --- a/tests/test_cli_results_matrix.py +++ b/tests/test_cli_results_matrix.py @@ -5,6 +5,7 @@ from pathlib import Path from typing import Any +import pytest from click.testing import CliRunner from assert_ai.cli import cli @@ -368,3 +369,112 @@ def test_matrix_warns_that_split_halves_have_different_denominators(tmp_path: Pa assert result.exit_code == 0, result.output assert "denominator" in result.output.lower() + + +# --- prompt + scenario pooling ------------------------------------------- +# +# A run's prompt and scenario rows are separate metric sets. Reporting whichever +# was present first silently drops the other, and they are not interchangeable: +# on the career-health CV-injection baseline the prompt rows score 64% and the +# scenario rows 88%, so a prompt-only cell understated the run by 12 points with +# nothing on screen to say half the data was excluded. + + +def _mixed_run( + results_root: Path, + suite_id: str, + run_id: str, + behavior_name: str, + prompt_flags: list[bool], + scenario_flags: list[bool], +) -> None: + run_dir = results_root / suite_id / run_id + run_dir.mkdir(parents=True) + (run_dir / "manifest.json").write_text( + json.dumps({"status": "completed", "stages": {"judge": "completed"}}), encoding="utf-8" + ) + (run_dir / "config.yaml").write_text(f"behavior:\n name: {behavior_name}\n", encoding="utf-8") + rows = [_score_row(flag) for flag in prompt_flags] + for flag in scenario_flags: + row = _score_row(flag) + row["tester_model"] = "test-tester" + rows.append(row) + _write_jsonl(run_dir / "scores.jsonl", rows) + + +def test_matrix_pools_prompt_and_scenario_rows(tmp_path: Path) -> None: + results_root = tmp_path / "results" + # 1/4 prompt + 3/4 scenario = 4/8 pooled. Prompt-only would report 0.25. + _mixed_run(results_root, "beh", "beh-baseline", "beh", + [True, False, False, False], [True, True, True, False]) + _mixed_run(results_root, "beh", "beh-governed", "beh", + [False, False, False, False], [False, False, False, False]) + + result = CliRunner().invoke( + cli, + ["results", "matrix", "--suite", "beh", "--results-dir", str(results_root), + "--metric", "policy_violation", "--json"], + ) + + assert result.exit_code == 0, result.output + assert json.loads(result.output)["cells"]["beh"]["baseline"] == 0.5 + + +def test_matrix_pools_from_counts_not_by_averaging_rates(tmp_path: Path) -> None: + """Unequal halves: the mean of the two rates is not the rate of the whole.""" + results_root = tmp_path / "results" + # 1/1 prompt (100%) + 1/9 scenario (11.1%) = 2/10 pooled (20%). + # Averaging the two rates would give 55.6%. + _mixed_run(results_root, "beh", "beh-baseline", "beh", + [True], [True] + [False] * 8) + _mixed_run(results_root, "beh", "beh-governed", "beh", [False], [False]) + + result = CliRunner().invoke( + cli, + ["results", "matrix", "--suite", "beh", "--results-dir", str(results_root), + "--metric", "policy_violation", "--json"], + ) + + assert result.exit_code == 0, result.output + baseline = json.loads(result.output)["cells"]["beh"]["baseline"] + assert baseline == pytest.approx(0.2), baseline + + +def test_matrix_pools_the_permissibility_split_across_both_halves(tmp_path: Path) -> None: + results_root = tmp_path / "results" + run_dir = results_root / "beh" / "beh-baseline" + run_dir.mkdir(parents=True) + (run_dir / "manifest.json").write_text( + json.dumps({"status": "completed", "stages": {"judge": "completed"}}), encoding="utf-8" + ) + (run_dir / "config.yaml").write_text("behavior:\n name: beh\n", encoding="utf-8") + (results_root / "beh" / "taxonomy.json").write_text( + json.dumps({"behavior_categories": [ + {"name": "must never", "permissible": False}, + {"name": "allowed", "permissible": True}, + ]}), + encoding="utf-8", + ) + # 1/2 impermissible in prompt, 1/2 in scenario -> 2/4 pooled. + rows = [ + _split_score_row(impermissible=True, permissible=False), + _split_score_row(impermissible=False, permissible=False), + ] + for flag in (True, False): + row = _split_score_row(impermissible=flag, permissible=False) + row["tester_model"] = "test-tester" + rows.append(row) + _write_jsonl(run_dir / "scores.jsonl", rows) + + _make_split_run(results_root, "beh2", "beh2-baseline", "beh2", [(False, False)]) + + result = CliRunner().invoke( + cli, + ["results", "matrix", "beh/beh-baseline", "beh2/beh2-baseline", + "--results-dir", str(results_root), "--json"], + ) + + assert result.exit_code == 0, result.output + payload = json.loads(result.output) + assert payload["metric"] == "policy_violation_not_permissible" + assert payload["cells"]["beh"]["baseline"] == 0.5 From a4de02dfde0913d22e23a37b4f1825e1b7b93168 Mon Sep 17 00:00:00 2001 From: Chang Liu Date: Wed, 12 Aug 2026 17:32:17 -0400 Subject: [PATCH 4/4] fix(cli): harden results matrix comparisons Preserve unprefixed run IDs, reject cell collisions and unknown metrics, normalize derived permissibility names, and keep count-pooled prompt/scenario rates. Add stale-taxonomy, malformed-config, deduplication, and real-value regression coverage. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 7cb46daf-b5ce-4ad5-a85d-977737e5c02b --- assert_ai/cli.py | 272 +++++++++++++++++-------------- tests/test_cli_results_matrix.py | 270 +++++++++++++++++++++++++++++- 2 files changed, 414 insertions(+), 128 deletions(-) diff --git a/assert_ai/cli.py b/assert_ai/cli.py index a945f3d0..61ba1add 100644 --- a/assert_ai/cli.py +++ b/assert_ai/cli.py @@ -42,6 +42,17 @@ DEFAULT_COMPARE_METRIC = "policy_violation" +_POLICY_VIOLATION_NOT_PERMISSIBLE = "policy_violation_not_permissible" +_POLICY_VIOLATION_PERMISSIBLE = "policy_violation_permissible" +_DERIVED_PERMISSIBILITY_RATE_KEYS = { + _POLICY_VIOLATION_NOT_PERMISSIBLE: "not_permissible_policy_violation_rate", + _POLICY_VIOLATION_PERMISSIBLE: "permissible_policy_violation_rate", +} +_DERIVED_PERMISSIBILITY_SUMMARY_KEYS = { + _POLICY_VIOLATION_NOT_PERMISSIBLE: "policy_violation_on_not_permissible", + _POLICY_VIOLATION_PERMISSIBLE: "policy_violation_on_permissible", +} + _RUNNER_MODULE: Any | None = None _TEST_SET_METRICS_MODULE: Any | None = None @@ -137,27 +148,10 @@ def _fmt_percent(value: Optional[float]) -> str: return f"{value * 100:.1f}%" -_PERMISSIBILITY_SPLIT_RATE_KEYS = ( - "not_permissible_policy_violation_rate", - "permissible_policy_violation_rate", -) - -# Canonical metric name -> the top-level rate key it is stored under. The split -# is derived rather than judged, so it never appears in ``dimensions``; both -# spellings are accepted so ``--metric`` works with either the viewer-facing -# name or the artifact key. -_PERMISSIBILITY_RATE_KEY_BY_METRIC = { - "policy_violation_not_permissible": "not_permissible_policy_violation_rate", - "policy_violation_permissible": "permissible_policy_violation_rate", - "not_permissible_policy_violation_rate": "not_permissible_policy_violation_rate", - "permissible_policy_violation_rate": "permissible_policy_violation_rate", -} - -# The bucket detail that carries the counts behind each stored rate, so prompt -# and scenario halves can be pooled from counts rather than averaged. -_PERMISSIBILITY_BUCKET_BY_RATE_KEY = { - "not_permissible_policy_violation_rate": "policy_violation_on_not_permissible", - "permissible_policy_violation_rate": "policy_violation_on_permissible", +_PERMISSIBILITY_SPLIT_RATE_KEYS = tuple(_DERIVED_PERMISSIBILITY_RATE_KEYS.values()) +_PERMISSIBILITY_METRIC_ALIASES = { + rate_key: metric + for metric, rate_key in _DERIVED_PERMISSIBILITY_RATE_KEYS.items() } #: The half of the split the matrix leads with. ``policy_violation`` unions @@ -165,7 +159,7 @@ def _fmt_percent(value: Optional[float]) -> str: #: them by the wrong thing entirely -- a behavior can carry a high union rate #: made up almost wholly of mishandled *permissible* work while another with a #: lower union rate is nearly all genuine impermissible failure. -_MATRIX_SPLIT_METRIC = "policy_violation_not_permissible" +_MATRIX_SPLIT_METRIC = _POLICY_VIOLATION_NOT_PERMISSIBLE def _has_permissibility_split(*metric_sets: Any) -> bool: @@ -426,7 +420,7 @@ def _load_dimensions() -> dict[str, Any]: def _complete_metric(_: click.Context, __: click.Parameter, incomplete: str) -> list[CompletionItem]: dims = _load_dimensions() - items = sorted(dims.keys()) + items = sorted(set(dims) | set(_DERIVED_PERMISSIBILITY_RATE_KEYS)) return [CompletionItem(name) for name in items if not incomplete or name.startswith(incomplete)] @@ -463,87 +457,6 @@ def _dimension_rate(metrics: dict[str, Any], metric: str) -> float | None: return float(rate) if isinstance(rate, (int, float)) else None -def _pooled_rate(pairs: Iterable[tuple[int, int]]) -> float | None: - """Pool ``(flagged, scored)`` pairs into one rate. - - Rates must be recombined from counts, never averaged: prompt and scenario - halves routinely differ in both rate and size, so the mean of two rates is - not the rate of the whole. - """ - flagged = scored = 0 - for hit, total in pairs: - flagged += hit - scored += total - return flagged / scored if scored else None - - -def _binary_counts(summary: Any) -> tuple[int, int] | None: - """Return ``(flagged, scored)`` for a binary dimension summary.""" - if not isinstance(summary, dict) or summary.get("kind") == "ordinal": - return None - counts = summary.get("counts") - if not isinstance(counts, dict): - return None - flagged = counts.get(1, counts.get("1", 0)) - passed = counts.get(0, counts.get("0", 0)) - if not isinstance(flagged, int) or not isinstance(passed, int): - return None - total = flagged + passed - return (flagged, total) if total else None - - -def _run_dimension_rate(run_summary: dict[str, Any], metric: str) -> float | None: - """Rate for ``metric`` across a run's prompt and scenario rows. - - Both halves are pooled. Reporting whichever half happened to be present - first silently drops the other, and they are not interchangeable -- on the - career-health CV-injection baseline the prompt rows score 64% and the - scenario rows 88%, so a prompt-only cell understates the run by 12 points - with nothing on screen to say half the data was excluded. - """ - prompt_metrics = run_summary.get("prompt_metrics") or {} - scenario_metrics = run_summary.get("scenario_metrics") or {} - halves = [m for m in (prompt_metrics, scenario_metrics) if isinstance(m, dict)] - - # The permissibility split is not a judged dimension -- it is derived from - # node judgments plus the taxonomy and stored as a top-level rate on the - # metrics dict, not under ``dimensions``. Looking it up like an ordinary - # dimension therefore returns None, and the matrix renders an empty cell - # under a correct-looking "Impermissible behavior violated" heading, which - # reads as "no violations" rather than "not wired up". - split_key = _PERMISSIBILITY_RATE_KEY_BY_METRIC.get(metric) - if split_key is not None: - bucket_key = _PERMISSIBILITY_BUCKET_BY_RATE_KEY[split_key] - pairs = [ - counts - for metrics in halves - if (counts := _binary_counts(metrics.get(bucket_key))) is not None - ] - if pairs: - return _pooled_rate(pairs) - # Fall back to the stored rate when counts are unavailable -- an older - # artifact may carry the rate without the bucket detail. - for metrics in halves: - rate = metrics.get(split_key) - if isinstance(rate, (int, float)): - return float(rate) - return None - - pairs = [ - counts - for metrics in halves - if isinstance(metrics.get("dimensions"), dict) - and (counts := _binary_counts(metrics["dimensions"].get(metric))) is not None - ] - if pairs: - return _pooled_rate(pairs) - - prompt_rate = _dimension_rate(prompt_metrics, metric) - if prompt_rate is not None: - return prompt_rate - return _dimension_rate(scenario_metrics, metric) - - def _reject_ordinal_compare(run_summaries: Iterable[dict[str, Any]], metric: str) -> None: for run_summary in run_summaries: for key in ("prompt_metrics", "scenario_metrics"): @@ -730,7 +643,12 @@ def _load_run_summary(run_dir: Path) -> dict[str, Any] | None: def _run_behavior_name(run_dir: Path, suite_id: str) -> str: config_path = run_dir / "config.yaml" - config = yaml.safe_load(config_path.read_text(encoding="utf-8")) if config_path.exists() else None + config = None + if config_path.exists(): + try: + config = yaml.safe_load(config_path.read_text(encoding="utf-8")) + except (OSError, UnicodeError, yaml.YAMLError): + pass behavior = config.get("behavior") if isinstance(config, dict) else None if isinstance(behavior, dict) and isinstance(behavior.get("name"), str) and behavior.get("name"): return behavior["name"] @@ -757,8 +675,6 @@ def _run_arm_label(run_id: str, suite_id: str) -> str: prefix = f"{suite_id}-" if run_id.startswith(prefix): return run_id[len(prefix):] or run_id - if "-" in run_id: - return run_id.rsplit("-", 1)[-1] or run_id return run_id @@ -774,6 +690,72 @@ def _ordered_arm_labels(arms: Iterable[str]) -> list[str]: ) +def _pooled_rate(pairs: Iterable[tuple[int, int]]) -> float | None: + """Pool ``(flagged, scored)`` pairs instead of averaging rates.""" + flagged = scored = 0 + for hit, total in pairs: + flagged += hit + scored += total + return flagged / scored if scored else None + + +def _binary_counts(summary: Any) -> tuple[int, int] | None: + """Return ``(flagged, scored)`` for a binary dimension summary.""" + if not isinstance(summary, dict) or summary.get("kind") == "ordinal": + return None + counts = summary.get("counts") + if not isinstance(counts, dict): + return None + flagged = counts.get(1, counts.get("1", 0)) + passed = counts.get(0, counts.get("0", 0)) + if not isinstance(flagged, int) or not isinstance(passed, int): + return None + total = flagged + passed + return (flagged, total) if total else None + + +def _run_dimension_rate(run_summary: dict[str, Any], metric: str) -> float | None: + """Pool one metric across a run's prompt and scenario rows. + + The permissibility split is derived and stored in top-level bucket summaries, + not in ``dimensions``. All rates are recombined from counts when possible. + """ + prompt_metrics = run_summary.get("prompt_metrics") or {} + scenario_metrics = run_summary.get("scenario_metrics") or {} + halves = [metrics for metrics in (prompt_metrics, scenario_metrics) if isinstance(metrics, dict)] + + canonical_metric = _PERMISSIBILITY_METRIC_ALIASES.get(metric, metric) + split_key = _DERIVED_PERMISSIBILITY_RATE_KEYS.get(canonical_metric) + if split_key is not None: + bucket_key = _DERIVED_PERMISSIBILITY_SUMMARY_KEYS[canonical_metric] + pairs = [ + counts + for metrics in halves + if (counts := _binary_counts(metrics.get(bucket_key))) is not None + ] + if pairs: + return _pooled_rate(pairs) + for metrics in halves: + rate = metrics.get(split_key) + if isinstance(rate, (int, float)): + return float(rate) + return None + + pairs = [ + counts + for metrics in halves + if isinstance(metrics.get("dimensions"), dict) + and (counts := _binary_counts(metrics["dimensions"].get(metric))) is not None + ] + if pairs: + return _pooled_rate(pairs) + + prompt_rate = _dimension_rate(prompt_metrics, metric) + if prompt_rate is not None: + return prompt_rate + return _dimension_rate(scenario_metrics, metric) + + def _parse_suite_run_arg(suite_run: str) -> tuple[str, str]: parts = suite_run.strip("/").split("/") if len(parts) == 1: @@ -1538,6 +1520,7 @@ def _run_within_suite_compare( @click.option( "--metric", default=None, + shell_complete=_complete_metric, help=( "Judge dimension to compare. Defaults to the impermissible half of the " "permissibility split when every run reports it, otherwise " @@ -1559,14 +1542,23 @@ def results_matrix( _error("Provide at least two SUITE/RUN arguments to compare, or use --suite SUITE.") results_root = _resolve_results_dir(results_dir) - resolved_suite_runs: list[tuple[str, str]] = [_parse_suite_run_arg(suite_run) for suite_run in suite_runs] + resolved_suite_runs: list[tuple[str, str]] = [] + seen_suite_runs: set[tuple[str, str]] = set() + for suite_run in suite_runs: + parsed = _parse_suite_run_arg(suite_run) + if parsed not in seen_suite_runs: + resolved_suite_runs.append(parsed) + seen_suite_runs.add(parsed) for suite in suites: suite_dir = results_root / suite if not suite_dir.exists(): _error(f"Suite not found: {suite}") for child in sorted(suite_dir.iterdir()): if child.is_dir() and (child / "scores.jsonl").exists(): - resolved_suite_runs.append((suite, child.name)) + parsed = (suite, child.name) + if parsed not in seen_suite_runs: + resolved_suite_runs.append(parsed) + seen_suite_runs.add(parsed) if len(resolved_suite_runs) < 2: _error("Provide at least two runs with scores to compare.") @@ -1575,6 +1567,7 @@ def results_matrix( behaviors: list[str] = [] arms: list[str] = [] cells: dict[str, dict[str, float | None]] = {} + cell_sources: dict[tuple[str, str], str] = {} seen_behaviors: set[str] = set() seen_arms: set[str] = set() @@ -1586,36 +1579,65 @@ def results_matrix( run_summary = _load_run_summary(run_dir) if run_summary is None: _error(f"No scores in {suite_id}/{run_id}") - run_summaries.append(run_summary) + matrix_summary = { + "prompt_metrics": run_summary.get("prompt_metrics"), + "scenario_metrics": run_summary.get("scenario_metrics"), + } + run_summaries.append(matrix_summary) behavior = _run_behavior_name(run_dir, suite_id) arm = _run_arm_label(run_id, suite_id) + source = f"{suite_id}/{run_id}" + cell_key = (behavior, arm) + if previous_source := cell_sources.get(cell_key): + _error( + f"Runs '{previous_source}' and '{source}' both resolve to " + f"behavior '{behavior}' and arm '{arm}'. Use distinct run IDs " + "or select only one run for that cell." + ) + cell_sources[cell_key] = source if behavior not in seen_behaviors: behaviors.append(behavior) seen_behaviors.add(behavior) if arm not in seen_arms: arms.append(arm) seen_arms.add(arm) - loaded.append((behavior, arm, run_summary)) + loaded.append((behavior, arm, matrix_summary)) - # Resolve the default only once every run is loaded: it depends on whether - # they all carry the split. Requiring *all* of them keeps the matrix from - # mixing halves -- one run contributing an impermissible-only rate while - # another contributes the union would put non-comparable numbers in the same - # table, which is worse than falling back to the union everywhere. + split_by_run = [ + any( + _run_dimension_rate(run_summary, split_metric) is not None + for split_metric in _DERIVED_PERMISSIBILITY_RATE_KEYS + ) + for run_summary in run_summaries + ] + + # Requiring every run to have usable split data keeps the matrix from mixing + # an impermissible-only rate with the legacy union. Split-shaped summaries + # whose taxonomy no longer matches their judgments do not count as usable. if metric is None: metric = ( _MATRIX_SPLIT_METRIC - if run_summaries - and all( - _has_permissibility_split( - run_summary.get("prompt_metrics"), - run_summary.get("scenario_metrics"), - ) - for run_summary in run_summaries - ) + if split_by_run and all(split_by_run) else DEFAULT_COMPARE_METRIC ) + metric = _PERMISSIBILITY_METRIC_ALIASES.get(metric, metric) + + available_metrics: set[str] = set() + for run_summary in run_summaries: + prompt_metrics = run_summary.get("prompt_metrics") + scenario_metrics = run_summary.get("scenario_metrics") + for metrics in (prompt_metrics, scenario_metrics): + dimensions = metrics.get("dimensions") if isinstance(metrics, dict) else None + if isinstance(dimensions, dict): + available_metrics.update(dimensions) + if any(split_by_run): + available_metrics.update(_DERIVED_PERMISSIBILITY_RATE_KEYS) + if metric not in available_metrics: + _error( + f"Metric '{metric}' was not found in the compared judgments. " + f"Available: {sorted(available_metrics)}" + ) for behavior, arm, run_summary in loaded: cells.setdefault(behavior, {})[arm] = _run_dimension_rate(run_summary, metric) @@ -1656,7 +1678,7 @@ def results_matrix( table.add_row(*row) console.print(table) - if metric in _PERMISSIBILITY_RATE_KEY_BY_METRIC: + if metric in _DERIVED_PERMISSIBILITY_RATE_KEYS: # Each half is scored only over the rows where a behavior in that bucket # was relevant, so the two halves have different denominators from each # other and from `policy_violation`. Say so, or a reader will try to add diff --git a/tests/test_cli_results_matrix.py b/tests/test_cli_results_matrix.py index 67d7d0c7..462a6ae5 100644 --- a/tests/test_cli_results_matrix.py +++ b/tests/test_cli_results_matrix.py @@ -111,8 +111,8 @@ def test_results_matrix_missing_cell_renders_null_and_dash(tmp_path: Path) -> No text_result = runner.invoke(cli, [*args, "--no-color"]) assert text_result.exit_code == 0, text_result.output - assert "behavior_b" in text_result.output - assert "-" in text_result.output + behavior_b_row = next(line for line in text_result.output.splitlines() if "behavior_b" in line) + assert behavior_b_row.rstrip().endswith("-") def test_results_matrix_suite_auto_expand_matches_explicit_args(tmp_path: Path) -> None: @@ -206,6 +206,165 @@ def test_results_matrix_behavior_name_falls_back_to_suite_id(tmp_path: Path) -> assert json.loads(result.output)["behaviors"] == ["fallback-suite"] +def test_results_matrix_preserves_full_non_prefixed_run_ids(tmp_path: Path) -> None: + results_root = tmp_path / "results" + _make_run( + results_root, + "suite-a", + "variant-c-baseline-prompt", + "behavior_a", + [True], + ) + _make_run( + results_root, + "suite-b", + "baseline-weak-prompt", + "behavior_b", + [False], + ) + + result = CliRunner().invoke( + cli, + [ + "results", + "matrix", + "suite-a/variant-c-baseline-prompt", + "suite-b/baseline-weak-prompt", + "--results-dir", + str(results_root), + "--json", + ], + ) + + assert result.exit_code == 0, result.output + arms = json.loads(result.output)["arms"] + assert arms == ["baseline-weak-prompt", "variant-c-baseline-prompt"] + assert "prompt" not in arms + + +def test_results_matrix_rejects_duplicate_behavior_arm_cells(tmp_path: Path) -> None: + results_root = tmp_path / "results" + _make_run(results_root, "suite-a", "suite-a-baseline", "shared_behavior", [True]) + _make_run(results_root, "suite-b", "suite-b-baseline", "shared_behavior", [False]) + + result = CliRunner().invoke( + cli, + [ + "results", + "matrix", + "suite-a/suite-a-baseline", + "suite-b/suite-b-baseline", + "--results-dir", + str(results_root), + "--json", + ], + ) + + assert result.exit_code == 1 + assert "suite-a/suite-a-baseline" in result.output + assert "suite-b/suite-b-baseline" in result.output + assert "behavior 'shared_behavior' and arm 'baseline'" in result.output + + +def test_results_matrix_rejects_unknown_metric(tmp_path: Path) -> None: + results_root = tmp_path / "results" + _make_run(results_root, "beh", "beh-baseline", "beh", [True]) + _make_run(results_root, "beh", "beh-prompted", "beh", [False]) + + result = CliRunner().invoke( + cli, + [ + "results", + "matrix", + "--suite", + "beh", + "--results-dir", + str(results_root), + "--metric", + "policy_violaton", + "--json", + ], + ) + + assert result.exit_code == 1 + assert "Metric 'policy_violaton' was not found" in result.output + assert "policy_violation" in result.output + + +def test_results_matrix_malformed_config_falls_back_to_manifest(tmp_path: Path) -> None: + results_root = tmp_path / "results" + for run_id, flagged in (("suite-a-baseline", True), ("suite-a-prompted", False)): + _make_run(results_root, "suite-a", run_id, None, [flagged]) + run_dir = results_root / "suite-a" / run_id + (run_dir / "config.yaml").write_text("behavior: [\n", encoding="utf-8") + (run_dir / "manifest.json").write_text( + json.dumps({ + "status": "completed", + "stages": {"judge": "completed"}, + "behavior_name": "manifest_behavior", + }), + encoding="utf-8", + ) + + result = CliRunner().invoke( + cli, + [ + "results", + "matrix", + "--suite", + "suite-a", + "--results-dir", + str(results_root), + "--json", + ], + ) + + assert result.exit_code == 0, result.output + assert json.loads(result.output)["behaviors"] == ["manifest_behavior"] + + +def test_results_matrix_deduplicates_explicit_and_suite_expansion(tmp_path: Path) -> None: + results_root = tmp_path / "results" + _make_run(results_root, "suite-a", "suite-a-baseline", "behavior_a", [True]) + _make_run(results_root, "suite-a", "suite-a-prompted", "behavior_a", [False]) + + result = CliRunner().invoke( + cli, + [ + "results", + "matrix", + "suite-a/suite-a-baseline", + "--suite", + "suite-a", + "--results-dir", + str(results_root), + "--json", + ], + ) + + assert result.exit_code == 0, result.output + assert json.loads(result.output)["cells"] == { + "behavior_a": {"baseline": 1.0, "prompted": 0.0} + } + + +def test_results_matrix_missing_suite_is_a_cli_error(tmp_path: Path) -> None: + result = CliRunner().invoke( + cli, + [ + "results", + "matrix", + "--suite", + "missing-suite", + "--results-dir", + str(tmp_path / "results"), + ], + ) + + assert result.exit_code == 1 + assert "Suite not found: missing-suite" in result.output + + # --- permissibility split ------------------------------------------------- # # `policy_violation` unions permissible and impermissible behaviors, so ranking @@ -316,7 +475,9 @@ def test_matrix_accepts_the_artifact_key_spelling_of_the_split(tmp_path: Path) - ) assert result.exit_code == 0, result.output - assert json.loads(result.output)["cells"]["beh"]["baseline"] == 0.5 + payload = json.loads(result.output) + assert payload["metric"] == "policy_violation_not_permissible" + assert payload["cells"]["beh"]["baseline"] == 0.5 def test_matrix_falls_back_to_policy_violation_without_a_taxonomy(tmp_path: Path) -> None: @@ -338,6 +499,41 @@ def test_matrix_falls_back_to_policy_violation_without_a_taxonomy(tmp_path: Path assert payload["cells"]["beh"]["baseline"] == 0.5 +def test_matrix_falls_back_when_taxonomy_no_longer_matches_judgments(tmp_path: Path) -> None: + results_root = tmp_path / "results" + for run_id, violated in (("beh-baseline", True), ("beh-governed", False)): + _make_split_run( + results_root, + "beh", + run_id, + "beh", + [(violated, False)], + ) + row = _split_score_row(impermissible=violated, permissible=False) + for index, node in enumerate(row["verdict"]["node_judgments"]): + node["node_index"] = index + 10 + node["node_name"] = f"stale-{index}" + _write_jsonl(results_root / "beh" / run_id / "scores.jsonl", [row]) + + result = CliRunner().invoke( + cli, + [ + "results", + "matrix", + "--suite", + "beh", + "--results-dir", + str(results_root), + "--json", + ], + ) + + assert result.exit_code == 0, result.output + payload = json.loads(result.output) + assert payload["metric"] == "policy_violation" + assert payload["cells"]["beh"]["baseline"] == 1.0 + + def test_matrix_does_not_mix_halves_when_only_some_runs_have_the_split(tmp_path: Path) -> None: """One run contributing an impermissible-only rate while another contributes the union would put non-comparable numbers in one table.""" @@ -478,3 +674,71 @@ def test_matrix_pools_the_permissibility_split_across_both_halves(tmp_path: Path payload = json.loads(result.output) assert payload["metric"] == "policy_violation_not_permissible" assert payload["cells"]["beh"]["baseline"] == 0.5 + + +def test_matrix_matches_cv_injection_prompt_scenario_totals(tmp_path: Path) -> None: + results_root = tmp_path / "results" + run_dir = results_root / "cv-injection" / "cv-injection-baseline" + run_dir.mkdir(parents=True) + (run_dir / "manifest.json").write_text( + json.dumps({"status": "completed", "stages": {"judge": "completed"}}), + encoding="utf-8", + ) + (run_dir / "config.yaml").write_text( + "behavior:\n name: cv_injection\n", + encoding="utf-8", + ) + (run_dir.parent / "taxonomy.json").write_text( + json.dumps({ + "behavior_categories": [ + {"name": "must never", "permissible": False}, + {"name": "allowed", "permissible": True}, + ] + }), + encoding="utf-8", + ) + + rows: list[dict[str, Any]] = [] + for index in range(25): + row = _split_score_row( + impermissible=index < 4, + permissible=index < 16, + ) + if index >= 21: + row["verdict"]["node_judgments"][0]["relevant"] = False + rows.append(row) + for index in range(25): + row = _split_score_row( + impermissible=index < 18, + permissible=index < 22, + ) + row["tester_model"] = "test-tester" + rows.append(row) + _write_jsonl(run_dir / "scores.jsonl", rows) + _make_split_run( + results_root, + "control", + "control-baseline", + "control", + [(False, False)], + ) + + base_args = [ + "results", + "matrix", + "cv-injection/cv-injection-baseline", + "control/control-baseline", + "--results-dir", + str(results_root), + "--json", + ] + runner = CliRunner() + union_result = runner.invoke(cli, [*base_args, "--metric", "policy_violation"]) + split_result = runner.invoke(cli, base_args) + + assert union_result.exit_code == 0, union_result.output + assert split_result.exit_code == 0, split_result.output + union = json.loads(union_result.output) + split = json.loads(split_result.output) + assert union["cells"]["cv_injection"]["baseline"] == pytest.approx(38 / 50) + assert split["cells"]["cv_injection"]["baseline"] == pytest.approx(22 / 46)