feat(filters): make formulation a real facet surface - #1858
Conversation
Formulation rendered twelve domains as radios. Both halves of that were wrong. **Radios were the wrong claim.** A mechanism carries 3.92 domains on average, so one-of-N told the reader they could not hold Affect and Risk at once. `domain` is now a `resultFilterFacetGroup` — many-of-N, OR within the group — and `searchFormulationMechanisms` takes a `domains` set alongside the older single `domain` the builder page still uses. **Three of the twelve could never match anything.** `formulationDomains` is the taxonomy; Biological, Social and Cultural are carried by 0 of the 12 mechanisms. Under union counting an empty option reports the unchanged total, so it looked identical to a full one. New `formulationDomainsInUse` derives the list from the mechanisms that carry it — 9, in taxonomy order — which is the contract's "derive the option list, never declare it". Counts use the same predicate as the filter, asked with the candidate added: "how many would I have if I ticked this as well". Verified monotonic — Affect (9) OR Risk (4) is 10, not 13. A zero that IS a consequence of the current query renders as a focusable dead end with its reason, never as a silent no-op, and never on an already-selected option, which would make a live constraint unremovable. **The Pattern group was not a filter.** It called `router.push` and replaced the query, discarding the search and its results with no warning and no undo — the thing docs/filter-contract.md section 1 says a control labelled "Filter" must not do. Those presets now render below the band as `AnswerSuggestionChips`, framed as a new search. All five, rather than the four the old `.slice(0, 4)` exposed; the fifth had been unreachable at every breakpoint. `ResultFilterFacetChips` is exported so the desktop rail renders the SAME control as the sheet. Lens modes got that from `SegmentedControl`; facets had no equivalent, and the alternative was a second hand-rolled chip row — which is how the breakpoints came to disagree in the first place. The desktop `<select>` of 13 options is gone. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011btGFwWKYFWDs5McQkqz9J
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:51 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 (7)
Comment |
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011btGFwWKYFWDs5McQkqz9J
Uh oh!
There was an error while loading. Please reload this page.
CI triageCI failed on this PR. Automated classification of the 2 failed job(s):
Compared with main CI run #9928 (cancelled). Classification is evidence routing, not permission to ignore a failure. Exact quarantined Playwright identities remain governed by the flake ledger. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit:9262d89701
ℹ️ 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.
docs/outstanding-issues.md #170 conflicted because both branches rewrote that row's Detail cell — the file deliberately carries no merge driver (union was tried and removed, #133) so overlapping edits fail loudly rather than concatenate. Neither side was a superset: main uniquely explained the closed one-of-N/many-of-N defect and the "unlike #1847" stop nuance, this branch uniquely carried the formulation facet work. Resolved by taking main's file and re-applying a combined row through npm run issues:update rather than hand-editing or taking one side wholesale. result-filter-control.tsx auto-merged cleanly and both changes survive: PR #1857's two aria-label fixes (the "All8" concatenation) and this branch's exported ResultFilterFacetChips plus the narrowed builder return type. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011btGFwWKYFWDs5McQkqz9J
…e lag guard Two review findings on PR #1858, both real. **P1 (codex) — the Chromium journey still drove the old sheet.** tests/ui-formulation.spec.ts:122 opened the phone sheet and asserted the `Pattern` radiogroup plus `Domain` radio semantics with an "All domains" option. All three are gone: Pattern was the query-replacing group evicted to suggestions, and Domain is now a facet. Left as-is this fails Production UI exactly the way ui-specifiers.spec.ts did on PR #1857. Rewritten to exercise the new behaviour rather than merely pass: the suggestions row is present, Pattern resolves to zero, Domain is a role="group" with no radios and nine derived options, and two toggles accumulate to "2 filters active". **LOW (sentry) — counts disagreed with results during the deferred lag.** For the frame where the live query has text but `useDeferredValue` has not caught up, `results` deliberately reports nothing rather than scoring the whole catalogue, but the counts were still computed against `searchQuery` — which is the empty deferred value, i.e. the full catalogue. The sheet would show "0 showing" beside nine non-zero counts. That is precisely the filter-vs-own- predicate disagreement this contract exists to remove, so it is worth fixing even at LOW severity. Counts now report 0 while ranking is pending, and the dead-end marker is suppressed there: a zero during the lag means "not scored yet", not "nothing matches", and would otherwise flash all nine options inert. Verified in a browser at 390px: every assertion in the rewritten journey passes against the running app. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011btGFwWKYFWDs5McQkqz9J
The Codex P2 on PR #1858 is correct — nine derived domains should be in the 6-20 dense-list band, and ResultFilterFacetChips has only a chip layout. Making the shared renderer density-aware is a design decision affecting every facet consumer and overlaps the tier PR F ports up from documents, so it is captured rather than folded into a mode adoption. Services hits the same rule via the >3-groups branch, so it should be settled before or with PR C. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011btGFwWKYFWDs5McQkqz9J
Summary
Step three of the filter contract rollout (
docs/filter-contract.md), and the first facet adoption. Formulation rendered twelve domains as radios; both halves of that were wrong.Radios were the wrong claim. A mechanism carries 3.92 domains on average (measured over all 12), so one-of-N told the reader they could not hold Affect and Risk at once.
domainis now aresultFilterFacetGroup— many-of-N, OR within the group — andsearchFormulationMechanismsaccepts adomainsset alongside the older singledomainthat the builder page's own select still uses.Three of the twelve could never match anything.
formulationDomainsis the taxonomy; Biological, Social and Cultural are carried by 0 of the 12 mechanisms. That is the exact failure mode the contract's companion rule exists for: under union counting an empty option reports the unchanged total, so it looks identical to a full one. NewformulationDomainsInUsederives the list from the mechanisms that carry it — 9, in taxonomy order — which is §3's "derive the option list from the data, never declare it".Counts use the same predicate as the filter, asked with the candidate added: "how many would I have if I ticked this as well". Verified monotonic, and verified non-additive — Affect (9) OR Risk (4) is 10, not 13, because the overlap is real. A zero that is a consequence of the current query renders as a focusable dead end carrying its reason, never a silent no-op — and never on an already-selected option, which would make a live constraint unremovable.
The
Patterngroup was not a filter. It calledrouter.pushand replaced the query, discarding the search and its results with no warning and no undo — precisely what §1 says a control labelled "Filter" must not do. Those presets now render below the band asAnswerSuggestionChips, framed as a new search. All five, rather than the four the old.slice(0, 4)exposed; the fifth had been unreachable at every breakpoint.ResultFilterFacetChipsis exported so the desktop rail renders the same control as the sheet. Lens modes get that fromSegmentedControl; facets had no equivalent, and the alternative was a second hand-rolled chip row per mode — which is how the breakpoints came to disagree in the first place. The desktop<select>of 13 options is gone.resultFilterFacetGroupnow returns the narrowResultFilterFacetGrouprather than the union, so one group object can feed both the sheet (union) and the rail (facet-only).No scope segment: 12 mechanisms total, so the two segments would show near-identical numbers.
docs/filter-contract.md§4 gates it on services.Ordering note
This shares
result-filter-control.tsxwith PR #1857 (the four lens modes), which is open and fixes the sheet's counted-option accessible name ("Attachment0"→"Attachment (1)"). I deliberately did not duplicate that fix here — it would conflict on identical lines. Land #1857 first; the fix then applies to these facet chips at both breakpoints for free, since they are now one shared renderer. If this lands first instead, #1857 rebases cleanly onto it.Verification
npm run verify:pr-local—failed: (none),not reached: (none). Every step green includingbuild, run with the dev server stopped and.nextdeleted so the/issues#210dev-types corruption could not reproduce:Test Files 558 passed (558),Tests 6101 passed | 4 skipped (6105), zero failures.tests/formulation.test.ts— 11 passed, +3 pinning the contract rules rather than the current numbers' shape:both < affect + risk; an empty set imposes no constraint rather than matching nothingnpm run check:bundle-budgetafterrm -rf .next && npm run build,.next/BUILD_IDmtime checked against the build:npm run verify:uiwhen UI, routing, styling, browser behavior, reduced-motion, or forced-colors behavior changed — UI verification not run as the full Chromium gate, for the known Playwright/Chromium revision drift (/issues#255, PW 1.62.1 wants chromium-1234, only 1194 installed); forcing a mismatched binary is disallowed. Proved instead against the running dev server, 0 px horizontal overflow at 1440 / 390 / 320 px:<select>count on the page is 0; suggestion row present with 5 chips?q=thoughts:Behaviourstarts a dead end (hint 0,aria-disabled=true), and after tickingAffectbecomeshint 1and selectable — the union widening, visible in the controlaria-pressedaccumulates acrossAffect+Behaviourrather than replacingradiogroupcount 0 (no lens groups left),Patternabsent, dead ends carry "No matches with your current filters.", footer1 showingNot run, with reasons:
eval:retrieval:quality/eval:rag— no retrieval, ranking or answer-path file is touched;searchFormulationMechanismsis static-content search over a 12-item bundle, not a RAG surface, andcheck:rag:fixturespassed inside the gate above (36 golden cases, 23 suites).verify:release— provider-backed, not authorised and not a release.Risk and rollout
ResultFilterFacetChipsis the existing renderer exported under a name, with no change to what it renders, andresultFilterFacetGroup's return type narrowed to a subtype, so every existing call site still typechecks and behaves identically.searchFormulationMechanismskeeps itsdomainoption untouched for the builder page.9262d89. Nothing else consumesformulationDomainsInUse, thedomainssearch option, or the exported chips.Notes
formulationDomainsis deliberately left exported and unchanged — it is the taxonomy, andMechanismDomainChipsand the builder still render against it. The derived list is a filter concern, not a redefinition of the domain set.Next: services (facet index + selection state + URL + the scope segment — the biggest of the remaining, and where the scope segment finally earns its place), then factsheets, therapy-compass, documents last.
Generated by Claude Code