diff --git a/AGENTS.md b/AGENTS.md index e392f7db3..6d9ed8309 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -26,7 +26,7 @@ convert external eval sources into it. one datastore PR per adapter. See its `README.md`. - `every_eval_ever/helpers/raw_capture.py` snapshots what an adapter fetched. Inert unless a sink is active, so a manual run behaves exactly as before. -- `every_eval_ever/converters/` — in-tree converters (`inspect`/`helm`/`lm_eval`, plus `alpaca_eval`; shared code in `common`), run via `uv run python -m every_eval_ever convert ...`. +- `every_eval_ever/converters/` — in-tree converters (`inspect`/`helm`/`lm_eval`, plus `alpaca_eval`; shared code in `common`), run via `uv run python -m every_eval_ever convert ...`. Cover one by adding a `ConverterCase` to `tests/converter_cases.py`; see "Testing a converter" in `converters/README.md`. - `every_eval_ever/validator/` — the schema + **semantic** merge gate (path shape, UUID4 names, companion pairing, score bounds, deployment axes). `REGISTERED_CHECKS` is the list. - Validate: `uv run python -m every_eval_ever validate ` (`.json`→aggregate, diff --git a/every_eval_ever/converters/README.md b/every_eval_ever/converters/README.md index 21f3331ee..b9733fdd6 100644 --- a/every_eval_ever/converters/README.md +++ b/every_eval_ever/converters/README.md @@ -12,6 +12,25 @@ uv sync --extra helm # + HELM uv sync --extra all # + all ``` +### Testing a converter + +Add a `ConverterCase` to `tests/converter_cases.py` pointing at a log the upstream tool +really wrote, plus the CLI arguments a user would pass, and state what the conversion +should yield. `tests/test_converter_conversion.py` then converts it with the real CLI and +requires the published files to pass `validate` with the semantic checks on and no +warnings — which is what shows a converter's records are submittable, since +`validate_file()` runs with those checks off. There is no test file to write, and nothing +to regenerate when the schema changes. + +A case whose converter is behind an optional extra skips when the extra is not installed, +so declaring one costs nothing in the core install. + +This does not check semantics: a metric that changes from a percentage to a proportion +upstream passes it. Nor does it see a new upstream release — for that, a script under +`tools/upstream_smoke/` makes the upstream tool produce a log with its own fake-model +mode (`--model dummy` for lm-eval, `--model mockllm/model` for Inspect) and converts +that, on a schedule. + ### Inspect The conversion script from `Inspect AI` to the unified schema can be run using `every_eval_ever/converters/inspect/__main__.py`. diff --git a/tests/converter_cases.py b/tests/converter_cases.py new file mode 100644 index 000000000..8919b42d1 --- /dev/null +++ b/tests/converter_cases.py @@ -0,0 +1,212 @@ +"""Shared machinery for testing a converter against a committed upstream log. + +One `ConverterCase` per converter is all it takes to be covered by +`tests/test_converter_conversion.py`: point it at a real log the upstream tool wrote, +give the CLI arguments a user would give, and state what the conversion should yield. + +Adding a converter here does not require writing a test. +""" + +from __future__ import annotations + +import importlib +import json +from dataclasses import dataclass +from pathlib import Path +from typing import Any + +REPO_ROOT = Path(__file__).resolve().parents[1] + + +@dataclass(frozen=True) +class ConverterCase: + """One converter, one committed upstream log, and what converting it must yield.""" + + source: str + log_path: Path + # What the conversion is expected to produce. Kept small on purpose: enough that a + # silently dropped score, task, or sidecar fails, without restating the whole record. + aggregates: int + sidecars: int = 0 + # Total `evaluation_results` across every aggregate. For a converter that emits more + # results than are worth listing in `scores`, this is what catches one going missing. + results: int | None = None + model_id: str | None = None + # Keyed by `/`, since one task can be scored by several + # metrics and each becomes its own result. + scores: dict[str, float] | None = None + extra_argv: tuple[str, ...] = () + # Upstream key paths the converter cannot work without, `*` matching any one key. + required_source_paths: tuple[str, ...] = () + + @property + def id(self) -> str: + return self.source + + def source_payload(self) -> Any: + return json.loads(self.log_path.read_text(encoding='utf-8')) + + +CASES: tuple[ConverterCase, ...] = ( + ConverterCase( + source='lm_eval', + log_path=REPO_ROOT + / 'tests/data/lm_eval/results_2026-01-21T03-44-18.458309.json', + aggregates=2, + # Two tasks, one `exact_match` result each. Stated even though `scores` + # already lists both, because `scores` is a dict: without a count, a + # third result that collided on an existing key would be merged away. + results=2, + # The fixture ships a samples file for only one of its two tasks, so + # --include_samples would (correctly) report a partial conversion. + model_id=( + 'RylanSchaeffer/mem_Qwen3-93M_minerva_math_rep_0_sbst_1.0000_epch_1_ot_1' + ), + scores={ + 'math_perturbed_full/exact_match': 0.0, + 'math_rephrased_full/exact_match': 0.0004, + }, + required_source_paths=( + 'config.model', + 'config.model_args', + 'results.*', + 'configs.*.dataset_path', + ), + ), + ConverterCase( + source='inspect', + log_path=REPO_ROOT + / 'tests/data/inspect/data_cyse2_vuln_exploit_challenges.json', + aggregates=1, + sidecars=1, + # One scorer reporting three metrics, which is what makes this fixture worth + # using: a converter that collapses them to one result fails here. + results=3, + model_id='mistral/mistral-large-latest', + required_source_paths=( + 'eval.model', + 'eval.task', + 'eval.dataset.name', + 'results.scores.*.name', + 'results.scores.*.scorer', + 'results.scores.*.metrics', + 'results.total_samples', + ), + ), + ConverterCase( + source='helm', + log_path=REPO_ROOT + / 'tests/data/helm' + / 'commonsense-dataset=hellaswag,method=multiple_choice_joint,' + 'model=eleutherai_pythia-1b-v0', + aggregates=1, + sidecars=1, + # Eight metrics on the `valid` split, each also reported worst-case over the + # robustness and fairness perturbations. + results=24, + model_id='eleutherai/pythia-1b-v0', + # `--log_path` is a HELM run directory, not one file, so there is no single + # payload for `missing_paths` to address. The gate and the counts above are + # what cover this converter. + required_source_paths=(), + ), +) + + +def unavailable(case: ConverterCase) -> str | None: + """Why this case cannot run here, or None if it can. + + A converter behind an optional extra states its own missing dependency in an + `_..._IMPORT_ERROR` module global and raises from it when used. Reading that is what + lets a case be declared once and skip, rather than fail, in the core install. + """ + module = importlib.import_module( + f'every_eval_ever.converters.{case.source}.adapter' + ) + for name, value in vars(module).items(): + if name.endswith('_IMPORT_ERROR') and value is not None: + return ( + f'{case.source} converter dependencies are missing: {value!r}. ' + f'Install with: uv sync --extra {case.source}' + ) + return None + + +def convert(case: ConverterCase, tmp_path: Path) -> list[Path]: + """Run the real CLI over a case's log; return the published record files.""" + from every_eval_ever import cli + + data_dir = tmp_path / 'data' + exit_code = cli.main( + [ + 'convert', + case.source, + '--log_path', + str(case.log_path), + '--output_dir', + str(data_dir), + '--source_organization_name', + 'every-eval-ever-tests', + *case.extra_argv, + ] + ) + assert exit_code == 0, f'{case.source} conversion exited {exit_code}' + return sorted( + path + for path in data_dir.rglob('*') + if path.is_file() and path.suffix in {'.json', '.jsonl'} + ) + + +def gate_complaints(paths: list[Path], capsys) -> list[dict[str, Any]]: + """Run the validator CLI over `paths`; return every error and warning it reports. + + This is the merge gate: the semantic checks only run for a file at a canonical + `data////` path, which is what the converters + publish to, and `validate_file()` on its own leaves them off. + """ + from every_eval_ever.validate import main as validate_main + + capsys.readouterr() # drop what the conversion printed + exit_code = validate_main( + [str(path) for path in paths] + ['--format', 'json'] + ) + reports = json.loads(capsys.readouterr().out) + complaints = [ + { + 'file': report['file'], + 'errors': report['errors'], + 'warnings': report['warnings'], + } + for report in reports + if not report['valid'] or report['errors'] or report['warnings'] + ] + if exit_code != 0 and not complaints: + complaints.append( + { + 'file': '', + 'errors': ['validate exited non-zero'], + 'warnings': [], + } + ) + return complaints + + +def missing_paths(payload: Any, paths: tuple[str, ...]) -> list[str]: + """Return the declared key paths that the given source payload does not have.""" + + def resolve(node: Any, parts: list[str]) -> bool: + if not parts: + return True + head, rest = parts[0], parts[1:] + if head == '*': + if isinstance(node, dict): + return any(resolve(value, rest) for value in node.values()) + if isinstance(node, list): + return any(resolve(item, rest) for item in node) + return False + if isinstance(node, dict) and head in node: + return resolve(node[head], rest) + return False + + return [path for path in paths if not resolve(payload, path.split('.'))] diff --git a/tests/test_converter_conversion.py b/tests/test_converter_conversion.py new file mode 100644 index 000000000..c7b87a7d8 --- /dev/null +++ b/tests/test_converter_conversion.py @@ -0,0 +1,124 @@ +"""Convert a committed upstream log with the real CLI and put it through the gate. + +Someone ran an evaluation: can this repo still turn that output into a record the +datastore accepts? Each case converts a log the upstream tool wrote — never a +hand-built one — and requires the published files to pass `validate` with the semantic +checks on and no warnings, since a warning here is one every user inherits. + +A *new version* of the upstream tool is out of reach here; that is what a +`tools/upstream_smoke/` script covers, on a schedule. Neither layer checks semantics: a +metric that changes from a percentage to a proportion upstream passes both. + +Cases live in `tests/converter_cases.py`. +""" + +from __future__ import annotations + +import json + +import pytest + +from tests.converter_cases import ( + CASES, + ConverterCase, + convert, + gate_complaints, + missing_paths, + unavailable, +) + +CASE_IDS = [case.id for case in CASES] + + +@pytest.fixture(params=CASES, ids=CASE_IDS) +def case(request) -> ConverterCase: + reason = unavailable(request.param) + if reason is not None: + pytest.skip(reason) + return request.param + + +def test_conversion_passes_the_merge_gate(case, tmp_path, capsys): + """Everything the converter publishes must be submittable as-is.""" + paths = convert(case, tmp_path) + complaints = gate_complaints(paths, capsys) + + assert not complaints, ( + f'{case.source} conversion no longer passes the merge gate:\n' + + json.dumps(complaints, indent=2) + + f'\nFix every_eval_ever/converters/{case.source}/, or the schema, ' + 'validator or publisher change that caused it.' + ) + + +def test_conversion_yields_the_expected_records(case, tmp_path): + """Guard against a conversion that validates but quietly loses data.""" + paths = convert(case, tmp_path) + aggregates = [path for path in paths if path.suffix == '.json'] + sidecars = [path for path in paths if path.suffix == '.jsonl'] + + assert len(aggregates) == case.aggregates, ( + f'{case.source} produced {len(aggregates)} aggregate record(s), ' + f'expected {case.aggregates}: {[path.name for path in aggregates]}' + ) + assert len(sidecars) == case.sidecars, ( + f'{case.source} produced {len(sidecars)} instance-level sidecar(s), ' + f'expected {case.sidecars}' + ) + + logs = [json.loads(path.read_text(encoding='utf-8')) for path in aggregates] + if case.model_id is not None: + assert {log['model_info']['id'] for log in logs} == {case.model_id} + if case.results is not None: + converted = sum(len(log['evaluation_results']) for log in logs) + assert converted == case.results, ( + f'{case.source} converted {converted} result(s), ' + f'expected {case.results}' + ) + if case.scores is not None: + scored = [ + ( + f'{result["evaluation_name"]}/' + f'{result.get("evaluation_result_id") or result["metric_config"]["evaluation_description"]}', + result['score_details']['score'], + ) + for log in logs + for result in log['evaluation_results'] + ] + # Collected as pairs, not straight into a dict: two results sharing a key + # is the quietly-lost data this test exists to catch, and a dict would + # merge them before the comparison below could see the collision. + keys = [key for key, _ in scored] + assert len(keys) == len(set(keys)), ( + f'{case.source} produced two results with the same ' + f'evaluation_name/evaluation_result_id: ' + f'{sorted(key for key in set(keys) if keys.count(key) > 1)}' + ) + assert dict(scored) == case.scores + + for log, path in zip(logs, aggregates, strict=True): + detailed = log.get('detailed_evaluation_results') + if detailed is None: + continue + # The sidecar pointer must name the file that was written beside it, since + # the gate resolves it as a repository path. + assert detailed['file_path'].endswith(f'{path.stem}_samples.jsonl') + assert detailed['total_rows'] > 0 + + +def test_required_source_paths_are_present_in_the_fixture(case): + """The keys the converter reads must still exist in the committed log. + + A fixture refreshed from a newer upstream release fails here by name, instead of + as a stack trace from inside the converter. + """ + if not case.required_source_paths: + pytest.skip(f'{case.source} declares no required source paths') + + absent = missing_paths(case.source_payload(), case.required_source_paths) + + assert not absent, ( + f'{case.source} reads upstream keys that {case.log_path.name} does not have: ' + f'{absent}\nEither the fixture came from an incompatible version, or the ' + f'converter and tests/converter_cases.py disagree about what it reads.' + )