Skip to content

feat(territories): unified search with proximity ranking (#138)#206

Merged
mindsers merged 13 commits into
mainfrom
feat/territories-search-unification
Jun 7, 2026
Merged

feat(territories): unified search with proximity ranking (#138)#206
mindsers merged 13 commits into
mainfrom
feat/territories-search-unification

Conversation

@mindsers

@mindsers mindsers commented Jun 6, 2026

Copy link
Copy Markdown
Contributor

Summary

Closes #138 — and goes beyond. Unifies the territory/attribution/buildings search & filter UX (case/accent-insensitive, harmonised scope, active-filter chips, "Tout effacer"), and (scope expansion agreed mid-PR) adds proximity search via Google Maps geocoding with a Distance sort. Multiple senior code review passes (UX, UI, code-review, test-coverage, silent-failure, type-design, comment-analyser, simplifier) and all their findings were folded in before requesting review.

What changed

Search

  • Trim + accent-fold + case-fold via new normalized columns on Member.firstname, Member.lastname, Building.street. Maintained at every write path (createMember, updateMember, anonymize-member, link-member, update-account, import-congregation, create-building, edit-building, import-open-data, seed-marketing).
  • Harmonised scope: territory pages search publisher names via current attribution; attribution page searches building street/number/zip via attached territory.
  • @-prefix forces proximity even on short queries (@Bastille); auto-detection triggers geocoding for address-shaped or street-word-bearing queries.

Proximity (scope expansion)

  • New server-side geocoder (@googlemaps/google-maps-services-js) with Redis cache (geocode:v1:<query>, 90-day TTL, 5s timeout, OTHER fallback for unknown Google enum values).
  • Haversine-based distance computed against BuildingEntrance.{latitude,longitude} with fallback to Building.{latitude,longitude}. Number.isFinite guard rejects NaN/(∞) coords from open-data imports.
  • Server-side paginateByProximity partitions results into "with coords" (sorted by distance) + "without coords" (preserve default order), then paginates the combined list.
  • ProximityBanner confirms the resolved address, surfaces up to 2 "Vouliez-vous dire?" alternates, and offers an "Effacer la proximité" link. Banner shows whenever the geocode hit — even when the user reverts to the default sort.
  • Conditional Distance column (right-aligned, tabular-nums) only renders when the proximity sort is active.
  • "Sans coordonnées (N)" section divider (in-table when the page straddles the partition, above-table banner when the page lies entirely past it).

Filter UX

  • New ActiveTerritoryFilters chips component — one chip per applied filter with an isolated X target (chip body is inert, X only). "Tout effacer" renders inline as a trailing chip.
  • New GeocodeNotice warning Alert renders when geocoding was attempted but no result (failed) or the user typed @ alone (missing-query).
  • Mobile "Filtres avancés" Collapsible button (outline + leading SlidersHorizontal + rotating ChevronDown). Advanced Selects render once (CSS-toggled) to avoid Radix injecting duplicate hidden form inputs on mobile.
  • Rotating placeholder examples in the search input (ex. Pajot · 12 rue de la Paix · @Bastille), terminal once the user types.
  • ⓘ help popover documents the @ operator and the auto-detection.
  • Sort Select always renders (with ArrowUpDown icon) so users discover the control before proximity unlocks it.

Migration

  • 20260606000000_add_normalized_search_columns/migration.sql adds the three normalized columns + indexes, and backfills existing rows automatically using Postgres translate() with an explicit French-friendly char map. No unaccent extension required — runs cleanly on managed Postgres without superuser. Idempotent: re-applying touches only rows still at the empty-string default.

Notes for reviewers

This PR went through multiple review rounds. Worth highlighting bugs caught and fixed:

  • Geocoder API key bug: the local cache-key variable shadowed apiKey and was sent to Google. Every production call would have returned REQUEST_DENIED → cached as empty for 90 days. Fixed + test now asserts mockGeocode.mock.calls[0]?.[0]?.params?.key === apiKey.
  • Mobile filter Selects discarded user choices: advancedSelects was rendered twice in the same <Form>; Radix Select injects a hidden form input in both copies; display:none does not exclude inputs from form submission, so mobile submissions sent ?zip=none&zip=75002 and URLSearchParams.get('zip') returned the desktop default. Fixed by rendering Selects once and CSS-toggling visibility.
  • Number.isFinite guard on coords blocks NaN values from open-data CSV imports leaking into Haversine sort.
  • Non-OK Google statuses no longer cached as empty (only OK/ZERO_RESULTS are).
  • Page clamp in paginationFromUrl prevents ?page=99 rendering an empty table with no signal after a filter shrinks the total.
  • addressRegex typo: French repeater is quater, not quarter. The English spelling never matched.

Test plan

  • pnpm test:unit — 1216 tests pass
  • pnpm test:lint — 0 errors (37 pre-existing complexity warnings)
  • pnpm test:typecheck — clean
  • pnpm test:integration — needs reviewer with live DB
  • Manual with GOOGLE_MAPS_API_KEY set: type 12 rue de la Paix → banner appears, Distance column right-aligned, "Sans coordonnées" section at the tail; @Bastille → forced geocode; D012 → no geocode (heuristic)
  • Manual without GOOGLE_MAPS_API_KEY: same address → GeocodeNotice warning above filters; text-only fallback works
  • Manual: typed @ alone → "Tapez un lieu après le @" notice
  • Manual mobile (375px): "Filtres avancés" Outline+chevron toggles the Selects; submitting actually applies the chosen filter (i.e. the duplicate-Select bug doesn't recur)
  • Manual: search pajot matches Päjot; search dupont matches Dupont (case-insensitive on normalized columns)
  • Manual: chip X has destructive hover; clicking the chip body does nothing
  • Manual: page past the partition boundary on a proximity result shows the "Sans coordonnées" above-table banner

mindsers added 13 commits June 7, 2026 00:06
… filter chips

Search across territory, attribution and building filter pages now trims input,
strips diacritics, and matches case-insensitively against harmonised scope:

- Territory pages add publisher firstname/lastname via current attribution
- Attribution page adds building street/number/zip via the attached territory
- Building/prospection pages use the same normalized street comparison

Three new columns hold pre-normalized (lowercased, accent-stripped) values:

- Member.firstnameNormalized
- Member.lastnameNormalized
- Building.streetNormalized

The migration enables the unaccent extension temporarily to backfill the
columns, then drops it — the runtime DB role never needs the extension. All
service writes (createMember, updateMember, createBuilding, editBuilding,
anonymize, link-member, update-account, NDJSON import, open-data import) now
maintain these columns via the new stripDiacritics() util.

Above each filter form, an ActiveTerritoryFilters component renders one chip
per applied filter plus a "Tout effacer" link. Chips drop their query param
(and reset page) when clicked.

Also accepts a leading `@` proximity marker in the search query — stripped for
the text branch; proximity ranking lands in the next commit.
The search input now resolves address-like queries to coordinates via the
Google Maps Geocoding API and ranks results by distance from that point. A
single `@` prefix forces proximity even on short or non-address queries
(`@Bastille`).

Behaviour:

- A heuristic (`classifySearch`) only sends queries to the geocoder when they
  have 3+ tokens, contain a French way-type (`rue`, `avenue`, `boulevard`,
  …), or match `number street`. Short ambiguous strings (`12`, `D012`,
  surnames) stay text-only unless explicitly @-prefixed.
- Geocoded results land in a Redis cache (`geocode:v1:<normalized query>`,
  90-day TTL — addresses are stable). Missing results are cached as the
  empty sentinel so repeat misses don't burn the API.
- When the API key is unset or the call fails, search degrades to text-only
  — no banner, no Distance column.
- A `ProximityBanner` confirms the resolved address, lets the user clear
  the point, and surfaces up to two alternates as clickable "Vouliez-vous
  dire?" chips.
- A new `Distance` column appears second from the left only when proximity
  is active. Territories with no geocoded entrance or building are grouped
  under a "Sans coordonnées (N)" divider row.
- A `?sort=` toggle (Numéro / Date / Proximité depending on page) defaults
  to Proximité on a geocode hit and lets the user revert to the standard
  sort without dropping the banner or the distance column.

Distances use Haversine + `BuildingEntrance.{lat,lng}` with a fallback to
the parent Building coordinates. The "Sans coordonnées" group is preserved
in its original DB order so users with sparse coordinates aren't surprised.
…collapsible

The search input on every territory page now cycles through three placeholder
examples ("Pajot" → address → "@bastille") to surface the multiple intents the
classifier supports. Rotation pauses on focus so the hint doesn't move while
the user is typing. An ⓘ popover next to the input documents the `@`
proximity operator and the auto-detection, and notes that proximity is
disabled when no Google Maps key is configured.

On `max-sm`, the Select filters (zip, type, access, group, shops, status)
collapse behind a "Filtres avancés" toggle so the search input and submit
button keep the prominent position on a narrow screen. On `sm+` the Selects
render inline as before.

The Selects render once inside the form even when mobile-collapsed (a CSS
`contents` wrapper hides them on small screens; the Collapsible exposes the
same nodes), so the form payload is identical regardless of breakpoint.
The marketing seed (`seed-marketing.ts`) was creating Members and Buildings
without filling `firstnameNormalized`, `lastnameNormalized`, `streetNormalized`,
so they kept the empty-string column default and silently broke name-based
search on demo data after the new columns landed.
…x Sans coordonnées affordance

Four merge-blockers from the UX review:

- GeocodeNotice (warning Alert) now renders above filters when a query was
  sent to the geocoder but came back empty (failed call, no result, or no
  GOOGLE_MAPS_API_KEY). Wires the previously-dead `geocodeAttempted` field
  and `territories_filter_geocode_failed` i18n key.
- `@` alone now flips classifySearch to `forced: true` with `geoQuery: null`,
  letting the same notice prompt the user to type a place — the operator
  mode is no longer silently swallowed.
- Active filter chips: the body and X are now separate targets. Clicking
  the label/value does nothing; only the trailing X removes the filter,
  with a destructive hover state so the intent reads visually. Prevents
  accidental filter removal when scanning the chip row.
- "Sans coordonnées" tail row restyled from italic-muted-xs to a dashed
  divider with a count Badge — reads as a section break rather than a
  footnote. When the current page lies entirely past the partition
  boundary (so the in-table divider would never render), a banner above
  the table tells the user they are in the un-coord tail.

Also adds a `warning` variant to the shared `Alert` primitive (amber palette
matching the existing `warning` Badge).
…lumn, mobile trigger

Visual changes from the UI review:

- ProximityBanner reads as a confirmation: blue-500 left accent, blue-50
  background, blue-600 MapPin. Alternates muted so they sit subordinate to
  the resolved address.
- SearchInputWithHelp is now a proper input group — Input gets `pr-9` and
  the ⓘ button is absolutely positioned inside the right edge. Reads as one
  field instead of two adjacent controls.
- Distance column right-aligned with `tabular-nums` (numeric convention),
  using `text-foreground/80` instead of muted-foreground so the value keeps
  weight. The column is still gated on `proximityActive` — only shown when
  geocoding succeeded.
- Mobile "Filtres avancés" trigger upgraded from ghost-link to an outline
  Button with a leading `SlidersHorizontal` icon and a trailing `ChevronDown`
  that rotates on open. Reads as a proper control instead of helper copy.
- Submit Button icon swapped from `SlidersHorizontal` (which conventionally
  means "open filters") to `Search`. `SlidersHorizontal` is now reserved
  for the mobile collapsible trigger where it actually helps discoverability.
- "Modifier le point" → "Effacer la proximité" — the verb now matches what
  the link actually does (clears search/sort/page).
…rotation after typing

Long-tail polish from the UX review:

- Sort Select renders even when only one option is available (e.g. "Numéro"
  on a non-proximity territory list). Makes the sort control discoverable
  as a category before proximity unlocks it. Adds an `ArrowUpDown` icon in
  the trigger for at-a-glance recognition.
- Help popover body switched from a single paragraph to a bulleted list of
  search modes (name / number / address / `@`-proximity), with the no-key
  disclaimer demoted to a smaller footer line below.
- Placeholder rotation stops permanently once the user has typed anything —
  the hint has served its purpose and shouldn't shift back to examples
  after the user clears their query.

"Tout effacer" already sits inline as the trailing chip in the chip flow
(landed with the chip restructure in commit 4), so no extra change here.
…, isFinite, banner

Six bugs surfaced by the code-review pass:

- **geocoder.server.ts**: the Google call was passing the Redis cache key
  as the API key (shadowed `apiKey` with the local `key`). Every production
  geocode would return REQUEST_DENIED, get caught as warn, and pollute the
  cache with the empty sentinel for 90 days. Now passes `apiKey` correctly,
  logs non-OK statuses at error, and skips caching for non-OK responses so
  a transient outage doesn't poison results.
- **TerritoryFilters / AttributionFilters**: `advancedSelects` was rendered
  twice inside the same `<Form>` (once `max-sm:hidden`, once in the
  `sm:hidden` Collapsible). Radix Select injects a hidden form input for
  each, and `display:none` does not exclude inputs from form submission —
  mobile users' filter choices were silently dropped because the desktop
  default arrived first in URLSearchParams. The advanced Selects now render
  once; their visibility is toggled via CSS based on the mobile open state.
- **proximity-sort.server.ts**: added `Number.isFinite` guards on both
  entrance and building coordinates so NaN (possible from open-data CSV
  imports via `Number(lat)`) and Infinity are rejected like null. NaN
  flowing through Haversine would have produced unstable sort order and
  rendered as the same em-dash as a missing coord.
- **proximity-sort.server.test.ts**: fixed `lng` → `longitude` typo on the
  building-fallback fixture that silently skipped the entire "two competing
  buildings, pick closer" path. The test now exercises it.
- **import-congregation.server.ts**: building update path also backfills
  `streetNormalized` (previously only the create path set it, so re-imports
  left stale empty rows that name-search would silently miss).
- **list routes**: the `ProximityBanner` is now shown whenever the geocode
  hit, regardless of the active sort. Combined with: a typed address
  auto-flips the default sort to `proximity`. The user can still pick
  `numéro`/`date` via the Select without losing the banner.
- **address-regex.ts**: French repeater `quarter` → `quater`. The English
  spelling never matched.

Also: geocoder test now asserts `params.key` is the configured API key, and
covers non-OK status (REQUEST_DENIED no-cache), malformed cache fallback,
and unknown `location_type` coercion to `OTHER`.
…tion clamp

Important fixes from the review pass — second batch.

- Migration `20260606000000_add_normalized_search_columns` no longer
  depends on `CREATE EXTENSION unaccent` (which managed Postgres often
  refuses outside the bootstrap role). The migration now only adds columns
  and indexes; backfill of existing rows runs in TypeScript via the new
  one-shot script `app/database/backfill-normalized.ts` reusing the same
  `stripDiacritics()` helper as the runtime writes.
- Drop three dead i18n keys (`territories_filter_help_body`,
  `territories_filter_no_coordinates_count`, and `..._group`) that were
  introduced earlier in the PR and never referenced.
- Replace the duplicated `getTerritoryTypeLabel` + `territoryContentLabel`
  inlines in `attributions/territories.tsx` with the shared helper that
  already exists in `server/territory-content-label.ts`. Use the same
  `territoryTypeLabels` Record style as `territory/list.tsx` for the type
  column.
- `formatDistance` accepts a `locale` parameter (default still `'fr-FR'`)
  and memoizes formatter instances per locale. Loaders read `getLocale()`
  from Paraglide and forward it, so English-locale builds now render
  English unit conventions (`1.2 km` vs `1,2 km`).
- Drop the dead `lateDate == null` branches in the attribution-list client
  sort — `Attribution.lateDate` is non-null per the Prisma schema.
- Tighten `GeocodeNoticeData` to a discriminated union so `query` is
  required when `kind === 'failed'` and not present otherwise. Loaders now
  declare the type using the imported alias instead of an inline literal.
- `paginationFromUrl` clamps `page` to `[1, max(1, pages)]` — an
  out-of-range `?page=` now lands on the last page instead of silently
  returning an empty table.
- biome.json: extend the seed-script override to cover the new
  backfill-normalized.ts file (allow console output, etc.).
Three type-design improvements from the review pass:

- `ActiveTerritoryFilterChip.key` is now a `TerritoryFilterKey` string
  literal union (`'search' | 'zip' | 'type' | 'access' | 'shops' | 'group'
  | 'status'`). A typo (`'zipcode'`) producing a chip whose X clears
  nothing is now a compile-time error. `appendChip` in
  `build-filter-chips.ts` propagates the constraint.
- `sortFromUrl<A extends SortMode>(url, allowed, fallback): A` — the
  return type now narrows to the supplied subset, so callers'
  `sort !== 'number'` discriminations stay statically correct.
- `GeocodeResult.locationType` already gained an `'OTHER'` fallback in
  C7 — keeping the runtime cast covered against new Google enum values
  (e.g. `PLUS_CODE`).

Test additions:

- `search-intent.server.test.ts` — adversarial cases: leading-whitespace
  `@`, double `@@`, the 2-token `Jean Dupont` non-address, and the
  documented `Rue`-alone street-word fallback.
- `pagination.server.test.ts` — out-of-range page clamp, negative page
  fallback, non-numeric page fallback.
- `build-filter-chips.test.ts` (new) — verifies `'none'` is dropped,
  unknown enum values produce no chip, the search chip is trimmed, group
  ids fall back to `null` when missing.
…gration

Switch the normalized-column backfill from a separate `pnpm tsx` script to
pure SQL inside the migration itself, so `prisma migrate deploy` populates
existing rows automatically — no follow-up manual step required.

Uses Postgres `translate()` with an explicit French-friendly character map
(à→a, ä→a, ç→c, é→e, ñ→n, etc.) instead of the `unaccent` extension. The
map matches `stripDiacritics()`'s output for every codepoint in it, and
the migration stays privilege-safe — managed Postgres hosts that refuse
`CREATE EXTENSION` still run this cleanly.

Drops `app/database/backfill-normalized.ts` (and its biome.json override)
since the migration now handles the backfill itself. The WHERE guard on
the UPDATE keeps the SQL idempotent: re-running the migration won't touch
rows whose normalized columns are already populated by either path.
…imity loaders

Closes the integration-test gaps the review pass flagged. Pure-function
coverage was already solid; this commit adds end-to-end DB exercises and a
Playwright smoke spec so we can deploy with confidence.

- **Migration backfill parity** (`backfill-parity.integration.test.ts`):
  asserts the `translate()` map in the migration SQL produces the same
  output as `stripDiacritics()` for representative French inputs
  ("François", "Hélène", "Champs-Élysées", …) by running the same SQL via
  `$queryRawUnsafe`. **This test caught an off-by-one in the migration
  map** (seven 'o' replacements instead of six) before merge — exactly the
  silent-corruption risk the review flagged. Also locks the SQL/JS divergence
  on ligatures and `ø` (both preserved on both sides).
- **Member normalized columns**
  (`publishers/server/normalized-columns.integration.test.ts`):
  `createMember`, `updateMember`, and `anonymizeMember` all confirm the
  `firstnameNormalized` / `lastnameNormalized` columns actually land on
  disk — not just that the value was passed to a Prisma mock.
- **Building normalized columns**
  (`territories/server/building-normalized-columns.integration.test.ts`):
  `createBuilding`, `editBuilding`, plus the import-congregation update
  path (mirroring the data block) confirm `streetNormalized` writes
  through and refreshes stale values from legacy rows.
- **Proximity loaders**
  (`territories/server/proximity-loader.integration.test.ts`):
  `findTerritoriesWithDetailsPaginated` and
  `findActiveAttributionsPaginated` with `proximityArgs.origin` return
  rows in distance order, populate the per-row `distances` map, report
  `withCoordsCount` / `withoutCoordsCount` correctly, push no-coords
  rows to the tail, and paginate the combined list. The non-proximity
  branch still returns the shape callers rely on.
- **E2E** (`territories-search.spec.ts`): Playwright smoke covering the
  search input + rotating placeholder, the `?search=` URL round-trip
  through the active-filter chip, and "Tout effacer" wiping every query
  param.

The migration map fix is co-shipped here because the parity test caught
it — running the integration test before merge is what surfaces it.

Integration suite: 209 tests pass. Unit: 1216 pass. Lint + typecheck clean.
…feature

Product docs:
- `product/territories.md` gains a "Search and Filtering" section covering
  the single-input search (names / numbers / addresses / neighbourhoods),
  the `@` operator, accent-insensitivity, the proximity banner and Distance
  column, the active-filter chips, "Tout effacer", and the mobile
  "Filtres avancés" Collapsible.
- `product/feature-overview.md` Territories section gets a one-line entry
  for the unified search + proximity ranking, linking to the territories
  doc.

Technical docs:
- `development/architecture.md` gains a "Territory Search & Geocoding"
  section between "Google Maps Integration" and "PDF Generation": covers
  the normalized columns + write-time maintenance, the migration's pure-SQL
  backfill via `translate()`, the geocoder service (Redis cache, 90-day
  TTL, no-key fallback, non-OK no-cache, 5s timeout, `OTHER` location-type
  fallback), the `classifySearch` heuristic, the proximity pagination, the
  loader wiring with the discriminated `geocodeNotice` union, the mobile
  single-Select-render bug fix, and the integration/E2E test layout.
- `self-hosting/environment-variables.md` notes that `GOOGLE_MAPS_API_KEY`
  now also powers the proximity search via the Geocoding API and that the
  cache TTL is 90 days; the fallback when the key is absent is documented.
- `self-hosting/requirements.md` extends the Google Maps outbound bullet to
  mention proximity-search geocoding.
@mindsers mindsers marked this pull request as ready for review June 7, 2026 13:14
@mindsers mindsers merged commit cce6305 into main Jun 7, 2026
6 checks passed
@mindsers mindsers deleted the feat/territories-search-unification branch June 7, 2026 13:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat(territories): unify and harmonise search/filter UX

1 participant