spec: ct-tidy — canonical-type-names follow-up sweep

docs/specs/2026-05-12-ct-tidy.md

@@ -0,0 +1,423 @@
+# Canonical-Type-Names Tidy Sweep — Design Spec
+
+**Date:** 2026-05-12
+**Status:** Draft — awaiting user spec review
+**Authors:** Brummel (orchestrator) + Claude
+
+## Goal
+
+Close three follow-up todos that the canonical-type-names milestone
+(ct.1 → ct.4, closed 2026-05-11) explicitly deferred or surfaced.
+All three touch workspace / env / registry mechanics and share the
+theme "structural gaps the canonical-type-names milestone declared
+out-of-scope". Bundling them into one tidy milestone keeps the
+closeout coherent rather than scattering three orphan tidy patches
+across the next several weeks.
+
+The three items:
+
+1. **env-overlay shape ratification + narrowing.** Decide whether
+   the env's parallel `types` / `ctor_index` maps should collapse
+   into one overlay or stay split, anchor the decision in
+   `docs/DESIGN.md`, and narrow `check_in_workspace`'s per-module
+   overlay rebuild symmetric to ct.3.2's mono-side fix.
+2. **`Registry.type_def_module` re-key.** Replace the
+   `BTreeMap<String, String>` keyed by bare type name with a key
+   that survives cross-module bare-name collisions.
+3. **`KindMismatch` retire.** Delete the
+   `WorkspaceLoadError::KindMismatch` variant and `walk_kind_mismatch`
+   helper — structurally unreachable post-ct.1, ratified by the
+   already-green test `class_param_in_applied_position_fires_canonical_form_rejection`
+   at `crates/ailang-core/src/workspace.rs:1867`.
+
+Items deliberately **not** in scope: CLI human-mode diagnostic
+surface for `WorkspaceLoadError` (different layer — diagnostic
+routing, not workspace mechanics); Split `BadCrossModuleTypeRef`
+(diagnostic shape); Workspace search beyond entry-module's
+directory (CLI / IO layer); `io/print_float` `.0` fallback
+(runtime printer asymmetry); Boehm retirement (memory
+subsystem); rustdoc warning sweep (documentation). Each of these
+is a coherent tidy in its own right but does not share the
+workspace-mechanics theme.
+
+## Architecture
+
+The milestone is three iterations, ordered from highest-design-
+substance to lowest. The order is deliberate: the env-overlay
+decision in ctt.1 is the only one carrying a real choice, so it
+happens first while the design context is fresh. ctt.2 and ctt.3
+are mechanical follow-ups whose shape does not depend on ctt.1.
+
+### Iteration ctt.1 — env-overlay shape ratification + narrowing
+
+**Design substance:** Today `Env` carries two parallel maps that
+are rebuilt in lockstep:
+
+- `env.types: IndexMap<String, TypeDef>` — bare-type-name →
+  full TypeDef. The owning index for type definitions.
+- `env.ctor_index: IndexMap<String, CtorRef>` — bare-ctor-name →
+  owning-type-name. A reverse index over `env.types`.
+
+The "shape question" from `docs/journals/2026-05-10` ("Audit close:
+env-construction unify") is: collapse into one overlay
+(`Map<Name, EntryKind>` with `Type(TypeDef) | Ctor(owner)`) or
+stay split?
+
+**Decision: stay split.** Reason (semantic, not effort): the two
+maps have distinct semantic roles. `env.types` is an *owning*
+index — it stores the data. `env.ctor_index` is a *reverse*
+index — it accelerates `ctor-name → type-name` lookup over the
+same data. A collapsed variant-keyed map would conflate these
+roles and force every consumer to discriminate `EntryKind` at
+the call site. The current split lets type-consumers iterate
+`env.types` directly and ctor-consumers query `env.ctor_index`
+directly; neither needs to know the other exists.
+
+This rationale gets anchored in `docs/DESIGN.md` as a new
+top-level section `## Env construction`, placed between the
+existing `## Convention: qualified cross-module references`
+(`docs/DESIGN.md:1941`) and `## Data model` (`docs/DESIGN.md:1959`).
+DESIGN.md today has no env / overlay section, so this iter
+introduces the heading. The initial body is one short paragraph
+naming the split (`env.types` owning vs. `env.ctor_index`
+reverse-index) with the rationale above; future env / overlay
+mechanics can extend the section without re-anchoring. Future
+tidies cannot re-litigate the split without first amending this
+section.
+
+**Code change: one new pinning test.** The DESIGN.md anchor is
+the primary deliverable, plus the roadmap close. The iter also
+adds one positive test that pins the load-bearing
+`DuplicateCtor` consumer, because the in-band check at
+`crates/ailang-check/src/lib.rs:1366-1376` has no in-tree test
+ratifying it today (the variant is emitted in the production
+code path but no fixture currently triggers it). The pinning
+test is what makes the ratification load-bearing rather than
+documentation-of-belief — the asymmetry-with-mono-side argument
+rests on this consumer being real.
+
+Inspection of the existing per-module rebuild at
+`crates/ailang-check/src/lib.rs:1342-1384` confirms both halves
+are load-bearing:
+
+- `env.types.clear()` + rebuild: needed for the local
+  `DuplicateType` diagnostic at `lib.rs:1358-1364` and for
+  bare-name `Type::Con` lookup against the current module.
+- `env.ctor_index.clear()` + rebuild: emitted into the in-band
+  `DuplicateCtor` diagnostic at `lib.rs:1366-1376`, which
+  consults the rebuilt per-module `env.ctor_index` to detect
+  duplicates *within the current module*. The new pinning test
+  is the consumer evidence.
+
+The ct.3.2 mono-side fix narrowed the *mono overlay* because the
+mono-side `env.ctor_index` was only feeding runtime ctor lookup
+(type-driven post-ct.2.2). The check-side overlay is shaped
+differently — it serves the workspace-build-time duplicate
+diagnostic — so the analogous narrowing does not apply.
+
+The roadmap todo "check_in_workspace per-module overlay narrowing"
+closes via this finding: the rebuild stays, the asymmetry with
+the mono-side is intentional and now documented. The roadmap
+todo "types / ctor_index overlay shape question" closes via the
+DESIGN.md anchor recording the split rationale.
+
+If a future recon surfaces a non-DuplicateCtor consumer of the
+post-rebuild `env.ctor_index` that *could* survive removal of
+the per-module rebuild, the question re-opens; until then the
+ratification holds.
+
+### Iteration ctt.2 — `Registry.type_def_module` re-key
+
+**Bug being closed:** `crates/ailang-core/src/workspace.rs:106`
+declares `type_def_module: BTreeMap<String, String>`. Today this
+map is bare-name → defining-module. The doc-comment at
+`workspace.rs:97-105` flags the issue: if two modules each
+declare `type Foo`, the second insert at
+`workspace.rs:547` silently overwrites the first; subsequently
+`normalize_type_for_registry` (`workspace.rs:892`) qualifies
+`Foo` to whichever module won the insert race.
+
+For the current corpus (every in-tree fixture uses distinct bare
+type names across modules) the bug is dormant. The proper fix is
+explicitly written into the doc-comment: re-key as
+`(owning_module, bare_name) -> defining_module` and thread the
+calling module through every consumer site.
+
+**Re-keying choice.** Two shapes are conceivable:
+
+- `BTreeMap<(String, String), String>` keyed by
+  `(owning_module, bare_name)` — straightforward extension.
+- `BTreeMap<String, Vec<(String, String)>>` keyed by bare name,
+  value being a list of `(owner, defining_module)` pairs —
+  preserves the bare-name-as-primary-lookup intuition but forces
+  every consumer to handle the multi-candidate case.
+
+**Decision: tuple key.** Reason (semantic): the function the map
+serves — "given a type reference written in module M, where is
+its type defined?" — already requires the owning module M as
+input. The tuple key makes that input explicit at the API
+surface. The list-valued alternative would defer
+multi-candidate handling to every consumer.
+
+**Consumer site count.** `grep "type_def_module"` returns 13 hits
+in `workspace.rs`. Most are inside the workspace-load function
+where the calling module is local context (loop variable
+`mod_name`), so threading is trivial. The interesting site is
+`normalize_type_for_registry` at `workspace.rs:879-928`, which
+today takes `type_def_module: &BTreeMap<String, String>` as a
+parameter. The signature changes to accept the calling module
+as well, and the map type changes to
+`&BTreeMap<(String, String), String>`. All call sites already
+have the calling module in scope (the workspace-load loop
+threads it through `mod_name`).
+
+The `Registry::normalize_type_for_lookup` method at
+`workspace.rs:121-123` exposes `normalize_type_for_registry`
+publicly without a calling-module argument. The signature gains
+a `caller_module: &str` argument. The two test-only call sites
+at `workspace.rs:2525, 2549` use synthetic modules; both update
+trivially.
+
+**Test:** add a positive fixture where two modules each declare
+`type Foo` with distinct ctors, an instance on each, and confirm
+both instances resolve correctly under registry lookup. This
+catches the regression: under the old bare-name key, one of the
+two instances would silently lose its registry entry.
+
+### Iteration ctt.3 — `KindMismatch` retire
+
+**Path being deleted:**
+
+- `WorkspaceLoadError::KindMismatch` variant
+  (`crates/ailang-core/src/workspace.rs:259-264`).
+- `walk_kind_mismatch` helper
+  (`crates/ailang-core/src/workspace.rs:834-854`).
+- Dispatch site in `validate_classdefs`
+  (`crates/ailang-core/src/workspace.rs:786-795`).
+- Display arm in `crates/ail/src/main.rs:1163` for the
+  human-mode diagnostic.
+
+**Ratification:** Already-green test
+`class_param_in_applied_position_fires_canonical_form_rejection`
+(`crates/ailang-core/src/workspace.rs:1867-1888`) demonstrates
+that the malformed shape `class C f { foo : f Int -> f Int }`
+fires `BareCrossModuleTypeRef` before `validate_classdefs`
+runs. This is the structural-unreachability claim made by the
+roadmap entry, ratified by a workspace-loaded test that
+currently passes.
+
+The test must keep passing after the deletion. It asserts on
+`BareCrossModuleTypeRef`, not `KindMismatch`, so no test edit
+is required.
+
+**Defensive vs. retired:** The doc comment at
+`workspace.rs:1858-1866` reads "stays in the codebase as
+dead-but-defensive code; a future tidy may retire it." This
+iter is that future tidy. The "defensive" framing was a
+deliberate hedge during ct.1 in case a canonical-form bug let
+malformed shapes through later — none has surfaced. Carrying
+the dead branch indefinitely is itself a hazard: future
+refactors of the workspace error-enum touch shapes that no
+code path produces. Retire now, before the dead branch
+accumulates churn.
+
+If a future canonical-form bug *does* let a malformed shape
+through, the right repair is to fix the canonical-form
+validator, not to resurrect `KindMismatch`. The diagnostic
+that fires today (`BareCrossModuleTypeRef`) is structurally
+correct: a class param `f` used as `f Int` is, post-ct.1, a
+reference to a non-existent type `f` in the owning module —
+that *is* a bare-cross-module type ref. The earlier
+`KindMismatch` diagnostic was a less-precise pre-ct.1 framing.
+
+## Components
+
+### Files touched per iter
+
+**ctt.1 — env-overlay shape ratification + DuplicateCtor pin:**
+- `docs/DESIGN.md` — new top-level section `## Env construction`
+  inserted between `## Convention: qualified cross-module
+  references` (line 1941) and `## Data model` (line 1959).
+  Initial body: one paragraph naming the `env.types` /
+  `env.ctor_index` split with the owning-vs-reverse-index
+  rationale.
+- `crates/ailang-check/tests/` — new test (path TBD by planner —
+  candidate: a new file `duplicate_ctor_pin.rs`, or appended to
+  an existing fixture-driven test module) that builds a one-
+  module fixture with two `Def::Type`s declaring same-named
+  ctors and asserts `check_workspace` returns
+  `CheckError::DuplicateCtor { ctor, a, b }`. The exact
+  assertion path follows the existing `CheckError`-asserting
+  test conventions in the crate.
+- `docs/roadmap.md` — strike both relevant todo lines
+  ("types / ctor_index overlay shape question" and
+  "check_in_workspace per-module overlay narrowing") with a
+  reference to `docs/journals/<this iter>.md`.
+
+**ctt.2 — `Registry.type_def_module` re-key:**
+- `crates/ailang-core/src/workspace.rs:106` — field type change
+  to `BTreeMap<(String, String), String>`.
+- `crates/ailang-core/src/workspace.rs:121-123` —
+  `Registry::normalize_type_for_lookup` gains `caller_module: &str`.
+- `crates/ailang-core/src/workspace.rs:537, 547` — Pass 1
+  collection: insert `(mod_name.clone(), t.name.clone())` instead
+  of `t.name.clone()`.
+- `crates/ailang-core/src/workspace.rs:656-657` —
+  `type_def_module.get(&type_repr)` becomes a per-module-aware
+  lookup (the call-site context provides `mod_name`).
+- `crates/ailang-core/src/workspace.rs:879-928` —
+  `normalize_type_for_registry` signature + body adapt to tuple
+  key + `caller_module` argument.
+- `crates/ailang-core/src/workspace.rs:2525, 2549` — test-only
+  call sites update.
+- `crates/ailang-core/src/workspace.rs:97-105` — doc-comment
+  flagging the bug is replaced with the new shape's invariant.
+- New test fixture: `examples/ct_tidy_cross_module_foo_collision.ail.json`
+  (or analogous), exercising two-module bare-name collision.
+- `crates/ailang-core/tests/` or `crates/ail/tests/e2e.rs` —
+  test that exercises the new fixture and asserts both
+  instances resolve correctly.
+
+**ctt.3 — `KindMismatch` retire:**
+- `crates/ailang-core/src/workspace.rs:259-264` — variant deletion.
+- `crates/ailang-core/src/workspace.rs:786-795` — dispatch site
+  deletion (the `walk_kind_mismatch(&method.ty, &c.param)` call
+  inside `validate_classdefs`).
+- `crates/ailang-core/src/workspace.rs:834-854` — helper deletion.
+- `crates/ail/src/main.rs:1163` — Display arm deletion.
+- `crates/ailang-core/src/workspace.rs:1858-1866` — the
+  "dead-but-defensive" doc comment on the existing test gets
+  dropped (the test stays; its rationale is now "the canonical-form
+  rejection is the path", which the test name already states).
+
+### Files NOT touched
+
+- `crates/ailang-check/src/lib.rs:1342-1384` — the per-module
+  overlay rebuild stays as-is after ctt.1's recon confirms
+  `DuplicateCtor` depends on it. The narrowing question closes
+  via DESIGN.md ratification, not code change. If planner-recon
+  for ctt.1 surfaces a consumer that *would* survive narrowing,
+  this changes; default expectation is no edit here.
+- `crates/ailang-codegen/` — no codegen edits. None of the three
+  items touch IR or runtime.
+- Snapshots — no IR-shape snapshots are affected. The bench
+  baseline is not affected (no codepath frequency change).
+
+## Data flow
+
+The three iters do not interact at the data level:
+
+- ctt.1 records a decision in DESIGN.md; no code-side data flow
+  change.
+- ctt.2 changes the registry's internal key shape but preserves
+  the public `Registry::normalize_type_for_lookup` semantics for
+  every call site that already has its calling module in scope.
+  External canonical-form behaviour is unchanged for the current
+  corpus (which has no bare-name collisions); the fix is
+  load-bearing only when a future workspace introduces a
+  collision.
+- ctt.3 deletes an unreachable branch; no data flow.
+
+Canonical-form bytes: unchanged. None of the three iters touches
+serialised AST shapes. ctt.2's registry-key change is internal —
+the canonical-form output of `normalize_type_for_registry` for
+any *valid* input is the same as today (the bug surfaces only on
+the malformed-collision input that the current corpus doesn't
+contain, and the new behaviour is "qualify to the correct
+owner" rather than "qualify to whichever module won the race",
+which is by definition a fix not a shape change).
+
+## Error handling
+
+**ctt.1:** no new diagnostics. The ratification is documentation-
+only at the analyser level.
+
+**ctt.2:** no new diagnostics. The `DuplicateInstance` /
+`OrphanInstance` paths at `workspace.rs:682, 660` already cover
+the collision-error space; the re-key fixes the silent
+collision by ensuring the right registry entry survives, which
+means a previously-masked instance becomes visible to the
+existing `DuplicateInstance` detector when its canonical-form
+key matches another (and is not falsely promoted to "duplicate"
+when it actually belongs to a different owner).
+
+**ctt.3:** one diagnostic variant goes away. The Display path
+in `main.rs:1163` deletes the human-mode `[kind-mismatch]`
+line; the structurally-identical class-param-in-applied-position
+malformed shape now produces `[bare-cross-module-type-ref]`
+instead. Existing user-facing diagnostic for the canonical
+malformed-fixture is the latter (the existing test confirms
+this).
+
+## Testing strategy
+
+**ctt.1:**
+- New pinning test: one-module fixture with two `Def::Type`s
+  declaring same-named ctors. Asserts `check_workspace` returns
+  `CheckError::DuplicateCtor { ctor, a, b }` with `a` and `b`
+  naming the two types. Catches any future regression that
+  silently removes the per-module `env.ctor_index` rebuild —
+  if the rebuild goes away, this test goes red, which is
+  exactly the load-bearing-asymmetry-argument's safety net.
+- `cargo test --workspace` continues to pass with all other
+  tests unchanged.
+- DESIGN.md anchor presence verified manually (no schema-drift
+  test exists for the env section yet — see roadmap item
+  "design_schema_drift.rs fidelity widening", out-of-scope here).
+- Roadmap close: both relevant lines struck through.
+
+**ctt.2:**
+- New fixture: two modules each declaring `type Foo` with
+  distinct ctors; an instance on each. Test: both instances
+  registerable, both retrievable by their canonical-form
+  registry key, no `DuplicateInstance` false positive, no
+  silent overwrite.
+- Existing test suite must continue to pass (no current fixture
+  exercises the collision case, so no existing test regresses).
+
+**ctt.3:**
+- Existing test `class_param_in_applied_position_fires_canonical_form_rejection`
+  (`crates/ailang-core/src/workspace.rs:1867`) continues to
+  pass, asserting `BareCrossModuleTypeRef` on the malformed
+  fixture.
+- `cargo test --workspace` continues to pass (no other test
+  references `KindMismatch` — `grep KindMismatch` returns the
+  three deletion sites only).
+- `cargo build --workspace` succeeds (no orphan match arms,
+  no dead-code warnings introduced).
+
+**Workspace-wide gate (all three iters):**
+- `bench/cross_lang.py`, `bench/compile_check.py`,
+  `bench/check.py` all green. No metric should plausibly
+  shift, but the audit at milestone close confirms.
+
+## Acceptance criteria
+
+- `docs/DESIGN.md` carries a new top-level section
+  `## Env construction` (placed between `## Convention:
+  qualified cross-module references` and `## Data model`)
+  whose initial paragraph anchors the `env.types` /
+  `env.ctor_index` split with the semantic rationale (owning
+  index vs. reverse index).
+- A new pinning test in `crates/ailang-check/tests/` (path
+  finalised in planning) asserts `CheckError::DuplicateCtor`
+  fires for a one-module fixture with two `Def::Type`s
+  declaring same-named ctors — ratifies the consumer the
+  ctt.1 ratification rests on.
+- `Registry.type_def_module` key is `(String, String)`, value
+  is `String`. Every consumer site provides the calling module
+  at the call site. A regression test exercises two-module
+  bare-name collision and confirms both instances resolve
+  correctly.
+- `WorkspaceLoadError::KindMismatch`, `walk_kind_mismatch`, its
+  dispatch site, and its `main.rs` Display arm are deleted. The
+  existing test
+  `class_param_in_applied_position_fires_canonical_form_rejection`
+  still passes, asserting `BareCrossModuleTypeRef`.
+- `cargo test --workspace`, `bench/cross_lang.py`,
+  `bench/compile_check.py`, `bench/check.py` all green at
+  milestone close.
+- `docs/roadmap.md` has the three relevant todo lines struck
+  through with reference to this spec or the per-iter journals.
+- `docs/WhatsNew.md` carries one entry covering the milestone
+  in user-as-reader prose (no internals, no iter codes).