feat(config)!: remove the top-level slices key (it never did anything) - #25
Merged
Merged
Conversation
The server pairs a run with its hosted baseline only on an equal `eval_config_hash`, and the hash covers the whole `evaluator_config` snapshot, which has always carried a `"slices"` key. Pin the value the CLI computes today (af4e6fe) for a fixed config without a `slices:` block, so removing the key from the config model cannot silently move the hash and orphan existing baselines. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
The top-level `slices:` block was validated and recorded in the run bundle, but analysis never read it: `analyze` builds one slice per distinct example tag plus `all`, so `name`, `filter` and `applies_to` changed nothing. Remove the field and `SliceConfig`. As with `thresholds`, `extra="forbid"` alone would greet an existing config with "Extra inputs are not permitted", which reads as a typo. The same `mode="before"` validator now names the removal, says the key never had an effect, that slices come from example `tags`, and that per-slice budgets go under `migration_policy.slices` keyed by tag. A config carrying both retired keys gets both messages at once. `validate` prints the message in its panel and `doctor` fails the config row, the same way both already handled `thresholds` (now pinned for both keys). The bundle's `evaluator_config` keeps `"slices": []` as a constant: `eval_config_hash` is computed over that dict and the server pairs runs with baselines only on an equal hash, so dropping the key would move the hash of every config. The pinned-hash test from the previous commit passes unchanged. The reserved-`overall` check on `slices[].name` goes with the block; the two surfaces that still create slice names, example tags and `migration_policy.slices` keys, keep theirs, and the load-level reserved-name tests now go through `migration_policy.slices`. The two shipped examples lose their `slices:` blocks so they still load. BREAKING CHANGE: `slices:` is no longer a valid top-level key in `evalshift.yaml`. A config that still sets it fails to load with an error naming the removal; delete the block (it never had an effect). Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
`build_slices` and `build_unmeasured` took an optional tag -> slice-name mapping meant for the top-level `slices:` block, but no caller ever passed one, and with that block removed nothing can. A tag is its slice's name; say so in the module docstring instead of pointing at a config block that no longer exists. Behaviour is unchanged: every call site already used the identity fallback. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Removing `slices:` broke the two example configs that set it, and only those two happen to run end to end elsewhere; agent-traces and capture-first had no load check at all. Mirror the existing golden.jsonl guard in test_suite_loader for the configs readers copy first. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
#23 documented the block as validated but not applied; it is now gone, and every copy of that description would hand a reader a config that fails to load. DOCS.md, llms-full.txt and docs/configuration.md drop it from the schema and gain a removal note beside the thresholds one, quoting the load error, saying slices come from example tags and that per-slice budgets live under `migration_policy.slices`. The `overall` reservation now names the two surfaces that remain. docs/hosted.md and llms-full.txt describe `evaluator_config.slices` as the legacy empty list it now is. The hash consequence is stated where the key is: a config that never set `slices:` (or set `[]`) keeps its `eval_config_hash`; deleting a non-empty block moves it, so the next pushed runs need a fresh baseline. DOCS.md and llms-full.txt also still said `version:` bumps when a field is removed, which docs/configuration.md stopped saying when thresholds left; this is the second removal that keeps `version: 1`, so both now match the config version policy. CHANGELOG gains the `### Removed` entry, and the earlier docs-audit bullet points at it. test_docs_currency now fails on any doc or example that shows a top-level `slices:` or `thresholds:` key. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
README.md still said `version: 1` changes when a field is "renamed, removed, or given new semantics", contradicting DOCS.md, llms-full.txt and docs/configuration.md. Removals that fail the load by name ride the CLI version, as `thresholds` and top-level `slices` both did. Link the config version policy instead of restating it. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
The Unreleased docs-audit bullet still described the top-level
`slices:` block in the present tense ("is validated ... never reads
it"), including a parenthetical about its reserved-name check, which
went with the block. It is gone in this release, so say what it was.
Fix "does since 1.1.0" in the Removed entry and re-wrap both bullets to
the file's width; the audit bullet had an overlong line from the
earlier edit.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
doctor's `evalshift.yaml` row showed only `ConfigError.summary` for any config error: "1 schema problem found" names neither the offending key nor the fix, so a user hitting the removed `slices:` or `thresholds:` key learned nothing from it. The row has room for the summary only, so append "— run `evalshift validate` for details", which prints every problem. DOCS.md, llms-full.txt and getting-started say so; CHANGELOG gains a `### Changed` line. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
`match="overall"` also matches the error's location path, so a `migration_policy.slices.overall` entry that failed for any other reason (an unknown field, say, which stops the guard from ever running) satisfied the test. Match each guard's message instead: "tag 'overall' is reserved" and "migration_policy.slices key 'overall' is reserved". Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Breaking for configs that still set
slices:. It follows thethresholdsprecedent from 1.1.0: the config fails to load with an error naming the key, andversion:stays 1.Why
The top-level
slices:block (name/filter/applies_to) was accepted and recorded in the run bundle, but analysis never read it.analyzebuilds one slice per distinct example tag, plusall, whatever the block said. The docs-currency audit exposed this, and #23 documented it as "not applied". The maintainer decided to remove it now rather than later.What changes
Loading. A config with
slices:now fails with:_reject_removed_fieldsmechanism asthresholds. If both keys are present, one error names both.evalshift validateprints the message.doctor's failing config row now says "— runevalshift validatefor details". That applies to any config error.Unchanged. Tag-based slicing,
migration_policy.slices, and the reservedoverallname on tags and policy keys.Hosted baselines are unaffected. The bundle's
evaluator_configkeeps a constant"slices": []. A test pinseval_config_hashfor a slices-less config to the value computed bymain, and a reviewer recomputed it independently. The only exception is a project whose config had a non-empty block: deleting it gives a new hash, so that project needs a fresh baseline. This is documented.Cleanup.
SliceConfigand the deadtag_to_sliceparameter.slices:blocks, and a new test checks that every shipped example config loads.slices:andthresholds:in the docs and examples.Docs and CHANGELOG.
docs/configuration.mdanddocs/hosted.mdnow say the block was removed, with a removal note next to thethresholdsone.version:stays 1.### Removedentry and a### Changedentry for the doctor hint.Checks
make ciis green: 2382 tests, 94.43% coverage.check-schemamatches the server.Related server-doc and website PRs follow. The website one is a draft to merge after the release.
🤖 Generated with Claude Code