Skip to content

feat(formulation): wire CatalogueToolbar into the builder page (#039) - #2085

Closed
BigSimmo wants to merge 1 commit into
mainfrom
claude/catalogue-toolbar-formulation-039
Closed

feat(formulation): wire CatalogueToolbar into the builder page (#039)#2085
BigSimmo wants to merge 1 commit into
mainfrom
claude/catalogue-toolbar-formulation-039

Conversation

@BigSimmo

Copy link
Copy Markdown
Owner

Summary

Verification

  • npm run verify:pr-localnot run: this environment has only Node 22 installed, and the repo pins engines: ">=24.15.0 <25" with engine-strict=true in .npmrc, so npm ci/npm install hard-fails before installing anything (same constraint noted on PR Show the form's own name on the forms source card, tidy badges #2041). No toolchain was available to run tests, typecheck, or lint locally.
  • In lieu of running them, I reviewed the diff by hand: verified brace/paren balance in the rewritten component, traced every CatalogueToolbarProps field used in formulation-builder-page.tsx against the type definition, and walked each of the four tests/catalogue-toolbar.dom.test.tsx cases against the fixed component logic to confirm none regress (in particular the "disabled when no toggle handler" and singular/plural count tests, since those are exactly the areas I touched).
  • Please run npm run test:focused -- --files src/components/ui/catalogue-toolbar.tsx,src/components/formulation/formulation-builder-page.tsx,tests/catalogue-toolbar.dom.test.tsx (or npm run verify:pr-local) before merge.
  • npm run verify:ui — not run, same environment constraint; this is a page-scoped component swap (shared ResultFilterSheet/ResultFilterFacetChips were not touched), so per docs/testing.md's speed playbook a focused DOM/component check is the appropriate tier — recommend a quick manual or CI Production UI check of /formulation/build before merge.
  • npm run verify:release — not applicable.
  • Known follow-up needed before merge:npm run design-system:adoption:update (writes docs/design-system/adoption-manifest.json / ADOPTION.md / COMPONENTS.md) was not run for the same toolchain reason — CatalogueToolbar is now used in production code but the adoption manifest doesn't reflect it yet, so check:design-system-contract will likely fail until someone with a Node 24 environment runs that regeneration command and commits the result. I deliberately did not hand-edit those generated files.

Risk and rollout

  • Risk: Low — page-scoped UI change (one ad hoc control pairing replaced by a shared component) plus a new, currently-unconsumed-elsewhere component. No API, schema, ingestion, or answer-generation code touched.
  • Rollback: Revert this commit; no data or migration involved.
  • Provider or production effects: None.

Notes


Generated by Claude Code

Extracts the CatalogueToolbar component from PR #2073 and wires it into
formulation-builder-page.tsx in place of the ad hoc TextField+Select
pairing, without that PR's DoseLine/AnswerFooter adoption-manifest
changes (see #2073's closure comment: those extract new components but
never wire them into a real consumer, which inflates the design-system
adoption count without satisfying #267's actual backend-payload
requirement).
Also fixes three CodeRabbit-flagged correctness issues in
catalogue-toolbar.tsx while taking ownership of the component:
- isFilterTriggerProps only recognized objects carrying onToggle or
activeCount, so a filterTrigger config with just e.g. { label,
disabled } fell through to being rendered as a React child and threw.
Now checks all seven declared keys.
- The plural-noun fallback derived plurals by appending "s", which is
wrong for irregular nouns (e.g. "status" -> "statu"/"status" instead
of "statuses"). Added an explicit pluralNoun override, defaulting to
the previous "${noun}s" behavior for existing callers.
- Applied-filter chips rendered "undefined: value" when a chip had no
groupLabel, and always announced groupLabel in the remove button's
aria-label even when accessibleLabel was supplied. Now renders the
group prefix only when present and prefers accessibleLabel.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RnhwRP5FsY5JktkbwAcubD
@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, add credits to your account and enable them for code reviews in your settings.

@supabase

supabaseBot commented Aug 18, 2026

Copy link
Copy Markdown

This pull request has been ignored for the connected project sjrfecxgysukkwxsowpy because there are no changes detected in supabase directory. You can change this behaviour in Project Integrations Settings ↗︎.


Preview Branches by Supabase.
Learn more about Supabase Branching ↗︎.

@coderabbitai

Copy link
Copy Markdown
Contributor

Warning

Review limit reached

You’ve reached a temporary PR review limit under our Fair Usage Limits Policy.

Your recent review volume is higher than typical usage, so adaptive limits are currently applied.

Next review available in:57 minutes

Limit details: You’ve used all 1 included review currently available under your plan. You completed 99 included PR reviews in the past 7 days; at that activity level, included reviews refill at 1 review per hour.

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 @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

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 configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 24397074-19e5-44b5-af13-96524a4dcebe

📥 Commits

Reviewing files that changed from the base of the PR and between 2412391 and f491ab0.

📒 Files selected for processing (4)
  • src/components/catalogue-toolbar.tsx
  • src/components/formulation/formulation-builder-page.tsx
  • src/components/ui/catalogue-toolbar.tsx
  • tests/catalogue-toolbar.dom.test.tsx

Comment @coderabbitai help to get the list of available commands.

@github-actions

Copy link
Copy Markdown
Contributor

CI triage

CI failed on this PR. Automated classification of the 3 failed job(s):

  • Static PR checksneeds investigation: inspect the failing step and uploaded diagnostics; rerun only after classifying the cause.
  • Unit coverageneeds investigation: inspect the failing step and uploaded diagnostics; rerun only after classifying the cause.
  • PR requiredneeds investigation: inspect the failing step and uploaded diagnostics; rerun only after classifying the cause.

Compared with main CI run #11840 (cancelled).

Classification is evidence routing, not permission to ignore a failure. Exact quarantined Playwright identities remain governed by the flake ledger.

@BigSimmo

Copy link
Copy Markdown
OwnerAuthor

Superseded by #2086 (same catalogue-toolbar.tsx / barrel / formulation-builder-page.tsx work, #2086 is the superset — also wires DoseLine + AnswerFooter). Both currently red on Static PR checks + Unit coverage. Closing to avoid two PRs racing the same files; fix continues on #2086.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@BigSimmo@claude