From 08abfdf7910a234e6a6738d9f793c76bf051000a Mon Sep 17 00:00:00 2001 From: Brummel Date: Sun, 10 May 2026 02:00:41 +0200 Subject: [PATCH] plan: env-construction unify --- .../2026-05-10-env-construction-unify.md | 650 ++++++++++++++++++ 1 file changed, 650 insertions(+) create mode 100644 docs/plans/2026-05-10-env-construction-unify.md diff --git a/docs/plans/2026-05-10-env-construction-unify.md b/docs/plans/2026-05-10-env-construction-unify.md new file mode 100644 index 0000000..727620a --- /dev/null +++ b/docs/plans/2026-05-10-env-construction-unify.md @@ -0,0 +1,650 @@ +# Env-construction unify — Implementation Plan + +> **Parent spec:** `docs/specs/2026-05-10-env-construction-unify.md` +> +> **For agentic workers:** REQUIRED SUB-SKILL: use `skills/implement` +> to run this plan. Steps use `- [ ]` checkboxes for tracking. + +**Goal:** Eliminate structural drift between `check_in_workspace` +and `mono::build_workspace_env` by extracting a single shared +`build_check_env(ws) -> Env` helper that both paths consume. + +**Architecture:** Introduce `pub fn build_check_env(ws: &Workspace) -> +Env` in `crates/ailang-check/src/lib.rs` covering every workspace-flat +seeding step (`effect_ops`, `types`, `ctor_index`, `module_globals`, +`module_types`, `module_imports`, `class_methods`, `class_superclasses`, +`workspace_registry`). Both call sites (`check_in_workspace` and +`mono::build_workspace_env`) become thin consumers; per-module overlay +fields (`current_module`, `globals`, `imports`, `rigid_vars`) stay at +the call sites because they depend on call context. + +**Tech Stack:** `crates/ailang-check` (lib.rs, mono.rs, tests/), +`indexmap`, `BTreeMap` / `BTreeSet`, the existing `Workspace` / +`Module` / `Def` types from `ailang-core`. + +**Files this plan creates or modifies:** + +- Create: `crates/ailang-check/tests/env_construction_pin.rs` — + drift-shape integration test that asserts `build_check_env(&ws)` + agrees with the pre-refactor inline seeding on every + workspace-flat field. +- Modify: `crates/ailang-check/src/lib.rs` — add + `pub fn build_check_env(ws: &Workspace) -> Env` (workspace-flat + source-of-truth); refactor `check_in_workspace` (lines 1088-1202) + to call it and drop the duplicated inline seeding while keeping + per-module overlay logic. +- Modify: `crates/ailang-check/src/mono.rs` — reduce + `build_workspace_env` (lines 367-446) to a one-line wrapper around + `build_check_env`; delete the drift-risk comment (lines 368-370). +- Modify: `docs/JOURNAL.md` — append milestone-close entry. + +--- + +## Task 1: RED — drift-shape pin test + +The pin test is the tripwire that protects the unify against future +drift. Per the plan-skill TDD discipline (and `ailang-implementer`'s +independent TDD layer), it lands RED first: it fails to compile +because `ailang_check::build_check_env` does not yet exist. + +**Files:** +- Create: `crates/ailang-check/tests/env_construction_pin.rs` + +- [ ] **Step 1: Write the integration test** + +Write the file `crates/ailang-check/tests/env_construction_pin.rs`: + +```rust +//! Drift-shape pin for env construction (env-construction unify +//! milestone). Asserts that `build_check_env(&ws)` agrees with an +//! in-test reproduction of the pre-refactor inline seeding on every +//! workspace-flat field. Future-you adds a new workspace-flat env +//! field without updating `build_check_env` -> this test fails before +//! the bug ships. +//! +//! Fixture: a two-module workspace covering one `Def::Class`, one +//! `Def::Instance`, one user ADT, and one cross-module import (the +//! same shape that `test_mono_imports_*` use). + +use ailang_check::{build_check_env, build_module_globals, build_module_types, Env}; +use ailang_core::ast::Def; +use ailang_core::load_workspace; +use std::collections::BTreeMap; +use std::path::Path; + +fn examples_dir() -> std::path::PathBuf { + let manifest = env!("CARGO_MANIFEST_DIR"); + Path::new(manifest).parent().unwrap().parent().unwrap().join("examples") +} + +/// In-test reproduction of the pre-refactor `check_in_workspace` +/// workspace-flat seeding (lib.rs as of commit a9c685d). Kept as a +/// frozen reference: any future divergence between this body and +/// `build_check_env` makes the assertion below fail. +fn build_env_inline_pre_refactor(ws: &ailang_core::workspace::Workspace) -> Env { + let mut env = Env::default(); + ailang_check::builtins::install(&mut env); + + // env.types + env.ctor_index — workspace-flat from every Def::Type. + for m in ws.modules.values() { + for d in &m.defs { + if let Def::Type(td) = d { + for c in &td.ctors { + env.ctor_index.insert( + c.name.clone(), + ailang_check::CtorRef { type_name: td.name.clone() }, + ); + } + env.types.insert(td.name.clone(), td.clone()); + } + } + } + + // env.module_globals — per-module .fns projection. + let mg = build_module_globals(ws).expect("build_module_globals"); + env.module_globals = mg + .iter() + .map(|(k, v)| (k.clone(), v.fns.clone())) + .collect(); + + // env.module_types — workspace-wide. + env.module_types = build_module_types(ws); + + // env.module_imports — per-module alias map. + for m in ws.modules.values() { + let mut import_map: BTreeMap = BTreeMap::new(); + for imp in &m.imports { + let key = imp.alias.clone().unwrap_or_else(|| imp.module.clone()); + import_map.insert(key, imp.module.clone()); + } + env.module_imports.insert(m.name.clone(), import_map); + } + + // env.class_methods — workspace-merged from each module's + // ModuleGlobals.class_methods. + for g in mg.values() { + for (n, e) in &g.class_methods { + env.class_methods.insert(n.clone(), e.clone()); + } + } + + // env.class_superclasses — walk every module's Def::Class. + for m in ws.modules.values() { + for d in &m.defs { + if let Def::Class(cd) = d { + if let Some(sc) = &cd.superclass { + env.class_superclasses.insert(cd.name.clone(), sc.class.clone()); + } + } + } + } + + // env.workspace_registry — clone of ws.registry. + env.workspace_registry = ws.registry.clone(); + + env +} + +#[test] +fn build_check_env_matches_inline_seeding() { + let entry = examples_dir().join("test_mono_imports_main.ail.json"); + let ws = load_workspace(&entry).expect("load test_mono_imports_main"); + + let env_helper = build_check_env(&ws); + let env_inline = build_env_inline_pre_refactor(&ws); + + // env.types — IndexMap: compare key-set + name. + assert_eq!( + env_helper.types.keys().collect::>(), + env_inline.types.keys().collect::>(), + "env.types key-set differs" + ); + for k in env_helper.types.keys() { + assert_eq!( + env_helper.types[k].name, + env_inline.types[k].name, + "env.types[{k}] type-def name differs" + ); + } + + // env.ctor_index — IndexMap: compare key-set + + // type_name. + assert_eq!( + env_helper.ctor_index.keys().collect::>(), + env_inline.ctor_index.keys().collect::>(), + "env.ctor_index key-set differs" + ); + for k in env_helper.ctor_index.keys() { + assert_eq!( + env_helper.ctor_index[k].type_name, + env_inline.ctor_index[k].type_name, + "env.ctor_index[{k}].type_name differs" + ); + } + + // env.module_globals — BTreeMap>. + assert_eq!( + env_helper.module_globals.keys().collect::>(), + env_inline.module_globals.keys().collect::>(), + "env.module_globals key-set differs" + ); + for k in env_helper.module_globals.keys() { + assert_eq!( + env_helper.module_globals[k].keys().collect::>(), + env_inline.module_globals[k].keys().collect::>(), + "env.module_globals[{k}] inner key-set differs" + ); + } + + // env.module_types — same shape as module_globals. + assert_eq!( + env_helper.module_types.keys().collect::>(), + env_inline.module_types.keys().collect::>(), + "env.module_types key-set differs" + ); + for k in env_helper.module_types.keys() { + assert_eq!( + env_helper.module_types[k].keys().collect::>(), + env_inline.module_types[k].keys().collect::>(), + "env.module_types[{k}] inner key-set differs" + ); + } + + // env.module_imports — BTreeMap>: + // direct equality. + assert_eq!( + env_helper.module_imports, env_inline.module_imports, + "env.module_imports differs" + ); + + // env.class_methods — BTreeMap: + // compare key-set (entry shape has no PartialEq). + assert_eq!( + env_helper.class_methods.keys().collect::>(), + env_inline.class_methods.keys().collect::>(), + "env.class_methods key-set differs" + ); + + // env.class_superclasses — direct equality. + assert_eq!( + env_helper.class_superclasses, env_inline.class_superclasses, + "env.class_superclasses differs" + ); + + // env.effect_ops — IndexMap: compare key-set. + assert_eq!( + env_helper.effect_ops.keys().collect::>(), + env_inline.effect_ops.keys().collect::>(), + "env.effect_ops key-set differs" + ); + + // env.workspace_registry — entries key-set must match. + assert_eq!( + env_helper.workspace_registry.entries.keys().collect::>(), + env_inline.workspace_registry.entries.keys().collect::>(), + "env.workspace_registry.entries key-set differs" + ); + + // Per-call overlay fields must NOT be seeded by the helper. + assert_eq!(env_helper.current_module, "", "current_module is overlay"); + assert!(env_helper.globals.is_empty(), "globals is overlay"); + assert!(env_helper.imports.is_empty(), "imports is overlay"); + assert!(env_helper.rigid_vars.is_empty(), "rigid_vars is overlay"); +} +``` + +- [ ] **Step 2: Run test to verify it fails (compile error, RED)** + +Run: `cargo test --workspace -p ailang-check --test env_construction_pin 2>&1 | head -40` +Expected: FAIL with `error[E0432]: unresolved import \`ailang_check::build_check_env\`` (or equivalent — `build_check_env` does not yet exist as a public symbol). + +- [ ] **Step 3: Commit RED** + +```bash +git add crates/ailang-check/tests/env_construction_pin.rs +git commit -m "test: red for env-construction drift-shape pin" +``` + +--- + +## Task 2: GREEN — add `build_check_env` + reduce `mono::build_workspace_env` to wrapper + +Introduces the workspace-flat source-of-truth. After this task, the +pin test from Task 1 goes green; the mono pass uses the helper; the +drift-risk comment is gone. `check_in_workspace` is *not yet* +refactored — its inline seeding lives alongside the helper for one +commit (intentional intermediate state — both produce equivalent +envs, which is exactly the property the pin test verifies). + +**Files:** +- Modify: `crates/ailang-check/src/lib.rs` — add + `pub fn build_check_env(ws: &Workspace) -> Env` near + `build_module_globals` / `build_module_types` (around line 985, + before `check_in_workspace`). +- Modify: `crates/ailang-check/src/mono.rs:367-446` — replace body + of `build_workspace_env`. + +- [ ] **Step 1a: Elevate `build_module_types` from `pub(crate)` to `pub`** + +In `crates/ailang-check/src/lib.rs` line 885, change +`pub(crate) fn build_module_types(` to `pub fn build_module_types(`. +Reason: the new `crates/ailang-check/tests/env_construction_pin.rs` +(another crate) needs to call this helper to reproduce the +pre-refactor seeding. `build_module_globals` is already `pub`; this +elevation makes the two sibling helpers symmetric. + +- [ ] **Step 1b: Add `build_check_env` to lib.rs** + +Insert immediately before `fn check_in_workspace` (around line +1088). Place in the same `mod` scope as the existing +`build_module_globals` and `build_module_types` so it can call them +without re-export. + +```rust +/// Build the workspace-flat `Env` shared by `check_in_workspace` +/// and `mono::build_workspace_env`. Populates every field whose +/// contents are derivable from `Workspace` alone — no +/// `current_module` overlay applied. Both call sites are thin +/// consumers: each clones this env and applies the per-call +/// overlay (`current_module`, `globals`, `imports`, `rigid_vars`) +/// at the call site. +/// +/// Pre-condition: typecheck has succeeded for the workspace +/// (otherwise `build_module_globals` panics — same caller-contract +/// as the pre-refactor `mono::build_workspace_env`). +pub fn build_check_env(ws: &Workspace) -> Env { + let mut env = Env::default(); + builtins::install(&mut env); + + // env.types + env.ctor_index — workspace-flat from every + // module's Def::Type. Safe to flatten because `check_workspace` + // has already accepted the workspace, so duplicate names + // cannot occur. Mirrors the per-module loop in the pre-refactor + // `check_in_workspace` (which fail-fast aborted on duplicates, + // unreachable here). + for m in ws.modules.values() { + for d in &m.defs { + if let Def::Type(td) = d { + for c in &td.ctors { + env.ctor_index.insert( + c.name.clone(), + CtorRef { type_name: td.name.clone() }, + ); + } + env.types.insert(td.name.clone(), td.clone()); + } + } + } + + // env.module_globals + env.class_methods — built from + // `build_module_globals(ws)`. The .fns projection feeds + // `module_globals`; the .class_methods entries are merged + // workspace-wide into `class_methods` (uniqueness enforced + // earlier by `workspace::build_registry`'s + // method-name-collision check). + let mg = build_module_globals(ws) + .expect("build_module_globals: pre-condition (typecheck succeeded) violated"); + env.module_globals = mg + .iter() + .map(|(k, v)| (k.clone(), v.fns.clone())) + .collect(); + for g in mg.values() { + for (n, e) in &g.class_methods { + env.class_methods.insert(n.clone(), e.clone()); + } + } + + // env.module_types — per-module ADT index. + env.module_types = build_module_types(ws); + + // env.module_imports — per-module alias-or-name -> module-name. + // Per-module because aliases collide across modules; sibling + // shape to `module_globals`. + for m in ws.modules.values() { + let mut import_map: BTreeMap = BTreeMap::new(); + for imp in &m.imports { + let key = imp.alias.clone().unwrap_or_else(|| imp.module.clone()); + import_map.insert(key, imp.module.clone()); + } + env.module_imports.insert(m.name.clone(), import_map); + } + + // env.class_superclasses — walk every module's Def::Class. + for m in ws.modules.values() { + for d in &m.defs { + if let Def::Class(cd) = d { + if let Some(sc) = &cd.superclass { + env.class_superclasses.insert(cd.name.clone(), sc.class.clone()); + } + } + } + } + + // env.workspace_registry — clone of ws.registry. Used by + // `check_fn`'s concrete-residual no-instance check. + env.workspace_registry = ws.registry.clone(); + + env +} +``` + +- [ ] **Step 2: Verify lib.rs builds** + +Run: `cargo build --workspace -p ailang-check 2>&1 | tail -10` +Expected: builds clean (no warnings about unused fn — +`build_check_env` is `pub`; `build_module_types` is now `pub` and +already had a workspace consumer in `mono::build_workspace_env`). + +- [ ] **Step 3: Replace `mono::build_workspace_env` body** + +In `crates/ailang-check/src/mono.rs`, replace lines 367-446 with: + +```rust +/// Iter 22b.3 / 2026-05-10 unify: thin wrapper over +/// [`crate::build_check_env`]. The mono pass needs the same +/// workspace-flat `Env` shape as `check_in_workspace`, so both +/// share one source of truth. Per-fn entry points +/// (`collect_mono_targets`, `collect_residuals_ordered`) clone the +/// env and apply per-fn overlay (current_module, globals from +/// module_globals, imports from module_imports, rigid_vars). +pub fn build_workspace_env(ws: &Workspace) -> crate::Env { + crate::build_check_env(ws) +} +``` + +The drift-risk comment on lines 368-370 is removed by this +replacement (the new body has no such comment). + +- [ ] **Step 4: Verify mono.rs builds and the pin test passes** + +Run: `cargo build --workspace 2>&1 | tail -10` +Expected: builds clean. Any unused-import warning (e.g. `Def`, +`BTreeMap` if no longer referenced in mono.rs) is fixed in this +step by removing the import. + +Run: `cargo test --workspace -p ailang-check --test env_construction_pin 2>&1 | tail -15` +Expected: PASS — `build_check_env_matches_inline_seeding ... ok`. + +- [ ] **Step 5: Verify the three RED-test regression suite stays green** + +Run: `cargo test --workspace --test typeclass_22c --test mono_recursive_fn --test mono_xmod_qualified_ref 2>&1 | tail -20` +Expected: PASS for all three (they exercise the mono pass through +`ail build`, which now goes via the helper). + +- [ ] **Step 6: Commit GREEN** + +```bash +git add crates/ailang-check/src/lib.rs crates/ailang-check/src/mono.rs +git commit -m "iter env-unify.1: extract build_check_env, mono uses it" +``` + +--- + +## Task 3: Refactor `check_in_workspace` to consume `build_check_env` + +Drops the duplicated inline seeding from `check_in_workspace`. After +this task, every workspace-flat field has exactly one construction +site (`build_check_env`); `check_in_workspace` only does per-module +overlay (`current_module`, `globals`, `imports`). + +**Files:** +- Modify: `crates/ailang-check/src/lib.rs:1088-1202` — + `check_in_workspace` body. + +- [ ] **Step 1: Replace `check_in_workspace` body** + +Replace the body of `check_in_workspace` (lines 1088-1202 — keep +the doc comment and signature, replace the body) with: + +```rust +fn check_in_workspace( + m: &Module, + ws: &Workspace, + module_globals: &BTreeMap, +) -> Vec { + let mut env = build_check_env(ws); + let mut errors: Vec = Vec::new(); + + // Type-def setup phase — the workspace-flat `env.types` / + // `env.ctor_index` populated by `build_check_env` cover the + // resolution path. Duplicate-type / duplicate-ctor diagnostics + // for THIS module are still per-module and fail-fast: a + // duplicate corrupts later body checks for this module, so we + // abort this module on the first such error and let the outer + // loop continue with siblings. + let mut seen_types: BTreeSet = BTreeSet::new(); + let mut seen_ctors: BTreeMap = BTreeMap::new(); + for def in &m.defs { + if let Def::Type(td) = def { + if !seen_types.insert(td.name.clone()) { + errors.push(CheckError::Def( + td.name.clone(), + Box::new(CheckError::DuplicateType(td.name.clone())), + )); + return errors; + } + for c in &td.ctors { + if let Some(prev) = seen_ctors.get(&c.name) { + errors.push(CheckError::Def( + td.name.clone(), + Box::new(CheckError::DuplicateCtor { + ctor: c.name.clone(), + a: prev.clone(), + b: td.name.clone(), + }), + )); + return errors; + } + seen_ctors.insert(c.name.clone(), td.name.clone()); + } + } + } + + // Per-module overlay: seed `globals` from this module's fns, + // build `imports` from this module's import list, set + // `current_module`. These three fields are call-context- + // dependent — they belong to the caller, not to + // `build_check_env`. + if let Some(g) = module_globals.get(&m.name) { + for (n, t) in &g.fns { + env.globals.insert(n.clone(), t.clone()); + } + } + let mut import_map: BTreeMap = BTreeMap::new(); + for imp in &m.imports { + let key = imp.alias.clone().unwrap_or_else(|| imp.module.clone()); + import_map.insert(key, imp.module.clone()); + } + env.imports = import_map; + env.current_module = m.name.clone(); + + for def in &m.defs { + if let Err(e) = check_def(def, &env) { + errors.push(CheckError::Def(def.name().to_string(), Box::new(e))); + } + } + errors +} +``` + +Note on duplicate-type/ctor handling: the pre-refactor body +mutated `env.types` / `env.ctor_index` per-def to detect duplicates +in-band. With the workspace-flat env already populated by +`build_check_env`, we use a per-module `seen_types` / `seen_ctors` +overlay to preserve the same fail-fast diagnostics without +re-mutating the shared env. + +- [ ] **Step 2: Verify build is clean** + +Run: `cargo build --workspace 2>&1 | tail -10` +Expected: builds clean. Any now-unused import in lib.rs gets removed. + +- [ ] **Step 3: Verify the drift-shape pin test still passes** + +Run: `cargo test --workspace -p ailang-check --test env_construction_pin 2>&1 | tail -10` +Expected: PASS. + +- [ ] **Step 4: Verify the three RED-test regression suite still green** + +Run: `cargo test --workspace --test typeclass_22c --test mono_recursive_fn --test mono_xmod_qualified_ref 2>&1 | tail -20` +Expected: PASS for all three. + +- [ ] **Step 5: Verify the workspace check tests still green** + +Run: `cargo test --workspace -p ailang-check 2>&1 | tail -15` +Expected: PASS — including +`happy_path_resolves_qualified_import`, +`unknown_import_is_reported`, and the rest of `tests/workspace.rs`. + +- [ ] **Step 6: Commit** + +```bash +git add crates/ailang-check/src/lib.rs +git commit -m "iter env-unify.2: check_in_workspace consumes build_check_env" +``` + +--- + +## Task 4: Verify workspace + bench gates + JOURNAL entry + +Final verification across the full test suite and the three bench +gates, then close the milestone with a JOURNAL entry. + +**Files:** +- Modify: `docs/JOURNAL.md` — append milestone-close entry. + +- [ ] **Step 1: Run full workspace test suite** + +Run: `cargo test --workspace 2>&1 | tail -20` +Expected: PASS — every test green (~344 tests pre-milestone; the +new pin test brings it to ~345). + +- [ ] **Step 2: Run bench/check.py** + +Run: `python3 bench/check.py 2>&1 | tail -10` +Expected: exit 0; report ends with "0 regressed" (performance +must be neutral — the helper is logically the same work as the +pre-refactor inline). + +- [ ] **Step 3: Run bench/compile_check.py** + +Run: `python3 bench/compile_check.py 2>&1 | tail -10` +Expected: exit 0; report ends with "0 regressed". + +- [ ] **Step 4: Run bench/cross_lang.py** + +Run: `python3 bench/cross_lang.py 2>&1 | tail -10` +Expected: exit 0; report ends with "0 regressed". + +- [ ] **Step 5: Append JOURNAL entry** + +Append to `docs/JOURNAL.md`: + +```markdown +## 2026-05-10 — Iteration env-construction unify + +Single-iteration milestone retiring the structural drift between +`check_in_workspace` and `mono::build_workspace_env`. Three +consecutive bug fixes (`5c5180f` env.types/ctor_index, `13b36cc` +env.globals, `a9c685d` env.imports) were patches at three +different fields of the same drift class. The unify replaces both +construction paths with a single source-of-truth helper +`build_check_env(ws) -> Env` covering every workspace-flat field; +per-module overlay (`current_module`, `globals`, `imports`, +`rigid_vars`) stays at the call sites where it semantically +belongs. + +After this iteration, when `synth` learns to read a new `Env` +field, exactly one construction site needs the seeding edit. A +new `crates/ailang-check/tests/env_construction_pin.rs` integration +test serves as the tripwire: any future divergence between the +helper and a frozen pre-refactor reproduction of the inline +seeding fails the test before the bug ships. + +Tasks: +- env-unify.1: extract `build_check_env`; `mono::build_workspace_env` + becomes a one-line wrapper; drift-risk comment deleted. +- env-unify.2: `check_in_workspace` consumes `build_check_env`; + per-module overlay logic retained; in-band duplicate-type / + duplicate-ctor diagnostics moved to a per-module overlay set + (preserves pre-refactor fail-fast behaviour without re-mutating + the shared env). + +Spec: `docs/specs/2026-05-10-env-construction-unify.md`. Plan: +`docs/plans/2026-05-10-env-construction-unify.md`. Tests: pin test +green, the three RED tests +(`typeclass_22c`, `mono_recursive_fn`, `mono_xmod_qualified_ref`) +remain green, full workspace test suite green, bench gates 0/0/0. + +Known debt: none. Out-of-scope items from the spec +(pipeline-topology changes, per-module overlay refactor, +primitive-name-set consolidation, new env fields) remain queued +for separate milestones. +``` + +- [ ] **Step 6: Commit JOURNAL** + +```bash +git add docs/JOURNAL.md +git commit -m "iter env-unify: journal entry, milestone close" +```