iter 22b.2: typecheck arms — 8 diagnostics shipped + skill-system post-mortem

2026-05-09 19:21:38 +02:00
parent 73965a6cbd
commit b1ae865e9d
1 changed files with 190 additions and 0 deletions
@@ -11309,3 +11309,193 @@ relevant skill.
 - Per-skill TDD pressure-tests (RED + 3 GREEN/REFACTOR runs each)
  on the build-out iteration. Deferred to a follow-up audit if the
  first end-to-end exercise reveals gaps.
 ## 2026-05-09 — Iteration 22b.2: typecheck arms
 First milestone-iteration routed entirely through the new skill
 pipeline (`brainstorm` was retrospective for milestone 22 in iter
 22b.2 prelude; `plan` produced `docs/plans/2026-05-09-22b2-typecheck-arms.md`;
 `implement` ran the 10 tasks plus E2E; `audit` will close the
 milestone separately).
 **What shipped (8 new diagnostics + schema + infra):**
 - Schema: `Constraint { class, type_ }` struct; `Type::Forall` gains
  `constraints: Vec<Constraint>` gated by
  `#[serde(default, skip_serializing_if = "Vec::is_empty")]`. Pre-22b.2
  fixtures hash bit-identical (regression test in `hash.rs`).
 - Class-schema (3, in `validate_classdefs` running before
  `build_registry`): `kind-mismatch` (HKT use forbidden, Decision 11
  axis 5), `invalid-superclass-param` (superclass `type` must equal
  class `param`, axis 1), `constraint-references-unbound-type-var`
  (class-method constraint vars must be bound).
 - Workspace coherence (3, in `build_registry`):
  `overriding-non-existent-method` (instance bodies for undeclared
  methods), `method-name-collision` (single code with `kind: "class-class"`
  vs `"class-fn"` ctx field; backed by structural `enum Origin` not
  string-prefix matching; fn-fn delegated to existing `DuplicateDef`),
  `missing-superclass-instance` (post-loop superclass-chain walk with
  `BTreeSet<&str>` cycle-termination guard).
 - Per-FnDef typecheck (2, in `check_fn`): `missing-constraint`
  (residual at class-method call site doesn't match expanded
  declared constraints, where expansion walks the superclass chain
  one step per Decision 11), `no-instance` (concrete-type residual
  not in workspace registry; reuses `canonical::type_hash`).
 - Infra: `ModuleGlobals { fns, class_methods }` two-channel split
  (class methods get a separate map preserving definition order via
  `IndexMap`); `ClassMethodEntry { class_name, class_param, method_ty,
  defining_module }` accessor; `Env.class_methods`,
  `Env.class_superclasses` (`BTreeMap<String, String>` — absence
  means no superclass), `Env.workspace_registry` threaded through
  `check_in_workspace`.
 **Tests (cross-cutting + e2e):** 12 tests in
 `crates/ail/tests/typeclass_22b2.rs` (4 task-level + 4 e2e
 cross-module + 4 superclass-walk asymmetry). Workspace-load tests
 under each diagnostic in `crates/ailang-core/src/workspace.rs`.
 11 fixtures under `examples/test_22b2_*.ail.json`.
 **Per-task subjects (mirrors commit messages):**
 - iter 22b.2.1 — Constraint struct + Forall.constraints
 - iter 22b.2.2 — kind-mismatch
 - iter 22b.2.3 — invalid-superclass-param
 - iter 22b.2.4 — constraint-references-unbound-type-var
 - iter 22b.2.5 — overriding-non-existent-method
 - iter 22b.2.6 — method-name-collision
 - iter 22b.2.7 — missing-superclass-instance
 - iter 22b.2.8 — register class methods in module globals
 - iter 22b.2.9 — missing-constraint per-fn
 - iter 22b.2.10 — no-instance per-fn
 - iter 22b.2.e2e — cross-module class resolution + multi-fn aggregation
 19 commits total on branch `iter-22b2-typecheck-arms` (10 task
 commits + 8 round-2 fixes from the spec/quality review loops + 1
 e2e commit). Two-stage review surfaced real issues the implementer
 or task carrier alone wouldn't have caught — see "Skill-system
 post-mortem" entry below.
 **Known debt deliberately not touched:**
 - The `constraint-references-unbound-type-var` walker is shallow:
  it only inspects constraints whose `type_` is a bare `Type::Var`.
  `(Bar, Maybe z)` with `z` unbound would slip past. Enough for
  schema-level coherence per Decision 11's stated convention;
  recursive walk is a follow-up if a real fixture needs it.
 - Superclass-cycle DETECTION as a first-class diagnostic is
  queued. The chain walks now terminate via `BTreeSet<&str>`
  visited-set in `missing-superclass-instance` and via single-step
  expansion in `expand_declared_constraints`; surfacing cycles as
  their own error is a future arm.
 - `Env.workspace_registry` is `Default::default()` in the
  standalone `check_module` path. Acceptable because that path is
  used only for diagnostics that don't need cross-module instance
  lookup; if a future feature needs registry-backed checks in
  module-only mode, the field must be made `Option<Registry>` or
  fed from a builder.
 **22b.3 next:** monomorphisation pass — synthesise FnDefs from
 `(method, type-hash)` pairs, rewrite calls, with a synthetic
 class+instance fixture for end-to-end mono validation before the
 Prelude lands in 22b.4.
 ## 2026-05-09 — Skill-system post-mortem (first end-to-end run)
 Iteration 22b.2 was the first milestone-iteration routed entirely
 through the `brainstorm → plan → implement` pipeline (the build-out
 iteration 22c.live shipped the system itself but didn't exercise
 it on a real ten-task milestone). Observations:
 **What the skill system caught that an unstructured run wouldn't:**
 - **Spec drift in carrier construction.** Task 5 spec reviewer
  flagged a placement issue (`OverridingNonExistentMethod` after
  `MissingMethod` vs after `UnboundConstraintTypeVar`) that came
  from the *orchestrator's* carrier text diverging from the literal
  plan. The two-stage review made the divergence visible at the
  point it landed, not three iters later. Cost: one fix-round
  commit (`1f6d232`). Without spec review I'd have shipped the
  drift.
 - **Quality-stage caught real bugs spec-stage couldn't see.**
  Task 6's first round used `prior.starts_with("class ")` to
  decide the class-class-vs-class-fn discriminator — semantically
  correct against the task text (so spec-compliant) but coupled
  the discriminator to an ad-hoc display string. Quality reviewer
  surfaced both that and the fn-fn-misreports-as-class-fn
  correctness bug. The structural `enum Origin` rework
  (`b252f83`) was ten lines of code but the only way the bug got
  caught was by separating "did you build the right thing" from
  "did you build it well".
 - **Test-coverage gaps.** Task 9 shipped without a superclass-walk
  exercise (the `expand_declared_constraints` code path was
  unverified); quality stage flagged it as Important. Task 10
  shipped without a positive `instance-present` test (registry
  threading bugs would have slipped past); also flagged. In both
  cases the per-task plan was complete but the implementer didn't
  add cross-cutting coverage at task level. Quality review caught
  the gap right when it would otherwise have rotted.
 - **Termination guards.** Task 7's chain-walk had no cycle bound;
  quality stage surfaced the hang risk. Implementer's first round
  was a clean implementation of the plan, but the plan didn't
  ask for a guard and a malicious workspace would have hung
  `build_registry` indefinitely. Quality reviewer caught the
  defensive-validation question and the fix landed in round 2.
 **What the skill system cost:**
 - 8 round-2 fix commits across the iter (out of 19 total). Every
  one was a justified correction. Review-loop overhead is real but
  the marginal cost per finding is much lower than the downstream
  cost of catching those issues weeks later. Per-iter latency: ~50%
  more wall-clock vs a single-pass implementation, but that wall-
  clock is delegated subagent time, not orchestrator context.
 - **Carrier-text duplication.** Each implementer + spec reviewer
  + quality reviewer dispatch carries the full task text and
  scene-set. Token-cost-wise, this is the single biggest line item.
  An optimisation to consider: have the spec reviewer accept a
  "task_text_ref: docs/plans/<X>.md#task-N" and load the file
  itself. But this would break the "agents do not open
  docs/plans/" convention from `skills/README.md`. Trade-off worth
  revisiting if iter cost balloons.
 **What surprised me:**
 - **Implementer self-corrections were strong.** Three implementer
  rounds returned `DONE_WITH_CONCERNS` flagging an unrequested
  edit they made because the task plan was underspecified (e.g.
  Task 2's `round_trip.rs` skip-list extension; Task 6's
  `"kind": "string"` → `"str"` fixture fix). These were always
  correct. The DONE_WITH_CONCERNS protocol works — the
  implementer doesn't push through silently, but doesn't demand
  permission either when the call is clear.
 - **Spec-stage failed cheaply.** When non_compliant fired, the
  fix-rounds were always single-file edits. The two-stage split
  prevented quality-review from spending time on diffs that would
  later be moved or reverted.
 - **Quality stage's "trust the implementer to choose the fix"
  discipline held.** Reviewers consistently described issues
  rather than prescribing fixes. The implementer made structurally
  sound rework calls (e.g. `enum Origin` instead of patching the
  prefix-string check in place). The reviewer instructions in
  `ailang-quality-reviewer.md` were doing real work.
 **Skill-system tweaks for the audit phase:**
 - The `ailang-spec-reviewer` carrier should default to "the
  literal plan task text" rather than orchestrator paraphrase.
  Task 5's drift came from my paraphrase adding "after X". The
  fix is for me as orchestrator to copy plan-task text verbatim
  into the carrier; not a SKILL.md change but a discipline note.
 - `ailang-tester` Step 3 was excellent for this iter: identified
  cross-module gaps + multi-fn aggregation that per-task tests
  missed, AND honestly reported when a candidate property wasn't
  E2E-testable rather than padding the suite. Worth preserving
  the "DONE with no new tests if coverage is exhaustive" carve-
  out in the agent file.
 **Net assessment:** the skill pipeline shipped 22b.2 with materially
 higher quality than a single-pass run would have, at proportional
 cost. First end-to-end exercise is a positive signal; the
 remaining open follow-ups from 2026-05-09 (skill bugs surfaced by
 real iter use) get queued as targeted SKILL.md tweaks rather than
 a wholesale revisit.