From a25bb818a9540abda202e7bee9d9d3353558334b Mon Sep 17 00:00:00 2001 From: Eli Date: Thu, 3 Sep 2026 16:52:31 -0400 Subject: [PATCH 1/3] Reuse mypy caches across type checks --- .../handlers/llm/harness/validation/mypy.py | 68 ++++++++++++++----- .../handlers/llm/harness/validation/ty.py | 9 +-- ...st_handlers_llm_harness_validation_mypy.py | 49 +++++++++++++ 3 files changed, 105 insertions(+), 21 deletions(-) create mode 100644 tests/test_handlers_llm_harness_validation_mypy.py diff --git a/effectful/handlers/llm/harness/validation/mypy.py b/effectful/handlers/llm/harness/validation/mypy.py index 0a9ac2fca..dd99f9dbf 100644 --- a/effectful/handlers/llm/harness/validation/mypy.py +++ b/effectful/handlers/llm/harness/validation/mypy.py @@ -21,6 +21,7 @@ import subprocess import sys import tempfile +import threading import typing # trigger mypy installation errors early @@ -59,6 +60,35 @@ class MypyTypeChecker(PromptInjectingInterpretation): "--disable-error-code=empty-body", ) + # Mypy is incremental by default, but only when successive invocations see the + # same cache. Keep one private cache root for this handler's lifetime and split + # it by leniency because those modes use cache-affecting options; alternating + # them through one cache would invalidate the previous mode on every call. + # `TemporaryDirectory` removes the whole root when the handler is discarded. + _cache_root: tempfile.TemporaryDirectory = dataclasses.field( + default_factory=lambda: tempfile.TemporaryDirectory( + prefix="effectful_mypy_cache_" + ), + init=False, + repr=False, + compare=False, + ) + + # A handler may be shared by concurrent Skill calls. Independent mypy processes + # must not write the same incremental cache at once, so serialize checks that + # share a mode. Strict and lenient calls use different caches and can still run + # concurrently. The synthesized source gets its own directory below as well. + _cache_locks: dict[bool, threading.Lock] = dataclasses.field( + default_factory=lambda: {False: threading.Lock(), True: threading.Lock()}, + init=False, + repr=False, + compare=False, + ) + + def _cache_dir(self, lenient: bool) -> str: + """The persistent cache for one cache-compatible checking mode.""" + return os.path.join(self._cache_root.name, "lenient" if lenient else "strict") + @staticmethod def _region_errors( stdout: str, lo: int | None, hi: int | None @@ -114,23 +144,27 @@ def type_check( tf_path = os.path.join(tmpdir, "_synthesized.py") with open(tf_path, "w", encoding="utf-8") as f: f.write(source) - proc = subprocess.run( - [ - sys.executable, - "-m", - "mypy", - tf_path, - "--cache-dir", - os.path.join(tmpdir, "cache"), - "--no-error-summary", - "--output=json", - "--ignore-missing-imports", - "--disable-error-code=import-untyped", - *(self.lenient_flags if lenient else []), - ], - capture_output=True, - text=True, - ) + # Keep the source path unique per call even though the cache persists. + # Mypy can accept a same-path, same-size source from its mtime fast path; + # changing the path makes it validate the source hash before reuse. + with self._cache_locks[lenient]: + proc = subprocess.run( + [ + sys.executable, + "-m", + "mypy", + tf_path, + "--cache-dir", + self._cache_dir(lenient), + "--no-error-summary", + "--output=json", + "--ignore-missing-imports", + "--disable-error-code=import-untyped", + *(self.lenient_flags if lenient else []), + ], + capture_output=True, + text=True, + ) stdout, stderr, status = proc.stdout, proc.stderr, proc.returncode finally: shutil.rmtree(tmpdir, ignore_errors=True) diff --git a/effectful/handlers/llm/harness/validation/ty.py b/effectful/handlers/llm/harness/validation/ty.py index 057af52a0..d0f18b2a7 100644 --- a/effectful/handlers/llm/harness/validation/ty.py +++ b/effectful/handlers/llm/harness/validation/ty.py @@ -3,10 +3,11 @@ `TyTypeChecker` is interchangeable with `~effectful.handlers.llm.harness.validation.mypy.MypyTypeChecker` -- same operation, same contract, a different checker behind it. ty is a compiled binary -that needs no per-call cache and builds no module graph in this process, so a -check costs milliseconds where mypy's costs seconds, and on the failure path it -reports the offending line with ty's own hints rather than a line of JSON. Prefer -it unless a stack specifically needs mypy's analysis. +that needs no cache warmup and builds no module graph in this process, so a check +costs milliseconds where mypy's first check costs seconds; mypy reuses a private +incremental cache for later checks. On the failure path ty reports the offending +line with its own hints rather than a line of JSON. Prefer it unless a stack +specifically needs mypy's analysis. Either checker is independent of any executor: it says how generated code is *checked*, not how it is parsed, compiled or run, so it is installed alongside diff --git a/tests/test_handlers_llm_harness_validation_mypy.py b/tests/test_handlers_llm_harness_validation_mypy.py new file mode 100644 index 000000000..5a1f75e71 --- /dev/null +++ b/tests/test_handlers_llm_harness_validation_mypy.py @@ -0,0 +1,49 @@ +import gc +import os +import subprocess + +import pytest + +from effectful.handlers.llm.harness.validation.mypy import MypyTypeChecker + + +def test_mypy_reuses_mode_specific_caches_and_fresh_source_paths(monkeypatch): + calls: list[list[str]] = [] + + def run(args, **kwargs): + calls.append(args) + return subprocess.CompletedProcess(args, 0, stdout="", stderr="") + + monkeypatch.setattr(subprocess, "run", run) + checker = MypyTypeChecker() + + checker.type_check("x: int = 1\n") + checker.type_check("x: int = 2\n") + checker.type_check("x: int = 3\n", lenient=True) + checker.type_check("x: int = 4\n", lenient=True) + + cache_dirs = [args[args.index("--cache-dir") + 1] for args in calls] + assert cache_dirs[0] == cache_dirs[1] + assert cache_dirs[2] == cache_dirs[3] + assert cache_dirs[0] != cache_dirs[2] + + source_paths = [args[3] for args in calls] + assert len(set(source_paths)) == len(source_paths) + assert all(not os.path.exists(path) for path in source_paths) + + cache_root = checker._cache_root.name + assert all(os.path.dirname(path) == cache_root for path in cache_dirs) + assert os.path.isdir(cache_root) + del checker + gc.collect() + assert not os.path.exists(cache_root) + + +def test_mypy_rechecks_changed_same_size_source_with_shared_cache(): + checker = MypyTypeChecker() + checker.type_check("x: int = 1\n") + + # The replacement has the same byte length. A stable source path and mtime can + # make mypy's fast cache check miss this change; type_check uses a fresh path. + with pytest.raises(TypeError, match="Incompatible types in assignment"): + checker.type_check("x: str = 1\n") From 6bd183a388f5820546cb7141cb9efbc81e5eb1cb Mon Sep 17 00:00:00 2001 From: Eli Date: Thu, 3 Sep 2026 16:54:37 -0400 Subject: [PATCH 2/3] Keep mypy cache change focused --- .../handlers/llm/harness/validation/ty.py | 9 ++-- ...st_handlers_llm_harness_validation_mypy.py | 49 ------------------- 2 files changed, 4 insertions(+), 54 deletions(-) delete mode 100644 tests/test_handlers_llm_harness_validation_mypy.py diff --git a/effectful/handlers/llm/harness/validation/ty.py b/effectful/handlers/llm/harness/validation/ty.py index d0f18b2a7..057af52a0 100644 --- a/effectful/handlers/llm/harness/validation/ty.py +++ b/effectful/handlers/llm/harness/validation/ty.py @@ -3,11 +3,10 @@ `TyTypeChecker` is interchangeable with `~effectful.handlers.llm.harness.validation.mypy.MypyTypeChecker` -- same operation, same contract, a different checker behind it. ty is a compiled binary -that needs no cache warmup and builds no module graph in this process, so a check -costs milliseconds where mypy's first check costs seconds; mypy reuses a private -incremental cache for later checks. On the failure path ty reports the offending -line with its own hints rather than a line of JSON. Prefer it unless a stack -specifically needs mypy's analysis. +that needs no per-call cache and builds no module graph in this process, so a +check costs milliseconds where mypy's costs seconds, and on the failure path it +reports the offending line with ty's own hints rather than a line of JSON. Prefer +it unless a stack specifically needs mypy's analysis. Either checker is independent of any executor: it says how generated code is *checked*, not how it is parsed, compiled or run, so it is installed alongside diff --git a/tests/test_handlers_llm_harness_validation_mypy.py b/tests/test_handlers_llm_harness_validation_mypy.py deleted file mode 100644 index 5a1f75e71..000000000 --- a/tests/test_handlers_llm_harness_validation_mypy.py +++ /dev/null @@ -1,49 +0,0 @@ -import gc -import os -import subprocess - -import pytest - -from effectful.handlers.llm.harness.validation.mypy import MypyTypeChecker - - -def test_mypy_reuses_mode_specific_caches_and_fresh_source_paths(monkeypatch): - calls: list[list[str]] = [] - - def run(args, **kwargs): - calls.append(args) - return subprocess.CompletedProcess(args, 0, stdout="", stderr="") - - monkeypatch.setattr(subprocess, "run", run) - checker = MypyTypeChecker() - - checker.type_check("x: int = 1\n") - checker.type_check("x: int = 2\n") - checker.type_check("x: int = 3\n", lenient=True) - checker.type_check("x: int = 4\n", lenient=True) - - cache_dirs = [args[args.index("--cache-dir") + 1] for args in calls] - assert cache_dirs[0] == cache_dirs[1] - assert cache_dirs[2] == cache_dirs[3] - assert cache_dirs[0] != cache_dirs[2] - - source_paths = [args[3] for args in calls] - assert len(set(source_paths)) == len(source_paths) - assert all(not os.path.exists(path) for path in source_paths) - - cache_root = checker._cache_root.name - assert all(os.path.dirname(path) == cache_root for path in cache_dirs) - assert os.path.isdir(cache_root) - del checker - gc.collect() - assert not os.path.exists(cache_root) - - -def test_mypy_rechecks_changed_same_size_source_with_shared_cache(): - checker = MypyTypeChecker() - checker.type_check("x: int = 1\n") - - # The replacement has the same byte length. A stable source path and mtime can - # make mypy's fast cache check miss this change; type_check uses a fresh path. - with pytest.raises(TypeError, match="Incompatible types in assignment"): - checker.type_check("x: str = 1\n") From 85be18aba7f3f0bf868f96fc17339c1bc38ae2f7 Mon Sep 17 00:00:00 2001 From: Eli Date: Thu, 3 Sep 2026 16:58:12 -0400 Subject: [PATCH 3/3] Use explicit mypy cache state --- .../handlers/llm/harness/validation/mypy.py | 40 ++++++++++++------- 1 file changed, 25 insertions(+), 15 deletions(-) diff --git a/effectful/handlers/llm/harness/validation/mypy.py b/effectful/handlers/llm/harness/validation/mypy.py index dd99f9dbf..a51d693f7 100644 --- a/effectful/handlers/llm/harness/validation/mypy.py +++ b/effectful/handlers/llm/harness/validation/mypy.py @@ -60,14 +60,20 @@ class MypyTypeChecker(PromptInjectingInterpretation): "--disable-error-code=empty-body", ) - # Mypy is incremental by default, but only when successive invocations see the - # same cache. Keep one private cache root for this handler's lifetime and split - # it by leniency because those modes use cache-affecting options; alternating - # them through one cache would invalidate the previous mode on every call. - # `TemporaryDirectory` removes the whole root when the handler is discarded. - _cache_root: tempfile.TemporaryDirectory = dataclasses.field( + # Strict and lenient checks use cache-affecting options, so each gets its own + # incremental cache for this handler's lifetime. `TemporaryDirectory` removes + # each cache when the handler is discarded. + _strict_cache: tempfile.TemporaryDirectory = dataclasses.field( default_factory=lambda: tempfile.TemporaryDirectory( - prefix="effectful_mypy_cache_" + prefix="effectful_mypy_strict_cache_" + ), + init=False, + repr=False, + compare=False, + ) + _lenient_cache: tempfile.TemporaryDirectory = dataclasses.field( + default_factory=lambda: tempfile.TemporaryDirectory( + prefix="effectful_mypy_lenient_cache_" ), init=False, repr=False, @@ -78,16 +84,18 @@ class MypyTypeChecker(PromptInjectingInterpretation): # must not write the same incremental cache at once, so serialize checks that # share a mode. Strict and lenient calls use different caches and can still run # concurrently. The synthesized source gets its own directory below as well. - _cache_locks: dict[bool, threading.Lock] = dataclasses.field( - default_factory=lambda: {False: threading.Lock(), True: threading.Lock()}, + _strict_lock: threading.Lock = dataclasses.field( + default_factory=threading.Lock, + init=False, + repr=False, + compare=False, + ) + _lenient_lock: threading.Lock = dataclasses.field( + default_factory=threading.Lock, init=False, repr=False, compare=False, ) - - def _cache_dir(self, lenient: bool) -> str: - """The persistent cache for one cache-compatible checking mode.""" - return os.path.join(self._cache_root.name, "lenient" if lenient else "strict") @staticmethod def _region_errors( @@ -147,7 +155,9 @@ def type_check( # Keep the source path unique per call even though the cache persists. # Mypy can accept a same-path, same-size source from its mtime fast path; # changing the path makes it validate the source hash before reuse. - with self._cache_locks[lenient]: + cache = self._lenient_cache if lenient else self._strict_cache + lock = self._lenient_lock if lenient else self._strict_lock + with lock: proc = subprocess.run( [ sys.executable, @@ -155,7 +165,7 @@ def type_check( "mypy", tf_path, "--cache-dir", - self._cache_dir(lenient), + cache.name, "--no-error-summary", "--output=json", "--ignore-missing-imports",