fix(plugin-tree): detect a tree parent pointer only when it is this object's own (#7839) - #8030
Merged
Merged
Conversation
… object's own `detectParentField` returned the first `type: 'tree'` field whatever its `reference` said, so a foreign-shaped one was silently picked as the parent pointer and the forest was grouped on a pointer into a table it does not point at. The `tree` arm now mirrors `@objectstack/spec`'s own kernel predicate `hasDetectableParentField` term for term — accept when `reference` is absent, or when it equals the bound `objectName` — including the `objectName` guard, since an object whose name we do not know cannot be self-referenced. Skipping rather than returning also removes a masking bug the old early-return hid: a foreign `tree` declared before a self-referencing `lookup` used to win by position and the lookup never got its turn. The `lookup` / `master_detail` arm is otherwise unchanged. Fixture hygiene, same rule: four unit fixtures declared a `type: 'tree'` field with a target naming another object, a shape the parse door refuses. The key is optional on a `tree` and none of the four assertions reads it — every one of them selects by declared TYPE — so the key is dropped rather than renamed. None of the four declares an object identity a self-reference could name, and in the plugin-detail fixture the key was the retired snake_case spelling `FieldSchema` refuses BY NAME, so renaming it would have turned a refused key into an accepted self-annotation the fixture never made. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QnpvbdoRisQdRAczkLwnf5
This was referenced Sep 6, 2026
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-steve
marked this pull request as ready for review
September 6, 2026 11:54
This was referenced Sep 6, 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 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 #7839
Both items of the card, one PR. Branched from
origin/main@f5c8b8e(ahead of the dispatch anchor83a9d22); all six anchors in the claim comment re-verified there before editing.Item 1 — the
treearm mirrors the parse doorpackages/plugin-tree/src/ObjectTree.tsxdetectParentFieldreturned the firsttype: 'tree'field whatever itsreferencesaid. It now accepts one only whenreferenceis absent or equals the boundobjectName.Measured, not paraphrased. Read on objectstack
mainrather than taken from the order.refuseForeignTreeReference(packages/spec/src/data/object.zod.ts) skips a field unless it is foreign-shaped:so the accept set is exactly {absent, own name} —
referencestays OPTIONAL on atree, where the rule makes it a redundant self-annotation, and only a value naming another object is refused. The spec's kernel predicatehasDetectableParentField(packages/spec/src/kernel/functional-completeness.ts) reads the identical rule and carries the own-name guard asown !== undefined && def.reference === own; its docblock names this reader as the one place still out of step. That guard is mirrored too: an object whose name we do not know cannot be self-referenced. No signature change —objectNamewas already in scope.One behavioural half worth naming separately. The arm now skips rather than returns, so a foreign
treecan no longer mask a self-referencinglookupdeclared after it. Under the old early-return it won by position and the lookup never got its turn; that case renders flat today and is pinned below.Pin:
packages/plugin-tree/src/ObjectTree.treeArmOwnReference-7839.test.tsx, besideObjectTree.referenceArms-6837.test.tsxand in that file's form (2 live arms, 2 refusals, a degenerate-pass guard, 2lookupregression controls, 2 spec-anchored controls).The renderer tightens AHEAD of the spec copy this repo installs
Probed two-directionally on the copy this branch resolves through
packages/plugin-tree—@objectstack/spec@17.2.0, on an object namedbusiness_unit:{ type: 'tree' }{ type: 'tree', reference: 'business_unit' }{ type: 'tree', reference: 'other_object' }refuseForeignTreeReferencelanded on objectstackmainafter 17.2.0 was cut, and objectui tracks the spec by npm semver (^17.0.0/^17.2.0), not by SHA — so there is no pin to move and noBlocked-by:, exactly as the card says, but the reason is sharper than "does not depend on the pin": until that version bump, this function is the only door. The pin therefore asserts only the half 17.2.0 can answer (referenceis optional on atree, so the accept-when-absent arm is not blessing a shape the schema rejects) and records the missing leg as prose with instructions to add it, not delete it, when the version moves.Item 2 — the four fixtures: I dropped the key, and did not rename
The judgement call the card is wrong about. The card says all four spell it
reference. The fourth —packages/plugin-detail/src/__tests__/expandableFamily.identity-5874.test.tsx— spells itreference_to(triage 5556381290 caught this; re-confirmed here). So the second question had to be answered before choosing, and it was, by probing the installedFieldSchemawith a positive and a negative control:treefieldreferencereference_tounrecognized_keys, with the rename hint "Did you meanreference_totoreference?"unrecognized_keys, no rename hintIt is not a legal key. The rename hint is attached to a refusal, not to an acceptance — the negative control is what shows that, since it is refused too and gets no hint. So "rename to a self-reference" and "drop the key" are not equivalent, and the card authorising both is not the same as them being interchangeable.
Chosen: drop the key, in all four. One rule, applied uniformly:
Per fixture, with the reader that decides it:
core/.../expand-fields.test.tsf_tree->showcase_categorybuildExpandFieldsmatchesEXPANDABLE_FIELD_TYPESontypecore/.../predicate-record.test.tsparent->showcase_categorytoPredicateRecordcollapses by declared typeplugin-detail/.../identity-5874.test.tsxparent_node, retired snake key ->dealsdrawerField('parent_node').readonly === trueplugin-dashboard/.../identity-5692.test.tsparent_node->nodesisLookupType/computeLookupExpandreadtypeSo mechanism assumption 2 of the order holds for all four — nothing to stop and report.
Two reasons renaming was rejected rather than merely not-chosen:
nameat all (two are bare field maps; the dashboard schema declares noname), so a "self-reference" would be an arbitrary string no reader of the fixture could check against anything. In the one container that is named (name: 'deal'), the key was the refused spelling — renaming there would have converted a key the canonical schema refuses into one it accepts, i.e. added a spec-legal self-annotation the fixture never made, and left it the only field in that map in the canonical dialect while its siblings stay in the legacy one.f_tree -> showcase_categoryis a stale mirror of the showcase zoo; upstream corrected it (examples/app-showcase/src/data/objects/field-zoo.object.tsnow hasf_tree: { type: 'tree', label: 'Tree (self-reference)', reference: 'showcase_field_zoo' }). Re-pointing the mirror re-arms exactly that staleness. Dropping a key the spec calls redundant removes the class.No ruled flip. Mechanism assumption 3 checked: no existing pin asserted the foreign-shaped behaviour.
ObjectTree.settledSchemaKeying-6481.test.tsxcarries twotype: 'tree'fields, but both are genuine self-references (business_unit->business_unit,territory->territory) and it declaresparentFieldexplicitly anyway. Nothing needed a flip word.One comment in
ObjectTree.referenceArms-6837.test.tsxwas made stale by item 1 and is corrected in place (it said the detector "returns atreefield before it ever reads a target"). The correction strengthens that pin's own design rather than weakening it: with the tightening, atreecarrying only a refused spelling reads to this arm as having noreferenceat all and is therefore accepted, so probing the spelling chain on atreewould invert every refusal to green.lookupremains the right probe type.Verification
Ablation direction predicted before running, and matched. Restoring
if (def?.type === 'tree') return key;on the committed tree:Exactly the two refusal cases red; both live arms, the degenerate-pass guard, both
lookupcontrols, both spec-anchored controls, and the sibling-6837and-6481pins all green. Mutation proven on disk by anchor counts either side (guarded arm 1 to 0, bare arm 0 to 1) and by a blob-hash change; restored withgit checkout HEAD -- path, proven bygit diff HEADempty and by the hash returning to the HEAD blob. No rebuild leg: the pin imports the component by relative source path, so both legs resolve to source.Green run on the restored tree, at
dc16481:pnpm exec vitest run packages/plugin-tree/ packages/core/ packages/plugin-detail/ packages/plugin-dashboard/— exit 0, 355 files / 4608 tests passedpnpm exec turbo run type-checkover the same four — exit 0, 17 tasks. Test files are in scope: each package's script istsc --noEmit && tsc -p tsconfig.test.json, and the new pin was confirmed present in the plugin-tree program by--listFilespnpm exec turbo run lintover the same four — exit 0, 5 tasksGate family derived by hand from the changed paths (
scripts/pm/dispatch-gates.mjsdoes not exist in this repo); every exit code captured before any pipe, and each verdict quoted from the gate's own line:check:control-bytescheck-changeset-presencesrc/file changedcheck-changeset-fixedcheck-changeset-no-majorcheck-changeset-overwritecheck:phantom-deps@objectstack/spec/datain plugin-tree (declared:^17.0.0)check:vi-mock-specifierscheck:vi-mock-inheritcheck:unreferenced-sourcescheck:spec-symbolscheck-governed-queue-guard --testChangeset:
.changeset/7839-tree-arm-own-reference.md,'@object-ui/plugin-tree': patch. Not the empty-frontmatter form — this moves published behaviour insrc/, so it declares a real bump.skip-changesetdeliberately not applied; it is inert in this repo.Out of scope, filed not fixed
The
reference_tospelling that made the card's own enumeration miss its fourth fixture also hides three more foreign-shapedtype: 'tree'fixtures the card never names — inplugin-view,plugin-listandplugin-calendar. Same defect class, but outside this claim's declared file surface, andplugin-calendaris held by another in-flight claim, so they are filed rather than swept in here: see #8031 (no assignee, for PM triage). That issue is not addressed by this PR and remains open after it merges.Generated by Claude Code