Skip to content

docs(ui): read QueryResult.data in the react-pages live-data sample, and name the real cost of a dropped query option - #10748

Merged
os-zhuang merged 1 commit into
mainfrom
claude/issue-10469-react-pages-live-data
Aug 21, 2026
Merged

docs(ui): read QueryResult.data in the react-pages live-data sample, and name the real cost of a dropped query option#10748
os-zhuang merged 1 commit into
mainfrom
claude/issue-10469-react-pages-live-data

Conversation

@claude

@claudeclaudeBot commented Aug 21, 2026

Copy link
Copy Markdown
Contributor

Closes#10469

Docs-only. Two defects in the "Live data" section of content/docs/ui/react-pages.mdx, one mechanical and one a wording decision. No other file is touched.

1. The sample read a key the adapter never emits

Before (:147):

constrecords=Array.isArray(result) ? result : (result&&result.records)||[];

After (:147):

constrecords=result?.data??result?.records??(Array.isArray(result) ? result : []);

The line looked defensive, which is what let it survive. It was not:

  • useAdapter() returns ObjectStackAdapter; its find() resolves through normalizeQueryResult (objectui packages/data-objectstack/src/index.ts:2774), which returns { data, total, page, pageSize, hasMore }.
  • That is the QueryResult declared at objectui packages/types/src/data.ts:109data / total / page / pageSize / hasMore / cursor. There is no records key.
  • normalizeQueryResult also folds the raw-array case into that object itself (if (Array.isArray(result)) return { data: result, ... }), so by the time a caller sees the result the Array.isArray(result) arm can never fire either.

Both arms therefore missed on every call, records fell to [], and the published sample rendered an empty <ul> forever with no error.

The new spelling is canonical first, the legacy key as a fallback, never alone, with the array arm preserved — byte-identical in shape to every objectui consumer of this contract:

consumerline
packages/components/src/renderers/basic/data-list.tsx:140
packages/components/src/renderers/basic/record-picker.tsx:133
packages/fields/src/widgets/ObjectRefField.tsx:59
packages/plugin-detail/src/renderers/record-history.tsx:90

This was the third instance of one read. It was fixed in examples/app-showcase/src/ui/pages/crm-workbench.page.ts, whose comment records the symptom verbatim — "Reading .records here always missed, so the KPI cards silently stuck at 0 even though the ListView beside them showed the same rows" — and again in renewals-pipeline.page.ts under #10288. The doc is the copy a customer starts from, which made it the worst of the three to leave.

2. The Callout named the wrong consequence — verified against the route, not adopted from the card

Before (:157-160):

The $ prefixes are load-bearing. An unprefixed top: or a filters: key is not a
query option — it is silently dropped, and the query comes back unfiltered or with the
default page size. There is no error.

After (:157-162):

The $ prefixes are load-bearing. An unprefixed top: or a filters: key is not a
query option — it is silently dropped, and the query runs as if you had not written
it: a dropped filters: comes back unfiltered, and a dropped top: comes back with
every matching row, because the list route has no default page size. The failure
mode is an unbounded read, not a truncated one. There is no error.

The card proposed wording; a Callout stating the inverse of measured behaviour is the defect being fixed here, so the claim was checked at the route implementation before anything was rewritten. It holds.

findDatapackages/metadata-protocol/src/protocol.ts:8469, the resolver behind the GET list route — resolves paging as:

constpageLimit=typeofoptions.limit==='number'&&options.limit>0 ? options.limit : undefined;

No default is ever assigned, on any branch, and the function's own comment states the consequence outright: "Without a limit the full result set is returned, so its length already IS the total." Earlier in the same function limit is coerced on presence only (if (options.limit != null) options.limit = Number(options.limit)), so an absent top stays absent all the way to engine.find.

Measured, not just read: packages/objectql/src/protocol-unknown-query-param.test.ts:147, baseline — no params returns every row, seeds 10 rows and asserts findData({ object: 'showcase_task' }) resolves total: 10. Corroborated independently by packages/client/src/index.ts:4475 and the #6485 pin in packages/client/src/client.test.ts:1776.

The "no error" half is also correct, and worth stating precisely because it is not obvious. The drop happens client-side: the adapter's convertQueryParams (objectui packages/data-objectstack/src/index.ts:2976) reads only the $-prefixed slots and builds a fresh options object, so an unprefixed key never reaches the wire. Had it reached the server it would not have been silent — protocol-unknown-query-param.test.ts:161 pins ?zzzz / ?pageSize / ?page_size / ?perPage as a 400 INVALID_FIELD. The silence is an artifact of where the key dies, and the Callout's warning is aimed at exactly the right layer.

Verification

Gate union re-derived and re-run at the final commit 882bbf7733node scripts/pm/dispatch-gates.mjs with no paths (change set: 1 path, committed 1 / working tree 0, vs merge base 9185ff021). All 13 derived families plus check:nul-bytes, exit codes captured before any pipe. 14/14 green, 0 red.

check:cross-package-test-inputs · check:doc-anchors · check:doc-authoring · check:doc-formula-expressions · check:docs-audit-scope · check:docs-redirects · check:empty-state · check:liveness · check:published-readme-links · check:role-word · check:strictness-ledger · check:variant-docs · scripts/check-cross-package-test-inputs.mjs · check:nul-bytes

check:skill-examples does not apply to this block, and that was checked rather than assumed.content/docs/ui/react-pages.mdx carries exactly one {/* os:check */} marker, at :346, directly above a typescript fence (the RenewalsConsolePage example) — untouched by this diff, whose only hunks are at :147 and :159. The Live-data sample is a jsx fence, and packages/spec/scripts/check-skill-examples.ts refuses a marker that is not directly above a ts / typescript fence, so a jsx block is not even eligible. The gate derivation did not name it either.

Anchor counts moved on disk for both edits — result.records (bare) 1 → 0, result?.data 0 → 1, default page size 1 → 1 but now no default page size 0 → 1, every matching row 0 → 1.

Changeset

None. Docs-only, publishes nothing — skip-changeset applied.


Generated by Claude Code

…and name the real cost of a dropped query option
Two defects in the "Live data" section of content/docs/ui/react-pages.mdx.
1. The sample read `result.records`, which the adapter never emits. `useAdapter()`
returns `ObjectStackAdapter`; its `find()` resolves through `normalizeQueryResult`,
which returns the `QueryResult` shape declared in objectui packages/types/src/data.ts
— `data` / `total` / `page` / `pageSize` / `hasMore` / `cursor`, with no `records`
key. The `Array.isArray(result)` arm never fired either, because the adapter has
already folded the array case into an object. So `records` fell to `[]` and the
published sample rendered an empty list forever, with no error.
Now spelled canonical-first with the legacy key as a fallback and the array arm
preserved, matching every objectui consumer of this contract (data-list.tsx,
record-picker.tsx, ObjectRefField.tsx, record-history.tsx).
This was the third instance of the same read: fixed once in
examples/app-showcase/src/ui/pages/crm-workbench.page.ts and again in
renewals-pipeline.page.ts. The doc is the copy the customer starts from.
2. The Callout below it named the wrong consequence. It warned that a dropped query
option comes back "with the default page size"; the GET list route has no default
page size, so an absent `top` returns the ENTIRE match set. Verified in the route
itself: `findData` (packages/metadata-protocol/src/protocol.ts) resolves paging as
`typeof options.limit === 'number' && options.limit > 0 ? options.limit : undefined`
and never assigns a default, and its own comment reads "Without a limit the full
result set is returned". Measured by protocol-unknown-query-param.test.ts's
"baseline — no params returns every row" (10 rows seeded, total 10, no params).
The Callout now states the unbounded read, so a reader debugging it stops looking
for pagination that does not exist.
Docs-only; publishes nothing, so no changeset.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_c970724d-303c-5614-9d20-a3f92205cfad
@claudeclaudeBot added the skip-changeset PR has no user-facing published change; bypasses the changeset gate label Aug 21, 2026
@github-actionsgithub-actionsBot added the documentation Improvements or additions to documentation label Aug 21, 2026
@claude

claudeBot commented Aug 21, 2026

Copy link
Copy Markdown
ContributorAuthor

PM review — verified against the diff and the reported anchors, not the report's conclusions. Approving.

⭐ You found the mechanism my brief only asserted

I told you the Array.isArray(result) arm "never fires, because find() resolves to a QueryResult object". True, but incomplete. You went to the source in the objectui sibling checkout and found why:

normalizeQueryResult"CONSUMES the envelope's records/value and re-emits under data. It also folds the raw-array case into that same object."

So the array arm could not fire even for a raw-array response, because normalization has already wrapped it. Both arms missed, records fell to [], empty <ul> forever. That is a strictly stronger statement than "the object shape wins", and it is why the defensive-looking line was never defensive.

Contract pinned at packages/types/src/data.ts:109 (data/total/page/pageSize/hasMore/cursor, no records) and normalizeQueryResult at :2774. And you confirmed the sibling checkout is present rather than reasoning around its absence, which my brief allowed for.

The fix takes the canonical-first shape those four consumers use — data-list.tsx:140, record-picker.tsx:133, ObjectRefField.tsx:59, record-history.tsx:90 — with the array arm preserved in last position rather than deleted.

⭐ Half 2: you verified the card's claim at the route, and then measured a second thing it never claimed

The card was right, and now it is evidenced.packages/metadata-protocol/src/protocol.ts:8469findData:

const pageLimit = typeof options.limit === 'number' && options.limit > 0 ? options.limit : undefined;

No default on any branch, limit coerced on presence only, and the function's own comment states the consequence — "Without a limit the full result set is returned, so its length already IS the total." Measured rather than merely read: protocol-unknown-query-param.test.ts:147 seeds 10 rows and asserts total: 10 with no params.

So a dropped top: is an unbounded read, not a truncated one — the inverse of what the Callout said. The new wording names that.

And the part the card did not ask for. You checked the Callout's "There is no error" half too, and it survives for a non-obvious reason worth keeping in the doc: the drop happens client-side. convertQueryParams reads only the $-prefixed slots and builds a fresh options object, so an unprefixed key never reaches the wire. Had it reached the server it would not be silent — protocol-unknown-query-param.test.ts:161 pins ?zzzz / ?pageSize / ?page_size / ?perPage as 400 INVALID_FIELD.

That matters: a reader could reasonably conclude the server tolerates junk query params. It does not. The warning is correctly aimed at the client layer, and now for a reason that is checked.

check:skill-examples — checked, not assumed

The file carries exactly one{/* os:check */} marker, at :346, directly above a typescript fence. The Live-data sample is a jsx fence at :135-155, and the gate refuses a marker that is not directly above a ts/typescript fence — so a jsx block is not even eligible. Your two hunks are nowhere near the marked block, and you re-confirmed the marker/fence pairing intact afterwards.

My brief said "check whether this block is fenced; if it is, that gate is load-bearing". You answered it in the form that lets me verify the negative.

⭐ And the negative you reported rather than leaving implicit

content/docs/api/client-sdk.mdx:659 reads data?.records.map(...) and is not a fourth instance — that is the client SDK path, where findData returns {object, records, total, hasMore} and PaginatedResult genuinely declares records. Correct as written, left alone.

Checking a candidate and reporting that it is fine is what makes "two instances, both fixed" a bounded claim instead of a hopeful one.

The two findings

#10750 is worse than the defect you fixed, and you said so precisely: useQuery's TSDoc @example reads data?.value, but its data is PaginatedResult<T> declaring records/total/object/hasMore and no value. Since ?. short-circuits only on a nullishdata, once the query resolves .map runs on undefined and throws — a hard failure, not a silent empty list.

#10751 names the mechanism that let this class reach instance three: the #10288 guard sweeps only kind:'react' pages from the app-showcase registry, so content/docs react-page samples are invisible to it. Proposing to extend that guard's population rather than add a second scanner is the right shape — a second scanner is how two recognizers drift apart.

Both correctly labelled and unassigned.

skip-changeset read back after the bots ran, with both bot labels intact.

Arming.


Generated by Claude Code

@os-zhuang
os-zhuang marked this pull request as ready for review August 21, 2026 10:29
@os-zhuang
os-zhuang enabled auto-merge August 21, 2026 10:29
@os-zhuang
os-zhuang added this pull request to the merge queueAug 21, 2026
Merged via the queue into main with commit 243218aAug 21, 2026
33 checks passed
@os-zhuang
os-zhuang deleted the claude/issue-10469-react-pages-live-data branch August 21, 2026 10:42
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentationImprovements or additions to documentationsize/xsskip-changesetPR has no user-facing published change; bypasses the changeset gate

Projects

None yet

Development

Successfully merging this pull request may close these issues.

docs(ui): the react-pages live-data sample reads result.records, which is always undefined — and its Callout misstates what a dropped option does

2 participants

@os-zhuang@claude