diff --git a/src/benchflow/diagnostics.py b/src/benchflow/diagnostics.py index f00c41cc..17022a19 100644 --- a/src/benchflow/diagnostics.py +++ b/src/benchflow/diagnostics.py @@ -93,8 +93,8 @@ class Diagnostic: summary_description: ClassVar[str] = "" def to_dict(self) -> dict[str, Any]: - """Serialize for result.json — drops Nones is the caller's choice.""" - return asdict(self) + """Serialize for result.json, omitting unpopulated optional fields.""" + return {k: v for k, v in asdict(self).items() if v is not None} def format_issue(self, task_name: str) -> str: """Render the per-task line check_results emits for this diagnostic.""" @@ -237,24 +237,7 @@ def to_dict(self) -> dict[str, Any]: the same way so result.json stays compact and tests that compare explicit dicts continue to work. """ - raw = asdict(self) - out: dict[str, Any] = {} - for k, v in raw.items(): - if v is None and k in { - "stderr_snippet", - "process_pid", - "sandbox_reachable", - "sandbox_probe_rc", - "sandbox_probe_stdout", - "sandbox_probe_error", - "sandbox_probe_error_type", - "sandbox_probe_traceback", - }: - continue - if k == "raw_message" and not v: - continue - out[k] = v - return out + return {k: v for k, v in super().to_dict().items() if k != "raw_message" or v} @dataclass diff --git a/src/benchflow/sandbox/daytona_dind.py b/src/benchflow/sandbox/daytona_dind.py index d8fd7edc..11c44bbd 100644 --- a/src/benchflow/sandbox/daytona_dind.py +++ b/src/benchflow/sandbox/daytona_dind.py @@ -96,6 +96,15 @@ def _raise_if_startup_provider_error(env: DaytonaSandbox, exc: BaseException) -> ) from exc +def _startup_config_error(env: DaytonaSandbox, message: str) -> SandboxStartupError: + return SandboxStartupError( + message, + sandbox_state="config_error", + attempts=0, + build_timeout_sec=env.task_env_config.build_timeout_sec, + ) + + def _with_long_exec_heartbeat(command: str, timeout_sec: int | None) -> str: if ( timeout_sec is None @@ -346,8 +355,12 @@ async def start(self, force_build: bool) -> None: env._client_manager = await _sdk.DaytonaClientManager.get_instance() - dind_image: str = env._kwargs.get("dind_image", "docker:28.3.3-dind") - dind_snapshot: str | None = env._kwargs.get("dind_snapshot") + dind_image = env._kwargs.get("dind_image", "docker:28.3.3-dind") + if not isinstance(dind_image, str): + raise _startup_config_error(env, "dind_image must be a string") + dind_snapshot = env._kwargs.get("dind_snapshot") + if dind_snapshot is not None and not isinstance(dind_snapshot, str): + raise _startup_config_error(env, "dind_snapshot must be a string") params: _SandboxParams if dind_snapshot: diff --git a/tests/test_acp.py b/tests/test_acp.py index 403246a9..dfdf3e3e 100644 --- a/tests/test_acp.py +++ b/tests/test_acp.py @@ -1766,6 +1766,64 @@ def test_diagnostic_reason_constants_share_scoring_source(self) -> None: DIAGNOSTIC_REASON_TRANSPORT_CLOSED, ) + def test_diagnostic_serialization_drops_none_fields_uniformly(self) -> None: + """Guards the fix from PR #895 for issue #531: diagnostic info dicts use one None-policy.""" + from benchflow.diagnostics import ( + AgentPromptTimeoutDiagnostic, + IdleTimeoutDiagnostic, + ProviderApiErrorDiagnostic, + SandboxStartupDiagnostic, + SuspectedApiErrorDiagnostic, + TransportClosedDiagnostic, + VerifierTimeoutDiagnostic, + ) + + expected_keys_by_cls = { + IdleTimeoutDiagnostic: { + "reason", + "idle_timeout_sec", + "idle_duration_sec", + "wall_clock_elapsed_sec", + "n_tool_calls", + "n_message_chunks", + "n_thought_chunks", + "last_activity_at", + }, + AgentPromptTimeoutDiagnostic: { + "reason", + "timeout_sec", + "n_tool_calls", + "pending_tool_call_ids", + "terminal_event_recorded", + "terminal_trajectory_complete", + }, + SandboxStartupDiagnostic: {"reason", "attempts", "raw_message"}, + TransportClosedDiagnostic: {"reason", "transport_diagnosis"}, + VerifierTimeoutDiagnostic: { + "timeout_budget_sec", + "elapsed_sec", + "task_name", + }, + ProviderApiErrorDiagnostic: { + "subcategory", + "transient", + "total_requests", + "failed_requests", + "fingerprint", + }, + SuspectedApiErrorDiagnostic: { + "total_tokens", + "n_tool_calls", + "total_requests", + "failed_requests", + }, + } + + for diag_cls, expected_keys in expected_keys_by_cls.items(): + payload = diag_cls().to_dict() + assert set(payload) == expected_keys + assert all(value is not None for value in payload.values()) + def test_summary_warning_uses_registry_metadata(self) -> None: """Summary warning text comes from the registry's ``summary_description``, not a per-call f-string in evaluation.py.""" diff --git a/tests/test_daytona_dind_compose_up.py b/tests/test_daytona_dind_compose_up.py index 96c92e57..0b0e032e 100644 --- a/tests/test_daytona_dind_compose_up.py +++ b/tests/test_daytona_dind_compose_up.py @@ -149,6 +149,20 @@ def test_daytona_sdk_error_during_dind_start_is_structured_startup_error(): assert "Daytona DinD startup failed" in str(raised.value) +def test_daytona_dind_type_guard_uses_structured_startup_error(): + """Guards the follow-up fix from PR #895 for invalid DinD config values.""" + from benchflow.sandbox.daytona_dind import _startup_config_error + + env = SimpleNamespace( + task_env_config=SimpleNamespace(build_timeout_sec=1800), + ) + err = _startup_config_error(env, "dind_image must be a string") + assert isinstance(err, SandboxStartupError) + assert err.diagnostic.sandbox_state == "config_error" + assert err.diagnostic.build_timeout_sec == 1800 + assert "dind_image must be a string" in err.diagnostic.raw_message + + def test_long_exec_heartbeat_only_wraps_long_commands(): command = "python preprocess.py" assert _with_long_exec_heartbeat(command, 599) == command