From 55ed3efc48ddd6fe5ee2c16c83ecdb05971cf73f Mon Sep 17 00:00:00 2001 From: Nitjsefnie Date: Sat, 15 Aug 2026 11:03:47 +0200 Subject: [PATCH 1/2] fix(exceptions): record explicitly-supplied falsy details instead of dropping them (#280) Detail-recording guards used truthiness tests, so an explicitly passed "", 0, False or [] was silently dropped from .details. Only None means "not supplied". Switch all 21 guards in the exceptions package to `is not None`, matching the fix that landed for #68 in PR #279. Message-text construction guards and __str__ formatting are unchanged. Co-Authored-By: Kimi K3 --- openagent_eval/exceptions/cli.py | 2 +- openagent_eval/exceptions/config.py | 4 +- openagent_eval/exceptions/corpus.py | 8 +- openagent_eval/exceptions/dataset.py | 6 +- openagent_eval/exceptions/metric.py | 6 +- openagent_eval/exceptions/plugin.py | 6 +- openagent_eval/exceptions/provider.py | 8 +- openagent_eval/exceptions/synthesis.py | 2 +- tests/unit/test_exceptions.py | 192 +++++++++++++++++++++++++ 9 files changed, 213 insertions(+), 21 deletions(-) diff --git a/openagent_eval/exceptions/cli.py b/openagent_eval/exceptions/cli.py index caf9503..7b19901 100644 --- a/openagent_eval/exceptions/cli.py +++ b/openagent_eval/exceptions/cli.py @@ -28,7 +28,7 @@ def __init__( details: Additional context about the error. """ error_details = details or {} - if command: + if command is not None: error_details["command"] = command super().__init__(message=message, details=error_details) diff --git a/openagent_eval/exceptions/config.py b/openagent_eval/exceptions/config.py index 4b85ff5..a32adc6 100644 --- a/openagent_eval/exceptions/config.py +++ b/openagent_eval/exceptions/config.py @@ -31,9 +31,9 @@ def __init__( details: Additional context about the error. """ error_details = details or {} - if config_path: + if config_path is not None: error_details["config_path"] = config_path - if field: + if field is not None: error_details["field"] = field super().__init__(message=message, details=error_details) diff --git a/openagent_eval/exceptions/corpus.py b/openagent_eval/exceptions/corpus.py index 4559af2..351e48f 100644 --- a/openagent_eval/exceptions/corpus.py +++ b/openagent_eval/exceptions/corpus.py @@ -28,7 +28,7 @@ def __init__( details: Additional context about the error. """ error_details = details or {} - if corpus_path: + if corpus_path is not None: error_details["corpus_path"] = corpus_path super().__init__(message=message, details=error_details) @@ -72,7 +72,7 @@ def __init__( details: Additional context about the error. """ error_details = details or {} - if validation_errors: + if validation_errors is not None: error_details["validation_errors"] = validation_errors super().__init__(message=message, corpus_path=corpus_path, details=error_details) @@ -100,9 +100,9 @@ def __init__( details: Additional context about the error. """ error_details = details or {} - if analyzer_name: + if analyzer_name is not None: error_details["analyzer_name"] = analyzer_name - if original_error: + if original_error is not None: error_details["original_error"] = str(original_error) super().__init__(message=message, corpus_path=corpus_path, details=error_details) diff --git a/openagent_eval/exceptions/dataset.py b/openagent_eval/exceptions/dataset.py index 0663936..15d276d 100644 --- a/openagent_eval/exceptions/dataset.py +++ b/openagent_eval/exceptions/dataset.py @@ -28,7 +28,7 @@ def __init__( details: Additional context about the error. """ error_details = details or {} - if dataset_path: + if dataset_path is not None: error_details["dataset_path"] = dataset_path super().__init__(message=message, details=error_details) @@ -79,7 +79,7 @@ def __init__( details: Additional context about the error. """ error_details = details or {} - if data_format: + if data_format is not None: error_details["format"] = data_format if line_number is not None: error_details["line_number"] = line_number @@ -112,7 +112,7 @@ def __init__( details: Additional context about the error. """ error_details = details or {} - if validation_errors: + if validation_errors is not None: error_details["validation_errors"] = validation_errors super().__init__(message=message, dataset_path=dataset_path, details=error_details) diff --git a/openagent_eval/exceptions/metric.py b/openagent_eval/exceptions/metric.py index b43ee7e..c4455f6 100644 --- a/openagent_eval/exceptions/metric.py +++ b/openagent_eval/exceptions/metric.py @@ -28,7 +28,7 @@ def __init__( details: Additional context about the error. """ error_details = details or {} - if metric_name: + if metric_name is not None: error_details["metric_name"] = metric_name super().__init__(message=message, details=error_details) @@ -52,7 +52,7 @@ def __init__( details: Additional context about the error. """ error_details = details or {} - if available_metrics: + if available_metrics is not None: error_details["available_metrics"] = available_metrics message = f"Metric not found: {metric_name}" @@ -86,7 +86,7 @@ def __init__( details: Additional context about the error. """ error_details = details or {} - if original_error: + if original_error is not None: error_details["original_error"] = str(original_error) super().__init__(message=message, metric_name=metric_name, details=error_details) diff --git a/openagent_eval/exceptions/plugin.py b/openagent_eval/exceptions/plugin.py index 2eef347..8a4bb5b 100644 --- a/openagent_eval/exceptions/plugin.py +++ b/openagent_eval/exceptions/plugin.py @@ -28,7 +28,7 @@ def __init__( details: Additional context about the error. """ error_details = details or {} - if plugin_name: + if plugin_name is not None: error_details["plugin_name"] = plugin_name super().__init__(message=message, details=error_details) @@ -52,7 +52,7 @@ def __init__( details: Additional context about the error. """ error_details = details or {} - if available_plugins: + if available_plugins is not None: error_details["available_plugins"] = available_plugins message = f"Plugin not found: {plugin_name}" @@ -86,7 +86,7 @@ def __init__( details: Additional context about the error. """ error_details = details or {} - if original_error: + if original_error is not None: error_details["original_error"] = str(original_error) super().__init__(message=message, plugin_name=plugin_name, details=error_details) diff --git a/openagent_eval/exceptions/provider.py b/openagent_eval/exceptions/provider.py index a6ed894..53028a3 100644 --- a/openagent_eval/exceptions/provider.py +++ b/openagent_eval/exceptions/provider.py @@ -31,7 +31,7 @@ def __init__( details: Additional context about the error. """ error_details = details or {} - if provider_name: + if provider_name is not None: error_details["provider_name"] = provider_name super().__init__(message=message, details=error_details) @@ -68,7 +68,7 @@ def __init__( details: Additional context about the error. """ error_details = details or {} - if available_providers: + if available_providers is not None: error_details["available_providers"] = available_providers message = f"Provider not found: {provider_name}" @@ -104,7 +104,7 @@ def __init__( details: Additional context about the error. """ error_details = details or {} - if original_error: + if original_error is not None: error_details["original_error"] = str(original_error) super().__init__(message=message, provider_name=provider_name, details=error_details) @@ -136,7 +136,7 @@ def __init__( details: Additional context about the error. """ error_details = details or {} - if original_error: + if original_error is not None: error_details["original_error"] = str(original_error) super().__init__(message=message, provider_name=provider_name, details=error_details) diff --git a/openagent_eval/exceptions/synthesis.py b/openagent_eval/exceptions/synthesis.py index 165294f..196d42c 100644 --- a/openagent_eval/exceptions/synthesis.py +++ b/openagent_eval/exceptions/synthesis.py @@ -48,7 +48,7 @@ def __init__( details: Additional context about the error. """ error_details = details or {} - if original_error: + if original_error is not None: error_details["original_error"] = str(original_error) super().__init__(message=message, details=error_details) diff --git a/tests/unit/test_exceptions.py b/tests/unit/test_exceptions.py index 309c1f2..8ec645a 100644 --- a/tests/unit/test_exceptions.py +++ b/tests/unit/test_exceptions.py @@ -6,6 +6,9 @@ CLIError, CommandError, ConfigurationError, + CorpusAuditError, + CorpusError, + CorpusValidationError, DatasetError, DatasetNotFoundError, DatasetValidationError, @@ -23,6 +26,7 @@ ProviderError, ProviderExecutionError, ProviderNotFoundError, + SynthesisExecutionError, ValidationError, ) @@ -278,3 +282,191 @@ def test_validation_error_omits_none_field_and_value(self) -> None: assert "value" not in error.details assert "field" not in str(error) assert "value" not in str(error) + + +class TestFalsyDetailRecording: + """Explicitly-supplied falsy values (not None) must be recorded in .details. + + Guards must distinguish "not supplied" (None) from a falsy value such as + "", 0, False or []. Only None may be dropped from .details. + """ + + # --- config.py --- + + def test_configuration_error_preserves_empty_config_path(self) -> None: + error = ConfigurationError("Invalid config", config_path="") + assert "config_path" in error.details + + def test_configuration_error_preserves_empty_field(self) -> None: + error = ConfigurationError("Invalid config", field="") + assert "field" in error.details + + def test_configuration_error_omits_none_parameters(self) -> None: + error = ConfigurationError("Invalid config") + assert "config_path" not in error.details + assert "field" not in error.details + + # --- cli.py --- + + def test_cli_error_preserves_empty_command(self) -> None: + error = CLIError("Invalid command", command="") + assert "command" in error.details + + def test_cli_error_omits_none_command(self) -> None: + error = CLIError("Invalid command") + assert "command" not in error.details + + # --- corpus.py --- + + def test_corpus_error_preserves_empty_corpus_path(self) -> None: + error = CorpusError("Corpus error", corpus_path="") + assert "corpus_path" in error.details + + def test_corpus_error_omits_none_corpus_path(self) -> None: + error = CorpusError("Corpus error") + assert "corpus_path" not in error.details + + def test_corpus_validation_error_preserves_empty_validation_errors(self) -> None: + error = CorpusValidationError("Validation failed", validation_errors=[]) + assert "validation_errors" in error.details + + def test_corpus_validation_error_omits_none_validation_errors(self) -> None: + error = CorpusValidationError("Validation failed") + assert "validation_errors" not in error.details + + def test_corpus_audit_error_preserves_empty_analyzer_name(self) -> None: + error = CorpusAuditError("Audit failed", analyzer_name="") + assert "analyzer_name" in error.details + + def test_corpus_audit_error_records_original_error(self) -> None: + error = CorpusAuditError("Audit failed", original_error=ValueError("boom")) + assert "original_error" in error.details + + def test_corpus_audit_error_omits_none_parameters(self) -> None: + error = CorpusAuditError("Audit failed") + assert "analyzer_name" not in error.details + assert "original_error" not in error.details + + # --- dataset.py --- + + def test_dataset_error_preserves_empty_dataset_path(self) -> None: + error = DatasetError("Dataset error", dataset_path="") + assert "dataset_path" in error.details + + def test_dataset_error_omits_none_dataset_path(self) -> None: + error = DatasetError("Dataset error") + assert "dataset_path" not in error.details + + def test_invalid_dataset_error_preserves_empty_data_format(self) -> None: + error = InvalidDatasetError("Invalid format", data_format="") + assert "format" in error.details + + def test_invalid_dataset_error_omits_none_data_format(self) -> None: + error = InvalidDatasetError("Invalid format") + assert "format" not in error.details + + def test_dataset_validation_error_preserves_empty_validation_errors(self) -> None: + error = DatasetValidationError("Validation failed", validation_errors=[]) + assert "validation_errors" in error.details + + def test_dataset_validation_error_omits_none_validation_errors(self) -> None: + error = DatasetValidationError("Validation failed") + assert "validation_errors" not in error.details + + # --- metric.py --- + + def test_metric_error_preserves_empty_metric_name(self) -> None: + error = MetricError("Metric error", metric_name="") + assert "metric_name" in error.details + + def test_metric_error_omits_none_metric_name(self) -> None: + error = MetricError("Metric error") + assert "metric_name" not in error.details + + def test_metric_not_found_error_preserves_empty_available_metrics(self) -> None: + error = MetricNotFoundError("my_metric", available_metrics=[]) + assert "available_metrics" in error.details + + def test_metric_not_found_error_omits_none_available_metrics(self) -> None: + error = MetricNotFoundError("my_metric") + assert "available_metrics" not in error.details + + def test_metric_execution_error_records_original_error(self) -> None: + error = MetricExecutionError("Failed", original_error=ValueError("boom")) + assert "original_error" in error.details + + def test_metric_execution_error_omits_none_original_error(self) -> None: + error = MetricExecutionError("Failed") + assert "original_error" not in error.details + + # --- plugin.py --- + + def test_plugin_error_preserves_empty_plugin_name(self) -> None: + error = PluginError("Plugin error", plugin_name="") + assert "plugin_name" in error.details + + def test_plugin_error_omits_none_plugin_name(self) -> None: + error = PluginError("Plugin error") + assert "plugin_name" not in error.details + + def test_plugin_not_found_error_preserves_empty_available_plugins(self) -> None: + error = PluginNotFoundError("my_plugin", available_plugins=[]) + assert "available_plugins" in error.details + + def test_plugin_not_found_error_omits_none_available_plugins(self) -> None: + error = PluginNotFoundError("my_plugin") + assert "available_plugins" not in error.details + + def test_plugin_load_error_records_original_error(self) -> None: + error = PluginLoadError("Failed to load", original_error=ImportError("boom")) + assert "original_error" in error.details + + def test_plugin_load_error_omits_none_original_error(self) -> None: + error = PluginLoadError("Failed to load") + assert "original_error" not in error.details + + # --- provider.py --- + + def test_provider_error_preserves_empty_provider_name(self) -> None: + error = ProviderError("Provider error", provider_name="") + assert "provider_name" in error.details + + def test_provider_error_omits_none_provider_name(self) -> None: + error = ProviderError("Provider error") + assert "provider_name" not in error.details + + def test_provider_not_found_error_preserves_empty_available_providers(self) -> None: + error = ProviderNotFoundError("my_provider", available_providers=[]) + assert "available_providers" in error.details + + def test_provider_not_found_error_omits_none_available_providers(self) -> None: + error = ProviderNotFoundError("my_provider") + assert "available_providers" not in error.details + + def test_provider_connection_error_records_original_error(self) -> None: + error = ProviderConnectionError( + "Failed to connect", original_error=ConnectionError("boom") + ) + assert "original_error" in error.details + + def test_provider_connection_error_omits_none_original_error(self) -> None: + error = ProviderConnectionError("Failed to connect") + assert "original_error" not in error.details + + def test_provider_execution_error_records_original_error(self) -> None: + error = ProviderExecutionError("API call failed", original_error=RuntimeError("boom")) + assert "original_error" in error.details + + def test_provider_execution_error_omits_none_original_error(self) -> None: + error = ProviderExecutionError("API call failed") + assert "original_error" not in error.details + + # --- synthesis.py --- + + def test_synthesis_execution_error_records_original_error(self) -> None: + error = SynthesisExecutionError("Synthesis failed", original_error=RuntimeError("boom")) + assert "original_error" in error.details + + def test_synthesis_execution_error_omits_none_original_error(self) -> None: + error = SynthesisExecutionError("Synthesis failed") + assert "original_error" not in error.details From 6aa4183dc5a5dc0eca039f67b7c4226761700eb6 Mon Sep 17 00:00:00 2001 From: Nitjsefnie Date: Sat, 15 Aug 2026 11:26:15 +0200 Subject: [PATCH 2/2] test(exceptions): cover falsy original errors Co-Authored-By: GPT-5.6-Luna --- tests/unit/test_exceptions.py | 43 +++++++++++++++++++++++++++++++++++ 1 file changed, 43 insertions(+) diff --git a/tests/unit/test_exceptions.py b/tests/unit/test_exceptions.py index 8ec645a..bba486f 100644 --- a/tests/unit/test_exceptions.py +++ b/tests/unit/test_exceptions.py @@ -31,6 +31,13 @@ ) +class FalsyException(Exception): + """Exception whose truthiness is false despite being an exception.""" + + def __bool__(self) -> bool: + return False + + class TestOpenAgentEvalError: """Tests for the base exception class.""" @@ -342,6 +349,12 @@ def test_corpus_audit_error_records_original_error(self) -> None: error = CorpusAuditError("Audit failed", original_error=ValueError("boom")) assert "original_error" in error.details + def test_corpus_audit_error_preserves_falsy_original_error(self) -> None: + original_error = FalsyException("boom") + error = CorpusAuditError("Audit failed", original_error=original_error) + assert "original_error" in error.details + assert error.original_error is original_error + def test_corpus_audit_error_omits_none_parameters(self) -> None: error = CorpusAuditError("Audit failed") assert "analyzer_name" not in error.details @@ -395,6 +408,12 @@ def test_metric_execution_error_records_original_error(self) -> None: error = MetricExecutionError("Failed", original_error=ValueError("boom")) assert "original_error" in error.details + def test_metric_execution_error_preserves_falsy_original_error(self) -> None: + original_error = FalsyException("boom") + error = MetricExecutionError("Failed", original_error=original_error) + assert "original_error" in error.details + assert error.original_error is original_error + def test_metric_execution_error_omits_none_original_error(self) -> None: error = MetricExecutionError("Failed") assert "original_error" not in error.details @@ -421,6 +440,12 @@ def test_plugin_load_error_records_original_error(self) -> None: error = PluginLoadError("Failed to load", original_error=ImportError("boom")) assert "original_error" in error.details + def test_plugin_load_error_preserves_falsy_original_error(self) -> None: + original_error = FalsyException("boom") + error = PluginLoadError("Failed to load", original_error=original_error) + assert "original_error" in error.details + assert error.original_error is original_error + def test_plugin_load_error_omits_none_original_error(self) -> None: error = PluginLoadError("Failed to load") assert "original_error" not in error.details @@ -449,6 +474,12 @@ def test_provider_connection_error_records_original_error(self) -> None: ) assert "original_error" in error.details + def test_provider_connection_error_preserves_falsy_original_error(self) -> None: + original_error = FalsyException("boom") + error = ProviderConnectionError("Failed to connect", original_error=original_error) + assert "original_error" in error.details + assert error.original_error is original_error + def test_provider_connection_error_omits_none_original_error(self) -> None: error = ProviderConnectionError("Failed to connect") assert "original_error" not in error.details @@ -457,6 +488,12 @@ def test_provider_execution_error_records_original_error(self) -> None: error = ProviderExecutionError("API call failed", original_error=RuntimeError("boom")) assert "original_error" in error.details + def test_provider_execution_error_preserves_falsy_original_error(self) -> None: + original_error = FalsyException("boom") + error = ProviderExecutionError("API call failed", original_error=original_error) + assert "original_error" in error.details + assert error.original_error is original_error + def test_provider_execution_error_omits_none_original_error(self) -> None: error = ProviderExecutionError("API call failed") assert "original_error" not in error.details @@ -467,6 +504,12 @@ def test_synthesis_execution_error_records_original_error(self) -> None: error = SynthesisExecutionError("Synthesis failed", original_error=RuntimeError("boom")) assert "original_error" in error.details + def test_synthesis_execution_error_preserves_falsy_original_error(self) -> None: + original_error = FalsyException("boom") + error = SynthesisExecutionError("Synthesis failed", original_error=original_error) + assert "original_error" in error.details + assert error.original_error is original_error + def test_synthesis_execution_error_omits_none_original_error(self) -> None: error = SynthesisExecutionError("Synthesis failed") assert "original_error" not in error.details