Repository navigation
fix(fields): lookup candidate queries expand the reference columns they display (objectui#10223) - #10341
Merged
Conversation
…ey display (objectui#10223) Opening a lookup's dropdown sent a candidate query with no `$expand`, so every previewed lookup / master_detail column came back as a bare foreign key and the lookup cell renderer resolved each one with its own `findOne`: one request per candidate per such column on every open. The browse-all picker had the same shape. Both candidate queries now ask for `$expand` over the reference columns they display, by `buildExpandFields`' rule (the inline dropdown's previewed columns; the picker's rendered columns minus the id column). The recents rail asks for the same expansion as the main list. Expansion stays a display concern: options, the `titleFormat` reading of a row and the records handed to `onSelectRecord` / `onSelectRecords` are built from the row with its relations collapsed back to ids (core's `toPredicateRecord`), so labels, committed values and host payloads read as before; only the previews and table cells render the expanded record. Co-authored-by: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BP8CMtACxTdLjqR6rhd33C
…icker-agreement prose (objectui#10223) - The candidate-expand suite now also pins that a previewed `user` column rides `buildExpandFields`' rule and names the person. - `LookupField.pickerAgreement.test.tsx` said neither surface's query carries `$expand`; both now do, and that file's backend ignores the parameter, which is what makes it the bare-id fallback pin. Prose corrected, assertions unchanged. - Changeset: patch for `@object-ui/fields`. Co-authored-by: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BP8CMtACxTdLjqR6rhd33C
… (objectui#10223) Co-authored-by: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BP8CMtACxTdLjqR6rhd33C
Contributor
✅ Console Performance Budget
The eager closure is every chunk the entry reaches through static imports — what the browser fetches and parses before the app renders. The entry chunk on its own is a small fraction of it. 📦 Bundle Size Report
Size Limits
|
…-expand Brings the branch past the Spec Main Shape Gate fix (1dbb993). No overlap with this branch's files. Co-authored-by: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BP8CMtACxTdLjqR6rhd33C
…and FLS gate (objectui#10223) @object-ui/permissions depends only on @object-ui/types, so the edge adds no cycle. The lockfile hunk is the new importer link alone, generated by 'pnpm install --lockfile-only' over an installed tree. Co-authored-by: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BP8CMtACxTdLjqR6rhd33C
…n (objectui#10223) The dropdown's candidate query, its recents rail and RecordPickerDialog now filter `buildExpandFields`' output through `usePermissions().checkField( object, field, 'read')` once the policy has loaded, the shape the objectui#7429 sweep applied at every other call site; with no policy loaded nothing is filtered, and `perms` in the memo deps rebuilds the list when the answer arrives. Pins (real PermissionProvider): a denied `task_version.task` is left out of `$expand` while the readable `owner` stays, on the dropdown, the recents rail and the picker; a readable `task` is expanded; no provider filters nothing. `lookupColumnDisplay.tsx`: its module header said the two surfaces agree "without either query changing", which this change made false; that one sentence is corrected. Changeset updated for the gate and the new dependency. Co-authored-by: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BP8CMtACxTdLjqR6rhd33C
Contributor
|
changeset-claim-re-read
|
Contributor
✅ Console Performance Budget
The eager closure is every chunk the entry reaches through static imports — what the browser fetches and parses before the app renders. The entry chunk on its own is a small fraction of it. 📦 Bundle Size Report
Size Limits
|
Contributor
Author
Contract reviewServed-tier: Rendered by an isolated review subagent spawned by the ① Derived judgments
② Semver level
③ Boundary flags
Implemented-by: VERDICT: FAIL Generated by Claude Code |
…-expand main touched pnpm-lock.yaml (two workspace links in another importer); the branch's own lockfile hunk is untouched by it. Co-authored-by: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BP8CMtACxTdLjqR6rhd33C
…e queries (objectui#10223) `buildExpandFields` reads an EMPTY column list as "no restriction" and returns every relation the object declares. The dropdown's preview list is empty whenever its picker columns are the display field alone (a `highlightFields` naming only it, or every other field system-managed), so the dropdown and the recents rail asked for every declared relation, none of which they render. Both candidate expansions now return nothing for an empty column list; the picker's list (its columns minus the id column) gets the same one-line guard for the `columns: ['id']` / `displayField === idField` shape. Pins: `highlightFields ['name']` over an object declaring `created_by` (user), `owner_id` (lookup), `task` (master_detail) and `owner` (user) sends no `$expand` key on the dropdown or the recents rail; a picker whose only column is the id sends none either. `.changeset/lookup-dropdown-cell-renderer-5492.md` (pending, same package) said in the present tense that neither surface's request carries populate; this PR makes that false, so the clause now reads as history and names the change that superseded it. Frontmatter untouched. Co-authored-by: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BP8CMtACxTdLjqR6rhd33C
Contributor
✅ Console Performance Budget
The eager closure is every chunk the entry reaches through static imports — what the browser fetches and parses before the app renders. The entry chunk on its own is a small fraction of it. 📦 Bundle Size Report
Size Limits
|
Contributor
Author
Contract review (re-review, patch round 1)Served-tier: Rendered by an isolated review subagent spawned by the ① Derived judgments
② Semver levelUnchanged from the prior record: ③ Boundary flags
Implemented-by: VERDICT: PASS Generated by Claude Code |
os-litant
marked this pull request as ready for review
September 24, 2026 20:33
akarma-synetal
pushed a commit
to akarma-synetal/objectui
that referenced
this pull request
Sep 28, 2026
…unwrap `rows` (objectui#7028) (objectstack-ai#10363) Fixes objectstack-ai#7028 Clause-②: yes ## What `aggregate()` now reads the `client.analytics.query` answer in one spelling: `rows` on the post-unwrap `AnalyticsResult`. This is condition 3 of the objectstack#13079 ruling's landing requirements (director batch objectstack-ai#19, option A, maintainer 2026-08-31), quoted verbatim: 「objectui 容错链同波收紧:已立 objectui#7028,带时序门(严格在收敛合并、objectui 采版之后 —— 该链今天是承重的)」. The card's sequencing gate is met: `@objectstack/client` 17.3.0 carries the convergence, and the workspace lockfile resolves 17.4.0. - **The ladder had five spellings, not the card's two.** It read a bare array, `rows`, `data` as an array, `data.data.rows` and `results`, and answered anything else with `[]`. Only `rows` survives. Any other value throws the new exported `AnalyticsResultShapeError` (`code: 'ANALYTICS_RESULT_SHAPE_INVALID'`, `envelope: true` for the pre-17.3.0 envelope). - **Loud means a throw, and the throw escapes the fallback.** The row read sits inside `aggregate()`'s `try`. Its `catch` would have classified the error as `unknown` and answered it with `aggregateViaFind`'s client-side numbers, so the catch now rethrows `AnalyticsResultShapeError` before it classifies anything. I chose a throw over a diagnostic because the file already treats contract violations that way: `AnalyticsQueryRejectedError` refuses the fallback for the same reason (framework#3878). A `[]` would repeat the confident zero that objectui#5954 removed on the failure side. - **The widened `AnalyticsResult` alias is gone**, along with the branches it existed for. - **The `@objectstack/client` floor moves from `^17.0.0` to `^17.3.0`** in `packages/data-objectstack` and `apps/console`. Only the lockfile specifier changes; the resolution stays at 17.4.0. `QUICK_REFERENCE.md`'s Client row was rewritten by `pnpm quick-reference:sync`. ## The premise, measured - **This was never a server question.** The comment above the old ladder (and the objectui#7122 changeset) described deleting the branches as "a runtime compatibility decision about servers older than #13079". The convergence commit `db16b94` changes only `packages/client` and the spec migration registry, and no server code. `POST /analytics/query` answers `deps.success(result)` in objectstack's `domains/analytics.ts` from its first extraction (`8f124a7b7`, 2026-07-27, before 17.0.0 shipped on 2026-08-14) to `main`. Every 17.x server therefore sends the same `{ success, data }` envelope. Only the client decides whether it is unwrapped: `unwrapResponse` strips it at 17.3.0+, while 17.2.0 ended `return res.json()` (read in the installed 17.4.0 `dist/index.mjs` and a packed 17.2.0 tarball). No server is dropped. The compatibility axis is the **client** range, and that is why the floor moves. - **Which producer each branch served** (`git log -S` on the unshallowed history): - `rows` and `data.data.rows` came from `70cb62b85` (2026-04-01): the post-unwrap and envelope forms of the one route. - A bare array, `data` as an array and `results` came from `f25e6c288` (2026-02-25). That commit targeted a raw `GET /api/v1/analytics/{resource}` fetch, which `d91f2e2a0` replaced with `client.analytics.query` ten minutes later, and the three branches were carried over without shape evidence. - Even the retired in-kernel shim (removed by objectstack `77fadbfca` before 17.0.0) answered `{ success, data: { rows, fields } }`. No producer of this call site ever returned the other three shapes. - **`analytics.meta` and `analytics.explain` have no call sites in objectui.** `git grep` finds only `analytics.query` here, with a positive control on the same pattern. In the installed 17.4.0 client, `meta` and `explain` both end `return this.unwrapResponse(res)`, declared `AnalyticsMetadataResponse["data"]` and `AnalyticsSqlResponse["data"]`. `analytics.query` is declared as a Promise of `AnalyticsResult` (the generic is spelled out in words because GitHub strips angle-bracket spans). ## Surface beyond the claim, stated The claim named only `index.ts` (the ladder), the tests beside it and one changeset. Two extensions were needed: 1. **The rethrow line in `aggregate()`'s `catch` plus the new error class.** Without them the throw is silently answered by the fallback, which the pins require it must not be. 2. **The client floor, in `packages/data-objectstack/package.json`, `apps/console/package.json`, the two `pnpm-lock.yaml` specifier lines and `QUICK_REFERENCE.md`.** Leaving `^17.0.0` would publish a range this change makes false. `scripts/__tests__/quick-reference-current-release-4143.test.ts` went red on the adapter-only bump, because that row anchors both manifests. No open pull request holds these lines: objectstack-ai#10341 and objectstack-ai#8941 touch the lockfile and the console manifest in disjoint hunks, and objectstack-ai#5400 is the release pull request. `isLegacyOverlayRow` and the rest of `index.ts` are untouched. **Seat amendment (2026-09-24T20:14Z).** The seat accepted both extensions and widened the claim's File surface to match (claim comment `5820950909`, edited). Line 2 now reads `Clause-②: yes`, because the diff adds a public export (`AnalyticsResultShapeError`); a contract review is owed before enqueue. The changeset level moved from `patch` to `minor` at head `e6e26e35d` (objectui precedent objectstack-ai#9061 / objectstack-ai#9175). ## Pins (site-scoped, new file) `packages/data-objectstack/src/aggregate-rows-post-unwrap-7028.test.ts`, 7 tests: - The unwrapped shape yields its rows. The server's `{ success, data: { rows } }` wire envelope goes through the real client. - An envelope at the boundary throws, and `/api/v1/data` is never requested. This is tested twice: through the real client (a success-less `{ data: { rows } }` body that `unwrapResponse` leaves alone), and as the value a pre-17.3.0 client handed back. - The other three retired spellings throw instead of degrading to `[]`. - CONTROL: rows missing the measure still fall back to client-side aggregation. ## Verification (final head `52f5a6c21`) - `pnpm --filter @object-ui/data-objectstack type-check`: exit 0. `--listFiles` confirms the new test file is in that program. - `pnpm exec vitest run packages/data-objectstack/` from the repo root: `Test Files 68 passed (68)`, `Tests 933 passed (933)`. - These root suites read the touched files off disk, outside the package graph: every `scripts/__tests__` suite naming `data-objectstack`, `QUICK_REFERENCE`, `apps/console/package.json` or `pnpm-lock.yaml`. Result: `Test Files 27 passed (27)`, `Tests 1137 passed (1137)`. - `eslint .` in the package: 77 files linted (from `--format json`), 0 errors. Type-aware linting is off, so this diff cannot move any verdict in untouched files. - Gates, all exit 0: `check:control-bytes`, `check:new-line-citations` (0 new), `check-changeset-presence`, `check:changeset-claims`, `check:lockfile-integrity`, `check:lockfile-dedupe`, `check:installed-pin-claims`, `check:pending-changeset-literals`, `quick-reference:check`. - `check:changeset-claims` lists `5793`, `6361` and `7122` for naming `pnpm-lock.yaml`. I read each paragraph, and none is falsified by this diff. - `check:spec-floors`: NOT MEASURED. It refuses on an unbuilt workspace (`no-artifact`), and it reads `@objectstack/spec` floors only, which this diff does not move. - **Ablation 1:** I restored the five-spelling ladder with `ablation-replace.mjs` (anchor 1 to 0, blob `0b7bc68a9ed1` to `3b936fc60bb8`). Result: `Tests 5 failed | 2 passed (7)`, with every failure being `aggregate() resolved; it was expected to throw AnalyticsResultShapeError`. The two greens are the positive pin and the control. The restore is proven: blob equals HEAD `0b7bc68a9ed1` and `git diff HEAD` is empty. - **Ablation 2:** I deleted the `catch` rethrow. Result: the same 5 fail and 2 pass, because the error is answered by the fallback. The restore is proven the same way. - Both ablations run from source through a relative `./index` import, so no `dist` leg applies. ## Acceptance notes - `analytics.meta` and `analytics.explain` are not called anywhere in this repo, so they have nothing to tighten. - The pending `.changeset/7122-objectstack-family-17-3-0.md` still says the branches were kept "rather than deleted" and frames the question as one about servers. It is true of its own change and will publish in the same release as this one, so the new changeset names it and supersedes it rather than editing it. - The existing `aggregate-capability.test.ts` case "a `{ success, data: { rows } }` envelope is still a result" stays green. It is the server's wire envelope, which the installed client unwraps. After this change it is a positive control, not tolerance. - Dependents' type-check: `plugin-charts`, `plugin-dashboard` and `components` read `aggregate()` results through the `DataSource` interface and do not depend on this package. The only export-surface change is one added class: `export` lines in the diff are `+1 -0`, and the `aggregate` signature is untouched. No export-star re-export exists, and no other declaration of the name exists, so no importer's type-check input changes. The full workspace Type Check runs in CI. Session: `https://claude.ai/code/session_01BA3nKVUwKQJf8DBxrSVtNC` (dev run dispatched by the `domain:ui` seat 1). --- _Generated by [Claude Code](https://claude.ai/code/session_01BA3nKVUwKQJf8DBxrSVtNC)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #10223
What changes
LookupField: the dropdown's candidate query now sendsexpand=buildExpandFields(referenced schema fields, previewed columns). The previewed columns are the columns the option subtitle shows (by default the leadinghighlightFields, display field excluded). The recently-used rail asks for the same expansion.RecordPickerDialog: its query now sendsexpand=buildExpandFields(fieldsMeta, rendered columns minus the id column). The id column is left out becausegetRecordIdreads it raw.No displayed column ⇒ no
$expand.buildExpandFieldsreads an EMPTY column list as "no restriction" and returns every relation the object declares. So both expansions return nothing when their column list is empty:highlightFieldsnaming only it, or every other field system-managed or hidden);columnsis just the id, ordisplayFieldequalsidFieldwith no columns.Without the guard, the dropdown and the recents rail asked for every declared relation (
created_by,owner_id, …) and rendered none of them.Field-level security gates both expansions. It uses the objectui#7429 sweep's caller-side shape on
buildExpandFields' output:!perms.isLoaded || perms.checkField(referenced object, f, 'read')throughusePermissions(), withpermsin the memo deps. Once the policy has loaded, a relation it denies is not asked for; before it loads, nothing is filtered. This covers the dropdown, its recents rail and the picker.New dependency:
@object-ui/fields→@object-ui/permissions(workspace:*).@object-ui/permissionsdepends only on@object-ui/types, so the edge adds no cycle. Againstmain, thepnpm-lock.yamlhunk is the new importer link alone (3 lines), generated bypnpm install --lockfile-onlyover an installed tree.Expansion stays a display concern. Options, the
titleFormatreading of a row, the committed value and the records handed toonSelectRecord/onSelectRecordsare all built from the row with its relations collapsed back to ids, using core'stoPredicateRecord. Only the preview cells and table cells render the expanded record. Without that collapse, atitleFormatnaming the expanded field printedC-0 - [object Object](ablation 2a below).useRecordQueryis untouched. It already forwardsexpandas$expand.Prose made false by this change, corrected:
lookupColumnDisplay.tsx's module header that said the two surfaces agree "without either query changing";LookupField.pickerAgreement.test.tsx, which now names what that file pins: a backend that ignores$expand. Its assertions are unchanged;.changeset/lookup-dropdown-cell-renderer-5492.md, which publishes into the same@object-ui/fieldsrelease notes. It said in the present tense that neither surface's request carries populate; that clause now reads as history ("at the time") and names this change. Frontmatter is untouched.origin/mainis merged in twice (merge commits177e5b5f0anda2de421be, bringing main to4215ed76band then86982ace0), which carry the Spec Main Shape Gate fix1dbb9933c. Of this PR's files, the second merge touched onlypnpm-lock.yaml, and only in another importer.Measured: fixture mirroring the card
The fixture has 50 candidates.
highlightFieldsiscode,task,version, withtaskamaster_detailfield totask. There are nolookup_columns, and no permission policy is loaded. The "before" leg ran with both source files at base8b1f06619. The "after" leg ran at8f70a8b74; neither the gate nor the empty-list guard adds a request.$expand· per-rowfindOne['task']· 0['task']· 0Schema reads are a constant on both legs: the
task_versionschema at mount, plus thetaskschema once (module-cached) when a task cell renders.Mechanism assumptions
LookupCellRenderergives an expanded object no primitive id, souseLookupNamenever fetches. Its object branch names the record.packages/fields/src/index.tsxis not edited.useRecordQueryforwardsexpand.@object-ui/fieldshad no path to the policy. The seat widened the claim's surface (issue comment 5820197003), and the gate now lands here, in the family's shape.RelatedList's are);PeoplePicker's$expandis ungated.Tests (HEAD
9796fdbb4)LookupField.candidateExpand-10223.test.tsx, 13 tests:usercolumn, the control and the picker count;highlightFields ['name']over an object declaringcreated_by(user),owner_id(lookup),task(master_detail) andowner(user): neither the dropdown request nor the recents-rail request carries a$expandkey;$expandkey;titleFormatnaming the expanded field, subtitles, and the callback payloads are identical whether or not the backend honours$expand;PermissionProvider:task_version.taskis left out while the readableownerstays, on the dropdown, the recents rail and the picker;taskis kept;@object-ui/fieldssuite, run in two locked halves:widgets/: 75 files passed, 696 tests passed;packages/fieldsthat names the picker or lookup trigger, plus every test that mocks@object-ui/permissions. 88 files passed, 1026 tests passed.pnpm --filter @object-ui/fields run type-checkexits 0. It ran afterpnpm --filter '@object-ui/fields^...' run build, a closure that includes@object-ui/permissions. The new test file is in the test program.LookupField.tsx59,RecordPickerDialog.tsx35.Ablations
Each ablation ran with the fix committed. The restore was
git checkout HEAD, proved by blob hash and an emptygit diff HEAD.expected undefined to deeply equal [ 'task' ].C-0 - [object Object].C-0 - [object Object]in 2c.LookupField's FLS filter removed. The dropdown and recents-rail deny pins go red.RecordPickerDialog's FLS filter removed. The picker deny pin goes red.b7ee692eb. Both empty-list pins go red (expected true to be falseon the$expandkey); 2 failed, 11 passed.Gates (exit 0 at
9796fdbb4)check-changeset-presencecheck-changeset-no-majorcheck:new-line-citations: 0 newcheck:phantom-depscheck:unused-depscheck:control-bytescheck:changeset-claimscheck:pending-changeset-literalscheck:lockfile-integrity: cleancheck:lockfile-dedupe: dedupedAcceptance notes
usercolumns.buildExpandFieldsincludesuser, so a previewedusercolumn now renders an avatar plus a name, where it used to show the unresolved raw-id marker. The avatar inside the one-line dropdown subtitle was not checked in a browser (NOT MEASURED: visual).toPredicateRecordreturns ids as strings, so on a numeric-id backend that honours$expand, callback payloads carry the string form of an expanded column's key. The committed value is unaffected. The changeset says so.$expandand a second one with it. A permission policy landing after the dropdown opens triggers a refetch only when it removes a name from the list.pnpm install --lockfile-onlyalso re-resolved unrelatedesbuildpeer variants. Run over an installed tree, it gave the 3-line edge alone, and that is the hunk committed.check:lockfile-integrityandcheck:lockfile-dedupeare both clean.Session:
https://claude.ai/code/session_01BP8CMtACxTdLjqR6rhd33CGenerated by Claude Code