Synthesizer drops _-prefixed module imports even when referenced by context type stubs

[datvo06]

Failing test that should pass

A Template that synthesises a callable, called from a pytest test method, fails mypy validation in the synthesizer because the request fixture's type (_pytest.fixtures.TopRequest) is added to the context stubs but _pytest is never imported.

import pytest
from typing import Callable
from effectful.handlers.llm import Template
from effectful.handlers.llm.evaluation import UnsafeEvalProvider
from effectful.ops.semantics import handler


class TestSynthesisRegression:
    @pytest.fixture(autouse=False)
    def _dummy(self): yield

    def test_synthesis_with_pytest_request_in_scope(self, request):
        threshold = 0.85

        @Template.define
        def make() -> Callable[[float], bool]:
            """Use the `threshold` reader, then `def above(x: float) -> bool: return x > <value>`."""
            raise NotImplementedError

        with handler(UnsafeEvalProvider()):
            fn = make()           # currently raises ResultDecodingError

        assert fn(0.9) is True

The LLM call itself succeeds and emits def above(x: float) -> bool: return x > 0.85. The failure surfaces inside mypy_type_check.

Root cause

effectful/handlers/llm/evaluation.py:251 collect_imports:

modules: set[tuple[str, str | None]] = set(
    (k, None)
    for k in sys.modules.keys()
    if k not in SKIPPED_GLOBALS and not k.startswith("_") and k[0].isalpha()
)

The not k.startswith("_") clause filters out every module whose name begins with an underscore — including _pytest, which pytest's public surface re-exports types from. When collect_variable_declarations later emits a stub line like

request: _pytest.fixtures.TopRequest

(because the runtime request value's class is _pytest.fixtures.TopRequest), the qualified type name still references _pytest, but the import has been silently dropped. mypy then reports

<string>:17: error: Name "_pytest" is not defined  [name-defined]

and mypy_type_check raises TypeError, which surfaces as ResultDecodingError.

[datvo06]

I discovered this why working on PR #670. It exposes every name in a Template's lexical context (modulo the name.isidentifier() filter) to the synthesizer's ctx. Test functions that take pytest fixtures or define instance helpers now have request/self/__cached__ etc. in ctx. Before PR #670, those values were never propagated to mypy_type_check, so the missing-import bug never fired.

For Fixing, I'm thinking of emitting imports for any module whose qualified name appears in the context type stubs, regardless of whether the module's own name starts with _. Concretely, walk the types in ctx first, collect every referenced module:

referenced_modules: set[str] = set()
for value in ctx.values():
    typ = type(value)
    mod = getattr(typ, "__module__", None)
    if mod and isinstance(mod, str):
        referenced_modules.add(mod)
        # Also walk qualified parts: _pytest.fixtures → also add _pytest
        for part in mod.split("."):
            referenced_modules.add(part)

modules = {
    (k, None)
    for k in sys.modules.keys()
    if k not in SKIPPED_GLOBALS
    and k[0].isalpha()
    and (not k.startswith("_") or k in referenced_modules)
}

i.e. the not k.startswith("_") clause is overridden when the module is actually referenced by something we're emitting.

Regression test that should pass once this lands:

def test_collect_imports_includes_private_module_for_referenced_type():
    import _pytest.fixtures  # noqa: F401
    ctx = {"req": _pytest.fixtures.TopRequest.__new__(_pytest.fixtures.TopRequest)}
    stmts = collect_imports(ctx)
    module_names = {
        alias.name
        for stmt in stmts
        if isinstance(stmt, ast.Import)
        for alias in stmt.names
    }
    assert "_pytest" in module_names

Downstream

PR #670's test_template_synthesis_uses_lexical_reader cannot land its fixture recording until either this is fixed or the test is restructured to avoid pytest fixture parameters in the Template's lexical scope.

References

• Discovered while recording fixtures for PR Synthetic readers for lexical context (addresses #497) #670's Test A.
• Affected code: effectful/handlers/llm/evaluation.py:251 (collect_imports).

[datvo06]

Also not sure how complete it is, but we either do this or we keep track of every skipped name. _ heuristic seems to be ok to keep.

[datvo06]

Another alternative is to introspect the already-emitted AST rather than the runtime types, then union the discovered module-qualified names with the existing sys.modules filter. This is strictly more complete than walking type(value).__module__ because it catches every dotted reference the synthesizer actually writes — including generic parameters, Annotated metadata, and inherited class chains:

def _module_qualnames(node: ast.AST) -> set[str]:
    """Collect every `a.b.c` reference reachable from `node`."""
    out: set[str] = set()
    for sub in ast.walk(node):
        if isinstance(sub, ast.Attribute):
            parts: list[str] = []
            cur: ast.AST = sub
            while isinstance(cur, ast.Attribute):
                parts.append(cur.attr)
                cur = cur.value
            if isinstance(cur, ast.Name):
                parts.append(cur.id)
                # `_pytest.fixtures.TopRequest` → `_pytest.fixtures`
                out.add(".".join(reversed(parts[1:])))
    return out

# Collect after building the stubs that reference the modules:
referenced_modules: set[str] = set()
for stmt in collect_variable_declarations(ctx) + collect_runtime_type_stubs(ctx):
    referenced_modules |= _module_qualnames(stmt)
# Also add parent packages so `import _pytest.fixtures` brings in `_pytest`:
for ref in list(referenced_modules):
    while "." in ref:
        ref = ref.rsplit(".", 1)[0]
        referenced_modules.add(ref)

modules = {
    (k, None)
    for k in sys.modules.keys()
    if k not in SKIPPED_GLOBALS
    and k[0].isalpha()
    and (not k.startswith("_") or k in referenced_modules)
}

Trade-offs against the type(value).__module__ approach:

• Soundness: avoids the bare-segment collision in the simpler fix (e.g. _pytest.fixtures would add "fixtures" to referenced_modules, which might match a top-level fixtures package on PyPI and pull it in unnecessarily). The AST-walking variant only sees the qualified prefix, so fixtures alone never enters the set.
• Completeness: catches dict[str, _internal.Stuff], Annotated[int, _internal.Marker], and any other reference path that the variable/stub emitter writes but type(value).__module__ doesn't surface.
• Coupling: requires the discovery step to run after collect_variable_declarations + collect_runtime_type_stubs instead of in parallel with collect_imports. Small refactor; either you reorder the calls in mypy_type_check or you compute the stubs once at the top and pass them down.

Same regression test in the OP should pass either way.

[eb8680]

I think this is only relevant for typechecking, so we should just remove the offending k.startswith("_") heuristic.