test(ui-smoke): scope the DSM page-root assertions to the visible owner - #2714
test(ui-smoke): scope the DSM page-root assertions to the visible owner#2714BigSimmo wants to merge 8 commits into
Conversation
Clicking into "Find a mode" in the header mode menu drew a sharp accent rectangle around the text while the magnifier icon sat outside it. The input's Tailwind `outline-none` cannot win against the unlayered shared text-field rule in globals.css, which paints an outline on any nested input on focus-visible. Adopt the established fix pair instead of inventing a new override: the container becomes the `search-shell` focus owner and the input carries `search-shell-input`, whose unlayered rule clears the outline and the inset shadow. The Tailwind `focus-within:border-*` utility is dropped because `.search-shell:focus-within` already owns that border shift, so the control keeps a single focus affordance. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WJ1C29Kazq72SbLHUYZLQX
Production UI shard 1 failed on PR #2638 with a strict-mode violation: `getByTestId("dsm-search-page")` resolved to two elements, the live one under `mobile-composer-reserve-pad` and a hidden streaming twin beside it. The PR's diff was two class names on the header mode-menu search box, which that test never opens, and the same shard passed on the base commit minutes earlier. This is the known `#93` artefact: Next streaming can leave a hidden duplicate page root in the tree under full-suite load. The repository already adjudicated it as a test-side concern and ships `visibleByTestId` for exactly this — its docstring names page-root and shell surfaces as the intended callers, and `tests/dsm-ui-smoke.spec.ts` already scopes the same testid that way. These two call sites in `ui-smoke.spec.ts` were simply never converted, and the file already imports the helper. `expectSingleSettledOwner` is deliberately not used here: the duplicate is hidden and invisible to users, so requiring it to disappear would assert more than the established decision supports. Verified: `npm run test:e2e -- tests/ui-smoke.spec.ts --project=chromium --grep "DSM"` — 2 passed (8.0s); eslint clean on the changed file. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WJ1C29Kazq72SbLHUYZLQX
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Advanced Run ID: Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_4e02d503-6fbd-4a04-890f-ae8d6f31df5e) |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 293d0641cf
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Addresses the Codex review finding on PR #2714. Scoping only the `dsm-search-page` root assertion left every locator after it resolving across both copies, so the same strict-mode violation would reappear one line later as soon as the #93 streaming twin exists: the query ribbon and `dsm-search-result` in the first test, `dsm-category-filter-desktop` in the second, and the diagnosis page root and its h1 after navigation. Each test now owns its visible page root and derives the in-page locators from it, as tests/dsm-ui-smoke.spec.ts already does. Two locators deliberately stay page-scoped, because scoping them is wrong rather than merely unnecessary: `dsm-category-filter-panel` renders through `OverlayPortal` (result-filter-control.tsx), and the "Back to dsm-5" control sits in the in-page nav header, which also portals out of the page root. Both live outside the subtree, so a page-root scope cannot find them — confirmed by the back-link assertion failing under the over-scoped form before this correction. Verified: `npm run test:e2e -- tests/ui-smoke.spec.ts --project=chromium --grep "DSM"` — 2 passed (6.4s); eslint and prettier clean on the file. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WJ1C29Kazq72SbLHUYZLQX
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_e4501e9b-fae3-46ee-92ed-28743a9875a5) |
2 participants
Summary
getByTestId("dsm-search-page")resolved to two elements — the live one undermobile-composer-reserve-padand a hidden streaming twin beside it.maincommit it merged by exactly those two lines, and that same commit ran the identical shard minutes earlier and passed.#093artefact recorded intests/playwright-settlement.ts: Next streaming can leave a hidden duplicate page root in the tree under full-suite load, so a baregetByTestIdtrips strict mode.visibleByTestIdfor it. Its docstring names page-root and shell surfaces as the intended callers,tests/dsm-ui-smoke.spec.tsalready scopes the same testid that way, andui-smoke.spec.tsalready imports the helper. These two call sites were simply never converted.expectSingleSettledOwneris deliberately not used: the duplicate is hidden and invisible to users, so requiring it to disappear would assert more than the established decision supports.Verification
npm run test:e2e -- tests/ui-smoke.spec.ts --project=chromium --grep "DSM"—2 passed (8.0s), covering both changed assertionsnpx eslint tests/ui-smoke.spec.ts --max-warnings 0— cleannpx vitest run tests/playwright-project-cadence.test.ts—Test Files 1 passed (1) / Tests 2 passed (2)npm run format— no change to the edited filenpm run verify:pr-local— not run in this session; deferred to CInpm run verify:ui— UI verification not run in full: focused browser proof at the two changed tests only, full suite left to CInpm run verify:release— not applicable, no release or handoff confidence claimNote on what the focused run does and does not prove: the duplicate only appears under full-suite load, so a local pass shows the change is correct and non-regressive, not that it exercises the failing condition. The helper it adopts is the repository's existing, documented remedy for exactly that condition.
Risk and rollout
Clinical Governance Preflight
Not applicable. Test-only change to browser assertion scoping; no ingestion, answer generation, search or ranking, source rendering, document access, privacy, production environment, or clinical output is affected.
Notes
main) so this follow-up fast-forwards rather than needing a force-push. The net diff againstmainis the two assertion lines.🤖 Generated with Claude Code
https://claude.ai/code/session_01WJ1C29Kazq72SbLHUYZLQX
Generated by Claude Code
Note
Low Risk
Test-only assertion scoping in one spec file; no production, auth, or data paths touched.
Overview
Fixes Playwright strict-mode failures when Next.js streaming leaves a hidden duplicate of DSM page roots in the DOM during full-suite runs.
Two DSM tests in
ui-smoke.spec.tsnow resolvedsm-search-pageanddsm-diagnosis-pagevia the existingvisibleByTestIdhelper and chain ribbon, result rows, and diagnosis headings from that scoped root—matchingdsm-ui-smoke.spec.tsand the#093settlement documented inplaywright-settlement.ts. Comments note that category filter panels and breadcrumb back links stay page-scoped because they render through portals outside the page subtree.Reviewed by Cursor Bugbot for commit 053891b. Configure here.