feat: make the elision verdict inspectable and provable per app - #1312
feat: make the elision verdict inspectable and provable per app#1312vivek7405 wants to merge 23 commits into
Conversation
…eport The elision report told an app which pages ship when they could have been elided, which is the benign over-ship direction. Nothing reported the other direction: which components were DROPPED, and on what evidence. That is the direction where a wrong verdict silently loses interactivity in production. analyzeElision now records WHY each shipping component ships, alongside every mustShip write, and returns it as componentVerdicts. analyzeAppElision projects the whole verdict (components, route modules, orphans, summary) into one sorted, app-relative, JSON-serializable object. Nothing is re-analysed: the data was already in memory and was being discarded. maskJsSet moves into a shared leaf so the framework's own differential guard and the app-facing one cannot drift apart on what the JS-loaded set even is.
…tial An app could not ask what elision dropped from its pages, and the SSR differential that proves elision changes nothing was a framework test an app could not run. `webjs elision` prints the verdict per module with the evidence behind every ship, --json emits it for a tool, and --verify boots two handlers with WEBJS_ELIDE flipped and diffs the masked SSR bytes across the app's own static route corpus. It exits non-zero on a divergence and on a corpus where nothing was compared, since a vacuous pass is the failure mode a verification command must not have. The success output states its own boundary: it proves the served bytes did not change, not post-hydration behaviour. Also corrects a claim four surfaces carried. `static interactive = true` does not rescue a component whose own registration tag is computed: the scanner requires a literal tag, so that component gets no verdict for the override to attach to.
…tests `list_elision` returns `analyzeAppElision` verbatim: that function already owns the app-level contract, so unlike list_routes there is no projector leaf and nothing to keep in sync. list_components deliberately stays a cheap lexical inventory; growing an elided flag onto it would make it build a module graph. The dev server prints one summary line per warm analysis, re-emitted after each fs.watch rebuild. Deliberately NOT a browser push: an inert route ships zero application JS, and the most useful manual check an author has is opening the network tab on that route and seeing nothing, which a dev-only boot script would corrupt on exactly the pages this feature exists to prove. The contract tests pin both documented residuals as MEASURED, which was prose until now: a computed whenDefined observer leaves its badge elided, an external stylesheet's :defined rule does too, and `static interactive = true` rescues both. A computed Class.register(tag) is recorded as a different and worse shape that the override does NOT rescue, since the scanner never sees that component. examples/blog gains forced-badge, display-only in every respect apart from the override, so the e2e probe can assert the browser really keeps its module.
…override The CLI tests drive every --verify exit path, including the two that matter most and are easy to get wrong: a corpus where nothing could be compared exits 1 rather than reporting a vacuous pass, and a real divergence is produced for real rather than stubbed (a component whose SSR output reads WEBJS_ELIDE is exactly the class of bug the differential exists to catch). The doctor tests keep the three carrier cases unchanged, which is the regression guard for rewiring that check onto the shared report, and pin the new check to pass-except-orphans plus its gate contract. The e2e probe is the first coverage anywhere above the analyser unit that static interactive = true actually keeps a module on the wire; build-stamp on the same run is the negative control.
Elision was described across six docs pages with no page owning it, and the opt-out appeared on none of them, which is exactly how the switch went undocumented. A capability with its own command, config key, env override, doctor check, and MCP tool has outgrown being a paragraph inside five topics. The skill gets the agent-facing half: how to read the evidence values, what to do with each verdict, and the two-run recipe for the behaviour half that the byte differential cannot see. Dogfooding the new doctor check turned up a real defect in the orphan scan. It read raw source, so every WebComponent subclass written inside an html template as a CODE SAMPLE counted as an unregistered component, and the repo own website reported 17 false orphans. That was tolerable as dev-console noise and is not tolerable as a doctor warning, so the scan now redacts strings and templates exactly as extractComponents already did. Both dogfood apps go from 17 and 0 false orphans to none, and a real orphan in the same tree still fires.
b770ca1 to
ef13733
Compare
The scaffold gallery-coverage gate requires every server export to be either demoed or exempted; maskJsSet and staticPageRoutes are verify-command plumbing, so they are exempt with the reason they exist at all. The elision-report fixtures move to template literals. The scanner-fuzz corpus sweep reads every file under test/elision and compares its lexical class window against a real AST, and redaction blanks a template body while keeping a plain string verbatim, so a fixture class in a plain string skews that differential.
|
Design rationale: why the report carries evidence, and why an elided row carries none The verdict data was already in memory and was being thrown away. First write wins, matching the analyser's first-match convention everywhere else. A component forced by several rules reports the rule that reached it first, which is a truthful account of why the module is on the wire even when it is not the only one. An elided row deliberately reports The one hole in that design is a future rule that adds to |
|
Decision: the orphan scan now redacts, because dogfooding turned a tolerable warning into a wrong one
I found it by running doctor over our own site: the website reported 17 orphans, every one of them a documentation code sample. Shipping a check that cries wolf on our own dogfood app is the exact failure the check's pass-except-orphans design was trying to avoid, so I fixed the scan rather than soften the check. The fix is the one This is strictly a false-positive fix, not a verdict change. Elision decides nothing differently. |
|
Measured: what Four surfaces said the override covers "a dynamically-computed tag string". I drove the analyser against real on-disk fixtures to check, and that wording conflated two different shapes with opposite outcomes. What it DOES rescue is the OBSERVER's tag, not the registration's. A shipping module writing What it does NOT rescue is a component whose OWN registration tag is computed. So the fix for that case is documentary plus visibility, not code. Invariant 3 already requires a literal tag, the docs now say so where they used to promise the override, and the shape surfaces as an |
…ile column The help renderer hardcoded the heading Config:, which is what the doctor severity gate is and what its help test pins. The elision block is a caveat about what --verify proves, not configuration, so an entry now names its own heading and doctor keeps the default. The verdict table capped every column at the same width, which is right for the tag column (a file registering five tags is the rare case) and wrong for the file column: on a freshly scaffolded app almost every module path is longer than the cap, so almost every row overflowed instead of one outlier.
vivek7405
left a comment
vivek7405
left a comment
There was a problem hiding this comment.
Went back over this one properly and it is not ready. The shape of the change is right, and the analyser side holds up: recording the evidence at each mustShip write rather than deriving it later is the correct call, and the verdict is provably unchanged against main on both dogfood apps.
The problem is that two of the three things this PR sells can currently lie. --verify never forces elision ON, so on an app with elide: false it renders the same thing twice and reports the routes identical with elision on vs off, which is a false statement about a run where elision was never on. That is the exact vacuous-pass class the zero-route exit code was added to prevent, and it is worse than the zero-route case because it prints a confident pass. The e2e probe has the same flavour of problem: its declared negative control asserts a module that is not on that page at all, so it cannot fail and the test is really a single positive assertion wearing a control.
The other two are smaller but both mislead a reader: the new docs page's meta description tells people to run a command that does not exist, and list_elision is missing from six places that enumerate the MCP tools, including the published README and the CLI's own help text.
Worth saying what is good here, because the review reads harshly: the residual contract tests are the most valuable part of the diff. Pinning what static interactive = true does and does not rescue, measured rather than asserted from the docs, is what stops the next person repeating the mistake this PR is correcting.
Three things this PR sells could lie. --verify deleted WEBJS_ELIDE for its ON side, which only falls back to webjs.elide, so on an app that opts out BOTH handlers ran with elision off and the command reported the routes identical with elision on vs off and exited 0. That is a confident pass on a run where elision was never on, worse than the zero-route vacuity the exit code already guarded. The ON side is now forced on through the override, which wins over the config key, and the run reports how many modules elision actually dropped so a trivially-true pass is visible too. The e2e probe asserted build-stamp was not downloaded on /observed as its negative control, but build-stamp is only on /, so that assertion was true whether or not elision worked and the test was one positive assertion wearing a control. /observed now renders it, and WEBJS_ELIDE=0 reds the control. The new docs page meta description named two commands that do not exist, and list_elision was missing from six surfaces that enumerate the MCP tools, including the published README and the CLI help text.
vivek7405
left a comment
vivek7405
left a comment
There was a problem hiding this comment.
Read the fix commit again with fresh eyes, tracing what each change touches across the rest of the branch. The two substantive fixes hold up: forcing the ON side really does make an opted-out app compare a real pair, and the e2e control now reds under WEBJS_ELIDE=0.
What it missed is its own blast radius on the docs. The commit set out to add list_elision everywhere the MCP tools are enumerated and got six of eight: the CLI-reference block in AGENTS.md (the same file it edited, different section) and the start-work skill still list the old four. Separately, examples/blog/AGENTS.md owns an "Elision fixtures" section that enumerates exactly these files, and it is now wrong twice over: build-stamp is documented as rendered on / only, and forced-badge, a fixture this PR adds specifically to be e2e-pinned, is not listed at all. That section is the thing a future reader consults before touching a fixture.
Two smaller ones on the new docs page: the meta description carries literal backticks, which land verbatim in the <meta> tag and the llms.txt entry rather than rendering as code, and the paragraph explaining that --verify forces the ON side was inserted after the two-run e2e code block, so it reads as describing that recipe instead of --verify.
On the exit code for a corpus where elision dropped nothing: I am leaving that at 0 deliberately, reasoning in the thread.
The previous commit added list_elision to six surfaces and missed two: the CLI-reference block in AGENTS.md, which is a different section of the same file it edited, and the start-work skill. examples/blog/AGENTS.md owns the section that enumerates the elision fixtures, which is what a future reader consults before touching one, and it was wrong twice: build-stamp is now the negative control on /observed as well as /, and forced-badge was not listed at all. It also now records the doc-comment discipline those fixtures depend on, since prose naming a tag or a whenDefined call would register as a real signal and make the tests pass vacuously. The new docs page carried literal backticks into its meta description, which is emitted verbatim into the meta tag and the llms.txt entry, and the paragraph explaining that --verify forces the ON side sat after the two-run e2e block, so it read as describing that recipe instead. The zero-drop message said the run proves nothing about elision while exiting 0, which reads as a self-contradiction. Exit 0 is right there (the question was answered, and a corpus with nothing elidable is a legitimate app), so the message now says what actually happened instead of disowning the run.
|
Resolution: the two path-level findings from this round Two of this round's findings had no line in the diff to hang off, so recording them here. The
|
vivek7405
left a comment
vivek7405
left a comment
There was a problem hiding this comment.
This round caught me codifying something that is not true. The paragraph I added to the blog's fixture inventory says the analyser reads tags and whenDefined calls out of COMMENTS, so the fixtures avoid writing them. That was true once and #179 closed it: maskComments runs before every signal scan. I drove it to be sure, with a display-only badge whose doc comment names its own tag in angle brackets AND calls whenDefined on it, and the badge is still elided. So the discipline I was promoting from an in-file aside into a normative rule guards nothing.
Worse than being wrong, it was wrong in the one place a future reader goes before touching a fixture, and it would have taught them to write around a mechanism that does not exist. Removing it rather than correcting its count, which also settles the second finding.
The third is a real seam I left open: --verify forces the override on, the plain report path reads the config, and the zero-drop message sends the author to a bare webjs elision. On the opted-out app that forcing exists FOR, that command answers elision is disabled and no verdict at all, so the hint dead-ends exactly where it is most needed.
The fixture inventory I added said the analyser reads tags and whenDefined calls out of COMMENTS, so the fixtures deliberately avoid that prose. That was true once; #179 closed it by masking comments before every signal scan. Driving a display-only badge whose doc comment names its own tag in angle brackets AND calls whenDefined on it leaves the badge elided, so the discipline guards nothing. Removing it rather than correcting its count, since a rule that protects against a closed failure mode is worse in the fixture inventory than in the aside it came from. forced-badge.ts carried the same claim and is new here, so it goes too. The zero-drop hint also dead-ended: --verify forces the override on while the plain report path reads the config, so on the opted-out app that forcing exists for, a bare `webjs elision` answers "elision is disabled" and no verdict. The hint now carries the same override the run used.
vivek7405
left a comment
vivek7405
left a comment
There was a problem hiding this comment.
Fair, and it is a mess of my own making. Last round I established the comment-scanning claim was false and deleted it from the fixture INDEX and from the one fixture this PR adds, then left it stated in the two original fixtures and in an analyze.test.js rationale. So the repo ended up asserting the rule in three places and denying it in two, which is a worse state than before I touched it: a future editor of observed-badge.ts reads the rule as live and writes around a mechanism that has not existed since #179.
Corrected all three. The two fixture comments now say the awkward prose is a habit from before #179 rather than a rule, so the next person knows they can write normally without wondering what breaks. The analyze.test.js one needed a distinction rather than a deletion: the claim IS true of analyzeComponentSource, which is the leaf that test calls directly and which does not mask, and false of the pipeline, which masks before calling it. The comment now says which is which and points at the test that pins the other end.
These three files are not otherwise in this PR, so folding them in is a judgement call. I am making it because the inconsistency is a direct product of this branch, and leaving it would mean shipping a repo that contradicts itself about its own analyser.
…tures The previous commit established the claim was false and deleted it from the fixture index and from the one fixture this branch adds, which left it stated in the two original fixtures and in an analyze.test.js rationale. Three places asserting the rule and two denying it is worse than the state I started from. The two fixture comments now say the awkward prose is a habit from before #179 rather than a rule, so the next editor knows they can write normally. The test rationale needed a distinction rather than a deletion: it is true of analyzeComponentSource, the leaf that test calls directly and which does not mask, and false of the pipeline, which masks before calling it.
The post enumerates what the read-only MCP server exposes, and the count plus the list went stale the moment list_elision landed. The website publishes the blog directory directly, so a reader lands on a page that undercounts the tool set of the thing it is describing.
vivek7405
left a comment
vivek7405
left a comment
There was a problem hiding this comment.
Third time on the same claim, and this round is the one that actually closed it, because it stopped trusting my summary of the mechanism and went and read it.
I said two fixtures carried the stale comment-scanning line. It was three: ssr-derived-badge.ts has it too, in the same directory a future editor reads together. Fixed, and I grepped the whole repo afterwards rather than counting from memory again.
The other two are wrong text I wrote in this branch while correcting the first mistake, which is the part worth noting. My replacement rationale in analyze.test.js said the leaf does not mask comments, so a stray marker there only over-detects. I drove it: a lifecycle hook in a class-body comment is NOT detected, a signal import in a comment is NOT detected, and only the four template-marker regexes that run on raw src see one. So the leaf masks for most signals and my sentence was over-broad in exactly the direction the original was. And the component-scanner.js comment claimed redactToPlaceholders preserves positions; it does not, a 44-character line comes back 22, and position preservation is redactStringsAndTemplates's property. Harmless today because an orphan is reported by class and file, so I said so and named the function to reach for if that ever changes.
ssr-derived-badge.ts was the third fixture carrying the closed-since-#179 comment-scanning claim, not the second. Grepped the repo this time instead of counting from memory. The analyze.test.js rationale I wrote to replace it was over-broad in the same direction. The leaf DOES mask comments for most signals: a lifecycle hook in a class-body comment is not detected, nor is a signal import in a comment. Only the four template-marker regexes run on raw source, which is what that test actually covers. The component-scanner.js comment claimed redactToPlaceholders preserves positions. It does not, since a placeholder is a different length than the body it replaces; that property belongs to redactStringsAndTemplates. No consequence today because an orphan is reported by class name and file, so the comment now says that and names the function to use if the scan ever reports a position.
vivek7405
left a comment
vivek7405
left a comment
There was a problem hiding this comment.
The second finding here is the one that matters, and it is a user-facing misdiagnosis rather than a comment nit. findOrphanComponents reports TWO shapes under one name: a class whose registration tag is computed, and a class with no registration call at all. The second is the ORIGINAL case the function was written for and the one the dev server has always warned about. I routed that function into a doctor warning and into webjs elision while describing only the computed-tag shape, so someone who simply forgot to register a class would have been told their tag is computed, and the CLI's fix line offered only "pass a literal tag", which is not the fix for a class that has no registration call. Both messages now name both shapes, and there is a doctor test for the forgot-to-register one.
The first is my own forward-looking advice from last round, and it was actively dangerous. I pointed a future editor at redactStringsAndTemplates for position-preserving redaction, but the default form keeps plain-string bodies AND single-line untagged templates verbatim, so following that advice would reintroduce the exact false orphan this PR fixed. Checked it: the class name matches again under the bare call and not under (src, true). The comment now names the argument.
This is the fifth delta round in a row to produce fixes, so I am stopping the chain here rather than spawning a sixth.
findOrphanComponents reports a class with a computed registration tag AND a class with no registration call at all. The second is the original case it was written for and what the dev server has always warned about. Routing it into a doctor warning and into the elision report while describing only the computed tag meant someone who forgot to register a class was told their tag is computed, and the CLI offered a fix that does not apply. Both messages now name both shapes, with a doctor test for the forgot-to-register one. The redaction comment I added last round pointed a future editor at redactStringsAndTemplates for position-preserving redaction. Its default form keeps plain-string bodies and single-line untagged templates verbatim, so following that advice would reintroduce the false orphan this branch fixed. It now names the blank-strings argument that makes it safe.
vivek7405
left a comment
vivek7405
left a comment
There was a problem hiding this comment.
Caught the obvious hole in last round's fix. I wrote an invariant saying any message about findOrphanComponents must cover BOTH shapes it reports, fixed the doctor and the CLI, and left the third consumer, the dev-server warning, describing only one. So webjs dev told an author with a computed tag that they have no registration call, which is the wrong diagnosis and a fix that does not apply, while the docs I wrote in the same commit claimed dev warns about exactly that shape. It also recommended customElements.define where the convention is Class.register. Both fixed, and the message now names both shapes like its two siblings.
The type declaration for the same function said it finds classes that no page or component IMPORTS. It computes nothing of the kind, and never has: it reports a class its own file never registers with a literal tag. That is the published type surface for the symbol this whole round is about, in a file this PR already edits, so it is fixed here.
Third, packages/cli/AGENTS.md had no row for the command this PR adds and its doctor row predated the second elision check. Root AGENTS.md and the server package doc both got those two facts; the package that actually owns the command did not.
Last commit wrote an invariant saying any message about findOrphanComponents must cover both shapes it reports, fixed the doctor and the CLI, and left the third consumer alone. So webjs dev told an author with a computed tag that they have no registration call, a wrong diagnosis with a fix that does not apply, while the docs from that same commit claimed dev warns about that shape. It also recommended customElements.define where the convention is Class.register. The published type declaration for that function claimed it finds classes no page or component IMPORTS. It computes nothing of the kind and never has. packages/cli/AGENTS.md had no row for the command this branch adds, and its doctor row predated the second elision check.
vivek7405
left a comment
vivek7405
left a comment
There was a problem hiding this comment.
The fourth finding here is the best one anyone has raised on this PR, because it is about the framework rather than about my prose. Every surface I wrote says an orphan means the element never upgrades. That is unconditionally true for a class with no registration call, and NOT true for the computed-tag shape: Badge.register(TAG) is ordinary code that runs whenever the module reaches the browser. I built the case and confirmed it, a page that ships whole for its own reason emits its module AND keeps the import, so the element upgrades perfectly well. What is actually lost either way is the verdict, the tag-to-module registry entry, and the preload hint, plus the upgrade itself only when the importer is elided or inert. Every surface now says that, and a new contract test pins the shipping-importer case next to the inert one that the old claim was generalised from.
The other three are the same sweep problem I keep having. The function's OWN JSDoc still described one shape and named only customElements.define, so the in-source doc contradicted the .d.ts I fixed beside it, and the module header still called customElements.define the convention when the scanner matches Class.register first. That JSDoc also claimed it matches subclasses; it does not, and I checked (a class extending a component subclass is not reported).
Third, I rewrote a user-facing diagnostic with no test, which is the workflow's own rule. There is one now, and it goes red on the old wording.
Every surface I wrote said an orphan means the element never upgrades. That holds for a class with no registration call. It does not hold for a computed tag: Badge.register(TAG) is ordinary code that runs whenever the module reaches the browser. Built the case to check, and a page that ships whole for its own reason emits its module and keeps the import, so the element upgrades. What is actually lost either way is the elision verdict, the tag-to-module registry entry, and the preload hint; the upgrade is lost only when the importer is elided or inert. Every surface now says that, and a contract test pins the shipping-importer case beside the inert one the old claim was generalised from. The scanner is self-consistent again too. Its function JSDoc described one shape and named only customElements.define, contradicting the .d.ts fixed beside it, and claimed it matches subclasses, which it does not. The module header still called customElements.define the convention when the scanner matches Class.register first. The dev warning rewritten last commit shipped with no test. It has one, and it goes red on the old wording.
vivek7405
left a comment
vivek7405
left a comment
There was a problem hiding this comment.
All five are one defect: I corrected the claim paragraph by paragraph and left the sentences either side of each one saying the opposite. Two of them literally contradict text I wrote in the SAME commit, one ends the very sentence I rewrote with "both fail the same way", and the MCP tool description, which is the agent-facing surface, still carried the retracted claim verbatim.
The cause is that I have been patching whatever the reviewer pointed at instead of sweeping the claim. So this time I grepped every phrasing of it across the repo first, listed every hit, decided each one, and then re-grepped to prove none survives. That check is in the commit: what is left of "never upgrades" is only the conditional form, and "fail the same way" / "fail identically" / "dropped silently" no longer appear anywhere near an orphan.
Worth separating two claims that I had been treating as one, since that is what made the sweep sloppy. "Gets no elision verdict" is TRUE for both shapes and was never retracted, because the class is not in the component set at all. Only "never upgrades" and "its module is dropped" were over-broad. The surfaces now keep the first and qualify the second.
Correcting the never-upgrades claim paragraph by paragraph left the sentences either side of each one saying the opposite. Two contradicted text written in the same commit, one ended the very sentence that was rewritten, and the MCP tool description still carried the claim verbatim. This pass greps every phrasing across the repo, decides each hit, and re-greps to prove none survives: what is left of never-upgrades is only the conditional form, and fail-the-same-way / fail-identically / dropped-silently no longer appear near an orphan. Separating two claims that had been conflated, which is what made the earlier passes sloppy. Gets no elision verdict is TRUE for both shapes and was never retracted, since the class is not in the component set at all. Only never-upgrades and its-module-is-dropped were over-broad.
vivek7405
left a comment
vivek7405
left a comment
There was a problem hiding this comment.
The third finding is the one that matters and it is a correction to my correction. When I fixed the never-upgrades claim I wrote that the element still upgrades unless its importer is "elided or inert". That enumeration is missing IMPORT-ONLY, and import-only is the ordinary case: any page mixing one interactive component with a display-only orphan. I ran it, and the boot emits only the real component, so the page module is dropped, its import of the orphan goes with it, and register(TAG) never runs.
So four surfaces were telling an author their element upgrades in precisely the situation they are most likely to be in. The root cause is that I stated the rule as a list of losing verdicts, and a list can always be missing one. It is now stated positively, the element upgrades only when its importer ships WHOLE, which cannot silently omit a case. There is a contract test for the import-only shape next to the ships-whole and inert ones.
The other two are more of the same leftover: the doctor MESSAGE still opened with "are dropped with NO elision verdict" even though the commit removed that wording from the JSDoc above it and the CLI heading beside it, and packages/cli/AGENTS.md still carried it too. I re-grepped for both phrasings afterwards and neither survives anywhere.
Correcting the never-upgrades claim, I wrote that the element still upgrades unless its importer is elided or inert. That enumeration misses IMPORT-ONLY, which is the shape an author actually hits: any page mixing one interactive component with a display-only orphan. Ran it, and the boot emits only the real component, so the page module is dropped, its import of the orphan goes with it, and register(TAG) never runs. Four surfaces were telling an author their element upgrades in exactly the situation they are most likely to be in. Stating the rule as a list of losing verdicts is what allowed a case to go missing, so it is now positive: the element upgrades only when its importer ships WHOLE. A contract test pins the import-only shape beside the ships-whole and inert ones. The doctor message and the CLI package AGENTS row still carried the retracted dropped wording that this branch removed from their neighbours.
vivek7405
left a comment
vivek7405
left a comment
There was a problem hiding this comment.
Nine rounds in a row have now found stale prose about ONE concept, and the code has not changed since the second. That is not nine unlucky misses, it is a structural problem I kept treating as a copy-editing one: the orphan semantics are re-explained in full on eight surfaces, so every correction has seven places left to contradict it, and I kept hand-editing whichever one was pointed at.
So this round changes the shape rather than the words. The full explanation now lives in exactly one place, the findOrphanComponents JSDoc, and every other surface states only the part that is ALWAYS true (no verdict, no registry entry, no preload hint) plus one qualified clause about the upgrade. There is much less prose left that can be wrong.
The first finding is the sharpest: my "import-only is the ordinary case SINCE an orphan is not in componentFiles" fused two independent facts into a false causal claim. Read literally it argues the opposite, because a page whose ONLY component-shaped import is the orphan has an empty frontier and is inert, which is what the test two files over asserts. The real reason is that a page usually renders a real component alongside the orphan, and that mix is import-only. Corrected.
Also fixed: two paragraphs still ENDED on the retracted reassurance, the dev warning implied by omission that a computed tag upgrades, the report JSDoc still said "registers fine", and my own new test reintroduced the plain-string fixture this PR banned two commits earlier, in the directory the fuzz corpus reads.
Nine review rounds running have found stale prose about one concept while the code stayed unchanged. The cause is that the orphan semantics were re-explained in full on eight surfaces, so every correction left seven places to contradict it. The full explanation now lives only in the findOrphanComponents JSDoc, and every other surface states the part that is always true (no verdict, no registry entry, no preload hint) plus one qualified clause about the upgrade. The causal claim in that JSDoc was also wrong: import-only is not the ordinary case BECAUSE an orphan is missing from componentFiles. Read literally that argues the opposite, since a page whose only component-shaped import is the orphan has an empty frontier and is inert, which is what the neighbouring test asserts. It is ordinary because a page usually renders a real component alongside the orphan. Two paragraphs still ended on the retracted reassurance, the dev warning implied by omission that a computed tag upgrades, and the report JSDoc still said registers fine. The new test also reintroduced the plain-string fixture this branch banned two commits earlier, in the directory the fuzz corpus reads.
vivek7405
left a comment
vivek7405
left a comment
There was a problem hiding this comment.
Two of three stand, one does not, and I checked the one I am rejecting rather than arguing it.
REJECTED, the plain-string fixture rule is real. The claim is that the fuzz sweep asserts only the miss direction and accepts over-matches, so the rule I committed is a false constraint. I reverted the fixture to a plain string and ran the sweep: it FAILS. The path is the internal assert.equal(extractWebComponentClassBodies(masked).length, count) inside lexerWebComponentClasses, which THROWS, and the corpus test's try/catch pushes that throw into misses, which is asserted. So a bare over-match is indeed accepted, and a plain-string class body is not an over-match, it is a count mismatch. My comment deserved the challenge though, because it said the fixture "skews the differential", which reads as exactly the accepted direction. Both comments now name the real mechanism and say it was verified by reverting. Getting a second opinion on the rejection.
The other two are right, and the second is the more embarrassing: I consolidated the orphan explanation out of seven surfaces last round and left index.d.ts re-explaining it in full, which is the copy an app consumer's editor renders. It had also drifted to the reassuring version, and its closing line "Nothing here is about import reachability" sat directly after two clauses about whether the importing module ships. It now states only what is always true and points at the canonical.
The third is my replacement causal claim carrying the SAME over-broad-universal defect as the one it replaced. "A page that renders any real component alongside the orphan IS import-only" is false when the page also does its own client work, which ships it whole, and that is precisely the case the sibling test pins as the one where the element DOES upgrade. Third time I have written a universal where a qualified claim belonged.
…rsal index.d.ts still re-explained the orphan semantics in full, which is what the previous commit removed from seven other surfaces, and it is the copy an app consumer editor renders. It had drifted to the reassuring version, and its closing line about import reachability sat directly after two clauses about whether the importing module ships. It now states what is always true and points at the canonical explanation. The replacement causal claim carried the same over-broad-universal defect as the one it replaced: a page rendering a real component alongside the orphan is import-only UNLESS it also does its own client work, which ships it whole, and that is exactly the case the sibling test pins as the one where the element does upgrade. The fixture rule was challenged as unfounded. It is real, and reverting a fixture to a plain string reds the corpus sweep, but the comments said it skews the differential, which reads as the over-match direction that sweep explicitly accepts. They now name the mechanism: the class-body count mismatch throws into the asserted miss list.
The rule was challenged as unfounded and it is not, but the comments justified it as skewing the differential, which reads as the over-match direction the corpus sweep explicitly accepts. The real path is narrower and worth writing down: a plain-string body survives redaction verbatim, so the embedded html-template backtick reaches the mask, matchClosingBrace returns -1, and the class-body extractor finds 0 bodies against 1 name-window match. That count assert throws into the asserted miss list.
|
Adjudicated: the template-literal fixture rule holds, with a sharper reason A reviewer argued the rule I added (fixtures under Both checks agree, and the second produced a better mechanism than mine. A plain-string body survives The challenge was fair even so: my comments justified the rule as "skews the differential", which reads as exactly the direction that sweep accepts. They now state the mechanism above, so the next person can tell which failure they are avoiding and why a plain string is the thing that triggers it. |
vivek7405
left a comment
vivek7405
left a comment
There was a problem hiding this comment.
Both findings land, and the second is the one I should have caught myself: my "consolidation" two rounds ago DROPPED coverage on the two user-facing surfaces rather than consolidating it. I deleted the both-shapes sentence from the skill reference and the docs page and kept the long computed-tag explanation, so a user who hits the far more common forgot-to-register warning was sent to a section headed "A computed registration tag is a different problem" that never mentions their case. The skill's own fix line said "give the class a literal registration tag", which is not the fix when there is no registration call at all. Both surfaces now name both shapes and both fixes.
The first is the over-broad universal AGAIN, in the one file I had not rewritten: the skill still said import-only is the ordinary case for "any page mixing one interactive component with a display-only orphan", which is the exact sentence I corrected in the scanner last round. That file is copied verbatim into every scaffolded app, so it was the worst place to leave it.
Rather than patch and hope, I ran the doc-surface map for this feature and then checked every surface mechanically for both shapes. That flagged two more, and one was a real gap (the root AGENTS.md CLI line described orphans without saying what one is, now fixed) while the other was my own grep being line-wrap-blind, which is worth noting because a line-based grep is how I have been "verifying" these sweeps all along. The check is normalised now.
Consolidating the orphan explanation deleted the both-shapes sentence from the skill reference and the docs page while keeping the long computed-tag section, so a user hitting the far more common forgot-to-register warning was sent to a heading about computed tags that never mentions their case. The skill fix line also said to give the class a literal tag, which is not the fix when there is no registration call. Both surfaces now name both shapes and both fixes. The skill also still carried the over-broad universal corrected in the scanner last round, and that file is copied verbatim into every scaffolded app. Checked every surface mechanically for both shapes afterwards rather than by eye. That found one more real gap, the root CLI reference describing orphans without saying what one is, and showed my line-based greps were wrap-blind, which is how these sweeps kept missing things. The check is normalised now.
vivek7405
left a comment
vivek7405
left a comment
There was a problem hiding this comment.
The third finding is a real defect in the check, not in the prose about it, and it turns out I had been documenting a false positive as "always a bug".
findOrphanComponents computed declared-and-registered PER FILE, while extractComponents matches a registration in ANY file. So a class declared in one module and registered by a sibling, which the scanner's own header calls equally supported, was reported as an orphan while registering fine, getting a tag, a verdict, a registry entry, a preload hint, and upgrading normally. I built that case and confirmed it: the class came back an orphan AND came back from scanComponents with its tag. Six surfaces then repeated "never upgrades" and "loses the verdict, registry entry, preload hint" about it, all false in that case.
Since this PR is what promotes an orphan from dev-console noise to a doctor WARNING, a false warning on a legitimate pattern is the same failure I fixed earlier for docs code samples, so I fixed the scan rather than the sentence. Registration is now cross-referenced app-wide. The deliberate trade-off is written down: the cross-reference is by class NAME, so two same-named classes where one is registered hides the other, which errs toward silence instead of toward a wrong accusation. Test covers the sibling case with a real orphan in the same tree so it cannot pass by going blind.
The other two are more universals of mine. "Exactly two shapes escape the analyser" is contradicted by three surviving statements about a string-selector consumer, which is a genuine third shape, and the progressive-enhancement page still prescribed adding an @event for a computed REGISTRATION tag, a remedy this PR's own test proves does nothing.
findOrphanComponents computed declared-and-registered per FILE while extractComponents matches a registration in any file. A class declared in one module and registered by a sibling, which the scanner header calls equally supported, was reported as an orphan while registering fine, getting a tag, a verdict, a registry entry, a preload hint, and upgrading. Confirmed by building the case: it came back an orphan and also came back from scanComponents with its tag. This branch is what promotes an orphan to a doctor warning, so a false warning on a legitimate pattern is the same failure already fixed for docs code samples. Registration is now cross-referenced app-wide. The trade-off is recorded: matching is by class name, so two same-named classes where one is registered hides the other, which errs toward silence rather than a wrong accusation. Also drops two more over-broad universals: exactly-two-shapes is contradicted by a string-selector consumer, and the progressive-enhancement page still prescribed adding an @event for a computed registration tag, which this branch own test proves does nothing.
1 participant
Closes #1308
Summary
Elision is automatic and stays automatic, but until now only the benign half of its verdict was visible.
webjs doctorcould tell an app which pages and layouts ship when they could have been elided, whose worst outcome is a few wasted kilobytes. Nothing reported the other half: which component modules were DROPPED, and on what evidence. That is the half where a wrong verdict silently costs an app its interactivity in production, and an author had no way to ask what the framework removed from their page. The framework protects itself with a differential on/off test; an app had no equivalent, so every app inherited a guarantee it could not check locally.This makes the verdict inspectable and provable per app.
What changed
webjs elisionprints the per-module verdict: every component as elided or shipped, every shipped one naming the evidence that forced it (own/observed/closure/render/import/unreadable) and the module that did the forcing, every page and layout as inert / import-only / ships-whole, and any orphan class that gets no verdict at all.--jsonemits the same object, and the new MCPlist_elisiontool returns it verbatim.webjs elision --verifyboots two request handlers withWEBJS_ELIDEflipped, renders the app's own static route corpus through both, and diffs the masked SSR bytes. It is the framework's own guard pointed at an arbitrary app. It exits non-zero on a divergence AND on a corpus where nothing could be compared, because a verification command that can pass while comparing nothing is worse than no command. Its success output states its own boundary: it proves the bytes you serve did not change, not post-hydration behaviour.maskJsSethas exactly one definition now, inpackages/server/src/elision-differential.js, imported by both the framework's differential test and the CLI, so the two guards cannot drift on what "the JS-loaded set" even means.webjs doctorgrowsELISION_COMPONENTS, which passes with the elided inventory and warns only on an orphan, the one shape that is dropped with no verdict and no escape hatch. Both elision checks share ONEanalyzeAppElisioncall, so the module graph is still built once per doctor run.webjs devprints one summary line per warm analysis, re-emitted after each rebuild. Deliberately not a browser push, for the reason in the comment on the code: an inert route ships zero application JS, and a dev-only boot script would corrupt the network-tab check on exactly the pages this exists to prove.Two corrections the measurement forced.
static interactive = truedoes not rescue a component whose own registration tag is computed (four surfaces claimed it did), and the orphan scan no longer reports documentation code samples as orphans. Both have their own comments on this PR.Test plan
Every layer, all run and green.
npm test: 3981 pass, 0 fail. New:residual-contract.test.js(both residuals + the not-a-residual case, measured),elision-report.test.js(the whole contract, which had no test at all before),elision-differential.test.js, thesigil-coverageevidence guard,test/cli/elision.test.mjs, the doctor block, the MCP drift test, the orphan code-sample case.npm run test:browser: Chromium 755, Firefox 741, WebKit 755, 0 failed on each.WEBJS_E2E=1: 92 pass, 0 fail, including the newstatic interactive = truenetwork probe.node scripts/run-bun-tests.js: 297 pass, 0 genuine fail, no new DENYLIST entry.test/bun/elision-report.mjsasserts an identical verdict undernodeandbun(both run).findOrphanComponentsreports both a computed registration tag and a class with no registration call at all, and the doctor / CLI messages name both, with a test for the forgot-to-register one.noteShipcall reds the evidence guard and the matchingevidencecase; dropping theDOCTOR_CODESentry reds the gate case; deletingstatic interactive = truefrom the fixture reds the e2e probe.origin/main's side by side overexamples/blogandwebsiteand diffed the full elidable / inert / import-only / ships-whole sets. IDENTICAL on both. This change records why a component ships; it changes nothing about whether one does./,/docs/elision,/docs/configuration,/docs/components,/docs/data-fetching,/docs/progressive-enhancement,/ui,/ui/buttonin dist mode with no broken preloads;webjs checkclean on both apps;webjs doctorexits 0 on both.Docs
website/app/docs/elision/page.tsplus its nav entry. Elision was described across six pages with no page owning it, and the opt-out appeared on none of them.website/app/docs/configuration/page.tsgains thewebjs.elide/WEBJS_ELIDEsection, which existed on no docs page..agents/skills/webjs/):references/components.mdgains the evidence table, what the override does and does not rescue, and what to do with each verdict;references/testing.mdgains the two-run recipe;SKILL.mdgains the routing row.AGENTS.md(CLI reference, the elision bullet, the async-render paragraph),packages/server/AGENTS.md(three module-map rows plus invariant 7),packages/mcp/AGENTS.md,packages/core/src/component.d.ts.server-clock.ts. The agent skill is copied from the repo-root canonical at scaffold time, so there is no second copy to edit.README.md(no headline capability change), the editor plugins (no template grammar, snippet, or language-service surface changed), marketing copy (no positioning claim changed), a version bump (no published-package release in this PR).