diff --git a/.changeset/nearby-business-identity.md b/.changeset/nearby-business-identity.md new file mode 100644 index 000000000..81a3b0c41 --- /dev/null +++ b/.changeset/nearby-business-identity.md @@ -0,0 +1,5 @@ +--- +"@openmapx/core": patch +--- + +Preserve distinct nearby businesses and co-located tenants during autocomplete deduplication. Require corroborated address and source evidence for ordinary POIs while retaining shared-identity, city, station and landmark reconciliation. diff --git a/apps/web/src/components/search/SearchBar.test.tsx b/apps/web/src/components/search/SearchBar.test.tsx index e583840c4..b0a923679 100644 --- a/apps/web/src/components/search/SearchBar.test.tsx +++ b/apps/web/src/components/search/SearchBar.test.tsx @@ -5,6 +5,7 @@ import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import { getMapObstructionInsets, publishMapObstruction } from "@/lib/mapObstructions"; import { act, createFakeMap, createQueryWrapper, fireEvent, render, screen, waitFor } from "@/test"; import businessLocationFixture from "../../../../../packages/core/src/utils/__tests__/fixtures/business-location-intent.json"; +import distinctBusinesses from "../../../../../packages/core/src/utils/__tests__/fixtures/distinct-businesses.json"; vi.mock("next-intl", async () => (await import("@/test/intl")).mockNextIntl()); @@ -469,6 +470,60 @@ describe("SearchBar", () => { expect(useRecentSearchStore.getState().entries).toEqual(["Berlin Hbf"]); }); + it.each([ + ["click", "branch-a", "Street A 1", [13.4, 52.52]], + ["click", "branch-b", "Street B 2", [13.405, 52.52]], + ["keyboard", "branch-a", "Street A 1", [13.4, 52.52]], + ["keyboard", "branch-b", "Street B 2", [13.405, 52.52]], + ] satisfies Array<[string, string, string, [number, number]]>)( + "selects distinct %s business %s from the real list", + async (method, id, address, coordinates) => { + fakeMap.state.center = { lng: 13.4, lat: 52.52 }; + fakeMap.state.zoom = 15; + useAutocompleteMock.mockReturnValue({ data: distinctBusinesses, isFetching: false }); + useSearchStore.setState({ query: "REWE", isFocused: true }); + renderBar(); + const input = screen.getByLabelText("search.ariaLabel"); + const a = await screen.findByRole("option", { name: /Street A 1/ }); + const b = await screen.findByRole("option", { name: /Street B 2/ }); + expect(screen.getAllByRole("option").slice(0, 2)).toEqual([a, b]); + if (method === "click") fireEvent.click(id === "branch-a" ? a : b); + else { + fireEvent.keyDown(input, { key: "ArrowDown" }); + if (id === "branch-b") fireEvent.keyDown(input, { key: "ArrowDown" }); + fireEvent.keyDown(input, { key: "Enter" }); + } + expect(usePlaceStore.getState().selectedPlace).toMatchObject({ + ids: { maptiler: id }, + address: expect.stringContaining(address), + coordinates, + }); + expect(flyToMock).toHaveBeenCalledWith(coordinates, 15); + expect(useSidebarStore.getState().activeSidebarId).toBe(PANEL.PLACE); + }, + ); + + it("merges a corroborated cross-source branch duplicate while retaining the other branch", async () => { + fakeMap.state.center = { lng: 13.4, lat: 52.52 }; + fakeMap.state.zoom = 15; + useAutocompleteMock.mockReturnValue({ data: distinctBusinesses, isFetching: false }); + useSearchSuggestionsMock.mockReturnValue({ + data: aggregateResponse([ + aggregateSuggestion({ + ...distinctBusinesses[0], + id: "osm:node/1", + coordinates: [13.4, 52.52], + } as Partial), + ]), + isFetching: false, + }); + useSearchStore.setState({ query: "REWE", isFocused: true }); + renderBar(); + await screen.findByRole("option", { name: /Street B 2/ }); + expect(screen.getAllByRole("option", { name: /Street A 1/ })).toHaveLength(1); + expect(screen.getAllByRole("option", { name: /Street B 2/ })).toHaveLength(1); + }); + it("shows Berlin first for weak remote address-prefix evidence and plain Enter searches the area", async () => { fakeMap.state.center = { lng: 13.416, lat: 52.5194 }; fakeMap.state.zoom = 15; diff --git a/docs/docs/developer/architecture.md b/docs/docs/developer/architecture.md index c49ea17d4..a2d9d24ba 100644 --- a/docs/docs/developer/architecture.md +++ b/docs/docs/developer/architecture.md @@ -274,6 +274,15 @@ domain deliberately does not replace the geocoding fallback chain; `SearchBar` merges both responses after each has applied its own semantics. +The client merge in `packages/core/src/utils/searchSuggestion.ts` gives exact +result IDs and shared external identities precedence over spatial heuristics. +Ordinary POIs require independent source-record namespaces, equivalent concrete +address context and compatible category/entity evidence; same name and proximity +alone must not erase separate branches or tenants. Manifest `sourceIds` are +attribution only. City, transit and landmark reconciliation remain entity-specific. +The [search policy](../features/search.md#keeping-distinct-places-in-the-list) +documents the fallback's conservative address limits and source-identity rules. + The OSM alias index is an ODbL-derived data product owned by `data-manager`. The manager streams a selected PBF through Osmium into an `osm_search__staging` schema, validates it, and atomically swaps it into diff --git a/docs/docs/features/search.md b/docs/docs/features/search.md index 0ecf4c326..4b18444a5 100644 --- a/docs/docs/features/search.md +++ b/docs/docs/features/search.md @@ -56,6 +56,38 @@ differ from the server's candidate order without changing candidate retrieval. Coordinates and Plus Codes are detected client side and resolved without a round trip to a geocoder at all. +### Keeping distinct places in the list + +Nearby businesses with the same name remain separate choices, with their own +IDs, addresses and coordinates. Name and distance alone do not establish +business identity, including for co-located tenants or POIs without a category. +An identical result ID or a shared non-empty external identity still joins +records for the same entity, even when providers report different addresses. + +Without shared identity, ordinary POIs merge only when their normalized names +match, they are less than 1,000 metres apart, and different source-record +namespaces corroborate the same concrete address. Address comparison removes a +repeated business name and normalizes spelling such as `Friedrichstr.` / +`Friedrichstraße`, while retaining unit/floor and locality context. Missing, +city-only or postcode-only context is insufficient: the street component must +contain a house number and a recognized street term, such as `Street`, `Road`, +`Rue` or `-straße`. Conflicting Wikidata items +or reported categories prevent this heuristic merge. Different provider IDs or +OSM node/way IDs alone do not prove different real-world entities; manifest +`sourceIds` describe attribution, not place identity. + +This fallback is deliberately conservative, not an international address +parser: incomplete or differently formatted addresses and multiple records in +one source may remain as duplicate choices until a shared identity is supplied. +Stations and entrances retain their existing name/distance reconciliation, and +a station stays separate from its same-named square. Geographic features, +recognized landmarks and airport/notable-place catalogs retain spatial +reconciliation; a city from the notable-place index can still join its geocoder +record within 15 kilometres. A business category does not gain that exemption +from fame or a Wikidata item alone. Contradictory concrete addresses or Wikidata +entities also prevent landmark/catalog spatial merging; shared identity still +takes precedence. Commercial galleries require ordinary POI evidence. + ### Famous places far away A geocoder tells which of several namesakes is famous only if it has data on diff --git a/packages/core/src/utils/__tests__/fixtures/distinct-businesses.json b/packages/core/src/utils/__tests__/fixtures/distinct-businesses.json new file mode 100644 index 000000000..203efc27e --- /dev/null +++ b/packages/core/src/utils/__tests__/fixtures/distinct-businesses.json @@ -0,0 +1,18 @@ +[ + { + "id": "maptiler:branch-a", + "label": "REWE", + "sublabel": "REWE, Street A 1, Berlin", + "coordinates": [13.4, 52.52], + "type": "poi", + "rawCategory": "shop/supermarket" + }, + { + "id": "maptiler:branch-b", + "label": "REWE", + "sublabel": "REWE, Street B 2, Berlin", + "coordinates": [13.405, 52.52], + "type": "poi", + "rawCategory": "shop/supermarket" + } +] diff --git a/packages/core/src/utils/__tests__/search-eval/search-eval.test.ts b/packages/core/src/utils/__tests__/search-eval/search-eval.test.ts index f1a155253..563fa5b0c 100644 --- a/packages/core/src/utils/__tests__/search-eval/search-eval.test.ts +++ b/packages/core/src/utils/__tests__/search-eval/search-eval.test.ts @@ -108,6 +108,18 @@ const results = EVAL_CASES.map((evalCase) => { }); describe("search ranking eval", () => { + it("reconciles the captured Eiffel Tower geocoder and catalog rows once", () => { + const evalCase = EVAL_CASES.find((item) => item.id === "berlin-eiffel-tower"); + expect(evalCase).toBeDefined(); + if (!evalCase) return; + const rows = rankCase(evalCase); + const parisTower = rows.filter( + (row) => row.id === "osm:way/5013364" || row.id === "wikidata:Q243", + ); + expect(parisTower).toHaveLength(1); + expect(parisTower[0].ids?.wikidata).toBe("Q243"); + }); + for (const { evalCase, rows, ranks, action, passed } of results) { const title = `${evalCase.id}: “${evalCase.query}”`; const report = `ranks ${JSON.stringify(ranks)}; Enter: ${describeAction(action)}; ${describeRows(rows)}`; diff --git a/packages/core/src/utils/__tests__/searchSuggestion.test.ts b/packages/core/src/utils/__tests__/searchSuggestion.test.ts index 67abcd5d7..23bd04ed5 100644 --- a/packages/core/src/utils/__tests__/searchSuggestion.test.ts +++ b/packages/core/src/utils/__tests__/searchSuggestion.test.ts @@ -14,6 +14,168 @@ import { const BERLIN: [number, number] = [13.405, 52.52]; +describe("business duplicate evidence", () => { + const branch = (id: string, extra: Partial = {}): AutocompleteResult => ({ + id, + label: "REWE", + sublabel: "REWE, Friedrichstraße 1, Berlin", + coordinates: BERLIN, + type: "poi", + rawCategory: "shop/supermarket", + ...extra, + }); + const merge = (rows: AutocompleteResult[]) => + mergeAutocompleteSuggestions(rows, { query: "REWE" }); + + it.each([ + ["missing address", { sublabel: undefined }], + ["city-only context", { sublabel: "Berlin" }], + ["postcode-only context", { sublabel: "10115 Berlin" }], + ["short postcode and country", { sublabel: "1000 Brussels, Belgium" }], + ["locality before short postcode", { sublabel: "Brussels 1000, Belgium" }], + [ + "street without house number followed by postcode", + { sublabel: "Street A, 1000 Brussels, Belgium" }, + ], + ["missing category and address", { rawCategory: undefined, sublabel: undefined }], + ] satisfies Array<[string, Partial]>)("preserves %s", (_name, extra) => { + const rows = merge([branch("maptiler:a", extra), branch("osm:node/2", extra)]); + expect(rows.map((row) => row.id).sort()).toEqual(["maptiler:a", "osm:node/2"]); + }); + + it("preserves nearby contradictory addresses across sources", () => { + expect( + merge([ + branch("maptiler:a"), + branch("osm:node/2", { coordinates: [13.4, 52.52], sublabel: "REWE, Street B 2, Berlin" }), + ]), + ).toHaveLength(2); + }); + + it("preserves co-located same-source tenants at one address", () => { + expect(merge([branch("maptiler:a"), branch("maptiler:b")])).toHaveLength(2); + }); + + it("preserves co-located tenants with distinct unit addresses across sources", () => { + const rows = merge([ + branch("maptiler:a", { sublabel: "REWE, Friedrichstraße 1, Unit 1, Berlin" }), + branch("osm:node/2", { sublabel: "REWE, Friedrichstraße 1, Unit 2, Berlin" }), + ]); + expect(rows).toHaveLength(2); + }); + + it("preserves contradictory entity identities even with an equivalent address", () => { + const rows = merge([ + branch("maptiler:a", { ids: { wikidata: "Q100" } }), + branch("osm:node/2", { ids: { wikidata: "Q200" } }), + ]); + expect(rows).toHaveLength(2); + }); + + it("preserves contradictory primary Wikidata row identities without ids payloads", () => { + expect( + merge([ + branch("wikidata:Q100", { rawCategory: undefined, fame: 0.9, ids: { wikidata: "Q100" } }), + branch("wikidata:Q200", { rawCategory: undefined }), + ]), + ).toHaveLength(2); + }); + + it("preserves nearby commercial galleries without address evidence", () => { + expect( + merge([ + branch("maptiler:a", { rawCategory: "tourism/gallery", sublabel: "Berlin" }), + branch("osm:node/2", { rawCategory: "tourism/gallery", sublabel: "Berlin" }), + ]), + ).toHaveLength(2); + }); + + it("preserves different business categories at the same address", () => { + expect( + merge([branch("maptiler:a"), branch("osm:node/2", { rawCategory: "amenity/cafe" })]), + ).toHaveLength(2); + }); + + it("merges equivalent concrete addresses across sources despite different provider IDs", () => { + const rows = merge([ + branch("maptiler:a", { provider: "geocoding-maptiler", ids: { osm: "way/1" } }), + branch("osm:node/2", { + sublabel: "Friedrichstr. 1, Berlin", + provider: "geocoding-photon", + ids: { osm: "node/2" }, + sourceIds: ["photon"], + }), + ]); + expect(rows).toHaveLength(1); + expect(rows[0].contributingProviders).toEqual(["geocoding-maptiler", "geocoding-photon"]); + expect(rows[0].sourceIds).toEqual(["photon"]); + }); + + it("merges valid shared identity despite conflicting addresses and provider IDs", () => { + const rows = merge([ + branch("maptiler:a", { ids: { wikidata: "Q100" }, provider: "geocoding-maptiler" }), + branch("maptiler:b", { ids: { wikidata: "Q100" }, sublabel: "REWE, Street B 2, Berlin" }), + ]); + expect(rows).toHaveLength(1); + expect(rows[0].ids).toEqual({ wikidata: "Q100" }); + }); + + it("does not let business fame override contradictory addresses", () => { + expect( + merge([ + branch("maptiler:a", { fame: 0.9, ids: { wikidata: "Q100" } }), + branch("osm:node/2", { sublabel: "REWE, Friedrichstraße 2, Berlin" }), + ]), + ).toHaveLength(2); + }); + + it.each(["tourism/gallery", "tourism/museum", undefined])( + "does not let landmark or catalog context %s override contradictory addresses", + (rawCategory) => { + expect( + merge([ + branch("maptiler:a", { rawCategory, fame: 0.9, ids: { wikidata: "Q100" } }), + branch("osm:node/2", { rawCategory, sublabel: "REWE, Street B 2, Berlin" }), + ]), + ).toHaveLength(2); + }, + ); + + it.each(["tourism/gallery", "tourism/museum", undefined])( + "does not let landmark or catalog context %s override contradictory Wikidata identities", + (rawCategory) => { + expect( + merge([ + branch("maptiler:a", { rawCategory, fame: 0.9, ids: { wikidata: "Q100" } }), + branch("wikidata:Q200", { rawCategory, fame: 0.8, ids: { wikidata: "Q200" } }), + ]), + ).toHaveLength(2); + }, + ); + + it("preserves station and entrance reconciliation at the existing station radius", () => { + const rows = merge([ + branch("station:a", { label: "Berlin Hbf", rawCategory: "railway/station" }), + branch("osm:node/2", { + label: "Berlin Hbf", + rawCategory: "railway/subway_entrance", + coordinates: [13.41, 52.52], + }), + ]); + expect(rows).toHaveLength(1); + }); + + it("does not let a corroborated duplicate bridge two distinct addresses", () => { + const rows = merge([ + branch("maptiler:a"), + branch("osm:node/2", { sublabel: "Friedrichstr. 1, Berlin" }), + branch("maptiler:b", { sublabel: "REWE, Friedrichstraße 2, Berlin" }), + ]); + expect(rows.map((row) => row.id)).toEqual(["maptiler:a", "maptiler:b"]); + expect(rows[1].sublabel).toBe("REWE, Friedrichstraße 2, Berlin"); + }); +}); + describe("search suggestion primitives", () => { it("counts edits, a swap of neighbours as one, up to a limit", () => { expect(editDistance("neuschwanstien", "neuschwanstein", 2)).toBe(1); diff --git a/packages/core/src/utils/__tests__/suggestionRanking.test.ts b/packages/core/src/utils/__tests__/suggestionRanking.test.ts index 3960525a8..943c41033 100644 --- a/packages/core/src/utils/__tests__/suggestionRanking.test.ts +++ b/packages/core/src/utils/__tests__/suggestionRanking.test.ts @@ -10,6 +10,7 @@ import { presetSuggestionRows, rankAutocompleteRows, } from "../suggestionRanking"; +import distinctBusinesses from "./fixtures/distinct-businesses.json"; const BERLIN: [number, number] = [13.405, 52.52]; @@ -277,6 +278,21 @@ describe("rankAutocompleteRows", () => { ); expect(rows).toHaveLength(1); }); + + it("preserves both controlled nearby branches with their original identity and address", () => { + const rows = rankAutocompleteRows( + { places: distinctBusinesses as AutocompleteResult[] }, + { query: "REWE", proximity: [13.4, 52.52], zoom: 15 }, + ); + expect(rows).toMatchObject([ + { id: "maptiler:branch-a", sublabel: "REWE, Street A 1, Berlin", coordinates: [13.4, 52.52] }, + { + id: "maptiler:branch-b", + sublabel: "REWE, Street B 2, Berlin", + coordinates: [13.405, 52.52], + }, + ]); + }); }); describe("matchCategorySuggestions", () => { diff --git a/packages/core/src/utils/searchSuggestion.ts b/packages/core/src/utils/searchSuggestion.ts index 64a721042..2a434211e 100644 --- a/packages/core/src/utils/searchSuggestion.ts +++ b/packages/core/src/utils/searchSuggestion.ts @@ -9,10 +9,10 @@ const AIRPORT_RAW_CATEGORY = "aeroway/aerodrome"; const UNKNOWN_PROXIMITY_METERS = Number.MAX_SAFE_INTEGER; /** - * Two same-named suggestions closer than this are treated as one place. Wide + * Spatial bound for corroborated duplicates and non-business destinations. Wide * enough that a station record from a rail operator, the OSM station node and * a timetable stop (which can sit several hundred metres apart on a large - * station) collapse; the label check keeps distinct neighbours apart. + * station) collapse. Ordinary POIs also need entity evidence below. */ const SAME_PLACE_MAX_METERS = 1_000; /** @@ -643,6 +643,99 @@ function transitKind(item: AutocompleteResult): "transit" | "other" | "unknown" return item.type === "poi" ? "unknown" : "other"; } +function categoryKey(item: AutocompleteResult): string { + return item.rawCategory?.trim().toLowerCase().replace(/ +/g, "/") ?? ""; +} + +/** Named geographic features and landmark catalogs retain their spatial reconciliation. */ +function isLandmarkCategory(category: string): boolean { + return ( + /^(historic|natural|place|boundary)\//u.test(category) || + /^(aeroway\/aerodrome|man_made\/tower|tourism\/(museum|attraction|artwork))$/u.test(category) + ); +} + +function needsEntityEvidence(a: AutocompleteResult, b: AutocompleteResult): boolean { + if (transitKind(a) === "transit" || transitKind(b) === "transit") return false; + if (a.type !== "poi" && b.type !== "poi") return false; + // Unknown POIs may be businesses. A known shop/hotel/etc. never inherits + // the catalog exemption from the other row's fame or Wikidata item. + const categories = [categoryKey(a), categoryKey(b)].filter(Boolean); + if (categories.some((category) => !isLandmarkCategory(category))) return true; + const catalogued = [a, b].some( + (item) => + Boolean(item.ids?.icao || item.ids?.iata) || + (Boolean(item.ids?.wikidata) && (item.fame ?? 0) > 0), + ); + return categories.length === 0 && !catalogued; +} + +/** ID namespaces describe source records; manifest sourceIds only describe attribution. */ +function sourceDomain(item: AutocompleteResult): string | undefined { + const separator = item.id.indexOf(":"); + return separator > 0 ? item.id.slice(0, separator) : item.provider; +} + +/** + * Conservative address agreement, without guessing international address structure. + * Drop a repeated business name; retain unit/floor, street, number and locality. + * City/postcode-only context is insufficient. Missing or differently formatted + * addresses need an explicit shared identity rather than a spatial guess. + */ +function concreteAddressKey(item: AutocompleteResult): string | undefined { + let address = matchKey(item.sublabel ?? ""); + const label = matchKey(item.label); + if (label && address.startsWith(`${label} `)) address = address.slice(label.length + 1); + const tokens = words(address); + const parts = (item.sublabel ?? "").split(",").map(matchKey); + if (parts[0] === label) parts.shift(); + let street = parts[0] ?? ""; + if (label && street.startsWith(`${label} `)) street = street.slice(label.length + 1); + const streetTokens = words(street); + // A postcode in a later locality component is not a house number. + if (tokens.length < 3 || !streetTokens.some((token) => /^\d{1,4}\p{L}?$/u.test(token))) + return undefined; + // A short postcode also looks like a house number. Require a recognized + // street term rather than interpreting arbitrary locality text as a street. + // This deliberately leaves unsupported address formats to shared identity. + if ( + !streetTokens.some((token) => + /(?:strasse|street|road|avenue|lane|drive|boulevard|allee|platz|gasse|weg)$/u.test(token), + ) && + !streetTokens.some((token) => /^(rue|route|via|calle)$/u.test(token)) + ) + return undefined; + return address; +} + +function hasContradictoryEntityEvidence(a: AutocompleteResult, b: AutocompleteResult): boolean { + // Apply contradictions before landmark/catalog spatial reconciliation too. + // Primary Wikidata row IDs are entity identities even without an ids payload. + const wikidata = (item: AutocompleteResult) => + item.ids?.wikidata ?? (item.id.startsWith("wikidata:") ? item.id.slice(9) : undefined); + const aEntity = wikidata(a); + const bEntity = wikidata(b); + if (aEntity && bEntity && aEntity !== bEntity) return true; + const aAddress = concreteAddressKey(a); + const bAddress = concreteAddressKey(b); + return aAddress !== undefined && bAddress !== undefined && aAddress !== bAddress; +} + +function hasCorroboratedEntityLocation(a: AutocompleteResult, b: AutocompleteResult): boolean { + // Two records in one source can be distinct tenants at exactly the same + // address. Different record IDs do not prove distinct entities, but also + // do not independently corroborate a heuristic merge. Shared IDs win above. + const aSource = sourceDomain(a); + const bSource = sourceDomain(b); + if (!aSource || !bSource || aSource === bSource) return false; + // Different OSM nodes/ways can represent one entity, so inequality is not a veto. + const aCategory = categoryKey(a); + const bCategory = categoryKey(b); + if (aCategory && bCategory && aCategory !== bCategory) return false; + const address = concreteAddressKey(a); + return address !== undefined && address === concreteAddressKey(b); +} + function hasSameCanonicalLocation(a: AutocompleteResult, b: AutocompleteResult): boolean { if (!a.coordinates || !b.coordinates) return false; if (normalizeSearchTerm(a.label) !== normalizeSearchTerm(b.label)) return false; @@ -650,6 +743,13 @@ function hasSameCanonicalLocation(a: AutocompleteResult, b: AutocompleteResult): // from the square; merging them hid the station for "alexanderplatz". const kinds = new Set([transitKind(a), transitKind(b)]); if (kinds.has("transit") && kinds.has("other")) return false; + if ( + !kinds.has("transit") && + (a.type === "poi" || b.type === "poi") && + hasContradictoryEntityEvidence(a, b) + ) + return false; + if (needsEntityEvidence(a, b) && !hasCorroboratedEntityLocation(a, b)) return false; const maxMeters = isCityFromTwoSources(a, b) ? SAME_CITY_MAX_METERS : SAME_PLACE_MAX_METERS; return haversineDistance(a.coordinates, b.coordinates) < maxMeters; }