refactor(program): the resolved where vocabulary lives beside the nodes a consumer dispatches on - #321
Closed
FBumann wants to merge 2 commits into
Closed
refactor(program): the resolved where vocabulary lives beside the nodes a consumer dispatches on#321FBumann wants to merge 2 commits into
FBumann wants to merge 2 commits into
Conversation
…es a consumer dispatches on Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01D59sEp5d7CecJAwrzACbRh
FBumann
force-pushed
the
feat/where-vocabulary
branch
from
September 1, 2026 14:15
587dfef to
9f28164
Compare
…s a name, NameDims Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01D59sEp5d7CecJAwrzACbRh
Contributor
Author
|
Note The following content was generated by AI. Superseded by #327, which combines the reviewed stack into a single diff on the maintainer's call. The review history — two consumer reviews and one consolidated agent review, with mutation tables — lives in this stack's bodies; the tree at #327 is byte-identical to the stack's top. |
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.
Note
The following content was generated by AI.
The resolved where-node classes, their unions and the question functions (
atoms,names_read,dims_read) move fromwhere_parserintoprogram, so a consumer dispatches on masks and expressions out of one module.where_parserkeeps the grammar,parse_whereand theUnresolved*nodes resolution rewrites away, importing the four node classes it constructs fromprogram— the dependency inverts, and stays acyclic.program.WhereNodeis the resolved-only union (the old one carried theUnresolved*members);parse_where's docstring says its annotation is the type a tree has once resolution types every leaf. That forced honest internal signatures inresolution.py: the error paths that return a node unresolved now say so, with one_resolved_childhelper carrying the invariant that a tree rebuilt over an unresolved node never escapesresolve_where/resolve_expression.A line held here, superseded two PRs up the stack: the free
atoms/names_read/dims_readstay public at this point because a consumer holding a synthetic predicate (aNotNodecomplement, anAndNodeconjunction it built itself) has noMaskto ask. #324 removes the reason — leaves carry their own dims, soMask(root)wraps any resolved predicate — and retires the free functions, on the same reviewer's proposal. TheNameDimsalias this PR givesdims_read's mapping goes with them, so this PR should land only with the rest of the stack.Stacked on #319.
Verified
pixi run ciexit 0 in this worktree: lint clean (pyrefly 0 errors), 931 passed (930 + one new pin: the connectivesparse_wherebuilds are identical class objects toprogram's),mkdocs build --strictbuilds, 27 TeX documents compile. Both import orders smoke-tested (where_parserfirst andprogramfirst).schema/andtests/typesetting/golden/byte-identical — nothing that renders or validates changed behaviour, only where the classes live.The failing-first guard for this PR is structural:
tests/test_public_surface.pyassertsprogram.__all__equals every top-level public definition in both directions, so the move without the 19 new__all__entries fails it.Deliberately not done: per-atom dims on the node classes (nothing asks yet), and any behaviour change — a consumer's
isinstanceanswers exactly as before, only the import path moved. Breaking for importers ofmath_spec.where_parser's resolved names, described here rather than in the title per the refused breaking-marker rule; a consumer bumping across this should sweep everywhere_parserimport wholesale, since the re-imported connectives keep resolving on the old path by accident while the leaf classes raise.