fix(exceptions): record explicitly-supplied falsy details instead of dropping them - #281
Merged
himanshu231204 merged 2 commits intoAug 16, 2026
Conversation
…dropping them (OpenAgentHQ#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 OpenAgentHQ#68 in PR OpenAgentHQ#279. Message-text construction guards and __str__ formatting are unchanged. Co-Authored-By: Kimi K3 <noreply@kimi.com>
Co-Authored-By: GPT-5.6-Luna <noreply@openai.com>
This was referenced Aug 15, 2026
|
🎉 Congratulations @Nitjsefnie! Your pull request has been successfully merged into main. 🚀 Thank you for contributing to OpenAgentHQ and helping improve the project. We truly appreciate your contribution and hope to see you back with more amazing PRs! Happy Open Sourcing! ❤️ |
Member
|
@Nitjsefnie Hi, also checkout this repo https://github.com/OpenAgentHQ/localmem-mcp https://github.com/OpenAgentHQ/localmem-mcp waiting for your pr. |
Contributor
Author
|
Thanks for the pointer — I’ll take a look at localmem-mcp and find a focused contribution. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Exception constructors guarded their detail recording with a truthiness test, so an explicitly-supplied empty string,
0,False, or[]was silently dropped from.detailswhile onlyNoneshould have meant "not supplied". This changes 21 such guards across the eight exception modules to testis not None.The same defect was fixed for
ValidationErrorin #279 (issue #68). Issue #280 asked whetherConfigurationError.fieldhad it too — it does, and so do nineteen further guards in the same package, so this fixes the family rather than the one reported line.Type of Change
Related Issues
Closes #280
How Has This Been Tested?
uv run pytest)uv run ruff check .)uv run mypy openagent_eval/)tests/unit/test_exceptions.pygrew from 29 to 78 cases: one preservation case per touched module, non-string falsy values where the parameter type allows, and a negative case per module asserting that an unsupplied (None) parameter is still absent from.details— without that, "fix" could quietly degrade into "record everything".The tests were confirmed to actually fail against unfixed sources: with the new tests in place and
openagent_eval/exceptions/restored tomain, they fail; with the fix, 78 pass. They assert membership (assert "field" in exc.details) rather than indexing, so a missing key fails as an assertion rather than aKeyError.Six of the guards take an
original_error. Ordinary exception instances are truthy, so a test passing a plainValueErrorwould have passed against the old code too and proved nothing — those six are covered with an exception subclass whose__bool__returnsFalse, which is a real shape (an exception wrapping an empty collection inherits its__len__).Full
pytest tests/unitand the coverage gate ran green on a runner for this branch on Python 3.11 and 3.12.The two unticked boxes are unticked deliberately rather than overlooked.
ruff check .fails atmainon pre-existing findings unrelated to this change, so it cannot pass here;ruff checklimited to the changed files is clean.mypy openagent_eval/is likewise not clean atmain. Neither is a blocking gate in CI, and reporting an unrun or failing gate as passing seemed worse than saying so.Checklist
Additional Notes
Guards deliberately left alone, in case it looks like an omission:
base.py:26andprovider.py:42,44,46format details that are already recorded rather than deciding what to record;provider.py:75,metric.py:59andplugin.py:59build message text, so changing them would alter user-visible wording;diagnosis.py:41,dataset.py:84,cli.py:90,cli.py:92andmetric.py:119already testis not None.Generated by Claude Opus 5 (brief, review), Kimi K3 (implementation), GPT-5.6-Luna (implementation), GPT-5.6-Sol (verification)