fix(filters): address the final review findings (#188)

Restores the escape hatch for a status list matching no preset. filtersFromSearchParams accepts any non-empty subset of {available, reserved, sold}, and only three of those seven lists are presets; the other four reported as the not-sold fallback and produced no chip, so ?status=reserved was an empty grid reading "No items yet - check back soon" with no Clear filters button and no way out but editing the URL. hasActiveFilters covered all seven before this branch, so that was a regression against main. availabilityDimension now emits a chip for any status that is not the not-sold preset, labelled from SALE_STATE_LABELS when the list matches one and from statusLabel when it does not. The Segmented still reads "Not sold" beside such a chip, which is a cosmetic wart and the cheaper half of the trade.

Runs the frontend unit suite in CI. The 22 tests added over the dimensions were invoked by nothing: the workflow's only frontend steps were the build and the end-to-end run, and its test:unit:cov step is the backend's. The new step is guarded and named in the gate like every other suite, per the invariant workflowGate.test.ts asserts.

Restores the comment explaining why availabilityDimension's render and chips pass different fallbacks to saleStateFromStatuses. The control must read "All" while sold favorites are on screen; chips must stay silent because the customer never chose it. Unifying them would give every signed-in favorites view a phantom All chip and a tally of 2, and nothing said so after the old markup was deleted.

Derives the storefront's "is anything filtered" and FilterBar's tally through one exported chipsFor rather than two expressions over two identically-built contexts. They agreed only by convention, which is the exact defect this branch exists to remove.

Documents that statusDimension and availabilityDimension are alternatives over one field, since composing both type-checks and would render two controls that double-count it, and asserts the price chip's label text, which was the only chip label nothing checked.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

.gitea/workflows/sonarqube.yml

@@ -87,6 +87,20 @@ jobs:
         run: npm run build
         working-directory: frontend
 
+      # The frontend's build was the only thing this workspace ran, so the unit
+      # suite #188 added over the filter dimensions was run by nothing but the
+      # author's terminal. A suite CI never runs decays into a record of what
+      # the code used to do, and its value is highest exactly here: chips() is
+      # pure, and the end-to-end run reaches it only through a browser.
+      #
+      # Guarded and named in the gate like every suite: a failing test should
+      # fail the job at the end, not abort it and take the scan with it.
+      - name: Frontend unit tests
+        id: frontend_unit
+        continue-on-error: true
+        run: npm run test:unit
+        working-directory: frontend
+
       - name: Check the Sonar tsconfig has not drifted
         run: node scripts/check-sonar-tsconfig.js
 
@@ -233,6 +247,7 @@ jobs:
       - name: Fail if any guarded step failed
         if: >-
           always() && (
+            steps.frontend_unit.outcome == 'failure' ||
             steps.unit.outcome == 'failure' ||
             steps.integration.outcome == 'failure' ||
             steps.backend.outcome == 'failure' ||

frontend/src/App.tsx

@@ -17,6 +17,7 @@ import { useCatalogue } from './useCatalogue';
 import ItemCard from './components/ItemCard';
 import BrandMark from './components/BrandMark';
 import FilterBar from './components/filters/FilterBar';
+import { chipsFor } from './components/filters/dimension';
 import {
   availabilityDimension,
   categoryDimension,
@@ -149,8 +150,10 @@ export default function App() {
   }, [setSearchParams]);
 
   // The parent needs to know whether anything is filtering — for the empty
-  // state's wording — but has no chip row of its own to count. Computed the
-  // same way FilterBar computes its own tally, from the same dimensions.
+  // state's wording — but has no chip row of its own to count. Through the same
+  // chipsFor call FilterBar's tally goes through, not a second expression over
+  // the same dimensions: those agreed only by convention, which is the defect
+  // #188 exists to remove.
   const filterContext = {
     filters,
     onChange: applyFilters,
@@ -158,7 +161,7 @@ export default function App() {
     tags: options?.tags ?? [],
     priceRange: options?.priceRange ?? null
   };
-  const filtered = STOREFRONT_DIMENSIONS.some((dimension) => dimension.chips(filterContext).length > 0);
+  const filtered = chipsFor(STOREFRONT_DIMENSIONS, filterContext).length > 0;
 
   return (
     <Layout style={{ minHeight: '100vh' }}>

frontend/src/components/filters/FilterBar.tsx

@@ -5,6 +5,7 @@ import Grid from 'antd/es/grid';
 import { FilterOutlined } from '@ant-design/icons';
 import type { Category, Tag as ItemTag } from '../../api';
 import type { ItemFilters } from '../../filters';
+import { chipsFor } from './dimension';
 import type { FilterContext, FilterDimension } from './dimension';
 import FilterChips from './FilterChips';
 
@@ -60,7 +61,11 @@ export default function FilterBar({
   // Derived, not stored, and derived without rendering anything — the drawer
   // unmounts its sections when closed, so anything that needed them mounted
   // would lose the chips exactly when they matter.
-  const chips = dimensions.flatMap((dimension) => dimension.chips(context));
+  //
+  // Through chipsFor rather than inline: the page asks the same question for its
+  // empty state, and one shared call is what stops the tally and the chip row
+  // from being two expressions that only happen to agree.
+  const chips = chipsFor(dimensions, context);
 
   const barDimensions = dimensions.filter((dimension) => dimension.placement === 'bar');
   const drawerDimensions = dimensions.filter((dimension) => dimension.placement === 'drawer');

frontend/src/components/filters/dimension.ts

@@ -49,3 +49,17 @@ export interface FilterDimension {
   /** Empty when this dimension is filtering nothing. */
   chips(ctx: FilterContext): Chip[];
 }
+
+/**
+ * Every chip a set of dimensions reports, in the order the screen composed them.
+ *
+ * The single derivation both sides share. FilterBar's tally is this list's
+ * length, and a page's "is anything filtered" — which is what decides whether an
+ * empty grid reads as "no matches, here is the way out" or as an empty shop — is
+ * whether it is empty. Those were two separate expressions over two
+ * identically-built contexts until #188's review, agreeing only by convention.
+ * That is precisely the tally-versus-chip-row disagreement this design exists to
+ * make impossible, so there is one call and no second opinion.
+ */
+export const chipsFor = (dimensions: FilterDimension[], ctx: FilterContext): Chip[] =>
+  dimensions.flatMap((dimension) => dimension.chips(ctx));

frontend/src/components/filters/standardDimensions.tsx

@@ -234,6 +234,15 @@ export const favoritesDimension: FilterDimension = {
       : []
 };
 
+/**
+ * The statuses themselves, as a multi-select.
+ *
+ * Mutually exclusive with availabilityDimension below, which is the preset
+ * alternative over the same `filters.status` field: a screen composes one or
+ * the other, never both. Composing both type-checks and renders two controls
+ * over one field, each fighting the other's writes and each contributing chips,
+ * so the tally counts the same filter twice.
+ */
 export const statusDimension: FilterDimension = {
   key: 'status',
   placement: 'drawer',
@@ -287,6 +296,11 @@ const SALE_STATE_LABELS: Record<SaleState, string> = {
  * makes and is worth having visible without opening anything. Before #188 it
  * was hand-written markup in App.tsx because the shared component had no way to
  * say "this belongs in the bar", which is the gap that design closed.
+ *
+ * Mutually exclusive with statusDimension above, which is the multi-select
+ * alternative over the same `filters.status` field: a screen composes one or
+ * the other, never both, or two controls write one field and it is counted
+ * twice.
  */
 export const availabilityDimension: FilterDimension = {
   key: 'availability',
@@ -295,6 +309,12 @@ export const availabilityDimension: FilterDimension = {
   render: ({ filters, onChange }) => (
     <Segmented
       aria-label="Filter by availability"
+      // The fallback depends on favoritesOnly, and must. Favorites deliberately
+      // include sold items (see ItemFilters), so with the favorites switch on
+      // and no explicit status the grid really is showing everything — a
+      // control reading "Not sold" over sold items on screen would be lying
+      // about what the customer is looking at. chips() below passes a different
+      // fallback on purpose; the two are not a copy of each other to unify.
       value={saleStateFromStatuses(
         filters.status,
         STOREFRONT_SALE_STATUSES,
@@ -318,12 +338,38 @@ export const availabilityDimension: FilterDimension = {
   ),
 
   chips: ({ filters, onChange }) => {
-    const state = saleStateFromStatuses(filters.status, STOREFRONT_SALE_STATUSES, 'not-sold');
-    if (state === 'not-sold') return [];
+    const statuses = filters.status;
+    // No preference is not a filter, whatever the control happens to read.
+    if (statuses === null) return [];
+
+    // 'not-sold' unconditionally, deliberately unlike render above: that
+    // fallback follows favoritesOnly so the control tells the truth about what
+    // is on screen, but the customer never *chose* All, and a filter nobody
+    // chose must not produce a chip or count toward the tally. Unifying the two
+    // calls would give every signed-in favorites view a phantom All chip and a
+    // tally of 2.
+    const state = saleStateFromStatuses(statuses, STOREFRONT_SALE_STATUSES, 'not-sold');
+    // saleStateFromStatuses reports a list matching no preset as its fallback,
+    // so asking twice with different fallbacks is what separates a real match
+    // from a fallback: the answers agree only when the list matched something.
+    const isPreset = state === saleStateFromStatuses(statuses, STOREFRONT_SALE_STATUSES, 'all');
+
+    // Only the actual default earns silence. filtersFromSearchParams accepts
+    // any non-empty subset of {available, reserved, sold} — seven lists, of
+    // which three are presets — and before #188 hasActiveFilters reported all
+    // seven as filtered. Without a chip for the other four, ?status=reserved is
+    // an empty grid reading "No items yet — check back soon" with no Clear
+    // filters button, and the only way out is editing the URL by hand.
+    if (isPreset && state === 'not-sold') return [];
+
     return [
       {
         key: 'availability',
-        label: SALE_STATE_LABELS[state],
+        // A known cosmetic wart, and the cheaper half of the trade: for a list
+        // matching no preset the Segmented still reads "Not sold" while this
+        // chip is showing, because there is no fourth position to move it to.
+        // A control one word out beats a dead end with no way back.
+        label: isPreset ? SALE_STATE_LABELS[state] : statuses.map(statusLabel).join(', '),
         onRemove: () => onChange({ ...filters, status: null })
       }
     ];

frontend/tests/unit/filterDimensions.test.ts

@@ -105,6 +105,14 @@ describe('priceDimension', () => {
     expect(priceDimension.chips(contextFor({ maxPriceCents: 5000 }).ctx)).toHaveLength(1);
   });
 
+  // formatPriceRange's real output, en dash and all. The only chip whose label
+  // text nothing asserted on, which is how a chip reading "$1000–$5000" — cents
+  // shown as dollars — would have reached a customer unnoticed.
+  it('labels the chip with the formatted range', () => {
+    const { ctx } = contextFor({ minPriceCents: 1000, maxPriceCents: 5000 });
+    expect(priceDimension.chips(ctx)[0]?.label).toBe('$10–$50');
+  });
+
   it('clears both ends when removed', () => {
     const { ctx, state } = contextFor({ minPriceCents: 1000, maxPriceCents: 5000 });
     priceDimension.chips(ctx)[0]?.onRemove();
@@ -188,4 +196,30 @@ describe('availabilityDimension', () => {
     availabilityDimension.chips(ctx)[0]?.onRemove();
     expect(state.latest?.status).toBeNull();
   });
+
+  // filtersFromSearchParams accepts any non-empty subset of the three public
+  // statuses — seven lists, of which only three are presets. The other four
+  // report as the 'not-sold' fallback, so a chip keyed off the preset alone
+  // leaves ?status=reserved as an empty grid reading "No items yet" with no
+  // Clear filters button. hasActiveFilters covered all seven before #188; this
+  // is what replaces it.
+  it('reports one chip for a status list matching no preset', () => {
+    const chips = availabilityDimension.chips(contextFor({ status: ['reserved'] }).ctx);
+    expect(chips.map((c) => c.label)).toEqual(['Reserved']);
+
+    // Several statuses read as a list rather than as a preset's name.
+    expect(
+      availabilityDimension
+        .chips(contextFor({ status: ['available', 'sold'] }).ctx)
+        .map((c) => c.label)
+    ).toEqual(['Available, Sold']);
+  });
+
+  it('clears the filter when a chip for no preset is removed', () => {
+    const { ctx, state } = contextFor({ status: ['reserved', 'sold'] });
+    const chips = availabilityDimension.chips(ctx);
+    expect(chips).toHaveLength(1);
+    chips[0]?.onRemove();
+    expect(state.latest?.status).toBeNull();
+  });
 });