BUG-HUNT: [error-handling] ToolResult validation raises generic ValueError #1722

New issue

Open

opened 2026-04-02 23:35:59 +00:00 by freemo · 1 comment

freemo commented

2026-04-02 23:35:59 +00:00

Owner

Metadata

Branch: fix/tool-result-validation-error-type
Commit Message: fix(tool): replace generic ValueError with ToolResultValidationError in ToolResult validator
Milestone: v3.7.0
Parent Epic: #1669

Background and Context

The ToolResult model in src/cleveragents/tool/runtime.py uses a Pydantic @model_validator to enforce consistency between the success and error fields. When the fields are inconsistent, the validator raises a generic ValueError. While this correctly identifies the invalid state, it makes it harder for callers to programmatically distinguish this specific validation failure from other ValueError exceptions that may be raised elsewhere in the call stack.

Per the project's error-handling standards, code should fail fast and provide precise, meaningful exceptions that allow callers to handle errors appropriately.

Current Behavior

The _validate_success_error_consistency validator in ToolResult raises a generic ValueError:

# src/cleveragents/tool/runtime.py, lines 131–147

@model_validator(mode="after")
def _validate_success_error_consistency(self) -> ToolResult:
    """Ensure success and error fields are logically consistent."""
    if self.success and self.error is not None:
        raise ValueError(
            "ToolResult with success=True must not have an error message"
        )
    if not self.success and self.error is None:
        raise ValueError(
            "ToolResult with success=False must include an error message"
        )
    return self

This makes it impossible for callers to except ToolResultValidationError specifically — they must catch the broad ValueError, risking swallowing unrelated errors.

Expected Behavior

The validator should raise a specific, custom exception class (e.g., ToolResultValidationError) that inherits from ValueError (for backwards compatibility). This allows callers to catch the specific error type while still being compatible with existing code that catches ValueError.

class ToolResultValidationError(ValueError):
    """Raised when ToolResult success/error fields are logically inconsistent."""
    pass

class ToolResult(BaseModel):
    @model_validator(mode="after")
    def _validate_success_error_consistency(self) -> ToolResult:
        if self.success and self.error is not None:
            raise ToolResultValidationError(
                "ToolResult with success=True must not have an error message"
            )
        if not self.success and self.error is None:
            raise ToolResultValidationError(
                "ToolResult with success=False must include an error message"
            )
        return self

Acceptance Criteria

A ToolResultValidationError custom exception class is defined, inheriting from ValueError.
The _validate_success_error_consistency validator raises ToolResultValidationError instead of ValueError.
ToolResultValidationError is exported from the tool module's public API.
Existing callers that catch ValueError continue to work (backwards compatibility preserved via inheritance).
Behave unit tests cover both invalid states and verify the specific exception type raised.
All nox sessions pass (nox).
Coverage remains ≥ 97%.

Supporting Information

File: src/cleveragents/tool/runtime.py
Class: ToolResult
Lines: 139–146
Severity: Low — only triggered by incorrect programmatic construction of ToolResult, not typical runtime conditions.
Impact: Low — improves error handling precision without changing functional behaviour.

Subtasks

Define ToolResultValidationError(ValueError) in src/cleveragents/tool/runtime.py (or a dedicated exceptions module within the tool package)
Update _validate_success_error_consistency to raise ToolResultValidationError in both branches
Export ToolResultValidationError from the tool package's __init__.py
Tests (Behave): Add/update scenarios in features/ to assert ToolResultValidationError is raised for both invalid states (success=True with error, success=False without error)
Run nox -e typecheck — fix any type errors
Run nox -e unit_tests — confirm all tests pass
Run nox -e coverage_report — verify coverage ≥ 97%
Run nox (all default sessions) — fix any remaining errors

Definition of Done

This issue is complete when:

All subtasks above are completed and checked off.
A Git commit is created where the first line of the commit message matches the Commit Message in Metadata exactly (fix(tool): replace generic ValueError with ToolResultValidationError in ToolResult validator), followed by a blank line, then additional lines providing relevant implementation details.
The commit is pushed to the remote on the branch matching the Branch in Metadata exactly (fix/tool-result-validation-error-type).
The commit is submitted as a pull request to master, reviewed, and merged before this issue is marked done.
All nox stages pass.
Coverage ≥ 97%.

Automated by CleverAgents Bot
Supervisor: Bug Hunting | Agent: ca-new-issue-creator

## Metadata - **Branch**: `fix/tool-result-validation-error-type` - **Commit Message**: `fix(tool): replace generic ValueError with ToolResultValidationError in ToolResult validator` - **Milestone**: v3.7.0 - **Parent Epic**: #1669 ## Background and Context The `ToolResult` model in `src/cleveragents/tool/runtime.py` uses a Pydantic `@model_validator` to enforce consistency between the `success` and `error` fields. When the fields are inconsistent, the validator raises a generic `ValueError`. While this correctly identifies the invalid state, it makes it harder for callers to programmatically distinguish this specific validation failure from other `ValueError` exceptions that may be raised elsewhere in the call stack. Per the project's error-handling standards, code should fail fast and provide precise, meaningful exceptions that allow callers to handle errors appropriately. ## Current Behavior The `_validate_success_error_consistency` validator in `ToolResult` raises a generic `ValueError`: ```python # src/cleveragents/tool/runtime.py, lines 131–147 @model_validator(mode="after") def _validate_success_error_consistency(self) -> ToolResult: """Ensure success and error fields are logically consistent.""" if self.success and self.error is not None: raise ValueError( "ToolResult with success=True must not have an error message" ) if not self.success and self.error is None: raise ValueError( "ToolResult with success=False must include an error message" ) return self ``` This makes it impossible for callers to `except ToolResultValidationError` specifically — they must catch the broad `ValueError`, risking swallowing unrelated errors. ## Expected Behavior The validator should raise a specific, custom exception class (e.g., `ToolResultValidationError`) that inherits from `ValueError` (for backwards compatibility). This allows callers to catch the specific error type while still being compatible with existing code that catches `ValueError`. ```python class ToolResultValidationError(ValueError): """Raised when ToolResult success/error fields are logically inconsistent.""" pass class ToolResult(BaseModel): @model_validator(mode="after") def _validate_success_error_consistency(self) -> ToolResult: if self.success and self.error is not None: raise ToolResultValidationError( "ToolResult with success=True must not have an error message" ) if not self.success and self.error is None: raise ToolResultValidationError( "ToolResult with success=False must include an error message" ) return self ``` ## Acceptance Criteria - [ ] A `ToolResultValidationError` custom exception class is defined, inheriting from `ValueError`. - [ ] The `_validate_success_error_consistency` validator raises `ToolResultValidationError` instead of `ValueError`. - [ ] `ToolResultValidationError` is exported from the `tool` module's public API. - [ ] Existing callers that catch `ValueError` continue to work (backwards compatibility preserved via inheritance). - [ ] Behave unit tests cover both invalid states and verify the specific exception type raised. - [ ] All nox sessions pass (`nox`). - [ ] Coverage remains ≥ 97%. ## Supporting Information - **File**: `src/cleveragents/tool/runtime.py` - **Class**: `ToolResult` - **Lines**: 139–146 - **Severity**: Low — only triggered by incorrect programmatic construction of `ToolResult`, not typical runtime conditions. - **Impact**: Low — improves error handling precision without changing functional behaviour. ## Subtasks - [ ] Define `ToolResultValidationError(ValueError)` in `src/cleveragents/tool/runtime.py` (or a dedicated exceptions module within the `tool` package) - [ ] Update `_validate_success_error_consistency` to raise `ToolResultValidationError` in both branches - [ ] Export `ToolResultValidationError` from the `tool` package's `__init__.py` - [ ] Tests (Behave): Add/update scenarios in `features/` to assert `ToolResultValidationError` is raised for both invalid states (`success=True` with error, `success=False` without error) - [ ] Run `nox -e typecheck` — fix any type errors - [ ] Run `nox -e unit_tests` — confirm all tests pass - [ ] Run `nox -e coverage_report` — verify coverage ≥ 97% - [ ] Run `nox` (all default sessions) — fix any remaining errors ## Definition of Done This issue is complete when: - All subtasks above are completed and checked off. - A Git commit is created where the **first line** of the commit message matches the Commit Message in Metadata exactly (`fix(tool): replace generic ValueError with ToolResultValidationError in ToolResult validator`), followed by a blank line, then additional lines providing relevant implementation details. - The commit is pushed to the remote on the branch matching the **Branch** in Metadata exactly (`fix/tool-result-validation-error-type`). - The commit is submitted as a **pull request** to `master`, reviewed, and **merged** before this issue is marked done. - All nox stages pass. - Coverage ≥ 97%. --- **Automated by CleverAgents Bot** Supervisor: Bug Hunting | Agent: ca-new-issue-creator

freemo added this to the v3.7.0 milestone

2026-04-02 23:36:17 +00:00

freemo commented

2026-04-02 23:42:21 +00:00

Author

Owner

Issue triaged by project owner:

State: Verified
Priority: Priority/Low (confirmed)
MoSCoW: MoSCoW/Could Have — raising a generic ValueError instead of a specific error type is a code quality issue. The validation works, just with a less specific exception. Could Have.

Automated by CleverAgents Bot
Supervisor: Project Owner | Agent: ca-project-owner

Issue triaged by project owner: - **State**: Verified - **Priority**: Priority/Low (confirmed) - **MoSCoW**: MoSCoW/Could Have — raising a generic ValueError instead of a specific error type is a code quality issue. The validation works, just with a less specific exception. Could Have. --- **Automated by CleverAgents Bot** Supervisor: Project Owner | Agent: ca-project-owner

freemo added the

MoSCoW

Could have

label

2026-04-02 23:42:21 +00:00

freemo added a new dependency

2026-04-02 23:43:12 +00:00

#1669 Bug Hunting Session

freemo referenced this issue

2026-04-02 23:44:57 +00:00

BUG-HUNT: [error-handling] Broad exception in LangChainChatProvider.generate_changes #1761