From 7004d5ebcde87630f5d806f0731fb891d8727157 Mon Sep 17 00:00:00 2001 From: Alex Ngo Date: Thu, 13 Aug 2026 17:33:27 -0700 Subject: [PATCH 1/2] feat(skill): run-assert-skill smoke run before full inference. --- .claude/skills/run-assert-eval/README.md | 34 +- .../skills/run-assert-eval/SETUP-CHECKLIST.md | 9 + .claude/skills/run-assert-eval/SKILL.md | 28 +- .claude/skills/run-assert-eval/smoke_slice.py | 298 +++++++++++++++ .../run-assert-eval/tests/test_smoke_slice.py | 341 ++++++++++++++++++ .../workflows/govern-and-remeasure.md | 20 + .../workflows/measure-clarity-failures.md | 106 +++++- .cursor/rules/assert.mdc | 20 +- .github/prompts/run-assert-eval.prompt.md | 15 +- 9 files changed, 862 insertions(+), 9 deletions(-) create mode 100644 .claude/skills/run-assert-eval/smoke_slice.py create mode 100644 .claude/skills/run-assert-eval/tests/test_smoke_slice.py diff --git a/.claude/skills/run-assert-eval/README.md b/.claude/skills/run-assert-eval/README.md index f0a7d9bf..ef2260d6 100644 --- a/.claude/skills/run-assert-eval/README.md +++ b/.claude/skills/run-assert-eval/README.md @@ -16,7 +16,8 @@ per risk** — without leaving the coding assistant. Risk discovery is owned by | `workflows/govern-and-remeasure.md` | The ACS governance workflow: turn a measured failure into a deployable ACS policy (`assert-ai acs generate`), wrap the agent, and re-run the same eval to prove the failure rate dropped. | | `workflows/diagnose-acs-delta.md` | Fallback reference manual for when a governed run's delta comes out wrong (no drop, or over-gating rose) — symptom-indexed, 15 rules. Most are prevented by the pre-flight classification in `govern-and-remeasure.md` Step 1a. | | `clarity_intake.py` | Dependency-free parser: Clarity failure docs → ASSERT candidate behaviors. | -| `tests/` | Pytest suite + real Clarity fixtures for the parser. | +| `smoke_slice.py` | Slices N real rows out of a generated test set so a config can be validated before the full suite. | +| `tests/` | Pytest suite + real Clarity fixtures for the parser and the slicer. | | `SETUP-CHECKLIST.md` | One-time in-IDE MCP setup + end-to-end verification. | Keep the three skill surfaces (`SKILL.md`, the Copilot prompt, the Cursor rule) @@ -76,6 +77,37 @@ Run the tests: python -m pytest .claude/skills/run-assert-eval/tests/test_clarity_intake.py ``` +## The smoke slicer (`smoke_slice.py`) + +``` +python .claude/skills/run-assert-eval/smoke_slice.py \ + --config evals/.yaml --count 3 +``` + +Carves the first N rows of a given kind out of a suite's **already generated** +test set and writes them to `artifacts/smoke/--.jsonl`, so +`pipeline.inference.test_set_path` can point at a handful of real cases. Emits a +JSON summary (`source`, `resolved_via`, `out`, `written`, `available`, +`test_case_ids`). Use `--suite` instead of `--config` to skip the PyYAML import. + +- **Resolves through `latest.json`**, the pointer ASSERT itself maintains. + Version dirs (`v0001`, `v0002`, …) are allocated fresh on every cache miss, so + they are never assumed; a stale published copy is only a fallback. +- **Copies raw lines**, so the slice is byte-identical to the source rows — + the smoke run scores cases the full run will also score. +- **Refuses to write inside the suite root**, which could clobber the published + `test_set.jsonl` and invalidate the cache the smoke run exists to protect. +- **Why not just lower `sample_size`**: that block feeds the test_set stage's + `config_hash`, so changing it invalidates the cached test set and cascades + downstream — and under `pairwise` sampling it yields a different design, not a + subset. See `workflows/measure-clarity-failures.md` Step 5a. + +Run the tests: + +``` +python -m pytest .claude/skills/run-assert-eval/tests/test_smoke_slice.py +``` + ## Worked example A full end-to-end walkthrough (one P1 — `user_disengagement` — from parse through diff --git a/.claude/skills/run-assert-eval/SETUP-CHECKLIST.md b/.claude/skills/run-assert-eval/SETUP-CHECKLIST.md index d8664baa..90f47aad 100644 --- a/.claude/skills/run-assert-eval/SETUP-CHECKLIST.md +++ b/.claude/skills/run-assert-eval/SETUP-CHECKLIST.md @@ -47,6 +47,15 @@ once per workspace, then the `run-assert-eval` skill's discovery front door ``` python -m pytest .claude/skills/run-assert-eval/tests/test_clarity_intake.py ``` +- [ ] **Smoke slicer**: after any run that generated a test set, + `python .claude/skills/run-assert-eval/smoke_slice.py --suite --count 3` + writes a 3-row slice and reports `resolved_via: latest.json`; run the unit tests: + ``` + python -m pytest .claude/skills/run-assert-eval/tests/test_smoke_slice.py + ``` +- [ ] **Smoke gate**: the workflow offers a smoke run before the full suite, and a + deliberately broken `target.callable` fails at the smoke step rather than + after a full 25+25 run. - [ ] **Single-P1 run**: from an existing `failures.md`, the workflow presents triage, you pick one P1, **exactly one** config is generated with a variants-derived dimension, `assert-ai run` completes, and the results table diff --git a/.claude/skills/run-assert-eval/SKILL.md b/.claude/skills/run-assert-eval/SKILL.md index 1eb7ed1b..ec5fa252 100644 --- a/.claude/skills/run-assert-eval/SKILL.md +++ b/.claude/skills/run-assert-eval/SKILL.md @@ -254,13 +254,39 @@ single helper call at the top of the callable module — see `docs/targets/calla ### 5. Run the pipeline +**Offer a smoke run first.** A suite is 25 prompt + 25 scenario cases, and +plumbing errors (wrong `callable`, missing credentials, a callable that raises +on its first tool call, tool-schema mismatch, undeployed judge model) surface +only once inference starts. Validate on 3 real cases first: + +``` +# 1. artifacts only, no inference cost +assert-ai run --config evals/.yaml \ + --override inference.enabled=false --override judge.enabled=false + +# 2. slice 3 real rows out of the generated test set +python .claude/skills/run-assert-eval/smoke_slice.py \ + --config evals/.yaml --count 3 + +# 3. inference + judge on those rows only +assert-ai run --config evals/.yaml \ + --override run=-smoke \ + --override inference.test_set_path= +``` + +If it fails, stop and report — do not start the full run. Three cases is not a +measurement, so never report a rate from a smoke run. Never lower +`test_set.sample_size` instead: it invalidates the cached test set and does not +produce a subset. Full detail in `workflows/measure-clarity-failures.md` +Step 5a. + ``` assert-ai run --config evals/.yaml --output json ``` This is long-running (systematize -> test_set -> inference -> judge). Stream status to the user as each stage completes. For N configs, run them sequentially and track -each `suite`/`run`. +each `suite`/`run`. After a smoke run the first two stages report CACHED. - To re-run from a specific stage: `--force-stage ` - Note the `suite` and `run` names from the config for Step 6. diff --git a/.claude/skills/run-assert-eval/smoke_slice.py b/.claude/skills/run-assert-eval/smoke_slice.py new file mode 100644 index 00000000..bf45f96d --- /dev/null +++ b/.claude/skills/run-assert-eval/smoke_slice.py @@ -0,0 +1,298 @@ +"""Slice a few real rows out of a generated ASSERT test set for a smoke run. + +A full suite is 25 prompt + 25 scenario cases, and plumbing errors (wrong +``callable``, missing credentials, a target that raises on its first tool call, +a tool-schema mismatch, an undeployed judge model) only surface once inference +starts. This module lets the skill validate a config on a handful of *real* +cases first, then run the full suite unchanged. + +Why slicing, and not a smaller ``sample_size`` +---------------------------------------------- +Lowering ``pipeline.test_set.prompt.sample_size`` does not work for this. The +raw ``pipeline.test_set`` block feeds the stage's ``config_hash``, so changing it +invalidates the cached test set (and, transitively, inference and judge). It +also does not produce a subset: under ``sampling.method: pairwise`` the sample +size is divided across the covering-array tuples, so a small value drops most +tuples entirely, and case text is regenerated by fresh model calls. You would +validate cases that never appear in the real run, then pay to regenerate it. + +Pointing ``pipeline.inference.test_set_path`` at a slice of the *already +generated* test set avoids both problems: ``pipeline.test_set`` is untouched, so +the full run is still a cache hit, and the smoke cases are literally rows the +full run will score. + +Design notes +------------ +* The current test set is resolved through the suite's ``latest.json`` pointer, + which is what ASSERT itself writes and reads. Artifact version directories + (``v0001``, ``v0002``, …) are allocated fresh on every cache miss, so they are + never assumed. +* Rows are copied as raw text rather than re-serialised, so the slice is + byte-identical to the corresponding lines of the source test set. +* The output path may not land inside the suite root. Writing there could + overwrite the published ``test_set.jsonl`` and corrupt the cache the smoke run + exists to protect. +* Stdlib only, except for an optional PyYAML import used when ``--config`` is + passed. ``--suite`` needs no third-party package at all. +""" + +from __future__ import annotations + +import argparse +import json +import sys +from pathlib import Path + +# Mirrors assert_ai.core.artifact_cache.{LATEST_FILE,ARTIFACTS_DIR} and +# assert_ai.stages.test_set.TEST_SET_FILE. +LATEST_FILE = "latest.json" +ARTIFACTS_DIR = "artifacts" +TEST_SET_FILE = "test_set.jsonl" + +KINDS = ("prompt", "scenario") +DEFAULT_COUNT = 3 + + +class SmokeSliceError(Exception): + """Raised for user-correctable problems, reported without a traceback.""" + + +def _repo_root() -> Path: + """Repository root, four levels up from .claude/skills/run-assert-eval/.""" + + return Path(__file__).resolve().parents[3] + + +def load_config(config_path: str | Path) -> dict: + """Read an eval config. Requires PyYAML; use ``--suite`` to avoid it.""" + + try: + import yaml + except ImportError as exc: # pragma: no cover - depends on environment + raise SmokeSliceError( + "PyYAML is not installed, so --config cannot be read. " + "Pass --suite (and --results-dir if non-default) instead." + ) from exc + + path = Path(config_path) + if not path.is_file(): + raise SmokeSliceError(f"config not found: {path}") + raw = yaml.safe_load(path.read_text(encoding="utf-8")) + if not isinstance(raw, dict): + raise SmokeSliceError(f"config is not a YAML mapping: {path}") + return raw + + +def resolve_results_dir(raw: dict, *, root: Path | None = None) -> Path: + """Resolve ``results_dir`` the way assert_ai.config does.""" + + root = root or _repo_root() + artifacts_root = Path(str(raw.get("artifacts_root") or ARTIFACTS_DIR)).expanduser() + if not artifacts_root.is_absolute(): + artifacts_root = (root / artifacts_root).resolve() + + results_dir_raw = raw.get("results_dir") + if not results_dir_raw: + return (artifacts_root / "results").resolve() + + results_dir = Path(str(results_dir_raw)).expanduser() + if results_dir.is_absolute(): + return results_dir.resolve() + return (artifacts_root / results_dir).resolve() + + +def resolve_test_set(suite_root: Path) -> tuple[Path, str]: + """Locate the current test set for a suite. + + Prefers the ``latest.json`` pointer ASSERT maintains, and falls back to the + copy published at the suite root. Returns the path and how it was found. + """ + + if not suite_root.is_dir(): + raise SmokeSliceError( + f"suite root not found: {suite_root}\n" + "Generate the test set first:\n" + " assert-ai run --config " + "--override inference.enabled=false --override judge.enabled=false" + ) + + latest_path = suite_root / LATEST_FILE + if latest_path.is_file(): + try: + latest = json.loads(latest_path.read_text(encoding="utf-8")) + except (OSError, json.JSONDecodeError): + latest = None + if isinstance(latest, dict): + ref = (latest.get("artifacts") or {}).get("test_set") or {} + relative = ref.get("path") + if relative: + candidate = (suite_root / str(relative)).resolve() + if _is_within(candidate, suite_root) and candidate.is_file(): + return candidate, f"{LATEST_FILE} -> {relative}" + + published = suite_root / TEST_SET_FILE + if published.is_file(): + return published.resolve(), f"published {TEST_SET_FILE}" + + raise SmokeSliceError( + f"no test set found under {suite_root}\n" + "Generate it first:\n" + " assert-ai run --config " + "--override inference.enabled=false --override judge.enabled=false" + ) + + +def _is_within(candidate: Path, base: Path) -> bool: + try: + return candidate.resolve().is_relative_to(base.resolve()) + except OSError: # pragma: no cover - defensive + return False + + +def select_rows(lines: list[str], kind: str, count: int) -> tuple[list[str], list[str], int]: + """Return (selected raw lines, their test_case_ids, total rows of that kind). + + Malformed lines are skipped rather than fatal: the test set is generated + upstream, and a smoke run should surface a bad target, not die on one + unparseable row. + """ + + selected: list[str] = [] + case_ids: list[str] = [] + available = 0 + + for line in lines: + text = line.strip() + if not text: + continue + try: + row = json.loads(text) + except json.JSONDecodeError: + continue + if not isinstance(row, dict) or row.get("type") != kind: + continue + available += 1 + if len(selected) < count: + selected.append(text) + case_ids.append(str(row.get("test_case_id", ""))) + + return selected, case_ids, available + + +def build_slice( + *, + suite: str, + results_dir: Path, + kind: str = "prompt", + count: int = DEFAULT_COUNT, + out_path: Path | None = None, +) -> dict: + """Write a smoke slice and return a summary describing what was written.""" + + if kind not in KINDS: + raise SmokeSliceError(f"kind must be one of {', '.join(KINDS)}; got {kind!r}") + if count < 1: + raise SmokeSliceError(f"count must be at least 1; got {count}") + + suite_root = (results_dir / suite).resolve() + source, resolved_via = resolve_test_set(suite_root) + + lines = source.read_text(encoding="utf-8").splitlines() + selected, case_ids, available = select_rows(lines, kind, count) + if not selected: + raise SmokeSliceError( + f"no rows of type {kind!r} in {source}. " + f"Check that pipeline.test_set.{kind} is configured." + ) + + if out_path is None: + out_path = _repo_root() / ARTIFACTS_DIR / "smoke" / f"{suite}-{kind}-{len(selected)}.jsonl" + out_path = Path(out_path).expanduser().resolve() + + # Writing inside the suite root could clobber the published test set and + # invalidate the very cache this slice exists to preserve. + if _is_within(out_path, suite_root): + raise SmokeSliceError( + f"refusing to write inside the suite root ({suite_root}); " + "choose an --out path outside it, such as artifacts/smoke/." + ) + + out_path.parent.mkdir(parents=True, exist_ok=True) + out_path.write_text("\n".join(selected) + "\n", encoding="utf-8") + + return { + "suite": suite, + "kind": kind, + "requested": count, + "written": len(selected), + "available": available, + "source": str(source), + "resolved_via": resolved_via, + "out": str(out_path), + "test_case_ids": case_ids, + } + + +def main(argv: list[str] | None = None) -> int: + parser = argparse.ArgumentParser( + description="Slice N real rows from a generated ASSERT test set for a smoke run.", + ) + source = parser.add_mutually_exclusive_group(required=True) + source.add_argument("--config", help="Eval config to read `suite` and `results_dir` from.") + source.add_argument("--suite", help="Suite id, if you would rather not read the config.") + parser.add_argument( + "--results-dir", + help="Overrides the config value. Defaults to /results.", + ) + parser.add_argument("--kind", choices=KINDS, default="prompt", help="Default: prompt.") + parser.add_argument( + "--count", type=int, default=DEFAULT_COUNT, help=f"Default: {DEFAULT_COUNT}." + ) + parser.add_argument( + "--out", + help="Output path. Defaults to artifacts/smoke/--.jsonl.", + ) + args = parser.parse_args(argv) + + try: + if args.config: + raw = load_config(args.config) + suite = raw.get("suite") + if not suite: + raise SmokeSliceError(f"config has no `suite` key: {args.config}") + results_dir = ( + Path(args.results_dir).expanduser().resolve() + if args.results_dir + else resolve_results_dir(raw) + ) + else: + suite = args.suite + results_dir = ( + Path(args.results_dir).expanduser().resolve() + if args.results_dir + else resolve_results_dir({}) + ) + + summary = build_slice( + suite=str(suite), + results_dir=results_dir, + kind=args.kind, + count=args.count, + out_path=Path(args.out) if args.out else None, + ) + except SmokeSliceError as exc: + print(f"error: {exc}", file=sys.stderr) + return 1 + + print(json.dumps(summary, indent=2)) + if summary["written"] < summary["requested"]: + print( + f"note: only {summary['written']} {summary['kind']} row(s) available, " + f"requested {summary['requested']}.", + file=sys.stderr, + ) + return 0 + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/.claude/skills/run-assert-eval/tests/test_smoke_slice.py b/.claude/skills/run-assert-eval/tests/test_smoke_slice.py new file mode 100644 index 00000000..d0d9dd4e --- /dev/null +++ b/.claude/skills/run-assert-eval/tests/test_smoke_slice.py @@ -0,0 +1,341 @@ +"""Tests for smoke_slice: carve a few real rows out of a generated test set. + +Every test builds its own suite tree under ``tmp_path``, so nothing here reads or +writes the repo's own ``artifacts/results/``. + +Run standalone: + python -m pytest .claude/skills/run-assert-eval/tests/test_smoke_slice.py +""" + +from __future__ import annotations + +import json +import sys +from pathlib import Path + +import pytest + +# Make the skill dir importable without installing anything. +SKILL_DIR = Path(__file__).resolve().parent.parent +sys.path.insert(0, str(SKILL_DIR)) + +import smoke_slice as ss # noqa: E402 + + +def _row(case_id: str, kind: str) -> str: + return json.dumps( + {"type": kind, "test_case_id": case_id, "behavior": "b", "dimensions": {}} + ) + + +def _make_suite( + tmp_path: Path, + suite: str = "demo-suite", + *, + prompts: int = 5, + scenarios: int = 4, + versioned: bool = True, + publish_copy: bool = True, + version: str = "v0001", +) -> tuple[Path, Path]: + """Build a suite tree. Returns (results_dir, suite_root).""" + + results_dir = tmp_path / "artifacts" / "results" + suite_root = results_dir / suite + suite_root.mkdir(parents=True) + + lines = [_row(f"prompt_{i:03d}", "prompt") for i in range(prompts)] + lines += [_row(f"scenario_{i:03d}", "scenario") for i in range(scenarios)] + body = "\n".join(lines) + "\n" + + if versioned: + artifact_dir = suite_root / ss.ARTIFACTS_DIR / "test_set" / version + artifact_dir.mkdir(parents=True) + (artifact_dir / ss.TEST_SET_FILE).write_text(body, encoding="utf-8") + (suite_root / ss.LATEST_FILE).write_text( + json.dumps( + { + "schema_version": 1, + "artifacts": { + "test_set": { + "version": version, + "path": f"{ss.ARTIFACTS_DIR}/test_set/{version}/{ss.TEST_SET_FILE}", + } + }, + } + ), + encoding="utf-8", + ) + if publish_copy: + (suite_root / ss.TEST_SET_FILE).write_text(body, encoding="utf-8") + + return results_dir, suite_root + + +# --- resolve_test_set ------------------------------------------------------- + + +def test_resolves_through_latest_json_not_by_guessing_v0001(tmp_path): + """The pointer wins, so a later version dir is picked up automatically.""" + + results_dir, suite_root = _make_suite(tmp_path, version="v0007", publish_copy=False) + path, how = ss.resolve_test_set(suite_root) + + assert path.parent.name == "v0007" + assert "latest.json" in how + + +def test_latest_json_wins_over_the_published_copy(tmp_path): + results_dir, suite_root = _make_suite(tmp_path) + # Make the published copy distinguishable from the versioned artifact. + (suite_root / ss.TEST_SET_FILE).write_text(_row("stale_000", "prompt") + "\n", encoding="utf-8") + + path, _ = ss.resolve_test_set(suite_root) + + assert path.parent.name == "v0001" + + +def test_falls_back_to_published_copy_without_latest_json(tmp_path): + results_dir, suite_root = _make_suite(tmp_path, versioned=False) + path, how = ss.resolve_test_set(suite_root) + + assert path.name == ss.TEST_SET_FILE + assert path.parent == suite_root + assert "published" in how + + +def test_malformed_latest_json_falls_back_instead_of_crashing(tmp_path): + results_dir, suite_root = _make_suite(tmp_path) + (suite_root / ss.LATEST_FILE).write_text("{not json", encoding="utf-8") + + path, how = ss.resolve_test_set(suite_root) + + assert path.parent == suite_root + assert "published" in how + + +def test_latest_json_pointing_outside_the_suite_is_ignored(tmp_path): + """A path-traversal pointer must not pull in an arbitrary file.""" + + results_dir, suite_root = _make_suite(tmp_path) + (tmp_path / "evil.jsonl").write_text(_row("evil_000", "prompt") + "\n", encoding="utf-8") + (suite_root / ss.LATEST_FILE).write_text( + json.dumps({"artifacts": {"test_set": {"path": "../../../evil.jsonl"}}}), + encoding="utf-8", + ) + + path, how = ss.resolve_test_set(suite_root) + + assert path.name == ss.TEST_SET_FILE + assert "published" in how + + +def test_missing_suite_root_explains_how_to_generate_it(tmp_path): + with pytest.raises(ss.SmokeSliceError) as exc: + ss.resolve_test_set(tmp_path / "artifacts" / "results" / "nope") + + assert "inference.enabled=false" in str(exc.value) + + +def test_suite_without_any_test_set_is_an_error(tmp_path): + results_dir, suite_root = _make_suite( + tmp_path, versioned=False, publish_copy=False + ) + with pytest.raises(ss.SmokeSliceError): + ss.resolve_test_set(suite_root) + + +# --- select_rows ------------------------------------------------------------ + + +def test_selects_only_the_requested_kind_and_reports_availability(tmp_path): + lines = [_row("p1", "prompt"), _row("s1", "scenario"), _row("p2", "prompt")] + selected, case_ids, available = ss.select_rows(lines, "prompt", 5) + + assert case_ids == ["p1", "p2"] + assert available == 2 + assert len(selected) == 2 + + +def test_takes_the_first_n_in_file_order(tmp_path): + lines = [_row(f"p{i}", "prompt") for i in range(10)] + _, case_ids, available = ss.select_rows(lines, "prompt", 3) + + assert case_ids == ["p0", "p1", "p2"] + assert available == 10 + + +def test_selected_lines_are_byte_identical_to_the_source(tmp_path): + original = _row("p1", "prompt") + selected, _, _ = ss.select_rows([original, _row("s1", "scenario")], "prompt", 1) + + assert selected == [original] + + +def test_blank_and_malformed_lines_are_skipped_not_fatal(tmp_path): + lines = ["", " ", "{not json", _row("p1", "prompt"), "[]"] + selected, case_ids, available = ss.select_rows(lines, "prompt", 3) + + assert case_ids == ["p1"] + assert available == 1 + assert len(selected) == 1 + + +# --- build_slice ------------------------------------------------------------ + + +def test_writes_slice_outside_the_suite_and_summarises(tmp_path): + results_dir, suite_root = _make_suite(tmp_path) + out = tmp_path / "smoke" / "slice.jsonl" + + summary = ss.build_slice( + suite="demo-suite", results_dir=results_dir, count=3, out_path=out + ) + + assert summary["written"] == 3 + assert summary["available"] == 5 + assert summary["kind"] == "prompt" + assert summary["test_case_ids"] == ["prompt_000", "prompt_001", "prompt_002"] + assert out.read_text(encoding="utf-8").splitlines() == [ + _row("prompt_000", "prompt"), + _row("prompt_001", "prompt"), + _row("prompt_002", "prompt"), + ] + + +def test_scenario_kind_is_supported(tmp_path): + results_dir, _ = _make_suite(tmp_path) + out = tmp_path / "smoke" / "scenario.jsonl" + + summary = ss.build_slice( + suite="demo-suite", results_dir=results_dir, kind="scenario", count=2, out_path=out + ) + + assert summary["test_case_ids"] == ["scenario_000", "scenario_001"] + + +def test_requesting_more_than_available_writes_what_exists(tmp_path): + results_dir, _ = _make_suite(tmp_path, prompts=2) + out = tmp_path / "smoke" / "slice.jsonl" + + summary = ss.build_slice( + suite="demo-suite", results_dir=results_dir, count=10, out_path=out + ) + + assert summary["requested"] == 10 + assert summary["written"] == 2 + + +def test_refuses_to_write_inside_the_suite_root(tmp_path): + """Writing there could clobber test_set.jsonl and invalidate the cache.""" + + results_dir, suite_root = _make_suite(tmp_path) + + with pytest.raises(ss.SmokeSliceError) as exc: + ss.build_slice( + suite="demo-suite", + results_dir=results_dir, + out_path=suite_root / ss.TEST_SET_FILE, + ) + + assert "suite root" in str(exc.value) + # The real test set is untouched. + assert len((suite_root / ss.TEST_SET_FILE).read_text(encoding="utf-8").splitlines()) == 9 + + +def test_default_out_path_lands_under_artifacts_smoke(tmp_path, monkeypatch): + results_dir, _ = _make_suite(tmp_path) + monkeypatch.setattr(ss, "_repo_root", lambda: tmp_path) + + summary = ss.build_slice(suite="demo-suite", results_dir=results_dir, count=2) + + out = Path(summary["out"]) + assert out.parent == (tmp_path / ss.ARTIFACTS_DIR / "smoke").resolve() + assert out.name == "demo-suite-prompt-2.jsonl" + + +def test_missing_kind_in_test_set_is_an_actionable_error(tmp_path): + results_dir, _ = _make_suite(tmp_path, scenarios=0) + + with pytest.raises(ss.SmokeSliceError) as exc: + ss.build_slice( + suite="demo-suite", + results_dir=results_dir, + kind="scenario", + out_path=tmp_path / "smoke" / "x.jsonl", + ) + + assert "pipeline.test_set.scenario" in str(exc.value) + + +@pytest.mark.parametrize("count", [0, -1]) +def test_count_must_be_positive(tmp_path, count): + results_dir, _ = _make_suite(tmp_path) + + with pytest.raises(ss.SmokeSliceError): + ss.build_slice(suite="demo-suite", results_dir=results_dir, count=count) + + +def test_unknown_kind_is_rejected(tmp_path): + results_dir, _ = _make_suite(tmp_path) + + with pytest.raises(ss.SmokeSliceError): + ss.build_slice(suite="demo-suite", results_dir=results_dir, kind="promt") + + +# --- resolve_results_dir ---------------------------------------------------- + + +def test_results_dir_defaults_to_artifacts_results(tmp_path): + resolved = ss.resolve_results_dir({}, root=tmp_path) + assert resolved == (tmp_path / "artifacts" / "results").resolve() + + +def test_artifacts_root_is_honoured(tmp_path): + resolved = ss.resolve_results_dir({"artifacts_root": "out"}, root=tmp_path) + assert resolved == (tmp_path / "out" / "results").resolve() + + +def test_relative_results_dir_resolves_under_artifacts_root(tmp_path): + resolved = ss.resolve_results_dir({"results_dir": "runs"}, root=tmp_path) + assert resolved == (tmp_path / "artifacts" / "runs").resolve() + + +def test_absolute_results_dir_is_used_as_is(tmp_path): + absolute = (tmp_path / "elsewhere").resolve() + resolved = ss.resolve_results_dir({"results_dir": str(absolute)}, root=tmp_path) + assert resolved == absolute + + +# --- CLI -------------------------------------------------------------------- + + +def test_cli_emits_json_summary(tmp_path, capsys): + results_dir, _ = _make_suite(tmp_path) + out = tmp_path / "smoke" / "slice.jsonl" + + code = ss.main( + [ + "--suite", "demo-suite", + "--results-dir", str(results_dir), + "--count", "2", + "--out", str(out), + ] + ) + + assert code == 0 + summary = json.loads(capsys.readouterr().out) + assert summary["written"] == 2 + assert summary["suite"] == "demo-suite" + + +def test_cli_reports_errors_without_a_traceback(tmp_path, capsys): + code = ss.main(["--suite", "missing", "--results-dir", str(tmp_path)]) + + assert code == 1 + assert "error:" in capsys.readouterr().err + + +def test_cli_requires_config_or_suite(tmp_path): + with pytest.raises(SystemExit): + ss.main(["--count", "3"]) diff --git a/.claude/skills/run-assert-eval/workflows/govern-and-remeasure.md b/.claude/skills/run-assert-eval/workflows/govern-and-remeasure.md index 54a5b1d0..6959ced4 100644 --- a/.claude/skills/run-assert-eval/workflows/govern-and-remeasure.md +++ b/.claude/skills/run-assert-eval/workflows/govern-and-remeasure.md @@ -538,6 +538,26 @@ eval spec: assert-ai run --config evals/_governed.yaml ``` +**Smoke the governed callable first.** This is the single most likely place for a +wrong `target.callable`, an unimportable `agent_guarded` module, or a manifest +path that doesn't resolve — and the governed run is the second half of an A/B, so +a failure here wastes the whole comparison. The baseline already generated the +test set, so a smoke run costs three cases: + +``` +python .claude/skills/run-assert-eval/smoke_slice.py \ + --config evals/_governed.yaml --count 3 + +assert-ai run --config evals/_governed.yaml \ + --override run=acs-governed-smoke \ + --override inference.test_set_path= +``` + +Confirm the smoke run logs `systematize` and `test_set` as CACHED — if either +regenerates, the two configs have already drifted and the A/B is broken before +the full governed run starts. Do not read rates off three cases; this only proves +the governed target executes. See `measure-clarity-failures.md` Step 5a. + For an ASSERT worked example this governed config is temporary local measurement output: keep it uncommitted and remove it after recording the delta. In a user's product repo, commit it only when they choose to keep the policy as a deployed or diff --git a/.claude/skills/run-assert-eval/workflows/measure-clarity-failures.md b/.claude/skills/run-assert-eval/workflows/measure-clarity-failures.md index 7f24aa10..c7b1d87c 100644 --- a/.claude/skills/run-assert-eval/workflows/measure-clarity-failures.md +++ b/.claude/skills/run-assert-eval/workflows/measure-clarity-failures.md @@ -251,6 +251,95 @@ For each generated config, show the user: `behavior.name`, `behavior.description the stratify `dimensions`, the `target`, and the `judge` settings. Apply any requested edits. **Run only on explicit go-ahead.** +## Step 5a — Smoke run before the full suite (offer by default) + +A suite is 25 prompt + 25 scenario cases, and plumbing errors surface only once +inference starts — after systematize and test_set have already run. Offer a +smoke run on a few **real** cases first: + +> Test set ready: 25 prompt / 25 scenario. +> Smoke test 3 prompt cases before the full run? [Y/n] + +Skip the offer only when the user has asked to run everything unattended. + +**1. Produce the artifacts without paying for inference.** + +``` +assert-ai run --config evals/.yaml \ + --override inference.enabled=false --override judge.enabled=false +``` + +Runs systematize and test_set only, producing the **full** taxonomy and the +**full** 25+25 test set, cached under the real config's key. + +**2. Slice a few real rows.** + +``` +python .claude/skills/run-assert-eval/smoke_slice.py \ + --config evals/.yaml --count 3 +``` + +Prints a JSON summary and writes `artifacts/smoke/-prompt-3.jsonl`. Take +the path from the summary's `out` field. Add `--kind scenario` only when the +risk is inherently multi-turn; scenario cases cost far more per case. + +**3. Run inference and judge on the slice only.** + +``` +assert-ai run --config evals/.yaml \ + --override run=-smoke \ + --override inference.test_set_path= +``` + +**4. Gate.** If the smoke run fails, **stop and report** — do not start the full +run. Typical causes: a wrong `target.callable` path, missing credentials, the +callable raising on its first tool call, a tool-schema mismatch, an undeployed +judge model. Fix, repeat step 3 (steps 1-2 remain valid), then continue. + +The run prints its own `Headline:` block on success — target, judge model, and +the scored counts. To re-read it, or to show the user where it landed: + +``` +assert-ai results status -smoke +``` + +and in the viewer it is a **run inside the existing suite**, not a new suite +card — the suite grid never shows it: + +``` +http://localhost:5174/suite//-smoke +``` + +A smoke run says the config *executes*. It says nothing about the rates — three +cases is not a measurement, so never report a number from it. Two things read as +breakage but are normal: the viewer's **audit/scenario tab is empty** (the slice +is prompt-only, so `viewer_audit_rows.json` is `[]`), and a dimension may show a +**smaller scored count than the slice size** when a case doesn't apply to it. + +**5. Full run** — Step 6, unchanged. `systematize` and `test_set` report CACHED. + +**Never substitute these:** + +- **Do not** lower `pipeline.test_set.prompt.sample_size` for a cheap run. That + block feeds the stage's `config_hash`, so changing it invalidates the cached + test set and cascades into inference and judge. It also yields no subset: + under `sampling.method: pairwise` the sample size is divided across the + covering-array tuples, so a small value drops most tuples and case text is + regenerated. You would validate cases the full run never scores, then pay to + regenerate it. +- **Do not** reuse the real `run:` label. A separate label keeps smoke results + out of the real run's directory and prevents writing its + `.inference_config_hash` / `.judge_config_hash`. +- **Do not** write the slice inside the suite root — `smoke_slice.py` refuses, + because it could clobber the published `test_set.jsonl`. + +**Why the full run stays cheap:** `pipeline.test_set` is never modified, and +artifacts live at `//artifacts/`, a sibling of the run dirs +keyed by suite rather than run — the same mechanism that lets `baseline` and +`acs-governed` share a cached systematization. Leaving systematize and test_set +enabled in step 3 costs nothing (both are cache hits) and keeps the judge +supplied with its taxonomy from context, so no `taxonomy_path` wiring is needed. + ## Step 6 — Run sequentially ``` @@ -258,8 +347,10 @@ assert-ai run --config evals/.yaml ``` Run one at a time. Stream stage status (systematize → test_set → inference → -judge). If one run fails, **report it and continue** with the remaining configs. -Note each `suite`/`run` for the report. +judge). After a smoke run the first two stages report CACHED; if either +regenerates, something changed the config — stop and find out what before +trusting the comparison. If one run fails, **report it and continue** with the +remaining configs. Note each `suite`/`run` for the report. ## Step 7 — Report @@ -331,12 +422,17 @@ Do this at the end of the domain you just measured: (7 values folded into its description), `prompt.sample_size: 25` (the size the user chose, applied to `scenario` too), `inference.max_turns: 10`, and **no `judge.dimensions` block** — `policy_violation` + `overrefusal` are built in. -6. Confirm → `assert-ai run` → results table: one `user_disengagement` column. +6. Confirm → offer a smoke run (Step 5a): generate artifacts with + `--override inference.enabled=false --override judge.enabled=false`, slice 3 + real prompt cases with `smoke_slice.py`, run them under `run=baseline-smoke`. + They pass, so continue. +7. `assert-ai run` → systematize and test_set report CACHED → results table: one + `user_disengagement` column. Headline the permissibility split from `results status --json` — `not_permissible_policy_violation_rate` (real harm got through) and `permissible_policy_violation_rate` (an allowed behavior was broken) — with `overrefusal` alongside as the separate availability check, plus 3–5 cited examples. -7. Offer `record_suggestion` back to Clarity: "user_disengagement now has a +8. Offer `record_suggestion` back to Clarity: "user_disengagement now has a measured baseline at evals/user_disengagement.yaml." -8. Curate the example (Step 9): keep the atomic config and README, and export +9. Curate the example (Step 9): keep the atomic config and README, and export `.clarity-protocol/` outside `examples/` only if the user wants the raw record. diff --git a/.cursor/rules/assert.mdc b/.cursor/rules/assert.mdc index 9b140470..481c9b30 100644 --- a/.cursor/rules/assert.mdc +++ b/.cursor/rules/assert.mdc @@ -129,10 +129,28 @@ To extend an existing config, use `--from `. **Check the built-in presets ### 5. Run the pipeline +**Offer a smoke run first.** Plumbing errors (wrong `callable`, missing credentials, a +callable that raises on its first tool call, tool-schema mismatch, undeployed judge model) +surface only once inference starts. Validate on 3 real cases: + +``` +assert-ai run --config evals/.yaml \ + --override inference.enabled=false --override judge.enabled=false +python .claude/skills/run-assert-eval/smoke_slice.py \ + --config evals/.yaml --count 3 +assert-ai run --config evals/.yaml \ + --override run=-smoke --override inference.test_set_path= +``` + +If it fails, stop and report — do not start the full run. Three cases is not a measurement, +so never report a rate from it. Never lower `test_set.sample_size` instead: that invalidates +the cached test set and does not produce a subset. Detail in +`.claude/skills/run-assert-eval/workflows/measure-clarity-failures.md` Step 5a. + `assert-ai run --config evals/.yaml --output json` This is long-running (systematize -> test_set -> inference -> judge). Stream status as each stage -completes. For N configs, run them sequentially and track each `suite`/`run`. Re-run from a stage with +completes. After a smoke run the first two stages report CACHED. For N configs, run them sequentially and track each `suite`/`run`. Re-run from a stage with `--force-stage `. Note the `suite` and `run` names for Step 6. ### 6. Report results — never collapse to one number diff --git a/.github/prompts/run-assert-eval.prompt.md b/.github/prompts/run-assert-eval.prompt.md index 516a5aa0..5b206f21 100644 --- a/.github/prompts/run-assert-eval.prompt.md +++ b/.github/prompts/run-assert-eval.prompt.md @@ -102,11 +102,24 @@ Help the user set the right target in the config: ### 5. Run the pipeline +**Offer a smoke run first.** Plumbing errors (wrong `callable`, missing credentials, a callable that raises on its first tool call, tool-schema mismatch, undeployed judge model) surface only once inference starts, after the upstream stages have already run. Validate on 3 real cases: + +``` +assert-ai run --config evals/.yaml \ + --override inference.enabled=false --override judge.enabled=false +python .claude/skills/run-assert-eval/smoke_slice.py \ + --config evals/.yaml --count 3 +assert-ai run --config evals/.yaml \ + --override run=-smoke --override inference.test_set_path= +``` + +If it fails, stop and report — do not start the full run. Three cases is not a measurement, so never report a rate from it. Never lower `test_set.sample_size` instead: that invalidates the cached test set and does not produce a subset. Detail in `.claude/skills/run-assert-eval/workflows/measure-clarity-failures.md` Step 5a. + ``` assert-ai run --config evals/.yaml --output json ``` -This is long-running (systematize -> test_set -> inference -> judge). Stream status to the user as each stage completes. For N configs, run them sequentially and track each `suite`/`run`. Re-run from a stage with `--force-stage `. Note the `suite` and `run` names from the config for Step 6. +This is long-running (systematize -> test_set -> inference -> judge). Stream status to the user as each stage completes. For N configs, run them sequentially and track each `suite`/`run`. After a smoke run the first two stages report CACHED. Re-run from a stage with `--force-stage `. Note the `suite` and `run` names from the config for Step 6. ### 6. Report results — never collapse to one number From 0222d7e6b186b43df4c25643dc1000f44ad6743b Mon Sep 17 00:00:00 2001 From: Alex Ngo Date: Fri, 14 Aug 2026 14:02:12 -0700 Subject: [PATCH 2/2] fix(skill): treat --suite as an identifier and match ASSERT's results_dir resolution. --- .claude/skills/run-assert-eval/README.md | 4 + .claude/skills/run-assert-eval/smoke_slice.py | 97 ++++++++++++++- .../run-assert-eval/tests/test_smoke_slice.py | 112 ++++++++++++++++++ 3 files changed, 208 insertions(+), 5 deletions(-) diff --git a/.claude/skills/run-assert-eval/README.md b/.claude/skills/run-assert-eval/README.md index 3f1082b8..0ed30084 100644 --- a/.claude/skills/run-assert-eval/README.md +++ b/.claude/skills/run-assert-eval/README.md @@ -101,6 +101,10 @@ JSON summary (`source`, `resolved_via`, `out`, `written`, `available`, the smoke run scores cases the full run will also score. - **Refuses to write inside the suite root**, which could clobber the published `test_set.jsonl` and invalidate the cache the smoke run exists to protect. +- **Treats `--suite` as an identifier, not a path** — same slug rule ASSERT + applies to `suite`, and the resolved suite root must stay under the results + directory. `--config` resolves `results_dir` exactly as `assert_ai.config` + does, artifact-root prefix included, so both flags read the tree ASSERT wrote. - **Why not just lower `sample_size`**: that block feeds the test_set stage's `config_hash`, so changing it invalidates the cached test set and cascades downstream — and under `pairwise` sampling it yields a different design, not a diff --git a/.claude/skills/run-assert-eval/smoke_slice.py b/.claude/skills/run-assert-eval/smoke_slice.py index bf45f96d..ca121e21 100644 --- a/.claude/skills/run-assert-eval/smoke_slice.py +++ b/.claude/skills/run-assert-eval/smoke_slice.py @@ -32,6 +32,15 @@ * The output path may not land inside the suite root. Writing there could overwrite the published ``test_set.jsonl`` and corrupt the cache the smoke run exists to protect. +* ``--suite`` is a suite *identifier*, not a path. It is joined onto + ``results_dir`` and interpolated into the default output filename, so it is + validated against the same slug rule ASSERT uses and the resolved suite root + is required to stay under ``results_dir``. +* ``resolve_results_dir`` mirrors ``assert_ai.config._resolve_path``, including + its artifact-root prefix stripping, so ``--config`` points at the same tree + ASSERT would use. It is mirrored rather than imported because + ``assert_ai.config`` pulls in PyYAML at module scope, which would break the + stdlib-only ``--suite`` path below. * Stdlib only, except for an optional PyYAML import used when ``--config`` is passed. ``--suite`` needs no third-party package at all. """ @@ -40,6 +49,7 @@ import argparse import json +import re import sys from pathlib import Path @@ -49,6 +59,10 @@ ARTIFACTS_DIR = "artifacts" TEST_SET_FILE = "test_set.jsonl" +# Mirrors assert_ai.config._SAFE_ID_RE, which itself must match the viewer's +# SAFE_ID_RE in artifacts.ts: /^[a-z0-9][a-z0-9._-]*$/i +_SAFE_ID_RE = re.compile(r"^[a-zA-Z0-9][a-zA-Z0-9._-]*$") + KINDS = ("prompt", "scenario") DEFAULT_COUNT = 3 @@ -57,6 +71,29 @@ class SmokeSliceError(Exception): """Raised for user-correctable problems, reported without a traceback.""" +def validate_suite_id(value: str) -> str: + """Reject suite ids that are not safe slugs. + + Mirrors ``assert_ai.config._validate_identifier``. ``suite`` is an + identifier, never a path: it is joined onto ``results_dir``, and it also + feeds the default output filename. Without this, an absolute path or a + ``..`` segment would read from — and write to — an arbitrary location. + """ + + if not value: + raise SmokeSliceError("suite must not be empty") + if len(value) > 255: + raise SmokeSliceError("suite exceeds maximum length of 255 characters") + if ".." in value: + raise SmokeSliceError("suite must not contain '..'") + if not _SAFE_ID_RE.match(value): + raise SmokeSliceError( + "suite must start with an alphanumeric character and contain only " + f"alphanumerics, dots, hyphens, or underscores; got: {value!r}" + ) + return value + + def _repo_root() -> Path: """Repository root, four levels up from .claude/skills/run-assert-eval/.""" @@ -83,22 +120,62 @@ def load_config(config_path: str | Path) -> dict: return raw +def _strip_artifact_root_prefix(path: Path, artifacts_root: Path) -> Path | None: + """Return the artifact-relative suffix for paths starting with an artifacts root. + + Mirrors ``assert_ai.config._strip_artifact_root_prefix``. + """ + + parts = path.parts + if not parts: + return None + if parts[0] not in {ARTIFACTS_DIR, artifacts_root.name}: + return None + if len(parts) == 1: + return Path() + return Path(*parts[1:]) + + def resolve_results_dir(raw: dict, *, root: Path | None = None) -> Path: - """Resolve ``results_dir`` the way assert_ai.config does.""" + """Resolve ``results_dir`` the way assert_ai.config does. + + Mirrors ``assert_ai.config._resolve_path(..., use_artifacts_root=True)``, + including its artifact-root prefix stripping. That stripping is what keeps + a config like ``artifacts_root: artifacts`` plus ``results_dir: + artifacts/custom`` resolving to ``/artifacts/custom`` rather than + ``/artifacts/artifacts/custom`` — the latter would send ``--config`` + looking in a tree ASSERT never writes to. + """ root = root or _repo_root() artifacts_root = Path(str(raw.get("artifacts_root") or ARTIFACTS_DIR)).expanduser() if not artifacts_root.is_absolute(): artifacts_root = (root / artifacts_root).resolve() + else: + artifacts_root = artifacts_root.resolve() results_dir_raw = raw.get("results_dir") if not results_dir_raw: return (artifacts_root / "results").resolve() - results_dir = Path(str(results_dir_raw)).expanduser() - if results_dir.is_absolute(): - return results_dir.resolve() - return (artifacts_root / results_dir).resolve() + candidate = Path(str(results_dir_raw)).expanduser() + if candidate.is_absolute(): + # Absolute paths are explicitly specified by the user; allow them, as + # assert_ai.config does. The suite-id validation and the suite_root + # containment check still bound everything opened underneath. + return candidate.resolve() + + artifact_relative = _strip_artifact_root_prefix(candidate, artifacts_root) + resolved = ( + artifacts_root / artifact_relative + if artifact_relative is not None + else artifacts_root / candidate + ).resolve() + if not _is_within(resolved, artifacts_root): + raise SmokeSliceError( + f"results_dir escapes its artifacts root ({artifacts_root}): {results_dir_raw!r}" + ) + return resolved def resolve_test_set(suite_root: Path) -> tuple[Path, str]: @@ -194,7 +271,17 @@ def build_slice( if count < 1: raise SmokeSliceError(f"count must be at least 1; got {count}") + # `suite` is an identifier, not a path. Validate before it is joined onto + # results_dir or interpolated into the default output filename. + suite = validate_suite_id(str(suite)) + + results_dir = Path(results_dir).expanduser().resolve() suite_root = (results_dir / suite).resolve() + if not _is_within(suite_root, results_dir): + raise SmokeSliceError( + f"suite_root escapes its expected root directory ({results_dir}): {suite_root}" + ) + source, resolved_via = resolve_test_set(suite_root) lines = source.read_text(encoding="utf-8").splitlines() diff --git a/.claude/skills/run-assert-eval/tests/test_smoke_slice.py b/.claude/skills/run-assert-eval/tests/test_smoke_slice.py index d0d9dd4e..b2c72b38 100644 --- a/.claude/skills/run-assert-eval/tests/test_smoke_slice.py +++ b/.claude/skills/run-assert-eval/tests/test_smoke_slice.py @@ -283,6 +283,84 @@ def test_unknown_kind_is_rejected(tmp_path): ss.build_slice(suite="demo-suite", results_dir=results_dir, kind="promt") +# --- suite is an identifier, not a path ------------------------------------- + + +def _plant_outside_suite(tmp_path: Path) -> tuple[Path, Path]: + """Create a readable test set outside results_dir, plus an empty results_dir.""" + + outside = tmp_path / "outside" + outside.mkdir(parents=True) + (outside / ss.TEST_SET_FILE).write_text( + json.dumps({"type": "prompt", "test_case_id": "leaked_001"}) + "\n", + encoding="utf-8", + ) + results_dir = tmp_path / "artifacts" / "results" + results_dir.mkdir(parents=True) + return results_dir, outside + + +def test_absolute_path_as_suite_is_rejected(tmp_path): + """An absolute --suite must not be read as a suite root outside results_dir.""" + + results_dir, outside = _plant_outside_suite(tmp_path) + + with pytest.raises(ss.SmokeSliceError) as exc: + ss.build_slice( + suite=str(outside), + results_dir=results_dir, + out_path=tmp_path / "smoke" / "x.jsonl", + ) + + assert "suite must" in str(exc.value) + assert not (tmp_path / "smoke" / "x.jsonl").exists() + + +def test_traversal_in_suite_is_rejected(tmp_path): + """`..` must not walk out of results_dir.""" + + results_dir, _ = _plant_outside_suite(tmp_path) + + with pytest.raises(ss.SmokeSliceError) as exc: + ss.build_slice( + suite="../../outside", + results_dir=results_dir, + out_path=tmp_path / "smoke" / "y.jsonl", + ) + + assert "'..'" in str(exc.value) + assert not (tmp_path / "smoke" / "y.jsonl").exists() + + +@pytest.mark.parametrize( + "suite", + ["", "-leading-hyphen", "has space", "has/slash", "has\\backslash", "a" * 256], +) +def test_unsafe_suite_ids_are_rejected(tmp_path, suite): + results_dir, _ = _plant_outside_suite(tmp_path) + + # Match the validation message specifically: a plain SmokeSliceError would + # also be raised further downstream ("suite root not found"), which would + # let this pass without any identifier validation at all. + with pytest.raises(ss.SmokeSliceError, match=r"suite (must|exceeds)"): + ss.build_slice( + suite=suite, + results_dir=results_dir, + out_path=tmp_path / "smoke" / "z.jsonl", + ) + + +def test_suite_id_validation_runs_before_any_read(tmp_path): + """A rejected suite must not leave a default output file behind either.""" + + results_dir, _ = _plant_outside_suite(tmp_path) + + with pytest.raises(ss.SmokeSliceError, match=r"suite must not contain"): + ss.build_slice(suite="../escape", results_dir=results_dir) + + assert not (tmp_path / ss.ARTIFACTS_DIR / "smoke").exists() + + # --- resolve_results_dir ---------------------------------------------------- @@ -307,6 +385,40 @@ def test_absolute_results_dir_is_used_as_is(tmp_path): assert resolved == absolute +def test_results_dir_with_artifacts_prefix_is_not_double_nested(tmp_path): + """Matches assert_ai.config: `artifacts/custom` resolves under artifacts_root once. + + ASSERT strips a leading artifacts-root segment before joining, so this config + resolves to /artifacts/custom. Double-nesting it to + /artifacts/artifacts/custom would send --config looking in a tree + ASSERT never writes to. + """ + + resolved = ss.resolve_results_dir( + {"artifacts_root": "artifacts", "results_dir": "artifacts/custom"}, root=tmp_path + ) + + assert resolved == (tmp_path / "artifacts" / "custom").resolve() + assert resolved != (tmp_path / "artifacts" / "artifacts" / "custom").resolve() + + +def test_results_dir_prefix_matches_a_renamed_artifacts_root(tmp_path): + """The stripped segment may be the artifacts_root's own name, not just 'artifacts'.""" + + resolved = ss.resolve_results_dir( + {"artifacts_root": "out", "results_dir": "out/custom"}, root=tmp_path + ) + + assert resolved == (tmp_path / "out" / "custom").resolve() + + +def test_results_dir_escaping_artifacts_root_is_rejected(tmp_path): + with pytest.raises(ss.SmokeSliceError) as exc: + ss.resolve_results_dir({"results_dir": "../../etc"}, root=tmp_path) + + assert "escapes" in str(exc.value) + + # --- CLI --------------------------------------------------------------------