Skip to content

fix: resolve mutable defaults, narrow exception handling, and sys.modules leak - #33

Merged
zurfjereluhmie merged 1 commit into
mainfrom
fix/code-quality-issues
Apr 22, 2026
Merged

fix: resolve mutable defaults, narrow exception handling, and sys.modules leak#33
zurfjereluhmie merged 1 commit into
mainfrom
fix/code-quality-issues

Conversation

@zurfjereluhmie

@zurfjereluhmie zurfjereluhmie commented Apr 22, 2026

Copy link
Copy Markdown
Owner
  • Mutable default arguments — Replace shared list defaults in DatasetUploader.__init__ with None sentinels to prevent shared-state bugs (B006)
  • Overly broad exception handling — Narrow except Exception to except ValueError in uploader skip/append paths, matching Phoenix's actual not-found exception; non-recoverable errors (network, auth) now propagate correctly
  • sys.modules leak — Clean up partially-initialized modules from sys.modules when exec_module fails in runner._load_attribute
  • contains evaluator docs — Add prominent note clarifying that only the first expected item is checked

Tests

  • Added test_non_not_found_error_propagates to verify RuntimeError from Phoenix is not swallowed
  • Added test_cleans_sys_modules_on_import_error to verify broken modules are removed from sys.modules
  • Updated all existing tests to use ValueError instead of generic Exception for not-found scenarios
  • 191 tests pass, 2 skipped (langgraph extra)

Files changed

File Change
src/evalwire/uploader.py Mutable defaults fix + narrowed exception catch
src/evalwire/runner.py sys.modules cleanup on import failure
src/evalwire/evaluators/contains.py Improved docstring
tests/test_uploader.py Updated exception types + new propagation test
tests/test_runner.py New sys.modules cleanup test
tests/test_cli.py Updated exception type in mock

…ules leak

- Replace mutable list defaults in DatasetUploader.__init__ with None
  sentinels to avoid shared-state bugs (B006)
- Narrow exception catch in uploader skip/append paths from bare
  Exception to ValueError, matching Phoenix's actual not-found
  exception; non-recoverable errors now propagate correctly
- Clean up sys.modules on exec_module failure in runner._load_attribute
  to prevent partially-initialized modules from leaking
- Improve contains evaluator docstring to clarify single-item behavior
- Add regression tests for exception propagation and sys.modules cleanup
@zurfjereluhmie
zurfjereluhmie merged commit 788c642 into main Apr 22, 2026
8 checks passed
@zurfjereluhmie
zurfjereluhmie deleted the fix/code-quality-issues branch April 22, 2026 12:21
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.

1 participant