Skip to content

fix(validation): fix unreachable allow_override code in llm_validator#2038

Open
Waqar53 wants to merge 2 commits into567-labs:mainfrom
Waqar53:fix/llm-validator-allow-override
Open

fix(validation): fix unreachable allow_override code in llm_validator#2038
Waqar53 wants to merge 2 commits into567-labs:mainfrom
Waqar53:fix/llm-validator-allow-override

Conversation

@Waqar53
Copy link
Contributor

@Waqar53 Waqar53 commented Jan 26, 2026

Fixes a critical bug where the allow_override feature in llm_validator() was completely broken - the code was unreachable dead code due to an assert statement.

The Bug

# Line 69 - Raises AssertionError if resp.is_valid is False
assert resp.is_valid, resp.reason
# Lines 71-73 - UNREACHABLE! (assert already crashed)
if allow_override and not resp.is_valid and resp.fixed_value is not None:
    return resp.fixed_value  # This line NEVER executes
Impact: Users who set allow_override=True expecting auto-correction get an AssertionError instead of the corrected value.

The Fix
python
if not resp.is_valid:
    if allow_override and resp.fixed_value is not None:
        return resp.fixed_value
    raise ValueError(resp.reason)
return v
ChangesRestructure logic so 
allow_override
 check runs before any exceptionUse ValueError instead of assert for better error handlingUpdate docstring to match actual parametersAdd 5 comprehensive tests for 
allow_override
 functionality
Type of Change
 Bug fix (non-breaking change that fixes an issue)
Testing
Added 
tests/test_llm_validators.py
 with 5 tests covering:

Valid input returns unchanged
Invalid with allow_override=True returns 
fixed_value
Invalid with allow_override=False raises ValueError
Invalid without 
fixed_value
 raises ValueError
Default allow_override=False behavior

The allow_override feature was completely broken - the assert on line 69
would raise AssertionError before the override check on lines 71-73
could ever execute.

Changes:
- Restructure logic so allow_override check runs before any exception
- Use ValueError instead of assert for better error handling
- Update docstring to match actual parameters
- Add 5 comprehensive tests for allow_override functionality

Fixes the documented allow_override feature that was previously dead code.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants