Refactor container classification and zero-arg callable detection

[briansrls]

Summary

This PR refactors container type classification and centralizes zero-arg callable reference detection across the compiler backends.

Key Changes

Container Classification Refactoring

• Replaced hardcoded coercion_keyed_container_names() and coercion_element_container_names() functions with dynamic is_keyed_container() and is_element_container() functions
• New functions derive container classification from container_expected_arity() instead of maintaining separate lists
• Keyed containers are identified by arity 2 (e.g., Map, PartialFunction)
• Element containers are identified by arity 1 (e.g., List, Set, NonEmptyList, etc.)
• Updated both DAG source files and generated Rust code

Zero-Arg Callable Reference Detection

• Moved is_zero_arg_callable_ref() function from v2_compiler_emit_rust.rs to shared v2_compiler_emit.rs
• This function detects when a variable reference to a zero-argument function needs explicit parentheses in code generation
• Checks for FunctionValueBinding kind and Callable type with zero parameters
• Now used consistently across all backends (Rust, Go, Python)

Backend-Specific Updates

• Go emitter: Enhanced emit_go_expr_var() to handle zero-arg callable references and special keywords (none/true/false)
• Python emitter: Enhanced emit_py_expr_var() with same improvements as Go
• Rust emitter: Removed local is_zero_arg_callable_ref() definition and now imports from shared emit module
• Updated imports in all three backend files to include VarBindingKind and FunctionValueBinding

Implementation Details

• Container arity checking now delegates to the standard library's container_expected_arity() function, reducing duplication
• Zero-arg callable detection is now a single source of truth used by all code generators
• Variable reference emission now properly handles function references that require invocation syntax

https://claude.ai/code/session_01HcSSdXfjWoKWiPkquYnCCH

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 1e4a5b8424

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

Remove uppercase None from null-literal check

None is not a reserved DAG keyword (dag_keyword_set only marks lowercase none/null), so a user can legally bind an identifier named None and rely on emit_ident to escape it for the target language. This new branch rewrites ExprVar named None to the null literal, so references to that binding are emitted as constants (nil in Go, and similarly in emit_py_expr_var) instead of the variable value, which changes program semantics.

Useful? React with 👍 / 👎.

[briansrls]

Good catch — fixed in 19af457. Removed the uppercase None check from both Go and Python emit_expr_var. Only lowercase "none" (the actual DAG keyword) is now mapped to the null literal.

Note: the Rust emitter has the same pre-existing name == "None" check at emit_var_ref (05_emit_rust.dag:1714). That's a known L1 violation to be resolved upstream via resolver normalization, not in this PR.

---

Generated by Claude Code

[briansrls]

Review (INVARIANTS: 2, MODELING: 0+/0-, ROADMAP: 0✓/1!)

INVARIANTS — Violations (2)

ROADMAP — Incomplete

• CG-2 TLC-1: The PR extends the emitter-side nullary-call guardrail to Go and Python, but the item remains partial because nullary invocation is still decided in emit instead of being normalized to ExprCall upstream.

ROADMAP — Unclaimed

• MM-3 nullary invocation / Go-Python nullary refs: This PR advances the roadmap subtask that Go and Python emit should handle nullary function references, but ROADMAP.md was not updated to record that partial progress.

The PR improves cross-backend coverage, but it still deepens an acknowledged emitter-side nullary-call workaround and introduces a coercion helper whose classification is less faithful than the data it replaces.

[briansrls]

Invariant violation: is_zero_arg_callable_ref still makes an emit-time nullary-call decision by testing rt.name == "Callable", which uses a name proxy for missing upstream call structure and violates Boundary sufficiency and Emission is translation, not decision-making.

[briansrls]

Acknowledged — this is a pre-existing L1 violation (was in 05_emit_rust.dag, moved to shared 05_emit.dag). Added documentation comment on the function marking the boundary sufficiency violation and noting the dissolution path (M4/Tier 2.6: inference normalizes ExprVar→ExprCall for nullary invocation). ROADMAP TLC-1 updated to record the Go/Python progress and the remaining upstream work. Fixed in 6bdb2de.

---

Generated by Claude Code

[briansrls]

Fixed in 6bdb2de. Added coercion_container_arity that resolves both surface names (via container_expected_arity) AND algebra names (via container_to_algebra → InhabitantDecl.arity). is_keyed_container("PartialFunction") and is_element_container("FreeMonoid") now return true.

---

Generated by Claude Code

[briansrls]

ChatGPT Review

I reviewed the uploaded diff directly; the GitHub PR page itself wasn’t accessible from here.

My take: good direction on container classification, but I’d leave one blocking review comment and two non-blocking modeling concerns.

1) Blocking: shared zero-arg callable detection is still an emitter-side heuristic

In src/v2/05_emit.dag, the new shared helper added around the diff’s lines 209–225:

dagfn is_zero_arg_callable_ref(binding_kind: VarBindingKind?, resolved_type: InferredNode?) -> Bool {  ...  Some { value: Resolved { node: rt } } => rt.name == "Callable" && (rt.params |> count) == 0

The code comment is already honest that this is an L1 violation. I agree with that assessment.

Why I’d block on it:

• It is still using a name proxy (rt.name == "Callable") plus shape/arity to recover a semantic fact that should already be explicit at the emit boundary.
• Moving it from Rust-only into shared 05_emit.dag, and then consuming it from Go/Python (05_emit_go.dag lines ~251–269, 05_emit_python.dag lines ~295–311), widens the same invariant violation to more backends.
• That conflicts with the attached INVARIANTS.md sections on Boundary sufficiency, Heuristics indicate lost structure, Facts flow forward, and Emission is translation, not decision-making: emit should not be rediscovering “is this a call or a reference?” from type names and arity.

Behaviorally, this also bakes in one side of an unresolved ambiguity: a zero-arg function value reference now becomes name() in more places, but the actual call-vs-reference distinction is still not modeled upstream.

So I’d comment something like:

This helper is useful as a temporary guardrail, but sharing it across all backends cements an emitter-side semantic decision that the roadmap still says is unresolved. Please either keep it scoped as a temporary backend patch with very explicit follow-up/tests, or move the call/reference distinction upstream before broadening it.

2) Non-blocking but important: TypeRendering.connective looks like speculative / inconsistent metadata

src/v2/04_emit_info.dag adds:

dagtype TypeRendering {  type_name: String  connective: Connective  ...}

That is a modeling smell for two reasons.

First, ROADMAP’s TypeRendering direction says the shape is supposed to be emergent from populated edges, not tag-driven. Adding an explicit connective field moves back toward tag metadata rather than structural derivation.

Second, the assignments are not consistent enough yet to be authoritative:

• In the leaf path, tuple/container-ish cases are set to NoConnective (diff lines ~95–144).
• In build_type_rendering_conj, tuple/map/element containers are set to Conj (diff lines ~148–178).
• Even fallback/error-ish constructions like Refined without a child or anonymous product/coproduct errors get Conj / Disj.

So today connective looks less like “canonical structural fact” and more like “which builder path produced this value.”

That brushes against the INVARIANTS.md guidance that annotations are not facts, and that new boundary facts should land end-to-end rather than as speculative metadata.

I would suggest one of two paths:

• remove connective from this PR until a same-PR consumer justifies it, or
• define very tight semantics for it and compute it in one canonical place so the same type cannot be NoConnective in one path and Conj in another.

3) Container classification refactor is directionally good

The src/v2/coercion.dag change around diff lines ~351–399 is the strongest part of the PR.

Replacing hardcoded lists:

• coercion_keyed_container_names
• coercion_element_container_names

with arity-based classification via:

• std.types.container_expected_arity
• fallback through dag_container_to_algebra(...)
• lookup_inhabitant(...).arity

is much closer to the repo’s “import from the authority / don’t define at use-site” rule from INVARIANTS.md.

So on compositional modeling quality, this part is a win.

My caveats:

• The fallback currently hardcodes lookup_inhabitant(target: Rust, ...). That makes Rust’s extdep registry the accidental authority for a shared classification question. Since arity is really a target-independent structural fact, long-term this should come from target-agnostic type/algebra declarations, not one backend.
• This is still not the ROADMAP Tier 3 end state. It is better than string lists, but it still starts from names and maps rather than deriving identity from the type declarations themselves.

4) ROADMAP alignment: the wording is honest, but this does not complete the lane

The ROADMAP.md edit is accurate as written:

• it says is_zero_arg_callable_ref moved to shared 05_emit.dag
• it explicitly still marks TLC-1 as Partial
• it explicitly says rt.name == "Callable" remains an L1 violation

That is good. No overclaim there.

What I would call out in review is that this PR should not be interpreted as closing the underlying roadmap items around:

• CallableOf
• cleanup of partial_function_templates
• deriving kernel/container identity from structural declarations instead of proxy name maps

Those remain open.

5) General code review notes

A few smaller points:

• I didn’t see any test changes in the diff. Given that this PR changes emit behavior across three backends, I’d want regression tests before merge.
• Minimum test set:
  • zero-arg local function ref emits as call in Rust/Go/Python
  • non-zero-arg function ref stays as a reference
  • imported zero-arg function ref (ROADMAP says this is still untested)
  • callback-as-value case, so the unresolved ambiguity is at least visible in tests
  • container classification by both surface names (List, Map) and algebra names (FreeMonoid, PartialFunction)
• I’d also do a full-tree grep for any remaining callers of the deleted coercion_keyed_container_names / coercion_element_container_names. I didn’t see caller rewrites in this diff.
• The stage0 mirrors do appear updated in lockstep with the .dag changes, so I do not see a separate stage0 divergence issue inside this PR.

Merge recommendation

I’d mark this as request changes on the shared is_zero_arg_callable_ref change.

Everything else is either good progress (coercion.dag) or follow-up debt (TypeRendering.connective), but broadening an acknowledged emitter-side L1 heuristic across all backends is the one thing here I wouldn’t merge without either upstream modeling or stronger guardrail tests.

---

View conversation

[briansrls]

Review (INVARIANTS: 2, MODELING: 1+/1-, ROADMAP: 1✓/1!)

INVARIANTS — Violations (2)

MODELING — Strengths

• src/v2/04_emit_info.dag Reusing the existing Connective primitive on TypeRendering is directionally compositional because it models product/coproduct form with an existing structural authority instead of inventing a renderer-only enum.

MODELING — Improvements

• src/v2/04_emit_info.dag TypeRendering.connective should either replace an existing render-time structural decision now or stay out of the boundary until a consumer exists, otherwise it is duplicate semantic state rather than a faithful authority.

ROADMAP — Verified

• CG/TLC-1 partial: The diff does move is_zero_arg_callable_ref into shared 05_emit.dag and wires Go/Python emit_*_expr_var through it, which supports the new roadmap claim.

ROADMAP — Incomplete

• CG/TLC-1: The work is still only partial because is_zero_arg_callable_ref continues to infer call syntax from rt.name == "Callable" in emit instead of from upstream IR call structure.

The PR makes the intended partial TLC-1 progress and fixes the earlier container-domain regression, but it still leaves the known emit-time Callable-name proxy in place and introduces one unused boundary fact plus one Rust-specific authority for shared container arity.

[briansrls]

Invariant violation: Adding connective to TypeRendering without any downstream consumer makes the new boundary fact speculative duplicate state, violating Explicit boundary contracts' rule that new semantic boundaries must land end-to-end.

[briansrls]

Agreed — reverted in 1365e88. Removed TypeRendering.connective entirely. Will re-add with an end-to-end consumer when a concrete render-time decision reads it.

---

Generated by Claude Code

[briansrls]

Fixed in 1365e88. Replaced lookup_inhabitant(target: Rust, ...) with a target-independent algebra_arity data table. Arity is an algebra property — the Rust-specific dispatch was wrong. The new table is the cross-language authority until std/algebra.dag carries arity natively.

---

Generated by Claude Code

[briansrls]

Review (INVARIANTS: 1, MODELING: 1+/0-, ROADMAP: 1✓/1!)

INVARIANTS — Violations (1)

MODELING — Strengths

• src/v2/coercion.dag The new coercion_container_arity composes surface container names with algebra names, which is more faithful than the prior surface-name-only model.

ROADMAP — Verified

• TLC-1: Call syntax / reference distinction: The diff moves is_zero_arg_callable_ref into shared 05_emit.dag and wires both Go and Python emit_*_expr_var paths to consult it, matching the roadmap note.

ROADMAP — Incomplete

• TLC-1: Call syntax / reference distinction: The item remains partial because the code still relies on emit-time callable-name/arity detection and adds no coverage for imported zero-arg function refs, exactly as the roadmap caveat says.

The PR resolves the two earlier coercion-authority issues and accurately records the TLC-1 refactor, but it replaces them with a new local algebra-arity table that already diverges from the canonical algebra definitions.

[briansrls]

Agreed — deleted entirely in 69a4cf5. algebra_arity was wrong (OrderedRing is arity 1, not 0) and duplicative of information already in algebra type declarations. Also removed the dead coercion_container_arity, is_keyed_container, and is_element_container functions — they had no consumers (replaced dead data with dead functions).

---

Generated by Claude Code

[briansrls]

Review (INVARIANTS: 2, MODELING: 1+/1-, ROADMAP: 1✓/0!)

INVARIANTS — Violations (2)

MODELING — Strengths

• dsl/std/types.dag Moving container-to-algebra knowledge next to the container aliases is directionally better than leaving it buried in v2.compiler.coercion.

MODELING — Improvements

• dsl/std/types.dag The mapping should be derived from the alias/algebra declarations themselves, not maintained as a string table of surface-name variants and algebra self-names.

ROADMAP — Verified

• TLC-1 partial: The code does move is_zero_arg_callable_ref into shared 05_emit.dag, and both Go and Python emit_*_expr_var now call it.

The PR fixes the earlier coercion-arity regressions and shares nullary-call handling across backends, but it still introduces a duplicate string-keyed container authority and reintroduces backend-parallel item classification logic.

[briansrls]

Invariant violation: container_to_algebra re-states the List/Set/Map to algebra facts as a hand-maintained Map<String, String>, creating a duplicate string-keyed authority instead of deriving from the alias declarations, which violates No duplicate representations and the Modeling Faithfulness invariant.

[briansrls]

Acknowledged — this is a pre-existing duplicate (was in coercion.dag, moved to std/types.dag for co-location). The table already has a TODO: "derive programmatically from type alias declarations once M4 structural identity provides a reflection mechanism." The .dag language currently has no reflection/introspection capability to read type alias bodies at the data-declaration level.

---

Generated by Claude Code

[briansrls]

Fixed in a3e0936. Extracted shared boolean predicates (is_type_def_item, is_function_item, etc.) in 05_emit.dag. All 3 backends now call the shared predicates — no parallel structural logic. The predicates are composable queries on Node fields, not a taxonomy enum.

---

Generated by Claude Code

[briansrls]

Review (INVARIANTS: 0, MODELING: 1+/1-, ROADMAP: 1✓/1!)

MODELING — Strengths

• dsl/std/types.dag Moving the container-to-algebra relation up into std/types.dag is better layering than keeping that fact in src/v2/coercion.dag, because the relation is language-level rather than compiler-local.

MODELING — Improvements

• dsl/std/types.dag Make container_to_algebra_name a derivation from the alias declarations themselves so naming variants do not become a second string-keyed authority for the same algebra fact.

ROADMAP — Verified

• TLC-1 partial: The diff does move is_zero_arg_callable_ref into shared emit code and wires Go and Python variable emission through it, matching the roadmap update.

ROADMAP — Incomplete

• TLC-3: Indexing / character access semantics: The roadmap now marks TLC-3 done, but this diff does not change any index/slice emission paths or IndexingSemantics data, so the claimed completion is not supported by the code shown.

The PR improves shared emit structure and resolves some prior review points, but it still relies on already-known modeling debt and it overstates TLC-3 as complete in ROADMAP.md.

[briansrls]

Review (INVARIANTS: 0, MODELING: 1+/1-, ROADMAP: 1✓/1!)

MODELING — Strengths

• dsl/std/types.dag Moving the container-to-algebra lookup out of src/v2/coercion.dag and up beside the standard type aliases improves layering by keeping the fact at the L0 type authority instead of a compiler helper.

MODELING — Improvements

• dsl/std/types.dag container_to_algebra is still a hand-maintained string map for facts already expressed by the alias declarations, so it should be derived from those declarations or algebra edges rather than duplicated.

ROADMAP — Verified

• TLC-1 partial keyword mapping: The new Go/Python ExprVar branches route none, true, and false through emit_keyword, which supports the narrowed keyword-mapping claim.

ROADMAP — Incomplete

• TLC-3: Indexing / character access semantics: ROADMAP.md marks TLC-3 done, but this diff does not land the claimed Go/Python per-collection indexing changes; the shown code changes are item-dispatch refactors and keyword handling.

ROADMAP — Unclaimed

• MM-1: Item identity: This PR partially advances MM-1 by removing TypedItemKind-based backend dispatch in favor of structural predicates, but ROADMAP.md does not record that partial progress or the remaining shared-walk gap.

The diff resolves the earlier coercion-arity issues and makes some structural progress, but it still carries the previously flagged nullary-call/container-authority problems and overclaims TLC-3 completion in ROADMAP.

[briansrls]

Review (INVARIANTS: 0, MODELING: 1+/1-, ROADMAP: 2✓/1!)

MODELING — Strengths

• dsl/std/types.dag Moving the container-to-algebra fact into std.types puts it at the standard type-model layer instead of a v2 compiler tool layer.

MODELING — Improvements

• dsl/std/types.dag Derive container_to_algebra from the List = FreeMonoid and Map = PartialFunction alias authorities rather than maintaining a parallel Map<String, String>.

ROADMAP — Verified

• CG-2 TLC-1 partial: The diff adds Go/Python keyword emission for none/true/false while keeping zero-arg callable detection Rust-only, matching the updated partial-status note.
• MM-1 taxonomy dissolution: The diff deletes TypedItemKind and classify_typed_item and rewires Rust, Go, and Python emitters to shared structural item predicates in 05_emit.dag.

ROADMAP — Incomplete

• MM-1 fail-closed boundaries: emit_py_typed_item still has a # unhandled node: ... fail-open branch, so the new [x] claim that fail-open fallbacks are deleted is not supported by the code.

The refactor makes real progress on shared authorities, but ROADMAP.md overclaims MM-1 completion while a Python fail-open item-emission path still remains.