Give Sources' Topics and Publishers the chrome the Catalogue already has - #2593
Conversation
Topics and Publishers said "ADHD 5" and "Beyond Blue · 1 source". A count answers "how much" and nothing else: five sources under a topic can be five current WA guidelines or five review-required orphans with no publisher, and the reader could not tell without opening the filtered catalogue five times. `browse-facets.ts` aggregates the three signals the catalogue tile already leads with — quality band, jurisdiction, currency — per topic and per publisher, plus the highest-rated member so a heading can name its own best source. It reuses the catalogue's own `compareQuality`, `compareText` and `normalizeSearchValue` (now exported) rather than reimplementing them, so a facet list cannot order or match itself differently from the catalogue it links into. Expiry is deliberately not a recency candidate: a source that expires in 2027 is not thereby current, and reading a future expiry that way is exactly the inference the Method page forbids. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Sh8qHzezbEjWGp4MGJtjyk
Two of the four Sources tabs were bare server-rendered lists: no count, no filter, no empty state, and — the defect a reader actually hits — no reaction to the composer. The query stayed on screen while Topics listed every topic in the catalogue, which is a browse result asserted for a search that never ran. Both tabs now render through `SearchResultsLayout` + `SearchResultsHeaderBand` + `SearchResultsEmptyState`, so the count, the query subject, the applied-filter shelf, the badged phone filter trigger and the one-line phone band are the same components the Catalogue uses. `?q=` narrows the list and the band reports the honest count for it. The partial-catalogue note now appears here too: topic and publisher coverage is derived from the same entries, so it is exactly as incomplete when the hosted-document loader is unreachable. A row says what its sources are worth. The quality mix is a segmented meter with its tally stated in words beside it — band is a clinical signal, so it is never carried by hue alone — plus attention count, latest review, top publishers or topics, and where PsychSift uses them. Publishers keeps its jurisdiction sections and WA-first order, but the section heading becomes a compact label row instead of the `text-xl` block that cost a publisher of phone screen each time, and the rows replace three-line cards. Jurisdiction is also a filter facet, so a phone reader is no longer forced to scroll the Unknown bucket to reach anything. That bucket now says it is a gap in the records rather than presenting itself as a place. Elsewhere in the mode: catalogue tiles gain source type, record count and currency; the record's topics and publisher become routes back into the filtered catalogue; and Method gains a band-browse row. The record's deliberate compactness is untouched — the scoring workings stay off it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Sh8qHzezbEjWGp4MGJtjyk
…m ratchets The Playwright spec exercised only the Catalogue, so nothing at real phone width proved the new band stays on one line, that the badged trigger opens its sheet, or that a browse row lands on the filtered catalogue. It now walks Topics at 390px through to the catalogue, checks a query that matches nothing offers a way back, and runs the 320px forced-colors and axe pass over all three Sources surfaces rather than the Catalogue alone. Three design-system contract findings from the new markup, all fixed rather than ratcheted: - The quality meter's segments were a status hue on a childless box with no name in scope, which GATES.md section 3 reads as colour-only. Folding the meter into the row's labelled link puts the segments inside the name, beside the tally in words that carries the state when hue cannot. - Two new catalogue entry points declared a 32px minimum height. They take the repo's tap floor and the 40px compact desktop step, like every other control in this band. - The count numeral joined the row title on one type step instead of mixing text-sm with text-sm-minus in one component. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Sh8qHzezbEjWGp4MGJtjyk
Screenshots of the live corpus at 390px showed four faults the fixture tests could not: - Publisher names truncated to "Australian Government Depa…" because a stacked "SOURCES" caption held a column open beside them. The caption is gone — the unit is already in the row's accessible name and in the line below it — and the title wraps to two lines instead of truncating. - Almost every heading in this corpus is one band, so the quality meter drew 166 identical solid bars: a page of coloured rules reading as an alert while saying nothing the sentence beside them did not. The meter now renders only where there is a mix, and is thinner and width-capped when it does. - The supporting line — publishers, topics, where PsychSift uses them — was hidden below `sm`, which removed the whole reason a row is worth reading on the device most likely to be reading it. - "latest July 2026" did not say which date that was. It names its field now, as the catalogue tile already does. Stored topic values mix snake_case and kebab-case, and blanket title-casing produced "Conditions-Risk-And-Safety". `sourceTopicLabel` splits on both and leaves joining words lower-case unless they lead, and the source record's topic chips share it, so one topic reads the same on every surface. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Sh8qHzezbEjWGp4MGJtjyk
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
|
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: Team 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 |
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_4a8625ca-4981-4a29-ad0e-e29230b363b0) |
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. |
![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: 064762cfd6
ℹ️ 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".
|
@codex resolve actionable Codex review findings for this pull request and current head using the repository instructions. This is the pull request's single automatic repair pass: do not perform a fresh review, create new standalone findings, or request another review. Work only the existing unresolved Codex threads on the current head. The only repository destination is BigSimmo/Database, and the only branch destination is the pull request head branch claude/sources-mode-redesign-73uxmc at starting commit 2221f0a; never publish fixes to a detached or synthetic work branch and never create a stacked pull request. Use the authenticated GitHub connector to commit each approved fix to BigSimmo/Database:claude/sources-mode-redesign-73uxmc, then verify that the pull request head contains the published commit before reporting success. Always fix P0 and P1 findings. For P2 and lower findings, fix only clear, scoped, low-risk issues; otherwise disposition them with a concise reason. For a fixed thread, reply with as the first line and as the second line. For a no-code disposition, use followed by . These result markers authorize the workflow to close that exact thread only after it verifies a fixed commit is the pull request head; a local-only commit is not a fix. If publication or verification fails, do not use either result marker, do not claim success, and leave the thread open with the blocker. If human input or new authorization is required, do the same. Finish only after every actionable thread is fixed or dispositioned and closed, or explicitly left open for a human decision. Do not update the branch from main, address unrelated reviews, broaden scope, or create more than one scoped fix commit. Do not use external APIs, paid services, credentials, dependency changes, or broad refactors unless explicitly authorized. Add targeted tests where behavior changes and run the narrowest relevant validation. |
All three are real, and the first is the one this surface exists to prevent. **A query found a source the catalogue could show, and reported zero headings.** `matchesBrowseQuery` searched only the display fields — capped at three entries and carrying just the lead member's title — so a query matching a non-lead title or any alias missed the heading that source sits under. The catalogue finds "Form 10B: Written bodily restraint order"; Topics reported nothing, because Form 10A leads the group. Each summary now carries a pre-normalised `searchText` of every member's title, aliases, publisher, topics and type, newline-joined so a needle — which can never contain a newline after normalisation — cannot match across two fields that are merely adjacent in the blob. **"Clear search" silently wiped every filter.** It navigated to the bare pathname, discarding band, jurisdiction and usage, while the same empty state offered filter removal as its own separate control. `clearQuery` now drops only the query, mirroring `clearFilters`, which drops only the filters. **A recorded month moved by a timezone.** `Date.parse` reads a bare YYYY-MM-DD as UTC midnight, so formatting it west of UTC rendered `2026-01-01` as "Dec 2025" — and differently in the browser than on the server that rendered it. Reproduced at `TZ=America/Los_Angeles`, where the old expression prints "Dec 2025" and the new one prints "Jan 2026". `formatCatalogueMonth` parses a calendar date as local midnight and is now the one formatter both surfaces use. The ordering comparison keeps `Date.parse`: every candidate shifts equally, so the winner is unchanged. Six regression tests, four of them running green under a west-of-UTC timezone. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Sh8qHzezbEjWGp4MGJtjyk
…com/BigSimmo/Database into claude/sources-mode-redesign-73uxmc
|
Codex Review: Didn't find any major issues. Keep them coming! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
CI triageCI failed on this PR. Automated classification of the 2 failed job(s):
Compared with main CI run #15396 (success). That run's conclusion is an aggregate and did not exercise Classification is evidence routing, not permission to ignore a failure. Exact quarantined Playwright identities remain governed by the flake ledger. |
…erPreview The Production UI CI job on this branch was failing on tests/ui-smoke.spec.ts's "setup status endpoint returns non-secret checklist state" test, unrelated to this PR's Sources Topics/Publishers changes. main commit f552a2f (merged into this branch via PR #2590, "fix(build): ship the stamped service worker...") added a 7th "answerPreview" check to /api/setup-status but never updated this test's hardcoded expectation of 6 checks. Bring the test in line with the endpoint's actual (and correct) behaviour. Verified locally: this spec plus tests/ui-sources.spec.ts and tests/ui-mode-nav-density.spec.ts (the two specs this PR's own body names as touching /sources) all pass, 65/65. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UTZh9kpgzN6RteWQ1CTyBv
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_27ae6fd1-6139-414e-a990-d6f56ac2c1a9) |
Production UI shard 1 went red on this branch after it merged main, on a
test this PR does not touch:
strict mode violation: getByTestId('sources-publishers-main')
resolved to 2 elements
1) …aka getByTestId('mobile-composer-reserve-pad').getByTestId(…)
2) …aka getByTestId('sources-publishers-main').nth(1)
Not this branch's. The same test fails identically on origin/main at
7ca2ea1 with none of this branch's changes — there on
`sources-topics-main`, which is the same defect landing on whichever browse
route the run reaches first. The Sources redesign (#2593) added this spec,
and its own push run on main was cancelled, so Production UI never returned
a verdict on it.
The mechanism is the one `tests/playwright-settlement.ts` already documents
as #93: Next streams the page, React first renders the `<main>` outside
`mobile-composer-reserve-pad` and then relocates it in, and for that window
both copies are in the tree. Measured on a local production build at 320px:
at `domcontentloaded` the element sits outside the pad with `offsetParent`
null, and by 1.5s the same test id resolves inside the pad. `page.goto` can
return inside that window, and Playwright strict mode fails on the second
element the moment it sees it — it does not retry.
So the seven page-root assertions in this spec now go through
`expectSingleSettledOwner`, which is the repo's own remedy for this class
and is a *stronger* assertion than the `toBeVisible()` it replaces: it
polls until the test id resolves to exactly one element and that element is
visible. A duplicate `<main>` is an accessibility defect in its own right,
so requiring convergence rather than filtering to the visible copy is the
right reading here — `visibleByTestId` would have passed while two landmarks
were still in the tree. Nothing is skipped, disabled or quarantined.
Reproduced and fixed, both on a local production build via the repo runner:
- before, on origin/main 7ca2ea1 in a clean worktree: 1 failed, the
strict-mode violation quoted above.
- after, on this branch: `tests/ui-sources.spec.ts` 7 passed (51.0s). The
previously failing case takes 28.1s, which is the settle poll waiting out
the relocation — evidence the duplicate is real and does converge, not a
locator dodged around it.
Also green: lint, typecheck, and check:diff-integrity (153 -> 154 cases).
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01K8rMooUReCqvW8jLLx3k43
… map main landed the Sources mode redesign (#2593) with route descriptions that match the new behaviour; this branch still described the pre-redesign structure. Kept main's Sources block and this branch's /services/search entry, took main's ward-flow README count line (same 50, with its provenance), and regenerated docs/site-map.md with npm run sitemap:update rather than resolving it by hand. npm run sitemap:check passes. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DHSyfuC6mS98ystWFiitAR
2 participants
Summary
InformationPageShell: no count, no filter, no empty state, and — the defect a reader actually hits — no reaction to the composer. The query stayed on screen while Topics listed every topic in the catalogue, which is a browse result asserted for a search that never ran.SearchResultsLayout+SearchResultsHeaderBand+SearchResultsEmptyState, so the count, the query subject, the applied-filter shelf, the badged phone filter trigger and the one-line phone band are the same components the Catalogue uses.?q=narrows the list and the band reports the honest count for it.text-xlblock, and rows replace three-line cards, so a 390px screen shows roughly 5–6 publishers instead of 2. Jurisdiction is also a filter facet, so a reader is no longer forced to scroll the Unknown bucket to reach anything, and that bucket now says it is a gap in the records rather than presenting itself as a place.src/lib/sources/browse-facets.tsholds all the summary maths, offline and unit-tested. It reuses the catalogue's owncompareQuality,compareTextandnormalizeSearchValue(now exported) instead of reimplementing them, so a facet list cannot order or match itself differently from the catalogue it links into.No new routes. The Catalogue remains the only surface that lists results — every browse row links into
/sources/search?topic=…or?publisher=…&jurisdiction=…, and those hrefs are unchanged and still pinned by tests.Deliberately not done: the plan included adding the six-dimension rating breakdown to the source record.
tests/sources-mode.dom.test.tsxpins that page's compactness ("scoring workings and empty fields go") as a considered decision, so reversing it is the owner's call, not this PR's.Verification
npm run verify:pr-localVerification not run:
verify:pr-localwas not run on this branch. What was run locally, all green:npm run verify:cheap— twice, including after the final refinements. Decisive line:Design-system contract passed (1172 production files; raw colors 0; literal shadows 0; legacy tap classes 0; sub-floor interactive min-heights 5; edge conflicts 5; 1px shadow spreads 0).plusStatus-colour boundary: colour-only status indicators 0; status-coloured numerals 0; image inversions 0.tests/sources-browse-facets.test.ts(new, 21 cases),tests/sources-mode.dom.test.tsx(13 → 27 cases), plustests/source-catalogue-core.test.ts,tests/mode-secondary-navigation.test.tsandtests/search-shell-props.test.tsre-run unchanged:Test Files 5 passed (5) · Tests 145 passed (145).npx prettier --checkover every changed path:All matched files use Prettier code style!Three failures inside
verify:cheapare pre-existing and unrelated:tests/clinical-hazard-controls.test.ts,tests/privacy-readiness-contract.test.ts,tests/rag-plan-package-parity.test.ts. They fail identically on the base commita5c56cebefore any of this work — the dev container holds a 52-commit shallow clone and these tests resolve commit SHAs it does not have (reviewedCommit does not exist …,manifest reconciledBase is unavailable locally …). Neither reads anything undersrc/lib/sources,src/components/sources, or the changed test files. They should pass in CI, which has full history.npm run verify:uiwhen UI, routing, styling, browser behavior, reduced-motion, or forced-colors behavior changedUI verification not run: the full Chromium suite was left to CI, which runs it on this non-draft PR. Focused browser proof was run instead, in Chromium, against the isolated production server:
tests/ui-sources.spec.ts—7 passed (1.0m), including the new@critical390px journey (band visible,role="status"reports topics, phone filter trigger opens its sheet, a browse row lands on the filtered catalogue), the new zero-result query test, and the 320px reduced-motion + forced-colors + axe pass now widened from the Catalogue alone to all three Sources surfaces.tests/ui-mode-nav-density.spec.ts—57 passed (1.2m)(the only other spec that visits/sources).npm run plan:browserescalated to the full suite only because it cannot attribute two brand-new component files to any spec (an unattributable UI change fails closed to the full suite). Per the repo's testing-cost policy that verdict was not bought twice locally. This is a focused browser proof, notverify:ui.npm run verify:release— not a release or handoff confidence claim.npm run eval:retrieval:quality— not applicable; no retrieval, ranking, selection, chunking or scoring behaviour changed.npm run eval:rag -- --limit 15+npm run eval:quality -- --rag-only— not applicable; answer generation is untouched.npm run check:production-readiness— not run; no clinical workflow, privacy, environment, Supabase, or deployment behaviour changed. Source governance presentation changed, and it changed conservatively (see the Preflight below).npm run check:deployment-readiness— not applicable; no deployment startup, hosting, or rollout behaviour changed.Risk and rollout
loadSourceCatalogue()the Catalogue already calls, and hand the client a narrower projection (counts, band tallies, a lead title) than the full entries the Catalogue client already receives. The three helpers newly exported fromcatalogue-view.tsare export-only — no logic changed, andtests/source-catalogue-core.test.tsstill pins them.2943afc,357648e,e436cb4,064762c) or the squashed merge commit. There is no migration, no persisted state, no feature flag, and no schema or URL contract to unwind — the topic and publisher hrefs are byte-identical to before, so any saved or shared catalogue link keeps working either way.src/lib/rag/, the retrieval RPCs,scripts/eval-retrieval.ts, the golden fixtures, or the ranking contract tests is touched, andclassifyPullRequestFilesover this diff returnsragRanking: false. The catalogue's own display sort (filterAndSortSourceCatalogue) is unmodified.Clinical Governance Preflight
Clinical KB Database(sjrfecxgysukkwxsowpy)How each was established, in the same order:
loadSourceCatalogue()(import "server-only", backed byloadVisibleDocumentSourceReferences) and pass derived summaries to the client — a strictly smaller projection than the Catalogue client already receives.Notes
text-smwithtext-sm-minus.sm, and an unlabelled date. Stored topic values mixsnake_caseandkebab-case, sosourceTopicLabelsplits on both and leaves joining words lower-case — "Conditions-Risk-And-Safety" is now "Conditions Risk and Safety", and the source record shares the same rule.tests/sources-mode.dom.test.tsxgoes from 13 to 27. No test case was removed; the two derived-page cases were rewritten against the new markup with their href and aria-label contracts preserved.mainwhen opened (fix(rag): stop the out-of-corpus guard refusing in-corpus psychiatric queries #2546, a RAG guard fix touching different files).git merge-treeshowed no conflict.🤖 Generated with Claude Code
https://claude.ai/code/session_01Sh8qHzezbEjWGp4MGJtjyk
Note
Low Risk
UI-only Sources presentation and client-side URL filtering over existing
loadSourceCatalogue()data; no API, auth, or RAG changes, with catalogue href contracts preserved by tests.Overview
Topics and Publishers stop being static count-only lists and share the Catalogue’s results chrome: honest match counts, facet filters (jurisdiction, quality band, used-in, sort), empty states, and
?q=narrowing that uses the same normalized terms as the catalogue so the composer query is no longer ignored.A new
SourcesBrowseClientplusbrowse-facets.tsderive per-topic/publisher summaries (band mix, attention, currency, usage, searchable member text) and keep publisher rows scoped per jurisdiction so counts match existing/sources/searchlinks. Publishers stay WA-first with tighter mobile section headers; both tabs show the partial-catalogue note when hosted documents are unavailable.Elsewhere: catalogue tiles add source type, record usage totals, and review/publish month; Method adds band browse links; source detail links publisher and topics back into filtered search.
formatCatalogueMonthfixes timezone drift forYYYY-MM-DDdates. Tests cover facet math, DOM behaviour, and Playwright browse journeys.Reviewed by Cursor Bugbot for commit 87b1d5e. Configure here.