feat(filters): converge factsheets category filter onto SegmentedControl (PR D) - #1872
Conversation
…rol (PR D) factsheets' category dimension is a real lens (filterFactsheets ANDs query and category, preserving q), not a query-replacing preset -- an earlier draft of docs/filter-contract.md incorrectly grouped it with services' quick filters. Corrects that, and fixes the actual defect: the desktop rail used raw <Link> chips while the phone sheet correctly used resultFilterGroup, so the two breakpoints agreed on values but disagreed on component. Converges both onto one shared, counted option array via SegmentedControl, matching the pattern already used for differentials/medication/applications/ specifiers. Counts are derived from filterFactsheets(query), not a declared total, so they stay honest under an active search. No scope segment (8 total factsheets). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01K5c1ZWQUTwmhgXXdVd2wuK
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:13 minutes 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 for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling 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 (5)
Comment |
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01K5c1ZWQUTwmhgXXdVd2wuK
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit:53a7f17b5e
ℹ️ 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".
Uh oh!
There was an error while loading. Please reload this page.
BigSimmo
commented
Aug 12, 2026
@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/filter-factsheets-composer at starting commit c67e3e4; 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/filter-factsheets-composer, 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. |
Codex Review: Didn't find any major issues. You're on a roll. 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". |
BigSimmo
commented
Aug 12, 2026
@codex Address all currently unresolved inline review comments on this PR against the current GitHub head. Make minimal repository-grounded fixes, add/update focused regression tests, run proportionate verification, and push the fixes to this PR branch. Do not merge, close, rebase, force-push, deploy, or alter auto-merge. Please report the exact commit SHA and test results. |
Summary
Testing
|
Merge the latest main branch into PR #1872 while preserving current shared files and the PR's design-system metadata. Derive category choices from query matches, retain only an already-selected zero-count category as an explained inert state, and add focused DOM regression coverage.
BigSimmo
commented
Aug 12, 2026
@claude Fix the currently unresolved review thread on this PR at the current GitHub head. Category filter options must not allow selecting an unrelated zero-count category under the current query. Derive available options from the query-matched result set or disable zero-count options while keeping any already-selected zero state reachable. Add a regression proving an unrelated zero-count category cannot be activated. Make the smallest repository-consistent fix, run focused tests/format/typecheck as proportionate, push only to this PR branch, and report exact SHA/tests. Do not merge, close, rebase, force-push, deploy, or alter auto-merge. |
Hi @BigSimmo — thanks for the @claude mention. Open this in Claude Code on the web to continue here: https://claude.ai/code Context: Automated backlink from the Claude backlink workflow. Claude Code can pick up the mention and push a scoped fix to the branch. |
Claude encountered an error after 2s —— View job I'll analyze this and get back to you. |
Restore repository-standard trailing newlines in the focused factsheets source and DOM regression test.
BigSimmo
left a comment
There was a problem hiding this comment.
Final Codex review-and-fix summary
- Final head:
eaee186507c458c960275e71de54beecaef26801; base:b850a58166b68fa9cdd9ff6248c5e93e34c87f73. The branch is 5 commits ahead / 0 behind and GitHub reports it mergeable. The final head is a clean merge of currentmainonto the reviewed repair and has the same tree as the preceding reviewed head, so no additional source delta was introduced by the sync. Auto-merge remains armed and was not altered. - Fixed/dispositioned: the reported P2 zero-count-category defect is fixed. Factsheet category choices are derived from the current query matches; unrelated zero-count categories are omitted; an already-selected zero-count category remains visible only as a disabled, explained dead end; navigation is guarded against disabled/absent options. Focused DOM tests cover desktop and phone behaviour, query preservation, dead-end inertness, and zero-result escape behaviour.
- Review threads: the single actionable Codex thread was replied to with the current fixed head and resolved. No unresolved actionable review thread remains.
- Independent adversarial pass: I performed a distinct manual pass because a separate review subagent/configured review bot was not available in this environment. I checked the changed component against
SegmentedControlandResultFilterSheetcontracts, query/category semantics, disabled-state behaviour, URL preservation, empty states, accessibility naming, and the focused regression tests. No additional high-confidence PR-introduced P0/P1/P2 defect was found. Highest residual risk is visual/browser interaction coverage while Production UI is still pending. - Verification: the PR already records
npm run test(6135 passed, 4 skipped), lint, typecheck,verify:pr-local, bundle budget, outstanding-issues integrity, and manual browser checks at 1440/800/390/320. The final merge-only head preserves the exact reviewed tree. I did not run provider-backed/live/production tests or Lighthouse. - Exact-head required CI:
CIrun #10138 is currently in progress oneaee1865…;Production UI criticalis queued, Build/Safety/Static checks are in progress, so the required aggregate is not yet green and remains pending.Secret Scanon the exact final head is successful.SASTis still in progress. These pending states remain blockers until their required aggregate settles successfully. - Earlier Secret Scan failure on the superseded head was infrastructure-related at the Gitleaks install step and skipped the actual scan. I reran that failed job; the new exact final head has a successful Secret Scan.
- Advisory/external: Supabase preview was skipped because there are no Supabase changes. CodeRabbit hit its review usage/spending limit, so it did not provide a fresh independent review. No code change was made for either advisory condition.
- Ledger: a review ledger commit already exists in this PR history. I did not create a ledger-only final tip, consistent with repository policy. The required base sync was already performed as the final merge commit; its tree introduced no source changes.
- No permission blocker prevented review-thread resolution or CI retry. Remaining risk is limited to pending exact-head CI/UI checks.
The PR was not merged. Final merge is left to the user.
Keep the shared outstanding-issues ledger byte-identical to latest main during conflict resolution, avoiding stale rebase guidance and preserving the factsheets rollout record in the dedicated filter-contract document.
CI triageCI failed on this PR. Automated classification of the 3 failed job(s):
Compared with main CI run #10239 (cancelled). Classification is evidence routing, not permission to ignore a failure. Exact quarantined Playwright identities remain governed by the flake ledger. |
Uh oh!
There was an error while loading. Please reload this page.
Summary
docs/filter-contract.md, tracked in#170): factsheets' category dimension converges onto the sharedSegmentedControl, sharing one counted option array between the desktop rail and the phone sheet.filterFactsheets(query, category)ANDs the two, so selecting a category narrows within the current search and preservesq— a reallens(docs/filter-contract.md section 1), not a query-replacing preset. There was nothing here that behaved like a preset to evict. The actual defect matches the original audit finding much more closely: the desktop rail rendered raw<Link>chips while the phone sheet correctly usedresultFilterGroup/role="radiogroup"— the two breakpoints agreed on values but disagreed on component, exactly the "same dimension rendered three different ways" drift PR A fixed for the four lens modes. Fixed the same way: one shared, counted option array,SegmentedControlon desktop,ResultFilterSheeton phone.docs/filter-contract.mdsection 1's own example ("Services' quick filters and factsheets' categories do not filter...") is corrected in the same commit — it was asserting something the code didn't actually do.What changed
src/components/factsheets/factsheets-search-page.tsx— replaced the raw<Link>chip row (filterChips) with auseMemod, countedcategoryOptions: ReadonlyArray<ResultFilterOption<string>>, derived fromfilterFactsheets(query)(query-matched, category-unfiltered) so every option's count answers "how many for this category, at the current query" — including options that aren't currently active, per section 3's counting rule. Desktop now rendersSegmentedControl; the phoneResultFilterSheet'sresultFilterGroupcall andonClearAllnow reuse the samecategoryOptionsarray and a sharedapplyCategoryhandler instead of duplicating the navigation logic inline.docs/filter-contract.md— corrects section 1's factsheets example, documents the actual fix in the Rollout section, and notes that services (PR C,#1866) is open on a pre-PR-B base and will need a rebase —origin/mainpicked up PR B (#1858, formulation) and its#309density-gap row while PR C was in flight.docs/outstanding-issues.md#170— records factsheets' completion and the current true state of the rollout (PR A + PR B merged, PR C open needing rebase, PR D this one).docs/design-system/{COMPONENTS.md,adoption-manifest.json}— regenerated for the newSegmentedControlconsumer (factsheets desktop rail).tests/factsheets-search-page.dom.test.tsx(new, 5 tests) — shared counted options on both breakpoints, selecting a category preservesqin the pushed href, "Clear filters" preservesq, counts read0under a query with zero matches rather than a stale/declared total. Updatedtests/ui-smoke.spec.ts's existing factsheets E2E test: the "All" radio's accessible name is now"All (1)"(counted), not the previous plain"All".Verification
npm run test— 6135 passed, 4 skipped, 0 failednpm run lint/npm run typecheck— cleannpm run verify:pr-local— all 15 steps green, run against the final diff after formattingnpm run check:bundle-budgeton a freshly rebuilt.next— production 1300.2 KiB gzip vs 1279.1 KiB baseline (within 10% tolerance), mockups unchanged relative to baselinenpm run verify:ui— not run: same Playwright chromium/chrome-headless-shell revision mismatch (1234 wanted, 1194 preinstalled in this environment) as PR feat(filters): adopt the lens contract across the four one-of-N modes #1857/feat(filters): make formulation a real facet surface #1858/feat(filters): services adopts the filter contract (PR C) #1866. Delegated to CI's Production UI job.All 8,Medications 3,Conditions 3,Therapies 1,Tests & procedures 1) on both the desktop rail and phone sheet; selecting a category navigates and preservesq(?q=sertraline&category=Medications); counts correctly narrow under an active query (?q=sertraline→All (1),Medications (1), everything else(0)), confirming they're live, not a declared total.npm run check:outstanding-issues— 307 rows (112 open, 195 archived), no ids deleted from baseRisk and rollout
result-filter-control.tsx(no changes to the shared component), so no cross-mode blast radius.Notes
classifyPullRequestFilesreturnsclinicalRisk: false,operationalRisk: false,ragRanking: falsefor this diff, so the Clinical Governance Preflight andRAG impact:sections are not required.#1866) is open on a base that predates PR B (#1858) merging — its own PR body already flagged the expectedresult-filter-control.tsxconflict; this PR's#170update records the same thing from the other side.Generated by Claude Code