fix(decl-impl-match): resolve symbols in full annex subtree#5549
Merged
marsninja merged 3 commits intojaseci-labs:mainfrom Apr 14, 2026
Merged
fix(decl-impl-match): resolve symbols in full annex subtree#5549marsninja merged 3 commits intojaseci-labs:mainfrom
marsninja merged 3 commits intojaseci-labs:mainfrom
Conversation
DeclImplMatchPass only called _resolve_symbols_in_subtree on (TypeAlias, GlobalVars, ModuleCode) items of each annex module. Standalone top-level constructs in .impl.jac files - def, obj, node, edge, walker, enum, class - had their body reads left with .sym=None, breaking type inference on locals and emitting spurious W2001 "may be undefined" warnings for every downstream use. Drop the whitelist and resolve the whole module subtree instead. _resolve_symbols_in_subtree is already idempotent (skips Names whose sym is set), so the matched-impl path continues to own those bodies. Adds a regression test exercising def / obj / node / walker constructs defined entirely in a .impl.jac annex with no decl in the primary file.
PyastGenPass used to rewrite `import from foo { X }` into
`if typing.TYPE_CHECKING: from foo import X` whenever every recorded
use of X looked like a type annotation. That classification is unsound:
dataclass, Pydantic, attrs, msgspec, SQLAlchemy, and anything else that
calls typing.get_type_hints() resolves string annotations against the
module's runtime globals, so names like ClassVar / Protocol / TypedDict /
NamedTuple / custom model types need to stay importable at runtime.
The classifier only appeared to work because its input (sym.uses) was
under-populated. Once DeclImplMatchPass began resolving annotation-side
reads correctly, ClassVar got demoted from the runtimelib/test.jac
generated Python, dataclass stopped recognizing
JacTestCheck.test_suite_path as a class variable, and every downstream
.test.jac crashed at pytest collection with
`ValueError: mutable default <class 'dict'>`.
Remove _process_type_only_imports, _is_type_annotation_use, and
_has_runtime_annotation along with their call site in exit_module.
Explicit `with entry { if TYPE_CHECKING { ... } }` blocks in Jac source
continue to work unchanged and remain the right way to break import
cycles. The unused `# jac:runtime` opt-out comment (never referenced by
any in-tree .jac) goes with the classifier.
Adds a regression test that compiles a fixture with a ClassVar-typed
class variable, asserts the generated module has no top-level
TYPE_CHECKING guard, and execs the module to verify dataclass can
resolve the annotation at runtime.
Audit: 1852 .jac files recompile with no regressions. Full jac test
suite: 3235 passed, 0 failed. Downstream packages (jac-mcp, jac-byllm,
jac-client) clean; jac-scale has net -5 failures vs baseline (6
pre-existing parallel-run flakes now pass).
The broadened `_resolve_symbols_in_subtree` walks every Name and
AtomTrailer in the module, including bodies that the prior whitelist
never visited. When the type checker pulls in an imported `.py` file
via `pyast_load_pass`, the synthesized AST occasionally contains
nodes whose parent chain doesn't reach a scope (e.g. `self[k.upper()]`
in `redis/client.py:89`), and `UniScopeNode.sym_tab` raises
`ValueError("No scope found")` when the resolver tries to look them
up. That single node was aborting resolution for the entire imported
module and surfacing through `jac check` as
`Error checking '<file>.jac': No scope found`, with the failed-files
summary printing `0 errors, 0 warnings` because the exception path in
`analysis.impl.jac` returns empty diagnostic lists.
Wrap each `use_lookup` / `chain_use_lookup` call in a `try/except
ValueError`. Symbol resolution is best-effort: failing to bind a single
detached Name leaves its `.sym` as None (the same state the rest of
the compiler already tolerates), but no longer poisons resolution for
the surrounding subtree. Restores `jac check` parity with `origin/main`
on the full corpus (323 passed, 0 failed).
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
1 participant
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.
Summary
DeclImplMatchPasspreviously called_resolve_symbols_in_subtreeonly on(TypeAlias, GlobalVars, ModuleCode)items of each annex module. Standalone top-level constructs in.impl.jacfiles —def,obj,node,edge,walker,enum,class— fell through the whitelist, so their body reads were left with.sym = None. That broke type inference on every local inside those constructs and produced spuriousW2001: Name 'x' may be undefinedwarnings for every downstream read.Concrete symptom (from a real jac-client file):
Change
Drop the whitelist in
DeclImplMatchPass.transformand resolve the whole module subtree for each primary + annex + variant module._resolve_symbols_in_subtreeis already idempotent (skips Names whose.symis set), so the matched-impl path — which runs first and owns those bodies — keeps its responsibility. The broadening only fills in the gaps.After the fix, the same file type-checks cleanly and the evaluator returns the expected types:
Test plan
jac/tests/compiler/passes/main/test_decl_impl_match_pass.jac— new regression teststandalone def/obj/node/walker in .impl.jac get symbol resolutionwith fixturesstandalone_def_in_annex.jac/standalone_def_in_annex.impl.jac. Asserts.symis resolved on local reads inside each construct type defined entirely in an annex.jac/tests/compiler+jac/tests/languagesuite: 2695 passed, 57 skipped, 1 xfailed, zero new failures. (test_checker_pass.jac::checker_import_missing_modulefails identically onmain— pre-existing, unrelated.)jac checkon the original jac-clientbun_installer.impl.jacthat motivated the report now passes cleanly with full type flow on locals.