✨ Validate a supplied document without executing it - #654
Merged
Conversation
Add `validateDocument()`: one core operation that reads a supplied root and the Markdown source closure ordinary component selection discovers, and answers with deterministic version-1 data — one closed code per diagnostic, the normalized schema issues execution already produces, and one record per authored invocation saying whether it is valid, invalid, or not decidable without running the document. Traversal follows source rather than execution reachability: the selected root projection first, then definitions in FIFO discovery order, each source identity read once, so a cycle terminates and is not a failure. An invocation inside authored control flow is checked like any other, and no branch is evaluated to decide whether it would run. The rules come from execution rather than a second catalog. The body output/return contract moves to `body-structure.ts`, and the static parts of every structural construct move to `structural-rules.ts`; expansion renders the same facts as the printed errors it always produced. Frontmatter and definition parsing report the phase a failure belongs to, so nothing classifies an error by reading its message. The declaration-only registry inspection built is now shared. Observation performs no document effect: no expression is evaluated, no component invoked, no repository module imported, no identity factory called, no provider installed, no journal opened, and nothing the document authored is written.
…cord (#653) Two version-1 contracts were wrong. The root did not enter the source cache, so a root living where a component name resolves was scanned twice: a retained root at `components/Foo.md` containing `<Foo />` returned the invocation names `["Foo", "Foo"]` and read the file again. The root now registers under its own path, so a selection finding it terminates the walk the way any other cycle does and reads nothing. A structural fact a parent discovered about a descendant went to the parent alone. A misplaced `<Else>` found through its `<If>` named `If`, and the `<Else>` it was about stayed valid. Such a finding now names and is positioned at the element it identifies, reaches that element's record, and still leaves the `<If>` whose structure is malformed invalid. Preorder guarantees the parent is visited first, so the child picks it up rather than losing it. An `<If>` with an unknown prop also returned early, hiding the missing condition and a malformed `<Else>` beside it. Every independent static check now runs. Expansion's first-refusal behavior is unchanged: it stops at the first violation over the same shared facts, and validation reads all of them. Each regression was confirmed discriminating by reverting its fix.
… selected (#653) One path is bytes; what is parsed from them depends on who asked. Caching the root under its path conflated the two, so a targeted root's projection stood in for the full Markdown definition an ordinary component selection asks for — and a `<Foo />` in the selected section returned `valid` while the sections the root never selected held a definite `body-shape-invalid` failure. Reading and parsing are now separate. A path is read once, whatever views the walk needs; the full-definition view is registered per path. An untargeted root's body is the whole file, so it registers as that view and a name resolving there finds it already scanned — the self-cycle deduplication is unchanged. A targeted root registers no view: a component selecting that path parses the full definition from the bytes already read and enters the FIFO queue in discovery order like any other source. Both halves of the regression were confirmed discriminating: substituting the projection again, and bypassing the content cache so the second view re-reads.
| this.#draft(ordinal, "source-unreadable", { message: unreadable(path), cause: error }); | ||
| return undefined; | ||
| } | ||
| // Supplied or read, these are this path's bytes for the rest of the walk. |
There was a problem hiding this comment.
Redundant comment — restates what the code does.
Suggested change
| // Supplied or read, these are this path's bytes for the rest of the walk. |
| for (const violation of this.#structuralViolations(segment, context)) { | ||
| // A violation names the construct it is about and, when the check | ||
| // walked past the element it was given, the element it is about. Both | ||
| // the diagnostic and the record that points at it follow that element. |
There was a problem hiding this comment.
Redundant comment — restates what the code does.
Suggested change
| // the diagnostic and the record that points at it follow that element. |
| } | ||
|
|
||
| // Presence, not the resolved value: `value={undefined}` names the direct | ||
| // source exactly as `value={42}` does, and a whitespace child is a body. |
There was a problem hiding this comment.
Redundant comment — restates what the code does.
Suggested change
| // source exactly as `value={42}` does, and a whitespace child is a body. |
| } from "./src/inspect.ts"; | ||
| export { ComponentIncludeError } from "./src/components/candidates.ts"; | ||
| // Document validation — one supplied document read as authored program | ||
| // structure, with nothing in it executed. |
There was a problem hiding this comment.
Redundant comment — restates what the code does.
Suggested change
| // structure, with nothing in it executed. |
| this.#draft(ordinal, "source-unreadable", { message: unreadable(path), cause: error }); | ||
| return undefined; | ||
| } | ||
| // Supplied or read, these are this path's bytes for the rest of the walk. |
There was a problem hiding this comment.
Redundant comment — restates what the code does.
Suggested change
| // Supplied or read, these are this path's bytes for the rest of the walk. |
| for (const violation of this.#structuralViolations(segment, context)) { | ||
| // A violation names the construct it is about and, when the check | ||
| // walked past the element it was given, the element it is about. Both | ||
| // the diagnostic and the record that points at it follow that element. |
There was a problem hiding this comment.
Redundant comment — restates what the code does.
Suggested change
| // the diagnostic and the record that points at it follow that element. |
| // Its contract lives on the module's exports, and reading them would run | ||
| // the module's top-level code. Nothing is assumed in its place: no forms, | ||
| // no props schema, no captures, no return mode. Only the engine-owned | ||
| // checks above applied, and what is left is unknown rather than accepted. |
There was a problem hiding this comment.
Redundant comment — restates what the code does.
Suggested change
| // checks above applied, and what is left is unknown rather than accepted. |
| const source = yield* this.#loadSource(segment.name, selected.path); | ||
| if (source.state === "failed") { | ||
| // Every invocation that selected this definition points at the one | ||
| // failure the source has, and none of them invents a second. |
There was a problem hiding this comment.
Redundant comment — restates what the code does.
Suggested change
| // failure the source has, and none of them invents a second. |
| } | ||
| // Both remaining outcomes resolved, so both report an origin. A record | ||
| // reaching here without one would mean selection answered and nothing | ||
| // wrote down what it answered. |
There was a problem hiding this comment.
Redundant comment — restates what the code does.
Suggested change
| // wrote down what it answered. |
| // Every one of these is decided from what the author wrote, so the whole | ||
| // catalog lives in `structural-rules.ts` where validation reads it too. The | ||
| // first is reported and the rest of the construct does not run, which is what | ||
| // a `<Let>` whose declaration is wrong has always done. |
There was a problem hiding this comment.
Redundant comment — restates what the code does.
Suggested change
| // a `<Let>` whose declaration is wrong has always done. |
| return [yield* raise(eachError(`Prop "as" on <Each /> ${asResult.error.message}`))]; | ||
| // Decided from source alone, so the catalog is shared with validation. A | ||
| // literal `in` is checked here too; an expression is a value the document | ||
| // computes, and its answer is checked below where it arrives. |
There was a problem hiding this comment.
Redundant comment — restates what the code does.
Suggested change
| // computes, and its answer is checked below where it arrives. |
| (name) => !IF_PROPS.has(name), | ||
| ); | ||
| // Decided from source alone and shared with validation: which props were | ||
| // written, and how the body splits at its `<Else>`. |
There was a problem hiding this comment.
Redundant comment — restates what the code does.
Suggested change
| // written, and how the body splits at its `<Else>`. |
| } | ||
|
|
||
| // Presence, not the resolved value: `value={undefined}` names the direct | ||
| // source exactly as `value={42}` does, and a whitespace child is a body. |
There was a problem hiding this comment.
Redundant comment — restates what the code does.
Suggested change
| // source exactly as `value={42}` does, and a whitespace child is a body. |
PR #654: ✨ Validate a supplied document without executing it14 files, +4350 / -799 Scope🔴 PR has 5149 lines changed. Split into focused PRs. 🟡 5149 lines changed. PRs under 400 receive more thorough review. StructuralOxlint structural signals:
Slop
Oxlint slop signals:
Static AnalysisOxlint: 31 diagnostics across 4 files (13 rules) no-unused-vars (7): packages/core/src/expand.ts CorrectnessNo extraneous code patterns detected. |
`Array.prototype.toSorted` is ES2023 and `tsconfig.node.json` targets ES2022, so every `test-node` shard failed at its typecheck before running a test. Deno's own lib is newer, which is why `deno check` and the whole local suite were green. Both call sites already build a fresh array, so sorting in place mutates nothing anyone else holds. That reinstates an oxlint `no-array-sort` warning — the shape the rest of the repository is in, and a warning rather than the error this was. Verified with the check CI runs: `tsc --project tsconfig.node.json --noEmit`, plus the validation suite under all three runtimes.
| this.#draft(ordinal, "source-unreadable", { message: unreadable(path), cause: error }); | ||
| return undefined; | ||
| } | ||
| // Supplied or read, these are this path's bytes for the rest of the walk. |
There was a problem hiding this comment.
Redundant comment — restates what the code does.
Suggested change
| // Supplied or read, these are this path's bytes for the rest of the walk. |
| for (const violation of this.#structuralViolations(segment, context)) { | ||
| // A violation names the construct it is about and, when the check | ||
| // walked past the element it was given, the element it is about. Both | ||
| // the diagnostic and the record that points at it follow that element. |
There was a problem hiding this comment.
Redundant comment — restates what the code does.
Suggested change
| // the diagnostic and the record that points at it follow that element. |
| } | ||
|
|
||
| // Engine-owned and independent of any contract: `as` names a binding, and | ||
| // whether the author wrote a name at all is decided from the syntax. |
There was a problem hiding this comment.
Redundant comment — restates what the code does.
Suggested change
| // whether the author wrote a name at all is decided from the syntax. |
| const source = yield* this.#loadSource(segment.name, selected.path); | ||
| if (source.state === "failed") { | ||
| // Every invocation that selected this definition points at the one | ||
| // failure the source has, and none of them invents a second. |
There was a problem hiding this comment.
Redundant comment — restates what the code does.
Suggested change
| // failure the source has, and none of them invents a second. |
| return; | ||
| } | ||
| // A definition whose own body contract is broken is broken for every | ||
| // caller, at the position its own source states. |
There was a problem hiding this comment.
Redundant comment — restates what the code does.
Suggested change
| // caller, at the position its own source states. |
| } | ||
| // Both remaining outcomes resolved, so both report an origin. A record | ||
| // reaching here without one would mean selection answered and nothing | ||
| // wrote down what it answered. |
There was a problem hiding this comment.
Redundant comment — restates what the code does.
Suggested change
| // wrote down what it answered. |
| // Every one of these is decided from what the author wrote, so the whole | ||
| // catalog lives in `structural-rules.ts` where validation reads it too. The | ||
| // first is reported and the rest of the construct does not run, which is what | ||
| // a `<Let>` whose declaration is wrong has always done. |
There was a problem hiding this comment.
Redundant comment — restates what the code does.
Suggested change
| // a `<Let>` whose declaration is wrong has always done. |
| return [yield* raise(eachError(`Prop "as" on <Each /> ${asResult.error.message}`))]; | ||
| // Decided from source alone, so the catalog is shared with validation. A | ||
| // literal `in` is checked here too; an expression is a value the document | ||
| // computes, and its answer is checked below where it arrives. |
There was a problem hiding this comment.
Redundant comment — restates what the code does.
Suggested change
| // computes, and its answer is checked below where it arrives. |
| (name) => !IF_PROPS.has(name), | ||
| ); | ||
| // Decided from source alone and shared with validation: which props were | ||
| // written, and how the body splits at its `<Else>`. |
There was a problem hiding this comment.
Redundant comment — restates what the code does.
Suggested change
| // written, and how the body splits at its `<Else>`. |
| } | ||
|
|
||
| // Presence, not the resolved value: `value={undefined}` names the direct | ||
| // source exactly as `value={42}` does, and a whitespace child is a body. |
There was a problem hiding this comment.
Redundant comment — restates what the code does.
Suggested change
| // source exactly as `value={42}` does, and a whitespace child is a body. |
Validation still reimplemented four authored decisions expansion owns: an ordinary component's `as` form and binding name, the requirement that a component declaring `returns` is invoked with `as`, and the `<Answers>` and `<Answer>` prop, delegate, body-shape, template-form and required-value rules. Sharing the sentences was not sharing the decisions. New `invocation-rules.ts` owns the ordinary-component rules, and the `<Answers>` and `<Answer>` rules in `structural-rules.ts` split into the phases their callers ask in. Extraction is phase-appropriate rather than phase-bound: whether `as` was written as an expression is answered from the scanned element, while what a binding name may be is answered from whatever value each caller has — expansion passes the prop it resolved, validation passes the literal the author wrote. Expansion refuses at the first of them, in the phase it always did, with byte-identical prose; validation aggregates every independent one. The regressions run each document twice — once through `execute()`, once through `validateDocument()` — and require one sentence from both. Each was confirmed discriminating by giving expansion back a private copy of the rule. The source/view contract is also corrected everywhere it was stated. Bytes are read once per path; each semantic view of them is parsed and scanned once. An untargeted root is that path's full-definition view as well. A targeted root's projection and a component-selected full definition are two distinct views of one path. The claim that a source identity is parsed or scanned once is gone from architecture.md, the spec, the public comments and the tests.
| for (const violation of this.#structuralViolations(segment, context)) { | ||
| // A violation names the construct it is about and, when the check | ||
| // walked past the element it was given, the element it is about. Both | ||
| // the diagnostic and the record that points at it follow that element. |
There was a problem hiding this comment.
Redundant comment — restates what the code does.
Suggested change
| // the diagnostic and the record that points at it follow that element. |
| } | ||
|
|
||
| // The same rule expansion applies, asked about the capture this site names. | ||
| // A site whose `as` was already refused is not asked again. |
There was a problem hiding this comment.
Redundant comment — restates what the code does.
Suggested change
| // A site whose `as` was already refused is not asked again. |
| // Its contract lives on the module's exports, and reading them would run | ||
| // the module's top-level code. Nothing is assumed in its place: no forms, | ||
| // no props schema, no captures, no return mode. Only the engine-owned | ||
| // checks above applied, and what is left is unknown rather than accepted. |
There was a problem hiding this comment.
Redundant comment — restates what the code does.
Suggested change
| // checks above applied, and what is left is unknown rather than accepted. |
| // Every one of these is decided from source alone, so every one of them | ||
| // is reported. Expansion stops at the first and expands neither branch, | ||
| // which is its own established behavior over the same shared facts — | ||
| // an author reading a validation result is owed all of them. |
There was a problem hiding this comment.
Redundant comment — restates what the code does.
Suggested change
| // an author reading a validation result is owed all of them. |
| default: | ||
| // `<Return>` is the remaining case, and its whole contract belongs to | ||
| // the body that declares — or fails to declare — `returns`, which the | ||
| // source's own facts already stated. |
There was a problem hiding this comment.
Redundant comment — restates what the code does.
Suggested change
| // source's own facts already stated. |
| } | ||
| // Both remaining outcomes resolved, so both report an origin. A record | ||
| // reaching here without one would mean selection answered and nothing | ||
| // wrote down what it answered. |
There was a problem hiding this comment.
Redundant comment — restates what the code does.
Suggested change
| // wrote down what it answered. |
| // Every one of these is decided from what the author wrote, so the whole | ||
| // catalog lives in `structural-rules.ts` where validation reads it too. The | ||
| // first is reported and the rest of the construct does not run, which is what | ||
| // a `<Let>` whose declaration is wrong has always done. |
There was a problem hiding this comment.
Redundant comment — restates what the code does.
Suggested change
| // a `<Let>` whose declaration is wrong has always done. |
| } | ||
|
|
||
| // Presence, not the resolved value: `value={undefined}` names the direct | ||
| // source exactly as `value={42}` does, and a whitespace child is a body. |
There was a problem hiding this comment.
Redundant comment — restates what the code does.
Suggested change
| // source exactly as `value={42}` does, and a whitespace child is a body. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #653.
Why
xmd prompt(#260) has to reject a generated program with a missing componentor an invalid invocation before asking a person to approve it. Today those
failures surface only when execution begins. Core's inspection reads the root
and validates its metadata, but resolves nothing in the body.
This adds one reusable core boundary so the CLI, and any other host, reads one
versioned answer instead of reproducing expansion's rules.
What changes
Before:
inspectDocument()answered for the root's declaration and stopped there.After:
validateDocument()reads a supplied root and the Markdown source closureordinary component selection discovers, and returns deterministic version-1
data: one closed code per diagnostic, the normalized Ajv issues execution
already produces, and one record per authored invocation reporting
valid,invalid, ornot-statically-checkable.How it works
Traversal follows source, not execution reachability. An invocation inside
<If>,<Loop>or<Each>is authored program structure and is checked likeany other; no branch is evaluated, and no projection is predicted. Discovered
definitions enter one FIFO queue in first-invocation order.
Reading and parsing are separate concerns. A path's bytes are read once,
whatever views the walk needs. The full-definition view — what an ordinary
component selection asks for — is cached per path. An untargeted root's body is
the whole file, so it registers as that view and a name resolving there finds it
already scanned; that is how a root invoking itself terminates. A targeted
root registers no view: its body is one section and stands in for nothing, so a
component selecting the same path parses the full definition from the bytes
already read.
Full props-schema validation runs only when every schema-visible prop is static.
A missing required key is still definite beside a dynamic prop, because presence
needs no value; every other schema conclusion waits for the one full-schema
call. No partial JSON Schema solving.
Review guide
Start with:
specs/executable-mdx-spec.md§5.3 "Validating a supplieddocument:
validateDocument()" — the whole contract in one place.Then review:
packages/core/src/document-validation.ts— the public types, thenValidationState.run,#loadSource,#visit,#checkContract,finish.packages/core/src/body-structure.tsandstructural-rules.ts— the rulesextracted out of
expand.tsso both callers read one catalog.packages/core/src/expand.ts— the same rules, now consumed rather thanowned. Messages are byte-identical.
packages/core/src/definition.ts/frontmatter.ts— phased parsing.Look carefully at:
finish()— the sort is the public ordering contract: source ordinal,unpositioned before positioned, authored offset, closed code order, then
discovery as the non-public tie breaker.
#deferred— a fact one construct discovers about a descendant (a misplaced<Else>found through its<If>) reaches both records.document-validation.ts. It is the enforceableno-execution seam: no
execute.ts, noexpandComponent, noimportComponent, no journal, command, eval, Agent, Elicitation or provider.What must stay true
list and by taking no stream or installation; checked by
DV13, whichinstalls a refusal at every
API.Fswrite,API.Process,API.FetchandAPI.Env.compile, an identity factory that throws, and a registeredcomponent whose body throws.
duplication —
expand.tscalls the same functions; checked byDV14and bythe existing structural suites, which are unchanged.
DV5.over tokens rather than assigning indexes during the walk; checked by
DV11.How to verify it
DV1/DV2prove the two acceptance cases from the issue and fail ifresolution or schema validation regresses.
DV5proves root-then-FIFO order, one read per path, cycle termination, andthat a targeted root's projection never answers for the full definition a
component selected. It fails if either cache is keyed wrongly.
DV7/DV8prove that opacity never swallows a definite failure and that nopartial schema conclusion is drawn across a mixed static/dynamic object.
DV9proves a repository.tscomponent is never imported and never givenan invented contract.
DV13fails if any execution, provider, journal, command, elicitation, agentor document-authored write occurs.
Scope
Included
validateDocument()and its version-1 types, exported from@executablemd/core.body-structure.ts,structural-rules.ts,components/declared-registry.ts.definition.tsandfrontmatter.ts.architecture.mdandspecs/executable-mdx-spec.md.Intentionally unchanged
xmd promptCLI, ACP generation, repair, approval or execute flow — thoseare Add
xmd prompt: generate an executable markdown program from a request, approve it, run it #260's.exactly the meaning execution's scanner gives it.
violation; validation reads all of that construct's shared facts, because an
author reading a result is owed all of them at once.
component-ambiguousis deliberately dormant: main's precedence alwaysselects exactly one component, so the code and its rank exist for a selector
that can one day answer otherwise. Ambiguity is not manufactured from
precedence tiers, include order, or a refused host declaration.
New abstractions
body-structure.tsexists because the<Output>/<Return>body contract hastwo consumers with different needs: expansion wants the one aggregate printed
error it always produced (
renderBodyStructure()), validation wants eachviolation separately under its own code. One walk, two renderings.
structural-rules.tsexists for the same reason across every construct, andis what keeps validation from owning a second rule catalog.
components/declared-registry.tsexists becauseinspectSyntax()andvalidateDocument()must resolve declared names identically; it was privateto
inspect.ts.DefinitionPhaseError/FrontmatterPhaseErrorexist so a failure's phase isknown without matching its message. The raising entry points re-throw
original, so execution and inspection prose are unchanged.Risks and limitations
invocation-form-invalidcarries engine prose rather than the component's ownrefuse()sentence, because callingrefusewould execute component code.The declared forms are the no-execution answer.
reports the first. That is deliberate and documented, but it means the two
surfaces show different counts for one malformed construct.
expand.tsbroadly (−699 lines). Messages arebyte-identical and the existing structural suites are unchanged, which is the
evidence for that.
Scope confirmation