mirror of
https://github.com/mauriceboe/TREK.git
synced 2026-10-18 21:16:47 +00:00
fix(maps): category pills ask the index by Overture path, near misses are left out
The index matches a term anywhere inside a place's category text, so the bare names behind the pills caught whatever was spelled like them: "park" brought car parks and an amusement park, "garden" furniture stores and beer gardens, "bar" barbers, "pub" publishers and public squares, "market" marketing agencies. In Vienna four of sixty "Nature & parks" pins were parks. Every pill now names its Overture categories by full path, a place counts only when its path is that path or lies under it, and a page of near misses only falls through to Overpass. Terms Overture no longer has (nightclub, nature_preserve, shopping_center ...) got their current names. Closes #2663
This commit is contained in:
@@ -9,6 +9,7 @@ import { buildOsmDetails, parsePoiCategories, samePlace, toApiLang } from './map
|
||||
import { POI_RESULT_CAP, type PoiSearchResult } from './providers/osm.client';
|
||||
import {
|
||||
POI_CATEGORY_TO_TREK,
|
||||
indexRowTerm,
|
||||
isOsmHit,
|
||||
osmPlaceId,
|
||||
toPlaceRecord,
|
||||
@@ -55,22 +56,15 @@ export function indexPoiPlan(category: string, bbox: PoiBbox, limit: number): In
|
||||
// by Overpass, and a mixed answer would silently drop it.
|
||||
const indexKnowsAll = wanted.length > 0 && wanted.every((key) => POI_CATEGORY_TO_TREK[key]?.length);
|
||||
if (!indexKnowsAll) return null;
|
||||
// The index matches a term as a SUBSTRING of `category` and `category_path`
|
||||
// (see POI_CATEGORY_TO_TREK), so an exact lookup misses every leaf that is
|
||||
// not literally a term: `italian_restaurant` is a hit for `restaurant` and
|
||||
// finds nothing here. Falling through to wanted[0] then labelled it with
|
||||
// whichever pill the user happened to tap first, and the corridor panel
|
||||
// groups and colours on exactly that field, so a trattoria came back as a
|
||||
// petrol station. Longest match wins, because `fast_food` must not lose to
|
||||
// `food` when both are terms of different categories.
|
||||
// A row belongs to the term whose path it lies under, not to one it is merely
|
||||
// spelled like: the leaf alone is rarely a term (`italian_restaurant` sits
|
||||
// under `food_and_drink>restaurant`), and falling through to wanted[0] then
|
||||
// labelled it with whichever pill the user tapped first, so a trattoria came
|
||||
// back as a petrol station. The most specific term wins, so a water park
|
||||
// answers the activity pill rather than the park it sits under.
|
||||
const labelFor = (leaf: string | null, path: string | null): string | undefined => {
|
||||
const haystack = `${leaf ?? ''} ${path ?? ''}`;
|
||||
let best: string | undefined;
|
||||
for (const term of categoryOfTerm.keys()) {
|
||||
if (!haystack.includes(term)) continue;
|
||||
if (best === undefined || term.length > best.length) best = term;
|
||||
}
|
||||
return best === undefined ? undefined : categoryOfTerm.get(best);
|
||||
const term = indexRowTerm(categoryOfTerm.keys(), leaf, path);
|
||||
return term === undefined ? undefined : categoryOfTerm.get(term);
|
||||
};
|
||||
const lat = (bbox.south + bbox.north) / 2;
|
||||
const lng = (bbox.west + bbox.east) / 2;
|
||||
@@ -109,7 +103,7 @@ export function indexPoiAnswer(
|
||||
): PoiSearchResult {
|
||||
const { wanted, labelFor, cap, radius, reach } = plan;
|
||||
return {
|
||||
pois: found.map((p) => ({
|
||||
pois: belonging(found, plan).map((p) => ({
|
||||
osm_id: `gers:${p.gers}`,
|
||||
name: p.name,
|
||||
lat: p.lat,
|
||||
@@ -138,6 +132,8 @@ export function indexPoiAnswer(
|
||||
source: 'trek-places' as const,
|
||||
})),
|
||||
source: 'trek-places' as const,
|
||||
// The page the index sent, not what was left of it: a full page means more
|
||||
// places, however many of them this one had to leave out.
|
||||
truncated: found.length >= cap,
|
||||
// A wide viewport is narrowed here too, and the caller is told so
|
||||
// for the same reason the Overpass path tells it.
|
||||
@@ -145,6 +141,18 @@ export function indexPoiAnswer(
|
||||
};
|
||||
}
|
||||
|
||||
/**
|
||||
* The rows that belong to a pill the request asked for. The index matches its
|
||||
* terms inside the category text, so a near miss can still come back, and it
|
||||
* is left out here (#2663). A row the index holds without any category cannot
|
||||
* be told either way and stays, as it always has.
|
||||
*/
|
||||
export function belonging<T extends TrekPlace & { categoryPath?: string | null }>(found: T[], plan: IndexPoiPlan): T[] {
|
||||
return found.filter(
|
||||
(p) => (!p.category && !p.categoryPath) || plan.labelFor(p.category ?? null, p.categoryPath ?? null) !== undefined,
|
||||
);
|
||||
}
|
||||
|
||||
const regionNamesByLang = new Map<string, Intl.DisplayNames | null>();
|
||||
|
||||
/**
|
||||
|
||||
@@ -4,7 +4,7 @@ import type { AppSettingsRepository } from '../../db/repositories/AppSettings.re
|
||||
import type { GeoLane } from '../geo/nominatim.client';
|
||||
import { PlacePhotoCacheService } from '../place-photos/place-photo-cache.service';
|
||||
import type { ApiKeySource } from '../settings/instance-api-keys';
|
||||
import { indexPoiAnswer, indexPoiPlan, indexSuggestions } from './maps-index.helpers';
|
||||
import { belonging, indexPoiAnswer, indexPoiPlan, indexSuggestions } from './maps-index.helpers';
|
||||
import { NEARBY_DEFAULT_LIMIT, NEARBY_DEFAULT_RADIUS_M, nearbyCacheKey, nearestFirst } from './maps-nearby.helpers';
|
||||
import { MapsUrlResolver, type ResolvedMapsUrl } from './maps-url.resolver';
|
||||
import { toApiLang, mergeSearchResults } from './maps.helpers';
|
||||
@@ -214,7 +214,8 @@ export class MapsService {
|
||||
limit: plan.cap,
|
||||
category: plan.terms.join(','),
|
||||
});
|
||||
if (found.length > 0) return indexPoiAnswer(found, plan);
|
||||
// A page of near misses only is a miss too, not an empty answer.
|
||||
if (belonging(found, plan).length > 0) return indexPoiAnswer(found, plan);
|
||||
} catch (err: unknown) {
|
||||
console.warn('TREK Places nearby failed, falling back:', (err as Error).message);
|
||||
}
|
||||
|
||||
@@ -345,31 +345,70 @@ export function indexHitsOnly(hits: TrekSearchHit[]): TrekPlace[] {
|
||||
*
|
||||
* Deliberately a hand-written map rather than a guess at runtime: the two
|
||||
* taxonomies do not line up, and a pill that quietly returns the wrong kind of
|
||||
* place is worse than one that returns none. Each entry is a substring matched
|
||||
* against `category` and `category_path`, so `eat_and_drink>cafe` answers the
|
||||
* cafe pill without listing every leaf.
|
||||
* place is worse than one that returns none. Each entry is a category path, and
|
||||
* a place belongs to it when its own path is that path or lies under it, so
|
||||
* `food_and_drink>restaurant` answers the italian restaurant two levels down.
|
||||
*
|
||||
* Paths, not bare leaf names: the index matches a term anywhere inside
|
||||
* `category` and `category_path`, and a bare `park` brought car parks, `garden`
|
||||
* furniture stores and `bar` barbers (#2663). A full path does not occur inside
|
||||
* an unrelated one, and {@link indexRowTerm} drops whatever still slips through.
|
||||
* Overture renames a branch now and then; a path that stops existing answers
|
||||
* nothing, and the pill falls back to Overpass instead of answering wrong.
|
||||
*/
|
||||
export const POI_CATEGORY_TO_TREK: Record<string, string[]> = {
|
||||
restaurant: ['restaurant', 'casual_eatery', 'fast_food'],
|
||||
cafe: ['cafe', 'coffee_shop'],
|
||||
bar: ['bar', 'pub', 'nightclub', 'brewery'],
|
||||
hotel: ['hotel', 'lodging', 'hostel', 'motel', 'bed_and_breakfast'],
|
||||
sights: ['historic_site', 'landmark_and_historical_building', 'monument', 'castle', 'tourist_attraction'],
|
||||
museum: ['museum', 'art_gallery', 'performing_arts', 'theater'],
|
||||
nature: ['park', 'garden', 'beach', 'nature_preserve', 'mountain'],
|
||||
activity: ['amusement_park', 'zoo', 'aquarium', 'water_park', 'theme_park'],
|
||||
shopping: ['shopping', 'shopping_center', 'department_store', 'market'],
|
||||
supermarket: ['grocery_store', 'food_and_beverage_store', 'convenience_store', 'supermarket'],
|
||||
restaurant: ['food_and_drink>restaurant', 'food_and_drink>casual_eatery'],
|
||||
cafe: ['food_and_drink>casual_eatery>cafe', 'food_and_drink>non_alcoholic_beverage_venue>coffee_shop'],
|
||||
bar: ['food_and_drink>alcoholic_beverage_venue', 'arts_and_entertainment>nightlife_venue'],
|
||||
hotel: ['lodging'],
|
||||
sights: ['cultural_and_historic>historic_site', 'cultural_and_historic>memorial_site>monument'],
|
||||
museum: [
|
||||
'arts_and_entertainment>museum',
|
||||
'arts_and_entertainment>arts_and_crafts_space>art_gallery',
|
||||
'arts_and_entertainment>performing_arts_venue',
|
||||
'arts_and_entertainment>movie_theater',
|
||||
],
|
||||
nature: [
|
||||
'sports_and_recreation>park',
|
||||
'geographic_entities>built_feature>garden',
|
||||
'geographic_entities>land_feature>beach',
|
||||
'geographic_entities>land_feature>nature_reserve',
|
||||
'geographic_entities>land_feature>mountain',
|
||||
],
|
||||
activity: [
|
||||
'arts_and_entertainment>amusement_attraction>amusement_park',
|
||||
'arts_and_entertainment>animal_attraction>zoo',
|
||||
'arts_and_entertainment>animal_attraction>aquarium',
|
||||
'sports_and_recreation>park>water_park',
|
||||
],
|
||||
shopping: ['shopping'],
|
||||
supermarket: ['shopping>food_and_beverage_store', 'shopping>convenience_store'],
|
||||
// What a drive needs rather than what a city visit does (#1797). The corridor
|
||||
// search asks for these by name, and without them every corridor query fell
|
||||
// through to Overpass — the one path where that hurts most, because a single
|
||||
// search fans out over sixteen boxes.
|
||||
fuel: ['gas_station', 'fueling_station'],
|
||||
charging: ['ev_charging_station'],
|
||||
rest_area: ['rest_stop'],
|
||||
campsite: ['campground', 'rv_park'],
|
||||
fuel: ['travel_and_transportation>fueling_station>gas_station'],
|
||||
charging: ['travel_and_transportation>fueling_station>ev_charging_station'],
|
||||
rest_area: ['travel_and_transportation>ground_transport_facility_or_service>road_structures_and_service>rest_stop'],
|
||||
campsite: ['lodging>campground', 'lodging>rv_park'],
|
||||
};
|
||||
|
||||
/**
|
||||
* The term of {@link POI_CATEGORY_TO_TREK} a row belongs to, the most specific
|
||||
* one when several fit, or undefined when none does. A row is judged by its path
|
||||
* where it has one and by its leaf where the index left the path empty.
|
||||
*/
|
||||
export function indexRowTerm(terms: Iterable<string>, leaf: string | null, path: string | null): string | undefined {
|
||||
let best: string | undefined;
|
||||
for (const term of terms) {
|
||||
const fits = path
|
||||
? path === term || path.startsWith(`${term}>`)
|
||||
: !!leaf && leaf === term.slice(term.lastIndexOf('>') + 1);
|
||||
if (fits && (best === undefined || term.length > best.length)) best = term;
|
||||
}
|
||||
return best;
|
||||
}
|
||||
|
||||
export interface TrekNearbyPlace extends TrekPlace {
|
||||
distanceMetres: number;
|
||||
}
|
||||
|
||||
@@ -49,7 +49,7 @@ const FULL = {
|
||||
lat: 48.2101,
|
||||
lng: 16.3657,
|
||||
category: 'coffee_shop',
|
||||
categoryPath: 'eat_and_drink>cafe>coffee_shop',
|
||||
categoryPath: 'food_and_drink>non_alcoholic_beverage_venue>coffee_shop',
|
||||
confidence: 0.94,
|
||||
address: { freeform: 'Herrengasse 14', locality: 'Wien', postcode: '1010', region: null, country: 'AT' },
|
||||
contact: { website: 'https://cafecentral.wien/', phone: '+43 1 5333763', email: null, socials: null },
|
||||
@@ -285,7 +285,7 @@ describe('MapsService.pois answered from the index', () => {
|
||||
radius: 7872,
|
||||
// The caller's budget, per category, the way the Overpass path spends it.
|
||||
limit: 60,
|
||||
category: 'cafe,coffee_shop',
|
||||
category: 'food_and_drink>casual_eatery>cafe,food_and_drink>non_alcoholic_beverage_venue>coffee_shop',
|
||||
});
|
||||
expect(out.clamped).toBe(false);
|
||||
});
|
||||
@@ -296,8 +296,20 @@ describe('MapsService.pois answered from the index', () => {
|
||||
// Overpass — on the one path where that hurts most, because a single search
|
||||
// fans out over sixteen boxes and each races four public mirrors.
|
||||
mockNearby.mockResolvedValue([
|
||||
{ ...FULL, gers: 'f-1', name: 'Aral', category: 'gas_station' },
|
||||
{ ...FULL, gers: 'c-1', name: 'Ionity', category: 'ev_charging_station' },
|
||||
{
|
||||
...FULL,
|
||||
gers: 'f-1',
|
||||
name: 'Aral',
|
||||
category: 'gas_station',
|
||||
categoryPath: 'travel_and_transportation>fueling_station>gas_station',
|
||||
},
|
||||
{
|
||||
...FULL,
|
||||
gers: 'c-1',
|
||||
name: 'Ionity',
|
||||
category: 'ev_charging_station',
|
||||
categoryPath: 'travel_and_transportation>fueling_station>ev_charging_station',
|
||||
},
|
||||
]);
|
||||
const svc = make();
|
||||
const overpass = stubOverpass(svc);
|
||||
@@ -310,7 +322,8 @@ describe('MapsService.pois answered from the index', () => {
|
||||
0,
|
||||
0,
|
||||
expect.objectContaining({
|
||||
category: 'gas_station,fueling_station,ev_charging_station',
|
||||
category:
|
||||
'travel_and_transportation>fueling_station>gas_station,travel_and_transportation>fueling_station>ev_charging_station',
|
||||
// Per category, so a mixed search does not spend the whole allowance on
|
||||
// whichever kind happens to be densest.
|
||||
limit: 120,
|
||||
@@ -333,9 +346,15 @@ describe('MapsService.pois answered from the index', () => {
|
||||
gers: 'r-1',
|
||||
name: 'Trattoria',
|
||||
category: 'italian_restaurant',
|
||||
categoryPath: 'eat_and_drink>restaurant>italian_restaurant',
|
||||
categoryPath: 'food_and_drink>restaurant>european_restaurant>italian_restaurant',
|
||||
},
|
||||
{
|
||||
...FULL,
|
||||
gers: 'f-1',
|
||||
name: 'Aral',
|
||||
category: 'gas_station',
|
||||
categoryPath: 'travel_and_transportation>fueling_station>gas_station',
|
||||
},
|
||||
{ ...FULL, gers: 'f-1', name: 'Aral', category: 'gas_station', categoryPath: 'automotive>gas_station' },
|
||||
]);
|
||||
const svc = make();
|
||||
stubOverpass(svc);
|
||||
@@ -352,22 +371,66 @@ describe('MapsService.pois answered from the index', () => {
|
||||
expect(out.pois[0].poi_type).toBe('italian_restaurant');
|
||||
});
|
||||
|
||||
it('MAPS-POIS-013c: the longest matching term wins, so fast_food does not answer as a cafe', async () => {
|
||||
it('MAPS-POIS-013c: the most specific term wins, so a cafe under casual_eatery answers the cafe pill', async () => {
|
||||
mockNearby.mockResolvedValue([
|
||||
{
|
||||
...FULL,
|
||||
gers: 'q-1',
|
||||
name: 'Imbiss',
|
||||
category: 'fast_food_restaurant',
|
||||
categoryPath: 'eat_and_drink>fast_food>fast_food_restaurant',
|
||||
categoryPath: 'food_and_drink>casual_eatery>fast_food_restaurant',
|
||||
},
|
||||
{ ...FULL, gers: 'q-2', name: 'Kaffeehaus', category: 'cafe', categoryPath: 'food_and_drink>casual_eatery>cafe' },
|
||||
]);
|
||||
const svc = make();
|
||||
stubOverpass(svc);
|
||||
|
||||
const out = await svc.pois('cafe,restaurant', BOX);
|
||||
|
||||
expect(out.pois[0].category).toBe('restaurant');
|
||||
expect(out.pois.map((p) => [p.name, p.category])).toEqual([
|
||||
['Imbiss', 'restaurant'],
|
||||
['Kaffeehaus', 'cafe'],
|
||||
]);
|
||||
});
|
||||
|
||||
it('MAPS-POIS-013d: a place only spelled like the pill is left out, nature brings parks and no car parks (#2663)', async () => {
|
||||
// What the index answered for Vienna: its terms match inside the category
|
||||
// text, so `park` and `garden` brought car parks, furniture stores and a
|
||||
// beer garden, and the parks were four in sixty.
|
||||
const near = (gers: string, name: string, categoryPath: string) => ({
|
||||
...FULL,
|
||||
gers,
|
||||
name,
|
||||
category: categoryPath.slice(categoryPath.lastIndexOf('>') + 1),
|
||||
categoryPath,
|
||||
});
|
||||
mockNearby.mockResolvedValue([
|
||||
near('n-1', 'Stadtpark', 'sports_and_recreation>park'),
|
||||
near('n-2', 'Garage am Ring', 'travel_and_transportation>parking>parking_garage'),
|
||||
near('n-3', 'Möbelhaus', 'shopping>specialty_store>hardware_home_and_garden_store>furniture_store'),
|
||||
near('n-4', 'Schweizerhaus', 'food_and_drink>alcoholic_beverage_venue>beer_garden'),
|
||||
near('n-5', 'Prater', 'arts_and_entertainment>amusement_attraction>amusement_park'),
|
||||
near('n-6', 'Botanischer Garten', 'geographic_entities>built_feature>garden>botanical_garden'),
|
||||
near('n-7', 'Hundezone', 'sports_and_recreation>park>dog_park'),
|
||||
]);
|
||||
const svc = make();
|
||||
const overpass = stubOverpass(svc);
|
||||
|
||||
const out = await svc.pois('nature', BOX);
|
||||
|
||||
expect(overpass).not.toHaveBeenCalled();
|
||||
expect(out.pois.map((p) => p.name)).toEqual(['Stadtpark', 'Botanischer Garten', 'Hundezone']);
|
||||
// Seven came back, so the page was not full; leaving four out does not make it so.
|
||||
expect(out.truncated).toBe(false);
|
||||
});
|
||||
|
||||
it('MAPS-POIS-013e: a page holding near misses only is a miss, and Overpass answers', async () => {
|
||||
mockNearby.mockResolvedValue([{ ...FULL, category: 'parking', categoryPath: 'travel_and_transportation>parking' }]);
|
||||
const svc = make();
|
||||
const overpass = stubOverpass(svc);
|
||||
|
||||
expect(await svc.pois('nature', BOX, 'de')).toEqual(OVERPASS_ANSWER);
|
||||
expect(overpass).toHaveBeenCalledWith('nature', BOX, 'de', 60);
|
||||
});
|
||||
|
||||
it('MAPS-POIS-014: a category the index has no terms for sends the whole query to Overpass', async () => {
|
||||
|
||||
@@ -8,6 +8,7 @@
|
||||
import {
|
||||
DEFAULT_TREK_PLACES_URL,
|
||||
POI_CATEGORY_TO_TREK,
|
||||
indexRowTerm,
|
||||
toPlaceRecord,
|
||||
trekPlacesArea,
|
||||
trekPlacesBaseUrl,
|
||||
@@ -745,4 +746,52 @@ describe('POI_CATEGORY_TO_TREK', () => {
|
||||
expect(terms.length, pill).toBeGreaterThan(0);
|
||||
}
|
||||
});
|
||||
|
||||
it('names every category by its full path, so no bare word matches inside another (#2663)', () => {
|
||||
// A bare `park` is inside `parking`, `bar` inside `barber`, `market` inside
|
||||
// `marketing_agency`. Only the top-level `shopping` stands alone, and it is
|
||||
// a whole branch on purpose.
|
||||
for (const [pill, terms] of Object.entries(POI_CATEGORY_TO_TREK)) {
|
||||
for (const term of terms) {
|
||||
if (term === 'shopping' || term === 'lodging') continue;
|
||||
expect(term, pill).toContain('>');
|
||||
}
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
describe('indexRowTerm (#2663)', () => {
|
||||
const nature = POI_CATEGORY_TO_TREK.nature;
|
||||
|
||||
it('takes a place whose path is the term or lies under it', () => {
|
||||
expect(indexRowTerm(nature, 'park', 'sports_and_recreation>park')).toBe('sports_and_recreation>park');
|
||||
expect(indexRowTerm(nature, 'national_park', 'sports_and_recreation>park>national_park')).toBe(
|
||||
'sports_and_recreation>park',
|
||||
);
|
||||
});
|
||||
|
||||
it('leaves out a place whose path only contains the word', () => {
|
||||
expect(indexRowTerm(nature, 'parking', 'travel_and_transportation>parking')).toBeUndefined();
|
||||
expect(indexRowTerm(nature, 'beer_garden', 'food_and_drink>alcoholic_beverage_venue>beer_garden')).toBeUndefined();
|
||||
expect(
|
||||
indexRowTerm(
|
||||
nature,
|
||||
'furniture_store',
|
||||
'shopping>specialty_store>hardware_home_and_garden_store>furniture_store',
|
||||
),
|
||||
).toBeUndefined();
|
||||
});
|
||||
|
||||
it('judges a place without a path by its leaf', () => {
|
||||
expect(indexRowTerm(nature, 'beach', null)).toBe('geographic_entities>land_feature>beach');
|
||||
expect(indexRowTerm(nature, 'beach_bar', null)).toBeUndefined();
|
||||
expect(indexRowTerm(nature, null, null)).toBeUndefined();
|
||||
});
|
||||
|
||||
it('picks the most specific term when several fit', () => {
|
||||
const terms = [...POI_CATEGORY_TO_TREK.nature, ...POI_CATEGORY_TO_TREK.activity];
|
||||
expect(indexRowTerm(terms, 'water_park', 'sports_and_recreation>park>water_park')).toBe(
|
||||
'sports_and_recreation>park>water_park',
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user