fix(filter): finish the counted-lens migration and stop the API misreporting its catalogue - #2729
fix(filter): finish the counted-lens migration and stop the API misreporting its catalogue#2729BigSimmo wants to merge 1 commit into
Conversation
…porting its catalogue Four follow-ups from the filter panel work, each the smallest change that actually closes the item. Derive the segmented renderer instead of opting into it. A one-of-N filter whose options all carry counts is a segmented bar because of what it is, not because a call site asked; `renderAs` is gone and the option list decides. That was always the stated end state — the flag existed only so modes could move one at a time — and leaving it in kept twelve surfaces on the wrapping counted chips section 5 rejects. Bounded at five options, where the chip tier ends: past that a segmented bar wraps into rows and stops reading as one control. Split the count on SegmentedControl. It used one field for both the visible figure and the accessible name, so a count carrying a unit had to choose between an unreadable segment and a name that dropped the unit — which is why documents' Source locality stayed on chips. It now takes the same `hint`/`hintLabel` pair an option already has: "1 loaded source" announced, "1" displayed. Report the catalogue size from /api/differentials. `total` measured the records being returned, which under a query are the ranked matches, so a caller asking how many differentials exist got its own result count back — while the route's other three branches all reported the catalogue. That is what left the differentials filter unable to state the figure. Pinned by a test that fails on the old code with "expected 10 to be 201". Drop the redundant "together" from the differentials filter subtitle. Two defects found and fixed while doing the above, both of which would otherwise have shipped: A state-dependent renderer made the control change shape mid-interaction. Documents' Source locality marks an option dead the moment its count reaches zero, and the first derivation vetoed the segmented bar on any dead end — so the filter morphed from a bar into a chip row while the reader was using it. The renderer now depends on the option list alone. Fixing that by redefining `disabled` was wrong. `SegmentedControl` is shared well beyond filters, and skipping genuinely unavailable options with the arrow keys is a deliberate contract with a test pinning it. Dead ends are a different state and now have their own field: `disabled` leaves the arrow path, `deadEnd` stays on it with `aria-disabled` and a stated reason, as section 3 requires. No consumer passed `disabled` to a segment, so existing output is unchanged. Verified: typecheck, lint, whole-tree prettier, check:design-system-contract, and the full offline unit suite — 18,038 passing, no failures. The three failures that were red on main earlier in this work are gone, fixed upstream. Full Chromium suite still running at commit time; result reported separately. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GMaWYk6iF8bsMwEx38egVX
|
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 |
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
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. |
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_311d51b0-69d8-42b5-829b-8e224efb8fef) |
Summary
Four follow-ups left open by #2666, each the smallest change that actually closes the item.
renderAsis gone and the option list decides. That was always the stated end state — the flag existed only so modes could migrate one at a time — and leaving it in kept twelve surfaces on the wrapping counted chips §5 rejects. Bounded at five options, where the chip tier ends: past that a segmented bar wraps into rows and stops reading as one control. This also restores §1 ("No call site picks chips, rows or a segmented control") to an unqualified rule.SegmentedControl. It used one field for both the visible figure and the accessible name, so a count carrying a unit had to choose between an unreadable segment and a name that dropped the unit — which is why documents' Source locality stayed on chips. It now takes the samehint/hintLabelpair a filter option already has:"1 loaded source"announced,1displayed./api/differentials.totalmeasured the records being returned, which under a query are the ranked matches — so a caller asking how many differentials exist got its own result count back, while the route's other three branches all reported the catalogue. That is what left the differentials filter unable to state the figure.Two defects found while doing the above
Both would have shipped, and both are worth reading:
disabledwas wrong.SegmentedControlis shared well beyond filters, and skipping genuinely unavailable options with the arrow keys is a deliberate contract with a test pinning it (ui-v2-components.dom.test.tsx, "disabled skipping"). Dead ends are a different state and now have their own field:disabledmeans "not on offer" and leaves the arrow path;deadEndmeans "your own narrowing emptied this" and stays on it witharia-disabledand a stated reason, asdocs/filter-contract.md§3 requires. No consumer passesdisabledto a segment today, so existing output is unchanged.Verification
npm run verify:ui— 737 passed (21.2m), exit 0. Full Chromium suite, not a narrowed run:plan:browserfails closed here because the change touches a shared component no spec attributes, and the derivation restyles up to thirteen filter surfaces, so the full sweep was the right gate.mainearlier in this work —clinical-hazard-controls,privacy-readiness-contract,rag-plan-package-parity— are gone, fixed upstream by clinical(governance): harden query guard, alert tiers, status derivation, and lexicon rationales #2698.)npm run lint,npm run typecheck, whole-treeprettier --check .npm run check:design-system-contract—Design-system contract passed (1268 production files; raw colors 0; literal shadows 0; legacy tap classes 0; ... 1px shadow spreads 0), adoption + design-sync greennpm run check:type-scale,npm run check:icon-scale— both✓New coverage: a dead end stays on the arrow path with selection withheld and a stated reason;
hintis announced with its unit whilehintLabelis displayed; andtotaldoes not move when a query is applied — the last fails on the old code withexpected 10 to be 201.Risk and rollout
SegmentedControlchange is additive —deadEndis a new field no existing consumer sets./api/differentialschanges which number it reports in an existing field; it makes no new calls and touches no Supabase or OpenAI surface.Clinical Governance Preflight
Clinical KB Database(sjrfecxgysukkwxsowpy)This PR edits
src/app/api/differentials/route.ts, so it classifies as clinical-risk and carries the full preflight. Each box above was checked on its own merits — the basis for each, in the same order:supabase/changes on this PR.publicDifferentialPayload, the public/demo path. No key handling, auth resolution, rate limiting or access check is altered, and the owner-scoped branches are untouched.demoMode: trueexactly as before.governancefield is untouched.totalhas no consumer (verified: nothing insrc/reads it), so no clinical decision-support behaviour changes.Two points worth stating beyond the checklist:
totalmakes the API more truthful, not less — it previously reported a figure that was wrong whenever a query was present. Filter counts themselves are untouched and still come from the same predicate as the filter, per §3.Notes
The
deadEnd/disabledsplit is the part most worth a second opinion. Conflating them is the intuitive move and it is wrong in both directions: skipping a dead end hides the explanation from the keyboard user who caused it, and keeping a genuinely unavailable option on the arrow path invites selection of something that is not on offer. The repo's ownrequire-button-wiringrule caught the halfway state — a button carrying bothdisabledandaria-disabled— which is why the two are now rendered exclusively rather than both being spelled onto the element.🤖 Generated with Claude Code
https://claude.ai/code/session_01GMaWYk6iF8bsMwEx38egVX