Skip to content

Ground variant owner by expected type at each call site (§3: single authority) - #5891

Merged
briansrls merged 9 commits into
mainfrom
session/proud-bee-631
Jun 28, 2026
Merged

briansrls merged 9 commits into
mainfrom
session/proud-bee-631

Conversation

@gunbai-bot

@gunbai-bot gunbai-bot Bot commented Jun 28, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Fixes the latent §3 collision flagged as residue in #5879: when a variant name appears in two coproducts that are BOTH in scope (e.g. `WriteThenCommit` ∈ `AtomicityModel` ∩ `CacheWriteSemantics`), the correct owning enum at each emit site is determined by the declared expected/field type at that site — not by scope-order or alpha-sort.

Authority site: src/v1/04_infer.dag — expected_type_override_enum helper added at line 515; ExprVar branch updated to call it. The .rs seed (v1_compiler_infer.rs) is hand-synced (not full-regen — correct per emitter-nondeterminism constraint).

What changed

src/v1/04_infer.dag (AUTHORITY) + v1_compiler_infer.rs (hand-synced mirror) — infer_expr ExprVar branch:
When the scope binds a shared variant to enum A (by import-order or alpha-sort), but:

  1. The declared expected type at the call site is a different enum B, AND
  2. B is a Disj type that also contains this variant

…then enum B is used as the owner. Both binding_kind (→ VariantValueBinding { parent_enum: B }) and the inferred resolved type are updated in lockstep, so the emit path in effective_variant_parent sees a consistent owner.

This is §3 single-authority grounding applied to variant-constructor sites: the EXPECTED TYPE is the authoritative single source, not whichever enum happens to be first in the scope map.

dsl/test/claim/variant_owner_expected_type_test.dag — floor-auto-enrolled .dag discriminating witness:
expected_type_picks_variant_owner_not_alpha_order(): AEarlyOwner and ZLaterOwner both define SharedVariant; make_z_variant() -> ZLaterOwner { SharedVariant } must infer ZLaterOwner as owner. Without the fix, scope-order picks AEarlyOwner → TypeMismatch → red. Verified green-by-execution.

variant_owner_disambiguation_test.rs — two new §2 .rs witnesses:

  • shared_variant_resolves_by_expected_type_not_alpha_order: alpha-first default would pick the wrong owner; expected type overrides it. Red-on-revert.
  • shared_variant_resolves_per_site_independently: two functions with different declared return types in the same module each get the correct owner — proves the fix is per-site, not per-module.

Concrete corpus case fixed

In dsl/extdeps/cache/github_actions.dag:

write_semantics: WriteThenCommit   # field type: CacheWriteSemantics → emits CacheWriteSemantics::WriteThenCommit ✓
atomicity:       WriteThenCommit   # field type: AtomicityModel       → emits AtomicityModel::WriteThenCommit ✓

Previously both emitted AtomicityModel::WriteThenCommit (alpha wins) — the write_semantics field was wrong.


Open residual (tracked, dissolution-gated)

Genuinely-ambiguous sites — expected=None → scope-order fallback (unchanged #5879 behavior):

When the expected type is None (e.g., variant used inside an anonymous record literal where record_lit_expanded_from_expected returns none for type_name=Absent), the override does not fire and the code falls back to scope_parent — the alpha-first enum via binding.resolved. This is the same deterministic scope-order behavior as #5879.

Classification: §5 "wall-after-grounding" — decidable but gated on a corpus migration. The strict fix (TypeAmbiguous diagnostic when expected=None and variant is shared) would break existing anonymous-record sites. Interim behavior is latent-safe: emit-host MVP smoke is green, emitted Rust is self-consistent.

Dissolution trigger: corpus migration to named struct literals or qualified variant syntax at ambiguous sites (e.g. CacheInterfaceCatalogIoSemantics { write_semantics: WriteThenCommit } instead of bare { write_semantics: WriteThenCommit }). Once migrated, the TypeAmbiguous wall can turn on without breaking anonymous-record sites.


Test plan

  • shared_variant_resolves_to_locally_defined_owner_not_transitive (§1 — existing, still green)
  • shared_variant_resolves_by_expected_type_not_alpha_order (§2 new — red without fix)
  • shared_variant_resolves_per_site_independently (§2b new — red without fix)
  • expected_type_picks_variant_owner_not_alpha_order (.dag floor witness — red without fix)
  • CI green at 61d6812b — compile-clean + emit-host MVP smokes pass
  • cargo fmt --check passes
  • cargo clippy -p v1-compiler -- -D warnings passes

🤖 Generated with Claude Code

briansrls and others added 3 commits June 28, 2026 02:27
…prep)

- v1_compiler_infer.rs (ExprVar): when the scope binds a shared variant to
  enum A (by import-order or alpha-sort), but the declared expected type at
  the call site is a different enum B that also contains the variant, prefer
  B. Both binding_kind and inferred resolved-type are updated so the emit
  path sees a consistent owner without needing its own fallback.

- variant_owner_disambiguation_test.rs: two new §2 witnesses:
  · shared_variant_resolves_by_expected_type_not_alpha_order — AEarlyEnum
    and ZLaterEnum both define SharedVarAmbig; fn return type ZLaterEnum must
    emit ZLaterEnum::SharedVarAmbig not AEarlyEnum (alpha-first default).
  · shared_variant_resolves_per_site_independently — two functions in the
    same module with different declared return types each get the correct
    variant owner independently, proving the fix is per-site not per-module.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@gunbai-bot
gunbai-bot Bot marked this pull request as ready for review June 28, 2026 02:45
@gunbai-bot gunbai-bot Bot changed the title Variant-owner-by-expected-type §3 grounding (KEYSTONE, correctness wall): pick a variant's owning enum by the EXPECTED/FIELD type AT EACH SITE, not scope-order/alpha. Deeper than #5879 (which grounded order, not scope-independence). Gates #5884 + any change that tips emit-visible scope. Design-first Ground variant owner by expected type at each call site (§3: single authority) Jun 28, 2026
@gunbai-bot

gunbai-bot Bot commented Jun 28, 2026

Copy link
Copy Markdown
Contributor Author

Req 5 — genuinely-ambiguous-site fallback: stated

When expected = None (or the expected type is not a Disj containing the variant), expected_type_override_enum returns Absent and the ExprVar branch falls back to scope_parent — the enum returned by lookup_variant_parent_enum. That function resolves to authored_name_at(binding.resolved), which is the first coproduct in source-order that contains the variant (the same alpha-sort anchor #5879 established for the scope-level case).

Why this is the correct residue (not a §5 hole):

The fix grounds every site where the expected type IS propagated — function return sites, named struct field sites where record_lit_expanded_from_expected fires. The genuinely-ambiguous residue is sites where expected = None AND the variant is shared across two enums. In those cases:

  • The scope-order fallback is deterministic (not random, not non-deterministic) — same result every run, so the emitted code is self-consistent and rustc-clean.
  • The correct fix at those sites is source-level disambiguation: use qualified syntax or named struct literals so the field expected type propagates. Example: github_actions.dag uses anonymous io: { write_semantics: WriteThenCommit } — the fix is io: CacheInterfaceCatalogIoSemantics { write_semantics: WriteThenCommit }. That is a separate cleanup PR, not this one.
  • A fail-closed diagnostic (TypeAmbiguous when expected=None and variant is shared) would be the §5-correct wall, but it would break every existing anonymous-record site in the corpus and needs its own runway.

Current status: CI green at 61d6812b, all 3 discriminating witnesses pass (2 .rs integration tests + 1 .dag floor witness), authority in 04_infer.dag. Blocked on 2nd-provider review (3 claude approvals, 0 openai).

— sent from proud-bee-631

@gunbai-bot

gunbai-bot Bot commented Jun 28, 2026

Copy link
Copy Markdown
Contributor Author

Correction on review #32785 — Rust file is hand-synced, not regenerated

The review states "The Rust file is now marked generated and regenerated cleanly" — this is inaccurate and worth correcting for future readers.

v1_compiler_infer.rs is hand-synced in this PR, not regenerated. regen_stage0 was run during development and produced the equivalent output for my two changes (expected_type_override_enum function + ExprVar branch), but the full regen also activated pre-existing VariantCollision logic from 04_infer.dag line 5815 — a blocking diagnostic that was never hand-synced and would break std.cache_interface and other corpus modules that define or import two coproducts sharing a variant name. The regen output was reverted; the hand-sync is the committed state.

Faithfulness is confirmed: the regen's two relevant hunks (the new function and the updated ExprVar branch) are behaviorally identical to the hand-sync — the regen calls expected_type_override_enum(...) as an extracted helper while the hand-sync inlines the same logic. Zero semantic gap. The parent (stern-moth-225) holds the VariantCollision X/Y decision (relax-vs-de-collide) which gates the regen-pass becoming safe to commit.

The file does not carry a "Generated by v1 compiler" header — that would be inserted by regen but is absent from the hand-synced version.

— sent from proud-bee-631

briansrls and others added 3 commits June 28, 2026 04:10
The regen activates pre-existing VariantCollision logic in 04_infer.dag
(line 5815) that is a blocking diagnostic and breaks the corpus (e.g.
std.cache_interface defines both AtomicityModel and CacheWriteSemantics
sharing WriteThenCommit — regen fires VariantCollision there). This is a
known X/Y modeling decision being escalated; the hand-sync is correct and
faithful for the expected_type_override_enum grounding in this PR.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Tests 2+3 previously imported both AEarlyEnum and ZLaterEnum from a
separate owner module. Since they share a variant, the pre-existing
imported-both guard (curr_is_imported && prev_is_imported) fires a
VariantCollision diagnostic, which would block the regen path.

Fix: collapse each two-module scenario into a single module with locally-
defined enums and no imports. imported_enum_names is empty → no collision
possible. Discrimination is preserved via variant_locals_from_items
last-write-wins: ZLaterEnum defined first, AEarlyEnum second → AEarlyEnum
wins scope; the function return type (ZLaterEnum) triggers the expected-
type override on the site that needs it.

Verified: removing expected_type_override_enum → tests 2+3 go red
(AEarlyEnum::SharedVarField emitted for make_late); restoring → green.
Test 1 (local-vs-transitive, #5879 fix) unaffected.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@briansrls
briansrls merged commit 4f40cf4 into main Jun 28, 2026
2 checks passed
@briansrls
briansrls deleted the session/proud-bee-631 branch June 28, 2026 14:41
briansrls pushed a commit that referenced this pull request Jun 28, 2026
* Restore emit-path variant-owner determinism lost in #5873 regen (#5879 grounding)

Port #5879's variant-owner disambiguation back into .dag authority and hand-sync
the v1 Rust seed: imported_variants (source-module local items), alpha-sorted
variant_fold bindings, sorted export-set map_keys, and field-type-hint struct
disambiguation. Regen in #5873 overwrote these fixes; this re-lands them on
current main (#5899) without disturbing expected_type_override_enum (#5891).

Co-authored-by: Cursor <cursoragent@cursor.com>

* Fix #5899 func_sigs merge regression in variant-owner restore

Re-land imported_variants + sorted variant_fold on top of #5899's
func_sigs/all_declared_sigs path (not the reverted imported_sigs fork).
Restores compile-clean hand-sync; variant_owner_disambiguation 3/3 green.

Co-authored-by: Cursor <cursoragent@cursor.com>

* Ground emit-path variant-owner determinism in .dag authority with pure regen.

Fix imported_variants map type so regen emits valid Rust (Map<String,String>
for unambiguous owners only), then regen stage0 to match .dag fixed point.

Co-authored-by: Cursor <cursoragent@cursor.com>

---------

Co-authored-by: Brian Searls <briansearls1@gmail.com>
Co-authored-by: Cursor <cursoragent@cursor.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