Architecture refactor scout: github-29726330621

cc @algolia/frontend-experiences-web

# Architecture Refactor Scout

Run: `github-29726330621`

## Summary

I inspected the `instantsearch.js` core package — where every connector, the InstantSearch runtime (`src/lib/InstantSearch.ts`, routing, state mappings), the Chat/AI subsystem, and the insights/event machinery live — plus the cross-flavor UI seam in `instantsearch-ui-components` and its React/Vue consumers. The dominant friction is **shallow, copy-pasted logic inside the connector layer**: the facet connectors each re-implement the same UI-state pruning, facet-type-conflict validation, and show-more caching dance inline, so the same normalization and ordering knowledge is duplicated 4–6 times with no owning module and no single test surface. A second cluster of friction is **callers knowing too much about ordering** (`InstantSearch` must call `refreshUiState()` before reading UI state) and **one over-wide connector** (`connectChat`, 847 lines) that wires transport construction, request rewriting, and Agent Studio URL building inline rather than behind a module. The candidates below prefer high locality and small interface improvements; each maps to one reviewable PR.

## Candidate Shortlist

candidate-1: Deepen index-UI-state pruning behind one module

- Recommendation strength: `Strong`
- Files:
- `packages/instantsearch.js/src/connectors/menu/connectMenu.ts:403-420`
- `packages/instantsearch.js/src/connectors/numeric-menu/connectNumericMenu.ts:488-505`
- `packages/instantsearch.js/src/connectors/refinement-list/connectRefinementList.ts` (local `removeEmptyRefinementsFromUiState`)
- `packages/instantsearch.js/src/connectors/hierarchical-menu/connectHierarchicalMenu.ts`
- `packages/instantsearch.js/src/connectors/rating-menu/connectRatingMenu.ts`
- `packages/instantsearch.js/src/connectors/breadcrumb/connectBreadcrumb.ts`
- new: `packages/instantsearch.js/src/lib/utils/` (owning module) + `__tests__`
- Problem: Six connectors each define a private `removeEmptyRefinementsFromUiState(indexUiState, attribute)` with the identical three-step shape — bail if the widget key is absent, delete the attribute when its value is "empty", delete the widget key when it becomes empty. The only thing that varies is the per-widget "empty" sentinel (`menu` uses `=== undefined`, `numericMenu` uses `=== ':'`, others an empty array/string). This is a shallow function copied per connector: the normalization rule for "an index UI-state slice is empty and should be pruned" has no owning module, so a bug fix or edge case must be applied in six places and there is no single place to test the invariant.
- Proposed change: Introduce one small utility in `src/lib/utils` that owns the prune-empty-refinement invariant, parameterized by the widget UI-state key and a per-widget emptiness predicate (the only genuine variation). Each connector calls it instead of defining its own copy; delete the six local functions.
- Benefits: Locality — the pruning rule lives in one file with one focused test, instead of being smeared across six connectors. Leverage — callers learn one tiny interface instead of re-deriving the pattern. Testability — the emptiness/normalization edge cases (the sentinels that differ per widget) become the natural test surface of one module rather than incidental coverage inside six connector suites.
- Risks: Low. Pure refactor, no behavior change intended. Must preserve each connector's exact emptiness predicate (the sentinels genuinely differ) — the migration is easy to get subtly wrong by unifying predicates that should stay distinct. Covered by existing connector `getWidgetUiState` tests + common connector tests.
- Verification: `yarn jest packages/instantsearch.js/src/connectors/{menu,numeric-menu,refinement-list,hierarchical-menu,rating-menu,breadcrumb}`; `yarn jest common-connectors`; add a focused unit test for the new util covering each sentinel.

Before:

```mermaid
flowchart LR
Menu[connectMenu] --> P1[removeEmpty copy]
Numeric[connectNumericMenu] --> P2[removeEmpty copy]
Refine[connectRefinementList] --> P3[removeEmpty copy]
Hier[connectHierarchicalMenu] --> P4[removeEmpty copy]
```

After:

```mermaid
flowchart LR
Menu[connectMenu] --> Deep[pruneIndexUiState]
Numeric[connectNumericMenu] --> Deep
Refine[connectRefinementList] --> Deep
Hier[connectHierarchicalMenu] --> Deep
Deep --> Rule[emptiness rule hidden]
```

candidate-2: Consolidate facet-type conflict validation

- Recommendation strength: `Worth exploring`
- Files:
- `packages/instantsearch.js/src/connectors/refinement-list/connectRefinementList.ts:496-517`
- `packages/instantsearch.js/src/connectors/menu/connectMenu.ts:352-363`
- `packages/instantsearch.js/src/connectors/hierarchical-menu/connectHierarchicalMenu.ts:441-464`
- `packages/instantsearch.js/src/connectors/toggle-refinement/connectToggleRefinement.ts:444-455`
- new owning module under `packages/instantsearch.js/src/lib/utils/`
- Problem: Inside `getWidgetSearchParameters`, several facet connectors each hand-roll the same guard: check whether `attribute` is already registered as a hierarchical / conjunctive / disjunctive facet by a different widget, emit the same-shaped warning, and return `searchParameters` unchanged on conflict. Each connector therefore has to know which facet types conflict, the warning wording, and the "return state untouched on conflict" contract. This is orchestration/validation knowledge leaked identically into every faceting connector; there is no module that owns "is it safe for this widget to claim this attribute as facet type X?".
- Proposed change: Extract a single validation helper that, given the search parameters, an attribute, the facet type this widget wants, and the widget name, reports whether a conflict exists and produces the standard warning. Connectors call it and branch on the result; the warning text and conflict matrix live in one place.
- Benefits: Leverage — connectors express intent ("I want this attribute as a disjunctive facet") instead of re-deriving the conflict matrix. Locality — warning wording and the set of conflicting types change in one file. Testability — the conflict matrix becomes a directly testable unit rather than being probed through four connector suites.
- Risks: Moderate. Warning messages are semi-public (developers assert on console output in tests); wording and `__DEV__` gating must be preserved byte-for-byte. Connectors differ slightly in what they log — the extraction must keep per-connector widget names and not over-unify.
- Verification: `yarn jest packages/instantsearch.js/src/connectors/{refinement-list,menu,hierarchical-menu,toggle-refinement}`; grep tests asserting the warning text still pass; add a unit test for the conflict matrix.

Before:

```mermaid
flowchart LR
Refine[connectRefinementList] --> V1[conflict check copy]
Menu[connectMenu] --> V2[conflict check copy]
Toggle[connectToggleRefinement] --> V3[conflict check copy]
```

After:

```mermaid
flowchart LR
Refine[connectRefinementList] --> Deep[assertFacetType]
Menu[connectMenu] --> Deep
Toggle[connectToggleRefinement] --> Deep
Deep --> Matrix[conflict matrix + warning hidden]
```

candidate-3: Fold refreshUiState() into the UI-state read

- Recommendation strength: `Worth exploring`
- Files:
- `packages/instantsearch.js/src/lib/InstantSearch.ts:843-891` (`setUiState`, `getUiState`)
- `packages/instantsearch.js/src/widgets/index/index.ts:1070-1079` (`refreshUiState`, `getWidgetUiState`)
- Problem: Reading UI state correctly requires the caller to know an ordering invariant: `InstantSearch.setUiState` and `getUiState` must call `this.mainIndex.refreshUiState()` *before* `this.mainIndex.getWidgetUiState({})`, because `getWidgetUiState` reads a `localUiState` cache that `refreshUiState()` rebuilds from the child widgets. This is exactly "callers know too much about ordering": the index module's interface exposes a stale cache plus a manual refresh step, and every reader must remember to sequence them (and `getUiState` even guards the refresh behind `this.started`). The seam leaks a cache-invalidation detail that belongs inside the module.
- Proposed change: Make the index UI-state read responsible for its own freshness so callers get correct state without sequencing a refresh — e.g. have the read recompute (or lazily invalidate) `localUiState` internally, and remove the explicit `refreshUiState()` calls at the two `InstantSearch` call sites. Keep any refresh entry point only if a genuine external trigger still needs it.
- Benefits: Depth — a real ordering invariant moves from the caller into the module; the interface becomes "ask for UI state, get correct UI state." Locality — cache-freshness logic concentrates in `index.ts`. Testability — correctness no longer depends on callers replaying a two-step ritual, so tests target the read directly.
- Risks: Medium. `refreshUiState()` may be deliberately separated for performance (avoid recomputing on every read) or called from other paths; must confirm callers and the render loop don't rely on the current explicit-refresh timing. Behavior-sensitive around `started`/SSR — routing and SSR tests must stay green.
- Verification: `yarn jest packages/instantsearch.js/src/lib/__tests__/routing`; `yarn jest packages/instantsearch.js/src/widgets/index`; run the `getUiState`/`setUiState` and server-side tests; confirm no other `refreshUiState()` caller regresses.

Before:

```mermaid
flowchart LR
IS[InstantSearch.getUiState] --> R[refreshUiState first]
IS --> G[getWidgetUiState]
G --> C[stale localUiState cache]
```

After:

```mermaid
flowchart LR
IS[InstantSearch.getUiState] --> Deep[getWidgetUiState]
Deep --> C[freshness handled inside]
```

candidate-4: Extract chat transport setup out of connectChat

- Recommendation strength: `Worth exploring`
- Files:
- `packages/instantsearch.js/src/connectors/chat/connectChat.ts` (transport block ~445-557, `agentId` handling ~359, options types ~133-160)
- `packages/instantsearch.js/src/lib/ai-lite/transport.ts` (`DefaultChatTransport`)
- Problem: `connectChat` (847 lines — the largest connector) inlines transport construction as one of several unrelated jobs. In the transport block it decides between a caller-supplied `transport` and a `DefaultChatTransport`, re-wraps the caller's `prepareSendMessagesRequest` to also strip `data-*` message parts (handling both sync and promise return paths), extracts credentials from `agentId`, and builds Agent Studio request URLs. The connector caller must understand that `agentId` and `transport` are mutually exclusive, that data-part filtering is layered *after* user preparation, and the sync/async branching of the wrapped preparer. This transport-wiring knowledge is a distinct concern with its own invariants that currently has no module.
- Proposed change: Move transport selection + `prepareSendMessagesRequest` wrapping + data-part filtering + Agent Studio URL/credential derivation into one module that takes the chat options and returns a ready transport. `connectChat` calls it once and stops knowing how a transport is assembled. Scope the PR to the transport seam only — leave streaming/tool orchestration alone.
- Benefits: Depth — a lot of setup behavior sits behind a small "build the transport for these options" interface. Locality — the `agentId`-vs-`transport` rules and data-part filtering live in one testable module instead of the connector body. It shrinks the most over-wide connector without touching its riskier streaming/tool paths.
- Risks: Medium. Chat is actively evolving (most recent commit touched it), so rebase risk is real; keep the PR narrow. Must preserve the exact sync/async handling of a caller-provided async `prepareSendMessagesRequest` and the mutual-exclusivity typing of `agentId`/`transport`.
- Verification: `yarn jest common-widgets -t "Chat widget common tests"`; `yarn jest packages/instantsearch.js/src/connectors/chat`; add unit tests for the transport module covering agentId path, custom-transport path, and data-part filtering with sync + async preparers.

Before:

```mermaid
flowchart LR
Caller[connectChat] --> T1[choose transport]
Caller --> T2[wrap prepareSendMessagesRequest]
Caller --> T3[filter data parts]
Caller --> T4[Agent Studio URL + creds]
```

After:

```mermaid
flowchart LR
Caller[connectChat] --> Deep[createChatTransport options]
Deep --> T2[request rewriting hidden]
Deep --> T4[agentId wiring hidden]
```

candidate-5: Deepen the show-more toggle state into a helper

- Recommendation strength: `Worth exploring`
- Files:
- `packages/instantsearch.js/src/connectors/refinement-list/connectRefinementList.ts:240-256`
- `packages/instantsearch.js/src/connectors/menu/connectMenu.ts` (show-more block ~185-198)
- `packages/instantsearch.js/src/connectors/hierarchical-menu/connectHierarchicalMenu.ts` (~214-235)
- `packages/instantsearch.js/src/connectors/rating-menu/connectRatingMenu.ts` (~220-230)
- Problem: Every connector that supports `showMore` re-implements the same stateful caching dance: a mutable `isShowingMore` flag, a stable `cachedToggleShowMore` wrapper delegating to a reassignable `toggleShowMore`, and a `createToggleShowMore(renderOptions, widget)` factory that flips the flag and re-renders. The reason for the indirection — the render function must hand the caller the *same* function reference across renders so it only binds once — is a non-obvious invariant re-encoded in four connectors. The knowledge "how to expose a stable toggle over mutable show-more state" has no owning module.
- Proposed change: Extract a small helper that owns the show-more state and returns a stable toggle plus the current `isShowingMore`, given a re-render callback. Each connector instantiates it once and reads/exposes its outputs, dropping the hand-rolled cache/flag/factory triad.
- Benefits: Depth — the stable-reference invariant (the actual subtle part) lives behind a tiny interface. Locality — a change to how show-more re-renders happens in one place. Testability — the "same reference across renders" guarantee becomes directly unit-testable rather than implicitly relied upon in four suites.
- Risks: Low–medium. Must preserve reference stability exactly (regressing it would force callers to re-bind and could break memoized flavor wrappers). Connectors differ slightly in defaults (`limit`/`showMoreLimit`); the helper must not swallow those differences.
- Verification: `yarn jest packages/instantsearch.js/src/connectors/{refinement-list,menu,hierarchical-menu,rating-menu}`; `yarn jest common-connectors`; add a unit test asserting the toggle keeps a stable identity across renders and flips state.

Before:

```mermaid
flowchart LR
Refine[connectRefinementList] --> S1[flag + cache + factory]
Menu[connectMenu] --> S2[flag + cache + factory]
Hier[connectHierarchicalMenu] --> S3[flag + cache + factory]
```

After:

```mermaid
flowchart LR
Refine[connectRefinementList] --> Deep[createShowMoreState]
Menu[connectMenu] --> Deep
Hier[connectHierarchicalMenu] --> Deep
Deep --> Inv[stable-reference invariant hidden]
```

## Top Recommendation

Implement **candidate-1** first. It has the highest locality (six adjacent connectors plus one new util), the smallest and lowest-risk diff (pure deduplication of an already-private function, no public interface change), and the clearest natural test surface (the per-widget emptiness sentinels become one module's unit tests instead of incidental connector coverage). It also establishes the "give a repeated connector rule an owning module" pattern that candidates 2 and 5 extend, making it a good sequencing anchor. If maximum depth-per-risk is preferred over safety, candidate-3 is the deepest single win (it removes an ordering invariant from callers) but it touches behavior-sensitive routing/SSR paths, so it is better as a follow-up once the safe dedup lands.

## 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

- **Splitting `AbstractChat.processStreamWithCallbacks` (~680-line stream state machine in `ai-lite/abstract-chat.ts`).** Genuinely the deepest friction in the repo, but it is not one reviewable PR — it entangles streaming, tool lifecycle, and partial-JSON accumulation, and chat is under active change. Too broad and too risky for this stage; candidate-4 carves off the safe, self-contained transport seam instead.
- **Sharing `SearchBox` markup into `instantsearch-ui-components`.** The same form/input/buttons/icons markup is duplicated across JS, React, and Vue, but CLAUDE.md documents that SearchBox layout is intentionally flavor-specific (IME/composition handling, refs, state). This conflicts with an existing decision and would be a large cross-flavor change, not a locality-preserving refactor.
- **Making the insights `sendEventToInsights` a real seam / breaking the `InsightsEvent` reverse import from `middlewares` into connectors and utils.** There is a real leak (connectors import a middleware type; the core keeps a `noop` stub the middleware overwrites at runtime), but untangling it touches middleware init, the automatic-insights path, and the event factory at once — closer to a broad rewrite than a small interface improvement. Left out to avoid inflating a speculative seam.
- **Introducing a generic `defer`/`debounce`-based `SchedulingManager` in `InstantSearch.ts`.** Consolidating `scheduleSearch`/`scheduleRender`/`scheduleStalledRender` is tempting, but the scheduling utils are already deep and the coordination is intertwined with status transitions and the render loop; extracting it risks a speculative abstraction with no varying adapter behind the seam.
- **Building a single `FacetRefinementConnectorFactory` that all faceting connectors sit behind.** This would fold candidates 1, 2, and 5 into one mega-module, but it is a broad rewrite of the connector layer with high review and migration risk. Prefer the incremental, independently-reviewable extractions above.

Choose one candidate first, then read the named connector or InstantSearch files and its existing tests; the shortlist provides the relevant entry points and verification commands. Done means one focused refactor with behavior preserved, a focused test for the extracted invariant where applicable, and the listed connector, routing, or widget tests passing.

Written by the indexing model from the issue text.