Commit a4fd82a
fix(sdui-parser): an html page literal of the wrong type is a compile error, not a warning (#21678)
Fixes #21671
Clause-②: no (narrowing)
## What changed
The html-tier compiler (`@objectstack/sdui-parser` `compile`) now grades
every `type-mismatch` as `error`, not only the ones whose input declares
an `enum` arm. `aggregate="count"` on an `object-metric` (repo-root
`sdui.manifest.json` declares `aggregate` as `type: "object"`) now fails
the compile, so `os build` fails on it. Before, it compiled `ok` with
one warning, the build stayed green, and the tile drew no number. The
diagnostic code and message are unchanged. No new code and no new gate.
## The mechanism, measured
The brief assumed that literals and expressions both reach `checkType`,
and that a signal would have to be passed in to tell them apart.
Measured on `93a54b87e7`, that is not how it works:
- `validateTree` (`packages/sdui-parser/src/validate.ts`) sends a value
that `isExpr` matches (the parser's deferred `$expr` marker) to
`inert-expression`, which is a `warning`. It calls `checkType` only in
the `else` branch. So **every value that reaches `checkType` is already
a literal**: a quoted attribute (a string), a bare attribute (`true`),
or a braced value that `interpretBrace` materialized in full.
- `interpretBrace` is all-or-nothing. Probed: `["a", foo]`, `{"a": foo}`
and `{a: 1, b: x.y}` each become ONE `$expr` marker; `[1,2]` and
`{function: "count"}` materialize in full. So a container never reaches
the type check with an expression inside it.
So the expression case is already separate before `checkType` runs, and
no new signal is needed. The change is the severity, plus a restated
header that names the second certain fact (a literal's coarse type) next
to the enum's closed list. A braced expression still gets the
`inert-expression` warning, and a test pins that.
**`checkMemberTypes` follows the same rule** (the brief left this to
measurement). Its members come from a container that was materialized in
full, so each member is a literal too, and a member that no declared arm
accepts is just as certain a mismatch. Its header already said "Severity
mirrors `checkType`'s rule", and that stays true. `member-type-mismatch`
is now `error`.
**Unchanged on purpose:** the single-arm `invalid-enum` diagnostic is
byte-identical, severity included (the existing pin in
`union-arm-type-mismatch.test.ts` passes untouched).
## Census (first step): every stored html page source, compiled against
the committed `sdui.manifest.json`
It was run with `compile` from `@objectstack/sdui-parser` (source), not
with grep. The page modules were imported and each `kind: 'html'`
export's `source` was compiled (`capability-map.page.ts` interpolates,
so a regex would have read a different string). Every fenced block in
`skills/**/*.md` and `content/docs/ui/*.mdx` that has a lowercase tag
was compiled too. A positive control (`aggregate="count"`) was compiled
in the same run.
| source | literal handed to a non-string input | verdict |
|---|---|---|
| `examples/app-showcase/src/ui/pages/command-center-jsx.page.ts`
(`CommandCenterJsxPage`) | none (0 diagnostics) | clean, nothing to fix
|
| `examples/app-showcase/src/ui/pages/capability-map.page.ts`
(`CapabilityMapPage`) | none (0 diagnostics) | clean, nothing to fix |
| `examples/app-showcase/src/ui/pages/start-here.page.ts`
(`StartHerePage`) | none (0 diagnostics) | clean, nothing to fix |
| `skills/objectstack-ui/rules/pages.md:154` block (its line 160 is the
`object-metric` example) | none: line 160 reads
`aggregate={{"function":"count"}}`, so PR #21667's fix is confirmed on
`origin/main` | clean, nothing to fix |
| hotcrm | no `kind: 'html'` page in this repo (both
`packages/metadata/src/__fixtures__/hotcrm-*.artifact.json` have 0) |
not applicable |
| other fenced blocks in `skills/**` and `content/docs/ui/**` |
React-tier or non-page code (each fails at `no-root` or `forbidden-tag`,
which shows they are not html-tier sources) | not applicable |
| control: `aggregate="count"` | before: `ok=true`, `[warning]
type-mismatch`; after: `ok=false`, `[error] type-mismatch` | the census
can see the case |
There are zero writers to fix, so no example or skill file changes in
this PR.
## Pins
(`packages/sdui-parser/src/__tests__/literal-type-mismatch-error.test.ts`)
The inputs are copied verbatim from the tracked `sdui.manifest.json` and
written inline, so the test reads nothing outside its package.
- `aggregate="count"`: exactly one `{ severity: 'error', code:
'type-mismatch', message: 'object-metric prop "aggregate" expected an
object' }` (the real message has the tag in angle brackets), and `ok ===
false`.
- `aggregate={{"function":"count"}}`: zero diagnostics, `ok === true`.
- A string literal on a `number` (`object-kanban` `limit`), a `boolean`
(`invert`) and an `array` (`filter`) input: each gives one `error`
`type-mismatch`, and `ok === false`.
- An expression handed to an object input (`aggregate={count}`), and a
container that holds an expression: each gives exactly one `warning`
`inert-expression`, no `type-mismatch`, and `ok === true`.
Three existing pins described the old rule, and they were updated: the
non-enum union case and the single string-arm case in
`union-arm-type-mismatch.test.ts`, and the member severity in
`member-type-mismatch.test.ts`. Their codes and messages are unchanged.
Only severity, `ok`, and the wording that called these "byte-identical"
were edited.
## `os build` probe (the html-tier path through
`packages/cli/src/utils/sdui-manifest.ts`)
A scratch project with one `kind: 'html'` page, the committed
`sdui.manifest.json` copied beside its config, and the CLI run from
source (`bin/run-dev.js build`):
| page source | before (severity reverted, rebuilt) | after (this PR) |
|---|---|---|
| `aggregate="count"` | exit 0, a warning that the `aggregate` prop
expected an object, `Build complete` | **exit 1**, `Author-time rules
failed (1 issue)`, a failure that the `aggregate` prop expected an
object |
| `aggregate={{"function":"count"}}` | not run | exit 0, `Build
complete` |
The before leg is a one-off ablation run from the committed fix. It used
`scripts/ablation-replace.mjs` (anchor hit, 1 marker on disk), then
`pnpm --filter @objectstack/sdui-parser build`, then
`ablation-dist-preflight.mjs` (marker present in `dist/`, exit 0). After
the probe it was restored with `git checkout HEAD --`: `git diff HEAD`
was empty and the blob matched HEAD (`86cc5784`). The package was
rebuilt, the `--absent` preflight passed for both readings, and the
probe was re-run with exit 1. No permanent test file was left behind.
## Tests run (at `79df1db84f`)
- `pnpm --filter @objectstack/sdui-parser test`: 14 files, 225 tests
passed. `typecheck`: exit 0.
- Downstream consumers, after rebuilding `sdui-parser` (`dist` checked:
0 copies of the old ternary left). `pnpm --filter @objectstack/lint exec
vitest run`: 119 files, 5627 tests passed. `pnpm --filter
@objectstack/metadata-protocol exec vitest run`: 209 files passed and 3
skipped, 3463 tests passed. CLI unit tier, limited to the 4 files that
compile html pages (`src/utils/sdui-manifest.test.ts`,
`test/validate-build-gate-parity.test.ts`,
`test/platform-page-i18n-parity.test.ts`,
`test/i18n-section-coverage.test.ts`): 129 tests passed. The rest of the
CLI unit tier and its integration tier are left to CI.
- `node scripts/pm/dispatch-gates.mjs --commands` gave 63 derived
commands. 61 exited 0, including `check:sdui-lockstep`,
`check:nul-bytes`, `check:cross-package-test-inputs`,
`check-adr-0087-registration` and `check-changeset-no-major`. **NOT
MEASURED: `check:dual-build-cjs-loads`**: `PREREQUISITE NOT MET`,
because `embedder-openai` and `service-cluster-redis` have no `dist/`
(packages this diff does not touch). **NOT MEASURED:
`check:type-check-debt`**: it is a whole-tree tsc ratchet and hit the
240s local timeout. CI runs both.
- eslint was run on only the 4 changed `.ts` files, with
`--no-inline-config --format json`: 4 files, 0 errors, 0 warnings. The
repo config has no `parserOptions.project` (type-aware linting is off),
so this diff cannot change the lint result of any file it does not
touch. CI runs the full `pnpm lint`.
## Changeset
`.changeset/21671-html-literal-type-mismatch-error.md`:
`@objectstack/sdui-parser` `minor`. **Clause-② conflict for the seat to
resolve:** the claim states `Clause-②: no`, and this body carries that
line verbatim. The changeset declares `Clause-②: no (narrowing)`,
because a page that used to compile (and save, where the host has a
component manifest) is now refused. Earlier PRs treated a new refusal at
an authoring door as an accept-set narrowing (`21459`, `20827`). The
changeset therefore carries the `**BREAKING**` header, a `minor` bump
under the launch-window convention, and an ADR-0087 `not-required
(no-migration-prescription)` disposition: no key, declaration or stored
shape moves. `check-adr-0087-registration` and
`check-changeset-no-major` both pass, whether the body line has the arm
or not (both were simulated locally).
## objectui lockstep (declared, not acted on)
objectui has its own copy of this validator,
`packages/sdui-parser/src/validate.ts`, at the `.objectui-sha` pin
`ab187972`. It still has the old ternary in both `checkMemberTypes`
(`:418`) and `checkType` (`:465`). After this PR the two copies agree on
codes, messages and the accepted grammar, and differ only in
**severity**. `check:sdui-lockstep` compares grammar, codes and the
containment predicate, not severity, so it passes. This repo's copy is
the stricter one (save gate and `os build`). The dangerous direction, a
page that saves clean and then renders inert, cannot come from this
difference. The lockstep header in `validate.ts` now records this lead.
The port belongs to objectui's lane, and this PR does not write to
objectui.
## Acceptance notes
- The new severity covers every literal mismatch, including a number
literal handed to a string input (`label={42}`). That was the narrower
reading in the triage title ("a string literal against a declared
non-string input"). The claim and the brief specify the general rule ("a
literal whose coarse type no declared arm accepts"), and the measurement
shows that every value at this point is a literal, so the general rule
is the one implemented. The census found no writers of either form in
the repo.
---
_Generated by [Claude
Code](https://claude.ai/code/session_01VDtqoecgES7ScQYGbFVDRv)_
---------
Co-authored-by: Claude <noreply@anthropic.com>1 parent 7d07814 commit a4fd82a
5 files changed
Lines changed: 172 additions & 30 deletions
File tree
- .changeset
- packages/sdui-parser/src
- __tests__
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
Lines changed: 99 additions & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
| 47 | + | |
| 48 | + | |
| 49 | + | |
| 50 | + | |
| 51 | + | |
| 52 | + | |
| 53 | + | |
| 54 | + | |
| 55 | + | |
| 56 | + | |
| 57 | + | |
| 58 | + | |
| 59 | + | |
| 60 | + | |
| 61 | + | |
| 62 | + | |
| 63 | + | |
| 64 | + | |
| 65 | + | |
| 66 | + | |
| 67 | + | |
| 68 | + | |
| 69 | + | |
| 70 | + | |
| 71 | + | |
| 72 | + | |
| 73 | + | |
| 74 | + | |
| 75 | + | |
| 76 | + | |
| 77 | + | |
| 78 | + | |
| 79 | + | |
| 80 | + | |
| 81 | + | |
| 82 | + | |
| 83 | + | |
| 84 | + | |
| 85 | + | |
| 86 | + | |
| 87 | + | |
| 88 | + | |
| 89 | + | |
| 90 | + | |
| 91 | + | |
| 92 | + | |
| 93 | + | |
| 94 | + | |
| 95 | + | |
| 96 | + | |
| 97 | + | |
| 98 | + | |
| 99 | + | |
Lines changed: 2 additions & 1 deletion
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
67 | 67 | | |
68 | 68 | | |
69 | 69 | | |
70 | | - | |
| 70 | + | |
| 71 | + | |
71 | 72 | | |
72 | 73 | | |
73 | 74 | | |
| |||
Lines changed: 14 additions & 12 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
21 | 21 | | |
22 | 22 | | |
23 | 23 | | |
24 | | - | |
25 | | - | |
26 | | - | |
27 | | - | |
28 | | - | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
29 | 31 | | |
30 | 32 | | |
31 | 33 | | |
| |||
91 | 93 | | |
92 | 94 | | |
93 | 95 | | |
94 | | - | |
| 96 | + | |
95 | 97 | | |
96 | 98 | | |
97 | 99 | | |
98 | | - | |
| 100 | + | |
99 | 101 | | |
100 | 102 | | |
101 | 103 | | |
102 | 104 | | |
103 | 105 | | |
104 | | - | |
105 | | - | |
| 106 | + | |
| 107 | + | |
106 | 108 | | |
107 | 109 | | |
108 | 110 | | |
| |||
120 | 122 | | |
121 | 123 | | |
122 | 124 | | |
123 | | - | |
124 | | - | |
| 125 | + | |
| 126 | + | |
125 | 127 | | |
126 | 128 | | |
127 | 129 | | |
128 | | - | |
| 130 | + | |
129 | 131 | | |
130 | 132 | | |
131 | 133 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
331 | 331 | | |
332 | 332 | | |
333 | 333 | | |
334 | | - | |
| 334 | + | |
| 335 | + | |
| 336 | + | |
| 337 | + | |
| 338 | + | |
| 339 | + | |
| 340 | + | |
335 | 341 | | |
336 | 342 | | |
337 | 343 | | |
| |||
417 | 423 | | |
418 | 424 | | |
419 | 425 | | |
420 | | - | |
421 | | - | |
422 | | - | |
423 | | - | |
| 426 | + | |
| 427 | + | |
| 428 | + | |
| 429 | + | |
| 430 | + | |
| 431 | + | |
| 432 | + | |
| 433 | + | |
424 | 434 | | |
425 | 435 | | |
426 | 436 | | |
| |||
433 | 443 | | |
434 | 444 | | |
435 | 445 | | |
436 | | - | |
| 446 | + | |
437 | 447 | | |
438 | 448 | | |
439 | 449 | | |
| |||
454 | 464 | | |
455 | 465 | | |
456 | 466 | | |
457 | | - | |
458 | | - | |
459 | | - | |
460 | | - | |
461 | | - | |
462 | | - | |
463 | | - | |
464 | | - | |
465 | | - | |
466 | | - | |
| 467 | + | |
| 468 | + | |
| 469 | + | |
| 470 | + | |
| 471 | + | |
| 472 | + | |
| 473 | + | |
| 474 | + | |
| 475 | + | |
| 476 | + | |
| 477 | + | |
| 478 | + | |
| 479 | + | |
| 480 | + | |
| 481 | + | |
| 482 | + | |
| 483 | + | |
| 484 | + | |
| 485 | + | |
| 486 | + | |
| 487 | + | |
| 488 | + | |
| 489 | + | |
467 | 490 | | |
468 | 491 | | |
469 | 492 | | |
| |||
480 | 503 | | |
481 | 504 | | |
482 | 505 | | |
483 | | - | |
| 506 | + | |
484 | 507 | | |
485 | 508 | | |
486 | 509 | | |
| |||
0 commit comments