refactor(program): a direction carries what a consumer reads, not the check the loader makes - #581
Merged
FBumann merged 2 commits intoSep 20, 2026
Conversation
… check the loader makes `Direction.is_single_valued` is a module-private function in resolution.py. It is the discriminator between `at` and `sum(by=)`, it has two callers in one function, and no consumer asks it: a program's node type already says which operator a call became. The refusal table in docs/about/what-counts-as-public-api.md refuses a function whose answer a declaration could give. The `Predicate` union said it was what a lowered mask's root is built of. That is false. It also holds `ArithmeticComparison`, which lowering rewrites into an `ExpressionComparison`, so a consumer walking a program meets every other member and never that one. The comment and the reading page now say so. docs/reference/reading.md: n 52, avg 14.9, median 13, over25 7. The long sentences all predate this change; the two added are 11 and 9 words. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MKUDdoyCtxHXm5Gcuze24W
Documentation build overview
24 files changed ·
|
FBumann
added this pull request to stack #583
September 20, 2026 20:35
…that asks it Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JJfEUsCDtwXuXV8CANCHuR
FBumann
removed this pull request from stack #583
September 20, 2026 20:53
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.
Direction.is_single_valuedleaves the public record, and thePredicateunion stops claiming something false about itself. Nothing the language accepts, refuses or prints changes.Stacked on #580.
What this changes
Direction.is_single_valuedis a local of the call that refuses on it. It decides betweenatandsum(by=)at load:atneeds the read single-valued andsumneeds it not, because a sum landing on the relation's key has one term per group and adds nothing up. One test, used plain and negated, produces both refusals. It had no consumer: a program's node type already says which operator a call became, which is why lpspec never asks. The refusal table in what counts as public API refuses a function whose answer a declaration could give.It is a local rather than a private helper because
_directionalready holds every part:shape.key,into_rolesandjoinedare named on the lines above, so a helper taking aDirectionwould reach back through an object for what the caller just built, and evaluate the same set twice for two refusals on adjacent lines.The
Predicateunion's comment said it was "what a lowered mask'srootis built of". That is false.ArithmeticComparisonis a member, and lowering rewrites every one into anExpressionComparison, so a lowered mask never holds one. The comment now names the member a program never carries, and the reading page says the same to consumers.Why the union is not split
The false comment is the honest fix, not the union.
assert_neveris a static check, so a consumer matching a predicate exhaustively has to write a branch for every member, including one it can never receive. Narrowing the union would spare lpspec two such branches, inlinopy/where.pyand the polarspredicates.py, so I tried it.It does not stop at the union.
Not,AndandOrcarryPredicateoperands and are shared by both stages, so narrowing them propagates into every spec-side caller, andMaskholds a root of one stage or the other. The type checker priced it at eleven errors that only a per-stage copy of the predicate tree resolves. That is a layer, and two branches in one consumer do not buy one.The audit this came from
Every name in
program.__all__, checked for a qualified use in lpspec. lpspec is pinned toalpha.106, so the check ran against the spellings it imports today rather than the ones #580 introduces.Sixty-nine of eighty-one exported names have one. The surface is not carrying weight it does not need, so nothing else is removed here.
Twelve have no qualified lpspec use, and each is kept for a reason:
Footprint,QuadraticPosition,TypedPredicate,VariableAbsence,DerivationExpressionComparison,ArithmeticComparison,Connective,Reachwalk_regionswalkandchildrenQUADRATIC_POSITIONSDIMENSION_DTYPES, whichmath_spec.__all__exports as a vocabulary to pin a table againstI had proposed unexporting the last two before reading
tests/test_public_surface.py. It pinsprogram.__all__to every public name the module defines, so there is no unexported-but-public state: unexporting means making private. Both are documented promises, so both stay.Gates
pixi.shis refused by this environment's egress proxy, so the gates ran from auvenvironment on Python 3.12 with the pinnedruff==0.16.1andpyrefly==1.2.0. That is a departure from the "every command runs underpixi run" default.pytest -q -n autotests/test_public_surface.py__all__is unchanged, since a property is not a module-level nameruff check,ruff format --checkpyrefly checkmkdocs build --strictdocs.python.orginventory dropped for the run, which the proxy refuses with a 403prettier --checkon every Markdowntypos,reuse lint,taplo,zizmor,compile-texmkdocs build --strictandprettierran on the first commit, which holds the only Markdown in the diff; the second commit touchessrc/alone.docs/reference/reading.md, by the measurement in the docs-writing skill: n 52, avg 14.9, median 13, over25 7. The long sentences all predate this change; the two added are 11 and 9 words.No test moved. Nothing read
is_single_valuedoutside the two call sites.Two claims this body made, corrected
Both were mine, and both were checked only after review had started.
"Moving
ArithmeticComparisonto_expression_parserwould be a runtime import cycle." It would not.program.pyalready imports that module at runtime, on line 29, forComparisonOperator. I made the move and ran the suite: 1437 passed, 1 skipped, no cycle.The conclusion is unchanged, but the reason above is the real one. The move on its own makes matters worse rather than better: the union would still list the member, so a consumer still needs the branch, and could no longer name the type it is matching. The move is only worth making as part of narrowing the union, which is what costs eleven errors.
"lpspec has two such walkers … both ending in
assert_never. Narrowing the union would delete those two branches." The branches do not exist. lpspec pinsv0.0.0-alpha.106and has zero references toExpressionComparisonorArithmeticComparison, because #566, which introduces them, has not landed. Narrowing would spare lpspec from adding two dead branches, not delete two it has. The body above now says so.That is work the stack owes lpspec and no PR here records: when this lands, both walkers need an
ExpressionComparisonbranch orassert_neverstops type-checking.What this deliberately does not do
Predicateunion is not narrowed, for the reason above. The experiment was reverted, not left half-applied.walk_regionsandQUADRATIC_POSITIONSstay exported, against my own earlier proposal.ArithmeticComparisonstays inprogram, for the reason in the corrections above: the move is cheap and the narrowing it would have to come with is not.Defined(read)andCompare(left, op, right)was considered and dropped. A genericComparemakes five combinations representable that the language refuses, so five messages that the node type carries today would have to be rebuilt as guards; the right-hand side is a literal in every form but one, so the symmetry is not there; and both lpspec walkers dispatch on where a value comes from rather than on the comparison, so their branch count does not fall. Thewheregrammar's own overlap, wherep > 5and1 * p > 5mean the same and give different trees, belongs to feat(language): a where may compare arithmetic over parameters #566 and is noted there.<type>/<topic>from a worktree.🤖 Generated with Claude Code
https://claude.ai/code/session_01JJfEUsCDtwXuXV8CANCHuR