Skip to content

feat(spec)!: a navigation entry whose visible reads data must declare dataSource, refused at parse - #24490

Merged
objectstack-fleet[bot] merged 4 commits into
mainfrom
claude/issue-24450-nav-visible-data-needs-datasource
Oct 19, 2026
Merged

objectstack-fleet[bot] merged 4 commits into
mainfrom
claude/issue-24450-nav-visible-data-needs-datasource

Conversation

@objectstack-fleet

Copy link
Copy Markdown
Contributor

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 ✨ Set up Copilot instructions #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

@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/spec, touching 14 documentable anchor(s). ⚠️ 1 changed file(s) yielded no anchor (packages/spec/dropped-refinements.baseline.json), so the pages documenting them are NOT COVERED by this run — this is not a clean bill of health for those files.

1 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:

  • content/docs/ui/dashboards.mdx (via sortBy (literal, a string literal in CEL_BINDING_MACROS))

⛔ 3 release-owned page(s) also name something this change touched. These are read-only:

  • content/docs/releases/v17/17-0.mdx (via sortBy (literal, a string literal in CEL_BINDING_MACROS))
  • content/docs/releases/v17/17-5.mdx (via sortBy (literal, a string literal in CEL_BINDING_MACROS))
  • content/docs/releases/v17/17-6.mdx (via sortBy (literal, a string literal in CEL_BINDING_MACROS))

content/docs/releases/ is RELEASE-OWNED (AGENTS.md "Documentation Guardrails"): release
notes are written centrally at release time, and a code PR that edits them is the exact PR
that guardrail exists to stop. They are still audited — read-only. If one of them is actually
wrong, file an issue or open a dedicated docs-only PR; do not edit it here.

What this run could not see
  • 1 changed file(s) yielded no anchor (packages/spec/dropped-refinements.baseline.json) — pages documenting those are invisible to this run
  • 4 name(s) were too generic to anchor anything (single lowercase words)
  • the SDK route bridge reached 54 of 206 client-bound route-ledger rows — the other 152 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 152: 0 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 55 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 97 are undecided (no in-repo declaration, on a ledger that has other in-repo registrars — absence and an unreadable spelling are not distinguishable here). The rows themselves: node scripts/docs-audit/affected-docs.mjs --bridge-coverage
  • a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.
  • a key NAME is not a key, so the hand re-read the line above prescribes can land on the wrong schema. The same spelling is authorable on one governed type and a [REMOVED] tombstone on another for each of active, aria, joins, objects, template, tools and version (censused on [finding] tools is a key on BOTH AgentSchema (tombstoned, dead) and SkillSchema (live, cloud-attested), so a name-based search attributes skill examples to the agent key — it produced a false stop-the-line alarm on PR #19059 #19093 over the liveness ledger's governed types, top-level keys); nothing in a search result distinguishes the two, so a grep hit on a LIVE example reads as evidence about the DEAD key. Measured on fix(spec): the agent.tools liveness row says dead — it claimed live on a key the schema tombstoned #19059: content/docs/ai/agents.mdx was reported as contradicting the agent.tools tombstone over its tools: example at :161, which is inside the defineSkill({ block opened at :155 — the page was already correct. Settle ownership by PARSING the value against both schemas, never by the name: that literal PASSES SkillSchema, and as an AgentSchema it FAILS at tools with the tombstone prescription. ⛔ These names are not the whole class — a key retired through a .strict() guidance map leaves no tombstone in the walked shape and none of them here (tool.category, live as AIToolDefinition.category).

Coarse fallback — 143 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json ae6bd1a696fa630cf8a3fc052489dd7af03cfcdc → packageMentionDocs.

Which tree this was computed on

This run read content/docs from aac99f95ae59ddd126ffcdc937e975f0dbba5948 — the merge of head b911ad135b1fe2c62f71e060fa9a7781975bbc89 into base ae6bd1a696fa630cf8a3fc052489dd7af03cfcdc, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin aac99f95ae59ddd126ffcdc937e975f0dbba5948 && git checkout aac99f95ae59ddd126ffcdc937e975f0dbba5948
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin ae6bd1a696fa630cf8a3fc052489dd7af03cfcdc b911ad135b1fe2c62f71e060fa9a7781975bbc89 && git checkout -B drift-repro ae6bd1a696fa630cf8a3fc052489dd7af03cfcdc && git merge --no-ff b911ad135b1fe2c62f71e060fa9a7781975bbc89

node scripts/docs-audit/affected-docs.mjs --json ae6bd1a696fa630cf8a3fc052489dd7af03cfcdc

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

Advisory only, and a precision-first one (#9192): a page is listed because it names a
symbol, wire route or SDK method this diff touched — not because it mentions a changed
package. Each row says which anchor put it there, so a wrong row is reportable rather than
merely annoying. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs ae6bd1a696fa630cf8a3fc052489dd7af03cfcdc → pass the list as
args.docs, on the commit named under Which tree this was computed on.

@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

Review: ACCEPT · PR #24490 @ b911ad135b1fe2c62f71e060fa9a7781975bbc89 · domain:spec seat 3 · 2026-10-19T07:24Z

Session session_01D6F9axfoPzR7t8G1J7Ytzz · baozhoutao. Reviewer of record; checked against GitHub, not the report.

  • Shape: draft, Fixes #24450, Clause-②: no (narrowing); +451/−49, 6 files. check-governed-merges --pr 24490: 0 of 6 paths hit the register, 500 lines ≤ 3000 → NOT governed, ordinary queue landing.
  • The ruling (spec / objectui navigation: an app's navigation item cannot read data, so a label or a visible gate cannot follow a live count (the control plane's 「环境」 for a single-environment organization) #24332 Q2, letter A): an entry with no dataSource whose CEL visible reads root data is refused at parse, at the entry's visible, with the fix named. Mounted once as a superRefine on NavigationItemSchema, so it reaches navigation, areas[].navigation, nested children and contribution items.
  • The reader: celReadsRootData is module-private, imports no runtime package (Prime Directive ✨ Set up Copilot instructions #2), and errs toward NOT refusing: member selections, calls, map/field keys, strings, comments, and any predicate whose macro binds data are skipped. The one known under-refusal (a data map-literal key) is in the PR's acceptance notes. Unsure means accept, which is the right direction for a parse refusal.
  • Pins: 23 tests; ablation of the guard line turns all 12 refusals red and leaves all 11 acceptances green; restored to green.
  • Reach: 11 navigation entries with visible re-measured at 26501ae24; 0 read data without a source (the ruling's 0), control 3 current_user reads.
  • Migration & changeset: ADR-0087 D3 semantic entry nav-item-visible-data-needs-data-source (no D2: a missing source cannot be invented); minor BREAKING changeset with FROM → TO and one disposition marker. Generated app.mdx and the dropped-refinements ledger (+31 sites the build printed) ride along, as the gates require.
  • Gates: 112 derived, 112 exit 0 (six re-run after a closure build).
  • CI on the head: read at this act; the seat lands on green through the merge queue.

Generated by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation protocol:ui size/l tests tooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

spec(ui): a navigation entry with no dataSource whose visible reads data.* is refused at parse (BREAKING, ADR-0087), question 2 of #24332's ruling

2 participants