Skip to content

ui-react: hygiene-review layout test reads the item before it renders #1685

Open

Description

@schlessera

Context

On PR #1628, CI run 38175368489, job layout (1) (114576698355) failed once:

FAIL ui-react-layout (chromium) ../ui-react/tests/browser/hygiene-review.layout.tsx > Later confirms the stored hygiene option and prints the server's waitUntil receipt
TypeError: Cannot read properties of null (reading 'dataset')
 ❯ ../ui-react/tests/browser/hygiene-review.layout.tsx:374:23

The test reads current().dataset.hygieneId immediately after await scene() (test("Later confirms the stored hygiene option, packages/ui-react/tests/browser/hygiene-review.layout.tsx:372-374). current() (packages/ui-react/tests/browser/hygiene-review.layout.tsx:111-113) is a synchronous querySelector with a non-null assertion. If the hygiene item has not rendered when scene() resolves, it returns null. PR #1628 changed only failed-turn report code. On the next head (e968372a, run 38176899684), the same layout shard passed with no code change near hygiene review. The failure looks like a rendering race in the test setup.

Scope

  • Make the test wait until the hygiene item is rendered before it reads it. For example, scene() or this test could poll for [data-hygiene-id], and the other tests that call current() straight after scene() should get the same fix.

Out of scope

Acceptance criteria

  • No test in hygiene-review.layout.tsx reads current() before the item is known to be rendered.
  • Repeated local runs of the file in the pinned browser image (node scripts/visual.mjs --project=ui-react-layout) pass. A mutation that delays the item's render makes the old code fail on the null read and the new code pass.

Verification

  • Repeated ui-react-layout runs, with the delayed-render mutation receipt recorded in the PR.

Contract impact

None.

Current-source diagnostic handoff — 2026-10-12

Inspected main acec591c83a730e5d68f36bae2b807f95bb5d9c9: scene (packages/ui-react/tests/browser/hygiene-review.layout.tsx:57-110) mounts through flushSync then waits a fixed 160ms settle (:53-55). current (:111-113) synchronously queries the item, while the Later test immediately reads its dataset (:372-374). flushSync does not establish completion of the review request and item rendering. The original CI null read is observed, but request/render phase timing is not captured; The controlled delayed-response reproduction below resolves that readiness gap; it does not identify the original CI runner scheduling event. Scope is harness readiness, so type: test is appropriate; preserve explicitly deferred p3. Native #736 fits the deterministic keyless-verification outcome.

Controlled pinned-browser reproduction — 2026-10-12

Source acec591c83a730e5d68f36bae2b807f95bb5d9c9, frozen install and built exports; Bun 1.4.2, Vitest 4.1.11, pinned mcr.microsoft.com/playwright:v1.63.0-noble image ID sha256:eff16c30e6f3f4af0a03fa4b706120d5e9b0891c344a27d64559aff5900a4a27, checksum-verified fonts, network disabled, fileParallelism=false. From ui-kit, run node /repo/node_modules/vitest/vitest.mjs run --project=ui-react-layout --browser.fileParallelism=false ../ui-react/tests/browser/hygiene-review.layout.tsx -t 'Later confirms the stored hygiene option' in the pinned offline container with the fixture’s Bun binary available.

  • Unmodified focused control: exit 0; one selected test passes, 57 filtered.
  • Add only await new Promise(resolve => setTimeout(resolve, 600)) after reviewReads++ in the existing review-request interception, before delegating to real fetch. The actual Later test fails with TypeError: Cannot read properties of null (reading 'dataset') at the unchanged current().dataset.hygieneId read (instrumented line 375; original line 374). Exit 1, one named test fails; request/fixture startup succeeds and no test deadline is relaxed.
  • Restore the source byte-for-byte and rerun the same focused selection; exit 0, one selected test passes and 57 are filtered, with the same named command.

This establishes that fixed scene settling does not wait for actual review data/render completion. Implement condition-based readiness in the existing scene/appropriate callers, preserve Later receipt and neighboring hygiene assertions, and prove the correction under this delayed-response condition. No product fix was implemented by triage. With the complete existing brief and a controlled failing boundary, remove needs: repro and add agent-ready; preserve explicit p3 and native #736 membership.

Activity

  1. added
    type: fixSomething is broken and must behave the way it is documented.
    area: ui-reactReact chat/files/voice components, stores, WebSocket transport.
    area: ciCI workflows, the gates they run, release scripts, packaging.
    priority: p3Someday. Needs a driving use case before it starts.
    needs: reproBlocked on a reproduction.
    type: testCoverage, harness or proof work with no behaviour change.
    and removed
    type: fixSomething is broken and must behave the way it is documented.
    needs: reproBlocked on a reproduction.
    on Oct 11, 2026
  2. added
    agent-readyContext and acceptance criteria are complete enough for an agent to take this unattended.
    on Oct 11, 2026
  3. schlessera commented on Oct 11, 2026

    @schlessera
    OwnerAuthor

    2026-10-12 triage: the real pinned-browser test passes unmodified, then fails at the named null-dataset assertion when only the review response is delayed 600ms. The source is restored; this is reproduction, not an implemented repair. Folded conditions, commands and outcomes into the body; changed type to test, removed needs: repro and added agent-ready. Explicit p3 is preserved. Native #736 owns the complete deterministic harness-readiness outcome.

  4. schlessera commented on Oct 24, 2026

    @schlessera
    OwnerAuthor

    2026-10-24: moved from #736 to its nested browser-verification epic #2753. #736 hit GitHub's 100-direct-sub-issue limit, so its browser children now sit under #2753. This issue's scope, labels and readiness are unchanged.

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    agent-readyContext and acceptance criteria are complete enough for an agent to take this unattended.area: ciCI workflows, the gates they run, release scripts, packaging.area: ui-reactReact chat/files/voice components, stores, WebSocket transport.priority: p3Someday. Needs a driving use case before it starts.type: testCoverage, harness or proof work with no behaviour change.

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions