From 79cf70de81aed547e52fd159131c3d6df429a4bc Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 19 Sep 2026 07:34:02 +0000 Subject: [PATCH 1/3] fix(plugin-kanban,plugin-timeline,plugin-detail): refuse a non-positive row cap before it reaches `$top` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three read points spelled their row cap `?? DEFAULT`. `??` rejects only `null` and `undefined`, so a value `@objectstack/spec` declares refused (every one of these members is `z.number().int().positive()`) was not nullish and survived as a real fetch window: `$top: 0` left for the adapter, the block asked the server for nothing, and the empty board / rail / card named no cause. All three now go through one resolver per site, mirroring the shape objectui#9853 landed on `ObjectGrid` and objectui#9897 repeated on `ListView` — one resolver at every entry is what keeps the answer single. Fail-soft: the value is dropped, the site's own default is used, and one conditional, deduped `console.warn` names the block, the object and the value. The repair is at the READ POINT because each block has two entrances into one key: a `dataSource` binding lowers a named view's `pagination.pageSize` into `schema.limit`, and a block with no binding at all carries the authored `limit` straight through. A repair at the lowering layer closes only the first. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_018HrVaotisyhgmot9o2MLRq --- .changeset/9925-row-cap-non-positive.md | 39 +++ ...nce-rail.rowLimitNonPositive-9925.test.tsx | 248 +++++++++++++++++ .../src/renderers/record-reference-rail.tsx | 109 +++++++- packages/plugin-kanban/src/ObjectKanban.tsx | 88 +++++- .../rowLimitNonPositive-9925.test.tsx | 262 ++++++++++++++++++ .../plugin-timeline/src/ObjectTimeline.tsx | 81 +++++- .../rowLimitNonPositive-9925.test.tsx | 260 +++++++++++++++++ 7 files changed, 1081 insertions(+), 6 deletions(-) create mode 100644 .changeset/9925-row-cap-non-positive.md create mode 100644 packages/plugin-detail/src/renderers/__tests__/record-reference-rail.rowLimitNonPositive-9925.test.tsx create mode 100644 packages/plugin-kanban/src/__tests__/rowLimitNonPositive-9925.test.tsx create mode 100644 packages/plugin-timeline/src/__tests__/rowLimitNonPositive-9925.test.tsx diff --git a/.changeset/9925-row-cap-non-positive.md b/.changeset/9925-row-cap-non-positive.md new file mode 100644 index 0000000000..924a0f60e2 --- /dev/null +++ b/.changeset/9925-row-cap-non-positive.md @@ -0,0 +1,39 @@ +--- +'@object-ui/plugin-kanban': patch +'@object-ui/plugin-timeline': patch +'@object-ui/plugin-detail': patch +--- + +Refuse a row cap the contract already refuses before it reaches `$top`, at the +three read points that still forwarded one — `object-kanban`, `object-timeline` +and each `record:reference_rail` entry (objectui#9925). + +Each of these blocks spelled its row cap as a bare `?? DEFAULT`. `??` rejects +only `null` and `undefined`, so a value the contract refuses was not nullish and +survived as a real fetch window: it reached the adapter as `$top: 0`, the block +asked the server for nothing, and the empty board / empty rail / empty card +named no cause. A negative went out the same way, and a non-integer became a +fractional window. + +All three now go through one resolver per site, mirroring the shape objectui#9853 +landed on `ObjectGrid` and objectui#9897 repeated on `ListView` — one resolver at +every entry is what keeps the answer single. A refused value is dropped, the +site's own default is used, and one `console.warn` names the block, the object +and the value. The warning is conditional and deduped: an absent `limit` and a +usable one both stay silent, and one declaration warns once rather than once per +render. + +This closes BOTH entrances into these blocks, which is why the repair is at the +read point. Each block reads one key, and two authoring shapes fill it: a +`dataSource` binding lowers a named view's `pagination.pageSize` into it, and a +block with no binding at all carries the authored `limit` straight through. A +repair at the lowering layer would close only the first. + +Refusing these is not a renderer choosing a meaning. `@objectstack/spec` declares +every one of these members a positive integer — `object-kanban`'s `limit`, the +element data source `limit` a binding lowers into `object-timeline`'s, and the +rail entry's own `limit` on `ReferenceRailEntrySchema`. + +⛔ No fallback literal changed. Each site keeps the default it already +documented; the rail's was spelled inline twice and is now one named constant so +its two read points cannot drift. diff --git a/packages/plugin-detail/src/renderers/__tests__/record-reference-rail.rowLimitNonPositive-9925.test.tsx b/packages/plugin-detail/src/renderers/__tests__/record-reference-rail.rowLimitNonPositive-9925.test.tsx new file mode 100644 index 0000000000..7181b03781 --- /dev/null +++ b/packages/plugin-detail/src/renderers/__tests__/record-reference-rail.rowLimitNonPositive-9925.test.tsx @@ -0,0 +1,248 @@ +/** + * ObjectUI + * Copyright (c) 2024-present ObjectStack Inc. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +/** + * objectui#9925 — `record:reference_rail` spent each entry's preview-row cap + * with a bare `??`, and `??` rejects only `null`/`undefined`, so a value the + * contract refuses survived as a real fetch window and reached the adapter as + * `$top`. + * + * ## Why this site is in the card at all + * + * The card names three blocks and derives its population by CONCEPT, not by one + * identifier. Inside this package the concept turns up at five read points, and + * four of them already refuse a non-positive value before it reaches `$top` + * (`record:related_list` tests the number is positive; `record:history` and + * `record:activity` floor theirs). This entry was the one that did not, so it + * is the one this file pins. + * + * ## Why "refuse it" is not this file inventing a meaning + * + * `@objectstack/spec` already answers what `limit: 0` means: the rail entry's + * own member is declared a POSITIVE INTEGER on `ReferenceRailEntrySchema` + * (`z.number().int().positive().optional()`, described there as the `$top` of + * the one query the entry issues). So `0` is not a spelling whose meaning a + * consumer may choose; it is a value the contract refuses. + * + * ## Why the silence matters more here than at the two sibling sites + * + * This rail degrades silently by design — a failed or empty entry renders "—" + * rather than blanking the rail — so an entry asked for nothing draws a card + * that looks merely empty. There is no on-screen channel to say it on, which is + * why the diagnostic assertions below are not decoration. + * + * ## What the assertions are, and what each control buys + * + * The subject is the RELATION, never a literal: a refused value does not reach + * `$top` and the site's own default does. Each refusal is paired with a control + * that must NOT fire, so a rail that ignores the member entirely cannot pass + * for a measurement and an always-on diagnostic cannot pass for a diagnosis. + */ +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import { render, cleanup, waitFor } from '@testing-library/react'; +import '@testing-library/jest-dom'; +import React from 'react'; +import { MemoryRouter } from 'react-router-dom'; +import { RecordContextProvider } from '@object-ui/react'; + +import { + RecordReferenceRailRenderer, + DEFAULT_REFERENCE_RAIL_LIMIT, +} from '../record-reference-rail'; + +/** + * The rail gates its queries on an IntersectionObserver. Report intersecting + * immediately so the fetch effect runs deterministically under jsdom. + */ +class ImmediateIO { + constructor(private cb: (records: { isIntersecting: boolean }[]) => void) {} + observe() { this.cb([{ isIntersecting: true }]); } + disconnect() {} + unobserve() {} +} + +/** The three values `??` and the resolver DISAGREE about. */ +const REFUSED = [0, -5, 2.5]; + +const makeDataSource = () => ({ + find: vi.fn(async () => ({ data: [{ id: 'c1', name: 'Ada' }], total: 1 })), +}); + +const railWith = (limit: unknown) => ({ + hideEmpty: false, + entries: [ + { + objectName: 'contact', + relationshipField: 'account_id', + title: 'Contacts', + ...(limit === undefined ? {} : { limit }), + }, + ], +}); + +const renderRail = (schema: Record, dataSource: any) => + render( + + + + + , + ); + +const tops = async (ds: ReturnType) => { + await waitFor(() => expect(ds.find).toHaveBeenCalled()); + return ds.find.mock.calls.map((c: any[]) => (c[1] as any)?.$top); +}; + +let warnings: string[] = []; +beforeEach(() => { + warnings = []; + vi.stubGlobal('IntersectionObserver', ImmediateIO as unknown as typeof IntersectionObserver); + vi.spyOn(console, 'warn').mockImplementation((...args: unknown[]) => { + warnings.push(args.map(String).join(' ')); + }); +}); +afterEach(() => { + cleanup(); + vi.unstubAllGlobals(); + vi.restoreAllMocks(); +}); + +const rowCapWarnings = () => warnings.filter((w) => w.includes('RecordReferenceRail row cap')); + +describe('record:reference_rail — a row cap the contract refuses never reaches the wire (objectui#9925)', () => { + it.each(REFUSED)('never sends `$top: %s`', async (bad) => { + const ds = makeDataSource(); + renderRail(railWith(bad), ds); + + const sent = await tops(ds); + expect(sent.length).toBeGreaterThan(0); + expect(sent, `the authored ${bad} reached the wire`).not.toContain(bad); + for (const top of sent) { + expect(typeof top).toBe('number'); + expect(Number.isInteger(top)).toBe(true); + expect(top).toBeGreaterThan(0); + } + }); + + it.each(REFUSED)('falls back to this entry’s OWN default instead of %s', async (bad) => { + const ds = makeDataSource(); + renderRail(railWith(bad), ds); + + const sent = await tops(ds); + // The relation, not the literal: whatever this site documents as its + // default is what a refused declaration falls back to. + for (const top of sent) expect(top).toBe(DEFAULT_REFERENCE_RAIL_LIMIT); + }); + + it('CONTROL — a legitimate authored limit still reaches `$top` unchanged', async () => { + const ds = makeDataSource(); + renderRail(railWith(7), ds); + + const sent = await tops(ds); + // Without this row, "never 0" is satisfied by a rail that ignores the + // member entirely and always sends its own default. + expect(sent).toContain(7); + expect(sent).not.toContain(DEFAULT_REFERENCE_RAIL_LIMIT); + }); + + it('CONTROL — declaring no limit at all still sends the default', async () => { + const ds = makeDataSource(); + renderRail(railWith(undefined), ds); + + const sent = await tops(ds); + expect(sent).toContain(DEFAULT_REFERENCE_RAIL_LIMIT); + }); + + it('CONTROL — the entry’s parent scope still arrives when its limit is refused', async () => { + // ⛔ No capability removed: refusing one member must not cost the query the + // entry exists to issue. + const ds = makeDataSource(); + renderRail(railWith(0), ds); + + await waitFor(() => expect(ds.find).toHaveBeenCalled()); + const [object, params] = ds.find.mock.calls[0] as unknown as [string, any]; + expect(object).toBe('contact'); + expect(params.$filter).toEqual({ account_id: 'A1' }); + expect(params.$count).toBe(true); + }); + + it('refuses ONE entry without disarming its neighbour', async () => { + // A rail is a list, and the resolver runs per entry: a refused entry must + // not take the legitimate one's window with it, in either direction. + const ds = makeDataSource(); + renderRail( + { + hideEmpty: false, + entries: [ + { objectName: 'contact', relationshipField: 'account_id', limit: 0 }, + { objectName: 'task', relationshipField: 'account_id', limit: 7 }, + ], + }, + ds, + ); + + await waitFor(() => expect(ds.find).toHaveBeenCalledTimes(2)); + const byObject = new Map( + ds.find.mock.calls.map((c: any[]) => [c[0] as string, (c[1] as any)?.$top]), + ); + expect(byObject.get('contact')).toBe(DEFAULT_REFERENCE_RAIL_LIMIT); + expect(byObject.get('task')).toBe(7); + }); + + // ── THE DIAGNOSTIC ─────────────────────────────────────────────────────── + it.each(REFUSED)('says so, naming the block, the entry’s object and the value %s', async (bad) => { + const ds = makeDataSource(); + renderRail(railWith(bad), ds); + + await waitFor(() => expect(rowCapWarnings().length).toBeGreaterThan(0)); + const message = rowCapWarnings()[0]; + // This rail is silent on screen by construction, so substituting a number + // the author never wrote would otherwise be entirely unobservable. + expect(message).toContain('record:reference_rail'); + expect(message).toContain('contact'); + expect(message).toContain('limit'); + expect(message).toContain(String(bad)); + }); + + it('fires exactly ONCE for one declaration, across re-renders', async () => { + const ds = makeDataSource(); + const { rerender } = renderRail(railWith(0), ds); + + await waitFor(() => expect(rowCapWarnings().length).toBeGreaterThan(0)); + // A fresh schema OBJECT carrying the same declaration. The effect is keyed + // on the declaration's CONTENT and deduped per (object, value), so this + // must say nothing more. + rerender( + + + + + , + ); + await waitFor(() => expect(ds.find).toHaveBeenCalled()); + expect(rowCapWarnings()).toHaveLength(1); + }); + + it('CONTROL — a legitimate limit produces no such diagnostic', async () => { + const ds = makeDataSource(); + renderRail(railWith(7), ds); + + await waitFor(() => expect(ds.find).toHaveBeenCalled()); + // An always-on marker states nothing. + expect(rowCapWarnings()).toHaveLength(0); + }); + + it('CONTROL — declaring no limit at all produces no diagnostic', async () => { + const ds = makeDataSource(); + renderRail(railWith(undefined), ds); + + await waitFor(() => expect(ds.find).toHaveBeenCalled()); + expect(rowCapWarnings()).toHaveLength(0); + }); +}); diff --git a/packages/plugin-detail/src/renderers/record-reference-rail.tsx b/packages/plugin-detail/src/renderers/record-reference-rail.tsx index 6d3e1bab08..e843786886 100644 --- a/packages/plugin-detail/src/renderers/record-reference-rail.tsx +++ b/packages/plugin-detail/src/renderers/record-reference-rail.tsx @@ -144,6 +144,79 @@ interface EntryState { error?: string; } +/** + * Preview rows fetched for one entry when the author declared no `limit`. + * + * Named rather than spelled inline because it was spelled TWICE — once in the + * `$top` the entry's query carries and once in the fetch signature that decides + * whether to re-issue it — and objectui#9925 gave both a resolver, which needs + * one fallback to agree on. + */ +export const DEFAULT_REFERENCE_RAIL_LIMIT = 3; + +/** + * What the contract admits as a preview-row cap for a rail entry. + * + * `@objectstack/spec` has already answered what `limit: 0` means: this entry's + * own member is declared a POSITIVE INTEGER on `ReferenceRailEntrySchema` + * (`z.number().int().positive().optional()`, described there as "Preview rows + * per card, and the `$top` of the one query this entry issues"). So `0` is not + * a spelling whose meaning this renderer may choose; it is a value the contract + * refuses. + */ +function isUsableRowLimit(value: unknown): value is number { + return typeof value === 'number' && Number.isInteger(value) && value > 0; +} + +/** + * The ONE resolver for a rail entry's row cap, for the reason objectui#9853 + * gave when it landed the same shape on `ObjectGrid` and objectui#9897 repeated + * on `ListView`: one resolver at every entry is what keeps the answer single. + * + * Before objectui#9925 both read points were a bare `entry.limit ?? 3`, and + * `??` rejects only `null` and `undefined` — so an authored `limit: 0` was not + * nullish and survived as a real window, reaching the wire as `$top: 0`. This + * rail degrades silently by design (a failed entry renders "—"), so an entry + * asked for nothing rendered an empty card with a zero badge and named no + * cause. A negative goes out the same way. + * + * ⚠️ The refusal is FAIL-SOFT on purpose, as it is at the two sibling sites + * this card repairs: throwing would take out the whole rail over one entry's + * declaration, which is a worse outcome than the defect. The value is dropped, + * this rail's own default is used, and `describeRefusedRowLimit` states it + * through the developer channel — the only channel available, because the + * suppression this rail already does is silent on screen by construction. + * ⛔ Not a silent clamp, and ⛔ not a clamp to 1. + */ +function resolveRowLimit(authored: unknown, fallback: number): number { + return isUsableRowLimit(authored) ? authored : fallback; +} + +/** + * The diagnostic half. `null` means "nothing to say" — an absent `limit` is not + * a mistake, and a usable one is not either, so the message is CONDITIONAL and + * the silence controls in the pin are what keep it from being an always-on + * marker that states nothing. + * + * ⛔ NOT a second guard: the predicate lives once, in `isUsableRowLimit`, and + * this reads it. Two predicates would be free to drift, and the drift would be + * invisible — a value refused by one and admitted by the other. + */ +function describeRefusedRowLimit(authored: unknown, objectName: unknown): string | null { + if (authored === undefined || authored === null) return null; + if (isUsableRowLimit(authored)) return null; + const where = + typeof objectName === 'string' && objectName + ? `record:reference_rail entry for ${objectName}` + : 'record:reference_rail entry'; + return ( + `[ObjectUI] RecordReferenceRail row cap: ${where} declared limit: ${String(authored)}, ` + + 'which is not a positive integer. A row cap must be a positive integer ' + + '(the spec refuses zero and negative values), so it was ignored and this ' + + `entry fell back to its default preview-row cap (${DEFAULT_REFERENCE_RAIL_LIMIT}).` + ); +} + const humanize = (s: string) => s .replace(/[_-]+/g, ' ') @@ -258,7 +331,39 @@ export const RecordReferenceRailRenderer: React.FC>(new Set()); - const entriesSig = JSON.stringify(entries.map((e) => `${e.objectName}:${e.relationshipField}:${e.limit ?? 3}`)); + // [objectui#9925] One warning per (object, refused value) per mounted rail — + // the same dedupe shape as the link suppression above, and for the same + // reason: an entry that asked for nothing is silent on screen by + // construction, so the developer channel is the only place it can be said. + // Fired from an effect, never from render, and keyed on the DECLARATION so a + // re-render with the same authored value says nothing a second time. + const warnedRefusedLimits = React.useRef>(new Set()); + + // [objectui#9925] Through the resolver, so this signature names the window + // that actually leaves — two entries whose refused `limit`s differ (`0` and + // `-5`) issue the SAME query and must not read as two different fetches. + const entriesSig = JSON.stringify( + entries.map( + (e) => + `${e.objectName}:${e.relationshipField}:${resolveRowLimit(e.limit, DEFAULT_REFERENCE_RAIL_LIMIT)}`, + ), + ); + // [objectui#9925] The DECLARATION, kept apart from the signature above: the + // diagnostic has to re-fire when the authored value changes even though the + // resolved window does not, which is exactly the pair the resolver collapses. + const authoredLimitSig = JSON.stringify(entries.map((e) => [e.objectName, e.limit ?? null])); + React.useEffect(() => { + for (const entry of entries) { + const message = describeRefusedRowLimit(entry.limit, entry.objectName); + if (!message) continue; + const key = `${entry.objectName}:${String(entry.limit)}`; + if (warnedRefusedLimits.current.has(key)) continue; + warnedRefusedLimits.current.add(key); + console.warn(message); + } + // eslint-disable-next-line react-hooks/exhaustive-deps -- `entries` is tracked by CONTENT (`authoredLimitSig`), the way the fetch effect below tracks it by `entriesSig`; an inline array on a schema node is a new object every render. + }, [authoredLimitSig]); + React.useEffect(() => { if (!railVisible) return; if (!dataSource?.find || !parentId || entries.length === 0) return; @@ -290,7 +395,7 @@ export const RecordReferenceRailRenderer: React.FC = { */ export const DEFAULT_KANBAN_LIMIT = 100; +/** + * What the contract admits as a row cap for this board. + * + * `@objectstack/spec` has already answered what `limit: 0` means. The + * `object-kanban` props declare the member a POSITIVE INTEGER + * (`z.number().int().positive().optional()`, described there as the row cap + * "lowered to the query's top-level `$top`"), and the element data source + * `limit` that a `dataSource` binding lowers into this SAME key is declared + * positive as well. So `0` is not a spelling whose meaning this renderer may + * choose; it is a value the contract refuses, and a renderer that forwards it + * to the wire is the only party not saying so. + */ +function isUsableRowLimit(value: unknown): value is number { + return typeof value === 'number' && Number.isInteger(value) && value > 0; +} + +/** + * The ONE resolver for this board's row cap, for the reason objectui#9853 gave + * when it landed the same shape on `ObjectGrid` and objectui#9897 repeated on + * `ListView`: one resolver at every entry is what keeps the answer single. + * + * Before objectui#9925 this read was a bare `schema.limit ?? DEFAULT_KANBAN_LIMIT`, + * and `??` rejects only `null` and `undefined` — so an authored `limit: 0` was + * not nullish and survived as a real window. It reached the wire as `$top: 0`, + * the board asked the server for nothing, and the empty board named no cause. + * A negative goes out the same way. Both ENTRANCES converge on this key: a + * `dataSource` binding lowers a view's `pagination.pageSize` into `schema.limit` + * before this component sees it, and a board with no binding at all reads the + * authored `limit` from the same place — so resolving HERE covers both, which a + * repair at the lowering layer could not. + * + * ⚠️ The refusal is FAIL-SOFT on purpose. Throwing would take out the whole + * board over one declaration, which is a worse outcome than the defect. The + * value is dropped, this board's own default is used, and + * `describeRefusedRowLimit` states it once through the channel this component + * already uses for "you declared it, the renderer dropped it". ⛔ Not a silent + * clamp: without the loud half this is a substitution the author cannot see, + * and ⛔ not a clamp to 1 either — the author's number is not repaired, it is + * refused, and the board falls back to the window it documents. + */ +function resolveRowLimit(authored: unknown, fallback: number): number { + return isUsableRowLimit(authored) ? authored : fallback; +} + +/** + * The diagnostic half. `null` means "nothing to say" — an absent `limit` is not + * a mistake, and a usable one is not either, so the message is CONDITIONAL and + * the silence controls in the pin are what keep it from being an always-on + * marker that states nothing. + * + * ⛔ NOT a second guard: the predicate lives once, in `isUsableRowLimit`, and + * this reads it. Two predicates would be free to drift, and the drift would be + * invisible — a value refused by one and admitted by the other. + */ +function describeRefusedRowLimit(authored: unknown, objectName: unknown): string | null { + if (authored === undefined || authored === null) return null; + if (isUsableRowLimit(authored)) return null; + const where = + typeof objectName === 'string' && objectName + ? `object-kanban on ${objectName}` + : 'object-kanban'; + return ( + `[ObjectUI] ObjectKanban row cap: ${where} declared limit: ${String(authored)}, ` + + 'which is not a positive integer. A row cap must be a positive integer ' + + '(the spec refuses zero and negative values), so it was ignored and this ' + + `board fell back to its default row cap (${DEFAULT_KANBAN_LIMIT}).` + ); +} + /** * Safe wrapper for useObjectTranslation that falls back to the English defaults * above when no `I18nProvider` is mounted (standalone board, tests). @@ -475,6 +544,16 @@ export const ObjectKanban: React.FC = ({ } }, [externalLoading, hasExternalData]); + // [objectui#9925] The loud half of the row-cap refusal, on the channel this + // component already uses for "you declared it, the renderer dropped it". + // Keyed on the DECLARATION, so it is one warning per declaration rather than + // one per render — and it fires from an effect, never from render, so a + // re-render with the same authored value says nothing a second time. + useEffect(() => { + const message = describeRefusedRowLimit(schema.limit, schema.objectName); + if (message) console.warn(message); + }, [schema.limit, schema.objectName]); + useEffect(() => { // Skip internal fetch when data is managed by a parent component if (hasExternalData) return; @@ -578,15 +657,18 @@ export const ObjectKanban: React.FC = ({ // the saturation reading below (objectui#8307) compares the row // count against `query.$top` — the very number this request // carried. One spelling of the window, read back from the request - // itself: a second `schema.limit ?? DEFAULT_KANBAN_LIMIT` kept in a + // itself: a second `resolveRowLimit(schema.limit, …)` kept in a // local for the comparison could drift from the one on the wire, // and a marker computed against a window the server was never asked - // for is exactly the silent wrongness objectui#8307 is about. + // for is exactly the silent wrongness objectui#8307 is about. That + // reasoning is why objectui#9925's refusal was put INSIDE this + // named object rather than beside it: the resolver runs once, and + // the saturation reading keeps reading the number that left. // Keeping it inline here also keeps the spelling objectui#7322 // pins off disk (`object-kanban-group-by-limit-7322.test.ts`). const query = { $filter: schema.filter, - $top: schema.limit ?? DEFAULT_KANBAN_LIMIT, + $top: resolveRowLimit(schema.limit, DEFAULT_KANBAN_LIMIT), ...(expand.length > 0 ? { $expand: expand } : {}), }; const results = await dataSource.find(schema.objectName, query); diff --git a/packages/plugin-kanban/src/__tests__/rowLimitNonPositive-9925.test.tsx b/packages/plugin-kanban/src/__tests__/rowLimitNonPositive-9925.test.tsx new file mode 100644 index 0000000000..5ac0198fd2 --- /dev/null +++ b/packages/plugin-kanban/src/__tests__/rowLimitNonPositive-9925.test.tsx @@ -0,0 +1,262 @@ +/** + * ObjectUI + * Copyright (c) 2024-present ObjectStack Inc. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +/** + * objectui#9925 — `object-kanban` spent its row cap with a bare `??`, and `??` + * rejects only `null`/`undefined`, so a value the contract refuses survived as + * a real fetch window and reached the adapter as `$top`. + * + * ## Why "refuse it" is not this file inventing a meaning + * + * `@objectstack/spec` already answers what `limit: 0` means. The + * `object-kanban` props declare the member a POSITIVE INTEGER + * (`z.number().int().positive().optional()`), and the element data source + * `limit` a `dataSource` binding lowers into this same key is declared positive + * as well. So `0` is not a spelling whose meaning a consumer may choose; it is + * a value the contract refuses. + * + * ## BOTH entrances, because there are two and only one of them has a gate + * + * The board reads ONE key, `schema.limit`, and two different authoring shapes + * fill it: a `dataSource` binding lowers a named view's `pagination.pageSize` + * into it before this component sees it, and a board with NO binding at all + * carries the authored `limit` straight through. A repair at the lowering layer + * closes only the first. Every refusal row below is therefore run twice, once + * per entrance, and the two are asserted separately rather than in one loop + * whose failure would not say which entrance broke. + * + * ## What the assertions are, and what each control buys + * + * The subject is the RELATION, never a literal: a refused value does not reach + * `$top` and the site's own default does. Each refusal is paired with a control + * that must NOT fire, so a board that simply ignores the member and always + * sends its default cannot pass for a measurement, and an always-on diagnostic + * cannot pass for a diagnosis. + */ +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import { render, cleanup, waitFor } from '@testing-library/react'; +import React from 'react'; +import { SchemaRenderer, SchemaRendererProvider } from '@object-ui/react'; +import { DEFAULT_KANBAN_LIMIT } from '../ObjectKanban'; +// Registers `object-kanban` (and the ElementDataSourceGate wiring that carries +// the bound entrance). +import '../index'; +// The lane titles render inside `KanbanRenderer`'s `React.lazy` boundary; +// importing the chunk at module scope bills the cold transform to the import +// phase instead of racing a `waitFor` budget (the objectui#3010 rule). +import '../KanbanImpl'; + +const LANES = [ + { id: 'open', title: 'Open' }, + { id: 'won', title: 'Won' }, +]; + +/** The three values `??` and the resolver DISAGREE about. */ +const REFUSED = [0, -5, 2.5]; + +/** + * A view whose `pagination.pageSize` is what the binding lowers into + * `schema.limit`. Built per test so each row can name its own page size. + */ +const viewWithPageSize = (pageSize: unknown) => ({ + name: 'hot', + label: 'Hot accounts', + columns: ['name', 'rating'], + filter: [['rating', '=', 'hot']], + pagination: { pageSize }, +}); + +function makeAdapter(listViews: Record = {}) { + return { + find: vi.fn().mockResolvedValue({ data: [{ id: '1', name: 'Acme', status: 'open' }] }), + findOne: vi.fn(), + create: vi.fn(), + update: vi.fn(), + delete: vi.fn(), + getObjectSchema: vi.fn().mockResolvedValue({ + name: 'account', + fields: { name: { type: 'text' }, status: { type: 'text' }, rating: { type: 'text' } }, + listViews, + }), + }; +} + +const renderBlock = (schema: Record, adapter: ReturnType) => + render( + + + , + ); + +/** Every `$top` that left this board, in call order. */ +const tops = async (adapter: ReturnType) => { + await waitFor(() => expect(adapter.find).toHaveBeenCalled()); + return adapter.find.mock.calls.map((c: any[]) => c[1]?.$top); +}; + +/** ENTRANCE 1 — authored `limit`, no `dataSource` binding anywhere. */ +const authoredBoard = (limit: unknown) => ({ + type: 'object-kanban', + objectName: 'account', + groupBy: 'status', + columns: LANES, + ...(limit === undefined ? {} : { limit }), +}); + +/** ENTRANCE 2 — no authored `limit`; a bound view's page size fills the key. */ +const boundBoard = () => ({ + type: 'object-kanban', + groupBy: 'status', + columns: LANES, + dataSource: { object: 'account', view: 'hot' }, +}); + +let warnings: string[] = []; +beforeEach(() => { + warnings = []; + vi.spyOn(console, 'warn').mockImplementation((...args: unknown[]) => { + warnings.push(args.map(String).join(' ')); + }); +}); +afterEach(() => { + cleanup(); + vi.restoreAllMocks(); +}); + +const rowCapWarnings = () => warnings.filter((w) => w.includes('ObjectKanban row cap')); + +describe('object-kanban — a row cap the contract refuses never reaches the wire (objectui#9925)', () => { + // ── ENTRANCE 1: AUTHORED, NO BINDING ─────────────────────────────────── + describe('authored `limit`, with no `dataSource` binding at all', () => { + it.each(REFUSED)('never sends `$top: %s`', async (bad) => { + const adapter = makeAdapter(); + renderBlock(authoredBoard(bad), adapter); + + const sent = await tops(adapter); + expect(sent.length).toBeGreaterThan(0); + expect(sent, `the authored ${bad} reached the wire`).not.toContain(bad); + for (const top of sent) { + expect(typeof top).toBe('number'); + expect(Number.isInteger(top)).toBe(true); + expect(top).toBeGreaterThan(0); + } + }); + + it.each(REFUSED)('falls back to this board’s OWN default instead of %s', async (bad) => { + const adapter = makeAdapter(); + renderBlock(authoredBoard(bad), adapter); + + const sent = await tops(adapter); + // The relation, not the literal: whatever this site documents as its + // default is what a refused declaration falls back to. + for (const top of sent) expect(top).toBe(DEFAULT_KANBAN_LIMIT); + }); + + it('CONTROL — a legitimate authored limit still reaches `$top` unchanged', async () => { + const adapter = makeAdapter(); + renderBlock(authoredBoard(7), adapter); + + const sent = await tops(adapter); + // Without this row, "never 0" is satisfied by a board that ignores the + // member entirely and always sends its own default. + expect(sent).toContain(7); + expect(sent).not.toContain(DEFAULT_KANBAN_LIMIT); + }); + + it('CONTROL — declaring no limit at all still sends the default', async () => { + const adapter = makeAdapter(); + renderBlock(authoredBoard(undefined), adapter); + + const sent = await tops(adapter); + expect(sent).toContain(DEFAULT_KANBAN_LIMIT); + }); + }); + + // ── ENTRANCE 2: A BOUND VIEW'S PAGE SIZE ─────────────────────────────── + describe('a bound view’s `pagination.pageSize`, lowered into the same key', () => { + it.each(REFUSED)('never sends `$top: %s`', async (bad) => { + const adapter = makeAdapter({ hot: viewWithPageSize(bad) }); + renderBlock(boundBoard(), adapter); + + const sent = await tops(adapter); + expect(sent.length).toBeGreaterThan(0); + expect(sent, `the view's ${bad} reached the wire`).not.toContain(bad); + for (const top of sent) expect(top).toBe(DEFAULT_KANBAN_LIMIT); + }); + + it('CONTROL — a legitimate page size still becomes the board’s window', async () => { + const adapter = makeAdapter({ hot: viewWithPageSize(7) }); + renderBlock(boundBoard(), adapter); + + const sent = await tops(adapter); + expect(sent).toContain(7); + }); + + it('CONTROL — the view’s filter still arrives when its page size is refused', async () => { + // ⛔ No capability removed: refusing one member must not cost the rest of + // the binding. + const adapter = makeAdapter({ hot: viewWithPageSize(0) }); + renderBlock(boundBoard(), adapter); + + await waitFor(() => expect(adapter.find).toHaveBeenCalled()); + const [object, params] = adapter.find.mock.calls[0] as [string, any]; + expect(object).toBe('account'); + expect(params.$filter).toEqual([['rating', '=', 'hot']]); + }); + }); + + // ── THE DIAGNOSTIC ───────────────────────────────────────────────────── + describe('the diagnostic half', () => { + it.each(REFUSED)('names this block, the object and the value %s', async (bad) => { + const adapter = makeAdapter(); + renderBlock(authoredBoard(bad), adapter); + + await waitFor(() => expect(rowCapWarnings().length).toBeGreaterThan(0)); + const message = rowCapWarnings()[0]; + // Substituting a number the author never wrote is the quieter half of the + // same defect; this is what makes it a diagnosis. + expect(message).toContain('object-kanban'); + expect(message).toContain('account'); + expect(message).toContain('limit'); + expect(message).toContain(String(bad)); + }); + + it('fires exactly ONCE for one declaration, across re-renders', async () => { + const adapter = makeAdapter(); + const { rerender } = renderBlock(authoredBoard(0), adapter); + + await waitFor(() => expect(rowCapWarnings().length).toBeGreaterThan(0)); + // A fresh schema OBJECT carrying the same declaration. The effect is + // keyed on the declared primitives, so this must say nothing more. + rerender( + + + , + ); + await waitFor(() => expect(adapter.find).toHaveBeenCalled()); + expect(rowCapWarnings()).toHaveLength(1); + }); + + it('CONTROL — a legitimate limit produces no such diagnostic', async () => { + const adapter = makeAdapter(); + renderBlock(authoredBoard(7), adapter); + + await waitFor(() => expect(adapter.find).toHaveBeenCalled()); + // An always-on marker states nothing. + expect(rowCapWarnings()).toHaveLength(0); + }); + + it('CONTROL — declaring no limit at all produces no diagnostic', async () => { + const adapter = makeAdapter(); + renderBlock(authoredBoard(undefined), adapter); + + await waitFor(() => expect(adapter.find).toHaveBeenCalled()); + expect(rowCapWarnings()).toHaveLength(0); + }); + }); +}); diff --git a/packages/plugin-timeline/src/ObjectTimeline.tsx b/packages/plugin-timeline/src/ObjectTimeline.tsx index 3a64820ec3..c53f8dc891 100644 --- a/packages/plugin-timeline/src/ObjectTimeline.tsx +++ b/packages/plugin-timeline/src/ObjectTimeline.tsx @@ -28,6 +28,73 @@ import { useTimelineTranslation } from './useTimelineTranslation'; */ export const DEFAULT_TIMELINE_LIMIT = 100; +/** + * What the contract admits as a row cap for this rail. + * + * `@objectstack/spec` has already answered what `limit: 0` means: the element + * data source `limit` that a `dataSource` binding lowers into this key is + * declared a POSITIVE INTEGER (`z.number().int().positive().optional()`), and + * so is the `pagination.pageSize` of a named view that fills it. So `0` is not + * a spelling whose meaning this renderer may choose; it is a value the contract + * refuses, and a renderer that forwards it to the wire is the only party not + * saying so. + */ +function isUsableRowLimit(value: unknown): value is number { + return typeof value === 'number' && Number.isInteger(value) && value > 0; +} + +/** + * The ONE resolver for this rail's row cap, for the reason objectui#9853 gave + * when it landed the same shape on `ObjectGrid` and objectui#9897 repeated on + * `ListView`: one resolver at every entry is what keeps the answer single. + * + * Before objectui#9925 this read was a bare `schema.limit ?? DEFAULT_TIMELINE_LIMIT`, + * and `??` rejects only `null` and `undefined` — so an authored `limit: 0` was + * not nullish and survived as a real window. It reached the wire as `$top: 0`, + * the rail asked the server for nothing, and the empty rail named no cause. A + * negative goes out the same way. Both ENTRANCES converge on this key: a + * `dataSource` binding lowers a view's `pagination.pageSize` into `schema.limit` + * before this component sees it, and a rail with no binding at all reads the + * authored `limit` from the same place — so resolving HERE covers both, which a + * repair at the lowering layer could not. + * + * ⚠️ The refusal is FAIL-SOFT on purpose. Throwing would take out the whole + * rail over one declaration, which is a worse outcome than the defect. The + * value is dropped, this rail's own default is used, and + * `describeRefusedRowLimit` states it once through the channel this component + * already uses for "you declared it, the renderer dropped it" (the same + * `console.warn` the timeline-config parse above writes to). ⛔ Not a silent + * clamp, and ⛔ not a clamp to 1: the author's number is refused, not repaired. + */ +function resolveRowLimit(authored: unknown, fallback: number): number { + return isUsableRowLimit(authored) ? authored : fallback; +} + +/** + * The diagnostic half. `null` means "nothing to say" — an absent `limit` is not + * a mistake, and a usable one is not either, so the message is CONDITIONAL and + * the silence controls in the pin are what keep it from being an always-on + * marker that states nothing. + * + * ⛔ NOT a second guard: the predicate lives once, in `isUsableRowLimit`, and + * this reads it. Two predicates would be free to drift, and the drift would be + * invisible — a value refused by one and admitted by the other. + */ +function describeRefusedRowLimit(authored: unknown, objectName: unknown): string | null { + if (authored === undefined || authored === null) return null; + if (isUsableRowLimit(authored)) return null; + const where = + typeof objectName === 'string' && objectName + ? `object-timeline on ${objectName}` + : 'object-timeline'; + return ( + `[ObjectUI] ObjectTimeline row cap: ${where} declared limit: ${String(authored)}, ` + + 'which is not a positive integer. A row cap must be a positive integer ' + + '(the spec refuses zero and negative values), so it was ignored and this ' + + `timeline fell back to its default row cap (${DEFAULT_TIMELINE_LIMIT}).` + ); +} + /** * The variants an OBJECT-BOUND timeline can render. * @@ -201,6 +268,18 @@ export const ObjectTimeline: React.FC = ({ } }, [schema]); + // [objectui#9925] The loud half of the row-cap refusal, on the same channel + // as the parse warning just above. Keyed on the DECLARATION, so it is one + // warning per declaration rather than one per render — and it fires from an + // effect, never from render, so a re-render with the same authored value says + // nothing a second time. (`TimelineExtensionSchema` above declares no + // `limit`, and it is a non-strict object, so an authored `limit: 0` passed + // that parse in silence; this is the only place it is stated.) + useEffect(() => { + const message = describeRefusedRowLimit(schema.limit, schema.objectName); + if (message) console.warn(message); + }, [schema.limit, schema.objectName]); + const boundData = useDataScope(schema.bind); /** @@ -325,7 +404,7 @@ export const ObjectTimeline: React.FC = ({ const results = await dataSource.find(schema.objectName, { $filter: schema.filter, $orderby: convertSortToQueryParams(schema.sort), - $top: schema.limit ?? DEFAULT_TIMELINE_LIMIT, + $top: resolveRowLimit(schema.limit, DEFAULT_TIMELINE_LIMIT), ...(expand.length > 0 ? { $expand: expand } : {}), }); const data = extractRecords(results); diff --git a/packages/plugin-timeline/src/__tests__/rowLimitNonPositive-9925.test.tsx b/packages/plugin-timeline/src/__tests__/rowLimitNonPositive-9925.test.tsx new file mode 100644 index 0000000000..ef15b0f13b --- /dev/null +++ b/packages/plugin-timeline/src/__tests__/rowLimitNonPositive-9925.test.tsx @@ -0,0 +1,260 @@ +/** + * ObjectUI + * Copyright (c) 2024-present ObjectStack Inc. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +/** + * objectui#9925 — `object-timeline` spent its row cap with a bare `??`, and + * `??` rejects only `null`/`undefined`, so a value the contract refuses + * survived as a real fetch window and reached the adapter as `$top`. + * + * ## Why "refuse it" is not this file inventing a meaning + * + * The element data source `limit` a `dataSource` binding lowers into + * `schema.limit` is declared a POSITIVE INTEGER by `@objectstack/spec` + * (`z.number().int().positive().optional()`), and so is the + * `pagination.pageSize` of a named view that fills it. So `0` is not a spelling + * whose meaning a consumer may choose; it is a value the contract refuses. + * + * ⚠️ This renderer's own `TimelineExtensionSchema.safeParse` is NOT that + * refusal and cannot be read as one: it declares no `limit` member and is a + * non-strict object, so an authored `limit: 0` passed it in silence. The + * control at the foot of this file pins that separation — the parse warning + * and the row-cap warning are different messages on the same channel. + * + * ## BOTH entrances, because there are two and only one of them has a gate + * + * The rail reads ONE key, `schema.limit`, and two authoring shapes fill it: a + * `dataSource` binding lowers a named view's `pagination.pageSize` into it, and + * a rail with NO binding at all carries the authored `limit` straight through. + * A repair at the lowering layer closes only the first, so every refusal row is + * run once per entrance and the two are asserted separately. + * + * ## What the assertions are, and what each control buys + * + * The subject is the RELATION, never a literal: a refused value does not reach + * `$top` and the site's own default does. Each refusal is paired with a control + * that must NOT fire, so a rail that simply ignores the member cannot pass for + * a measurement and an always-on diagnostic cannot pass for a diagnosis. + */ +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import { render, cleanup, waitFor } from '@testing-library/react'; +import React from 'react'; +import { SchemaRenderer, SchemaRendererProvider } from '@object-ui/react'; +import { DEFAULT_TIMELINE_LIMIT } from '../ObjectTimeline'; +// Registers `object-timeline` (and the ElementDataSourceGate wiring that +// carries the bound entrance). +import '../index'; + +const TIMELINE = { startDateField: 'start_date', endDateField: 'end_date', titleField: 'name' }; + +/** The three values `??` and the resolver DISAGREE about. */ +const REFUSED = [0, -5, 2.5]; + +const viewWithPageSize = (pageSize: unknown) => ({ + name: 'hot', + label: 'Hot campaigns', + columns: ['name', 'stage'], + filter: [['stage', '=', 'live']], + pagination: { pageSize }, +}); + +function makeAdapter(listViews: Record = {}) { + return { + find: vi.fn().mockResolvedValue({ data: [] }), + findOne: vi.fn(), + create: vi.fn(), + update: vi.fn(), + delete: vi.fn(), + getObjectSchema: vi.fn().mockResolvedValue({ + name: 'campaign', + fields: { + name: { type: 'text' }, + stage: { type: 'text' }, + start_date: { type: 'datetime' }, + end_date: { type: 'datetime' }, + }, + listViews, + }), + }; +} + +const renderBlock = (schema: Record, adapter: ReturnType) => + render( + + + , + ); + +const tops = async (adapter: ReturnType) => { + await waitFor(() => expect(adapter.find).toHaveBeenCalled()); + return adapter.find.mock.calls.map((c: any[]) => c[1]?.$top); +}; + +/** ENTRANCE 1 — authored `limit`, no `dataSource` binding anywhere. */ +const authoredRail = (limit: unknown) => ({ + type: 'object-timeline', + objectName: 'campaign', + timeline: TIMELINE, + ...(limit === undefined ? {} : { limit }), +}); + +/** ENTRANCE 2 — no authored `limit`; a bound view's page size fills the key. */ +const boundRail = () => ({ + type: 'object-timeline', + timeline: TIMELINE, + dataSource: { object: 'campaign', view: 'hot' }, +}); + +let warnings: string[] = []; +beforeEach(() => { + warnings = []; + vi.spyOn(console, 'warn').mockImplementation((...args: unknown[]) => { + warnings.push(args.map(String).join(' ')); + }); +}); +afterEach(() => { + cleanup(); + vi.restoreAllMocks(); +}); + +const rowCapWarnings = () => warnings.filter((w) => w.includes('ObjectTimeline row cap')); + +describe('object-timeline — a row cap the contract refuses never reaches the wire (objectui#9925)', () => { + // ── ENTRANCE 1: AUTHORED, NO BINDING ─────────────────────────────────── + describe('authored `limit`, with no `dataSource` binding at all', () => { + it.each(REFUSED)('never sends `$top: %s`', async (bad) => { + const adapter = makeAdapter(); + renderBlock(authoredRail(bad), adapter); + + const sent = await tops(adapter); + expect(sent.length).toBeGreaterThan(0); + expect(sent, `the authored ${bad} reached the wire`).not.toContain(bad); + for (const top of sent) { + expect(typeof top).toBe('number'); + expect(Number.isInteger(top)).toBe(true); + expect(top).toBeGreaterThan(0); + } + }); + + it.each(REFUSED)('falls back to this rail’s OWN default instead of %s', async (bad) => { + const adapter = makeAdapter(); + renderBlock(authoredRail(bad), adapter); + + const sent = await tops(adapter); + for (const top of sent) expect(top).toBe(DEFAULT_TIMELINE_LIMIT); + }); + + it('CONTROL — a legitimate authored limit still reaches `$top` unchanged', async () => { + const adapter = makeAdapter(); + renderBlock(authoredRail(7), adapter); + + const sent = await tops(adapter); + expect(sent).toContain(7); + expect(sent).not.toContain(DEFAULT_TIMELINE_LIMIT); + }); + + it('CONTROL — declaring no limit at all still sends the default', async () => { + const adapter = makeAdapter(); + renderBlock(authoredRail(undefined), adapter); + + const sent = await tops(adapter); + expect(sent).toContain(DEFAULT_TIMELINE_LIMIT); + }); + }); + + // ── ENTRANCE 2: A BOUND VIEW'S PAGE SIZE ─────────────────────────────── + describe('a bound view’s `pagination.pageSize`, lowered into the same key', () => { + it.each(REFUSED)('never sends `$top: %s`', async (bad) => { + const adapter = makeAdapter({ hot: viewWithPageSize(bad) }); + renderBlock(boundRail(), adapter); + + const sent = await tops(adapter); + expect(sent.length).toBeGreaterThan(0); + expect(sent, `the view's ${bad} reached the wire`).not.toContain(bad); + for (const top of sent) expect(top).toBe(DEFAULT_TIMELINE_LIMIT); + }); + + it('CONTROL — a legitimate page size still becomes the rail’s window', async () => { + const adapter = makeAdapter({ hot: viewWithPageSize(7) }); + renderBlock(boundRail(), adapter); + + const sent = await tops(adapter); + expect(sent).toContain(7); + }); + + it('CONTROL — the view’s filter still arrives when its page size is refused', async () => { + // ⛔ No capability removed: refusing one member must not cost the rest of + // the binding. + const adapter = makeAdapter({ hot: viewWithPageSize(0) }); + renderBlock(boundRail(), adapter); + + await waitFor(() => expect(adapter.find).toHaveBeenCalled()); + const [object, params] = adapter.find.mock.calls[0] as [string, any]; + expect(object).toBe('campaign'); + expect(params.$filter).toEqual([['stage', '=', 'live']]); + }); + }); + + // ── THE DIAGNOSTIC ───────────────────────────────────────────────────── + describe('the diagnostic half', () => { + it.each(REFUSED)('names this block, the object and the value %s', async (bad) => { + const adapter = makeAdapter(); + renderBlock(authoredRail(bad), adapter); + + await waitFor(() => expect(rowCapWarnings().length).toBeGreaterThan(0)); + const message = rowCapWarnings()[0]; + expect(message).toContain('object-timeline'); + expect(message).toContain('campaign'); + expect(message).toContain('limit'); + expect(message).toContain(String(bad)); + }); + + it('fires exactly ONCE for one declaration, across re-renders', async () => { + const adapter = makeAdapter(); + const { rerender } = renderBlock(authoredRail(0), adapter); + + await waitFor(() => expect(rowCapWarnings().length).toBeGreaterThan(0)); + rerender( + + + , + ); + await waitFor(() => expect(adapter.find).toHaveBeenCalled()); + expect(rowCapWarnings()).toHaveLength(1); + }); + + it('CONTROL — a legitimate limit produces no such diagnostic', async () => { + const adapter = makeAdapter(); + renderBlock(authoredRail(7), adapter); + + await waitFor(() => expect(adapter.find).toHaveBeenCalled()); + expect(rowCapWarnings()).toHaveLength(0); + }); + + it('CONTROL — declaring no limit at all produces no diagnostic', async () => { + const adapter = makeAdapter(); + renderBlock(authoredRail(undefined), adapter); + + await waitFor(() => expect(adapter.find).toHaveBeenCalled()); + expect(rowCapWarnings()).toHaveLength(0); + }); + + it('is a SEPARATE message from this renderer’s own config parse warning', async () => { + // The parse above the resolver declares no `limit` and is non-strict, so + // it says nothing about this value. Without this row, "a warning fired" + // could be satisfied by the pre-existing parse warning. + const adapter = makeAdapter(); + renderBlock(authoredRail(0), adapter); + + await waitFor(() => expect(rowCapWarnings().length).toBeGreaterThan(0)); + expect( + warnings.filter((w) => w.includes('Invalid timeline configuration')), + 'the config parse refused this value, so the row-cap warning proves nothing', + ).toHaveLength(0); + }); + }); +}); From cebc39270e921f41a74370229571a45eb21d5fd8 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 19 Sep 2026 07:36:23 +0000 Subject: [PATCH 2/3] test(plugin-detail): pin that the rail's row-cap diagnostic survives a neighbour arriving The dedupe claim is not the effect's dependency key: adding a second entry changes the declaration, the effect re-runs and walks the list again, and without a per-(object, value) memory the untouched entry is named twice. The row ablates against removing that memory. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_018HrVaotisyhgmot9o2MLRq --- ...nce-rail.rowLimitNonPositive-9925.test.tsx | 32 +++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/packages/plugin-detail/src/renderers/__tests__/record-reference-rail.rowLimitNonPositive-9925.test.tsx b/packages/plugin-detail/src/renderers/__tests__/record-reference-rail.rowLimitNonPositive-9925.test.tsx index 7181b03781..aac0c7ca00 100644 --- a/packages/plugin-detail/src/renderers/__tests__/record-reference-rail.rowLimitNonPositive-9925.test.tsx +++ b/packages/plugin-detail/src/renderers/__tests__/record-reference-rail.rowLimitNonPositive-9925.test.tsx @@ -229,6 +229,38 @@ describe('record:reference_rail — a row cap the contract refuses never reaches expect(rowCapWarnings()).toHaveLength(1); }); + it('stays silent about an entry it has already named when a NEIGHBOUR is added', async () => { + // The dedupe is not the effect's dependency key: adding a second entry + // genuinely changes the declaration, so the effect re-runs and walks the + // list again. Without a per-(object, value) memory the untouched first + // entry would be named a second time — one authoring change, two warnings + // about a value that did not move. + const ds = makeDataSource(); + const { rerender } = renderRail(railWith(0), ds); + await waitFor(() => expect(rowCapWarnings().length).toBe(1)); + + rerender( + + + + + , + ); + await waitFor(() => expect(ds.find).toHaveBeenCalledTimes(3)); + expect( + rowCapWarnings(), + 'the untouched first entry was named again when its neighbour arrived', + ).toHaveLength(1); + }); + it('CONTROL — a legitimate limit produces no such diagnostic', async () => { const ds = makeDataSource(); renderRail(railWith(7), ds); From 28a24f50e35e0ece18147fe738a0bea897158562 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 19 Sep 2026 08:26:54 +0000 Subject: [PATCH 3/3] test(types): re-point the objectui#7322 `limit` read pin to the spelling this PR ships MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit objectui#9925 moved this board's row cap from `$top: schema.limit ?? DEFAULT_KANBAN_LIMIT` to `$top: resolveRowLimit(schema.limit, DEFAULT_KANBAN_LIMIT)`, and the source-text pin in `object-kanban-group-by-limit-7322.test.ts` still held the retired spelling. That is the one red check on this branch, and the pin is doing exactly the job it was built to do. The pin states its own contract: line numbers live in the docblocks' prose and the READ is the fact. The read survived — same file, same named `query` object, same `schema.limit`, still lowered into the query's top-level `$top` — and only the spelling moved, because `??` rejects `null` and `undefined` and nothing else, so an authored `limit: 0` rode through as a real fetch window on a key `@objectstack/spec` declares a positive integer. So the constant moves to the new spelling at unchanged strength: one `toContain`, same reader, same key, and a string the retired `??` form cannot satisfy. The two declaration docblocks the failing assertion names — "as the exact text the docblocks cite" — move with it. Leaving them behind would make the assertion's own stated semantic false with nothing checking the disagreement. Both of those also carried a cross-file `ObjectKanban.tsx:264` address, already measured as drifted twice. Per commandment #11 a cross-file `path:line` is the banned form, so these are REMOVED and the citation left to the quoted expression beside them — the same repair objectui#8478 recorded for this exact address ("rewritten to cite the same fact by identifier/behavior instead of by file:line, which survives a line renumbering that a bare address would not"). The differential gate reads zero new citations before and after. The renderer's own cross-reference is restated true: what keeping the read inline preserves is the READ the pin reads out of the named `query` object, not the retired spelling. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_018HrVaotisyhgmot9o2MLRq --- packages/plugin-kanban/src/ObjectKanban.tsx | 9 ++++- .../object-kanban-group-by-limit-7322.test.ts | 38 ++++++++++++++++--- packages/types/src/objectql.ts | 13 ++++--- packages/types/src/zod/objectql.zod.ts | 15 ++++---- 4 files changed, 54 insertions(+), 21 deletions(-) diff --git a/packages/plugin-kanban/src/ObjectKanban.tsx b/packages/plugin-kanban/src/ObjectKanban.tsx index 124648549d..31a390eb28 100644 --- a/packages/plugin-kanban/src/ObjectKanban.tsx +++ b/packages/plugin-kanban/src/ObjectKanban.tsx @@ -664,8 +664,13 @@ export const ObjectKanban: React.FC = ({ // reasoning is why objectui#9925's refusal was put INSIDE this // named object rather than beside it: the resolver runs once, and // the saturation reading keeps reading the number that left. - // Keeping it inline here also keeps the spelling objectui#7322 - // pins off disk (`object-kanban-group-by-limit-7322.test.ts`). + // Keeping it inline here also keeps this read where objectui#7322 + // pins it off disk (`object-kanban-group-by-limit-7322.test.ts`): + // that pin reads the `$top` expression out of THIS named object, so + // the refusal landing inside it moved the SPELLING and not the + // read. objectui#9925 re-pointed the pin's `READ_TEXT` entry to the + // expression below; the pinned fact — `schema.limit` lowered into + // the query's top-level `$top` — is the same one it always held. const query = { $filter: schema.filter, $top: resolveRowLimit(schema.limit, DEFAULT_KANBAN_LIMIT), diff --git a/packages/types/src/__tests__/object-kanban-group-by-limit-7322.test.ts b/packages/types/src/__tests__/object-kanban-group-by-limit-7322.test.ts index f5f8308325..13f3fa5567 100644 --- a/packages/types/src/__tests__/object-kanban-group-by-limit-7322.test.ts +++ b/packages/types/src/__tests__/object-kanban-group-by-limit-7322.test.ts @@ -21,13 +21,15 @@ * ## The defect * * `packages/plugin-kanban/src/ObjectKanban.tsx` — the component the - * `object-kanban` registration renders — reads `schema.groupBy` at thirteen + * `object-kanban` registration renders — read `schema.groupBy` at thirteen * sites (lane materialisation, card moves, their effect deps) and * `schema.limit` at two (`$top: schema.limit ?? DEFAULT_KANBAN_LIMIT` and the - * effect deps). `groupField` has ZERO read sites anywhere under - * `packages/plugin-kanban/`. Yet the declaration in `../objectql.ts` REQUIRED - * `groupField` and declared neither `groupBy` nor `limit`, and the zod mirror in - * `../zod/objectql.zod.ts` restated it. Measured on `53ded82b` from source: + * effect deps) WHEN THIS CARD MEASURED IT — the `limit` half has since moved, + * see the objectui#9925 note closing this section. `groupField` has ZERO read + * sites anywhere under `packages/plugin-kanban/`. Yet the declaration in + * `../objectql.ts` REQUIRED `groupField` and declared neither `groupBy` nor + * `limit`, and the zod mirror in `../zod/objectql.zod.ts` restated it. + * Measured on `53ded82b` from source: * the documented, tested, working shape — `{ type: 'object-kanban', * objectName, groupBy, limit }` — FAILED `ObjectKanbanSchema.safeParse` and * `safeValidateSchema` on the missing `groupField`, while a `groupField`-only @@ -35,6 +37,18 @@ * `limit` only ever reached the renderer through `BaseSchema`'s * `[key: string]: any` and `.passthrough()` — admitted, never examined. * + * ⚠️ THE `limit` READING ABOVE IS THE ONE objectui#7322 MEASURED, NOT TODAY'S + * (objectui#9925). That card made the board REFUSE a non-positive row cap + * before it reaches the wire, and both halves of the reading moved with it: + * the spelling is now `$top: resolveRowLimit(schema.limit, + * DEFAULT_KANBAN_LIMIT)`, and the `schema.limit` CODE read sites are FOUR, not + * two — that `$top`, its effect's deps, the refusal diagnostic and that + * diagnostic's own effect deps. The older reading is left standing rather than + * rewritten away because it is the measurement that motivated the declaration; + * what is LIVE is `READ_TEXT` below, which a test re-derives off disk every + * run, while this paragraph is prose nothing re-checks. The thirteen + * `groupBy` sites are untouched by that card. + * * ## What this file pins, and the shapes it borrows * * The declare half is `chat-message-avatar-keys-7295.test.ts` / @@ -82,6 +96,18 @@ const RETIRED = 'groupField'; /** * Exact source text of the reads, as they stand today. Line numbers drift and * live in the docblocks' prose only; the READ is the fact. + * + * The `limit` text was RE-POINTED by objectui#9925, which is what that rule + * prescribes rather than an exception to it. The pinned FACT is that the + * renderer lowers `schema.limit` into the query's top-level `$top`; that fact + * survived intact and only its spelling moved. `??` rejects `null` and + * `undefined` and nothing else, so an authored `limit: 0` was not nullish, rode + * through as a real window and reached the wire as `$top: 0` — on a key + * `@objectstack/spec` declares a POSITIVE integer. `resolveRowLimit` refuses + * that value instead of forwarding it. Same file, same reader, same named + * `query` object, same key, and still ONE `toContain`: the pin keeps the + * strength it had, and the new string is narrow enough that the retired `??` + * spelling could not satisfy it either. */ const READ_TEXT: Record = { groupBy: [ @@ -89,7 +115,7 @@ const READ_TEXT: Record = { 'if (schema.groupBy && objectDef?.fields?.[schema.groupBy]?.options) {', 'const groupBy = schema.groupBy;', ], - limit: ['$top: schema.limit ?? DEFAULT_KANBAN_LIMIT'], + limit: ['$top: resolveRowLimit(schema.limit, DEFAULT_KANBAN_LIMIT)'], }; /** The default the `limit` docblock names. */ const DEFAULT_LIMIT_TEXT = 'export const DEFAULT_KANBAN_LIMIT = 100;'; diff --git a/packages/types/src/objectql.ts b/packages/types/src/objectql.ts index b0951b67e7..7d0a01db9c 100644 --- a/packages/types/src/objectql.ts +++ b/packages/types/src/objectql.ts @@ -3713,12 +3713,13 @@ export interface ObjectKanbanSchema extends BaseSchema { }>; /** * Row cap — the most records the board fetches, sent as a real `$top` on - * the query (`packages/plugin-kanban/src/ObjectKanban.tsx:264`, - * `$top: schema.limit ?? DEFAULT_KANBAN_LIMIT`; objectui#4025). The board - * renders every fetched record into a lane and offers no pagination, so - * this is the author's window on the object rather than a page size. A - * bound `dataSource` (its own `limit`, or the named view's - * `pagination.pageSize`) sets it too. Undeclared until objectui#7322. + * the query (`packages/plugin-kanban/src/ObjectKanban.tsx`, + * `$top: resolveRowLimit(schema.limit, DEFAULT_KANBAN_LIMIT)`; objectui#4025, + * re-spelled by objectui#9925, which refuses a non-positive cap rather than + * forwarding it). The board renders every fetched record into a lane and + * offers no pagination, so this is the author's window on the object rather + * than a page size. A bound `dataSource` (its own `limit`, or the named + * view's `pagination.pageSize`) sets it too. Undeclared until objectui#7322. * * @default 100 — `DEFAULT_KANBAN_LIMIT` */ diff --git a/packages/types/src/zod/objectql.zod.ts b/packages/types/src/zod/objectql.zod.ts index f579544b47..a8e768826e 100644 --- a/packages/types/src/zod/objectql.zod.ts +++ b/packages/types/src/zod/objectql.zod.ts @@ -2093,13 +2093,14 @@ function requireKanbanRecordSource( } // objectui#7322 — `groupBy` and `limit` are the keys `ObjectKanban.tsx` reads -// (thirteen `schema.groupBy` sites; `$top: schema.limit ?? DEFAULT_KANBAN_LIMIT` -// at `:264`); until this card neither was declared and both rode `BaseSchema`'s -// `.passthrough()` unexamined, while the REQUIRED `groupField` had zero read -// sites. `groupField` is now a `retirementTombstone()` — still a member, so -// the parity ratchet's key sets stay equal and an authored value is refused -// BY NAME rather than stripped — and it is node-local: the VIEW-LEVEL alias -// `KanbanConfig.groupField` above is live and untouched. +// (thirteen `schema.groupBy` sites; the row cap lowered into the query as +// `$top: resolveRowLimit(schema.limit, DEFAULT_KANBAN_LIMIT)`, re-spelled by +// objectui#9925); until this card neither was declared and both rode +// `BaseSchema`'s `.passthrough()` unexamined, while the REQUIRED `groupField` +// had zero read sites. `groupField` is now a `retirementTombstone()` — still +// a member, so the parity ratchet's key sets stay equal and an authored value +// is refused BY NAME rather than stripped — and it is node-local: the +// VIEW-LEVEL alias `KanbanConfig.groupField` above is live and untouched. export const ObjectKanbanSchema = BaseSchema.extend({ type: z.literal('object-kanban'), objectName: z.string().optional().describe('ObjectQL object name — the LAST rung of the board ladder, after the pre-fetched data prop, bind and the inline row array on data; one of bind, data, objectName must be present (objectui#7780)'),