Skip to content

feat(cli): results matrix — behavior × arm comparison table - #274

Open
Chang Liu (changliu2) wants to merge 9 commits into
mainfrom
changliu2/cli-results-matrix
Open

feat(cli): results matrix — behavior × arm comparison table#274
Chang Liu (changliu2) wants to merge 9 commits into
mainfrom
changliu2/cli-results-matrix

Conversation

@changliu2

@changliu2 Chang Liu (changliu2) commented Jul 22, 2026

Copy link
Copy Markdown
Collaborator

Summary

Adds assert-ai results matrix, a behavior-by-arm comparison for evaluation runs across one or more suites.

  • Rows are behavior names.
  • Columns are run arms such as baseline, prompted, and acs.
  • Cells pool prompt and scenario judgments by scored counts.
  • --suite expands all scored runs in a suite and may be repeated.
  • Explicit SUITE/RUN arguments can select individual runs.
  • --metric selects any Boolean judge dimension.
  • --json emits the same matrix as machine-readable output.

Default metric

The matrix defaults to policy_violation_not_permissible only when every selected run has a non-empty denominator for that exact metric. Otherwise it falls back to the union policy_violation rate.

This prevents an all-empty matrix for one-sided taxonomies that contain only permissible behavior.

Permissibility-split rates use only rows where a behavior in that bucket was relevant. The permissible and not-permissible rates can therefore have different denominators and do not sum to the union rate.

Historical taxonomy integrity

Each run is interpreted using the versioned taxonomy recorded in manifest.artifact_versions.systematize.path, rather than the suite's mutable current taxonomy.json.

Manifest paths are constrained to the suite directory. Valid node_name values are authoritative; node_index is used only for legacy judgments without a usable name. Regenerating or reordering the suite taxonomy therefore cannot silently reverse historical split results.

Safety and error handling

  • Duplicate behavior/arm cells fail with both source runs named.
  • Unknown metrics fail instead of rendering an all-empty table.
  • Non-prefixed run IDs are preserved rather than guessed from suffixes.
  • Malformed run configs fall back to manifest metadata.
  • Prompt and scenario rows are pooled instead of dropping one half.

Documentation

The command and its denominator semantics are documented in:

  • docs/cli/commands.md
  • docs/guides/results.md
  • CHANGELOG.md

Validation

python -m pytest tests/test_cli.py tests/test_cli_results_matrix.py tests/test_results.py tests/test_suite_results_export.py -q
69 passed, 3 skipped, 2 subtests passed

git diff --cached --check
passed

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
`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
@changliu2

Copy link
Copy Markdown
Collaborator Author

The split-vs-union default and the denominator note are the right calls, and the permissibility-split lookup fix is well justified by its tests. Two things block merge for me, both about the arm labeling being lossy on this repo's actual run-id conventions:

  1. _run_arm_label falls back to the last hyphen-delimited segment of the run id, which mislabels most existing runs in artifacts/results (variant-a-unguardedunguarded, variant-c-baseline-promptprompt, baseline-weak-promptprompt, langgraph-multimulti).
  2. Because cells are written with a plain dict assignment, two runs that collapse to the same (behavior, arm) key silently overwrite each other — one run's numbers just vanish from the table with no warning and exit code 0. The two examples above (variant-c-baseline-prompt and baseline-weak-prompt, in a --suite a --suite b invocation over behaviors that share a name) collide today.

Everything else is comment-level: an unvalidated --metric renders a full grid of - (indistinguishable from "no violations") on a typo, and the prompt-then-scenario fallback silently picks one half of a run that has both.

Inline notes (line numbers on the PR head b02773e, assert_ai/cli.py)

cli.py:1558 — must fix.

cells.setdefault(behavior, {})[arm] = _run_dimension_rate(run_summary, metric)

Two runs that resolve to the same (behavior, arm) key silently overwrite each other; the surviving cell looks authoritative and the dropped run leaves no trace. This is reachable today: with --suite bank-manager-agent-shield --suite incident-triage-agent-v1, variant-c-baseline-prompt and baseline-weak-prompt both label as prompt. It is also what you get from any repeated-seed setup (run-1, run-2 of the same arm) — exactly the "multi-run" case the command is for.
Suggested fix: detect an existing key and either error naming both run ids, or aggregate deliberately (mean over runs) and say so in the output. Silent last-write-wins is the one behavior that shouldn't survive.

cli.py:693 — must fix.

if "-" in run_id:
    return run_id.rsplit("-", 1)[-1] or run_id

The rsplit fallback only produces a meaningful label when the run id happens to end in the arm name. Against the run ids actually in artifacts/results it produces: variant-a-unguardedunguarded, variant-b-guardedguarded, variant-c-baseline-promptprompt, guarded-with-shieldshield, langgraph-multimulti (next to langgraphlanggraph). Column headers become wrong-but-plausible, which is worse than verbose.
Suggested fix: when the run id does not carry the {suite_id}- prefix, return the full run_id rather than guessing a suffix. Optionally add an explicit --arm SUITE/RUN=LABEL override for the cases where users want short headers.

cli.py:1560 — should fix. _reject_ordinal_compare is the only validation applied to metric, and it only rejects ordinal dimensions. A typo (--metric policy_violaton) resolves to nothing, every cell renders -, and the command exits 0 under a correct-looking title. Suggested fix: after loading runs, if metric is neither a permissibility key nor present in any run's dimensions, error out with the available dimension names.

cli.py:478 — comment.

prompt_rate = _dimension_rate(prompt_metrics, metric)
if prompt_rate is not None:
    return prompt_rate
return _dimension_rate(scenario_metrics, metric)

For a run with both prompt and scenario rows, the cell is the prompt rate only and the scenario half is dropped without any indication. Worth either a --rows prompt|scenario selector or a footnote when the loaded runs are not homogeneous.

cli.py:670 — comment.

config = yaml.safe_load(config_path.read_text(encoding="utf-8")) if config_path.exists() else None

Unguarded, unlike _load_dimensions (line ~174) which wraps the same call in try/except. A malformed or non-UTF-8 config.yaml in any one run aborts the whole matrix with a raw traceback instead of a CLI error. Wrap and fall through to the manifest/suite-id fallbacks already implemented below.

cli.py:1466 — nice to have. --metric here lacks shell_complete=_complete_metric, which results compare has.

cli.py:1506 — nice to have. --suite expansion appends without deduping against the positional args, so matrix suite-a/run-1 --suite suite-a loads and re-parses run-1 twice. Harmless output-wise but doubles the file I/O.

Performance: no super-linear patterns — loading is O(runs), rendering O(behaviors × arms). The one scale concern is memory, not time: _load_run_summary retains prompt_rows/scenario_rows for every run, and these stay alive for the whole command even though the matrix only reads the computed metrics. Dropping the row lists after the metrics are computed would fix it.

Tests: the split coverage is genuinely good. Gaps worth closing alongside the fixes: arm-label collision, non-conforming run ids like variant-a-unguarded, unknown-metric typo, and --suite pointing at a missing suite. Also, test_results_matrix_missing_cell_renders_null_and_dash asserts "-" in text_result.output, which is satisfied by any hyphen in the table (including behavior names) and would not fail if missing cells rendered wrongly.

Verdict: Request Changes — the arm-label collision silently drops an entire run's numbers from a table whose only job is cross-arm comparison, and the suffix-based labeling mislabels the majority of run ids already in artifacts/results.

Must fix before merge

  1. Silent cell overwrite on duplicate (behavior, arm) (cli.py:1558).
  2. _run_arm_label suffix heuristic mislabels real run ids (cli.py:693).

Should fix
3. Unvalidated --metric renders an all-dash grid at exit 0 (cli.py:1560).

Nice to have
4. Prompt-over-scenario fallback silently picks one half (cli.py:478).
5. Unguarded yaml.safe_load on run config.yaml (cli.py:670).
6. Missing shell_complete on --metric; no dedupe between positional args and --suite; retained score rows inflate memory at scale; the four test gaps above.

Chang Liu (changliu2) and others added 4 commits August 11, 2026 18:25
`_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
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 7cb46daf-b5ce-4ad5-a85d-977737e5c02b
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
# Conflicts:
#	assert_ai/cli.py
@changliu2

Copy link
Copy Markdown
Collaborator Author

Synced current main into this branch in 79c3525, preserving the results-matrix fixes and picking up the Phoenix <19.18.0 pin. The prior correctness blockers are closed: duplicate cells no longer overwrite silently, descriptive arm IDs are preserved, malformed configs surface clean errors, and unknown metrics are rejected. Focused matrix/results tests pass (47 passed, 18 subtests). The full local suite reached 1178 passed before one viewer test hit missing local npm dependencies; after npm ci, the viewer citation suite passes (16 passed). Ready for CI/re-review.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two correctness blockers remain on exact head 79c3525:

  1. A one-sided permissibility taxonomy makes the default matrix render an all-null result. The default-selection logic marks a run as split-capable when either side of the permissibility split has data, then always selects policy_violation_not_permissible. For an all-permissible taxonomy, every run legitimately has data only in the permissible bucket. I reproduced the resulting successful JSON response:

    {
      "metric": "policy_violation_not_permissible",
      "cells": {"allowed_behavior": {"baseline": null, "prompted": null}}
    }

    The same text output is a complete row of -, which looks like missing data despite real scored rows. This conflicts with the command's stated fallback to policy_violation when the preferred metric is unavailable. Select the impermissible default only when that exact metric has a usable denominator in every run; otherwise fall back to the union. Please add an all-permissible regression.

  2. A regenerated taxonomy can silently reverse historical split results. _load_run_summary() recomputes every run's split using the suite's current taxonomy.json. compute_policy_violation_by_permissibility() trusts node_index before node_name, even when the current category at that index has a different name. I created historical rows where index 0 was must never and index 1 was allowed, then reordered those same categories in the current suite taxonomy. A real impermissible baseline violation was reported as 0.0 rather than 1.0, and the command exited successfully. The existing stale-taxonomy test misses this because it changes indices to values outside the current range; ordinary reordering reuses valid indices and produces wrong-but-plausible numbers. Require the index and name to agree, fall back to a stable name match when possible, and fall back to the union when the historical taxonomy cannot be established. Add a reordered-taxonomy regression.

Smaller product-surface gap: this adds a public CLI command with nuanced defaults and collision behavior, but current docs/cli/commands.md, docs/guides/results.md, and CHANGELOG.md do not mention it. Please document the command and its metric/denominator semantics before release.

The prior arm-label and overwrite blockers are fixed: unprefixed run IDs are preserved, duplicate behavior/arm cells fail with both source runs named, unknown metrics fail, prompt and scenario counts are pooled correctly, and malformed configs fall back cleanly. Verification on a synthetic merge with current main: focused results tests 36 passed; full suite 1459 passed, 26 skipped, 840 subtests; viewer check/build passed; wheel and sdist passed twine check; pip check passed; and pip-audit found no known Python vulnerabilities.

Default to the impermissible policy-violation split only when every
compared run has a usable denominator for that exact metric. Load each
run's versioned taxonomy from its manifest, keep artifact paths inside
the suite, and treat valid node names as authoritative over stale
indices.

Add one-sided and reordered-taxonomy regressions and document the
behavior-by-arm matrix command.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 7cb46daf-b5ce-4ad5-a85d-977737e5c02b
@changliu2

Copy link
Copy Markdown
Collaborator Author

Jake Present (@jakepresent) Addressed both exact-head blockers and synced the branch with current main.

  • The default now selects policy_violation_not_permissible only when that exact metric has a usable denominator in every run; all-permissible and other one-sided taxonomies fall back to the union.
  • Each run now loads its versioned taxonomy from manifest.artifact_versions.systematize.path, with paths constrained to the suite directory.
  • Valid node_name values are authoritative; node_index is used only for legacy rows without a usable name, so later taxonomy reordering cannot reverse historical results.
  • Added regressions for all-permissible taxonomies, reordered taxonomies, stale indices, and escaping artifact paths.
  • Documented results matrix and its denominator semantics in the CLI reference, results guide, and changelog.

Validation: 69 passed, 3 skipped, 2 subtests passed; git diff --cached --check passed.

Ready for re-review.

Preserve the results-matrix changelog entry alongside the 0.3 dependency-boundary release notes.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 7cb46daf-b5ce-4ad5-a85d-977737e5c02b
@changliu2

Copy link
Copy Markdown
Collaborator Author

Jake Present (@jakepresent) Synced the reviewed results-matrix fixes onto current main after #336 (4bd6b71).

The only merge conflict was the Unreleased changelog; both the results-matrix entry and the 0.3 dependency-boundary notes are preserved. Focused validation still passes: 69 passed, 3 skipped, 2 subtests passed; diff hygiene is clean.

Ready for exact-head re-review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants