Skip to content

LanguageModel.canonical_symbols may be a decorative field: resolve accepts a tree whose root atom was removed from the prelude set (gunbc#8505 probe A class 4) - #8542

Merged
briansrls merged 4 commits into
mainfrom
session/royal-deer-610
Aug 19, 2026

Conversation

@briansrls

Copy link
Copy Markdown
Contributor

Auto-opened by session-dashboard for session royal-deer-610.
Pushing to session/royal-deer-610 advances this PR.

Worker attestation

Before flipping this PR to ready for review, confirm each item:

  • Title describes the change (not the session id or branch).
  • PR body summarises what and why (replace the TODO below).
  • Tests run: name the command (e.g. npm test, cargo test) and the result.
  • If this closes a work item, the body contains a Closes #N directive.
  • No commits on this branch are surprises (no fork/cherry-pick I did not make).
  • No secrets / credentials / large binaries staged.

Summary

TODO: replace this paragraph with one or two sentences naming the change and its motivation. Reviewers read this first.

Test plan

  • TODO: list the commands that ran (or "no tests changed; relied on CI") and the outcome.

Brian Searls and others added 2 commits August 19, 2026 09:09
build_program_namespace (and the shared name_resolve admission path via
namespace_bindings_augment_from_module_atoms) deep-harvested every Atom
node anywhere in the resolved tree via harvest_direct_atom_binding and
self-bound it, before any scope/symbol_index/canonical_symbols check ran.
Since resolving an atom requires it to appear as an Atom node in the tree
being resolved, this pre-empted the fallback chain for every reference,
making LanguageModel.canonical_symbols (and cross-module symbol_index
lookups within a single tree) decorative for the resolve() stage used by
v2.compiler.compile's compile-frontend.

Narrow both call sites to harvest_conj_named_bindings (direct top-level
declarations only); param/bind-target scoping already runs its own
narrowly-scoped harvest_direct_atom_binding at traversal time and is
untouched. Adds a discriminating regression: a root atom absent from both
declarations and canonical_symbols must now be rejected.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
The first version routed through normalize(), which desugars the
tested atom before resolve() sees it -- the same "mis-aimed mutation"
trap flagged in gunbc#8505 probe A, this time in the test itself
rather than the original probe. Rebuilt it as a hand-built Conj-rooted
tree admitted directly via admit_normalized_tree, isolating the
module-root self-bind defect: a legitimately declared root whose
declaration target is an undeclared, non-canonical atom reference.
Verified by execution against the fixed namespace_bindings_augment_from_module_atoms.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@gunbai-bot
gunbai-bot Bot marked this pull request as ready for review August 19, 2026 10:58
# Conflicts:
#	src/v2/test/claim/claim_pipeline/resolve.dag
@briansrls
briansrls merged commit 075f4fd into main Aug 19, 2026
1 check failed
@briansrls
briansrls deleted the session/royal-deer-610 branch August 19, 2026 20:37
gunbai-bot Bot pushed a commit that referenced this pull request Aug 19, 2026
Six commits, 11 files. Two .dag conflicts and one Rust API change.

BOTH .dag CONFLICTS RESOLVED IN MAIN'S FAVOUR, on the established rule: main is
the authority for CONTENT, this branch for SPELLING. Checked rather than
assumed which side was newer -- 03_resolve.dag looked backwards at first glance
because the BRANCH side had MORE code. It was main that deliberately deleted it:
#8542 collapses namespace_bindings_augment_from_module_atoms to
harvest_conj_named_bindings and removes add_module_named_exports, so resolve
accepts a tree whose root atom left the prelude set. Taking the branch's larger
side would have reverted that.

Re-applied the cut to both files afterwards: 339 names qualified (65 record
pattern heads, 20 payload-free heads, 6 string occurrences left alone), then 33
re-exporter repoints.

THE RUST CHANGE IS A REAL IMPROVEMENT AND IS ADOPTED, NOT PAPERED OVER. #8547
made parse_module_binding return Result<ModuleBindingOutcome,
ModuleBindingRefusal> instead of an Option that panicked on a header-bearing
parse failure. A branch call site still used the old `?.ok_or_else(...)` shape.
It now matches both arms, and keeps them DISTINCT rather than collapsing them:

  a parse REFUSAL reports the parser's own located diagnostic;
  ModuleBindingUnclassified reports "missing module declaration".

Those are different facts -- a file that failed to parse is not a file that
parsed fine and had no module header -- and folding the first into the second
would have thrown away exactly the typed refusal #8547 landed to create.

Receipts: v1_src_dag_parse 46 file(s) parse-clean, exit 0; the specimen witness
w_the_three_blocker_kinds_are_distinct still returns `true`.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant