Uh oh!
There was an error while loading. Please reload this page.
fix(plugin-grid): feed the inline cell editor the row as dependent values, so a dependsOn lookup column is no longer permanently uneditable - #7193
Merged
Conversation
…lues
A `dependsOn` lookup column in an editable ObjectGrid was permanently
uneditable. `LookupField` resolves
`dependentValues ?? ctx.formValues ?? ctx.data ?? {}` and the grid's
`renderCellEditor` supplied none of the three, so the resolved record was
`{}` for every row, `dependenciesMissing` stayed `true`, and the trigger
rendered disabled ("Select region first") even when the row carried the
parent value.
PR #2216closed#2215 in two halves: the form renderer injects its live
watched record, and every picker takes the `dependsOn` chain as a hard
`baseFilter`. Half 2 is host-independent and was already live here; half 1
is per-host and the grid never got it. This supplies that missing input and
re-implements no cascade.
INTERIM: `ctx.row` is the saved record, so a parent edited but not yet
saved in the same row does not re-scope the child. Carrying the staged
record needs a seventh member on `renderCellEditor`'s published context
type (declared in `@object-ui/types`, #6882, and pinned by exact type
equality), which is a contract change tracked separately.
The `dependsOn` case in `lookupPickerKeys-7154.test.tsx` pinned the gated
behaviour and is UPDATED, not deleted.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012wwHa4aaFybxXrfmfHioDMCo-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012wwHa4aaFybxXrfmfHioDM
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
|
1 similar comment
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
|
os-warren
commented
Sep 1, 2026
CollaboratorAuthor
os-dev-report {
"issue": 7165,
"status": "done",
"branch": "claude/issue-7165-grid-dependent-values",
"pr": "https://github.com/objectstack-ai/objectui/pull/7193",
"premise_still_valid": true,
"summary": "MEASUREMENT 1: B IS Clause 2-engaged — so option A shipped as the labelled interim and ⛔ #7165 MUST STAY OPEN (the PR says 'Part of #7165', never a closing keyword). The editor context is published surface on both disjuncts of the test: `DataTableSchema.renderCellEditor` is declared in packages/types/src/data-display.ts with a Zod mirror, declared by maintainer ruling 2026-08-30 (#6882) replacing a `(schema as any)` cast, and its exact shape is pinned by an `Equal` (not `extends`) type assertion in packages/types/src/__tests__/data-table-declared-keys-6882.test.ts whose comment states the shape is pinned deliberately; ObjectGrid.tsx independently fences that package as 'published surface with its own review floor'. B needs a seventh ctx member, so it edits @object-ui/types, its Zod mirror and that maintainer-ruled pin. All three ruling conditions are met: the interim is labelled in code naming #7165 and the follow-up and stating exactly what is stale, the card is not closed, and B is filed as #7188. Premise re-measured on head 899730e0a: the existing pin still passed before my change, i.e. the defect was still live. A2.1/A2.3/A2.4/A2.5 all confirmed; A2.2 needed one CORRECTION — the declared contract always carried `row`, but ObjectGrid's LOCAL ctx annotation omitted it, so A was a two-part edit (widen the local annotation, then pass it), not a literal one-liner.",
"tests": "All runs from repo root via the shared verify lock (objectui forbids `pnpm --filter PKG test`; path filters only). BASELINE on 899730e0a before any edit: `pnpm exec vitest run packages/plugin-grid/src/__tests__/lookupPickerKeys-7154.test.tsx` -> 'Test Files 1 passed (1) / Tests 5 passed (5)' — the gated pin still passed, so the defect was live on head. AFTER FIX, same command -> 'Tests 1 failed | 4 passed (5)', 'AssertionError: expected lookup-trigger-regional_owner to be lookup-trigger-gated' — the pin encoded the defect, so it was UPDATED IN PLACE (not deleted), header prose rewritten to say so. NEW FILE packages/plugin-grid/src/__tests__/gridDependentValues-7165.test.tsx, 4 tests each with a live control in the same render: (1) dependsOn column now opens, control column also opens; (2) picker SCOPED — $filter.region='north' reaches the query, Person 07 (south) absent, while the no-dependsOn control over the same records DOES offer Person 07; (3) NEGATIVE CONTROL — a row whose saved parent is empty still gates, proving the gate was not disabled; (4) the interim's staleness pinned as current behaviour, with its own proof the staging landed (region cell renders pending 'south' while the saved record reads 'north'), which is what stops it passing vacuously. Both files together: 'Test Files 2 passed (2) / Tests 9 passed (9)'. ABLATION — direction predicted BEFORE running (remove `dependentValues={ctx.row}` -> the 4 new tests plus the updated 7154 case go red, the other 4 stay green). NO REBUILD NEEDED, and that is measured not assumed: root vitest.config.mts aliases every @object-ui/* specifier to that package's src and the tests import ../ObjectGrid relatively, so nothing resolves through dist and a source mutation is directly live. Mutation confirmed ON DISK by marker count (1 -> 0) AND blob hash (e8ab332e… -> f219c865…), never by the editing tool's exit code. Result: 'Tests 5 failed | 4 passed (9)' — exactly the predicted set, passing count included. Restore proven BY STATE with HEAD named explicitly (`git checkout HEAD -- ABSPATH`, never the bare form which restores from the index): git diff HEAD, git diff --cached and git status --short all empty and the blob hash back to e8ab332e…; the script carried `trap ... EXIT INT TERM` with absolute paths. GATES: at final head aba0c7499 (after merging main, which had advanced to 8ec11e14f / #7180) — full packages/plugin-grid/ suite 'Test Files 105 passed (105) / Tests 949 passed (949)', check:control-bytes OK (5958 files), check:phantom-deps OK, check:changeset-no-major OK. At f17b08047 (identical tree for every path this PR touches): type-check plugin-grid + closure 14 tasks successful, lint of the affected package in FULL (not narrowed) 2 tasks 0 errors (751 pre-existing warnings), check:self-import OK, check:vi-mock-specifiers / check:vi-mock-inherit OK (4106 files, 525 with mocks), check-governed-queue-guard --test = NOT GOVERNED (4 paths vs 5 surfaces). Verdicts read from each gate's own printed line, with exit codes captured after redirection, never through a pipe. NOT-MEASURED check cleared: `tsc -p tsconfig.test.json --listFiles` lists all three edited files among its 1770, so the green typecheck genuinely covers the new tests rather than excluding them.",
"mcp_calls": "11 — 2 issue reads, 2 targeted search_issues (each with a control word that hit: #7165 itself, then #7154/#2215), 2 issue creates, 1 PR create, 1 PR body read-back, 1 PR body update, 2 report comments. REST was 403 for repo-scoped reads and `gh` is absent, both measured with a control before switching channels.",
"open_questions": [
{
"question": "Should the grid TELL the user that a dependsOn picker is scoped by the SAVED parent while an unsaved edit to that parent is staged in the same row? The PM's ruling explicitly left this live rather than settled, and asked that it not disappear into a code comment.",
"options": [
"A. Say nothing (what this PR ships). The picker silently uses the persisted parent. Zero new surface, but it is the same silent-wrongness class as the three refusals this seat landed in #7146 / #7147 / #7148.",
"B. Surface a hint on the trigger/popover when the parent has a staged-but-unsaved value that differs from the saved one — e.g. 'scoped by the saved Region; save the row to re-scope'. Needs the staged record, i.e. it is gated behind exactly the same seam change as option B of this card.",
"C. Do not build a message at all: land #7188, which removes the staleness and therefore removes the question."
],
"recommendation": "C, and treat B as dead. Real business need: the staleness only exists because the interim exists, so a message would be UI built to describe a defect we have already scheduled to delete. Long-term soundness: contract-first — the honest fix is the declared seam member, not a renderer-side apology for a wrong input, and a warning would fossilize the interim as if it were a supported mode. Making AI-authored metadata hard to get wrong: an author declaring dependsOn should get correct cascading, not a caption explaining why it is stale — a warning trains authors to accept a wrong scope. Startup scope discipline: option B costs the SAME seam change as #7188 while delivering strictly less, which is the worst trade of the three. ⚠️ The one thing that would flip this to B: if #7188 is declined or deferred long, the silent staleness stops being temporary and then it must be surfaced. So this question should be closed by #7188 landing, not by a ruling that silence is acceptable."
}
],
"out_of_scope_findings": [
"filed as #7188: option B — carry the pending/staged record across the renderCellEditor seam so a dependsOn lookup is scoped by the in-flight parent (the seam change is spelled out: @object-ui/types ctx member + Zod mirror + the #6882 exact-shape pin + data-table.tsx passing `{ ...row, ...rowChanges }` + ObjectGrid reading it). Clause 2-engaged; #7165 stays open for it.",
"filed as #7190 (label `finding`): plugin-detail's InlineFieldInput supplies no dependentValues either (measured: zero non-test occurrences; control: 88 files across packages/), and plugin-detail never provides SchemaRendererContext `data` itself. ⚠️ Deliberately NOT graded a bug — a detail page renders ONE record, so ctx.data may legitimately rescue the cascade there, and no rendering measurement was made. The single probe that settles it is written into the card.",
"NOT filed, reported here as an observation for PM triage: ObjectGrid.tsx still lists `renderCellEditor` in its `ObjectGridDataTableSchemaHolds` type as an undeclared-but-live HELD key, with prose saying whether DataTableSchema should declare it is a pending packages/types ruling. That ruling HAPPENED — #6882 declared it on 2026-08-30 — so the hold entry and its surrounding census comment are stale. Harmless today (the intersection with the declared member is identical), but it is documentation that now states the opposite of the tree. Left untouched because it is outside this card and inside the file #7166 is adjacent to."
]
}Generated by Claude Code Generated by Claude Code |
os-warren
commented
Sep 1, 2026
CollaboratorAuthor
PM review — |
This was referenced Sep 1, 2026
os-warren
marked this pull request as ready for review
September 1, 2026 10:48
Uh oh!
There was an error while loading. Please reload this page.
This was referenced Sep 1, 2026
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 freeto 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.
Part of #7165
⛔ #7165 must stay open. This ships option A, the interim, and is deliberately not a closing reference to that card — the card stays open for option B, filed as #7188. See "Measurement 1" below — that is the fact that determined this.
Measurement 1 — B is Clause ②-engaged, so A ships as the labelled interim
The PM ruling on #7165 made the A/B choice depend on one measurement: is
renderCellEditor's editor context type published surface, or internal toplugin-grid? Measured on899730e0a, both disjuncts of that test are true:Reachable from
@object-ui/types—packages/types/src/data-display.tsdeclaresrenderCellEditor?: (ctx: { column; row; value; stage; commit; cancel }) => React.ReactNodeonDataTableSchema, with a Zod mirror inpackages/types/src/zod/data-display.zod.ts.Part of
DataTableSchema's declared editor-context contract — declared by maintainer ruling 2026-08-30 ([Decision] DeclarerenderCellEditorand schema-levelcellClassNameonDataTableSchema? — the two live undeclared keys the #6459 census measured #6882), replacing the(schema as any)cast that preceded it. The shape is pinned by exact type equality, notextends, inpackages/types/src/__tests__/data-table-declared-keys-6882.test.ts:(angle brackets transcribed as square brackets — GitHub's body sanitizer eats
tag-shaped fragments, including inside a fence. The source uses angle brackets.)
with the stated reason: "Declaring the key with any other shape is a different (and false) statement about the renderer, so the shape is pinned, not just the membership."
packages/plugin-grid/src/ObjectGrid.tsxstates the same fence in its own words: "⛔ Do not 'fix' either hold by declaring the key onDataTableSchemaas a rider — that package is published surface with its own review floor."⇒ B needs a seventh member on that declared context. That edits
@object-ui/types, its Zod mirror, and a maintainer-ruled exact-shape pin. Per the ruling, A ships under all three of its conditions: labelled in code, this card not closed, and B filed (#7188).The defect
LookupFieldreads the record it gates on asdependentValues ?? ctx.formValues ?? ctx.data ?? {}. The grid's inline cell editor supplied none of the three —renderCellEditorrenderedFieldEditWidgetwithfield/value/onChangeonly,SchemaRendererContexthas noformValues, and the grid sets noctx.dataper row. The record was therefore{}for every row,dependenciesMissingwas permanentlytrue, and a column declaringdependsOnrendered a disabled trigger reading "Select region first" even when the row carried the parent value.PR #2216 was the two-half answer to #2215. Half 2 — every picker taking the
dependsOnchain as a hardbaseFilter— is host-independent and was already live on the grid path, which is why the gate fired at all (verified: A2.5 holds, seeLookupField'sdependentFiltermemo). Half 1 is per-host and the grid never got it. This PR supplies that missing input and re-implements no cascade.The change
One line of behaviour in
ObjectGrid.renderCellEditor—dependentValues={ctx.row}— plusrow: anyadded to the local ctx annotation, which had omitted it (the declared contract always carriedrow; only this local annotation did not).FieldEditWidgetalready forwardsdependentValuesto the widget, since #7008.The interim is labelled in the code, naming this card and #7188, and stating precisely what is stale: a parent edited but not yet saved in the same row does not re-scope the child.
Evidence
Rendering proof,
packages/plugin-grid/src/__tests__/gridDependentValues-7165.test.tsx(4 tests, each with a live control in the same render):dependsOncolumn opens (waslookup-trigger-gated+disabled; nowlookup-trigger-regional_owner+ enabled) — control: a no-dependsOncolumn in the same render is also enabled, so the picker path is proven reached.region: 'north'reaches the query as a hard$filterand Person 07 (south) is absent — control: the no-dependsOnsibling over the same records does offer Person 07, so the absence is scoping and not a dead picker.'south'while the saved record still reads'north'), because without that the assertion would pass for the trivial reason that nothing was ever staged. The grid's inline cell editor scopes adependsOnlookup by the SAVED row, not the staged one — carry the pending record across therenderCellEditorseam (option B of #7165) #7188 flips this test.The #7154 pin was UPDATED, not deleted. The
dependsOncase inlookupPickerKeys-7154.test.tsxpinned the gated behaviour, i.e. it encoded the defect. A deleted pin is indistinguishable from a pin that never existed, so it is rewritten in place to assert the fixed behaviour plus the scoping, and its header prose is rewritten to say so.Ablation — direction predicted before running: removing the single
dependentValues={ctx.row}line turns red the 4 new tests plus the updated #7154 case, and leaves the other 4 #7154 cases green.vitest.config.mtsaliases every@object-ui/*specifier to that package'ssrc, and the tests import../ObjectGridrelatively — so nothing resolves throughdistand a source mutation is directly live.dependentValues={ctx.row}: 1 → 0) and blob hash (e8ab332e…→f219c865…), not by the editor's exit code.Tests 5 failed | 4 passed (9)— exactly the predicted set, passing count included.HEADnamed explicitly (git checkout HEAD -- ..., never the bare form, which restores from the index):git diff HEAD,git diff --cachedandgit status --shortall empty, and the blob hash back toe8ab332e…. The script carried atrap ... EXIT INT TERMwith absolute paths.Gates
f17b08047;mainthen advanced (#7180 landed), so it was merged in and the affected-package suite plus the cheap ratchet gates were re-run at the current headaba0c7499. Nothing below is quoted from a tree that is no longer this branch's head.Re-run at
aba0c7499(current head, post-merge):packages/plugin-grid/full suitecheck:control-bytescheck:phantom-depscheck:changeset-no-majorRun at
f17b08047(same tree for every path this PR touches — the merge brought in onlypackages/components/src/renderers/layout/containers.tsx, a.changeset, a docs page, a components test and onevitest.config.mtsline, none of which this PR edits):type-check(plugin-grid + closure)lint(affected package, full, not narrowed)check:self-importcheck:vi-mock-specifiers/check:vi-mock-inheritcheck-governed-queue-guard --testThe typecheck genuinely covers the new tests:
tsc -p tsconfig.test.json --listFileslists all three edited files among its 1770 (this package typechecks tests via a dedicated project, so "typecheck green" is not silently excluding them).aba0c7499. It touchesrelationalMetaKeys.tsand theapplyRelationalMetacopy set; this PR touchesObjectGrid.renderCellEditor's context construction. Disjoint symbols, but this branch will need a fresh merge if #7166 lands first.Incidence — measured, and ⛔ not "no impact"
Two corpora, each instrument run with a live control:
dependsOn|depends_on, and none is this defect — the gantt hit isdepends_onas task predecessor links (a different key entirely) and the other is aselectinside a form. Control: the same instrument returns 88 files acrosspackages/.hotcrm(a real shipped app, anonymous shallow clone): 6dependsOndeclarations on lookup fields across 5 objects (crm_contract×2,crm_case,crm_quote×2,crm_opportunity), all scoping contacts tocrm_account. But 0 editable grids in its views (control: the views directory exists and was read).⇒ Neither measured corpus reaches the defect today, because none combines
dependsOnwith an editable grid — buthotcrmis one metadata key away:editable: trueon any of those five objects' list views hits it immediately. Customer apps are unmeasured, and the parent pattern is demonstrably real in shipped metadata. This is a bounded reading, not an all-clear.Filed, not ridden
dependsOnlookup by the SAVED row, not the staged one — carry the pending record across therenderCellEditorseam (option B of #7165) #7188 — option B, with the seam change spelled out. bug(plugin-grid): adependsOnlookup column is permanently uneditable in ObjectGrid — the inline editor supplies no dependent values, so the picker gates forever #7165 stays open for it.InlineFieldInputsupplies nodependentValueseither — whether adependsOnlookup gates on the detail page is UNMEASURED and turns onctx.data#7190 —finding:plugin-detail'sInlineFieldInputsupplies nodependentValueseither (measured: zero non-test occurrences; control: 88 acrosspackages/).ctx.data, which a one-record detail page plausibly does — so it is afindingwith the probe written down, not a claim that the detail page is broken.Session:
session_012wwHa4aaFybxXrfmfHioDM(https://claude.ai/code/session_012wwHa4aaFybxXrfmfHioDM) — kept in prose because a body edit rewrites the footer below to its bare form.Generated by Claude Code