feat(services): adopt the filter contract with real facets (PR C) - #1882
Conversation
Services gains its first real filtering: five typed dimensions (catchments, age_groups, setting_flags, acuity_flags, housing_flags) become facets with union counts, substance_flags ships as a lens (an exact partition — every service carries exactly one of general/aod), and a scope segment escapes a filtered-to-zero state without discarding the query. The six query-replacing "quick filters" are evicted from the filter sheet to AnswerSuggestionChips, closing the navigate-kind defect docs/filter-contract.md exists to remove. The shared filter renderer (ResultFilterSheet/ResultFilterFacetChips) grows a dense tier — find-a-filter plus collapse-by-default — for services' 5 groups / 24 options, resolving the open density decision tracked as /issues #309. The OR-within-group/AND-across-group selection algorithm is extracted from document-tags.ts into facet-selection.ts so services and documents share one counting rule rather than each inventing their own. Live-registry verification that catalog_payload.tags is fully populated on the production Supabase project is still outstanding — rowToServiceRecord degrades conservatively to empty facets if it isn't, and this is called out explicitly for review. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01K1kfPCeWWSrbprFz56UeDD
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01K1kfPCeWWSrbprFz56UeDD # Conflicts: # docs/outstanding-issues.md
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:57 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 (17)
Comment |
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01K1kfPCeWWSrbprFz56UeDD
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit:f93c2639e3
ℹ️ 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.
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/artifact-build-ygfit8 at starting commit 6098a53; 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/artifact-build-ygfit8, 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. |
BigSimmo
commented
Aug 12, 2026
Fixed the current services findings on the exact head: widening to All items now preserves q/query while clearing only narrowing scope, facet projections are computed from the active substance lens population, and the focused UI journey verifies scope widening, query preservation, facet reapplication, and independent clearing. The actionable threads are resolved. The failed Secret Scan was an Install Gitleaks infrastructure failure, so its failed job was rerun; CI remains in progress on the new head. |
Codex Review: Didn't find any major issues. What shall we delve into next? 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 #10259 (success). Classification is evidence routing, not permission to ignore a failure. Exact quarantined Playwright identities remain governed by the flake ledger. |
BigSimmo
commented
Aug 12, 2026
@claude Repair only the exact-head CI regressions currently proven on PR #1882 head |
Claude encountered an error after 2s —— View job I'll analyze this and get back to you. |
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. |
BigSimmo
commented
Aug 12, 2026
@claude The exact-head CI repair is already on head |
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. |
BigSimmo
commented
Aug 12, 2026
Formatted the two files reported by the exact-head static check ( |
BigSimmo
commented
Aug 13, 2026
@claude Please main-sync this PR from exact head This is an older competing Services filter-contract implementation; preserve its current tested behavior and all already-fixed review findings (query preservation, active care-lens projections), while retaining every newer Run the focused service facets/catalog/registry tests, search-results DOM tests, the relevant Services UI selector, design-system/adoption checks if applicable, typecheck and format. Report the new head and exact outcomes. |
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. |
BigSimmo
commented
Aug 13, 2026
@copilot Merge This is an older competing Services filter implementation. Preserve its already-fixed query-preservation and active care-lens count projections, but retain every newer current-main shared filter, registry, mapper/ranker, navigation, accessibility, performance and payload-safety change. For Run focused service facets/catalog/registry tests, search-results DOM tests, relevant Services UI selector, design-system/adoption and ledger/issues integrity/write-discipline checks, typecheck and format. Report the pushed 40-character SHA and exact outcomes. |
PR #1878 has now landed the Services filter-contract rollout on main with the same capabilities plus subsequent CI and regression fixes. Resolve this competing branch by retaining the current main tree rather than overwriting it with the older implementation.
Uh oh!
There was an error while loading. Please reload this page.
BigSimmo
commented
Aug 13, 2026
Final review status
Merge notice: the PR was already merged externally while this review was in progress. No merge action was performed by this review. |
…onent Deletes the ~500-line bespoke DocumentFilterPanel/DocumentFilterTrigger and rebuilds documents' filter sheet on ResultFilterSheet/ResultFilterTrigger (src/components/clinical-dashboard/result-filter-control.tsx), reusing the dense tier (find-a-filter, collapse-by-default, disclosure headers) that PR C already generalized from documents' own >3-groups rule. This is the last mode in the docs/filter-contract.md rollout. Three small additive extensions to the shared component, all optional and inert for the six modes that adopted earlier: - ResultFilterSheet gains meterContent (the "N of M documents shown" bar, rendered first in the body). - ResultFilterSheet gains footerOverride (replaces the default footer entirely, for documents' "Show N documents" + "Browse all sources"). - resultFilterGroup() gains an optional note (the "one only" annotation on the source-type lens, now that it shares a sheet with facet groups for the first time). toggleTagFacet narrows from SmartDocumentTagFacet to its key. Two testids move from literal strings to the shared component's own derivation (document-filter-clear -> document-filter-panel-clear, document-filter-find -> document-filter-panel-find); document-filter-done and document-filter-browse-library stay unchanged as custom footerOverride JSX. The dead-end facet's sr-only reason text changed from documents' bespoke copy to the shared component's generic message; the guard mechanics (disabled, focusable, click-blocked) are unchanged. decoration-on-text contract checks move to result-filter-control.tsx along with the markup they guard. docs/filter-contract.md's Rollout section is updated to close out; a new /issues follow-up (#312) notes services has the same lens-beside-facets shape without the annotation, and /issues #170 records documents as done. Stacked on claude/artifact-build-ygfit8 (PR C, #1882, not yet merged) — this branch's diff includes PR C's commits until #1882 merges.
Flagged in the previous commit's PR body but not actually captured as a trackable item: the services filter-contract adoption (#1878, then #1882) was independently implemented twice by concurrent sessions, and whatever conflict resolution happened between them when both merged was never reviewed line-by-line. A spot check found registry-records.ts has no ServiceRecord.facets carrier and no corresponding test file on main, so #1878's approach is what survived rather than #1882's — likely equivalent, not verified. Queues a P2 add request via the outstanding-issues inbox so this doesn't only live as prose inside an archived row's outcome text.
Summary
navigate-kind defectdocs/filter-contract.mdexists to remove — each calledrouter.pushand discarded the search with no warning).CatalogServiceTagsdimensions (catchments,age_groups,setting_flags,acuity_flags,housing_flags) become facets with union counts (OR within group, AND across groups);substance_flagsships as a lens — it's an exact partition (every one of the 219 catalogue services carries exactly one ofgeneral/aod), so a facet would falsely claim a service could be both.ServiceRecord.facets) is populated at both entry points intoServiceRecord(the static/demo mapper and the live registry row mapper), derived fromcatalog_payload.tags/CatalogService.tagsrather than the flattenedServiceRecord.tagsarray —flattenTagsmerges five dimensions plus sections/aliases/id into one deduped array, so a token like"general"(which means three different things acrosssetting_flags/substance_flags/housing_flags) collapses to one indistinguishable entry there. A facet filter built on the flattened array would be wrong by construction.These results N | All items N) escapes a filtered-to-zero state without discarding the query, perdocs/filter-contract.md§4.ResultFilterSheet/ResultFilterFacetChips) grows a dense tier — find-a-filter plus collapse-by-default — for services' 5 groups / 24 options, resolving/issues #309(now closed). Below the density threshold nothing changes (formulation's existing 9-option group still renders as a plain chip row).document-tags.tsinto a newsrc/lib/facet-selection.tsso services and documents share one counting rule rather than each inventing their own —document-tags.tsnow imports it back with no behavioral change (its own 23-test suite still passes unchanged).docs/filter-contract.md§5 updated to describe the two states actually implemented (documents' owndenseboolean never shipped a distinct three-tier scheme either).Verification
npm run verify:pr-local— full run, all steps green:check:runtime, check:installed-lock-parity, format:changed, sitemap:check, docs:check-index, docs:check-inventory, docs:check-scripts, docs:check-links, check:branch-review-ledger, check:outstanding-issues, lint, typecheck, test, build, check:rag:fixtures.testreportedTest Files 565 passed (565)/Tests 6165 passed | 4 skipped (6169), and the full suite was re-run after mergingorigin/mainin:Test Files 566 passed (566)/Tests 6172 passed | 4 skipped (6176).npm run check:bundle-budgetafter a cleanrm -rf .next && npm run build— production 1307.9 KiB gzip (baseline 1279.1 KiB, within the 10% tolerance), mockups 294.5 KiB gzip (baseline 267.5 KiB, within the 25% tolerance).npm run verify:uicannot run in this environment (/issues #255— Playwright wants chromium-1234, only chromium-1194 installed). Proved UI directly: a Playwright script from the repo root withexecutablePath: "/opt/pw-browsers/chromium-1194/chrome-linux/chrome"against a real dev server — facet sheet opens, a facet toggle narrows results while leavingquntouched, Clear filters removes facets/substance without touchingq, the scope segment renders when catalogue (219) > results and "All items" clears back to the full catalogue, the search-shortcut chips still run a query-replacing search, 0px horizontal overflow at 1440/800/390/320. 8/8 checks passed.tests/ui-tools.spec.tsthat exercised the old query-replacing "Quick filters" radio were rewritten for the new facet/chip behavior and run against a real production build vianode scripts/run-playwright.mjswith the same executable override — both pass.tests/service-facets.test.ts(facet index derivation, OR/AND selection, projected-count semantics, a live guard thatsubstance_flagsis still an exact partition across the real 219-service catalogue), plus extended coverage intests/services-catalog.test.tsand a newtests/registry-records.test.tsfor thefacetsderivation at bothServiceRecordentry points (including conservative degrade to all-empty dimensions on missing/malformedcatalog_payload).Risk and rollout
classifyPullRequestFilesreturnsclinicalRisk: truefor this diff, but by inspection that's driven bysrc/lib/service-ranker.ts's filename containing "rank" (the pattern matches on path, not content) — the actual change there is additive-only (a new optionalfacetsfield onServiceRecord), not a change to ranking/scoring logic.registry-seed.ts/registry-fixtures.tsconfirm the fixture/demo seeding path populatescatalog_payloadwith the full typed tags, but live verification that the production registry'sservicerows actually carrycatalog_payload.tagswith all six dimensions populated has not been done.rowToServiceRecorddegrades conservatively (all-empty facet dimensions, never a throw) if it isn't — so the worst case in production is an empty filter sheet, not a crash — but this should be confirmed against the liveClinical KB Databaseproject before or shortly after merge.Clinical Governance Preflight
Clinical KB Database(sjrfecxgysukkwxsowpy) — no Supabase config changed.Notes
/issues #309records the reasoning.docs/outstanding-issues.mdconflicted withorigin/mainduring a routine merge (two new rows,#310/#311, landed onmainwhile this branch's#309closure was in flight) — resolved by hand perAGENTS.md's no-merge-driver rule for that file: kept both sides' content (my#309archival, plus#310/#311frommain), verified withnpm run check:outstanding-issues.Generated by Claude Code