`refactor: extract per-facet reducers, filter chips, and tests into d…

[fshowalter]

…edicated facets modules`

[claude]

Claude finished @fshowalter's task in 2m 19s —— View job

---

PR Review

• Gather context and read AGENTS.md
• Review facet module structure and reducers
• Review filter chip components
• Review feature-level changes
• Review tests
• Final summary

---

This is a large, well-structured refactor that extracts per-facet reducers, filter chips, and tests into dedicated modules. The architecture is clean and the code quality is high. Here are my findings:

What Works Well

• composeReducers is an elegant solution for building composite reducers without boilerplate. The AIDEV-NOTE correctly explains that composition order doesn't affect correctness since each facet operates on its own state keys.
• omitPendingKey solves the right problem — ensuring selectHasPendingFilters returns false correctly by deleting keys rather than setting them to undefined.
• Shared test suites (gradeFilterFacetTests, kindFacetTests, etc.) eliminate duplication and make it easy to add a facet to a new feature with confidence.
• Export consolidation in feature reducers (re-exporting from facet modules) creates a clean API surface — consumers only import from one place.
• filtersLifecycleReducer AIDEV-NOTE correctly documents why removeAppliedFilter is not handled there — good for maintainability.

Observations / Minor Issues

1. toChipSlug handles spaces only — potential collision risk

src/facets/toChipSlug.ts:7

return value.toLowerCase().replaceAll(" ", "-");

This only converts spaces to hyphens. Values that already contain hyphens (e.g., an edition called "e-book") round-trip correctly. However, two distinct values that differ only in casing or spacing (e.g., "Kindle" and "kindle") would map to the same slug, causing both to be removed from the filter when only one chip is clicked.

This is likely a non-issue for the current set of known values (edition types, kind types, reviewedStatus are fixed enums), but worth noting if the value sets ever become user-driven.

2. nameReducer uses NAME_CHIP_ID as both the chip ID and the state key — coupling is implicit

src/facets/name/nameReducer.ts:33

pendingFilterValues: omitPendingKey(state.pendingFilterValues, NAME_CHIP_ID),

NAME_CHIP_ID = "name" and the state key is also "name" — this works because the constant happens to equal the state key. A future reader could accidentally break this if they change the chip ID without realizing it also doubles as the state key. Compare with reviewYearFacetReducer which uses REVIEW_YEAR_CHIP_ID = "reviewYear" — same implicit coupling.

This is a style concern rather than a bug. titleFacetReducer and nameFacetReducer follow the same pattern consistently.

3. buildSearchChip hardcodes category: "Search"

src/components/filter-and-sort-container/filterChipBuilders.ts:36

category: "Search",
displayText: `Search: ${trimmed}`,

Both buildTitleFilterChip and buildNameFilterChip delegate to buildSearchChip and will display "Search: <value>" in the chip. This looks intentional and consistent with existing UX.

4. Feature reducer exports are verbose

src/features/author-titles/AuthorTitles.reducer.ts:32–43

export { createApplyFiltersAction } from "~/facets/filtersReducer";
export { createClearFiltersAction } from "~/facets/filtersReducer";
export { createRemoveAppliedFilterAction } from "~/facets/filtersReducer";
// ...

Seven individual export-from lines for filters alone. This is fully correct but could be collapsed:

export {
  createApplyFiltersAction,
  createClearFiltersAction,
  createRemoveAppliedFilterAction,
  createResetFiltersAction,
  selectHasPendingFilters,
} from "~/facets/filtersReducer";

(Compare: Authors.reducer.ts already uses this grouped style.) This is a cosmetic inconsistency between feature reducers, not a functional issue. Fix this →

5. gradeFacetReducer uses a local GRADE_STATE_KEY constant instead of re-using an existing identifier

src/facets/grade/gradeReducer.ts:8

const GRADE_STATE_KEY = "gradeValue" as const;

Other reducers (title, name, reviewYear, workYear) pass the chip ID constant directly to omitPendingKey because the chip ID happens to equal the state key. The grade reducer explicitly separates them — GRADE_CHIP_ID = "grade" vs state key "gradeValue" — which is correct and intentional. The local constant name GRADE_STATE_KEY makes this explicit. Good pattern.

Summary

The refactor is clean and well-executed. The composeReducers + filtersLifecycleReducer pattern is a sound design that scales well to additional facets. Test coverage is comprehensive through the shared facet test suites. The only actionable item worth fixing before merge is the grouped exports style inconsistency in AuthorTitles.reducer.ts — everything else is either a non-issue or an intentional design choice.

[github-actions]

🚀 Deployed on https://69c1e79c89e14dde3c57c7a0--fantastic-squirrel-0241b4.netlify.app