REVIEW.md — Building a Gradual Type Checker for Lua

Reviewed by Esme, 2026-04-21

Overall

This is a well-structured tutorial that builds incrementally from “hello Salsa” to a working language server simulation. The progression is natural — each chapter introduces one new Salsa concept and shows why it matters. The writing is clear and conversational. The main issues are around code correctness, significant code duplication across chapters, and a few conceptual gaps.

Chapter 1: Hello Salsa

[clarity] The spreadsheet analogy (inputs = cells, tracked functions = formulas, revisions = “after you edit a cell”) is excellent. This is exactly the kind of concrete-before-abstract approach that works.

[error] The contains_text tracked function takes needle: String as a key parameter. This means every unique needle string creates a separate cache entry. For a tutorial, this is fine to demonstrate key parameters, but it’s worth noting that using String as a query key means allocations on every call. A reader might assume this is idiomatic — it’s not, for hot paths.

[suggestion] The README links point to chapters/ch01-hello-salsa.md but the actual files are chapters/ch01-hello-salsa/README.md. The links in README.md won’t resolve correctly.

[style] The code comments are thorough and well-organized with section headers (Step 1, Step 2, etc.). This is great for learning — keep this pattern.

Chapter 2: Parsing Lua

[error] The salsa::Update derive macro is used on LuaAst, Statement, Expression, BinOp, UnaryOp. The salsa::Update trait is not a standard derive in Salsa 0.26 — it may be salsa::Update or may not exist as a derive. This needs verification against the actual Salsa 0.26 API. If it doesn’t exist, the code won’t compile.

[clarity] The README says “Why Convert the AST?” but doesn’t fully explain the 'db lifetime issue. The comment in code mentions it, but the README should note that tracked function return values must be 'static (or at least not borrow from the input), which is why analisar’s Cow<'a, str> AST can’t be returned directly.

[error] The Suffixed expression handling has a logic problem. When s.computed is true (index access a[b]), the match arm ast::Expression::Suffixed(s) if !s.computed && !s.method won’t match, and it falls through to the wildcard _ => Expression::Nil. This silently drops index expressions. Should at least have a comment noting this is intentionally simplified.

[clarity] The top_level_names function’s comment about Salsa skipping re-runs when parse returns the same LuaAst is slightly misleading. Salsa re-runs top_level_names if parse is re-invoked in a new revision — it doesn’t compare the LuaAst value. The comment says “same value” which implies value equality, but Salsa tracks at the revision level, then checks if the inputs to the derived query changed. This distinction matters for understanding Salsa’s actual invalidation strategy.

Chapter 3: Interned Symbols

[error] The Symbol<'db> interned struct is defined but never actually used in the symbol_table or lookup_name queries in a way that demonstrates the benefit. The lookup_name function interns both the lookup name and each definition name, then compares by ID — but it’s doing Symbol::new(db, def.name.clone()) inside a loop, creating new interned values on every comparison. The real benefit of interning is that you store Symbol in your data structures (not String) and then comparison is free. The current code actually does MORE work than a plain string comparison (intern + compare vs. just compare). This undermines the lesson.

[suggestion] The chapter should show Definition { name: Symbol<'db>, ... } instead of Definition { name: String, ... } to demonstrate the actual benefit: store symbols once, compare by ID forever after.

[clarity] The README’s “Interned vs Input” table is great — clear and concise. More of these comparison tables throughout.

[error] The Symbol<'db> lifetime parameter means it can’t be stored in Definition (which is plain data with salsa::Update). This is a real design tension that the tutorial doesn’t address. The reader will try to put Symbol<'db> in their data structures and hit lifetime issues. This deserves explicit discussion.

Chapter 4: Tracked Structs

[error] The FuncDef::new(db, func_name, param_count, body) constructor signature doesn’t match how #[salsa::tracked] structs work in Salsa 0.26. Tracked structs typically use a builder pattern or have specific field registration. The exact API needs verification.

[clarity] The “wrapper/data pattern” in the README is a key concept that deserves more explanation. The README mentions it but the code doesn’t really demonstrate it — FuncDef has body_text: String as a tracked field, but there’s no separate “data” struct inside it. The pattern is: tracked struct holds an ID, a separate data struct holds the content. The current code puts the content directly in the tracked struct, which is the simpler approach. Either demonstrate the actual wrapper/data pattern or remove the reference.

[error] The parsing in parse_functions is extremely fragile — it uses simple string splitting to parse function name(params) body end. This is fine for a demo, but the line if let Some(close_paren) = after_paren.find(')') will break on nested parentheses in the body. A comment acknowledging this simplification would help.

[suggestion] This chapter drops the analisar parser entirely in favor of manual string parsing. The transition is jarring — chapters 2-3 built up the analisar-based AST, and now we’re parsing by hand. A brief explanation of why (tracked structs want a different granularity than the file-level parse) would help the reader understand the architectural shift.

Chapter 5: Type Inference

[error] The infer_type function passes Expression (a large clone-heavy enum) and TypeEnv as query keys. Every recursive call clones the entire environment and expression tree. In a real Salsa program, this would be catastrophically slow for non-trivial programs. The code acknowledges this in the “IMPORTANT DESIGN NOTE” comment, but the comment says “the granularity is per-file” — this isn’t quite right. The granularity is actually per-(source, expr, env) triple, which means different expressions get separate cache entries, but the env cloning makes this extremely expensive.

[clarity] The “IMPORTANT DESIGN NOTE” is good but buried in a comment. This should be in the README and the main text — it’s a critical architectural limitation that the reader needs to understand.

[error] Expression::String was renamed to Expression::StringLiteral in this chapter (vs String in ch02-04). This is actually a good rename (avoids confusion with Rust’s String type) but it’s inconsistent with earlier chapters. Should be backported or noted.

[clarity] The Dynamic type explanation is excellent. “Dynamic is NOT the same as Error. Dynamic means ‘the programmer chose not to annotate this’ — it’s intentional. Error means ‘we found a contradiction’ — it’s a bug.” This is the kind of precise distinction that makes a tutorial valuable.

[suggestion] The is_compatible_with method treats Error as compatible with everything. This means once you have one error, it can suppress other real errors (error cascading is hidden). A brief note about this design choice would be helpful.

Chapter 6: Diagnostics

[error] The Diagnostic::emit method calls self.accumulate(db). In Salsa 0.26, the accumulator API may be different — accumulate might not be a method on the accumulated value itself. This needs verification against the actual Salsa 0.26 accumulator API.

[clarity] The “Why Not Result<Type, Diagnostic>?” section in the code comments is excellent — it clearly explains the three-option tradeoff. This belongs in the README too.

[style] The code formatting in ch06 and ch07 is significantly compressed compared to earlier chapters (e.g., fn new() -> Self { TypeEnv { bindings: Vec::new() } } on one line). This makes the code harder to read, especially in a tutorial context. The earlier chapters’ formatting was better.

Chapter 7: Language Server

[error] The LanguageServer struct holds db: Database (owned). But SourceFile::set_text requires &mut db, and after mutation, any SourceFile references obtained before the mutation are still valid (Salsa guarantees this). However, the diagnostics method takes &self, which means it calls type_check::accumulated with an immutable reference. This should work, but the pattern of holding owned Database in a struct while also holding SourceFile IDs is worth a note — it’s a common pattern in Salsa but not immediately obvious.

[suggestion] The language server simulation doesn’t demonstrate the key benefit: timing. Add std::time::Instant measurements to show that re-checking after a small edit is faster than the initial check. Without timing, the “Salsa skips work” claim is invisible to the reader.

[clarity] The architecture diagram in the code comments is great. Should be in the README.

Cross-Cutting Issues

[error] MASSIVE CODE DUPLICATION. The AST types (Statement, Expression, BinOp, UnaryOp), the convert_stmt/convert_expr functions, TypeEnv, and infer_type are copy-pasted across chapters 2-7 with minor variations. This is ~200 lines duplicated per chapter. While this makes each chapter self-contained (good), it means any bug found in one chapter exists in all of them. Consider: (a) extracting shared code into a library crate that chapters depend on, or (b) explicitly noting that this is intentional for self-containedness and that production code would share the definitions.

[error] The salsa::Update derive macro is used extensively but never explained. The reader has no idea what it does or why it’s needed. A one-paragraph explanation (it enables Salsa to compare old and new values for fine-grained invalidation) would help.

[clarity] The README chapter links use .md extensions pointing to files that don’t exist. The actual content is in chapters/chXX-name/README.md.

[suggestion] No chapter discusses how Salsa handles cycles. The README mentions “Cycle detection — recursive queries that would loop forever are caught” but no chapter demonstrates this. A brief example (even just in an appendix) would round out the coverage.

[style] The code quality degrades noticeably from ch05 onward — compressed formatting, longer lines, less whitespace. Earlier chapters are much more readable.

What’s Working

• The chapter progression is excellent — each one builds naturally on the last
• The Salsa concepts are introduced at the right pace: inputs → tracked functions → interned → tracked structs → accumulators
• The “key concepts” recaps at the end of each chapter are valuable
• The spreadsheet analogy in ch1 is memorable and accurate
• The gradual typing explanation in ch5 is precise and well-articulated
• The accumulator pattern (sentinel return + accumulated diagnostic) is clearly explained
• The per-file isolation demo in ch7 effectively shows Salsa’s incremental value

Deep Dive Additions (2026-04-21 — Second Pass)

I re-read all 7 chapters’ source code, compiled each one, and ran all binaries. The initial review raised API concerns about Salsa 0.26 and analisar 0.4 — these were verified:

• salsa::Update derive macro exists in Salsa 0.26 ✓
• The accumulator API type_check::accumulated::<Diagnostic>(&db, file) works as shown ✓
• All chapters compile and run successfully ✓

However, the deeper code-level review found these new issues:

[error] Suffixed expression regression in ch03, ch05, ch06, ch07

Chapter 2’s convert_expr handles ast::Expression::Suffixed completely: field access (a.b), index access (a[b]), and method calls (a:f()). But chapters 3, 5, 6, and 7 changed the match arm to:

#![allow(unused)]
fn main() {
ast::Expression::Suffixed(s) if !s.computed && !s.method => { ... }
}

The guard !s.computed && !s.method means:

• a[b] (index expression, s.computed == true) → falls to _ => Expression::Nil
• a:f() (method call, s.method == true) → falls to _ => Expression::Nil

This is a regression from Chapter 2. Index and method expressions are silently converted to Nil across 4 chapters. No comment explains this is intentional.

Fix: Restore the full Suffixed handling from Chapter 2, or add a comment explaining that index/method expressions are intentionally unhandled.

[error] convert_args drops table arguments silently

In convert_args, the ast::Args::Table(_) arm returns vec![]:

#![allow(unused)]
fn main() {
ast::Args::Table(_) => vec![], // Table args dropped silently
}

Function calls with table arguments like foo({a = 1}) are converted to zero arguments. For a tutorial showing Lua parsing, this is misleading — readers writing Lua code with table arguments will get broken behavior without explanation.

[error] BinOp::Concat returns Type::String unconditionally

In both ch05 (type inference) and ch06 (diagnostics), the concatenation operator returns:

#![allow(unused)]
fn main() {
BinOp::Concat => Type::String,
}

This returns Type::String regardless of whether the operands are actually strings. In a gradual type checker:

• String .. String → Type::String (correct)
• Dynamic .. anything → should be Type::Dynamic (currently Type::String — wrong)
• Number .. String → should be Type::Error (currently Type::String — wrong)

The current code has no type checking for .. — it’s a blind shortcut. By contrast, BinOp::Add (and other arithmetic ops) in ch06 correctly checks type compatibility and emits diagnostics. BinOp::Concat should do the same.

Note: There’s no test case exercising the .. operator, so this bug isn’t caught by the asserts.

[error] Unused import / dead code warnings

• ch03: use salsa::Setter; — unused import generates warning
• ch03: let mut db = Database::default() — never calls setters, mut generates warning
• ch07: fn display(&self) -> &str — dead code, never called, generates warning

These generate compiler warnings that would confuse readers following along.

[suggestion] No test case exercises the .. operator

None of the chapter tests use Lua’s string concatenation operator (..). Adding a test like "hello" .. " world" and 42 .. "text" would verify the BinOp::Concat case and expose the bug described above.

[clarity] Per-file granularity claim is technically imprecise

Chapter 5’s README says “the granularity is per-file” but the actual granularity is per-statement — check_stmt is called for each statement in the AST, and infer_type is called for each expression within each statement. A change to any statement in the file invalidates all downstream queries for that file. The README should clarify: “granularity is per-file for now; tracked structs (ch04) would enable per-node granularity.”

[suggestion] Add comment explaining per-file granularity limitation

The README says “per-file isolation” in Chapter 7 but the actual behavior is: when a file is edited, ALL statements in that file are re-checked because type_check calls check_stmt for each statement in sequence. Only other files’ queries return cached values. The README should clarify that the per-file isolation means “other files are not re-checked” not “other statements in the same file are not re-checked.”

Priority Fixes from Second Pass

1. Fix suffixed expression regression — ch03, ch05, ch06, ch07 silently drop a[b] and a:f() expressions. Either restore full handling or add a comment explaining it’s intentional.
2. Fix BinOp::Concat type checking — should return Type::Dynamic when either operand is Dynamic, and Type::Error + emit diagnostic when both are incompatible concrete types.
3. Clean up compiler warnings — remove unused Setter import in ch03, change let mut db to let db in ch03, remove dead display method in ch07.
4. Add table argument handling or comment — convert_args silently drops table arguments in all chapters. Either handle them or explain the simplification.
5. Add .. operator test case — currently untested, and the bug from #2 can’t be caught without it.

Chapter 10: Type Annotations (Reviewed 2026-04-24)

This chapter is structurally strong — the opening example (annotated greet vs unannotated) immediately shows the value, the LuaCATS convention is explained concretely, and the gradual guarantee is articulated with precision. The writing stays in the Lin Clark / Julia Evans register: conversational, concrete before abstract, honest about simplifications. The code compiles and all six tests pass.

But there are real bugs in the annotation-association logic that would produce wrong results in multi-function files. These need fixing.

[error] ---@param and ---@return annotations are matched globally, not per-function

The Function arm of check_stmt looks up parameter annotations like this:

#![allow(unused)]
fn main() {
let annotated = annotations.iter().find_map(|a| match a {
    Annotation::Param { name: n, ty } if n == p => Some(ty.clone()),
    _ => None,
});
}

annotations is the full list for the entire file. find_map returns the first Annotation::Param with that name, regardless of which function it belongs to. In a file with two functions that both have a parameter named x:

---@param x number
local function add(x) return x + 1 end

---@param x string
local function greet(x) return "hi " .. x end

Both functions would get x: number — the second function’s annotation is silently ignored. The same bug affects ---@return: find_map returns the first Annotation::Return in the entire file, so every function with a return annotation gets the first one found.

Fix: The extract_annotations function already groups params and returns by function (the pending-params/return flushing logic). But after flushing, the annotations lose their association with the function they belong to. Either: (a) add a function_name: String field to Annotation::Param and Annotation::Return so they can be matched to the right function, or (b) change the extraction to return a HashMap<String, FunctionAnnotations> keyed by function name. Option (b) is cleaner.

[error] apply_annotation matches the first empty-name ---@type to any variable

apply_annotation iterates ALL annotations and returns the first Annotation::Type { name: "", ty } it finds, regardless of which variable it should apply to. In a file with multiple ---@type annotations:

---@type string
local x = "hello"
---@type number
local y = 42

When checking y, apply_annotation finds the first ---@type string annotation and applies it, making y: string instead of y: number.

Fix: Either track which ---@type annotations have already been consumed (consume-on-read), or — since the README acknowledges that ---@type position matching isn’t implemented — don’t apply ---@type annotations at all. The current half-implementation produces wrong results silently, which is worse than not applying them.

[error] Pending annotations are never cleared when a non-function line breaks the association

The LuaCATS position rule says annotations must appear immediately above the thing they annotate. A comment or blank line between ---@param and the function should break the association. The code’s own comment says this: “stray comments between annotations and their targets — they break the association.” But the code doesn’t implement it:

#![allow(unused)]
fn main() {
if !trimmed.starts_with("---@") && !trimmed.is_empty() {
    if pending_params.is_empty() && pending_return.is_none() {
        // Nothing to clear.
    }
}
}

This only enters the block when pending is already empty — the case where clearing is needed (pending is NOT empty) is skipped. Pending params and returns are never cleared on a breaking line.

Fix: Clear pending_params and pending_return when a non-annotation, non-function, non-empty line appears:

#![allow(unused)]
fn main() {
if !trimmed.starts_with("---@") && !trimmed.is_empty()
    && !trimmed.starts_with("local function") && !trimmed.starts_with("function") {
    pending_params.clear();
    pending_return = None;
}
}

[clarity] “Annotations are inputs, not derived values” — contradicted two paragraphs later

The “Annotations as Salsa Inputs” section opens with: “annotations are inputs, not derived values.” But the very next paragraph explains that we extract them via a tracked function (making them derived), and the diagram shows them as a tracked function between the input and type_check. The text eventually resolves the tension (“they’re derived from the source text — they’re a parsed view of what’s already in the input”), but the opening claim is flatly wrong and will confuse the reader before they reach the nuance.

Fix: Reword the opening to something like: “Annotations feel like inputs — the programmer writes them, they don’t change unless the source changes. But architecturally, they’re extracted from the source text by a tracked function, which means Salsa handles invalidation automatically.” Start with the accurate description, not the wrong one.

[clarity] Code snippet in README uses fe without declaration

The “How Annotations Change Type Checking” section shows this code:

#![allow(unused)]
fn main() {
for p in params {
    let annotated = annotations.iter().find_map(|a| match a {
        Annotation::Param { name: n, ty } if n == p => Some(ty.clone()),
        _ => None,
    });
    let pt = annotated.unwrap_or(Type::Dynamic);
    param_types.push(pt.clone());
    fe = fe.extend(p.clone(), pt);
}
}

The variable fe appears from nowhere. The actual code has let mut fe = env.clone(); above the loop, but the README snippet omits it. Readers tracing through the code won’t know what fe is. Either include the declaration or rename to something self-explanatory like func_env.

[clarity] Annotation::Type { name: String, ty: Type } has a name field that’s always empty

The ---@type variant has a name field, but the parser always sets it to String::new(). The README acknowledges that ---@type doesn’t carry a variable name. So why does the data structure have the field? This will confuse readers who look at the type definition and wonder when name is non-empty. Either remove the field or add a comment on the definition explaining it’s a placeholder for future position-based matching.

[style] The opening example is excellent

The greet example at the top of the chapter is exactly right. It shows the problem concretely (passing 42 to a function that concatenates), explains why the type checker misses it (parameter is Dynamic), and motivates the whole chapter in three paragraphs. No “What You’ll Learn” section, no hand-waving. More of this.

[style] “The Gradual Guarantee” section is precise and well-articulated

The before/after example (unannotated → Dynamic → no errors vs annotated → type checking activates) is clean. The closing line — “Dynamic is not a failure of the type checker — it’s the whole point of gradual typing” — is the kind of precise distinction that makes a tutorial valuable. More of this.

[suggestion] Add a test case with multiple annotated functions

All six test cases use single-function source files. A test with two annotated functions would expose the annotation-association bugs described above. For example:

#![allow(unused)]
fn main() {
let source = SourceFile::new(&mut db, "test.lua", [
    "---@param x number",
    "local function add(x) return x + 1 end",
    "---@param x string",
    "local function greet(x) return \"hi \" .. x end",
    "greet(42)",
].join("\n"));
}

This should catch greet(42) as a type error (passing number to string param), but with the current code it won’t, because greet’s x gets the number annotation from add.

[fixed] BinOp::Concat type checking is now correct in Ch10

The second-pass review flagged that BinOp::Concat unconditionally returned Type::String regardless of operand types (ch05/ch06). Ch10’s infer_type now handles all cases correctly: String..String → String, Dynamic..anything → Dynamic, Error → Error, and incompatible types emit a diagnostic and return Type::Error. This fix is in Ch10’s code only — ch05 and ch06 still have the bug.

[suggestion] The “What We’re Simplifying” section could be slightly more honest about the ---@type bug — [fixed] ✅

The section now says: “We match it to the next variable that requests an annotation, then consume it so it’s not reused.” This accurately describes the consume-on-read pattern implemented in the fix.

2026-04-25 Verification — Ch10 Annotation-Association Fixes

Lola fixed the three [error] items in ch10’s annotation logic:

1. ---@param / ---@return scoped to function — [fixed] ✅ — Annotation::Param and Annotation::Return now have a func_name: String field. extract_annotations records the function name when flushing pending params/returns. check_stmt matches only annotations where func_name == name.

2. apply_annotation consumes ---@type on use — [fixed] ✅ — apply_annotation now takes &mut Vec<Annotation> and removes each ---@type annotation after applying it. This prevents the same annotation from being applied to every variable.

3. Pending annotations cleared on breaking lines — [fixed] ✅ — When a non-annotation, non-function, non-empty line appears with pending params/returns, the pending state is now cleared. The previous code had the logic inverted — it only entered the clearing block when pending was already empty.

New test added: multiple_functions_with_same_param_name — verifies that greet(42) fails when greet’s x is annotated as string even though add’s x is annotated as number. All 7 tests pass.

README updated: code snippets reflect func_name field, “What We’re Simplifying” section accurately describes the consume-on-read pattern.

2026-04-29 — Bug Fixes in Chapters 5, 6, 7

While doing a review pass, I went through the flagged issues in Esme’s review and verified the actual state of the code. Several were already fixed (the suffixed expression regression in ch05/ch06/ch07 — the broken guard pattern from ch02 had already been replaced with proper if s.computed branching). Two real issues remained:

[fixed] Method calls returned Nil type in ch05, ch06, ch07 — [fixed] ✅

The convert_expr function for ast::Expression::Suffixed had this branch:

#![allow(unused)]
fn main() {
} else if s.method {
    // Method calls handled by FuncCall; fall back
    Expression::Nil
}

When a Lua method call (a:b()) appeared without a surrounding function call prefix, it returned Type::Nil with no diagnostic. This silently propagates — if a method reference is used anywhere, the type checker would silently treat it as nil. Fixed by using the <method-call> placeholder from ch02, which produces a clear “unknown variable” diagnostic instead.

[fixed] Table arguments silently dropped without comment in ch05, ch06, ch07 — [fixed] ✅

The convert_args match used _,_=>vec![] to catch Table args (and everything else), with no comment explaining the drop. This meant foo({a = 1}) silently passes zero arguments — potentially masking real type errors. Replaced with:

#![allow(unused)]
fn main() {
// All other arg forms (Table, etc.) are dropped. We'd need
// Expression::Table and proper table type inference to handle them.
_=>vec![]
}

Remaining known limitations (not bugs, just not yet implemented)

• convert_args drops table args — still the case, now documented. Would need Expression::Table variant.
• Symbol<'db> not used in Definition — ch03 Definition still uses String, not Symbol<'db>. Using interned symbols in plain data structs requires careful lifetime handling.
• BinOp::Concat wrong type in ch05/ch06 — only fixed in ch10. Earlier chapters still return Type::String unconditionally.

All 23 tests pass. cargo build produces zero warnings.

2026-05-01 Deep Re-Review — Ch11: Union Types and Table Classes

Reviewed by Esme, 2026-05-01

No new commits since the 09:52 UTC review. Deep re-review of Ch11 — a chapter I hadn’t scrutinized closely before.

[error] is_compatible_with for Union→expected uses any() — should use all()

The is_compatible_with implementation for union types:

#![allow(unused)]
fn main() {
(Type::Union(variants), expected) => {
    variants.iter().any(|v| v.is_compatible_with(expected))
}
}

This says: “a union value is compatible with the expected type if any variant is compatible.” But that’s wrong for type safety. If x: Number | Boolean and you’re assigning it to y: Number | String, the assignment should fail because Boolean isn’t compatible with Number | String. With any(), it passes — because Number IS compatible with Number | String.

The correct subtyping rule for unions is: all variants of the source union must be compatible with the target. The fix:

#![allow(unused)]
fn main() {
(Type::Union(variants), expected) => {
    variants.iter().all(|v| v.is_compatible_with(expected))
}
}

The second arm (actual value → Union expected) is correct with any() — a concrete Number only needs to fit one variant of Number | String.

The chapter discusses is_compatible_with vs is_exactly but frames the problem as overly lenient for arithmetic (“a Number | String in arithmetic position passes the check”). That’s a real issue, but the union-to-union comparison bug is more fundamental — it gives wrong answers for assignments, not just operations. A Number | Boolean would be incorrectly accepted where Number | String is expected.

Why this matters pedagogically: A reader building a type checker from this chapter would implement union compatibility wrong. They’d get confusing results when unions appear on both sides of an assignment, and the bug would be hard to trace because it only manifests for multi-variant unions where the variants differ.

Fix: Change any() to all() in the Union→expected arm. Update the text’s discussion of is_compatible_with limitations to note that the fix addresses both the assignment-correctness issue AND the arithmetic-lenience issue (with all(), Number | String is NOT compatible with Number, which means arithmetic on a union value correctly fails).

[clarity] parse_type_name maps unknown type names to Dynamic without warning

The parse_type_name function maps unknown type names to Type::Dynamic with a comment: // Unknown type name → Dynamic (with warning, in a real checker). But the code doesn’t emit any warning. A reader building a checker would want to know when they’ve misspelled a type name (nmuber instead of number). The comment acknowledges the gap but doesn’t explain why the tutorial omits it or what the reader should do about it.

Fix: Either add a brief explanation (“We don’t emit warnings here because the annotation parser runs before diagnostics are collected — a production checker would buffer these as parse warnings”) or add a // TODO: emit diagnostic for unknown type name to make the omission explicit.

Status

• 0 outstanding [error] items ✅ — is_compatible_with Union→expected any() → all() fixed (d94ac94)
• 0 outstanding [clarity] items ✅ — parse_type_name unknown-type-name mapping now has inline comment explaining the omission (code line 249-250)