plan: ctt.1 — env-overlay shape ratification, 3 tasks

2026-05-12 22:02:58 +02:00
parent ff5dc7f4e7
commit 0a36294e61
1 changed files with 339 additions and 0 deletions
@@ -0,0 +1,339 @@
+# ctt.1 — Env-overlay shape ratification — Implementation Plan
+
+> **Parent spec:** `docs/specs/2026-05-12-ct-tidy.md`
+>
+> **For agentic workers:** REQUIRED SUB-SKILL: use `skills/implement`
+> to run this plan. Steps use `- [ ]` checkboxes for tracking.
+
+**Goal:** Anchor the `env.types` / `env.ctor_index` split decision
+in `docs/DESIGN.md`, pin the `DuplicateCtor` consumer the spec's
+asymmetry-with-mono-side argument rests on, and close two
+roadmap todos.
+
+**Architecture:** One new behavioural-pin integration test under
+`crates/ailang-check/tests/duplicate_ctor_pin.rs` exercising
+`CheckError::DuplicateCtor` via the public `check_workspace`
+entry point. One DESIGN.md top-level section insertion
+(`## Env construction`). Two strike-out edits in
+`docs/roadmap.md`. No production-code edits.
+
+**Tech Stack:** `ailang-check` (`check_workspace`, `Diagnostic`
+shape), `ailang-core` (`Module` / `TypeDef` / `Ctor` / `Workspace`
+inline construction via `SCHEMA`).
+
+**Files this plan creates or modifies:**
+
+- Create: `crates/ailang-check/tests/duplicate_ctor_pin.rs` —
+  pinning test for `CheckError::DuplicateCtor` emitted by the
+  per-module overlay rebuild at
+  `crates/ailang-check/src/lib.rs:1366-1376`.
+- Modify: `docs/DESIGN.md:1957-1959` — insert new top-level
+  section `## Env construction` between
+  `## Convention: qualified cross-module references` and
+  `## Data model`.
+- Modify: `docs/roadmap.md:79` — strike `[ ]` → `[x]` on the
+  "`types` / `ctor_index` overlay shape question" todo.
+- Modify: `docs/roadmap.md:183` — strike `[ ]` → `[x]` on the
+  "`check_in_workspace` per-module overlay narrowing" todo.
+
+---
+
+### Task 1: Pinning test for `CheckError::DuplicateCtor`
+
+**Files:**
+- Create: `crates/ailang-check/tests/duplicate_ctor_pin.rs`
+
+This task pins existing production behaviour — the
+`DuplicateCtor` variant is emitted by the per-module rebuild at
+`crates/ailang-check/src/lib.rs:1366-1376` and surfaced as a
+`Diagnostic` with `code == "duplicate-ctor"` via the
+`CheckError::code()` arm at `lib.rs:610`. No production code
+changes; the test catches a future regression that silently
+removes the per-module `env.ctor_index` rebuild.
+
+The TDD discipline an implementer might apply (write the test
+first, expect it to fail) does not map cleanly to a
+behavioural-pin test: the behaviour already exists. The
+expected outcome on first run is PASS. If the test fails on
+first run, that is itself a finding — it means either the
+inline-fixture is malformed or the production-code path has
+drifted from its documented shape.
+
+- [ ] **Step 1: Write the pinning test file**
+
+Path: `crates/ailang-check/tests/duplicate_ctor_pin.rs`
+
+```rust
+//! Pin for `CheckError::DuplicateCtor` — same-named ctors in two
+//! different ADTs within one module. Ratifies the load-bearing
+//! consumer of the per-module `env.ctor_index` rebuild in
+//! `check_in_workspace` that DESIGN.md §"Env construction" rests
+//! on as the asymmetry-with-mono-side argument. If the
+//! per-module rebuild is ever removed without an equivalent
+//! replacement path, this test goes red.
+
+use ailang_check::check_workspace;
+use ailang_core::ast::{Ctor, Def, Module, TypeDef};
+use ailang_core::workspace::{Registry, Workspace};
+use ailang_core::SCHEMA;
+use std::collections::BTreeMap;
+
+#[test]
+fn duplicate_ctor_across_two_adts_in_one_module() {
+    let m = Module {
+        schema: SCHEMA.into(),
+        name: "M".into(),
+        imports: vec![],
+        defs: vec![
+            Def::Type(TypeDef {
+                name: "Cat".into(),
+                vars: vec![],
+                ctors: vec![Ctor {
+                    name: "Twins".into(),
+                    fields: vec![],
+                }],
+                doc: None,
+                drop_iterative: false,
+            }),
+            Def::Type(TypeDef {
+                name: "Dog".into(),
+                vars: vec![],
+                ctors: vec![Ctor {
+                    name: "Twins".into(),
+                    fields: vec![],
+                }],
+                doc: None,
+                drop_iterative: false,
+            }),
+        ],
+    };
+    let mut modules = BTreeMap::new();
+    modules.insert(m.name.clone(), m.clone());
+    let ws = Workspace {
+        entry: m.name.clone(),
+        modules,
+        root_dir: std::path::PathBuf::from("."),
+        registry: Registry::default(),
+    };
+
+    let diags = check_workspace(&ws);
+    let dup = diags
+        .iter()
+        .find(|d| d.code == "duplicate-ctor")
+        .unwrap_or_else(|| {
+            panic!(
+                "expected at least one `duplicate-ctor` diagnostic, got: {diags:?}"
+            )
+        });
+    assert!(
+        dup.message.contains("Twins"),
+        "expected message to name the duplicate ctor `Twins`, got: {}",
+        dup.message
+    );
+    assert!(
+        dup.message.contains("Cat") && dup.message.contains("Dog"),
+        "expected message to name both ADTs `Cat` and `Dog`, got: {}",
+        dup.message
+    );
+}
+```
+
+- [ ] **Step 2: Run the test, confirm PASS**
+
+Run: `cargo test --workspace -p ailang-check --test duplicate_ctor_pin`
+Expected: `test duplicate_ctor_across_two_adts_in_one_module ... ok` (1 passed; 0 failed).
+
+If the test fails: the production-code path has drifted from
+its documented shape. Inspect
+`crates/ailang-check/src/lib.rs:1366-1376`; the in-band
+`DuplicateCtor` emission is the load-bearing consumer the test
+pins. Drift here invalidates the spec's asymmetry-with-mono-side
+argument and is itself a finding — pause the iter and surface it.
+
+- [ ] **Step 3: Confirm full workspace test suite stays green**
+
+Run: `cargo test --workspace`
+Expected: all existing tests pass (no regression). The new test
+adds one passing case; no existing test exercises the
+two-`Def::Type`-same-ctor shape, so no test changes its outcome.
+
+---
+
+### Task 2: DESIGN.md `## Env construction` section
+
+**Files:**
+- Modify: `docs/DESIGN.md:1957-1959`
+
+Inserts a new top-level section between the existing
+`## Convention: qualified cross-module references` (DESIGN.md:1941)
+and `## Data model` (DESIGN.md:1959). The initial body is
+three paragraphs: one naming the split, one stating the
+rationale (owning-vs-reverse-index roles), one documenting the
+check-side / mono-side asymmetry as intentional.
+
+- [ ] **Step 1: Insert the new section**
+
+Apply this exact Edit to `docs/DESIGN.md`:
+
+`old_string`:
+```
+Hash stability: no new AST node, no renamed fields — all previous module
+hashes stay bit-identical.
+
+## Data model
+```
+
+`new_string`:
+```
+Hash stability: no new AST node, no renamed fields — all previous module
+hashes stay bit-identical.
+
+## Env construction
+
+The check environment carries two parallel per-module overlays for
+ADT definitions: `env.types: IndexMap<String, TypeDef>` is the
+**owning index** — it stores the full definition keyed by bare type
+name. `env.ctor_index: IndexMap<String, CtorRef>` is the **reverse
+index** — it maps each constructor name back to its owning type, so
+ctor-by-name lookups run in O(1) rather than searching every type's
+ctor list.
+
+The split is not a refactor wart. The two maps have distinct
+semantic roles: an owning data index, and a derived lookup
+accelerator over the same data. Collapsing them into one
+variant-keyed map (`Map<Name, EntryKind>`) would force every
+type-iterator and every ctor-lookup site to discriminate the variant
+tag, reversing the optimisation the reverse index exists to
+provide.
+
+The check-side per-module rebuild of both overlays at
+`crates/ailang-check/src/lib.rs:1342-1384` is intentionally
+retained: the in-band `DuplicateCtor` diagnostic at the same site
+consumes the rebuilt per-module `env.ctor_index`. The mono-side
+overlay was narrowed to types-only at iter ct.3.2 because its
+consumer is the runtime ctor lookup, which became type-driven
+post-ct.2.2 — a different consumer story, hence a different
+overlay shape. The asymmetry between the check side and the mono
+side is by design and is pinned by
+`crates/ailang-check/tests/duplicate_ctor_pin.rs`.
+
+## Data model
+```
+
+- [ ] **Step 2: Verify section presence and ordering**
+
+Run: `grep -n '^## ' docs/DESIGN.md | grep -E 'Convention: qualified|Env construction|Data model'`
+Expected: three lines, in this order (line numbers approximate;
+the relative ordering is what matters):
+```
+1941:## Convention: qualified cross-module references
+1959:## Env construction
+1989:## Data model
+```
+The "Env construction" heading must appear between the other two.
+
+---
+
+### Task 3: roadmap.md strikes
+
+**Files:**
+- Modify: `docs/roadmap.md:79`
+- Modify: `docs/roadmap.md:183`
+
+Closes two P2 `[todo]` lines now superseded by the DESIGN.md
+anchor in Task 2 and the pinning test in Task 1. The strike
+follows the roadmap's `[ ]` → `[x]` convention; per
+`docs/roadmap.md`'s own header guidance, "a finished entry may
+stay briefly for context, then is removed (with a one-line
+mirror in `docs/journals/`)". The mirror lands in the per-iter
+journal at iter close.
+
+- [ ] **Step 1: Strike the "overlay shape question" todo**
+
+Apply this exact Edit to `docs/roadmap.md`:
+
+`old_string`:
+```
+- [ ] **\[todo\]** `types` / `ctor_index` overlay shape question —
+  decide whether the env's two parallel ctor maps should collapse
+  into one overlay, or stay split. Surfaced during the
+  env-construction unify audit.
+  - context: JOURNAL 2026-05-10 ("Audit close: env-construction unify")
+```
+
+`new_string`:
+```
+- [x] **\[todo\]** `types` / `ctor_index` overlay shape question —
+  decide whether the env's two parallel ctor maps should collapse
+  into one overlay, or stay split. Surfaced during the
+  env-construction unify audit.
+  - context: JOURNAL 2026-05-10 ("Audit close: env-construction unify"); closed by iter ctt.1 — DESIGN.md §"Env construction" anchors the split decision.
+```
+
+- [ ] **Step 2: Strike the "check_in_workspace per-module overlay narrowing" todo**
+
+Apply this exact Edit to `docs/roadmap.md`:
+
+`old_string`:
+```
+- [ ] **\[todo\]** `check_in_workspace` per-module overlay narrowing —
+  `crates/ailang-check/src/lib.rs:1234` still clears+rebuilds
+  `env.ctor_index` per-module; ct.3.2 narrowed the analogous mono
+  overlay to types-only. The typecheck-side overlay's `env.ctor_index`
+  half serves the duplicate-detection diagnostic at workspace-build
+  time, not the runtime ctor lookup (which is type-driven post-ct.2.2).
+  Narrowing is mechanical but needs a careful read to confirm no
+  other consumer survives.
+  - context: JOURNAL 2026-05-11 ("Iteration ct.4") — milestone close
+    follow-up.
+```
+
+`new_string`:
+```
+- [x] **\[todo\]** `check_in_workspace` per-module overlay narrowing —
+  `crates/ailang-check/src/lib.rs:1234` still clears+rebuilds
+  `env.ctor_index` per-module; ct.3.2 narrowed the analogous mono
+  overlay to types-only. The typecheck-side overlay's `env.ctor_index`
+  half serves the duplicate-detection diagnostic at workspace-build
+  time, not the runtime ctor lookup (which is type-driven post-ct.2.2).
+  Narrowing is mechanical but needs a careful read to confirm no
+  other consumer survives.
+  - context: JOURNAL 2026-05-11 ("Iteration ct.4") — milestone close
+    follow-up; closed by iter ctt.1 — recon confirmed the rebuild is the load-bearing consumer of in-band DuplicateCtor (pinned by `crates/ailang-check/tests/duplicate_ctor_pin.rs`); the asymmetry with the mono side is intentional.
+```
+
+- [ ] **Step 3: Verify both strikes**
+
+Run: `grep -nE '^- \[x\] \*\*\\\[todo\\\]\*\* (`types`|`check_in_workspace`)' docs/roadmap.md`
+Expected: two lines, both with `[x]`:
+```
+79:- [x] **\[todo\]** `types` / `ctor_index` overlay shape question —
+183:- [x] **\[todo\]** `check_in_workspace` per-module overlay narrowing —
+```
+
+- [ ] **Step 4: Workspace-wide regression check**
+
+Run: `cargo test --workspace`
+Expected: all tests pass, including the new pinning test from
+Task 1.
+
+The bench scripts (`bench/check.py`, `bench/compile_check.py`,
+`bench/cross_lang.py`) are not run per-iter at the plan level —
+they are run by the audit skill at milestone close. ctt.1 makes
+zero codegen-relevant edits, so no bench movement is plausibly
+attributable to it; the milestone-close audit confirms.
+
+---
+
+## Acceptance criteria recap (from spec)
+
+- `docs/DESIGN.md` carries the new `## Env construction` section
+  with the three-paragraph initial body. ✓ Task 2.
+- A new pinning test in `crates/ailang-check/tests/` asserts
+  `code == "duplicate-ctor"` for a one-module fixture with two
+  `Def::Type`s declaring same-named ctors, plus message-substring
+  match for the ctor name and both ADT names. ✓ Task 1.
+- Both relevant roadmap todos struck `[x]` with a one-line
+  forward reference to this iter. ✓ Task 3.
+- `cargo test --workspace` green. ✓ Task 1 Step 3 and Task 3
+  Step 4.