Architecture refactor scout: github-30248671647

cc @algolia/frontend-experiences-web

# Architecture Refactor Scout

Run: `github-30248671647`

## Summary

I scouted the whole monorepo, focusing on where behavior actually lives: the connectors in `packages/instantsearch.js/src/connectors/`, the runtime in `src/lib/` (`InstantSearch.ts`, routing, insights), and the shared UI layer in `instantsearch-ui-components`. No `CONTEXT.md` or `docs/adr/` exist. The recurring friction is **shallow orchestration with leaked details**: the same facet-connector bookkeeping (UI-state cleanup, `maxValuesPerFacet`, show-more state) is copy-pasted across six connectors instead of sitting behind one deep module; the search **status/stalled** state machine is smeared across `InstantSearch.ts` and the `index` widget as loose timers and manual `status` assignments; and the router+stateMapping seam splits normalization between `createRouterMiddleware` and `history.ts`. The strongest, most reviewable wins are the facet-connector consolidations and the stalled-search coordinator — each is high-locality, has a small natural test surface, and maps cleanly to one PR.

## Candidate Shortlist

candidate-1: Deepen facet UI-state normalization

- Recommendation strength: `Strong`
- Files:
- `packages/instantsearch.js/src/connectors/refinement-list/connectRefinementList.ts` (`removeEmptyRefinementsFromUiState` at ~569–589; `maxValuesPerFacet` at ~530–542)
- `packages/instantsearch.js/src/connectors/menu/connectMenu.ts` (~403–420)
- `packages/instantsearch.js/src/connectors/hierarchical-menu/connectHierarchicalMenu.ts` (~508–528)
- `packages/instantsearch.js/src/connectors/numeric-menu/connectNumericMenu.ts` (~488–505)
- `packages/instantsearch.js/src/connectors/rating-menu/connectRatingMenu.ts` (~479–496)
- `packages/instantsearch.js/src/connectors/breadcrumb/connectBreadcrumb.ts` (~337–357)
- New home under `packages/instantsearch.js/src/lib/utils/`
- Problem: Every facet connector defines its own `removeEmptyRefinementsFromUiState(indexUiState, attribute)` — six near-identical copies that only differ in the widget key (`refinementList` vs `menu` vs `numericMenu`…) and the "is this refinement empty?" test (`.length === 0`, `=== undefined`, `=== ':'`, `typeof !== 'number'`). The same shape appears again for the `maxValuesPerFacet` bump in the three list-style connectors. This is pass-through bookkeeping: the knowledge of *how a facet clears itself out of `uiState`* is scattered instead of living in one place, so a bug in the cleanup logic must be fixed six times, and each connector's `getWidgetUiState` reader must re-derive the same invariant.
- Proposed change: Move the empty-refinement cleanup (and, as a second commit, the `maxValuesPerFacet` computation) into a single shared helper module that takes the widget key and an "is-empty" predicate, and have each connector call it. The connectors keep only their widget-specific predicate; the traversal/deletion/parent-pruning invariant lives once behind a small interface.
- Benefits: Locality — one place owns "remove an empty facet refinement from `uiState` without leaving dangling parent keys." Leverage — six call sites collapse onto one interface. Testability — the invariant becomes directly unit-testable instead of being re-verified through each connector's `getWidgetUiState` tests.
- Risks: Low. Pure functions, no lifecycle. Main care is preserving each connector's exact empty-check semantics (the `':'` and `typeof number` cases are easy to get subtly wrong). Cross-flavor common connector tests already exercise the behavior.
- Verification: `yarn jest packages/instantsearch.js/src/connectors/{refinement-list,menu,hierarchical-menu,numeric-menu,rating-menu,breadcrumb}` and `yarn jest common-connectors`.

Before:

```mermaid
flowchart LR
RL[connectRefinementList] --> C1[copy: removeEmpty + maxValues]
Menu[connectMenu] --> C2[copy: removeEmpty + maxValues]
HM[connectHierarchicalMenu] --> C3[copy: removeEmpty + maxValues]
NM[connectNumericMenu] --> C4[copy: removeEmpty]
RM[connectRatingMenu] --> C5[copy: removeEmpty]
BC[connectBreadcrumb] --> C6[copy: removeEmpty]
```

After:

```mermaid
flowchart LR
RL[connectRefinementList] --> Deep[facet uiState util]
Menu[connectMenu] --> Deep
HM[connectHierarchicalMenu] --> Deep
NM[connectNumericMenu] --> Deep
RM[connectRatingMenu] --> Deep
BC[connectBreadcrumb] --> Deep
Deep --> Detail[hidden: traversal + parent pruning + predicate]
```

candidate-2: Extract a stalled-search / status scheduling coordinator

- Recommendation strength: `Strong`
- Files:
- `packages/instantsearch.js/src/lib/InstantSearch.ts` (`_searchStalledTimer` at 230/385/779; `scheduleRender` at 811–827; `scheduleStalledRender` at 829–836; `status`/`error` assignments)
- `packages/instantsearch.js/src/widgets/index/index.ts` (the derived-helper `'search'` listener that calls `scheduleStalledRender()`)
- Problem: The search **status state machine** (`idle` → `loading` → `stalled` → `idle`) is not a module — it is a set of loose fields and timers on `InstantSearch`. `scheduleRender` both renders *and* conditionally clears `_searchStalledTimer` and resets `status`/`error` (811–820); `scheduleStalledRender` owns a raw `setTimeout` that flips `status = 'stalled'` (829–836); `index/index.ts` reaches back in to call `scheduleStalledRender()` on every derived `'search'`; and `dispose` must remember to `clearTimeout(this._searchStalledTimer)` (779). To understand "why is the UI showing a stalled spinner," a reader must bounce between three regions and reconstruct the timing (`_stalledSearchDelay`) and the reset rules themselves. The status transition is an implementation detail leaked into the render scheduler and the index widget.
- Proposed change: Give the status/stalled concern its own module owned by `InstantSearch`: it holds the timer and the `status`/`error` transitions, exposes a small set of intent methods (e.g. "a search started," "a render happened," "dispose") and a readable status, and internally decides when to schedule a stalled render and when to reset. `scheduleRender` stops hand-managing the timer; `index/index.ts` signals search activity through the coordinator instead of poking `scheduleStalledRender` directly.
- Benefits: Depth — a genuine state machine (timer + transitions + reset invariants) sits behind a tiny interface. Locality — all stalled/status logic lands in one file with one test surface, and `dispose` can't forget the timer. Leverage — `InstantSearch.ts` shrinks toward pure orchestration.
- Risks: Medium — touches the core search lifecycle, so mis-ordering a transition could change when widgets see `status`. Mitigated by the existing InstantSearch lifecycle and stalled-search tests; keep the public `status`/`error` fields behaviorally identical.
- Verification: `yarn jest packages/instantsearch.js/src/lib/__tests__/InstantSearch` plus any stalled-search specs; confirm `status` timing is unchanged in common widget tests that assert loading/stalled states.

Before:

```mermaid
flowchart LR
Index[index widget] --> Stalled[scheduleStalledRender timer]
Render[scheduleRender] --> Stalled
Render --> Status[status/error fields]
Dispose[dispose] --> Stalled
```

After:

```mermaid
flowchart LR
Index[index widget] --> Coord[status coordinator]
Render[scheduleRender] --> Coord
Dispose[dispose] --> Coord
Coord --> Detail[hidden: timer + status transitions + reset]
```

candidate-3: Extract facet show-more state into one module

- Recommendation strength: `Worth exploring`
- Files:
- `packages/instantsearch.js/src/connectors/refinement-list/connectRefinementList.ts` (~240–260, `canToggleShowMore` at ~431–435)
- `packages/instantsearch.js/src/connectors/menu/connectMenu.ts` (~185–202, ~305)
- `packages/instantsearch.js/src/connectors/hierarchical-menu/connectHierarchicalMenu.ts` (~216–239, ~398)
- Problem: The three list-style facet connectors each re-implement the same show-more bookkeeping: an `isShowingMore` flag, a `cachedToggleShowMore`/`createToggleShowMore` pair (to keep a stable function reference across renders), a `getLimit()` that switches between `limit` and `showMoreLimit`, and a `canToggleShowMore` derivation. Nine `isShowingMore`/`cachedToggleShowMore`/`getLimit` occurrences per connector confirm the copy. Callers of `getWidgetRenderState` must understand a subtle lazy-initialization dance (the toggle starts as a no-op and is reassigned mid-render). The interface each connector exposes is nearly as complex as the implementation — a shallow, duplicated pattern.
- Proposed change: Extract the show-more state (flag + stable toggle factory + `getLimit` + `canToggleShowMore` derivation) into one module the three connectors instantiate. Each connector supplies its `limit`/`showMoreLimit`/`showMore` config and the render trigger; the stability and limit-switching invariants live once.
- Benefits: Locality and testability of the toggle-stability + limit-switching logic; the three connectors keep only wiring. Removes a recurring source of subtle "stale toggle reference" bugs.
- Risks: Medium — the toggle factory closes over `renderOptions`/`widget.render`, so the extracted module must not leak that coupling back out; `canToggleShowMore` differs slightly (refinement-list factors in `hasExhaustiveItems`, menu/hierarchical use raw counts), so the predicate must stay caller-supplied. Overlaps thematically with candidate-1 but is a distinct PR (render state, not `uiState`).
- Verification: `yarn jest packages/instantsearch.js/src/connectors/{refinement-list,menu,hierarchical-menu}` and the show-more common tests.

Before:

```mermaid
flowchart LR
RL[connectRefinementList] --> S1[copy: isShowingMore + toggle + getLimit]
Menu[connectMenu] --> S2[copy: isShowingMore + toggle + getLimit]
HM[connectHierarchicalMenu] --> S3[copy: isShowingMore + toggle + getLimit]
```

After:

```mermaid
flowchart LR
RL[connectRefinementList] --> Deep[show-more state module]
Menu[connectMenu] --> Deep
HM[connectHierarchicalMenu] --> Deep
Deep --> Detail[hidden: stable toggle + limit switching]
```

candidate-4: Consolidate router + stateMapping orchestration

- Recommendation strength: `Worth exploring`
- Files:
- `packages/instantsearch.js/src/middlewares/createRouterMiddleware.ts` (orchestration: `topLevelCreateURL`, `subscribe`/`onStateChange`, dedupe via `isEqual`)
- `packages/instantsearch.js/src/lib/routers/history.ts` (`writeDelay`, `cleanUrlOnDispose`, `windowTitle`, `parseURL`/`createURL`, `shouldWrite`)
- `packages/instantsearch.js/src/lib/stateMappings/{simple,singleIndex}.ts`
- `packages/instantsearch.js/src/lib/InstantSearch.ts` (routing wired at construction, ~403–406)
- Problem: The router+stateMapping seam is leaky. Route de-duplication happens in the middleware (`isEqual(lastRouteState, routeState)`) while a separate `shouldWrite` check lives in `history.ts`; write normalization (`writeDelay`, `cleanUrlOnDispose`, `windowTitle`) sits on the router but the middleware owns the round-trip `routeToState`/`stateToRoute` composition and initial-state merge. Understanding one URL update means bouncing between `InstantSearch.ts`, `createRouterMiddleware.ts`, and `history.ts`, and the internal generic `BrowserHistoryArgs` leaks out to `react-instantsearch-router-nextjs`. The round-trip invariant `routeToState(stateToRoute(x)) ≈ x` is documented but nowhere enforced.
- Proposed change: Pull the router↔stateMapping dance into one internal orchestration module that the middleware delegates to — it owns the compose order, the single de-dupe point, and lifecycle sequencing — leaving `createRouterMiddleware` as thin glue and keeping the public `Router`/`StateMapping` contracts untouched.
- Benefits: One place owns "map uiState↔route and decide when to write," so the two de-dupe checks unify and the normalization split closes. No public API change; flavors keep depending only on the middleware/router/stateMapping contracts.
- Risks: Medium — routing is timing-sensitive (subscribe runs before `started`, popstate ordering), so behavior parity must be proven against the existing routing/RoutingManager tests; larger surface than candidates 1–3, so scope carefully to stay one PR.
- Verification: `yarn jest packages/instantsearch.js/src/lib/routers` and the routing/RoutingManager middleware tests; smoke the Next.js router integration.

Before:

```mermaid
flowchart LR
IS[InstantSearch] --> MW[router middleware]
MW --> Dedupe1[dedupe: isEqual]
MW --> Map[stateMapping compose]
MW --> Hist[history.ts]
Hist --> Dedupe2[shouldWrite + normalization]
```

After:

```mermaid
flowchart LR
IS[InstantSearch] --> MW[thin middleware]
MW --> Orch[routing orchestrator]
Orch --> Detail[hidden: compose order + single dedupe + normalization]
```

candidate-5: Deepen the insights event-sending seam

- Recommendation strength: `Speculative`
- Files:
- `packages/instantsearch.js/src/lib/utils/createSendEventForHits.ts` (payload building, chunking, "internal" click dedupe at ~166–194)
- `packages/instantsearch.js/src/lib/utils/createSendEventForFacet.ts`
- `packages/instantsearch.js/src/middlewares/createInsightsMiddleware.ts` (automatic/internal tagging, `viewedObjectIDs` dedupe)
- Facet connectors that build filter payloads inline (`connectRatingMenu`, `connectToggleRefinement`, `connectNumericMenu`, `connectMenu`)
- Problem: To follow a single click event you hop through connector → `createSendEventForHits` → payload builder → `sendEventToInsights` → middleware → insights client. Payload shape, chunk size, "internal" de-dupe, and serialization are split between the two factory utilities and the middleware, and several facet connectors hand-assemble `filters: [...]` payloads inline. Callers know too much about payload structure and enrichment ordering (`__queryID`/`__position` must already be set), and the automatic-vs-manual insights distinction is invisible to connectors.
- Proposed change: Concentrate event-shape construction (and its escaping/chunking/dedupe) behind one seam so connectors emit an intent by type rather than assembling payloads, and the middleware consumes a uniform event. Keep a compatibility path for existing `sendEvent`/`bindEvent` callers.
- Benefits: Would hide payload/serialization details behind a small interface and remove the inline filter-payload duplication in facet connectors, concentrating event knowledge in one place.
- Risks: High/uncertain — insights is behavior-sensitive and widely relied on; the natural boundary isn't obvious yet and a full registry could balloon past a single reviewable PR. Marked `Speculative`: worth a focused mini-explore before committing, and likely better split (e.g. just the inline facet-filter payloads first).
- Verification: `yarn jest packages/instantsearch.js/src/lib/utils/__tests__/createSendEventFor*` and the insights middleware tests; `yarn jest common-connectors` for `sendEvent` coverage.

Before:

```mermaid
flowchart LR
Conn[connectors] --> Hits[createSendEventForHits]
Conn --> Facet[createSendEventForFacet]
Conn --> Inline[inline filter payloads]
Hits --> MW[insights middleware]
Facet --> MW
Inline --> MW
MW --> Detail[tagging + dedupe + client]
```

After:

```mermaid
flowchart LR
Conn[connectors] --> Deep[event seam]
Deep --> MW[insights middleware]
Deep --> Detail[hidden: payload shape + chunk + dedupe + serialize]
```

## Top Recommendation

Implement **candidate-1 (Deepen facet UI-state normalization)** first. It is the cleanest match for the rubric: six connectors carry a near-identical `removeEmptyRefinementsFromUiState` (plus the `maxValuesPerFacet` bump in three of them), so the change has maximum **locality** (one new util owns the invariant), obvious **leverage** (six call sites collapse to one small interface), and real **depth** (parent-key pruning + the widget-specific empty predicate hide behind a tiny surface). The **PR size** is small and low-risk — pure functions with an existing cross-flavor test net — making it the safest first step before the higher-risk lifecycle work in candidate-2.

## Next Step

To implement a selected candidate, run:

`/implement candidate-1`

Replace `candidate-1` with the id of the candidate you want to implement (`candidate-1` through `candidate-5` from the shortlist above).

## Non-Candidates

- **Unified cross-flavor UI adapter for the `createComponent({ createElement, Fragment })` factories.** Each flavor's glue (React hooks, Vue `renderCompat`/pragma shims, the Preact renderer) is fundamentally framework-specific; a single adapter would become a meta-framework. The only genuinely local sub-win — consolidating React's duplicated `Highlight`/`ReverseHighlight`/`Snippet` class-name wrappers — is presentation glue with little depth, so it doesn't clear the bar.
- **Refactoring `algoliasearch-helper`.** Per repo conventions it is mature and separately versioned; behavior fixes belong in connectors, not here.
- **A broad `InstantSearch.ts` decomposition.** Splitting scheduling + uiState sync + middleware orchestration together would be 500+ lines and unreviewable. candidate-2 deliberately carves out only the stalled/status concern.
- **`ai-lite/abstract-chat.ts` (the largest file at ~54KB) and the chat connector/runtime.** Recently added and still in flux; refactoring now risks churn against active development rather than deepening a settled module.
- **Extracting `init()`/`render()` and `getRenderState()` boilerplate from every connector.** Tempting (100% duplicated) but the widget factory isn't class-based, so any extraction adds an awkward higher-order wrapper for near-zero depth gain — pass-through, fails the deletion test.

This issue is an architecture scout with five candidate refactors rather than one selected change. Start by choosing a candidate, then read its named entry points such as the facet connectors, InstantSearch.ts, or createRouterMiddleware.ts and run the corresponding Jest commands listed in that section. Done means one focused refactor has a defined interface, preserves behavior, and passes its named tests.

Written by the indexing model from the issue text.