todo: file the EDAG flat-example re-notation - #1760
Conversation
#1756 corrected the places `edag-stage1-discussion.md` is read as normative — the structural-operations table, subject 4's resolution and history, the object-constructor validation rule — after review found the design specifying flat `["[]", ...node]` where the schema nests. Its worked examples were left, because they do not all want the same treatment and deciding per site is a judgement rather than a sweep. That reasoning lived only in the review thread, which REVIEW.md names as the one place an answer does not survive a merge; this is it written down. The inventory splits three ways. About a dozen examples describe today's shape and are mechanical — including every `["{}"]`, which the schema writes `["{}", []]`; one sharing pair recurs in four sections and should read the same in all four. Three passages quote a superseded proposal, where the flat spelling is what the quote said and rewriting it would falsify the record rather than correct it — though one of those is a decision record also read as current, and one is an objection the nested form happens to answer. The remaining prose ellipses (`["[]", ...]` for "the array node") make no claim about operand grouping. P4: the normative table is correct, `fjs/edag/README.md` is correct, and no code is wrong — the risk is a reader copying a worked example instead of the table. That is why it is filed rather than swept. Sites are named by section, not line number, per tokenizer-line-citations. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01T7vAocRuxfmWJDnujCoMup
|
You have reached your Codex usage limits for security reviews. Please try again later. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d54d63c801
ℹ️ 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".
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
functionalscript | aee1747 | Commit Preview URL Branch Preview URL |
Aug 28 2026, 07:34 PM |
o2alexanderfedin
left a comment
o2alexanderfedin
left a comment
There was a problem hiding this comment.
Approved.
Checked every worked example the note cites against edag-stage1-discussion.md rather than sampling — all seven class-1 sites are flat as described, and both class-2 quotations are where and as claimed. The schema claim matches shipped code: types.ts has readonly['[]', readonly Items[]] and readonly['{}', readonly Properties[]], so the nested-operand target is the right one.
Swept independently for both quote styles and multi-line literals; nothing turned up that the table misses. The one instance not listed — ["{}"] in §5's validation prose — is a bare tag reference, not a worked example, so it falls under the note's own class-3 rationale rather than being an omission.
Consistent with the two recent notes: it correctly finds compile-modules-to-edag.md and edag-spec.md already clean, and the null-terminated continuation in the frame example is right for today's shipped schema — option-terminated-lambdas.md is still open, so there is no contradiction.
Nothing in the repo generates or validates these markdown examples, so the re-notation really is manual and the gate expectation is right. Status open, priority P4, all links resolve, no changelog needed for a todo-only PR. tsc 0 in a clean worktree; npm test skipped — one markdown file, zero code.
One I could not check: the note says the original finding carried P2. #1756's merge message discusses the finding but states no priority, so I have nothing to confirm it against.
Review caught a trap in this issue's own instructions. "Every `["{}"]` →
`["{}", []]`" reads as a blanket rewrite, but one of the seven occurrences
in `edag-stage1-discussion.md` is not an empty object: the validation rule
writes "every element of a `["{}"]` node's entry array must be a recognized
entry form", where the tag names *an object-constructor node*. Rewriting
that one with the examples makes the sentence contradict itself — an empty
entry array has no elements to check — so an implementer following this
issue literally would have introduced the defect.
It is also the occurrence #1756 introduced rather than inherited: the
previous wording said "every `["{}", ...]` operand", which stopped
describing the shape once the operands became one array.
Added as a fourth class with its own treatment. The shorthand only became
ambiguous when `["{}", []]` acquired a meaning, so it wants a nonempty
placeholder — `["{}", entries]` — rather than the empty form or a bare tag.
The counts in the problem statement, the class-1 blurb and the task list
are qualified to match: six genuine empty objects, one shorthand.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01T7vAocRuxfmWJDnujCoMup
o2alexanderfedin
left a comment
o2alexanderfedin
left a comment
There was a problem hiding this comment.
Approved. Class 4 is a real distinction and it sharpens my earlier read: I had accepted the seventh ["{}"] as prose that says nothing about grouping, so leaving it alone was safe. Your reason is better — the shorthand only became ambiguous once ["{}", []] acquired a meaning, and rewriting it as the empty form would contradict its own sentence, since an empty entry array has no elements to check. ["{}", entries] fixes the ambiguity without asserting emptiness.
Splitting six-plus-one out of the blanket rewrite is exactly the kind of thing a mechanical sweep gets wrong, and the task list now says which is which.
3 participants
Follow-up to #1756, which corrected the places
todo/edag-stage1-discussion.mdis read as normative — the structural-operations table, subject 4's resolution and history, the object-constructor validation rule — after review found the design specifying flat["[]", ...node]where the shipped schema nests. Its worked examples were left behind, because they do not all want the same treatment and deciding per site is a judgement rather than a sweep.That reasoning lived only in the review thread on a merged pull request, which
REVIEW.mdnames as the one place an answer does not survive. This is it written down.The inventory splits four ways, and only the first is mechanical:
["{}"]are genuine empty objects and become["{}", []]. One sharing pair —["[]", x, x]against["[]", ["{}"], ["{}"]]— recurs in four sections and should read identically in all four.["[]", a, b]reads as both a two-element array anda[b]— is an argument the nested form happens to answer, which that section does not yet make.["[]", ...]standing for "the array node") make no claim about operand grouping and want nothing.["{}"]is not an empty object. The validation rule says "every element of a["{}"]node's entry array must be a recognized entry form", where the tag names an object-constructor node. Rewriting it with the class-1 examples makes the sentence contradict itself, since an empty entry array has no elements to check — so it wants a nonempty placeholder,["{}", entries], instead. This is the occurrence Normalize markdown heading levels and remove @module JSDoc tags #1756 introduced rather than inherited: its previous wording, "every["{}", ...]operand", stopped describing the shape once the operands became one array, and the bare tag it was replaced with collides with the empty-object form that same PR gave meaning to. Caught in review here.P4 rather than the P2 the original finding carried: the normative table is correct,
fjs/edag/README.mdis correct, and no code is wrong. The residual risk is a reader copying a worked example instead of the table.Sites are named by section rather than line number, per
tokenizer-line-citations.Touches
todo/only — one new file, no behaviour or public-API change, so no changelog entry.🤖 Generated with Claude Code
https://claude.ai/code/session_01T7vAocRuxfmWJDnujCoMup