[myzie]

What changes for the user

Program.Check(env) finds an expression's mistakes before it runs. You describe the env as a map[string]*expr.Type. Check returns the result type and a *TypeError that lists every problem. Field names can now also be Go keywords, such as event.type.

See it

go run ./examples/type_checking checks expressions against a typed env. Part of its output:

issue.titel
    `issue.titel`: no field "titel" in {kind: "bug" | "feature" | "question", labels: list<string>, score: number, title: string} (did you mean "title"?)
issue.kind == "bugfix"
    `issue.kind == "bugfix"`: the value is one of "bug" | "feature" | "question", never "bugfix"
review.verdict
    `review`: may be nil here; use ?. or try(...)
issue.score + issue.title
    `issue.score + issue.title`: cannot add number and string
split(issue.title, 1)
    `1`: split: argument 2 must be string, not number

Why

Hosts often store expressions: in config, in rules, or in workflow definitions. Today a mistake in a stored expression shows up only when some input reaches it. That can be days after someone saved it. Check reports the mistake when the expression is saved. The workflow library's typed definitions (deepnoodle-ai/workflow#42) use it to check every condition and template in a definition at load time.

GitHub webhook payloads use type, range and default as field names. Go's parser rejects x.type, so these expressions did not compile before this PR.

What Check reports

• Names that are not in env, a binding, or the registered functions.
• Fields that a closed object does not have, with "did you mean".
• Operations on the wrong kind of value, such as adding a number and a string.
• Builtin calls with the wrong number or kind of arguments.
• Reads of values that may be nil, and of keys that may be absent.
• == and != that can never be true, such as a string outside an enum.

Template.Check(env) checks every ${...} in a template. WithFuncType declares the type of a function that a host registers.

Decisions to check

• Check follows Run, not an ideal type system. && and || return an operand, so s || 0 types as the union 0 | string. A nil check narrows by the text of the expression. Reason: a check that disagrees with Run rejects good expressions or passes bad ones. Rejected: type && and || as bool.
• Type is a struct with exported fields. Hosts build types directly, for example from a JSON Schema. A union is a KindAny type with Variants. Reason: hosts can build and inspect types without a builder API. Cost: the struct layout becomes public API at v1.3.0. Rejected: an opaque type with constructors only.
• Any turns checking off. A nil *Type in env or in Fields also means Any. Reason: a host can start with a partly typed env and add types later. Rejected: require a type for every name.
• A key that may be absent needs ?. or ?[. This applies to an unlisted field of an open object with a typed Elem. It also applies to an index by a non-literal string, such as scores[name], on any object. Reason: Run fails on a missing key. Rejected: accept the read and let Run fail.
• Keyword field names use a source rewrite. Before parsing, x.type becomes x.__expr_kw_type. After parsing, the name changes back on the syntax tree. Reason: expr keeps using go/parser. Rejected: a parser of our own.

How to verify

go test ./...
go run ./examples/type_checking

TestCheckMatchesRun holds about 50 expressions that Check rejects, and Run must fail on each of them. It also holds about 20 that Check accepts, and Run must succeed on each of them. TestGuide_TypeChecking and TestReadme_TypeChecking test every claim in the new guide and in the README.

Start reading at docs/guides/type-checking.md, then types.go, then check.go.

Risk and rollout

• The change adds API. Existing programs that do not call Check behave as before.
• Expressions with keyword field names failed to compile before. Now they compile.
• Identifiers beginning with __expr_ or __try_ are reserved, as they were on main. A field literally named __expr_kw_type now reads the field type. On main, x.__expr_map__ already read the field map. The spec now states the rule.
• Compile no longer precomputes == and != between two literals. The result is the same. The change lets Check see comparisons such as "a" == 1.
• After merge, tag v1.3.0. Then move workflow#42 from the pseudo-version to that tag.

Not in this PR

• A list index that is out of range. Check cannot see values, so Run still reports this.
• Types for registered functions without WithFuncType. These return Any.

Reviews

• An independent review tried to make expressions pass Check and then fail in Run. It found 10 cases in the workflow library. Each case is now a regression test here or in workflow#42.
• The review bot found S1 to S5 and N1. All are fixed in 2fb599e and ae64ca3.
• A second review found B1 to B7 and M1 to M3. All are fixed in cbb2e25, and each case has a test in TestCheckMatchesRun. A follow-up review confirmed the fixes. Its one nit is fixed in e636a20.
• The documentation is in 9aae17d: a guide, an example program, a README section, and notes in the spec and llms.txt.
• No acceptance tester ran. The author ran every example program, and the workflow#42 test suite with Postgres, against this branch.

🤖 Generated with Claude Code

[mobiusops]

Comment: Two Should-fix items need to land before merge: S1 can make Check panic on a nil field type, and S2 lets a x != nil check leak into a binding that shadows x.

This is the checker that deepnoodle-ai/workflow#42 depends on, so the public Type surface should be settled before that merges. The spec section matches the code except for S5.

Should fix

S1. Nil field types panic in Check (check.go:409)
A field typed nil, such as ObjectOf(map[string]*Type{"items": nil}), flows into code that reads .Kind. first(obj.items) panics in elemFunc (check.go:717), sameFunc panics the same way (check.go:723), and obj[k] panics in index (check.go:462) when k is a nil-typed field. ident already maps a nil env entry to Any, so fields should match. Fix: in field, return orAny(f) in the case ok: branch so a nil type never leaves field.

S2. A nil check leaks into a binding that shadows the name (check.go:148, check.go:600)
narrowed is keyed by source text and is never cleared when any, map, and the other forms rebind a name. With x: {a: number}? and xs: list<{a: number}?>, x != nil && any(xs, x, x.a > 1) passes. Inside any, x is a list element that can be nil, but applyNarrowing still drops its ?. Fix: in iterating, remove narrowed entries equal to bind or starting with bind + "." while the body is checked, then restore them.

S3. A host function named like a builtin gets the builtin's type (check.go:540)
A registered function takes the builtin's type whenever the name matches, even when WithFunctions replaced the builtin. WithFunctions({"len": myLen}), where myLen takes numbers, makes len(5) fail with "len of number", and Run accepts it. Fix: skip builtinTypes for any name a WithFunctions option replaced. This can wait for a follow-up.

S4. x[i] on a value that is always nil passes (check.go:446)
field reports "the value is always nil" (check.go:394), but index returns Null() with no report. nil[0] passes Check, and indexValue fails it at run time with "cannot index nil" (program.go:1358). Fix: in index, report the same way field does when !optional && c.lenient == 0. This can wait for a follow-up.

S5. Two single values are never compared (check.go:379)
The test needs len(set.Enum) > 1, so "a" == "b" passes, and so does EnumOf("a") == "b". The spec says literals are single-value enums and that a literal outside the other side's enum is reported. Fix: drop the > 1 test. This can wait for a follow-up.

Nits

• N1. restoreKeywordFields strips __expr_kw_ from every selector, so a user's x.__expr_kw_foo now reads foo. Limit the strip to names the rewrite created, or accept it and leave a comment. (engine.go:392)

Questions

• Q1. For S1, should a nil field type mean "unknown" (Any), the way a nil env entry does, or should ObjectOf reject it? The fix differs, so the answer decides which one to write. (check.go:409)

Sensitive paths

• CLAUDE.md: adds the types.go and check.go row to the layout table. The row matches the files.

Prompt for your AI tool: fix S1, S2, S3, S4, S5

Fix these review findings in deepnoodle-ai/expr, PR #29.

1. S1, check.go, func (c *checker) field: a field typed nil leaves field
   unchanged and later code reads .Kind on it. Wanted: return orAny(f) in
   the `case ok:` branch so nil never leaves field. Also check
   func elemFunc and func sameFunc, which read a[0].Kind. Test: an
   ObjectOf with a nil field, then first(obj.items), sameFunc callers, and
   obj[k] with a nil-typed k. Each must return without panicking.
2. S2, check.go, func (c *checker) iterating: narrowed entries for the
   bound name leak into the body, so x != nil && any(xs, x, x.a > 1)
   passes. Wanted: while checking body, remove c.narrowed entries equal to
   bind or starting with bind + ".", then restore them. Test: that
   expression must report "may be nil".
3. S3, check.go, func (c *checker) call: builtinTypes applies to a name
   that WithFunctions replaced. Wanted: skip builtinTypes for those names.
   Test: WithFunctions({"len": numberFn}) with len(5) must not be rejected.
4. S4, check.go, func (c *checker) index: index on a value that is always
   nil returns Null() silently. Wanted: report "the value is always nil"
   when !optional && c.lenient == 0, as func (c *checker) field does. Test:
   nil[0] must fail Check.
5. S5, check.go, func (c *checker) comparable: the test len(set.Enum) > 1
   skips single-value enums. Wanted: report "a" == "b" and EnumOf("a") ==
   "b". Test: both must fail Check with a never-true message.

Run the existing tests after the changes.

[myzie]

Addressed in 2fb599e and ae64ca3. 2fb599e on its own doesn't build: one test needed a fix, which is in ae64ca3.

• S1 (fixed): a nil field type now means Any, the same as a nil env entry, which answers Q1. first/reverse over it, obj[obj.k] and obj.items.deep no longer panic.
• S2 (fixed): while a form's body is checked, its binding hides nil checks on the name it shadows, and on index too. x != nil && any(xs, x, x.a > 1) now reports "may be nil".
• S3 (fixed): a builtin's result type applies only when the registered function is the builtin itself, compared by code pointer. A host len that takes numbers is no longer rejected.
• S4 (fixed): nil[0] is reported as always nil.
• S5 (fixed): comparisons against single-value enums are checked, so e == "b" with e: "a" is reported.
• N1 (fixed): the rewrite restores a field name only when it is a keyword, so a user's x.__expr_kw_foo reads that field as written.

🤖 Generated with Claude Code

[mobiusops]

Re-review of the new commits on this branch: 2fb599e ("fixes from review of #29") and ae64ca3 (test-only). I read the diff from 7d6c5fd. I did not run tests, builds, or linters, so CI is the check that these compile and pass.

Verdict: Comment. No blocking findings.

Previous findings, all addressed in code with regression tests in TestCheckReviewFixes:

• S1 (nil field types panic): fixed. field treats a nil field type as Any. elemFunc and sameFunc nil-check their argument, and index uses orAny.
• S2 (narrowing leaks into a shadowing binding): fixed. iterating hides narrowings on the binding name and index while the body is checked, then restores them. x != nil && any(xs, x, x.a > 1) now fails with "may be nil".
• S3 (host function inherits a builtin's type): fixed. isBuiltin compares the registered function's code pointer with the builtin's. engine.go stores registered functions unwrapped, so the match holds for real builtins.
• S4 (index on always-nil value): fixed. Reports "the value is always nil" outside try.
• S5 (single-value enum comparison): fixed. The check runs for len(set.Enum) > 0. It only runs after the kinds match, and Join keeps an enum only when both sides carry one, so a union cannot trigger a false report.
• N1 (prefix stripped from every selector): fixed. restoreKeywordFields restores only a prefixed name whose remainder is a Go keyword.
• Q1 (meaning of a nil field type): answered in code. A nil field is unknown, matching a nil env entry. docs/reference/spec.md does not say so yet.

New, nit, non-blocking:

• N2: ae64ca3 weakens a test assertion. TestKeywordPrefixOnlyRestoresKeywords changed from require.EqualValues(t, 1, v) to require.Equal(t, "1", fmt.Sprint(v)). The commit message says "fix test build", but I can't see a compile error in the file at 2fb599e. The new form also passes for any value that prints as 1. Could you say what failed? If the original compiled and only the assertion was wrong, please restore a typed check.
• N3: spec. One sentence under "Static type checking" saying a nil field type is treated as any would document Q1.

The earlier follow-ups (S3–S5) are fixed too, so nothing from the previous review is still open.

[mobiusops]

Approve: Updated for e636a20. Every earlier finding is fixed, and I found nothing new that blocks merge.

Fixed: B1–B7, M1–M3 from the second review (cbb2e25), S1–S5, N1, N2, and Q1 from the first. S1 and S2 need the fix, S3–S5 were follow-ups, and N2 was the weaker keyword test assertion, which is restored to require.Equal(t, 1, v).

Checked: the Check/Run agreement on name resolution (env, then registered functions, then forms, matching evalCall and resolveCallable), keyword rewrite now running before optaccess (no keyword is renamed after ?., so no string literal gets the prefix), union handling in Join/isAny, the builtin signatures against the native implementations (join, min/max, sprintf), nil collections in map/filter (Run treats nil as empty, so no nil check is needed there), and the try result join. I did not run tests, builds, or linters.

Draft status and "do not merge" are unchanged.

[mobiusops]

Approve at 9aae17d. No blocking findings.

The new commit is docs only: docs/guides/type-checking.md, examples/type_checking/, README and examples README, llms.txt, the spec, CLAUDE.md, engine.go doc comment, and docs_guides_test.go. I checked the guide's claims against the code at this head:

• Every API the guide, README, and example use exists: Null, Bool, Number, String, EnumOf, ListOf, ObjectOf, OpenObject, Join, MathFuncs/StringFuncs/CollectionFuncs, WithFuncType, Template.Check, NewTemplate, and TypeError.Unwrap to ErrType.
• The open-object Elem example matches types.go.
• count in check.go resolves env, then registered functions, then forms, so the "registering count replaces the form" line is accurate.
• The 0 | string union rendering matches the spec and the new TestGuide_TypeChecking expectations.

Not run: tests, builds, linters. I read the new test and example but did not compile them.

Nit (N3, non-blocking): the engine.go package doc example calls p.Check(env) without defining p. It's a comment, so nothing breaks, but a copy-paste will stop on it. Adding p, _ := Compile(...) above it would fix it.

The PR is still a draft with "Do not merge yet" in its body. This approval does not change that.

[mobiusops]

Reviewed ee08a3b (the new head, one commit on top of 9aae17d).

Verdict: Approve. Blocking: 0.

What changed: one commit, ee08a3b, adding three lines to docs/reference/spec.md (after the keyword field-name paragraph, around line 304). It states that identifiers beginning with __expr_ or __try_ are reserved for expr's internal use, and that this applies to field names.

Checked against the code:

• The internal names that use these prefixes are __expr_map__, __expr_if__, __expr_kw_ (keyword field rewrite), __try_select__, and __try_index__ (engine.go, higher_order.go, internal/optaccess). The new sentence matches them.
• The spec's "Reserved binding names" section is unchanged and still consistent.
• No Go code changed in this push.

Not checked: I did not run tests, the example, or the docs tests, per the review constraints. I read the diff and the code it describes only. The earlier approval at 9aae17d still holds for everything other than this sentence.

Non-blocking, unchanged from before: the engine.go package doc example calls p.Check(env) without defining p.