From 064e7616f8a7f90977b0b4d945aa1f7d333eeaa6 Mon Sep 17 00:00:00 2001 From: LoopX Agent <337587101+loopx-agent@users.noreply.github.com> Date: Thu, 5 Nov 2026 07:42:03 +0800 Subject: [PATCH] fix(benchmark): retain safe experiment board rejection reasons Signed-off-by: LoopX Agent <337587101+loopx-agent@users.noreply.github.com> --- .../benchmark_toolkit/__init__.py | 2 + .../benchmark_toolkit/experiment_board.py | 30 ++++++++--- .../benchmark_experiment_board.py | 3 ++ .../test_benchmark_experiment_board.py | 50 ++++++++++++++++++- 4 files changed, 78 insertions(+), 7 deletions(-) diff --git a/loopx/capabilities/benchmark_toolkit/__init__.py b/loopx/capabilities/benchmark_toolkit/__init__.py index 77286d8705..f024c7c7e9 100644 --- a/loopx/capabilities/benchmark_toolkit/__init__.py +++ b/loopx/capabilities/benchmark_toolkit/__init__.py @@ -47,6 +47,7 @@ select_exact_docker_container, ) from .experiment_board import ( + BenchmarkExperimentBoardValidationError, BENCHMARK_EXPERIMENT_BOARD_LEDGER_FILENAME, BENCHMARK_EXPERIMENT_BOARD_ROW_SCHEMA_VERSION, BENCHMARK_EXPERIMENT_BOARD_SCHEMA_VERSION, @@ -252,6 +253,7 @@ ) __all__ = [ + "BenchmarkExperimentBoardValidationError", "ATIF_SCHEMA_VERSION", "BENCHMARK_ADAPTIVE_CONCURRENCY_DECISION_SCHEMA_VERSION", "BENCHMARK_ADAPTIVE_CONCURRENCY_POLICY_SCHEMA_VERSION", diff --git a/loopx/capabilities/benchmark_toolkit/experiment_board.py b/loopx/capabilities/benchmark_toolkit/experiment_board.py index 9eced596f6..22fa8c198a 100644 --- a/loopx/capabilities/benchmark_toolkit/experiment_board.py +++ b/loopx/capabilities/benchmark_toolkit/experiment_board.py @@ -64,6 +64,14 @@ } +class BenchmarkExperimentBoardValidationError(ValueError): + """A board rejection whose diagnostic is safe for caller-facing output. + + Messages must contain only fixed contract text or normalized status values, + never paths, raw field names, or values from unvalidated input. + """ + + def benchmark_run_status_is_terminal(value: object) -> bool: """Return whether a normalized run status closes its lifecycle.""" @@ -310,17 +318,27 @@ def normalize_benchmark_experiment_board_row( countability = _countability(payload.get("countability")) if countability["official_result_present"]: if primary_metric not in metrics: - raise ValueError("official result must include primary_metric") + raise BenchmarkExperimentBoardValidationError( + "official result must include primary_metric" + ) missing_guardrails = [name for name in guardrails if name not in metrics] if missing_guardrails: - raise ValueError("official result is missing declared guardrail metrics") + raise BenchmarkExperimentBoardValidationError( + "official result is missing declared guardrail metrics" + ) if countability["score_countable"]: if status != "completed": - raise ValueError("score_countable requires completed status") + raise BenchmarkExperimentBoardValidationError( + "score_countable requires completed status" + ) if not countability["integrity_qualified"]: - raise ValueError("score_countable requires integrity_qualified") + raise BenchmarkExperimentBoardValidationError( + "score_countable requires integrity_qualified" + ) if not countability["official_result_present"]: - raise ValueError("score_countable requires official_result_present") + raise BenchmarkExperimentBoardValidationError( + "score_countable requires official_result_present" + ) row: dict[str, Any] = { "schema_version": BENCHMARK_EXPERIMENT_BOARD_ROW_SCHEMA_VERSION, @@ -437,7 +455,7 @@ def _validate_benchmark_run_status_transition( existing_status = str(existing.get("status") or "") candidate_status = str(candidate.get("status") or "") if candidate_status not in _RUN_STATUS_TRANSITIONS.get(existing_status, set()): - raise ValueError( + raise BenchmarkExperimentBoardValidationError( f"benchmark run status cannot move from {existing_status} to {candidate_status}" ) diff --git a/loopx/cli_commands/benchmark_experiment_board.py b/loopx/cli_commands/benchmark_experiment_board.py index 942f997b36..03d0db3f02 100644 --- a/loopx/cli_commands/benchmark_experiment_board.py +++ b/loopx/cli_commands/benchmark_experiment_board.py @@ -9,6 +9,7 @@ from ..capabilities.benchmark_toolkit import ( BENCHMARK_EXPERIMENT_BOARD_ROW_SCHEMA_VERSION, + BenchmarkExperimentBoardValidationError, benchmark_experiment_board_row_key, build_benchmark_experiment_board, default_benchmark_experiment_board_path, @@ -274,6 +275,8 @@ def handle_benchmark_experiment_board_command( payload["path_recorded"] = False if write is not None: payload["write"] = write + except BenchmarkExperimentBoardValidationError as exc: + args.benchmark_experiment_board_parser.error(str(exc)) except (OSError, UnicodeError, json.JSONDecodeError, TypeError, ValueError): args.benchmark_experiment_board_parser.error( "invalid benchmark experiment board input" diff --git a/tests/capabilities/test_benchmark_experiment_board.py b/tests/capabilities/test_benchmark_experiment_board.py index ef6331039d..70c8a8c846 100644 --- a/tests/capabilities/test_benchmark_experiment_board.py +++ b/tests/capabilities/test_benchmark_experiment_board.py @@ -964,10 +964,58 @@ def test_cli_compact_upsert_rejects_invalid_transition_without_writing(tmp_path: "--project", str(tmp_path), "--row-json", str(row_path), "--execute", "--format", "json", ], check=False) assert result.returncode != 0 - assert "invalid benchmark experiment board input" in result.stderr + assert "benchmark run status cannot move from completed to running" in result.stderr assert ledger.read_bytes() == before +@pytest.mark.parametrize("execute", [False, True]) +def test_cli_identifies_missing_guardrails_and_allows_correction( + tmp_path: Path, execute: bool, +) -> None: + ledger = default_benchmark_experiment_board_path(project=tmp_path, goal_id="fixture-goal") + row_path = tmp_path / "row.json" + row = _baseline() + del row["metrics"]["reward"] + row_path.write_text(json.dumps(row), encoding="utf-8") + command = ["benchmark", "experiment-board-upsert", "--goal-id", "fixture-goal", + "--project", str(tmp_path), "--row-json", str(row_path), "--format", "json"] + if execute: + command.append("--execute") + + rejected = run_source_cli(command, check=False) + assert rejected.returncode == 2 + assert "official result is missing declared guardrail metrics" in rejected.stderr + assert not ledger.exists() + + row_path.write_text(json.dumps(_baseline()), encoding="utf-8") + corrected = json.loads(run_source_cli(command).stdout) + assert corrected["row"]["metrics"]["reward"]["value"] == 0 + assert corrected["write"]["write_performed"] is execute + assert ledger.exists() is execute + + +@pytest.mark.parametrize("failure", ["missing_file", "invalid_json", "unknown_field", "non_object"]) +def test_cli_keeps_untrusted_input_errors_generic(tmp_path: Path, failure: str) -> None: + private_text = "private-input-marker" + row_path = tmp_path / (private_text + ".json") + if failure == "invalid_json": + row_path.write_text('{"' + private_text + '":', encoding="utf-8") + elif failure == "unknown_field": + row_path.write_text(json.dumps(_baseline(**{private_text: "secret"})), encoding="utf-8") + elif failure == "non_object": + row_path.write_text(json.dumps([private_text]), encoding="utf-8") + ledger = default_benchmark_experiment_board_path(project=tmp_path, goal_id="fixture-goal") + result = run_source_cli([ + "benchmark", "experiment-board-upsert", "--goal-id", "fixture-goal", + "--project", str(tmp_path), "--row-json", str(row_path), "--execute", "--format", "json", + ], check=False) + assert result.returncode == 2 + assert "invalid benchmark experiment board input" in result.stderr + assert private_text not in result.stderr + assert str(tmp_path) not in result.stderr + assert not ledger.exists() + + def test_cli_show_projects_factorial_contrast_from_explicit_contract( tmp_path: Path, ) -> None: