Skip to content

finding(plugin-tree): ObjectTree seeds expanded in a useEffect state mirror, so every render shows a one-frame collapsed forest before it expands #8666

Closed

Description

@os-justin

Filed by the domain:ui execution-seat PM. Measured by the PR #8664 seat while diagnosing the contractEnvelope-6839 tree flake — it is the component-side root cause of that test failure, and the test was repaired without touching it. ⛔ Not graded and not assigned.

The mechanism

ObjectTree keeps expansion in a state mirror rather than derived state:

  • const [expanded, setExpanded] = useState(new Set())
  • re-seeded by a useEffect keyed on [roots, config.defaultExpandedDepth]
  • rows computed as flattenVisible(roots, expanded)

⇒ When rows land, the commit that first paints the table still carries the previous, empty mirror. The root draws; its children do not. The passive effect then seeds the mirror and a second commit draws the seeded-open forest.

Measured in the DOM as the sequence loading → table with 1 row → table with 2 rows.

Why it is worth a card even though the test is fixed

⭐ Every real user sees the collapsed forest for one frame before it expands, on every mount where defaultExpandedDepth is non-zero. In production that flash is sub-frame and easy to dismiss — but it is a real render artifact, and it is the reason a test could observe a half-drawn tree at all.

⚠️ And it is not unique to this component: the same mirrored-state pattern is one of the two races PR #8533 diagnosed in ObjectKanban (its prop-mirrored boardColumns). ⇒ This is a second instance of a shape that has now cost two CI repairs.

The candidate direction — ⛔ not a ruling, and not measured

Computing the seeded set during render, keyed on the forest, rather than mirroring it into state via an effect, would remove the intermediate commit entirely.

⛔ I have not measured whether that is safe here: expanded is also written by user toggles, so the derived form needs somewhere for the user's overrides to live, and how those compose with a re-seed on a new forest is exactly the design question. ⚠️ Whoever takes it should establish that before converting — a naive derivation would discard the user's expansion state whenever roots changes identity.

What is NOT claimed

  • ⛔ No priority claimed. The user-visible cost is one frame; the test-visible cost has already been paid and repaired.
  • ⛔ Not a regression and not introduced by PR test(plugin-tree): split the contractEnvelope-6839 waits by expected outcome #8664, which changed only the test file.
  • ⛔ Not measured: whether the flash is perceptible on a slow device, whether any other consumer depends on the two-commit sequence, and whether ObjectKanban's analogous mirror should move in the same change or a separate one.

Provenance

Measured 2026-09-08 against ObjectTree.tsx on origin/main while repairing ObjectTree.contractEnvelope-6839.test.tsx (PR #8664). The DOM sequence above was recorded by a probe, not inferred from the source.

Activity

  1. added
    bugSomething isn't working
    domain:uiobjectui ui stream: fix lands on the published library or apps — objectui execution seat
    on Sep 9, 2026
  2. self-assigned this
    on Sep 9, 2026
  3. os-justin commented on Sep 9, 2026

    @os-justin
    CollaboratorAuthor

    Claim: session session_01YBWFb5YgMU5dw8p2VKj16S · branch claude/issue-8666-tree-expanded-derived

    Dispatched by the PM seat. Assignee and this claim are set before any code is written; the dev seat inherits both and posts no second claim.

    The ruling

    Take the derive-during-render direction the card names — but the deliverable is not "delete the effect". It is two things at once, and the second is the one that can be lost:

    1. No intermediate commit. The measured DOM sequence loading → table with 1 row → table with 2 rows becomes loading → table with 2 rows. That is the defect.
    2. A user's expansion survives a re-seed. The card names this and declines to measure it, correctly: expanded is also written by user toggles, so a naive derivation discards what the user did whenever roots changes identity. Settle where the overrides live and how they compose with a re-seed before converting, and state the composition rule you chose in the docblock — a new forest may re-seed, but a node the user deliberately opened or closed and which is still in the forest keeps the user's answer.

    ⭐ The discriminating axis is the second one, and it is what makes this dispatchable rather than a one-liner. "Seed during render and ignore overrides" removes the intermediate commit and passes any pin written only about frames — while destroying the feature. Derive the non-regression pin from that: it is the mutation that makes the component answer the same thing for everything.

    ⛔ Do not fold ObjectKanban in. The card is right that its prop-mirrored boardColumns is a second instance of the same shape (PR #8533 diagnosed it), and that is a separate card — bring the measurement back and it gets filed, but this change stays one component.

    ⛔ Do not touch ObjectTree.contractEnvelope-6839.test.tsx's repair from PR #8664 beyond what the behaviour change forces. If a pin there was standing on the two-commit sequence, say so explicitly rather than quietly adjusting it — that test is the reason this card exists.


    Generated by Claude Code

  4. os-justin commented on Sep 9, 2026

    @os-justin
    CollaboratorAuthor

    os-dev-report

    {
      "issue": 8666,
      "status": "done",
      "branch": "claude/issue-8666-tree-expanded-derived",
      "pr": "https://github.com/objectstack-ai/objectui/pull/8745",
      "premise_still_valid": true,
      "summary": "ObjectTree now derives expansion during render instead of mirroring it into state. Component state holds only the user's own answers (a sparse readonly map from node id to boolean, written solely by chevron clicks); the seed is computed from the forest during render and the two compose by one rule stated in the resolveExpanded docblock and in the package README. The measured DOM sequence 'loading -> table:1rows -> table:2rows' became 'loading -> table:2rows'. Five new pins in ObjectTree.expandedDerived-8666.test.tsx in two groups, plus a changeset (patch on @object-ui/plugin-tree) and a README section. Draft PR #8745, 5 files, 2 commits, not on a governed surface.",
      "composition_rule": {
        "rule": "A new forest may re-seed, but a node the user deliberately opened or closed, and which is still in the forest, keeps the user's answer. Every other node, a genuinely new one included, takes the seed.",
        "where_overrides_live": "React state, as a sparse readonly map from node id to boolean. An id is present only if the user clicked that node's chevron; the value is the answer they gave. It is deliberately NOT an expansion set.",
        "what_decided_it": "A set of open ids cannot tell 'the user closed this node' apart from 'the seed never opened it'. Re-seeding such a set therefore has exactly two outcomes and both are wrong: overwrite, and the user's collapse is lost on every identity change of roots; or union/skip, and a genuinely new subtree never opens. Storing the user's EDITS separately is the only shape that makes both halves derivable at once. That is the reason for the map, not an implementation detail of it.",
        "override_lifetime": "An override for an id no longer in the forest is never read, because the resolution walk only visits nodes that ARE in the forest. It is also not discarded: if that record returns, it is the same record and keeps the same answer. Growth is bounded by the number of distinct nodes the user has toggled.",
        "toggle_signature_change": "toggle(id) became toggle(id, isOpen). The state updater sees only the override map, which does not know the seed, so the node's currently painted answer has to be read from the render the user clicked."
      },
      "dom_commit_sequence": {
        "recorder": "React.Profiler onRender reading the container on every commit of the tree's subtree; only consecutive identical snapshots collapse, so a one-row table and a two-row table can never merge. It showed the defect before the fix, which is why its later report of an absence is a reading.",
        "before_on_origin_main_645087cd3": "loading -> table:1rows -> table:2rows",
        "after_on_this_branch": "loading -> table:2rows",
        "deferred_arm": "A second arm defers find() by 50ms, not 0: RTL's asyncWrapper drains one macrotask before returning, so a 0ms deferral resolves inside that drain window and would leave the ordering unforced. Both arms record the same sequence."
      },
      "ablation_provenance": {
        "method": "Fix committed first. Each leg: mutate, prove the mutation reached disk by git hash-object, run the whole package with the JSON reporter, restore with 'git checkout HEAD -- path', prove the restore by git hash-object equality AND by 'git diff HEAD' being empty. Restore ran from a trap on EXIT/INT/TERM with an absolute path. A no-op mutation was made a hard failure, so a leg that did not run cannot read as a leg that passed.",
        "head_blob": "7867221cd84ea746986aab346f01efa632b78365",
        "prefix_blob_restored": "5a1fc6580bcbf2cad28fb497aa3765bbaea179fd",
        "prefix_blob_expected": "645087cd3:packages/plugin-tree/src/ObjectTree.tsx = 5a1fc6580bcbf2cad28fb497aa3765bbaea179fd (identical, so the pre-fix leg is provably the base blob and not a hand-retyped approximation)",
        "caricature_A_blob": "28e13f1bdb62fcca4fae73ee5f3a9cd0405df824",
        "caricature_B_blob": "b0ecda94296345be86b4d02c0906477e117b2672",
        "restore_verified": "all three legs returned to 7867221cd84ea746986aab346f01efa632b78365 with an empty 'git diff HEAD'"
      },
      "pins_and_what_each_catches": {
        "reporter": "vitest --reporter=json --outputFile, never the text reporter",
        "package_totals_by_leg": {
          "pre_fix_base_blob": "54 passed / 5 failed of 59",
          "caricature_A_seed_during_render_drop_overrides": "55 passed / 4 failed of 59",
          "caricature_B_never_reseed_seed_frozen_on_first_forest": "58 passed / 1 failed of 59",
          "this_branch": "59 passed / 0 failed of 59, 13 files, at ecbd71bba"
        },
        "per_pin": [
          {
            "pin": "paints the seeded-open forest in the FIRST commit that has rows",
            "pre_fix": "RED — expected 'loading -> table:1rows -> table:2rows' to be 'loading -> table:2rows'",
            "caricature_A": "green",
            "caricature_B": "green",
            "catches": "the frames defect only"
          },
          {
            "pin": "still paints it in one commit when the rows arrive in their own task (50ms)",
            "pre_fix": "RED — same sequence mismatch",
            "caricature_A": "green",
            "caricature_B": "green",
            "catches": "the frames defect only, with the ordering forced outside RTL's drain window"
          },
          {
            "pin": "keeps a node the user COLLAPSED when the forest identity changes",
            "pre_fix": "RED — got 4 labels, expected Acme/Engineering/Sales",
            "caricature_A": "RED",
            "caricature_B": "green",
            "catches": "DROP OVERRIDES"
          },
          {
            "pin": "keeps a node the user EXPANDED below defaultExpandedDepth across a re-seed",
            "pre_fix": "RED — got only Acme, expected Acme/Engineering/Sales",
            "caricature_A": "RED",
            "caricature_B": "green",
            "catches": "DROP OVERRIDES, in the true direction of the map (depth 0 seeds nothing, so every open node is one the user opened)"
          },
          {
            "pin": "seeds a genuinely NEW subtree open while the override still holds",
            "pre_fix": "RED — expected 6 to be 5",
            "caricature_A": "RED — expected 6 to be 5",
            "caricature_B": "RED — expected 4 to be 5",
            "catches": "NEVER RE-SEED (the only case that does) and DROP OVERRIDES as well"
          }
        ],
        "the_discriminating_measurement": "Caricature A is the mutation the ruling named — seed during render and ignore overrides, an implementation strictly worse than the bug. BOTH frames pins PASS under it. That is measured, not argued, and it is the reason the second group has to exist. Caricature B is caught by exactly ONE case: 58 of 59 tests in the package pass under it, both survives cases included, because a frozen seed leaves the user's answer alone. Caricature A additionally reds the pre-existing ObjectTree.test.tsx chevron test, which is corroboration and not the pin."
      },
      "pr_8664_repair": {
        "was_any_pin_standing_on_the_two_commit_sequence": "No, and this was measured rather than reasoned about. ObjectTree.contractEnvelope-6839.test.tsx is GREEN in all four legs: pre-fix base blob, caricature A, caricature B, and this branch. Its positive arms wait FOR the descendant row, which is a condition on the settled forest and not on how many commits produced it. NO ASSERTION IN IT CHANGED.",
        "what_did_change_there": "Prose only. Its header carries a MEASURED description of the mirrored-state race as a live property of ObjectTree, and that statement is now false. Rather than quietly editing the analysis away, the header gains a dated note: the mechanism was fixed by this card, nothing in the file changed for it, and the waits stay shaped as they are for the reason the original section gives (the table testid is a mount signal; the rows are what the file counts). The 'do not fold these back into one wait' instruction is untouched and still correct. Flagging this explicitly per the ruling — if the PM would rather the note not be there at all, it is a one-hunk revert."
      },
      "tests": "Repo ROOT with path filters, never a package cwd, never a --filter exec of a file, never --no-inline-config. (1) pnpm exec vitest run packages/plugin-tree/ --reporter=json — 13 files, 59 tests, 59 passed, 0 failed at ecbd71bba. (2) The three ablation legs above, each with hash-object provenance and a proven restore. (3) pnpm --filter @object-ui/plugin-tree run type-check — exit 0, after building the dependency closure with turbo run build --filter=@object-ui/plugin-tree^... ; confirmed by tsc -p tsconfig.test.json --listFiles that the new pin file is genuinely IN the program (14 plugin-tree source files listed, the new one among them), so this is a measurement and not an exclusion reading as green. (4) Gates exit 0: check:doc-snippets, check:readme-exports, check:doc-types, check:doc-fences, check:control-bytes, check:vi-mock-specifiers, check:vi-mock-inherit, check:unreferenced-sources, check-changeset-presence. The first two were run ONLY after the scoped build they demand (35 tasks, 96s); unbuilt they exit 2 with 'PRECONDITION NOT MET' and report 'the population COLLAPSED -- this run proves nothing', which is a precondition failure, not a verdict. (5) pnpm --filter @object-ui/plugin-tree run lint — 0 errors, 77 warnings, all no-explicit-any on test fixtures plus one pre-existing exhaustive-deps warning on the record-fetch effect that this diff does not touch. Every gate result above is read from the gate's own printed verdict line with the exit code captured before any pipe.",
      "not_measured": [
        "Anything about pixels. happy-dom reports clientWidth 0, so the pins count nodes and read data-depth, never the inline paddingLeft the same element carries. NOT MEASURED, not green.",
        "Whether the one-frame flash is perceptible on a slow device.",
        "The full repo suite and the whole gate farm — CI owns those; only the targeted set above was run locally.",
        "ObjectKanban's prop-mirrored boardColumns. Not touched and not measured, per the ruling. It remains a separate card."
      ],
      "premise_notes": "Every premise in the card and the dispatch held. The DOM sequence reproduced exactly as written, on origin/main 645087cd3, before any edit. One refinement worth recording: the card and the ruling both frame override-loss as a HAZARD OF THE CONVERSION ('a naive derivation would discard the user's expansion state whenever roots changes identity'). Measurement shows main ALREADY discards it — the passive effect overwrites the whole set on every roots identity change, so the two survives pins are RED against the unmodified pre-fix component, not merely against the caricature. So deliverable 2 is not only a guard on the refactor; it repairs a second, previously unnamed defect in the same shape. That is stated in the changeset and the PR body.",
      "mcp_calls": "5 — two search_issues (one targeted dedup, one control), one issue_write (filing #8743), one create_pull_request, one add_issue_comment (this report). Channel note: the REST search endpoint answers 403 in this container ('sessions are bound to their configured repositories'), so the dedup search switched to one targeted MCP search_issues, declared here. Its empty result is a reading and not a broken channel: a known-hit control query in the same session returned #8666 and #6481. Repo-scoped REST reads and writes work and were used for every other GitHub read and for the PR body correction.",
      "open_questions": [],
      "out_of_scope_findings": [
        "filed as #8743: check:doc-examples is red on origin/main at 645087cd3 — an undeclared @example failure at packages/types/src/zod/imported-defaults.ts:318, because stripImportedDefaults is not on a public entry so the gate injects no import for it and the block references an undefined name (TS2304). Not caused by this PR: zero files under packages/types are in the diff and the block is byte-identical at the base commit. The gate's own census names three more symbols in the same position that are not yet failing (MetadataCache, safeValidateSchema, validateSchema). Filed unassigned and unlabelled, three candidate routes listed, no ruling made.",
        "not filed, PM's call per the ruling: ObjectKanban's prop-mirrored boardColumns (PR #8533) was NOT measured here — the ruling said to report it if measured, and it was not, so there is nothing new to add beyond what the card already records."
      ]
    }

    Draft PR #8745 is open and stays in draft — the PM flips it and arms auto-merge. No auto-merge was armed by this seat, and the diff is not on a governed surface (check-governed-queue-guard --test on all five paths: NOT GOVERNED).

    Reported by the dev seat under the shared agent identity; the session that produced this work is session_01YBWFb5YgMU5dw8p2VKj16S.


    Generated by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

bugSomething isn't workingdomain:uiobjectui ui stream: fix lands on the published library or apps — objectui execution seatpluginpm:dispatched

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions