Repository navigation
Commit 8016a9d
feat(spec)!: a navigation entry whose visible reads data must declare dataSource, refused at parse (#24490)
Fixes #24450
Clause-②: no (narrowing)
Question 2 of the ruling recorded on #24332 (comment 6259393328), letter
A, verbatim as the director recorded it:
> **Question 2 — A.** A navigation item without a `dataSource` whose
`visible` reads `data.*` is refused at parse, BREAKING with its ADR-0087
conversion, as its own `domain:spec` card, p1 `target:v18`, `Clause-②:
no (narrowing)`; in-repo impact measured at 0 declarations.
## What this does
- `packages/spec/src/ui/app.zod.ts`: a module-private refinement,
`navItemVisibleDataNeedsDataSource`, mounted once on
`NavigationItemSchema` itself (a `superRefine` on the discriminated
union). An entry with no `dataSource` whose `visible` is a CEL predicate
that reads the root `data` gets one `custom` issue at its own `visible`.
The message names the fix: declare `dataSource` on this entry, or drop
the `data.*` read and gate on `current_user` or `features`. Because the
check sits on the union, it reaches every navigation door:
`app.navigation`, `app.areas[].navigation`, every `children` array at
any depth, and a navigation contribution's `items`. The exported
per-branch schemas do not chain it. That is the same posture as
`objectNavTargetExclusivity`, and no runtime consumer parses a branch
schema directly (measured with `git grep`: 0 non-test hits).
- The reader is `celReadsRootData`, a small CEL lexer in the same file
(about 100 lines, no new export). It refuses only when it is sure. It
skips strings (single, double, triple-quoted, and `r` / `b` prefixed)
and `//` comments. It does not count member selection (`x.data`), a call
named `data`, or a field or key name before `:` in a `{ ... }` literal.
If a comprehension macro (`exists`, `all`, `map`, `filter`, `cel.bind`,
...) binds a variable named `data`, the reader returns "no read" for the
whole predicate. A leading-dot absolute name (`.data.x`) and a
`${...}`-wrapped predicate are read as root reads. It imports nothing
from a runtime package (Prime Directive #2). The precedent for a
predicate reader next to the schema that refuses on it is the settings
manifest's `visible` grammar check in `system/settings-manifest.zod.ts`.
- The `visible` describe and TSDoc now say the read is refused without a
`dataSource`. `content/docs/references/ui/app.mdx` was regenerated by
`gen:docs`.
-
`packages/spec/src/migrations/entries/semantic/18.nav-item-visible-data-needs-data-source.ts`:
the D3 semantic entry (protocol 18). No key is removed, so there is no
tombstone. There is no D2 conversion because the shape cannot be
converted: only the author knows which endpoint the entry meant to read,
and dropping the read would change what the gate means. The
chart-binding narrowing (`view-chart-binding-dataset-required`) has the
same shape.
- `packages/spec/dropped-refinements.baseline.json`: +31 sites, one
`...lazy` site per published schema that carries `NavigationItemSchema`.
The build printed the corrected entries and they are copied verbatim.
`measured.droppedRefinementSites` goes from 781 to 812. JSON Schema
cannot express a CEL lexer, so these are real new gaps of the
projection, not of the rule.
- `.changeset/24450-nav-visible-data-needs-data-source.md`:
`@objectstack/spec` `minor` with the **BREAKING** banner (pre mode),
`Clause-②: no (narrowing)`, one ADR-0087 marker (`registered
nav-item-visible-data-needs-data-source`), and the FROM → TO mapping
plus a one-line fix.
## Premise, measured on `origin/main` `26501ae24` before building
| PM assumption | Reading |
|:--|:--|
| An in-spec CEL root reader exists
(`expression-scope-data-root.pin.test.ts`) | **Does not hold.** That
file pins prose only (describe text) and reads no expression. No CEL
identifier reader exists under `packages/spec/src`. The only predicate
tokenizer in spec is the settings manifest's, and it is a different
grammar (not CEL). So one was written, small and conservative, as
described above. |
| Reach: 0 `data.` reads in navigation `visible` predicates | **Holds.**
An AST census (TypeScript compiler API) of every tracked `.ts` / `.tsx`
/ `.mts` file at `26501ae24` counted object literals with an `id`, a
navigation `type` and a `visible`. Result: 11 entries in 4 files. 2 read
`data`, and both are in `app-nav-item-api-data-source.test.ts` and
declare a `dataSource`. **0 read `data` without one.** Control leg: 3 of
the same entries read `current_user`. The settings-manifest `visible`
predicates (`service-settings/src/manifests/*`, 20 `${data.provider
...}` hits) are a different schema and were excluded from the census by
path. JSON/YAML: 0 `"visible": "...data."` hits, against a control of 0
`"visible"` string values at all, so the JSON corpus has nothing to say
here either way. objectui and cloud were not measured. |
| The refusal belongs at the entry's `visible` path | Holds. The path is
`visible` relative to the entry, for example
`navigation.0.children.0.visible`. |
## Tests (at `b911ad135b`, the head after merging `origin/main`
`31ce45c07`)
- New
`packages/spec/src/ui/app-nav-visible-data-needs-data-source.test.ts`,
23 tests. Refused, each with exactly one `custom` issue at `visible` and
a message that names the fix: `data.total == 1`, `!data.has_more`, bare
`data`, `has(data.total)`, a mix with `current_user`, `.data.total`,
`${data.total == 1}`, `data['total']`, a read after a raw string ending
in a backslash, the `{ dialect: 'cel', source }` envelope, the
`dashboard` / `page` / `url` / `group` branches, and every door:
`navigation`, a group's child whose parent declares a `dataSource` (the
binding is not inherited), `areas[].navigation`, and a contribution's
`items`. Accepted: the same entry with a `dataSource` (its `visible`
envelope unchanged), `current_user`, `features`, both together,
`current_user.data.total`, `data` inside a string, inside a
triple-quoted string or inside a comment, a macro iteration variable
named `data`, a selection on a call result, an entry with no `visible`,
and a separator.
- Ablation, once, through `node scripts/ablation-replace.mjs`. Anchor
`if (!celReadsRootData(visible.source)) return;`, count x1 → x0, blob
`90cf1bbc` → `56f1eb19`. Result: **12 failed / 11 passed**. Every
refusal case went red and every acceptance case stayed green, which is
the expected direction. Restored: blob == HEAD (`90cf1bbc`) and `git
diff HEAD` is empty. The test imports `./app.zod` from source, so there
is no dist leg.
- `pnpm --filter @objectstack/spec test`: 699 files passed, 21290 tests
passed (1 todo).
- `pnpm --filter @objectstack/spec test:repo`: 70 files, 1162 tests
passed.
- `pnpm --filter @objectstack/spec typecheck`: exit 0.
- `pnpm --filter @objectstack/spec check:generated`: all 14 artifacts up
to date after a rebuild of the merged tree.
- Consumers (accept-set narrowing, so downstream; these packages consume
`@objectstack/spec`):
- `@objectstack/lint`: full suite, 145 files, 6866 tests passed.
- `@objectstack/metadata-protocol`: full suite, 246 files passed (3
skipped), 20655 tests passed.
- `@objectstack/cli`: unit tier, NARROWED to the navigation and i18n
files (`*nav*.test.ts`, `i18n-*.test.ts`). Of 35 named files, 27 are in
the unit project: 27 files, 308 tests passed. The rest of the unit tier
and the integration tier are declared to CI.
- Pin sweep: I searched the whole repo for the refusal text and for the
old describe sentence ("has no `data` to read"). The only hits are the
regenerated reference page. No test elsewhere pins the old acceptance.
## Gates
`node scripts/pm/dispatch-gates.mjs --commands --repo
objectstack-ai/objectstack` on this diff derived 112 families. All 112
ran on `b911ad135b`, and `--ran` reconciles **112 of 112 run, 0 NOT
MEASURED**, every one at exit 0. Six of them
(`check:doc-formula-expressions`, `check:doc-security-posture`,
`check:skill-examples`, `check:docs-transcript-drift`,
`check:dual-build-cjs-loads`, `check:lean-entry-closure`) first exited 3
(PREREQUISITE NOT MET: no lint / formula / objectql dist). After a turbo
build of the `@objectstack/lint...`, `@objectstack/objectql...`,
`@objectstack/client...` and `@objectstack/formula...` closures (36
tasks, all cache hits), all six exited 0. The derivation warned that
`origin/main` had moved 8 commits past the merge, touching
`.github/workflows/ci.yml`, `.github/workflows/spec-shape-gate.yml` and
`scripts/ci/spec-shape-gate.mjs`. CI reads those from the merge ref.
## Acceptance notes
- **Not built, per the dispatch:** the page-element `visibleWhen` twin.
A page component whose `visibleWhen` reads `data.*` with no API
`dataSource` binding is the same shape. It is outside this ruling, which
covers navigation entries only.
- The reader does not count `data` as a map-literal key (`{data: 1}`),
which in CEL is an expression that reads `data`. That spelling is
under-refused on purpose, because it cannot be told apart from a message
field name without a type table.
- The metadata write door is reached through `METADATA_TYPE_SCHEMAS.app`
= `AppSchema` (`packages/spec/src/kernel/metadata-type-schemas.ts`). Its
HTTP answer (`422 INVALID_METADATA`) is not measured here, and stored
`sys_metadata` app rows are not measured either. The census found no
such row in this repo.
- Labels: zero writes. The dispatch's write budget names no label.
---
_Generated by [Claude
Code](https://claude.ai/code/session_01D6F9axfoPzR7t8G1J7Ytzz)_
---------
Co-authored-by: Claude <noreply@anthropic.com>1 parent 45bec43 commit 8016a9d
6 files changed
Lines changed: 451 additions & 49 deletions
File tree
- .changeset
- content/docs/references/ui
- packages/spec
- src
- migrations/entries/semantic
- ui
| 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 | + | |
0 commit comments