edag: end chain lambdas by arity, not a null terminator - #1761
Conversation
Implements fjs/edag/todo/option-terminated-lambdas.md, deleted here. A chain now ends by leaving the continuation operand out: a plain read is ['.', a, 'b'], a terminal call step is ['|()', c], and ['|!()', c] closes a region. Every kind that can end is an or() of its two closed arities, so null is a primitive again with no reading in a continuation position, and the argument the old spelling rested on is inverted: closedness by length is what rejects a continuation smuggled onto a terminal, which is why the terminal needs no explicit third operand. A trailing hole matches neither arm — the short one is bounded by length, the long one has no option member — so validate(exp) keeps rejecting it, pinned in the proofs alongside rejections for a present null, a present undefined, and the smuggled continuation. Sparse values are built with concat(new Array(1)), FunctionalScript having no hole literal. amnesia reads a continuation by destructuring everywhere, skip included: destructuring stops at length, so an absent continuation reads as undefined and never reaches the prototype, where an indexed k[2] would. Downstream designs that prescribe the old spellings are respelled (compile-modules-to-edag, interpret-edag, the bun-parentheses blocker); released changelog entries stay as written. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AQH2SZCpDhyQUgByxkV46e
|
You have reached your Codex usage limits for security reviews. Please try again later. |
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AQH2SZCpDhyQUgByxkV46e
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
functionalscript | f95c95b | Commit Preview URL Branch Preview URL |
Aug 28 2026, 08:18 PM |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e6b1982455
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: dcf7210d50
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| export const dot = or( | ||
| /** @type {const} */ (['.', exp, index]), | ||
| /** @type {const} */ (['.', exp, index, propertyLambda]), |
There was a problem hiding this comment.
Dispatch arity before recursively validating dot operands
For a valid expression that repeatedly wraps a continued Dot as the next dot's object, each long node first tries the three-element arm, recursively validates the entire exp prefix, fails only on closed-tuple length, and then validates that prefix again through the four-element arm. This makes validate(exp) exponential in nesting depth: a 16-node chain took about 3.1 seconds and an 18-node chain about 12 seconds here, while the previous schema validated even 100 levels in milliseconds. Select the arm from the tuple length before recursively walking the shared operands.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Reproduced, and it is real — but the fix is in rtti, not in this schema, so I am raising it rather than pushing it. Measured on this head with validate(exp):
| shape | depth 8 | 12 | 14 | 16 |
|---|---|---|---|---|
| nested continued dots | 36ms | 155ms | 466ms | 1897ms |
| nested plain reads | 0ms | 0ms | 0ms | 0ms |
Roughly 4× per two levels, i.e. 2^depth, and it matters for an ordinary shape: method chaining (a.b(x).c(y).d(z)) nests continued dots exactly this way.
Why arity dispatch cannot be done here. constContainerValidate validates every declared member before checking the tuple's length, so both arms walk the shared exp at index 1 before either can fail. A 4-element dot fails the 3-element arm only at the length check, after the walk; a 3-element dot fails the 4-element arm only at the absent index 3, likewise after the walk. So no ordering of the arms and no edag-side spelling of "select by length" helps — the schema has no way to make the discriminator cheaper than the operand walk.
Proposed patch (verified locally, then reverted — this PR does not contain it): bound the container before reading its members, in constContainerValidate, keeping every existing late check.
if (!isContainer(value)) { return verror('unexpected value') }
if (!fits(value, declared.length)) { return verror('unexpected value') } // added
const r = eachEntry(…)That makes both shapes linear — every depth above drops to 0–2ms — and it costs nothing on structs, where fits is () => true. lengthDoesNotBoundTheWalk still passes: it is about enumeration not materializing the index range, which this does not touch.
The one blocker, and why it is rtti's call. It breaks sameAcceptanceAsParse. Acceptance is unchanged — the gate only rejects earlier what the late check rejects anyway — but for a value failing both a member and the length, validate would report the structural error (path: []) where parse still reports the first bad member (path: ['0']), and the two readers are pinned to report the same error. Completing it therefore means the same gate in parse and the data-form reader, which changes which error wins for that class of values across every consumer. That is an rtti design decision about error precedence, not something an edag PR should decide.
So: shall I extend this PR with the three-reader change, file it as an rtti issue and land it first, or keep the current spelling and accept the cost? I would take the first — the patch is small and the precedence change is defensible (a structural mismatch is the more useful error) — but it is your call.
Generated by Claude Code
Review finding on #1761: edag-stage1-discussion.md is an open working design the staged compiler work links to, and its operations table and worked examples still prescribed null-terminated dots and call steps, so following them now produces graphs the schema rejects. Respelled to the two arities, with the prefix argument updated to match the module's. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AQH2SZCpDhyQUgByxkV46e
Review finding on #1761. The scoping this rests on lived in the todo this PR deletes, and AGENTS.md requires a design decision to be captured in a README or JSDoc before the issue file goes: under a hostile host every read style has its own hole — an unchecked index reads the prototype, and destructuring dispatches an own Symbol.iterator that can yield past length — neither peculiar to ending by arity, since the walkers destructured before it too. amnesia assumes a DJS value on a pristine host, which is one more reason it is not a VM; hermetic reads are rtti's tracked question. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AQH2SZCpDhyQUgByxkV46e
o2alexanderfedin
left a comment
There was a problem hiding this comment.
The migration is sound; you already closed the one finding I was about to file.
Verified at dcf7210d5: both trailing-hole spellings reject — built with .length = n so n in value === false, not a null in disguise — while all 18 rows of the README's Spellings table and 14 hand-built shapes still accept, so no over-broad rejection. Re-admitting ['|()', exp, optionLambda] into the terminal-only propertyLambda fails terminalsTakeNoContinuation and callTerminatesPropertyLambda. Amnesia's four checks moved to undefined, skip destructures, and its 44 leaf names are unchanged, so the +75/-75 is pure respelling. All five vanished leaves elsewhere are inversions — assertions flipping from "no match" to "ok" as the short arity became valid — not losses. Gates 3531/3531, tsc clean. Deleting the todo matches todo/README.md and nothing cites it.
I had todo/edag-stage1-discussion.md written up as the headline finding — open, P2, in this lineage, still prescribing [".", o, p, null] in about fifteen places. 9e9ae8684 respells it; zero null-terminated chain spellings remain there now. Good catch, and it was the one that mattered.
Two small things still standing:
fjs/edag/amnesia/proof.f.mjskeeps two stale comments, both wrapping across lines, which is likely why the sweep missed them: "A.node with anullcontinuation computes the same function value and drops it" (~316) and "the whole node when the continuation isnull" (~370). The second sits directly aboveeq(['?.', ['{}', [[':', 'a', 7]]], 'a'], 7)— the short arity, no continuation at all — so it now contradicts its own example.changelog/unreleased/1761.mdis 1050 characters over 15 lines against the ~250-per-entry guideline, and folds in rationalechangelog/README.mdputs in the PR description.
Review findings on #1761. The two amnesia comments wrapped across lines, which is why the sweep missed them; the second sat directly above a short-arity example it contradicted. The changelog entry ran to 15 lines where changelog/README.md asks for about three (~250 characters) and puts rationale in the pull request description, where it already is. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AQH2SZCpDhyQUgByxkV46e
|
Both fixed in 2627297 — thanks, and the line-wrapping diagnosis was exactly right.
One item is still open and needs a maintainer decision rather than a push — the P1 on Generated by Claude Code |
o2alexanderfedin
left a comment
There was a problem hiding this comment.
Approved. Both stale comments in amnesia/proof.f.mjs are gone, including the one that had been contradicting the short-arity example directly beneath it, and the changelog entry is down from 1050 characters to 280 while still naming what breaks and showing both spellings.
Implements
fjs/edag/todo/option-terminated-lambdas.md(filed in #1755), deleted here.A chain ends by leaving the continuation operand out rather than by carrying a terminator: a plain read is
['.', a, 'b'], a terminal call step is['|()', c], and['|!()', c]closes a region. Every kind that can end is anorof its two closed arities, sonullis a primitive again with no reading in a continuation position — and nooptionappears anywhere, so rtti is untouched.The old argument is inverted, not dropped. "Terminals state their
null" existed to stop a continuation being smuggled onto a two-element terminal. That was written against open-by-default tuples; under closed-by-length the smuggled continuation is a third element the terminal does not declare, so length rejects it.terminalsTakeNoContinuationpins exactly that, including the oldnullspelling.Trailing holes stay rejected, which was the gate the design put on this migration. A hole matches neither arm — the short one is bounded by length, the long one has no
optionmember — pinned on.,?.,?.()and on steps, alongside rejections for a presentnull, a presentundefined, and the smuggled continuation. Sparse values are built withconcat(new Array(1)), FunctionalScript having no hole literal.amnesia reads a continuation by destructuring everywhere,
skipincluded — it previously readk[0]/k[1]/k[2]directly, which was safe only while every step carried an own third member. Destructuring stops atlength, so an absent continuation reads asundefinedand never reaches the prototype.Downstream designs that prescribed the old spellings are respelled (
compile-modules-to-edag,interpret-edag, the bun-parentheses blocker); releasedchangelog/entries stay as written, being history rather than prescription.Verification
npx tscclean. This is load-bearing rather than incidental: theCheck/Check3assertions inproof.f.mjscompare each hand-written type against the type its schema derives to, so the new types and schemas are proven to agree exactly.fjs/edag/**/module.f.mjs: 100% lines, branches, and functions.Changelog
edag: a chain ends by arity, not by anullterminator. A plain property read is
['.', a, 'b'], a terminal call stepis
['|()', c], and['|!()', c]closes a region — each one elementshorter than before, with the continuation operand present only where a
chain actually continues. Every kind that can end is now an
orof its twoclosed arities, so
nullis a primitive again and has no reading in acontinuation position: graphs written against the old spelling
(
['.', a, 'b', null],['|()', c, null]) no longer validate, and theDot,OptionDot,OptionCall,PropertyLambda,OptionLambdaandOptionPropertyLambdatypes no longer admit them. Closedness by length iswhat keeps a continuation from being smuggled onto a terminal, which is
what the explicit
nullused to guard; a trailing hole matches neitherarity and is rejected as before.
amnesiareads a continuation bydestructuring everywhere,
skipincluded, so an absent one never reachesthe prototype.
🤖 Generated with Claude Code
https://claude.ai/code/session_01AQH2SZCpDhyQUgByxkV46e