diff --git a/envs/agent_world_model_env/server/_verifier_runner.py b/envs/agent_world_model_env/server/_verifier_runner.py index a90d39d4e4..53a8fb8c16 100644 --- a/envs/agent_world_model_env/server/_verifier_runner.py +++ b/envs/agent_world_model_env/server/_verifier_runner.py @@ -27,8 +27,17 @@ For mode == "code": {"result": "complete"|"others", "execution_status": "success"|"error", + "uses_final_answer"?: bool, "raw_result"?: , - "error_message"?: str} + "error_message"?: str, + "error_kind"?: "compile"|"not_found"|"execution"|"invalid_return"} + + ``uses_final_answer`` reports the verifier contract: whether the + verifier function consumes the final answer (a ``final_answer`` + parameter, or a positional parameter in that slot). ``error_kind`` + separates failures raised by the verifier's own code ("execution") + from broken verifier code; subprocess/infrastructure failures are + produced by the parent process and carry neither field. """ from __future__ import annotations @@ -291,6 +300,20 @@ def _sanitize_for_json(obj: Any) -> Any: return str(obj) +def _verifier_uses_final_answer(verify_func: Any) -> bool: + """Whether calling the verifier passes it the final answer. + + Mirrors the dispatch in _call_verifier_func: the answer is passed + when the function declares a ``final_answer`` parameter, or when it + is called positionally with at least three parameters. + """ + params = inspect.signature(verify_func).parameters + if "final_answer" in params: + return True + named = {"initial_db_path", "final_db_path"} & params.keys() + return not named and len(params) >= 3 + + def _call_verifier_func( verify_func: Any, initial_db_path: str, @@ -341,6 +364,7 @@ def _run(payload: dict[str, Any]) -> dict[str, Any]: return { "result": "others", "execution_status": "error", + "error_kind": "compile", "error_message": f"Compile error: {e}", } return {"execution_status": "error", "error_message": f"Compile error: {e}"} @@ -351,6 +375,7 @@ def _run(payload: dict[str, Any]) -> dict[str, Any]: return { "result": "others", "execution_status": "error", + "error_kind": "not_found", "error_message": f"Function '{function_name}' not found", } return { @@ -367,6 +392,7 @@ def _run(payload: dict[str, Any]) -> dict[str, Any]: return { "result": "others", "execution_status": "error", + "error_kind": "execution", "error_message": f"Execution error: {e}", } return {"execution_status": "error", "error_message": f"Execution error: {e}"} @@ -376,6 +402,7 @@ def _run(payload: dict[str, Any]) -> dict[str, Any]: return { "result": "others", "execution_status": "error", + "error_kind": "invalid_return", "error_message": f"Invalid return format: {type(result).__name__}", } result_value = result.get("result", "others") @@ -384,6 +411,7 @@ def _run(payload: dict[str, Any]) -> dict[str, Any]: return { "result": result_value, "execution_status": "success", + "uses_final_answer": _verifier_uses_final_answer(verify_func), "raw_result": _sanitize_for_json(result), } diff --git a/envs/agent_world_model_env/server/verifier.py b/envs/agent_world_model_env/server/verifier.py index ae69469058..21e8af92bc 100644 --- a/envs/agent_world_model_env/server/verifier.py +++ b/envs/agent_world_model_env/server/verifier.py @@ -10,9 +10,11 @@ import logging import os import re +import sqlite3 import subprocess import sys import textwrap +from pathlib import Path from typing import Any from openai import AsyncOpenAI @@ -184,6 +186,18 @@ def execute_code_verifier( return result +def _database_contents_equal(initial_db_path: str, final_db_path: str) -> bool: + """Compare SQLite contents, including committed WAL changes.""" + dumps = [] + for path in (initial_db_path, final_db_path): + conn = sqlite3.connect(Path(path).resolve().as_uri() + "?mode=ro", uri=True) + try: + dumps.append("\n".join(conn.iterdump())) + finally: + conn.close() + return dumps[0] == dumps[1] + + def run_verifier( verifier_entry: dict, verifier_mode: str, @@ -219,7 +233,45 @@ def run_verifier( ) if result.get("execution_status") == "error": return "judge_error", result - return result.get("result", "others"), result + reward_type = result.get("result", "others") + if reward_type == "complete": + try: + unchanged = _database_contents_equal(initial_db_path, final_db_path) + except (OSError, sqlite3.Error) as exc: + return "judge_error", { + "execution_status": "error", + "error_message": f"Database comparison failed: {exc}", + } + if unchanged: + if ( + final_answer + and final_answer.strip() + and result.get("uses_final_answer") is not False + ): + # Answer-task baseline contract: rerun the same + # verifier with an empty answer on the same databases. + # The two runs differ only in the answer, so a + # different outcome is the answer's doing: + # - baseline "others": the answer earned the pass. + # - any baseline error fails closed as judge_error. + # Verifiers for answer tasks must return "others" + # for an empty answer before parsing it (for + # example before float(final_answer) or + # json.loads(final_answer)). An execution error + # during the baseline, whether from parsing, a + # locked database, or an unrelated conversion, + # cannot be attributed to the answer, so it never + # preserves a pass. + baseline = execute_code_verifier( + code, func_name, initial_db_path, final_db_path, None + ) + if baseline.get("execution_status") == "error": + return "judge_error", baseline + if baseline.get("result") == "others": + return reward_type, result + result = {**result, "result": "others", "no_db_change": True} + return "others", result + return reward_type, result result = execute_sql_verifier(code, func_name, initial_db_path, final_db_path) if isinstance(result, dict) and result.get("execution_status") == "error": diff --git a/tests/envs/test_awm_environment.py b/tests/envs/test_awm_environment.py index a4d8bbcb5b..c8279310d8 100644 --- a/tests/envs/test_awm_environment.py +++ b/tests/envs/test_awm_environment.py @@ -483,8 +483,10 @@ def test_run_verifier_code_mode(self): } with tempfile.TemporaryDirectory() as td: initial, final = self._setup_db(td) - reward_type, result = run_verifier(verifier_entry, "code", initial, final) - assert reward_type == "complete" + reward_type, result = run_verifier( + verifier_entry, "code", initial, final, final_answer="done" + ) + assert reward_type == "others" def test_run_verifier_code_mode_2arg(self): """Bug #8: run_verifier in code mode with 2-arg function name.""" @@ -496,10 +498,299 @@ def test_run_verifier_code_mode_2arg(self): ), }, } + with tempfile.TemporaryDirectory() as td: + initial, final = self._setup_db(td) + reward_type, result = run_verifier( + verifier_entry, "code", initial, final, final_answer="done" + ) + assert reward_type == "others" + + def test_run_verifier_code_mode_no_db_change_is_not_complete(self): + """A pass that was already true at reset should not pay out (#1475). + + This verifier only checks the final db, and the seeded data + already has the row, so it says complete on an untouched db. + """ + verifier_entry = { + "verification": { + "code": ( + "def verify_task_completion(initial_db_path, final_db_path, final_answer):\n" + " import sqlite3\n" + " conn = sqlite3.connect(final_db_path)\n" + " count = conn.execute('SELECT COUNT(*) FROM products').fetchone()[0]\n" + " conn.close()\n" + " if count >= 1:\n" + " return {'result': 'complete'}\n" + " return {'result': 'others'}\n" + ), + }, + } with tempfile.TemporaryDirectory() as td: initial, final = self._setup_db(td) reward_type, result = run_verifier(verifier_entry, "code", initial, final) + assert reward_type == "others" + assert result["result"] == "others" + assert result["no_db_change"] is True + + def test_run_verifier_code_mode_complete_after_db_change(self): + """Same style of verifier, but the agent did change the db.""" + verifier_entry = { + "verification": { + "code": ( + "def verify_task_completion(initial_db_path, final_db_path, final_answer):\n" + " import sqlite3\n" + " conn = sqlite3.connect(final_db_path)\n" + " count = conn.execute('SELECT COUNT(*) FROM products').fetchone()[0]\n" + " conn.close()\n" + " if count > 2:\n" + " return {'result': 'complete'}\n" + " return {'result': 'others'}\n" + ), + }, + } + with tempfile.TemporaryDirectory() as td: + initial, final = self._setup_db(td) + conn = sqlite3.connect(final) + conn.execute( + "INSERT INTO products (id, name, price) VALUES (3, 'Item3', 30.0)" + ) + conn.commit() + conn.close() + reward_type, result = run_verifier(verifier_entry, "code", initial, final) + assert reward_type == "complete" + + def test_run_verifier_code_mode_no_change_with_answer_stays_complete(self): + """Answer-based tasks do not change the db, so they keep their pass.""" + verifier_entry = { + "verification": { + "code": ( + "def verify_task_completion(initial_db_path, final_db_path, final_answer):\n" + " if final_answer == 'Widget':\n" + " return {'result': 'complete'}\n" + " return {'result': 'others'}\n" + ), + }, + } + with tempfile.TemporaryDirectory() as td: + initial, final = self._setup_db(td) + reward_type, result = run_verifier( + verifier_entry, "code", initial, final, final_answer="Widget" + ) + assert reward_type == "complete" + + def test_run_verifier_numeric_answer_stays_complete(self): + """Guarded numeric answer verifiers keep their pass on an untouched db. + + The verifier returns 'others' for an empty answer before calling + float(), so the no-answer baseline evaluates cleanly to 'others' + and the answer earns the pass. + """ + verifier_entry = { + "verification": { + "code": ( + "def verify_task_completion(initial_db_path, final_db_path, final_answer):\n" + " if not final_answer:\n" + " return {'result': 'others'}\n" + " if float(final_answer) == 42:\n" + " return {'result': 'complete'}\n" + " return {'result': 'others'}\n" + ), + }, + } + with tempfile.TemporaryDirectory() as td: + initial, final = self._setup_db(td) + reward_type, result = run_verifier( + verifier_entry, "code", initial, final, final_answer="42" + ) + assert reward_type == "complete" + assert result["result"] == "complete" + + def test_run_verifier_json_answer_stays_complete(self): + """Guarded JSON answer verifiers keep their pass on an untouched db. + + The verifier returns 'others' for an empty answer before calling + json.loads(), so the baseline evaluates cleanly to 'others'. + """ + verifier_entry = { + "verification": { + "code": ( + "def verify_task_completion(initial_db_path, final_db_path, final_answer):\n" + " import json\n" + " if not final_answer:\n" + " return {'result': 'others'}\n" + " if json.loads(final_answer)['value'] == 42:\n" + " return {'result': 'complete'}\n" + " return {'result': 'others'}\n" + ), + }, + } + with tempfile.TemporaryDirectory() as td: + initial, final = self._setup_db(td) + reward_type, result = run_verifier( + verifier_entry, "code", initial, final, final_answer='{"value": 42}' + ) assert reward_type == "complete" + assert result["result"] == "complete" + + def test_run_verifier_db_verifier_ignores_answer_text(self): + """A db-state verifier gains nothing from answer text (#1475). + + The verifier only reads the seeded rows and ignores the answer, + so a no-op agent that returns text still does not get the pass. + """ + verifier_entry = { + "verification": { + "code": ( + "def verify_task_completion(initial_db_path, final_db_path, final_answer):\n" + " import sqlite3\n" + " conn = sqlite3.connect(final_db_path)\n" + " count = conn.execute('SELECT COUNT(*) FROM products').fetchone()[0]\n" + " conn.close()\n" + " if count >= 1:\n" + " return {'result': 'complete'}\n" + " return {'result': 'others'}\n" + ), + }, + } + with tempfile.TemporaryDirectory() as td: + initial, final = self._setup_db(td) + reward_type, result = run_verifier( + verifier_entry, "code", initial, final, final_answer="done" + ) + assert reward_type == "others" + assert result["no_db_change"] is True + + def test_run_verifier_baseline_infra_error_fails_closed(self, monkeypatch): + """Infrastructure failure in the baseline run stays judge_error. + + Every baseline error fails closed, so a subprocess timeout in + the baseline preserves no pass either. + """ + import envs.agent_world_model_env.server.verifier as verifier_module + + calls = [] + + def fake_execute_code_verifier( + code, func_name, initial_db_path, final_db_path, final_answer=None + ): + calls.append(final_answer) + if len(calls) == 1: + return { + "result": "complete", + "execution_status": "success", + "uses_final_answer": True, + } + return { + "result": "others", + "execution_status": "error", + "error_message": "Verifier timed out after 20.0s", + } + + monkeypatch.setattr( + verifier_module, "execute_code_verifier", fake_execute_code_verifier + ) + verifier_entry = { + "verification": { + "code": ( + "def verify_task_completion(initial_db_path, final_db_path, final_answer):\n" + " return {'result': 'complete'}\n" + ), + }, + } + with tempfile.TemporaryDirectory() as td: + initial, final = self._setup_db(td) + reward_type, result = run_verifier( + verifier_entry, "code", initial, final, final_answer="42" + ) + assert reward_type == "judge_error" + assert result["execution_status"] == "error" + + def test_run_verifier_baseline_execution_error_fails_closed(self, monkeypatch): + """A baseline error raised by verifier code still fails closed. + + Regression for the two reproductions on #1477: an exclusive + SQLite lock during the baseline ("database is locked") and an + unrelated database-value conversion raising ValueError both + surface as execution errors. Neither can be attributed to the + answer, so neither preserves the pass. + """ + import envs.agent_world_model_env.server.verifier as verifier_module + + verifier_entry = { + "verification": { + "code": ( + "def verify_task_completion(initial_db_path, final_db_path, final_answer):\n" + " return {'result': 'complete'}\n" + ), + }, + } + for error_message in ( + "database is locked", + "could not convert string to float: 'n/a'", + ): + calls = [] + + def fake_execute_code_verifier( + code, + func_name, + initial_db_path, + final_db_path, + final_answer=None, + _message=error_message, + ): + calls.append(final_answer) + if len(calls) == 1: + return { + "result": "complete", + "execution_status": "success", + "uses_final_answer": True, + } + return { + "result": "others", + "execution_status": "error", + "error_kind": "execution", + "error_message": _message, + } + + monkeypatch.setattr( + verifier_module, "execute_code_verifier", fake_execute_code_verifier + ) + with tempfile.TemporaryDirectory() as td: + initial, final = self._setup_db(td) + reward_type, result = run_verifier( + verifier_entry, "code", initial, final, final_answer="42" + ) + assert reward_type == "judge_error" + assert result["execution_status"] == "error" + assert result["error_kind"] == "execution" + + def test_run_verifier_corrupt_db_fails_closed(self): + entry = { + "verification": { + "code": "def verify_task(a, b, answer):\n return {'result': 'complete'}\n" + } + } + with tempfile.TemporaryDirectory() as td: + initial, final = self._setup_db(td) + with open(final, "wb") as db: + db.write(b"not a database") + reward, result = run_verifier(entry, "code", initial, final) + assert reward == "judge_error" + assert result["execution_status"] == "error" + + def test_database_comparison_reads_wal(self): + from envs.agent_world_model_env.server.verifier import _database_contents_equal + + with tempfile.TemporaryDirectory() as td: + initial, final = self._setup_db(td) + conn = sqlite3.connect(final) + try: + conn.execute("PRAGMA journal_mode=WAL") + conn.execute("UPDATE products SET price = 99") + conn.commit() + assert not _database_contents_equal(initial, final) + finally: + conn.close() def test_run_verifier_no_code(self): verifier_entry = {"verification": {"code": ""}}