Merge main, and address the second review

[oflatt-claude]

Two things: the merge with main after #60 landed, and the three regressions from the second review.

Merge

The only conflict was two doc bullets added at the same place in the crate docs — unstable-subst and named arguments — and both are kept. Both sides already pin the same egglog (9063586), so the manifest and lockfile merged cleanly.

The three regressions

All three came from moving expansion to the command-macro stage, and the first one is the reason to move back.

[P1] Cloned e-graphs shared field mappings. CommandMacro::transform receives only a SymbolGen and a TypeInfo, so the field names had to live in the macro object — whose Arc every clone of an e-graph shares. There is no per-e-graph storage at that stage, so keying by (name, arity) was never going to be enough, as the review says.

The parser turns out to be the right home after all: it is part of the e-graph, so it is cloned with one and snapshotted by push/pop, and a per-name expression macro only fires for the declaration that registered it. Expansion is back in the parser, and the two things the original parse-time version was missing are fixed directly:

• (include ...) is read once while parsing, so the declarations inside register their field names before the calls that follow the include are parsed. The command itself is passed through untouched, so egglog still reads and runs the file exactly as before — only the registration moves earlier. The read is best-effort and never reports: a missing or unparsable file is left for egglog to fail on from its own include handling, so this can only add registrations. That is the original [P2].
• A positional declaration registers a pass-through macro instead of nothing, shadowing field names registered for that name earlier. egglog rejects redeclaring a bound name (Function already bound R, even inside a (push)), so the only way one name is declared twice is (push)/declare/(pop)/declare — where the later declaration in the source is exactly the one live afterwards. That is the original [P3], and it makes source order match declaration scope without needing the command stage.

[P2] Named calls inside extension commands. Gone with the move: parse-time expression macros fire wherever parse_expr runs, including user-defined command arguments, so (extract (C :x 1)) works again.

[P3] Positional tables rejecting :-prefixed variables. Also gone: a macro is only registered for names that were actually declared, so (relation R (i64)) / (let :value 1) / (R :value) is never inspected for markers. The speculative "not declared with named fields" error is removed.

Tests

Regression tests for all six reported cases, each confirmed to fail on the implementation it was reported against:

Case	Test
Ellipsis collisions	tests/named-args-ellipsis-fresh.egg
Included declarations	tests/named-args-include.egg
Scoped redeclaration	tests/named-args-scope.egg
Cloned e-graphs	test_named_args_do_not_leak_between_cloned_egraphs
Extension commands	tests/named-args-extract.egg
Positional variables	tests/named-args-positional-vars.egg

The include test covers both halves: the included relation is usable by name, and it is declared exactly once — a second execution of the include would fail with Function already bound.

The earlier succinctness changes are kept: register_named_args is the only public item, one read_named_field validator is shared between schemas and datatype variants, and register_named_call is gone. The declare() note from the second review no longer applies — that function belonged to the command-macro version.

make test and make nits pass: 175 nextest tests, doctests, cargo clippy --tests -- -D warnings, cargo fmt --check.

🤖 Generated with Claude Code