From 783f9df110eef8c5cdd04cf9fa4cb96882f38efc Mon Sep 17 00:00:00 2001 From: shekharprateek Date: Tue, 27 Oct 2026 22:42:28 -0400 Subject: [PATCH] fix(harness): stop omp from silently benchmarking a different model Ports upstream d8c07b6 ("Stop omp from running a different model than the one requested", #218), which landed after this repository's last substantive sync and never came across. Unlike the earlier gaps this is a whole missing commit, not a half-finished port, and upstream shipped tests with it. omp's --model is a FUZZY match. Hand it a Bedrock id that is not in the catalog omp currently has loaded and it resolves to the nearest id it knows instead of erroring: upstream recorded omp 18.1.15 sending every us.anthropic.claude-haiku-5-5 request to Haiku 4.5. A caching detail is what exposes it. `omp -p` in a fresh agent dir sees only the snapshot bundled in the binary, while `omp models --json` fetches the full catalog and caches it in that dir. The harness gives every task its own PI_CODING_AGENT_DIR for isolation, so every task starts from the stale bundled snapshot, and any model newer than the installed binary gets silently downgraded. The failure mode is the dangerous kind: the run SUCCEEDS. Six artifacts, a populated metrics.json, real judge scores and a cost figure, all filed under the folder for the model that was requested, with nothing recording that a different model did the work. For a repository whose output is per-model cost/quality comparisons that is a silently corrupted data point. It matters most for omp specifically, which is the default harness in the README's /benchmark example and the one behind the published frontier, so its numbers are what reach vend/swe-router/models.json. Two independent layers, because neither alone is enough: - Before each Bedrock task, list the catalog in that task's agent dir and assert the exact wire id is in it. This both proves the id exists AND warms the dir's cache, so the following `omp -p` resolves correctly rather than falling back to the bundled snapshot. The warming is why this is a correctness fix and not only a check. On failure it names the remedies: `omp update`, `omp models find `, and the id as `aws bedrock list-inference-profiles` reports it. - After the run, every assistant message in omp's stream names the model that answered. _omp_models_used collects them and _flag_omp_model_mismatch fails the result if any differs from what was asked for, so a mismatch is caught from omp's own reporting rather than trusted. Supporting pieces: a fail-fast check in main() so a bad id dies once up front instead of once per task across a long batch; models_used recorded in metrics.json so the model that actually answered stays visible in the data; and the distinct subtype error_model_mismatch marked non-retryable, because a retry resolves the same id the same way and would only spend money to fail again. Verified: all 10 of upstream's tests error against main and pass with the fix. The mismatch test reproduces the original incident (haiku-5-5 answered by haiku-4-5). The main() preflight exits 1 before any repo is cloned, and a claude dry-run mentions omp zero times, so no other agent is affected. 652 tests pass, 10 new. ruff, ruff format, mypy and bandit (0 issues at every severity) clean. --- benchmarks/scripts/run-swe-headless.py | 157 ++++++++++++++++++++++ benchmarks/tests/test_run_swe_headless.py | 87 ++++++++++++ 2 files changed, 244 insertions(+) diff --git a/benchmarks/scripts/run-swe-headless.py b/benchmarks/scripts/run-swe-headless.py index aa92d96..3464ae3 100644 --- a/benchmarks/scripts/run-swe-headless.py +++ b/benchmarks/scripts/run-swe-headless.py @@ -33,6 +33,7 @@ import shutil import subprocess # nosec B404 - used with list args, no shell, hardcoded command import sys +import tempfile import threading import time from concurrent.futures import ThreadPoolExecutor, as_completed @@ -125,6 +126,16 @@ def _skill_path(config: RunnerConfig) -> Path: # diverge without a silent coupling. OMP_PROVIDER_VLLM = "vllm" OMP_PROVIDER_BEDROCK = "amazon-bedrock" +# omp's --model is a FUZZY match: a Bedrock id missing from the catalog omp has +# loaded silently resolves to the nearest one it knows (omp 18.1.15 sent every +# us.anthropic.claude-haiku-5-5 request to Haiku 4.5). In a fresh agent dir +# `omp -p` sees only the snapshot bundled in the binary; `omp models` fetches the +# full catalog and caches it in that dir. So each Bedrock task first lists the +# catalog in its own agent dir (which both warms it and proves the exact id is +# there), and every task checks the model each assistant message names. A +# mismatch is a distinct, non-retryable subtype. +OMP_CATALOG_TIMEOUT_SECONDS = 60 +OMP_MODEL_MISMATCH_SUBTYPE = "error_model_mismatch" # kiro-cli binary and the parsers for the one-line summary it prints on stderr at # the end of a non-interactive run, e.g. "▸ Credits: 0.21 • Time: 17s". kiro-cli @@ -755,11 +766,128 @@ def _build_omp_cmd(config: RunnerConfig, prompt: str) -> list[str]: return [*cmd, "--", full_prompt] +def _omp_bedrock_catalog(env: dict[str, str]) -> set[str]: + """List omp's Amazon Bedrock model ids, caching the catalog in the agent dir. + + Runs ``omp models amazon-bedrock --json`` with ``env`` (which pins + ``PI_CODING_AGENT_DIR``). Besides returning the ids, this writes the fetched + catalog into that dir's ``models.db``, so a following ``omp -p`` there resolves + ids the bundled snapshot lacks (see ``OMP_CATALOG_TIMEOUT_SECONDS``). + + Args: + env: Environment for the subprocess, as built by ``_build_omp_env``. + + Returns: + The set of model ids omp lists under its ``amazon-bedrock`` provider. + + Raises: + RuntimeError: If omp is missing, fails, times out, or prints bad JSON. + """ + cmd = ["omp", "models", OMP_PROVIDER_BEDROCK, "--json"] + try: + proc = subprocess.run( # nosec B603 B607 - hardcoded command, list args + cmd, + env=env, + capture_output=True, + text=True, + check=True, + timeout=OMP_CATALOG_TIMEOUT_SECONDS, + stdin=subprocess.DEVNULL, + ) + catalog = json.loads(proc.stdout) + except FileNotFoundError as exc: + raise RuntimeError("omp CLI not found on PATH; see docs/omp-setup.md") from exc + except subprocess.TimeoutExpired as exc: + raise RuntimeError( + f"'{' '.join(cmd)}' timed out after {OMP_CATALOG_TIMEOUT_SECONDS}s" + ) from exc + except subprocess.CalledProcessError as exc: + raise RuntimeError( + f"'{' '.join(cmd)}' failed (exit {exc.returncode}): {exc.stderr[:500]}" + ) from exc + except json.JSONDecodeError as exc: + raise RuntimeError(f"'{' '.join(cmd)}' printed invalid JSON: {exc}") from exc + models = catalog.get("models") if isinstance(catalog, dict) else None + if not isinstance(models, list): + raise RuntimeError(f"'{' '.join(cmd)}' printed no 'models' list") + return {m["id"] for m in models if isinstance(m, dict) and m.get("id")} + + +def _require_omp_bedrock_model(config: RunnerConfig, env: dict[str, str]) -> None: + """Fail unless omp, in ``env``'s agent dir, lists the exact Bedrock model id. + + Call this before every omp Bedrock task: listing the catalog also warms it, + without which a fresh agent dir fuzzy-matches ids the bundled snapshot lacks. + + Args: + config: The runner config (model). + env: The omp subprocess environment for this task's agent dir. + + Raises: + RuntimeError: If the catalog cannot be read or lacks the model id. + """ + wire_id = model_to_wire_id(config.model or "") + if wire_id in _omp_bedrock_catalog(env): + return + raise RuntimeError( + f"omp's Amazon Bedrock catalog does not list '{wire_id}', so " + f"'--model {OMP_PROVIDER_BEDROCK}/{wire_id}' would fuzzy-match a different " + "model. Upgrade omp ('omp update'), confirm with " + f"'omp models find {wire_id}', and check the id against " + "'aws bedrock list-inference-profiles'." + ) + + +def _omp_models_used(events: list[dict[str, Any]]) -> list[str]: + """Return the sorted model ids named by the assistant messages in an omp stream. + + Args: + events: The parsed JSON-lines events omp emitted. + + Returns: + Every distinct ``message.model`` on an assistant ``message_end`` event. + """ + models: set[str] = set() + for event in events: + if event.get("type") != "message_end": + continue + message = event.get("message") or {} + if message.get("role") == "assistant" and message.get("model"): + models.add(message["model"]) + return sorted(models) + + +def _flag_omp_model_mismatch( + result: dict[str, Any], + expected_model: str, + models_used: list[str], +) -> None: + """Mark ``result`` failed when omp answered with a model other than requested. + + Args: + result: The normalized result dict, updated in place. + expected_model: The model id the harness asked omp for. + models_used: The model ids the stream's assistant messages named. + """ + unexpected = [m for m in models_used if m != expected_model] + if not unexpected: + return + message = ( + f"omp answered with {unexpected} instead of the requested " + f"'{expected_model}': its --model fuzzy-matched a different model" + ) + logger.error(message) + result["is_error"] = True + result["subtype"] = OMP_MODEL_MISMATCH_SUBTYPE + result["result"] = message + + def _run_omp( cmd: list[str], env: dict[str, str], timeout: int, stream_log: Path | None = None, + expected_model: str | None = None, ) -> dict[str, Any]: """Run ``omp -p --mode json`` and normalize its events to a result dict. @@ -843,6 +971,9 @@ def _run_omp( ) result = _pi_result_from_events(events, elapsed) result["_elapsed_seconds"] = round(elapsed, 1) + result["models_used"] = _omp_models_used(events) + if expected_model: + _flag_omp_model_mismatch(result, expected_model, result["models_used"]) return result @@ -1266,6 +1397,10 @@ def _metrics_from_result(result: dict[str, Any], elapsed: float) -> dict[str, An # cost is auditable back to what the CLI actually charged. if result.get("kiro_credits") is not None: metrics["kiro_credits"] = result["kiro_credits"] + # agent=omp only: the model ids omp actually answered with, so a fuzzy-matched + # substitute is visible in the recorded metrics (see _flag_omp_model_mismatch). + if result.get("models_used"): + metrics["models_used"] = result["models_used"] # Capture the error message so failures are diagnosable from metrics.json # without re-running the task by hand. if is_error: @@ -3275,6 +3410,10 @@ def _run_task( _write_omp_config(config, omp_agent_dir) cmd = _build_omp_cmd(config, prompt) env = _build_omp_env(config, omp_agent_dir) + if config.is_bedrock: + # Warm this task's catalog and prove the exact id is in it; a + # fresh agent dir otherwise fuzzy-matches (see _omp_bedrock_catalog). + _require_omp_bedrock_model(config, env) omp_cap = ( f"max-time {config.agent_max_time_seconds}s" if config.agent_max_time_seconds @@ -3367,6 +3506,11 @@ def _run_task( config.timeout_seconds, stream_log=_artifact_dir(config, dataset, task) / "omp-stream.jsonl", + expected_model=( + model_to_wire_id(config.model or "") + if config.is_bedrock + else config.model + ), ) elif config.is_kiro: # kiro-cli streams ANSI text and prints a Credits/Time summary on @@ -3598,6 +3742,9 @@ def _summary_is_retryable(summary: dict[str, Any]) -> bool: metrics = summary.get("metrics") or {} if metrics.get("result_subtype") == "error_max_turns": return False + # omp answered with the wrong model: a retry resolves the same id the same way. + if metrics.get("result_subtype") == OMP_MODEL_MISMATCH_SUBTYPE: + return False # Defensive fallback: no subtype recorded but the run clearly ran the turn # budget dry (e.g. an older claude that omits the subtype). Treat a run that # burned >=95% of max_turns without finishing the design as turn exhaustion. @@ -4269,6 +4416,16 @@ def main() -> None: if args.dry_run: _dry_run(config, dataset, tasks) return + if config.is_omp and config.is_bedrock: + # Fail before the first task rather than once per task (with retries). + try: + with tempfile.TemporaryDirectory() as agent_dir: + _require_omp_bedrock_model( + config, _build_omp_env(config, Path(agent_dir)) + ) + except RuntimeError as exc: + logger.error("%s", exc) + sys.exit(1) if config.is_strands: try: _require_strands_sdk(harness=config.is_strands_harness) diff --git a/benchmarks/tests/test_run_swe_headless.py b/benchmarks/tests/test_run_swe_headless.py index 7107a23..eac98b0 100644 --- a/benchmarks/tests/test_run_swe_headless.py +++ b/benchmarks/tests/test_run_swe_headless.py @@ -2312,3 +2312,90 @@ def test_a_run_with_no_input_count_keeps_its_shape(self) -> None: metrics = {"input_tokens": 0, "output_tokens": 0} s = harness._summary_metrics(metrics, self._vllm(1000, 800), 0.0, agent="codex") self.assertEqual(s["input_tokens"], 0) + + +class OmpModelResolutionTest(unittest.TestCase): + """omp fuzzy-matches --model, so the harness must catch a substituted model.""" + + _HAIKU_5_5 = "us.anthropic.claude-haiku-5-5" + _HAIKU_4_5 = "us.anthropic.claude-haiku-4-5-20251001-v1:0" + + def _bedrock_config(self) -> RunnerConfig: + return _config( + agent="omp", provider="bedrock", endpoint=None, model=self._HAIKU_5_5 + ) + + def _events(self, *models: str) -> list[dict[str, object]]: + events: list[dict[str, object]] = [ + {"type": "message_end", "message": {"role": "user"}} + ] + for model in models: + events.append( + { + "type": "message_end", + "message": {"role": "assistant", "model": model}, + } + ) + return events + + def test_models_used_lists_assistant_models_once(self) -> None: + events = self._events(self._HAIKU_4_5, self._HAIKU_4_5) + self.assertEqual(harness._omp_models_used(events), [self._HAIKU_4_5]) + + def test_mismatch_marks_result_failed(self) -> None: + result: dict[str, object] = {"is_error": False, "subtype": "success"} + harness._flag_omp_model_mismatch(result, self._HAIKU_5_5, [self._HAIKU_4_5]) + self.assertEqual( + (result["is_error"], result["subtype"]), + (True, harness.OMP_MODEL_MISMATCH_SUBTYPE), + ) + + def test_matching_model_leaves_result_alone(self) -> None: + result: dict[str, object] = {"is_error": False, "subtype": "success"} + harness._flag_omp_model_mismatch(result, self._HAIKU_5_5, [self._HAIKU_5_5]) + self.assertEqual(result, {"is_error": False, "subtype": "success"}) + + def test_mismatch_is_not_retried(self) -> None: + summary = { + "ok": False, + "metrics": {"result_subtype": harness.OMP_MODEL_MISMATCH_SUBTYPE}, + } + self.assertFalse(harness._summary_is_retryable(summary)) + + def test_metrics_record_models_used(self) -> None: + result = {"usage": {}, "models_used": [self._HAIKU_4_5]} + metrics = harness._metrics_from_result(result, 1.0) + self.assertEqual(metrics["models_used"], [self._HAIKU_4_5]) + + def test_require_model_raises_when_catalog_lacks_id(self) -> None: + with mock.patch.object( + harness, "_omp_bedrock_catalog", return_value={self._HAIKU_4_5} + ): + with self.assertRaisesRegex(RuntimeError, "fuzzy-match"): + harness._require_omp_bedrock_model(self._bedrock_config(), {}) + + def test_require_model_passes_when_catalog_lists_id(self) -> None: + with mock.patch.object( + harness, "_omp_bedrock_catalog", return_value={self._HAIKU_5_5} + ): + self.assertIsNone( + harness._require_omp_bedrock_model(self._bedrock_config(), {}) + ) + + def test_catalog_parses_model_ids(self) -> None: + stdout = json.dumps({"models": [{"id": self._HAIKU_5_5}, {"name": "no id"}]}) + completed = mock.Mock(stdout=stdout) + with mock.patch.object(harness.subprocess, "run", return_value=completed): + self.assertEqual(harness._omp_bedrock_catalog({}), {self._HAIKU_5_5}) + + def test_catalog_without_models_list_raises_runtime_error(self) -> None: + completed = mock.Mock(stdout=json.dumps([{"id": self._HAIKU_5_5}])) + with mock.patch.object(harness.subprocess, "run", return_value=completed): + with self.assertRaisesRegex(RuntimeError, "no 'models' list"): + harness._omp_bedrock_catalog({}) + + def test_catalog_timeout_raises_runtime_error(self) -> None: + timeout = harness.subprocess.TimeoutExpired(cmd="omp", timeout=60) + with mock.patch.object(harness.subprocess, "run", side_effect=timeout): + with self.assertRaisesRegex(RuntimeError, "timed out"): + harness._omp_bedrock_catalog({})