diff --git a/docs/JOURNAL.md b/docs/JOURNAL.md index 10b2943..a1515d3 100644 --- a/docs/JOURNAL.md +++ b/docs/JOURNAL.md @@ -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` 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` — 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` 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/.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.