diff --git a/CHANGELOG.md b/CHANGELOG.md index 82bd8e6..9b86a32 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -22,6 +22,46 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 placeholder `reasoning_content`: DeepSeek requires it on tool requests and ignores it otherwise. +### Changed + +- `evalshift doctor`'s `evalshift.yaml` row now ends a failure with "— run + `evalshift validate` for details". The row has room for the summary only + ("1 schema problem found"), which names neither the offending key nor the + fix; `validate` prints both. + +### Removed + +- **The top-level `slices:` key is gone from `evalshift.yaml`, and its + removal is breaking.** The block (`name`, `filter`, `applies_to`) was + validated and recorded in the run bundle, but analysis never read it: + slices have always come from example `tags`, one per distinct tag plus + `all`, so none of its fields renamed, filtered or scoped anything, and a + run reports the same slices without it. **Migration: delete the block.** + Per-slice budgets, the one thing it looked like it configured, go under + `migration_policy.slices`, keyed by tag. + + A config that still sets `slices:` now **fails to load** instead of being + quietly ignored, the same way `thresholds` has since 1.1.0, naming what + happened and the fix: + + ```text + `slices` was removed: it never had any effect. Slices come from example `tags` automatically (one per distinct tag, plus `all`). Delete it from evalshift.yaml; per-slice budgets go under migration_policy.slices, keyed by tag. + ``` + + `evalshift validate` prints it; `evalshift doctor` fails its config row + and points at `validate`. `version:` stays `1`, under the same config + version policy. The two shipped example configs that set the block no + longer do, and `tests/unit/test_docs_currency.py` now fails if any doc or + example shows a top-level `slices:` or `thresholds:` key. + + Hosted baselines are unaffected for every config that never set the key + (or set `slices: []`): the bundle's `evaluator_config` still carries + `"slices": []`, so `eval_config_hash` is byte-identical to what earlier + CLIs computed and existing baselines keep matching. Deleting a *non-empty* + block does change that hash, so runs pushed afterwards are not comparable + to baselines pushed before until the base branch pushes a run with the + edited config. + ### Fixed - Under SQLAlchemy 2.1, which fresh installs resolve (`sqlalchemy>=2.0`), a @@ -87,29 +127,27 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 places, and all of them now say what the CLI does. Nothing about CLI behaviour changed. The one that matters most concerns slices: the docs said a top-level `slices:` block picks examples by tag, names the slice, - and scopes it to prompts. It does none of that. The block is validated - (the reserved `overall` name is still rejected) and recorded in the run - bundle, but analysis never reads it. Slices come from example `tags`, one - per distinct tag plus `all`, and per-slice budgets are keyed by tag under - `migration_policy.slices`. The docs now say the block is not applied, and - so does the `SliceConfig` docstring, which described `filter` as a Python - expression. Imported agent traces (`traces import`) stay local; the bundle - carries only the replay's own tool-call trace, without tool results or - `model_call` events. The response cache serves only tool-less examples, - so every `run` of an agent suite is live and full price. `--resume` - hashes the suite's path, not its contents. `push ` uploads an - existing bundle as-is instead of rebuilding it, and `bundle` needs a git - SHA. `--policy-gate` also fails when no `migration_policy` is configured. - The `init` profile table had the wrong `model-upgrade` numbers and no - tool-divergence column. The multi-turn suite example failed to load - because it had no `tools`. The failure-label list was missing - `TOOL_GROUND_TRUTH_MISS`. Upstream model-call failures and evaluator - failures are handled the same way, as errored rows excluded from the - statistics. The GitHub Action docs gained `require-policy` and the other - missing inputs. `record_model_call` examples now pass the required - `tools=`. DOCS.md's header said version 1.0.1; a new check in - `tests/unit/test_docs_currency.py` keeps the version in DOCS.md and - llms-full.txt equal to the package's. + and scopes it to prompts. It did none of that. The block was validated and + recorded in the run bundle, but analysis never read it. Slices come from + example `tags`, one per distinct tag plus `all`, and per-slice budgets are + keyed by tag under `migration_policy.slices`. The docs now say so, and the + block itself is removed in this release (see Removed above). Imported + agent traces (`traces import`) stay local; the bundle carries only the + replay's own tool-call trace, without tool results or `model_call` events. + The response cache serves only tool-less examples, so every `run` of an + agent suite is live and full price. `--resume` hashes the suite's path, + not its contents. `push ` uploads an existing bundle as-is instead + of rebuilding it, and `bundle` needs a git SHA. `--policy-gate` also fails + when no `migration_policy` is configured. The `init` profile table had the + wrong `model-upgrade` numbers and no tool-divergence column. The + multi-turn suite example failed to load because it had no `tools`. The + failure-label list was missing `TOOL_GROUND_TRUTH_MISS`. Upstream + model-call failures and evaluator failures are handled the same way, as + errored rows excluded from the statistics. The GitHub Action docs gained + `require-policy` and the other missing inputs. `record_model_call` + examples now pass the required `tools=`. DOCS.md's header said version + 1.0.1; a new check in `tests/unit/test_docs_currency.py` keeps the version + in DOCS.md and llms-full.txt equal to the package's. ## [1.1.0] - 2026-09-19 diff --git a/DOCS.md b/DOCS.md index 60baea7..11331cb 100644 --- a/DOCS.md +++ b/DOCS.md @@ -207,7 +207,7 @@ init → doctor → run → evaluate → analy .json (if policy) ``` -- **`doctor`** validates local config and shows which provider keys are visible. Exit 1 only when an existing `evalshift.yaml` fails validation; missing keys are soft warnings. Its second row, `evalshift-sdk`, reports the SDK version the `evalshift` import name resolves to in this environment (`warn` when the SDK is missing or shadowed by an older CLI's leftover files; never a failure). It also reports the toolset each configured suite carries (or the flat `golden.jsonl`) and flags a suite whose examples carry more than one distinct toolset — legal (each example dispatches its own), but also the shape a wiring mistake takes. When a workflow under `.github/workflows/` uses the GitHub Action it adds a `ci pin` row: `ok` (`pinned to `) when CI installs this CLI version, `warn` when the pin is older, absent, or newer than the local CLI (see [Pin drift](#pin-drift)). When the config wires an `llm_judge` evaluator and names both `defaults.source_model` and `target_model`, a `judge family` row warns for every `judge_model` that resolves to the same provider as an arm (self-preference bias; `ok` "from a third family" otherwise, no row when either arm is unset) — advisory, never a failure; `validate` prints the same line and the report repeats it above the verdict (see [`evaluators.llm_judge`](docs/configuration.md#evaluatorsllm_judge)). +- **`doctor`** validates local config and shows which provider keys are visible. Exit 1 only when an existing `evalshift.yaml` fails validation — the row gives the problem count and points at `evalshift validate`, which prints each problem; missing keys are soft warnings. Its second row, `evalshift-sdk`, reports the SDK version the `evalshift` import name resolves to in this environment (`warn` when the SDK is missing or shadowed by an older CLI's leftover files; never a failure). It also reports the toolset each configured suite carries (or the flat `golden.jsonl`) and flags a suite whose examples carry more than one distinct toolset — legal (each example dispatches its own), but also the shape a wiring mistake takes. When a workflow under `.github/workflows/` uses the GitHub Action it adds a `ci pin` row: `ok` (`pinned to `) when CI installs this CLI version, `warn` when the pin is older, absent, or newer than the local CLI (see [Pin drift](#pin-drift)). When the config wires an `llm_judge` evaluator and names both `defaults.source_model` and `target_model`, a `judge family` row warns for every `judge_model` that resolves to the same provider as an arm (self-preference bias; `ok` "from a third family" otherwise, no row when either arm is unset) — advisory, never a failure; `validate` prints the same line and the report repeats it above the verdict (see [`evaluators.llm_judge`](docs/configuration.md#evaluatorsllm_judge)). - **`run`** parses prompts, validates every example against every prompt, estimates cost, then dispatches `(prompt × example × {source, target})` calls through an async orchestrator under a concurrency semaphore. Responses to tool-less examples are cached (tool-calling examples are always dispatched live — see [Response cache](#response-cache)); progress is checkpointed every 50 completions. - **`evaluate`** scores each (source, target) pair with the configured evaluators, one `EvalRecord` per pair × evaluator. Scoring runs under the same `defaults.concurrency` semaphore as `run`, and the embedding/judge calls it makes go through the same response cache. - **`analyze`** runs paired statistics per `(prompt, evaluator, slice)`, applies Benjamini–Hochberg FDR correction, classifies severities, and — when a `migration_policy` is configured — computes a pass/fail verdict. @@ -314,7 +314,6 @@ suites: {} | `prompts` | list, required, ≥1 | Prompt definitions (unique ids enforced) | | `defaults` | block | Run defaults, below | | `evaluators` | block | Evaluator configs, see [Evaluators](#evaluators) | -| `slices` | list | Validated and recorded in the bundle (and in its `eval_config_hash`), but **not applied** — slices come from example tags. See below | | `migration_policy` | block \| absent | Regression budgets, see [Migration policy](#migration-policy-and-ci-gating) | | `suites` | map | Named suites (`{name: {source: captured\|jsonl, path: ..., evaluators: ..., managed: true}}`); the block between the `>>> evalshift suites` markers is managed by `capture sync`. See [Per-suite evaluators](#per-suite-evaluators) | | `retention` | block | `max_runs_per_suite` (default 20, `0` disables), `run_ttl_days` (default off) | @@ -327,6 +326,14 @@ suites: {} Nothing replaced it. Delete the block; express any gate you meant by it as a `migration_policy` budget. +A top-level `slices` list was removed the same way. It was validated and recorded in the run bundle, but analysis never read it — slices come from example tags (see [Slices](#slices)) — so a config that still carries it fails to load too: + +```text +`slices` was removed: it never had any effect. Slices come from example `tags` automatically (one per distinct tag, plus `all`). Delete it from evalshift.yaml; per-slice budgets go under migration_policy.slices, keyed by tag. +``` + +Delete the block; a run reports the same slices without it. Hosted baselines are unaffected for any config that never set it (or set `slices: []`): the bundle's evaluator config still carries an empty `slices` list, so `eval_config_hash` does not move. Deleting a *non-empty* block does change that hash: runs pushed afterwards are not comparable to baselines pushed before, until the base branch pushes a run with the edited config. + ### `defaults` | Field | Default | Meaning | @@ -341,18 +348,18 @@ Nothing replaced it. Delete the block; express any gate you meant by it as a `mi | `max_tokens` | `4096` | Completion cap per call (per-prompt `prompts[].max_tokens` overrides). Truncated calls are excluded from the regression statistics | | `samples_per_example` | `1` (1–20) | Repeats each (prompt, example) this many times per model. Each sample pair is scored on its own; `scores.jsonl` keeps one row per example holding the mean over samples, with the per-sample scores and `delta_variance` under `metadata.samples`. Paired tests run over examples, so `n` is unchanged. Cost and calls multiply by it; the cache keys on the sample index so every sample is a live call | -### `slices` +### Slices -Slices come from the suite, not from this block: every distinct example `tag` becomes a slice under its own name, alongside the implicit `all` slice. Every configured evaluator is analysed once overall and once per slice. Per-slice budgets go under [`migration_policy.slices`](#migration-policy-and-ci-gating), keyed by the tag. +Slices come from the suite; there is nothing to configure. Every distinct example `tag` becomes a slice under its own name, alongside the implicit `all` slice, and every configured evaluator is analysed once overall and once per slice. Per-slice budgets go under [`migration_policy.slices`](#migration-policy-and-ci-gating), keyed by the tag: ```yaml -slices: # validated and recorded in the run bundle; NOT applied by analysis - - name: security - filter: security # a literal tag - applies_to: ["*"] # glob list of prompt ids +migration_policy: + slices: + security: # the example tag + max_overall_regression_rate: 0.0 ``` -The top-level `slices:` block still loads — it is validated and copied into the run bundle's evaluator config — but analysis does not read it today: `name`, `filter` and `applies_to` rename, filter and scope nothing, and a run reports the same slices with or without it. It is, however, part of the bundle's `eval_config_hash`, so editing or removing it breaks hosted baseline compatibility with earlier runs. `overall` is reserved — it names the run-level scope in the run bundle — and is rejected as a slice `name`, as an example tag, and as a `migration_policy.slices` key. +There is no top-level `slices:` key — it was removed (see [Top-level fields](#top-level-fields)). `overall` is reserved — it names the run-level scope in the run bundle — and is rejected as an example tag and as a `migration_policy.slices` key. Slices holding exactly the same examples are collapsed to one before any test runs — duplicates restate the same numbers as if they were independent findings and skew the Benjamini–Hochberg correction anti-conservatively (extra copies of a p-value shrink every adjusted p-value in the family, so results look more significant than they are). `all` and any slice named under `migration_policy.slices` always survive; otherwise the provenance tag `captured` (written by `capture promote`) loses to an ordinary tag, then alphabetical order decides. Drops are reported on the terminal and as `collapsed_slices` in `analysis.json`. See [docs/methodology.md](docs/methodology.md). @@ -881,7 +888,7 @@ EvalShift follows [Semantic Versioning](https://semver.org). From **1.0.0** onwa - The HTML report's markup, styling, and internal structure. Its *content* is described here; its DOM is not. - Console output wording, progress rendering, and log formatting. -**Config schema evolution** has its own rule, and it is deliberately not tied to the CLI's major version: `version:` in `evalshift.yaml` bumps only when a field is renamed, removed, or given a different meaning. Additive fields ride the CLI version instead. See [Config version policy](docs/configuration.md#config-version-policy) for what that requires of your CI pin. +**Config schema evolution** has its own rule, and it is deliberately not tied to the CLI's major version: `version:` in `evalshift.yaml` bumps only when a field is renamed or given a different meaning. Additive fields ride the CLI version instead, and so do removals that fail the load with a message naming the key. See [Config version policy](docs/configuration.md#config-version-policy) for what that requires of your CI pin. **Renames keep the old name.** When a command is renamed, the previous name stays registered as a hidden alias that still works — it stops being advertised, not accepted. `evalshift all` became `evalshift compare` in 1.0.0 and `all` still runs. Removing such an alias would itself be a breaking change, so it cannot happen inside a major version. diff --git a/README.md b/README.md index 76dfa4f..8a7e7f0 100644 --- a/README.md +++ b/README.md @@ -70,9 +70,11 @@ CI enforces a 90% coverage floor on the source. The CLI is published on PyPI as `evalshift`, the capture SDK as `evalshift-sdk`, and the hosted service runs at `api.evalshift.dev`. -The `evalshift.yaml` schema is versioned: `version: 1` changes only for a -breaking change — a field renamed, removed, or given new semantics — so configs -and CI pipelines keep working across releases. +The `evalshift.yaml` schema is versioned: `version: 1` changes only when a +field is renamed or given new semantics. A removed field does not bump it: a +config that still sets one fails to load with an error naming the key, so no +config is ever silently misread across releases. See +[Config version policy](docs/configuration.md#config-version-policy). ## Install diff --git a/docs/configuration.md b/docs/configuration.md index 5f69624..c679e71 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -18,7 +18,6 @@ migration_policy: {...} # optional local migration verdict policy prompts: [...] # required, at least one defaults: {...} # optional evaluators: {...} # optional (but at least one is needed for `evaluate`) -slices: [...] # optional; validated but not applied (see below) suites: {...} # optional named suites for `run --suite-name`, each with # its own optional `evaluators:` block retention: {...} # optional run-history pruning policy @@ -37,9 +36,10 @@ do *not* bump it; they ride on the CLI version instead. Nor does removing a field, as long as a config that still sets it **fails to load and says why**. The literal exists to catch silent misreadings, and an error naming the removed key is the opposite of silent — it cannot be mistaken -for a config that still works. `thresholds` left in 1.1.0 that way, and -`version` stayed `1`; bumping it would have forced an edit on every config, -including the majority that never set the key. +for a config that still works. `thresholds` left in 1.1.0 that way, and the +top-level `slices` list after it; `version` stayed `1` both times. Bumping it +would have forced an edit on every config, including the majority that never +set the key. Because unknown keys are rejected, that puts one rule on you: the CLI that *reads* a config must be at least as new as the CLI that *wrote* it. In @@ -90,6 +90,29 @@ A config that still sets the key now **fails to load**: The fix is to delete the block. If you were using it to express a gate, encode that as a `migration_policy` budget instead. +### Top-level `slices` was removed + +`evalshift.yaml` also used to accept a top-level `slices:` list (`name`, +`filter`, `applies_to`). It was validated and recorded in the run bundle, but +analysis never read it: slices come from example `tags` (see +[Slices](#slices)), so none of its fields renamed, filtered or scoped anything. + +A config that still sets the key now **fails to load**: + +```text +`slices` was removed: it never had any effect. Slices come from example `tags` automatically (one per distinct tag, plus `all`). Delete it from evalshift.yaml; per-slice budgets go under migration_policy.slices, keyed by tag. +``` + +The fix is to delete the block; a run reports the same slices without it. For +per-slice budgets, use [`migration_policy.slices`](#migration_policy), keyed +by tag. + +Hosted baselines are unaffected for any config that never set the key (or set +`slices: []`): the bundle's evaluator config still carries an empty `slices` +list, so `eval_config_hash` does not move. Deleting a *non-empty* block does +change that hash: runs pushed afterwards are not comparable to baselines pushed +before, until the base branch pushes a run with the edited config. + ## `migration_policy` Optional local regression budget used by `evalshift analyze`, @@ -616,31 +639,19 @@ bias) and produces strict-JSON `{"winner": "A"|"B"|"tie", "reason": error preserved. `blocking` defaults to `true` in the library but `init` writes `false` — see [`blocking`](#blocking-every-evaluator) for why. -## `slices` - -Slices come from the suite, not from this block: every distinct example -`tag` becomes a slice under its own name, alongside the implicit `"all"` -slice, and each is analysed separately. Per-slice budgets go under -[`migration_policy.slices`](#migration_policy), keyed by the tag. - -The top-level `slices:` list is still accepted — it is validated and copied -into the run bundle's evaluator config — but analysis does not read it today: -`name`, `filter` and `applies_to` rename, filter and scope nothing, and a run -reports the same slices with or without it. It is, however, part of the -bundle's `eval_config_hash`, so editing or removing it breaks hosted baseline -compatibility with earlier runs. - -`overall` is reserved and cannot be used as a slice `name`, as an example tag, -or as a `migration_policy.slices` key. It names the run-level scope in the run -bundle, so a slice by that name would shadow the whole-run numbers wherever the -two are rendered together. All three spellings are rejected when the config or -suite loads. - -| Field | Type | Required | Description | -| ------------- | ------ | -------- | ----------- | -| `name` | string | yes | Slice name. Not applied: reports name each slice after its tag. | -| `filter` | string | yes | A literal tag. Not applied: every tag is already its own slice. | -| `applies_to` | list | optional | Glob list of prompt ids (default `["*"]`). Not applied. | +## Slices + +Slices come from the suite; there is nothing to configure. Every distinct +example `tag` becomes a slice under its own name, alongside the implicit +`"all"` slice, and each is analysed separately. Per-slice budgets go under +[`migration_policy.slices`](#migration_policy), keyed by the tag. There is no +top-level `slices:` key — it was +[removed](#top-level-slices-was-removed). + +`overall` is reserved and cannot be used as an example tag or as a +`migration_policy.slices` key. It names the run-level scope in the run bundle, +so a slice by that name would shadow the whole-run numbers wherever the two are +rendered together. Both spellings are rejected when the config or suite loads. Slices with identical membership are collapsed to one before analysis, so duplicate tags cannot inflate the Benjamini–Hochberg correction. `all` and any diff --git a/docs/getting-started.md b/docs/getting-started.md index ed686a8..8f79b1b 100644 --- a/docs/getting-started.md +++ b/docs/getting-started.md @@ -90,8 +90,8 @@ You'll see a short table: * Green ✓ — check passes. * Yellow ✗ — informational warning (e.g. an unset API key, or no `evalshift.yaml` here yet). Doctor still exits 0. -* Red ✗ — hard failure (e.g. an `evalshift.yaml` that doesn't validate). - Doctor exits 1. +* Red ✗ — hard failure (e.g. an `evalshift.yaml` that doesn't validate; + run `evalshift validate` to see each problem). Doctor exits 1. The second row, `evalshift-sdk`, confirms that `import evalshift` in this environment is the capture SDK — yellow when it is missing or shadowed by an diff --git a/docs/hosted.md b/docs/hosted.md index f6831be..57d7a73 100644 --- a/docs/hosted.md +++ b/docs/hosted.md @@ -292,7 +292,7 @@ The bundle itself contains: | `aggregate`, `analysis`, `decision`, `economics` | Pass/fail counts, statistical comparisons, the migration verdict, and per-role token/cost/latency rollups. Numbers and verdict labels, not content. `decision.policy` is the resolved `migration_policy` this run's verdict was computed under — every top-level budget with its default applied, plus `slices` — or `null` when no `migration_policy` is configured. It is what lets the hosted gate check a pull request against the exact budgets the verdict used, instead of a separate policy configured elsewhere. | | `methodology_notes` | The model ids and the statistical-contract sentences shown in every report. | | `insights` | The machine-written run narrative, when one was generated. It is prose *about* your run and can paraphrase or quote the regressions it summarizes. | -| `evaluator_config` | Config version; the prompt list **metadata only** — prompt names, file paths, and variable names, with every prompt body replaced by a `content_hash`; the whole `defaults` block (model ids, concurrency, cache flag, cost ceiling, max_tokens, samples_per_example); slice definitions (recorded, not applied); and the full evaluators block — which includes each `llm_judge` entry's `criterion_prompt` text, so keep judge criteria free of secrets. | +| `evaluator_config` | Config version; the prompt list **metadata only** — prompt names, file paths, and variable names, with every prompt body replaced by a `content_hash`; the whole `defaults` block (model ids, concurrency, cache flag, cost ceiling, max_tokens, samples_per_example); an always-empty `slices` list (a legacy key, kept so `eval_config_hash` does not change — the top-level `slices:` config key was removed); and the full evaluators block — which includes each `llm_judge` entry's `criterion_prompt` text, so keep judge criteria free of secrets. | | `dataset_snapshot` | Suite path, example count, slice names, and one `examples_hash`. **No example content.** | ### What never leaves your machine diff --git a/examples/agent/evalshift.yaml b/examples/agent/evalshift.yaml index 72f454d..2878a9b 100644 --- a/examples/agent/evalshift.yaml +++ b/examples/agent/evalshift.yaml @@ -37,18 +37,5 @@ evaluators: severity_floor: high # Slices come from each example's `tags` automatically: one per distinct tag, -# plus `all`. This block is validated but not applied by analysis today -- -# the run reports the same slices without it. It is still part of the hosted -# bundle's eval_config_hash, so editing it breaks baseline compatibility with -# earlier hosted runs. -slices: - - name: security - filter: security - - name: routine - filter: routine - - name: refund - filter: refund - - name: customer_lookup - filter: customer_lookup - - name: text_only - filter: text_only +# plus `all` -- there is nothing to configure. Per-slice budgets go under +# `migration_policy.slices`, keyed by tag. diff --git a/examples/simple/evalshift.yaml b/examples/simple/evalshift.yaml index ad31554..9386154 100644 --- a/examples/simple/evalshift.yaml +++ b/examples/simple/evalshift.yaml @@ -35,12 +35,5 @@ evaluators: max_chars: 200 # Slices come from each example's `tags` automatically: one per distinct tag, -# plus `all`. This block is validated but not applied by analysis today -- -# the run reports the same slices without it. It is still part of the hosted -# bundle's eval_config_hash, so editing it breaks baseline compatibility with -# earlier hosted runs. -slices: - - name: formal - filter: formal - - name: casual - filter: casual +# plus `all` -- there is nothing to configure. Per-slice budgets go under +# `migration_policy.slices`, keyed by tag. diff --git a/llms-full.txt b/llms-full.txt index 4c6bfae..35a089e 100644 --- a/llms-full.txt +++ b/llms-full.txt @@ -139,8 +139,9 @@ evalshift init [-f/--force] [-d/--directory DIR] [--ci] [--wire-agents/--no-wire cost-reduction: .02/0/.97/.01/.02/.05/.30 | local-model: .05/0/.90/.02/.05/.00/.50 quantization: .02/0/.97/.005/.02/.00/.20 | provider-switch: .03/0/.95/.01/.03/.20/.40 evalshift doctor - Env/config check. Exit 1 ONLY when an existing evalshift.yaml fails validation; missing API - keys are soft warnings (exit 0). + Env/config check. Exit 1 ONLY when an existing evalshift.yaml fails validation (the row shows + the problem count + "run `evalshift validate` for details"; validate prints each problem); + missing API keys are soft warnings (exit 0). Row 2 `evalshift-sdk`: ok (" (import name `evalshift`)"); warn (exit 0) when the SDK is not installed, fails to import, or the name resolves to something else (an older CLI's leftover files, a local evalshift/ directory). @@ -426,7 +427,8 @@ NOT PUBLIC -- may change in any release, do not build on it: - Console wording, progress rendering and log formatting. Config schema version is NOT tied to the CLI major: `version:` in evalshift.yaml bumps only when -a field is renamed, removed or redefined; additive fields ride the CLI version. Consequence: the +a field is renamed or redefined; additive fields, and removals that fail the load with a message +naming the key, ride the CLI version. Consequence: the CLI that READS a config must be at least as new as the CLI that WROTE it -- which is what the CI pin check enforces. @@ -458,7 +460,16 @@ Strict Pydantic, extra="forbid" everywhere — unknown keys fail at load. `thres REMOVED and rejected by name (it was free-form and gated nothing): a config still carrying it fails to load with "`thresholds` was removed: it was free-form and gated nothing. Delete it from evalshift.yaml; migration_policy is the single source of truth for gating." Nothing replaced it -— delete the block, express any gate you meant by it as a migration_policy budget. +— delete the block, express any gate you meant by it as a migration_policy budget. A top-level +`slices:` list is REMOVED the same way (it was validated and recorded, but analysis never read +it): a config still carrying it fails to load with "`slices` was removed: it never had any +effect. Slices come from example `tags` automatically (one per distinct tag, plus `all`). Delete +it from evalshift.yaml; per-slice budgets go under migration_policy.slices, keyed by tag." Delete +the block; runs report the same slices without it. Hosted baselines: the bundle's +evaluator_config still carries a constant `"slices": []`, so eval_config_hash is unchanged for a +config that never set the key (or set `slices: []`); deleting a NON-EMPTY block changes the hash, +so runs pushed afterwards are not comparable to earlier baselines until the base branch pushes a +run with the edited config. version: 1 # literal 1, default 1 (optional) project: str|null # hosted slug, regex ^[a-z0-9-]+/[a-z0-9-]+$ @@ -587,16 +598,6 @@ evaluators: # every evaluator config also takes blocking check_missing_verification: bool = true verification_tools: [str] dangerous_tools: [str] -slices: # VALIDATED + recorded in the bundle, NOT APPLIED: - - name: str # name/filter/applies_to rename, filter and scope - filter: str # nothing today (filter is a literal tag). Slices - applies_to: ["*"] # come from example tags instead: one per distinct - # tag, plus "all". "overall" is RESERVED (run-level - # scope in the bundle): rejected as slice name, - # example tag, and policy slice key. Still part of - # the bundle's eval_config_hash: editing or removing - # it breaks hosted baseline compatibility with - # earlier runs. suites: # managed block; capture sync rewrites between : # ">>> evalshift suites" markers source: captured|jsonl = captured @@ -1003,9 +1004,10 @@ content leaves the machine. summarizes. - evaluator_config: config version; prompt list METADATA (names, file paths, variable names — every prompt body replaced by content_hash); the whole defaults block (model ids, - concurrency, cache, max_cost_usd, max_tokens, samples_per_example); slices (the top-level - block, recorded though not applied); full evaluators block INCLUDING each llm_judge - criterion_prompt text (keep judge criteria free of secrets). + concurrency, cache, max_cost_usd, max_tokens, samples_per_example); slices (always [] -- + a legacy key kept so eval_config_hash stays stable; the top-level config key is removed); + full evaluators block INCLUDING each llm_judge criterion_prompt text (keep judge criteria + free of secrets). - dataset_snapshot: suite path, size, slice names, examples_hash. No example content. - NEVER uploads: provider API keys; the hosted token (never inside a bundle); prompt bodies / system prompts (manual content -> content_hash; python_string bodies never enter the config); @@ -1116,10 +1118,10 @@ content leaves the machine. cases, so dedup spans sync runs. - Slices: every distinct example tag is a slice under its own name, plus "all"; each evaluator is analyzed overall AND per slice; per-slice budgets go under migration_policy.slices keyed by the - tag and inherit unset fields from the top level. The top-level slices: block is validated and - recorded in the bundle but NOT applied: name/filter/applies_to have no effect on analysis - today. It IS part of the bundle's eval_config_hash, so editing or removing it breaks hosted - baseline compatibility with earlier runs. + tag and inherit unset fields from the top level. Nothing to configure: there is no top-level + slices key (REMOVED -- a config still setting it fails to load; see the schema section). + "overall" is RESERVED (run-level scope in the bundle): rejected as an example tag and as a + migration_policy.slices key. - Slice dedup: slices holding identical (prompt, evaluator, example) triples collapse to one before any test runs. Duplicates restate the same finding AND skew BH-FDR anti-conservatively: k extra copies of a p-value raise both n and the rank the copies reach, and (n+k)/(r+k) < n/r, @@ -1197,7 +1199,7 @@ prompts: path: prompts.py variable: AGENT_SYSTEM_PROMPT variables: [query] -# No slices: block needed -- the "security" tag on the rows below is already a slice. +# No slice config -- the "security" tag on the rows below is already a slice. # >>> evalshift suites (managed by `evalshift capture sync`) >>> suites: briefing: # tool-free rows -> no evaluators: block @@ -1326,6 +1328,8 @@ retries them (cache serves the tool-less successes; tool-calling examples are al Config rejected -> extra="forbid": check for typo'd keys; error names the exact path. "`thresholds` was removed" -> the key is gone from evalshift.yaml; delete it. It gated nothing; migration_policy is the single source of truth. Nothing replaced it. +"`slices` was removed" -> the top-level key is gone from evalshift.yaml; delete it. It never had +an effect: slices come from example tags. Per-slice budgets: migration_policy.slices, keyed by tag. python_string rejected -> the variable isn't a plain string literal; use detection: manual. Captures not found -> capture commands read .evalshift/captures/ under the CWD (or $EVALSHIFT_DIR); run from the directory your agent wrote to. diff --git a/src/evalshift_cli/analysis/slicing.py b/src/evalshift_cli/analysis/slicing.py index c312b50..d3362ce 100644 --- a/src/evalshift_cli/analysis/slicing.py +++ b/src/evalshift_cli/analysis/slicing.py @@ -1,8 +1,10 @@ """Group evaluation records into slices for analysis. -A *slice* is a named subset of suite examples — typically defined by a -tag in ``evalshift.yaml``'s ``slices:`` block. The implicit ``"all"`` -slice always exists and contains every example. +A *slice* is a named subset of suite examples: one per distinct example +tag, named after the tag. The implicit ``"all"`` slice always exists and +contains every example. There is nothing to configure — ``evalshift.yaml`` +has no slice definitions; per-slice budgets are keyed by tag under +``migration_policy.slices``. The output of :func:`build_slices` is a mapping from slice name to a list of ``(prompt_id, evaluator_name, kind, example_id, source_score, @@ -82,7 +84,6 @@ def build_slices( *, records: list[EvalRecord], suite: Suite, - tag_to_slice: dict[str, str] | None = None, coverage: Sequence[EvaluatorCoverage] = (), ) -> dict[str, list[SlicedScore]]: """Group evaluation records into slices keyed by slice name. @@ -90,10 +91,6 @@ def build_slices( Args: records: Every :class:`EvalRecord` from ``scores.jsonl``. suite: The loaded suite, used to look up an example's ``tags``. - tag_to_slice: Mapping from a configured slice's tag (the value - of ``filter`` in MVP, simplified to a literal tag) to the - slice name surfaced in reports. ``None`` falls back to a - tag-name == slice-name identity mapping. coverage: The run's per-evaluator coverage. Only its unmeasured pairs are read, and only to *seed* slice names: a slice whose every row was a non-measurement still has to exist here, or it @@ -121,12 +118,12 @@ def build_slices( delta=rec.delta, kind=rec.kind, ) - for slice_name in _slices_of(rec.example_id, by_id, tag_to_slice): + for slice_name in _slices_of(rec.example_id, by_id): out[slice_name].append(sliced) for entry in coverage: for pair in entry.unmeasured: - for slice_name in _slices_of(pair.example_id, by_id, tag_to_slice): + for slice_name in _slices_of(pair.example_id, by_id): out.setdefault(slice_name, []) return dict(out) @@ -136,7 +133,6 @@ def build_unmeasured( *, coverage: Sequence[EvaluatorCoverage], suite: Suite, - tag_to_slice: dict[str, str] | None = None, ) -> UnmeasuredCounts: """Count, per slice and evaluator, the pairs that produced no row. @@ -148,7 +144,6 @@ def build_unmeasured( Args: coverage: The run's per-evaluator coverage, from ``state.json``. suite: The loaded suite, used to look up an example's ``tags``. - tag_to_slice: As :func:`build_slices`. Returns: ``{slice_name: {ComparisonKey: count}}``, empty when every @@ -162,26 +157,21 @@ def build_unmeasured( for entry in coverage: for pair in entry.unmeasured: key = (pair.prompt_id, entry.evaluator_name, entry.kind) - for slice_name in _slices_of(pair.example_id, by_id, tag_to_slice): + for slice_name in _slices_of(pair.example_id, by_id): out[slice_name][key] += 1 return {name: dict(counts) for name, counts in out.items()} -def _slices_of( - example_id: str, - by_id: dict[str, SuiteExample], - tag_to_slice: dict[str, str] | None, -) -> list[str]: - """Every slice an example belongs to, ``"all"`` first. +def _slices_of(example_id: str, by_id: dict[str, SuiteExample]) -> list[str]: + """Every slice an example belongs to, ``"all"`` first, then one per tag. - An example the suite no longer carries lands in ``"all"`` only — the - same fate a record for it already had. + A tag *is* its slice's name. An example the suite no longer carries lands + in ``"all"`` only — the same fate a record for it already had. """ names = [ALL_SLICE] example = by_id.get(example_id) if example is not None: - for tag in example.tags: - names.append(tag_to_slice.get(tag, tag) if tag_to_slice else tag) + names.extend(example.tags) return names diff --git a/src/evalshift_cli/cli/commands/doctor.py b/src/evalshift_cli/cli/commands/doctor.py index 995895c..81a1105 100644 --- a/src/evalshift_cli/cli/commands/doctor.py +++ b/src/evalshift_cli/cli/commands/doctor.py @@ -229,10 +229,12 @@ def _config_check(cwd: Path) -> CheckResult: try: cfg = load_config(cfg_path) except ConfigError as exc: + # The row has room for the summary ("1 schema problem found") but not + # the per-field reasons, so point at the command that prints them. return CheckResult( name=CONFIG_FILENAME, status="fail", - detail=exc.summary, + detail=f"{exc.summary} — run `evalshift validate` for details", ) n = len(cfg.prompts) return CheckResult( diff --git a/src/evalshift_cli/config/models.py b/src/evalshift_cli/config/models.py index f898c6a..d1aef5f 100644 --- a/src/evalshift_cli/config/models.py +++ b/src/evalshift_cli/config/models.py @@ -34,6 +34,13 @@ ) """Error text for a config that still carries the removed ``thresholds`` block.""" +_REMOVED_SLICES_MESSAGE = ( + "`slices` was removed: it never had any effect. Slices come from example " + "`tags` automatically (one per distinct tag, plus `all`). Delete it from " + "evalshift.yaml; per-slice budgets go under migration_policy.slices, keyed by tag." +) +"""Error text for a config that still carries the removed top-level ``slices`` block.""" + class _StrictModel(BaseModel): """Base for every config model: forbid extra keys, validate on assignment.""" @@ -412,36 +419,6 @@ def tool_evaluator_names(self) -> frozenset[str]: ) -class SliceConfig(_StrictModel): - """One entry of the top-level ``slices:`` block. - - The block is validated (the reserved ``overall`` name is rejected) and - recorded in the run bundle, but analysis does not read it today: slices - come from example ``tags`` -- one per distinct tag, plus ``all`` -- and - per-slice budgets are keyed by tag under ``migration_policy.slices``. - None of the fields below renames, filters, or scopes anything. - - Attributes: - name: Slice name. - filter: A literal tag, not an expression. - applies_to: Glob list of prompt IDs. - """ - - name: str = Field(min_length=1) - filter: str = Field(min_length=1) - applies_to: list[str] = Field(default_factory=lambda: ["*"]) - - @model_validator(mode="after") - def _reject_reserved_name(self) -> Self: - """``overall`` is the run-level scope in the bundle, never a slice.""" - if self.name == RESERVED_SLICE_NAME: - raise ValueError( - f"slice name {RESERVED_SLICE_NAME!r} is reserved: it names the run-level " - "scope in the run bundle -- pick another name", - ) - return self - - class SliceMigrationPolicy(_StrictModel): """Per-slice migration budget overrides. @@ -678,7 +655,6 @@ class EvalShiftConfig(_StrictModel): prompts: list[PromptDefinition] = Field(min_length=1) defaults: Defaults = Field(default_factory=Defaults) evaluators: EvaluatorsConfig = Field(default_factory=EvaluatorsConfig) - slices: list[SliceConfig] = Field(default_factory=list) migration_policy: MigrationPolicy | None = None # Named suites (e.g. promoted captures) resolvable via `run --suite-name`. # Empty by default so every pre-existing config stays valid. @@ -731,14 +707,24 @@ def evaluators_for(self, suite_name: str | None) -> EvaluatorsConfig: def _reject_removed_fields(cls, data: Any) -> Any: """Name the fields that were removed instead of calling them typos. - ``extra="forbid"`` already rejects ``thresholds``, but it says "Extra - inputs are not permitted" — which reads as a misspelling and sends the - reader hunting for the correct name of a field that is gone. Runs - before validation because a forbidden extra never reaches an - ``after`` validator. + ``extra="forbid"`` already rejects ``thresholds`` and ``slices``, but + it says "Extra inputs are not permitted" — which reads as a + misspelling and sends the reader hunting for the correct name of a + field that is gone. Runs before validation because a forbidden extra + never reaches an ``after`` validator. """ - if isinstance(data, dict) and "thresholds" in data: - raise ValueError(_REMOVED_THRESHOLDS_MESSAGE) + if isinstance(data, dict): + # Both at once, so deleting one does not uncover the other next run. + messages = [ + message + for key, message in ( + ("thresholds", _REMOVED_THRESHOLDS_MESSAGE), + ("slices", _REMOVED_SLICES_MESSAGE), + ) + if key in data + ] + if messages: + raise ValueError(" ".join(messages)) return data @model_validator(mode="after") @@ -760,7 +746,6 @@ def _check_unique_prompt_ids(self) -> Self: "LLMJudgeConfig", "PromptDefinition", "SemanticEvaluatorConfig", - "SliceConfig", "StructuralEvaluatorConfig", "SuiteEvaluatorsOverride", "SuiteSource", diff --git a/src/evalshift_cli/hosted/bundle.py b/src/evalshift_cli/hosted/bundle.py index 8b6af4f..f7e7996 100644 --- a/src/evalshift_cli/hosted/bundle.py +++ b/src/evalshift_cli/hosted/bundle.py @@ -269,7 +269,15 @@ def _evaluator_config_snapshot( "prompts": dumped["prompts"], "defaults": dumped["defaults"], "evaluators": evaluators.model_dump(mode="json"), - "slices": dumped["slices"], + # Legacy constant. This key used to carry the top-level ``slices:`` + # block, which was removed from evalshift.yaml because nothing ever + # read it; a config that still sets it no longer loads. The key stays, + # always ``[]``, because ``eval_config_hash`` is computed over this + # whole dict and the server pairs a run with its baseline only on an + # equal hash: dropping it would change the hash of every config -- + # including the ones that never set ``slices:``, for which ``[]`` is + # exactly what was hashed before -- and orphan every hosted baseline. + "slices": [], } @@ -316,10 +324,10 @@ def _wire_evaluator_config(full: dict[str, Any]) -> dict[str, Any]: Same contract as ``_wire_dataset_snapshot``: ``eval_config_hash`` is computed over the full snapshot, inline prompt bodies included, and each non-null ``prompts[].content`` ships as a ``content_hash`` instead of the - text. Everything else — evaluators, defaults, slices — is methodology, not - content, and ships as-is. A ``python_string`` prompt's body never entered - the config at all (its dump carries ``path`` + ``variable``), so only - ``manual`` prompts have anything to strip. + text. Everything else — evaluators, defaults, the legacy empty ``slices`` + — is methodology, not content, and ships as-is. A ``python_string`` + prompt's body never entered the config at all (its dump carries ``path`` + + ``variable``), so only ``manual`` prompts have anything to strip. """ prompts: list[dict[str, Any]] = [] for prompt in full["prompts"]: diff --git a/src/evalshift_cli/suite/models.py b/src/evalshift_cli/suite/models.py index 392abe3..113f4d3 100644 --- a/src/evalshift_cli/suite/models.py +++ b/src/evalshift_cli/suite/models.py @@ -158,8 +158,9 @@ class SuiteExample(_StrictModel): inputs: Mapping of template-variable name to value. The orchestrator substitutes these into each prompt template via :func:`evalshift_cli.utils.templating.render`. - tags: Optional labels used by :class:`evalshift_cli.config.models.SliceConfig` - to group examples for slice-level statistical analysis. An + tags: Optional labels that group examples for slice-level + statistical analysis: each distinct tag becomes a slice of the + same name (see :mod:`evalshift_cli.analysis.slicing`). An example may carry multiple tags. expected: Optional reference output for evaluators that take an "expected" answer (most evaluators in the MVP do not). @@ -268,8 +269,8 @@ class SuiteExample(_StrictModel): def _reject_reserved_slice_tag(self) -> Self: """Refuse a tag that would become the reserved ``overall`` slice. - Tags are slice names: ``analysis.slicing._slices_of`` maps an untranslated - tag straight through. The bundle contract reserves ``overall`` for the + Tags are slice names: ``analysis.slicing._slices_of`` maps each tag + straight through. The bundle contract reserves ``overall`` for the run-level scope, so the suite is where the collision has to be caught -- the alternative is a full run followed by a rejection at finalize. """ diff --git a/src/evalshift_cli/suite/tags.py b/src/evalshift_cli/suite/tags.py index d3f5501..5e5d4ea 100644 --- a/src/evalshift_cli/suite/tags.py +++ b/src/evalshift_cli/suite/tags.py @@ -27,9 +27,9 @@ `decision.overall` is the whole-run summary and `BudgetResult.scope` defaults to `"overall"`, so a slice by that name shadows the run's own numbers wherever the two render side by side. `BUNDLE_SPEC.md` has always said so; the server enforces it at -finalize. A slice name reaches a bundle from a suite tag, from `SliceConfig.name`, or -from a `migration_policy.slices` key, and all three refuse it — the literal lives in -this module because it is the only one all three can import without a cycle. +finalize. A slice name reaches a bundle from a suite tag or from a +`migration_policy.slices` key, and both refuse it — the literal lives in this module +because it is the only one both can import without a cycle. """ __all__ = ["CAPTURED_TAG", "PROVENANCE_TAGS", "RESERVED_SLICE_NAME"] diff --git a/tests/integration/test_validate_command.py b/tests/integration/test_validate_command.py index 6d264d4..6fb147c 100644 --- a/tests/integration/test_validate_command.py +++ b/tests/integration/test_validate_command.py @@ -80,6 +80,34 @@ def test_missing_config_exits_one( assert result.exit_code == 1 assert "Invalid config" in result.stdout + @pytest.mark.parametrize( + ("block", "removed"), + [ + ("thresholds:\n pass_rate_min: 0.9\n", "`thresholds` was removed"), + ("slices:\n - {name: refunds, filter: refunds}\n", "`slices` was removed"), + ], + ids=["thresholds", "slices"], + ) + def test_a_removed_key_is_named_not_called_a_typo( + self, + monkeypatch: pytest.MonkeyPatch, + tmp_path: Path, + block: str, + removed: str, + ) -> None: + """Both retired keys fail the same way: the panel names the removal.""" + (tmp_path / "evalshift.yaml").write_text( + "prompts:\n - {id: a, detection: manual, content: hi}\n" + block, + encoding="utf-8", + ) + monkeypatch.chdir(tmp_path) + # Wide enough that Rich does not wrap the message inside the panel. + result = runner.invoke(app, ["validate"], env={"COLUMNS": "400"}) + assert result.exit_code == 1 + assert "Invalid config" in result.stdout + assert removed in result.stdout + assert "Extra inputs are not permitted" not in result.stdout + def test_missing_suite_exits_one( self, monkeypatch: pytest.MonkeyPatch, diff --git a/tests/unit/test_bundle_shape.py b/tests/unit/test_bundle_shape.py index cf625bf..7689b3a 100644 --- a/tests/unit/test_bundle_shape.py +++ b/tests/unit/test_bundle_shape.py @@ -713,6 +713,67 @@ def test_dataset_hash_is_still_content_derived( assert snapshot_before["examples_hash"] != snapshot_after["examples_hash"] +class TestEvaluatorConfigKeepsTheLegacySlicesKey: + """Removing the top-level ``slices:`` key must not move ``eval_config_hash``. + + The server pairs a run with its baseline only on an equal + ``eval_config_hash``, and that hash is computed over the whole + ``evaluator_config`` snapshot — which has always carried a ``"slices"`` + key. Dropping the key would change the hash of every config, including + the ones that never set ``slices:``, and orphan every hosted baseline. + """ + + _CONFIG = """ + version: 1 + project: acme/model-migration + prompts: + - id: greet + detection: manual + content: "Hello {name}" + variables: [name] + defaults: + source_model: gemini/gemini-2.5-flash + target_model: gemini/gemini-3.1-flash-lite-preview + evaluators: + structural: + - type: length + min_chars: 1 + llm_judge: + - criterion_name: tone + criterion_prompt: Which reply is friendlier? + migration_policy: + max_overall_regression_rate: 0.10 + slices: + checkout: + max_overall_regression_rate: 0.05 + """ + + #: ``manifest.eval_config_hash`` for :attr:`_CONFIG`, computed by the CLI at + #: origin/main af4e6fe — the last commit that still accepted ``slices:``. + #: If this changes, every hosted baseline stops matching its next run. + _HASH_BEFORE_THE_REMOVAL = ( + "sha256:37936052974aecda1876fb406cb98a60c30b91b403b5ca59f6b53f0f1c1827e5" + ) + + def _bundle(self, run_fixture: RunFixture) -> dict[str, Any]: + run_fixture.config.write_text(self._CONFIG, encoding="utf-8") + return _load(run_fixture.build().path) + + def test_eval_config_hash_of_a_slices_less_config_is_unchanged( + self, run_fixture: RunFixture + ) -> None: + manifest = self._bundle(run_fixture)["manifest"] + assert isinstance(manifest, dict) + assert manifest["eval_config_hash"] == self._HASH_BEFORE_THE_REMOVAL + + def test_evaluator_config_still_ships_an_empty_slices_list( + self, run_fixture: RunFixture + ) -> None: + config = self._bundle(run_fixture)["evaluator_config"] + assert isinstance(config, dict) + assert config["slices"] == [] + + def _rewrite_suite_history(suite_path: Path, system_prompt: str) -> None: """Give every suite example a conversation prefix carrying ``system_prompt``.""" rows = [ diff --git a/tests/unit/test_config_loader.py b/tests/unit/test_config_loader.py index 1d1a5de..9cb2ade 100644 --- a/tests/unit/test_config_loader.py +++ b/tests/unit/test_config_loader.py @@ -88,9 +88,6 @@ def test_full_config_round_trip(self, tmp_path: Path) -> None: llm_judge: - criterion_name: factuality criterion_prompt: Which output preserves more factual detail? - slices: - - name: long - filter: "len(conversation) > 1000" """, ) cfg = load_config(path) @@ -98,7 +95,6 @@ def test_full_config_round_trip(self, tmp_path: Path) -> None: assert cfg.defaults.max_cost_usd == 25.0 assert cfg.evaluators.semantic is not None assert len(cfg.evaluators.llm_judge) == 1 - assert cfg.slices[0].name == "long" # --------------------------------------------------------------------------- @@ -233,6 +229,25 @@ def test_removed_thresholds_key_explains_the_removal(self, tmp_path: Path) -> No assert "`thresholds` was removed" in rendered assert "migration_policy is the single source of truth" in rendered + def test_removed_slices_key_explains_the_removal(self, tmp_path: Path) -> None: + path = _write( + tmp_path, + """ + prompts: + - id: a + detection: manual + content: hi + slices: + - name: refunds + filter: refunds + """, + ) + with pytest.raises(ConfigError) as info: + load_config(path) + rendered = info.value.format_plain() + assert "`slices` was removed" in rendered + assert "migration_policy.slices" in rendered + def test_multiple_errors_collected(self, tmp_path: Path) -> None: path = _write( tmp_path, @@ -311,3 +326,29 @@ class TestFormatLoc: ) def test_format(self, loc: tuple[int | str, ...], expected: str) -> None: assert _format_loc(loc) == expected + + +_EXAMPLES_DIR = Path(__file__).resolve().parents[2] / "examples" + + +class TestLoadConfigCheckedInExamples: + """Every ``examples/**/evalshift.yaml`` shipped in this repo must load. + + They are the configs readers copy first. Removing a config key breaks any + of them that still sets it, and only two are exercised end to end by other + tests -- the sibling ``golden.jsonl`` check lives in ``test_suite_loader``. + """ + + def test_every_checked_in_evalshift_yaml_loads(self) -> None: + configs = sorted(_EXAMPLES_DIR.glob("**/evalshift.yaml")) + # Guard the guard: an empty glob means the path is broken. + assert len(configs) >= 4, f"expected at least 4 example configs, found {configs}" + + failures: list[str] = [] + for path in configs: + try: + load_config(path) + except ConfigError as exc: + failures.append(f"{path.relative_to(_EXAMPLES_DIR.parent)}: {exc.format_plain()}") + + assert not failures, "example config(s) failed to load:\n" + "\n".join(failures) diff --git a/tests/unit/test_config_models.py b/tests/unit/test_config_models.py index 895b7cb..b3cd364 100644 --- a/tests/unit/test_config_models.py +++ b/tests/unit/test_config_models.py @@ -19,7 +19,6 @@ MigrationPolicy, PromptDefinition, SemanticEvaluatorConfig, - SliceConfig, SliceMigrationPolicy, StructuralEvaluatorConfig, ToolArgumentsEvaluatorConfig, @@ -212,25 +211,6 @@ def test_empty_criterion_prompt_fails(self) -> None: LLMJudgeConfig(criterion_name="x", criterion_prompt="") -# --------------------------------------------------------------------------- -# SliceConfig -# --------------------------------------------------------------------------- - - -class TestSliceConfig: - def test_defaults(self) -> None: - s = SliceConfig(name="long", filter="len(conversation) > 1000") - assert s.applies_to == ["*"] - - def test_empty_name_fails(self) -> None: - with pytest.raises(ValidationError): - SliceConfig(name="", filter="True") - - def test_empty_filter_fails(self) -> None: - with pytest.raises(ValidationError): - SliceConfig(name="long", filter="") - - # --------------------------------------------------------------------------- # Defaults # --------------------------------------------------------------------------- @@ -483,7 +463,6 @@ def test_minimal_valid(self) -> None: assert cfg.version == 1 assert isinstance(cfg.defaults, Defaults) assert isinstance(cfg.evaluators, EvaluatorsConfig) - assert cfg.slices == [] assert cfg.project is None assert cfg.migration_policy is None assert cfg.evaluators.agent_trace == [] @@ -530,6 +509,55 @@ def test_removed_thresholds_field_is_rejected_by_name(self) -> None: assert "migration_policy is the single source of truth" in message assert "Extra inputs are not permitted" not in message + def test_removed_slices_field_is_rejected_by_name(self) -> None: + """A config that still sets top-level ``slices`` says what happened to it. + + The block was validated and recorded but never read: slices come from + example tags. Calling it an "extra input" would send the user looking + for its new spelling instead of telling them it is gone, and where the + one thing it looked like it did (per-slice budgets) actually lives. + """ + with pytest.raises(ValidationError) as info: + EvalShiftConfig.model_validate( + { + "prompts": [{"id": "cs", "detection": "manual", "content": "hi {n}"}], + "slices": [{"name": "long", "filter": "long"}], + }, + ) + + message = str(info.value) + assert "`slices` was removed" in message + assert "never had any effect" in message + assert "example `tags`" in message + assert "migration_policy.slices" in message + assert "keyed by tag" in message + assert "Extra inputs are not permitted" not in message + + def test_an_empty_top_level_slices_list_is_rejected_too(self) -> None: + """``slices: []`` did nothing either; the key itself is what is gone.""" + with pytest.raises(ValidationError, match="`slices` was removed"): + EvalShiftConfig.model_validate( + { + "prompts": [{"id": "cs", "detection": "manual", "content": "hi"}], + "slices": [], + }, + ) + + def test_both_removed_fields_are_named_at_once(self) -> None: + """Fixing one removed key should not uncover the other on the next run.""" + with pytest.raises(ValidationError) as info: + EvalShiftConfig.model_validate( + { + "prompts": [{"id": "cs", "detection": "manual", "content": "hi"}], + "thresholds": {"pass_rate_min": 0.9}, + "slices": [], + }, + ) + + message = str(info.value) + assert "`thresholds` was removed" in message + assert "`slices` was removed" in message + def test_invalid_hosted_project_slug_fails(self) -> None: with pytest.raises(ValidationError): EvalShiftConfig( @@ -566,7 +594,6 @@ def test_round_trip_through_dump(self) -> None: ), ], ), - slices=[SliceConfig(name="long", filter="len(conversation)>1000")], ) recreated = EvalShiftConfig.model_validate(original.model_dump()) assert recreated == original diff --git a/tests/unit/test_docs_currency.py b/tests/unit/test_docs_currency.py index 879f8b9..0f13ccc 100644 --- a/tests/unit/test_docs_currency.py +++ b/tests/unit/test_docs_currency.py @@ -165,3 +165,37 @@ def test_reference_header_version_matches_the_package(name: str) -> None: assert match.group(1) == evalshift_cli.__version__, ( f"{name} says version {match.group(1)}; the package is {evalshift_cli.__version__}" ) + + +def _yaml_bearing_docs() -> list[str]: + """Every checked-in file a reader might copy an `evalshift.yaml` block from.""" + found = [ + *PROSE_FILES, + *(str(p.relative_to(REPO_ROOT)) for p in sorted((REPO_ROOT / "docs").glob("*.md"))), + *(str(p.relative_to(REPO_ROOT)) for p in sorted((REPO_ROOT / "examples").rglob("*.md"))), + *( + str(p.relative_to(REPO_ROOT)) + for p in sorted((REPO_ROOT / "examples").rglob("evalshift.yaml")) + ), + ] + return sorted(set(found)) + + +#: Top-level `evalshift.yaml` keys that were removed. A config that still sets +#: one fails to load, so a doc that shows one hands the reader a broken config. +REMOVED_TOP_LEVEL_KEYS: tuple[str, ...] = ("thresholds", "slices") + + +@pytest.mark.parametrize("name", _yaml_bearing_docs()) +def test_no_doc_shows_a_removed_top_level_key(name: str) -> None: + """A top-level YAML key starts in column 0; a nested one never does. + + That keeps `migration_policy.slices` -- indented under its parent, and very + much alive -- out of the match, while catching a copied-in `slices:` or + `thresholds:` block wherever it appears. + """ + text = (REPO_ROOT / name).read_text(encoding="utf-8") + pattern = re.compile(rf"^(?:{'|'.join(REMOVED_TOP_LEVEL_KEYS)}):", re.MULTILINE) + + hits = [m.group(0) for m in pattern.finditer(text)] + assert not hits, f"{name} shows removed top-level key(s) {hits}; that config would not load" diff --git a/tests/unit/test_doctor.py b/tests/unit/test_doctor.py index 5647de7..032f00d 100644 --- a/tests/unit/test_doctor.py +++ b/tests/unit/test_doctor.py @@ -135,6 +135,43 @@ def test_invalid_config_fails(self, tmp_path: Path) -> None: row = _by_name(run_checks(cwd=tmp_path, env=_empty_env()), CONFIG_FILENAME) assert row.status == "fail" + def test_a_removed_slices_key_fails_like_removed_thresholds(self, tmp_path: Path) -> None: + """Both retired keys land on the same failing row; neither loads.""" + rows = [] + for name, block in ( + ("thresholds", "thresholds:\n pass_rate_min: 0.9\n"), + ("slices", "slices:\n - {name: refunds, filter: refunds}\n"), + ): + project = tmp_path / name + project.mkdir() + (project / CONFIG_FILENAME).write_text( + "prompts:\n - {id: a, detection: manual, content: hi}\n" + block, + encoding="utf-8", + ) + rows.append(_by_name(run_checks(cwd=project, env=_empty_env()), CONFIG_FILENAME)) + thresholds_row, slices_row = rows + assert thresholds_row.status == slices_row.status == "fail" + assert slices_row.detail == thresholds_row.detail + + def test_invalid_config_points_at_validate_for_the_details(self, tmp_path: Path) -> None: + """The row has room for the summary only; the reasons live in `validate`. + + "1 schema problem found" alone names neither the key nor the fix, so + the row says where to get them rather than leaving the user to guess. + """ + (tmp_path / CONFIG_FILENAME).write_text("prompts: []\n", encoding="utf-8") + row = _by_name(run_checks(cwd=tmp_path, env=_empty_env()), CONFIG_FILENAME) + assert row.detail == "1 schema problem found — run `evalshift validate` for details" + + def test_unparseable_yaml_points_at_validate_too(self, tmp_path: Path) -> None: + (tmp_path / CONFIG_FILENAME).write_text( + "prompts:\n - id: a\n detection: manual\n content: bad-indent\n", + encoding="utf-8", + ) + row = _by_name(run_checks(cwd=tmp_path, env=_empty_env()), CONFIG_FILENAME) + assert row.detail.startswith("failed to parse YAML") + assert row.detail.endswith(" — run `evalshift validate` for details") + def test_unparseable_yaml_fails(self, tmp_path: Path) -> None: (tmp_path / CONFIG_FILENAME).write_text( "prompts:\n - id: a\n detection: manual\n content: bad-indent\n", @@ -195,8 +232,9 @@ def test_doctor_with_invalid_config_exits_one( ) -> None: self._isolate(monkeypatch, tmp_path) (tmp_path / CONFIG_FILENAME).write_text("prompts: []\n", encoding="utf-8") - result = runner.invoke(app, ["doctor"]) + result = runner.invoke(app, ["doctor"], env={"COLUMNS": "200"}) assert result.exit_code == 1 + assert "run `evalshift validate` for details" in result.stdout def test_doctor_with_set_keys_renders_them_ok( self, diff --git a/tests/unit/test_reserved_slice_name.py b/tests/unit/test_reserved_slice_name.py index 22f9cc8..0394e18 100644 --- a/tests/unit/test_reserved_slice_name.py +++ b/tests/unit/test_reserved_slice_name.py @@ -3,22 +3,29 @@ `BUNDLE_SPEC.md` has always reserved it — `decision.overall` is the whole-run summary and `BudgetResult.scope` defaults to `"overall"` — and the server now rejects a bundle whose `decision.slices` carries that key. A slice name reaches -the bundle from three authoring surfaces, so all three are refused here, where -the error can point at the line the user wrote rather than at an upload that -already happened. +the bundle from two authoring surfaces -- a suite example tag and a +`migration_policy.slices` key -- so both are refused here, where the error can +point at the line the user wrote rather than at an upload that already happened. """ from __future__ import annotations +import re from pathlib import Path import pytest from pydantic import ValidationError -from evalshift_cli.config.models import MigrationPolicy, SliceConfig +from evalshift_cli.config.models import MigrationPolicy from evalshift_cli.suite.tags import RESERVED_SLICE_NAME from tests.unit.suite_examples import suite_example +# The guards' own wording, not just the literal: a bare "overall" would also +# match the error's location path (e.g. `migration_policy.slices.overall`), +# so any failure on that key -- for any reason -- would satisfy it. +_TAG_RESERVED = re.escape(f"tag {RESERVED_SLICE_NAME!r} is reserved") +_POLICY_KEY_RESERVED = re.escape(f"migration_policy.slices key {RESERVED_SLICE_NAME!r} is reserved") + def test_reserved_name_is_the_scope_the_bundle_spec_names() -> None: assert RESERVED_SLICE_NAME == "overall" @@ -26,7 +33,7 @@ def test_reserved_name_is_the_scope_the_bundle_spec_names() -> None: def test_suite_example_rejects_a_tag_named_overall() -> None: """Tags become slice names directly — see `analysis.slicing._slices_of`.""" - with pytest.raises(ValidationError, match="overall"): + with pytest.raises(ValidationError, match=_TAG_RESERVED): suite_example(id="ex1", tags=["captured", "overall"]) @@ -34,18 +41,9 @@ def test_suite_example_still_accepts_ordinary_tags() -> None: assert suite_example(id="ex1", tags=["captured", "refunds"]).tags == ["captured", "refunds"] -def test_slice_config_rejects_the_reserved_name() -> None: - with pytest.raises(ValidationError, match="overall"): - SliceConfig(name="overall", filter="refunds") - - -def test_slice_config_accepts_an_ordinary_name() -> None: - assert SliceConfig(name="refunds", filter="refunds").name == "refunds" - - def test_migration_policy_rejects_a_per_slice_override_keyed_overall() -> None: """The override key is the slice name, so the same reservation applies.""" - with pytest.raises(ValidationError, match="overall"): + with pytest.raises(ValidationError, match=_POLICY_KEY_RESERVED): MigrationPolicy.model_validate({"slices": {"overall": {}}}) @@ -75,19 +73,20 @@ def test_config_load_surfaces_the_reserved_name(tmp_path: Path) -> None: structural: - type: length min_chars: 1 -slices: - - name: overall - filter: refunds +migration_policy: + slices: + overall: + max_overall_regression_rate: 0.05 """, encoding="utf-8", ) - with pytest.raises(ConfigError, match="overall"): + with pytest.raises(ConfigError, match=_POLICY_KEY_RESERVED): load_config(tmp_path / "evalshift.yaml") def test_an_ordinary_slice_name_still_loads(tmp_path: Path) -> None: - """Sanity: the guard rejects one literal, not the `slices:` block.""" + """Sanity: the guard rejects one literal, not per-slice budgets.""" from evalshift_cli.config.loader import load_config (tmp_path / "evalshift.yaml").write_text( @@ -106,11 +105,15 @@ def test_an_ordinary_slice_name_still_loads(tmp_path: Path) -> None: structural: - type: length min_chars: 1 -slices: - - name: refunds - filter: refunds +migration_policy: + slices: + refunds: + max_overall_regression_rate: 0.05 """, encoding="utf-8", ) - assert [s.name for s in load_config(tmp_path / "evalshift.yaml").slices] == ["refunds"] + policy = load_config(tmp_path / "evalshift.yaml").migration_policy + + assert policy is not None + assert set(policy.slices) == {"refunds"}