The diagnostics worklist, ranked, so it stops floating

docs/DIAGNOSTICS-AUDIT.md

@@ -0,0 +1,225 @@
+# Diagnostics audit — Flan against Elm
+
+Read-only audit, 2026-09-20. Feeds the fix pass that starts once the two live
+lanes merge. Untracked on purpose: this is a worklist, not a spec.
+
+## The contract being graded against
+
+Elm's, plus this repo's own two additions:
+
+- **(S) Show / locate** — the exact code, a caret, at the *most specific* span.
+- **(U) Understood vs conflicted** — say what the compiler took the code to
+  mean, and what that collides with.
+- **(F) Fix by name** — name the thing to write instead, not the category.
+- **(R) Register** — plain language. FIX.org's literary voice is banned here.
+
+Grades are per-dimension letters. A = Elm-class, B = good with one gap,
+C = states the fact and stops, D = misleading or actively unhelpful.
+
+## The machinery that already exists (lib/loc.ml)
+
+Worth stating up front, because most of the worklist below is *underuse*, not
+absence.
+
+- Spans, not points (`Loc.t` carries `eline`/`ecol`), and `squiggle` draws a
+  multi-column underline from them.
+- **Secondary notes with their own locations and severities** (`Loc.note`,
+  rendered by `report` as their own `file:line:col: info:` entry, marked `-`
+  rather than `^`, sorted into source order). This is Elm's "this here… but
+  that there" and it is fully built.
+- `kind` — a stable id per diagnostic (`"check/unknown-field"`), deliberately
+  never printed.
+- `expansion` — names the macro a form came out of, automatically, off the
+  location.
+- GNU `file:line:col:` first line, so `M-x compile` and `next-error` work.
+
+What it does **not** have: any notion of "expected because of *that*
+signature" beyond a free-form note; anything a runtime trap can use (the
+runtime has only preformatted loc strings, no access to source text).
+
+---
+
+## The worst 20, ranked by (badness × how often a user hits it)
+
+| # | Message | Site | Trigger (one-liner) | S | U | F | R | Fix direction |
+|---|---------|------|---------------------|---|---|---|---|---------------|
+| 1 | `unknown name p.x` | check.ml:2732, 4046 | `(defstruct P [x i32])` then `p.x` anywhere | B | D | D | C | The dot-infix habit from C/Go/Odin. The checker can see the head `p` is bound and is a struct with field `x` — split on `.`, and say "field access is written `(.x p)`". The repo's own comment in examples/core-2d-camera.flan:20 writes `camera.rotation` in prose while the code is `(.rotation camera)`, so the author trips it too. |
+| 2 | `dyn +: int and text, and it takes two numbers — (+ 3 "hi")` | flan_dyn.c:724 (`trap2`), :734 (`trap1`), :746 (`trap_range`), :814 (`trap_oom`) | `(defn add [x y] dyn (+ x y))` + `(add 3 "hi")`, then `flan run` | D | B | D | B | **No location at all** — no file, no line, nothing. In a dynamic-first language these *are* the type errors. flan_rt.c's bounds and arith traps already take an emitter-threaded `(loc, loclen)` pair, so the ABI precedent exists; the dyn entry points simply were never given one. Highest-leverage runtime fix. |
+| 3 | `expected i32, found string` | check.ml:1866 (`expect`), 6292 (call args) | `(defn add [a i32 b i32] i32 …)` + `(add 1 "two")` | B | C | D | A | The most-hit message in the compiler. Caret is right; it never says *which* argument of *which* function, and never points at the parameter that wanted it. Elm's whole hallmark. `Loc.note` on the `Ast.field`'s `floc` is a five-line change: "the 2nd argument of `add`" + "`b` is declared `i32` here". |
+| 4 | `unknown function prinltn` / `unknown name n` | check.ml:6259, 2732 | `(prinltn "hi")` | B | C | D | B | No did-you-mean, although `near_miss` (check.ml:670) is written, tested and wired — to **types only**. Point it at `env.fns`, `env.globals` and the local scope. Cheapest structural win on the list. |
+| 5 | `expected bool, found i32` | check.ml:1866 via `check_truthy` (3596) | `(let [x 1] (if x …))` | A | C | D | A | Caret is exactly right (the `check_truthy` loc work paid off). But the message never states Flan's truthiness rule — bool or dyn, nothing else — and never names the fix (`(not= x 0)`). Special-case the condition position. |
+| 6 | `binding 5 has no value — let takes name/value pairs` | parse.ml:670 | `(let [x i32 5] …)` | C | D | D | B | A type annotation in `let` is the single most natural thing for someone arriving from a typed language, and `let` has none. The message reads as if the user miscounted. Detect "middle form names a type" and say so: "`let` bindings take no type annotation — write `[x 5]`". |
+| 7 | `get takes 2 arguments, given 1` against the **user's own** `(defn get [p P] …)` | check.ml:5296 (builtin dispatch) + 6233 | `(defn get [p P] i32 …)` + `(get p)` | D | D | D | B | A user defn whose name collides with a builtin is silently shadowed, and then the arity refusal is measured against the *builtin*, pointing at the user's call. Either refuse the shadowing definition at its `dloc` with a note, or report the arity against the definition the user can see. |
+| 8 | `unhandled Boom` | flan_rt.c:646 | `(defstruct Boom [why i32])` + `(error (Boom {.why 7}))`, `flan run` | D | C | D | B | Three words. No location (not even the `error` site, which the emitter knows), no field values, no list of the handlers that were in scope. The condition system is a headline feature and this is its failure mode. |
+| 9 | `the collection nosuch: is a directory named nosuch somewhere above /…/. , and there is none` | load.ml:120 | `(import zz "nosuch:thing")` | B | C | C | D | Reads as an assertion immediately contradicted. Also emits a bare `/.` on the path. Rewrite as a plain statement of the search ("no directory named `nosuch` between here and the root") and list what collections *were* found. |
+| 10 | `and`'s last operand gets the previous operand's caret | parse.ml `shortcircuit`, via check.ml:3632 | `(println (and true true (vec-new i32)))` | D | B | C | A | Already diagnosed in FIX.org:1036 with three rejected fixes; the accepted one — `check_if` preferring the arm that is not a compiler temp when choosing which to blame — is a check.ml change nobody owned. This pass owns check.ml. |
+| 11 | `unterminated string` | reader.ml:92 | `(println "oops` | C | C | C | A | Caret is one column on the opening quote, and there is **no** "the input ends here" note — unlike `reader/unclosed` (241) and `reader/mismatched-closer` (250), which both have one. Copy their shape. |
+| 12 | `unknown type i — did you mean i8? A parameter with no type is dyn, so this would otherwise be read as a second parameter called i` | check.ml:850 | `(defn idx [v i] dyn …)` | A | B | C | C | Locates and explains well, but the did-you-mean fires on a *lowercase* name the user plainly meant as a parameter, so the suggestion is a false accusation. Suppress `near_miss` when the name is lowercase and in a parameter vector; lead with the dyn-parameter reading instead. |
+| 13 | `expected a type, found 1. This is the return type, which every defn states -- a function that returns nothing writes ()` | parse.ml:1156 | `(defn f [x i32] (+ x 1))` | C | B | B | C | Says the fix, which is good. Two warts: the caret lands on the `1` deep inside the body rather than on the position where the return type belongs; and the literal `--` where the house uses `—` everywhere else. |
+| 14 | `f64 is not a struct, so it has no fields` | check.ml:4028 | `(match s (Circle c) (.r c))` — single-field case binds the payload directly | B | C | D | A | The user wrote what looks like a destructuring pattern and got a type fact. Say what the pattern bound (`c` is the payload, an `f64`) and that the field is already in hand. |
+| 15 | `% is a constant` | check.ml:4043 | `(defconst k 3)` + `(set k 4)` | A | C | D | A | Four words. Needs a `Loc.note` at the `defconst` and the named fix (`defvar`). `no_container_defconst` (7447) already shows how the house writes this well — imitate it. |
+| 16 | `% is a parameter, and parameters are not assignable places (spec-memory.md) — bind a local with let` | check.ml:4037 | `(defn f [x i32] i32 (set x 1) x)` | A | B | B | C | Names the fix. Register wart: a diagnostic should not cite a spec filename at the user; put the rule in words and drop `(spec-memory.md)`. Same for `(plan.org, Types)` at 4609/4614 and `(see plan.org)` at 423. |
+| 17 | `% is not implemented yet — milestone %d (see plan.org)` | check.ml:423 (`unimplemented`), used widely | `(Result i32 string)` in a type position | B | B | D | D | Sends the user to a planning document. Say what is missing in one clause and what to write in the meantime, or nothing. |
+| 18 | `% takes numbers, found string` | check.ml:4219 (`binary`) | `(+ "a" "b")` | C | B | C | A | Caret covers the whole form rather than the offending operand — the exact "whole form vs operand" regression class this repo already fixed once in `check_truthy`. The operand's `Tast.eloc` is right there. Also: no mention of `str-cat`/the concatenation route, which is what the user wanted. |
+| 19 | `expanding this declaration produced %d of them` / `expanding this expression produced %d forms, and an expression is one` | parse.ml:1541, 1569 | a `defmacro` returning two forms | B | C | D | C | "of them" has no antecedent. The `expansion` field on the diagnostic can name the macro automatically; use it, and suggest `do`. |
+| 20 | `this fn has %d parameters and %s was wanted here` / `nothing here says what this fn's parameters are` | check.ml:2801, 2809 | passing an `fn` literal where no signature is in view | B | B | C | A | Correct and readable; the arity one should note the parameter list it was measured against, which is the one thing the reader cannot see from the caret. |
+
+### Also seen, not ranked
+
+- `an index is an i32, and %s is wider — write (i32 …), because …` (4113) is
+  **fine** — S/U/F/R ≈ A/A/A/B, listed only so the fix pass does not touch it.
+- The `--warn-memory` / `check/no-gc` sites (8566, 8723) are warnings rather
+  than refusals and were not exercised; they build `Loc.diag` directly and
+  carry no notes.
+
+---
+
+## The house's best — imitate these, not just Elm
+
+These are internal precedent and they already satisfy the contract. The fix
+pass should copy **their shape**, so the corpus converges on one voice.
+
+**1. Unknown field, with the declaration shown.** check.ml:3706 / 2433 /
+3793, via `declared_note` (169). This is the model.
+
+```
+v6.flan:3:23: P has no field z
+  3 |   (let [p (P {.x 1 .z 9})] (println (.x p))))
+    |                       ^
+v6.flan:1:1: info: P is declared here, with x
+  1 | (defstruct P [x i32])
+    | ---------------------
+```
+
+Primary span on the exact offending key; secondary span on the declaration;
+the available names enumerated. All four dimensions, in five lines.
+
+**2. Non-exhaustive match.** check.ml:3983.
+
+```
+w4.flan:3:3: this match is not exhaustive — Shape.Tri has no arm. Add it, or a _ arm for the rest
+  3 |   (match s
+    |   ^^^^^^^^
+w4.flan:1:1: info: Shape is declared here, with Circle, Square, Tri
+```
+
+Names the missing case *and* both fixes, with the declaration alongside.
+Elm-class.
+
+**3. The defconst pair**, landed 2026-09-20. `const_defconst_init` (7527)
+and `no_container_defconst` (7447). No secondary span, but the prose does the
+whole contract: what was understood ("the constant `n` is computed"), what it
+conflicts with ("a defconst is what the linker writes into the image and has
+nowhere to run"), and two named ways out (`defvar`, or a folded literal). The
+best *prose-only* message in the tree.
+
+**4. The dyn view-lifetime refusal.** `view_not_permanent` (1570). Explains
+the rule, gives the one shape that does work (`defvar g …`), and enumerates
+what is refused. Borderline long — it is the closest thing in the tree to the
+banned essay register, and a fix pass should cut it by a third rather than
+lengthen anything toward it.
+
+**5. Reader's unclosed / mismatched-closer.** reader.ml:241, 250. The only
+two-span diagnostics outside check.ml, and both are right:
+
+```
+u4.flan:2:25: expected ')' to close '(', found ']'
+u4.flan:2:12: info: '(' is opened here
+```
+
+**6. `flan_arith_fail`'s overflow sentence** (flan_rt.c:911). A runtime trap
+that explains *why* one pair of operands overflows a division. Good register,
+good detail — it just lacks nothing except company.
+
+---
+
+## Structural gaps — things Elm does that no Flan message does
+
+**1. Two-span "this here… but that there" as a habit, not an exception.**
+*Not absent — underused.* `Loc.note` is fully built and rendered, and roughly
+eight of ~250 sites use it (`check/duplicate-field`, `check/unknown-field`,
+`check/defined-twice`, `check/duplicate-parameter`,
+`parse/enum-autoincrement-collision`, `reader/unclosed`,
+`reader/mismatched-closer`, `check/non-exhaustive-match`). **Feasible today,
+no machinery needed** — the gap is that `expect` (1866) and the call-argument
+path (6292) don't have the wanting-side location threaded to them. `Ast.field`
+already carries `floc`; the defn record is reachable from the call site. This
+is the single highest-value structural change and it is plumbing, not design.
+
+**2. Did-you-mean on anything but types.** `near_miss` (670) is a real
+one-edit matcher with a real candidate set, wired at exactly one call site
+(771). **Feasible today**: the candidate sets for functions, globals and
+locals are all in `ctx.env` / the scope stack at the raise points
+(check.ml:6259, 2732, 4046). Field names are already gathered by
+`declared_note`, so field-typo suggestions are nearly free too.
+
+**3. Error titles / anchors / doc links.** Elm prints `-- TYPE MISMATCH ---`
+and links a hint page. Flan has the ingredient — every diagnostic carries a
+stable `kind` — and loc.ml:109 says explicitly that it is "never printed as
+the reason". **Feasible today** at zero cost: print it as a trailing
+`[check/unknown-field]` or route it to a docs anchor. Needs a decision, not
+work. (Caveat: the first line is load-bearing for compilation-mode, so the id
+belongs at the end of the line or on an indented continuation.)
+
+**4. Any caret at all at runtime.** Runtime traps carry a preformatted
+`file:line:col` string and nothing else — flan_rt.c has no access to the
+source text, and flan_dyn.c has not even the string. **Partly feasible**: the
+`flan_trap_hook` (flan_rt.c:598) already hands control to the dev session,
+which *is* in-process with the compiler and can read the file. Rendering a
+squiggle there is the natural home — but dev.ml belongs to another lane, so
+this is a hand-off, not a row in this pass. The cheap half — threading a loc
+into flan_dyn.c's four trap functions — is entirely within this pass.
+
+**5. "What I understood" as structure rather than prose.** No message in the
+tree separates the two halves the way Elm's body does ("This function expects
+… / But you gave it …"). The good ones (defconst, view-lifetime) achieve it
+with a paragraph. **Feasible as convention**, not machinery: the `diag`
+record has `dmsg` plus notes and nothing between, so the shape would have to
+be a writing rule — "first clause is what was read, second clause is the
+collision, last clause names the fix" — enforced by review rather than types.
+
+**6. Multiple errors without cascade.** `Loc.sink` exists and finishes at
+phase boundaries only, which loc.ml:178 admits is "deliberately crude". Not
+graded here (no program was written to exercise cascade), but it is the
+remaining Elm behaviour with machinery that only half exists.
+
+---
+
+## Register notes for the fix pass
+
+- **Never cite a repo file at the user.** `(spec-memory.md)`, `(plan.org,
+  Types)`, `(see plan.org)` appear in at least six messages. Say the rule.
+- **One dash convention.** parse.ml:1156 uses `--`; every other message uses
+  `—`.
+- **"%s of them" / "this one"** — pronouns with no antecedent once the
+  message is read cold. parse.ml:1541 is the clearest case.
+- **Length.** The tree's long messages are mostly *earning* their length
+  (defconst, runaway-instantiation, predicate-not-carried). The one to watch
+  is `view_not_permanent`, which is a paragraph with a subordinate clause
+  nested three deep. The rule the repo wants — plain language, no essay
+  register — bites there and nowhere else so far.
+
+---
+
+## Coverage — honest statement
+
+- **Harvested**: every `Loc.fail` / `Loc.failk` / `Loc.diag` / `Loc.note`
+  call site in lib/check.ml, lib/parse.ml, lib/reader.ml and lib/load.ml —
+  roughly 250 sites — plus every `fprintf(stderr, …)` trap in
+  runtime/flan_rt.c and runtime/flan_dyn.c.
+- **Rendered by hand**: about 30 messages, by writing the triggering program
+  and running `_build/default/bin/main.exe check` or `run` on it, and reading
+  the full output including carets and secondary lines. Scratch programs are
+  in the session scratchpad, not the repo.
+- **Graded from source only**: the remaining ~220. Their S grade is inferred
+  from which location value they are raised against, which is reliable for
+  "is it the operand or the whole form" only where the code makes it obvious.
+  Treat those as provisional.
+- **Out of scope**: lib/emit.ml, lib/x86.ml, lib/cimport.ml, lib/session.ml
+  and lib/macro.ml internals; the wasm32 and JS backends; anything printed by
+  `flan dev`'s break loop.
+- **Not touched**: lib/dev.ml, which belongs to another lane. Gap 4's dev-side
+  half is written up as a hand-off for that reason.
+- **Not exercised**: the `--warn-memory` warning path, macro-expansion
+  diagnostics with a non-trivial `expansion` chain, and multi-error cascade
+  behaviour through `Loc.sink`.