Skip to content

Commit 362035c

Browse files
os-billclaude
andauthored
fix(spec): react-tier ListView onNavigate declares the mode union it actually emits (#19074)
Fixes #19057 Clause-②: yes Maintainer decision batch #166 item 1 — 「同意」 to option **C** on objectui#9547: the false sentence is in the protocol, so the protocol is corrected first and the sibling's four declaration faces align after. `packages/spec/src/ui/react-blocks.ts:321` declared the ListView `onNavigate` second argument as `'view' | 'edit'`. `'edit'` is emitted by no call site in the reference implementation and read by no branch; `'new_window'` — what a Cmd/Ctrl- or middle-click, and an authored `navigation: { mode: 'new_window' }`, actually send — was not declared at all. An author reading this contract wrote a handler with one dead arm and one missing arm. ## The change — one source line `packages/spec/src/ui/react-blocks.ts:321`, before: ```ts { name: 'onNavigate', type: "(recordId, action: 'view' | 'edit') => void", kind: 'callback', description: 'Called for page-level navigation.' }, ``` after: ```ts { name: 'onNavigate', type: "(recordId, action: 'view' | 'new_window') => void", kind: 'callback', description: "Called for page-level navigation. The second argument is a navigation-MODE token from a CLOSED vocabulary — 'view' opens the record page, 'new_window' opens it in a new browser tab; the reference implementation emits no other value and reads no other branch." }, ``` The description now names both modes and states that the slot is a closed vocabulary. The closedness itself is not new: the protocol's own retirement note for `view.list.navigation.view` (`packages/spec/src/ui/view.zod.ts:1818`, removed in 17.5.0) already records that anything outside the mode vocabulary "matched no branch". What moves here is the MEMBERSHIP of that vocabulary, not its closedness — and `view.zod.ts` is read-only on this card, untouched. ## The generated half — zero hand edits `skills/objectstack-ui/references/react-blocks.md` is the `OUT_MD` of `packages/spec/scripts/build-react-blocks-contract.ts`, and it is a PD-14 governed path whose queue-guard generated-artifact exception (`scripts/pm/check-governed-merges.mjs`, register row `spec-react-blocks`) is recomputed byte-exactly at merge-group time. It was produced here by running the generator and nothing else. `skills/objectstack-ui/references/react-blocks.md:67`, before: | `onNavigate` | `(recordId, action: 'view' \| 'edit') => void` | callback | | Called for page-level navigation. | after: | `onNavigate` | `(recordId, action: 'view' \| 'new_window') => void` | callback | | Called for page-level navigation. The second argument is a navigation-MODE token from a CLOSED vocabulary — 'view' opens the record page, 'new_window' opens it in a new browser tab; the reference implementation emits no other value and reads no other branch. | Idempotence, run after the generator: `pnpm --filter @objectstack/spec check:react-blocks` exits 0 with "1 generated files in sync with packages/spec", and `git status --porcelain` is unchanged by that run. Post-merge, `pnpm --filter @objectstack/spec check:generated` reports all 16 generated artifacts up to date, `check:react-blocks` among them, and leaves a clean tree. ## Readings Every zero below carries its instrument's reachable radius and one known target necessarily outside it (charter rule, PR #18921). **1. The retired literal, repo-wide — 0 in code or generated output.** `git grep -F` for the old spelling at `92a80ed740` returns exactly one line, and it is this PR's own changeset prose quoting the removed declaration; zero in source or in any generated artifact. Radius: the CONTENTS OF TRACKED FILES of `objectstack-ai/objectstack` only — excludes `node_modules`, excludes untracked files, excludes the sibling repository. Known target outside that radius: objectui's four declaration faces (`ObjectGridComponentProps.onNavigate`, `ObjectGridSchema.onNavigate`, `ListViewRuntimeProps.onNavigate`, `UseNavigationOverlayOptions.onNavigate`) — another repository, structurally unreadable by this instrument; they move under objectui#9547 with its bump to `@objectstack/spec` >= 17.5.0. Firing control, same instrument, same tree: `onNavigate` matches 18 lines across 12 files. **2. DARK — `'edit'` is gone from the declaration line: 0.** `sed -n '321p' packages/spec/src/ui/react-blocks.ts | grep -c "'edit'"` prints `0`. Radius: one line of one file, nothing else. Known target outside that radius: `packages/spec/src/ui/view.zod.ts:1818`, whose retirement note still quotes `edit` verbatim and is deliberately untouched — this instrument cannot see it. Firing control, same instrument, same line: `grep -c "'view'"` prints `1`. **3. Hand edits in the governed generated file: 0.** The generator was run, then re-run in `--check` mode, which compares its own emitted bytes against what is on disk; it exits 0. Radius: exactly the one path `OUT_MD` names. It says nothing about any other file under `skills/`. Known target outside that radius: `skills/objectstack-ui/SKILL.md` — this generator does not emit it and this comparison never reads it. (It is unmodified by this PR; that is a separate statement, from `git diff --name-only`, which lists three paths in all.) Firing control: the same file's `git diff` against the branch point is non-empty on exactly one line, so the byte comparison is demonstrably capable of seeing a difference. **4. Published-skill-surface budget.** The changed file is GENERATOR-OWNED, so it is measured and not ratcheted — `check-skills-token-ratchet` classifies it that way by name. Whole-file: 115 lines before, 115 lines after (+0); 12504 bytes before, 12737 after (+233); 3126 tokens before, 3185 after (+59). Whole bundle: 140038 tokens before, 140097 after (+59), of which the authored/ratcheted half is unchanged at 129392 against a ceiling of 145656. Both ratchets exit 0. The "before" figures are taken at `2767af8e83`, whose `skills/` tree is byte-identical to this branch's merge base (`git diff --stat 2767af8 d4cb05c -- skills/` is empty). ## Verification Anchored at `92a80ed740`, after the merge of `origin/main` and after the final commit. - `pnpm --filter '@objectstack/spec...' build` — exit 0. - `pnpm --filter @objectstack/spec check:generated` — exit 0; all 16 generated artifacts up to date. - `pnpm --filter @objectstack/spec typecheck` — exit 0. - `pnpm --filter @objectstack/spec exec vitest run --project local --maxWorkers=2` — exit 0; **491 test files, 14299 tests passed**. - `pnpm lint` (repo-wide `eslint . --no-inline-config`) — exit 0. - Gate families derived from the real diff with `node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack`: **87 derived, 86 run, 0 UNRUN, 1 NOT MEASURED**, reconciled with `--ran` over a record carrying each command's exit code captured before any pipe. - NOT MEASURED: `pnpm check:dual-build-cjs-loads` — exit 3, PREREQUISITE NOT MET. It reads the emitted CJS bytes of every published package and 66 packages in this worktree have no `dist/`; clearing it needs a whole-repo `pnpm build`, which on this shared container means holding the single heavy-verify lock for far longer than this diff can justify. Declared, not silently dropped. CI's `Build Core` and `Lint & Repo Gates` cover it on a fresh checkout. - `pnpm --filter @objectstack/spec run check:react-declaration-parity` is recorded at exit 1 because the derived spelling passes no `MANIFEST`, and that gate exits 1 rather than skipping when it has none. Run as CI runs it — `MANIFEST="$PWD/sdui.manifest.json" pnpm --filter @objectstack/spec check:react-declaration-parity --baseline react-declaration-parity.baseline.json --strict` — it exits **0**: no new declaration divergence versus the accepted baseline. - Two further families exited 3 on first contact (`check:doc-formula-expressions`, `check:lean-entry-closure`); their prerequisites were built and both then exited 0. Those are the exit codes recorded. ## Acceptance notes Noted, not filed: in the reference implementation two of the four `onNavigate` call sites spell the mode argument `view ?? 'view'` (`useNavigationOverlay.ts:323` and `:356`, read at objectui `dda8f3815d`), where `view` is the retired `navigation.view` key. On the currently pinned sibling an authored view name can therefore still flow into this closed slot — which is exactly the defect ADR-0049 retired in 17.5.0 and which the card routes to objectui#9547 and its pin bump. It is already filed there; nothing new is owed. Successor: objectui#9547. Noted, not filed: `check:react-declaration-parity` reports 111 spec-only declaration divergences and one block missing from the registry, all held flat by its accepted baseline. Pre-existing, unrelated to this line, and the baseline ratchet is green. Successor: none — this is the gate's standing state, not a finding of this round. ## 维护者速读(草稿) **改了什么** — 协议里 ListView 的 `onNavigate` 回调,第二个参数的取值声明从 `'view' | 'edit'` 改成 `'view' | 'new_window'`,并在说明里点名这两个模式、写明这个槽位是封闭词表。改动是一行源码,外加由它自动再生的一份 AI 参考页。没有类型签名移动,没有运行时行为变化。 **为什么改** — 这句声明两个方向都是假的:`edit` 从来没有任何调用点发出过,也没有任何分支读它;而用户按住 Cmd/Ctrl 点一行、或配置 `mode: 'new_window'` 时真正发出的 `new_window`,声明里根本没有。照这份契约写代码的人(以及照它写代码的 AI)会写出一条永远不执行的分支,同时漏掉真正会发生的那条——表现为"新窗口打开记录"这个动作静默地什么都不做。 **风险与代价(含回滚)** — 风险很低:改的是一份"说明书",不是执行代码,本仓库没有任何程序读这两个字面量(全仓只有这一处)。代价是消费方需要把 `edit` 那条分支换成 `new_window`,changeset 里写了一行对照表。回滚就是 revert 这个 PR 再跑一次生成器,没有数据迁移、没有存量元数据受影响。需要留意的一点:`skills/` 是受管面,本 PR 里那份 `.md` 完全由生成器产出、零手改,走的是队列闸对已登记生成物的豁免路径;闸在 merge-group 时会逐字节重算,任何一处手写内容都会让豁免失效。 **席位意见** — (待席位填写) **你要做的** — 确认"把 `edit` 从词表里去掉、把 `new_window` 加进来"这个方向是你要的(这正是 objectui#9547 上你已经同意的 C 案)。其余无需你操作:入队与落地归 PM,本 PR 停在 draft。 --- _Generated by [Claude Code](https://claude.ai/code/session_01JbZnqu8bt6YqfJsr9vaFb3)_ Co-authored-by: Claude <noreply@anthropic.com>
1 parent 5aa3a46 commit 362035c

3 files changed

Lines changed: 41 additions & 2 deletions

File tree

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
---
2+
'@objectstack/spec': minor
3+
---
4+
5+
React-tier `<ListView>`: the `onNavigate` declaration becomes
6+
`(recordId, action: 'view' | 'new_window') => void` — a declared value **no branch ever
7+
emitted** is removed, and the value **two reference call sites do emit** is added.
8+
9+
`REACT_BLOCKS`' ListView overlay declared the second argument as `'view' | 'edit'`. That
10+
sentence was false in both directions. `'edit'` is emitted by no call site in the
11+
reference implementation and read by no branch; `'new_window'` — what a Cmd/Ctrl- or
12+
middle-click, and an authored `navigation: { mode: 'new_window' }`, actually send — was
13+
not declared at all. An author reading this contract wrote a handler with one dead arm
14+
and one missing arm.
15+
16+
The second argument is a navigation-MODE token with a **closed vocabulary**, and the
17+
declaration now says so. That closedness is not new: the protocol's own retirement note
18+
for `view.list.navigation.view` (removed in 17.5.0, ADR-0049) records that anything
19+
outside the mode vocabulary "matched no branch". What this change corrects is the
20+
membership of the vocabulary, not its closedness.
21+
22+
## FROM → TO
23+
24+
| you wrote | write instead |
25+
| --- | --- |
26+
| `onNavigate={(id, action) => { if (action === 'edit') … }}` | delete that arm — nothing ever called it |
27+
| a handler with no `'new_window'` arm | handle `'new_window'`: open the record in a new browser tab. Omitting the arm leaves the modifier-click path doing nothing |
28+
| `onNavigate={(id) => …}` (one argument) | unchanged — the arity is untouched |
29+
30+
**The one-line fix:** replace the `'edit'` arm with a `'new_window'` arm.
31+
32+
Scope: this moves a **declaration**, not a type or a runtime check. `REACT_BLOCKS` types
33+
this prop as a documentation string (`ReactBlockDef[]`), so no `.d.ts` signature moves
34+
and nothing that compiles today stops compiling. The behaviour it describes is the
35+
reference implementation's, which already emits exactly these two values; the sibling's
36+
four declaration faces are corrected under objectui#9547 and its bump to
37+
`@objectstack/spec` >= 17.5.0.
38+
39+
Clause-②: yes

‎packages/spec/src/ui/react-blocks.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -318,7 +318,7 @@ export const REACT_BLOCKS: ReactBlockDef[] = [
318318
{ name: 'filters', type: "FilterArray e.g. ['status','=','active']", kind: 'controlled', description: 'ObjectQL base filter; drive from React state for tabbed/searched lists. ([field, op, value]; ops =, !=, >, <, contains, in; compound: [\"and\", […], […]]).' },
319319
{ name: 'navigation', type: "{ mode: 'page' | 'drawer' | 'modal' | 'split' | 'none' }", kind: 'binding', description: 'What a row click does. Use { mode: \"none\" } when you handle clicks via onRowClick.' },
320320
{ name: 'onRowClick', type: '(record) => void', kind: 'callback', description: "Called with the clicked row's record — the hook for master/detail." },
321-
{ name: 'onNavigate', type: "(recordId, action: 'view' | 'edit') => void", kind: 'callback', description: 'Called for page-level navigation.' },
321+
{ name: 'onNavigate', type: "(recordId, action: 'view' | 'new_window') => void", kind: 'callback', description: "Called for page-level navigation. The second argument is a navigation-MODE token from a CLOSED vocabulary — 'view' opens the record page, 'new_window' opens it in a new browser tab; the reference implementation emits no other value and reads no other branch." },
322322
{ name: 'fields', type: 'string[]', kind: 'binding', description: 'Limit/order the columns shown (defaults to the object list fields).' },
323323
{ name: 'options', type: 'Record<string, any>', kind: 'binding', description: 'View-type-specific options bag (kanban/calendar/gantt extras); prefer the typed spec props where they exist.' },
324324
],

‎skills/objectstack-ui/references/react-blocks.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ Server-connected object table with toolbar and switchable visualizations (grid/k
6464
| `options` | `Record<string, any>` | binding | | View-type-specific options bag (kanban/calendar/gantt extras); prefer the typed spec props where they exist. |
6565
| `filters` | `FilterArray e.g. ['status','=','active']` | controlled | | ObjectQL base filter; drive from React state for tabbed/searched lists. ([field, op, value]; ops =, !=, >, <, contains, in; compound: ["and", […], […]]). |
6666
| `onRowClick` | `(record) => void` | callback | | Called with the clicked row's record — the hook for master/detail. |
67-
| `onNavigate` | `(recordId, action: 'view' \| 'edit') => void` | callback | | Called for page-level navigation. |
67+
| `onNavigate` | `(recordId, action: 'view' \| 'new_window') => void` | callback | | Called for page-level navigation. The second argument is a navigation-MODE token from a CLOSED vocabulary — 'view' opens the record page, 'new_window' opens it in a new browser tab; the reference implementation emits no other value and reads no other branch. |
6868
| `columns` | `string[] \| object[]` | data | ✓ | Fields to display as columns |
6969
| `type` | `'grid' \| 'kanban' \| 'gallery' \| 'calendar' \| 'timeline' \| 'gantt' \| 'map' \| 'chart' \| 'tree'` | data | | |
7070
| `sort` | `object[]` | data | | |

0 commit comments

Comments
 (0)