fix(eval): cache custom-coded evaluator modules instead of leaking sys.modules#1821
Open
adrian-badea wants to merge 1 commit into
Open
fix(eval): cache custom-coded evaluator modules instead of leaking sys.modules#1821adrian-badea wants to merge 1 commit into
adrian-badea wants to merge 1 commit into
Conversation
…s.modules _create_custom_coded_evaluator_internal named the dynamically loaded module `_custom_evaluator_<stem>_<id(data)>`. Because `data` is a fresh dict on every call, id(data) was unique each time, so the sys.modules cache never hit: the evaluator module was re-exec()'d on every call and a new module object (plus the classes/pydantic schemas it defines) was inserted into sys.modules and never removed. Any consumer building custom-coded evaluators repeatedly leaks memory and CPU without bound — building evaluators per datapoint turns an eval run into O(n^2) with multi-GB RAM growth. Key the module name on a hash of the resolved file path so sys.modules caches it: the module loads once and is reused. Pop the entry if exec fails so a broken load is not left cached (and can be retried). Add regression tests: repeated creation reuses one module (no leak), a failed load is not cached, and two files sharing a basename load as distinct modules. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
github-actions
Bot
added
test:uipath-langchain
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes unbounded sys.modules growth and repeated exec() of custom-coded evaluator modules by switching to a stable, path-hash-based module name so dynamically loaded evaluator files are cached and reused across EvaluatorFactory.create_evaluator(...) calls.
Changes:
- Cache custom-coded evaluator modules in
sys.modulesusing a module name derived from the evaluator file’s resolved path (sha256-based), avoiding per-call module re-execution and memory leaks. - Ensure failed module loads don’t leave a half-initialized module cached by
popping the entry onexec_modulefailure. - Add regression tests covering caching behavior, failure cleanup, and disambiguation for same-basename files in different directories.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| packages/uipath/src/uipath/eval/evaluators/evaluator_factory.py | Changes custom evaluator module naming/loading to be path-hash keyed and cached, with cleanup on import failure. |
| packages/uipath/tests/evaluators/test_evaluator_factory.py | Adds tests verifying module caching, retry-after-failure behavior, and no collisions for same stem in different directories. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+164
to
+166
| spec = importlib.util.spec_from_file_location(module_name, resolved_path) | ||
| if spec is None or spec.loader is None: | ||
| raise ValueError(f"Could not load module from {file_path}") |
Comment on lines
+175
to
+177
| raise ValueError( | ||
| f"Error executing module from {file_path}: {str(e)}" | ||
| ) from e |
|
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
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.
Problem
EvaluatorFactory._create_custom_coded_evaluator_internalnames the dynamically loaded evaluator module_custom_evaluator_<stem>_<id(data)>. Becausedatais a fresh dict on every call,id(data)is unique each time, so thesys.modulescache never hits: the module is re-exec()'d on every call and a new module object (plus all the classes / pydantic schemas it defines) is inserted intosys.modulesand never removed.Any consumer that builds custom-coded evaluators repeatedly leaks memory and CPU without bound. Building evaluators per datapoint turns an eval run into O(n²) with unbounded
sys.modulesgrowth — observed downstream as one core pinned for 2h+ at ~9 GB RSS on a ~47K-datapoint run, before any inference ran.Fix
Key the module name on a
sha256of the resolved file path sosys.modulesacts as a real load-once cache: the module is loaded once and reused. The load is guarded withmodule = sys.modules.get(name); if module is None:, and the entry ispopped ifexec_modulefails so a broken load isn't left cached (and can be retried after the file is fixed).Per-config validation (
TypeAdapter(cls).validate_python(data)) is unchanged, so distinct configs still yield distinct, correctly-configured evaluator instances despite sharing one class object.Tests
New
test_evaluator_factory.py::TestCustomCodedEvaluatorModuleLoading:sys.modulesentry (this assertion fails on the pre-fix code);ruff,mypy, and the fulltests/evaluatorssuite (500 passed) pass locally.🤖 Generated with Claude Code