Architecture refactor scout: github-33367089977

cc @algolia/frontend-experiences-web

# Architecture Refactor Scout

Run: `github-33367089977`

## Summary

I scouted the whole monorepo, concentrating on `packages/instantsearch.js` where the connectors, runtime (`src/lib`), routing, and insights logic live (this is the shared core behind every flavor). No `CONTEXT.md` or `docs/adr/` exist. The recurring friction is **shallow, duplicated orchestration inside connectors and small runtime modules**: the same "show more" facet-limit state machine is hand-rolled in three refinement connectors; a `connectorState` lazy-init guard is copy-pasted across seven connectors; the two `stateMapping` adapters each redefine the identical "strip `configure` before it reaches the URL" rule with no shared seam; and insights events flow through a `sendEventToInsights` property that starts as `noop` and is silently rewired mid-lifecycle, forcing callers to know the timing. Each is a case where the interface a connector must learn is nearly as large as the behavior behind it — good targets for deepening a small module rather than spreading the logic. The newer chat/AI subsystem has similar shape but is explicitly `not yet stable` (`connectChat.ts:477`), so I mark those ideas Speculative.

## Candidate Shortlist

candidate-1: Deepen show-more facet state behind one module

- Recommendation strength: `Strong`
- Files:
- `packages/instantsearch.js/src/connectors/refinement-list/connectRefinementList.ts` (`isShowingMore` L240, `cachedToggleShowMore` L244, `createToggleShowMore` L248, `getLimit` L258, wiring L422/L432/L454-456)
- `packages/instantsearch.js/src/connectors/menu/connectMenu.ts` (same four: L185-201, wiring L292/L327-328)
- `packages/instantsearch.js/src/connectors/hierarchical-menu/connectHierarchicalMenu.ts` (same four: L219-238, wiring L307/L412-413)
- Problem: Three connectors independently implement the identical "show more" limit machine: a mutable `isShowingMore` flag, a `createToggleShowMore(renderOptions, widget)` factory that flips the flag and triggers a re-render, a `cachedToggleShowMore` proxy that keeps the render-state callback stable across renders, and a `getLimit()` that picks between `limit` and `showMoreLimit`. Every connector must also re-know *when* to (re)assign `toggleShowMore = createToggleShowMore(...)` (only once results exist) and how to compute `canToggleShowMore`. The interface each connector must learn (four interlocking functions + a re-render side effect + a caching rule) is as large as the behavior itself — a shallow, copy-pasted module.
- Proposed change: Extract the show-more limit state machine into one small module in `src/connectors` (or `src/lib/utils`) that owns the flag, the toggle, the stable-callback caching, and the `getLimit`/`canToggleShowMore` derivation. The three connectors consume it and read back the derived values instead of re-implementing them. Keep it internal to the connectors; do not change the public render-state shape (`isShowingMore`, `toggleShowMore`, `canToggleShowMore` stay identical).
- Benefits: Locality — the toggle/caching/limit rule lives and is tested in one place instead of three drifting copies. Leverage — a facet connector opts into paging by learning one call, not four functions plus a caching convention. Testability — the state machine becomes the natural test surface (toggle flips limit, callback identity stays stable across renders) rather than being asserted three times through full connector render cycles.
- Risks: The three call sites differ in small ways (hierarchical-menu computes `hasMoreItems` differently; refinement-list compares against `lastItemsFromMainSearch`), so the extracted module must expose those derivations without over-generalizing. Must preserve exact `toggleShowMore` reference stability or React/Vue wrappers may re-render differently. Covered by existing common + per-connector tests.
- Verification: `yarn jest connectRefinementList connectMenu connectHierarchicalMenu`; `yarn jest common-widgets -t "RefinementList"` / `-t "Menu"` / `-t "HierarchicalMenu"`; confirm `toggleShowMore` identity is stable across renders and toggling swaps `limit`↔`showMoreLimit`.

Before:

```mermaid
flowchart LR
RL[connectRefinementList] --> M1[isShowingMore + createToggleShowMore + cachedToggleShowMore + getLimit]
MN[connectMenu] --> M2[isShowingMore + createToggleShowMore + cachedToggleShowMore + getLimit]
HM[connectHierarchicalMenu] --> M3[isShowingMore + createToggleShowMore + cachedToggleShowMore + getLimit]
```

After:

```mermaid
flowchart LR
RL[connectRefinementList] --> Deep[Show-more state module]
MN[connectMenu] --> Deep
HM[connectHierarchicalMenu] --> Deep
Deep --> Detail[flag + toggle + stable callback + limit rule]
```

candidate-2: Centralize route-sync filtering in stateMapping

- Recommendation strength: `Worth exploring`
- Files:
- `packages/instantsearch.js/src/lib/stateMappings/simple.ts` (`getIndexStateWithoutConfigure` L3-8, used L23 + L37)
- `packages/instantsearch.js/src/lib/stateMappings/singleIndex.ts` (`getIndexStateWithoutConfigure` L3-8, used L18 + L22)
- `packages/instantsearch.js/src/lib/stateMappings/index.ts`
- `packages/instantsearch.js/src/middlewares/createRouterMiddleware.ts` (consumes `stateToRoute`/`routeToState`, merges the partial `routeToState` output back into `initialUiState`)
- Problem: Both stateMapping adapters privately redefine the *same* rule — "the `configure` widget's state is never persisted to the URL" — as a duplicated `getIndexStateWithoutConfigure` helper. The knowledge that routing must exclude `configure` (and only `configure`) is a cross-cutting invariant scattered across two adapters, with a hand-written comment in `simple.ts` explaining the typing. `routeToState` also returns a deliberately *partial* index state that the router middleware must know to merge back — that contract lives in the caller, not the seam. Adding another non-syncable field (e.g. transient chat/AI state) means editing every adapter and hoping they stay in sync.
- Proposed change: Give the stateMapping module one shared place that owns "what is excluded from the route," so each adapter declares/uses it instead of re-deriving `configure` stripping. Make the "`routeToState` yields a partial state that is merged onto the prior ui state" contract explicit at the seam rather than implied by the middleware's merge. Do not redesign the `StateMapping` type surface yet — just remove the duplicated rule and name the merge contract.
- Benefits: Locality — the "not URL-synced" rule is defined once; a future non-syncable field is a one-line change at the seam. Leverage — adapter authors satisfy the `StateMapping` interface without re-learning the exclusion rule. Testability — the exclusion rule gets a direct unit test instead of being asserted indirectly through two adapters and the router middleware.
- Risks: Very small surface but touches URL encoding — a subtle change in what is stripped would alter user-facing URLs, so behavior must stay byte-identical. `simple` operates over all indices while `singleIndex` operates over one, so the shared piece must not assume shape.
- Verification: `yarn jest stateMappings`; `yarn jest createRouterMiddleware`; assert `configure` is stripped in both `stateToRoute` and `routeToState` for both adapters and that generated URLs are unchanged.

Before:

```mermaid
flowchart LR
Router[createRouterMiddleware] --> Simple[simpleStateMapping]
Router --> Single[singleIndexStateMapping]
Simple --> R1[own strip-configure rule]
Single --> R2[own strip-configure rule]
```

After:

```mermaid
flowchart LR
Router[createRouterMiddleware] --> Simple[simpleStateMapping]
Router --> Single[singleIndexStateMapping]
Simple --> Deep[route-sync filter seam]
Single --> Deep
Deep --> Rule[single strip-configure rule]
```

candidate-3: Hide connectorState lazy-init behind a stable-callbacks helper

- Recommendation strength: `Worth exploring`
- Files (each declares a `connectorState` object and lazily builds `refine`/`createURL`/`sendEvent` with `if (!connectorState.x)` guards inside `getWidgetRenderState`):
- `packages/instantsearch.js/src/connectors/pagination/connectPagination.ts`
- `packages/instantsearch.js/src/connectors/sort-by/connectSortBy.ts`
- `packages/instantsearch.js/src/connectors/numeric-menu/connectNumericMenu.ts`
- `packages/instantsearch.js/src/connectors/breadcrumb/connectBreadcrumb.ts`
- `packages/instantsearch.js/src/connectors/hits-per-page/connectHitsPerPage.ts`
- `packages/instantsearch.js/src/connectors/rating-menu/connectRatingMenu.ts`
- `packages/instantsearch.js/src/connectors/relevant-sort/connectRelevantSort.ts`
- (plus `clear-refinements`, `toggle-refinement`, `autocomplete`, `configure` referencing `connectorState`)
- Problem: `getWidgetRenderState` runs on every render, but the callbacks it exposes (`refine`, `createURL`, `sendEvent`) must be created *once* so downstream React/Vue wrappers keep stable references. Seven-plus connectors solve this identically by hand: declare a mutable `connectorState = {}` at factory scope, then guard each callback with `if (!connectorState.refine) { connectorState.refine = ... }`. Every connector author must know this caching invariant and re-implement the guard, and a missed guard silently causes reference churn / extra renders. The pattern is boilerplate lifecycle management, not per-connector logic.
- Proposed change: Provide a small helper that memoizes per-widget callbacks across renders, so a connector declares its callbacks once and receives stable references without writing manual `if (!connectorState.x)` guards. Connectors keep their own callback bodies; only the caching mechanics move behind the helper.
- Benefits: Locality — the "create once, reuse across renders" rule lives in one tested helper. Leverage — a new connector gets stable callbacks by calling one helper instead of re-learning the guard idiom. Testability — reference-stability is verified once on the helper rather than re-proven per connector.
- Risks: Connectors vary in *which* callbacks they cache and in closure capture (some capture `helper`, some `instantSearchInstance`), so the helper must not constrain the closure. Reference stability is load-bearing for the React/Vue wrappers, so the migration must be verified render-over-render. Best done incrementally (convert a few connectors first) rather than all at once.
- Verification: `yarn jest connectPagination connectSortBy connectNumericMenu connectBreadcrumb`; assert `refine`/`createURL`/`sendEvent` keep identical identity across successive `getWidgetRenderState` calls; run `yarn jest common-widgets` for the affected widgets.

Before:

```mermaid
flowchart LR
C1[connectPagination] --> G1[manual if-not-connectorState guards]
C2[connectSortBy] --> G2[manual if-not-connectorState guards]
C3[7+ connectors] --> G3[manual if-not-connectorState guards]
```

After:

```mermaid
flowchart LR
C1[connectPagination] --> Deep[stable-callbacks helper]
C2[connectSortBy] --> Deep
C3[7+ connectors] --> Deep
Deep --> Detail[create-once / reuse-across-renders]
```

candidate-4: Deepen the insights event dispatch seam

- Recommendation strength: `Worth exploring`
- Files:
- `packages/instantsearch.js/src/lib/InstantSearch.ts` (`public sendEventToInsights` L254, initialized `= noop` L401)
- `packages/instantsearch.js/src/middlewares/createInsightsMiddleware.ts` (rewires `instantSearchInstance.sendEventToInsights = ...` during `started()` L385; resets to `noop` on unuse L520)
- `packages/instantsearch.js/src/lib/utils/createSendEventForHits.ts` (222 lines building `InsightsEvent` payloads)
- `packages/instantsearch.js/src/lib/utils/createSendEventForFacet.ts` (67 lines building `InsightsEvent` payloads)
- Consumed by ~12 connectors (hits, infinite-hits, refinement-list, menu, hierarchical-menu, geo-search, autocomplete, related-products, trending-items, looking-similar, frequently-bought-together, chat)
- Problem: Insights events travel through a mutable public property, `instantSearchInstance.sendEventToInsights`, that begins life as `noop` and is swapped to a real implementation only when the insights middleware reaches its `started()` phase — and swapped back to `noop` on unuse. Connectors capture this property and fire hand-built `InsightsEvent` objects (via `createSendEventForHits`/`createSendEventForFacet`) into it, with no signal about whether the event is being handled, dropped pre-start, or dropped because insights isn't configured. The seam leaks lifecycle timing (must know it's `noop` until `started()`) and the full event shape to every caller. The `noop`↔real swap is an implicit ordering contract enforced only by `start()`'s call order.
- Proposed change: Turn the raw `sendEventToInsights` property into a deeper dispatch seam that owns the pre-start buffering / drop policy and the "is insights wired yet" state, so connectors dispatch through one stable entry point regardless of middleware timing. Keep the existing `InsightsEvent` payload shape for now; the goal is to hide the `noop`-then-rewire timing behind the seam, not to redesign the event vocabulary.
- Benefits: Locality — the "wired or not, before/after start" decision lives in one module instead of being implied by property reassignment. Leverage — connectors fire events without knowing the middleware lifecycle. Testability — dispatch-before-start and dispatch-when-disabled become directly testable at the seam instead of through full middleware start-up.
- Risks: Broader blast radius than candidates 1–3 (touches the public `InstantSearch` surface and every event-sending connector) and interacts with middleware start/unuse ordering, so it risks changing observable event timing; must preserve current no-op-before-start semantics exactly. Best kept internal — do not change the documented public property signature.
- Verification: `yarn jest createInsightsMiddleware createSendEventForHits createSendEventForFacet`; `yarn jest insights`; assert events fired before `start()` and while insights is disabled behave exactly as today; run the hits/refinement-list common suites.

Before:

```mermaid
flowchart LR
Conn[~12 connectors] --> Prop[sendEventToInsights property]
Prop --> Timing[noop until started, swapped by middleware]
Conn --> Shape[hand-built InsightsEvent payloads]
```

After:

```mermaid
flowchart LR
Conn[~12 connectors] --> Deep[insights dispatch seam]
Deep --> Timing[wired/not + pre-start policy hidden]
Deep --> MW[insights middleware]
```

candidate-5: Consolidate chat message-part introspection

- Recommendation strength: `Speculative`
- Files:
- `packages/instantsearch.js/src/connectors/chat/connectChat.ts` (suggestion extraction / part filtering ~L541-602, transport data-part filtering ~L672-679, stability warning L477)
- `packages/instantsearch.js/src/lib/ai-lite/abstract-chat.ts` (generic data-part handling in the streaming path)
- Problem: Understanding "which message parts are client-only `data-*` vs. sent to the server, and how to read a `data-suggestions` part" requires bouncing between the connector and `abstract-chat.ts`. The connector hand-implements part-scanning helpers (find the last assistant message, locate `data-suggestions`, derive status) directly over raw `UIMessage[]`, duplicating a parts-inspection idiom that isn't owned anywhere. Any caller needing a new data-part re-derives the scan.
- Proposed change: Give message-part introspection (filter by predicate, extract typed `data-*` parts, derive suggestion status) a single home the connector and streaming path both consume, so raw `parts` scanning stops being re-implemented. Deliberately no interface design here — the subsystem is young.
- Benefits: Locality — part-shape knowledge concentrates in one module. Leverage — new data-part consumers call one helper instead of re-learning the `parts` array shape. Testability — introspection gets unit tests independent of a live stream.
- Risks: **High.** Chat is explicitly `not yet stable and will change in the future` (`connectChat.ts:477`) and `ai-lite` is a deliberate fork tracking AI SDK 5 semantics; refactoring now risks churn against imminent upstream-shaped changes. Recommend deferring until the chat surface settles.
- Verification: `yarn jest connectChat`; `yarn jest common-widgets -t "Chat widget common tests"`; assert suggestion extraction and data-part filtering are byte-identical before/after.

Before:

```mermaid
flowchart LR
Conn[connectChat] --> Scan1[hand-rolled parts scan]
Chat[abstract-chat] --> Scan2[separate data-part handling]
```

After:

```mermaid
flowchart LR
Conn[connectChat] --> Deep[message-part introspection]
Chat[abstract-chat] --> Deep
Deep --> Detail[filter / extract data-* / suggestion status]
```

## Top Recommendation

Implement **candidate-1 (Deepen show-more facet state behind one module)** first. It is the deepest, most local win: three connectors (`connectRefinementList`, `connectMenu`, `connectHierarchicalMenu`) each hand-roll the *same* four-function toggle/limit machine plus a subtle callback-caching rule, so the interface a facet connector must learn is as large as the behavior it hides — the textbook shallow module. Consolidating it is high-leverage (every current and future facet-paging connector benefits), high-locality (the logic and its tests move to one place with no public render-state change), and scoped to a single reviewable PR touching three adjacent files with strong existing test coverage (`common-widgets` plus per-connector suites). Candidate-2 is an easy follow-up; candidate-4 is the highest-leverage but broadest and should come later.

## Next Step

To trigger implementation of 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` as listed above).

## Non-Candidates

- **Rewriting `abstract-chat.ts`'s ~758-line `processStream` into an event-emitting builder.** Real depth friction exists, but it is a large rewrite of an explicitly-unstable, upstream-shaped fork (`connectChat.ts:477`) — fails the "small, reviewable, non-speculative" bar. Captured narrowly and Speculative as candidate-5 instead.
- **Formalizing the `Router` interface into a state machine / shared base for `history.ts`.** `BrowserHistory` carries real complexity (debounce, popstate, `shouldWrite`), but a base-class abstraction is a speculative seam with only one production adapter — the rubric says don't introduce a seam unless something varies across it.
- **A generic "remove empty refinements from uiState" helper across 6 connectors.** Genuine small duplication, but each connector's empty-check differs enough that a shared helper risks over-generalizing for ~5–10 lines saved; lower leverage than candidates 1–3, so left out to keep the shortlist to the strongest five.
- **Extracting a `MiddlewareRegistry` to hide the `started` flag / middleware-array introspection in `InstantSearch.use()`.** Plausible depth improvement, but it reshapes core lifecycle wiring with broad blast radius and overlaps candidate-4's seam; too large/risky for a first PR and better revisited after candidate-4.
- **Any dependency upgrade, formatting, or cross-flavor markup churn** — out of scope per the run's constraints (module depth only).

This issue is a shortlist rather than one defined implementation. Start by reading the named connector and state-mapping files, then run the candidate-specific Jest commands to compare scope and behavior. Done means selecting one candidate, documenting its concrete boundary, preserving the public behavior described, and adding or updating the relevant tests.

Written by the indexing model from the issue text.