Repository navigation
Dissolve the float-atom predicate into the canonical atom-identity accessor - #11618
Merged
Merged
Conversation
…cessor
Review 67607 raised this against gunbc#11575 and it is still open against main: the predicate
hand-rolled `match node.kind { TypeNode { connective: Atom { identity: id } } => ... }` instead of
consuming `node_atom_identity_optional`, the accessor that already owns that read and is consumed at
`v2.compiler.ingest`, `v2.compiler.body_lowering_fold` and a dozen further sites. That is DESIGN
section 2's horizontal redundancy and section 3's single authority: a fifth private re-spelling of
one storage-shape read. The three sibling copies beside it are evidence of debt rather than
precedent -- `v2.std.compilers.target_model` already carries a dissolve-on row about
`dag_node_is_int_literal_atom` being duplicated -- and the new row arrived with no disposition tag
at all.
HOW THIS CAME TO BE OPEN ON MAIN, recorded because the failure is mine and the next person deserves
the real story rather than a tidy one. The fix existed: gunbc#11615 carried it. I closed that PR as a
duplicate of gunbc#11575 on its author's instruction, and I "verified" the claim by checking that
its eight files EXIST on main. That is a membership check. It is blind to a content edit, which is
precisely the distinction I had put in writing to another lane in the same hour. The files existed;
one of them differed; the difference was this repair.
Closing gunbc#11615 was still the right outcome for a reason I had not checked either: its
`compile_cli.dag` predates gunbc#11561, so merging it would have reverted the workflow-clause
correction that landed hours earlier. A right answer reached by a wrong check is not a correct
decision, it is a lucky one, and the luck did not extend to this file.
WHAT IS NOT CLAIMED: this changes no behaviour. `node_atom_identity_optional` answers Present with
the identity exactly where the hand-rolled match bound `id`, and Absent everywhere it fell to
`false`. The executing evidence for the float predicate is gunbc#11575's enrolled
`weather_g0_prefix_parse_probe_test` rows, which are unchanged by this and continue to discriminate
the parse, the typed refusal and the int control.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Review 67887 is right and this is the repair. The first commit migrated ONE predicate onto `node_atom_identity_optional` while three byte-identical siblings kept hand-rolling the same storage-shape read four lines away, so the file carried TWO SPELLINGS OF ONE FACT -- which is worse than the debt it replaced, because a reader now has to decide which spelling is current. DESIGN section 3 names exactly this: editing X from its leaves upward preserves its root while every intermediate state becomes observable and load-bearing, so the migration creates more authority before it removes any. MIGRATED, all in `v2.extdeps.languages.dag`, no new import (the accessor was already imported) and no new concept: `dag_node_is_kw_true_atom`, `dag_node_is_kw_false_atom`, `dag_node_is_int_literal_atom`, and the float one from the first commit. AND ONE THE REVIEW DID NOT NAME, which is the sharpest instance of its own finding. `dag_node_is_eval_literal_atom` CALLS those three predicates and then hand-rolled the same read INLINE for its fourth case -- both spellings inside a single expression, three lines apart. Scoping to the three named siblings would have left the function that composes them carrying the defect the review is about. Migrated. WHAT IS DELIBERATELY LEFT, so the boundary is stated rather than discovered: two occurrences of that shape survive in this file and neither is this fact. `dag_inhabitant_atom` CONSTRUCTS an atom node (a write, not a read), and `dag_binding_branded_atom_identity` returns `Optional<Symbol>` under branding semantics rather than answering an identity comparison. Tree-wide the pattern appears 761 times per the review's own count; that is pre-existing debt this PR neither introduces nor is obliged to clear, and the review scoped itself the same way. NO BEHAVIOUR CHANGE, for the same reason as the first commit: `node_atom_identity_optional` answers `Present` with the identity exactly where each hand-rolled match bound `id`, and `Absent` everywhere each fell through to `false`. The executing evidence is unchanged -- gunbc#11575's enrolled `weather_g0_prefix_parse_probe_test` rows for the float predicate, and the existing parse claims for the int and keyword ones. PROCESS NOTE: this PR was already ENQUEUED on a side-chat LAND ruling when the review landed. I dequeued it rather than let a known half-migration compose. The ruling was scoped to the previous sha and is void; a fresh one is required before this re-enters the queue. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This was referenced Sep 19, 2026
Closed
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.
Closes the open finding from review 67607 against #11575. The predicate hand-rolled the storage-shape match instead of consuming
node_atom_identity_optional:That accessor already owns the read and is consumed at
v2.compiler.ingest,v2.compiler.body_lowering_foldand ~a dozen further sites — DESIGN §2 horizontal redundancy, §3 single authority. The three sibling copies beside it are evidence of debt, not precedent:v2.std.compilers.target_modelalready carries a dissolve-on row aboutdag_node_is_int_literal_atombeing duplicated, and the new row arrived with no disposition tag at all.Why this was open on main at all
The fix existed — #11615 carried it. I closed that PR as a duplicate of #11575 on its author's instruction, and "verified" by checking that its eight files exist on main.
That is a membership check, and it is blind to a content edit. It's the exact distinction I had written to another lane in the same hour. The files existed; one of them differed; the difference was this repair.
Closing #11615 was still the right outcome, for a reason I hadn't checked either: its
compile_cli.dagpredates #11561, so merging it would have reverted the workflow-clause correction that landed hours earlier. A right answer reached by a wrong check is a lucky one, not a correct one — and the luck didn't extend to this file.What is not claimed
No behaviour change.
node_atom_identity_optionalanswersPresentwith the identity exactly where the hand-rolled match boundid, andAbsenteverywhere it fell through tofalse. The executing evidence for this predicate remains #11575's enrolledweather_g0_prefix_parse_probe_testrows — the parse, the typedfloat_literal_not_loweredrefusal, and the int control — which this does not touch.🤖 Generated with Claude Code