Skip to content

Commit 8997889

Browse files
os-zhuangclaude
andauthored
fix(spec-parity): the Tier-3 spec values render instead of red-boxing (#2943) (#3011)
This tier fails LOUDLY — `SchemaRenderer` renders a red `role="alert"` panel naming the missing type and dumping the schema JSON. Three rows: **Dashboard chart dispatch.** Three surfaces restated the chart vocabulary independently and disagreed: `DatasetWidget` covered all 19 `ChartTypeSchema` values, `DashboardRenderer` 15, `DashboardGridLayout` 8. A type a surface didn't name fell through to `{ ...widget }`, whose `type` no component registers. The sharpest part was already half-done: `METRIC_LIKE_TYPES` existed with a docstring saying gauge/solid-gauge/kpi/bullet "render as a metric card rather than a chart" — and was consulted only to pick a grid span, so the tile was sized correctly and the widget was never routed. One shared `widgetDispatch` module now classifies a widget type into its family (series / metric / table / pivot / custom / unsupported / passthrough) and both renderers consume it. The single-value families reach the metric card, the dropped families get a labelled placeholder in the grid layout too, and the `list` branch folds into `table` while keeping its lighter chrome. **`ai:chat_window` was offered by Studio with a config panel and has no renderer.** `placeholders.tsx` documents a deliberate decision to exclude it so it fails loudly; the palette was never told, so an author dragged a block Studio advertised and got the red box. Pruned from `block-types.ts` and `block-config.ts` per the audit's recommendation — the floating chat overlay (plugin-chatbot) stays canonical. **Placeholder coverage was host-dependent.** `registerPlaceholders()` is opt-in and only `apps/console` calls it, so four blocks the palette DOES offer (`nav:menu`, `nav:breadcrumb`, `global:search`, `ai:suggestion`) red-boxed in every other host. Those four now register eagerly through the `./renderers` barrel; the rest of the protocol vocabulary stays opt-in on purpose, because a genuinely missing renderer must keep failing loudly. **The guard is inverted, as the issue asked.** `block-config.test.ts` used to hand-assert a few palette EXCLUSIONS, which locks drift in — a new spec block type could land and never reach the palette with nothing failing. It now derives coverage from `PageComponentType` (34 values): every value must be offered or listed in the new `PALETTE_EXCLUSIONS` with a reason, no value can be both, no exclusion may name a non-spec type, and a block with a config panel must be authorable (the seam `ai:chat_window` hid in). Inverting it immediately caught four wrong entries in my own first exclusion list. Refs #2943, #2901 Co-authored-by: Jack Zhuang <277994282+os-zhuang@users.noreply.github.com> Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
1 parent c199895 commit 8997889

10 files changed

Lines changed: 515 additions & 124 deletions

File tree

packages/app-shell/src/views/metadata-admin/previews/__tests__/block-config.test.ts

Lines changed: 68 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,15 @@
11
import { describe, it, expect } from 'vitest';
2+
import { PageComponentType } from '@objectstack/spec/ui';
23
import { BLOCK_CONFIG, blockHasConfig } from '../block-config';
3-
import { BLOCK_TYPE_META } from '../block-types';
4+
import { BLOCK_TYPE_META, PALETTE_EXCLUSIONS } from '../block-types';
45

56
describe('block-config', () => {
67
it('exposes a configurable panel for every content block with authorable props', () => {
78
for (const type of [
89
'element:text', 'element:image', 'element:number', 'element:button',
910
'page:header', 'page:card', 'page:tabs', 'page:accordion',
1011
'record:related_list', 'record:highlights', 'record:details', 'record:alert',
11-
'record:path', 'record:quick_actions', 'ai:chat_window', 'ai:input',
12+
'record:path', 'record:quick_actions', 'ai:input',
1213
'element:definition-list', 'element:repeater',
1314
]) {
1415
expect(blockHasConfig(type), type).toBe(true);
@@ -22,19 +23,6 @@ describe('block-config', () => {
2223
expect(blockHasConfig(undefined)).toBe(false);
2324
});
2425

25-
it('prunes shell-singleton and unimplemented blocks from the page palette', () => {
26-
for (const type of [
27-
'app:launcher', 'global:notifications', 'user:profile', // shell singletons
28-
'element:form', 'element:filter', 'element:record_picker', // no renderer
29-
]) {
30-
expect((BLOCK_TYPE_META as any)[type]).toBeUndefined();
31-
}
32-
// page-content navigation stays; the real list primitives are now in the palette
33-
expect((BLOCK_TYPE_META as any)['nav:menu']).toBeTruthy();
34-
expect((BLOCK_TYPE_META as any)['element:definition-list']).toBeTruthy();
35-
expect((BLOCK_TYPE_META as any)['element:repeater']).toBeTruthy();
36-
});
37-
3826
it('also exposes the array-valued blocks', () => {
3927
for (const type of ['page:tabs', 'record:details', 'record:highlights']) {
4028
expect(blockHasConfig(type)).toBe(true);
@@ -61,3 +49,68 @@ describe('block-config', () => {
6149
}
6250
});
6351
});
52+
53+
/**
54+
* Palette coverage ↔ spec `PageComponentType` (#2943).
55+
*
56+
* The previous version of this suite hand-asserted a handful of palette
57+
* EXCLUSIONS (`expect(BLOCK_TYPE_META['element:form']).toBeUndefined()`), which
58+
* locks drift in rather than detecting it: a new spec block type could land and
59+
* simply never reach the palette, with nothing failing. These derive coverage
60+
* from the enum instead, so every value must be an explicit decision —
61+
* offered, or excluded with a documented reason.
62+
*/
63+
describe('page palette ↔ spec PageComponentType coverage', () => {
64+
const specNames: string[] = (() => {
65+
const raw = (PageComponentType as unknown as { options?: readonly string[] }).options;
66+
return Array.isArray(raw) ? [...raw] : [];
67+
})();
68+
69+
it('reads a non-empty enum from the spec', () => {
70+
expect(specNames, 'could not read PageComponentType.options from the spec').not.toEqual([]);
71+
});
72+
73+
it('every spec block type is either offered or explicitly excluded', () => {
74+
const undecided = specNames.filter(
75+
(t) => !(t in (BLOCK_TYPE_META as Record<string, unknown>)) && !(t in PALETTE_EXCLUSIONS),
76+
);
77+
// If this fails: a spec page-block type has no palette decision. Either add
78+
// it to BLOCK_TYPE_META (offered — it needs a renderer!) or to
79+
// PALETTE_EXCLUSIONS with the reason it is unauthorable.
80+
expect(undecided).toEqual([]);
81+
});
82+
83+
it('no type is both offered and excluded', () => {
84+
const both = Object.keys(PALETTE_EXCLUSIONS).filter(
85+
(t) => t in (BLOCK_TYPE_META as Record<string, unknown>),
86+
);
87+
expect(both, 'a block cannot be offered and excluded at once').toEqual([]);
88+
});
89+
90+
it('every exclusion names a real spec type and carries a reason', () => {
91+
for (const [type, reason] of Object.entries(PALETTE_EXCLUSIONS)) {
92+
expect(specNames, `'${type}' is not a spec PageComponentType — stale exclusion`).toContain(type);
93+
expect(reason.length, `'${type}' needs a reason`).toBeGreaterThan(10);
94+
}
95+
});
96+
97+
it('ai:chat_window is not offered — it has no inline renderer', () => {
98+
// The palette used to offer it WITH a config panel while
99+
// `components/renderers/placeholders.tsx` deliberately excluded it to force
100+
// a loud error: an author dragged a block Studio advertised and got a red
101+
// "Unknown component type" box.
102+
expect((BLOCK_TYPE_META as any)['ai:chat_window']).toBeUndefined();
103+
expect(blockHasConfig('ai:chat_window')).toBe(false);
104+
expect(PALETTE_EXCLUSIONS['ai:chat_window']).toBeTruthy();
105+
});
106+
107+
it('a block with a config panel is a block the palette offers', () => {
108+
// A panel for an unauthorable block is how the ai:chat_window
109+
// contradiction stayed invisible. Non-spec objectui blocks (`object-grid`,
110+
// `grid`, …) are exempt — they are palette-native, not PageComponentType.
111+
const orphanPanels = Object.keys(BLOCK_CONFIG).filter(
112+
(t) => specNames.includes(t) && !(t in (BLOCK_TYPE_META as Record<string, unknown>)),
113+
);
114+
expect(orphanPanels, 'these expose a config panel but cannot be authored').toEqual([]);
115+
});
116+
});

packages/app-shell/src/views/metadata-admin/previews/block-config.ts

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -349,10 +349,9 @@ export const BLOCK_CONFIG: Record<string, BlockPropField[]> = {
349349
],
350350

351351
// ── AI ────────────────────────────────────────────────────────────────────
352-
'ai:chat_window': [
353-
{ name: 'agentName', label: 'Agent', kind: 'text', placeholder: 'agent name' },
354-
{ name: 'placeholder', label: 'Input placeholder', kind: 'text' },
355-
],
352+
// No `ai:chat_window` panel: the block is not in the palette (no inline
353+
// renderer — see block-types.ts PALETTE_EXCLUSIONS, #2943). A config panel
354+
// for an unauthorable block is how the contradiction stayed invisible.
356355
'ai:input': [
357356
{ name: 'agentName', label: 'Agent', kind: 'text', placeholder: 'agent name' },
358357
{ name: 'placeholder', label: 'Input placeholder', kind: 'text' },

packages/app-shell/src/views/metadata-admin/previews/block-types.ts

Lines changed: 35 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ export type BlockTypeId =
6464
// global:*
6565
| 'global:search'
6666
// ai:*
67-
| 'ai:chat_window' | 'ai:suggestion'
67+
| 'ai:suggestion'
6868
// element:*
6969
| 'element:text' | 'element:number' | 'element:image' | 'element:divider'
7070
| 'element:button' | 'element:definition-list' | 'element:repeater';
@@ -113,7 +113,12 @@ export const BLOCK_TYPE_META: Record<BlockTypeId, Omit<BlockTypeMeta, 'id'>> = {
113113
'global:search': { label: 'Global search', category: 'navigation', Icon: Search },
114114

115115
// AI
116-
'ai:chat_window': { label: 'AI chat window', category: 'ai', Icon: Bot },
116+
// `ai:chat_window` is deliberately NOT in the palette: the floating chat
117+
// overlay (plugin-chatbot) is the canonical entry point and there is no
118+
// inline page-level renderer — `components/renderers/placeholders.tsx`
119+
// documents that exclusion. The palette used to offer it (with a config
120+
// panel), so an author dragged a block Studio advertised and got a red
121+
// "Unknown component type" box (#2943). See PALETTE_EXCLUSIONS.
117122
'ai:suggestion': { label: 'AI suggestion', category: 'ai', Icon: Sparkles },
118123

119124
// Elements
@@ -126,6 +131,34 @@ export const BLOCK_TYPE_META: Record<BlockTypeId, Omit<BlockTypeMeta, 'id'>> = {
126131
'element:repeater': { label: 'Repeater', category: 'element', Icon: Rows3 },
127132
};
128133

134+
/**
135+
* Every spec `PageComponentType` the page palette deliberately does NOT offer,
136+
* each with the reason it is unauthorable. This is the inverse of the old
137+
* guard (#2943): `block-config.test.ts` used to hand-assert a few exclusions,
138+
* which locks drift IN — a new spec block type could land and simply never
139+
* appear in the palette, unnoticed. The test now derives coverage from
140+
* `PageComponentType` and requires every value to be either in
141+
* {@link BLOCK_TYPE_META} or listed here, so a new one fails until someone
142+
* decides about it.
143+
*
144+
* A palette entry with no renderer is the failure this closes: `ai:chat_window`
145+
* was offered WITH a config panel while `placeholders.tsx` deliberately
146+
* excluded it to force a loud error — an author dragged a block Studio
147+
* advertised and got a red box.
148+
*/
149+
export const PALETTE_EXCLUSIONS: Record<string, string> = {
150+
// Shell singletons — chrome the app shell owns, not page content.
151+
'app:launcher': 'shell singleton — the app shell renders it, not a page',
152+
'global:notifications': 'shell singleton — lives in the app shell header',
153+
'user:profile': 'shell singleton — lives in the app shell header',
154+
// No renderer, by decision.
155+
'ai:chat_window': 'no inline renderer — the floating chat overlay (plugin-chatbot) is canonical',
156+
'element:filter': 'no renderer — list surfaces own filtering (userFilters / filter builder)',
157+
'element:form': 'no renderer — use the object-bound `object-form` block',
158+
'element:record_picker': 'no renderer — record picking is a field widget, not a page block',
159+
'element:text_input': 'no renderer — bare inputs belong to a form, not a page block',
160+
};
161+
129162
export const CATEGORY_LABEL_EN: Record<BlockCategory, string> = {
130163
data: 'Data',
131164
layout: 'Layout',
Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
/**
2+
* ObjectUI
3+
* Copyright (c) 2024-present ObjectStack Inc.
4+
*
5+
* This source code is licensed under the MIT license found in the
6+
* LICENSE file in the root directory of this source tree.
7+
*/
8+
9+
/**
10+
* Palette-offered page blocks render in EVERY host (#2943).
11+
*
12+
* Placeholder registration was entirely opt-in (`registerPlaceholders()`), and
13+
* only `apps/console` calls it. So `nav:menu`, `nav:breadcrumb`,
14+
* `global:search` and `ai:suggestion` — blocks the Studio palette offers —
15+
* rendered `SchemaRenderer`'s red `role="alert"` panel in every other host,
16+
* dumping the page JSON, purely because that host skipped an optional
17+
* bootstrap.
18+
*
19+
* These four now register eagerly through the `./renderers` barrel. The rest of
20+
* the protocol vocabulary stays opt-in on purpose: a genuinely missing renderer
21+
* must keep failing loudly so the misconfiguration gets fixed at the source.
22+
*/
23+
import { describe, it, expect, beforeAll } from 'vitest';
24+
import '@testing-library/jest-dom';
25+
import { ComponentRegistry } from '@object-ui/core';
26+
import { PALETTE_PLACEHOLDER_BLOCKS } from '../renderers/placeholders';
27+
28+
beforeAll(async () => {
29+
// Import the barrel ONLY — never `registerPlaceholders()`, which is what
30+
// this test exists to prove is not required for these blocks.
31+
await import('../renderers');
32+
}, 30000);
33+
34+
describe('palette-offered blocks render without the optional bootstrap', () => {
35+
it('declares a non-empty eager set', () => {
36+
expect(PALETTE_PLACEHOLDER_BLOCKS.length).toBeGreaterThan(0);
37+
});
38+
39+
it.each(PALETTE_PLACEHOLDER_BLOCKS)('%s resolves to a renderer', (type) => {
40+
expect(
41+
ComponentRegistry.get(type),
42+
`${type} is offered by the Studio palette — it must not red-box in a host that skips registerPlaceholders()`,
43+
).toBeTruthy();
44+
});
45+
46+
it('keeps the rest of the protocol vocabulary opt-in (loud by design)', () => {
47+
// A sample of PROTOCOL_COMPONENTS members outside the eager set: these are
48+
// NOT palette blocks, and a page referencing one is a misconfiguration that
49+
// should stay loud.
50+
for (const type of ['view:kanban', 'widget:heatmap', 'field:duration']) {
51+
expect(
52+
ComponentRegistry.get(type),
53+
`${type} must stay opt-in — auto-registering it would mask a missing renderer`,
54+
).toBeFalsy();
55+
}
56+
});
57+
58+
it('ai:chat_window stays unregistered — the exclusion is deliberate', () => {
59+
// The floating chat overlay (plugin-chatbot) is canonical; #2943 pruned the
60+
// palette entry rather than adding a placeholder for it.
61+
expect(ComponentRegistry.get('ai:chat_window')).toBeFalsy();
62+
});
63+
});

packages/components/src/renderers/index.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,3 +16,7 @@ import './overlay';
1616
import './disclosure';
1717
import './complex';
1818
import './action';
19+
// Registers placeholders for the page blocks the Studio palette offers but no
20+
// package renders yet, so they don't red-box in hosts that skip the optional
21+
// `registerPlaceholders()` bootstrap (#2943). The rest stay opt-in.
22+
import './placeholders';

packages/components/src/renderers/placeholders.tsx

Lines changed: 35 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -102,11 +102,40 @@ const PROTOCOL_COMPONENTS = [
102102
'ai:input', 'ai:suggestion', 'ai:feedback'
103103
];
104104

105+
/**
106+
* Page blocks the Studio palette OFFERS but no package renders yet.
107+
*
108+
* These are registered eagerly (see the side-effect call below, reached through
109+
* the `./renderers` barrel) rather than waiting for a host to opt in via
110+
* {@link registerPlaceholders} — which only `apps/console` does. A block Studio
111+
* advertises must not render a red "Unknown component type" panel in every
112+
* other host just because that host didn't call an optional bootstrap (#2943).
113+
*
114+
* Deliberately NARROW: the rest of {@link PROTOCOL_COMPONENTS} stays opt-in so
115+
* a genuinely missing renderer keeps failing loudly, which is what makes the
116+
* misconfiguration get fixed at the source (the same reasoning that keeps
117+
* `ai:chat_window` out of this file entirely — and, since #2943, out of the
118+
* palette too).
119+
*/
120+
export const PALETTE_PLACEHOLDER_BLOCKS = [
121+
'nav:menu', 'nav:breadcrumb', 'global:search', 'ai:suggestion',
122+
];
123+
124+
/** Register one placeholder, never overwriting a real implementation. */
125+
function registerPlaceholder(type: string) {
126+
if (!ComponentRegistry.get(type)) {
127+
ComponentRegistry.register(type, PlaceholderRenderer, { namespace: 'protocol-placeholder' });
128+
}
129+
}
130+
131+
/**
132+
* Register placeholders for the whole protocol vocabulary. Opt-in: hosts that
133+
* want every declared component to render *something* (the console's preview
134+
* gallery) call this; others keep the loud unknown-type error.
135+
*/
105136
export function registerPlaceholders() {
106-
PROTOCOL_COMPONENTS.forEach(type => {
107-
// Only register if not already registered (to avoid overwriting real implementations)
108-
if (!ComponentRegistry.get(type)) {
109-
ComponentRegistry.register(type, PlaceholderRenderer, { namespace: 'protocol-placeholder' });
110-
}
111-
});
137+
PROTOCOL_COMPONENTS.forEach(registerPlaceholder);
112138
}
139+
140+
// Palette-offered page blocks render everywhere, in every host (see above).
141+
PALETTE_PLACEHOLDER_BLOCKS.forEach(registerPlaceholder);

0 commit comments

Comments
 (0)