fix(dictionary): repair the stat-list markup, drop the nested main landmarks, and finish the header sweep - #2132
Conversation
…ndmarks, and give both zero-result views a state An axe sweep of every Dictionary route at desktop found four things the first pass left behind. The sources stat strip was a serious `definition-list` violation: a `div` inside a `dl` may contain only `dt`/`dd`, and each tile put the hint in a sibling `p`. The hint moves inside the `dd`. `/dictionary/sources` is now axe-clean. Three routes rendered a second `main` landmark inside the one `InformationPageShell` already provides — the topics list, the topic detail terms column, and the term page body — tripping `landmark-no-duplicate-main`, `landmark-main-is-top-level` and `landmark-unique`. Each becomes a `div`; no id, target or scroll anchor moves. A search that matched nothing dropped its card and stood bare on the page background, reading as a failed render rather than an answer, and its advice named a filter even when none was applied. It now wears the same card as the result list, and the no-filter case offers Browse all terms. A topics list narrowed to nothing by a `?kind=` or `?q=` deep link rendered "0 collections" above an empty rule and no explanation. It gets an empty state, and Clear now drops both narrowing inputs rather than only `kind` — clearing just `kind` left a `?q=` visitor stuck on an empty page. Two findings are deliberately left: `region` (3 nodes) and `landmark-no-duplicate-banner` both come from the global composer and universal header, reproduce on Dictionary routes this branch never touched, and belong to shared chrome rather than this change. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QMJ4s1H93vDAUjK3KudBzZ
…ed-chrome a11y finding Browse was the one Dictionary route whose header this branch had not touched, so it still carried the descriptive line the other four lost — "Scan the same source-linked result system by letter or abbreviation." — leaving the mode inconsistent with itself. It goes, and the sort control drops `ml-auto` below `sm`: on a phone the toolbar wraps, and a pushed-right sort landed alone on a second line hanging off the right edge instead of reading as part of the control group. Two headers are deliberately kept. The topic-detail description states that collection's scope, which is content rather than decoration, and the Dictionary mode-home subtitle comes from the shared mode-home template that all thirteen modes use. Also queues one ledger request: an axe sweep of the six Dictionary routes found `region` (3 nodes) and `landmark-no-duplicate-banner` coming from the global composer and universal header, not from this mode. Both reproduce on routes this branch never touched, both live in shared chrome across all thirteen modes, and repo axe gates only fail on critical/serious — so they have never gone red. Recorded rather than fixed here: a shared-chrome change under a Dictionary PR is the wrong blast radius. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QMJ4s1H93vDAUjK3KudBzZ
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
This pull request has been ignored for the connected project Preview Branches by Supabase. |
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in:34 minutes Limit details: You’ve used the included review currently available. Your 101 included PR review attempts over the past 7 days set your current allowance at 1 review per hour. Your organization has reached its usage spending cap. Adjust your spending cap in the billing tab. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits within each organization. For paid Pro and Pro+ reviews, CodeRabbit uses a developer's included PR review attempts over the past 7 days to set the current hourly allowance. At typical activity levels, the full plan allowance applies. Higher sustained activity can lower the allowance until earlier attempts leave the 7-day window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (6)
Comment |
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QMJ4s1H93vDAUjK3KudBzZ
No code fix needed — required CI checks all pass on head db3069a (the PR-required failure was lighthouse-budget being cancelled with no real failure); 0 actionable review threads. Attempted a main sync but the push was blocked by the ledger-write-discipline guard, which misreads this PR's own still-pending inbox request as a partially applied reconciliation batch from main. Not overridden; PR head left unchanged pending human review of the guard interaction. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015qDHwBNZYyRLdJHqSkiYTg
Uh oh!
There was an error while loading. Please reload this page.
Summary
Follow-up to #2114, which merged while these two commits were still being written. They were pushed to that branch after the squash landed, so they never reached
main; this PR replays them on the current base. No content is new since they were written — the branch was restarted frommainand the two commits cherry-picked unchanged.definition-list(serious axe violation) — the/dictionary/sourcesstat strip put each tile's hint in a<p>beside the<dt>/<dd>, and a<div>inside a<dl>may contain only<dt>/<dd>. The hint moves inside the<dd>./dictionary/sourcesis now axe-clean.mainlandmarks on three routes — the topics list, the topic-detail terms column and the term page each rendered a second<main>inside the oneInformationPageShellalready provides, trippinglandmark-no-duplicate-main,landmark-main-is-top-levelandlandmark-unique. Each becomes adiv; no id, scroll anchor or nav target moves. This predates feat(dictionary): retitle the search route, strip page search bars, and rebuild the sources page #2114 — the repo's axe assertions only fail on critical/serious, so it never went red.?kind=or?q=deep link showed "0 collections" above an empty rule and no explanation. It gets an empty state, and Clear now drops both narrowing inputs rather than onlykind: clearing justkindleft a?q=visitor stuck on an empty page.ml-autobelowsm: on a phone the toolbar wraps and a pushed-right sort landed alone on a second line, hanging off the right edge instead of reading as part of the control group.docs/outstanding-issues-inbox/91536913….json) recording the two axe findings that are not this mode's:region(3 nodes — the composer label, the privacy-warning span and the privacy link, all outside any landmark) andlandmark-no-duplicate-banner/landmark-unique(the global#searchuniversal header renders a second banner). Both reproduce on Dictionary routes this branch never touched, both live in shared chrome across all thirteen modes, and both are recorded rather than fixed here — a shared-chrome change under a Dictionary PR is the wrong blast radius.Two headers are deliberately kept: the topic-detail description states that collection's scope (content, not decoration), and the Dictionary mode-home subtitle comes from the shared mode-home template every mode uses.
Verification
Re-run against this base after the replay, not carried over from the pre-merge branch.
npm run lint— clean,--max-warnings 0npm run typecheck— cleannpm run test—Test Files 673 passed (673) · Tests 7276 passed | 4 skipped (7280)npm run verify:uiwhen UI, routing, styling, browser behavior, reduced-motion, or forced-colors behavior changed — run as the affected Chromium journeys against an isolated production server:tests/ui-dictionary.spec.ts→6 passed (27.3s), covering the 320/390/1440 overflow sweep and the axe pass under dark + forced colors.@axe-core/playwrightsweep of the six Dictionary routes at 1440px confirms/dictionary/sourcesis clean and the three landmark rules no longer fire on/dictionary/topics,/dictionary/topics/[slug]or/dictionary/[slug]. Screenshots taken at 390px and 1440px for the changed states.npm run verify:pr-local— its heavy scope repeats the lint, typecheck and unit results pasted above, and the browser evidence for this diff is the journey set named.npm run eval:retrieval:quality,npm run eval:rag,npm run eval:quality,npm run check:production-readiness,npm run verify:release— all provider-backed, and no retrieval, ranking, selection, chunking, scoring, answer-generation or environment behaviour changed.Risk and rollout
mainelements becomedivs, one<p>moves inside its<dd>, two empty states gain copy and a card, and one header line is removed. No route, handler, data path or contract changes. The landmark change makesgetByRole("main")less ambiguous rather than more.git reverteither commit independently; they touch overlapping files but not overlapping hunks.Clinical Governance Preflight
Clinical KB Database(sjrfecxgysukkwxsowpy)No clinical content changes. The sources page keeps its "Source linked is not specialist clinical approval" callout, its approval-pending wording, and its per-source region and accessed dates verbatim; the stat tiles remain derived from
dictionary-data.tsrather than authored. The two new empty states assert nothing about clinical content — they explain why a list is empty and offer a way back. No SaMD-relevant decision-support behaviour changed.RAG impact: no retrieval behaviour change — no file under
src/lib/rag/**, clinical-search, retrieval-selection, ranking-config, answer-ranking, the eval harness or the golden fixture is touched; the diff is four presentation files plus one queued ledger request.Notes
mainand the commits cherry-picked; the pre-restart tip is kept locally asbackup/dictionary-orphans-1f69abe.npm run issues:reconcilefrom a serialized branch after this lands.Generated by Claude Code