Repository navigation
fix(plugin-grid): the import preview checks a time column and the user import email the way the server does (objectui#11913) - #11973
Merged
objectstack-fleet[bot] merged 6 commits intoOct 8, 2026
Conversation
…r import's email as the server does The preview marks a cell exactly where the server's import refuses it, and the import button counts the rows nobody marked (objectui#11814, #11889). Two columns still went unchecked: - A `time` cell: `checkImportCell` had no `time` arm. It now mirrors core's `parseDateCell(cell, 'time')` (`readTimeOfDayCell`, then the year-first form) and refuses with `invalid_time`, in a new localized sentence `grid.import.invalidTime` in every pack. - The user import's email: `identityImportFields` typed it `text`. It is now typed `email` and flagged `emailRule: 'identity'`, and the email arm asks the user import endpoint's rule (`isLikelyEmail` and `isPlaceholderEmail`) instead of the record validator's. Refs objectui#11913. Claude-Session: https://claude.ai/code/session_01CGZy1BGCjdN5cXqL9cnvB8 Co-authored-by: Claude <noreply@anthropic.com>
The de pack's new `grid.import.invalidTime` row quotes the cell with one matched pair, so the pinned span and pair counts move from 75 to 76. Claude-Session: https://claude.ai/code/session_01CGZy1BGCjdN5cXqL9cnvB8 Co-authored-by: Claude <noreply@anthropic.com>
…eld type `ObjectView` passes `identityImportFields` or `importTargetFields` into the wizard's `fields` through one conditional, which compiles only while this return type is a supertype of `ImportTargetField`. The email entry is typed at its literal against `ImportWizardProps['fields'][number]`, so `emailRule` is still checked where it is written. Claude-Session: https://claude.ai/code/session_01CGZy1BGCjdN5cXqL9cnvB8 Co-authored-by: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CGZy1BGCjdN5cXqL9cnvB8 Co-authored-by: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CGZy1BGCjdN5cXqL9cnvB8 Co-authored-by: Claude <noreply@anthropic.com>
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
|
Standing down on
Generated by Claude Code |
Claude-Session: https://claude.ai/code/session_01CGZy1BGCjdN5cXqL9cnvB8 Co-authored-by: Claude <noreply@anthropic.com>
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 2026-10-08T18:05Z by the at-tier reviewer of Check-runs on the head: 43 completed, 40 success, 3 skipped ( ① Derived judgmentsEvery accept-set and public-surface change the diff implies, each judged against the server it mirrors:
Nothing in the 18 files lies outside the claim plus its amendment, except the one region the dev named, ruled on in ③. ② Semver level
The PR body carries ③ Boundary flags
Acceptance notes and out-of-scope findings, each answered:
One observation of my own, not a flag: better-auth revalidates an address at real-import time beyond Implemented-by: VERDICT: PASS |
objectstack-fleet
Bot
deleted the
claude/issue-11913-import-time-identity-email
branch
October 8, 2026 18:29
akarma-synetal
pushed a commit
to akarma-synetal/objectui
that referenced
this pull request
Oct 9, 2026
…ls before reading them (objectui#11978) (objectstack-ai#11986) Fixes objectstack-ai#11978 Clause-②: no ## What changed One test, `ObjectHooksPanel.newHookTarget-11820.test.tsx`, "opens with this package's objects first and every other object under the reach warning": every read whose answer the object roster draws now waits for that answer. - The two "This package" checkbox reads (`Invoice (invoice)`, `Invoice line (invoice_line)`) and the two "Other objects" checkbox reads (`User (sys_user)`, `Employee (hr_employee)`) use `await within(…).findByRole(…)` at the default timeout instead of a synchronous `getByRole`. - The "Other objects" group is found with `await screen.findByRole` too: that group is drawn only once the roster answers. - An empty-frontmatter changeset declares no release. The file sits under `src/`, and `check-changeset-presence.mjs` has no carve-out for tests. The component is unchanged: `HookDefaultInspector`'s bare-name interim state is deliberate (objectui#10585). ## Why it was red on timing alone Two reads answer at their own pace. - `ObjectHooksPanel` reads this package's objects in a mount effect and hands them to the editor as `HookTargetScopeContext`. That draws the "This package" group. - `HookDefaultInspector` reads the object roster itself (`useObjectOptions`), after New opens it. Until the roster answers, `pickerOptions` holds only the selected object, labelled by its bare name. So "This package" can be on screen holding one checked checkbox named `invoice` while "Other objects" is not drawn yet. The test awaited only the group, then read the label synchronously. Which read answered first decided the verdict: this is the red that took PR objectstack-ai#11972 out of the merge queue and turned PR objectstack-ai#11973's `Test (shard 6/8)` red. ## Measurements, on this branch **H1: the fix closes the window.** A throwaway, untracked copy of the test (deleted afterwards) delays only the catalog `list('object')` answer; the package-scoped read is not delayed. One copy of the test as on `main` (`989b19082`), one of the test as changed here. | copy | roster delay | runs | result | |---|---|---|---| | as on `main` (the lit control) | 120 ms | 3 | 3 failed, each at the `Invoice (invoice)` read. The dump shows "This package" holding one checked checkbox named `invoice`: the queue's signature. | | as changed here | 120 ms | 3 | 3 passed | | as changed here | 300 ms | 1 | passed | | as changed here | 600 ms | 1 | passed | | as changed here | 1500 ms | 1 | failed at the new `findByRole('checkbox', { name: 'Invoice (invoice)' })`: the wait is bounded by the default 1000 ms timeout. | **H2: the test still tests its claim.** Two ablations of `HookDefaultInspector.tsx` through objectstack's `scripts/ablation-replace.mjs` in wrap mode. Each was restored to the HEAD blob `3bb69262f529` with `git diff HEAD` empty, and a bash trap re-checked the same hash. - `invoice` moved from "This package" to "Other objects" (both `scope.packageObjects.has(o.value)` partition reads replaced; anchor 2 to 0, blob changed). The test went red at the new `await within(own).findByRole('checkbox', { name: 'Invoice (invoice)' })` after its default timeout. - The reach warning removed (the `hook-object-outside-reach` paragraph deleted; anchor 1 to 0). Red at `getByTestId('hook-object-outside-reach')`. - The first attempt at the first ablation was a no-op. Its replacement text contained the anchor, so the anchor count could not drop; the tool refused with exit 1 before running the test, and restored. The reading above is the retake, with a replacement that does not contain the anchor. **H3: the same pattern elsewhere.** Read only; no other test file is touched. Two text scans over the 594 test files under `packages/app-shell/src/views/studio-design/` and `packages/app-shell/src/views/metadata-admin/`. Each has a lit positive control: run on `main`, each hits this card's test; on this branch, neither does. 1. An awaited container (`const X = await …findBy…`), then a synchronous named read inside it: 15 hits in 10 files. None shares the race. The names are static i18n strings drawn in the same commit as their container (buttons, a tab, a placeholder). The one data-derived name, `DocPreview.test.tsx` "keeps a group key no book declares visible", is gated by an interposed `await waitFor(() => expect(picker).toHaveValue('retired_section'))`, which can pass only once the books answered. 2. A synchronous read of a roster-shaped `Label (api_name)` name: 25 hits in 11 files. None shares the race. Most are synchronous renders with no async read at all (static strings such as `Values (measures)`, `role (deprecated)`). `HookDefaultInspector.reach-11820.test.tsx` awaits `findByRole('checkbox', { name: 'Account (crm_account)' })`, the roster's answer, before its synchronous reads. `AccessExplainPanel.test.tsx` waits for the Object field to become a select, which is drawn once the roster answered. `ObjectFieldInspector.usePicklist-10202.test.tsx` awaits one served option before reading its sibling from the same answer. The scans' blind spot: a data-derived name in another form, read through `screen.getBy…` after an unrelated await, is outside both. ## Gates, at `04a3cad2a` - `pnpm exec vitest run --maxWorkers=2 packages/app-shell/src/views/studio-design/ packages/app-shell/src/views/metadata-admin/inspectors/` (verify lock): `Test Files 296 passed (296)`, `Tests 2935 passed | 1 skipped (2936)`. - `pnpm exec vitest run --maxWorkers=2 scripts/__tests__/` (verify lock): `Test Files 179 passed | 2 skipped (181)`, `Tests 5452 passed | 2 skipped (5454)`. - `pnpm --filter @object-ui/app-shell type-check` (verify lock; echoes `@object-ui/app-shell@17.7.0 type-check`, which runs `tsc --noEmit && tsc -p tsconfig.test.json`): exit 0, after `turbo run build --filter='@object-ui/app-shell^...'` built the dependency closure (28 of 28 tasks). `tsc -p tsconfig.test.json --listFilesOnly` lists this test file. A first run before the closure was built could not resolve the `@object-ui/*` declarations: NOT MEASURED, not a red. - `node scripts/check-changeset-presence.mjs`: exit 0 (an empty frontmatter, the explicit exemption). `check-changeset-no-major.mjs`, `check-changeset-overwrite.mjs`, `pnpm check:changeset-claims`, `pnpm check:pending-changeset-literals`: exit 0. - `pnpm check:new-line-citations`: `0 new citation(s)`, exit 0. `pnpm check:control-bytes`: exit 0. - `pnpm check:vi-mock-specifiers`, `check:vi-mock-inherit`, `check:vi-mock-override-shape`, `check:test-path-roots`: exit 0. - ESLint, narrowed to the changed file, with an unchanged sibling test as control: 2 files in the JSON result, 0 errors and 0 warnings each. `--print-config` shows 119 rules active on the changed file, so it is in ESLint's population. The config enables no type-aware linting, so this diff cannot move another file's verdict. The repo-wide `pnpm lint` and the full sharded `pnpm test` are CI's. ## Acceptance notes - The sibling test of the same picker, `HookDefaultInspector.reach-11820.test.tsx`, already waits the way this change does: its mount helper awaits the roster-labelled checkbox before any synchronous read. - objectui#11913 waits on this card (`Blocked-by: objectstack-ai#11978`). PR objectstack-ai#11972 re-enters the queue once this lands. Session: `https://claude.ai/code/session_01DBZ9bntPZ7VKyQNtJeNsgw` --- _Generated by [Claude Code](https://claude.ai/code/session_01DBZ9bntPZ7VKyQNtJeNsgw)_ 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 #11913
Clause-②: yes
The Import Wizard's preview marks a cell exactly where the server's import refuses it, because the import button counts the rows nobody marked (objectui#11814, objectui#11889). This PR extends the arms objectui#11889 left in
checkImportCell, without forking them, to cover the two columns the card measured unchecked.Implemented by the
os-devsession dispatched by thedomain:uiseat 3 PM, sessionhttps://claude.ai/code/session_01CGZy1BGCjdN5cXqL9cnvB8.What changes
1. A
timecolumn is checked (invalid_time).checkImportCellgains atimearm right after thedate/datetimearm, the order the server'scoerceFieldValueruns them in. The arm asksisImportableTimeCell, a new mirror inimportCoercionContract.tsbesideisImportableDateCell.parseDateCell(cell, 'time')reads it in the published 17.7.0. It asksreadTimeOfDayCellfirst, thenreadYearFirstCellalone, neverreadIsoTemporalCell.readTimeOfDayCellasks core's comparand rule, which takes two forms:ClockTimeValueSchematakes;@objectstack/spec/data(ClockTimeValueSchema), not restated, because the server reads that same schema. The ISO and year-first readings reuse the file's existing private grammar:ISO_TEMPORAL_CELL,YEAR_FIRST_CELL,namesRealCalendarDay,isWallClockandzonedCellUtcYear.grid.import.invalidTime(en:"{{value}}" is not a valid time). The same sentence renders aninvalid_timerow error from the server's dry run (formatDryRunError).record-validator.tsat 17.7.0 judges a writtentimeby the same core rule (isUninterpretableTemporalComparand). So a time the coercion stores is never refused after it.2. The user import's email column is checked by the user import endpoint's rule (
INVALID_EMAIL).identityImportFieldstypes the columnemail, where it wastext, and flags itemailRule: 'identity'.checkImportCell's email arm asksisIdentityEmailfor a flagged field andisRecordEmailfor any other.isIdentityEmailmirrors plugin-auth'sresolveRowIdentityat the@objectstack/plugin-auth@17.7.0tag, which refuses in two cases:isLikelyEmailrefuses the cell. It takes at most 254 characters of printable ASCII with no whitespace, one@that is neither first nor last, and a domain whose last dot is neither its first nor its last character.isPlaceholderEmailtakes it: an address onplaceholder.invalid, in any case.a@b..c, which the record validator refuses. It refuses735431496@柴仟.com, which the record validator takes. A plainemailfield keeps the record rule exactly as before.3. The count. The button counts unmarked rows (objectui#11814), so both new marks move it. Each leg is pinned through the real wizard:
Import 1 Rowon five time rows, and on three identity email rows.Widenings (Clause-②)
@object-ui/plugin-grid(minor).ImportWizardProps['fields'][number]gainsemailRule?: 'identity', which is optional and a single literal. Left out, anemailcolumn keeps the record validator's rule.index.d.tsre-exportsImportWizard, and the typesImportWizardPropsandImportResult. The new mirrors (isImportableTimeCell,isIdentityEmail) and the widenedCellRefusalcode union are module-private, and the packageexportsmap only.and./style.css.dist:emailRule?: 'identity'appears indist/ImportWizard.d.ts, anddist/index.d.tsnames neither new mirror.'record' | 'identity': an explicit'record'would be a second spelling of leaving the key out.@object-ui/i18n(minor). A new key,grid.import.invalidTime, with a{{value}}placeholder, is added besidegrid.import.invalidDatein all ten packs: en, zh, ja, ko, de, es, fr, pt, ru, ar. It is mirrored in the wizard'sIMPORT_DEFAULT_TRANSLATIONS, whichImportWizard.i18n-parity.test.tspins to the en pack.@object-ui/app-shell(patch). No reachable change:identityImportFieldsis not re-exported from the package entry.Changeset:
.changeset/11913-import-time-identity-email.md.File surface
6059931513, plus amendment6060072703, which addsimportCoercionContract.tsand its test.importCoercionContract.ts. It changes in the regions the amendment names: the time reader besideisImportableDateCell,isIdentityEmailbesideisRecordEmail, and the doc comments (isRecordEmail's, which said the user import's column "reaches the wizard typedtext", and the module header). ⚠ One more region, named here so the review can rule on it: the year-first tail of the privateimportDateCellYearmoved into a new privateyearFirstCellYear. The date and time readers now share one year-first reading instead of keeping two copies. The behaviour is unchanged: the existingisImportableDateCellpins pass unmodified.packages/i18n/src/__tests__/de-quote-pairing-3876.test.ts. This is a test beside the packs. Its de census pins (correctly paired spans, and the „ / “ counts) move from 75 to 76, because the new de row adds one matched pair. The history comment is extended in the file's own style.identityImportFields' return type stays the narrow shape. The email entry is typed at its literal againstImportWizardProps['fields'][number], soemailRuleis still checked where it is written.ObjectViewpassesidentityImportFields(...)orimportTargetFields(...)intofieldsthrough one conditional. That expression compiles only because subtype reduction collapses it to the identity shape.importTargetFields'ImportTargetField.options?: unknownis not assignable to the wizard'soptions.ImportWizardProps['fields'], or as aPickthat namesemailRule, turnspnpm --filter @object-ui/app-shell type-checkred at that conditional (TS2322). Both files are outside the surface, so this is recorded under Acceptance notes.Verification, at HEAD
551e5ae8(after mergingmain0bbb67b9)isImportableTimeCellwas compared with the installed@objectstack/core17.7.0's exportedparseDateCell(cell, 'time')over 1592 cells. These are the card's cells, 7 days × 11 clocks × 10 zones × 2 separators, bare days, the year-first forms and assorted prose. The server takes 281 of them, and the two disagree on 0.isIdentityEmailwas compared with the two 17.7.0 functions, restated verbatim from the tag because plugin-auth is not installed here. 19 cells, 0 disagreements.pnpm exec vitest run packages/plugin-grid/ packages/i18n/:Test Files 277 passed (277),Tests 3153 passed | 13 skipped (3166).pnpm exec vitest runon the four app-shell import files (identityImportFields.emailRule-11913,identityImport,identityImportSavedMapping,importTargetFields) plusObjectHooksPanel.newHookTarget-11820(the test objectui#11986 fixed onmain):Test Files 5 passed (5),Tests 31 passed (31).identityImportFieldsor the identity wrapper, or derive the generic target set. CI runs the whole suite.turbo run build --filter='@object-ui/app-shell^...'(28/28):pnpm --filter @object-ui/plugin-grid type-check,pnpm --filter @object-ui/i18n type-checkandpnpm --filter @object-ui/app-shell type-checkall exit 0. Each script name is echoed, and each runs bothtsc --noEmitandtsc -p tsconfig.test.json. The test configs includesrc/**/*.test.tsx, so the new test files are compiled too.pnpm exec eslinton the 17 touched TypeScript files: 0 errors and 11 warnings, all on pre-existing lines this diff does not add.check:control-bytes,check:test-path-roots,check:changeset-claims,check:pending-changeset-literals,check:i18n-keys,check:i18n-drift(1 key(s) addedagainst merge-base0bbb67b9),check:i18n-dead-keys,check:i18n-designer-parity,check:new-line-citations,check:spec-symbols,check:installed-pin-claimsandcheck:phantom-deps.scripts/check-changeset-presence.mjsandscripts/check-changeset-no-major.mjsalso exit 0.check:spec-floorsandcheck:readme-exportsexit 1 withno-artifactfindings only, on packages this branch did not build (app-shellin the floors gate,plugin-gantt,plugin-timeline,plugin-tree,cli). Neither namesplugin-grid, whose builtdistimportsClockTimeValueSchemafrom@objectstack/spec/data. That symbol exists at its^17.5.0floor.check:eager-locale-cataloguesand the eager-closure budget need a console build, so they are left to CI'sBundle Analysis.Reverse checks
These ran at
34ade8bd; the mutated files and the tests they ran are byte-identical at551e5ae8. Each one went throughscripts/ablation-replace.mjsin wrap mode: the anchor hit 1 → 0, the blob changed, and the restore was proven with blob equal to HEAD and an emptygit diff HEAD.timearm deleted fromcheckImportCell.ImportWizard.cellCheck-11913.test.tsxgoes red: 2 failed, 7 passed. Red: "marks what the server refuses with invalid_time" and "marks the time cells the server refuses and counts only the clean row". The10:00pin stays green, as expected with no arm.isRecordEmail(value). The plugin-grid and app-shell11913files go red: 4 failed, 7 passed. Red: the endpoint-rule marks, "not both", and the wizard count in both packages. The first attempt was refused by the tool before it ran anything, because the replacement text was a substring of the anchor. It was re-run with a unique replacement.emailRule: 'identity'flag dropped fromidentityImportFields. The app-shell11913file goes red: 2 failed. Red: the flag pin and the wizard count.The tree was clean (
git statusempty) after each one.Acceptance notes
Not filed, and not fixed here:
ImportTargetField.options?: unknown(importTargetFields.ts) is not assignable toImportWizardProps['fields'][number]['options'].ObjectView's importfieldsconditional compiles only through subtype reduction to the narrower identity shape. It is type-level only: nothing renders differently. The carrier is the next PR that touchesimportTargetFields.tsorObjectView's import wiring; there is no named one today.INVALID_EMAIL,INVALID_PHONE,NO_IDENTITY).formatDryRunErrorhas no case for them, so the endpoint's English text is shown. This is a read, not measured through the public door. Carrier: none.^17.5.0floor.@object-ui/plugin-griddeclares@objectstack/spec^17.5.0, and 17.5.x'sClockTimeValueSchemastill took a zone suffix. 17.6.0 dropped it. A host resolving spec 17.5.x therefore gets 17.5's clock grammar in the preview. This repository's lockfile installs 17.7.0. Carrier: none.Generated by Claude Code